Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Libbpf-side __arg_ctx fallback support #6205

Closed

Conversation

kernel-patches-daemon-bpf[bot]
Copy link

Pull request for series with
subject: Libbpf-side __arg_ctx fallback support
version: 2
url: https://patchwork.kernel.org/project/netdevbpf/list/?series=813905

@kernel-patches-daemon-bpf
Copy link
Author

Upstream branch: 5abde62
series: https://patchwork.kernel.org/project/netdevbpf/list/?series=813905
version: 2

@kernel-patches-daemon-bpf
Copy link
Author

Upstream branch: 5abde62
series: https://patchwork.kernel.org/project/netdevbpf/list/?series=813905
version: 2

@kernel-patches-daemon-bpf
Copy link
Author

Upstream branch: 9c9d9f6
series: https://patchwork.kernel.org/project/netdevbpf/list/?series=813905
version: 2

@kernel-patches-daemon-bpf
Copy link
Author

Upstream branch: 2ab1efa
series: https://patchwork.kernel.org/project/netdevbpf/list/?series=813905
version: 2

@kernel-patches-daemon-bpf
Copy link
Author

Upstream branch: a640de4
series: https://patchwork.kernel.org/project/netdevbpf/list/?series=813905
version: 2

@kernel-patches-daemon-bpf
Copy link
Author

Upstream branch: b456005
series: https://patchwork.kernel.org/project/netdevbpf/list/?series=813905
version: 2

@kernel-patches-daemon-bpf
Copy link
Author

Upstream branch: b456005
series: https://patchwork.kernel.org/project/netdevbpf/list/?series=813905
version: 2

@kernel-patches-daemon-bpf
Copy link
Author

Upstream branch: 417fa6d
series: https://patchwork.kernel.org/project/netdevbpf/list/?series=813905
version: 2

@kernel-patches-daemon-bpf
Copy link
Author

Upstream branch: 417fa6d
series: https://patchwork.kernel.org/project/netdevbpf/list/?series=814242
version: 3

It makes future grepping and code analysis a bit easier.

Acked-by: Jiri Olsa <jolsa@kernel.org>
Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
Instead of inferring whether map already point to previously
created/pinned BPF map (which user can specify with bpf_map__reuse_fd()) API),
use explicit map->reused flag that is set in such case.

Acked-by: Jiri Olsa <jolsa@kernel.org>
Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
With the upcoming switch to preallocated placeholder FDs for maps,
switch various getters/setter away from checking map->fd. Use
map_is_created() helper that detect whether BPF map can be modified based
on map->obj->loaded state, with special provision for maps set up with
bpf_map__reuse_fd().

For backwards compatibility, we take map_is_created() into account in
bpf_map__fd() getter as well. This way before bpf_object__load() phase
bpf_map__fd() will always return -1, just as before the changes in
subsequent patches adding stable map->fd placeholders.

We also get rid of all internal uses of bpf_map__fd() getter, as it's
more oriented for uses external to libbpf. The above map_is_created()
check actually interferes with some of the internal uses, if map FD is
fetched through bpf_map__fd().

Acked-by: Jiri Olsa <jolsa@kernel.org>
Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
Move map creation to later during BPF object loading by pre-creating
stable placeholder FDs (utilizing memfd_create()). Use dup2()
syscall to then atomically make those placeholder FDs point to real
kernel BPF map objects.

This change allows to delay BPF map creation to after all the BPF
program relocations. That, in turn, allows to delay BTF finalization and
loading into kernel to after all the relocations as well. We'll take
advantage of the latter in subsequent patches to allow libbpf to adjust
BTF in a way that helps with BPF global function usage.

Clean up a few places where we close map->fd, which now shouldn't
happen, because map->fd should be a valid FD regardless of whether map
was created or not. Surprisingly and nicely it simplifies a bunch of
error handling code. If this change doesn't backfire, I'm tempted to
pre-create such stable FDs for other entities (progs, maybe even BTF).
We previously did some manipulations to make gen_loader work with fake
map FDs, with stable map FDs this hack is not necessary for maps (we
still have it for BTF, but I left it as is for now).

Acked-by: Jiri Olsa <jolsa@kernel.org>
Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
Move the logic of finding and assigning exception callback indices from
BTF sanitization step to program relocations step, which seems more
logical and will unblock moving BTF loading to after relocation step.

Exception callbacks discovery and assignment has no dependency on BTF
being loaded into the kernel, it only uses BTF information. It does need
to happen before subprogram relocations happen, though. Which is why the
split.

