Re: [PATCH net 5/6] seg6: account for the MAC header in skb checksum on L2 encapsulation

[Hangbin Liu]

On Wed, Nov 04, 2026 at 09:25:46PM +0100, Andrea Mayer wrote:
> On Tue, 3 Nov 2026 10:37:34 +0800
> Hangbin Liu <hangbin.liu@xxxxxxxxx> wrote:
> 
> > [snip]
> > >  		skb_mac_header_rebuild(skb);
> > > -		skb_push(skb, skb->mac_len);
> > > +		skb_push_rcsum(skb, skb->mac_len);
> > >  
> > >  		if (tinfo->mode == SEG6_IPTUN_MODE_L2ENCAP)
> > >  			err = __seg6_do_srh_encap(skb, tinfo->srh,
> > > -- 
> > > 2.43.0
> > > 
> > 
> > Should we put the skb_push_rcsum() inside of __seg6_do_srh_encap() and
> > seg6_do_srh_encap_red()? They all do skb_mac_header_rebuild() again.
> > [snip]
> 
> Hi Hangbin,
> 
> I am not sure where exactly you would like to put skb_push_rcsum() in
> these two functions. I can read it in two ways.
> 
> 1) Right after skb_mac_header_rebuild(), for example in
>    __seg6_do_srh_encap():
> 
>  	skb_push(skb, tot_len);
>  	skb_reset_network_header(skb);
>  	skb_mac_header_rebuild(skb);
> +	if (proto == IPPROTO_ETHERNET)
> +		skb_push_rcsum(skb, skb->mac_len);
>  	hdr = ipv6_hdr(skb);
> 
>    Here the outer headers are already pushed, and skb_mac_header_rebuild()
>    moves the MAC header in front of them, outside skb->data. The push then
>    makes the packet start with that MAC header, instead of the outer
>    IPv6 header.
> 
> 2) Before the outer headers are pushed, for example in
>    __seg6_do_srh_encap():
> 
> +	if (proto == IPPROTO_ETHERNET)
> +		skb_push_rcsum(skb, skb->mac_len);
> +
>  	inner_hdr = ipv6_hdr(skb);
>  	flowlabel = seg6_make_flowlabel(net, skb, inner_hdr);
> 
>    This can work, with the push removed from seg6_do_srh(), but the L2
>    case in seg6_do_srh() would still keep the skb_mac_header_was_set()
>    check and the headroom for the MAC header, and part of the L2 handling
>    would move into __seg6_do_srh_encap() and seg6_do_srh_encap_red().
>    __seg6_do_srh_encap() is also used by the ENCAP mode, End.B6.Encaps
>    and bpf_push_seg6_encap(), and seg6_do_srh_encap_red() by the
>    ENCAP_RED mode.
> 
> So the first way does not work. In the second one I do not see a real
> advantage, and it would split the L2 handling between three functions.
> For this reason I would like to keep the change as it is in the patch.
> 
> What do you think? If you meant something else, please let me know.
> 

Oh, I miss-read the change. This is rcsum is only for inner header. The
outer header will be updated via later skb_postpush_rcsum().

Thanks
Hangbin