Re: [PATCH bpf-next 0/3] bpf: Refactor how the verifier matches kfunc checks

[Chengkaitao]

On Wed, Apr 8, 2026 at 2:41 AM Ihor Solodrai <ihor.solodrai@xxxxxxxxx> wrote:
>
> On 4/4/26 3:38 AM, Chengkaitao wrote:
> > 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. ...
>
> Kaitao,
>
> I think the maintainability pain in the current verifier-side kfunc
> handling is clear, I don't think anyone would argue that it's not a
> problem. Your bpf_list_* work is a good demonstration of that. I agree
> it can be improved.

Hi Ihor,

There is already discussion of this patch series at the link below.
Please keep follow-up replies on this thread so the conversation
stays easy to track.
https://lore.kernel.org/all/20260329140506.9595-2-pilgrimtao@xxxxxxxxx/

Adding more logic in verifier.c is not necessarily a good thing—it
is already over 26000 lines. The .BTF_ids approach does not reduce
the verifier's logical complexity, but it can cut down redundant
verifier code and hide the intricate logic from kfunc developers.

To address what you raised (for example, lowering verifier complexity
and making the verifier more modular), I need to think about whether
there is a better approach that balances these goals.

> That said, I don't think extending .BTF_ids and/or BTF is the right
> way to solve it.
>
> > ... 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.
>
> Defining a kfunc metadata in one place is a reasonable direction in
> principle, I agree.
>
> Where I disagree is the mechanism. The complexity does not disappear,
> it just moves into .BTF_ids, new macros and resolve_btfids. The
> primary consumer of this metadata is the verifier, and I think it is
> better to keep that metadata in verifier-local C definitions instead
> of encoding it into .BTF_ids. Adding more indirection to this will
> only hurt maintainability IMO.
>
> >
> > ****** 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.
>
> This is not an argument for the .BTF_ids extension. Removing
> `special_kfunc_type` mirroring is useful, but that will be addressed
> directly by the targeted cleanup already under discussion.
>
> >
> > ****** 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.
>
> The repetition is real. But removing is_bpf_*() helpers is not
> automatically a simplification if the same information is now encoded
> indirectly through section layout or local const arrays.
>
> The same metadata is being maintained somewhere else, with more
> indirection than before. The complexity just moved around, and was not
> reduced.
>
> The scattered "btf_id == *" will remain as long as it is necessary for
> the verifier to do certain checks or other work depending on the
> kfunc. Whether it's an array lookup, helper call or anything else in
> the condition, the condition must be there. Refactoring can't reduce
> this complexity, without changes in the verifier control flow.
>
> >
> > ****** 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
>
> Again, I agree with the premise that verifier safety checks could
> become more modular where possible. But I think we should first
> separate two questions:
>
> 1. What kfunc properties should be declared centrally?
> 2. Where that declaration should live?
>
> While I'd like to answer (1) with "all of them", I am not convinced
> the answer to (2) is .BTF_ids or BTF. A better C side declarative
> representation would give us most of the benefit here without making
> the BTF tooling more complex.
>
> Here is how I think we should move forward:
>
>   1. Your bpf_list_* work is orthogonal to BTF_ID refactoring, so it's
>   reasonable to first focus on landing it without changes to generic
>   kfunc handling.
>
>   2. I plan to send patches (soon) for resolve_btfids, and then for
>   BTF_ID macrology to eliminate the enum + array pattern. You are
>   welcome to join the discussion and review / test the patches.
>
>   3. After all of the above lands, we can come back to the general
>   BTF_ID / kfunc handling discussion. If you are interested in
>   developing this further, I suggest to re-think the approach and come
>   up with a "single kfunc metadata definition" that doesn't require
>   significant changes in .BTF_ids section layout.
>
> A slightly off-topic comment: the usage of `_impl` pattern for kfuncs
> should be considered deprecated. Any new kfuncs that work with
> verifier-supplied arguments should use KF_IMPLICIT_ARGS mechanism.  So
> the `bpf_list_add_impl` in your seires should only have one version:
> `bpf_list_add` marked with KF_IMPLICIT_ARGS flag.
>

-- 
Yours,
Chengkaitao