* [PATCH bpf-next 1/2] libbpf: handle nulled-out program in struct_ops correctly
@ 2024-04-28 3:09 Andrii Nakryiko
2024-04-28 3:09 ` [PATCH bpf-next 2/2] selftests/bpf: validate nulled-out struct_ops program is handled properly Andrii Nakryiko
2024-04-30 0:00 ` [PATCH bpf-next 1/2] libbpf: handle nulled-out program in struct_ops correctly patchwork-bot+netdevbpf
0 siblings, 2 replies; 6+ messages in thread
From: Andrii Nakryiko @ 2024-04-28 3:09 UTC (permalink / raw)
To: bpf, ast, daniel, martin.lau; +Cc: andrii, kernel-team
If struct_ops has one of program callbacks set declaratively and host
kernel is old and doesn't support this callback, libbpf will allow to
load such struct_ops as long as that callback was explicitly nulled-out
(presumably through skeleton). This is all working correctly, except we
won't reset corresponding program slot to NULL before bailing out, which
will lead to libbpf not detecting that BPF program has to be not
auto-loaded. Fix this by unconditionally resetting corresponding program
slot to NULL.
Fixes: c911fc61a7ce ("libbpf: Skip zeroed or null fields if not found in the kernel type.")
Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
---
tools/lib/bpf/libbpf.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
index 97eb6e5dd7c8..898d5d34ecea 100644
--- a/tools/lib/bpf/libbpf.c
+++ b/tools/lib/bpf/libbpf.c
@@ -1148,6 +1148,7 @@ static int bpf_map__init_kern_struct_ops(struct bpf_map *map)
* presented in the kernel BTF.
*/
if (libbpf_is_mem_zeroed(mdata, msize)) {
+ st_ops->progs[i] = NULL;
pr_info("struct_ops %s: member %s not found in kernel, skipping it as it's set to zero\n",
map->name, mname);
continue;
--
2.43.0
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH bpf-next 2/2] selftests/bpf: validate nulled-out struct_ops program is handled properly 2024-04-28 3:09 [PATCH bpf-next 1/2] libbpf: handle nulled-out program in struct_ops correctly Andrii Nakryiko @ 2024-04-28 3:09 ` Andrii Nakryiko 2024-04-29 21:29 ` Martin KaFai Lau 2024-04-30 0:00 ` [PATCH bpf-next 1/2] libbpf: handle nulled-out program in struct_ops correctly patchwork-bot+netdevbpf 1 sibling, 1 reply; 6+ messages in thread From: Andrii Nakryiko @ 2024-04-28 3:09 UTC (permalink / raw) To: bpf, ast, daniel, martin.lau; +Cc: andrii, kernel-team Add a selftests validating that it's possible to have some struct_ops callback set declaratively, then disable it (by setting to NULL) programmatically. Libbpf should detect that such program should be loaded, even if host kernel doesn't have type information for it. Signed-off-by: Andrii Nakryiko <andrii@kernel.org> --- .../bpf/prog_tests/test_struct_ops_module.c | 18 ++++++++++++++++-- .../selftests/bpf/progs/struct_ops_module.c | 7 +++++++ 2 files changed, 23 insertions(+), 2 deletions(-) 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 7cf2b9ddd3e1..bd39586abd5a 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 @@ -66,6 +66,7 @@ static void test_struct_ops_load(void) * auto-loading, or it will fail to load. */ bpf_program__set_autoload(skel->progs.test_2, false); + bpf_map__set_autocreate(skel->maps.testmod_zeroed, false); err = struct_ops_module__load(skel); if (!ASSERT_OK(err, "struct_ops_module_load")) @@ -103,6 +104,10 @@ static void test_struct_ops_not_zeroed(void) if (!ASSERT_OK_PTR(skel, "struct_ops_module_open")) return; + skel->struct_ops.testmod_zeroed->zeroed = 0; + /* zeroed_op prog should be not loaded automatically now */ + skel->struct_ops.testmod_zeroed->zeroed_op = NULL; + err = struct_ops_module__load(skel); ASSERT_OK(err, "struct_ops_module_load"); @@ -118,6 +123,7 @@ static void test_struct_ops_not_zeroed(void) * value of "zeroed" is non-zero. */ skel->struct_ops.testmod_zeroed->zeroed = 0xdeadbeef; + skel->struct_ops.testmod_zeroed->zeroed_op = NULL; err = struct_ops_module__load(skel); ASSERT_ERR(err, "struct_ops_module_load_not_zeroed"); @@ -148,15 +154,23 @@ static void test_struct_ops_incompatible(void) { struct struct_ops_module *skel; struct bpf_link *link; + int err; - skel = struct_ops_module__open_and_load(); - if (!ASSERT_OK_PTR(skel, "open_and_load")) + skel = struct_ops_module__open(); + if (!ASSERT_OK_PTR(skel, "struct_ops_module_open")) return; + bpf_map__set_autocreate(skel->maps.testmod_zeroed, false); + + err = struct_ops_module__load(skel); + if (!ASSERT_OK(err, "skel_load")) + goto cleanup; + link = bpf_map__attach_struct_ops(skel->maps.testmod_incompatible); if (ASSERT_OK_PTR(link, "attach_struct_ops")) bpf_link__destroy(link); +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 63b065dae002..40109be2b3ae 100644 --- a/tools/testing/selftests/bpf/progs/struct_ops_module.c +++ b/tools/testing/selftests/bpf/progs/struct_ops_module.c @@ -63,10 +63,17 @@ struct bpf_testmod_ops___zeroed { int zeroed; }; +SEC("?struct_ops/test_3") +int BPF_PROG(zeroed_op) +{ + return 1; +} + SEC(".struct_ops.link") struct bpf_testmod_ops___zeroed testmod_zeroed = { .test_1 = (void *)test_1, .test_2 = (void *)test_2_v2, + .zeroed_op = (void *)zeroed_op, }; struct bpf_testmod_ops___incompatible { -- 2.43.0 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH bpf-next 2/2] selftests/bpf: validate nulled-out struct_ops program is handled properly 2024-04-28 3:09 ` [PATCH bpf-next 2/2] selftests/bpf: validate nulled-out struct_ops program is handled properly Andrii Nakryiko @ 2024-04-29 21:29 ` Martin KaFai Lau 2024-04-29 22:35 ` Andrii Nakryiko 0 siblings, 1 reply; 6+ messages in thread From: Martin KaFai Lau @ 2024-04-29 21:29 UTC (permalink / raw) To: Andrii Nakryiko; +Cc: bpf, ast, daniel, martin.lau, kernel-team On 4/27/24 8:09 PM, Andrii Nakryiko wrote: > Add a selftests validating that it's possible to have some struct_ops > callback set declaratively, then disable it (by setting to NULL) > programmatically. Libbpf should detect that such program should be such program should be /not/ loaded ? > loaded, even if host kernel doesn't have type information for it. > > @@ -103,6 +104,10 @@ static void test_struct_ops_not_zeroed(void) > if (!ASSERT_OK_PTR(skel, "struct_ops_module_open")) > return; > > + skel->struct_ops.testmod_zeroed->zeroed = 0; > + /* zeroed_op prog should be not loaded automatically now */ > + skel->struct_ops.testmod_zeroed->zeroed_op = NULL; > + > err = struct_ops_module__load(skel); > ASSERT_OK(err, "struct_ops_module_load"); > > @@ -118,6 +123,7 @@ static void test_struct_ops_not_zeroed(void) > * value of "zeroed" is non-zero. > */ > skel->struct_ops.testmod_zeroed->zeroed = 0xdeadbeef; > + skel->struct_ops.testmod_zeroed->zeroed_op = NULL; > err = struct_ops_module__load(skel); > ASSERT_ERR(err, "struct_ops_module_load_not_zeroed"); > ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH bpf-next 2/2] selftests/bpf: validate nulled-out struct_ops program is handled properly 2024-04-29 21:29 ` Martin KaFai Lau @ 2024-04-29 22:35 ` Andrii Nakryiko 2024-04-29 23:46 ` Martin KaFai Lau 0 siblings, 1 reply; 6+ messages in thread From: Andrii Nakryiko @ 2024-04-29 22:35 UTC (permalink / raw) To: Martin KaFai Lau Cc: Andrii Nakryiko, bpf, ast, daniel, martin.lau, kernel-team On Mon, Apr 29, 2024 at 2:29 PM Martin KaFai Lau <martin.lau@linux.dev> wrote: > > On 4/27/24 8:09 PM, Andrii Nakryiko wrote: > > Add a selftests validating that it's possible to have some struct_ops > > callback set declaratively, then disable it (by setting to NULL) > > programmatically. Libbpf should detect that such program should be > > such program should be /not/ loaded ? yep, can you fix it up while applying or should I send a new revision? > > > loaded, even if host kernel doesn't have type information for it. > > > > > @@ -103,6 +104,10 @@ static void test_struct_ops_not_zeroed(void) > > if (!ASSERT_OK_PTR(skel, "struct_ops_module_open")) > > return; > > > > + skel->struct_ops.testmod_zeroed->zeroed = 0; > > + /* zeroed_op prog should be not loaded automatically now */ > > + skel->struct_ops.testmod_zeroed->zeroed_op = NULL; > > + > > err = struct_ops_module__load(skel); > > ASSERT_OK(err, "struct_ops_module_load"); > > > > @@ -118,6 +123,7 @@ static void test_struct_ops_not_zeroed(void) > > * value of "zeroed" is non-zero. > > */ > > skel->struct_ops.testmod_zeroed->zeroed = 0xdeadbeef; > > + skel->struct_ops.testmod_zeroed->zeroed_op = NULL; > > err = struct_ops_module__load(skel); > > ASSERT_ERR(err, "struct_ops_module_load_not_zeroed"); > > > ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH bpf-next 2/2] selftests/bpf: validate nulled-out struct_ops program is handled properly 2024-04-29 22:35 ` Andrii Nakryiko @ 2024-04-29 23:46 ` Martin KaFai Lau 0 siblings, 0 replies; 6+ messages in thread From: Martin KaFai Lau @ 2024-04-29 23:46 UTC (permalink / raw) To: Andrii Nakryiko Cc: Andrii Nakryiko, bpf, ast, daniel, martin.lau, kernel-team On 4/29/24 3:35 PM, Andrii Nakryiko wrote: > On Mon, Apr 29, 2024 at 2:29 PM Martin KaFai Lau <martin.lau@linux.dev> wrote: >> >> On 4/27/24 8:09 PM, Andrii Nakryiko wrote: >>> Add a selftests validating that it's possible to have some struct_ops >>> callback set declaratively, then disable it (by setting to NULL) >>> programmatically. Libbpf should detect that such program should be >> >> such program should be /not/ loaded ? > > yep, can you fix it up while applying or should I send a new revision? I will take care of it. No need to respin. ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH bpf-next 1/2] libbpf: handle nulled-out program in struct_ops correctly 2024-04-28 3:09 [PATCH bpf-next 1/2] libbpf: handle nulled-out program in struct_ops correctly Andrii Nakryiko 2024-04-28 3:09 ` [PATCH bpf-next 2/2] selftests/bpf: validate nulled-out struct_ops program is handled properly Andrii Nakryiko @ 2024-04-30 0:00 ` patchwork-bot+netdevbpf 1 sibling, 0 replies; 6+ messages in thread From: patchwork-bot+netdevbpf @ 2024-04-30 0:00 UTC (permalink / raw) To: Andrii Nakryiko; +Cc: bpf, ast, daniel, martin.lau, kernel-team Hello: This series was applied to bpf/bpf-next.git (master) by Martin KaFai Lau <martin.lau@kernel.org>: On Sat, 27 Apr 2024 20:09:53 -0700 you wrote: > If struct_ops has one of program callbacks set declaratively and host > kernel is old and doesn't support this callback, libbpf will allow to > load such struct_ops as long as that callback was explicitly nulled-out > (presumably through skeleton). This is all working correctly, except we > won't reset corresponding program slot to NULL before bailing out, which > will lead to libbpf not detecting that BPF program has to be not > auto-loaded. Fix this by unconditionally resetting corresponding program > slot to NULL. > > [...] Here is the summary with links: - [bpf-next,1/2] libbpf: handle nulled-out program in struct_ops correctly https://git.kernel.org/bpf/bpf-next/c/f973fccd43d3 - [bpf-next,2/2] selftests/bpf: validate nulled-out struct_ops program is handled properly https://git.kernel.org/bpf/bpf-next/c/1bba3b3d373d You are awesome, thank you! -- Deet-doot-dot, I am a bot. https://korg.docs.kernel.org/patchwork/pwbot.html ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2024-04-30 0:00 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2024-04-28 3:09 [PATCH bpf-next 1/2] libbpf: handle nulled-out program in struct_ops correctly Andrii Nakryiko 2024-04-28 3:09 ` [PATCH bpf-next 2/2] selftests/bpf: validate nulled-out struct_ops program is handled properly Andrii Nakryiko 2024-04-29 21:29 ` Martin KaFai Lau 2024-04-29 22:35 ` Andrii Nakryiko 2024-04-29 23:46 ` Martin KaFai Lau 2024-04-30 0:00 ` [PATCH bpf-next 1/2] libbpf: handle nulled-out program in struct_ops correctly patchwork-bot+netdevbpf
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.