On 10/30/23 18:53, Martin KaFai Lau wrote:
On 10/30/23 12:28 PM, thinker.li@xxxxxxxxx wrote:
From: Kui-Feng Lee <thinker.li@xxxxxxxxx>
Giving a BTF, the bpf_struct_ops knows the right place to look up type
info
associated with a type ID. This enables a user space program to load a
struct_ops object linked to a struct_ops type defined by a module, by
providing the module BTF (fd).
This describes about the struct_ops map creation change (by adding
value_type_btf_obj_fd)? It could be described more clearly in the commit
message, like specify the value_type_btf_obj_fd addition and how it is
used in the struct_ops map creation.
I will rephrase this part as the following words.
Every kernel module has its BTF, comprising information on types defined
in the module. The BTF fd (attr->value_type_btf_obj_fd) passed from
userspace helps the bpf_struct_ops to lookup type information and
description of the struct_ops type, which is necessary for parsing the
layout of map element values and registering maps. The descriptions are
looked up by matching a type id (attr->btf_vmlinux_value_type_id)
against bpf_struct_ops_desc(s) defined in a BTF. If a struct_ops type
is defined in a module, the bpf_struct_ops needs to know the module BTF
to lookup the bpf_struct_ops_desc.
The bpf_prog includes attach_btf in aux which is passed along with the
bpf_attr when loading the program. The purpose of attach_btf is to
determine the btf type of attach_btf_id. The attach_btf_id is then
used to
identify the traced function for a trace program. In the case of
struct_ops
programs, it is used to identify the struct_ops type of the struct_ops
object that a program is attached to.
Does attach_btf_obj_fd also work?
aux->attach_btf is from attach_btf_obj_fd, being set by bpf_prog_load().
I will rephrase it to make it clear.
The bpf_prog includes attach_btf in aux which is passed along with the
bpf_attr (attr->attach_btf_obj_fd) when loading the program.
[ ... ]
diff --git a/kernel/bpf/bpf_struct_ops.c b/kernel/bpf/bpf_struct_ops.c
index 256516aba632..db2bbba50e38 100644
--- a/kernel/bpf/bpf_struct_ops.c
+++ b/kernel/bpf/bpf_struct_ops.c
@@ -694,6 +694,7 @@ static void __bpf_struct_ops_map_free(struct
bpf_map *map)
bpf_jit_uncharge_modmem(PAGE_SIZE);
}
bpf_map_area_free(st_map->uvalue);
+ btf_put(st_map->st_ops_desc->btf);
bpf_map_area_free(st_map);
}
@@ -735,16 +736,31 @@ static struct bpf_map
*bpf_struct_ops_map_alloc(union bpf_attr *attr)
const struct btf_type *t, *vt;
struct module *mod = NULL;
struct bpf_map *map;
+ struct btf *btf;
int ret;
- st_ops_desc = bpf_struct_ops_find_value(btf_vmlinux,
attr->btf_vmlinux_value_type_id);
- if (!st_ops_desc)
- return ERR_PTR(-ENOTSUPP);
+ if (attr->value_type_btf_obj_fd) {
+ /* The map holds btf for its whole life time. */
It took me a while to parse this comment and connect it with the
btf_put(st_map->st_ops_desc->btf) in the __bpf_struct_ops_map_free() above.
It is now like "btf" owns "struct_ops_desc" which also stores a pointer
pointing back to itself, like "btf->struct_ops_desc->btf". The
struct_ops_desc->btf was not initialized by st_map but st_map will
increment its refcount much later.
Can btf be directly stored in the st_map->btf instead and map_alloc
holds the refcnt of st_map->btf and btf_put(st_map->btf) in map_free?
This topic has been discussed several times.
I will add a pointer to btf in st_map to make
people happy :)
+ btf = btf_get_by_fd(attr->value_type_btf_obj_fd);
+ if (IS_ERR(btf))
+ return ERR_PTR(PTR_ERR(btf));
+
+ if (btf != btf_vmlinux) {
+ mod = btf_try_get_module(btf);
+ if (!mod) {
+ ret = -EINVAL;
+ goto errout;
+ }
+ }
+ } else {
+ btf = btf_vmlinux;
+ btf_get(btf);
+ }
- if (st_ops_desc->btf != btf_vmlinux) {
- mod = btf_try_get_module(st_ops_desc->btf);
- if (!mod)
- return ERR_PTR(-EINVAL);
+ st_ops_desc = bpf_struct_ops_find_value(btf,
attr->btf_vmlinux_value_type_id);
+ if (!st_ops_desc) {
+ ret = -ENOTSUPP;
+ goto errout;
}
vt = st_ops_desc->value_type;
