On 10/17/23 14:49, Andrii Nakryiko wrote:
On Tue, Oct 17, 2023 at 9:23 AM <thinker.li@xxxxxxxxx> wrote:
From: Kui-Feng Lee <thinker.li@xxxxxxxxx>
Locate the module BTFs for struct_ops maps and progs and pass them to the
kernel. This ensures that the kernel correctly resolves type IDs from the
appropriate module BTFs.
For the map of a struct_ops object, mod_btf is added to bpf_map to keep a
reference to the module BTF. The FD of the module BTF is passed to the
kernel as mod_btf_fd when the struct_ops object is loaded.
For a bpf_struct_ops prog, attach_btf_obj_fd of bpf_prog is the FD of a
module BTF in the kernel.
Signed-off-by: Kui-Feng Lee <thinker.li@xxxxxxxxx>
---
tools/lib/bpf/bpf.c | 4 ++-
tools/lib/bpf/bpf.h | 5 +++-
tools/lib/bpf/libbpf.c | 68 +++++++++++++++++++++++++++---------------
3 files changed, 51 insertions(+), 26 deletions(-)
I have a few nits, please accommodate them, and with that please add
my ack on libbpf side of things
Acked-by: Andrii Nakryiko <andrii@xxxxxxxxxx>
Thanks for reviewing!
diff --git a/tools/lib/bpf/bpf.c b/tools/lib/bpf/bpf.c
index b0f1913763a3..af46488e4ea9 100644
--- a/tools/lib/bpf/bpf.c
+++ b/tools/lib/bpf/bpf.c
@@ -169,7 +169,8 @@ int bpf_map_create(enum bpf_map_type map_type,
__u32 max_entries,
const struct bpf_map_create_opts *opts)
{
- const size_t attr_sz = offsetofend(union bpf_attr, map_extra);
+ const size_t attr_sz = offsetofend(union bpf_attr,
+ value_type_btf_obj_fd);
union bpf_attr attr;
int fd;
@@ -191,6 +192,7 @@ int bpf_map_create(enum bpf_map_type map_type,
attr.btf_key_type_id = OPTS_GET(opts, btf_key_type_id, 0);
attr.btf_value_type_id = OPTS_GET(opts, btf_value_type_id, 0);
attr.btf_vmlinux_value_type_id = OPTS_GET(opts, btf_vmlinux_value_type_id, 0);
+ attr.value_type_btf_obj_fd = OPTS_GET(opts, value_type_btf_obj_fd, 0);
attr.inner_map_fd = OPTS_GET(opts, inner_map_fd, 0);
attr.map_flags = OPTS_GET(opts, map_flags, 0);
diff --git a/tools/lib/bpf/bpf.h b/tools/lib/bpf/bpf.h
index 74c2887cfd24..1733cdc21241 100644
--- a/tools/lib/bpf/bpf.h
+++ b/tools/lib/bpf/bpf.h
@@ -51,8 +51,11 @@ struct bpf_map_create_opts {
__u32 numa_node;
__u32 map_ifindex;
+
+ __u32 value_type_btf_obj_fd;
+ size_t:0;
};
-#define bpf_map_create_opts__last_field map_ifindex
+#define bpf_map_create_opts__last_field value_type_btf_obj_fd
LIBBPF_API int bpf_map_create(enum bpf_map_type map_type,
const char *map_name,
diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
index 3a6108e3238b..d8a60fb52f5c 100644
--- a/tools/lib/bpf/libbpf.c
+++ b/tools/lib/bpf/libbpf.c
@@ -519,6 +519,7 @@ struct bpf_map {
struct bpf_map_def def;
__u32 numa_node;
__u32 btf_var_idx;
+ int mod_btf_fd;
__u32 btf_key_type_id;
__u32 btf_value_type_id;
__u32 btf_vmlinux_value_type_id;
@@ -893,6 +894,8 @@ bpf_object__add_programs(struct bpf_object *obj, Elf_Data *sec_data,
return 0;
}
+static int load_module_btfs(struct bpf_object *obj);
+
you don't need this forward declaration, do you?
I will remove it.
static const struct btf_member *
find_member_by_offset(const struct btf_type *t, __u32 bit_offset)
{
@@ -922,22 +925,29 @@ find_member_by_name(const struct btf *btf, const struct btf_type *t,
return NULL;
}
[...]
if (obj->btf && btf__fd(obj->btf) >= 0) {
create_attr.btf_fd = btf__fd(obj->btf);
@@ -7700,9 +7718,9 @@ static int bpf_object__read_kallsyms_file(struct bpf_object *obj)
return libbpf_kallsyms_parse(kallsyms_cb, obj);
}
-static int find_ksym_btf_id(struct bpf_object *obj, const char *ksym_name,
- __u16 kind, struct btf **res_btf,
- struct module_btf **res_mod_btf)
+static int find_module_btf_id(struct bpf_object *obj, const char *kern_name,
+ __u16 kind, struct btf **res_btf,
+ struct module_btf **res_mod_btf)
I actually find "find_module" terminology confusing, because it might
not be in the module after all, right? I think "find_ksym_btf_id" is a
totally appropriate name, and it's just that in some cases that kernel
symbol (ksym) will be found in the kernel module instead of in vmlinux
image itself. Still, it's a kernel. Let's keep the name?
Agree!
{
struct module_btf *mod_btf;
struct btf *btf;
[...]
