Re: [PATCH net v4 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 newly added TX drain loop and `outstanding_tx` clamping introduce a race condition that causes premature exit from `wait_for_tx_completion()`, leaving valid packets in the TX ring and causing flaky test timeouts.
--

commit dbc5e902ae609e02c24da36756b02d3a7d8a3445
Author: Jason Xing <kerneljasonxing@gmail.com>
Subject: selftests/xsk: drain CQ to wait for TX completion

This commit addresses an issue where dropped multi-buffer descriptors publish
their addresses to the completion queue via the skb destructor. It modifies
complete_pkts() to clamp outstanding_tx to zero and adds a drain loop in
wait_for_tx_completion().

> diff --git a/tools/testing/selftests/bpf/prog_tests/test_xsk.c b/tools/testing/selftests/bpf/prog_tests/test_xsk.c
> --- a/tools/testing/selftests/bpf/prog_tests/test_xsk.c
> +++ b/tools/testing/selftests/bpf/prog_tests/test_xsk.c
[ ... ]
> @@ -950,17 +950,11 @@ static int complete_pkts(struct xsk_socket_info *xsk, int batch_size)
>  
>  	rcvd = xsk_ring_cons__peek(&xsk->umem->cq, batch_size, &idx);
>  	if (rcvd) {
> -		if (rcvd > xsk->outstanding_tx) {
[ ... ]
> -			return TEST_FAILURE;
> -		}
> -
>  		xsk_ring_cons__release(&xsk->umem->cq, rcvd);
> -		xsk->outstanding_tx -= rcvd;
> +		if (rcvd > xsk->outstanding_tx)
> +			xsk->outstanding_tx = 0;

[Severity: Medium]
Does this clamping of xsk->outstanding_tx to 0 cause the main wait loop in
wait_for_tx_completion() to exit prematurely?

> +		else
> +			xsk->outstanding_tx -= rcvd;
>  	}
>  
>  	return TEST_PASS;
[ ... ]
> @@ -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);

[Severity: Medium]
Can this drain loop exit early if the hardware is actively processing packets
but the completion queue is momentarily empty?

If rcvd == 0 is momentarily true, the loop exits. The 100 microsecond sleep
might be insufficient on real hardware, which would leave valid descriptors
stranded in the TX ring because the TX thread exits and stops issuing wakeups.

Would it be safer to increment outstanding_tx by the total number of
descriptors pushed to the TX ring instead of clamping it?

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

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