BPF List
 help / color / mirror / Atom feed
* [PATCH bpf-next 1/4] libbpf: Add bpf_program__add_flags() and bpf_program__clear_flags()
@ 2026-09-03  9:24 Toke Høiland-Jørgensen
  2026-09-03  9:24 ` [PATCH bpf-next 2/4] selftests/bpf: Add assertions for bpf_program__{add,clear}_flags() Toke Høiland-Jørgensen
                   ` (3 more replies)
  0 siblings, 4 replies; 6+ messages in thread
From: Toke Høiland-Jørgensen @ 2026-09-03  9:24 UTC (permalink / raw)
  To: Andrii Nakryiko, Eduard Zingerman, Ihor Solodrai,
	Alexei Starovoitov, Daniel Borkmann, Kumar Kartikeya Dwivedi,
	Martin KaFai Lau, Song Liu, Yonghong Song, Jiri Olsa,
	Emil Tsalapatis
  Cc: Toke Høiland-Jørgensen, Andrii Nakryiko, bpf

When changing BPF program flags, applications often need to just add or
remove a single flag. The bpf_program__set_flags() function clobbers any
existing flags value, making this awkward to do non-destructively.

Add two new convenience helpers, bpf_program__add_flags() and
bpf_program__clear_flags(), which wraps bpf_program__set_flags() in the
bitwise operations required to avoid clobbering things.

Suggested-by: Andrii Nakryiko <andrii.nakryiko@gmail.com>
Signed-off-by: Toke Høiland-Jørgensen <toke@redhat.com>
---
 tools/lib/bpf/libbpf.c   | 10 ++++++++++
 tools/lib/bpf/libbpf.h   |  6 ++++++
 tools/lib/bpf/libbpf.map |  2 ++
 3 files changed, 18 insertions(+)

diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
index c036e8a91ed8..221c4fe1fb31 100644
--- a/tools/lib/bpf/libbpf.c
+++ b/tools/lib/bpf/libbpf.c
@@ -9938,6 +9938,16 @@ int bpf_program__set_flags(struct bpf_program *prog, __u32 flags)
 	return 0;
 }
 
