Re: [PATCH bpf-next v5 3/3] selftests: bpf: Test iterating a sockmap

[Yonghong Song <yhs@fb.com>]

On 9/9/20 9:27 AM, Lorenz Bauer wrote:
> Add a test that exercises a basic sockmap / sockhash iteration. For
> now we simply count the number of elements seen. Once sockmap update
> from iterators works we can extend this to perform a full copy.
> 
> Signed-off-by: Lorenz Bauer <lmb@cloudflare.com>
> ---
>   .../selftests/bpf/prog_tests/sockmap_basic.c  | 89 +++++++++++++++++++
>   tools/testing/selftests/bpf/progs/bpf_iter.h  |  9 ++
>   .../selftests/bpf/progs/bpf_iter_sockmap.c    | 43 +++++++++
>   .../selftests/bpf/progs/bpf_iter_sockmap.h    |  3 +
>   4 files changed, 144 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 0b79d78b98db..3215f4d22720 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 */
>   
> @@ -171,6 +174,88 @@ static void test_sockmap_invalid_update(void)
>   		test_sockmap_invalid_update__destroy(skel);
>   }
>   
> +static void test_sockmap_iter(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_link *link;
> +	struct bpf_map *src;
> +	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;
> +
> +	for (i = 0; i < ARRAY_SIZE(sock_fd); i++)
> +		sock_fd[i] = -1;
> +
> +	/* Make sure we have at least one "empty" entry to test iteration of
> +	 * an empty slot.
> +	 */
> +	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;
> +	}

I know you include the shared header progs/bpf_iter_sockmap.h to
supply SOCKMAP_MAX_ENTRIES in order to define sock_fd array.

I think it is easier to understand if just using bpf_map__max_entries() 
for both sockmap and sockhash to get max_elems and do dynamic allocation 
for sock_fd. WDYT?

> +
> +	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;
> +	}
> +
[...]