> -----Original Message-----
> From: Fijalkowski, Maciej <maciej.fijalkowski@xxxxxxxxx>
> Sent: Tuesday, June 25, 2024 11:11 PM
> To: Vyavahare, Tushar <tushar.vyavahare@xxxxxxxxx>
> Cc: bpf@xxxxxxxxxxxxxxx; netdev@xxxxxxxxxxxxxxx; bjorn@xxxxxxxxxx;
> Karlsson, Magnus <magnus.karlsson@xxxxxxxxx>;
> jonathan.lemon@xxxxxxxxx; davem@xxxxxxxxxxxxx; kuba@xxxxxxxxxx;
> pabeni@xxxxxxxxxx; ast@xxxxxxxxxx; daniel@xxxxxxxxxxxxx; Sarkar,
> Tirthendu <tirthendu.sarkar@xxxxxxxxx>
> Subject: Re: [PATCH bpf-next 2/2] selftests/xsk: Enhance batch size support
> with dynamic configurations
>
> On Wed, Jun 19, 2024 at 01:20:48PM +0000, Tushar Vyavahare wrote:
> > Introduce dynamic adjustment capabilities for fill_size, comp_size,
> > tx_size, and rx_size parameters to support larger batch sizes beyond
> > the
>
> you are only introducing fill_size and comp_size to xsk_umem_info. The latter
> two seem to be in place.
>
I will do it.
> > previous 2K limit.
> >
> > Update HW_SW_MAX_RING_SIZE test cases to evaluate AF_XDP's
> robustness
> > by pushing hardware and software ring sizes to their limits. This test
> > ensures AF_XDP's reliability amidst potential producer/consumer
> > throttling due to maximum ring utilization.
> >
> > Signed-off-by: Tushar Vyavahare <tushar.vyavahare@xxxxxxxxx>
> > ---
> > tools/testing/selftests/bpf/xskxceiver.c | 26
> > ++++++++++++++++++------ tools/testing/selftests/bpf/xskxceiver.h |
> > 2 ++
> > 2 files changed, 22 insertions(+), 6 deletions(-)
> >
> > diff --git a/tools/testing/selftests/bpf/xskxceiver.c
> > b/tools/testing/selftests/bpf/xskxceiver.c
> > index 088df53869e8..5b049f0296e6 100644
> > --- a/tools/testing/selftests/bpf/xskxceiver.c
> > +++ b/tools/testing/selftests/bpf/xskxceiver.c
> > @@ -196,6 +196,12 @@ static int xsk_configure_umem(struct ifobject
> *ifobj, struct xsk_umem_info *umem
> > };
> > int ret;
> >
> > + if (umem->fill_size)
> > + cfg.fill_size = umem->fill_size;
> > +
> > + if (umem->comp_size)
> > + cfg.comp_size = umem->comp_size;
> > +
> > if (umem->unaligned_mode)
> > cfg.flags |= XDP_UMEM_UNALIGNED_CHUNK_FLAG;
> >
> > @@ -265,6 +271,10 @@ static int __xsk_configure_socket(struct
> xsk_socket_info *xsk, struct xsk_umem_i
> > cfg.bind_flags |= XDP_SHARED_UMEM;
> > if (ifobject->mtu > MAX_ETH_PKT_SIZE)
> > cfg.bind_flags |= XDP_USE_SG;
> > + if (umem->fill_size)
> > + cfg.tx_size = umem->fill_size;
> > + if (umem->comp_size)
> > + cfg.rx_size = umem->comp_size;
>
> how is the fq related to txq ? and cq to rxq? shouldn't this be fq-rxq and cq-
> txq. What is the intent here? In the end they are the same in your test.
>
Yes, you are correct, updating code accordingly.
> >
> > txr = ifobject->tx_on ? &xsk->tx : NULL;
> > rxr = ifobject->rx_on ? &xsk->rx : NULL; @@ -1616,7 +1626,7 @@
> > static void xsk_populate_fill_ring(struct xsk_umem_info *umem, struct
> pkt_stream
> > if (umem->num_frames < XSK_RING_PROD__DEFAULT_NUM_DESCS)
> > buffers_to_fill = umem->num_frames;
> > else
> > - buffers_to_fill = XSK_RING_PROD__DEFAULT_NUM_DESCS;
> > + buffers_to_fill = umem->fill_size;
> >
> > ret = xsk_ring_prod__reserve(&umem->fq, buffers_to_fill, &idx);
> > if (ret != buffers_to_fill)
> > @@ -2445,7 +2455,7 @@ static int testapp_hw_sw_min_ring_size(struct
> > test_spec *test)
> >
> > static int testapp_hw_sw_max_ring_size(struct test_spec *test) {
> > - u32 max_descs = XSK_RING_PROD__DEFAULT_NUM_DESCS * 2;
> > + u32 max_descs = XSK_RING_PROD__DEFAULT_NUM_DESCS * 4;
> > int ret;
> >
> > test->set_ring = true;
> > @@ -2453,7 +2463,8 @@ static int testapp_hw_sw_max_ring_size(struct
> test_spec *test)
> > test->ifobj_tx->ring.tx_pending = test->ifobj_tx-
> >ring.tx_max_pending;
> > test->ifobj_tx->ring.rx_pending = test->ifobj_tx-
> >ring.rx_max_pending;
> > test->ifobj_rx->umem->num_frames = max_descs;
> > - test->ifobj_rx->xsk->rxqsize = max_descs;
>
> rxqsize is only used for setting xsk_socket_config::rx_size ?
>
Initially, we used the rxqsize field from the xsk_socket object, directly
assigning max_descs to it and then using this value to set cfg.rx_size.
However, we are now shifted to a different approach for test, where we are
setting cfg.rx_size based on the comp_size from the umem object, provided
that umem->fill_size is true.
> > + test->ifobj_rx->umem->fill_size = max_descs;
> > + test->ifobj_rx->umem->comp_size = max_descs;
> > test->ifobj_tx->xsk->batch_size =
> XSK_RING_PROD__DEFAULT_NUM_DESCS;
> > test->ifobj_rx->xsk->batch_size =
> XSK_RING_PROD__DEFAULT_NUM_DESCS;
> >
> > @@ -2461,9 +2472,12 @@ static int testapp_hw_sw_max_ring_size(struct
> test_spec *test)
> > if (ret)
> > return ret;
> >
> > - /* Set batch_size to 4095 */
> > - test->ifobj_tx->xsk->batch_size = max_descs - 1;
> > - test->ifobj_rx->xsk->batch_size = max_descs - 1;
> > + /* Set batch_size to 8152 for testing, as the ice HW ignores the 3
> lowest bits when updating
> > + * the Rx HW tail register.
>
> i would wrap the comment to 80 chars but that's personal taste.
>
I will do it.
> > + */
> > + test->ifobj_tx->xsk->batch_size = test->ifobj_tx->ring.tx_max_pending
> - 8;
> > + test->ifobj_rx->xsk->batch_size = test->ifobj_tx->ring.tx_max_pending
> - 8;
> > + pkt_stream_replace(test, max_descs, MIN_PKT_SIZE);
> > return testapp_validate_traffic(test); }
> >
> > diff --git a/tools/testing/selftests/bpf/xskxceiver.h
> > b/tools/testing/selftests/bpf/xskxceiver.h
> > index 906de5fab7a3..885c948c5d83 100644
> > --- a/tools/testing/selftests/bpf/xskxceiver.h
> > +++ b/tools/testing/selftests/bpf/xskxceiver.h
> > @@ -80,6 +80,8 @@ struct xsk_umem_info {
> > void *buffer;
> > u32 frame_size;
> > u32 base_addr;
> > + u32 fill_size;
> > + u32 comp_size;
> > bool unaligned_mode;
> > };
> >
> > --
> > 2.34.1
> >
