All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH bpf v2 1/2] bpf: Resolve fext program type when checking map compatibility
@ 2022-12-09 14:26 Toke Høiland-Jørgensen
  2022-12-09 14:26 ` [PATCH bpf v2 2/2] selftests/bpf: Add a test for using a cpumap from an freplace-to-XDP program Toke Høiland-Jørgensen
  2022-12-09 17:36 ` [PATCH bpf v2 1/2] bpf: Resolve fext program type when checking map compatibility Yonghong Song
  0 siblings, 2 replies; 5+ messages in thread
From: Toke Høiland-Jørgensen @ 2022-12-09 14:26 UTC (permalink / raw)
  To: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
	Martin KaFai Lau, Song Liu, Yonghong Song, John Fastabend,
	KP Singh, Stanislav Fomichev, Hao Luo, Jiri Olsa,
	Lorenzo Bianconi, Toke Hoiland-Jorgensen
  Cc: bpf

The bpf_prog_map_compatible() check makes sure that BPF program types are
not mixed inside BPF map types that can contain programs (tail call maps,
cpumaps and devmaps). It does this by setting the fields of the map->owner
struct to the values of the first program being checked against, and
rejecting any subsequent programs if the values don't match.

One of the values being set in the map owner struct is the program type,
and since the code did not resolve the prog type for fext programs, the map
owner type would be set to PROG_TYPE_EXT and subsequent loading of programs
of the target type into the map would fail.

This bug is seen in particular for XDP programs that are loaded as
PROG_TYPE_EXT using libxdp; these cannot insert programs into devmaps and
cpumaps because the check fails as described above.

Fix the bug by resolving the fext program type to its target program type
as elsewhere in the verifier. This requires constifying the parameter of
resolve_prog_type() to avoid a compiler warning from the new call site.

Fixes: f45d5b6ce2e8 ("bpf: generalise tail call map compatibility check")
Signed-off-by: Toke Høiland-Jørgensen <toke@redhat.com>
---
No changes since v1, this just adds a selftest (in patch 2).

 include/linux/bpf_verifier.h | 2 +-
 kernel/bpf/core.c            | 5 +++--
 2 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/include/linux/bpf_verifier.h b/include/linux/bpf_verifier.h
index 9e1e6965f407..0eb8f035b3d9 100644
--- a/include/linux/bpf_verifier.h
+++ b/include/linux/bpf_verifier.h
@@ -642,7 +642,7 @@ static inline u32 type_flag(u32 type)
 }
 
 /* only use after check_attach_btf_id() */
