Re: [PATCH net v4 4/5] xsk: drain continuation descs on invalid descriptor in __xsk_generic_xmit()

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] Failure to update TX ring consumer pointer when processing only invalid or dropped descriptors leads to a deadlocked TX ring.
--

commit 8b2ad485d536e9c6cf90b478357bdf68eb66e24c
Author: Jason Xing <kerneljasonxing@gmail.com>

xsk: drain continuation descs on invalid descriptor in __xsk_generic_xmit()

This commit addresses an issue where an invalid descriptor mid-packet causes
the partial skb to be dropped, but leaves continuation descriptors in the TX
ring. It introduces a drain_cont flag to flush the remaining fragments of
the corrupted multi-buffer packet on subsequent iterations.

> diff --git a/net/xdp/xsk.c b/net/xdp/xsk.c
> index f4add7be8c935..de953f38b9e2a 100644
> --- a/net/xdp/xsk.c
> +++ b/net/xdp/xsk.c
> @@ -1122,6 +1122,7 @@ static int __xsk_generic_xmit(struct sock *sk)
>  		if (xs->skb)
>  			xsk_drop_skb(xs->skb);
>  		xskq_cons_release(xs->tx);
> +		xs->drain_cont = xp_mb_desc(&desc);
>  	}
>  
>  out:

[Severity: High]
This is a pre-existing issue, but does skipping the TX release when there are
no successfully sent frames lead to a deadlocked TX ring?

In __xsk_generic_xmit(), the consumer pointer is updated and the socket is
awoken only if sent_frame is true:

out:
	if (sent_frame)
		__xsk_tx_release(xs);

The sent_frame flag is set to true only when a packet is successfully
transmitted via __dev_direct_xmit(). If a batch of processing consists
entirely of invalid descriptors, dropped packets, or drained continuations
(including the newly added drain_cont path), sent_frame remains false.

If sent_frame remains false, the function skips calling __xsk_tx_release().
As a result, the advanced cached_cons index is never published to the shared
ring->consumer pointer.

If a workload encounters enough invalid descriptors or drops to fill the
ring, the consumer pointer will never advance. Userspace will see the ring as
completely full and block in poll or sendmsg waiting for EPOLLOUT. Since the
kernel hasn't published the advanced consumer pointer nor called
sk_write_space(), could the socket permanently deadlock?

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