2023-12-08 06:17:22

by David Vernet

[permalink] [raw]
Subject: [PATCH bpf-next] bpf: Load vmlinux btf for any struct_ops map

In libbpf, when determining whether we need to load vmlinux btf, we're
currently (among other things) checking whether there is any struct_ops
program present in the object. This works for most realistic struct_ops
maps, as a struct_ops map is of course typically composed of one or more
struct_ops programs. However, that technically need not be the case. A
struct_ops interface could be defined which allows a map to be specified
which one or more non-prog fields, and which provides default behavior
if no struct_ops progs is actually provided otherwise. For sched_ext,
for example, you technically only need to specify the name of the
scheduler in the struct_ops map, with the core scheduler logic providing
default behavior if no prog is actually specified.

If we were to define and try to load such a struct_ops map, we would
crash in libbpf when initializing it as obj->btf_vmlinux will be NULL:

Reading symbols from minimal...
(gdb) r
Starting program: minimal_example
[Thread debugging using libthread_db enabled]
Using host libthread_db library "/usr/lib/libthread_db.so.1".

Program received signal SIGSEGV, Segmentation fault.
0x000055555558308c in btf__type_cnt (btf=0x0) at btf.c:612
612 return btf->start_id + btf->nr_types;
(gdb) bt
type_name=0x5555555d99e3 "sched_ext_ops", kind=4) at btf.c:914
kind=4) at btf.c:942
type=0x7fffffffe558, type_id=0x7fffffffe548, ...
data_member=0x7fffffffe568) at libbpf.c:948
kern_btf=0x0) at libbpf.c:1017
at libbpf.c:8059

So as to account for such bare-bones struct_ops maps, let's update
obj_needs_vmlinux_btf() to also iterate over an obj's maps and check
whether any of them are struct_ops maps.

Signed-off-by: David Vernet <[email protected]>
---
tools/lib/bpf/libbpf.c | 11 +++++++++++
1 file changed, 11 insertions(+)

diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
index ea9b8158c20d..ac54ebc0629f 100644
--- a/tools/lib/bpf/libbpf.c
+++ b/tools/lib/bpf/libbpf.c
@@ -3054,9 +3054,15 @@ static bool prog_needs_vmlinux_btf(struct bpf_program *prog)
return false;
}

+static bool map_needs_vmlinux_btf(struct bpf_map *map)
+{
+ return bpf_map__is_struct_ops(map);
+}
+
static bool obj_needs_vmlinux_btf(const struct bpf_object *obj)
{
struct bpf_program *prog;
+ struct bpf_map *map;
int i;

/* CO-RE relocations need kernel BTF, only when btf_custom_path
@@ -3081,6 +3087,11 @@ static bool obj_needs_vmlinux_btf(const struct bpf_object *obj)
return true;
}

+ bpf_object__for_each_map(map, obj) {
+ if (map_needs_vmlinux_btf(map))
+ return true;
+ }
+
return false;
}

--
2.42.1


2023-12-08 10:06:33

by Alan Maguire

[permalink] [raw]
Subject: Re: [PATCH bpf-next] bpf: Load vmlinux btf for any struct_ops map

On 08/12/2023 06:17, David Vernet wrote:
> In libbpf, when determining whether we need to load vmlinux btf, we're
> currently (among other things) checking whether there is any struct_ops
> program present in the object. This works for most realistic struct_ops
> maps, as a struct_ops map is of course typically composed of one or more
> struct_ops programs. However, that technically need not be the case. A
> struct_ops interface could be defined which allows a map to be specified
> which one or more non-prog fields, and which provides default behavior
> if no struct_ops progs is actually provided otherwise. For sched_ext,
> for example, you technically only need to specify the name of the
> scheduler in the struct_ops map, with the core scheduler logic providing
> default behavior if no prog is actually specified.
>
> If we were to define and try to load such a struct_ops map, we would
> crash in libbpf when initializing it as obj->btf_vmlinux will be NULL:
>
> Reading symbols from minimal...
> (gdb) r
> Starting program: minimal_example
> [Thread debugging using libthread_db enabled]
> Using host libthread_db library "/usr/lib/libthread_db.so.1".
>
> Program received signal SIGSEGV, Segmentation fault.
> 0x000055555558308c in btf__type_cnt (btf=0x0) at btf.c:612
> 612 return btf->start_id + btf->nr_types;
> (gdb) bt
> type_name=0x5555555d99e3 "sched_ext_ops", kind=4) at btf.c:914
> kind=4) at btf.c:942
> type=0x7fffffffe558, type_id=0x7fffffffe548, ...
> data_member=0x7fffffffe568) at libbpf.c:948
> kern_btf=0x0) at libbpf.c:1017
> at libbpf.c:8059
>
> So as to account for such bare-bones struct_ops maps, let's update
> obj_needs_vmlinux_btf() to also iterate over an obj's maps and check
> whether any of them are struct_ops maps.
>
> Signed-off-by: David Vernet <[email protected]>

Makes sense to me.

Reviewed-by: Alan Maguire <[email protected]>

> ---
> tools/lib/bpf/libbpf.c | 11 +++++++++++
> 1 file changed, 11 insertions(+)
>
> diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
> index ea9b8158c20d..ac54ebc0629f 100644
> --- a/tools/lib/bpf/libbpf.c
> +++ b/tools/lib/bpf/libbpf.c
> @@ -3054,9 +3054,15 @@ static bool prog_needs_vmlinux_btf(struct bpf_program *prog)
> return false;
> }
>
> +static bool map_needs_vmlinux_btf(struct bpf_map *map)
> +{
> + return bpf_map__is_struct_ops(map);
> +}
> +
> static bool obj_needs_vmlinux_btf(const struct bpf_object *obj)
> {
> struct bpf_program *prog;
> + struct bpf_map *map;
> int i;
>
> /* CO-RE relocations need kernel BTF, only when btf_custom_path
> @@ -3081,6 +3087,11 @@ static bool obj_needs_vmlinux_btf(const struct bpf_object *obj)
> return true;
> }
>
> + bpf_object__for_each_map(map, obj) {
> + if (map_needs_vmlinux_btf(map))
> + return true;
> + }
> +
> return false;
> }
>

2023-12-08 17:41:21

by patchwork-bot+netdevbpf

[permalink] [raw]
Subject: Re: [PATCH bpf-next] bpf: Load vmlinux btf for any struct_ops map

Hello:

This patch was applied to bpf/bpf-next.git (master)
by Andrii Nakryiko <[email protected]>:

On Fri, 8 Dec 2023 00:17:03 -0600 you wrote:
> In libbpf, when determining whether we need to load vmlinux btf, we're
> currently (among other things) checking whether there is any struct_ops
> program present in the object. This works for most realistic struct_ops
> maps, as a struct_ops map is of course typically composed of one or more
> struct_ops programs. However, that technically need not be the case. A
> struct_ops interface could be defined which allows a map to be specified
> which one or more non-prog fields, and which provides default behavior
> if no struct_ops progs is actually provided otherwise. For sched_ext,
> for example, you technically only need to specify the name of the
> scheduler in the struct_ops map, with the core scheduler logic providing
> default behavior if no prog is actually specified.
>
> [...]

Here is the summary with links:
- [bpf-next] bpf: Load vmlinux btf for any struct_ops map
https://git.kernel.org/bpf/bpf-next/c/8b7b0e5fe47d

You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html