Re: [PATCH bpf-next v1 1/2] bpf: Support struct btf_struct_meta via KF_IMPLICIT_ARGS

[Ihor Solodrai <ihor.solodrai@linux.dev>]

On 3/12/26 2:24 PM, Andrii Nakryiko wrote:
> On Thu, Mar 12, 2026 at 12:36 PM Ihor Solodrai <ihor.solodrai@linux.dev> wrote:
>>
>> The following kfuncs currently accept void *meta__ign argument:
>>   * bpf_obj_new_impl
>>   * bpf_obj_drop_impl
>>   * bpf_percpu_obj_new_impl
>>   * bpf_percpu_obj_drop_impl
>>   * bpf_refcount_acquire_impl
>>   * bpf_list_push_front_impl
>>   * bpf_rbtree_add_impl
>>
>> [...]
>>
>>                 /* rbtree_add has extra 'less' arg, so args-to-fixup are in diff regs */
>> -               if (desc->func_id == special_kfunc_list[KF_bpf_rbtree_add_impl]) {
>> +               if (is_bpf_rbtree_add_kfunc(desc->func_id)) {
>>                         struct_meta_reg = BPF_REG_4;
>>                         node_offset_reg = BPF_REG_5;
>>                 }
> 
> the amount of very specific "is this a special function X"
> special-casing is depressing... did you try to analyze if we can
> generalize this knowing which implicit argument we are dealing with?
> (not keeping my hopes high, but I had to ask)

I am open to suggestions, but fundamentally special_kfunc_list exists
because in special places in the verifier the function needs to be
handled in a special way ¯\_(ツ)_/¯

is_bpf_foo_kfunc() is needed with this change only because we have
to be backward compatible. It's either with helpers or the same thing
inlined, and helpers IMO is easier to read.

> 
>> diff --git a/tools/bpf/resolve_btfids/main.c b/tools/bpf/resolve_btfids/main.c
>> index 5208f650080f..f8a91fa7584f 100644
>> --- a/tools/bpf/resolve_btfids/main.c
>> +++ b/tools/bpf/resolve_btfids/main.c
>> @@ -1065,6 +1065,7 @@ static bool is_kf_implicit_arg(const struct btf *btf, const struct btf_param *p)
>>  {
>>         static const char *const kf_implicit_arg_types[] = {
>>                 "bpf_prog_aux",
>> +               "btf_struct_meta",
> 
> should we add typedef u64 bpf_implicit_offset_t or something like that
> to actually have unique and meaningful types for any kind of implicit
> argument?

I considered this, but didn't try.
Didn't see much benefit at this point.

We have to choose a convention, for example:

  "Implicit arguments can *only* be of particular types."
  (that's the current state of things, without the patch)

This implies that, at least in principle, the position of the implicit
arg can be arbitrary. But that's not true right now due to how
resolve_btfids is implemented (note the "break" in the loop) [1].

And because it's not true, I can get away with leaving u64 off as is.

If we also add to the convention that "in a kfunc, all implicit
arguments always follow all required arguments", then why would we
require all implicit arguments to be of specific types? Do we want to
enforce the BTF type in the verifier even if it's u64?

We are going to be in trouble if someone decides to add implicit
primitive arguments *only* (without a preceding implicit pointer).
Using an __impl suffix gives me an ick, so if we anticipate
such use-cases, that'd be a strong argument for special typedefs.

I think the is_kfunc_arg_implicit() in this patch is a good way to
determine implicitness in the verifier, so the above applies mostly to
how resolve_btfids determines which args are implicit.

[1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/tools/bpf/resolve_btfids/main.c?h=v7.0-rc3#n1187

> 
>>         };
>>         const struct btf_type *t;
>>         const char *name;
>> --
>> 2.53.0
>>