Commit 6cda7e17 authored by Andrii Nakryiko's avatar Andrii Nakryiko

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/20240312183245.341141-1-thinker.li@gmail.com/

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/20240313214139.685112-1-thinker.li@gmail.comSigned-off-by: default avatarAndrii Nakryiko <andrii@kernel.org>
parents 9bf48fa1 26a7cf2b
......@@ -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);
......
......@@ -93,9 +93,56 @@ static void test_struct_ops_load(void)
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();
}
......@@ -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,
};
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment