| Message ID | 20210204234836.1629791-1-yhs@fb.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | BPF |
| Headers | show |
| Series | bpf: add bpf_for_each_map_elem() helper | expand |
On Thu, Feb 4, 2021 at 5:53 PM Yonghong Song <yhs@fb.com> wrote: > > A test is added for arraymap and percpu arraymap. The test also > exercises the early return for the helper which does not > traverse all elements. > $ ./test_progs -n 44 > #44/1 hash_map:OK > #44/2 array_map:OK > #44 for_each:OK > Summary: 1/2 PASSED, 0 SKIPPED, 0 FAILED > > Signed-off-by: Yonghong Song <yhs@fb.com> > --- > .../selftests/bpf/prog_tests/for_each.c | 54 ++++++++++++++ > .../bpf/progs/for_each_array_map_elem.c | 71 +++++++++++++++++++ > 2 files changed, 125 insertions(+) > create mode 100644 tools/testing/selftests/bpf/progs/for_each_array_map_elem.c > [...] > + > + arraymap_fd = bpf_map__fd(skel->maps.arraymap); > + expected_total = 0; > + max_entries = bpf_map__max_entries(skel->maps.arraymap); > + for (i = 0; i < max_entries; i++) { > + key = i; > + val = i + 1; > + /* skip the last iteration for expected total */ > + if (i != max_entries - 1) > + expected_total += val; > + err = bpf_map_update_elem(arraymap_fd, &key, &val, BPF_ANY); > + if (CHECK(err, "map_update", "map_update failed\n")) > + goto out; > + } > + > + num_cpus = bpf_num_possible_cpus(); > + percpu_map_fd = bpf_map__fd(skel->maps.percpu_map); formatting is off, please check with checkfile.pl > + percpu_valbuf = malloc(sizeof(__u64) * num_cpus); > + if (CHECK_FAIL(!percpu_valbuf)) please don't use CHECK_FAIL, ASSERT_PTR_OK would work nicely here > + goto out; > + > + key = 0; > + 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); see previous patch, iter/tasks seems like an overkill for these tests > + > + ASSERT_EQ(skel->bss->called, 1, "called"); > + ASSERT_EQ(skel->bss->arraymap_output, expected_total, "array_output"); > + ASSERT_EQ(skel->bss->cpu + 1, skel->bss->percpu_val, "percpu_val"); > + > +out: > + free(percpu_valbuf); > + for_each_array_map_elem__destroy(skel); > +} > + [...] > +SEC("iter/task") > +int dump_task(struct bpf_iter__task *ctx) > +{ > + struct seq_file *seq = ctx->meta->seq; > + struct task_struct *task = ctx->task; > + struct callback_ctx data; > + > + /* only call once */ > + if (called > 0) > + return 0; > + > + called++; > + > + data.output = 0; > + bpf_for_each_map_elem(&arraymap, check_array_elem, &data, 0); > + arraymap_output = data.output; > + > + bpf_for_each_map_elem(&percpu_map, check_percpu_elem, 0, 0); nit: NULL, 0 ? > + > + return 0; > +} > -- > 2.24.1 >
On 2/8/21 10:35 AM, Andrii Nakryiko wrote: > On Thu, Feb 4, 2021 at 5:53 PM Yonghong Song <yhs@fb.com> wrote: >> >> A test is added for arraymap and percpu arraymap. The test also >> exercises the early return for the helper which does not >> traverse all elements. >> $ ./test_progs -n 44 >> #44/1 hash_map:OK >> #44/2 array_map:OK >> #44 for_each:OK >> Summary: 1/2 PASSED, 0 SKIPPED, 0 FAILED >> >> Signed-off-by: Yonghong Song <yhs@fb.com> >> --- >> .../selftests/bpf/prog_tests/for_each.c | 54 ++++++++++++++ >> .../bpf/progs/for_each_array_map_elem.c | 71 +++++++++++++++++++ >> 2 files changed, 125 insertions(+) >> create mode 100644 tools/testing/selftests/bpf/progs/for_each_array_map_elem.c >> > > [...] > >> + >> + arraymap_fd = bpf_map__fd(skel->maps.arraymap); >> + expected_total = 0; >> + max_entries = bpf_map__max_entries(skel->maps.arraymap); >> + for (i = 0; i < max_entries; i++) { >> + key = i; >> + val = i + 1; >> + /* skip the last iteration for expected total */ >> + if (i != max_entries - 1) >> + expected_total += val; >> + err = bpf_map_update_elem(arraymap_fd, &key, &val, BPF_ANY); >> + if (CHECK(err, "map_update", "map_update failed\n")) >> + goto out; >> + } >> + >> + num_cpus = bpf_num_possible_cpus(); >> + percpu_map_fd = bpf_map__fd(skel->maps.percpu_map); > > formatting is off, please check with checkfile.pl Sure. will do. > > >> + percpu_valbuf = malloc(sizeof(__u64) * num_cpus); >> + if (CHECK_FAIL(!percpu_valbuf)) > > please don't use CHECK_FAIL, ASSERT_PTR_OK would work nicely here Okay. > >> + goto out; >> + >> + key = 0; >> + 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); > > see previous patch, iter/tasks seems like an overkill for these tests Yes, will use bpf_prog_test_run() in v2. > >> + >> + ASSERT_EQ(skel->bss->called, 1, "called"); >> + ASSERT_EQ(skel->bss->arraymap_output, expected_total, "array_output"); >> + ASSERT_EQ(skel->bss->cpu + 1, skel->bss->percpu_val, "percpu_val"); >> + >> +out: >> + free(percpu_valbuf); >> + for_each_array_map_elem__destroy(skel); >> +} >> + > > [...] > >> +SEC("iter/task") >> +int dump_task(struct bpf_iter__task *ctx) >> +{ >> + struct seq_file *seq = ctx->meta->seq; >> + struct task_struct *task = ctx->task; >> + struct callback_ctx data; >> + >> + /* only call once */ >> + if (called > 0) >> + return 0; >> + >> + called++; >> + >> + data.output = 0; >> + bpf_for_each_map_elem(&arraymap, check_array_elem, &data, 0); >> + arraymap_output = data.output; >> + >> + bpf_for_each_map_elem(&percpu_map, check_percpu_elem, 0, 0); > > nit: NULL, 0 ? We do not NULL defined in vmlinux.h or bpf_helpers.h. Hence I am using 0, I should at least use (void *)0 here, I think. > >> + >> + return 0; >> +} >> -- >> 2.24.1 >>
On Mon, Feb 8, 2021 at 10:50 PM Yonghong Song <yhs@fb.com> wrote: > > > > On 2/8/21 10:35 AM, Andrii Nakryiko wrote: > > On Thu, Feb 4, 2021 at 5:53 PM Yonghong Song <yhs@fb.com> wrote: > >> > >> A test is added for arraymap and percpu arraymap. The test also > >> exercises the early return for the helper which does not > >> traverse all elements. > >> $ ./test_progs -n 44 > >> #44/1 hash_map:OK > >> #44/2 array_map:OK > >> #44 for_each:OK > >> Summary: 1/2 PASSED, 0 SKIPPED, 0 FAILED > >> > >> Signed-off-by: Yonghong Song <yhs@fb.com> > >> --- > >> .../selftests/bpf/prog_tests/for_each.c | 54 ++++++++++++++ > >> .../bpf/progs/for_each_array_map_elem.c | 71 +++++++++++++++++++ > >> 2 files changed, 125 insertions(+) > >> create mode 100644 tools/testing/selftests/bpf/progs/for_each_array_map_elem.c > >> > > > > [...] > > > >> + > >> + arraymap_fd = bpf_map__fd(skel->maps.arraymap); > >> + expected_total = 0; > >> + max_entries = bpf_map__max_entries(skel->maps.arraymap); > >> + for (i = 0; i < max_entries; i++) { > >> + key = i; > >> + val = i + 1; > >> + /* skip the last iteration for expected total */ > >> + if (i != max_entries - 1) > >> + expected_total += val; > >> + err = bpf_map_update_elem(arraymap_fd, &key, &val, BPF_ANY); > >> + if (CHECK(err, "map_update", "map_update failed\n")) > >> + goto out; > >> + } > >> + > >> + num_cpus = bpf_num_possible_cpus(); > >> + percpu_map_fd = bpf_map__fd(skel->maps.percpu_map); > > > > formatting is off, please check with checkfile.pl > > Sure. will do. > > > > > > >> + percpu_valbuf = malloc(sizeof(__u64) * num_cpus); > >> + if (CHECK_FAIL(!percpu_valbuf)) > > > > please don't use CHECK_FAIL, ASSERT_PTR_OK would work nicely here > > Okay. > > > > >> + goto out; > >> + > >> + key = 0; > >> + 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); > > > > see previous patch, iter/tasks seems like an overkill for these tests > > Yes, will use bpf_prog_test_run() in v2. > > > > >> + > >> + ASSERT_EQ(skel->bss->called, 1, "called"); > >> + ASSERT_EQ(skel->bss->arraymap_output, expected_total, "array_output"); > >> + ASSERT_EQ(skel->bss->cpu + 1, skel->bss->percpu_val, "percpu_val"); > >> + > >> +out: > >> + free(percpu_valbuf); > >> + for_each_array_map_elem__destroy(skel); > >> +} > >> + > > > > [...] > > > >> +SEC("iter/task") > >> +int dump_task(struct bpf_iter__task *ctx) > >> +{ > >> + struct seq_file *seq = ctx->meta->seq; > >> + struct task_struct *task = ctx->task; > >> + struct callback_ctx data; > >> + > >> + /* only call once */ > >> + if (called > 0) > >> + return 0; > >> + > >> + called++; > >> + > >> + data.output = 0; > >> + bpf_for_each_map_elem(&arraymap, check_array_elem, &data, 0); > >> + arraymap_output = data.output; > >> + > >> + bpf_for_each_map_elem(&percpu_map, check_percpu_elem, 0, 0); > > > > nit: NULL, 0 ? > > We do not NULL defined in vmlinux.h or bpf_helpers.h. Hence I am using > 0, I should at least use (void *)0 here, I think. yeah, this NULL problem is annoying. Let's just add NULL to vmlinux.h, you are not the first one to complain. We can do that separately from this patch set, of course. > > > > >> + > >> + return 0; > >> +} > >> -- > >> 2.24.1 > >>
diff --git a/tools/testing/selftests/bpf/prog_tests/for_each.c b/tools/testing/selftests/bpf/prog_tests/for_each.c index 7a399fbc89a4..58074212875b 100644 --- a/tools/testing/selftests/bpf/prog_tests/for_each.c +++ b/tools/testing/selftests/bpf/prog_tests/for_each.c @@ -2,6 +2,7 @@ /* Copyright (c) 2021 Facebook */ #include <test_progs.h> #include "for_each_hash_map_elem.skel.h" +#include "for_each_array_map_elem.skel.h" static int duration; @@ -84,8 +85,61 @@ static void test_hash_map(void) for_each_hash_map_elem__destroy(skel); } +static void test_array_map(void) +{ + int i, arraymap_fd, percpu_map_fd, err; + struct for_each_array_map_elem *skel; + __u32 key, num_cpus, max_entries; + __u64 *percpu_valbuf = NULL; + __u64 val, expected_total; + + skel = for_each_array_map_elem__open_and_load(); + if (CHECK(!skel, "for_each_array_map_elem__open_and_load", + "skeleton open_and_load failed\n")) + return; + + arraymap_fd = bpf_map__fd(skel->maps.arraymap); + expected_total = 0; + max_entries = bpf_map__max_entries(skel->maps.arraymap); + for (i = 0; i < max_entries; i++) { + key = i; + val = i + 1; + /* skip the last iteration for expected total */ + if (i != max_entries - 1) + expected_total += val; + err = bpf_map_update_elem(arraymap_fd, &key, &val, BPF_ANY); + if (CHECK(err, "map_update", "map_update failed\n")) + goto out; + } + + 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 = 0; + 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); + + ASSERT_EQ(skel->bss->called, 1, "called"); + ASSERT_EQ(skel->bss->arraymap_output, expected_total, "array_output"); + ASSERT_EQ(skel->bss->cpu + 1, skel->bss->percpu_val, "percpu_val"); + +out: + free(percpu_valbuf); + for_each_array_map_elem__destroy(skel); +} + void test_for_each(void) { if (test__start_subtest("hash_map")) test_hash_map(); + if (test__start_subtest("array_map")) + test_array_map(); } diff --git a/tools/testing/selftests/bpf/progs/for_each_array_map_elem.c b/tools/testing/selftests/bpf/progs/for_each_array_map_elem.c new file mode 100644 index 000000000000..f1f14dcd6e68 --- /dev/null +++ b/tools/testing/selftests/bpf/progs/for_each_array_map_elem.c @@ -0,0 +1,71 @@ +// SPDX-License-Identifier: GPL-2.0 +/* Copyright (c) 2021 Facebook */ +#include "bpf_iter.h" +#include <bpf/bpf_helpers.h> + +char _license[] SEC("license") = "GPL"; + +struct { + __uint(type, BPF_MAP_TYPE_ARRAY); + __uint(max_entries, 3); + __type(key, __u32); + __type(value, __u64); +} arraymap SEC(".maps"); + +struct { + __uint(type, BPF_MAP_TYPE_PERCPU_ARRAY); + __uint(max_entries, 1); + __type(key, __u32); + __type(value, __u64); +} percpu_map SEC(".maps"); + +struct callback_ctx { + int output; +}; + +static __u64 +check_array_elem(struct bpf_map *map, __u32 *key, __u64 *val, + struct callback_ctx *data) +{ + data->output += *val; + if (*key == 1) + return 1; /* stop the iteration */ + return 0; +} + +__u32 cpu = 0; +__u64 percpu_val = 0; + +static __u64 +check_percpu_elem(struct bpf_map *map, __u32 *key, __u64 *val, + struct callback_ctx *data) +{ + cpu = bpf_get_smp_processor_id(); + percpu_val = *val; + return 0; +} + +u32 called = 0; +u32 arraymap_output = 0; + +SEC("iter/task") +int dump_task(struct bpf_iter__task *ctx) +{ + struct seq_file *seq = ctx->meta->seq; + struct task_struct *task = ctx->task; + struct callback_ctx data; + + /* only call once */ + if (called > 0) + return 0; + + called++; + + data.output = 0; + bpf_for_each_map_elem(&arraymap, check_array_elem, &data, 0); + arraymap_output = data.output; + + bpf_for_each_map_elem(&percpu_map, check_percpu_elem, 0, 0); + + return 0; +}
A test is added for arraymap and percpu arraymap. The test also exercises the early return for the helper which does not traverse all elements. $ ./test_progs -n 44 #44/1 hash_map:OK #44/2 array_map:OK #44 for_each:OK Summary: 1/2 PASSED, 0 SKIPPED, 0 FAILED Signed-off-by: Yonghong Song <yhs@fb.com> --- .../selftests/bpf/prog_tests/for_each.c | 54 ++++++++++++++ .../bpf/progs/for_each_array_map_elem.c | 71 +++++++++++++++++++ 2 files changed, 125 insertions(+) create mode 100644 tools/testing/selftests/bpf/progs/for_each_array_map_elem.c