diff mbox series

[bpf-next,v2,4/4] selftests: bpf: Test copying a sockmap via bpf_iter

Message ID 20200901103210.54607-5-lmb@cloudflare.com
State Changes Requested
Delegated to: BPF Maintainers
Headers show
Series Sockmap iterator | expand

Commit Message

Lorenz Bauer Sept. 1, 2020, 10:32 a.m. UTC 
Add a test that exercises a basic sockmap / sockhash copy using bpf_iter.

Signed-off-by: Lorenz Bauer <lmb@cloudflare.com>
---
 .../selftests/bpf/prog_tests/sockmap_basic.c  | 88 +++++++++++++++++++
 tools/testing/selftests/bpf/progs/bpf_iter.h  |  9 ++
 .../selftests/bpf/progs/bpf_iter_sockmap.c    | 58 ++++++++++++
 .../selftests/bpf/progs/bpf_iter_sockmap.h    |  3 +
 4 files changed, 158 insertions(+)
 create mode 100644 tools/testing/selftests/bpf/progs/bpf_iter_sockmap.c
 create mode 100644 tools/testing/selftests/bpf/progs/bpf_iter_sockmap.h

Comments

Andrii Nakryiko Sept. 3, 2020, 5:35 a.m. UTC | #1 
On Tue, Sep 1, 2020 at 3:33 AM Lorenz Bauer <lmb@cloudflare.com> wrote:
>
> Add a test that exercises a basic sockmap / sockhash copy using bpf_iter.
>
> Signed-off-by: Lorenz Bauer <lmb@cloudflare.com>
> ---

just a bunch of nits, as I was passing by :-P

>  .../selftests/bpf/prog_tests/sockmap_basic.c  | 88 +++++++++++++++++++
>  tools/testing/selftests/bpf/progs/bpf_iter.h  |  9 ++
>  .../selftests/bpf/progs/bpf_iter_sockmap.c    | 58 ++++++++++++
>  .../selftests/bpf/progs/bpf_iter_sockmap.h    |  3 +
>  4 files changed, 158 insertions(+)
>  create mode 100644 tools/testing/selftests/bpf/progs/bpf_iter_sockmap.c
>  create mode 100644 tools/testing/selftests/bpf/progs/bpf_iter_sockmap.h
>
> diff --git a/tools/testing/selftests/bpf/prog_tests/sockmap_basic.c b/tools/testing/selftests/bpf/prog_tests/sockmap_basic.c
> index 9569bbac7f6e..f5b7b27f096f 100644
> --- a/tools/testing/selftests/bpf/prog_tests/sockmap_basic.c
> +++ b/tools/testing/selftests/bpf/prog_tests/sockmap_basic.c
> @@ -6,6 +6,9 @@
>  #include "test_skmsg_load_helpers.skel.h"
>  #include "test_sockmap_update.skel.h"
>  #include "test_sockmap_invalid_update.skel.h"
> +#include "bpf_iter_sockmap.skel.h"
> +
> +#include "progs/bpf_iter_sockmap.h"
>
>  #define TCP_REPAIR             19      /* TCP sock is under repair right now */
>
> @@ -196,6 +199,87 @@ static void test_sockmap_invalid_update(void)
>                 test_sockmap_invalid_update__destroy(skel);
>  }
>
> +static void test_sockmap_copy(enum bpf_map_type map_type)
> +{
> +       DECLARE_LIBBPF_OPTS(bpf_iter_attach_opts, opts);
> +       int err, len, src_fd, iter_fd, duration;
> +       union bpf_iter_link_info linfo = {0};

nit: misleading initialization, `= {}` is the same but doesn't imply
that you can fill union/struct with non-zeroes like this

> +       __s64 sock_fd[SOCKMAP_MAX_ENTRIES];
> +       __u32 i, num_sockets, max_elems;
> +       struct bpf_iter_sockmap *skel;
> +       struct bpf_map *src, *dst;
> +       struct bpf_link *link;
> +       char buf[64];
> +

[...]

> +SEC("iter/sockmap")
> +int copy_sockmap(struct bpf_iter__sockmap *ctx)
> +{
> +       struct bpf_sock *sk = ctx->sk;
> +       __u32 tmp, *key = ctx->key;
> +       int ret;
> +
> +       if (key == (void *)0)

nit: seems like a verbose way to just write `if (!key)`?

> +               return 0;
> +
> +       elems++;
> +
> +       /* We need a temporary buffer on the stack, since the verifier doesn't
> +        * let us use the pointer from the context as an argument to the helper.
> +        */
> +       tmp = *key;
> +       bpf_printk("key: %u\n", tmp);

is this intentional or a debugging leftover?

> +
> +       if (sk != (void *)0)
> +               return bpf_map_update_elem(&dst, &tmp, sk, 0) != 0;
> +
> +       ret = bpf_map_delete_elem(&dst, &tmp);
> +       return ret && ret != -ENOENT;
> +}
> diff --git a/tools/testing/selftests/bpf/progs/bpf_iter_sockmap.h b/tools/testing/selftests/bpf/progs/bpf_iter_sockmap.h
> new file mode 100644
> index 000000000000..f98ad727ac06
> --- /dev/null
> +++ b/tools/testing/selftests/bpf/progs/bpf_iter_sockmap.h
> @@ -0,0 +1,3 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +#define SOCKMAP_MAX_ENTRIES (64)
> --
> 2.25.1
>
Lorenz Bauer Sept. 3, 2020, 8:38 a.m. UTC | #2 
On Thu, 3 Sep 2020 at 06:35, Andrii Nakryiko <andrii.nakryiko@gmail.com> wrote:
>
> On Tue, Sep 1, 2020 at 3:33 AM Lorenz Bauer <lmb@cloudflare.com> wrote:
> >
> > Add a test that exercises a basic sockmap / sockhash copy using bpf_iter.
> >
> > Signed-off-by: Lorenz Bauer <lmb@cloudflare.com>
> > ---
>
> just a bunch of nits, as I was passing by :-P

Appreciated, as always :)

