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

[Chengkaitao]

On Sat, Apr 4, 2026 at 12:49 PM Ihor Solodrai <ihor.solodrai@xxxxxxxxx> wrote:
>
>
>
> 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.

Your analysis of the code implementation for the new proposal is correct.
Let me elaborate on the purpose behind my approach.

****** Purpose 1 ******

As described in this patch:
https://lore.kernel.org/bpf/20260303135219.33726-4-pilgrimtao@xxxxxxxxx/

If we want to add a new kfunc, bpf_list_add_impl, we would today have
to add "btf_id == special_kfunc_list[KF_bpf_list_back]" (or similar)
five times in verifier.c. Under the newer proposal, that is no longer
necessary: defining the kfunc and its verifier metadata in one place
is enough, for example:

__bpf_kfunc int bpf_list_add_impl(struct bpf_list_head *head,
 struct bpf_list_node *new,
 struct bpf_list_node *prev,
 void *meta__ign, u64 off)
{
/* kfunc implementation */
.......
}
BPF_VERIF_KFUNC_DEF(bpf_list_add_impl, list_api, graph_node_api, ... )

If BPF_VERIF_KFUNC_DEF is extended further, BTF_ID(func, bpf_list_add_impl)
and BTF_ID_FLAGS(func, bpf_list_add_impl) might also become unnecessary,
so the snippet above could eventually be close to all the code required
to add a new kfunc.

****** Purpose 2 ******

The kernel no longer needs enum special_kfunc_type to list every KF_bpf_*
entry. That information is folded into the .BTF_ids.##sfx section instead,
so kfunc authors do not have to touch or think about special_kfunc_type.

****** Purpose 3 ******

As described in this patch:
https://lore.kernel.org/bpf/20260303135219.33726-6-pilgrimtao@xxxxxxxxx/

In is_bpf_list_api_kfunc(u32 btf_id) there are on the order of eleven
"btf_id == special_kfunc_list[*]" comparisons. As more kfuncs are added,
every is_bpf_* helper will only grow longer and the verifier will get
more repetitive. With the new design, those is_bpf_* helpers can be
removed entirely, including the awkward scattered "btf_id == *" checks.

****** Purpose 4 ******

It pushes us to untangle messy verifier safety cases and make them modular,
so they can be expressed as parameters to BPF_VERIF_KFUNC_DEF

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

-- 
Yours,
Chengkaitao