+int bpf_program__add_flags(struct bpf_program *prog, __u32 flags)
+{
+	return bpf_program__set_flags(prog, prog->prog_flags | flags);
+}
+
+int bpf_program__clear_flags(struct bpf_program *prog, __u32 flags)
+{
+	return bpf_program__set_flags(prog, prog->prog_flags & ~flags);
+}
+
 __u32 bpf_program__log_level(const struct bpf_program *prog)
 {
 	return prog->log_level;
diff --git a/tools/lib/bpf/libbpf.h b/tools/lib/bpf/libbpf.h
index b965ad571540..3932bf9cb490 100644
--- a/tools/lib/bpf/libbpf.h
+++ b/tools/lib/bpf/libbpf.h
@@ -1011,6 +1011,12 @@ bpf_program__set_expected_attach_type(struct bpf_program *prog,
 LIBBPF_API __u32 bpf_program__flags(const struct bpf_program *prog);
 LIBBPF_API int bpf_program__set_flags(struct bpf_program *prog, __u32 flags);
 
+/* Convenience helpers to non-destructively add or clear the specified flags
+ * instead of clobbering them as bpf_program__set_flags() does.
+ */
+LIBBPF_API int bpf_program__add_flags(struct bpf_program *prog, __u32 flags);
+LIBBPF_API int bpf_program__clear_flags(struct bpf_program *prog, __u32 flags);
+
 /* Per-program log level and log buffer getters/setters.
  * See bpf_object_open_opts comments regarding log_level and log_buf
  * interactions.
diff --git a/tools/lib/bpf/libbpf.map b/tools/lib/bpf/libbpf.map
index 08ab2ea881fb..a811a1b3a085 100644
--- a/tools/lib/bpf/libbpf.map
+++ b/tools/lib/bpf/libbpf.map
@@ -458,7 +458,9 @@ LIBBPF_1.7.0 {
 
 LIBBPF_1.8.0 {
 	global:
+		bpf_program__add_flags;
 		bpf_program__attach_tracing_multi;
+		bpf_program__clear_flags;
 		bpf_program__clone;
 		btf__find_by_name_kind_own;
 		btf__new_empty_opts;
-- 
2.55.0


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH bpf-next 2/4] selftests/bpf: Add assertions for bpf_program__{add,clear}_flags()
  2026-09-03  9:24 [PATCH bpf-next 1/4] libbpf: Add bpf_program__add_flags() and bpf_program__clear_flags() Toke Høiland-Jørgensen
@ 2026-09-03  9:24 ` Toke Høiland-Jørgensen
  2026-09-03  9:32   ` sashiko-bot
  2026-09-03  9:24 ` [PATCH bpf-next 3/4] selftests/bpf: Adopt bpf_program__add_flags() helper Toke Høiland-Jørgensen
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 6+ messages in thread
From: Toke Høiland-Jørgensen @ 2026-09-03  9:24 UTC (permalink / raw)
  To: linux-kernel
  Cc: Toke Høiland-Jørgensen, Alexei Starovoitov,
	Daniel Borkmann, Andrii Nakryiko, Eduard Zingerman,
	Kumar Kartikeya Dwivedi, Martin KaFai Lau, Song Liu,
	Yonghong Song, Jiri Olsa, Emil Tsalapatis, Ihor Solodrai,
	Shuah Khan, bpf

Add assertions that round-tripping through bpf_program__add_flags() and
bpf_program__clear_flags() ends up with the original flags value.
Arbitrarily add these to the kernel_flag test prog since that's where
we're already checking for the bpf_program__flags() value (and the test
name contains "flag").

Signed-off-by: Toke Høiland-Jørgensen <toke@redhat.com>
---
 .../selftests/bpf/prog_tests/kernel_flag.c    | 20 +++++++++++++++++--
 1 file changed, 18 insertions(+), 2 deletions(-)

diff --git a/tools/testing/selftests/bpf/prog_tests/kernel_flag.c b/tools/testing/selftests/bpf/prog_tests/kernel_flag.c
index 25eb59f460ab..72c0ef8da409 100644
--- a/tools/testing/selftests/bpf/prog_tests/kernel_flag.c
+++ b/tools/testing/selftests/bpf/prog_tests/kernel_flag.c
@@ -10,15 +10,31 @@ void test_kernel_flag(void)
 	struct test_kernel_flag *lsm_skel;
 	struct kfunc_call_test *skel = NULL;
 	struct kfunc_call_test_lskel *lskel = NULL;
+	struct bpf_program *prog;
+	__u32 flags;
 	int ret;
 
-	lsm_skel = test_kernel_flag__open_and_load();
+	lsm_skel = test_kernel_flag__open();
 	if (!ASSERT_OK_PTR(lsm_skel, "lsm_skel"))
 		return;
 
-	ASSERT_EQ(bpf_program__flags(lsm_skel->progs.bpf) & BPF_F_SLEEPABLE,
+	prog = lsm_skel->progs.bpf;
+	flags = bpf_program__flags(prog);
+	ASSERT_EQ(flags & BPF_F_SLEEPABLE,
 		  BPF_F_SLEEPABLE, "sleepable in program flags");
 
+	ret = bpf_program__add_flags(prog, BPF_F_ANY_ALIGNMENT);
+	ASSERT_OK(ret, "bpf_program__add_flags ret");
+	ASSERT_EQ(bpf_program__flags(prog), flags | BPF_F_ANY_ALIGNMENT,
+		  "bpf_program__add_flags value");
+
+	ret = bpf_program__clear_flags(prog, BPF_F_ANY_ALIGNMENT);
+	ASSERT_OK(ret, "bpf_program__clear_flags ret");
+	ASSERT_EQ(bpf_program__flags(prog), flags, "bpf_program__clear_flags value");
+
+	ret = test_kernel_flag__load(lsm_skel);
+	ASSERT_OK(ret, "test_kernel_flag__load");
+
 	lsm_skel->bss->monitored_tid = sys_gettid();
 
 	ret = test_kernel_flag__attach(lsm_skel);
-- 
2.55.0


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH bpf-next 3/4] selftests/bpf: Adopt bpf_program__add_flags() helper
  2026-09-03  9:24 [PATCH bpf-next 1/4] libbpf: Add bpf_program__add_flags() and bpf_program__clear_flags() Toke Høiland-Jørgensen
  2026-09-03  9:24 ` [PATCH bpf-next 2/4] selftests/bpf: Add assertions for bpf_program__{add,clear}_flags() Toke Høiland-Jørgensen
@ 2026-09-03  9:24 ` Toke Høiland-Jørgensen
  2026-09-03  9:24 ` [PATCH bpf-next 4/4] bpftool: " Toke Høiland-Jørgensen
  2026-09-03  9:30 ` [PATCH bpf-next 1/4] libbpf: Add bpf_program__add_flags() and bpf_program__clear_flags() sashiko-bot
  3 siblings, 0 replies; 6+ messages in thread
From: Toke Høiland-Jørgensen @ 2026-09-03  9:24 UTC (permalink / raw)
  To: Alexei Starovoitov, Daniel Borkmann, David S. Miller,
	Jakub Kicinski, Jesper Dangaard Brouer, John Fastabend,
	Stanislav Fomichev, Andrii Nakryiko, Eduard Zingerman,
	Kumar Kartikeya Dwivedi, Martin KaFai Lau, Song Liu,
	Yonghong Song, Jiri Olsa, Emil Tsalapatis, Ihor Solodrai
  Cc: Toke Høiland-Jørgensen, Shuah Khan, bpf, netdev

Adopt the newly added bpf_program__add_flags() helper everywhere the
selftests non-destructively modifies program flags (which turns out to
be every use of bpf_program__set_flags() in the tests).

Signed-off-by: Toke Høiland-Jørgensen <toke@redhat.com>
---
 tools/testing/selftests/bpf/prog_tests/attach_probe.c      | 4 +---
 tools/testing/selftests/bpf/prog_tests/bpf_verif_scale.c   | 2 +-
 tools/testing/selftests/bpf/prog_tests/kprobe_multi_test.c | 3 +--
 tools/testing/selftests/bpf/prog_tests/xdp_metadata.c      | 4 ++--
 tools/testing/selftests/bpf/test_loader.c                  | 5 ++---
 tools/testing/selftests/bpf/testing_helpers.c              | 4 +---
 tools/testing/selftests/bpf/veristat.c                     | 4 ++--
 tools/testing/selftests/bpf/xdp_hw_metadata.c              | 2 +-
 8 files changed, 11 insertions(+), 17 deletions(-)

diff --git a/tools/testing/selftests/bpf/prog_tests/attach_probe.c b/tools/testing/selftests/bpf/prog_tests/attach_probe.c
index 7dadb90e7b68..41136e1c7752 100644
--- a/tools/testing/selftests/bpf/prog_tests/attach_probe.c
+++ b/tools/testing/selftests/bpf/prog_tests/attach_probe.c
@@ -543,9 +543,7 @@ static void test_kprobe_sleepable(void)
 		return;
 
 	/* sleepable kprobe test case needs flags set before loading */
-	if (!ASSERT_OK(bpf_program__set_flags(
-			       skel->progs.handle_kprobe_sleepable,
-			       bpf_program__flags(skel->progs.handle_kprobe_sleepable) | BPF_F_SLEEPABLE),
+	if (!ASSERT_OK(bpf_program__add_flags(skel->progs.handle_kprobe_sleepable, BPF_F_SLEEPABLE),
 		       "kprobe_sleepable_flags"))
 		goto cleanup;
 
diff --git a/tools/testing/selftests/bpf/prog_tests/bpf_verif_scale.c b/tools/testing/selftests/bpf/prog_tests/bpf_verif_scale.c
index 652307a1b22e..902a70d5afeb 100644
--- a/tools/testing/selftests/bpf/prog_tests/bpf_verif_scale.c
+++ b/tools/testing/selftests/bpf/prog_tests/bpf_verif_scale.c
@@ -35,7 +35,7 @@ static int check_load(const char *file, enum bpf_prog_type type)
 	}
 
 	bpf_program__set_type(prog, type);
-	bpf_program__set_flags(prog, bpf_program__flags(prog) | testing_prog_flags());
+	bpf_program__add_flags(prog, testing_prog_flags());
 	bpf_program__set_log_level(prog, 4 | extra_prog_load_log_flags);
 
 	err = bpf_object__load(obj);
diff --git a/tools/testing/selftests/bpf/prog_tests/kprobe_multi_test.c b/tools/testing/selftests/bpf/prog_tests/kprobe_multi_test.c
index ed3fd0a88dab..651123cd8602 100644
--- a/tools/testing/selftests/bpf/prog_tests/kprobe_multi_test.c
+++ b/tools/testing/selftests/bpf/prog_tests/kprobe_multi_test.c
@@ -361,8 +361,7 @@ static void test_attach_api_fails(void)
 
 	sl_skel->bss->user_ptr = sl_skel;
 
-	err = bpf_program__set_flags(sl_skel->progs.handle_kprobe_multi_sleepable,
-				     bpf_program__flags(sl_skel->progs.handle_kprobe_multi_sleepable) | BPF_F_SLEEPABLE);
+	err = bpf_program__add_flags(sl_skel->progs.handle_kprobe_multi_sleepable, BPF_F_SLEEPABLE);
 	if (!ASSERT_OK(err, "sleep_skel_set_flags"))
 		goto cleanup;
 
diff --git a/tools/testing/selftests/bpf/prog_tests/xdp_metadata.c b/tools/testing/selftests/bpf/prog_tests/xdp_metadata.c
index 047dfdc322a2..3ab963458903 100644
--- a/tools/testing/selftests/bpf/prog_tests/xdp_metadata.c
+++ b/tools/testing/selftests/bpf/prog_tests/xdp_metadata.c
@@ -408,14 +408,14 @@ void test_xdp_metadata(void)
 
 	prog = bpf_object__find_program_by_name(bpf_obj->obj, "rx");
 	bpf_program__set_ifindex(prog, rx_ifindex);
-	bpf_program__set_flags(prog, bpf_program__flags(prog) | BPF_F_XDP_DEV_BOUND_ONLY);
+	bpf_program__add_flags(prog, BPF_F_XDP_DEV_BOUND_ONLY);
 
 	/* Make sure we can load a dev-bound program that performs
 	 * XDP_REDIRECT into a devmap.
 	 */
 	new_prog = bpf_object__find_program_by_name(bpf_obj->obj, "redirect");
 	bpf_program__set_ifindex(new_prog, rx_ifindex);
-	bpf_program__set_flags(new_prog, bpf_program__flags(new_prog) | BPF_F_XDP_DEV_BOUND_ONLY);
+	bpf_program__add_flags(new_prog, BPF_F_XDP_DEV_BOUND_ONLY);
 
 	if (!ASSERT_OK(xdp_metadata__load(bpf_obj), "load skeleton"))
 		goto out;
diff --git a/tools/testing/selftests/bpf/test_loader.c b/tools/testing/selftests/bpf/test_loader.c
index 794a7dfb0579..28724de06322 100644
--- a/tools/testing/selftests/bpf/test_loader.c
+++ b/tools/testing/selftests/bpf/test_loader.c
@@ -748,7 +748,7 @@ static void prepare_case(struct test_loader *tester,
 			 struct bpf_object *obj,
 			 struct bpf_program *prog)
 {
-	int min_log_level = 0, prog_flags;
+	int min_log_level = 0;
 
 	if (env.verbosity > VERBOSE_NONE)
 		min_log_level = 1;
@@ -766,8 +766,7 @@ static void prepare_case(struct test_loader *tester,
 	else
 		bpf_program__set_log_level(prog, spec->log_level);
 
-	prog_flags = bpf_program__flags(prog);
-	bpf_program__set_flags(prog, prog_flags | spec->prog_flags);
+	bpf_program__add_flags(prog, spec->prog_flags);
 
 	tester->log_buf[0] = '\0';
 }
diff --git a/tools/testing/selftests/bpf/testing_helpers.c b/tools/testing/selftests/bpf/testing_helpers.c
index 3f037949e978..d1d60451c5bc 100644
--- a/tools/testing/selftests/bpf/testing_helpers.c
+++ b/tools/testing/selftests/bpf/testing_helpers.c
@@ -292,7 +292,6 @@ int bpf_prog_test_load(const char *file, enum bpf_prog_type type,
 	);
 	struct bpf_object *obj;
 	struct bpf_program *prog;
-	__u32 flags;
 	int err;
 
 	obj = bpf_object__open_file(file, &opts);
@@ -308,8 +307,7 @@ int bpf_prog_test_load(const char *file, enum bpf_prog_type type,
 	if (type != BPF_PROG_TYPE_UNSPEC && bpf_program__type(prog) != type)
 		bpf_program__set_type(prog, type);
 
-	flags = bpf_program__flags(prog) | testing_prog_flags();
-	bpf_program__set_flags(prog, flags);
+	bpf_program__add_flags(prog, testing_prog_flags());
 
 	err = bpf_object__load(obj);
 	if (err)
diff --git a/tools/testing/selftests/bpf/veristat.c b/tools/testing/selftests/bpf/veristat.c
index e70741c6b9b7..9cfc9b4b41c1 100644
--- a/tools/testing/selftests/bpf/veristat.c
+++ b/tools/testing/selftests/bpf/veristat.c
@@ -1722,9 +1722,9 @@ static int process_prog(const char *filename, struct bpf_object *obj, struct bpf
 	fixup_obj(obj, prog, base_filename);
 
 	if (env.force_checkpoints)
-		bpf_program__set_flags(prog, bpf_program__flags(prog) | BPF_F_TEST_STATE_FREQ);
+		bpf_program__add_flags(prog, BPF_F_TEST_STATE_FREQ);
 	if (env.force_reg_invariants)
-		bpf_program__set_flags(prog, bpf_program__flags(prog) | BPF_F_TEST_REG_INVARIANTS);
+		bpf_program__add_flags(prog, BPF_F_TEST_REG_INVARIANTS);
 
 	opts.log_buf = buf;
 	opts.log_size = buf_sz;
diff --git a/tools/testing/selftests/bpf/xdp_hw_metadata.c b/tools/testing/selftests/bpf/xdp_hw_metadata.c
index c5501b3fdf48..ab3f5fc56e86 100644
--- a/tools/testing/selftests/bpf/xdp_hw_metadata.c
+++ b/tools/testing/selftests/bpf/xdp_hw_metadata.c
@@ -845,7 +845,7 @@ int main(int argc, char *argv[])
 
 	prog = bpf_object__find_program_by_name(bpf_obj->obj, "rx");
 	bpf_program__set_ifindex(prog, ifindex);
-	bpf_program__set_flags(prog, bpf_program__flags(prog) | BPF_F_XDP_DEV_BOUND_ONLY);
+	bpf_program__add_flags(prog, BPF_F_XDP_DEV_BOUND_ONLY);
 
 	printf("load bpf program...\n");
 	ret = xdp_hw_metadata__load(bpf_obj);
-- 
2.55.0


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH bpf-next 4/4] bpftool: Adopt bpf_program__add_flags() helper
  2026-09-03  9:24 [PATCH bpf-next 1/4] libbpf: Add bpf_program__add_flags() and bpf_program__clear_flags() Toke Høiland-Jørgensen
  2026-09-03  9:24 ` [PATCH bpf-next 2/4] selftests/bpf: Add assertions for bpf_program__{add,clear}_flags() Toke Høiland-Jørgensen
  2026-09-03  9:24 ` [PATCH bpf-next 3/4] selftests/bpf: Adopt bpf_program__add_flags() helper Toke Høiland-Jørgensen
@ 2026-09-03  9:24 ` Toke Høiland-Jørgensen
  2026-09-03  9:30 ` [PATCH bpf-next 1/4] libbpf: Add bpf_program__add_flags() and bpf_program__clear_flags() sashiko-bot
  3 siblings, 0 replies; 6+ messages in thread
From: Toke Høiland-Jørgensen @ 2026-09-03  9:24 UTC (permalink / raw)
  To: Quentin Monnet
  Cc: Toke Høiland-Jørgensen, Alexei Starovoitov,
	Daniel Borkmann, Andrii Nakryiko, Eduard Zingerman,
	Kumar Kartikeya Dwivedi, Martin KaFai Lau, Song Liu,
	Yonghong Song, Jiri Olsa, Emil Tsalapatis, Ihor Solodrai, bpf

Adopt the newly added bpf_program__add_flags() helper to
non-destructively add the BPF_F_XDP_DEV_BOUND_ONLY flag to XDP programs.

Signed-off-by: Toke Høiland-Jørgensen <toke@redhat.com>
---
 tools/bpf/bpftool/prog.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/bpf/bpftool/prog.c b/tools/bpf/bpftool/prog.c
index 8c2f9255b36d..6ab911e82155 100644
--- a/tools/bpf/bpftool/prog.c
+++ b/tools/bpf/bpftool/prog.c
@@ -1769,7 +1769,7 @@ static int load_with_options(int argc, char **argv, bool first_prog_only)
 		}
 
 		if (prog_type == BPF_PROG_TYPE_XDP && xdpmeta_ifindex) {
-			bpf_program__set_flags(pos, bpf_program__flags(pos) | BPF_F_XDP_DEV_BOUND_ONLY);
+			bpf_program__add_flags(pos, BPF_F_XDP_DEV_BOUND_ONLY);
 			bpf_program__set_ifindex(pos, xdpmeta_ifindex);
 		} else {
 			bpf_program__set_ifindex(pos, offload_ifindex);
-- 
2.55.0


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH bpf-next 1/4] libbpf: Add bpf_program__add_flags() and bpf_program__clear_flags()
  2026-09-03  9:24 [PATCH bpf-next 1/4] libbpf: Add bpf_program__add_flags() and bpf_program__clear_flags() Toke Høiland-Jørgensen
                   ` (2 preceding siblings ...)
  2026-09-03  9:24 ` [PATCH bpf-next 4/4] bpftool: " Toke Høiland-Jørgensen
@ 2026-09-03  9:30 ` sashiko-bot
  3 siblings, 0 replies; 6+ messages in thread
From: sashiko-bot @ 2026-09-03  9:30 UTC (permalink / raw)
  To: Toke Høiland-Jørgensen; +Cc: bpf

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] The newly added multi-line comment in `libbpf.h` violates the required BPF subsystem comment style.
--

commit 0677df91f2b2dedb24874db25663854a4619dfdd
Author: Toke Høiland-Jørgensen <toke@redhat.com>

libbpf: Add bpf_program__add_flags() and bpf_program__clear_flags()

This commit introduces two convenience helpers, bpf_program__add_flags() and
bpf_program__clear_flags(), to non-destructively add or remove BPF program
flags. These wrap the existing bpf_program__set_flags() using bitwise
operations to avoid clobbering existing flags.

> diff --git a/tools/lib/bpf/libbpf.h b/tools/lib/bpf/libbpf.h
> index b965ad5715403..3932bf9cb4903 100644
> --- a/tools/lib/bpf/libbpf.h
> +++ b/tools/lib/bpf/libbpf.h
[ ... ]
> @@ -1011,6 +1011,12 @@ bpf_program__set_expected_attach_type(struct bpf_program *prog,
>  LIBBPF_API __u32 bpf_program__flags(const struct bpf_program *prog);
>  LIBBPF_API int bpf_program__set_flags(struct bpf_program *prog, __u32 flags);
>  
> +/* Convenience helpers to non-destructively add or clear the specified flags
> + * instead of clobbering them as bpf_program__set_flags() does.
> + */

[Severity: Low]
This isn't a bug, but does this newly added multi-line comment violate the BPF
subsystem comment style? In the BPF subsystem, multi-line comments should have
the opening /* on its own line, with the comment text starting on the next
line.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260903092416.223294-1-toke@redhat.com?part=1

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH bpf-next 2/4] selftests/bpf: Add assertions for bpf_program__{add,clear}_flags()
  2026-09-03  9:24 ` [PATCH bpf-next 2/4] selftests/bpf: Add assertions for bpf_program__{add,clear}_flags() Toke Høiland-Jørgensen
@ 2026-09-03  9:32   ` sashiko-bot
  0 siblings, 0 replies; 6+ messages in thread
From: sashiko-bot @ 2026-09-03  9:32 UTC (permalink / raw)
  To: Toke Høiland-Jørgensen; +Cc: bpf

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Missing early exit on failure to load BPF skeleton or modify flags, causing the test to continue execution on error.
--

commit bedacf91bd94531ce369ba064e4bc6158bdcecdb
Author: Toke Høiland-Jørgensen <toke@redhat.com>

selftests/bpf: Add assertions for bpf_program__{add,clear}_flags()

This commit adds tests to ensure that bpf_program__add_flags() and
bpf_program__clear_flags() correctly modify and restore program flags.
It inserts these checks into the existing kernel_flag test program.

> diff --git a/tools/testing/selftests/bpf/prog_tests/kernel_flag.c b/tools/testing/selftests/bpf/prog_tests/kernel_flag.c
> index 25eb59f460abe..72c0ef8da409e 100644
> --- a/tools/testing/selftests/bpf/prog_tests/kernel_flag.c
> +++ b/tools/testing/selftests/bpf/prog_tests/kernel_flag.c
[ ... ]
> @@ -10,15 +10,31 @@ void test_kernel_flag(void)
>  	struct test_kernel_flag *lsm_skel;
>  	struct kfunc_call_test *skel = NULL;
>  	struct kfunc_call_test_lskel *lskel = NULL;
> +	struct bpf_program *prog;
> +	__u32 flags;
>  	int ret;
>  
> -	lsm_skel = test_kernel_flag__open_and_load();
> +	lsm_skel = test_kernel_flag__open();
>  	if (!ASSERT_OK_PTR(lsm_skel, "lsm_skel"))
>  		return;
>  
> -	ASSERT_EQ(bpf_program__flags(lsm_skel->progs.bpf) & BPF_F_SLEEPABLE,
> +	prog = lsm_skel->progs.bpf;
> +	flags = bpf_program__flags(prog);
> +	ASSERT_EQ(flags & BPF_F_SLEEPABLE,
>  		  BPF_F_SLEEPABLE, "sleepable in program flags");
>  
> +	ret = bpf_program__add_flags(prog, BPF_F_ANY_ALIGNMENT);
> +	ASSERT_OK(ret, "bpf_program__add_flags ret");

[Severity: Medium]
Should this assertion be wrapped in an if (!...) condition to short-circuit
execution on failure?

If bpf_program__add_flags() fails, the test will log the error but continue
executing with an invalid state. A clean abort here could prevent cascading
failures in the test output.

> +	ASSERT_EQ(bpf_program__flags(prog), flags | BPF_F_ANY_ALIGNMENT,
> +		  "bpf_program__add_flags value");
> +
> +	ret = bpf_program__clear_flags(prog, BPF_F_ANY_ALIGNMENT);
> +	ASSERT_OK(ret, "bpf_program__clear_flags ret");

[Severity: Medium]
Similarly, does this need an early exit if clearing flags fails?

> +	ASSERT_EQ(bpf_program__flags(prog), flags, "bpf_program__clear_flags value");
> +
> +	ret = test_kernel_flag__load(lsm_skel);
> +	ASSERT_OK(ret, "test_kernel_flag__load");

[Severity: Medium]
If the skeleton fails to load, this standalone ASSERT_OK() allows the test to
continue execution to lsm_skel->bss->monitored_tid = sys_gettid() and
test_kernel_flag__attach(). 

Could this cause the test to crash or produce noisy cascading failures?
Wrapping this in if (!ASSERT_OK(...)) goto close_prog; would ensure we exit
cleanly if the load fails, similar to how the attach step is handled later in
the function.

>  
>  	lsm_skel->bss->monitored_tid = sys_gettid();
>  
>  	ret = test_kernel_flag__attach(lsm_skel);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260903092416.223294-1-toke@redhat.com?part=2

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2026-09-03  9:32 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-09-03  9:24 [PATCH bpf-next 1/4] libbpf: Add bpf_program__add_flags() and bpf_program__clear_flags() Toke Høiland-Jørgensen
2026-09-03  9:24 ` [PATCH bpf-next 2/4] selftests/bpf: Add assertions for bpf_program__{add,clear}_flags() Toke Høiland-Jørgensen
2026-09-03  9:32   ` sashiko-bot
2026-09-03  9:24 ` [PATCH bpf-next 3/4] selftests/bpf: Adopt bpf_program__add_flags() helper Toke Høiland-Jørgensen
2026-09-03  9:24 ` [PATCH bpf-next 4/4] bpftool: " Toke Høiland-Jørgensen
2026-09-03  9:30 ` [PATCH bpf-next 1/4] libbpf: Add bpf_program__add_flags() and bpf_program__clear_flags() sashiko-bot

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox