Re: [PATCH v3 4/4] selftests/bpf: Add selftests validating the user ringbuf

[David Vernet <void@manifault.com>]

On Wed, Aug 24, 2022 at 03:03:54PM -0700, Andrii Nakryiko wrote:

[...]

> > +                       CHECK(read <= 0, "snprintf_comm",
> > +                             "Failed to write index %d to comm\n", i);
> 
> please, no CHECK() use in new tests, we have ASSERT_xxx() covering all
> common cases

No problem, I'll make this change in v4. Unless I missed something,
CHECK() isn't documented as deprecated in test_progs.h. I'll take care
of adding that in a separate change.

> > +static long
> > +bad_access1(struct bpf_dynptr *dynptr, void *context)
> > +{
> > +       const struct sample *sample;
> > +
> > +       sample = bpf_dynptr_data(dynptr - 1, 0, sizeof(*sample));
> > +       bpf_printk("Was able to pass bad pointer %lx\n", (__u64)dynptr - 1);
> > +
> > +       return 0;
> > +}
> > +
> > +/* A callback that accesses a dynptr in a bpf_user_ringbuf_drain callback should
> > + * not be able to read before the pointer.
> > + */
> > +SEC("?raw_tp/sys_nanosleep")
> 
> there is no sys_nanosleep raw tracepoint, use SEC("?raw_tp") to
> specify type, that's enough

Got it, thanks for catching that. Will fix.

> > diff --git a/tools/testing/selftests/bpf/test_user_ringbuf.h b/tools/testing/selftests/bpf/test_user_ringbuf.h
> > new file mode 100644
> > index 000000000000..1643b4d59ba7
> > --- /dev/null
> > +++ b/tools/testing/selftests/bpf/test_user_ringbuf.h
> 
> nit: I'd probably put it under progs/test_user_ringbuf.h so it's
> closer to BPF source code. As it is right now, it's neither near
> user-space part of tests nor near BPF part.

Fair enough, will fix.

Thanks,
David