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

[Paolo Abeni <pabeni@redhat.com>]

On 8/25/25 3:31 PM, Richard Gobert wrote:
> 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?

I'm not sure I read correctly the above, let me rephrase. You are
suggesting that devices can set MANGLEID but they must ensure, for
encapsulated packets, to keep incrementing IDs for outer IP headers even
when MANGLEID is set. It that what you mean?

Note that most device exposing GSO_PARTIAL can't perform the above.

>> 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? 

In the following setup:

TCP socket -> UDP encap device (without 'df set') -> H/W NIC exposing
GSO partial for encap traffic

with the current kernel, if the TCP socket creates a GSO packet with
size MSS multiple, the wire packets will have the outer IP header with
the DF bit NOT set and will have ID fixed - for all the wire packets
corresponding to a given GSO one.

/P

> 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.

Note that GSO packets can be locally generated.

> GSO partial (and also regular GSO/TSO) generate incrementing IDs, so the
> IDs cannot be mangled. 

AFAIK, most device exposing GSO partial don't increment the outer IP ID
for encap packet (the silicon is not able to do that).

> With my patch, if the IDs were originally fixed,
> regardless of the DF-bit, TSO/GSO partial will not occur unless MANGLEID
> is enabled.

I think the above statement is a lit confusing, S/W segmentation can
happen even if MANGLEID is enabled on the egress device: for FIXEDID
pkts, with DF bit not set, both the current kernel code and your patch
will clear it.

/P