On Tue, Sep 1, 2020 at 3:33 AM Lorenz Bauer <lmb@xxxxxxxxxxxxxx> wrote:
>
> Add a test that exercises a basic sockmap / sockhash copy using bpf_iter.
>
> Signed-off-by: Lorenz Bauer <lmb@xxxxxxxxxxxxxx>
> ---
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
>
