| Message ID | 167906361094.2706833.8381428662566265476.stgit@firesoul (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | BPF |
| Headers | show |
| Series | XDP-hints kfuncs for Intel driver igc | expand |
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/progs/xdp_hw_metadata.c > b/tools/testing/selftests/bpf/progs/xdp_hw_metadata.c > index f2a3b70a9882..f2278ca2ad03 100644 > --- a/tools/testing/selftests/bpf/progs/xdp_hw_metadata.c > +++ b/tools/testing/selftests/bpf/progs/xdp_hw_metadata.c > @@ -76,10 +76,13 @@ int rx(struct xdp_md *ctx) > } else > meta->rx_timestamp = 0; /* Used by AF_XDP as not avail signal */ > - if (!bpf_xdp_metadata_rx_hash(ctx, &meta->rx_hash)) > - bpf_printk("populated rx_hash with %u", meta->rx_hash); > - else > + ret = bpf_xdp_metadata_rx_hash(ctx, &meta->rx_hash); > + if (ret >= 0) { > + bpf_printk("populated rx_hash with 0x%08X", meta->rx_hash); > + } else { > + bpf_printk("rx_hash not-avail errno:%d", ret); > meta->rx_hash = 0; /* Used by AF_XDP as not avail signal */ > + } > return bpf_redirect_map(&xsk, ctx->rx_queue_index, XDP_PASS); > } > 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? 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 :-) > + * > * RX: > * - UDP 9091 packets are diverted into AF_XDP > * - Metadata verified: > @@ -156,7 +159,7 @@ static void verify_xdp_metadata(void *data, clockid_t > clock_id) > meta = data - sizeof(*meta); > - printf("rx_hash: %u\n", meta->rx_hash); > + printf("rx_hash: 0x%08X\n", meta->rx_hash); > printf("rx_timestamp: %llu (sec:%0.4f)\n", meta->rx_timestamp, > (double)meta->rx_timestamp / NANOSEC_PER_SEC); > if (meta->rx_timestamp) {
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. > 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? --Jesper
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 >
On 21/03/2023 19.45, Stanislav Fomichev wrote: > 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/. Sorry, missed this... will fix in V3 > >>> 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. The output still work for me then I have zero in /sys/kernel/debug/tracing/events/bpf_trace/bpf_trace_printk/enable > That's why having something like enable_tracing() and dump_trace() > here might be helpful for whoever is running the prog. >
diff --git a/tools/testing/selftests/bpf/progs/xdp_hw_metadata.c b/tools/testing/selftests/bpf/progs/xdp_hw_metadata.c index f2a3b70a9882..f2278ca2ad03 100644 --- a/tools/testing/selftests/bpf/progs/xdp_hw_metadata.c +++ b/tools/testing/selftests/bpf/progs/xdp_hw_metadata.c @@ -76,10 +76,13 @@ int rx(struct xdp_md *ctx) } else meta->rx_timestamp = 0; /* Used by AF_XDP as not avail signal */ - if (!bpf_xdp_metadata_rx_hash(ctx, &meta->rx_hash)) - bpf_printk("populated rx_hash with %u", meta->rx_hash); - else + ret = bpf_xdp_metadata_rx_hash(ctx, &meta->rx_hash); + if (ret >= 0) { + bpf_printk("populated rx_hash with 0x%08X", meta->rx_hash); + } else { + bpf_printk("rx_hash not-avail errno:%d", ret); meta->rx_hash = 0; /* Used by AF_XDP as not avail signal */ + } return bpf_redirect_map(&xsk, ctx->rx_queue_index, XDP_PASS); } 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 + * * RX: * - UDP 9091 packets are diverted into AF_XDP * - Metadata verified: @@ -156,7 +159,7 @@ static void verify_xdp_metadata(void *data, clockid_t clock_id) meta = data - sizeof(*meta); - printf("rx_hash: %u\n", meta->rx_hash); + printf("rx_hash: 0x%08X\n", meta->rx_hash); printf("rx_timestamp: %llu (sec:%0.4f)\n", meta->rx_timestamp, (double)meta->rx_timestamp / NANOSEC_PER_SEC); if (meta->rx_timestamp) {
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> --- .../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(-)