| Message ID | 168132451707.317773.15960209122204110352.stgit@firesoul |
|---|---|
| State | Handled Elsewhere |
| Headers | show |
| Series | XDP-hints: API change for RX-hash kfunc bpf_xdp_metadata_rx_hash | expand |
On Wed, Apr 12, 2023 at 11:35 AM Jesper Dangaard Brouer <brouer@redhat.com> wrote: > > The tool xdp_hw_metadata can be used by driver developers > implementing XDP-hints metadata kfuncs. > > Remove all bpf_printk calls, as the tool already transfers all the > XDP-hints related information via metadata area to AF_XDP > userspace process. > > Add counters for providing remaining information about failure and > skipped packet events. > > Signed-off-by: Jesper Dangaard Brouer <brouer@redhat.com> Acked-by: Stanislav Fomichev <sdf@google.com> nit: maybe those ++ should be __sync_add_and_fetch instead? Then you should be able to drop volatile.. > --- > .../testing/selftests/bpf/progs/xdp_hw_metadata.c | 36 ++++++++++++-------- > tools/testing/selftests/bpf/xdp_hw_metadata.c | 4 ++ > 2 files changed, 24 insertions(+), 16 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..8a042343cb0c 100644 > --- a/tools/testing/selftests/bpf/progs/xdp_hw_metadata.c > +++ b/tools/testing/selftests/bpf/progs/xdp_hw_metadata.c > @@ -12,6 +12,10 @@ struct { > __type(value, __u32); > } xsk SEC(".maps"); > > +volatile __u64 pkts_skip = 0; > +volatile __u64 pkts_fail = 0; > +volatile __u64 pkts_redir = 0; > + > extern int bpf_xdp_metadata_rx_timestamp(const struct xdp_md *ctx, > __u64 *timestamp) __ksym; > extern int bpf_xdp_metadata_rx_hash(const struct xdp_md *ctx, > @@ -26,7 +30,7 @@ int rx(struct xdp_md *ctx) > struct udphdr *udp = NULL; > struct iphdr *iph = NULL; > struct xdp_meta *meta; > - int ret; > + int err; > > data = (void *)(long)ctx->data; > data_end = (void *)(long)ctx->data_end; > @@ -46,17 +50,20 @@ int rx(struct xdp_md *ctx) > udp = NULL; > } > > - if (!udp) > + if (!udp) { > + pkts_skip++; > return XDP_PASS; > + } > > - if (udp->dest != bpf_htons(9091)) > + /* Forwarding UDP:9091 to AF_XDP */ > + if (udp->dest != bpf_htons(9091)) { > + pkts_skip++; > 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); > + err = bpf_xdp_adjust_meta(ctx, -(int)sizeof(struct xdp_meta)); > + if (err) { > + pkts_fail++; > return XDP_PASS; > } > > @@ -65,20 +72,19 @@ int rx(struct xdp_md *ctx) > meta = data_meta; > > if (meta + 1 > data) { > - bpf_printk("bpf_xdp_adjust_meta doesn't appear to work"); > + pkts_fail++; > return XDP_PASS; > } > > - if (!bpf_xdp_metadata_rx_timestamp(ctx, &meta->rx_timestamp)) > - bpf_printk("populated rx_timestamp with %llu", meta->rx_timestamp); > - else > + err = bpf_xdp_metadata_rx_timestamp(ctx, &meta->rx_timestamp); > + if (err) > 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 > + err = bpf_xdp_metadata_rx_hash(ctx, &meta->rx_hash); > + if (err) > meta->rx_hash = 0; /* Used by AF_XDP as not avail signal */ > > + pkts_redir++; > 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 1c8acb68b977..3b942ef7297b 100644 > --- a/tools/testing/selftests/bpf/xdp_hw_metadata.c > +++ b/tools/testing/selftests/bpf/xdp_hw_metadata.c > @@ -212,7 +212,9 @@ static int verify_metadata(struct xsk *rx_xsk, int rxq, int server_fd) > while (true) { > errno = 0; > ret = poll(fds, rxq + 1, 1000); > - printf("poll: %d (%d)\n", ret, errno); > + printf("poll: %d (%d) skip=%llu fail=%llu redir=%llu\n", > + ret, errno, bpf_obj->bss->pkts_skip, > + bpf_obj->bss->pkts_fail, bpf_obj->bss->pkts_redir); > if (ret < 0) > break; > if (ret == 0) > >
On 12/04/2023 20.56, Stanislav Fomichev wrote: > On Wed, Apr 12, 2023 at 11:35 AM Jesper Dangaard Brouer > <brouer@redhat.com> wrote: >> The tool xdp_hw_metadata can be used by driver developers >> implementing XDP-hints metadata kfuncs. >> >> Remove all bpf_printk calls, as the tool already transfers all the >> XDP-hints related information via metadata area to AF_XDP >> userspace process. >> >> Add counters for providing remaining information about failure and >> skipped packet events. >> >> Signed-off-by: Jesper Dangaard Brouer<brouer@redhat.com> > Acked-by: Stanislav Fomichev<sdf@google.com> > > nit: maybe those ++ should be __sync_add_and_fetch instead? Then you > should be able to drop volatile.. That is a good suggestion. I will update the patch and add your ACK. --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..8a042343cb0c 100644 --- a/tools/testing/selftests/bpf/progs/xdp_hw_metadata.c +++ b/tools/testing/selftests/bpf/progs/xdp_hw_metadata.c @@ -12,6 +12,10 @@ struct { __type(value, __u32); } xsk SEC(".maps"); +volatile __u64 pkts_skip = 0; +volatile __u64 pkts_fail = 0; +volatile __u64 pkts_redir = 0; + extern int bpf_xdp_metadata_rx_timestamp(const struct xdp_md *ctx, __u64 *timestamp) __ksym; extern int bpf_xdp_metadata_rx_hash(const struct xdp_md *ctx, @@ -26,7 +30,7 @@ int rx(struct xdp_md *ctx) struct udphdr *udp = NULL; struct iphdr *iph = NULL; struct xdp_meta *meta; - int ret; + int err; data = (void *)(long)ctx->data; data_end = (void *)(long)ctx->data_end; @@ -46,17 +50,20 @@ int rx(struct xdp_md *ctx) udp = NULL; } - if (!udp) + if (!udp) { + pkts_skip++; return XDP_PASS; + } - if (udp->dest != bpf_htons(9091)) + /* Forwarding UDP:9091 to AF_XDP */ + if (udp->dest != bpf_htons(9091)) { + pkts_skip++; 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); + err = bpf_xdp_adjust_meta(ctx, -(int)sizeof(struct xdp_meta)); + if (err) { + pkts_fail++; return XDP_PASS; } @@ -65,20 +72,19 @@ int rx(struct xdp_md *ctx) meta = data_meta; if (meta + 1 > data) { - bpf_printk("bpf_xdp_adjust_meta doesn't appear to work"); + pkts_fail++; return XDP_PASS; } - if (!bpf_xdp_metadata_rx_timestamp(ctx, &meta->rx_timestamp)) - bpf_printk("populated rx_timestamp with %llu", meta->rx_timestamp); - else + err = bpf_xdp_metadata_rx_timestamp(ctx, &meta->rx_timestamp); + if (err) 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 + err = bpf_xdp_metadata_rx_hash(ctx, &meta->rx_hash); + if (err) meta->rx_hash = 0; /* Used by AF_XDP as not avail signal */ + pkts_redir++; 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 1c8acb68b977..3b942ef7297b 100644 --- a/tools/testing/selftests/bpf/xdp_hw_metadata.c +++ b/tools/testing/selftests/bpf/xdp_hw_metadata.c @@ -212,7 +212,9 @@ static int verify_metadata(struct xsk *rx_xsk, int rxq, int server_fd) while (true) { errno = 0; ret = poll(fds, rxq + 1, 1000); - printf("poll: %d (%d)\n", ret, errno); + printf("poll: %d (%d) skip=%llu fail=%llu redir=%llu\n", + ret, errno, bpf_obj->bss->pkts_skip, + bpf_obj->bss->pkts_fail, bpf_obj->bss->pkts_redir); if (ret < 0) break; if (ret == 0)
The tool xdp_hw_metadata can be used by driver developers implementing XDP-hints metadata kfuncs. Remove all bpf_printk calls, as the tool already transfers all the XDP-hints related information via metadata area to AF_XDP userspace process. Add counters for providing remaining information about failure and skipped packet events. Signed-off-by: Jesper Dangaard Brouer <brouer@redhat.com> --- .../testing/selftests/bpf/progs/xdp_hw_metadata.c | 36 ++++++++++++-------- tools/testing/selftests/bpf/xdp_hw_metadata.c | 4 ++ 2 files changed, 24 insertions(+), 16 deletions(-)