Netdev List
 help / color / mirror / Atom feed
* [PATCH bpf-next v2 1/4] libbpf: Move section-defined program flags to prog_flags
@ 2026-08-31 13:26 Toke Høiland-Jørgensen
  2026-08-31 13:26 ` [PATCH bpf-next v2 2/4] bpf: selftests: Check for flag presence in bpf_program__flags() Toke Høiland-Jørgensen
                   ` (2 more replies)
  0 siblings, 3 replies; 9+ messages in thread
From: Toke Høiland-Jørgensen @ 2026-08-31 13:26 UTC (permalink / raw)
  To: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
	Eduard Zingerman, Kumar Kartikeya Dwivedi, Martin KaFai Lau,
	Song Liu, Yonghong Song, Jiri Olsa, Emil Tsalapatis,
	Ihor Solodrai, David S. Miller, Jakub Kicinski,
	Jesper Dangaard Brouer, John Fastabend, Stanislav Fomichev
  Cc: Toke Høiland-Jørgensen, bpf, netdev

The libbpf section definition modifiers for XDP frags support and
sleepable programs stores the flags bits only in the private section
definition cookie from object open to load time. This has the
unfortunate consequence that API consumers cannot see (or manipulate)
the flag between object open and program load.

In particular, libxdp has special handling of frags-enabled programs to
make them compatible with the dispatcher. This doesn't work on XDP
programs that enable frags through the 'xdp.frags' section definition
because the flag is not visible through bpf_program__flags()[0].

Fix this by changing how libbpf loads the program flags from section
definitions: instead of using the private section definition cookie, add
a setup function to the default section definitions that stores the
flags for sleepable and XDP frags programs in the prog_flags field of
struct bpf_program.

Exposing the flags this way means that any use of
bpf_program__set_flags() will override the flags unless the caller takes
care of updating flags in a non-destructive way. This is unavoidable
with the set-only API, and any user setting flags unconditionally is
already broken in the sense that they will also override any other
current and future flags. A subsequent patch fixes up all in-tree users
of the API.

[0] https://github.com/xdp-project/xdp-tools/issues/587

Signed-off-by: Toke Høiland-Jørgensen <toke@redhat.com>
---
v2:
  - Use a generic section setup function that also applies to BPF_F_SLEEPABLE

 tools/lib/bpf/libbpf.c | 20 ++++++++++++++------
 1 file changed, 14 insertions(+), 6 deletions(-)

diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
index b749c01742ee..27779b4cddd0 100644
--- a/tools/lib/bpf/libbpf.c
+++ b/tools/lib/bpf/libbpf.c
@@ -7879,6 +7879,19 @@ static int tracing_multi_mod_fd(struct bpf_program *prog, int *btf_obj_fd)
 	return 0;
 }
 
+static int libbpf_setup_prog_flags(struct bpf_program *prog, long cookie)
+{
+	enum sec_def_flags def = cookie;
+
+	if (def & SEC_SLEEPABLE)
+		prog->prog_flags |= BPF_F_SLEEPABLE;
+
+	if (def & SEC_XDP_FRAGS)
+		prog->prog_flags |= BPF_F_XDP_HAS_FRAGS;
+
+	return 0;
+}
+
 /* this is called as prog->sec_def->prog_prepare_load_fn for libbpf-supported sec_defs */
 static int libbpf_prepare_prog_load(struct bpf_program *prog,
 				    struct bpf_prog_load_opts *opts, long cookie)