-static inline enum bpf_prog_type resolve_prog_type(struct bpf_prog *prog)
+static inline enum bpf_prog_type resolve_prog_type(const struct bpf_prog *prog)
 {
 	return prog->type == BPF_PROG_TYPE_EXT ?
 		prog->aux->dst_prog->type : prog->type;
diff --git a/kernel/bpf/core.c b/kernel/bpf/core.c
index 25a54e04560e..17ab3e15ac25 100644
--- a/kernel/bpf/core.c
+++ b/kernel/bpf/core.c
@@ -2088,6 +2088,7 @@ static unsigned int __bpf_prog_ret0_warn(const void *ctx,
 bool bpf_prog_map_compatible(struct bpf_map *map,
 			     const struct bpf_prog *fp)
 {
+	enum bpf_prog_type prog_type = resolve_prog_type(fp);
 	bool ret;
 
 	if (fp->kprobe_override)
@@ -2098,12 +2099,12 @@ bool bpf_prog_map_compatible(struct bpf_map *map,
 		/* There's no owner yet where we could check for
 		 * compatibility.
 		 */
-		map->owner.type  = fp->type;
+		map->owner.type  = prog_type;
 		map->owner.jited = fp->jited;
 		map->owner.xdp_has_frags = fp->aux->xdp_has_frags;
 		ret = true;
 	} else {
-		ret = map->owner.type  == fp->type &&
+		ret = map->owner.type  == prog_type &&
 		      map->owner.jited == fp->jited &&
 		      map->owner.xdp_has_frags == fp->aux->xdp_has_frags;
 	}
-- 
2.38.1


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

* [PATCH bpf v2 2/2] selftests/bpf: Add a test for using a cpumap from an freplace-to-XDP program
  2022-12-09 14:26 [PATCH bpf v2 1/2] bpf: Resolve fext program type when checking map compatibility Toke Høiland-Jørgensen
@ 2022-12-09 14:26 ` Toke Høiland-Jørgensen
  2022-12-09 18:07   ` Yonghong Song
  2022-12-13 23:17   ` Andrii Nakryiko
  2022-12-09 17:36 ` [PATCH bpf v2 1/2] bpf: Resolve fext program type when checking map compatibility Yonghong Song
  1 sibling, 2 replies; 5+ messages in thread
From: Toke Høiland-Jørgensen @ 2022-12-09 14:26 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.

Signed-off-by: Toke Høiland-Jørgensen <toke@redhat.com>
---
 .../selftests/bpf/prog_tests/fexit_bpf2bpf.c  | 53 +++++++++++++++++++
 .../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, 101 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..dac088217f0f 100644
--- a/tools/testing/selftests/bpf/prog_tests/fexit_bpf2bpf.c
+++ b/tools/testing/selftests/bpf/prog_tests/fexit_bpf2bpf.c
@@ -500,6 +500,57 @@ 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;
+
+	/* This will fail if the map is "owned" by a PROG_TYPE_EXT program,
+	 * which, prior to fixing the kernel, it will be since the map is used
+	 * from the 'redirect' prog above
+	 */
+	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 +576,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] 5+ messages in thread

* Re: [PATCH bpf v2 1/2] bpf: Resolve fext program type when checking map compatibility
  2022-12-09 14:26 [PATCH bpf v2 1/2] bpf: Resolve fext program type when checking map compatibility Toke Høiland-Jørgensen
  2022-12-09 14:26 ` [PATCH bpf v2 2/2] selftests/bpf: Add a test for using a cpumap from an freplace-to-XDP program Toke Høiland-Jørgensen
@ 2022-12-09 17:36 ` Yonghong Song
  1 sibling, 0 replies; 5+ messages in thread
From: Yonghong Song @ 2022-12-09 17:36 UTC (permalink / raw)
  To: Toke Høiland-Jørgensen, Alexei Starovoitov,
	Daniel Borkmann, Andrii Nakryiko, Martin KaFai Lau, Song Liu,
	Yonghong Song, John Fastabend, KP Singh, Stanislav Fomichev,
	Hao Luo, Jiri Olsa, Lorenzo Bianconi
  Cc: bpf



On 12/9/22 6:26 AM, Toke Høiland-Jørgensen wrote:
> The bpf_prog_map_compatible() check makes sure that BPF program types are
> not mixed inside BPF map types that can contain programs (tail call maps,
> cpumaps and devmaps). It does this by setting the fields of the map->owner
> struct to the values of the first program being checked against, and
> rejecting any subsequent programs if the values don't match.
> 
> One of the values being set in the map owner struct is the program type,
> and since the code did not resolve the prog type for fext programs, the map
> owner type would be set to PROG_TYPE_EXT and subsequent loading of programs
> of the target type into the map would fail.
> 
> This bug is seen in particular for XDP programs that are loaded as
> PROG_TYPE_EXT using libxdp; these cannot insert programs into devmaps and
> cpumaps because the check fails as described above.
> 
> Fix the bug by resolving the fext program type to its target program type
> as elsewhere in the verifier. This requires constifying the parameter of
> resolve_prog_type() to avoid a compiler warning from the new call site.
> 
> Fixes: f45d5b6ce2e8 ("bpf: generalise tail call map compatibility check")
> Signed-off-by: Toke Høiland-Jørgensen <toke@redhat.com>

Acked-by: Yonghong Song <yhs@fb.com>

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

* Re: [PATCH bpf v2 2/2] selftests/bpf: Add a test for using a cpumap from an freplace-to-XDP program
  2022-12-09 14:26 ` [PATCH bpf v2 2/2] selftests/bpf: Add a test for using a cpumap from an freplace-to-XDP program Toke Høiland-Jørgensen
@ 2022-12-09 18:07   ` Yonghong Song
  2022-12-13 23:17   ` Andrii Nakryiko
  1 sibling, 0 replies; 5+ messages in thread
From: Yonghong Song @ 2022-12-09 18:07 UTC (permalink / raw)
  To: Toke Høiland-Jørgensen, 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: Shuah Khan, bpf, netdev



On 12/9/22 6:26 AM, Toke Høiland-Jørgensen 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.
> 
> Signed-off-by: Toke Høiland-Jørgensen <toke@redhat.com>

LGTM with a small nit below.

Acked-by: Yonghong Song <yhs@fb.com>

> ---
>   .../selftests/bpf/prog_tests/fexit_bpf2bpf.c  | 53 +++++++++++++++++++
>   .../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, 101 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..dac088217f0f 100644
> --- a/tools/testing/selftests/bpf/prog_tests/fexit_bpf2bpf.c
> +++ b/tools/testing/selftests/bpf/prog_tests/fexit_bpf2bpf.c
> @@ -500,6 +500,57 @@ 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;
> +
> +	/* This will fail if the map is "owned" by a PROG_TYPE_EXT program,
> +	 * which, prior to fixing the kernel, it will be since the map is used
> +	 * from the 'redirect' prog above
> +	 */

The comment is confusing like 'which, prior to fixing the kernel, it 
will be'. IIUC, the verifier expects the map 'owner' program type is
PROG_TYPE_EXT, but it is XDP without this patch. Hence, the test will
fail without the patch 1.

> +	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 +576,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();
>   }
[...]

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

* Re: [PATCH bpf v2 2/2] selftests/bpf: Add a test for using a cpumap from an freplace-to-XDP program
  2022-12-09 14:26 ` [PATCH bpf v2 2/2] selftests/bpf: Add a test for using a cpumap from an freplace-to-XDP program Toke Høiland-Jørgensen
  2022-12-09 18:07   ` Yonghong Song
@ 2022-12-13 23:17   ` Andrii Nakryiko
  1 sibling, 0 replies; 5+ messages in thread
From: Andrii Nakryiko @ 2022-12-13 23:17 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 Fri, Dec 9, 2022 at 6:26 AM 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.
>
> Signed-off-by: Toke Høiland-Jørgensen <toke@redhat.com>
> ---
>  .../selftests/bpf/prog_tests/fexit_bpf2bpf.c  | 53 +++++++++++++++++++
>  .../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, 101 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..dac088217f0f 100644
> --- a/tools/testing/selftests/bpf/prog_tests/fexit_bpf2bpf.c
> +++ b/tools/testing/selftests/bpf/prog_tests/fexit_bpf2bpf.c
> @@ -500,6 +500,57 @@ 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;
> +
> +       /* This will fail if the map is "owned" by a PROG_TYPE_EXT program,
> +        * which, prior to fixing the kernel, it will be since the map is used
> +        * from the 'redirect' prog above
> +        */
> +       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 +576,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));

so old school... ;) have you tried:

__type(key, __u32);
__type(value, 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);
> +

oh, wow, wait, do we really need to add more legacy stuff like this?
Why can't you use BPF skeletons? Avoid all the lookups by name, no
need for helpers like this?


>         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] 5+ messages in thread

end of thread, other threads:[~2022-12-13 23:17 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-12-09 14:26 [PATCH bpf v2 1/2] bpf: Resolve fext program type when checking map compatibility Toke Høiland-Jørgensen
2022-12-09 14:26 ` [PATCH bpf v2 2/2] selftests/bpf: Add a test for using a cpumap from an freplace-to-XDP program Toke Høiland-Jørgensen
2022-12-09 18:07   ` Yonghong Song
2022-12-13 23:17   ` Andrii Nakryiko
2022-12-09 17:36 ` [PATCH bpf v2 1/2] bpf: Resolve fext program type when checking map compatibility Yonghong Song

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.