Re: [PATCH net v2 1/2] xsk: defer software checksum until the skb is complete

[Maciej Fijalkowski <maciej.fijalkowski@intel.com>]

On Sat, Oct 24, 2026 at 10:11:56PM +0800, Rongzheng Wang wrote:
> When XDP_UMEM_TX_SW_CSUM is used with multi-buffer TX, xsk_skb_metadata()
> calls skb_checksum_help() while only the first descriptor has been added to
> the skb. The checksum therefore covers just the first fragment, and
> skb_checksum_help() sets ip_summed to CHECKSUM_NONE, so the remaining
> fragments are never included in the checksum.
> 
> Move the software checksum calculation to the point where the last
> descriptor has been added to the skb.
> 
> On a checksum failure the packet is now dropped instead of being returned
> to the application for a retry. This path is not reachable with the
> existing metadata validation, but the behavior change is intentional.
> 
> Fixes: 11614723af26 ("xsk: Add option to calculate TX checksum in SW")
> Cc: stable@vger.kernel.org
> Signed-off-by: Rongzheng Wang <testwrzwrz@gmail.com>
> ---
>  net/xdp/xsk.c | 22 ++++++++++++++--------
>  1 file changed, 14 insertions(+), 8 deletions(-)
> 
> diff --git a/net/xdp/xsk.c b/net/xdp/xsk.c
> index 33475b180ea6..65be2a368325 100644
> --- a/net/xdp/xsk.c
> +++ b/net/xdp/xsk.c
> @@ -1011,14 +1011,6 @@ static int xsk_skb_metadata(struct sk_buff *skb, void *buffer,
>  		skb->csum_start = hr + csum_start;
>  		skb->csum_offset = csum_offset;
>  		skb->ip_summed = CHECKSUM_PARTIAL;
> -
> -		if (unlikely(pool->tx_sw_csum)) {
> -			int err;
> -
> -			err = skb_checksum_help(skb);
> -			if (err)
> -				return err;
> -		}
>  	}
>  
>  	if (flags & XDP_TXMD_FLAGS_LAUNCH_TIME)
> @@ -1030,6 +1022,14 @@ static int xsk_skb_metadata(struct sk_buff *skb, void *buffer,
>  	return 0;
>  }
>  
> +static int xsk_skb_sw_csum(struct sk_buff *skb, struct xsk_buff_pool *pool)
> +{
> +	if (!pool->tx_sw_csum || skb->ip_summed != CHECKSUM_PARTIAL)

If i am reading this right check against ip_summed is a way to tell that
first frag had XDP_TXMD_FLAGS_CHECKSUM flag set.

Agreed with Jason to preserve unlikely() when checking pool->tx_sw_csum.

Regarding skb drop, I am leaning towards this, I don't see much sense in
retry when packet is malformed? Not sure how this could be communicated up
to user space.

> +		return 0;
> +
> +	return skb_checksum_help(skb);
> +}
> +
>  static struct sk_buff *xsk_build_skb_zerocopy(struct xdp_sock *xs,
>  					      struct xdp_desc *desc)
>  {
> @@ -1275,6 +1275,12 @@ static int __xsk_generic_xmit(struct sock *sk)
>  		if (xp_mb_desc(&desc)) {
>  			xs->skb = skb;
>  			continue;
> +		} else {
> +			err = xsk_skb_sw_csum(skb, xs->pool);
> +			if (unlikely(err)) {
> +				xsk_drop_skb(skb);
> +				goto out;
> +			}
>  		}
>  
>  		err = __dev_direct_xmit(skb, xs->queue_id);
> -- 
> 2.39.5 (Apple Git-154)
>