Re: [PATCH net-next v6 4/5] net: gro: remove unnecessary df checks

[Paolo Abeni]

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@xxxxxxxxx/

for the whole discussion.

Thanks,

Paolo