* [PATCH bpf v4 2/2] selftests/bpf: Add a test for using a cpumap from an freplace-to-XDP program [not found] <20221214010517.668943-1-toke@redhat.com> @ 2022-12-14 1:05 ` Toke Høiland-Jørgensen 2022-12-14 19:35 ` Andrii Nakryiko 0 siblings, 1 reply; 3+ messages in thread From: Toke Høiland-Jørgensen @ 2022-12-14 1:05 UTC (permalink / raw) To: Alexei Starovoitov, Daniel Borkmann, David S. Miller, Jakub Kicinski, Jesper Dangaard Brouer, John Fastabend Cc: Toke Høiland-Jørgensen, Yonghong Song, Andrii Nakryiko, Mykola Lysenko, Martin KaFai Lau, Song Liu, KP Singh, Stanislav Fomichev, Hao Luo, Jiri Olsa, Shuah Khan, bpf, netdev This adds a simple test for inserting an XDP program into a cpumap that is "owned" by an XDP program that was loaded as PROG_TYPE_EXT (as libxdp does). Prior to the kernel fix this would fail because the map type ownership would be set to PROG_TYPE_EXT instead of being resolved to PROG_TYPE_XDP. v4: - Use skeletons for selftest v3: - Update comment to better explain the cause - Add Yonghong's ACK Acked-by: Yonghong Song <yhs@fb.com> Signed-off-by: Toke Høiland-Jørgensen <toke@redhat.com> --- .../selftests/bpf/prog_tests/fexit_bpf2bpf.c | 54 +++++++++++++++++++ .../selftests/bpf/progs/freplace_progmap.c | 24 +++++++++ 2 files changed, 78 insertions(+) create mode 100644 tools/testing/selftests/bpf/progs/freplace_progmap.c diff --git a/tools/testing/selftests/bpf/prog_tests/fexit_bpf2bpf.c b/tools/testing/selftests/bpf/prog_tests/fexit_bpf2bpf.c index d1e32e792536..efa1fc65840d 100644 --- a/tools/testing/selftests/bpf/prog_tests/fexit_bpf2bpf.c +++ b/tools/testing/selftests/bpf/prog_tests/fexit_bpf2bpf.c @@ -4,6 +4,8 @@ #include <network_helpers.h> #include <bpf/btf.h> #include "bind4_prog.skel.h" +#include "freplace_progmap.skel.h" +#include "xdp_dummy.skel.h" typedef int (*test_cb)(struct bpf_object *obj); @@ -500,6 +502,56 @@ static void test_fentry_to_cgroup_bpf(void) bind4_prog__destroy(skel); } +static void test_func_replace_progmap(void) +{ + struct bpf_cpumap_val value = { .qsize = 1 }; + struct freplace_progmap *skel = NULL; + struct xdp_dummy *tgt_skel = NULL; + int err, tgt_fd; + __u32 key = 0; + + skel = freplace_progmap__open(); + if (!ASSERT_OK_PTR(skel, "prog_open")) + return; + + tgt_skel = xdp_dummy__open_and_load(); + if (!ASSERT_OK_PTR(tgt_skel, "tgt_prog_load")) + goto out; + + tgt_fd = bpf_program__fd(tgt_skel->progs.xdp_dummy_prog); + + /* Change the 'redirect' program type to be a PROG_TYPE_EXT + * with an XDP target + */ + bpf_program__set_type(skel->progs.xdp_cpumap_prog, BPF_PROG_TYPE_EXT); + bpf_program__set_expected_attach_type(skel->progs.xdp_cpumap_prog, 0); + err = bpf_program__set_attach_target(skel->progs.xdp_cpumap_prog, + tgt_fd, "xdp_dummy_prog"); + if (!ASSERT_OK(err, "set_attach_target")) + goto out; + + err = freplace_progmap__load(skel); + if (!ASSERT_OK(err, "obj_load")) + goto out; + + /* Prior to fixing the kernel, loading the PROG_TYPE_EXT 'redirect' + * program above will cause the map owner type of 'cpumap' to be set to + * PROG_TYPE_EXT. This in turn will cause the bpf_map_update_elem() + * below to fail, because the program we are inserting into the map is + * of PROG_TYPE_XDP. After fixing the kernel, the initial ownership will + * be correctly resolved to the *target* of the PROG_TYPE_EXT program + * (i.e., PROG_TYPE_XDP) and the map update will succeed. + */ + value.bpf_prog.fd = bpf_program__fd(skel->progs.xdp_drop_prog); + err = bpf_map_update_elem(bpf_map__fd(skel->maps.cpu_map), + &key, &value, 0); + ASSERT_OK(err, "map_update"); + +out: + xdp_dummy__destroy(tgt_skel); + freplace_progmap__destroy(skel); +} + /* NOTE: affect other tests, must run in serial mode */ void serial_test_fexit_bpf2bpf(void) { @@ -525,4 +577,6 @@ void serial_test_fexit_bpf2bpf(void) test_func_replace_global_func(); if (test__start_subtest("fentry_to_cgroup_bpf")) test_fentry_to_cgroup_bpf(); + if (test__start_subtest("func_replace_progmap")) + test_func_replace_progmap(); } diff --git a/tools/testing/selftests/bpf/progs/freplace_progmap.c b/tools/testing/selftests/bpf/progs/freplace_progmap.c new file mode 100644 index 000000000000..68174c3d7b37 --- /dev/null +++ b/tools/testing/selftests/bpf/progs/freplace_progmap.c @@ -0,0 +1,24 @@ +// SPDX-License-Identifier: GPL-2.0 +#include <linux/bpf.h> +#include <bpf/bpf_helpers.h> + +struct { + __uint(type, BPF_MAP_TYPE_CPUMAP); + __uint(key_size, sizeof(__u32)); + __uint(value_size, sizeof(struct bpf_cpumap_val)); + __uint(max_entries, 1); +} cpu_map SEC(".maps"); + +SEC("xdp/cpumap") +int xdp_drop_prog(struct xdp_md *ctx) +{ + return XDP_DROP; +} + +SEC("xdp") +int xdp_cpumap_prog(struct xdp_md *ctx) +{ + return bpf_redirect_map(&cpu_map, 0, XDP_PASS); +} + +char _license[] SEC("license") = "GPL"; -- 2.38.1 ^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH bpf v4 2/2] selftests/bpf: Add a test for using a cpumap from an freplace-to-XDP program 2022-12-14 1:05 ` [PATCH bpf v4 2/2] selftests/bpf: Add a test for using a cpumap from an freplace-to-XDP program Toke Høiland-Jørgensen @ 2022-12-14 19:35 ` Andrii Nakryiko 2022-12-14 22:58 ` Toke Høiland-Jørgensen 0 siblings, 1 reply; 3+ messages in thread From: Andrii Nakryiko @ 2022-12-14 19:35 UTC (permalink / raw) To: Toke Høiland-Jørgensen Cc: Alexei Starovoitov, Daniel Borkmann, David S. Miller, Jakub Kicinski, Jesper Dangaard Brouer, John Fastabend, Yonghong Song, Andrii Nakryiko, Mykola Lysenko, Martin KaFai Lau, Song Liu, KP Singh, Stanislav Fomichev, Hao Luo, Jiri Olsa, Shuah Khan, bpf, netdev On Tue, Dec 13, 2022 at 5:05 PM Toke Høiland-Jørgensen <toke@redhat.com> wrote: > > This adds a simple test for inserting an XDP program into a cpumap that is > "owned" by an XDP program that was loaded as PROG_TYPE_EXT (as libxdp > does). Prior to the kernel fix this would fail because the map type > ownership would be set to PROG_TYPE_EXT instead of being resolved to > PROG_TYPE_XDP. > > v4: > - Use skeletons for selftest > v3: > - Update comment to better explain the cause > - Add Yonghong's ACK > > Acked-by: Yonghong Song <yhs@fb.com> > Signed-off-by: Toke Høiland-Jørgensen <toke@redhat.com> > --- > .../selftests/bpf/prog_tests/fexit_bpf2bpf.c | 54 +++++++++++++++++++ > .../selftests/bpf/progs/freplace_progmap.c | 24 +++++++++ > 2 files changed, 78 insertions(+) > create mode 100644 tools/testing/selftests/bpf/progs/freplace_progmap.c > > diff --git a/tools/testing/selftests/bpf/prog_tests/fexit_bpf2bpf.c b/tools/testing/selftests/bpf/prog_tests/fexit_bpf2bpf.c > index d1e32e792536..efa1fc65840d 100644 > --- a/tools/testing/selftests/bpf/prog_tests/fexit_bpf2bpf.c > +++ b/tools/testing/selftests/bpf/prog_tests/fexit_bpf2bpf.c > @@ -4,6 +4,8 @@ > #include <network_helpers.h> > #include <bpf/btf.h> > #include "bind4_prog.skel.h" > +#include "freplace_progmap.skel.h" > +#include "xdp_dummy.skel.h" > > typedef int (*test_cb)(struct bpf_object *obj); > > @@ -500,6 +502,56 @@ static void test_fentry_to_cgroup_bpf(void) > bind4_prog__destroy(skel); > } > > +static void test_func_replace_progmap(void) > +{ > + struct bpf_cpumap_val value = { .qsize = 1 }; > + struct freplace_progmap *skel = NULL; > + struct xdp_dummy *tgt_skel = NULL; > + int err, tgt_fd; > + __u32 key = 0; > + > + skel = freplace_progmap__open(); > + if (!ASSERT_OK_PTR(skel, "prog_open")) > + return; > + > + tgt_skel = xdp_dummy__open_and_load(); > + if (!ASSERT_OK_PTR(tgt_skel, "tgt_prog_load")) > + goto out; > + > + tgt_fd = bpf_program__fd(tgt_skel->progs.xdp_dummy_prog); > + > + /* Change the 'redirect' program type to be a PROG_TYPE_EXT > + * with an XDP target > + */ > + bpf_program__set_type(skel->progs.xdp_cpumap_prog, BPF_PROG_TYPE_EXT); > + bpf_program__set_expected_attach_type(skel->progs.xdp_cpumap_prog, 0); you shouldn't need this manual override if you mark xdp_cpumap_prog as SEC("freplace"), or am I missing something? but other than this minor thing looks good to me, thanks Acked-by: Andrii Nakryiko <andrii@kernel.org> > + err = bpf_program__set_attach_target(skel->progs.xdp_cpumap_prog, > + tgt_fd, "xdp_dummy_prog"); > + if (!ASSERT_OK(err, "set_attach_target")) > + goto out; > + > + err = freplace_progmap__load(skel); > + if (!ASSERT_OK(err, "obj_load")) > + goto out; > + > + /* Prior to fixing the kernel, loading the PROG_TYPE_EXT 'redirect' > + * program above will cause the map owner type of 'cpumap' to be set to > + * PROG_TYPE_EXT. This in turn will cause the bpf_map_update_elem() > + * below to fail, because the program we are inserting into the map is > + * of PROG_TYPE_XDP. After fixing the kernel, the initial ownership will > + * be correctly resolved to the *target* of the PROG_TYPE_EXT program > + * (i.e., PROG_TYPE_XDP) and the map update will succeed. > + */ > + value.bpf_prog.fd = bpf_program__fd(skel->progs.xdp_drop_prog); > + err = bpf_map_update_elem(bpf_map__fd(skel->maps.cpu_map), > + &key, &value, 0); > + ASSERT_OK(err, "map_update"); > + > +out: > + xdp_dummy__destroy(tgt_skel); > + freplace_progmap__destroy(skel); > +} > + > /* NOTE: affect other tests, must run in serial mode */ > void serial_test_fexit_bpf2bpf(void) > { > @@ -525,4 +577,6 @@ void serial_test_fexit_bpf2bpf(void) > test_func_replace_global_func(); > if (test__start_subtest("fentry_to_cgroup_bpf")) > test_fentry_to_cgroup_bpf(); > + if (test__start_subtest("func_replace_progmap")) > + test_func_replace_progmap(); > } > diff --git a/tools/testing/selftests/bpf/progs/freplace_progmap.c b/tools/testing/selftests/bpf/progs/freplace_progmap.c > new file mode 100644 > index 000000000000..68174c3d7b37 > --- /dev/null > +++ b/tools/testing/selftests/bpf/progs/freplace_progmap.c > @@ -0,0 +1,24 @@ > +// SPDX-License-Identifier: GPL-2.0 > +#include <linux/bpf.h> > +#include <bpf/bpf_helpers.h> > + > +struct { > + __uint(type, BPF_MAP_TYPE_CPUMAP); > + __uint(key_size, sizeof(__u32)); > + __uint(value_size, sizeof(struct bpf_cpumap_val)); ok, another minor nit which you ignored, libbpf should be smart enough to accept __type(key, __u32); __type(value, struct bpf_cpumap_val); And if it's not it would be good to know that it's not (and trivially fix it). > + __uint(max_entries, 1); > +} cpu_map SEC(".maps"); > + > +SEC("xdp/cpumap") > +int xdp_drop_prog(struct xdp_md *ctx) > +{ > + return XDP_DROP; > +} > + > +SEC("xdp") > +int xdp_cpumap_prog(struct xdp_md *ctx) > +{ > + return bpf_redirect_map(&cpu_map, 0, XDP_PASS); > +} > + > +char _license[] SEC("license") = "GPL"; > -- > 2.38.1 > ^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH bpf v4 2/2] selftests/bpf: Add a test for using a cpumap from an freplace-to-XDP program 2022-12-14 19:35 ` Andrii Nakryiko @ 2022-12-14 22:58 ` Toke Høiland-Jørgensen 0 siblings, 0 replies; 3+ messages in thread From: Toke Høiland-Jørgensen @ 2022-12-14 22:58 UTC (permalink / raw) To: Andrii Nakryiko Cc: Alexei Starovoitov, Daniel Borkmann, David S. Miller, Jakub Kicinski, Jesper Dangaard Brouer, John Fastabend, Yonghong Song, Andrii Nakryiko, Mykola Lysenko, Martin KaFai Lau, Song Liu, KP Singh, Stanislav Fomichev, Hao Luo, Jiri Olsa, Shuah Khan, bpf, netdev Andrii Nakryiko <andrii.nakryiko@gmail.com> writes: > On Tue, Dec 13, 2022 at 5:05 PM Toke Høiland-Jørgensen <toke@redhat.com> wrote: >> >> This adds a simple test for inserting an XDP program into a cpumap that is >> "owned" by an XDP program that was loaded as PROG_TYPE_EXT (as libxdp >> does). Prior to the kernel fix this would fail because the map type >> ownership would be set to PROG_TYPE_EXT instead of being resolved to >> PROG_TYPE_XDP. >> >> v4: >> - Use skeletons for selftest >> v3: >> - Update comment to better explain the cause >> - Add Yonghong's ACK >> >> Acked-by: Yonghong Song <yhs@fb.com> >> Signed-off-by: Toke Høiland-Jørgensen <toke@redhat.com> >> --- >> .../selftests/bpf/prog_tests/fexit_bpf2bpf.c | 54 +++++++++++++++++++ >> .../selftests/bpf/progs/freplace_progmap.c | 24 +++++++++ >> 2 files changed, 78 insertions(+) >> create mode 100644 tools/testing/selftests/bpf/progs/freplace_progmap.c >> >> diff --git a/tools/testing/selftests/bpf/prog_tests/fexit_bpf2bpf.c b/tools/testing/selftests/bpf/prog_tests/fexit_bpf2bpf.c >> index d1e32e792536..efa1fc65840d 100644 >> --- a/tools/testing/selftests/bpf/prog_tests/fexit_bpf2bpf.c >> +++ b/tools/testing/selftests/bpf/prog_tests/fexit_bpf2bpf.c >> @@ -4,6 +4,8 @@ >> #include <network_helpers.h> >> #include <bpf/btf.h> >> #include "bind4_prog.skel.h" >> +#include "freplace_progmap.skel.h" >> +#include "xdp_dummy.skel.h" >> >> typedef int (*test_cb)(struct bpf_object *obj); >> >> @@ -500,6 +502,56 @@ static void test_fentry_to_cgroup_bpf(void) >> bind4_prog__destroy(skel); >> } >> >> +static void test_func_replace_progmap(void) >> +{ >> + struct bpf_cpumap_val value = { .qsize = 1 }; >> + struct freplace_progmap *skel = NULL; >> + struct xdp_dummy *tgt_skel = NULL; >> + int err, tgt_fd; >> + __u32 key = 0; >> + >> + skel = freplace_progmap__open(); >> + if (!ASSERT_OK_PTR(skel, "prog_open")) >> + return; >> + >> + tgt_skel = xdp_dummy__open_and_load(); >> + if (!ASSERT_OK_PTR(tgt_skel, "tgt_prog_load")) >> + goto out; >> + >> + tgt_fd = bpf_program__fd(tgt_skel->progs.xdp_dummy_prog); >> + >> + /* Change the 'redirect' program type to be a PROG_TYPE_EXT >> + * with an XDP target >> + */ >> + bpf_program__set_type(skel->progs.xdp_cpumap_prog, BPF_PROG_TYPE_EXT); >> + bpf_program__set_expected_attach_type(skel->progs.xdp_cpumap_prog, 0); > > you shouldn't need this manual override if you mark xdp_cpumap_prog as > SEC("freplace"), or am I missing something? No, you're right; I guess I was just too focused on recreating the flow we use in libxdp. Will fix! >> + err = bpf_program__set_attach_target(skel->progs.xdp_cpumap_prog, >> + tgt_fd, "xdp_dummy_prog"); >> + if (!ASSERT_OK(err, "set_attach_target")) >> + goto out; >> + >> + err = freplace_progmap__load(skel); >> + if (!ASSERT_OK(err, "obj_load")) >> + goto out; >> + >> + /* Prior to fixing the kernel, loading the PROG_TYPE_EXT 'redirect' >> + * program above will cause the map owner type of 'cpumap' to be set to >> + * PROG_TYPE_EXT. This in turn will cause the bpf_map_update_elem() >> + * below to fail, because the program we are inserting into the map is >> + * of PROG_TYPE_XDP. After fixing the kernel, the initial ownership will >> + * be correctly resolved to the *target* of the PROG_TYPE_EXT program >> + * (i.e., PROG_TYPE_XDP) and the map update will succeed. >> + */ >> + value.bpf_prog.fd = bpf_program__fd(skel->progs.xdp_drop_prog); >> + err = bpf_map_update_elem(bpf_map__fd(skel->maps.cpu_map), >> + &key, &value, 0); >> + ASSERT_OK(err, "map_update"); >> + >> +out: >> + xdp_dummy__destroy(tgt_skel); >> + freplace_progmap__destroy(skel); >> +} >> + >> /* NOTE: affect other tests, must run in serial mode */ >> void serial_test_fexit_bpf2bpf(void) >> { >> @@ -525,4 +577,6 @@ void serial_test_fexit_bpf2bpf(void) >> test_func_replace_global_func(); >> if (test__start_subtest("fentry_to_cgroup_bpf")) >> test_fentry_to_cgroup_bpf(); >> + if (test__start_subtest("func_replace_progmap")) >> + test_func_replace_progmap(); >> } >> diff --git a/tools/testing/selftests/bpf/progs/freplace_progmap.c b/tools/testing/selftests/bpf/progs/freplace_progmap.c >> new file mode 100644 >> index 000000000000..68174c3d7b37 >> --- /dev/null >> +++ b/tools/testing/selftests/bpf/progs/freplace_progmap.c >> @@ -0,0 +1,24 @@ >> +// SPDX-License-Identifier: GPL-2.0 >> +#include <linux/bpf.h> >> +#include <bpf/bpf_helpers.h> >> + >> +struct { >> + __uint(type, BPF_MAP_TYPE_CPUMAP); >> + __uint(key_size, sizeof(__u32)); >> + __uint(value_size, sizeof(struct bpf_cpumap_val)); > > ok, another minor nit which you ignored, libbpf should be smart enough to accept > > __type(key, __u32); > __type(value, struct bpf_cpumap_val); > > And if it's not it would be good to know that it's not (and trivially > fix it). Ah, actually saw that comment on the previous version, and then completely forgot about it when I was fixing things up. Sorry about that; will change! -Toke ^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2022-12-14 22:59 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <20221214010517.668943-1-toke@redhat.com>
2022-12-14 1:05 ` [PATCH bpf v4 2/2] selftests/bpf: Add a test for using a cpumap from an freplace-to-XDP program Toke Høiland-Jørgensen
2022-12-14 19:35 ` Andrii Nakryiko
2022-12-14 22:58 ` 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; as well as URLs for NNTP newsgroup(s).