Message-ID: <CAEf4BzYuchyyw9M6eQo0Gou=09PcM-o_Ay7D8DM1gDitiG6Tbg@mail.gmail.com>
Date: Mon, 12 Jan 2026 08:51:43 -0800
From: Andrii Nakryiko <andrii.nakryiko@...il.com>
To: Ihor Solodrai <ihor.solodrai@...ux.dev>
Cc: Alexei Starovoitov <ast@...nel.org>, Andrii Nakryiko <andrii@...nel.org>, 
	Daniel Borkmann <daniel@...earbox.net>, Martin KaFai Lau <martin.lau@...ux.dev>, 
	Eduard Zingerman <eddyz87@...il.com>, Mykyta Yatsenko <yatsenko@...a.com>, Tejun Heo <tj@...nel.org>, 
	Alan Maguire <alan.maguire@...cle.com>, Benjamin Tissoires <bentiss@...nel.org>, 
	Jiri Kosina <jikos@...nel.org>, bpf@...r.kernel.org, linux-kernel@...r.kernel.org, 
	linux-input@...r.kernel.org, sched-ext@...ts.linux.dev
Subject: Re: [PATCH bpf-next v1 04/10] resolve_btfids: Support for KF_IMPLICIT_ARGS

On Fri, Jan 9, 2026 at 5:15 PM Ihor Solodrai <ihor.solodrai@...ux.dev> wrote:
>
> On 1/9/26 3:25 PM, Andrii Nakryiko wrote:
> > On Fri, Jan 9, 2026 at 10:49 AM Ihor Solodrai <ihor.solodrai@...ux.dev> wrote:
> >>
> >> Implement BTF modifications in resolve_btfids to support BPF kernel
> >> functions with implicit arguments.
> >>
> >> For a kfunc marked with KF_IMPLICIT_ARGS flag, a new function
> >> prototype is added to BTF that does not have implicit arguments. The
> >> kfunc's prototype is then updated to a new one in BTF. This prototype
> >> is the intended interface for the BPF programs.
> >>
> >> A <func_name>_impl function is added to BTF to make the original kfunc
> >> prototype searchable for the BPF verifier. If a <func_name>_impl
> >> function already exists in BTF, its interpreted as a legacy case, and
> >> this step is skipped.
> >>
> >> Whether an argument is implicit is determined by its type:
> >> currently only `struct bpf_prog_aux *` is supported.
> >>
> >> As a result, the BTF associated with kfunc is changed from
> >>
> >>     __bpf_kfunc bpf_foo(int arg1, struct bpf_prog_aux *aux);
> >>
> >> into
> >>
> >>     bpf_foo_impl(int arg1, struct bpf_prog_aux *aux);
> >>     __bpf_kfunc bpf_foo(int arg1);
> >>
> >> For more context see previous discussions and patches [1][2].
> >>
> >> [1] https://lore.kernel.org/dwarves/ba1650aa-fafd-49a8-bea4-bdddee7c38c9@linux.dev/
> >> [2] https://lore.kernel.org/bpf/20251029190113.3323406-1-ihor.solodrai@linux.dev/
> >>
> >> Signed-off-by: Ihor Solodrai <ihor.solodrai@...ux.dev>
> >> ---
> >>  tools/bpf/resolve_btfids/main.c | 282 ++++++++++++++++++++++++++++++++
> >>  1 file changed, 282 insertions(+)
> >>
> >> diff --git a/tools/bpf/resolve_btfids/main.c b/tools/bpf/resolve_btfids/main.c
> >> index df39982f51df..b361e726fa36 100644
> >> --- a/tools/bpf/resolve_btfids/main.c
> >> +++ b/tools/bpf/resolve_btfids/main.c
> >> @@ -152,6 +152,18 @@ struct object {
> >>         int nr_typedefs;
> >>  };
> >>
> >> +#define KF_IMPLICIT_ARGS (1 << 16)
> >> +#define KF_IMPL_SUFFIX "_impl"
> >> +#define MAX_BPF_FUNC_REG_ARGS 5
> >> +#define MAX_KFUNCS 256
> >> +#define MAX_DECL_TAGS (MAX_KFUNCS * 4)
> >
> > can't we get that from include/linux/bpf.h? seems like
> > resolve_btfids's main.c include internal headers just fine, so why
> > duplicate definitions?
>
> Hi Andrii, thank you for a quick review.
>
> Including internal include/linux/btf.h directly doesn't work, which is
> probably expected.
>
> resolve_btfids is currently built with:
>
> HOSTCFLAGS_resolve_btfids += -g \
>           -I$(srctree)/tools/include \
>           -I$(srctree)/tools/include/uapi \

so I don't know if that will solve the issue, but I don't think it
makes sense to build resolve_btfids using tools' version of includes.
tools/include is mostly for perf's benefit (maybe so that they don't
accidentally take some kernel-internal dependency, not sure). But
resolve_btfids is built for the kernel during the kernel build, we
should have access to full kernel headers. Try changing this and see
if build errors go away?

>           -I$(LIBBPF_INCLUDE) \
>           -I$(SUBCMD_INCLUDE) \
>           $(LIBELF_FLAGS) \
>           -Wall -Werror
>
> If I add -I$(srctree)/include option and then
>
>     #include <linux/btf.h>
>
> A bunch of build errors happen.
>
> AFAIU we'd have to create a stripped copy of relevant headers in
> tools/include first.  Is that what you're suggesting?

see above, the opposite -- just use -I$(srctree)/include directly

[...]

> >> +               addr = id->addr[0];
> >> +               off = addr - obj->efile.idlist_addr;
> >> +               set8 = data->d_buf + off;
> >> +
> >> +               for (i = 0; i < set8->cnt; i++) {
> >> +                       if (set8->pairs[i].flags & flags) {
> >
> > invert condition and continue, reduce nesting?
> >
> >> +                               if (nr_kfuncs >= kfunc_ids_sz) {
> >
> > it's silly to set static limits like this: we are not in NMI, you have
> > memory allocator, use it
>
> I kinda like that btf2btf_context is stack allocated, but I see your
> point. It's not necessary to set hard limits in resolve_btfids.
>

I don't think we'll notice the performance difference. We had similar
statically-sized things for BTF processing in pahole, and very quickly
ran into limitations and had to change them to dynamically allocated.
Let's not do this again. realloc() is a bit more annoying to use, but
no big deal, it's C after all.

> >
> >> +                                       pr_err("ERROR: resolve_btfids: too many kfuncs with flags %u - limit %d\n",
> >> +                                              flags, kfunc_ids_sz);
> >> +                                       return -E2BIG;
> >> +                               }
> >> +                               kfunc_ids[nr_kfuncs++] = set8->pairs[i].id;

[...]

> >> +               err = btf__add_decl_tag(btf, tag_name, new_func_id, -1);
> >
> > decl_tag can apply to arguments as well (that -1 will be actually >=
> > 0), we should copy those as well, no?
>
> I think you're right. Technically decl_tags can point to parameters as
> well.  Is this actually used in kernel BTF?

I don't remember, but it doesn't matter. We have to clone them as well.

>
> For the type tags we don't have to do anything though, because the
> param type should point to the top type tag, right?

Yeah, type tags are part of type chains, if we reuse types, we'll be
reusing type_tags.

>
> >
> >> +               if (err < 0) {
> >> +                       pr_err("ERROR: resolve_btfids: failed to add decl tag %s for %s\n",
> >> +                              tag_name, tmp_name);
> >> +                       return -EINVAL;
> >> +               }
> >> +       }
> >> +

[...]

Powered by blists - more mailing lists