Message-ID: <dd89dc81-6c1b-4753-9682-9876c18acffc@redhat.com>
Date: Thu, 25 Sep 2025 12:15:27 +0200
From: Paolo Abeni <pabeni@...hat.com>
To: Aaron Conole <aconole@...hat.com>, Eelco Chaudron <echaudro@...hat.com>,
 Ilya Maximets <i.maximets@....org>
Cc: davem@...emloft.net, edumazet@...gle.com, kuba@...nel.org,
 horms@...nel.org, corbet@....net, saeedm@...dia.com, tariqt@...dia.com,
 mbloch@...dia.com, leon@...nel.org, dsahern@...nel.org,
 ncardwell@...gle.com, ecree.xilinx@...il.com,
 Richard Gobert <richardbgobert@...il.com>, kuniyu@...gle.com,
 shuah@...nel.org, sdf@...ichev.me, aleksander.lobakin@...el.com,
 florian.fainelli@...adcom.com, alexander.duyck@...il.com,
 linux-kernel@...r.kernel.org, linux-net-drivers@....com,
 netdev@...r.kernel.org, willemdebruijn.kernel@...il.com
Subject: Re: [PATCH net-next v6 4/5] net: gro: remove unnecessary df checks

Adding the OVS maintainers for awareness..

On 9/22/25 10:19 AM, Richard Gobert wrote:
> Richard Gobert wrote:
>> Paolo Abeni wrote:
>>> On 9/16/25 4:48 PM, Richard Gobert wrote:
>>>> Currently, packets with fixed IDs will be merged only if their
>>>> don't-fragment bit is set. This restriction is unnecessary since
>>>> packets without the don't-fragment bit will be forwarded as-is even
>>>> if they were merged together. The merged packets will be segmented
>>>> into their original forms before being forwarded, either by GSO or
>>>> by TSO. The IDs will also remain identical unless NETIF_F_TSO_MANGLEID
>>>> is set, in which case the IDs can become incrementing, which is also fine.
>>>>
>>>> Note that IP fragmentation is not an issue here, since packets are
>>>> segmented before being further fragmented. Fragmentation happens the
>>>> same way regardless of whether the packets were first merged together.
>>>
>>> I agree with Willem, that an explicit assertion somewhere (in
>>> ip_do_fragmentation?!?) could be useful.
>>>
>>
>> As I replied to Willem, I'll mention ip_finish_output_gso explicitly in the
>> commit message.
>>
>> Or did you mean I should add some type of WARN_ON assertion that ip_do_fragment isn't
>> called for GSO packets?
>>
>>> Also I'm not sure that "packets are segmented before being further
>>> fragmented" is always true for the OVS forwarding scenario.
>>>
>>
>> If this is really the case, it is a bug in OVS. Segmentation is required before
>> fragmentation as otherwise GRO isn't transparent and fragments will be forwarded
>> that contain data from multiple different packets. It's also probably less efficient,
>> if the segment size is smaller than the MTU. I think this should be addressed in a
>> separate patch series.
>>
>> I'll also mention OVS in the commit message.
>>
> 
> I looked into it, and it seems that you are correct. Looks like fragmentation
> can occur without segmentation in the OVS forwarding case. As I said, this is
> a bug since generated fragments may contain data from multiple packets. Still,
> this can already happen for packets with incrementing IDs and nothing special
> in particular will happen for the packets discussed in this patch. This should
> be fixed in a separate patch series, as do all other cases where ip_do_fragment
> is called directly without segmenting the packets first.

TL;DR: apparently there is a bug in OVS segmentation/fragmentation code:
OVS can do fragmentation of GSO packets without segmenting them
beforehands, please see the threads under:

https://lore.kernel.org/netdev/20250916144841.4884-5-richardbgobert@gmail.com/

for the whole discussion.

Thanks,

Paolo

Powered by blists - more mailing lists