Re: [PATCH v4 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 more severe
issue(s) in other patches of this series. This mail is only an FYI, in case
you want to address the nit picks below when you respin.

Low priority / nit picks for your consideration:

- [Low] This patch fixes a hang that exists in the tree today, but it has
  no Fixes: tag.

--

selftests: xsk: size the TX UMEM in HW_SW_MAX_RING_SIZE

This commit gives the TX UMEM in testapp_hw_sw_max_ring_size() the same
size as the RX UMEM (8k frames and 8k rings). That way the test still
works with two interfaces once the single-interface mode is removed. It
also caps the second batch below the 8k XSK TX ring, so that
__send_pkts() cannot spin forever on NICs with large HW TX rings.

> diff --git a/tools/testing/selftests/bpf/prog_tests/test_xsk.c b/tools/testing/selftests/bpf/prog_tests/test_xsk.c
> index 2a13628740924..05457f77ca52c 100644
> --- a/tools/testing/selftests/bpf/prog_tests/test_xsk.c
> +++ b/tools/testing/selftests/bpf/prog_tests/test_xsk.c

[ ... ]

> @@ -2521,10 +2525,12 @@ int testapp_hw_sw_max_ring_size(struct test_spec *test)
>  		return ret;
>  
>  	/* Set batch_size to 8152 for testing, as the ice HW ignores the 3 lowest bits when
> -	 * updating the Rx HW tail register.
> +	 * updating the Rx HW tail register. __send_pkts() waits until a whole batch fits
> +	 * in the XSK TX ring, so keep it below that ring's size on NICs with larger rings.
>  	 */
> -	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;
> +	batch_size = min(test->ifobj_tx->ring.tx_max_pending, max_descs) - 8;
> +	test->ifobj_tx->xsk->batch_size = batch_size;
> +	test->ifobj_rx->xsk->batch_size = batch_size;

[Severity: Low]
Should this patch carry a Fixes: tag?

The commit message describes the change as preparation for removing the
single-interface mode. The min() cap, though, fixes a hang that can
already happen in the current tree.

In the single-interface (shared UMEM) mode, thread_common_ops_tx()
copies the RX UMEM. xsk_configure_socket() then creates the XSK TX ring
with tx_size = umem->comp_size = 8192.

Before this patch the second batch was tx_max_pending - 8. On a NIC that
reports tx_max_pending above 8200, the reserve in __send_pkts() can
never succeed:

tools/testing/selftests/bpf/prog_tests/test_xsk.c:__send_pkts() {
    ...
	while (xsk_ring_prod__reserve(&xsk->tx, xsk->batch_size, &idx) < xsk->batch_size) {
    ...
}

When use_poll is false this loop has no timeout, so the test spins
forever.

This looks reachable on real hardware. nfp supports AF_XDP zero-copy and
reports tx_max_pending as max_qc_size / tx_min_desc_per_pkt, with
max_qc_size being SZ_64K or SZ_256K.

The two-interface case is also broken before this patch. With a
1024-frame TX UMEM and a 2048 batch, the in-flight limit in
__send_pkts() goes negative and TX never sends.

Would a Fixes: tag pointing at the commit that added HW_SW_MAX_RING_SIZE
help stable maintainers and backporters pick this up?

>  	if (pkt_stream_replace(test, max_descs, MIN_PKT_SIZE)) {
>  		clean_sockets(test, test->ifobj_tx);
>  		clean_sockets(test, test->ifobj_rx);

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