Re: [PATCH net-next v3 3/5] net: gso: restore ids of outer ip headers correctly

[Richard Gobert <richardbgobert@gmail.com>]

Paolo Abeni wrote:
> On 8/21/25 9:30 AM, Richard Gobert wrote:
>> diff --git a/net/core/dev.c b/net/core/dev.c
>> index 68dc47d7e700..9941c39b5970 100644
>> --- a/net/core/dev.c
>> +++ b/net/core/dev.c
>> @@ -3772,10 +3772,9 @@ static netdev_features_t gso_features_check(const struct sk_buff *skb,
>>  	 * IPv4 header has the potential to be fragmented.
>>  	 */
>>  	if (skb_shinfo(skb)->gso_type & SKB_GSO_TCPV4) {
>> -		struct iphdr *iph = skb->encapsulation ?
>> -				    inner_ip_hdr(skb) : ip_hdr(skb);
>> -
>> -		if (!(iph->frag_off & htons(IP_DF)))
>> +		if (!(ip_hdr(skb)->frag_off & htons(IP_DF)) ||
>> +		    (skb->encapsulation &&
>> +		     !(inner_ip_hdr(skb)->frag_off & htons(IP_DF))))
>>  			features &= ~NETIF_F_TSO_MANGLEID;
> 
> FWIW, I think the above is the problematic part causing GSO PARTIAL issues.
> 
> By default UDP tunnels do not set the DF bit, and most/all devices
> implementing GSO_PARTIAL clear TSO for encapsulated packet when MANGLEID
> is not available.
> 
> I think the following should workaround the problem (assuming my email
> client did not corrupt the diff), but I also fear this change will cause
> very visible regressions in existing setups.
> 

Thanks for the thorough review!

To solve this issue, we can decide that MANGLEID cannot cause
incrementing IDs to become fixed for outer headers of encapsulated
packets (which is the current behavior), then just revert this diff.
I'll update the documentation in segmentation-offloads.rst to reflect this.
Do you think that would be a good solution?

> Note that the current status is incorrect - GSO partial devices are
> mangling the outer IP ID for encapsulated packets even when the outer
> header IP DF is not set.
> 
> /P

WDYM? Currently, when the DF-bit isn't set, it means that the IDs must
be incrementing. Otherwise, the packets wouldn't have been merged by GRO.
GSO partial (and also regular GSO/TSO) generate incrementing IDs, so the
IDs cannot be mangled. With my patch, if the IDs were originally fixed,
regardless of the DF-bit, TSO/GSO partial will not occur unless MANGLEID
is enabled.

> ---
> diff --git a/tools/testing/selftests/drivers/net/hw/tso.py
> b/tools/testing/selftests/drivers/net/hw/tso.py
> index 3370827409aa..b0c71a0d8028 100755
> --- a/tools/testing/selftests/drivers/net/hw/tso.py
> +++ b/tools/testing/selftests/drivers/net/hw/tso.py
> @@ -214,8 +214,8 @@ def main() -> None:
>              # name,       v4/v6  ethtool_feature
> tun:(type,    partial, args)
>              ("",            "4", "tx-tcp-segmentation",           None),
>              ("",            "6", "tx-tcp6-segmentation",          None),
> -            ("vxlan",        "", "tx-udp_tnl-segmentation",
> ("vxlan",  True,  "id 100 dstport 4789 noudpcsum")),
> -            ("vxlan_csum",   "", "tx-udp_tnl-csum-segmentation",
> ("vxlan",  False, "id 100 dstport 4789 udpcsum")),
> +            ("vxlan",        "", "tx-udp_tnl-segmentation",
> ("vxlan",  True,  "id 100 dstport 4789 noudpcsum df set")),
> +            ("vxlan_csum",   "", "tx-udp_tnl-csum-segmentation",
> ("vxlan",  False, "id 100 dstport 4789 udpcsum df set")),
>              ("gre",         "4", "tx-gre-segmentation",
> ("gre",    False,  "")),
>              ("gre",         "6", "tx-gre-segmentation",
> ("ip6gre", False,  "")),
>          )
>