Re: [PATCH bpf-next v9 1/9] bpf: refactor kfunc checks using table-driven approach in verifier

[Ihor Solodrai]

On 4/3/26 10:41 AM, Chengkaitao wrote:
> On Tue, Mar 31, 2026 at 1:05 AM Alexei Starovoitov
> <alexei.starovoitov@xxxxxxxxx> wrote:
>>
>> On Sun, Mar 29, 2026 at 7:05 AM Chengkaitao <pilgrimtao@xxxxxxxxx> wrote:
>>>
>>> From: Kaitao Cheng <chengkaitao@xxxxxxxxxx>
>>>
>>> [...]
>>> +
>>> +/* Kfunc family related to spin_lock. */
>>> +static const enum special_kfunc_type bpf_res_spin_lock_api_kfuncs[] = {
>>> +       KF_bpf_res_spin_lock,
>>> +       KF_bpf_res_spin_unlock,
>>> +       KF_bpf_res_spin_lock_irqsave,
>>> +       KF_bpf_res_spin_unlock_irqrestore,
>>> +};
>>
>> I think it's a step in the wrong direction.
>> I'd wait for Ihor's BTF_ID_NAMED cleanup.
> 
> After reading Ihor's messages on the list, if I understand correctly,
> our two approaches seem to target different problems. What Ihor's
> work appears to achieve is the ability to remove the entire enum
> special_kfunc_type. My goal, on the other hand, is to replace many
> scattered func_id == special_kfunc_list[...] comparisons with a
> table-driven approach.

Hi Kaitao,

I appreciate your efforts, however after a quick pass over the changes
you propose (both here and in the new series) with respect to BTF_ID
macros and special_kfuncs_list, I don't understand what problem you're
trying to solve.

The inherent complexity is in the fact that the verifier must know
when a particular BTF id identifies a specific kfunc, or whether it
belongs to some pre-defined set of ids. This is why
special_kfuncs_list and other BTF_ID_SET/LIST-s exist.

And so there is no way around defining those ids and sets *somewhere*,
and so far BTF_ID_* macros did a fine job of that, all things
considered.

AFAICT your changes simply move around the same definitions from
functions with if statements to constant arrays with a runtime search
on them (which is slower by the way). What is the benefit of that vs
the current implementation? We still have to maintain those arrays in
the same way we have to maintain the is_foo_kfunc helpers.

Your newer proposal [1] takes the same idea to the next level, by
introducing an entire new BTF kind, new ELF sections and a bunch of
macros that are no less complicated than existing. And all of that
just moves the same arrays "upstream" to the .BTF_ids section. Again,
I fail to see any benefits to that complexity. Having differentiation
between LIST and SET, and having to mark START and END is not a
problem that needs solving IMO.

The work I was discussing with Alexei [2] is targeted: get rid of
constant enums that mirror existing kfunc symbols by making BTF_ID
internals a bit smarter. And the benefit is clear and simple.

If you could explain what exact issue you're trying to address with
your BTF_ID refactoring patches, maybe we can try converging on a
reasonable approach to it.

Thanks!

[1] https://lore.kernel.org/bpf/20260403170900.58659-1-pilgrimtao@xxxxxxxxx/
[2] https://lore.kernel.org/bpf/21e5333c-0b57-46ce-99c8-f6c414270e70@xxxxxxxxx/


> 
> That said, once Ihor's solution lands, my approach would no longer
> be applicable as-is. So I have been thinking about an alternative,
> please see the patch linked below for details:
> https://lore.kernel.org/bpf/20260403170900.58659-1-pilgrimtao@xxxxxxxxx/
> 
>> Kaitao Cheng,
>>
>> also please start your part of code reviews.
>> Your patches are not going to be landing if you don't code review.
>>
>> https://lore.kernel.org/bpf/CAADnVQ+TKKptnNB25V3=bcdybh5G6c2DyW2sYtXvyRaVnPN8MA@xxxxxxxxxxxxxx/
> 
> I will do my best to carry this forward.
>