Re: [PATCH net v3 5/5] selftests/xsk: drain CQ to wait for TX completion

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The racy drain loop in `wait_for_tx_completion()` can exit prematurely and fail to drain pending CQ entries, leading to test flakiness.
--

commit 25f4d14929e097cf7028dd26ec4e4bae16aa1a5b
Author: Jason Xing <kerneljasonxing@gmail.com>

selftests/xsk: drain CQ to wait for TX completion

This patch addresses test timeouts in xsk multi-buffer invalid-desc tests
caused by dropped descriptors publishing their buffer addresses to the
completion queue. It clamps outstanding_tx to zero to handle extra CQ
completions and adds a drain loop to consume remaining entries.

> diff --git a/tools/testing/selftests/bpf/prog_tests/test_xsk.c b/tools/testing/selftests/bpf/prog_tests/test_xsk.c
> index 7950c504ed289..1f196c8ebc738 100644
> --- a/tools/testing/selftests/bpf/prog_tests/test_xsk.c
> +++ b/tools/testing/selftests/bpf/prog_tests/test_xsk.c

[ ... ]

> @@ -1293,6 +1289,17 @@ static int wait_for_tx_completion(struct xsk_socket_info *xsk)
>  		complete_pkts(xsk, xsk->batch_size);
>  	}
>  
> +	do {
> +		if (xsk_ring_prod__needs_wakeup(&xsk->tx))
> +			kick_tx(xsk);
> +		rcvd = xsk_ring_cons__peek(&xsk->umem->cq, xsk->batch_size, &idx);
> +		if (rcvd)
> +			xsk_ring_cons__release(&xsk->umem->cq, rcvd);
> +	} while (rcvd);

Could this drain loop exit prematurely? If the hardware or kernel is slightly
delayed in posting the remaining completions, xsk_ring_cons__peek() will
return 0 on the first iteration, causing the loop to terminate immediately
without waiting.

> +
> +	usleep(100);
> +	complete_pkts(xsk, xsk->batch_size);

Relying on a hardcoded usleep(100) and a single complete_pkts() call might
not be sufficient if the hardware takes longer than 100us or if the number
of remaining completions exceeds xsk->batch_size.

Would it be more robust to track the total number of submitted descriptors
(including invalid ones) in outstanding_tx instead of using a timeout-based
drain?

> +
>  	return TEST_PASS;
>  }
>

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260517063311.28921-1-kerneljasonxing@gmail.com?part=5