Re: [PATCH v3 net-next 02/15] selftests: xsk: size the TX UMEM in HW_SW_MAX_RING_SIZE

[netdev-bot+sashiko@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential
issue(s) to consider.

Critical: 0 · High: 0 · Medium: 1 · Low: 0

If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).

- [Medium] In testapp_hw_sw_max_ring_size()
  (tools/testing/selftests/bpf/prog_tests/test_xsk.c), the patch gives…

--

selftests: xsk: size the TX UMEM in HW_SW_MAX_RING_SIZE

In testapp_hw_sw_max_ring_size(), this patch gives the TX UMEM the same
8k frames, fill ring and completion ring as the RX UMEM. In two-interface
mode the TX UMEM has only 1k frames, so the 2k batch stalls in
__send_pkts(). The patch prepares for removing the single-interface mode
in the next patch.

> diff --git a/tools/testing/selftests/bpf/prog_tests/test_xsk.c b/tools/testing/selftests/bpf/prog_tests/test_xsk.c
> index 2a13628740924..16fe32e5afc06 100644
> --- a/tools/testing/selftests/bpf/prog_tests/test_xsk.c
> +++ b/tools/testing/selftests/bpf/prog_tests/test_xsk.c
> @@ -2513,6 +2513,9 @@ int testapp_hw_sw_max_ring_size(struct test_spec *test)
>  	test->ifobj_rx->xsk->umem->num_frames = max_descs;
>  	test->ifobj_rx->xsk->umem->fill_size = max_descs;
>  	test->ifobj_rx->xsk->umem->comp_size = max_descs;
> +	test->ifobj_tx->xsk->umem->num_frames = max_descs;
> +	test->ifobj_tx->xsk->umem->fill_size = max_descs;
> +	test->ifobj_tx->xsk->umem->comp_size = max_descs;

[Severity: Medium]
Does the second step still stall on NICs that report a tx_max_pending of
8200 or more?

max_descs is fixed at 4 * XSK_RING_PROD__DEFAULT_NUM_DESCS, which is 8192.
The step-2 batch later in testapp_hw_sw_max_ring_size() still comes from
the maximum the driver reports:

	test->ifobj_tx->xsk->batch_size = test->ifobj_tx->ring.tx_max_pending - 8;

Here is the in-flight limit check in __send_pkts():

	if (pkts_in_flight >= (int)((umem_size(umem) - xsk->batch_size * buffer_len) /
	    buffer_len) && !test_timeout) {

With batch_size == 8192 the limit is 0. With batch_size > 8192 the u64
subtraction wraps, and the truncated int is 8192 - batch_size, which is
negative. In both cases TX only kicks and returns TEST_CONTINUE.

Even if that check passed, xsk_ring_prod__reserve(&xsk->tx, batch_size,
...) can never reserve more than the 8192 slots of the TX ring. This is
because xsk_configure_socket() sets cfg.tx_size = umem->comp_size.

Some hardware that supports AF_XDP reports larger maximums:

- octeontx2: otx2_get_ringparam() reports tx_max_pending =
  Q_COUNT(Q_SIZE_MAX), which is 1048576. The batch is 1048568 and the
  limit is -1040376.
- hns3: reports HNS3_RING_MAX_PENDING, which is 32760. The batch is 32752
  and the limit is -24560.

ice (8160) and mlx5 (8192) fit within the limit.

On the larger NICs, TX never sends and RX times out. That seems to be the
same failure the commit message describes:

  "the in-flight limit in __send_pkts() turns negative for a 2k batch,
  so TX never sends and RX times out, and the second batch would not fit
  in the TX ring anyway."

At the end of the series, tools/testing/selftests/net/lib/xsk/test_xsk.c
still uses the fixed 8192 and xsk_get_cap(test, tx_max_pending) - 8. Once
the single-interface mode is gone, two-interface mode is the only one
left, so this case would keep failing on those NICs.

A 1M-frame UMEM would need 4 GiB. Could the step-2 batch be clamped to
the software ring instead, for example min(tx_max_pending, max_descs) - 8?

[ ... ]

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261014184539.774708-1-maciej.fijalkowski%40intel.com