diff options
author | Andrii Nakryiko <[email protected]> | 2024-03-14 13:47:05 -0700 |
---|---|---|
committer | Andrii Nakryiko <[email protected]> | 2024-03-14 13:54:43 -0700 |
commit | 6cda7e17392e0eca9fcafcb9b1d269c31fd737b7 (patch) | |
tree | 23b3d167b14c63d771404758606aebfb8aaaa812 | |
parent | 9bf48fa19a4b1d186e08b20bf7e5de26a15644fb (diff) | |
parent | 26a7cf2bbea656837583f9a1a0f9390db63d6cc3 (diff) |
Merge branch 'ignore-additional-fields-in-the-struct_ops-maps-in-an-updated-version'
Kui-Feng Lee says:
====================
Ignore additional fields in the struct_ops maps in an updated version.
According to an offline discussion, it would be beneficial to
implement a backward-compatible method for struct_ops types with
additional fields that are not present in older kernels.
This patchset accepts additional fields of a struct_ops map with all
zero values even if these fields are not in the corresponding type in
the kernel. This provides a way to be backward compatible. User space
programs can use the same map on a machine running an old kernel by
clearing fields that do not exist in the kernel.
For example, in a test case, it adds an additional field "zeroed" that
doesn't exist in struct bpf_testmod_ops of the kernel.
struct bpf_testmod_ops___zeroed {
int (*test_1)(void);
void (*test_2)(int a, int b);
int (*test_maybe_null)(int dummy, struct task_struct *task);
int zeroed;
};
SEC(".struct_ops.link")
struct bpf_testmod_ops___zeroed testmod_zeroed = {
.test_1 = (void *)test_1,
.test_2 = (void *)test_2_v2,
};
Here, it doesn't assign a value to "zeroed" of testmod_zeroed, and by
default the value of this field will be zero. So, the map will be
accepted by libbpf, but libbpf will skip the "zeroed" field. However,
if the "zeroed" field is assigned to any value other than "0", libbpf
will reject to load this map.
---
Changes from v1:
- Fix the issue about function pointer fields.
- Change a warning message, and add an info message for skipping
fields.
- Add a small demo of additional arguments that are not in the
function pointer prototype in the kernel.
v1: https://lore.kernel.org/all/[email protected]/
Kui-Feng Lee (3):
libbpf: Skip zeroed or null fields if not found in the kernel type.
selftests/bpf: Ensure libbpf skip all-zeros fields of struct_ops maps.
selftests/bpf: Accept extra arguments if they are not used.
tools/lib/bpf/libbpf.c | 24 +++-
.../bpf/prog_tests/test_struct_ops_module.c | 103 ++++++++++++++++++
.../bpf/progs/struct_ops_extra_arg.c | 49 +++++++++
.../selftests/bpf/progs/struct_ops_module.c | 16 ++-
4 files changed, 186 insertions(+), 6 deletions(-)
create mode 100644 tools/testing/selftests/bpf/progs/struct_ops_extra_arg.c
====================
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Andrii Nakryiko <[email protected]>
-rw-r--r-- | tools/lib/bpf/libbpf.c | 24 | ||||
-rw-r--r-- | tools/testing/selftests/bpf/prog_tests/test_struct_ops_module.c | 47 | ||||
-rw-r--r-- | tools/testing/selftests/bpf/progs/struct_ops_module.c | 16 |
3 files changed, 81 insertions, 6 deletions
diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c index 124883eaa0cf..604368cfbf02 100644 --- a/tools/lib/bpf/libbpf.c +++ b/tools/lib/bpf/libbpf.c @@ -1132,8 +1132,26 @@ static int bpf_map__init_kern_struct_ops(struct bpf_map *map) const char *mname; mname = btf__name_by_offset(btf, member->name_off); + moff = member->offset / 8; + mdata = data + moff; + msize = btf__resolve_size(btf, member->type); + if (msize < 0) { + pr_warn("struct_ops init_kern %s: failed to resolve the size of member %s\n", + map->name, mname); + return msize; + } + kern_member = find_member_by_name(kern_btf, kern_type, mname); if (!kern_member) { + /* Skip all zeros or null fields if they are not + * presented in the kernel BTF. + */ + if (libbpf_is_mem_zeroed(mdata, msize)) { + pr_info("struct_ops %s: member %s not found in kernel, skipping it as it's set to zero\n", + map->name, mname); + continue; + } + pr_warn("struct_ops init_kern %s: Cannot find member %s in kernel BTF\n", map->name, mname); return -ENOTSUP; @@ -1147,10 +1165,7 @@ static int bpf_map__init_kern_struct_ops(struct bpf_map *map) return -ENOTSUP; } - moff = member->offset / 8; kern_moff = kern_member->offset / 8; - - mdata = data + moff; kern_mdata = kern_data + kern_moff; mtype = skip_mods_and_typedefs(btf, member->type, &mtype_id); @@ -1230,9 +1245,8 @@ static int bpf_map__init_kern_struct_ops(struct bpf_map *map) continue; } - msize = btf__resolve_size(btf, mtype_id); kern_msize = btf__resolve_size(kern_btf, kern_mtype_id); - if (msize < 0 || kern_msize < 0 || msize != kern_msize) { + if (kern_msize < 0 || msize != kern_msize) { pr_warn("struct_ops init_kern %s: Error in size of member %s: %zd != %zd(kernel)\n", map->name, mname, (ssize_t)msize, (ssize_t)kern_msize); diff --git a/tools/testing/selftests/bpf/prog_tests/test_struct_ops_module.c b/tools/testing/selftests/bpf/prog_tests/test_struct_ops_module.c index ee5372c7f2c7..098776d00ab4 100644 --- a/tools/testing/selftests/bpf/prog_tests/test_struct_ops_module.c +++ b/tools/testing/selftests/bpf/prog_tests/test_struct_ops_module.c @@ -93,9 +93,56 @@ cleanup: struct_ops_module__destroy(skel); } +static void test_struct_ops_not_zeroed(void) +{ + struct struct_ops_module *skel; + int err; + + /* zeroed is 0, and zeroed_op is null */ + skel = struct_ops_module__open(); + if (!ASSERT_OK_PTR(skel, "struct_ops_module_open")) + return; + + err = struct_ops_module__load(skel); + ASSERT_OK(err, "struct_ops_module_load"); + + struct_ops_module__destroy(skel); + + /* zeroed is not 0 */ + skel = struct_ops_module__open(); + if (!ASSERT_OK_PTR(skel, "struct_ops_module_open_not_zeroed")) + return; + + /* libbpf should reject the testmod_zeroed since struct + * bpf_testmod_ops in the kernel has no "zeroed" field and the + * value of "zeroed" is non-zero. + */ + skel->struct_ops.testmod_zeroed->zeroed = 0xdeadbeef; + err = struct_ops_module__load(skel); + ASSERT_ERR(err, "struct_ops_module_load_not_zeroed"); + + struct_ops_module__destroy(skel); + + /* zeroed_op is not null */ + skel = struct_ops_module__open(); + if (!ASSERT_OK_PTR(skel, "struct_ops_module_open_not_zeroed_op")) + return; + + /* libbpf should reject the testmod_zeroed since the value of its + * "zeroed_op" is not null. + */ + skel->struct_ops.testmod_zeroed->zeroed_op = skel->progs.test_3; + err = struct_ops_module__load(skel); + ASSERT_ERR(err, "struct_ops_module_load_not_zeroed_op"); + + struct_ops_module__destroy(skel); +} + void serial_test_struct_ops_module(void) { if (test__start_subtest("test_struct_ops_load")) test_struct_ops_load(); + if (test__start_subtest("test_struct_ops_not_zeroed")) + test_struct_ops_not_zeroed(); } diff --git a/tools/testing/selftests/bpf/progs/struct_ops_module.c b/tools/testing/selftests/bpf/progs/struct_ops_module.c index 026cabfa7f1f..86e1e50c5531 100644 --- a/tools/testing/selftests/bpf/progs/struct_ops_module.c +++ b/tools/testing/selftests/bpf/progs/struct_ops_module.c @@ -23,7 +23,7 @@ void BPF_PROG(test_2, int a, int b) test_2_result = a + b; } -SEC("struct_ops/test_3") +SEC("?struct_ops/test_3") int BPF_PROG(test_3, int a, int b) { test_2_result = a + b + 3; @@ -54,3 +54,17 @@ struct bpf_testmod_ops___v2 testmod_2 = { .test_1 = (void *)test_1, .test_2 = (void *)test_2_v2, }; + +struct bpf_testmod_ops___zeroed { + int (*test_1)(void); + void (*test_2)(int a, int b); + int (*test_maybe_null)(int dummy, struct task_struct *task); + void (*zeroed_op)(int a, int b); + int zeroed; +}; + +SEC(".struct_ops.link") +struct bpf_testmod_ops___zeroed testmod_zeroed = { + .test_1 = (void *)test_1, + .test_2 = (void *)test_2_v2, +}; |