On Mon, Feb 8, 2021 at 10:50 PM Yonghong Song <yhs@xxxxxx> wrote:
>
>
>
> On 2/8/21 10:35 AM, Andrii Nakryiko wrote:
> > On Thu, Feb 4, 2021 at 5:53 PM Yonghong Song <yhs@xxxxxx> 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@xxxxxx>
> >> ---
> >> .../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
> >>
