Re: [PATCH net 3/6] seg6: update skb checksum when SRv6 behaviors advance to the next SID

[Hangbin Liu <hangbin.liu@linux.dev>]

Hi Andrea,
On Sat, Oct 31, 2026 at 11:50:30PM +0100, Andrea Mayer wrote:
> advance_nextseg() decrements Segments Left and writes the next segment into
> the destination address, and seg6_next_csid_advance_arg() advances the
> NEXT-C-SID argument in the destination address.
> With CHECKSUM_COMPLETE, neither updates skb->csum for these changes. The
> stale skb->csum is read when the packet is delivered on the same node, e.g.
> through End.B6.Encaps towards a local End.DT6 SID.
> As a consequence, when skb->csum is computed in software, for instance by
> conntrack, UDP can drop the packet as a checksum error. With skb->csum
> computed by the device, TCP can instead log a "hw csum failure", as if the
> device had computed a wrong skb->csum.
> 
> Update skb->csum in both functions with update_csum_diff16() for the
> destination address. advance_nextseg() also uses update_csum_diff4() for
> Segments Left.
> 
> Depends on patch "seg6: fix skb checksum update helpers".
> 
> Found by manual code inspection while working on
> commit 8ae3f4067ba4 ("seg6: ensure packet data is writable before modifying
> SRH and IPv6 DA"), and reproduced in a VM without kernel changes or error
> injection. A following commit adds a selftest that reproduces the problem
> with a tc ct action. With this patch, the selftest passes for the
> End.B6.Encaps and next-csid paths. Also tested against the srv6 selftests,
> and with a ConnectX-6 Lx NIC (mlx5) that provides CHECKSUM_COMPLETE.
> 
> Fixes: 140f04c33bbc ("ipv6: sr: implement several seg6local actions")
> Fixes: 848f3c0d4769 ("seg6: add NEXT-C-SID support for SRv6 End behavior")
> Signed-off-by: Andrea Mayer <andrea.mayer@uniroma2.it>
> ---
>  net/ipv6/seg6_local.c | 18 +++++++++++++++++-
>  1 file changed, 17 insertions(+), 1 deletion(-)
> 
> diff --git a/net/ipv6/seg6_local.c b/net/ipv6/seg6_local.c
> index 0c3d1be7f129..57a82477ce3f 100644
> --- a/net/ipv6/seg6_local.c
> +++ b/net/ipv6/seg6_local.c
> @@ -291,7 +291,9 @@ static struct ipv6_sr_hdr *advance_nextseg(struct sk_buff *skb,
>  					   struct ipv6_sr_hdr *srh,
>  					   enum skb_drop_reason *reason)
>  {
> +	struct in6_addr *daddr;
>  	struct in6_addr *addr;
> +	__be32 old;
>  	int srhoff;
>  	int wlen;
>  
> @@ -309,10 +311,19 @@ static struct ipv6_sr_hdr *advance_nextseg(struct sk_buff *skb,
>  
>  	/* skb_ensure_writable() may change skb pointers; evaluate srh again */
>  	srh = (struct ipv6_sr_hdr *)(skb->data + srhoff);
> +	daddr = &ipv6_hdr(skb)->daddr;
> +	/* nexthdr, hdrlen, type and segments_left */
> +	old = *(__be32 *)srh;
>  
>  	srh->segments_left--;
>  	addr = srh->segments + srh->segments_left;
> -	ipv6_hdr(skb)->daddr = *addr;
> +
> +	if (skb->ip_summed == CHECKSUM_COMPLETE) {
> +		update_csum_diff4(skb, old, *(__be32 *)srh);
> +		update_csum_diff16(skb, (__be32 *)daddr, (__be32 *)addr);
> +	}
> +
> +	*daddr = *addr;

Nit: we can re-use seg6_update_csum() if the transport header set correctly.
But that function is static and we need adjust header files.
So the current change looks good for me.

>  
>  	return srh;
>  }
> @@ -427,11 +438,13 @@ seg6_next_csid_advance_arg(struct sk_buff *skb,
>  	__u8 fnc_octects = seg6_flv_lcnode_func_octects(finfo);
>  	__u8 blk_octects = seg6_flv_lcblock_octects(finfo);
>  	struct in6_addr *addr;
> +	struct in6_addr old;
>  
>  	if (unlikely(skb_ensure_writable(skb, sizeof(struct ipv6hdr))))
>  		return SKB_DROP_REASON_NOMEM;
>  
>  	addr = &ipv6_hdr(skb)->daddr;
> +	old = *addr;
>  
>  	/* advance DA.Argument */
>  	memmove(&addr->s6_addr[blk_octects],
> @@ -440,6 +453,9 @@ seg6_next_csid_advance_arg(struct sk_buff *skb,
>  
>  	memset(&addr->s6_addr[16 - fnc_octects], 0x00, fnc_octects);
>  
> +	if (skb->ip_summed == CHECKSUM_COMPLETE)
> +		update_csum_diff16(skb, (__be32 *)&old, (__be32 *)addr);
> +
>  	return SKB_NOT_DROPPED_YET;
>  }
>  
> -- 
> 2.43.0
> 

Reviewed-by: Hangbin Liu <liuhangbin@kylinos.cn>