| Message ID | 168042420344.4051476.9107061652824513113.stgit@firesoul (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | BPF |
| Headers | show |
| Series | XDP-hints: API change for RX-hash kfunc bpf_xdp_metadata_rx_hash | expand |
On Sun, Apr 2, 2023 at 1:30 AM Jesper Dangaard Brouer <brouer@redhat.com> wrote: > > Update BPF selftests to use the new RSS type argument for kfunc > bpf_xdp_metadata_rx_hash. > > Signed-off-by: Jesper Dangaard Brouer <brouer@redhat.com> > Acked-by: Toke Høiland-Jørgensen <toke@redhat.com> > Acked-by: Stanislav Fomichev <sdf@google.com> > --- > .../selftests/bpf/prog_tests/xdp_metadata.c | 2 ++ > .../testing/selftests/bpf/progs/xdp_hw_metadata.c | 14 +++++++++----- > tools/testing/selftests/bpf/progs/xdp_metadata.c | 6 +++--- > tools/testing/selftests/bpf/progs/xdp_metadata2.c | 7 ++++--- > tools/testing/selftests/bpf/xdp_hw_metadata.c | 2 +- > tools/testing/selftests/bpf/xdp_metadata.h | 1 + > 6 files changed, 20 insertions(+), 12 deletions(-) > > diff --git a/tools/testing/selftests/bpf/prog_tests/xdp_metadata.c b/tools/testing/selftests/bpf/prog_tests/xdp_metadata.c > index aa4beae99f4f..8c5e98da9ae9 100644 > --- a/tools/testing/selftests/bpf/prog_tests/xdp_metadata.c > +++ b/tools/testing/selftests/bpf/prog_tests/xdp_metadata.c > @@ -273,6 +273,8 @@ static int verify_xsk_metadata(struct xsk *xsk) > if (!ASSERT_NEQ(meta->rx_hash, 0, "rx_hash")) > return -1; > > + ASSERT_EQ(meta->rx_hash_type, 0, "rx_hash_type"); > + > xsk_ring_cons__release(&xsk->rx, 1); > refill_rx(xsk, comp_addr); > > diff --git a/tools/testing/selftests/bpf/progs/xdp_hw_metadata.c b/tools/testing/selftests/bpf/progs/xdp_hw_metadata.c > index 4c55b4d79d3d..7b3fc12e96d6 100644 > --- a/tools/testing/selftests/bpf/progs/xdp_hw_metadata.c > +++ b/tools/testing/selftests/bpf/progs/xdp_hw_metadata.c > @@ -14,8 +14,8 @@ struct { > > 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, > - __u32 *hash) __ksym; > +extern int bpf_xdp_metadata_rx_hash(const struct xdp_md *ctx, __u32 *hash, > + enum xdp_rss_hash_type *rss_type) __ksym; > > SEC("xdp") > int rx(struct xdp_md *ctx) > @@ -74,10 +74,14 @@ 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 > + if (!bpf_xdp_metadata_rx_hash(ctx, &meta->rx_hash, &meta->rx_hash_type)) { > + bpf_printk("populated rx_hash:0x%X type:0x%X", > + meta->rx_hash, meta->rx_hash_type); > + if (!(meta->rx_hash_type & XDP_RSS_L4)) > + bpf_printk("rx_hash low quality L3 hash type"); > + } else { > meta->rx_hash = 0; /* Used by AF_XDP as not avail signal */ > + } Didn't we agree in the previous thread to remove these printks and replace them with actual stats that user space can see?
On Mon, Apr 3, 2023 at 8:08 AM Jesper Brouer <jbrouer@redhat.com> wrote: > > > > søn. 2. apr. 2023 17.50 skrev Alexei Starovoitov <alexei.starovoitov@gmail.com>: >> >> On Sun, Apr 2, 2023 at 1:30 AM Jesper Dangaard Brouer <brouer@redhat.com> wrote: >> > >> > Update BPF selftests to use the new RSS type argument for kfunc >> > bpf_xdp_metadata_rx_hash. >> > >> > Signed-off-by: Jesper Dangaard Brouer <brouer@redhat.com> >> > Acked-by: Toke Høiland-Jørgensen <toke@redhat.com> >> > Acked-by: Stanislav Fomichev <sdf@google.com> >> > --- >> > .../selftests/bpf/prog_tests/xdp_metadata.c | 2 ++ >> > .../testing/selftests/bpf/progs/xdp_hw_metadata.c | 14 +++++++++----- >> > tools/testing/selftests/bpf/progs/xdp_metadata.c | 6 +++--- >> > tools/testing/selftests/bpf/progs/xdp_metadata2.c | 7 ++++--- >> > tools/testing/selftests/bpf/xdp_hw_metadata.c | 2 +- >> > tools/testing/selftests/bpf/xdp_metadata.h | 1 + >> > 6 files changed, 20 insertions(+), 12 deletions(-) >> > >> > diff --git a/tools/testing/selftests/bpf/prog_tests/xdp_metadata.c b/tools/testing/selftests/bpf/prog_tests/xdp_metadata.c >> > index aa4beae99f4f..8c5e98da9ae9 100644 >> > --- a/tools/testing/selftests/bpf/prog_tests/xdp_metadata.c >> > +++ b/tools/testing/selftests/bpf/prog_tests/xdp_metadata.c >> > @@ -273,6 +273,8 @@ static int verify_xsk_metadata(struct xsk *xsk) >> > if (!ASSERT_NEQ(meta->rx_hash, 0, "rx_hash")) >> > return -1; >> > >> > + ASSERT_EQ(meta->rx_hash_type, 0, "rx_hash_type"); >> > + >> > xsk_ring_cons__release(&xsk->rx, 1); >> > refill_rx(xsk, comp_addr); >> > >> > diff --git a/tools/testing/selftests/bpf/progs/xdp_hw_metadata.c b/tools/testing/selftests/bpf/progs/xdp_hw_metadata.c >> > index 4c55b4d79d3d..7b3fc12e96d6 100644 >> > --- a/tools/testing/selftests/bpf/progs/xdp_hw_metadata.c >> > +++ b/tools/testing/selftests/bpf/progs/xdp_hw_metadata.c >> > @@ -14,8 +14,8 @@ struct { >> > >> > 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, >> > - __u32 *hash) __ksym; >> > +extern int bpf_xdp_metadata_rx_hash(const struct xdp_md *ctx, __u32 *hash, >> > + enum xdp_rss_hash_type *rss_type) __ksym; >> > >> > SEC("xdp") >> > int rx(struct xdp_md *ctx) >> > @@ -74,10 +74,14 @@ 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 >> > + if (!bpf_xdp_metadata_rx_hash(ctx, &meta->rx_hash, &meta->rx_hash_type)) { >> > + bpf_printk("populated rx_hash:0x%X type:0x%X", >> > + meta->rx_hash, meta->rx_hash_type); >> > + if (!(meta->rx_hash_type & XDP_RSS_L4)) >> > + bpf_printk("rx_hash low quality L3 hash type"); >> > + } else { >> > meta->rx_hash = 0; /* Used by AF_XDP as not avail signal */ >> > + } >> >> Didn't we agree in the previous thread to remove these printks and >> replace them with actual stats that user space can see? > > > This patchset is for bpf-tree RC version of kernel. > Thus, we keep changes to a minimum. > > I/we will do printk work on bpf-next. > (Once I get home from vacation next week) Sorry, but I insist on making them in this set. We did bigger changes in bpf tree, so size is not an issue. I don't want to remember to ping you every week or so to remind you to fix this later after bpf gets merged into bpf-next. Less work for me and less work for you to do it now.
diff --git a/tools/testing/selftests/bpf/prog_tests/xdp_metadata.c b/tools/testing/selftests/bpf/prog_tests/xdp_metadata.c index aa4beae99f4f..8c5e98da9ae9 100644 --- a/tools/testing/selftests/bpf/prog_tests/xdp_metadata.c +++ b/tools/testing/selftests/bpf/prog_tests/xdp_metadata.c @@ -273,6 +273,8 @@ static int verify_xsk_metadata(struct xsk *xsk) if (!ASSERT_NEQ(meta->rx_hash, 0, "rx_hash")) return -1; + ASSERT_EQ(meta->rx_hash_type, 0, "rx_hash_type"); + xsk_ring_cons__release(&xsk->rx, 1); refill_rx(xsk, comp_addr); diff --git a/tools/testing/selftests/bpf/progs/xdp_hw_metadata.c b/tools/testing/selftests/bpf/progs/xdp_hw_metadata.c index 4c55b4d79d3d..7b3fc12e96d6 100644 --- a/tools/testing/selftests/bpf/progs/xdp_hw_metadata.c +++ b/tools/testing/selftests/bpf/progs/xdp_hw_metadata.c @@ -14,8 +14,8 @@ struct { 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, - __u32 *hash) __ksym; +extern int bpf_xdp_metadata_rx_hash(const struct xdp_md *ctx, __u32 *hash, + enum xdp_rss_hash_type *rss_type) __ksym; SEC("xdp") int rx(struct xdp_md *ctx) @@ -74,10 +74,14 @@ 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 + if (!bpf_xdp_metadata_rx_hash(ctx, &meta->rx_hash, &meta->rx_hash_type)) { + bpf_printk("populated rx_hash:0x%X type:0x%X", + meta->rx_hash, meta->rx_hash_type); + if (!(meta->rx_hash_type & XDP_RSS_L4)) + bpf_printk("rx_hash low quality L3 hash type"); + } else { 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/progs/xdp_metadata.c b/tools/testing/selftests/bpf/progs/xdp_metadata.c index 77678b034389..d151d406a123 100644 --- a/tools/testing/selftests/bpf/progs/xdp_metadata.c +++ b/tools/testing/selftests/bpf/progs/xdp_metadata.c @@ -21,8 +21,8 @@ struct { 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, - __u32 *hash) __ksym; +extern int bpf_xdp_metadata_rx_hash(const struct xdp_md *ctx, __u32 *hash, + enum xdp_rss_hash_type *rss_type) __ksym; SEC("xdp") int rx(struct xdp_md *ctx) @@ -56,7 +56,7 @@ int rx(struct xdp_md *ctx) if (timestamp == 0) meta->rx_timestamp = 1; - bpf_xdp_metadata_rx_hash(ctx, &meta->rx_hash); + bpf_xdp_metadata_rx_hash(ctx, &meta->rx_hash, &meta->rx_hash_type); return bpf_redirect_map(&xsk, ctx->rx_queue_index, XDP_PASS); } diff --git a/tools/testing/selftests/bpf/progs/xdp_metadata2.c b/tools/testing/selftests/bpf/progs/xdp_metadata2.c index cf69d05451c3..85f88d9d7a78 100644 --- a/tools/testing/selftests/bpf/progs/xdp_metadata2.c +++ b/tools/testing/selftests/bpf/progs/xdp_metadata2.c @@ -5,17 +5,18 @@ #include <bpf/bpf_helpers.h> #include <bpf/bpf_endian.h> -extern int bpf_xdp_metadata_rx_hash(const struct xdp_md *ctx, - __u32 *hash) __ksym; +extern int bpf_xdp_metadata_rx_hash(const struct xdp_md *ctx, __u32 *hash, + enum xdp_rss_hash_type *rss_type) __ksym; int called; SEC("freplace/rx") int freplace_rx(struct xdp_md *ctx) { + enum xdp_rss_hash_type type = 0; u32 hash = 0; /* Call _any_ metadata function to make sure we don't crash. */ - bpf_xdp_metadata_rx_hash(ctx, &hash); + bpf_xdp_metadata_rx_hash(ctx, &hash, &type); called++; return XDP_PASS; } diff --git a/tools/testing/selftests/bpf/xdp_hw_metadata.c b/tools/testing/selftests/bpf/xdp_hw_metadata.c index 1c8acb68b977..4ca29e0c9646 100644 --- a/tools/testing/selftests/bpf/xdp_hw_metadata.c +++ b/tools/testing/selftests/bpf/xdp_hw_metadata.c @@ -141,7 +141,7 @@ static void verify_xdp_metadata(void *data) meta = data - sizeof(*meta); printf("rx_timestamp: %llu\n", meta->rx_timestamp); - printf("rx_hash: %u\n", meta->rx_hash); + printf("rx_hash: 0x%X RSS type:0x%X\n", meta->rx_hash, meta->rx_hash_type); } static void verify_skb_metadata(int fd) diff --git a/tools/testing/selftests/bpf/xdp_metadata.h b/tools/testing/selftests/bpf/xdp_metadata.h index f6780fbb0a21..899da872fee1 100644 --- a/tools/testing/selftests/bpf/xdp_metadata.h +++ b/tools/testing/selftests/bpf/xdp_metadata.h @@ -12,4 +12,5 @@ struct xdp_meta { __u64 rx_timestamp; __u32 rx_hash; + __u32 rx_hash_type; };