>
> >  .../selftests/bpf/prog_tests/sockmap_basic.c  | 88 +++++++++++++++++++
> >  tools/testing/selftests/bpf/progs/bpf_iter.h  |  9 ++
> >  .../selftests/bpf/progs/bpf_iter_sockmap.c    | 58 ++++++++++++
> >  .../selftests/bpf/progs/bpf_iter_sockmap.h    |  3 +
> >  4 files changed, 158 insertions(+)
> >  create mode 100644 tools/testing/selftests/bpf/progs/bpf_iter_sockmap.c
> >  create mode 100644 tools/testing/selftests/bpf/progs/bpf_iter_sockmap.h
> >
> > diff --git a/tools/testing/selftests/bpf/prog_tests/sockmap_basic.c b/tools/testing/selftests/bpf/prog_tests/sockmap_basic.c
> > index 9569bbac7f6e..f5b7b27f096f 100644
> > --- a/tools/testing/selftests/bpf/prog_tests/sockmap_basic.c
> > +++ b/tools/testing/selftests/bpf/prog_tests/sockmap_basic.c
> > @@ -6,6 +6,9 @@
> >  #include "test_skmsg_load_helpers.skel.h"
> >  #include "test_sockmap_update.skel.h"
> >  #include "test_sockmap_invalid_update.skel.h"
> > +#include "bpf_iter_sockmap.skel.h"
> > +
> > +#include "progs/bpf_iter_sockmap.h"
> >
> >  #define TCP_REPAIR             19      /* TCP sock is under repair right now */
> >
> > @@ -196,6 +199,87 @@ static void test_sockmap_invalid_update(void)
> >                 test_sockmap_invalid_update__destroy(skel);
> >  }
> >
> > +static void test_sockmap_copy(enum bpf_map_type map_type)
> > +{
> > +       DECLARE_LIBBPF_OPTS(bpf_iter_attach_opts, opts);
> > +       int err, len, src_fd, iter_fd, duration;
> > +       union bpf_iter_link_info linfo = {0};
>
> nit: misleading initialization, `= {}` is the same but doesn't imply
> that you can fill union/struct with non-zeroes like this

Ack.
>
> > +       __s64 sock_fd[SOCKMAP_MAX_ENTRIES];
> > +       __u32 i, num_sockets, max_elems;
> > +       struct bpf_iter_sockmap *skel;
> > +       struct bpf_map *src, *dst;
> > +       struct bpf_link *link;
> > +       char buf[64];
> > +
>
> [...]
>
> > +SEC("iter/sockmap")
> > +int copy_sockmap(struct bpf_iter__sockmap *ctx)
> > +{
> > +       struct bpf_sock *sk = ctx->sk;
> > +       __u32 tmp, *key = ctx->key;
> > +       int ret;
> > +
> > +       if (key == (void *)0)
>
> nit: seems like a verbose way to just write `if (!key)`?

Yeah, this is copypasta from the other iterator test. I'll change this.

>
> > +               return 0;
> > +
> > +       elems++;
> > +
> > +       /* We need a temporary buffer on the stack, since the verifier doesn't
> > +        * let us use the pointer from the context as an argument to the helper.
> > +        */
> > +       tmp = *key;
> > +       bpf_printk("key: %u\n", tmp);
>
> is this intentional or a debugging leftover?

Oops!

>
> > +
> > +       if (sk != (void *)0)
> > +               return bpf_map_update_elem(&dst, &tmp, sk, 0) != 0;
> > +
> > +       ret = bpf_map_delete_elem(&dst, &tmp);
> > +       return ret && ret != -ENOENT;
> > +}
> > diff --git a/tools/testing/selftests/bpf/progs/bpf_iter_sockmap.h b/tools/testing/selftests/bpf/progs/bpf_iter_sockmap.h
> > new file mode 100644
> > index 000000000000..f98ad727ac06
> > --- /dev/null
> > +++ b/tools/testing/selftests/bpf/progs/bpf_iter_sockmap.h
> > @@ -0,0 +1,3 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +
> > +#define SOCKMAP_MAX_ENTRIES (64)
> > --
> > 2.25.1
> >
diff mbox series

Patch

diff --git a/tools/testing/selftests/bpf/prog_tests/sockmap_basic.c b/tools/testing/selftests/bpf/prog_tests/sockmap_basic.c
index 9569bbac7f6e..f5b7b27f096f 100644
--- a/tools/testing/selftests/bpf/prog_tests/sockmap_basic.c
+++ b/tools/testing/selftests/bpf/prog_tests/sockmap_basic.c
@@ -6,6 +6,9 @@ 
 #include "test_skmsg_load_helpers.skel.h"
 #include "test_sockmap_update.skel.h"
 #include "test_sockmap_invalid_update.skel.h"
+#include "bpf_iter_sockmap.skel.h"
+
+#include "progs/bpf_iter_sockmap.h"
 
 #define TCP_REPAIR		19	/* TCP sock is under repair right now */
 
@@ -196,6 +199,87 @@  static void test_sockmap_invalid_update(void)
 		test_sockmap_invalid_update__destroy(skel);
 }
 
