Re: [PATCH net-next v3 1/5] net: core: use the packet's L3 protocol for the GSO size limit

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

Wang Zhan wrote:
> gso_features_check() compares the frame length against
> netif_get_gso_max_size(), which picks the IPv4 or the IPv6 limit from
> skb->protocol.  A tag which is already inside the frame replaces that field
> with the VLAN ethertype, as skb_vlan_push() does, and an IPv6 packet is
> then measured against the IPv4 limit and segmented although the device
> could send it as one TSO frame.
> 
> Let the limit lookup take the protocol as an argument, and pass the L3
> protocol, so where the tag sits does not decide which limit applies.  The
> helper cannot look behind the tag itself: it lives in netdevice.h, which
> cannot include if_vlan.h because that header includes netdevice.h.
> 
> Fixes: e609c959a9396 ("net: Fix gso_features_check to check for both dev->gso_{ipv4_,}max_size")
> Assisted-by: LLM
> Signed-off-by: Wang Zhan <wang.zhan@smartx.com>
> 
> ---
> v3:
> - new patch: the L3 protocol fix split out of the resegmentation patch
> v2: https://lore.kernel.org/20260918084651.3022878-4-wang.zhan@smartx.com/
> v1: https://lore.kernel.org/20260917063854.2011613-4-wang.zhan@smartx.com/
> ---
>  include/linux/netdevice.h | 4 ++--
>  net/core/dev.c            | 3 ++-
>  2 files changed, 4 insertions(+), 3 deletions(-)
> 
> diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
> index d037faff7c44b..1c28861fcbe2c 100644
> --- a/include/linux/netdevice.h
> +++ b/include/linux/netdevice.h
> @@ -5562,10 +5562,10 @@ netif_get_gro_max_size(const struct net_device *dev, const struct sk_buff *skb)
>  }
>  
>  static inline unsigned int
> -netif_get_gso_max_size(const struct net_device *dev, const struct sk_buff *skb)
> +netif_get_gso_max_size(const struct net_device *dev, __be16 protocol)
>  {
>  	/* pairs with WRITE_ONCE() in netif_set_gso(_ipv4)_max_size() */
> -	return skb->protocol == htons(ETH_P_IPV6) ?
> +	return protocol == htons(ETH_P_IPV6) ?
>  	       READ_ONCE(dev->gso_max_size) :
>  	       READ_ONCE(dev->gso_ipv4_max_size);
>  }
> diff --git a/net/core/dev.c b/net/core/dev.c
> index f660fccfc0dbc..ffa9b0c27788c 100644
> --- a/net/core/dev.c
> +++ b/net/core/dev.c
> @@ -3843,7 +3843,8 @@ static netdev_features_t gso_features_check(const struct sk_buff *skb,
>  	if (gso_segs > READ_ONCE(dev->gso_max_segs))
>  		return features & ~NETIF_F_GSO_MASK;
>  
> -	if (unlikely(skb->len >= netif_get_gso_max_size(dev, skb)))
> +	if (unlikely(skb->len >=
> +		     netif_get_gso_max_size(dev, vlan_get_protocol(skb))))

If netif_get_gso_max_size needs to inspect the result from 
vlan_get_protocol, call it inside that function directly? More robust
and a one line change.

>  		return features & ~NETIF_F_GSO_MASK;
>  
>  	if (!skb_shinfo(skb)->gso_type) {
> -- 
> 2.47.3
>