Re: [PATCH bpf-next v1 1/6] bpf: implement KF_IMPLICIT_PROG_AUX_ARG flag

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

On 9/25/25 2:49 AM, Alexei Starovoitov wrote:
> On Wed, Sep 24, 2025 at 10:17 PM Ihor Solodrai <ihor.solodrai@linux.dev> wrote:
>>
>> Define KF_IMPLICIT_PROG_AUX_ARG and handle it in the BPF verifier.
>>
>> The mechanism of patching is exactly the same as for __prog parameter
>> annotation: in check_kfunc_args() detect the relevant parameter and
>> remember regno in cur_aux(env)->arg_prog.
>>
>> Then the (unchanged in this patch) fixup_kfunc_call() adds a mov
>> instruction to set the actual pointer to prog_aux.
>>
>> The caveat for KF_IMPLICIT_PROG_AUX_ARG is in implicitness. We have to
>> separately check that the number of arguments is under
>> MAX_BPF_FUNC_REG_ARGS.
>>
>> Signed-off-by: Ihor Solodrai <ihor.solodrai@linux.dev>
>> ---
>>  include/linux/btf.h   |  3 +++
>>  kernel/bpf/verifier.c | 43 ++++++++++++++++++++++++++++++++++++-------
>>  2 files changed, 39 insertions(+), 7 deletions(-)
>>
>> diff --git a/include/linux/btf.h b/include/linux/btf.h
>> index f06976ffb63f..479ee96c2c97 100644
>> --- a/include/linux/btf.h
>> +++ b/include/linux/btf.h
>> @@ -79,6 +79,9 @@
>>  #define KF_ARENA_RET    (1 << 13) /* kfunc returns an arena pointer */
>>  #define KF_ARENA_ARG1   (1 << 14) /* kfunc takes an arena pointer as its first argument */
>>  #define KF_ARENA_ARG2   (1 << 15) /* kfunc takes an arena pointer as its second argument */
>> +/* kfunc takes a pointer to struct bpf_prog_aux as the last argument,
>> + * passed implicitly in BPF */
> 
> This is neither networking nor kernel comment style.
> Pls use proper kernel comment style in a new code,
> and reformat old net/bpf style when adjusting old comments.
> 
>> +#define KF_IMPLICIT_PROG_AUX_ARG (1 << 16)
> 
> The name is too verbose imo.
> How about
> KF_HIDDEN_PROG_ARG
> or
> KF_PROG_LAST_ARG
> 
> "Implicit" is not 100% correct, since it's very explicit
> in kfunc definition in C, but removed from BTF.
> "Hidden" is also not an exact fit for the same reasons.
> Hence my preference is KF_PROG_LAST_ARG.
> 
> "aux" part is also an implementation detail.
> 
>>  /*
>>   * Tag marking a kernel function as a kfunc. This is meant to minimize the
>> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
>> index e892df386eed..f1f9ea21f99b 100644
>> --- a/kernel/bpf/verifier.c
>> +++ b/kernel/bpf/verifier.c
>> @@ -11948,6 +11948,11 @@ static bool is_kfunc_rcu_protected(struct bpf_kfunc_call_arg_meta *meta)
>>         return meta->kfunc_flags & KF_RCU_PROTECTED;
>>  }
>>
>> +static bool is_kfunc_with_implicit_prog_aux_arg(struct bpf_kfunc_call_arg_meta *meta)
>> +{
>> +       return meta->kfunc_flags & KF_IMPLICIT_PROG_AUX_ARG;
>> +}
>> +
>>  static bool is_kfunc_arg_mem_size(const struct btf *btf,
>>                                   const struct btf_param *arg,
>>                                   const struct bpf_reg_state *reg)
>> @@ -12029,6 +12034,18 @@ static bool is_kfunc_arg_prog(const struct btf *btf, const struct btf_param *arg
>>         return btf_param_match_suffix(btf, arg, "__prog");
>>  }
>>
>> +static int set_kfunc_arg_prog_regno(struct bpf_verifier_env *env, struct bpf_kfunc_call_arg_meta *meta, u32 regno)
>> +{
>> +       if (meta->arg_prog) {
>> +               verifier_bug(env, "Only 1 prog->aux argument supported per-kfunc");
>> +               return -EFAULT;
>> +       }
>> +       meta->arg_prog = true;
>> +       cur_aux(env)->arg_prog = regno;
>> +
>> +       return 0;
>> +}
>> +
>>  static bool is_kfunc_arg_scalar_with_name(const struct btf *btf,
>>                                           const struct btf_param *arg,
>>                                           const char *name)
>> @@ -13050,6 +13067,21 @@ static int check_kfunc_args(struct bpf_verifier_env *env, struct bpf_kfunc_call_
>>                 return -EINVAL;
>>         }
>>
>> +       /* KF_IMPLICIT_PROG_AUX_ARG means that the kfunc has one less argument in BTF,
>> +        * so we have to set_kfunc_arg_prog_regno() outside the arg check loop.
>> +        */
> 
> Use kernel comment style.
> 
>> +       if (is_kfunc_with_implicit_prog_aux_arg(meta)) {
>> +               if (nargs + 1 > MAX_BPF_FUNC_REG_ARGS) {
>> +                       verifier_bug(env, "A kfunc with KF_IMPLICIT_PROG_AUX_ARG flag has %d > %d args",
>> +                                    nargs + 1, MAX_BPF_FUNC_REG_ARGS);
>> +                       return -EFAULT;
>> +               }
>> +               u32 regno = nargs + 1;
> 
> Variable declaration should be first in the block
> followed by a blank line.
> 
> Also I would remove this double "> MAX_BPF_FUNC_REG_ARGS" check.
> Move if (is_kfunc_with_prog_last_arg(meta))
> couple lines above before the check,
> and actual_nargs = nargs + 1;
> if (actual_nargs > MAX_BPF_FUNC_REG_ARGS)
> to cover both cases.
> I wouldn't worry that verbose() isn't too specific.
> If it prints nargs and actual_nargs whoever develops a kfunc
> can get an idea.
> Also in the future there is a good chance we will add more
> KF_FOO_LAST_ARG flags to cleanup other *_impl() kfuncs
> that have a special last argument, like bpf_rbtree_add_impl.
> If all of them copy paste "> MAX_BPF_FUNC_REG_ARGS" check
> it will be too verbose. Hence one nargs check for them all.

Hi Alexei, thank you for the review.

Sorry for the styling mistakes, forgot to run patches through
checkpatch.pl

In the other thread Eduard proposes a different approach to the
implementation [1].  Basically, leave BTF unmodified and move argument
hiding logic to bpftool's vmlinux.h generation.

IMO modifying BTF is more straightforward, but if the main goal is to
have a nice BPF C interface, maybe Eduard is onto something.

Curious to hear yours and Andrii's opinion on that.

Thanks.

[1] https://lore.kernel.org/bpf/b92d892f6a09fc7a411838ccf03dfebbba96384b.camel@gmail.com/

> 
> pw-bot: cr