[xdp-hints] Re: [PATCH bpf-next V1 4/7] selftests/bpf: xdp_hw_metadata RX hash return code info

[Stanislav Fomichev <sdf@google.com>]

On Tue, Mar 21, 2023 at 6:32 AM Jesper Dangaard Brouer
<jbrouer@redhat.com> wrote:
>
>
>
> On 17/03/2023 22.13, Stanislav Fomichev wrote:
> > On 03/17, Jesper Dangaard Brouer wrote:
> >> When driver developers add XDP-hints kfuncs for RX hash it is
> >> practical to print the return code in bpf_printk trace pipe log.
> >
> >> Print hash value as a hex value, both AF_XDP userspace and bpf_prog,
> >> as this makes it easier to spot poor quality hashes.
> >
> >> Signed-off-by: Jesper Dangaard Brouer <brouer@redhat.com>
> >
> > Acked-by: Stanislav Fomichev <sdf@google.com>
> >
> > (with a small suggestion below, maybe can do separately?)
> >
> >> ---
> >>   .../testing/selftests/bpf/progs/xdp_hw_metadata.c  |    9 ++++++---
> >>   tools/testing/selftests/bpf/xdp_hw_metadata.c      |    5 ++++-
> >>   2 files changed, 10 insertions(+), 4 deletions(-)
> [...]
> >> diff --git a/tools/testing/selftests/bpf/xdp_hw_metadata.c
> >> b/tools/testing/selftests/bpf/xdp_hw_metadata.c
> >> index 400bfe19abfe..f3ec07ccdc95 100644
> >> --- a/tools/testing/selftests/bpf/xdp_hw_metadata.c
> >> +++ b/tools/testing/selftests/bpf/xdp_hw_metadata.c
> >> @@ -3,6 +3,9 @@
> >>   /* Reference program for verifying XDP metadata on real HW.
> >> Functional test
> >>    * only, doesn't test the performance.
> >>    *
> >
> > [..]
> >
> >> + * BPF-prog bpf_printk info outout can be access via
> >> + * /sys/kernel/debug/tracing/trace_pipe
> >
> > Maybe we should just dump the contents of
> > /sys/kernel/debug/tracing/trace for every poll cycle?
> >
>
> I think this belongs to a separate patch.

SG. If you prefer to keep the comment let's also s/outout/outPut/.

> > We can also maybe enable tracing in this program transparently?
> > I usually forget 'echo 1 >
> > /sys/kernel/debug/tracing/events/bpf_trace/bpf_trace_printk/enable'
> > myself :-)
> >
> What is this trick?

On the recent kernels I think this event has to be explicitly enabled
for bpf_prink() to work? Not sure.
That's why having something like enable_tracing() and dump_trace()
here might be helpful for whoever is running the prog.

> --Jesper
>