Re: [PATCH bpf-next v2] selftests/bpf: test for bpf_get_file_path() from raw tracepoint

[Andrii Nakryiko <andrii.nakryiko@gmail.com>]

On Fri, Nov 1, 2019 at 6:08 AM Wenbo Zhang <ethercflow@gmail.com> wrote:
>
> trace fstat events by raw tracepoint sys_enter:newfstat, and handle events
> only produced by fd2path_loadgen, the fd2path_loadgen call fstat on several
> different types of files to test bpf_get_file_path's feature.
> ---

Unless there is a real reason for all this complexity (in which case,
please spell it out in commit or comments), I think this could be so
much simpler.

- you don't have to use perf_buffer to pass data back, just use global data;
- you can add a filter for PID to only capture data triggered by test
process and avoid the noise;
- why all those set_affinity dances? Is it just because you used
existing perf_buffer test which did that to specifically test
perf_buffer delivering data across every CPU core? Seems like your
test doesn't care about that...
- do we really need a separate binary generating hundreds of syscalls?
It's hard to synchronize with test and it seems much simpler to just
trigger necessary syscalls synchronously from the test itself, no?

I have a bunch of more minutia nits, but most of them will go away if
you simplify your testing approach anyway, so I'll postpone them till
then.

>  tools/testing/selftests/bpf/Makefile          |   8 +-
>  tools/testing/selftests/bpf/fd2path_loadgen.c |  75 ++++++++++
>  .../selftests/bpf/prog_tests/get_file_path.c  | 130 ++++++++++++++++++
>  .../selftests/bpf/progs/test_get_file_path.c  |  58 ++++++++
>  4 files changed, 269 insertions(+), 2 deletions(-)
>  create mode 100644 tools/testing/selftests/bpf/fd2path_loadgen.c
>  create mode 100644 tools/testing/selftests/bpf/prog_tests/get_file_path.c
>  create mode 100644 tools/testing/selftests/bpf/progs/test_get_file_path.c
>

[...]