+static void test_sockmap_copy(enum bpf_map_type map_type)
+{
+	DECLARE_LIBBPF_OPTS(bpf_iter_attach_opts, opts);
+	int err, len, src_fd, iter_fd, duration;
+	union bpf_iter_link_info linfo = {0};
+	__s64 sock_fd[SOCKMAP_MAX_ENTRIES];
+	__u32 i, num_sockets, max_elems;
+	struct bpf_iter_sockmap *skel;
+	struct bpf_map *src, *dst;
+	struct bpf_link *link;
+	char buf[64];
+
+	skel = bpf_iter_sockmap__open_and_load();
+	if (CHECK(!skel, "bpf_iter_sockmap__open_and_load",
+		  "skeleton open_and_load failed\n"))
+		return;
+
+	memset(sock_fd, 0xff, sizeof(sock_fd));
+
+	/* Make sure we have at least one "empty" entry to test iteration of
+	 * an empty slot in an array.
+	 */
+	num_sockets = ARRAY_SIZE(sock_fd) - 1;
+
+	if (map_type == BPF_MAP_TYPE_SOCKMAP) {
+		src = skel->maps.sockmap;
+		max_elems = bpf_map__max_entries(src);
+	} else {
+		src = skel->maps.sockhash;
+		max_elems = num_sockets;
+	}
+
+	dst = skel->maps.dst;
+	src_fd = bpf_map__fd(src);
+
+	for (i = 0; i < num_sockets; i++) {
+		sock_fd[i] = connected_socket_v4();
+		if (CHECK(sock_fd[i] == -1, "connected_socket_v4", "cannot connect\n"))
+			goto out;
+
+		err = bpf_map_update_elem(src_fd, &i, &sock_fd[i], BPF_NOEXIST);
+		if (CHECK(err, "map_update", "failed: %s\n", strerror(errno)))
+			goto out;
+	}
+
+	linfo.map.map_fd = src_fd;
+	opts.link_info = &linfo;
+	opts.link_info_len = sizeof(linfo);
+	link = bpf_program__attach_iter(skel->progs.copy_sockmap, &opts);
+	if (CHECK(IS_ERR(link), "attach_iter", "attach_iter failed\n"))
+		goto out;
+
+	iter_fd = bpf_iter_create(bpf_link__fd(link));
+	if (CHECK(iter_fd < 0, "create_iter", "create_iter failed\n"))
+		goto free_link;
+
+	/* do some tests */
+	while ((len = read(iter_fd, buf, sizeof(buf))) > 0)
+		;
+	if (CHECK(len < 0, "read", "failed: %s\n", strerror(errno)))
+		goto close_iter;
+
+	/* test results */
+	if (CHECK(skel->bss->elems != max_elems, "elems", "got %u expected %u\n",
+		  skel->bss->elems, max_elems))
+		goto close_iter;
+
+	compare_cookies(src, dst);
+
+close_iter:
+	close(iter_fd);
+free_link:
+	bpf_link__destroy(link);
+out:
+	for (i = 0; i < num_sockets; i++) {
+		if (sock_fd[i] >= 0)
+			close(sock_fd[i]);
+	}
+	bpf_iter_sockmap__destroy(skel);
+}
+
 void test_sockmap_basic(void)
 {
 	if (test__start_subtest("sockmap create_update_free"))
@@ -212,4 +296,8 @@  void test_sockmap_basic(void)
 		test_sockmap_update(BPF_MAP_TYPE_SOCKHASH);
 	if (test__start_subtest("sockmap update in unsafe context"))
 		test_sockmap_invalid_update();
+	if (test__start_subtest("sockmap copy"))
+		test_sockmap_copy(BPF_MAP_TYPE_SOCKMAP);
+	if (test__start_subtest("sockhash copy"))
+		test_sockmap_copy(BPF_MAP_TYPE_SOCKHASH);
 }
