Subject: Re: [PATCH v10 1/5] add infrastructure for tagging functions as error
 injectable
To: Masami Hiramatsu <mhiramat@kernel.org>,
        Josef Bacik <josef@toxicpanda.com>
References: <1513365176-6744-1-git-send-email-josef@toxicpanda.com>
 <1513365176-6744-2-git-send-email-josef@toxicpanda.com>
 <20171219152925.5789309c6c4d27807d42f11c@kernel.org>
CC: <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>
From: Alexei Starovoitov <ast@fb.com>
Message-ID: <7b73cbfa-959f-2efe-8a7c-f9d0b0c2ccaa@fb.com>
Date: Tue, 19 Dec 2017 18:14:17 -0800
User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.12; rv:45.0)
 Gecko/20100101 Thunderbird/45.8.0
MIME-Version: 1.0
In-Reply-To: <20171219152925.5789309c6c4d27807d42f11c@kernel.org>
Content-Type: text/plain; charset="windows-1252"; format=flowed
Content-Transfer-Encoding: 7bit
SpamDiagnosticOutput: 1:99
SpamDiagnosticMetadata: NSPM
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 20 Dec 2017 02:14:20.5729 (UTC)
X-MS-Exchange-CrossTenant-Network-Message-Id: 534bc27f-f6e0-4296-bbb1-08d5474f629f
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 8ae927fe-1255-47a7-a2af-5f3a069daaa2
X-MS-Exchange-Transport-CrossTenantHeadersStamped: DM3PR15MB0969
X-OriginatorOrg: fb.com
X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10432:,, definitions=2017-12-19_12:,,
 signatures=0
X-Proofpoint-Spam-Reason: safe
X-FB-Internal: Safe
Sender: linux-kernel-owner@vger.kernel.org
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org
Content-Length: 1056
Lines: 27

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.
The kernel enters and exists bpf program as normal.

> 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?

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

Thanks