On Thu, Feb 4, 2021 at 5:53 PM Yonghong Song <yhs@xxxxxx> wrote:
>
> A test case is added for hashmap and percpu hashmap. The test
> also exercises nested bpf_for_each_map_elem() calls like
> bpf_prog:
> bpf_for_each_map_elem(func1)
> func1:
> bpf_for_each_map_elem(func2)
> func2:
>
> $ ./test_progs -n 44
> #44/1 hash_map:OK
> #44 for_each:OK
> Summary: 1/1 PASSED, 0 SKIPPED, 0 FAILED
>
> Signed-off-by: Yonghong Song <yhs@xxxxxx>
> ---
> .../selftests/bpf/prog_tests/for_each.c | 91 ++++++++++++++++
> .../bpf/progs/for_each_hash_map_elem.c | 103 ++++++++++++++++++
> 2 files changed, 194 insertions(+)
> create mode 100644 tools/testing/selftests/bpf/prog_tests/for_each.c
> create mode 100644 tools/testing/selftests/bpf/progs/for_each_hash_map_elem.c
>
[...]
> + num_cpus = bpf_num_possible_cpus();
> + percpu_map_fd = bpf_map__fd(skel->maps.percpu_map);
> + percpu_valbuf = malloc(sizeof(__u64) * num_cpus);
> + if (CHECK_FAIL(!percpu_valbuf))
> + goto out;
> +
> + key = 1;
> + for (i = 0; i < num_cpus; i++)
> + percpu_valbuf[i] = i + 1;
> + err = bpf_map_update_elem(percpu_map_fd, &key, percpu_valbuf, BPF_ANY);
> + if (CHECK(err, "percpu_map_update", "map_update failed\n"))
> + goto out;
> +
> + do_dummy_read(skel->progs.dump_task);
why use iter/task programs to trigger your test BPF code? This test
doesn't seem to rely on anything iter-specific, so it's much simpler
(and less code) to just use the typical sys_enter approach with
usleep(1) as a trigger function, no?
> +
> + ASSERT_EQ(skel->bss->called, 1, "called");
> + ASSERT_EQ(skel->bss->hashmap_output, 4, "output_val");
> +
> + key = 1;
> + err = bpf_map_lookup_elem(hashmap_fd, &key, &val);
> + ASSERT_ERR(err, "hashmap_lookup");
> +
> + ASSERT_EQ(skel->bss->percpu_called, 1, "percpu_called");
> + CHECK_FAIL(skel->bss->cpu >= num_cpus);
please don't use CHECK_FAIL: use CHECK or one of ASSERT_xxx variants
> + ASSERT_EQ(skel->bss->percpu_key, 1, "percpu_key");
> + ASSERT_EQ(skel->bss->percpu_val, skel->bss->cpu + 1, "percpu_val");
> + ASSERT_EQ(skel->bss->percpu_output, 100, "percpu_output");
> +out:
> + free(percpu_valbuf);
> + for_each_hash_map_elem__destroy(skel);
> +}
> +
> +void test_for_each(void)
> +{
> + if (test__start_subtest("hash_map"))
> + test_hash_map();
> +}
[...]
> +
> +__u32 cpu = 0;
> +__u32 percpu_called = 0;
> +__u32 percpu_key = 0;
> +__u64 percpu_val = 0;
> +
> +static __u64
> +check_percpu_elem(struct bpf_map *map, __u32 *key, __u64 *val,
> + struct callback_ctx *data)
> +{
> + percpu_called++;
> + cpu = bpf_get_smp_processor_id();
It's a bit counter-intuitive (at least I was confused initially) that
for a per-cpu array for_each() will iterate only current CPU's
elements. I think it's worthwhile to emphasize this in
bpf_for_each_map_elem() documentation (at least), and call out in
corresponding patches adding per-cpu array/hash iteration support.
> + percpu_key = *key;
> + percpu_val = *val;
> +
> + bpf_for_each_map_elem(&hashmap, check_hash_elem, data, 0);
> + return 0;
> +}
> +
[...]
