Re: [PATCH net v2 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:
- [High] State machine logic flaw causing valid packet drop due to un-cleared drain_cont flag.
--

commit 8841b899f0208ddaf58f3df5eb5feff9db62147f
Author: Jason Xing <kerneljasonxing@gmail.com>

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

This commit addresses an issue in the TX loop where an invalid descriptor
mid-packet leaves remaining continuation fragments in the ring, which
corrupts the packet stream. It introduces a drain_cont flag to drain the
remaining fragments on the next iteration.

> diff --git a/net/xdp/xsk.c b/net/xdp/xsk.c
> index 9c51eed6ee99c..ac4884a87a0fb 100644
> --- a/net/xdp/xsk.c
> +++ b/net/xdp/xsk.c

[ ... ]

> @@ -1125,6 +1125,8 @@ static int __xsk_generic_xmit(struct sock *sk)
>  		if (xs->skb)
>  			xsk_drop_skb(xs->skb);
>  		xskq_cons_release(xs->tx);
> +		if (xp_mb_desc(&desc))
> +			xs->drain_cont = true;

Does this logic inadvertently leave the socket stuck in the drain state?

If xs->drain_cont is already true and the current invalid descriptor is the
final fragment of the packet (meaning XDP_PKT_CONTD is not set), the flag
is not explicitly cleared since there is no else branch.

If xs->drain_cont remains true, could the next invocation of
__xsk_generic_xmit() intercept the first fragment of a completely valid,
unrelated packet in the if (unlikely(xs->drain_cont)) drain loop? 

It appears the valid packet would then be drained to the completion queue
and silently dropped with -EOVERFLOW.

Would it be safer to unconditionally update the state based on the
descriptor boundary, such as:

xs->drain_cont = xp_mb_desc(&desc);

>  	}
>  
>  out:

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