Re: [PATCH bpf-next 3/3] selftests/bpf: add test for bpf_for_each_map_elem() with different maps

[Yonghong Song <yonghong.song@linux.dev>]

On 4/1/24 11:16 PM, Philo Lu wrote:
> A test is added for bpf_for_each_map_elem() with either an arraymap or a
> hashmap.
> $ tools/testing/selftests/bpf/test_progs -t for_each
>   #93/1    for_each/hash_map:OK
>   #93/2    for_each/array_map:OK
>   #93/3    for_each/write_map_key:OK
>   #93/4    for_each/multi_maps:OK
>   #93      for_each:OK
> Summary: 1/4 PASSED, 0 SKIPPED, 0 FAILED
>
> Signed-off-by: Philo Lu <lulie@linux.alibaba.com>
> ---
>   .../selftests/bpf/prog_tests/for_each.c       | 67 +++++++++++++++++++
>   .../selftests/bpf/progs/for_each_multi_maps.c | 49 ++++++++++++++
>   2 files changed, 116 insertions(+)
>   create mode 100644 tools/testing/selftests/bpf/progs/for_each_multi_maps.c
>
> diff --git a/tools/testing/selftests/bpf/prog_tests/for_each.c b/tools/testing/selftests/bpf/prog_tests/for_each.c
> index 8963f8a549f2..61c5e064f89c 100644
> --- a/tools/testing/selftests/bpf/prog_tests/for_each.c
> +++ b/tools/testing/selftests/bpf/prog_tests/for_each.c
> @@ -5,6 +5,7 @@
>   #include "for_each_hash_map_elem.skel.h"
>   #include "for_each_array_map_elem.skel.h"
>   #include "for_each_map_elem_write_key.skel.h"
> +#include "for_each_multi_maps.skel.h"
>   
>   static unsigned int duration;
>   
> @@ -143,6 +144,70 @@ static void test_write_map_key(void)
>   		for_each_map_elem_write_key__destroy(skel);
>   }
>   
> +static void test_multi_maps(void)
> +{
> +	struct for_each_multi_maps *skel;
> +	__u64 val, array_total, hash_total;
> +	__u32 key, max_entries;
> +	int i, err;
> +
> +	LIBBPF_OPTS(bpf_test_run_opts, topts,
> +		.data_in = &pkt_v4,
> +		.data_size_in = sizeof(pkt_v4),
> +		.repeat = 1,
> +	);
> +
> +	skel = for_each_multi_maps__open_and_load();
> +	if (!ASSERT_OK_PTR(skel, "for_each_multi_maps__open_and_load"))
> +		return;
> +
> +	array_total = 0;
> +	/* left last element as empty */
> +	max_entries = bpf_map__max_entries(skel->maps.arraymap) - 1;

Any particular reason you skip the last element?

> +	for (i = 0; i < max_entries; i++) {
> +		key = i;
> +		val = i + 1;
> +		array_total += val;
> +		err = bpf_map__update_elem(skel->maps.arraymap, &key, sizeof(key),
> +					   &val, sizeof(val), BPF_ANY);
> +		if (!ASSERT_OK(err, "array_map_update"))
> +			goto out;
> +	}
> +
> +	hash_total = 0;
> +	max_entries = bpf_map__max_entries(skel->maps.hashmap) - 1;

ditto.

> +	for (i = 0; i < max_entries; i++) {
> +		key = i + 100;
> +		val = i + 1;
> +		hash_total += val;
> +		err = bpf_map__update_elem(skel->maps.hashmap, &key, sizeof(key),
> +					   &val, sizeof(val), BPF_ANY);
> +		if (!ASSERT_OK(err, "hash_map_update"))
> +			goto out;
> +	}
> +
> +	skel->bss->data_output = 0;
> +	skel->bss->use_array = 1;
> +	err = bpf_prog_test_run_opts(bpf_program__fd(skel->progs.test_pkt_access), &topts);
> +	if (CHECK(err || topts.retval, "ipv4", "err %d errno %d retval %d\n",
> +		  err, errno, topts.retval))
> +		goto out;

Let us use ASSERT_* macros here. In this case, you can do
	ASSERT_OK(err, "bpf_prog_test_run_opts");
	ASSERT_OK(opts.retval, "retval");
There is no need to goto out, we can continue testing below.

> +
> +	ASSERT_EQ(skel->bss->data_output, array_total, "array output");
> +
> +	skel->bss->data_output = 0;
> +	skel->bss->use_array = 0;
> +	err = bpf_prog_test_run_opts(bpf_program__fd(skel->progs.test_pkt_access), &topts);
> +	if (CHECK(err || topts.retval, "ipv4", "err %d errno %d retval %d\n",
> +		  err, errno, topts.retval))
> +		goto out;

Do two ASSERT_OK similar to the above.

> +
> +	ASSERT_EQ(skel->bss->data_output, hash_total, "hash output");
> +
> +out:
> +	for_each_multi_maps__destroy(skel);
> +}
> +
> [...]