| Message ID | 168098188134.96582.7870014252568928901.stgit@firesoul (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | XDP-hints: API change for RX-hash kfunc bpf_xdp_metadata_rx_hash | expand |
On Sat, Apr 08, 2023 at 09:24:41PM +0200, Jesper Dangaard Brouer wrote: > The tool xdp_hw_metadata can be used by driver developers > implementing XDP-hints kfuncs. The tool transfers the > XDP-hints via metadata information to an AF_XDP userspace > process. When everything works the bpf_printk calls are > unncesssary. Thus, disable bpf_printk by default, but > make it easy to reenable for driver developers to use > when debugging their driver implementation. > > This also converts bpf_printk "forwarding UDP:9091 to AF_XDP" > into a code comment. The bpf_printk's that are important > to the driver developers is when bpf_xdp_adjust_meta fails. > The likely mistake from driver developers is expected to > be that they didn't implement XDP metadata adjust support. > > Signed-off-by: Jesper Dangaard Brouer <brouer@redhat.com> > --- > .../testing/selftests/bpf/progs/xdp_hw_metadata.c | 16 ++++++++++++++-- > 1 file changed, 14 insertions(+), 2 deletions(-) > > diff --git a/tools/testing/selftests/bpf/progs/xdp_hw_metadata.c b/tools/testing/selftests/bpf/progs/xdp_hw_metadata.c > index 4c55b4d79d3d..980eb60d8e5b 100644 > --- a/tools/testing/selftests/bpf/progs/xdp_hw_metadata.c > +++ b/tools/testing/selftests/bpf/progs/xdp_hw_metadata.c > @@ -5,6 +5,19 @@ > #include <bpf/bpf_helpers.h> > #include <bpf/bpf_endian.h> > > +/* Per default below bpf_printk() calls are disabled. Can be > + * reenabled manually for convenience by XDP-hints driver developer, > + * when troublshooting the drivers kfuncs implementation details. Hi Jesper, a minor nit from my side: nit: s/troublshooting/troubleshooting/ ...
On Sat, Apr 8, 2023 at 12:24 PM Jesper Dangaard Brouer <brouer@redhat.com> wrote: > > The tool xdp_hw_metadata can be used by driver developers > implementing XDP-hints kfuncs. The tool transfers the > XDP-hints via metadata information to an AF_XDP userspace > process. When everything works the bpf_printk calls are > unncesssary. Thus, disable bpf_printk by default, but > make it easy to reenable for driver developers to use > when debugging their driver implementation. > > This also converts bpf_printk "forwarding UDP:9091 to AF_XDP" > into a code comment. The bpf_printk's that are important > to the driver developers is when bpf_xdp_adjust_meta fails. > The likely mistake from driver developers is expected to > be that they didn't implement XDP metadata adjust support. > > Signed-off-by: Jesper Dangaard Brouer <brouer@redhat.com> > --- > .../testing/selftests/bpf/progs/xdp_hw_metadata.c | 16 ++++++++++++++-- > 1 file changed, 14 insertions(+), 2 deletions(-) > > diff --git a/tools/testing/selftests/bpf/progs/xdp_hw_metadata.c b/tools/testing/selftests/bpf/progs/xdp_hw_metadata.c > index 4c55b4d79d3d..980eb60d8e5b 100644 > --- a/tools/testing/selftests/bpf/progs/xdp_hw_metadata.c > +++ b/tools/testing/selftests/bpf/progs/xdp_hw_metadata.c > @@ -5,6 +5,19 @@ > #include <bpf/bpf_helpers.h> > #include <bpf/bpf_endian.h> > > +/* Per default below bpf_printk() calls are disabled. Can be > + * reenabled manually for convenience by XDP-hints driver developer, > + * when troublshooting the drivers kfuncs implementation details. > + * > + * Remember BPF-prog bpf_printk info output can be access via: > + * /sys/kernel/debug/tracing/trace_pipe > + */ > +//#define DEBUG 1 > +#ifndef DEBUG > +#undef bpf_printk > +#define bpf_printk(fmt, ...) ({}) > +#endif Are you planning to eventually do somethike similar to what I've mentioned in [0]? If not, should I try to send a patch? 0: https://lore.kernel.org/netdev/CAKH8qBupRYEg+SPMTMb4h532GESG7P1QdaFJ-+zrbARVN9xrdA@mail.gmail.com/ > + > struct { > __uint(type, BPF_MAP_TYPE_XSKMAP); > __uint(max_entries, 256); > @@ -49,11 +62,10 @@ int rx(struct xdp_md *ctx) > if (!udp) > return XDP_PASS; > > + /* Forwarding UDP:9091 to AF_XDP */ > if (udp->dest != bpf_htons(9091)) > return XDP_PASS; > > - bpf_printk("forwarding UDP:9091 to AF_XDP"); > - > ret = bpf_xdp_adjust_meta(ctx, -(int)sizeof(struct xdp_meta)); > if (ret != 0) { > bpf_printk("bpf_xdp_adjust_meta returned %d", ret); > >
On 12/04/2023 00.42, Stanislav Fomichev wrote: > On Sat, Apr 8, 2023 at 12:24 PM Jesper Dangaard Brouer > <brouer@redhat.com> wrote: >> >> The tool xdp_hw_metadata can be used by driver developers >> implementing XDP-hints kfuncs. The tool transfers the >> XDP-hints via metadata information to an AF_XDP userspace >> process. When everything works the bpf_printk calls are >> unncesssary. Thus, disable bpf_printk by default, but >> make it easy to reenable for driver developers to use >> when debugging their driver implementation. >> >> This also converts bpf_printk "forwarding UDP:9091 to AF_XDP" >> into a code comment. The bpf_printk's that are important >> to the driver developers is when bpf_xdp_adjust_meta fails. >> The likely mistake from driver developers is expected to >> be that they didn't implement XDP metadata adjust support. >> >> Signed-off-by: Jesper Dangaard Brouer <brouer@redhat.com> >> --- >> .../testing/selftests/bpf/progs/xdp_hw_metadata.c | 16 ++++++++++++++-- >> 1 file changed, 14 insertions(+), 2 deletions(-) >> >> diff --git a/tools/testing/selftests/bpf/progs/xdp_hw_metadata.c b/tools/testing/selftests/bpf/progs/xdp_hw_metadata.c >> index 4c55b4d79d3d..980eb60d8e5b 100644 >> --- a/tools/testing/selftests/bpf/progs/xdp_hw_metadata.c >> +++ b/tools/testing/selftests/bpf/progs/xdp_hw_metadata.c >> @@ -5,6 +5,19 @@ >> #include <bpf/bpf_helpers.h> >> #include <bpf/bpf_endian.h> >> >> +/* Per default below bpf_printk() calls are disabled. Can be >> + * reenabled manually for convenience by XDP-hints driver developer, >> + * when troublshooting the drivers kfuncs implementation details. >> + * >> + * Remember BPF-prog bpf_printk info output can be access via: >> + * /sys/kernel/debug/tracing/trace_pipe >> + */ >> +//#define DEBUG 1 >> +#ifndef DEBUG >> +#undef bpf_printk >> +#define bpf_printk(fmt, ...) ({}) >> +#endif > > Are you planning to eventually do somethike similar to what I've > mentioned in [0]? If not, should I try to send a patch? See next patch: - [PATCH bpf V7 2/7] selftests/bpf: Add counters to xdp_hw_metadata where I add these counters :-) > > 0: https://lore.kernel.org/netdev/CAKH8qBupRYEg+SPMTMb4h532GESG7P1QdaFJ-+zrbARVN9xrdA@mail.gmail.com/ >
On 04/12, Jesper Dangaard Brouer wrote: > > On 12/04/2023 00.42, Stanislav Fomichev wrote: > > On Sat, Apr 8, 2023 at 12:24 PM Jesper Dangaard Brouer > > <brouer@redhat.com> wrote: > > > > > > The tool xdp_hw_metadata can be used by driver developers > > > implementing XDP-hints kfuncs. The tool transfers the > > > XDP-hints via metadata information to an AF_XDP userspace > > > process. When everything works the bpf_printk calls are > > > unncesssary. Thus, disable bpf_printk by default, but > > > make it easy to reenable for driver developers to use > > > when debugging their driver implementation. > > > > > > This also converts bpf_printk "forwarding UDP:9091 to AF_XDP" > > > into a code comment. The bpf_printk's that are important > > > to the driver developers is when bpf_xdp_adjust_meta fails. > > > The likely mistake from driver developers is expected to > > > be that they didn't implement XDP metadata adjust support. > > > > > > Signed-off-by: Jesper Dangaard Brouer <brouer@redhat.com> > > > --- > > > .../testing/selftests/bpf/progs/xdp_hw_metadata.c | 16 ++++++++++++++-- > > > 1 file changed, 14 insertions(+), 2 deletions(-) > > > > > > diff --git a/tools/testing/selftests/bpf/progs/xdp_hw_metadata.c b/tools/testing/selftests/bpf/progs/xdp_hw_metadata.c > > > index 4c55b4d79d3d..980eb60d8e5b 100644 > > > --- a/tools/testing/selftests/bpf/progs/xdp_hw_metadata.c > > > +++ b/tools/testing/selftests/bpf/progs/xdp_hw_metadata.c > > > @@ -5,6 +5,19 @@ > > > #include <bpf/bpf_helpers.h> > > > #include <bpf/bpf_endian.h> > > > > > > +/* Per default below bpf_printk() calls are disabled. Can be > > > + * reenabled manually for convenience by XDP-hints driver developer, > > > + * when troublshooting the drivers kfuncs implementation details. > > > + * > > > + * Remember BPF-prog bpf_printk info output can be access via: > > > + * /sys/kernel/debug/tracing/trace_pipe > > > + */ > > > +//#define DEBUG 1 > > > +#ifndef DEBUG > > > +#undef bpf_printk > > > +#define bpf_printk(fmt, ...) ({}) > > > +#endif > > > > Are you planning to eventually do somethike similar to what I've > > mentioned in [0]? If not, should I try to send a patch? > > See next patch: > - [PATCH bpf V7 2/7] selftests/bpf: Add counters to xdp_hw_metadata > > where I add these counters :-) Oh, nice, let me take a look. I was assuming v7 is mostly the same as v6.. > > > > 0: https://lore.kernel.org/netdev/CAKH8qBupRYEg+SPMTMb4h532GESG7P1QdaFJ-+zrbARVN9xrdA@mail.gmail.com/ > > >
On 12/04/2023 18.06, Stanislav Fomichev wrote: > On 04/12, Jesper Dangaard Brouer wrote: >> On 12/04/2023 00.42, Stanislav Fomichev wrote: >>> On Sat, Apr 8, 2023 at 12:24 PM Jesper Dangaard Brouer >>>> [...] >>> >>> Are you planning to eventually do somethike similar to what I've >>> mentioned in [0]? If not, should I try to send a patch? >> >> See next patch: >> - [PATCH bpf V7 2/7] selftests/bpf: Add counters to xdp_hw_metadata >> >> where I add these counters :-) > > Oh, nice, let me take a look. I was assuming v7 is mostly the same as > v6.. > Alexei explicitly asked for these changes to be included in V7. Notice, I've already send out a [V8] (addressing Simon's notes). Please take a look at V8 instead of V7. We are at RC6 and I hope we soon have something we can agree on. [V8] https://lore.kernel.org/all/168130333143.150247.11159481574477358816.stgit@firesoul/ [patchwork] https://patchwork.kernel.org/project/netdevbpf/list/?series=739144&state=%2A&archive=both --Jesper
diff --git a/tools/testing/selftests/bpf/progs/xdp_hw_metadata.c b/tools/testing/selftests/bpf/progs/xdp_hw_metadata.c index 4c55b4d79d3d..980eb60d8e5b 100644 --- a/tools/testing/selftests/bpf/progs/xdp_hw_metadata.c +++ b/tools/testing/selftests/bpf/progs/xdp_hw_metadata.c @@ -5,6 +5,19 @@ #include <bpf/bpf_helpers.h> #include <bpf/bpf_endian.h> +/* Per default below bpf_printk() calls are disabled. Can be + * reenabled manually for convenience by XDP-hints driver developer, + * when troublshooting the drivers kfuncs implementation details. + * + * Remember BPF-prog bpf_printk info output can be access via: + * /sys/kernel/debug/tracing/trace_pipe + */ +//#define DEBUG 1 +#ifndef DEBUG +#undef bpf_printk +#define bpf_printk(fmt, ...) ({}) +#endif + struct { __uint(type, BPF_MAP_TYPE_XSKMAP); __uint(max_entries, 256); @@ -49,11 +62,10 @@ int rx(struct xdp_md *ctx) if (!udp) return XDP_PASS; + /* Forwarding UDP:9091 to AF_XDP */ if (udp->dest != bpf_htons(9091)) return XDP_PASS; - bpf_printk("forwarding UDP:9091 to AF_XDP"); - ret = bpf_xdp_adjust_meta(ctx, -(int)sizeof(struct xdp_meta)); if (ret != 0) { bpf_printk("bpf_xdp_adjust_meta returned %d", ret);
The tool xdp_hw_metadata can be used by driver developers implementing XDP-hints kfuncs. The tool transfers the XDP-hints via metadata information to an AF_XDP userspace process. When everything works the bpf_printk calls are unncesssary. Thus, disable bpf_printk by default, but make it easy to reenable for driver developers to use when debugging their driver implementation. This also converts bpf_printk "forwarding UDP:9091 to AF_XDP" into a code comment. The bpf_printk's that are important to the driver developers is when bpf_xdp_adjust_meta fails. The likely mistake from driver developers is expected to be that they didn't implement XDP metadata adjust support. Signed-off-by: Jesper Dangaard Brouer <brouer@redhat.com> --- .../testing/selftests/bpf/progs/xdp_hw_metadata.c | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-)