DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org C706D20671
Date: Wed, 20 Dec 2017 16:13:42 +0900
From: Masami Hiramatsu <mhiramat@kernel.org>
To: Alexei Starovoitov <ast@fb.com>
Cc: Josef Bacik <josef@toxicpanda.com>, <rostedt@goodmis.org>,
        <mingo@redhat.com>, <davem@davemloft.net>, <netdev@vger.kernel.org>,
        <linux-kernel@vger.kernel.org>, <ast@kernel.org>, <kernel-team@fb.com>,
        <daniel@iogearbox.net>, <linux-btrfs@vger.kernel.org>,
        <darrick.wong@oracle.com>, Josef Bacik <jbacik@fb.com>
Subject: Re: [PATCH v10 1/5] add infrastructure for tagging functions as
 error injectable
Message-Id: <20171220161342.44443047f186b7a62efdfe1a@kernel.org>
In-Reply-To: <7b73cbfa-959f-2efe-8a7c-f9d0b0c2ccaa@fb.com>
References: <1513365176-6744-1-git-send-email-josef@toxicpanda.com>
        <1513365176-6744-2-git-send-email-josef@toxicpanda.com>
        <20171219152925.5789309c6c4d27807d42f11c@kernel.org>
        <7b73cbfa-959f-2efe-8a7c-f9d0b0c2ccaa@fb.com>
Mime-Version: 1.0
Content-Type: text/plain; charset=US-ASCII
Content-Transfer-Encoding: 7bit
Sender: linux-kernel-owner@vger.kernel.org
Content-Length: 1763
Lines: 54

On Tue, 19 Dec 2017 18:14:17 -0800
Alexei Starovoitov <ast@fb.com> wrote:

> On 12/18/17 10:29 PM, Masami Hiramatsu wrote:
> >>
> >> +#if defined(__KERNEL__) && !defined(__ASSEMBLY__)
> >> +#ifdef CONFIG_BPF_KPROBE_OVERRIDE
> >
> > BTW, CONFIG_BPF_KPROBE_OVERRIDE is also confusable name.
> > Since this feature override a function to just return with
> > some return value (as far as I understand, or would you
> > also plan to modify execution path inside a function?),
> > I think it should be better CONFIG_BPF_FUNCTION_OVERRIDE or
> > CONFIG_BPF_EXECUTION_OVERRIDE.
> 
> I don't think such renaming makes sense.
> The feature is overriding kprobe by changing how kprobe returns.
> It doesn't override BPF_FUNCTION or BPF_EXECUTION.

No, I meant this is BPF's feature which override FUNCTION, so
BPF is a kind of namespace. (Is that only for a function entry
because it can not tweak stackframe at this morment?)

> The kernel enters and exists bpf program as normal.

Yeah, but that bpf program modifies instruction pointer, am I correct?

> 
> > Indeed, BPF is based on kprobes, but it seems you are limiting it
> > with ftrace (function-call trace) (I'm not sure the reason why),
> > so using "kprobes" for this feature seems strange for me.
> 
> do you have an idea how kprobe override can happen when kprobe
> placed in the middle of the function?

For example, if you know a basic block in the function, maybe
you can skip a block or something like that. But nowadays
it is somewhat hard because optimizer mixed it up.

> 
> Please make your suggestion as patches based on top of bpf-next.

bpf-next seems already pick this series. Would you mean I revert it and
write new patch?

Thank you,

> 
> Thanks
> 


-- 
Masami Hiramatsu <mhiramat@kernel.org>