Thread (9 messages) flat view 9 messages, 2 authors, 4d ago

Re: [PATCH bpf-next v2 4/4] selftests/bpf: Set BPF program flags non-destructively

From: bot+bpf-ci@kernel.org
Date: 2026-08-31 14:18:20
Also in: bpf

    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 [off-list ref]
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:
quoted hunk ↗ jump to hunk
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
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help