Re: [PATCH net v3] net: gso: Forbid IPv6 TSO with extensions on devices with only IPV6_CSUM

[Willem de Bruijn <willemdebruijn.kernel@gmail.com>]

xietangxin wrote:
> 
> 
> 在 2025/8/14 18:51, Jakub Ramaseuski 写道:
> > When performing Generic Segmentation Offload (GSO) on an IPv6 packet that
> > contains extension headers, the kernel incorrectly requests checksum offload
> > if the egress device only advertises NETIF_F_IPV6_CSUM feature, which has 
> > a strict contract: it supports checksum offload only for plain TCP or UDP 
> > over IPv6 and explicitly does not support packets with extension headers.
> > The current GSO logic violates this contract by failing to disable the feature
> > for packets with extension headers, such as those used in GREoIPv6 tunnels.
> > 
> > This violation results in the device being asked to perform an operation
> > it cannot support, leading to a `skb_warn_bad_offload` warning and a collapse
> > of network throughput. While device TSO/USO is correctly bypassed in favor
> > of software GSO for these packets, the GSO stack must be explicitly told not 
> > to request checksum offload.
> > 
> > Mask NETIF_F_IPV6_CSUM, NETIF_F_TSO6 and NETIF_F_GSO_UDP_L4
> > in gso_features_check if the IPv6 header contains extension headers to compute
> > checksum in software.
> > 
> > The exception is a BIG TCP extension, which, as stated in commit
> > 68e068cabd2c6c53 ("net: reenable NETIF_F_IPV6_CSUM offload for BIG TCP packets"):
> > "The feature is only enabled on devices that support BIG TCP TSO.
> > The header is only present for PF_PACKET taps like tcpdump,
> > and not transmitted by physical devices."
> > 
> > kernel log output (truncated):
> > WARNING: CPU: 1 PID: 5273 at net/core/dev.c:3535 skb_warn_bad_offload+0x81/0x140
> > ...
> > Call Trace:
> >  <TASK>
> >  skb_checksum_help+0x12a/0x1f0
> >  validate_xmit_skb+0x1a3/0x2d0
> >  validate_xmit_skb_list+0x4f/0x80
> >  sch_direct_xmit+0x1a2/0x380
> >  __dev_xmit_skb+0x242/0x670
> >  __dev_queue_xmit+0x3fc/0x7f0
> >  ip6_finish_output2+0x25e/0x5d0
> >  ip6_finish_output+0x1fc/0x3f0
> >  ip6_tnl_xmit+0x608/0xc00 [ip6_tunnel]
> >  ip6gre_tunnel_xmit+0x1c0/0x390 [ip6_gre]
> >  dev_hard_start_xmit+0x63/0x1c0
> >  __dev_queue_xmit+0x6d0/0x7f0
> >  ip6_finish_output2+0x214/0x5d0
> >  ip6_finish_output+0x1fc/0x3f0
> >  ip6_xmit+0x2ca/0x6f0
> >  ip6_finish_output+0x1fc/0x3f0
> >  ip6_xmit+0x2ca/0x6f0
> >  inet6_csk_xmit+0xeb/0x150
> >  __tcp_transmit_skb+0x555/0xa80
> >  tcp_write_xmit+0x32a/0xe90
> >  tcp_sendmsg_locked+0x437/0x1110
> >  tcp_sendmsg+0x2f/0x50
> > ...
> > skb linear:   00000000: e4 3d 1a 7d ec 30 e4 3d 1a 7e 5d 90 86 dd 60 0e
> > skb linear:   00000010: 00 0a 1b 34 3c 40 20 11 00 00 00 00 00 00 00 00
> > skb linear:   00000020: 00 00 00 00 00 12 20 11 00 00 00 00 00 00 00 00
> > skb linear:   00000030: 00 00 00 00 00 11 2f 00 04 01 04 01 01 00 00 00
> > skb linear:   00000040: 86 dd 60 0e 00 0a 1b 00 06 40 20 23 00 00 00 00
> > skb linear:   00000050: 00 00 00 00 00 00 00 00 00 12 20 23 00 00 00 00
> > skb linear:   00000060: 00 00 00 00 00 00 00 00 00 11 bf 96 14 51 13 f9
> > skb linear:   00000070: ae 27 a0 a8 2b e3 80 18 00 40 5b 6f 00 00 01 01
> > skb linear:   00000080: 08 0a 42 d4 50 d5 4b 70 f8 1a
> > 
> > Fixes: 04c20a9356f283da ("net: skip offload for NETIF_F_IPV6_CSUM if ipv6 header contains extension")
> > Reported-by: Tianhao Zhao <tizhao@redhat.com>
> > Suggested-by: Michal Schmidt <mschmidt@redhat.com>
> > Suggested-by: Willem de Bruijn <willemdebruijn.kernel@gmail.com>
> > Signed-off-by: Jakub Ramaseuski <jramaseu@redhat.com>
> > ---
> > ---
> >  net/core/dev.c | 12 ++++++++++++
> >  1 file changed, 12 insertions(+)
> > 
> > diff --git a/net/core/dev.c b/net/core/dev.c
> > index b28ce68830b2b..1d8a4d1da911e 100644
> > --- a/net/core/dev.c
> > +++ b/net/core/dev.c
> > @@ -3778,6 +3778,18 @@ static netdev_features_t gso_features_check(const struct sk_buff *skb,
> >  		if (!(iph->frag_off & htons(IP_DF)))
> >  			features &= ~NETIF_F_TSO_MANGLEID;
> >  	}
> > +
> > +	/* NETIF_F_IPV6_CSUM does not support IPv6 extension headers,
> > +	 * so neither does TSO that depends on it.
> > +	 */
> > +	if (features & NETIF_F_IPV6_CSUM &&
> > +	    (skb_shinfo(skb)->gso_type & SKB_GSO_TCPV6 ||
> > +	     (skb_shinfo(skb)->gso_type & SKB_GSO_UDP_L4 &&
> > +	      vlan_get_protocol(skb) == htons(ETH_P_IPV6))) &&
> > +	    skb_transport_header_was_set(skb) &&
> > +	    skb_network_header_len(skb) != sizeof(struct ipv6hdr) &&
> > +	    !ipv6_has_hopopt_jumbo(skb))
> > +		features &= ~(NETIF_F_IPV6_CSUM | NETIF_F_TSO6 | NETIF_F_GSO_UDP_L4);
> >  
> >  	return features;
> >  }
> question about this patch affecting tunneled IPv6-in-IPv4 packets
> 
> In our environment with a hinic NIC, we use VXLAN tunnels where
> the outer header is IPv4 and the inner is IPv6. After this commit,
> large packets no longer use hardware TSO and fall back to software segmentation.
> 
> In the VXLAN IPv6-in-IPv4 case, `skb_shinfo(skb)->gso_type` includes
> `SKB_GSO_TCPV6` (inner is IPv6 TCP), but the network header points to the outer
> IPv4 header. Thus `skb_network_header_len(skb)` returns the IPv4 header length
> (usually 20), which is not equal to `sizeof(struct ipv6hdr)` (40). This causes
> the condition to trigger and clears `NETIF_F_TSO6`, even though the inner IPv6
> packet has no extension headers and the device is capable of handling TSO for
> such packets.
> 
> Is it the intended behavior to disable TSO for all tunneled IPv6-in-IPv4 packets
> when the NIC lacks NETIF_F_HW_CSUM, even if the inner IPv6 header has no extensions?
> 
> Any feedback or guidance would be greatly appreciated.

