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

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

Ilya Maximets wrote:
> On 9/25/25 12:15 PM, Paolo Abeni wrote:
> > 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.
> 
> Hmm.  Thanks for pointing that out.  It does seem like OVS will fragment
> GSO packets without segmenting them first in case MRU of that packet is
> larger than the MTU of the destination port.  In practice though, MRU of
> a GSO packet should not exceed path MTU in a general case.  I suppose it
> can still happen in some corner cases, e.g. if MTU suddenly changed, in
> which case the packet should probably be dropped instead of re-fragmenting.
> 
> I also looked through other parts of the kernel and it seems like GSO
> packets are not fragmented after being segmented in other places like
> the br-netfilter code.  Which suggests that MRU supposed to be smaller
> than MTU and so the fragmentation is not necessary, otherwise the packets
> will be dropped.
> 
> Does that sound correct or am I missing some cases here?

One of the discussed cases is where a packet is transformed from
IPv4 to IPv6, e.g., with a BPF program. Similar would be tunnel encap.
Or just forwarding between devices with different MTU.