On 2/8/21 10:34 AM, Andrii Nakryiko wrote:
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?
I am aware of this. I did not change this in v1 mainly wanting to
get some comments on API and verifier change etc. for v1.
I will use bpf_prog_test_run() to call the program in v2.
+
+ 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
We do not have ASSERT_GE, I may invent one.
+ 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.
Right. Will emphasize this in uapi bpf.h and also comments in the code.
+ percpu_key = *key;
+ percpu_val = *val;
+
+ bpf_for_each_map_elem(&hashmap, check_hash_elem, data, 0);
+ return 0;
+}
+
[...]
