Re: [PATCH bpf-next v2 1/4] libbpf: Move section-defined program flags to prog_flags

bot+bpf-ci@kernel.org writes:

quoted

commit 24627f60026bd2b231e09d4373e14ca1975a2d96
Author: Toke Høiland-Jørgensen [off-list ref]
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.

quoted

@@ -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