* [PATCH bpf-next v2 00/15] libbpf: type suffixes and autocreate flag for struct_ops maps
@ 2024-03-02 1:19 Eduard Zingerman
2024-03-02 1:19 ` [PATCH bpf-next v2 01/15] libbpf: allow version suffixes (___smth) for struct_ops types Eduard Zingerman
` (14 more replies)
0 siblings, 15 replies; 17+ messages in thread
From: Eduard Zingerman @ 2024-03-02 1:19 UTC (permalink / raw)
To: bpf, ast
Cc: andrii, daniel, martin.lau, kernel-team, yonghong.song, void,
sinquersw, Eduard Zingerman
Tweak struct_ops related APIs to allow the following features:
- specify version suffixes for stuct_ops map types;
- share same BPF program between several map definitions with
different local BTF types, assuming only maps with same
kernel BTF type would be selected for load;
- toggle autocreate flag for struct_ops maps;
- automatically toggle autoload for struct_ops programs referenced
from struct_ops maps, depending on autocreate status of the
corresponding map;
- use SEC("?.struct_ops") and SEC("?.struct_ops.link")
to define struct_ops maps with autocreate == false after object open.
This would allow loading programs like below:
SEC("struct_ops/foo") int BPF_PROG(foo) { ... }
SEC("struct_ops/bar") int BPF_PROG(bar) { ... }
struct bpf_testmod_ops___v1 {
int (*foo)(void);
};
struct bpf_testmod_ops___v2 {
int (*foo)(void);
int (*bar)(void);
};
/* Assume kernel type name to be 'test_ops' */
SEC(".struct_ops.link")
struct test_ops___v1 map_v1 = {
/* Program 'foo' shared by maps with
* different local BTF type
*/
.foo = (void *)foo
};
SEC(".struct_ops.link")
struct test_ops___v2 map_v2 = {
.foo = (void *)foo,
.bar = (void *)bar
};
Assuming the following tweaks are done before loading:
/* to load v1 */
bpf_map__set_autocreate(skel->maps.map_v1, true);
bpf_map__set_autocreate(skel->maps.map_v2, false);
/* to load v2 */
bpf_map__set_autocreate(skel->maps.map_v1, false);
bpf_map__set_autocreate(skel->maps.map_v2, true);
Patch #8 ties autocreate and autoload flags for struct_ops maps and
programs. While discussion for v1 concluded that such feature is
useful, it is an outlier compared to the rest of libbpf API that
treats autoload / autocreate flags in most simple way possible.
An alternative might be a dedicated API call, e.g.:
int bpf_object__infer_struct_ops_progs_autoload(struct bpf_object *)
That would set programs autoload flags depending on user defined state
of autocreate flags of struct_ops map.
It might even be accompanied by an API call:
int bpf_object__infer_struct_ops_maps_autocreate(struct bpf_object *)
That would check which struct_ops maps definitions could be loaded
with current kernel, or report error if there are ambiguities.
Changelog:
- v1 [1] -> v2:
- fixed memory leak in patch #1 (Kui-Feng);
- improved error messages in patch #2 (Martin, Andrii);
- in bad_struct_ops selftest from patch #6 added .test_2
map member setup (David);
- added utility functions to capture libbpf log from selftests (David)
- in selftests replaced usage of ...__open_and_load by separate
calls to ..._open() and ..._load() (Andrii);
- removed serial_... in selftest definitions (Andrii);
- improved comments in selftest struct_ops_autocreate
from patch #7 (David);
- removed autoload toggling logic incompatible with shadow variables
from bpf_map__set_autocreate(), instead struct_ops programs
autoload property is computed at struct_ops maps load phase,
see patch #8 (Kui-Feng, Martin, Andrii);
- added support for SEC("?.struct_ops") and SEC("?.struct_ops.link")
(Andrii).
[1] https://lore.kernel.org/bpf/20240227204556.17524-1-eddyz87@gmail.com/
Eduard Zingerman (15):
libbpf: allow version suffixes (___smth) for struct_ops types
libbpf: tie struct_ops programs to kernel BTF ids, not to local ids
libbpf: honor autocreate flag for struct_ops maps
selftests/bpf: test struct_ops map definition with type suffix
selftests/bpf: utility functions to capture libbpf log in test_progs
selftests/bpf: bad_struct_ops test
selftests/bpf: test autocreate behavior for struct_ops maps
libbpf: sync progs autoload with maps autocreate for struct_ops maps
selftests/bpf: verify struct_ops autoload/autocreate sync
libbpf: replace elf_state->st_ops_* fields with SEC_ST_OPS sec_type
libbpf: struct_ops in SEC("?.struct_ops") and SEC("?.struct_ops.link")
libbpf: rewrite btf datasec names starting from '?'
selftests/bpf: test case for SEC("?.struct_ops")
bpf: allow '?' at the beginning of DATASEC names
selftests/bpf: test cases for '?' in BTF names
kernel/bpf/btf.c | 17 +-
tools/lib/bpf/features.c | 22 +++
tools/lib/bpf/libbpf.c | 174 ++++++++++++------
tools/lib/bpf/libbpf_internal.h | 2 +
.../selftests/bpf/bpf_testmod/bpf_testmod.c | 25 +++
.../selftests/bpf/bpf_testmod/bpf_testmod.h | 4 +
.../selftests/bpf/prog_tests/bad_struct_ops.c | 67 +++++++
tools/testing/selftests/bpf/prog_tests/btf.c | 46 +++++
.../bpf/prog_tests/struct_ops_autocreate.c | 124 +++++++++++++
.../bpf/prog_tests/test_struct_ops_module.c | 33 +++-
.../selftests/bpf/progs/bad_struct_ops.c | 25 +++
.../selftests/bpf/progs/bad_struct_ops2.c | 14 ++
.../bpf/progs/struct_ops_autocreate.c | 52 ++++++
.../selftests/bpf/progs/struct_ops_module.c | 21 ++-
tools/testing/selftests/bpf/test_progs.c | 57 ++++++
tools/testing/selftests/bpf/test_progs.h | 3 +
16 files changed, 614 insertions(+), 72 deletions(-)
create mode 100644 tools/testing/selftests/bpf/prog_tests/bad_struct_ops.c
create mode 100644 tools/testing/selftests/bpf/prog_tests/struct_ops_autocreate.c
create mode 100644 tools/testing/selftests/bpf/progs/bad_struct_ops.c
create mode 100644 tools/testing/selftests/bpf/progs/bad_struct_ops2.c
create mode 100644 tools/testing/selftests/bpf/progs/struct_ops_autocreate.c
--
2.43.0
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH bpf-next v2 01/15] libbpf: allow version suffixes (___smth) for struct_ops types
2024-03-02 1:19 [PATCH bpf-next v2 00/15] libbpf: type suffixes and autocreate flag for struct_ops maps Eduard Zingerman
@ 2024-03-02 1:19 ` Eduard Zingerman
2024-03-02 1:19 ` [PATCH bpf-next v2 02/15] libbpf: tie struct_ops programs to kernel BTF ids, not to local ids Eduard Zingerman
` (13 subsequent siblings)
14 siblings, 0 replies; 17+ messages in thread
From: Eduard Zingerman @ 2024-03-02 1:19 UTC (permalink / raw)
To: bpf, ast
Cc: andrii, daniel, martin.lau, kernel-team, yonghong.song, void,
sinquersw, Eduard Zingerman
E.g. allow the following struct_ops definitions:
struct bpf_testmod_ops___v1 { int (*test)(void); };
struct bpf_testmod_ops___v2 { int (*test)(void); };
SEC(".struct_ops.link")
struct bpf_testmod_ops___v1 a = { .test = ... }
SEC(".struct_ops.link")
struct bpf_testmod_ops___v2 b = { .test = ... }
Where both bpf_testmod_ops__v1 and bpf_testmod_ops__v2 would be
resolved as 'struct bpf_testmod_ops' from kernel BTF.
Acked-by: David Vernet <void@manifault.com>
Signed-off-by: Eduard Zingerman <eddyz87@gmail.com>
---
tools/lib/bpf/libbpf.c | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
index 6c2979f1b471..e2a4c409980b 100644
--- a/tools/lib/bpf/libbpf.c
+++ b/tools/lib/bpf/libbpf.c
@@ -948,7 +948,7 @@ static int find_btf_by_prefix_kind(const struct btf *btf, const char *prefix,
const char *name, __u32 kind);
static int
-find_struct_ops_kern_types(struct bpf_object *obj, const char *tname,
+find_struct_ops_kern_types(struct bpf_object *obj, const char *tname_raw,
struct module_btf **mod_btf,
const struct btf_type **type, __u32 *type_id,
const struct btf_type **vtype, __u32 *vtype_id,
@@ -958,8 +958,12 @@ find_struct_ops_kern_types(struct bpf_object *obj, const char *tname,
const struct btf_member *kern_data_member;
struct btf *btf;
__s32 kern_vtype_id, kern_type_id;
+ char tname[256];
__u32 i;
+ snprintf(tname, sizeof(tname), "%.*s",
+ (int)bpf_core_essential_name_len(tname_raw), tname_raw);
+
kern_type_id = find_ksym_btf_id(obj, tname, BTF_KIND_STRUCT,
&btf, mod_btf);
if (kern_type_id < 0) {
--
2.43.0
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH bpf-next v2 02/15] libbpf: tie struct_ops programs to kernel BTF ids, not to local ids
2024-03-02 1:19 [PATCH bpf-next v2 00/15] libbpf: type suffixes and autocreate flag for struct_ops maps Eduard Zingerman
2024-03-02 1:19 ` [PATCH bpf-next v2 01/15] libbpf: allow version suffixes (___smth) for struct_ops types Eduard Zingerman
@ 2024-03-02 1:19 ` Eduard Zingerman
2024-03-02 1:19 ` [PATCH bpf-next v2 03/15] libbpf: honor autocreate flag for struct_ops maps Eduard Zingerman
` (12 subsequent siblings)
14 siblings, 0 replies; 17+ messages in thread
From: Eduard Zingerman @ 2024-03-02 1:19 UTC (permalink / raw)
To: bpf, ast
Cc: andrii, daniel, martin.lau, kernel-team, yonghong.song, void,
sinquersw, Eduard Zingerman
Enforce the following existing limitation on struct_ops programs based
on kernel BTF id instead of program-local BTF id:
struct_ops BPF prog can be re-used between multiple .struct_ops &
.struct_ops.link as long as it's the same struct_ops struct
definition and the same function pointer field
This allows reusing same BPF program for versioned struct_ops map
definitions, e.g.:
SEC("struct_ops/test")
int BPF_PROG(foo) { ... }
struct some_ops___v1 { int (*test)(void); };
struct some_ops___v2 { int (*test)(void); };
SEC(".struct_ops.link") struct some_ops___v1 a = { .test = foo }
SEC(".struct_ops.link") struct some_ops___v2 b = { .test = foo }
Signed-off-by: Eduard Zingerman <eddyz87@gmail.com>
---
tools/lib/bpf/libbpf.c | 49 ++++++++++++++++++++++--------------------
1 file changed, 26 insertions(+), 23 deletions(-)
diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
index e2a4c409980b..2c0cb72bc7a4 100644
--- a/tools/lib/bpf/libbpf.c
+++ b/tools/lib/bpf/libbpf.c
@@ -1146,8 +1146,32 @@ static int bpf_map__init_kern_struct_ops(struct bpf_map *map)
if (mod_btf)
prog->attach_btf_obj_fd = mod_btf->fd;
- prog->attach_btf_id = kern_type_id;
- prog->expected_attach_type = kern_member_idx;
+
+ /* if we haven't yet processed this BPF program, record proper
+ * attach_btf_id and member_idx
+ */
+ if (!prog->attach_btf_id) {
+ prog->attach_btf_id = kern_type_id;
+ prog->expected_attach_type = kern_member_idx;
+ }
+
+ /* struct_ops BPF prog can be re-used between multiple
+ * .struct_ops & .struct_ops.link as long as it's the
+ * same struct_ops struct definition and the same
+ * function pointer field
+ */
+ if (prog->attach_btf_id != kern_type_id) {
+ pr_warn("struct_ops init_kern %s func ptr %s: invalid reuse of prog %s in sec %s with type %u: attach_btf_id %u != kern_type_id %u\n",
+ map->name, mname, prog->name, prog->sec_name, prog->type,
+ prog->attach_btf_id, kern_type_id);
+ return -EINVAL;
+ }
+ if (prog->expected_attach_type != kern_member_idx) {
+ pr_warn("struct_ops init_kern %s func ptr %s: invalid reuse of prog %s in sec %s with type %u: expected_attach_type %u != kern_member_idx %u\n",
+ map->name, mname, prog->name, prog->sec_name, prog->type,
+ prog->expected_attach_type, kern_member_idx);
+ return -EINVAL;
+ }
st_ops->kern_func_off[i] = kern_data_off + kern_moff;
@@ -9428,27 +9452,6 @@ static int bpf_object__collect_st_ops_relos(struct bpf_object *obj,
return -EINVAL;
}
- /* if we haven't yet processed this BPF program, record proper
- * attach_btf_id and member_idx
- */
- if (!prog->attach_btf_id) {
- prog->attach_btf_id = st_ops->type_id;
- prog->expected_attach_type = member_idx;
- }
-
- /* struct_ops BPF prog can be re-used between multiple
- * .struct_ops & .struct_ops.link as long as it's the
- * same struct_ops struct definition and the same
- * function pointer field
- */
- if (prog->attach_btf_id != st_ops->type_id ||
- prog->expected_attach_type != member_idx) {
- pr_warn("struct_ops reloc %s: cannot use prog %s in sec %s with type %u attach_btf_id %u expected_attach_type %u for func ptr %s\n",
- map->name, prog->name, prog->sec_name, prog->type,
- prog->attach_btf_id, prog->expected_attach_type, name);
- return -EINVAL;
- }
-
st_ops->progs[member_idx] = prog;
/* st_ops->data will be exposed to users, being returned by
--
2.43.0
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH bpf-next v2 03/15] libbpf: honor autocreate flag for struct_ops maps
2024-03-02 1:19 [PATCH bpf-next v2 00/15] libbpf: type suffixes and autocreate flag for struct_ops maps Eduard Zingerman
2024-03-02 1:19 ` [PATCH bpf-next v2 01/15] libbpf: allow version suffixes (___smth) for struct_ops types Eduard Zingerman
2024-03-02 1:19 ` [PATCH bpf-next v2 02/15] libbpf: tie struct_ops programs to kernel BTF ids, not to local ids Eduard Zingerman
@ 2024-03-02 1:19 ` Eduard Zingerman
2024-03-02 1:19 ` [PATCH bpf-next v2 04/15] selftests/bpf: test struct_ops map definition with type suffix Eduard Zingerman
` (11 subsequent siblings)
14 siblings, 0 replies; 17+ messages in thread
From: Eduard Zingerman @ 2024-03-02 1:19 UTC (permalink / raw)
To: bpf, ast
Cc: andrii, daniel, martin.lau, kernel-team, yonghong.song, void,
sinquersw, Eduard Zingerman
Skip load steps for struct_ops maps not marked for automatic creation.
This should allow to load bpf object in situations like below:
SEC("struct_ops/foo") int BPF_PROG(foo) { ... }
SEC("struct_ops/bar") int BPF_PROG(bar) { ... }
struct test_ops___v1 {
int (*foo)(void);
};
struct test_ops___v2 {
int (*foo)(void);
int (*does_not_exist)(void);
};
SEC(".struct_ops.link")
struct test_ops___v1 map_for_old = {
.test_1 = (void *)foo
};
SEC(".struct_ops.link")
struct test_ops___v2 map_for_new = {
.test_1 = (void *)foo,
.does_not_exist = (void *)bar
};
Suppose program is loaded on old kernel that does not have definition
for 'does_not_exist' struct_ops member. After this commit it would be
possible to load such object file after the following tweaks:
bpf_program__set_autoload(skel->progs.bar, false);
bpf_map__set_autocreate(skel->maps.map_for_new, false);
Acked-by: David Vernet <void@manifault.com>
Signed-off-by: Eduard Zingerman <eddyz87@gmail.com>
---
tools/lib/bpf/libbpf.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
index 2c0cb72bc7a4..25c452c20d7d 100644
--- a/tools/lib/bpf/libbpf.c
+++ b/tools/lib/bpf/libbpf.c
@@ -1209,7 +1209,7 @@ static int bpf_object__init_kern_struct_ops_maps(struct bpf_object *obj)
for (i = 0; i < obj->nr_maps; i++) {
map = &obj->maps[i];
- if (!bpf_map__is_struct_ops(map))
+ if (!bpf_map__is_struct_ops(map) || !map->autocreate)
continue;
err = bpf_map__init_kern_struct_ops(map);
@@ -8136,7 +8136,7 @@ static int bpf_object_prepare_struct_ops(struct bpf_object *obj)
int i;
for (i = 0; i < obj->nr_maps; i++)
- if (bpf_map__is_struct_ops(&obj->maps[i]))
+ if (bpf_map__is_struct_ops(&obj->maps[i]) && obj->maps[i].autocreate)
bpf_map_prepare_vdata(&obj->maps[i]);
return 0;
--
2.43.0
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH bpf-next v2 04/15] selftests/bpf: test struct_ops map definition with type suffix
2024-03-02 1:19 [PATCH bpf-next v2 00/15] libbpf: type suffixes and autocreate flag for struct_ops maps Eduard Zingerman
` (2 preceding siblings ...)
2024-03-02 1:19 ` [PATCH bpf-next v2 03/15] libbpf: honor autocreate flag for struct_ops maps Eduard Zingerman
@ 2024-03-02 1:19 ` Eduard Zingerman
2024-03-02 1:19 ` [PATCH bpf-next v2 05/15] selftests/bpf: utility functions to capture libbpf log in test_progs Eduard Zingerman
` (10 subsequent siblings)
14 siblings, 0 replies; 17+ messages in thread
From: Eduard Zingerman @ 2024-03-02 1:19 UTC (permalink / raw)
To: bpf, ast
Cc: andrii, daniel, martin.lau, kernel-team, yonghong.song, void,
sinquersw, Eduard Zingerman
Extend struct_ops_module test case to check if it is possible to use
'___' suffixes for struct_ops type specification.
Acked-by: David Vernet <void@manifault.com>
Signed-off-by: Eduard Zingerman <eddyz87@gmail.com>
---
.../selftests/bpf/bpf_testmod/bpf_testmod.c | 1 +
.../bpf/prog_tests/test_struct_ops_module.c | 33 ++++++++++++++-----
.../selftests/bpf/progs/struct_ops_module.c | 21 +++++++++++-
3 files changed, 45 insertions(+), 10 deletions(-)
diff --git a/tools/testing/selftests/bpf/bpf_testmod/bpf_testmod.c b/tools/testing/selftests/bpf/bpf_testmod/bpf_testmod.c
index 098ddd067224..b9ff88e3d463 100644
--- a/tools/testing/selftests/bpf/bpf_testmod/bpf_testmod.c
+++ b/tools/testing/selftests/bpf/bpf_testmod/bpf_testmod.c
@@ -564,6 +564,7 @@ static int bpf_dummy_reg(void *kdata)
{
struct bpf_testmod_ops *ops = kdata;
+ ops->test_1();
/* Some test cases (ex. struct_ops_maybe_null) may not have test_2
* initialized, so we need to check for NULL.
*/
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 7d6facf46ebb..ee5372c7f2c7 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
@@ -30,11 +30,29 @@ static void check_map_info(struct bpf_map_info *info)
close(fd);
}
+static int attach_ops_and_check(struct struct_ops_module *skel,
+ struct bpf_map *map,
+ int expected_test_2_result)
+{
+ struct bpf_link *link;
+
+ link = bpf_map__attach_struct_ops(map);
+ ASSERT_OK_PTR(link, "attach_test_mod_1");
+ if (!link)
+ return -1;
+
+ /* test_{1,2}() would be called from bpf_dummy_reg() in bpf_testmod.c */
+ ASSERT_EQ(skel->bss->test_1_result, 0xdeadbeef, "test_1_result");
+ ASSERT_EQ(skel->bss->test_2_result, expected_test_2_result, "test_2_result");
+
+ bpf_link__destroy(link);
+ return 0;
+}
+
static void test_struct_ops_load(void)
{
struct struct_ops_module *skel;
struct bpf_map_info info = {};
- struct bpf_link *link;
int err;
u32 len;
@@ -59,20 +77,17 @@ static void test_struct_ops_load(void)
if (!ASSERT_OK(err, "bpf_map_get_info_by_fd"))
goto cleanup;
- link = bpf_map__attach_struct_ops(skel->maps.testmod_1);
- ASSERT_OK_PTR(link, "attach_test_mod_1");
-
+ check_map_info(&info);
/* test_3() will be called from bpf_dummy_reg() in bpf_testmod.c
*
* In bpf_testmod.c it will pass 4 and 13 (the value of data) to
* .test_2. So, the value of test_2_result should be 20 (4 + 13 +
* 3).
*/
- ASSERT_EQ(skel->bss->test_2_result, 20, "check_shadow_variables");
-
- bpf_link__destroy(link);
-
- check_map_info(&info);
+ if (!attach_ops_and_check(skel, skel->maps.testmod_1, 20))
+ goto cleanup;
+ if (!attach_ops_and_check(skel, skel->maps.testmod_2, 12))
+ goto cleanup;
cleanup:
struct_ops_module__destroy(skel);
diff --git a/tools/testing/selftests/bpf/progs/struct_ops_module.c b/tools/testing/selftests/bpf/progs/struct_ops_module.c
index 25952fa09348..026cabfa7f1f 100644
--- a/tools/testing/selftests/bpf/progs/struct_ops_module.c
+++ b/tools/testing/selftests/bpf/progs/struct_ops_module.c
@@ -7,12 +7,14 @@
char _license[] SEC("license") = "GPL";
+int test_1_result = 0;
int test_2_result = 0;
SEC("struct_ops/test_1")
int BPF_PROG(test_1)
{
- return 0xdeadbeef;
+ test_1_result = 0xdeadbeef;
+ return 0;
}
SEC("struct_ops/test_2")
@@ -35,3 +37,20 @@ struct bpf_testmod_ops testmod_1 = {
.data = 0x1,
};
+SEC("struct_ops/test_2")
+void BPF_PROG(test_2_v2, int a, int b)
+{
+ test_2_result = a * b;
+}
+
+struct bpf_testmod_ops___v2 {
+ int (*test_1)(void);
+ void (*test_2)(int a, int b);
+ int (*test_maybe_null)(int dummy, struct task_struct *task);
+};
+
+SEC(".struct_ops.link")
+struct bpf_testmod_ops___v2 testmod_2 = {
+ .test_1 = (void *)test_1,
+ .test_2 = (void *)test_2_v2,
+};
--
2.43.0
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH bpf-next v2 05/15] selftests/bpf: utility functions to capture libbpf log in test_progs
2024-03-02 1:19 [PATCH bpf-next v2 00/15] libbpf: type suffixes and autocreate flag for struct_ops maps Eduard Zingerman
` (3 preceding siblings ...)
2024-03-02 1:19 ` [PATCH bpf-next v2 04/15] selftests/bpf: test struct_ops map definition with type suffix Eduard Zingerman
@ 2024-03-02 1:19 ` Eduard Zingerman
2024-03-02 1:19 ` [PATCH bpf-next v2 06/15] selftests/bpf: bad_struct_ops test Eduard Zingerman
` (9 subsequent siblings)
14 siblings, 0 replies; 17+ messages in thread
From: Eduard Zingerman @ 2024-03-02 1:19 UTC (permalink / raw)
To: bpf, ast
Cc: andrii, daniel, martin.lau, kernel-team, yonghong.song, void,
sinquersw, Eduard Zingerman
Several test_progs tests already capture libbpf log in order to check
for some expected output, e.g bpf_tcp_ca.c, kfunc_dynptr_param.c,
log_buf.c and a few others.
This commit provides a, hopefully, simple API to capture libbpf log
w/o necessity to define new print callback in each test:
/* Creates a global memstream capturing all output passed to
* libbpf_print_fn.
* Returns 0 on success, negative value on failure.
* On failure the description is printed using PRINT_FAIL and
* current test case is marked as fail.
*/
int start_libbpf_log_capture(void)
/* Destroys global memstream created by start_libbpf_log_capture().
* Returns a pointer to captured data which has to be freed.
* Returned buffer is null terminated.
*/
char *stop_libbpf_log_capture(void)
The intended usage is as follows:
if (start_libbpf_log_capture())
return;
use_libbpf();
char *log = stop_libbpf_log_capture();
ASSERT_HAS_SUBSTR(log, "... expected ...", "expected some message");
free(log);
As a safety measure, free(start_libbpf_log_capture()) is invoked in the
epilogue of the test_progs.c:run_one_test().
Signed-off-by: Eduard Zingerman <eddyz87@gmail.com>
---
tools/testing/selftests/bpf/test_progs.c | 57 ++++++++++++++++++++++++
tools/testing/selftests/bpf/test_progs.h | 3 ++
2 files changed, 60 insertions(+)
diff --git a/tools/testing/selftests/bpf/test_progs.c b/tools/testing/selftests/bpf/test_progs.c
index 808550986f30..698c7387b310 100644
--- a/tools/testing/selftests/bpf/test_progs.c
+++ b/tools/testing/selftests/bpf/test_progs.c
@@ -683,9 +683,65 @@ static const struct argp_option opts[] = {
{},
};
+static FILE *libbpf_capture_stream;
+
+static struct {
+ char *buf;
+ size_t buf_sz;
+} libbpf_output_capture;
+
+/* Creates a global memstream capturing all output passed to libbpf_print_fn.
+ * Returns 0 on success, negative value on failure.
+ * On failure the description is printed using PRINT_FAIL and
+ * current test case is marked as fail.
+ */
+int start_libbpf_log_capture(void)
+{
+ if (libbpf_capture_stream) {
+ PRINT_FAIL("%s: libbpf_capture_stream != NULL\n", __func__);
+ return -EINVAL;
+ }
+
+ libbpf_capture_stream = open_memstream(&libbpf_output_capture.buf,
+ &libbpf_output_capture.buf_sz);
+ if (!libbpf_capture_stream) {
+ PRINT_FAIL("%s: open_memstream failed errno=%d\n", __func__, errno);
+ return -EINVAL;
+ }
+
+ return 0;
+}
+
+/* Destroys global memstream created by start_libbpf_log_capture().
+ * Returns a pointer to captured data which has to be freed.
+ * Returned buffer is null terminated.
+ */
+char *stop_libbpf_log_capture(void)
+{
+ char *buf;
+
+ if (!libbpf_capture_stream)
+ return NULL;
+
+ fputc(0, libbpf_capture_stream);
+ fclose(libbpf_capture_stream);
+ libbpf_capture_stream = NULL;
+ /* get 'buf' after fclose(), see open_memstream() documentation */
+ buf = libbpf_output_capture.buf;
+ bzero(&libbpf_output_capture, sizeof(libbpf_output_capture));
+ return buf;
+}
+
static int libbpf_print_fn(enum libbpf_print_level level,
const char *format, va_list args)
{
+ if (libbpf_capture_stream) {
+ va_list args2;
+
+ va_copy(args2, args);
+ vfprintf(libbpf_capture_stream, format, args2);
+ }
+
if (env.verbosity < VERBOSE_VERY && level == LIBBPF_DEBUG)
return 0;
vfprintf(stdout, format, args);
@@ -1081,6 +1137,7 @@ static void run_one_test(int test_num)
cleanup_cgroup_environment();
stdio_restore();
+ free(stop_libbpf_log_capture());
dump_test_log(test, state, false, false, NULL);
}
diff --git a/tools/testing/selftests/bpf/test_progs.h b/tools/testing/selftests/bpf/test_progs.h
index 80df51244886..0ba5a20b19ba 100644
--- a/tools/testing/selftests/bpf/test_progs.h
+++ b/tools/testing/selftests/bpf/test_progs.h
@@ -397,6 +397,9 @@ int test__join_cgroup(const char *path);
system(cmd); \
})
+int start_libbpf_log_capture(void);
+char *stop_libbpf_log_capture(void);
+
static inline __u64 ptr_to_u64(const void *ptr)
{
return (__u64) (unsigned long) ptr;
--
2.43.0
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH bpf-next v2 06/15] selftests/bpf: bad_struct_ops test
2024-03-02 1:19 [PATCH bpf-next v2 00/15] libbpf: type suffixes and autocreate flag for struct_ops maps Eduard Zingerman
` (4 preceding siblings ...)
2024-03-02 1:19 ` [PATCH bpf-next v2 05/15] selftests/bpf: utility functions to capture libbpf log in test_progs Eduard Zingerman
@ 2024-03-02 1:19 ` Eduard Zingerman
2024-03-02 1:19 ` [PATCH bpf-next v2 07/15] selftests/bpf: test autocreate behavior for struct_ops maps Eduard Zingerman
` (8 subsequent siblings)
14 siblings, 0 replies; 17+ messages in thread
From: Eduard Zingerman @ 2024-03-02 1:19 UTC (permalink / raw)
To: bpf, ast
Cc: andrii, daniel, martin.lau, kernel-team, yonghong.song, void,
sinquersw, Eduard Zingerman
When loading struct_ops programs kernel requires BTF id of the
struct_ops type and member index for attachment point inside that
type. This makes it not possible to have same BPF program used in
struct_ops maps that have different struct_ops type.
Check if libbpf rejects such BPF objects files.
Signed-off-by: Eduard Zingerman <eddyz87@gmail.com>
---
.../selftests/bpf/bpf_testmod/bpf_testmod.c | 24 +++++++++++++
.../selftests/bpf/bpf_testmod/bpf_testmod.h | 4 +++
.../selftests/bpf/prog_tests/bad_struct_ops.c | 35 +++++++++++++++++++
.../selftests/bpf/progs/bad_struct_ops.c | 25 +++++++++++++
4 files changed, 88 insertions(+)
create mode 100644 tools/testing/selftests/bpf/prog_tests/bad_struct_ops.c
create mode 100644 tools/testing/selftests/bpf/progs/bad_struct_ops.c
diff --git a/tools/testing/selftests/bpf/bpf_testmod/bpf_testmod.c b/tools/testing/selftests/bpf/bpf_testmod/bpf_testmod.c
index b9ff88e3d463..2de7e80dbb4b 100644
--- a/tools/testing/selftests/bpf/bpf_testmod/bpf_testmod.c
+++ b/tools/testing/selftests/bpf/bpf_testmod/bpf_testmod.c
@@ -610,6 +610,29 @@ struct bpf_struct_ops bpf_bpf_testmod_ops = {
.owner = THIS_MODULE,
};
+static int bpf_dummy_reg2(void *kdata)
+{
+ struct bpf_testmod_ops2 *ops = kdata;
+
+ ops->test_1();
+ return 0;
+}
+
+static struct bpf_testmod_ops2 __bpf_testmod_ops2 = {
+ .test_1 = bpf_testmod_test_1,
+};
+
+struct bpf_struct_ops bpf_testmod_ops2 = {
+ .verifier_ops = &bpf_testmod_verifier_ops,
+ .init = bpf_testmod_ops_init,
+ .init_member = bpf_testmod_ops_init_member,
+ .reg = bpf_dummy_reg2,
+ .unreg = bpf_dummy_unreg,
+ .cfi_stubs = &__bpf_testmod_ops2,
+ .name = "bpf_testmod_ops2",
+ .owner = THIS_MODULE,
+};
+
extern int bpf_fentry_test1(int a);
static int bpf_testmod_init(void)
@@ -621,6 +644,7 @@ static int bpf_testmod_init(void)
ret = ret ?: register_btf_kfunc_id_set(BPF_PROG_TYPE_TRACING, &bpf_testmod_kfunc_set);
ret = ret ?: register_btf_kfunc_id_set(BPF_PROG_TYPE_SYSCALL, &bpf_testmod_kfunc_set);
ret = ret ?: register_bpf_struct_ops(&bpf_bpf_testmod_ops, bpf_testmod_ops);
+ ret = ret ?: register_bpf_struct_ops(&bpf_testmod_ops2, bpf_testmod_ops2);
if (ret < 0)
return ret;
if (bpf_fentry_test1(0) < 0)
diff --git a/tools/testing/selftests/bpf/bpf_testmod/bpf_testmod.h b/tools/testing/selftests/bpf/bpf_testmod/bpf_testmod.h
index 971458acfac3..c51c4eae9ed5 100644
--- a/tools/testing/selftests/bpf/bpf_testmod/bpf_testmod.h
+++ b/tools/testing/selftests/bpf/bpf_testmod/bpf_testmod.h
@@ -45,4 +45,8 @@ struct bpf_testmod_ops {
int data;
};
+struct bpf_testmod_ops2 {
+ int (*test_1)(void);
+};
+
#endif /* _BPF_TESTMOD_H */
diff --git a/tools/testing/selftests/bpf/prog_tests/bad_struct_ops.c b/tools/testing/selftests/bpf/prog_tests/bad_struct_ops.c
new file mode 100644
index 000000000000..9f5dbefa0dd9
--- /dev/null
+++ b/tools/testing/selftests/bpf/prog_tests/bad_struct_ops.c
@@ -0,0 +1,35 @@
+// SPDX-License-Identifier: GPL-2.0
+
+#include <test_progs.h>
+#include "bad_struct_ops.skel.h"
+
+static void invalid_prog_reuse(void)
+{
+ struct bad_struct_ops *skel;
+ char *log = NULL;
+ int err;
+
+ skel = bad_struct_ops__open();
+ if (!ASSERT_OK_PTR(skel, "bad_struct_ops__open"))
+ return;
+
+ if (start_libbpf_log_capture())
+ goto cleanup;
+
+ err = bad_struct_ops__load(skel);
+ log = stop_libbpf_log_capture();
+ ASSERT_ERR(err, "bad_struct_ops__load should fail");
+ ASSERT_HAS_SUBSTR(log,
+ "struct_ops init_kern testmod_2 func ptr test_1: invalid reuse of prog test_1",
+ "expected init_kern message");
+
+cleanup:
+ free(log);
+ bad_struct_ops__destroy(skel);
+}
+
+void test_bad_struct_ops(void)
+{
+ if (test__start_subtest("invalid_prog_reuse"))
+ invalid_prog_reuse();
+}
diff --git a/tools/testing/selftests/bpf/progs/bad_struct_ops.c b/tools/testing/selftests/bpf/progs/bad_struct_ops.c
new file mode 100644
index 000000000000..b7e175cd0af0
--- /dev/null
+++ b/tools/testing/selftests/bpf/progs/bad_struct_ops.c
@@ -0,0 +1,25 @@
+// SPDX-License-Identifier: GPL-2.0
+
+#include <vmlinux.h>
+#include <bpf/bpf_helpers.h>
+#include <bpf/bpf_tracing.h>
+#include "../bpf_testmod/bpf_testmod.h"
+
+char _license[] SEC("license") = "GPL";
+
+SEC("struct_ops/test_1")
+int BPF_PROG(test_1) { return 0; }
+
+SEC("struct_ops/test_2")
+int BPF_PROG(test_2) { return 0; }
+
+SEC(".struct_ops.link")
+struct bpf_testmod_ops testmod_1 = {
+ .test_1 = (void *)test_1,
+ .test_2 = (void *)test_2
+};
+
+SEC(".struct_ops.link")
+struct bpf_testmod_ops2 testmod_2 = {
+ .test_1 = (void *)test_1
+};
--
2.43.0
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH bpf-next v2 07/15] selftests/bpf: test autocreate behavior for struct_ops maps
2024-03-02 1:19 [PATCH bpf-next v2 00/15] libbpf: type suffixes and autocreate flag for struct_ops maps Eduard Zingerman
` (5 preceding siblings ...)
2024-03-02 1:19 ` [PATCH bpf-next v2 06/15] selftests/bpf: bad_struct_ops test Eduard Zingerman
@ 2024-03-02 1:19 ` Eduard Zingerman
2024-03-02 1:19 ` [PATCH bpf-next v2 08/15] libbpf: sync progs autoload with maps autocreate " Eduard Zingerman
` (7 subsequent siblings)
14 siblings, 0 replies; 17+ messages in thread
From: Eduard Zingerman @ 2024-03-02 1:19 UTC (permalink / raw)
To: bpf, ast
Cc: andrii, daniel, martin.lau, kernel-team, yonghong.song, void,
sinquersw, Eduard Zingerman
Check that bpf_map__set_autocreate() can be used to disable automatic
creation for struct_ops maps.
Signed-off-by: Eduard Zingerman <eddyz87@gmail.com>
---
.../bpf/prog_tests/struct_ops_autocreate.c | 77 +++++++++++++++++++
.../bpf/progs/struct_ops_autocreate.c | 42 ++++++++++
2 files changed, 119 insertions(+)
create mode 100644 tools/testing/selftests/bpf/prog_tests/struct_ops_autocreate.c
create mode 100644 tools/testing/selftests/bpf/progs/struct_ops_autocreate.c
diff --git a/tools/testing/selftests/bpf/prog_tests/struct_ops_autocreate.c b/tools/testing/selftests/bpf/prog_tests/struct_ops_autocreate.c
new file mode 100644
index 000000000000..c67d0b32b9dc
--- /dev/null
+++ b/tools/testing/selftests/bpf/prog_tests/struct_ops_autocreate.c
@@ -0,0 +1,77 @@
+// SPDX-License-Identifier: GPL-2.0
+
+#include <test_progs.h>
+#include "struct_ops_autocreate.skel.h"
+
+static void cant_load_full_object(void)
+{
+ struct struct_ops_autocreate *skel;
+ char *log;
+ int err;
+
+ skel = struct_ops_autocreate__open();
+ if (!ASSERT_OK_PTR(skel, "struct_ops_autocreate__open"))
+ return;
+
+ if (start_libbpf_log_capture())
+ goto cleanup;
+ /* The testmod_2 map BTF type (struct bpf_testmod_ops___v2) doesn't
+ * match the BTF of the actual struct bpf_testmod_ops defined in the
+ * kernel, so we should fail to load it if we don't disable autocreate
+ * for that map.
+ */
+ err = struct_ops_autocreate__load(skel);
+ log = stop_libbpf_log_capture();
+ if (!ASSERT_ERR(err, "struct_ops_autocreate__load"))
+ goto cleanup;
+
+ ASSERT_HAS_SUBSTR(log, "libbpf: struct_ops init_kern", "init_kern message");
+ ASSERT_EQ(err, -ENOTSUP, "errno should be ENOTSUP");
+
+cleanup:
+ free(log);
+ struct_ops_autocreate__destroy(skel);
+}
+
+static void can_load_partial_object(void)
+{
+ LIBBPF_OPTS(bpf_object_open_opts, opts);
+ struct struct_ops_autocreate *skel;
+ struct bpf_link *link = NULL;
+ int err;
+
+ skel = struct_ops_autocreate__open_opts(&opts);
+ if (!ASSERT_OK_PTR(skel, "struct_ops_autocreate__open_opts"))
+ return;
+
+ err = bpf_program__set_autoload(skel->progs.test_2, false);
+ if (!ASSERT_OK(err, "bpf_program__set_autoload"))
+ goto cleanup;
+
+ err = bpf_map__set_autocreate(skel->maps.testmod_2, false);
+ if (!ASSERT_OK(err, "bpf_map__set_autocreate"))
+ goto cleanup;
+
+ err = struct_ops_autocreate__load(skel);
+ if (ASSERT_OK(err, "struct_ops_autocreate__load"))
+ goto cleanup;
+
+ link = bpf_map__attach_struct_ops(skel->maps.testmod_1);
+ if (!ASSERT_OK_PTR(link, "bpf_map__attach_struct_ops"))
+ goto cleanup;
+
+ /* test_1() would be called from bpf_dummy_reg2() in bpf_testmod.c */
+ ASSERT_EQ(skel->bss->test_1_result, 42, "test_1_result");
+
+cleanup:
+ bpf_link__destroy(link);
+ struct_ops_autocreate__destroy(skel);
+}
+
+void test_struct_ops_autocreate(void)
+{
+ if (test__start_subtest("cant_load_full_object"))
+ cant_load_full_object();
+ if (test__start_subtest("can_load_partial_object"))
+ can_load_partial_object();
+}
diff --git a/tools/testing/selftests/bpf/progs/struct_ops_autocreate.c b/tools/testing/selftests/bpf/progs/struct_ops_autocreate.c
new file mode 100644
index 000000000000..294d48bb8e3c
--- /dev/null
+++ b/tools/testing/selftests/bpf/progs/struct_ops_autocreate.c
@@ -0,0 +1,42 @@
+// SPDX-License-Identifier: GPL-2.0
+
+#include <vmlinux.h>
+#include <bpf/bpf_helpers.h>
+#include <bpf/bpf_tracing.h>
+
+char _license[] SEC("license") = "GPL";
+
+int test_1_result = 0;
+
+SEC("struct_ops/test_1")
+int BPF_PROG(test_1)
+{
+ test_1_result = 42;
+ return 0;
+}
+
+SEC("struct_ops/test_1")
+int BPF_PROG(test_2)
+{
+ return 0;
+}
+
+struct bpf_testmod_ops___v1 {
+ int (*test_1)(void);
+};
+
+struct bpf_testmod_ops___v2 {
+ int (*test_1)(void);
+ int (*does_not_exist)(void);
+};
+
+SEC(".struct_ops.link")
+struct bpf_testmod_ops___v1 testmod_1 = {
+ .test_1 = (void *)test_1
+};
+
+SEC(".struct_ops.link")
+struct bpf_testmod_ops___v2 testmod_2 = {
+ .test_1 = (void *)test_1,
+ .does_not_exist = (void *)test_2
+};
--
2.43.0
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH bpf-next v2 08/15] libbpf: sync progs autoload with maps autocreate for struct_ops maps
2024-03-02 1:19 [PATCH bpf-next v2 00/15] libbpf: type suffixes and autocreate flag for struct_ops maps Eduard Zingerman
` (6 preceding siblings ...)
2024-03-02 1:19 ` [PATCH bpf-next v2 07/15] selftests/bpf: test autocreate behavior for struct_ops maps Eduard Zingerman
@ 2024-03-02 1:19 ` Eduard Zingerman
2024-03-04 19:13 ` Eduard Zingerman
2024-03-02 1:19 ` [PATCH bpf-next v2 09/15] selftests/bpf: verify struct_ops autoload/autocreate sync Eduard Zingerman
` (6 subsequent siblings)
14 siblings, 1 reply; 17+ messages in thread
From: Eduard Zingerman @ 2024-03-02 1:19 UTC (permalink / raw)
To: bpf, ast
Cc: andrii, daniel, martin.lau, kernel-team, yonghong.song, void,
sinquersw, Eduard Zingerman
Automatically select which struct_ops programs to load depending on
which struct_ops maps are selected for automatic creation.
E.g. for the BPF code below:
SEC("struct_ops/test_1") int BPF_PROG(foo) { ... }
SEC("struct_ops/test_2") int BPF_PROG(bar) { ... }
SEC(".struct_ops.link")
struct test_ops___v1 A = {
.foo = (void *)foo
};
SEC(".struct_ops.link")
struct test_ops___v2 B = {
.foo = (void *)foo,
.bar = (void *)bar,
};
And the following libbpf API calls:
bpf_map__set_autocreate(skel->maps.A, true);
bpf_map__set_autocreate(skel->maps.B, false);
The autoload would be enabled for program 'foo' and disabled for
program 'bar'.
To achieve this:
- for struct_ops programs referenced from struct_ops maps set autoload
property at open() to false;
- when creating struct_ops maps set autoload property of referenced
programs to true.
(Note: struct_ops programs not referenced from any map would have
their autoload property set to true by default.
If attach_btf_id and expected_attach_type properties would not be
specified for such programs manually, the load phase would fail).
Signed-off-by: Eduard Zingerman <eddyz87@gmail.com>
---
tools/lib/bpf/libbpf.c | 12 ++++++++++++
1 file changed, 12 insertions(+)
diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
index 25c452c20d7d..60d78badfc71 100644
--- a/tools/lib/bpf/libbpf.c
+++ b/tools/lib/bpf/libbpf.c
@@ -1151,6 +1151,7 @@ static int bpf_map__init_kern_struct_ops(struct bpf_map *map)
* attach_btf_id and member_idx
*/
if (!prog->attach_btf_id) {
+ prog->autoload = true;
prog->attach_btf_id = kern_type_id;
prog->expected_attach_type = kern_member_idx;
}
@@ -3187,6 +3188,11 @@ static bool obj_needs_vmlinux_btf(const struct bpf_object *obj)
}
bpf_object__for_each_program(prog, obj) {
+ /* Note: struct_ops programs referenced from struct_ops maps
+ * would have their autoload reset to false after open(),
+ * but that is fine as corresponding map would trigger
+ * "needs_vmlinux_btf" anyways.
+ */
if (!prog->autoload)
continue;
if (prog_needs_vmlinux_btf(prog))
@@ -9452,6 +9458,12 @@ static int bpf_object__collect_st_ops_relos(struct bpf_object *obj,
return -EINVAL;
}
+ /* struct_ops programs autoload is computed depending
+ * on autocreate property of corresponding maps,
+ * see bpf_map__init_kern_struct_ops().
+ */
+ prog->autoload = false;
+
st_ops->progs[member_idx] = prog;
/* st_ops->data will be exposed to users, being returned by
--
2.43.0
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH bpf-next v2 09/15] selftests/bpf: verify struct_ops autoload/autocreate sync
2024-03-02 1:19 [PATCH bpf-next v2 00/15] libbpf: type suffixes and autocreate flag for struct_ops maps Eduard Zingerman
` (7 preceding siblings ...)
2024-03-02 1:19 ` [PATCH bpf-next v2 08/15] libbpf: sync progs autoload with maps autocreate " Eduard Zingerman
@ 2024-03-02 1:19 ` Eduard Zingerman
2024-03-02 1:19 ` [PATCH bpf-next v2 10/15] libbpf: replace elf_state->st_ops_* fields with SEC_ST_OPS sec_type Eduard Zingerman
` (5 subsequent siblings)
14 siblings, 0 replies; 17+ messages in thread
From: Eduard Zingerman @ 2024-03-02 1:19 UTC (permalink / raw)
To: bpf, ast
Cc: andrii, daniel, martin.lau, kernel-team, yonghong.song, void,
sinquersw, Eduard Zingerman
Check that autocreate flag set to false for struct_ops map causes
autoload flag set to false for corresponding program.
Check that struct_ops program not referenced from any map fails to load.
Signed-off-by: Eduard Zingerman <eddyz87@gmail.com>
---
.../selftests/bpf/prog_tests/bad_struct_ops.c | 32 +++++++++++++++++++
.../bpf/prog_tests/struct_ops_autocreate.c | 11 ++++---
.../selftests/bpf/progs/bad_struct_ops2.c | 14 ++++++++
3 files changed, 52 insertions(+), 5 deletions(-)
create mode 100644 tools/testing/selftests/bpf/progs/bad_struct_ops2.c
diff --git a/tools/testing/selftests/bpf/prog_tests/bad_struct_ops.c b/tools/testing/selftests/bpf/prog_tests/bad_struct_ops.c
index 9f5dbefa0dd9..6a707213e46b 100644
--- a/tools/testing/selftests/bpf/prog_tests/bad_struct_ops.c
+++ b/tools/testing/selftests/bpf/prog_tests/bad_struct_ops.c
@@ -2,6 +2,7 @@
#include <test_progs.h>
#include "bad_struct_ops.skel.h"
+#include "bad_struct_ops2.skel.h"
static void invalid_prog_reuse(void)
{
@@ -28,8 +29,39 @@ static void invalid_prog_reuse(void)
bad_struct_ops__destroy(skel);
}
+static void unused_program(void)
+{
+ struct bad_struct_ops2 *skel;
+ char *log = NULL;
+ int err;
+
+ skel = bad_struct_ops2__open();
+ if (!ASSERT_OK_PTR(skel, "bad_struct_ops2__open"))
+ return;
+
+ /* struct_ops programs not referenced from any maps are open
+ * with autoload set to true.
+ */
+ ASSERT_TRUE(bpf_program__autoload(skel->progs.foo), "foo autoload == true");
+
+ if (start_libbpf_log_capture())
+ goto cleanup;
+
+ err = bad_struct_ops2__load(skel);
+ ASSERT_ERR(err, "bad_struct_ops2__load should fail");
+ log = stop_libbpf_log_capture();
+ ASSERT_HAS_SUBSTR(log, "prog 'foo': failed to load",
+ "message about 'foo' failing to load");
+
+cleanup:
+ free(log);
+ bad_struct_ops2__destroy(skel);
+}
+
void test_bad_struct_ops(void)
{
if (test__start_subtest("invalid_prog_reuse"))
invalid_prog_reuse();
+ if (test__start_subtest("unused_program"))
+ unused_program();
}
diff --git a/tools/testing/selftests/bpf/prog_tests/struct_ops_autocreate.c b/tools/testing/selftests/bpf/prog_tests/struct_ops_autocreate.c
index c67d0b32b9dc..765b0ec6383a 100644
--- a/tools/testing/selftests/bpf/prog_tests/struct_ops_autocreate.c
+++ b/tools/testing/selftests/bpf/prog_tests/struct_ops_autocreate.c
@@ -35,18 +35,19 @@ static void cant_load_full_object(void)
static void can_load_partial_object(void)
{
- LIBBPF_OPTS(bpf_object_open_opts, opts);
struct struct_ops_autocreate *skel;
struct bpf_link *link = NULL;
int err;
- skel = struct_ops_autocreate__open_opts(&opts);
+ skel = struct_ops_autocreate__open();
if (!ASSERT_OK_PTR(skel, "struct_ops_autocreate__open_opts"))
return;
- err = bpf_program__set_autoload(skel->progs.test_2, false);
- if (!ASSERT_OK(err, "bpf_program__set_autoload"))
- goto cleanup;
+ /* struct_ops programs referenced from maps are open with
+ * autoload set to false.
+ */
+ ASSERT_FALSE(bpf_program__autoload(skel->progs.test_1), "test_1 autoload == false");
+ ASSERT_FALSE(bpf_program__autoload(skel->progs.test_2), "test_2 autoload == false");
err = bpf_map__set_autocreate(skel->maps.testmod_2, false);
if (!ASSERT_OK(err, "bpf_map__set_autocreate"))
diff --git a/tools/testing/selftests/bpf/progs/bad_struct_ops2.c b/tools/testing/selftests/bpf/progs/bad_struct_ops2.c
new file mode 100644
index 000000000000..64a95f6be86d
--- /dev/null
+++ b/tools/testing/selftests/bpf/progs/bad_struct_ops2.c
@@ -0,0 +1,14 @@
+// SPDX-License-Identifier: GPL-2.0
+
+#include <linux/bpf.h>
+#include <bpf/bpf_helpers.h>
+
+char _license[] SEC("license") = "GPL";
+
+/* This is an unused struct_ops program, it lacks corresponding
+ * struct_ops map, which provides attachment information.
+ * W/o additional configuration attempt to load such
+ * BPF object file would fail.
+ */
+SEC("struct_ops/foo")
+void foo(void) {}
--
2.43.0
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH bpf-next v2 10/15] libbpf: replace elf_state->st_ops_* fields with SEC_ST_OPS sec_type
2024-03-02 1:19 [PATCH bpf-next v2 00/15] libbpf: type suffixes and autocreate flag for struct_ops maps Eduard Zingerman
` (8 preceding siblings ...)
2024-03-02 1:19 ` [PATCH bpf-next v2 09/15] selftests/bpf: verify struct_ops autoload/autocreate sync Eduard Zingerman
@ 2024-03-02 1:19 ` Eduard Zingerman
2024-03-02 1:19 ` [PATCH bpf-next v2 11/15] libbpf: struct_ops in SEC("?.struct_ops") and SEC("?.struct_ops.link") Eduard Zingerman
` (4 subsequent siblings)
14 siblings, 0 replies; 17+ messages in thread
From: Eduard Zingerman @ 2024-03-02 1:19 UTC (permalink / raw)
To: bpf, ast
Cc: andrii, daniel, martin.lau, kernel-team, yonghong.song, void,
sinquersw, Eduard Zingerman
The next patch would add two new section names for struct_ops maps.
To make working with multiple struct_ops sections more convenient:
- remove fields like elf_state->st_ops_{shndx,link_shndx};
- mark section descriptions hosting struct_ops as
elf_sec_desc->sec_type == SEC_ST_OPS;
After these changes struct_ops sections could be processed uniformly
by iterating bpf_object->efile.secs entries.
Signed-off-by: Eduard Zingerman <eddyz87@gmail.com>
---
tools/lib/bpf/libbpf.c | 62 ++++++++++++++++++++++--------------------
1 file changed, 33 insertions(+), 29 deletions(-)
diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
index 60d78badfc71..8ecfad091cb5 100644
--- a/tools/lib/bpf/libbpf.c
+++ b/tools/lib/bpf/libbpf.c
@@ -612,6 +612,7 @@ enum sec_type {
SEC_BSS,
SEC_DATA,
SEC_RODATA,
+ SEC_ST_OPS,
};
struct elf_sec_desc {
@@ -627,8 +628,6 @@ struct elf_state {
Elf *elf;
Elf64_Ehdr *ehdr;
Elf_Data *symbols;
- Elf_Data *st_ops_data;
- Elf_Data *st_ops_link_data;
size_t shstrndx; /* section index for section name strings */
size_t strtabidx;
struct elf_sec_desc *secs;
@@ -637,8 +636,7 @@ struct elf_state {
__u32 btf_maps_sec_btf_id;
int text_shndx;
int symbols_shndx;
- int st_ops_shndx;
- int st_ops_link_shndx;
+ bool has_st_ops;
};
struct usdt_manager;
@@ -1222,7 +1220,7 @@ static int bpf_object__init_kern_struct_ops_maps(struct bpf_object *obj)
}
static int init_struct_ops_maps(struct bpf_object *obj, const char *sec_name,
- int shndx, Elf_Data *data, __u32 map_flags)
+ int shndx, Elf_Data *data)
{
const struct btf_type *type, *datasec;
const struct btf_var_secinfo *vsi;
@@ -1284,7 +1282,8 @@ static int init_struct_ops_maps(struct bpf_object *obj, const char *sec_name,
map->def.key_size = sizeof(int);
map->def.value_size = type->size;
map->def.max_entries = 1;
- map->def.map_flags = map_flags;
+ map->def.map_flags = strcmp(sec_name, STRUCT_OPS_LINK_SEC) == 0
+ ? BPF_F_LINK : 0;
map->st_ops = calloc(1, sizeof(*map->st_ops));
if (!map->st_ops)
@@ -1319,15 +1318,25 @@ static int init_struct_ops_maps(struct bpf_object *obj, const char *sec_name,
static int bpf_object_init_struct_ops(struct bpf_object *obj)
{
- int err;
+ const char *sec_name;
+ int sec_idx, err;
- err = init_struct_ops_maps(obj, STRUCT_OPS_SEC, obj->efile.st_ops_shndx,
- obj->efile.st_ops_data, 0);
- err = err ?: init_struct_ops_maps(obj, STRUCT_OPS_LINK_SEC,
- obj->efile.st_ops_link_shndx,
- obj->efile.st_ops_link_data,
- BPF_F_LINK);
- return err;
+ for (sec_idx = 0; sec_idx < obj->efile.sec_cnt; ++sec_idx) {
+ struct elf_sec_desc *desc = &obj->efile.secs[sec_idx];
+
+ if (desc->sec_type != SEC_ST_OPS)
+ continue;
+
+ sec_name = elf_sec_name(obj, elf_sec_by_idx(obj, sec_idx));
+ if (!sec_name)
+ return -LIBBPF_ERRNO__FORMAT;
+
+ err = init_struct_ops_maps(obj, sec_name, sec_idx, desc->data);
+ if (err)
+ return err;
+ }
+
+ return 0;
}
static struct bpf_object *bpf_object__new(const char *path,
@@ -1365,8 +1374,6 @@ static struct bpf_object *bpf_object__new(const char *path,
obj->efile.obj_buf = obj_buf;
obj->efile.obj_buf_sz = obj_buf_sz;
obj->efile.btf_maps_shndx = -1;
- obj->efile.st_ops_shndx = -1;
- obj->efile.st_ops_link_shndx = -1;
obj->kconfig_map_idx = -1;
obj->kern_version = get_kernel_version();
@@ -1383,8 +1390,6 @@ static void bpf_object__elf_finish(struct bpf_object *obj)
elf_end(obj->efile.elf);
obj->efile.elf = NULL;
obj->efile.symbols = NULL;
- obj->efile.st_ops_data = NULL;
- obj->efile.st_ops_link_data = NULL;
zfree(&obj->efile.secs);
obj->efile.sec_cnt = 0;
@@ -2929,14 +2934,13 @@ static int bpf_object__sanitize_btf(struct bpf_object *obj, struct btf *btf)
static bool libbpf_needs_btf(const struct bpf_object *obj)
{
return obj->efile.btf_maps_shndx >= 0 ||
- obj->efile.st_ops_shndx >= 0 ||
- obj->efile.st_ops_link_shndx >= 0 ||
+ obj->efile.has_st_ops ||
obj->nr_extern > 0;
}
static bool kernel_needs_btf(const struct bpf_object *obj)
{
- return obj->efile.st_ops_shndx >= 0 || obj->efile.st_ops_link_shndx >= 0;
+ return obj->efile.has_st_ops;
}
static int bpf_object__init_btf(struct bpf_object *obj,
@@ -3642,12 +3646,12 @@ static int bpf_object__elf_collect(struct bpf_object *obj)
sec_desc->sec_type = SEC_RODATA;
sec_desc->shdr = sh;
sec_desc->data = data;
- } else if (strcmp(name, STRUCT_OPS_SEC) == 0) {
- obj->efile.st_ops_data = data;
- obj->efile.st_ops_shndx = idx;
- } else if (strcmp(name, STRUCT_OPS_LINK_SEC) == 0) {
- obj->efile.st_ops_link_data = data;
- obj->efile.st_ops_link_shndx = idx;
+ } else if (strcmp(name, STRUCT_OPS_SEC) == 0 ||
+ strcmp(name, STRUCT_OPS_LINK_SEC) == 0) {
+ sec_desc->sec_type = SEC_ST_OPS;
+ sec_desc->shdr = sh;
+ sec_desc->data = data;
+ obj->efile.has_st_ops = true;
} else {
pr_info("elf: skipping unrecognized data section(%d) %s\n",
idx, name);
@@ -6960,12 +6964,12 @@ static int bpf_object__collect_relos(struct bpf_object *obj)
data = sec_desc->data;
idx = shdr->sh_info;
- if (shdr->sh_type != SHT_REL) {
+ if (shdr->sh_type != SHT_REL || idx < 0 || idx >= obj->efile.sec_cnt) {
pr_warn("internal error at %d\n", __LINE__);
return -LIBBPF_ERRNO__INTERNAL;
}
- if (idx == obj->efile.st_ops_shndx || idx == obj->efile.st_ops_link_shndx)
+ if (obj->efile.secs[idx].sec_type == SEC_ST_OPS)
err = bpf_object__collect_st_ops_relos(obj, shdr, data);
else if (idx == obj->efile.btf_maps_shndx)
err = bpf_object__collect_map_relos(obj, shdr, data);
--
2.43.0
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH bpf-next v2 11/15] libbpf: struct_ops in SEC("?.struct_ops") and SEC("?.struct_ops.link")
2024-03-02 1:19 [PATCH bpf-next v2 00/15] libbpf: type suffixes and autocreate flag for struct_ops maps Eduard Zingerman
` (9 preceding siblings ...)
2024-03-02 1:19 ` [PATCH bpf-next v2 10/15] libbpf: replace elf_state->st_ops_* fields with SEC_ST_OPS sec_type Eduard Zingerman
@ 2024-03-02 1:19 ` Eduard Zingerman
2024-03-02 1:19 ` [PATCH bpf-next v2 12/15] libbpf: rewrite btf datasec names starting from '?' Eduard Zingerman
` (3 subsequent siblings)
14 siblings, 0 replies; 17+ messages in thread
From: Eduard Zingerman @ 2024-03-02 1:19 UTC (permalink / raw)
To: bpf, ast
Cc: andrii, daniel, martin.lau, kernel-team, yonghong.song, void,
sinquersw, Eduard Zingerman
Allow using two new section names for struct_ops maps:
- SEC("?.struct_ops")
- SEC("?.struct_ops.link")
To specify maps that have bpf_map->autocreate == false after open.
Signed-off-by: Eduard Zingerman <eddyz87@gmail.com>
---
tools/lib/bpf/libbpf.c | 17 ++++++++++++++++-
1 file changed, 16 insertions(+), 1 deletion(-)
diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
index 8ecfad091cb5..157d28aea186 100644
--- a/tools/lib/bpf/libbpf.c
+++ b/tools/lib/bpf/libbpf.c
@@ -497,6 +497,8 @@ struct bpf_struct_ops {
#define KSYMS_SEC ".ksyms"
#define STRUCT_OPS_SEC ".struct_ops"
#define STRUCT_OPS_LINK_SEC ".struct_ops.link"
+#define OPT_STRUCT_OPS_SEC "?.struct_ops"
+#define OPT_STRUCT_OPS_LINK_SEC "?.struct_ops.link"
enum libbpf_map_type {
LIBBPF_MAP_UNSPEC,
@@ -1278,6 +1280,15 @@ static int init_struct_ops_maps(struct bpf_object *obj, const char *sec_name,
return -ENOMEM;
map->btf_value_type_id = type_id;
+ /* Follow same convention as for programs autoload:
+ * SEC("?.struct_ops") means map is not created by default.
+ */
+ if (sec_name[0] == '?') {
+ map->autocreate = false;
+ /* from now on forget there was ? in section name */
+ sec_name++;
+ }
+
map->def.type = BPF_MAP_TYPE_STRUCT_OPS;
map->def.key_size = sizeof(int);
map->def.value_size = type->size;
@@ -3647,7 +3658,9 @@ static int bpf_object__elf_collect(struct bpf_object *obj)
sec_desc->shdr = sh;
sec_desc->data = data;
} else if (strcmp(name, STRUCT_OPS_SEC) == 0 ||
- strcmp(name, STRUCT_OPS_LINK_SEC) == 0) {
+ strcmp(name, STRUCT_OPS_LINK_SEC) == 0 ||
+ strcmp(name, OPT_STRUCT_OPS_SEC) == 0 ||
+ strcmp(name, OPT_STRUCT_OPS_LINK_SEC) == 0) {
sec_desc->sec_type = SEC_ST_OPS;
sec_desc->shdr = sh;
sec_desc->data = data;
@@ -3667,6 +3680,8 @@ static int bpf_object__elf_collect(struct bpf_object *obj)
if (!section_have_execinstr(obj, targ_sec_idx) &&
strcmp(name, ".rel" STRUCT_OPS_SEC) &&
strcmp(name, ".rel" STRUCT_OPS_LINK_SEC) &&
+ strcmp(name, ".rel" OPT_STRUCT_OPS_SEC) &&
+ strcmp(name, ".rel" OPT_STRUCT_OPS_LINK_SEC) &&
strcmp(name, ".rel" MAPS_ELF_SEC)) {
pr_info("elf: skipping relo section(%d) %s for section(%d) %s\n",
idx, name, targ_sec_idx,
--
2.43.0
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH bpf-next v2 12/15] libbpf: rewrite btf datasec names starting from '?'
2024-03-02 1:19 [PATCH bpf-next v2 00/15] libbpf: type suffixes and autocreate flag for struct_ops maps Eduard Zingerman
` (10 preceding siblings ...)
2024-03-02 1:19 ` [PATCH bpf-next v2 11/15] libbpf: struct_ops in SEC("?.struct_ops") and SEC("?.struct_ops.link") Eduard Zingerman
@ 2024-03-02 1:19 ` Eduard Zingerman
2024-03-02 1:19 ` [PATCH bpf-next v2 13/15] selftests/bpf: test case for SEC("?.struct_ops") Eduard Zingerman
` (2 subsequent siblings)
14 siblings, 0 replies; 17+ messages in thread
From: Eduard Zingerman @ 2024-03-02 1:19 UTC (permalink / raw)
To: bpf, ast
Cc: andrii, daniel, martin.lau, kernel-team, yonghong.song, void,
sinquersw, Eduard Zingerman
Optional struct_ops maps are defined using question mark at the start
of the section name, e.g.:
SEC("?.struct_ops")
struct test_ops optional_map = { ... };
This commit teaches libbpf to detect if kernel allows '?' prefix
in datasec names, and if it doesn't then to rewrite such names
by removing '?' prefix and adding ".optional" suffix.
For example:
DATASEC ?.struct_ops -> DATASEC .struct_ops.optional
Signed-off-by: Eduard Zingerman <eddyz87@gmail.com>
---
tools/lib/bpf/features.c | 22 ++++++++++++++++++++++
tools/lib/bpf/libbpf.c | 30 +++++++++++++++++++++++++++++-
tools/lib/bpf/libbpf_internal.h | 2 ++
3 files changed, 53 insertions(+), 1 deletion(-)
diff --git a/tools/lib/bpf/features.c b/tools/lib/bpf/features.c
index 6b0738ad7063..4e783cc7fc4b 100644
--- a/tools/lib/bpf/features.c
+++ b/tools/lib/bpf/features.c
@@ -147,6 +147,25 @@ static int probe_kern_btf_datasec(int token_fd)
strs, sizeof(strs), token_fd));
}
+static int probe_kern_btf_qmark_datasec(int token_fd)
+{
+ static const char strs[] = "\0x\0?.data";
+ /* static int a; */
+ __u32 types[] = {
+ /* int */
+ BTF_TYPE_INT_ENC(0, BTF_INT_SIGNED, 0, 32, 4), /* [1] */
+ /* VAR x */ /* [2] */
+ BTF_TYPE_ENC(1, BTF_INFO_ENC(BTF_KIND_VAR, 0, 0), 1),
+ BTF_VAR_STATIC,
+ /* DATASEC ?.data */ /* [3] */
+ BTF_TYPE_ENC(3, BTF_INFO_ENC(BTF_KIND_DATASEC, 0, 1), 4),
+ BTF_VAR_SECINFO_ENC(2, 0, 4),
+ };
+
+ return probe_fd(libbpf__load_raw_btf((char *)types, sizeof(types),
+ strs, sizeof(strs), token_fd));
+}
+
static int probe_kern_btf_float(int token_fd)
{
static const char strs[] = "\0float";
@@ -534,6 +553,9 @@ static struct kern_feature_desc {
[FEAT_ARG_CTX_TAG] = {
"kernel-side __arg_ctx tag", probe_kern_arg_ctx_tag,
},
+ [FEAT_BTF_QMARK_DATASEC] = {
+ "BTF DATASEC names starting from '?'", probe_kern_btf_qmark_datasec,
+ },
};
bool feat_supported(struct kern_feature_cache *cache, enum kern_feature_id feat_id)
diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
index 157d28aea186..af0bfb987928 100644
--- a/tools/lib/bpf/libbpf.c
+++ b/tools/lib/bpf/libbpf.c
@@ -2828,6 +2828,11 @@ static bool section_have_execinstr(struct bpf_object *obj, int idx)
return sh->sh_flags & SHF_EXECINSTR;
}
+static bool starts_with_qmark(const char *s)
+{
+ return s && s[0] == '?';
+}
+
static bool btf_needs_sanitization(struct bpf_object *obj)
{
bool has_func_global = kernel_supports(obj, FEAT_BTF_GLOBAL_FUNC);
@@ -2837,9 +2842,10 @@ static bool btf_needs_sanitization(struct bpf_object *obj)
bool has_decl_tag = kernel_supports(obj, FEAT_BTF_DECL_TAG);
bool has_type_tag = kernel_supports(obj, FEAT_BTF_TYPE_TAG);
bool has_enum64 = kernel_supports(obj, FEAT_BTF_ENUM64);
+ bool has_qmark_datasec = kernel_supports(obj, FEAT_BTF_QMARK_DATASEC);
return !has_func || !has_datasec || !has_func_global || !has_float ||
- !has_decl_tag || !has_type_tag || !has_enum64;
+ !has_decl_tag || !has_type_tag || !has_enum64 || !has_qmark_datasec;
}
static int bpf_object__sanitize_btf(struct bpf_object *obj, struct btf *btf)
@@ -2851,6 +2857,7 @@ static int bpf_object__sanitize_btf(struct bpf_object *obj, struct btf *btf)
bool has_decl_tag = kernel_supports(obj, FEAT_BTF_DECL_TAG);
bool has_type_tag = kernel_supports(obj, FEAT_BTF_TYPE_TAG);
bool has_enum64 = kernel_supports(obj, FEAT_BTF_ENUM64);
+ bool has_qmark_datasec = kernel_supports(obj, FEAT_BTF_QMARK_DATASEC);
int enum64_placeholder_id = 0;
struct btf_type *t;
int i, j, vlen;
@@ -2876,6 +2883,8 @@ static int bpf_object__sanitize_btf(struct bpf_object *obj, struct btf *btf)
char *name;
name = (char *)btf__name_by_offset(btf, t->name_off);
+ if (*name == '?')
+ *name++ = '_';
while (*name) {
if (*name == '.')
*name = '_';
@@ -2892,6 +2901,25 @@ static int bpf_object__sanitize_btf(struct bpf_object *obj, struct btf *btf)
vt = (void *)btf__type_by_id(btf, v->type);
m->name_off = vt->name_off;
}
+ } else if (!has_qmark_datasec && btf_is_datasec(t) &&
+ starts_with_qmark(btf__name_by_offset(btf, t->name_off))) {
+ /* remove '?' prefix and add '.optional' suffix for
+ * DATASEC names staring from '?':
+ *
+ * DATASEC ?.foo -> DATASEC .foo.optional
+ */
+ const char *name;
+ char buf[256];
+ int str;
+
+ name = btf__name_by_offset(btf, t->name_off);
+ snprintf(buf, sizeof(buf), "%s.optional", &name[1] /* skip '?' */);
+ str = btf__add_str(btf, buf);
+ if (str < 0)
+ return str;
+
+ t = (struct btf_type *)btf__type_by_id(btf, i);
+ t->name_off = str;
} else if (!has_func && btf_is_func_proto(t)) {
/* replace FUNC_PROTO with ENUM */
vlen = btf_vlen(t);
diff --git a/tools/lib/bpf/libbpf_internal.h b/tools/lib/bpf/libbpf_internal.h
index ad936ac5e639..864b36177424 100644
--- a/tools/lib/bpf/libbpf_internal.h
+++ b/tools/lib/bpf/libbpf_internal.h
@@ -374,6 +374,8 @@ enum kern_feature_id {
FEAT_UPROBE_MULTI_LINK,
/* Kernel supports arg:ctx tag (__arg_ctx) for global subprogs natively */
FEAT_ARG_CTX_TAG,
+ /* Kernel supports '?' at the front of datasec names */
+ FEAT_BTF_QMARK_DATASEC,
__FEAT_CNT,
};
--
2.43.0
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH bpf-next v2 13/15] selftests/bpf: test case for SEC("?.struct_ops")
2024-03-02 1:19 [PATCH bpf-next v2 00/15] libbpf: type suffixes and autocreate flag for struct_ops maps Eduard Zingerman
` (11 preceding siblings ...)
2024-03-02 1:19 ` [PATCH bpf-next v2 12/15] libbpf: rewrite btf datasec names starting from '?' Eduard Zingerman
@ 2024-03-02 1:19 ` Eduard Zingerman
2024-03-02 1:19 ` [PATCH bpf-next v2 14/15] bpf: allow '?' at the beginning of DATASEC names Eduard Zingerman
2024-03-02 1:19 ` [PATCH bpf-next v2 15/15] selftests/bpf: test cases for '?' in BTF names Eduard Zingerman
14 siblings, 0 replies; 17+ messages in thread
From: Eduard Zingerman @ 2024-03-02 1:19 UTC (permalink / raw)
To: bpf, ast
Cc: andrii, daniel, martin.lau, kernel-team, yonghong.song, void,
sinquersw, Eduard Zingerman
Check that "?.struct_ops" and "?.struct_ops.link" section names define
struct_ops maps with autocreate == false after open.
Signed-off-by: Eduard Zingerman <eddyz87@gmail.com>
---
.../bpf/prog_tests/struct_ops_autocreate.c | 58 +++++++++++++++++--
.../bpf/progs/struct_ops_autocreate.c | 10 ++++
2 files changed, 62 insertions(+), 6 deletions(-)
diff --git a/tools/testing/selftests/bpf/prog_tests/struct_ops_autocreate.c b/tools/testing/selftests/bpf/prog_tests/struct_ops_autocreate.c
index 765b0ec6383a..d5295ff2e925 100644
--- a/tools/testing/selftests/bpf/prog_tests/struct_ops_autocreate.c
+++ b/tools/testing/selftests/bpf/prog_tests/struct_ops_autocreate.c
@@ -33,10 +33,24 @@ static void cant_load_full_object(void)
struct_ops_autocreate__destroy(skel);
}
+static int check_test_1_link(struct struct_ops_autocreate *skel, struct bpf_map *map)
+{
+ struct bpf_link *link;
+ int err;
+
+ link = bpf_map__attach_struct_ops(skel->maps.testmod_1);
+ if (!ASSERT_OK_PTR(link, "bpf_map__attach_struct_ops"))
+ return -1;
+
+ /* test_1() would be called from bpf_dummy_reg2() in bpf_testmod.c */
+ err = ASSERT_EQ(skel->bss->test_1_result, 42, "test_1_result");
+ bpf_link__destroy(link);
+ return err;
+}
+
static void can_load_partial_object(void)
{
struct struct_ops_autocreate *skel;
- struct bpf_link *link = NULL;
int err;
skel = struct_ops_autocreate__open();
@@ -57,15 +71,45 @@ static void can_load_partial_object(void)
if (ASSERT_OK(err, "struct_ops_autocreate__load"))
goto cleanup;
- link = bpf_map__attach_struct_ops(skel->maps.testmod_1);
- if (!ASSERT_OK_PTR(link, "bpf_map__attach_struct_ops"))
+ check_test_1_link(skel, skel->maps.testmod_1);
+
+cleanup:
+ struct_ops_autocreate__destroy(skel);
+}
+
+static void optional_maps(void)
+{
+ struct struct_ops_autocreate *skel;
+ int err;
+
+ skel = struct_ops_autocreate__open();
+ if (!ASSERT_OK_PTR(skel, "struct_ops_autocreate__open"))
+ return;
+
+ err = !ASSERT_TRUE(bpf_map__autocreate(skel->maps.testmod_1),
+ "default autocreate for testmod_1");
+ err |= !ASSERT_TRUE(bpf_map__autocreate(skel->maps.testmod_2),
+ "default autocreate for testmod_2");
+ err |= !ASSERT_FALSE(bpf_map__autocreate(skel->maps.optional_map),
+ "default autocreate for optional_map");
+ err |= !ASSERT_FALSE(bpf_map__autocreate(skel->maps.optional_map2),
+ "default autocreate for optional_map2");
+ if (err)
goto cleanup;
- /* test_1() would be called from bpf_dummy_reg2() in bpf_testmod.c */
- ASSERT_EQ(skel->bss->test_1_result, 42, "test_1_result");
+ err = bpf_map__set_autocreate(skel->maps.testmod_1, false);
+ err |= bpf_map__set_autocreate(skel->maps.testmod_2, false);
+ err |= bpf_map__set_autocreate(skel->maps.optional_map2, true);
+ if (!ASSERT_OK(err, "bpf_map__set_autocreate"))
+ goto cleanup;
+
+ err = struct_ops_autocreate__load(skel);
+ if (ASSERT_OK(err, "struct_ops_autocreate__load"))
+ goto cleanup;
+
+ check_test_1_link(skel, skel->maps.optional_map2);
cleanup:
- bpf_link__destroy(link);
struct_ops_autocreate__destroy(skel);
}
@@ -75,4 +119,6 @@ void test_struct_ops_autocreate(void)
cant_load_full_object();
if (test__start_subtest("can_load_partial_object"))
can_load_partial_object();
+ if (test__start_subtest("optional_maps"))
+ optional_maps();
}
diff --git a/tools/testing/selftests/bpf/progs/struct_ops_autocreate.c b/tools/testing/selftests/bpf/progs/struct_ops_autocreate.c
index 294d48bb8e3c..703ad8a2914f 100644
--- a/tools/testing/selftests/bpf/progs/struct_ops_autocreate.c
+++ b/tools/testing/selftests/bpf/progs/struct_ops_autocreate.c
@@ -40,3 +40,13 @@ struct bpf_testmod_ops___v2 testmod_2 = {
.test_1 = (void *)test_1,
.does_not_exist = (void *)test_2
};
+
+SEC("?.struct_ops")
+struct bpf_testmod_ops___v1 optional_map = {
+ .test_1 = (void *)test_1,
+};
+
+SEC("?.struct_ops.link")
+struct bpf_testmod_ops___v1 optional_map2 = {
+ .test_1 = (void *)test_1,
+};
--
2.43.0
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH bpf-next v2 14/15] bpf: allow '?' at the beginning of DATASEC names
2024-03-02 1:19 [PATCH bpf-next v2 00/15] libbpf: type suffixes and autocreate flag for struct_ops maps Eduard Zingerman
` (12 preceding siblings ...)
2024-03-02 1:19 ` [PATCH bpf-next v2 13/15] selftests/bpf: test case for SEC("?.struct_ops") Eduard Zingerman
@ 2024-03-02 1:19 ` Eduard Zingerman
2024-03-02 1:19 ` [PATCH bpf-next v2 15/15] selftests/bpf: test cases for '?' in BTF names Eduard Zingerman
14 siblings, 0 replies; 17+ messages in thread
From: Eduard Zingerman @ 2024-03-02 1:19 UTC (permalink / raw)
To: bpf, ast
Cc: andrii, daniel, martin.lau, kernel-team, yonghong.song, void,
sinquersw, Eduard Zingerman
Currently kernel does not allow question marks in BTF names.
This commit makes an exception, allowing first character of the
DATASEC name to be a question mark.
The intent is to allow libbpf to use SEC("?.struct_ops") to identify
struct_ops maps that are optional, e.g. like in the following BPF code:
SEC("?.struct_ops")
struct test_ops optional_map = { ... };
Which yields the following BTF:
...
[13] DATASEC '?.struct_ops' size=0 vlen=...
...
To load such BTF libbpf rewrites DATASEC name before load.
After this patch the rewrite won't be necessary.
Signed-off-by: Eduard Zingerman <eddyz87@gmail.com>
---
kernel/bpf/btf.c | 17 +++++++++--------
1 file changed, 9 insertions(+), 8 deletions(-)
diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c
index 6ff0bd1a91d5..a25fb6bce808 100644
--- a/kernel/bpf/btf.c
+++ b/kernel/bpf/btf.c
@@ -761,12 +761,13 @@ static bool btf_name_offset_valid(const struct btf *btf, u32 offset)
return offset < btf->hdr.str_len;
}
-static bool __btf_name_char_ok(char c, bool first)
+static bool __btf_name_char_ok(char c, bool first, bool allow_qmark)
{
if ((first ? !isalpha(c) :
!isalnum(c)) &&
c != '_' &&
- c != '.')
+ c != '.' &&
+ (allow_qmark && first ? c != '?' : true))
return false;
return true;
}
@@ -783,20 +784,20 @@ static const char *btf_str_by_offset(const struct btf *btf, u32 offset)
return NULL;
}
-static bool __btf_name_valid(const struct btf *btf, u32 offset)
+static bool __btf_name_valid(const struct btf *btf, u32 offset, bool allow_qmark)
{
/* offset must be valid */
const char *src = btf_str_by_offset(btf, offset);
const char *src_limit;
- if (!__btf_name_char_ok(*src, true))
+ if (!__btf_name_char_ok(*src, true, allow_qmark))
return false;
/* set a limit on identifier length */
src_limit = src + KSYM_NAME_LEN;
src++;
while (*src && src < src_limit) {
- if (!__btf_name_char_ok(*src, false))
+ if (!__btf_name_char_ok(*src, false, false))
return false;
src++;
}
@@ -806,12 +807,12 @@ static bool __btf_name_valid(const struct btf *btf, u32 offset)
static bool btf_name_valid_identifier(const struct btf *btf, u32 offset)
{
- return __btf_name_valid(btf, offset);
+ return __btf_name_valid(btf, offset, false);
}
static bool btf_name_valid_section(const struct btf *btf, u32 offset)
{
- return __btf_name_valid(btf, offset);
+ return __btf_name_valid(btf, offset, true);
}
static const char *__btf_name_by_offset(const struct btf *btf, u32 offset)
@@ -4481,7 +4482,7 @@ static s32 btf_var_check_meta(struct btf_verifier_env *env,
}
if (!t->name_off ||
- !__btf_name_valid(env->btf, t->name_off)) {
+ !btf_name_valid_identifier(env->btf, t->name_off)) {
btf_verifier_log_type(env, t, "Invalid name");
return -EINVAL;
}
--
2.43.0
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH bpf-next v2 15/15] selftests/bpf: test cases for '?' in BTF names
2024-03-02 1:19 [PATCH bpf-next v2 00/15] libbpf: type suffixes and autocreate flag for struct_ops maps Eduard Zingerman
` (13 preceding siblings ...)
2024-03-02 1:19 ` [PATCH bpf-next v2 14/15] bpf: allow '?' at the beginning of DATASEC names Eduard Zingerman
@ 2024-03-02 1:19 ` Eduard Zingerman
14 siblings, 0 replies; 17+ messages in thread
From: Eduard Zingerman @ 2024-03-02 1:19 UTC (permalink / raw)
To: bpf, ast
Cc: andrii, daniel, martin.lau, kernel-team, yonghong.song, void,
sinquersw, Eduard Zingerman
Three test cases to verify when '?' is allowed in BTF names:
- allowed as first character in DATASEC name;
- not allowed as non-first character in DATASEC name;
- not allowed in any position in non-DATASEC names.
Signed-off-by: Eduard Zingerman <eddyz87@gmail.com>
---
tools/testing/selftests/bpf/prog_tests/btf.c | 46 ++++++++++++++++++++
1 file changed, 46 insertions(+)
diff --git a/tools/testing/selftests/bpf/prog_tests/btf.c b/tools/testing/selftests/bpf/prog_tests/btf.c
index 816145bcb647..88c71e3924b9 100644
--- a/tools/testing/selftests/bpf/prog_tests/btf.c
+++ b/tools/testing/selftests/bpf/prog_tests/btf.c
@@ -3535,6 +3535,49 @@ static struct btf_raw_test raw_tests[] = {
.value_type_id = 1,
.max_entries = 1,
},
+{
+ .descr = "datasec: name '?.foo' is ok",
+ .raw_types = {
+ /* int */
+ BTF_TYPE_INT_ENC(0, BTF_INT_SIGNED, 0, 32, 4), /* [1] */
+ /* VAR x */ /* [2] */
+ BTF_TYPE_ENC(1, BTF_INFO_ENC(BTF_KIND_VAR, 0, 0), 1),
+ BTF_VAR_STATIC,
+ /* DATASEC ?.data */ /* [3] */
+ BTF_TYPE_ENC(3, BTF_INFO_ENC(BTF_KIND_DATASEC, 0, 1), 4),
+ BTF_VAR_SECINFO_ENC(2, 0, 4),
+ BTF_END_RAW,
+ },
+ BTF_STR_SEC("\0x\0?.foo"),
+},
+{
+ .descr = "datasec: name '.?foo' is not ok",
+ .raw_types = {
+ /* int */
+ BTF_TYPE_INT_ENC(0, BTF_INT_SIGNED, 0, 32, 4), /* [1] */
+ /* VAR x */ /* [2] */
+ BTF_TYPE_ENC(1, BTF_INFO_ENC(BTF_KIND_VAR, 0, 0), 1),
+ BTF_VAR_STATIC,
+ /* DATASEC ?.data */ /* [3] */
+ BTF_TYPE_ENC(3, BTF_INFO_ENC(BTF_KIND_DATASEC, 0, 1), 4),
+ BTF_VAR_SECINFO_ENC(2, 0, 4),
+ BTF_END_RAW,
+ },
+ BTF_STR_SEC("\0x\0.?foo"),
+ .err_str = "Invalid name",
+ .btf_load_err = true,
+},
+{
+ .descr = "type name '?foo' is not ok",
+ .raw_types = {
+ /* union ?foo; */
+ BTF_TYPE_ENC(1, BTF_INFO_ENC(BTF_KIND_FWD, 1, 0), 0), /* [1] */
+ BTF_END_RAW,
+ },
+ BTF_STR_SEC("\0?foo"),
+ .err_str = "Invalid name",
+ .btf_load_err = true,
+},
{
.descr = "float test #1, well-formed",
@@ -4363,6 +4406,9 @@ static void do_test_raw(unsigned int test_num)
if (err || btf_fd < 0)
goto done;
+ if (!test->map_type)
+ goto done;
+
opts.btf_fd = btf_fd;
opts.btf_key_type_id = test->key_type_id;
opts.btf_value_type_id = test->value_type_id;
--
2.43.0
^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [PATCH bpf-next v2 08/15] libbpf: sync progs autoload with maps autocreate for struct_ops maps
2024-03-02 1:19 ` [PATCH bpf-next v2 08/15] libbpf: sync progs autoload with maps autocreate " Eduard Zingerman
@ 2024-03-04 19:13 ` Eduard Zingerman
0 siblings, 0 replies; 17+ messages in thread
From: Eduard Zingerman @ 2024-03-04 19:13 UTC (permalink / raw)
To: bpf, ast
Cc: andrii, daniel, martin.lau, kernel-team, yonghong.song, void,
sinquersw
On Sat, 2024-03-02 at 03:19 +0200, Eduard Zingerman wrote:
> Automatically select which struct_ops programs to load depending on
> which struct_ops maps are selected for automatic creation.
> E.g. for the BPF code below:
>
> SEC("struct_ops/test_1") int BPF_PROG(foo) { ... }
> SEC("struct_ops/test_2") int BPF_PROG(bar) { ... }
>
> SEC(".struct_ops.link")
> struct test_ops___v1 A = {
> .foo = (void *)foo
> };
>
> SEC(".struct_ops.link")
> struct test_ops___v2 B = {
> .foo = (void *)foo,
> .bar = (void *)bar,
> };
>
> And the following libbpf API calls:
>
> bpf_map__set_autocreate(skel->maps.A, true);
> bpf_map__set_autocreate(skel->maps.B, false);
>
> The autoload would be enabled for program 'foo' and disabled for
> program 'bar'.
>
> To achieve this:
> - for struct_ops programs referenced from struct_ops maps set autoload
> property at open() to false;
> - when creating struct_ops maps set autoload property of referenced
> programs to true.
>
> (Note: struct_ops programs not referenced from any map would have
> their autoload property set to true by default.
> If attach_btf_id and expected_attach_type properties would not be
> specified for such programs manually, the load phase would fail).
>
> Signed-off-by: Eduard Zingerman <eddyz87@gmail.com>
> ---
I talked to Andrii today and he asked to move all logic related to
these things to "load" phase. Will post v3 shortly.
[...]
^ permalink raw reply [flat|nested] 17+ messages in thread
end of thread, other threads:[~2024-03-04 19:14 UTC | newest]
Thread overview: 17+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-03-02 1:19 [PATCH bpf-next v2 00/15] libbpf: type suffixes and autocreate flag for struct_ops maps Eduard Zingerman
2024-03-02 1:19 ` [PATCH bpf-next v2 01/15] libbpf: allow version suffixes (___smth) for struct_ops types Eduard Zingerman
2024-03-02 1:19 ` [PATCH bpf-next v2 02/15] libbpf: tie struct_ops programs to kernel BTF ids, not to local ids Eduard Zingerman
2024-03-02 1:19 ` [PATCH bpf-next v2 03/15] libbpf: honor autocreate flag for struct_ops maps Eduard Zingerman
2024-03-02 1:19 ` [PATCH bpf-next v2 04/15] selftests/bpf: test struct_ops map definition with type suffix Eduard Zingerman
2024-03-02 1:19 ` [PATCH bpf-next v2 05/15] selftests/bpf: utility functions to capture libbpf log in test_progs Eduard Zingerman
2024-03-02 1:19 ` [PATCH bpf-next v2 06/15] selftests/bpf: bad_struct_ops test Eduard Zingerman
2024-03-02 1:19 ` [PATCH bpf-next v2 07/15] selftests/bpf: test autocreate behavior for struct_ops maps Eduard Zingerman
2024-03-02 1:19 ` [PATCH bpf-next v2 08/15] libbpf: sync progs autoload with maps autocreate " Eduard Zingerman
2024-03-04 19:13 ` Eduard Zingerman
2024-03-02 1:19 ` [PATCH bpf-next v2 09/15] selftests/bpf: verify struct_ops autoload/autocreate sync Eduard Zingerman
2024-03-02 1:19 ` [PATCH bpf-next v2 10/15] libbpf: replace elf_state->st_ops_* fields with SEC_ST_OPS sec_type Eduard Zingerman
2024-03-02 1:19 ` [PATCH bpf-next v2 11/15] libbpf: struct_ops in SEC("?.struct_ops") and SEC("?.struct_ops.link") Eduard Zingerman
2024-03-02 1:19 ` [PATCH bpf-next v2 12/15] libbpf: rewrite btf datasec names starting from '?' Eduard Zingerman
2024-03-02 1:19 ` [PATCH bpf-next v2 13/15] selftests/bpf: test case for SEC("?.struct_ops") Eduard Zingerman
2024-03-02 1:19 ` [PATCH bpf-next v2 14/15] bpf: allow '?' at the beginning of DATASEC names Eduard Zingerman
2024-03-02 1:19 ` [PATCH bpf-next v2 15/15] selftests/bpf: test cases for '?' in BTF names Eduard Zingerman
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox