[v2,bpf-next,8/9] libbpf: implement __arg_ctx fallback logic

Commit Message

Andrii Nakryiko Jan. 2, 2024, 7 p.m. UTC

Out of all special global func arg tag annotations, __arg_ctx is
practically is the most immediately useful and most critical to have
working across multitude kernel version, if possible. This would allow
end users to write much simpler code if __arg_ctx semantics worked for
older kernels that don't natively understand btf_decl_tag("arg:ctx") in
verifier logic.

Luckily, it is possible to ensure __arg_ctx works on old kernels through
a bit of extra work done by libbpf, at least in a lot of common cases.

To explain the overall idea, we need to go back at how context argument
was supported in global funcs before __arg_ctx support was added. This
was done based on special struct name checks in kernel. E.g., for
BPF_PROG_TYPE_PERF_EVENT the expectation is that argument type `struct
bpf_perf_event_data *` mark that argument as PTR_TO_CTX. This is all
good as long as global function is used from the same BPF program types
only, which is often not the case. If the same subprog has to be called
from, say, kprobe and perf_event program types, there is no single
definition that would satisfy BPF verifier. Subprog will have context
argument either for kprobe (if using bpf_user_pt_regs_t struct name) or
perf_event (with bpf_perf_event_data struct name), but not both.

This limitation was the reason to add btf_decl_tag("arg:ctx"), making
the actual argument type not important, so that user can just define
"generic" signature:

  __noinline int global_subprog(void *ctx __arg_ctx) { ... }

I won't belabor how libbpf is implementing subprograms, see a huge
comment next to bpf_object_relocate_calls() function. The idea is that
each main/entry BPF program gets its own copy of global_subprog's code
appended.

This per-program copy of global subprog code *and* associated func_info
.BTF.ext information, pointing to FUNC -> FUNC_PROTO BTF type chain
allows libbpf to simulate __arg_ctx behavior transparently, even if the
kernel doesn't yet support __arg_ctx annotation natively.

The idea is straightforward: each time we append global subprog's code
and func_info information, we adjust its FUNC -> FUNC_PROTO type
information, if necessary (that is, libbpf can detect the presence of
btf_decl_tag("arg:ctx") just like BPF verifier would do it).

The rest is just mechanical and somewhat painful BTF manipulation code.
It's painful because we need to clone FUNC -> FUNC_PROTO, instead of
reusing it, as same FUNC -> FUNC_PROTO chain might be used by another
main BPF program within the same BPF object, so we can't just modify it
in-place (and cloning BTF types within the same struct btf object is
painful due to constant memory invalidation, see comments in code).
Uploaded BPF object's BTF information has to work for all BPF
programs at the same time.

Once we have FUNC -> FUNC_PROTO clones, we make sure that instead of
using some `void *ctx` parameter definition, we have an expected `struct
bpf_perf_event_data *ctx` definition (as far as BPF verifier and kernel
is concerned), which will mark it as context for BPF verifier. Same
global subprog relocated and copied into another main BPF program will
get different type information according to main program's type. It all
works out in the end in a completely transparent way for end user.

Libbpf maintains internal program type -> expected context struct name
mapping internally. Note, not all BPF program types have named context
struct, so this approach won't work for such programs (just like it
didn't before __arg_ctx). So native __arg_ctx is still important to have
in kernel to have generic context support across all BPF program types.

Acked-by: Jiri Olsa <[e-mail]>
Signed-off-by: Andrii Nakryiko <[e-mail]>
---
 tools/lib/bpf/libbpf.c | 257 ++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 251 insertions(+), 6 deletions(-)

Comments

Eduard Zingerman Jan. 3, 2024, 8:57 p.m. UTC | #1

On Tue, 2024-01-02 at 11:00 -0800, Andrii Nakryiko wrote:

[...]

> +static int clone_func_btf_info(struct btf *btf, int orig_fn_id, struct bpf_program *prog)
> +{

[...]

> +	/* clone FUNC first, btf__add_func() enforces
> +	 * non-empty name, so use entry program's name as
> +	 * a placeholder, which we replace immediately
> +	 */
> +	fn_id = btf__add_func(btf, prog->name, btf_func_linkage(fn_t), fn_t->type);

Nit: Why not call this function near the end, when fn_proto_id is available?
     Thus avoiding need to "guess" fn_t->type.

[...]

> +static int bpf_program_fixup_func_info(struct bpf_object *obj, struct bpf_program *prog)
> +{

[...]

> +	for (i = 1, n = btf__type_cnt(btf); i < n; i++) {

[...]

> +
> +		/* clone fn/fn_proto, unless we already did it for another arg */
> +		if (func_rec->type_id == orig_fn_id) {
> +			int fn_id;
> +
> +			fn_id = clone_func_btf_info(btf, orig_fn_id, prog);
> +			if (fn_id < 0) {
> +				err = fn_id;
> +				goto err_out;
> +			}
> +
> +			/* point func_info record to a cloned FUNC type */
> +			func_rec->type_id = fn_id;

Would it be helpful to add a log here, saying that BTF for function
so and so is changed before load?

> +		}
> +
> +		/* create PTR -> STRUCT type chain to mark PTR_TO_CTX argument;
> +		 * we do it just once per main BPF program, as all global
> +		 * funcs share the same program type, so need only PTR ->
> +		 * STRUCT type chain
> +		 */
> +		if (ptr_id == 0) {
> +			struct_id = btf__add_struct(btf, ctx_name, 0);

Nit: Maybe try looking up existing id for type ctx_name first?

> +			ptr_id = btf__add_ptr(btf, struct_id);
> +			if (ptr_id < 0 || struct_id < 0) {
> +				err = -EINVAL;
> +				goto err_out;
> +			}
> +		}
> +

[...]

Andrii Nakryiko Jan. 3, 2024, 11:10 p.m. UTC | #2

On Wed, Jan 3, 2024 at 12:57 PM Eduard Zingerman <[e-mail]> wrote:
>
> On Tue, 2024-01-02 at 11:00 -0800, Andrii Nakryiko wrote:
>
> [...]
>
> > +static int clone_func_btf_info(struct btf *btf, int orig_fn_id, struct bpf_program *prog)
> > +{
>
> [...]
>
> > +     /* clone FUNC first, btf__add_func() enforces
> > +      * non-empty name, so use entry program's name as
> > +      * a placeholder, which we replace immediately
> > +      */
> > +     fn_id = btf__add_func(btf, prog->name, btf_func_linkage(fn_t), fn_t->type);
>
> Nit: Why not call this function near the end, when fn_proto_id is available?
>      Thus avoiding need to "guess" fn_t->type.
>

I think I did it to not have to remember btf_func_linkage(fn_t)
(because fn_t will be invalidated) and because name_off will be reused
for parameters. Neither is a big deal, I'll adjust to your suggestion.

But note, we are not guessing ID, it's guaranteed to be +1, it's an
API contract of btf__add_xxx() APIs.

> [...]
>
> > +static int bpf_program_fixup_func_info(struct bpf_object *obj, struct bpf_program *prog)
> > +{
>
> [...]
>
> > +     for (i = 1, n = btf__type_cnt(btf); i < n; i++) {
>
> [...]
>
> > +
> > +             /* clone fn/fn_proto, unless we already did it for another arg */
> > +             if (func_rec->type_id == orig_fn_id) {
> > +                     int fn_id;
> > +
> > +                     fn_id = clone_func_btf_info(btf, orig_fn_id, prog);
> > +                     if (fn_id < 0) {
> > +                             err = fn_id;
> > +                             goto err_out;
> > +                     }
> > +
> > +                     /* point func_info record to a cloned FUNC type */
> > +                     func_rec->type_id = fn_id;
>
> Would it be helpful to add a log here, saying that BTF for function
> so and so is changed before load?

Would it? We don't have global subprog's name readily available, it
seems. So I'd need to refetch it by fn_id, then btf__str_by_offset()
just to emit cryptic (for most users) notifications that something
about some func info was adjusted. And then the user would get this
same message for the same subprog but in the context of a different
entry program. Just confusing, tbh.

Unless you insist, I'd leave it as is. This logic is supposed to be
bullet-proof, so I'm not worried about debugging regressions with it
(but maybe I'm delusional).

>
> > +             }
> > +
> > +             /* create PTR -> STRUCT type chain to mark PTR_TO_CTX argument;
> > +              * we do it just once per main BPF program, as all global
> > +              * funcs share the same program type, so need only PTR ->
> > +              * STRUCT type chain
> > +              */
> > +             if (ptr_id == 0) {
> > +                     struct_id = btf__add_struct(btf, ctx_name, 0);
>
> Nit: Maybe try looking up existing id for type ctx_name first?

It didn't feel important and I didn't want to do another linear BTF
search for each such argument. It's trivial to look it up, but I still
feel like that's a waste... I tried to avoid many linear searches,
which is why I structured the logic to do one pass over BTF to find
all decl_tags instead of going over each function and arg and
searching for decl_tag.

Let's keep it as is, if there are any reasons to try to reuse struct
(if it is at all present, which for kprobe, for example, is quite
unlikely due to fancy bpf_user_pt_regs_t name), then we can easily add
it with no regressions.

>
> > +                     ptr_id = btf__add_ptr(btf, struct_id);
> > +                     if (ptr_id < 0 || struct_id < 0) {
> > +                             err = -EINVAL;
> > +                             goto err_out;
> > +                     }
> > +             }
> > +
>
> [...]
>
>

Eduard Zingerman Jan. 3, 2024, 11:43 p.m. UTC | #3

On Wed, 2024-01-03 at 15:10 -0800, Andrii Nakryiko wrote:
> On Wed, Jan 3, 2024 at 12:57 PM Eduard Zingerman <[e-mail]> wrote:
> > 
> > On Tue, 2024-01-02 at 11:00 -0800, Andrii Nakryiko wrote:
> > 
> > [...]
> > 
> > > +static int clone_func_btf_info(struct btf *btf, int orig_fn_id, struct bpf_program *prog)
> > > +{
> > 
> > [...]
> > 
> > > +     /* clone FUNC first, btf__add_func() enforces
> > > +      * non-empty name, so use entry program's name as
> > > +      * a placeholder, which we replace immediately
> > > +      */
> > > +     fn_id = btf__add_func(btf, prog->name, btf_func_linkage(fn_t), fn_t->type);
> > 
> > Nit: Why not call this function near the end, when fn_proto_id is available?
> >      Thus avoiding need to "guess" fn_t->type.
> > 
> 
> I think I did it to not have to remember btf_func_linkage(fn_t)
> (because fn_t will be invalidated) and because name_off will be reused
> for parameters. Neither is a big deal, I'll adjust to your suggestion.
> 
> But note, we are not guessing ID, it's guaranteed to be +1, it's an
> API contract of btf__add_xxx() APIs.

Noted, well, maybe just skip this nit in such a case.

> > [...]
> > 
> > > +static int bpf_program_fixup_func_info(struct bpf_object *obj, struct bpf_program *prog)
> > > +{
> > 
> > [...]
> > 
> > > +     for (i = 1, n = btf__type_cnt(btf); i < n; i++) {
> > 
> > [...]
> > 
> > > +
> > > +             /* clone fn/fn_proto, unless we already did it for another arg */
> > > +             if (func_rec->type_id == orig_fn_id) {
> > > +                     int fn_id;
> > > +
> > > +                     fn_id = clone_func_btf_info(btf, orig_fn_id, prog);
> > > +                     if (fn_id < 0) {
> > > +                             err = fn_id;
> > > +                             goto err_out;
> > > +                     }
> > > +
> > > +                     /* point func_info record to a cloned FUNC type */
> > > +                     func_rec->type_id = fn_id;
> > 
> > Would it be helpful to add a log here, saying that BTF for function
> > so and so is changed before load?
> 
> Would it? We don't have global subprog's name readily available, it
> seems. So I'd need to refetch it by fn_id, then btf__str_by_offset()
> just to emit cryptic (for most users) notifications that something
> about some func info was adjusted. And then the user would get this
> same message for the same subprog but in the context of a different
> entry program. Just confusing, tbh.
> 
> Unless you insist, I'd leave it as is. This logic is supposed to be
> bullet-proof, so I'm not worried about debugging regressions with it
> (but maybe I'm delusional).

I was thinking about someone finding out that actual in-kernel BTF
is different from that in the program object file, while debugging
something. Might be a bit surprising. I'm not insisting on this, though.

> > > +             }
> > > +
> > > +             /* create PTR -> STRUCT type chain to mark PTR_TO_CTX argument;
> > > +              * we do it just once per main BPF program, as all global
> > > +              * funcs share the same program type, so need only PTR ->
> > > +              * STRUCT type chain
> > > +              */
> > > +             if (ptr_id == 0) {
> > > +                     struct_id = btf__add_struct(btf, ctx_name, 0);
> > 
> > Nit: Maybe try looking up existing id for type ctx_name first?
> 
> It didn't feel important and I didn't want to do another linear BTF
> search for each such argument. It's trivial to look it up, but I still
> feel like that's a waste... I tried to avoid many linear searches,
> which is why I structured the logic to do one pass over BTF to find
> all decl_tags instead of going over each function and arg and
> searching for decl_tag.
>
> Let's keep it as is, if there are any reasons to try to reuse struct
> (if it is at all present, which for kprobe, for example, is quite
> unlikely due to fancy bpf_user_pt_regs_t name), then we can easily add
> it with no regressions.

I was thinking about possible interaction with btf_struct_access(),
but that is not used to verify context access at the moment.
So, probably not important.

Andrii Nakryiko Jan. 3, 2024, 11:59 p.m. UTC | #4

On Wed, Jan 3, 2024 at 3:43 PM Eduard Zingerman <[e-mail]> wrote:
>
> On Wed, 2024-01-03 at 15:10 -0800, Andrii Nakryiko wrote:
> > On Wed, Jan 3, 2024 at 12:57 PM Eduard Zingerman <[e-mail]> wrote:
> > >
> > > On Tue, 2024-01-02 at 11:00 -0800, Andrii Nakryiko wrote:
> > >
> > > [...]
> > >
> > > > +static int clone_func_btf_info(struct btf *btf, int orig_fn_id, struct bpf_program *prog)
> > > > +{
> > >
> > > [...]
> > >
> > > > +     /* clone FUNC first, btf__add_func() enforces
> > > > +      * non-empty name, so use entry program's name as
> > > > +      * a placeholder, which we replace immediately
> > > > +      */
> > > > +     fn_id = btf__add_func(btf, prog->name, btf_func_linkage(fn_t), fn_t->type);
> > >
> > > Nit: Why not call this function near the end, when fn_proto_id is available?
> > >      Thus avoiding need to "guess" fn_t->type.
> > >
> >
> > I think I did it to not have to remember btf_func_linkage(fn_t)
> > (because fn_t will be invalidated) and because name_off will be reused
> > for parameters. Neither is a big deal, I'll adjust to your suggestion.
> >
> > But note, we are not guessing ID, it's guaranteed to be +1, it's an
> > API contract of btf__add_xxx() APIs.
>
> Noted, well, maybe just skip this nit in such a case.
>

I already did the change locally, as I said it's a small change, so no problem.


> > > [...]
> > >
> > > > +static int bpf_program_fixup_func_info(struct bpf_object *obj, struct bpf_program *prog)
> > > > +{
> > >
> > > [...]
> > >
> > > > +     for (i = 1, n = btf__type_cnt(btf); i < n; i++) {
> > >
> > > [...]
> > >
> > > > +
> > > > +             /* clone fn/fn_proto, unless we already did it for another arg */
> > > > +             if (func_rec->type_id == orig_fn_id) {
> > > > +                     int fn_id;
> > > > +
> > > > +                     fn_id = clone_func_btf_info(btf, orig_fn_id, prog);
> > > > +                     if (fn_id < 0) {
> > > > +                             err = fn_id;
> > > > +                             goto err_out;
> > > > +                     }
> > > > +
> > > > +                     /* point func_info record to a cloned FUNC type */
> > > > +                     func_rec->type_id = fn_id;
> > >
> > > Would it be helpful to add a log here, saying that BTF for function
> > > so and so is changed before load?
> >
> > Would it? We don't have global subprog's name readily available, it
> > seems. So I'd need to refetch it by fn_id, then btf__str_by_offset()
> > just to emit cryptic (for most users) notifications that something
> > about some func info was adjusted. And then the user would get this
> > same message for the same subprog but in the context of a different
> > entry program. Just confusing, tbh.
> >
> > Unless you insist, I'd leave it as is. This logic is supposed to be
> > bullet-proof, so I'm not worried about debugging regressions with it
> > (but maybe I'm delusional).
>
> I was thinking about someone finding out that actual in-kernel BTF
> is different from that in the program object file, while debugging
> something. Might be a bit surprising. I'm not insisting on this, though.

Note the "/* check if existing parameter already matches verifier
expectations */", if program is using correct types, we don't touch
BTF for that subprog. If there was `void *ctx`, we don't really lose
any information.

If they use `struct pt_regs *ctx __arg_ctx`, then yeah, it will be
updated to `struct bpf_user_pt_regs_t *ctx __arg_ctx`, but even then,
original BTF has original FUNC -> FUNC_PROTO definition. You'd need to
fetch func_info and follow BTF IDs (I'm not sure if bpftool even shows
this today).

In short, I don't see why this would be a problem, but perhaps I
should just bite a bullet and do feature detector for this support.

>
> > > > +             }
> > > > +
> > > > +             /* create PTR -> STRUCT type chain to mark PTR_TO_CTX argument;
> > > > +              * we do it just once per main BPF program, as all global
> > > > +              * funcs share the same program type, so need only PTR ->
> > > > +              * STRUCT type chain
> > > > +              */
> > > > +             if (ptr_id == 0) {
> > > > +                     struct_id = btf__add_struct(btf, ctx_name, 0);
> > >
> > > Nit: Maybe try looking up existing id for type ctx_name first?
> >
> > It didn't feel important and I didn't want to do another linear BTF
> > search for each such argument. It's trivial to look it up, but I still
> > feel like that's a waste... I tried to avoid many linear searches,
> > which is why I structured the logic to do one pass over BTF to find
> > all decl_tags instead of going over each function and arg and
> > searching for decl_tag.
> >
> > Let's keep it as is, if there are any reasons to try to reuse struct
> > (if it is at all present, which for kprobe, for example, is quite
> > unlikely due to fancy bpf_user_pt_regs_t name), then we can easily add
> > it with no regressions.
>
> I was thinking about possible interaction with btf_struct_access(),
> but that is not used to verify context access at the moment.
> So, probably not important.

Right, and kernel can't trust user-provided BTF info, it will have to
find a match for kernel-side BTF info. I actually add this logic in a
patch set (pending locally for now) that adds support for
PTR_TO_BTF_ID arguments for global funcs.

Eduard Zingerman Jan. 4, 2024, 12:09 a.m. UTC | #5

On Wed, 2024-01-03 at 15:59 -0800, Andrii Nakryiko wrote:
[...]
> > > > > +     fn_id = btf__add_func(btf, prog->name, btf_func_linkage(fn_t), fn_t->type);
> > > > 
> > > > Nit: Why not call this function near the end, when fn_proto_id is available?
> > > >      Thus avoiding need to "guess" fn_t->type.
> > > > 
> > > 
> > > I think I did it to not have to remember btf_func_linkage(fn_t)
> > > (because fn_t will be invalidated) and because name_off will be reused
> > > for parameters. Neither is a big deal, I'll adjust to your suggestion.
> > > 
> > > But note, we are not guessing ID, it's guaranteed to be +1, it's an
> > > API contract of btf__add_xxx() APIs.
> > 
> > Noted, well, maybe just skip this nit in such a case.
> > 
> 
> I already did the change locally, as I said it's a small change, so no problem.

Oh, ok, thanks.

[...]

> > > > > +             /* clone fn/fn_proto, unless we already did it for another arg */
> > > > > +             if (func_rec->type_id == orig_fn_id) {
> > > > > +                     int fn_id;
> > > > > +
> > > > > +                     fn_id = clone_func_btf_info(btf, orig_fn_id, prog);
> > > > > +                     if (fn_id < 0) {
> > > > > +                             err = fn_id;
> > > > > +                             goto err_out;
> > > > > +                     }
> > > > > +
> > > > > +                     /* point func_info record to a cloned FUNC type */
> > > > > +                     func_rec->type_id = fn_id;
> > > > 
> > > > Would it be helpful to add a log here, saying that BTF for function
> > > > so and so is changed before load?
> > > 
> > > Would it? We don't have global subprog's name readily available, it
> > > seems. So I'd need to refetch it by fn_id, then btf__str_by_offset()
> > > just to emit cryptic (for most users) notifications that something
> > > about some func info was adjusted. And then the user would get this
> > > same message for the same subprog but in the context of a different
> > > entry program. Just confusing, tbh.
> > > 
> > > Unless you insist, I'd leave it as is. This logic is supposed to be
> > > bullet-proof, so I'm not worried about debugging regressions with it
> > > (but maybe I'm delusional).
> > 
> > I was thinking about someone finding out that actual in-kernel BTF
> > is different from that in the program object file, while debugging
> > something. Might be a bit surprising. I'm not insisting on this, though.
> 
> Note the "/* check if existing parameter already matches verifier
> expectations */", if program is using correct types, we don't touch
> BTF for that subprog. If there was `void *ctx`, we don't really lose
> any information.

But `void *ctx` would be changed to `struct bpf_user_pt_regs_t *ctx`, right?
And that might be a bit surprising. But whatever, if you think that adding
log entry here is too much of hassle -- let's leave it as is.

> If they use `struct pt_regs *ctx __arg_ctx`, then yeah, it will be
> updated to `struct bpf_user_pt_regs_t *ctx __arg_ctx`, but even then,
> original BTF has original FUNC -> FUNC_PROTO definition. You'd need to
> fetch func_info and follow BTF IDs (I'm not sure if bpftool even shows
> this today).
> 
> In short, I don't see why this would be a problem, but perhaps I
> should just bite a bullet and do feature detector for this support.

I like that current implementation does the transformation unconditionally,
it does no harm and avoids unnecessary branching.

[...]

Andrii Nakryiko Jan. 4, 2024, 12:27 a.m. UTC | #6

On Wed, Jan 3, 2024 at 4:09 PM Eduard Zingerman <[e-mail]> wrote:
>
> On Wed, 2024-01-03 at 15:59 -0800, Andrii Nakryiko wrote:
> [...]
> > > > > > +     fn_id = btf__add_func(btf, prog->name, btf_func_linkage(fn_t), fn_t->type);
> > > > >
> > > > > Nit: Why not call this function near the end, when fn_proto_id is available?
> > > > >      Thus avoiding need to "guess" fn_t->type.
> > > > >
> > > >
> > > > I think I did it to not have to remember btf_func_linkage(fn_t)
> > > > (because fn_t will be invalidated) and because name_off will be reused
> > > > for parameters. Neither is a big deal, I'll adjust to your suggestion.
> > > >
> > > > But note, we are not guessing ID, it's guaranteed to be +1, it's an
> > > > API contract of btf__add_xxx() APIs.
> > >
> > > Noted, well, maybe just skip this nit in such a case.
> > >
> >
> > I already did the change locally, as I said it's a small change, so no problem.
>
> Oh, ok, thanks.
>

np

> [...]
>
> > > > > > +             /* clone fn/fn_proto, unless we already did it for another arg */
> > > > > > +             if (func_rec->type_id == orig_fn_id) {
> > > > > > +                     int fn_id;
> > > > > > +
> > > > > > +                     fn_id = clone_func_btf_info(btf, orig_fn_id, prog);
> > > > > > +                     if (fn_id < 0) {
> > > > > > +                             err = fn_id;
> > > > > > +                             goto err_out;
> > > > > > +                     }
> > > > > > +
> > > > > > +                     /* point func_info record to a cloned FUNC type */
> > > > > > +                     func_rec->type_id = fn_id;
> > > > >
> > > > > Would it be helpful to add a log here, saying that BTF for function
> > > > > so and so is changed before load?
> > > >
> > > > Would it? We don't have global subprog's name readily available, it
> > > > seems. So I'd need to refetch it by fn_id, then btf__str_by_offset()
> > > > just to emit cryptic (for most users) notifications that something
> > > > about some func info was adjusted. And then the user would get this
> > > > same message for the same subprog but in the context of a different
> > > > entry program. Just confusing, tbh.
> > > >
> > > > Unless you insist, I'd leave it as is. This logic is supposed to be
> > > > bullet-proof, so I'm not worried about debugging regressions with it
> > > > (but maybe I'm delusional).
> > >
> > > I was thinking about someone finding out that actual in-kernel BTF
> > > is different from that in the program object file, while debugging
> > > something. Might be a bit surprising. I'm not insisting on this, though.
> >
> > Note the "/* check if existing parameter already matches verifier
> > expectations */", if program is using correct types, we don't touch
> > BTF for that subprog. If there was `void *ctx`, we don't really lose
> > any information.
>
> But `void *ctx` would be changed to `struct bpf_user_pt_regs_t *ctx`, right?
> And that might be a bit surprising. But whatever, if you think that adding
> log entry here is too much of hassle -- let's leave it as is.
>

Ok.

> > If they use `struct pt_regs *ctx __arg_ctx`, then yeah, it will be
> > updated to `struct bpf_user_pt_regs_t *ctx __arg_ctx`, but even then,
> > original BTF has original FUNC -> FUNC_PROTO definition. You'd need to
> > fetch func_info and follow BTF IDs (I'm not sure if bpftool even shows
> > this today).
> >
> > In short, I don't see why this would be a problem, but perhaps I
> > should just bite a bullet and do feature detector for this support.
>
> I like that current implementation does the transformation unconditionally,
> it does no harm and avoids unnecessary branching.
>

ack

> [...]

Patch

diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
index d5f5c1a8f543..edd80fd2728e 100644
--- a/tools/lib/bpf/libbpf.c
+++ b/tools/lib/bpf/libbpf.c
@@ -6152,7 +6152,7 @@  reloc_prog_func_and_line_info(const struct bpf_object *obj,
 	int err;
 
 	/* no .BTF.ext relocation if .BTF.ext is missing or kernel doesn't
-	 * supprot func/line info
+	 * support func/line info
 	 */
 	if (!obj->btf_ext || !kernel_supports(obj, FEAT_BTF_FUNC))
 		return 0;
@@ -6630,8 +6630,245 @@  static int bpf_prog_assign_exc_cb(struct bpf_object *obj, struct bpf_program *pr
 	return 0;
 }
 
-static int
-bpf_object_relocate(struct bpf_object *obj, const char *targ_btf_path)
+static struct {
+	enum bpf_prog_type prog_type;
+	const char *ctx_name;
+} global_ctx_map[] = {
+	{ BPF_PROG_TYPE_CGROUP_DEVICE,           "bpf_cgroup_dev_ctx" },
+	{ BPF_PROG_TYPE_CGROUP_SKB,              "__sk_buff" },
+	{ BPF_PROG_TYPE_CGROUP_SOCK,             "bpf_sock" },
+	{ BPF_PROG_TYPE_CGROUP_SOCK_ADDR,        "bpf_sock_addr" },
+	{ BPF_PROG_TYPE_CGROUP_SOCKOPT,          "bpf_sockopt" },
+	{ BPF_PROG_TYPE_CGROUP_SYSCTL,           "bpf_sysctl" },
+	{ BPF_PROG_TYPE_FLOW_DISSECTOR,          "__sk_buff" },
+	{ BPF_PROG_TYPE_KPROBE,                  "bpf_user_pt_regs_t" },
+	{ BPF_PROG_TYPE_LWT_IN,                  "__sk_buff" },
+	{ BPF_PROG_TYPE_LWT_OUT,                 "__sk_buff" },
+	{ BPF_PROG_TYPE_LWT_SEG6LOCAL,           "__sk_buff" },
+	{ BPF_PROG_TYPE_LWT_XMIT,                "__sk_buff" },
+	{ BPF_PROG_TYPE_NETFILTER,               "bpf_nf_ctx" },
+	{ BPF_PROG_TYPE_PERF_EVENT,              "bpf_perf_event_data" },
+	{ BPF_PROG_TYPE_RAW_TRACEPOINT,          "bpf_raw_tracepoint_args" },
+	{ BPF_PROG_TYPE_RAW_TRACEPOINT_WRITABLE, "bpf_raw_tracepoint_args" },
+	{ BPF_PROG_TYPE_SCHED_ACT,               "__sk_buff" },
+	{ BPF_PROG_TYPE_SCHED_CLS,               "__sk_buff" },
+	{ BPF_PROG_TYPE_SK_LOOKUP,               "bpf_sk_lookup" },
+	{ BPF_PROG_TYPE_SK_MSG,                  "sk_msg_md" },
+	{ BPF_PROG_TYPE_SK_REUSEPORT,            "sk_reuseport_md" },
+	{ BPF_PROG_TYPE_SK_SKB,                  "__sk_buff" },
+	{ BPF_PROG_TYPE_SOCK_OPS,                "bpf_sock_ops" },
+	{ BPF_PROG_TYPE_SOCKET_FILTER,           "__sk_buff" },
+	{ BPF_PROG_TYPE_XDP,                     "xdp_md" },
+	/* all other program types don't have "named" context structs */
+};
+
+static int clone_func_btf_info(struct btf *btf, int orig_fn_id, struct bpf_program *prog)
+{
+	int fn_id, fn_proto_id, ret_type_id, orig_proto_id;
+	int i, err, arg_cnt, name_off;
+	struct btf_type *fn_t, *fn_proto_t, *t;
+	struct btf_param *p;
+
+	/* caller already validated FUNC -> FUNC_PROTO validity */
+	fn_t = btf_type_by_id(btf, orig_fn_id);
+	fn_proto_t = btf_type_by_id(btf, fn_t->type);
+
+	/* Note that each btf__add_xxx() operation invalidates
+	 * all btf_type and string pointers, so we need to be
+	 * very careful when cloning BTF types. BTF type
+	 * pointers have to be always refetched. And to avoid
+	 * problems with invalidated string pointers, we
+	 * add empty strings initially, then just fix up
+	 * name_off offsets in place. Offsets are stable for
+	 * existing strings, so that works out.
+	 */
+	name_off = fn_t->name_off; /* we are about to invalidate fn_t */
+	ret_type_id = fn_proto_t->type; /* and fn_proto_t as well */
+	arg_cnt = btf_vlen(fn_proto_t);
+	orig_proto_id = fn_t->type; /* original FUNC_PROTO ID */
+
+	/* clone FUNC first, btf__add_func() enforces
+	 * non-empty name, so use entry program's name as
+	 * a placeholder, which we replace immediately
+	 */
+	fn_id = btf__add_func(btf, prog->name, btf_func_linkage(fn_t), fn_t->type);
+	if (fn_id < 0)
+		return -EINVAL;
+
+	fn_t = btf_type_by_id(btf, fn_id);
+	fn_t->name_off = name_off; /* reuse original string */
+	fn_t->type = fn_id + 1; /* we can predict FUNC_PROTO ID */
+
+	/* clone FUNC_PROTO and its params now */
+	fn_proto_id = btf__add_func_proto(btf, ret_type_id);
+	if (fn_proto_id < 0)
+		return -EINVAL;
+
+	for (i = 0; i < arg_cnt; i++) {
+		/* copy original parameter data */
+		t = btf_type_by_id(btf, orig_proto_id);
+		p = &btf_params(t)[i];
+		name_off = p->name_off;
+
+		err = btf__add_func_param(btf, "", p->type);
+		if (err)
+			return err;
+
+		fn_proto_t = btf_type_by_id(btf, fn_proto_id);
+		p = &btf_params(fn_proto_t)[i];
+		p->name_off = name_off; /* use remembered str offset */
+	}
+
+	return fn_id;
+}
+
+/* Check if main program or global subprog's function prototype has `arg:ctx`
+ * argument tags, and, if necessary, substitute correct type to match what BPF
+ * verifier would expect, taking into account specific program type. This
+ * allows to support __arg_ctx tag transparently on old kernels that don't yet
+ * have a native support for it in the verifier, making user's life much
+ * easier.
+ */
+static int bpf_program_fixup_func_info(struct bpf_object *obj, struct bpf_program *prog)
+{
+	const char *ctx_name = NULL, *ctx_tag = "arg:ctx";
+	struct bpf_func_info_min *func_rec;
+	struct btf_type *fn_t, *fn_proto_t;
+	struct btf *btf = obj->btf;
+	const struct btf_type *t;
+	struct btf_param *p;
+	int ptr_id = 0, struct_id, tag_id, orig_fn_id;
+	int i, n, arg_idx, arg_cnt, err, rec_idx;
+	int *orig_ids;
+
+	/* no .BTF.ext, no problem */
+	if (!obj->btf_ext || !prog->func_info)
+		return 0;
+
+	/* some BPF program types just don't have named context structs, so
+	 * this fallback mechanism doesn't work for them
+	 */
+	for (i = 0; i < ARRAY_SIZE(global_ctx_map); i++) {
+		if (global_ctx_map[i].prog_type != prog->type)
+			continue;
+		ctx_name = global_ctx_map[i].ctx_name;
+		break;
+	}
+	if (!ctx_name)
+		return 0;
+
+	/* remember original func BTF IDs to detect if we already cloned them */
+	orig_ids = calloc(prog->func_info_cnt, sizeof(*orig_ids));
+	if (!orig_ids)
+		return -ENOMEM;
+	for (i = 0; i < prog->func_info_cnt; i++) {
+		func_rec = prog->func_info + prog->func_info_rec_size * i;
+		orig_ids[i] = func_rec->type_id;
+	}
+
+	/* go through each DECL_TAG with "arg:ctx" and see if it points to one
+	 * of our subprogs; if yes and subprog is global and needs adjustment,
+	 * clone and adjust FUNC -> FUNC_PROTO combo
+	 */
+	for (i = 1, n = btf__type_cnt(btf); i < n; i++) {
+		/* only DECL_TAG with "arg:ctx" value are interesting */
+		t = btf__type_by_id(btf, i);
+		if (!btf_is_decl_tag(t))
+			continue;
+		if (strcmp(btf__str_by_offset(btf, t->name_off), ctx_tag) != 0)
+			continue;
+
+		/* only global funcs need adjustment, if at all */
+		orig_fn_id = t->type;
+		fn_t = btf_type_by_id(btf, orig_fn_id);
+		if (!btf_is_func(fn_t) || btf_func_linkage(fn_t) != BTF_FUNC_GLOBAL)
+			continue;
+
+		/* sanity check FUNC -> FUNC_PROTO chain, just in case */
+		fn_proto_t = btf_type_by_id(btf, fn_t->type);
+		if (!fn_proto_t || !btf_is_func_proto(fn_proto_t))
+			continue;
+
+		/* find corresponding func_info record */
+		func_rec = NULL;
+		for (rec_idx = 0; rec_idx < prog->func_info_cnt; rec_idx++) {
+			if (orig_ids[rec_idx] == t->type) {
+				func_rec = prog->func_info + prog->func_info_rec_size * rec_idx;
+				break;
+			}
+		}
+		/* current main program doesn't call into this subprog */
+		if (!func_rec)
+			continue;
+
+		/* some more sanity checking of DECL_TAG */
+		arg_cnt = btf_vlen(fn_proto_t);
+		arg_idx = btf_decl_tag(t)->component_idx;
+		if (arg_idx < 0 || arg_idx >= arg_cnt)
+			continue;
+
+		/* check if existing parameter already matches verifier expectations */
+		p = &btf_params(fn_proto_t)[arg_idx];
+		t = skip_mods_and_typedefs(btf, p->type, NULL);
+		if (btf_is_ptr(t) &&
+		    (t = skip_mods_and_typedefs(btf, t->type, NULL)) &&
+		    btf_is_struct(t) &&
+		    strcmp(btf__str_by_offset(btf, t->name_off), ctx_name) == 0) {
+			continue; /* no need for fix up */
+		}
+
+		/* clone fn/fn_proto, unless we already did it for another arg */
+		if (func_rec->type_id == orig_fn_id) {
+			int fn_id;
+
+			fn_id = clone_func_btf_info(btf, orig_fn_id, prog);
+			if (fn_id < 0) {
+				err = fn_id;
+				goto err_out;
+			}
+
+			/* point func_info record to a cloned FUNC type */
+			func_rec->type_id = fn_id;
+		}
+
+		/* create PTR -> STRUCT type chain to mark PTR_TO_CTX argument;
+		 * we do it just once per main BPF program, as all global
+		 * funcs share the same program type, so need only PTR ->
+		 * STRUCT type chain
+		 */
+		if (ptr_id == 0) {
+			struct_id = btf__add_struct(btf, ctx_name, 0);
+			ptr_id = btf__add_ptr(btf, struct_id);
+			if (ptr_id < 0 || struct_id < 0) {
+				err = -EINVAL;
+				goto err_out;
+			}
+		}
+
+		/* for completeness, clone DECL_TAG and point it to cloned param */
+		tag_id = btf__add_decl_tag(btf, ctx_tag, func_rec->type_id, arg_idx);
+		if (tag_id < 0) {
+			err = -EINVAL;
+			goto err_out;
+		}
+
+		/* all the BTF manipulations invalidated pointers, refetch them */
+		fn_t = btf_type_by_id(btf, func_rec->type_id);
+		fn_proto_t = btf_type_by_id(btf, fn_t->type);
+
+		/* fix up type ID pointed to by param */
+		p = &btf_params(fn_proto_t)[arg_idx];
+		p->type = ptr_id;
+	}
+
+	free(orig_ids);
+	return 0;
+err_out:
+	free(orig_ids);
+	return err;
+}
+
+static int bpf_object_relocate(struct bpf_object *obj, const char *targ_btf_path)
 {
 	struct bpf_program *prog;
 	size_t i, j;
@@ -6712,19 +6949,28 @@  bpf_object_relocate(struct bpf_object *obj, const char *targ_btf_path)
 			}
 		}
 	}
-	/* Process data relos for main programs */
 	for (i = 0; i < obj->nr_programs; i++) {
 		prog = &obj->programs[i];
 		if (prog_is_subprog(obj, prog))
 			continue;
 		if (!prog->autoload)
 			continue;
+
+		/* Process data relos for main programs */
 		err = bpf_object_relocate_data(obj, prog);
 		if (err) {
 			pr_warn("prog '%s': failed to relocate data references: %d\n",
 				prog->name, err);
 			return err;
 		}
+
+		/* Fix up .BTF.ext information, if necessary */
+		err = bpf_program_fixup_func_info(obj, prog);
+		if (err) {
+			pr_warn("prog '%s': failed to perform .BTF.ext fix ups: %d\n",
+				prog->name, err);
+			return err;
+		}
 	}
 
 	return 0;
@@ -7436,8 +7682,7 @@  static int bpf_program_record_relos(struct bpf_program *prog)
 	return 0;
 }
 
-static int
-bpf_object_load_progs(struct bpf_object *obj, int log_level)
+static int bpf_object_load_progs(struct bpf_object *obj, int log_level)
 {
 	struct bpf_program *prog;
 	size_t i;