diff --git a/tools/testing/selftests/bpf/progs/bpf_iter.h b/tools/testing/selftests/bpf/progs/bpf_iter.h
index c196280df90d..ac32a29f5153 100644
--- a/tools/testing/selftests/bpf/progs/bpf_iter.h
+++ b/tools/testing/selftests/bpf/progs/bpf_iter.h
@@ -13,6 +13,7 @@ 
 #define udp6_sock udp6_sock___not_used
 #define bpf_iter__bpf_map_elem bpf_iter__bpf_map_elem___not_used
 #define bpf_iter__bpf_sk_storage_map bpf_iter__bpf_sk_storage_map___not_used
+#define bpf_iter__sockmap bpf_iter__sockmap___not_used
 #include "vmlinux.h"
 #undef bpf_iter_meta
 #undef bpf_iter__bpf_map
@@ -26,6 +27,7 @@ 
 #undef udp6_sock
 #undef bpf_iter__bpf_map_elem
 #undef bpf_iter__bpf_sk_storage_map
+#undef bpf_iter__sockmap
 
 struct bpf_iter_meta {
 	struct seq_file *seq;
@@ -96,3 +98,10 @@  struct bpf_iter__bpf_sk_storage_map {
 	struct sock *sk;
 	void *value;
 };
+
+struct bpf_iter__sockmap {
+	struct bpf_iter_meta *meta;
+	struct bpf_map *map;
+	void *key;
+	struct bpf_sock *sk;
+};
diff --git a/tools/testing/selftests/bpf/progs/bpf_iter_sockmap.c b/tools/testing/selftests/bpf/progs/bpf_iter_sockmap.c
new file mode 100644
index 000000000000..d236bc76cc06
--- /dev/null
+++ b/tools/testing/selftests/bpf/progs/bpf_iter_sockmap.c
@@ -0,0 +1,58 @@ 
+// SPDX-License-Identifier: GPL-2.0
+/* Copyright (c) 2020 Cloudflare */
+#include "bpf_iter.h"
+#include "bpf_tracing_net.h"
+#include "bpf_iter_sockmap.h"
+#include <bpf/bpf_helpers.h>
+#include <bpf/bpf_tracing.h>
+#include <errno.h>
+
+char _license[] SEC("license") = "GPL";
+
+struct {
+	__uint(type, BPF_MAP_TYPE_SOCKMAP);
+	__uint(max_entries, SOCKMAP_MAX_ENTRIES);
+	__type(key, __u32);
+	__type(value, __u64);
+} sockmap SEC(".maps");
+
+struct {
+	__uint(type, BPF_MAP_TYPE_SOCKHASH);
+	__uint(max_entries, SOCKMAP_MAX_ENTRIES);
+	__type(key, __u32);
+	__type(value, __u64);
+} sockhash SEC(".maps");
+
+struct {
+	__uint(type, BPF_MAP_TYPE_SOCKHASH);
+	__uint(max_entries, SOCKMAP_MAX_ENTRIES);
+	__type(key, __u32);
+	__type(value, __u64);
+} dst SEC(".maps");
+
+__u32 elems = 0;
+
+SEC("iter/sockmap")
+int copy_sockmap(struct bpf_iter__sockmap *ctx)
+{
+	struct bpf_sock *sk = ctx->sk;
+	__u32 tmp, *key = ctx->key;
+	int ret;
+
+	if (key == (void *)0)
+		return 0;
+
+	elems++;
+
+	/* We need a temporary buffer on the stack, since the verifier doesn't
+	 * let us use the pointer from the context as an argument to the helper.
+	 */
+	tmp = *key;
+	bpf_printk("key: %u\n", tmp);
+
+	if (sk != (void *)0)
+		return bpf_map_update_elem(&dst, &tmp, sk, 0) != 0;
+
+	ret = bpf_map_delete_elem(&dst, &tmp);
+	return ret && ret != -ENOENT;
+}
diff --git a/tools/testing/selftests/bpf/progs/bpf_iter_sockmap.h b/tools/testing/selftests/bpf/progs/bpf_iter_sockmap.h
new file mode 100644
index 000000000000..f98ad727ac06
--- /dev/null
+++ b/tools/testing/selftests/bpf/progs/bpf_iter_sockmap.h
@@ -0,0 +1,3 @@ 
+// SPDX-License-Identifier: GPL-2.0
+
+#define SOCKMAP_MAX_ENTRIES (64)