Message-ID: <689512a45dabb_217ac1294e6@willemb.c.googlers.com.notmuch>
Date: Thu, 07 Aug 2025 16:55:00 -0400
From: Willem de Bruijn <willemdebruijn.kernel@...il.com>
To: Jakub Ramaseuski <jramaseu@...hat.com>, 
 netdev@...r.kernel.org
Cc: kuba@...nel.org, 
 horms@...nel.org, 
 pabeni@...hat.com, 
 Jakub Ramaseuski <jramaseu@...hat.com>, 
 Tianhao Zhao <tizhao@...hat.com>, 
 Michal Schmidt <mschmidt@...hat.com>, 
 Willem de Bruijn <willemdebruijn.kernel@...il.com>
Subject: Re: [PATCH net v2] net: mask NETIF_F_IPV6_CSUM flag on irregular
 packet header size

Jakub Ramaseuski wrote:
> On any driver that advertises NETIF_F_IP_CSUM and NETIF_F_IPV6_CSUM but
> not the superseding NETIF_F_HW_CSUM (e.g., ice/bnxt_en), the kernel

Minor point (repeated), the two are generally not combined. See also
the following in netdev_fix_features:

        /* Fix illegal checksum combinations */
        if ((features & NETIF_F_HW_CSUM) &&
            (features & (NETIF_F_IP_CSUM|NETIF_F_IPV6_CSUM))) {
                netdev_warn(dev, "mixed HW and IP checksum settings.\n");
                features &= ~(NETIF_F_IP_CSUM|NETIF_F_IPV6_CSUM);
        }

> incorrectly attempts GSO on IPv6 packets with extension headers.

This is not entirely correct. Hardware TSO/USO cannot be relied to
handle these packets, so GSO is correctly used.

But the GSO stack must not program checksum offload. Instead, compute
the checksum in software during segmentation.

This is tested from skb_segment with can_checksum_protocol. Which
could be amended for this IPv6 with extension header case directly.
But removing NETIF_F_IPV6_CSUM in gso_check_features will have the
same effect.

> Because the NETIF_F_IPV6_CSUM feature is incompatible with these
> headers, the failure results in a `skb_warn_bad_offload` warning and
> a collapse of throughput, as observed with GREoIPv6 traffic.
>
> 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.
> 
> 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
>  ? netif_skb_features+0xc1/0x2e0
>  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
>  ? srso_alias_return_thunk+0x5/0xfbef5
>  ? ip6_rt_copy_init+0xf0/0x290
>  ? srso_alias_return_thunk+0x5/0xfbef5
>  ? selinux_ip_postroute+0x1c5/0x420
>  ? srso_alias_return_thunk+0x5/0xfbef5
>  ip6_finish_output2+0x25e/0x5d0
>  ? srso_alias_return_thunk+0x5/0xfbef5
>  ? nf_hook_slow+0x47/0xf0
>  ? srso_alias_return_thunk+0x5/0xfbef5
>  ip6_finish_output+0x1fc/0x3f0
>  ip6_tnl_xmit+0x608/0xc00 [ip6_tunnel]
>  ? srso_alias_return_thunk+0x5/0xfbef5
>  ip6gre_tunnel_xmit+0x1c0/0x390 [ip6_gre]
>  dev_hard_start_xmit+0x63/0x1c0
>  __dev_queue_xmit+0x6d0/0x7f0
>  ? srso_alias_return_thunk+0x5/0xfbef5
>  ? chacha_block_generic+0x72/0xd0
>  ? srso_alias_return_thunk+0x5/0xfbef5
>  ? selinux_ip_postroute+0x1c5/0x420
>  ip6_finish_output2+0x214/0x5d0
>  ? srso_alias_return_thunk+0x5/0xfbef5
>  ? nf_hook_slow+0x47/0xf0
>  ip6_finish_output+0x1fc/0x3f0
>  ip6_xmit+0x2ca/0x6f0
>  ? __pfx_dst_output+0x10/0x10
>  ? srso_alias_return_thunk+0x5/0xfbef5
>  ? nf_hook_slow+0x47/0xf0
>  ip6_finish_output+0x1fc/0x3f0
>  ip6_xmit+0x2ca/0x6f0
>  ? __pfx_dst_output+0x10/0x10
>  ? srso_alias_return_thunk+0x5/0xfbef5
>  ? __sk_dst_check+0x41/0xc0
>  ? srso_alias_return_thunk+0x5/0xfbef5
>  ? inet6_csk_route_socket+0x12e/0x200
>  inet6_csk_xmit+0xeb/0x150
>  __tcp_transmit_skb+0x555/0xa80
>  tcp_write_xmit+0x32a/0xe90
>  ? srso_alias_return_thunk+0x5/0xfbef5
>  ? skb_do_copy_data_nocache+0xc9/0x150
>  tcp_sendmsg_locked+0x437/0x1110
>  ? srso_alias_return_thunk+0x5/0xfbef5
>  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")

As said, I don't think this introduced the bug.

If you go to 04c20a9356f283da~1, does this traffic work?

> Reported-by: Tianhao Zhao <tizhao@...hat.com>
> Suggested-by: Michal Schmidt <mschmidt@...hat.com>
> Suggested-by: Willem de Bruijn <willemdebruijn.kernel@...il.com>
> Signed-off-by: Jakub Ramaseuski <jramaseu@...hat.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;
>  }
> -- 
> 2.50.1
> 

Powered by blists - more mailing lists