No functional changes.

Acked-by: Jiri Olsa <jolsa@kernel.org>
Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
With all the preparations in previous patches done we are ready to
postpone BTF loading and sanitization step until after all the
relocations are performed.

Acked-by: Jiri Olsa <jolsa@kernel.org>
Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
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 <jolsa@kernel.org>
Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
Add a few extra cases of global funcs with context arguments. This time
rely on "arg:ctx" decl_tag (__arg_ctx macro), but put it next to
"classic" cases where context argument has to be of an exact type that
BPF verifier expects (e.g., bpf_user_pt_regs_t for kprobe/uprobe).

Colocating all these cases separately from other global func args that
rely on arg:xxx decl tags (in verifier_global_subprogs.c) allows for
simpler backwards compatibility testing on old kernels. All the cases in
test_global_func_ctx_args.c are supposed to work on older kernels, which
was manually validated during development.

Acked-by: Jiri Olsa <jolsa@kernel.org>
Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
Add a test validating that libbpf uploads BTF and func_info with
rewritten type information for arguments of global subprogs that are
marked with __arg_ctx tag.

Suggested-by: Eduard Zingerman <eddyz87@gmail.com>
Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
@kernel-patches-daemon-bpf
Copy link
Author

Upstream branch: f8506c5
series: https://patchwork.kernel.org/project/netdevbpf/list/?series=814242
version: 3

@kernel-patches-daemon-bpf
Copy link
Author

Upstream branch: c040e90
series: https://patchwork.kernel.org/project/netdevbpf/list/?series=814242
version: 3

Pull request is NOT updated. Failed to apply https://patchwork.kernel.org/project/netdevbpf/list/?series=814242
error message:

Cmd('git') failed due to: exit code(128)
  cmdline: git am --3way
  stdout: 'Applying: libbpf: make uniform use of btf__fd() accessor inside libbpf
Using index info to reconstruct a base tree...
M	tools/lib/bpf/libbpf.c
Falling back to patching base and 3-way merge...
Auto-merging tools/lib/bpf/libbpf.c
No changes -- Patch already applied.
Applying: libbpf: use explicit map reuse flag to skip map creation steps
Using index info to reconstruct a base tree...
M	tools/lib/bpf/libbpf.c
Falling back to patching base and 3-way merge...
Auto-merging tools/lib/bpf/libbpf.c
No changes -- Patch already applied.
Applying: libbpf: don't rely on map->fd as an indicator of map being created
Using index info to reconstruct a base tree...
M	tools/lib/bpf/libbpf.c
Falling back to patching base and 3-way merge...
Auto-merging tools/lib/bpf/libbpf.c
No changes -- Patch already applied.
Applying: libbpf: use stable map placeholder FDs
Using index info to reconstruct a base tree...
M	tools/lib/bpf/libbpf.c
M	tools/lib/bpf/libbpf_internal.h
Falling back to patching base and 3-way merge...
Auto-merging tools/lib/bpf/libbpf.c
CONFLICT (content): Merge conflict in tools/lib/bpf/libbpf.c
Patch failed at 0004 libbpf: use stable map placeholder FDs
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".'
  stderr: 'error: Failed to merge in the changes.
hint: Use 'git am --show-current-patch=diff' to see the failed patch'

conflict:

diff --cc tools/lib/bpf/libbpf.c
index c5a42ac309fd,a58569b7e4bf..000000000000
--- a/tools/lib/bpf/libbpf.c
+++ b/tools/lib/bpf/libbpf.c
@@@ -8355,7 -8102,7 +8355,10 @@@ static int bpf_object_load(struct bpf_o
  	err = err ? : bpf_object__sanitize_maps(obj);
  	err = err ? : bpf_object__init_kern_struct_ops_maps(obj);
  	err = err ? : bpf_object__relocate(obj, obj->btf_custom_path ? : target_btf_path);
++<<<<<<< HEAD
 +	err = err ? : bpf_object__sanitize_and_load_btf(obj);
++=======
++>>>>>>> libbpf: use stable map placeholder FDs
  	err = err ? : bpf_object__create_maps(obj);
  	err = err ? : bpf_object__load_progs(obj, extra_log_level);
  	err = err ? : bpf_object_init_prog_arrays(obj);

@kernel-patches-daemon-bpf
Copy link
Author

At least one diff in series https://patchwork.kernel.org/project/netdevbpf/list/?series=814242 irrelevant now. Closing PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant