On Mon, 12 May 2025 at 20:04, Alexei Starovoitov
<alexei.starovoitov@xxxxxxxxx> wrote:
>
> On Mon, May 12, 2025 at 2:02 PM Kumar Kartikeya Dwivedi
> <memxor@xxxxxxxxx> wrote:
> >
> > Instead of hardcoding the list of kfuncs that need prog->aux passed to
> > them with a combination of fixup_kfunc_call adjustment + __ign suffix,
> > combine both in __aux suffix, which ignores the argument passed in, and
> > fixes it up to the prog->aux. This allows kfuncs to have the prog->aux
> > passed into them without having to touch the verifier.
> >
> > Cc: Tejun Heo <tj@xxxxxxxxxx>
> > Signed-off-by: Kumar Kartikeya Dwivedi <memxor@xxxxxxxxx>
> > ---
> > include/linux/bpf_verifier.h | 1 +
> > kernel/bpf/helpers.c | 4 ++--
> > kernel/bpf/verifier.c | 33 +++++++++++++++++++++++++++------
> > 3 files changed, 30 insertions(+), 8 deletions(-)
> >
> > diff --git a/include/linux/bpf_verifier.h b/include/linux/bpf_verifier.h
> > index 9734544b6957..1d90e44a1d04 100644
> > --- a/include/linux/bpf_verifier.h
> > +++ b/include/linux/bpf_verifier.h
> > @@ -606,6 +606,7 @@ struct bpf_insn_aux_data {
> > bool calls_callback;
> > /* registers alive before this instruction. */
> > u16 live_regs_before;
> > + u16 arg_prog_aux;
> > };
> >
> > #define MAX_USED_MAPS 64 /* max number of maps accessed by one eBPF program */
> > diff --git a/kernel/bpf/helpers.c b/kernel/bpf/helpers.c
> > index fed53da75025..2b6bac4bf6e3 100644
> > --- a/kernel/bpf/helpers.c
> > +++ b/kernel/bpf/helpers.c
> > @@ -3012,9 +3012,9 @@ __bpf_kfunc int bpf_wq_start(struct bpf_wq *wq, unsigned int flags)
> > __bpf_kfunc int bpf_wq_set_callback_impl(struct bpf_wq *wq,
> > int (callback_fn)(void *map, int *key, void *value),
> > unsigned int flags,
> > - void *aux__ign)
> > + void *aux__aux)
>
> aux__aux is an odd name.
> "__aux" as a suffix also looks strange.
>
We can call it prog__aux.
> How about "__prog" suffix ?
> It will be similar to the existing "__map" suffix.
But it's a bit misleading, it's not the prog, it's prog->aux.
>
> We can also standardize the argument name as
> __bpf_kfunc int bpf_wq_set_callback_impl(.. , void *aux__prog)
>
> then the name is more or less explanatory.
You can call it foo__prog, the part before __ is arbitrary.
>
> > {
> > - struct bpf_prog_aux *aux = (struct bpf_prog_aux *)aux__ign;
> > + struct bpf_prog_aux *aux = (struct bpf_prog_aux *)aux__aux;
>
> and here it will be:
>
> + struct bpf_prog_aux *aux = (struct bpf_prog_aux *)aux__prog;
>
> which looks ok to me.
How about I rename the parameter to prog__aux?
Or __prog_aux as the tag name?
>
> pw-bot: cr
