Message-ID: <20240326113403.397786-1-atenart@kernel.org>
Date: Tue, 26 Mar 2024 12:33:57 +0100
From: Antoine Tenart <atenart@...nel.org>
To: davem@...emloft.net,
	kuba@...nel.org,
	pabeni@...hat.com,
	edumazet@...gle.com
Cc: Antoine Tenart <atenart@...nel.org>,
	steffen.klassert@...unet.com,
	willemdebruijn.kernel@...il.com,
	netdev@...r.kernel.org
Subject: [PATCH net v4 0/5] gro: various fixes related to UDP tunnels

Hello,

We found issues when a UDP tunnel endpoint is in a different netns than
where UDP GRO happens. This kind of setup is actually quite diverse,
from having one leg of the tunnel on a remove host, to having a tunnel
between netns (eg. being bridged in another one or on the host). In our
case that UDP tunnel was geneve.

UDP tunnel packets should not be GROed at the UDP level. The fundamental
issue here is such packet can't be detected in a foolproof way: we can't
know by looking at a packet alone and the current logic of looking up
UDP sockets is fragile (socket could be in another netns, packet could
be modified in between, etc). Because there is no way to make the GRO
code to correctly handle those packets in all cases, this series aims at
two things: making the net stack to correctly behave (as in, no crash
and no invalid packet) when such thing happens, and in some cases to
prevent this "early GRO" from happening.

First three patches fix issues when an "UDP tunneled" packet is being
GROed too early by rx-udp-gro-forwarding or rx-gro-list.

Last patch is preventing locally generated UDP tunnel packets from being
GROed. This turns out to be more complex than this patch alone as it
relies on skb->encapsulation which is currently untrusty in some cases
(see iptunnel_handle_offloads); but that should fix things in practice
and is acceptable for a fix. Future work is required to improve things
(prevent all locally generated UDP tunnel packets from being GROed),
such as fixing the misuse of skb->encapsulation in drivers; but that
would be net-next material.

Thanks!
Antoine

Since v3:
  - Fixed the udpgro_fwd selftest in patch 5 (Jakub Kicinski feedback).
  - Improved commit message on patch 3 (Willem de Bruijn feeback).

Since v2:
  - Fixed a build issue with IPv6=m in patch 1 (Jakub Kicinski
    feedback).
  - Fixed typo in patch 1 (Nikolay Aleksandrov feedback).
  - Added Reviewed-by tag on patch 2 (Willem de Bruijn feeback).
  - Added back conversion to CHECKSUM_UNNECESSARY but only from non
    CHECKSUM_PARTIAL in patch 3 (Paolo Abeni & Willem de Bruijn
    feeback).
  - Reworded patch 3 commit msg.

Since v1:
  - Fixed a build issue with IPv6 disabled in patch 1.
  - Reworked commit log in patch 2 (Willem de Bruijn feedback).
  - Added Reviewed-by tags on patches 1 & 4 (Willem de Bruijn feeback).

Antoine Tenart (5):
  udp: do not accept non-tunnel GSO skbs landing in a tunnel
  gro: fix ownership transfer
  udp: do not transition UDP GRO fraglist partial checksums to
    unnecessary
  udp: prevent local UDP tunnel packets from being GROed
  selftests: net: gro fwd: update vxlan GRO test expectations

 include/linux/udp.h                       | 28 +++++++++++++++++++++++
 net/core/gro.c                            |  3 ++-
 net/ipv4/udp.c                            |  7 ++++++
 net/ipv4/udp_offload.c                    | 23 +++++++++++--------
 net/ipv6/udp.c                            |  2 +-
 net/ipv6/udp_offload.c                    |  8 +------
 tools/testing/selftests/net/udpgro_fwd.sh | 10 ++------
 7 files changed, 54 insertions(+), 27 deletions(-)

-- 
2.44.0

Powered by blists - more mailing lists