Re: [PATCH net-next 2/8] udp: skip fwd/list GRO for tunnel packets

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

On Sun, Mar 21, 2021 at 1:01 PM Paolo Abeni <pabeni@redhat.com> wrote:
>
> If UDP GRO forwarding (or list) is enabled,

Please explicitly mention the gso type SKB_GSO_FRAGLIST. I, at least,
didn't immediately grasp that gro forwarding is an alias for that.

> and there are
> udp tunnel available in the system, we could end-up doing L4
> aggregation for packets targeting the UDP tunnel.

Is this specific to UDP tunnels, or can this also occur with others,
such as GRE? (not implying that this patchset needs to address those
at the same time)

> That could inner protocol corruption, as no overaly network
> parameters is taken in account at aggregation time.

nit: overaly .. is taken -> overlay .. are taken

You mean the packets on the frag list may have mtu exceeding the mtu
of the tunnel? Please make the constraint more explicit.

> Just skip the fwd GRO if this packet could land in an UDP
> tunnel.

Could you make more clear that this does not skip UDP GRO, only
switches from fraglist-based to pure SKB_GSO_UDP_L4.

> The current check is broader than what is strictly
> needed, as the UDP tunnel could be e.g. on top of a different
> device, but is simple and the performance downside looks not
> relevant.
>
> Fixes: 9fd1ff5d2ac7 ("udp: Support UDP fraglist GRO/GSO.")
> Fixes: 36707061d6ba ("udp: allow forwarding of plain (non-fraglisted) UDP GRO packets")
> Signed-off-by: Paolo Abeni <pabeni@redhat.com>