netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH bpf v3 2/2] selftests/bpf: Add a test for using a cpumap from an freplace-to-XDP program
       [not found] <20221213232441.652313-1-toke@redhat.com>
@ 2022-12-13 23:24 ` Toke Høiland-Jørgensen
  2022-12-13 23:26   ` Andrii Nakryiko
  0 siblings, 1 reply; 3+ messages in thread
From: Toke Høiland-Jørgensen @ 2022-12-13 23:24 UTC (permalink / raw)
  To: Andrii Nakryiko, Mykola Lysenko, Alexei Starovoitov,
	Daniel Borkmann, Martin KaFai Lau, Song Liu, Yonghong Song,
	John Fastabend, KP Singh, Stanislav Fomichev, Hao Luo, Jiri Olsa,
	David S. Miller, Jakub Kicinski, Jesper Dangaard Brouer
  Cc: Toke Høiland-Jørgensen, 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.

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  | 57 +++++++++++++++++++
 .../selftests/bpf/progs/freplace_progmap.c    | 24 ++++++++
 tools/testing/selftests/bpf/testing_helpers.c | 24 +++++++-
 tools/testing/selftests/bpf/testing_helpers.h |  2 +
 4 files changed, 105 insertions(+), 2 deletions(-)
 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..fa8e129d1a71 100644
--- a/tools/testing/selftests/bpf/prog_tests/fexit_bpf2bpf.c
+++ b/tools/testing/selftests/bpf/prog_tests/fexit_bpf2bpf.c
@@ -500,6 +500,61 @@ 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 bpf_object *obj, *tgt_obj = NULL;
+	struct bpf_program *drop, *redirect;
+	struct bpf_map *cpumap;
+	int err, tgt_fd;
+	__u32 key = 0;
+
+	err = bpf_prog_test_open("freplace_progmap.bpf.o", BPF_PROG_TYPE_UNSPEC, &obj);
+	if (!ASSERT_OK(err, "prog_open"))
+		return;
+
+	err = bpf_prog_test_load("xdp_dummy.bpf.o", BPF_PROG_TYPE_UNSPEC, &tgt_obj, &tgt_fd);
+	if (!ASSERT_OK(err, "tgt_prog_load"))
+		goto out;
+
+	drop = bpf_object__find_program_by_name(obj, "xdp_drop_prog");
+	redirect = bpf_object__find_program_by_name(obj, "xdp_cpumap_prog");
+	cpumap = bpf_object__find_map_by_name(obj, "cpu_map");
+
+	if (!ASSERT_OK_PTR(drop, "drop") || !ASSERT_OK_PTR(redirect, "redirect") ||
+	    !ASSERT_OK_PTR(cpumap, "cpumap"))
+		goto out;
+
+	/* Change the 'redirect' program type to be a PROG_TYPE_EXT
+	 * with an XDP target
+	 */
+	bpf_program__set_type(redirect, BPF_PROG_TYPE_EXT);
+	bpf_program__set_expected_attach_type(redirect, 0);
+	err = bpf_program__set_attach_target(redirect, tgt_fd, "xdp_dummy_prog");
+	if (!ASSERT_OK(err, "set_attach_target"))
+		goto out;
+
+	err = bpf_object__load(obj);
+	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(drop);
+	err = bpf_map_update_elem(bpf_map__fd(cpumap), &key, &value, 0);
+	ASSERT_OK(err, "map_update");
+
+out:
+	bpf_object__close(tgt_obj);
+	bpf_object__close(obj);
+}
+
 /* NOTE: affect other tests, must run in serial mode */
 void serial_test_fexit_bpf2bpf(void)
 {
@@ -525,4 +580,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";
diff --git a/tools/testing/selftests/bpf/testing_helpers.c b/tools/testing/selftests/bpf/testing_helpers.c
index 9695318e8132..2050244e6f24 100644
--- a/tools/testing/selftests/bpf/testing_helpers.c
+++ b/tools/testing/selftests/bpf/testing_helpers.c
@@ -174,8 +174,8 @@ __u32 link_info_prog_id(const struct bpf_link *link, struct bpf_link_info *info)
 
 int extra_prog_load_log_flags = 0;
 
-int bpf_prog_test_load(const char *file, enum bpf_prog_type type,
-		       struct bpf_object **pobj, int *prog_fd)
+int bpf_prog_test_open(const char *file, enum bpf_prog_type type,
+		       struct bpf_object **pobj)
 {
 	LIBBPF_OPTS(bpf_object_open_opts, opts,
 		.kernel_log_level = extra_prog_load_log_flags,
@@ -201,6 +201,26 @@ int bpf_prog_test_load(const char *file, enum bpf_prog_type type,
 	flags = bpf_program__flags(prog) | BPF_F_TEST_RND_HI32;
 	bpf_program__set_flags(prog, flags);
 
+	*pobj = obj;
+	return 0;
+err_out:
+	bpf_object__close(obj);
+	return err;
+}
+
+int bpf_prog_test_load(const char *file, enum bpf_prog_type type,
+		       struct bpf_object **pobj, int *prog_fd)
+{
+	struct bpf_program *prog;
+	struct bpf_object *obj;
+	int err;
+
+	err = bpf_prog_test_open(file, type, &obj);
+	if (err)
+		return err;
+
+	prog = bpf_object__next_program(obj, NULL);
+
 	err = bpf_object__load(obj);
 	if (err)
 		goto err_out;
diff --git a/tools/testing/selftests/bpf/testing_helpers.h b/tools/testing/selftests/bpf/testing_helpers.h
index 6ec00bf79cb5..977eb520d119 100644
--- a/tools/testing/selftests/bpf/testing_helpers.h
+++ b/tools/testing/selftests/bpf/testing_helpers.h
@@ -6,6 +6,8 @@
 
 int parse_num_list(const char *s, bool **set, int *set_len);
 __u32 link_info_prog_id(const struct bpf_link *link, struct bpf_link_info *info);
+int bpf_prog_test_open(const char *file, enum bpf_prog_type type,
+		       struct bpf_object **pobj);
 int bpf_prog_test_load(const char *file, enum bpf_prog_type type,
 		       struct bpf_object **pobj, int *prog_fd);
 int bpf_test_load_program(enum bpf_prog_type type, const struct bpf_insn *insns,
-- 
2.38.1


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

* Re: [PATCH bpf v3 2/2] selftests/bpf: Add a test for using a cpumap from an freplace-to-XDP program
  2022-12-13 23:24 ` [PATCH bpf v3 2/2] selftests/bpf: Add a test for using a cpumap from an freplace-to-XDP program Toke Høiland-Jørgensen
@ 2022-12-13 23:26   ` Andrii Nakryiko
  2022-12-14  0:30     ` Toke Høiland-Jørgensen
  0 siblings, 1 reply; 3+ messages in thread
From: Andrii Nakryiko @ 2022-12-13 23:26 UTC (permalink / raw)
  To: Toke Høiland-Jørgensen
  Cc: Andrii Nakryiko, Mykola Lysenko, Alexei Starovoitov,
	Daniel Borkmann, Martin KaFai Lau, Song Liu, Yonghong Song,
	John Fastabend, KP Singh, Stanislav Fomichev, Hao Luo, Jiri Olsa,
	David S. Miller, Jakub Kicinski, Jesper Dangaard Brouer,
	Shuah Khan, bpf, netdev

On Tue, Dec 13, 2022 at 3:24 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.
>
> 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>
> ---

what a race, I just replied a few minutes ago on your v2. Let's use
modern BPF skeleton approach instead of adding legacy stuff to
testing_helpers

>  .../selftests/bpf/prog_tests/fexit_bpf2bpf.c  | 57 +++++++++++++++++++
>  .../selftests/bpf/progs/freplace_progmap.c    | 24 ++++++++
>  tools/testing/selftests/bpf/testing_helpers.c | 24 +++++++-
>  tools/testing/selftests/bpf/testing_helpers.h |  2 +
>  4 files changed, 105 insertions(+), 2 deletions(-)
>  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..fa8e129d1a71 100644
> --- a/tools/testing/selftests/bpf/prog_tests/fexit_bpf2bpf.c
> +++ b/tools/testing/selftests/bpf/prog_tests/fexit_bpf2bpf.c
> @@ -500,6 +500,61 @@ 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 bpf_object *obj, *tgt_obj = NULL;
> +       struct bpf_program *drop, *redirect;
> +       struct bpf_map *cpumap;
> +       int err, tgt_fd;
> +       __u32 key = 0;
> +
> +       err = bpf_prog_test_open("freplace_progmap.bpf.o", BPF_PROG_TYPE_UNSPEC, &obj);
> +       if (!ASSERT_OK(err, "prog_open"))
> +               return;
> +
> +       err = bpf_prog_test_load("xdp_dummy.bpf.o", BPF_PROG_TYPE_UNSPEC, &tgt_obj, &tgt_fd);
> +       if (!ASSERT_OK(err, "tgt_prog_load"))
> +               goto out;
> +
> +       drop = bpf_object__find_program_by_name(obj, "xdp_drop_prog");
> +       redirect = bpf_object__find_program_by_name(obj, "xdp_cpumap_prog");
> +       cpumap = bpf_object__find_map_by_name(obj, "cpu_map");
> +
> +       if (!ASSERT_OK_PTR(drop, "drop") || !ASSERT_OK_PTR(redirect, "redirect") ||
> +           !ASSERT_OK_PTR(cpumap, "cpumap"))
> +               goto out;
> +
> +       /* Change the 'redirect' program type to be a PROG_TYPE_EXT
> +        * with an XDP target
> +        */
> +       bpf_program__set_type(redirect, BPF_PROG_TYPE_EXT);
> +       bpf_program__set_expected_attach_type(redirect, 0);
> +       err = bpf_program__set_attach_target(redirect, tgt_fd, "xdp_dummy_prog");
> +       if (!ASSERT_OK(err, "set_attach_target"))
> +               goto out;
> +
> +       err = bpf_object__load(obj);
> +       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(drop);
> +       err = bpf_map_update_elem(bpf_map__fd(cpumap), &key, &value, 0);
> +       ASSERT_OK(err, "map_update");
> +
> +out:
> +       bpf_object__close(tgt_obj);
> +       bpf_object__close(obj);
> +}
> +
>  /* NOTE: affect other tests, must run in serial mode */
>  void serial_test_fexit_bpf2bpf(void)
>  {
> @@ -525,4 +580,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";
> diff --git a/tools/testing/selftests/bpf/testing_helpers.c b/tools/testing/selftests/bpf/testing_helpers.c
> index 9695318e8132..2050244e6f24 100644
> --- a/tools/testing/selftests/bpf/testing_helpers.c
> +++ b/tools/testing/selftests/bpf/testing_helpers.c
> @@ -174,8 +174,8 @@ __u32 link_info_prog_id(const struct bpf_link *link, struct bpf_link_info *info)
>
>  int extra_prog_load_log_flags = 0;
>
> -int bpf_prog_test_load(const char *file, enum bpf_prog_type type,
> -                      struct bpf_object **pobj, int *prog_fd)
> +int bpf_prog_test_open(const char *file, enum bpf_prog_type type,
> +                      struct bpf_object **pobj)
>  {
>         LIBBPF_OPTS(bpf_object_open_opts, opts,
>                 .kernel_log_level = extra_prog_load_log_flags,
> @@ -201,6 +201,26 @@ int bpf_prog_test_load(const char *file, enum bpf_prog_type type,
>         flags = bpf_program__flags(prog) | BPF_F_TEST_RND_HI32;
>         bpf_program__set_flags(prog, flags);
>
> +       *pobj = obj;
> +       return 0;
> +err_out:
> +       bpf_object__close(obj);
> +       return err;
> +}
> +
> +int bpf_prog_test_load(const char *file, enum bpf_prog_type type,
> +                      struct bpf_object **pobj, int *prog_fd)
> +{
> +       struct bpf_program *prog;
> +       struct bpf_object *obj;
> +       int err;
> +
> +       err = bpf_prog_test_open(file, type, &obj);
> +       if (err)
> +               return err;
> +
> +       prog = bpf_object__next_program(obj, NULL);
> +
>         err = bpf_object__load(obj);
>         if (err)
>                 goto err_out;
> diff --git a/tools/testing/selftests/bpf/testing_helpers.h b/tools/testing/selftests/bpf/testing_helpers.h
> index 6ec00bf79cb5..977eb520d119 100644
> --- a/tools/testing/selftests/bpf/testing_helpers.h
> +++ b/tools/testing/selftests/bpf/testing_helpers.h
> @@ -6,6 +6,8 @@
>
>  int parse_num_list(const char *s, bool **set, int *set_len);
>  __u32 link_info_prog_id(const struct bpf_link *link, struct bpf_link_info *info);
> +int bpf_prog_test_open(const char *file, enum bpf_prog_type type,
> +                      struct bpf_object **pobj);
>  int bpf_prog_test_load(const char *file, enum bpf_prog_type type,
>                        struct bpf_object **pobj, int *prog_fd);
>  int bpf_test_load_program(enum bpf_prog_type type, const struct bpf_insn *insns,
> --
> 2.38.1
>

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

* Re: [PATCH bpf v3 2/2] selftests/bpf: Add a test for using a cpumap from an freplace-to-XDP program
  2022-12-13 23:26   ` Andrii Nakryiko
@ 2022-12-14  0:30     ` Toke Høiland-Jørgensen
  0 siblings, 0 replies; 3+ messages in thread
From: Toke Høiland-Jørgensen @ 2022-12-14  0:30 UTC (permalink / raw)
  To: Andrii Nakryiko
  Cc: Andrii Nakryiko, Mykola Lysenko, Alexei Starovoitov,
	Daniel Borkmann, Martin KaFai Lau, Song Liu, Yonghong Song,
	John Fastabend, KP Singh, Stanislav Fomichev, Hao Luo, Jiri Olsa,
	David S. Miller, Jakub Kicinski, Jesper Dangaard Brouer,
	Shuah Khan, bpf, netdev

Andrii Nakryiko <andrii.nakryiko@gmail.com> writes:

> On Tue, Dec 13, 2022 at 3:24 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.
>>
>> 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>
>> ---
>
> what a race, I just replied a few minutes ago on your v2. Let's use
> modern BPF skeleton approach instead of adding legacy stuff to
> testing_helpers

Sure, will respin...

-Toke


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

end of thread, other threads:[~2022-12-14  0:31 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
     [not found] <20221213232441.652313-1-toke@redhat.com>
2022-12-13 23:24 ` [PATCH bpf v3 2/2] selftests/bpf: Add a test for using a cpumap from an freplace-to-XDP program Toke Høiland-Jørgensen
2022-12-13 23:26   ` Andrii Nakryiko
2022-12-14  0: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;
as well as URLs for NNTP newsgroup(s).