Re: [PATCH net 4/6] seg6: discard CHECKSUM_COMPLETE after the End.BPF program

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

Hi Andrea,
On Sat, Oct 31, 2026 at 11:50:31PM +0100, Andrea Mayer wrote:
> The End.BPF program can change the packet through the seg6 BPF helpers,
> for instance the SRH Tag with bpf_lwt_seg6_store_bytes(). After the
> program, seg6_bpf_has_valid_srh() can also rewrite the SRH Hdr Ext Len.
> 
> With CHECKSUM_COMPLETE, the program does not always update skb->csum for
> the bytes it writes, and seg6_bpf_has_valid_srh() does not either. If the
> packet is then delivered on the same node and skb->csum was 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.
> 
> Discard CHECKSUM_COMPLETE with skb_checksum_complete_unset() after the
> program, since input_action_end_bpf() does not know which bytes the program
> changed. The checksum is then verified in software.
> 
> Found by manual code inspection and reproduced in a VM without kernel
> changes or error injection: with skb->csum computed by a tc ct action, an
> End.BPF program that writes the SRH Tag, followed by End.B6.Encaps and a
> local End.DT6 SID, makes UDP drop the packet. With this patch, the packet
> is delivered. Also tested against the lwt_seg6local BPF selftest, and with
> a ConnectX-6 Lx NIC (mlx5) that provides CHECKSUM_COMPLETE.
> 
> Fixes: 004d4b274e2a ("ipv6: sr: Add seg6local action End.BPF")
> Signed-off-by: Andrea Mayer <andrea.mayer@uniroma2.it>
> ---
>  net/ipv6/seg6_local.c | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/net/ipv6/seg6_local.c b/net/ipv6/seg6_local.c
> index 57a82477ce3f..3045a351f4e9 100644
> --- a/net/ipv6/seg6_local.c
> +++ b/net/ipv6/seg6_local.c
> @@ -1582,6 +1582,12 @@ static int input_action_end_bpf(struct sk_buff *skb,
>  		goto drop;
>  	}
>  
> +	/* The program may have changed the packet and seg6_bpf_has_valid_srh()
> +	 * may rewrite the SRH Hdr Ext Len below.
> +	 * skb->csum may then no longer be the checksum of the full packet.
> +	 */
> +	skb_checksum_complete_unset(skb);
> +

Should we move this a bit later?

>  	if (srh_state->srh && !seg6_bpf_has_valid_srh(skb))
>  		goto drop;
>  	local_unlock_nested_bh(&seg6_bpf_srh_states.bh_lock);
> -- 

e.g. after this. As there is not need to do the setting if the skb is dropped.

Thanks
Hangbin