That is definitely unintended.

Thanks for the clear analysis.

I was about to write a refinement that might catch this case,
something like

@@ -3819,8 +3819,10 @@ static netdev_features_t gso_features_check(const struct sk_buff *skb,
            (skb_shinfo(skb)->gso_type & SKB_GSO_TCPV6 ||
             (skb_shinfo(skb)->gso_type & SKB_GSO_UDP_L4 &&
              vlan_get_protocol(skb) == htons(ETH_P_IPV6))) &&
-           skb_transport_header_was_set(skb) &&
-           skb_network_header_len(skb) != sizeof(struct ipv6hdr))
+             ((!skb->encapsulation &&
+               skb_transport_header_was_set(skb) &&
+               skb_network_header_len(skb) != sizeof(struct ipv6hdr)) ||
+              (skb_inner_network_header_len(skb) != sizeof(struct ipv6hdr))))
                features &= ~(NETIF_F_IPV6_CSUM | NETIF_F_TSO6 | NETIF_F_GSO_UDP_L4);

But, how are these VXLAN IPv6-in-IPv4 packets having
vlan_get_protocol(skb) == htons(ETH_P_IPV6)?

Shouldn't that be the protocol of the outer headr, so ETH_P_IP, and
thus this branch not reached at all? (Which itself would leave a false
positive as now an inner network header with extensions would not be
caught..)