On Wed, 26 Jun 2024 at 10:36, Vyavahare, Tushar
<tushar.vyavahare@xxxxxxxxx> wrote:
>
>
>
> > -----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.
You can keep it at 100 chars since that is used in most of the file.
That is allowed these days unless I mistake myself. Though the file
started out at 80 chars.
> > > + */
> > > + 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
> > >
>
