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 --- 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 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 --- 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 When setting the XDP hints ifname, bpftool would set the BPF_F_XDP_DEV_BOUND_ONLY without looking at the existing program flags, overriding any other flag values. Change this to set the flag value non-destructively by OR'ing it with the existing flags. Fixes: f46392ee3dec ("bpftool: Specify XDP Hints ifname when loading program") Signed-off-by: Toke Høiland-Jørgensen --- 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 a9f730d407a9..8c2f9255b36d 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_F_XDP_DEV_BOUND_ONLY); + bpf_program__set_flags(pos, bpf_program__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 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 --- 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