@@ -7889,12 +7902,6 @@ static int libbpf_prepare_prog_load(struct bpf_program *prog,
 	if ((def & SEC_EXP_ATTACH_OPT) && !kernel_supports(prog->obj, FEAT_EXP_ATTACH_TYPE))
 		opts->expected_attach_type = 0;
 
-	if (def & SEC_SLEEPABLE)
-		opts->prog_flags |= BPF_F_SLEEPABLE;
-
-	if (prog->type == BPF_PROG_TYPE_XDP && (def & SEC_XDP_FRAGS))
-		opts->prog_flags |= BPF_F_XDP_HAS_FRAGS;
-
 	/* special check for usdt to use uprobe_multi link */
 	if ((def & SEC_USDT) && kernel_supports(prog->obj, FEAT_UPROBE_MULTI_LINK)) {
 		/* for BPF_TRACE_UPROBE_MULTI, user might want to query expected_attach_type
@@ -10099,6 +10106,7 @@ int bpf_program__clone(struct bpf_program *prog, const struct bpf_prog_load_opts
 	.prog_type = BPF_PROG_TYPE_##ptype,				    \
 	.expected_attach_type = atype,					    \
 	.cookie = (long)(flags),					    \
+	.prog_setup_fn = libbpf_setup_prog_flags,			    \
 	.prog_prepare_load_fn = libbpf_prepare_prog_load,		    \
 	__VA_ARGS__							    \
 }
-- 
2.55.0


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

* [PATCH bpf-next v2 2/4] bpf: selftests: Check for flag presence in bpf_program__flags()
  2026-08-31 13:26 [PATCH bpf-next v2 1/4] libbpf: Move section-defined program flags to prog_flags Toke Høiland-Jørgensen
@ 2026-08-31 13:26 ` Toke Høiland-Jørgensen
  2026-08-31 14:18   ` bot+bpf-ci
  2026-08-31 13:26 ` [PATCH bpf-next v2 4/4] selftests/bpf: Set BPF program flags non-destructively Toke Høiland-Jørgensen
  2026-08-31 14:32 ` [PATCH bpf-next v2 1/4] libbpf: Move section-defined program flags to prog_flags bot+bpf-ci
  2 siblings, 1 reply; 9+ messages in thread
From: Toke Høiland-Jørgensen @ 2026-08-31 13:26 UTC (permalink / raw)
  To: Alexei Starovoitov, Daniel Borkmann, David S. Miller,
	Jakub Kicinski, Jesper Dangaard Brouer, John Fastabend,
	Stanislav Fomichev
  Cc: Toke Høiland-Jørgensen, Andrii Nakryiko,
	Eduard Zingerman, Ihor Solodrai, Kumar Kartikeya Dwivedi,
	Martin KaFai Lau, Song Liu, Yonghong Song, Jiri Olsa,
	Emil Tsalapatis, Shuah Khan, bpf, netdev

Add a check that the BPF_F_XDP_HAS_FRAGS and BPF_F_SLEEPABLE flags show
up in bpf_program__flags() when opening a BPF program with the flag
definitions in their section definitions.

Signed-off-by: Toke Høiland-Jørgensen <toke@redhat.com>
---
 tools/testing/selftests/bpf/prog_tests/kernel_flag.c       | 3 +++
 tools/testing/selftests/bpf/prog_tests/xdp_adjust_frags.c  | 3 +++
 tools/testing/selftests/bpf/prog_tests/xdp_devmap_attach.c | 5 +++++
 3 files changed, 11 insertions(+)

diff --git a/tools/testing/selftests/bpf/prog_tests/kernel_flag.c b/tools/testing/selftests/bpf/prog_tests/kernel_flag.c
index 97b00c7efe94..25eb59f460ab 100644
--- a/tools/testing/selftests/bpf/prog_tests/kernel_flag.c
+++ b/tools/testing/selftests/bpf/prog_tests/kernel_flag.c
@@ -16,6 +16,9 @@ void test_kernel_flag(void)
 	if (!ASSERT_OK_PTR(lsm_skel, "lsm_skel"))
 		return;
 
+	ASSERT_EQ(bpf_program__flags(lsm_skel->progs.bpf) & BPF_F_SLEEPABLE,
+		  BPF_F_SLEEPABLE, "sleepable in program flags");
+
 	lsm_skel->bss->monitored_tid = sys_gettid();
 
 	ret = test_kernel_flag__attach(lsm_skel);
diff --git a/tools/testing/selftests/bpf/prog_tests/xdp_adjust_frags.c b/tools/testing/selftests/bpf/prog_tests/xdp_adjust_frags.c
index fce203640f8c..a894b1ab46f4 100644
--- a/tools/testing/selftests/bpf/prog_tests/xdp_adjust_frags.c
+++ b/tools/testing/selftests/bpf/prog_tests/xdp_adjust_frags.c
@@ -18,6 +18,9 @@ static void test_xdp_update_frags(void)
 		return;
 
 	prog = bpf_object__next_program(obj, NULL);
+	ASSERT_EQ(bpf_program__flags(prog) & BPF_F_XDP_HAS_FRAGS,
+		  BPF_F_XDP_HAS_FRAGS, "frags in program flags");
+
 	if (bpf_object__load(obj))
 		return;
 
diff --git a/tools/testing/selftests/bpf/prog_tests/xdp_devmap_attach.c b/tools/testing/selftests/bpf/prog_tests/xdp_devmap_attach.c
index a8ab05216c38..dff6b3e7266e 100644
--- a/tools/testing/selftests/bpf/prog_tests/xdp_devmap_attach.c
+++ b/tools/testing/selftests/bpf/prog_tests/xdp_devmap_attach.c
@@ -146,6 +146,11 @@ static void test_xdp_with_devmap_frags_helpers(void)
 	if (!ASSERT_OK_PTR(skel, "test_xdp_with_devmap_helpers__open_and_load"))
 		return;
 
+	ASSERT_EQ(bpf_program__flags(skel->progs.xdp_dummy_dm_frags) & BPF_F_XDP_HAS_FRAGS,
+		  BPF_F_XDP_HAS_FRAGS, "frags in program flags");
+	ASSERT_EQ(bpf_program__flags(skel->progs.xdp_dummy_dm) & BPF_F_XDP_HAS_FRAGS,
+		  0, "frags not in program flags");
+
 	dm_fd_frags = bpf_program__fd(skel->progs.xdp_dummy_dm_frags);
 	map_fd = bpf_map__fd(skel->maps.dm_ports);
 	err = bpf_prog_get_info_by_fd(dm_fd_frags, &info, &len);
-- 
2.55.0


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

* [PATCH bpf-next v2 4/4] selftests/bpf: Set BPF program flags non-destructively
  2026-08-31 13:26 [PATCH bpf-next v2 1/4] libbpf: Move section-defined program flags to prog_flags Toke Høiland-Jørgensen
  2026-08-31 13:26 ` [PATCH bpf-next v2 2/4] bpf: selftests: Check for flag presence in bpf_program__flags() Toke Høiland-Jørgensen
@ 2026-08-31 13:26 ` Toke Høiland-Jørgensen
  2026-08-31 14:18   ` bot+bpf-ci
  2026-08-31 14:32 ` [PATCH bpf-next v2 1/4] libbpf: Move section-defined program flags to prog_flags bot+bpf-ci
  2 siblings, 1 reply; 9+ messages in thread
From: Toke Høiland-Jørgensen @ 2026-08-31 13:26 UTC (permalink / raw)
  To: Alexei Starovoitov, Daniel Borkmann, David S. Miller,
	Jakub Kicinski, Jesper Dangaard Brouer, John Fastabend,
	Stanislav Fomichev, Andrii Nakryiko, Eduard Zingerman,
	Ihor Solodrai, Kumar Kartikeya Dwivedi, Martin KaFai Lau,
	Song Liu, Yonghong Song, Jiri Olsa, Emil Tsalapatis
  Cc: Toke Høiland-Jørgensen, Shuah Khan, bpf, netdev

A couple of the BPF selftests would set the program flags without
looking at the existing program flags, overriding any other flag values.
Change this to always set the flag value non-destructively by OR'ing it
with the existing flags.

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

diff --git a/tools/testing/selftests/bpf/prog_tests/attach_probe.c b/tools/testing/selftests/bpf/prog_tests/attach_probe.c
index e8c1a619e330..7dadb90e7b68 100644
--- a/tools/testing/selftests/bpf/prog_tests/attach_probe.c
+++ b/tools/testing/selftests/bpf/prog_tests/attach_probe.c
@@ -543,8 +543,10 @@ 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_F_SLEEPABLE), "kprobe_sleepable_flags"))
+	if (!ASSERT_OK(bpf_program__set_flags(
+			       skel->progs.handle_kprobe_sleepable,
+			       bpf_program__flags(skel->progs.handle_kprobe_sleepable) | BPF_F_SLEEPABLE),
+		       "kprobe_sleepable_flags"))
 		goto cleanup;
 
 	if (!ASSERT_OK(test_attach_kprobe_sleepable__load(skel),
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 2e0ddef77ba5..ed3fd0a88dab 100644
--- a/tools/testing/selftests/bpf/prog_tests/kprobe_multi_test.c
+++ b/tools/testing/selftests/bpf/prog_tests/kprobe_multi_test.c
@@ -362,7 +362,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_F_SLEEPABLE);
+				     bpf_program__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 5c31054ad4a4..047dfdc322a2 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_F_XDP_DEV_BOUND_ONLY);
+	bpf_program__set_flags(prog, bpf_program__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_F_XDP_DEV_BOUND_ONLY);
+	bpf_program__set_flags(new_prog, bpf_program__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/xdp_hw_metadata.c b/tools/testing/selftests/bpf/xdp_hw_metadata.c
index 6db3b5555a22..c5501b3fdf48 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_F_XDP_DEV_BOUND_ONLY);
+	bpf_program__set_flags(prog, bpf_program__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] 9+ messages in thread

* Re: [PATCH bpf-next v2 4/4] selftests/bpf: Set BPF program flags non-destructively
  2026-08-31 13:26 ` [PATCH bpf-next v2 4/4] selftests/bpf: Set BPF program flags non-destructively Toke Høiland-Jørgensen
@ 2026-08-31 14:18   ` bot+bpf-ci
  2026-09-01  8:11     ` Toke Høiland-Jørgensen
  0 siblings, 1 reply; 9+ messages in thread
From: bot+bpf-ci @ 2026-08-31 14:18 UTC (permalink / raw)
  To: toke, ast, daniel, davem, kuba, hawk, john.fastabend, sdf, andrii,
	eddyz87, ihor.solodrai, memxor, martin.lau, song, yonghong.song,
	jolsa, emil
  Cc: toke, shuah, bpf, netdev, ast, andrii, daniel, martin.lau,
	eddyz87, yonghong.song, mason, ihor.solodrai

[-- Attachment #1: Type: text/plain, Size: 4210 bytes --]

>     selftests/bpf: Set BPF program flags non-destructively
>
>     A couple of the BPF selftests would set the program flags without
>     looking at the existing program flags, overriding any other flag values.
>     Change this to always set the flag value non-destructively by OR'ing it
>     with the existing flags.
>
>     Signed-off-by: Toke Høiland-Jørgensen <toke@redhat.com>

This commit comes after patch 1 in the series (24627f60026bd, "libbpf:
Move section-defined program flags to prog_flags"). At that earlier
commit, prog_flags already carries the section-derived flags and
libbpf_prepare_prog_load() no longer OR's them in at load time. The
xdp_hw_metadata.c change below sets flags on a SEC("xdp.frags") program:

> diff --git a/tools/testing/selftests/bpf/xdp_hw_metadata.c b/tools/testing/selftests/bpf/xdp_hw_metadata.c
> index 6db3b5555a22e..c5501b3fdf48f 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_F_XDP_DEV_BOUND_ONLY);
> +	bpf_program__set_flags(prog, bpf_program__flags(prog) | BPF_F_XDP_DEV_BOUND_ONLY);

At commit 24627f60026bd, the assignment on the old line overwrites the
BPF_F_XDP_HAS_FRAGS bit and the program is loaded without multi-buffer
support. Patch 4 (this commit) undoes that behaviour change. The same
applies to bpftool and xdp.frags programs between patch 1 and patch 3.

The fixups are no-ops before patch 1 since prog_flags is 0 there, so
moving patches 3 and 4 ahead of the libbpf change would keep every
commit in the series behaviour-preserving. Should the commit message or
series note this dependency? The commit as written reads as an
independent cleanup with no indication that it is required to keep
xdp_hw_metadata working after the libbpf change.

[ ... ]

Patch 1 of this series states "A subsequent patch fixes up all in-tree
users of the API", and this commit is that patch. However,
check_load() in prog_tests/bpf_verif_scale.c still assigns flags
destructively:

	bpf_program__set_flags(prog, testing_prog_flags());

An exhaustive enumeration of the tree after this commit leaves exactly
one destructive caller:

  tools/bpf/bpftool/prog.c:1772
    -> OR'd (patch 3)

  tools/testing/selftests/bpf/testing_helpers.c:311
    -> already OR's: bpf_program__flags(prog) | testing_prog_flags()

  tools/testing/selftests/bpf/test_loader.c:769-770
    -> already OR's: prog_flags = bpf_program__flags(prog)

  tools/testing/selftests/bpf/veristat.c:1725,1727
    -> already OR's

  the four sites fixed by this commit

  tools/testing/selftests/bpf/prog_tests/bpf_verif_scale.c:38
    -> NOT converted

After patch 1, libbpf_setup_prog_flags() stores section-derived
BPF_F_SLEEPABLE or BPF_F_XDP_HAS_FRAGS into prog->prog_flags at
bpf_object_init_progs() time, and libbpf_prepare_prog_load() no longer
re-adds them at load time. A destructive set_flags() therefore now
silently drops them.

The identical fix used everywhere else applies:

	bpf_program__set_flags(prog, bpf_program__flags(prog) | testing_prog_flags())

Note that testing_helpers.c:311 already spells it exactly that way for
the same testing_prog_flags() value.

Impact today is latent, not observable: every object loaded by
bpf_verif_scale (test_verif_scale*, pyperf*, loop*, strobemeta*,
test_sysctl_loop*, test_xdp_loop, test_seg6_loop, twfw) has a first
program in a plain SEC("xdp"), SEC("raw_tracepoint/..."),
SEC("kprobe/..."), SEC("classifier"), SEC("tc"), or SEC("cgroup/...")
section, none of which contributes flags. It becomes a silent behaviour
change the moment a scale-test program is written with SEC("xdp.frags")
or a sleepable section.

Should the bpf_verif_scale.c site be converted as well, or does "all
in-tree users" exclude it for some reason?


---
AI reviewed your patch. Please fix the bug or email reply why it's not a bug.
See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md

CI run summary: https://github.com/kernel-patches/bpf/actions/runs/33398158091

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

* Re: [PATCH bpf-next v2 2/4] bpf: selftests: Check for flag presence in bpf_program__flags()
  2026-08-31 13:26 ` [PATCH bpf-next v2 2/4] bpf: selftests: Check for flag presence in bpf_program__flags() Toke Høiland-Jørgensen
@ 2026-08-31 14:18   ` bot+bpf-ci
  2026-09-01  8:03     ` Toke Høiland-Jørgensen
  0 siblings, 1 reply; 9+ messages in thread
From: bot+bpf-ci @ 2026-08-31 14:18 UTC (permalink / raw)
  To: toke, ast, daniel, davem, kuba, hawk, john.fastabend, sdf
  Cc: toke, andrii, eddyz87, ihor.solodrai, memxor, martin.lau, song,
	yonghong.song, jolsa, emil, shuah, bpf, netdev, ast, andrii,
	daniel, martin.lau, eddyz87, yonghong.song, mason, ihor.solodrai

[-- Attachment #1: Type: text/plain, Size: 724 bytes --]

> bpf: selftests: Check for flag presence in bpf_program__flags()

This isn't a bug, but would it be worth using the same 'selftests/bpf:'
prefix here as in the later patch of this series, so the whole series
reads consistently?

The sibling patch in this series that also touches
tools/testing/selftests/bpf uses 'selftests/bpf:' as its prefix, and
mixing 'bpf: selftests:' and 'selftests/bpf:' for the same directory
within one series makes it harder to scan in git log --oneline.


---
AI reviewed your patch. Please fix the bug or email reply why it's not a bug.
See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md

CI run summary: https://github.com/kernel-patches/bpf/actions/runs/33398158091

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

* Re: [PATCH bpf-next v2 1/4] libbpf: Move section-defined program flags to prog_flags
  2026-08-31 13:26 [PATCH bpf-next v2 1/4] libbpf: Move section-defined program flags to prog_flags Toke Høiland-Jørgensen
  2026-08-31 13:26 ` [PATCH bpf-next v2 2/4] bpf: selftests: Check for flag presence in bpf_program__flags() Toke Høiland-Jørgensen
  2026-08-31 13:26 ` [PATCH bpf-next v2 4/4] selftests/bpf: Set BPF program flags non-destructively Toke Høiland-Jørgensen
@ 2026-08-31 14:32 ` bot+bpf-ci
  2026-09-01  8:30   ` Toke Høiland-Jørgensen
  2 siblings, 1 reply; 9+ messages in thread
From: bot+bpf-ci @ 2026-08-31 14:32 UTC (permalink / raw)
  To: toke, ast, daniel, andrii, eddyz87, memxor, martin.lau, song,
	yonghong.song, jolsa, emil, ihor.solodrai, davem, kuba, hawk,
	john.fastabend, sdf
  Cc: toke, bpf, netdev, ast, andrii, daniel, martin.lau, eddyz87,
	yonghong.song, mason, ihor.solodrai

[-- Attachment #1: Type: text/plain, Size: 10279 bytes --]

> commit 24627f60026bd2b231e09d4373e14ca1975a2d96
> Author: Toke Høiland-Jørgensen <toke@redhat.com>
> Date:   Mon Aug 31 15:26:41 2026 +0200
>
>     libbpf: Move section-defined program flags to prog_flags
>
>     The libbpf section definition modifiers for XDP frags support and
>     sleepable programs stores the flags bits only in the private section
>     definition cookie from object open to load time. This has the
>     unfortunate consequence that API consumers cannot see (or manipulate)
>     the flag between object open and program load.
>
>     In particular, libxdp has special handling of frags-enabled programs to
>     make them compatible with the dispatcher. This doesn't work on XDP
>     programs that enable frags through the 'xdp.frags' section definition
>     because the flag is not visible through bpf_program__flags()[0].
>
>     Fix this by changing how libbpf loads the program flags from section
>     definitions: instead of using the private section definition cookie, add
>     a setup function to the default section definitions that stores the
>     flags for sleepable and XDP frags programs in the prog_flags field of
>     struct bpf_program.
>
>     Exposing the flags this way means that any use of
>     bpf_program__set_flags() will override the flags unless the caller takes
>     care of updating flags in a non-destructive way. This is unavoidable
>     with the set-only API, and any user setting flags unconditionally is
>     already broken in the sense that they will also override any other
>     current and future flags. A subsequent patch fixes up all in-tree users
>     of the API.

This looks like a bug fix for SEC(\"xdp.frags\") flag visibility. Should
this include:

  Fixes: 082c4bfba4f7 ("libbpf: Add SEC name for xdp frags programs")

The commit message states "A subsequent patch fixes up all in-tree users
of the API." Is this statement accurate?

Looking at the end of the series (645bdf199b3cb), there is one destructive
bpf_program__set_flags() call site that remains unconverted:

    tools/testing/selftests/bpf/prog_tests/bpf_verif_scale.c:38
        bpf_program__set_flags(prog, testing_prog_flags());

All other in-tree call sites are either converted to non-destructive
(OR-based) form or were already non-destructive. The verif_scale case
happens not to change behaviour today because check_load() calls
bpf_program__set_type() first, but should the changelog's "all in-tree
users" claim be softened or the call site converted for consistency?

> diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
> index f09cbfd8e729d..c036e8a91ed88 100644
> --- a/tools/lib/bpf/libbpf.c
> +++ b/tools/lib/bpf/libbpf.c
> @@ -7879,6 +7879,19 @@ static int tracing_multi_mod_fd(struct bpf_program *prog, int *btf_obj_fd)
>  	return 0;
>  }
>
> +static int libbpf_setup_prog_flags(struct bpf_program *prog, long cookie)
> +{
> +	enum sec_def_flags def = cookie;
> +
> +	if (def & SEC_SLEEPABLE)
> +		prog->prog_flags |= BPF_F_SLEEPABLE;
> +
> +	if (def & SEC_XDP_FRAGS)
> +		prog->prog_flags |= BPF_F_XDP_HAS_FRAGS;
> +
> +	return 0;
> +}
> +

The SEC_XDP_FRAGS handling here removed the prog->type == BPF_PROG_TYPE_XDP
guard that was present in the old code. The old libbpf_prepare_prog_load()
checked:

    if (prog->type == BPF_PROG_TYPE_XDP && (def & SEC_XDP_FRAGS))
        opts->prog_flags |= BPF_F_XDP_HAS_FRAGS;

At setup time this is harmless because bpf_object_init_progs() sets
prog->type = sec_def->prog_type immediately before invoking prog_setup_fn,
and all SEC_XDP_FRAGS entries declare BPF_PROG_TYPE_XDP.

However, moving the flag to open time makes it survive a later program-type
change via bpf_program__set_type(). The old code applied the flags from
prog_prepare_load_fn (reached only via prog->sec_def), so changing the type
used to drop the SEC-derived flags. Now they persist:

  - SEC("xdp.frags") + bpf_program__set_type(prog, BPF_PROG_TYPE_SCHED_CLS)
    + a non-zero ifindex now hits kernel/bpf/offload.c:234 checking
    attr->prog_flags & BPF_F_XDP_HAS_FRAGS without
    BPF_F_XDP_DEV_BOUND_ONLY -> -EINVAL, where the load previously succeeded.

  - A sleepable section (uprobe.s/fentry.s/lsm.s/tp_btf.s/syscall/struct_ops.s)
    whose type is changed to one that cannot be sleepable now hits
    kernel/bpf/verifier.c:20607 if (prog->sleepable && !can_be_sleepable(prog))
    -> -EINVAL ("Program of this type cannot be sleepable"), where it
    previously loaded.

This is reachable from bpftool's load_with_options():

    if (bpf_program__type(pos) != prog_type)
        bpf_program__set_type(pos, prog_type);

and from tools/testing/selftests/bpf/testing_helpers.c:309-312
bpf_prog_test_load().

Is keeping the flags across a type change intended? If so, that seems worth
noting in the changelog. If not, should the type check be kept or should
bpf_program__set_type() clear the SEC-derived bits?

>  /* this is called as prog->sec_def->prog_prepare_load_fn for libbpf-supported sec_defs */
>  static int libbpf_prepare_prog_load(struct bpf_program *prog,
>  				    struct bpf_prog_load_opts *opts, long cookie)
> @@ -7889,12 +7902,6 @@ static int libbpf_prepare_prog_load(struct bpf_program *prog,
>  	if ((def & SEC_EXP_ATTACH_OPT) && !kernel_supports(prog->obj, FEAT_EXP_ATTACH_TYPE))
>  		opts->expected_attach_type = 0;
>
> -	if (def & SEC_SLEEPABLE)
> -		opts->prog_flags |= BPF_F_SLEEPABLE;
> -
> -	if (prog->type == BPF_PROG_TYPE_XDP && (def & SEC_XDP_FRAGS))
> -		opts->prog_flags |= BPF_F_XDP_HAS_FRAGS;
> -

Moving the SEC()-derived flags from the load-time opts into prog->prog_flags
converts every destructive bpf_program__set_flags() call into a silent
flag-clobber. This commit lands before the in-tree users are fixed, so the
tree is broken at this commit (bisection hazard).

Old behaviour in libbpf_prepare_prog_load(): the SEC flags were OR'ed into
the load opts unconditionally at load time, so they survived any
bpf_program__set_flags() the application had made:

    if (def & SEC_SLEEPABLE)
        opts->prog_flags |= BPF_F_SLEEPABLE;
    if (prog->type == BPF_PROG_TYPE_XDP && (def & SEC_XDP_FRAGS))
        opts->prog_flags |= BPF_F_XDP_HAS_FRAGS;

New behaviour: the flags live in prog->prog_flags (set once at open in
bpf_object_init_progs()), and bpf_program__set_flags() does a plain
assignment:

    prog->prog_flags = flags;

bpf_object_load_prog() then passes exactly that:

    load_attr.prog_flags = prog->prog_flags;

with nothing re-adding the SEC bits.

Concrete in-tree regression at THIS commit (bpftool, production code):

  tools/bpf/bpftool/prog.c:1772 in load_with_options():

      if (prog_type == BPF_PROG_TYPE_XDP && xdpmeta_ifindex) {
          bpf_program__set_flags(pos, BPF_F_XDP_DEV_BOUND_ONLY);

  For bpftool prog load <obj with SEC("xdp.frags")> ... xdpmeta_dev <ifname>:

    - before: prog_flags = BPF_F_XDP_DEV_BOUND_ONLY, then prepare_load OR'ed
      in BPF_F_XDP_HAS_FRAGS -> kernel/bpf/syscall.c:3062 sets
      prog->aux->xdp_has_frags = true.

    - after: prog_flags = BPF_F_XDP_DEV_BOUND_ONLY only -> xdp_has_frags =
      false. The program silently loses multi-buffer support even though its
      section requested it.

  This is fixed two commits later by bd6f72f438f0a ("bpftool: Set
  BPF_F_XDP_DEV_BOUND_ONLY flag non-destructively"), so the library change
  precedes the user fix-ups.

The same window breaks
tools/testing/selftests/bpf/xdp_hw_metadata.c:848, whose BPF program is
SEC("xdp.frags") (tools/testing/selftests/bpf/progs/xdp_hw_metadata.c:27);
fixed later by 645bdf199b3cb.

For out-of-tree consumers the failure can be hard, not silent: a
destructive bpf_program__set_flags() on a SEC("syscall") program now clears
BPF_F_SLEEPABLE, and kernel/bpf/verifier.c:20602 rejects that outright
("Syscall programs can only be sleepable", -EINVAL). Same for
sleepable-only LSM/uprobe attach points.

Should the in-tree user fix-ups land before (or be folded into) this commit
to avoid the bisection hazard? The usual kernel ordering is to fix the
users first so that no intermediate commit regresses.

> @@ -10099,6 +10106,7 @@ int bpf_program__clone(struct bpf_program *prog, const struct bpf_prog_load_opts
>  	.prog_type = BPF_PROG_TYPE_##ptype,				    \
>  	.expected_attach_type = atype,					    \
>  	.cookie = (long)(flags),					    \
> +	.prog_setup_fn = libbpf_setup_prog_flags,			    \
>  	.prog_prepare_load_fn = libbpf_prepare_load_fn,		    \
>  	__VA_ARGS__							    \
>  }

Does bpf_program__clone() lose the SEC()-derived flags whenever the caller
supplies a non-zero opts->prog_flags?

Before this patch, bpf_program__clone() got the SEC flags back from the
sec_def callback it invokes:

    if (prog->sec_def && prog->sec_def->prog_prepare_load_fn)
        err = prog->sec_def->prog_prepare_load_fn(prog, &attr,
                                                   prog->sec_def->cookie);

which used to do opts->prog_flags |= BPF_F_SLEEPABLE / |= BPF_F_XDP_HAS_FRAGS.
So even when the caller passed its own prog_flags, a SEC("fentry.s/...") or
SEC("xdp.frags") program was still cloned with the right bit set.

After this patch libbpf_prepare_prog_load() no longer touches
opts->prog_flags, so this line in bpf_program__clone():

    attr.prog_flags = OPTS_GET(opts, prog_flags, 0) ?: prog->prog_flags;

discards prog->prog_flags entirely as soon as the caller sets any flag of
its own. Cloning a sleepable program with, say, opts.prog_flags =
BPF_F_TEST_STATE_FREQ now loads it without BPF_F_SLEEPABLE, which
kernel/bpf/verifier.c:20602-20610 rejects for BPF_PROG_TYPE_SYSCALL and for
sleepable-only LSM/tracing attach points (-EINVAL).

No in-tree caller triggers this today: the only opts-passing caller,
process_prog() in tools/testing/selftests/bpf/veristat.c:1730-1737, leaves
opts.prog_flags at 0, and none of the follow-up commits in this series add
such a caller. Would it be safer to OR the caller's flags into
prog->prog_flags, or is the all-or-nothing fallback intended to let callers
override SEC-derived flags explicitly?


---
AI reviewed your patch. Please fix the bug or email reply why it's not a bug.
See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md

CI run summary: https://github.com/kernel-patches/bpf/actions/runs/33398158091

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

* Re: [PATCH bpf-next v2 2/4] bpf: selftests: Check for flag presence in bpf_program__flags()
  2026-08-31 14:18   ` bot+bpf-ci
@ 2026-09-01  8:03     ` Toke Høiland-Jørgensen
  0 siblings, 0 replies; 9+ messages in thread
From: Toke Høiland-Jørgensen @ 2026-09-01  8:03 UTC (permalink / raw)
  To: bot+bpf-ci, ast, daniel, davem, kuba, hawk, john.fastabend, sdf
  Cc: andrii, eddyz87, ihor.solodrai, memxor, martin.lau, song,
	yonghong.song, jolsa, emil, shuah, bpf, netdev, ast, andrii,
	daniel, martin.lau, eddyz87, yonghong.song, mason, ihor.solodrai

bot+bpf-ci@kernel.org writes:

>> bpf: selftests: Check for flag presence in bpf_program__flags()
>
> This isn't a bug, but would it be worth using the same 'selftests/bpf:'
> prefix here as in the later patch of this series, so the whole series
> reads consistently?
>
> The sibling patch in this series that also touches
> tools/testing/selftests/bpf uses 'selftests/bpf:' as its prefix, and
> mixing 'bpf: selftests:' and 'selftests/bpf:' for the same directory
> within one series makes it harder to scan in git log --oneline.

Meh. Won't respin for this, but I can fix this if I end up respinning
for other reasons.

-Toke


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

* Re: [PATCH bpf-next v2 4/4] selftests/bpf: Set BPF program flags non-destructively
  2026-08-31 14:18   ` bot+bpf-ci
@ 2026-09-01  8:11     ` Toke Høiland-Jørgensen
  0 siblings, 0 replies; 9+ messages in thread
From: Toke Høiland-Jørgensen @ 2026-09-01  8:11 UTC (permalink / raw)
  To: bot+bpf-ci, ast, daniel, davem, kuba, hawk, john.fastabend, sdf,
	andrii, eddyz87, ihor.solodrai, memxor, martin.lau, song,
	yonghong.song, jolsa, emil
  Cc: shuah, bpf, netdev, ast, andrii, daniel, martin.lau, eddyz87,
	yonghong.song, mason, ihor.solodrai


> Should the bpf_verif_scale.c site be converted as well, or does "all
> in-tree users" exclude it for some reason?

No, I missed this one - will add it and repost.

-Toke


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

* Re: [PATCH bpf-next v2 1/4] libbpf: Move section-defined program flags to prog_flags
  2026-08-31 14:32 ` [PATCH bpf-next v2 1/4] libbpf: Move section-defined program flags to prog_flags bot+bpf-ci
@ 2026-09-01  8:30   ` Toke Høiland-Jørgensen
  0 siblings, 0 replies; 9+ messages in thread
From: Toke Høiland-Jørgensen @ 2026-09-01  8:30 UTC (permalink / raw)
  To: bot+bpf-ci, ast, daniel, andrii, eddyz87, memxor, martin.lau,
	song, yonghong.song, jolsa, emil, ihor.solodrai, davem, kuba,
	hawk, john.fastabend, sdf
  Cc: bpf, netdev, ast, andrii, daniel, martin.lau, eddyz87,
	yonghong.song, mason, ihor.solodrai

bot+bpf-ci@kernel.org writes:

>> commit 24627f60026bd2b231e09d4373e14ca1975a2d96
>> Author: Toke Høiland-Jørgensen <toke@redhat.com>
>> Date:   Mon Aug 31 15:26:41 2026 +0200
>>
>>     libbpf: Move section-defined program flags to prog_flags
>>
>>     The libbpf section definition modifiers for XDP frags support and
>>     sleepable programs stores the flags bits only in the private section
>>     definition cookie from object open to load time. This has the
>>     unfortunate consequence that API consumers cannot see (or manipulate)
>>     the flag between object open and program load.
>>
>>     In particular, libxdp has special handling of frags-enabled programs to
>>     make them compatible with the dispatcher. This doesn't work on XDP
>>     programs that enable frags through the 'xdp.frags' section definition
>>     because the flag is not visible through bpf_program__flags()[0].
>>
>>     Fix this by changing how libbpf loads the program flags from section
>>     definitions: instead of using the private section definition cookie, add
>>     a setup function to the default section definitions that stores the
>>     flags for sleepable and XDP frags programs in the prog_flags field of
>>     struct bpf_program.
>>
>>     Exposing the flags this way means that any use of
>>     bpf_program__set_flags() will override the flags unless the caller takes
>>     care of updating flags in a non-destructive way. This is unavoidable
>>     with the set-only API, and any user setting flags unconditionally is
>>     already broken in the sense that they will also override any other
>>     current and future flags. A subsequent patch fixes up all in-tree users
>>     of the API.
>
> This looks like a bug fix for SEC(\"xdp.frags\") flag visibility. Should
> this include:
>
>   Fixes: 082c4bfba4f7 ("libbpf: Add SEC name for xdp frags programs")

Sure, why not?

> The commit message states "A subsequent patch fixes up all in-tree users
> of the API." Is this statement accurate?
>
> Looking at the end of the series (645bdf199b3cb), there is one destructive
> bpf_program__set_flags() call site that remains unconverted:
>
>     tools/testing/selftests/bpf/prog_tests/bpf_verif_scale.c:38
>         bpf_program__set_flags(prog, testing_prog_flags());

As I replied to the other bot (why are there so many bots?), I missed
this one. Will add and respin.

[...]

> Is keeping the flags across a type change intended? If so, that seems worth
> noting in the changelog. If not, should the type check be kept or should
> bpf_program__set_type() clear the SEC-derived bits?

Sure, will note this in the patch description.

[...]

> Should the in-tree user fix-ups land before (or be folded into) this commit
> to avoid the bisection hazard? The usual kernel ordering is to fix the
> users first so that no intermediate commit regresses.

Sure, why not? Will reorder in the next version.

>> @@ -10099,6 +10106,7 @@ int bpf_program__clone(struct bpf_program *prog, const struct bpf_prog_load_opts
>>  	.prog_type = BPF_PROG_TYPE_##ptype,				    \
>>  	.expected_attach_type = atype,					    \
>>  	.cookie = (long)(flags),					    \
>> +	.prog_setup_fn = libbpf_setup_prog_flags,			    \
>>  	.prog_prepare_load_fn = libbpf_prepare_load_fn,		    \
>>  	__VA_ARGS__							    \
>>  }
>
> Does bpf_program__clone() lose the SEC()-derived flags whenever the caller
> supplies a non-zero opts->prog_flags?
>
> Before this patch, bpf_program__clone() got the SEC flags back from the
> sec_def callback it invokes:
>
>     if (prog->sec_def && prog->sec_def->prog_prepare_load_fn)
>         err = prog->sec_def->prog_prepare_load_fn(prog, &attr,
>                                                    prog->sec_def->cookie);
>
> which used to do opts->prog_flags |= BPF_F_SLEEPABLE / |= BPF_F_XDP_HAS_FRAGS.
> So even when the caller passed its own prog_flags, a SEC("fentry.s/...") or
> SEC("xdp.frags") program was still cloned with the right bit set.
>
> After this patch libbpf_prepare_prog_load() no longer touches
> opts->prog_flags, so this line in bpf_program__clone():
>
>     attr.prog_flags = OPTS_GET(opts, prog_flags, 0) ?: prog->prog_flags;
>
> discards prog->prog_flags entirely as soon as the caller sets any flag of
> its own. Cloning a sleepable program with, say, opts.prog_flags =
> BPF_F_TEST_STATE_FREQ now loads it without BPF_F_SLEEPABLE, which
> kernel/bpf/verifier.c:20602-20610 rejects for BPF_PROG_TYPE_SYSCALL and for
> sleepable-only LSM/tracing attach points (-EINVAL).
>
> No in-tree caller triggers this today: the only opts-passing caller,
> process_prog() in tools/testing/selftests/bpf/veristat.c:1730-1737, leaves
> opts.prog_flags at 0, and none of the follow-up commits in this series add
> such a caller. Would it be safer to OR the caller's flags into
> prog->prog_flags, or is the all-or-nothing fallback intended to let callers
> override SEC-derived flags explicitly?

Yes, giving the caller control over the flags is the point of this
patch, so this is expected.

-Toke


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

end of thread, other threads:[~2026-09-01  8:31 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-08-31 13:26 [PATCH bpf-next v2 1/4] libbpf: Move section-defined program flags to prog_flags Toke Høiland-Jørgensen
2026-08-31 13:26 ` [PATCH bpf-next v2 2/4] bpf: selftests: Check for flag presence in bpf_program__flags() Toke Høiland-Jørgensen
2026-08-31 14:18   ` bot+bpf-ci
2026-09-01  8:03     ` Toke Høiland-Jørgensen
2026-08-31 13:26 ` [PATCH bpf-next v2 4/4] selftests/bpf: Set BPF program flags non-destructively Toke Høiland-Jørgensen
2026-08-31 14:18   ` bot+bpf-ci
2026-09-01  8:11     ` Toke Høiland-Jørgensen
2026-08-31 14:32 ` [PATCH bpf-next v2 1/4] libbpf: Move section-defined program flags to prog_flags bot+bpf-ci
2026-09-01  8:30   ` Toke Høiland-Jørgensen

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