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

[Andrea Mayer]

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.

Thanks,
Andrea