This series aims to add support to bpf_snprintf_btf() and bpf_seq_printf_btf() allowing them to store string representations of module-specific types, as well as the kernel-specific ones they currently support.
Patch 1 removes the btf_module_mutex, as since we will need to look up module BTF during BPF program execution, we don't want to risk sleeping in the various contexts in which BPF can run. The access patterns to the btf module list seem to conform to classic list RCU usage so with a few minor tweaks this seems workable.
Patch 2 replaces the unused flags field in struct btf_ptr with an obj_id field, allowing the specification of the id of a BTF module. If the value is 0, the core kernel vmlinux is assumed to contain the type's BTF information. Otherwise the module with that id is used to identify the type. If the object-id based lookup fails, we again fall back to vmlinux BTF.
Patch 3 is a selftest that uses veth (when built as a module) and a kprobe to display both a module-specific and kernel-specific type; both are arguments to veth_stats_rx(). Currently it looks up the module-specific type and object ids using libbpf; in future, these lookups will likely be supported directly in the BPF program via __builtin_btf_type_id(); but I need to determine a good test to determine if that builtin supports object ids.
Changes since RFC
- add patch to remove module mutex - modify to use obj_id instead of module name as identifier in "struct btf_ptr" (Andrii)
Alan Maguire (3): bpf: eliminate btf_module_mutex as RCU synchronization can be used bpf: add module support to btf display helpers selftests/bpf: verify module-specific types can be shown via bpf_snprintf_btf
include/linux/btf.h | 12 ++ include/uapi/linux/bpf.h | 13 ++- kernel/bpf/btf.c | 49 +++++--- kernel/trace/bpf_trace.c | 44 ++++++-- tools/include/uapi/linux/bpf.h | 13 ++- .../selftests/bpf/prog_tests/snprintf_btf_mod.c | 124 +++++++++++++++++++++ tools/testing/selftests/bpf/progs/bpf_iter.h | 2 +- tools/testing/selftests/bpf/progs/btf_ptr.h | 2 +- tools/testing/selftests/bpf/progs/veth_stats_rx.c | 72 ++++++++++++ 9 files changed, 292 insertions(+), 39 deletions(-) create mode 100644 tools/testing/selftests/bpf/prog_tests/snprintf_btf_mod.c create mode 100644 tools/testing/selftests/bpf/progs/veth_stats_rx.c
btf_module_mutex is used when manipulating the BTF module list. However we will wish to look up this list from BPF program context, and such contexts can include interrupt state where we cannot sleep due to a mutex_lock(). RCU usage here conforms quite closely to the example in the system call auditing example in Documentation/RCU/listRCU.rst ; and as such we can eliminate the lock and use list_del_rcu()/call_rcu() on module removal, and list_add_rcu() for module addition.
Signed-off-by: Alan Maguire alan.maguire@oracle.com --- kernel/bpf/btf.c | 31 +++++++++++++++++-------------- 1 file changed, 17 insertions(+), 14 deletions(-)
diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c index 8d6bdb4..333f41c 100644 --- a/kernel/bpf/btf.c +++ b/kernel/bpf/btf.c @@ -5758,13 +5758,13 @@ bool btf_id_set_contains(const struct btf_id_set *set, u32 id) #ifdef CONFIG_DEBUG_INFO_BTF_MODULES struct btf_module { struct list_head list; + struct rcu_head rcu; struct module *module; struct btf *btf; struct bin_attribute *sysfs_attr; };
static LIST_HEAD(btf_modules); -static DEFINE_MUTEX(btf_module_mutex);
static ssize_t btf_module_read(struct file *file, struct kobject *kobj, @@ -5777,10 +5777,21 @@ struct btf_module { return len; }
+static void btf_module_free(struct rcu_head *rcu) +{ + struct btf_module *btf_mod = container_of(rcu, struct btf_module, rcu); + + if (btf_mod->sysfs_attr) + sysfs_remove_bin_file(btf_kobj, btf_mod->sysfs_attr); + btf_put(btf_mod->btf); + kfree(btf_mod->sysfs_attr); + kfree(btf_mod); +} + static int btf_module_notify(struct notifier_block *nb, unsigned long op, void *module) { - struct btf_module *btf_mod, *tmp; + struct btf_module *btf_mod; struct module *mod = module; struct btf *btf; int err = 0; @@ -5811,11 +5822,9 @@ static int btf_module_notify(struct notifier_block *nb, unsigned long op, goto out; }
- mutex_lock(&btf_module_mutex); btf_mod->module = module; btf_mod->btf = btf; - list_add(&btf_mod->list, &btf_modules); - mutex_unlock(&btf_module_mutex); + list_add_rcu(&btf_mod->list, &btf_modules);
if (IS_ENABLED(CONFIG_SYSFS)) { struct bin_attribute *attr; @@ -5845,20 +5854,14 @@ static int btf_module_notify(struct notifier_block *nb, unsigned long op,
break; case MODULE_STATE_GOING: - mutex_lock(&btf_module_mutex); - list_for_each_entry_safe(btf_mod, tmp, &btf_modules, list) { + list_for_each_entry(btf_mod, &btf_modules, list) { if (btf_mod->module != module) continue;
- list_del(&btf_mod->list); - if (btf_mod->sysfs_attr) - sysfs_remove_bin_file(btf_kobj, btf_mod->sysfs_attr); - btf_put(btf_mod->btf); - kfree(btf_mod->sysfs_attr); - kfree(btf_mod); + list_del_rcu(&btf_mod->list); + call_rcu(&btf_mod->rcu, btf_module_free); break; } - mutex_unlock(&btf_module_mutex); break; } out:
bpf_snprintf_btf and bpf_seq_printf_btf use a "struct btf_ptr *" argument that specifies type information about the type to be displayed. Augment this information to include an object id. If this id is 0, the assumption is that it refers to a core kernel type from vmlinux; otherwise the object id specifies the module the type is in, or if no such id is found in the module list, we fall back to vmlinux.
Signed-off-by: Alan Maguire alan.maguire@oracle.com --- include/linux/btf.h | 12 ++++++++++++ include/uapi/linux/bpf.h | 13 +++++++------ kernel/bpf/btf.c | 18 +++++++++++++++++ kernel/trace/bpf_trace.c | 44 +++++++++++++++++++++++++++++++----------- tools/include/uapi/linux/bpf.h | 13 +++++++------ 5 files changed, 77 insertions(+), 23 deletions(-)
diff --git a/include/linux/btf.h b/include/linux/btf.h index 4c200f5..688786a 100644 --- a/include/linux/btf.h +++ b/include/linux/btf.h @@ -214,6 +214,14 @@ static inline const struct btf_var_secinfo *btf_type_var_secinfo( const char *btf_name_by_offset(const struct btf *btf, u32 offset); struct btf *btf_parse_vmlinux(void); struct btf *bpf_prog_get_target_btf(const struct bpf_prog *prog); +#ifdef CONFIG_DEBUG_INFO_BTF_MODULES +struct btf *bpf_get_btf_module(__u32 obj_id); +#else +static inline struct btf *bpf_get_btf_module(__u32 obj_id) +{ + return ERR_PTR(-ENOTSUPP); +} +#endif #else static inline const struct btf_type *btf_type_by_id(const struct btf *btf, u32 type_id) @@ -225,6 +233,10 @@ static inline const char *btf_name_by_offset(const struct btf *btf, { return NULL; } +static inline struct btf *bpf_get_btf_module(__u32 obj_id) +{ + return ERR_PTR(-ENOTSUPP); +} #endif
#endif diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h index 1233f14..ccb75299 100644 --- a/include/uapi/linux/bpf.h +++ b/include/uapi/linux/bpf.h @@ -3641,7 +3641,9 @@ struct bpf_stack_build_id { * the pointer data is carried out to avoid kernel crashes during * operation. Smaller types can use string space on the stack; * larger programs can use map data to store the string - * representation. + * representation. Module-specific data structures can be + * displayed if the module BTF object id is supplied in the + * *ptr*->obj_id field. * * The string can be subsequently shared with userspace via * bpf_perf_event_output() or ring buffer interfaces. @@ -5115,15 +5117,14 @@ struct bpf_sk_lookup { /* * struct btf_ptr is used for typed pointer representation; the * type id is used to render the pointer data as the appropriate type - * via the bpf_snprintf_btf() helper described above. A flags field - - * potentially to specify additional details about the BTF pointer - * (rather than its mode of display) - is included for future use. - * Display flags - BTF_F_* - are passed to bpf_snprintf_btf separately. + * via the bpf_snprintf_btf() helper described above. The obj_id + * is used to specify an object id (such as a module); if unset + * a core vmlinux type id is assumed. */ struct btf_ptr { void *ptr; __u32 type_id; - __u32 flags; /* BTF ptr flags; unused at present. */ + __u32 obj_id; /* BTF object; vmlinux if 0 */ };
/* diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c index 333f41c..8ee691e 100644 --- a/kernel/bpf/btf.c +++ b/kernel/bpf/btf.c @@ -5777,6 +5777,24 @@ struct btf_module { return len; }
+struct btf *bpf_get_btf_module(__u32 obj_id) +{ + struct btf *btf = ERR_PTR(-ENOENT); + struct btf_module *btf_mod; + + rcu_read_lock(); + list_for_each_entry_rcu(btf_mod, &btf_modules, list) { + if (!btf_mod->btf || obj_id != btf_mod->btf->id) + continue; + + refcount_inc(&btf_mod->btf->refcnt); + btf = btf_mod->btf; + break; + } + rcu_read_unlock(); + return btf; +} + static void btf_module_free(struct rcu_head *rcu) { struct btf_module *btf_mod = container_of(rcu, struct btf_module, rcu); diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c index 23a390a..66d4120 100644 --- a/kernel/trace/bpf_trace.c +++ b/kernel/trace/bpf_trace.c @@ -75,8 +75,8 @@ static struct bpf_raw_event_map *bpf_get_raw_tracepoint_module(const char *name) u64 bpf_get_stack(u64 r1, u64 r2, u64 r3, u64 r4, u64 r5);
static int bpf_btf_printf_prepare(struct btf_ptr *ptr, u32 btf_ptr_size, - u64 flags, const struct btf **btf, - s32 *btf_id); + u64 flags, struct btf **btf, + bool *btf_is_vmlinux, s32 *btf_id);
/** * trace_call_bpf - invoke BPF program @@ -786,15 +786,22 @@ struct bpf_seq_printf_buf { BPF_CALL_4(bpf_seq_printf_btf, struct seq_file *, m, struct btf_ptr *, ptr, u32, btf_ptr_size, u64, flags) { - const struct btf *btf; + bool btf_is_vmlinux; + struct btf *btf; s32 btf_id; int ret;
- ret = bpf_btf_printf_prepare(ptr, btf_ptr_size, flags, &btf, &btf_id); + ret = bpf_btf_printf_prepare(ptr, btf_ptr_size, flags, &btf, + &btf_is_vmlinux, &btf_id); if (ret) return ret;
- return btf_type_seq_show_flags(btf, btf_id, ptr->ptr, m, flags); + ret = btf_type_seq_show_flags(btf, btf_id, ptr->ptr, m, flags); + /* modules refcount their BTF */ + if (!btf_is_vmlinux) + btf_put(btf); + + return ret; }
static const struct bpf_func_proto bpf_seq_printf_btf_proto = { @@ -1205,7 +1212,8 @@ static bool bpf_d_path_allowed(const struct bpf_prog *prog) BTF_F_PTR_RAW | BTF_F_ZERO)
static int bpf_btf_printf_prepare(struct btf_ptr *ptr, u32 btf_ptr_size, - u64 flags, const struct btf **btf, + u64 flags, struct btf **btf, + bool *btf_is_vmlinux, s32 *btf_id) { const struct btf_type *t; @@ -1216,7 +1224,14 @@ static int bpf_btf_printf_prepare(struct btf_ptr *ptr, u32 btf_ptr_size, if (btf_ptr_size != sizeof(struct btf_ptr)) return -EINVAL;
- *btf = bpf_get_btf_vmlinux(); + *btf_is_vmlinux = false; + + if (ptr->obj_id > 0) + *btf = bpf_get_btf_module(ptr->obj_id); + if (ptr->obj_id == 0 || IS_ERR(*btf)) { + *btf = bpf_get_btf_vmlinux(); + *btf_is_vmlinux = true; + }
if (IS_ERR_OR_NULL(*btf)) return PTR_ERR(*btf); @@ -1237,16 +1252,23 @@ static int bpf_btf_printf_prepare(struct btf_ptr *ptr, u32 btf_ptr_size, BPF_CALL_5(bpf_snprintf_btf, char *, str, u32, str_size, struct btf_ptr *, ptr, u32, btf_ptr_size, u64, flags) { - const struct btf *btf; + bool btf_is_vmlinux; + struct btf *btf; s32 btf_id; int ret;
- ret = bpf_btf_printf_prepare(ptr, btf_ptr_size, flags, &btf, &btf_id); + ret = bpf_btf_printf_prepare(ptr, btf_ptr_size, flags, &btf, + &btf_is_vmlinux, &btf_id); if (ret) return ret;
- return btf_type_snprintf_show(btf, btf_id, ptr->ptr, str, str_size, - flags); + ret = btf_type_snprintf_show(btf, btf_id, ptr->ptr, str, str_size, + flags); + /* modules refcount their BTF */ + if (!btf_is_vmlinux) + btf_put(btf); + + return ret; }
const struct bpf_func_proto bpf_snprintf_btf_proto = { diff --git a/tools/include/uapi/linux/bpf.h b/tools/include/uapi/linux/bpf.h index 1233f14..ccb75299 100644 --- a/tools/include/uapi/linux/bpf.h +++ b/tools/include/uapi/linux/bpf.h @@ -3641,7 +3641,9 @@ struct bpf_stack_build_id { * the pointer data is carried out to avoid kernel crashes during * operation. Smaller types can use string space on the stack; * larger programs can use map data to store the string - * representation. + * representation. Module-specific data structures can be + * displayed if the module BTF object id is supplied in the + * *ptr*->obj_id field. * * The string can be subsequently shared with userspace via * bpf_perf_event_output() or ring buffer interfaces. @@ -5115,15 +5117,14 @@ struct bpf_sk_lookup { /* * struct btf_ptr is used for typed pointer representation; the * type id is used to render the pointer data as the appropriate type - * via the bpf_snprintf_btf() helper described above. A flags field - - * potentially to specify additional details about the BTF pointer - * (rather than its mode of display) - is included for future use. - * Display flags - BTF_F_* - are passed to bpf_snprintf_btf separately. + * via the bpf_snprintf_btf() helper described above. The obj_id + * is used to specify an object id (such as a module); if unset + * a core vmlinux type id is assumed. */ struct btf_ptr { void *ptr; __u32 type_id; - __u32 flags; /* BTF ptr flags; unused at present. */ + __u32 obj_id; /* BTF object; vmlinux if 0 */ };
/*
Verify that specifying a module object id in "struct btf_ptr *" along with a type id of a module-specific type will succeed.
veth_stats_rx() is chosen because its function signature consists of a module-specific type "struct veth_stats" and a kernel-specific one "struct net_device".
Currently the tests take the messy approach of determining object and type ids for the relevant module/function; __builtin_btf_type_id() supports object ids by returning a 64-bit value, but need to find a good way to determine if that support is present.
Signed-off-by: Alan Maguire alan.maguire@oracle.com --- .../selftests/bpf/prog_tests/snprintf_btf_mod.c | 124 +++++++++++++++++++++ tools/testing/selftests/bpf/progs/bpf_iter.h | 2 +- tools/testing/selftests/bpf/progs/btf_ptr.h | 2 +- tools/testing/selftests/bpf/progs/veth_stats_rx.c | 72 ++++++++++++ 4 files changed, 198 insertions(+), 2 deletions(-) create mode 100644 tools/testing/selftests/bpf/prog_tests/snprintf_btf_mod.c create mode 100644 tools/testing/selftests/bpf/progs/veth_stats_rx.c
diff --git a/tools/testing/selftests/bpf/prog_tests/snprintf_btf_mod.c b/tools/testing/selftests/bpf/prog_tests/snprintf_btf_mod.c new file mode 100644 index 0000000..89805d7 --- /dev/null +++ b/tools/testing/selftests/bpf/prog_tests/snprintf_btf_mod.c @@ -0,0 +1,124 @@ +// SPDX-License-Identifier: GPL-2.0 +#include <test_progs.h> +#include <linux/btf.h> +#include <bpf/btf.h> +#include "veth_stats_rx.skel.h" + +#define VETH_NAME "bpfveth0" + +/* Demonstrate that bpf_snprintf_btf succeeds for both module-specific + * and kernel-defined data structures; veth_stats_rx() is used as + * it has both module-specific and kernel-defined data as arguments. + * This test assumes that veth is built as a module and will skip if not. + */ +void test_snprintf_btf_mod(void) +{ + struct btf *vmlinux_btf = NULL, *veth_btf = NULL; + struct veth_stats_rx *skel = NULL; + struct veth_stats_rx__bss *bss; + int err, duration = 0; + __u32 id; + + err = system("ip link add name " VETH_NAME " type veth"); + if (CHECK(err, "system", "ip link add veth failed: %d\n", err)) + return; + + vmlinux_btf = btf__parse_raw("/sys/kernel/btf/vmlinux"); + err = libbpf_get_error(vmlinux_btf); + if (CHECK(err, "parse vmlinux BTF", "failed parsing vmlinux BTF: %d\n", + err)) + goto cleanup; + veth_btf = btf__parse_raw_split("/sys/kernel/btf/veth", vmlinux_btf); + err = libbpf_get_error(veth_btf); + if (err == -ENOENT) { + printf("%s:SKIP:no BTF info for veth\n", __func__); + test__skip(); + goto cleanup; + } + + if (CHECK(err, "parse veth BTF", "failed parsing veth BTF: %d\n", err)) + goto cleanup; + + skel = veth_stats_rx__open(); + if (CHECK(!skel, "skel_open", "failed to open skeleton\n")) + goto cleanup; + + err = veth_stats_rx__load(skel); + if (CHECK(err, "skel_load", "failed to load skeleton: %d\n", err)) + goto cleanup; + + bss = skel->bss; + + /* This could all be replaced by __builtin_btf_type_id(); but need + * a way to determine if it supports object and type id. In the + * meantime, look up type id for veth_stats and object id for veth. + */ + bss->veth_stats_btf_id = btf__find_by_name(veth_btf, "veth_stats"); + + if (CHECK(bss->veth_stats_btf_id <= 0, "find 'struct veth_stats'", + "could not find 'struct veth_stats' in veth BTF: %d", + bss->veth_stats_btf_id)) + goto cleanup; + + bss->veth_obj_id = 0; + + for (id = 1; bpf_btf_get_next_id(id, &id) == 0; ) { + struct bpf_btf_info info; + __u32 len = sizeof(info); + char name[64]; + int fd; + + fd = bpf_btf_get_fd_by_id(id); + if (fd < 0) + continue; + + memset(&info, 0, sizeof(info)); + info.name_len = sizeof(name); + info.name = (__u64)name; + if (bpf_obj_get_info_by_fd(fd, &info, &len) || + strcmp((char *)info.name, "veth") != 0) + continue; + bss->veth_obj_id = info.id; + } + + if (CHECK(bss->veth_obj_id == 0, "get obj id for veth module", + "could not get veth module id")) + goto cleanup; + + err = veth_stats_rx__attach(skel); + if (CHECK(err, "skel_attach", "skeleton attach failed: %d\n", err)) + goto cleanup; + + /* generate stats event, then delete; this ensures the program + * triggers prior to reading status. + */ + err = system("ethtool -S " VETH_NAME " > /dev/null"); + if (CHECK(err, "system", "ethtool -S failed: %d\n", err)) + goto cleanup; + + system("ip link delete " VETH_NAME); + + /* Make sure veth_stats_rx program was triggered and it set + * expected return values from bpf_trace_printk()s and all + * tests ran. + */ + if (CHECK(bss->ret <= 0, + "bpf_snprintf_btf: got return value", + "ret <= 0 %ld test %d\n", bss->ret, bss->ran_subtests)) + goto cleanup; + + if (CHECK(bss->ran_subtests == 0, "check if subtests ran", + "no subtests ran, did BPF program run?")) + goto cleanup; + + if (CHECK(bss->num_subtests != bss->ran_subtests, + "check all subtests ran", + "only ran %d of %d tests\n", bss->num_subtests, + bss->ran_subtests)) + goto cleanup; + +cleanup: + system("ip link delete " VETH_NAME ">/dev/null 2>&1"); + if (skel) + veth_stats_rx__destroy(skel); +} diff --git a/tools/testing/selftests/bpf/progs/bpf_iter.h b/tools/testing/selftests/bpf/progs/bpf_iter.h index 6a12554..d6d9838 100644 --- a/tools/testing/selftests/bpf/progs/bpf_iter.h +++ b/tools/testing/selftests/bpf/progs/bpf_iter.h @@ -119,7 +119,7 @@ struct bpf_iter__sockmap { struct btf_ptr { void *ptr; __u32 type_id; - __u32 flags; + __u32 obj_id; };
enum { diff --git a/tools/testing/selftests/bpf/progs/btf_ptr.h b/tools/testing/selftests/bpf/progs/btf_ptr.h index c3c9797..4f84034 100644 --- a/tools/testing/selftests/bpf/progs/btf_ptr.h +++ b/tools/testing/selftests/bpf/progs/btf_ptr.h @@ -16,7 +16,7 @@ struct btf_ptr { void *ptr; __u32 type_id; - __u32 flags; + __u32 obj_id; };
enum { diff --git a/tools/testing/selftests/bpf/progs/veth_stats_rx.c b/tools/testing/selftests/bpf/progs/veth_stats_rx.c new file mode 100644 index 0000000..f04fb55 --- /dev/null +++ b/tools/testing/selftests/bpf/progs/veth_stats_rx.c @@ -0,0 +1,72 @@ +// SPDX-License-Identifier: GPL-2.0 +/* Copyright (c) 2020, Oracle and/or its affiliates. */ + +#include "btf_ptr.h" +#include <bpf/bpf_helpers.h> +#include <bpf/bpf_tracing.h> +#include <bpf/bpf_core_read.h> + +#include <errno.h> + +long ret = 0; +int num_subtests = 0; +int ran_subtests = 0; +s32 veth_stats_btf_id = 0; +s32 veth_obj_id = 0; + +#define STRSIZE 2048 + +#ifndef ARRAY_SIZE +#define ARRAY_SIZE(x) (sizeof(x) / sizeof((x)[0])) +#endif + +struct { + __uint(type, BPF_MAP_TYPE_PERCPU_ARRAY); + __uint(max_entries, 1); + __type(key, __u32); + __type(value, char[STRSIZE]); +} strdata SEC(".maps"); + +SEC("kprobe/veth_stats_rx") +int veth_stats_rx(struct pt_regs *ctx) +{ + static __u64 flags[] = { 0, BTF_F_COMPACT, BTF_F_ZERO, BTF_F_PTR_RAW, + BTF_F_NONAME, BTF_F_COMPACT | BTF_F_ZERO | + BTF_F_PTR_RAW | BTF_F_NONAME }; + static struct btf_ptr p = { }; + __u32 btf_ids[] = { 0, 0 }; + __u32 obj_ids[] = { 0, 0 }; + void *ptrs[] = { 0, 0 }; + __u32 key = 0; + int i, j; + char *str; + + btf_ids[0] = veth_stats_btf_id; + obj_ids[0] = veth_obj_id; + ptrs[0] = (void *)PT_REGS_PARM1_CORE(ctx); + + btf_ids[1] = bpf_core_type_id_kernel(struct net_device); + ptrs[1] = (void *)PT_REGS_PARM2_CORE(ctx); + + str = bpf_map_lookup_elem(&strdata, &key); + if (!str) + return 0; + + for (i = 0; i < ARRAY_SIZE(btf_ids); i++) { + p.type_id = btf_ids[i]; + p.obj_id = obj_ids[i]; + p.ptr = ptrs[i]; + for (j = 0; j < ARRAY_SIZE(flags); j++) { + ++num_subtests; + ret = bpf_snprintf_btf(str, STRSIZE, &p, sizeof(p), 0); + if (ret < 0) + bpf_printk("returned %d when writing id %d", + ret, p.type_id); + ++ran_subtests; + } + } + + return 0; +} + +char _license[] SEC("license") = "GPL";
On 12/4/20 10:48 AM, Alan Maguire wrote:
This series aims to add support to bpf_snprintf_btf() and bpf_seq_printf_btf() allowing them to store string representations of module-specific types, as well as the kernel-specific ones they currently support.
Patch 1 removes the btf_module_mutex, as since we will need to look up module BTF during BPF program execution, we don't want to risk sleeping in the various contexts in which BPF can run. The access patterns to the btf module list seem to conform to classic list RCU usage so with a few minor tweaks this seems workable.
Patch 2 replaces the unused flags field in struct btf_ptr with an obj_id field, allowing the specification of the id of a BTF module. If the value is 0, the core kernel vmlinux is assumed to contain the type's BTF information. Otherwise the module with that id is used to identify the type. If the object-id based lookup fails, we again fall back to vmlinux BTF.
Patch 3 is a selftest that uses veth (when built as a module) and a kprobe to display both a module-specific and kernel-specific type; both are arguments to veth_stats_rx(). Currently it looks up the module-specific type and object ids using libbpf; in future, these lookups will likely be supported directly in the BPF program via __builtin_btf_type_id(); but I need to determine a good test to determine if that builtin supports object ids.
__builtin_btf_type_id() is really only supported in llvm12 and 64bit return value support is pushed to llvm12 trunk a while back. The builtin is introduced in llvm11 but has a corner bug, so llvm12 is recommended. So if people use the builtin, you can assume 64bit return value. libbpf support is required here. So in my opinion, there is no need to do feature detection.
Andrii has a patch to support 64bit return value for __builtin_btf_type_id() and I assume that one should be landed before or together with your patch.
Just for your info. The following is an example you could use to determine whether __builtin_btf_type_id() supports btf object id at llvm level.
-bash-4.4$ cat t.c int test(int arg) { return __builtin_btf_type_id(arg, 1); }
Compile to generate assembly code with latest llvm12 trunk: clang -target bpf -O2 -S -g -mcpu=v3 t.c In the asm code, you should see one line with r0 = 1 ll
Or you can generate obj code: clang -target bpf -O2 -c -g -mcpu=v3 t.c and then you disassemble the obj file llvm-objdump -d --no-show-raw-insn --no-leading-addr t.o You should see below in the output r0 = 1 ll
Use earlier version of llvm12 trunk, the builtin has 32bit return value, you will see r0 = 1 which is a 32bit imm to r0, while "r0 = 1 ll" is 64bit imm to r0.
Changes since RFC
- add patch to remove module mutex
- modify to use obj_id instead of module name as identifier in "struct btf_ptr" (Andrii)
Alan Maguire (3): bpf: eliminate btf_module_mutex as RCU synchronization can be used bpf: add module support to btf display helpers selftests/bpf: verify module-specific types can be shown via bpf_snprintf_btf
include/linux/btf.h | 12 ++ include/uapi/linux/bpf.h | 13 ++- kernel/bpf/btf.c | 49 +++++--- kernel/trace/bpf_trace.c | 44 ++++++-- tools/include/uapi/linux/bpf.h | 13 ++- .../selftests/bpf/prog_tests/snprintf_btf_mod.c | 124 +++++++++++++++++++++ tools/testing/selftests/bpf/progs/bpf_iter.h | 2 +- tools/testing/selftests/bpf/progs/btf_ptr.h | 2 +- tools/testing/selftests/bpf/progs/veth_stats_rx.c | 72 ++++++++++++ 9 files changed, 292 insertions(+), 39 deletions(-) create mode 100644 tools/testing/selftests/bpf/prog_tests/snprintf_btf_mod.c create mode 100644 tools/testing/selftests/bpf/progs/veth_stats_rx.c
On 12/5/20 12:35 PM, Yonghong Song wrote:
On 12/4/20 10:48 AM, Alan Maguire wrote:
This series aims to add support to bpf_snprintf_btf() and bpf_seq_printf_btf() allowing them to store string representations of module-specific types, as well as the kernel-specific ones they currently support.
Patch 1 removes the btf_module_mutex, as since we will need to look up module BTF during BPF program execution, we don't want to risk sleeping in the various contexts in which BPF can run. The access patterns to the btf module list seem to conform to classic list RCU usage so with a few minor tweaks this seems workable.
Patch 2 replaces the unused flags field in struct btf_ptr with an obj_id field, allowing the specification of the id of a BTF module. If the value is 0, the core kernel vmlinux is assumed to contain the type's BTF information. Otherwise the module with that id is used to identify the type. If the object-id based lookup fails, we again fall back to vmlinux BTF.
Patch 3 is a selftest that uses veth (when built as a module) and a kprobe to display both a module-specific and kernel-specific type; both are arguments to veth_stats_rx(). Currently it looks up the module-specific type and object ids using libbpf; in future, these lookups will likely be supported directly in the BPF program via __builtin_btf_type_id(); but I need to determine a good test to determine if that builtin supports object ids.
__builtin_btf_type_id() is really only supported in llvm12 and 64bit return value support is pushed to llvm12 trunk a while back. The builtin is introduced in llvm11 but has a corner bug, so llvm12 is recommended. So if people use the builtin, you can assume 64bit return value. libbpf support is required here. So in my opinion, there is no need to do feature detection.
if people use llvm11 which may cause test to fail, we can add an entry in selftest README file to warn people this specific test needs llvm12.
Andrii has a patch to support 64bit return value for __builtin_btf_type_id() and I assume that one should be landed before or together with your patch.
Just for your info. The following is an example you could use to determine whether __builtin_btf_type_id() supports btf object id at llvm level.
-bash-4.4$ cat t.c int test(int arg) { return __builtin_btf_type_id(arg, 1); }
Compile to generate assembly code with latest llvm12 trunk: clang -target bpf -O2 -S -g -mcpu=v3 t.c In the asm code, you should see one line with r0 = 1 ll
Or you can generate obj code: clang -target bpf -O2 -c -g -mcpu=v3 t.c and then you disassemble the obj file llvm-objdump -d --no-show-raw-insn --no-leading-addr t.o You should see below in the output r0 = 1 ll
Use earlier version of llvm12 trunk, the builtin has 32bit return value, you will see r0 = 1 which is a 32bit imm to r0, while "r0 = 1 ll" is 64bit imm to r0.
Changes since RFC
- add patch to remove module mutex
- modify to use obj_id instead of module name as identifier
in "struct btf_ptr" (Andrii)
Alan Maguire (3): bpf: eliminate btf_module_mutex as RCU synchronization can be used bpf: add module support to btf display helpers selftests/bpf: verify module-specific types can be shown via bpf_snprintf_btf
include/linux/btf.h | 12 ++ include/uapi/linux/bpf.h | 13 ++- kernel/bpf/btf.c | 49 +++++--- kernel/trace/bpf_trace.c | 44 ++++++-- tools/include/uapi/linux/bpf.h | 13 ++- .../selftests/bpf/prog_tests/snprintf_btf_mod.c | 124 +++++++++++++++++++++ tools/testing/selftests/bpf/progs/bpf_iter.h | 2 +- tools/testing/selftests/bpf/progs/btf_ptr.h | 2 +- tools/testing/selftests/bpf/progs/veth_stats_rx.c | 72 ++++++++++++ 9 files changed, 292 insertions(+), 39 deletions(-) create mode 100644 tools/testing/selftests/bpf/prog_tests/snprintf_btf_mod.c create mode 100644 tools/testing/selftests/bpf/progs/veth_stats_rx.c
On Sat, 5 Dec 2020, Yonghong Song wrote:
__builtin_btf_type_id() is really only supported in llvm12 and 64bit return value support is pushed to llvm12 trunk a while back. The builtin is introduced in llvm11 but has a corner bug, so llvm12 is recommended. So if people use the builtin, you can assume 64bit return value. libbpf support is required here. So in my opinion, there is no need to do feature detection.
Andrii has a patch to support 64bit return value for __builtin_btf_type_id() and I assume that one should be landed before or together with your patch.
Just for your info. The following is an example you could use to determine whether __builtin_btf_type_id() supports btf object id at llvm level.
-bash-4.4$ cat t.c int test(int arg) { return __builtin_btf_type_id(arg, 1); }
Compile to generate assembly code with latest llvm12 trunk: clang -target bpf -O2 -S -g -mcpu=v3 t.c In the asm code, you should see one line with r0 = 1 ll
Or you can generate obj code: clang -target bpf -O2 -c -g -mcpu=v3 t.c and then you disassemble the obj file llvm-objdump -d --no-show-raw-insn --no-leading-addr t.o You should see below in the output r0 = 1 ll
Use earlier version of llvm12 trunk, the builtin has 32bit return value, you will see r0 = 1 which is a 32bit imm to r0, while "r0 = 1 ll" is 64bit imm to r0.
Thanks for this Yonghong! I'm thinking the way I'll tackle it is to simply verify that the upper 32 bits specifying the veth module object id are non-zero; if they are zero, we'll skip the test (I think a skip probably makes sense as not everyone will have llvm12). Does that seem reasonable?
With the additional few minor changes on top of Andrii's patch, the use of __builtin_btf_type_id() worked perfectly. Thanks!
Alan
On 12/5/20 4:43 PM, Alan Maguire wrote:
On Sat, 5 Dec 2020, Yonghong Song wrote:
__builtin_btf_type_id() is really only supported in llvm12 and 64bit return value support is pushed to llvm12 trunk a while back. The builtin is introduced in llvm11 but has a corner bug, so llvm12 is recommended. So if people use the builtin, you can assume 64bit return value. libbpf support is required here. So in my opinion, there is no need to do feature detection.
Andrii has a patch to support 64bit return value for __builtin_btf_type_id() and I assume that one should be landed before or together with your patch.
Just for your info. The following is an example you could use to determine whether __builtin_btf_type_id() supports btf object id at llvm level.
-bash-4.4$ cat t.c int test(int arg) { return __builtin_btf_type_id(arg, 1); }
Compile to generate assembly code with latest llvm12 trunk: clang -target bpf -O2 -S -g -mcpu=v3 t.c In the asm code, you should see one line with r0 = 1 ll
Or you can generate obj code: clang -target bpf -O2 -c -g -mcpu=v3 t.c and then you disassemble the obj file llvm-objdump -d --no-show-raw-insn --no-leading-addr t.o You should see below in the output r0 = 1 ll
Use earlier version of llvm12 trunk, the builtin has 32bit return value, you will see r0 = 1 which is a 32bit imm to r0, while "r0 = 1 ll" is 64bit imm to r0.
Thanks for this Yonghong! I'm thinking the way I'll tackle it is to simply verify that the upper 32 bits specifying the veth module object id are non-zero; if they are zero, we'll skip the test (I think a skip probably makes sense as not everyone will have llvm12). Does that seem reasonable?
This should work too and we do not need to add a note in README.rst for this test then.
With the additional few minor changes on top of Andrii's patch, the use of __builtin_btf_type_id() worked perfectly. Thanks!
Alan
On Sat, Dec 5, 2020 at 4:44 PM Alan Maguire alan.maguire@oracle.com wrote:
On Sat, 5 Dec 2020, Yonghong Song wrote:
__builtin_btf_type_id() is really only supported in llvm12 and 64bit return value support is pushed to llvm12 trunk a while back. The builtin is introduced in llvm11 but has a corner bug, so llvm12 is recommended. So if people use the builtin, you can assume 64bit return value. libbpf support is required here. So in my opinion, there is no need to do feature detection.
Andrii has a patch to support 64bit return value for __builtin_btf_type_id() and I assume that one should be landed before or together with your patch.
Just for your info. The following is an example you could use to determine whether __builtin_btf_type_id() supports btf object id at llvm level.
-bash-4.4$ cat t.c int test(int arg) { return __builtin_btf_type_id(arg, 1); }
Compile to generate assembly code with latest llvm12 trunk: clang -target bpf -O2 -S -g -mcpu=v3 t.c In the asm code, you should see one line with r0 = 1 ll
Or you can generate obj code: clang -target bpf -O2 -c -g -mcpu=v3 t.c and then you disassemble the obj file llvm-objdump -d --no-show-raw-insn --no-leading-addr t.o You should see below in the output r0 = 1 ll
Use earlier version of llvm12 trunk, the builtin has 32bit return value, you will see r0 = 1 which is a 32bit imm to r0, while "r0 = 1 ll" is 64bit imm to r0.
Thanks for this Yonghong! I'm thinking the way I'll tackle it is to simply verify that the upper 32 bits specifying the veth module object id are non-zero; if they are zero, we'll skip
Let's instead of the real veth module use selftests's bpf_testmod, which I added specifically to use for tests like these. We can define whatever types we need in there.
the test (I think a skip probably makes sense as not everyone will have llvm12). Does that seem reasonable?
With the additional few minor changes on top of Andrii's patch, the use of __builtin_btf_type_id() worked perfectly. Thanks!
Alan
linux-kselftest-mirror@lists.linaro.org
-
Alan Maguire -
Andrii Nakryiko -
Yonghong Song