Re: [PATCH bpf-next 4/6] bpf, selftests: add test for different array inner map size

[Yonghong Song <yhs@xxxxxx>]

On 10/8/20 2:31 PM, Daniel Borkmann wrote:
> Extend the "diff_size" subtest to also include a non-inlined array map variant
> where dynamic inner #elems are possible.
>
> Signed-off-by: Daniel Borkmann <daniel@xxxxxxxxxxxxx>

Ack with a minor comment below.

Acked-by: Yonghong Song <yhs@xxxxxx>

> ---
>   .../selftests/bpf/prog_tests/btf_map_in_map.c | 39 ++++++++++++-----
>   .../selftests/bpf/progs/test_btf_map_in_map.c | 43 +++++++++++++++++++
>   2 files changed, 72 insertions(+), 10 deletions(-)
>
> diff --git a/tools/testing/selftests/bpf/prog_tests/btf_map_in_map.c b/tools/testing/selftests/bpf/prog_tests/btf_map_in_map.c
> index 540fea4c91a5..e478bdec73b8 100644
> --- a/tools/testing/selftests/bpf/prog_tests/btf_map_in_map.c
> +++ b/tools/testing/selftests/bpf/prog_tests/btf_map_in_map.c
> @@ -55,10 +55,10 @@ static int kern_sync_rcu(void)
>   
>   static void test_lookup_update(void)
>   {
> -	int err, key = 0, val, i;
> +	int map1_fd, map2_fd, map3_fd, map4_fd, map5_fd, map1_id, map2_id;
> +	int outer_arr_fd, outer_hash_fd, outer_arr_dyn_fd;
>   	struct test_btf_map_in_map *skel;
> -	int outer_arr_fd, outer_hash_fd;
> -	int fd, map1_fd, map2_fd, map1_id, map2_id;
> +	int err, key = 0, val, i, fd;
>   
>   	skel = test_btf_map_in_map__open_and_load();
>   	if (CHECK(!skel, "skel_open", "failed to open&load skeleton\n"))
> @@ -70,32 +70,45 @@ static void test_lookup_update(void)
>   
>   	map1_fd = bpf_map__fd(skel->maps.inner_map1);
>   	map2_fd = bpf_map__fd(skel->maps.inner_map2);
> +	map3_fd = bpf_map__fd(skel->maps.inner_map3);
> +	map4_fd = bpf_map__fd(skel->maps.inner_map4);
> +	map5_fd = bpf_map__fd(skel->maps.inner_map5);
> +	outer_arr_dyn_fd = bpf_map__fd(skel->maps.outer_arr_dyn);
>   	outer_arr_fd = bpf_map__fd(skel->maps.outer_arr);
>   	outer_hash_fd = bpf_map__fd(skel->maps.outer_hash);
>   
> -	/* inner1 = input, inner2 = input + 1 */
> -	map1_fd = bpf_map__fd(skel->maps.inner_map1);
> +	/* inner1 = input, inner2 = input + 1, inner3 = input + 2 */
>   	bpf_map_update_elem(outer_arr_fd, &key, &map1_fd, 0);
> -	map2_fd = bpf_map__fd(skel->maps.inner_map2);
>   	bpf_map_update_elem(outer_hash_fd, &key, &map2_fd, 0);
> +	bpf_map_update_elem(outer_arr_dyn_fd, &key, &map3_fd, 0);
>   	skel->bss->input = 1;
>   	usleep(1);
> -
>   	bpf_map_lookup_elem(map1_fd, &key, &val);
>   	CHECK(val != 1, "inner1", "got %d != exp %d\n", val, 1);
>   	bpf_map_lookup_elem(map2_fd, &key, &val);
>   	CHECK(val != 2, "inner2", "got %d != exp %d\n", val, 2);
> +	bpf_map_lookup_elem(map3_fd, &key, &val);
> +	CHECK(val != 3, "inner3", "got %d != exp %d\n", val, 3);
>   
> -	/* inner1 = input + 1, inner2 = input */
> +	/* inner1 = input, inner2 = input + 1, inner4 = input + 2 */

The changed comments sound not right.


>   	bpf_map_update_elem(outer_arr_fd, &key, &map2_fd, 0);
>   	bpf_map_update_elem(outer_hash_fd, &key, &map1_fd, 0);
> +	bpf_map_update_elem(outer_arr_dyn_fd, &key, &map4_fd, 0);
>   	skel->bss->input = 3;
>   	usleep(1);
> -
>   	bpf_map_lookup_elem(map1_fd, &key, &val);
>   	CHECK(val != 4, "inner1", "got %d != exp %d\n", val, 4);

We have inner1 = input + 1 here.

>   	bpf_map_lookup_elem(map2_fd, &key, &val);
>   	CHECK(val != 3, "inner2", "got %d != exp %d\n", val, 3);

inner2 = input here.

> +	bpf_map_lookup_elem(map4_fd, &key, &val);
> +	CHECK(val != 5, "inner4", "got %d != exp %d\n", val, 5);
> +
[...]