On Wed, 2022-08-24 at 15:30 -0700, Andrii Nakryiko wrote:
> On Fri, Aug 19, 2022 at 3:09 PM Kui-Feng Lee <kuifeng@xxxxxx> wrote:
> >
> > Test iterators of vma, files, and tasks of tasks.
> >
> > Ensure the API works appropriately to visit all tasks,
> > tasks in a process, or a particular task.
> >
> > Signed-off-by: Kui-Feng Lee <kuifeng@xxxxxx>
> > ---
> > .../selftests/bpf/prog_tests/bpf_iter.c | 284
> > +++++++++++++++++-
> > .../selftests/bpf/prog_tests/btf_dump.c | 2 +-
> > .../selftests/bpf/progs/bpf_iter_task.c | 9 +
> > .../selftests/bpf/progs/bpf_iter_task_file.c | 9 +-
> > .../selftests/bpf/progs/bpf_iter_task_vma.c | 6 +-
> > .../bpf/progs/bpf_iter_uprobe_offset.c | 35 +++
> > 6 files changed, 326 insertions(+), 19 deletions(-)
> > create mode 100644
> > tools/testing/selftests/bpf/progs/bpf_iter_uprobe_offset.c
>
[...]
> > +
> > void test_bpf_iter(void)
> > {
> > + if (!ASSERT_OK(pthread_mutex_init(&do_nothing_mutex, NULL),
> > "pthread_mutex_init"))
> > + return;
> > +
>
> ditto, too paranoid, IMO
Right, for test cases, we can ease checks.
I would like to have something like
INFALLIBLE(pthread_mutex_init(......)...);
It will crash at the first point.
>
> > if (test__start_subtest("btf_id_or_null"))
> > test_btf_id_or_null();
> > if (test__start_subtest("ipv6_route"))
>
> [...]
>
> > diff --git
> > a/tools/testing/selftests/bpf/progs/bpf_iter_uprobe_offset.c
> > b/tools/testing/selftests/bpf/progs/bpf_iter_uprobe_offset.c
> > new file mode 100644
> > index 000000000000..825ca86678bd
> > --- /dev/null
> > +++ b/tools/testing/selftests/bpf/progs/bpf_iter_uprobe_offset.c
> > @@ -0,0 +1,35 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/* Copyright (c) 2022 Meta Platforms, Inc. and affiliates. */
> > +#include "bpf_iter.h"
> > +#include <bpf/bpf_helpers.h>
> > +
> > +char _license[] SEC("license") = "GPL";
> > +
> > +__u32 unique_tgid_cnt = 0;
> > +uintptr_t address = 0;
> > +uintptr_t offset = 0;
> > +__u32 last_tgid = 0;
> > +__u32 pid = 0;
> > +
> > +SEC("iter/task_vma") int get_uprobe_offset(struct
> > bpf_iter__task_vma *ctx)
>
> please keep SEC() on separate line
>
> > +{
> > + struct vm_area_struct *vma = ctx->vma;
> > + struct seq_file *seq = ctx->meta->seq;
> > + struct task_struct *task = ctx->task;
> > +
> > + if (task == NULL || vma == NULL)
> > + return 0;
> > +
> > + if (last_tgid != task->tgid)
> > + unique_tgid_cnt++;
> > + last_tgid = task->tgid;
> > +
> > + if (task->tgid != pid)
> > + return 0;
> > +
> > + if (vma->vm_start <= address && vma->vm_end > address) {
> > + offset = address - vma->vm_start + (vma->vm_pgoff
> > << 12);
>
> it's best not to assume page_size is 4K, you can pass actual value
> through global variable from user-space (we've previously fixed a
> bunch of tests with fixed page_size assumption as they break some
> platforms, let's not regress that)
A getpagesize() helper would help :)
>
> > + BPF_SEQ_PRINTF(seq, "OK\n");
> > + }
> > + return 0;
> > +}
> > --
> > 2.30.2
> >
