mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH net-next 0/2] amt: mark relay data as a UDP tunnel packet, with a selftest
@ 2026-10-01 17:10 Omar Ramadan
  2026-10-01 17:10 ` [PATCH net-next 1/2] amt: mark relay data as a UDP tunnel packet before sending it Omar Ramadan
  2026-10-01 17:10 ` [PATCH net-next 2/2] selftests: net: add an amt test for UDP_SEGMENT through the relay Omar Ramadan
  0 siblings, 2 replies; 5+ messages in thread
From: Omar Ramadan @ 2026-10-01 17:10 UTC (permalink / raw)
  To: Taehee Yoo, Andrew Lunn, David S . Miller, Eric Dumazet,
	Jakub Kicinski, Paolo Abeni, Shuah Khan
  Cc: Simon Horman, netdev, linux-kselftest, linux-kernel

This series follows the discussion of my earlier [PATCH net] "amt: do
not offer software GSO on the amt device", which I withdrew. Eric
Dumazet pointed out that tx checksum offload is off by default on amt,
so that case does not trigger by default, and that the better fix is to
call udp_tunnel_handle_offloads() in amt_send_multicast_data() like the
other UDP tunnels do. Patch 1 does that, and patch 2 adds the selftest
that I said I would send with it.

The earlier thread: https://lore.kernel.org/all/20260928181554.85766-1-omar@blockcast.net/

The trigger needs a non-default setting (ethtool -K <amt dev> tx on) and a
GSO source, such as a UDP_SEGMENT sender on the relay. The problem was
found by an LLM-assisted code review of drivers/net/amt.c, while
developing an IPv6 outer transport for amt.

Results, from the selftest in patch 2 in a KVM guest on net-next
commit eb0c18404c89 ("amt: pull the AMT header behind the transport
header in amt_parse_type()") with CONFIG_DEBUG_NET=y, eleven runs per
kernel:
without patch 1 the UDP_SEGMENT burst with tx on is dropped (0 of 900
datagrams arrive, tx_dropped of the relay's egress device +100 per run,
a trace shows __udp_gso_segment() returning -EINVAL during the
segmentation on that device); with patch 1 all 900 arrive intact and
tx_dropped does not move. With tx off, and with plain datagrams, both
kernels pass. The existing amt.sh passes with patch 1 (it was not run
on the unpatched kernel).

Not tested: hardware with UDP tunnel segmentation offload, hardware
checksumming on the egress device (the selftest turns it off there, so
the non-GSO CHECKSUM_PARTIAL packet that now leaves with
skb->encapsulation set is only checked through the software path),
KASAN, sparse, the udp_csum=false variant, and NETIF_F_GSO_FRAGLIST.

The selftest is a small sample so far: an earlier version of it failed
intermittently in about 3 of 25 full runs (listener counters rose but
the receiver got nothing, which I think was its idle timer expiring
before the sender started, inferred and not confirmed). The final
version passed or failed as expected in all 22 runs, but those were in
one 4-vCPU KVM guest, so it may still be flaky elsewhere.

Known and not touched here: amt advertises NETIF_F_GSO_FRAGLIST, and
skb_copy_expand() refuses a SKB_GSO_FRAGLIST skb with a WARN_ON_ONCE().
That is independent of this series, I have not reproduced it, and if it
is real it would be a separate fix for the net tree.

Assisted-by: LLM

Omar Ramadan (2):
  amt: mark relay data as a UDP tunnel packet before sending it
  selftests: net: add an amt test for UDP_SEGMENT through the relay

 drivers/net/amt.c                      |  11 +
 tools/testing/selftests/net/.gitignore |   1 +
 tools/testing/selftests/net/Makefile   |   2 +
 tools/testing/selftests/net/amt_gso.c  | 473 +++++++++++++++++++++++++
 tools/testing/selftests/net/amt_gso.sh | 269 ++++++++++++++
 5 files changed, 756 insertions(+)
 create mode 100644 tools/testing/selftests/net/amt_gso.c
 create mode 100755 tools/testing/selftests/net/amt_gso.sh

-- 
2.43.0


^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH net-next 1/2] amt: mark relay data as a UDP tunnel packet before sending it
  2026-10-01 17:10 [PATCH net-next 0/2] amt: mark relay data as a UDP tunnel packet, with a selftest Omar Ramadan
@ 2026-10-01 17:10 ` Omar Ramadan
  2026-10-01 17:33   ` Eric Dumazet
  2026-10-01 17:10 ` [PATCH net-next 2/2] selftests: net: add an amt test for UDP_SEGMENT through the relay Omar Ramadan
  1 sibling, 1 reply; 5+ messages in thread
From: Omar Ramadan @ 2026-10-01 17:10 UTC (permalink / raw)
  To: Taehee Yoo, Andrew Lunn, David S . Miller, Eric Dumazet,
	Jakub Kicinski, Paolo Abeni, Shuah Khan
  Cc: Simon Horman, netdev, linux-kselftest, linux-kernel

amt_send_multicast_data() copies the multicast packet, puts an AMT
multicast data header and a UDP header in front of it, and sends it with
udp_tunnel_xmit_skb(). Unlike the other UDP tunnels, it never calls
udp_tunnel_handle_offloads(), so the copy has neither skb->encapsulation
nor an SKB_GSO_UDP_TUNNEL* bit set. If the copy is a GSO skb, the lower
layers see a plain UDP_L4 skb that has an outer UDP header in front of
it.

A GSO skb only reaches amt_dev_xmit() when tx checksum offload has been
turned on for the amt device (it is off by default, in which case the
core segments the packet before ndo_start_xmit), for example with a
UDP_SEGMENT sender on the relay. The default configuration is not
affected.

Call udp_tunnel_handle_offloads() on the copy, as bareudp and geneve do.
The AMT header and the UDP header are pushed after that. Two details
need care:

 - udp_csum is true, because udp_tunnel_xmit_skb() is called with
   nocheck set to false. The GSO checksum of the outer UDP header is
   then completed for every segment, which needs
   SKB_GSO_UDP_TUNNEL_CSUM.

 - amt is ARPHRD_ETHER (amt_link_setup() ends with ether_setup()), and
   amt_dev_xmit() pulls the Ethernet header without moving the mac
   header. skb_copy_expand() keeps the mac header relative to the data,
   so, going by the code, in the copy it should sit 14 bytes before the
   inner IP header. The tunnel segmentation derives the length of the
   outer headers from inner_mac_header - transport_header, which would
   then be negative. I did not measure either value; what was observed
   is described below. Reset the mac header on the copy before the
   inner headers are recorded, so that inner_mac_header is the inner IP
   header, as it is for the other tunnels that have no link-layer
   header.

The call also changes what a plain, non-GSO datagram looks like when it
leaves amt. iptunnel_handle_offloads() sets skb->encapsulation on every
skb and clears it again only if ip_summed is not CHECKSUM_PARTIAL. A
CHECKSUM_PARTIAL datagram, which is what the stack hands to the driver
with tx checksum offload on, now has encapsulation set where it had
none before, so netif_skb_features() limits the features available for
it to those in hw_enc_features, and udp_set_csum() takes the local
checksum offload branch, which leaves the inner checksum to the lower
device. The other UDP tunnels do the same, but the selftest does not
cover hardware checksumming of such a packet: the egress device in it
has tx offload off, so skb_checksum_help() completes the checksum in
software.

This follows the suggestion made by Eric Dumazet on the earlier
[PATCH net] "amt: do not offer software GSO on the amt device", which
this replaces.

The problem was found by an LLM-assisted code review of
drivers/net/amt.c while developing an IPv6 outer transport for amt.

Tested with the selftest in the next patch, in a KVM guest running
net-next at commit eb0c18404c89 ("amt: pull the AMT header behind the
transport header in amt_parse_type()"), x86_64, CONFIG_DEBUG_NET=y, AMT
built in, eleven runs per kernel of the final selftest (22 guest boots,
two at a time on a busy host). See the next patch for how stable the
selftest itself has been. The sender is a local UDP_SEGMENT burst
of eight 1200-byte segments plus a 100-byte tail, 100 bursts, IPv4 and
IPv6 inner traffic, with "ethtool -K <amt relay dev> tx on" and the
relay's egress device doing its segmentation in software:

 - Without this patch, every GSO skb (9728 bytes for IPv4, 9748 for
   IPv6) reached amt_dev_xmit(), none of the 900 datagrams arrived at
   the listener, the tx_dropped counter of the relay's egress device
   went up by 100 (one per GSO skb), and nothing was put on the wire.
   A function-graph trace of one run showed __skb_gso_segment() on that
   device failing with -EINVAL, from __udp_gso_segment() under
   udp4_ufo_fragment(), and the skb being freed in validate_xmit_skb().

 - With this patch, all 900 datagrams arrived intact in every run, one
   AMT message per segment was seen on the wire, none of them larger
   than the MTU, tx_dropped did not move, and the gateway counted no UDP
   checksum errors. The trace showed skb_udp_tunnel_segment() doing the
   outer segmentation.

 - With this patch minus the skb_reset_mac_header() call (two runs, with
   an earlier version of the selftest), the packets were dropped in the
   same way as without the patch, and DEBUG_NET warned in
   skb_udp_tunnel_segment() (pskb_may_pull() with a length above
   INT_MAX). That fits a negative header length, but the value itself
   was not printed.

 - With tx offload off (the default), and with non-GSO datagrams with tx
   on, everything arrived with and without the patch.

 - The existing tools/testing/selftests/net/amt.sh passes with the patch
   (discovery, IPv4 and IPv6 forwarding, and both torture tests).

Not tested: hardware that offloads UDP tunnel segmentation or the
checksum of a CHECKSUM_PARTIAL packet, a forwarded UDP GRO packet as the
GSO source, NETIF_F_GSO_FRAGLIST, KASAN, and the udp_csum=false variant,
so the choice of true rests on reading the code and on the patched runs
above being clean, not on a failing false variant. sparse was not run,
and the existing amt.sh was run only with the patch, not on the
unpatched kernel. Only the IPv4 outer transport exists in this tree.

Assisted-by: LLM
Signed-off-by: Omar Ramadan <omar@blockcast.net>
---
 drivers/net/amt.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/drivers/net/amt.c b/drivers/net/amt.c
index 0277e4cac..1f0afc11e 100644
--- a/drivers/net/amt.c
+++ b/drivers/net/amt.c
@@ -1078,7 +1078,18 @@ static void amt_send_multicast_data(struct amt_dev *amt,
 	if (!skb)
 		return;
 
+	/* amt_dev_xmit() pulled the Ethernet header without moving the mac
+	 * header, so the copy's mac header sits 14 bytes before the inner IP
+	 * header. Make it coincide with it, as the inner segmentation code
+	 * expects for a device without a link-layer header.
+	 */
+	skb_reset_mac_header(skb);
 	skb_reset_inner_headers(skb);
+	if (udp_tunnel_handle_offloads(skb, true)) {
+		kfree_skb(skb);
+		return;
+	}
+
 	memset(&fl4, 0, sizeof(struct flowi4));
 	fl4.flowi4_oif         = amt->stream_dev->ifindex;
 	fl4.daddr              = tunnel->ip4;
-- 
2.43.0


^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH net-next 2/2] selftests: net: add an amt test for UDP_SEGMENT through the relay
  2026-10-01 17:10 [PATCH net-next 0/2] amt: mark relay data as a UDP tunnel packet, with a selftest Omar Ramadan
  2026-10-01 17:10 ` [PATCH net-next 1/2] amt: mark relay data as a UDP tunnel packet before sending it Omar Ramadan
@ 2026-10-01 17:10 ` Omar Ramadan
  1 sibling, 0 replies; 5+ messages in thread
From: Omar Ramadan @ 2026-10-01 17:10 UTC (permalink / raw)
  To: Taehee Yoo, Andrew Lunn, David S . Miller, Eric Dumazet,
	Jakub Kicinski, Paolo Abeni, Shuah Khan
  Cc: Simon Horman, netdev, linux-kselftest, linux-kernel

Add amt_gso.sh, which sends a UDP_SEGMENT burst from a local sender on
the relay to a listener behind a gateway, for IPv4 and IPv6, and checks
that every segment arrives with the right length and payload.

With the default features of the amt device, the core segments the burst
before amt_dev_xmit(), so that case is the control for the harness. With
"ethtool -K amtr tx on" the GSO skb reaches the driver unsegmented.
A capture on amtr, which sees what is handed to the driver, is used to
show that, and the case is reported as skipped if no frame larger than
the MTU reached amt_dev_xmit(). A capture on the gateway side of the
link checks that the AMT messages on the wire are not larger than the
MTU, that there is one for each segment, and that the gateway has not
counted checksum errors. The tx_dropped counter of the relay's egress
device must not move. Plain, non-GSO datagrams with tx on are sent as
well.

The relay's egress device has tx offload switched off so that the outer
segmentation and checksum are done in software and the result does not
depend on what the veth passes through.

amt_gso is a small helper because none of the existing tools can send
UDP_SEGMENT to a multicast group through a chosen interface and also
check payloads. It sends, receives and captures.

Without the previous patch the two cases with tx on and a UDP_SEGMENT
burst fail (the burst is dropped on the relay's egress device); the
control and the plain datagram cases pass. With it, all six pass. That
was the result in each of 11 runs per kernel.

About the stability of the test itself: an earlier version failed
intermittently, in about 3 of 25 full runs. The listener's interface
counted the packets but the receiver reported none, in a control case or
a tx-on case. I think the receiver's idle timer expired before the
sender started on a loaded guest, because the receiver had already
exited when the counters rose, but I only inferred that from the
counters. The receiver now waits up to 15 seconds for the first
datagram, and the final version has no warm-up traffic and no fixed
sleeps: the captures are drained after SIGTERM, once the receiver is
done. The final version was run 22 times (11 per kernel) in a 4-vCPU KVM
guest, two guests at a time on a busy host, with the expected result
each time. That is a small sample, so occasional flakiness cannot be
excluded, and it has not been run under a stock kselftest runner or on
another machine. The remaining waits are timeouts: 3 s idle, 15 s for
the first datagram, 5 s for each helper's READY line.

Assisted-by: LLM
Signed-off-by: Omar Ramadan <omar@blockcast.net>
---
 tools/testing/selftests/net/.gitignore |   1 +
 tools/testing/selftests/net/Makefile   |   2 +
 tools/testing/selftests/net/amt_gso.c  | 473 +++++++++++++++++++++++++
 tools/testing/selftests/net/amt_gso.sh | 269 ++++++++++++++
 4 files changed, 745 insertions(+)
 create mode 100644 tools/testing/selftests/net/amt_gso.c
 create mode 100755 tools/testing/selftests/net/amt_gso.sh

diff --git a/tools/testing/selftests/net/.gitignore b/tools/testing/selftests/net/.gitignore
index c9f46031a..0f11d58a8 100644
--- a/tools/testing/selftests/net/.gitignore
+++ b/tools/testing/selftests/net/.gitignore
@@ -1,4 +1,5 @@
 # SPDX-License-Identifier: GPL-2.0-only
+amt_gso
 bind_bhash
 bind_timewait
 bind_wildcard
diff --git a/tools/testing/selftests/net/Makefile b/tools/testing/selftests/net/Makefile
index d4ca82fec..b90a9a382 100644
--- a/tools/testing/selftests/net/Makefile
+++ b/tools/testing/selftests/net/Makefile
@@ -9,6 +9,7 @@ CFLAGS += -I../
 TEST_PROGS := \
 	altnames.sh \
 	amt.sh \
+	amt_gso.sh \
 	arp_ndisc_evict_nocarrier.sh \
 	arp_ndisc_untracked_subnets.sh \
 	bareudp.sh \
@@ -143,6 +144,7 @@ TEST_PROGS_EXTENDED := \
 # end of TEST_PROGS_EXTENDED
 
 TEST_GEN_FILES := \
+	amt_gso \
 	bind_bhash \
 	cmsg_sender \
 	fin_ack_lat \
diff --git a/tools/testing/selftests/net/amt_gso.c b/tools/testing/selftests/net/amt_gso.c
new file mode 100644
index 000000000..96c002e58
--- /dev/null
+++ b/tools/testing/selftests/net/amt_gso.c
@@ -0,0 +1,473 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Helper for amt_gso.sh.
+ *
+ * send:  send datagrams to a multicast group, optionally as one UDP_SEGMENT
+ *        burst per datagram, with a self-describing payload pattern.
+ * recv:  receive datagrams on a UDP port and check count, length and payload
+ *        of every one of them against the pattern used by "send".
+ * sniff: capture on an interface with AF_PACKET and report how large the
+ *        frames handed to the device were.
+ *
+ * Every datagram consists of "chunks" cnt chunks of seg bytes followed by an
+ * optional tail chunk of tail bytes. A chunk starts with a struct chunk_hdr and
+ * the rest of it is a function of (datagram number, chunk number, offset).
+ * With UDP_SEGMENT set to seg, every chunk is one segment on the wire.
+ */
+#define _GNU_SOURCE
+#include <arpa/inet.h>
+#include <errno.h>
+#include <linux/if_packet.h>
+#include <linux/if_ether.h>
+#include <net/if.h>
+#include <netinet/in.h>
+#include <poll.h>
+#include <signal.h>
+#include <stdbool.h>
+#include <stdint.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+#include <sys/socket.h>
+#include <time.h>
+#include <unistd.h>
+
+#ifndef UDP_SEGMENT
+#define UDP_SEGMENT	103
+#endif
+#ifndef SOL_UDP
+#define SOL_UDP		17
+#endif
+
+#define MAX_DGRAM	65507
+#define AMT_MSG_MCAST_DATA	6
+#define AMT_UDP_PORT_HDR	8
+
+struct chunk_hdr {
+	uint32_t seq;
+	uint16_t idx;
+	uint16_t len;
+};
+
+struct cfg {
+	bool ipv6;
+	bool gso;
+	bool ignore_out;	/* sniff: skip frames sent by this host */
+	bool only_out;		/* sniff: skip frames received */
+	bool amt;		/* sniff: count AMT multicast data messages */
+	const char *ifname;
+	const char *group;
+	const char *bind_addr;
+	unsigned int port;
+	unsigned int seg;
+	unsigned int cnt;
+	unsigned int tail;
+	unsigned int num;
+	unsigned int delay_us;
+	unsigned int idle;
+	unsigned int grace;	/* recv: extra wait for the first datagram */
+	unsigned int mtu;
+};
+
+static void die(const char *what)
+{
+	fprintf(stderr, "amt_gso: %s: %s\n", what, strerror(errno));
+	exit(2);
+}
+
+static uint8_t fill_byte(uint32_t seq, unsigned int idx, unsigned int off)
+{
+	return (uint8_t)(seq * 131 + idx * 17 + off * 7 + (off >> 8));
+}
+
+static unsigned int chunk_len(const struct cfg *c, unsigned int idx)
+{
+	return idx < c->cnt ? c->seg : c->tail;
+}
+
+static unsigned int chunks_per_dgram(const struct cfg *c)
+{
+	return c->cnt + (c->tail ? 1 : 0);
+}
+
+static void fill_dgram(const struct cfg *c, uint32_t seq, uint8_t *buf)
+{
+	unsigned int i, off, n = chunks_per_dgram(c);
+
+	for (i = 0; i < n; i++) {
+		unsigned int len = chunk_len(c, i);
+		struct chunk_hdr h = { .seq = seq, .idx = i, .len = len };
+
+		memcpy(buf, &h, sizeof(h));
+		for (off = sizeof(h); off < len; off++)
+			buf[off] = fill_byte(seq, i, off);
+		buf += len;
+	}
+}
+
+static int open_udp(const struct cfg *c)
+{
+	int fd = socket(c->ipv6 ? AF_INET6 : AF_INET, SOCK_DGRAM, 0);
+
+	if (fd < 0)
+		die("socket");
+	return fd;
+}
+
+static void fill_addr(const struct cfg *c, const char *str, unsigned int port,
+		      struct sockaddr_storage *ss, socklen_t *len)
+{
+	memset(ss, 0, sizeof(*ss));
+	if (c->ipv6) {
+		struct sockaddr_in6 *a = (void *)ss;
+
+		a->sin6_family = AF_INET6;
+		a->sin6_port = htons(port);
+		if (inet_pton(AF_INET6, str, &a->sin6_addr) != 1)
+			die("inet_pton");
+		*len = sizeof(*a);
+	} else {
+		struct sockaddr_in *a = (void *)ss;
+
+		a->sin_family = AF_INET;
+		a->sin_port = htons(port);
+		if (inet_pton(AF_INET, str, &a->sin_addr) != 1)
+			die("inet_pton");
+		*len = sizeof(*a);
+	}
+}
+
+static int do_send(const struct cfg *c)
+{
+	static uint8_t buf[MAX_DGRAM];
+	struct sockaddr_storage dst, src;
+	socklen_t dlen, slen;
+	unsigned int total, ifindex, i;
+	int fd, ttl = 8, zero = 0;
+
+	total = c->cnt * c->seg + c->tail;
+	if (total > MAX_DGRAM || (c->tail && c->tail < sizeof(struct chunk_hdr)) ||
+	    c->seg < sizeof(struct chunk_hdr)) {
+		fprintf(stderr, "amt_gso: bad sizes\n");
+		return 2;
+	}
+	ifindex = if_nametoindex(c->ifname);
+	if (!ifindex)
+		die("if_nametoindex");
+
+	fd = open_udp(c);
+	if (c->bind_addr) {
+		fill_addr(c, c->bind_addr, 0, &src, &slen);
+		if (bind(fd, (void *)&src, slen))
+			die("bind");
+	}
+	if (c->ipv6) {
+		if (setsockopt(fd, IPPROTO_IPV6, IPV6_MULTICAST_IF, &ifindex,
+			       sizeof(ifindex)))
+			die("IPV6_MULTICAST_IF");
+		if (setsockopt(fd, IPPROTO_IPV6, IPV6_MULTICAST_HOPS, &ttl,
+			       sizeof(ttl)))
+			die("IPV6_MULTICAST_HOPS");
+		if (setsockopt(fd, IPPROTO_IPV6, IPV6_MULTICAST_LOOP, &zero,
+			       sizeof(zero)))
+			die("IPV6_MULTICAST_LOOP");
+	} else {
+		struct ip_mreqn mr = { .imr_ifindex = ifindex };
+
+		if (setsockopt(fd, IPPROTO_IP, IP_MULTICAST_IF, &mr, sizeof(mr)))
+			die("IP_MULTICAST_IF");
+		if (setsockopt(fd, IPPROTO_IP, IP_MULTICAST_TTL, &ttl,
+			       sizeof(ttl)))
+			die("IP_MULTICAST_TTL");
+		if (setsockopt(fd, IPPROTO_IP, IP_MULTICAST_LOOP, &zero,
+			       sizeof(zero)))
+			die("IP_MULTICAST_LOOP");
+	}
+	if (c->gso) {
+		int gso = c->seg;
+
+		if (setsockopt(fd, SOL_UDP, UDP_SEGMENT, &gso, sizeof(gso)))
+			die("UDP_SEGMENT");
+	}
+	fill_addr(c, c->group, c->port, &dst, &dlen);
+
+	for (i = 0; i < c->num; i++) {
+		fill_dgram(c, i, buf);
+		if (sendto(fd, buf, total, 0, (void *)&dst, dlen) != (ssize_t)total)
+			die("sendto");
+		if (c->delay_us)
+			usleep(c->delay_us);
+	}
+	close(fd);
+	printf("SENT datagrams=%u bytes_each=%u gso=%d\n", c->num, total, c->gso);
+	return 0;
+}
+
+static int do_recv(const struct cfg *c)
+{
+	unsigned int per = chunks_per_dgram(c), expected = c->num * per;
+	unsigned int good = 0, bad_len = 0, bad_payload = 0, dup = 0, unk = 0;
+	static uint8_t buf[MAX_DGRAM + 1];
+	struct sockaddr_storage any;
+	socklen_t alen;
+	uint8_t *seen;
+	int fd, rcvbuf = 8 << 20, idle_ms = 0;
+	bool got_any = false;
+
+	seen = calloc(expected ? expected : 1, 1);
+	if (!seen)
+		die("calloc");
+	fd = open_udp(c);
+	setsockopt(fd, SOL_SOCKET, SO_RCVBUF, &rcvbuf, sizeof(rcvbuf));
+	fill_addr(c, c->ipv6 ? "::" : "0.0.0.0", c->port, &any, &alen);
+	if (bind(fd, (void *)&any, alen))
+		die("bind");
+	printf("READY\n");
+	fflush(stdout);
+
+	while (good < expected) {
+		struct pollfd pfd = { .fd = fd, .events = POLLIN };
+		struct chunk_hdr h;
+		unsigned int off;
+		ssize_t n;
+		int r = poll(&pfd, 1, 100);
+
+		if (r < 0)
+			die("poll");
+		if (!r) {
+			idle_ms += 100;
+			if (idle_ms >= (int)(c->idle + (got_any ? 0 : c->grace)) * 1000)
+				break;
+			continue;
+		}
+		idle_ms = 0;
+		got_any = true;
+		n = recv(fd, buf, sizeof(buf), 0);
+		if (n < 0)
+			die("recv");
+		if (n < (ssize_t)sizeof(h)) {
+			bad_len++;
+			continue;
+		}
+		memcpy(&h, buf, sizeof(h));
+		if (h.seq >= c->num || h.idx >= per) {
+			unk++;
+			continue;
+		}
+		if ((unsigned int)n != chunk_len(c, h.idx) || h.len != n) {
+			bad_len++;
+			continue;
+		}
+		for (off = sizeof(h); off < (unsigned int)n; off++)
+			if (buf[off] != fill_byte(h.seq, h.idx, off))
+				break;
+		if (off != (unsigned int)n) {
+			bad_payload++;
+			continue;
+		}
+		if (seen[h.seq * per + h.idx]++) {
+			dup++;
+			continue;
+		}
+		good++;
+	}
+	printf("RECV expected=%u good=%u bad_len=%u bad_payload=%u dup=%u unknown=%u\n",
+	       expected, good, bad_len, bad_payload, dup, unk);
+	free(seen);
+	return good == expected && !bad_len && !bad_payload && !dup && !unk ? 0 : 1;
+}
+
+static int sniff_stop;
+
+static void sniff_sig(int sig)
+{
+	__atomic_store_n(&sniff_stop, 1, __ATOMIC_RELAXED);
+}
+
+static int do_sniff(const struct cfg *c)
+{
+	static uint8_t buf[MAX_DGRAM + 256];
+	unsigned int n = 0, max_len = 0, over = 0, amt = 0;
+	struct sockaddr_ll sll = { .sll_family = AF_PACKET };
+	struct timespec t0, last, now;
+	struct tpacket_stats st = { 0 };
+	socklen_t stlen = sizeof(st);
+	int fd, rcvbuf = 32 << 20;
+
+	fd = socket(AF_PACKET, SOCK_DGRAM, htons(ETH_P_ALL));
+	if (fd < 0)
+		die("socket(AF_PACKET)");
+	sll.sll_protocol = htons(ETH_P_ALL);
+	sll.sll_ifindex = if_nametoindex(c->ifname);
+	if (!sll.sll_ifindex)
+		die("if_nametoindex");
+	if (bind(fd, (void *)&sll, sizeof(sll)))
+		die("bind(AF_PACKET)");
+	/* Do not lose frames to a full receive queue: the verdict counts them */
+	if (setsockopt(fd, SOL_SOCKET, SO_RCVBUFFORCE, &rcvbuf, sizeof(rcvbuf)))
+		setsockopt(fd, SOL_SOCKET, SO_RCVBUF, &rcvbuf, sizeof(rcvbuf));
+	signal(SIGTERM, sniff_sig);
+	printf("READY\n");
+	fflush(stdout);
+	clock_gettime(CLOCK_MONOTONIC, &t0);
+	last = t0;
+
+	for (;;) {
+		/* Once asked to stop, read what is still queued, then report */
+		int stopping = __atomic_load_n(&sniff_stop, __ATOMIC_RELAXED);
+		struct pollfd pfd = { .fd = fd, .events = POLLIN };
+		struct sockaddr_ll from;
+		socklen_t flen = sizeof(from);
+		unsigned int hl, dport;
+		const uint8_t *p = buf;
+		ssize_t len, got;
+		int r;
+
+		clock_gettime(CLOCK_MONOTONIC, &now);
+		if (!stopping && (now.tv_sec - t0.tv_sec > 120 ||
+				  now.tv_sec - last.tv_sec >= (long)c->idle))
+			break;
+		r = poll(&pfd, 1, stopping ? 0 : 100);
+		if (r < 0 && errno == EINTR)
+			continue;
+		if (r < 0)
+			die("poll");
+		if (!r) {
+			if (stopping)
+				break;
+			continue;
+		}
+		got = recvfrom(fd, buf, sizeof(buf), MSG_TRUNC,
+			       (void *)&from, &flen);
+		if (got < 0)
+			die("recvfrom");
+		len = got;
+		if (from.sll_pkttype == PACKET_OUTGOING ? c->ignore_out : c->only_out)
+			continue;
+		if (got > (ssize_t)sizeof(buf))
+			got = sizeof(buf);
+
+		if (ntohs(from.sll_protocol) == ETH_P_IP && got >= 28 &&
+		    (p[0] >> 4) == 4 && p[9] == IPPROTO_UDP) {
+			hl = (p[0] & 0xf) * 4;
+		} else if (ntohs(from.sll_protocol) == ETH_P_IPV6 && got >= 48 &&
+			   (p[0] >> 4) == 6 && p[6] == IPPROTO_UDP) {
+			hl = 40;
+		} else {
+			continue;
+		}
+		if (got < (ssize_t)hl + 8)
+			continue;
+		dport = (p[hl + 2] << 8) | p[hl + 3];
+		if (dport != c->port)
+			continue;
+		if (c->amt) {
+			const uint8_t *a = p + hl + AMT_UDP_PORT_HDR;
+
+			/* type in the low nibble, version 0 in the high one,
+			 * then one reserved byte, then the IP packet.
+			 */
+			if (got < (ssize_t)hl + 8 + 3 ||
+			    a[0] != AMT_MSG_MCAST_DATA || a[1] != 0)
+				continue;
+			if ((a[2] >> 4) != (c->ipv6 ? 6 : 4))
+				continue;
+			amt++;
+		}
+		last = now;
+		n++;
+		if ((unsigned int)len > max_len)
+			max_len = len;
+		if ((unsigned int)len > c->mtu)
+			over++;
+	}
+	getsockopt(fd, SOL_PACKET, PACKET_STATISTICS, &st, &stlen);
+	printf("SNIFF frames=%u max_len=%u over_mtu=%u amt_data=%u lost=%u\n",
+	       n, max_len, over, amt, st.tp_drops);
+	return 0;
+}
+
+static void usage(void)
+{
+	fprintf(stderr,
+		"amt_gso send|recv|sniff [-6] [-I ifname] [-g group] [-b srcaddr]\n"
+		"           [-p port] [-s seg] [-c chunks] [-t tail] [-n datagrams]\n"
+		"           [-G] [-d delay_us] [-T idle_s] [-S grace_s] [-M mtu] [-a] [-i] [-o]\n");
+	exit(2);
+}
+
+int main(int argc, char **argv)
+{
+	struct cfg c = { .port = 4000, .seg = 1200, .cnt = 8, .tail = 100,
+			 .num = 1, .idle = 2, .mtu = 1500 };
+	int opt;
+
+	if (argc < 2)
+		usage();
+	optind = 2;
+	while ((opt = getopt(argc, argv, "6I:g:b:p:s:c:t:n:Gd:T:S:M:aio")) != -1) {
+		switch (opt) {
+		case '6':
+			c.ipv6 = true;
+			break;
+		case 'I':
+			c.ifname = optarg;
+			break;
+		case 'g':
+			c.group = optarg;
+			break;
+		case 'b':
+			c.bind_addr = optarg;
+			break;
+		case 'p':
+			c.port = atoi(optarg);
+			break;
+		case 's':
+			c.seg = atoi(optarg);
+			break;
+		case 'c':
+			c.cnt = atoi(optarg);
+			break;
+		case 't':
+			c.tail = atoi(optarg);
+			break;
+		case 'n':
+			c.num = atoi(optarg);
+			break;
+		case 'G':
+			c.gso = true;
+			break;
+		case 'd':
+			c.delay_us = atoi(optarg);
+			break;
+		case 'S':
+			c.grace = atoi(optarg);
+			break;
+		case 'T':
+			c.idle = atoi(optarg);
+			break;
+		case 'M':
+			c.mtu = atoi(optarg);
+			break;
+		case 'a':
+			c.amt = true;
+			break;
+		case 'i':
+			c.ignore_out = true;
+			break;
+		case 'o':
+			c.only_out = true;
+			break;
+		default:
+			usage();
+		}
+	}
+	if (!strcmp(argv[1], "send") && c.ifname && c.group)
+		return do_send(&c);
+	if (!strcmp(argv[1], "recv"))
+		return do_recv(&c);
+	if (!strcmp(argv[1], "sniff") && c.ifname)
+		return do_sniff(&c);
+	usage();
+	return 2;
+}
diff --git a/tools/testing/selftests/net/amt_gso.sh b/tools/testing/selftests/net/amt_gso.sh
new file mode 100755
index 000000000..8b32f17b6
--- /dev/null
+++ b/tools/testing/selftests/net/amt_gso.sh
@@ -0,0 +1,269 @@
+#!/bin/bash
+# SPDX-License-Identifier: GPL-2.0
+#
+# Check that an AMT relay forwards a UDP GSO burst (UDP_SEGMENT) to a gateway
+# when transmit checksum offload is enabled on the amt device.
+#
+# With the default features the core segments the burst before it reaches
+# amt_dev_xmit(), so that case is the positive control for the harness. With
+# "tx on" the unsegmented GSO skb is handed to the driver, which must mark it
+# as a UDP tunnel packet before it hands it to the lower device.
+#
+# There are three network namespaces. The sender runs in the RELAY namespace,
+# because a forwarded GRO skb with DF set would be dropped by the multicast
+# router before it ever gets to amt.
+#
+#   LISTENER             GATEWAY                RELAY
+#  +---------+        +-------------+       +-----------------+
+#  |  l_gw   |--------| gw_l  br0   |       |                 |
+#  |         |        |       amtg  |       |  amtr  <- sender|
+#  +---------+        |  gw_relay   |-------| relay_gw        |
+#                     +-------------+       +-----------------+
+#
+# amt_gso, built from amt_gso.c, sends, receives and captures. The listener
+# reports datagrams by count, length and payload. A capture on amtr shows what
+# was handed to amt_dev_xmit(), and one on gw_relay shows what was put on the
+# wire.
+
+source lib.sh
+
+AMT_GSO=./amt_gso
+GRP4=239.0.0.1
+GRP6=ff0e::5:6
+SRC4=192.0.2.1
+SRC6=2001:db8:3::1
+PORT_AMT=2268
+SEG=1200
+TAIL=100
+BURST=8
+NUM=100
+PROBE_OPTS="-s 64 -c 1 -t 0 -n 1"
+
+TMPD=$(mktemp -d)
+
+cleanup()
+{
+	rm -rf "$TMPD"
+	cleanup_all_ns
+}
+
+trap cleanup EXIT
+
+dev_stat()
+{
+	ip netns exec "$1" cat "/sys/class/net/$2/statistics/$3"
+}
+
+udp_mib()
+{
+	ip netns exec "$1" awk -v f="$2" '
+		/^Udp:/ { if (!h) { split($0, k); h = 1 }
+			  else for (i = 2; i <= NF; i++) if (k[i] == f) print $i }
+	' /proc/net/snmp
+}
+
+field()
+{
+	grep -o " $2=[0-9]*" "$1" | tail -n 1 | cut -d= -f2
+}
+
+setup_topology()
+{
+	setup_ns LISTENER GATEWAY RELAY || exit $ksft_skip
+
+	ip link add l_gw netns "$LISTENER" type veth peer name gw_l \
+		netns "$GATEWAY"
+	ip link add gw_relay netns "$GATEWAY" type veth peer name relay_gw \
+		netns "$RELAY"
+
+	ip -n "$LISTENER" link set l_gw up
+	ip -n "$LISTENER" addr add 192.168.0.2/24 dev l_gw
+	ip -n "$LISTENER" addr add 2001:db8::2/64 dev l_gw nodad
+	ip -n "$LISTENER" route add default via 192.168.0.1 dev l_gw
+	ip -n "$LISTENER" addr add "$GRP4"/32 dev l_gw autojoin
+	ip -n "$LISTENER" addr add "$GRP6"/128 dev l_gw autojoin
+
+	ip -n "$GATEWAY" link set gw_l up
+	ip -n "$GATEWAY" link set gw_relay up
+	ip -n "$GATEWAY" addr add 192.168.0.1/24 dev gw_l
+	ip -n "$GATEWAY" addr add 2001:db8::1/64 dev gw_l nodad
+	ip -n "$GATEWAY" addr add 10.0.0.1/24 dev gw_relay
+	ip -n "$GATEWAY" link add br0 type bridge
+	ip -n "$GATEWAY" link set br0 up
+	ip -n "$GATEWAY" link set gw_l master br0
+	ip -n "$GATEWAY" link add amtg master br0 type amt mode gateway \
+		local 10.0.0.1 discovery 10.0.0.2 dev gw_relay \
+		gateway_port $PORT_AMT relay_port $PORT_AMT || exit $ksft_skip
+
+	ip -n "$RELAY" link set relay_gw up
+	ip -n "$RELAY" addr add 10.0.0.2/24 dev relay_gw
+	ip -n "$RELAY" link add amtr type amt mode relay local 10.0.0.2 \
+		dev relay_gw relay_port $PORT_AMT max_tunnels 4 || exit $ksft_skip
+	ip -n "$RELAY" addr add "$SRC4"/32 dev amtr
+	ip -n "$RELAY" addr add "$SRC6"/128 dev amtr nodad
+	ip -n "$RELAY" link set amtr up
+	ip -n "$GATEWAY" link set amtg up
+
+	# Segment and checksum in software on the relay's egress, so that the
+	# frames on the wire are at most one MTU and carry a final checksum
+	# whatever the veth can do.
+	ip netns exec "$RELAY" ethtool -K relay_gw tx off >/dev/null ||
+		exit $ksft_skip
+
+	AMTR_MTU=$(ip netns exec "$RELAY" cat /sys/class/net/amtr/mtu)
+}
+
+# Send one single-datagram probe every second until the listener sees it,
+# which means that discovery, request and update are done for this group.
+wait_tunnel()
+{
+	local fam=$1 v6="" grp=$GRP4 src=$SRC4 port=4999 i
+
+	[ "$fam" = 6 ] && { v6=-6; grp=$GRP6; src=$SRC6; port=6999; }
+
+	for i in $(seq 40); do
+		ip netns exec "$LISTENER" $AMT_GSO recv $v6 -p $port \
+			$PROBE_OPTS -T 1 >"$TMPD/probe.out" &
+		local pid=$!
+		busywait 5000 grep -q READY "$TMPD/probe.out"
+		ip netns exec "$RELAY" $AMT_GSO send $v6 -I amtr -g $grp \
+			-b $src -p $port $PROBE_OPTS >/dev/null
+		if wait $pid; then
+			return 0
+		fi
+	done
+	return 1
+}
+
+# run_burst <4|6> <gso 0|1>: send NUM datagrams and capture. Sets RECV_RC.
+run_burst()
+{
+	local fam=$1 gso=$2 v6="" grp=$GRP4 src=$SRC4 port=4000 cnt=$BURST
+	local tail=$TAIL gsoopt="" pid_r pid_a pid_g f
+
+	[ "$fam" = 6 ] && { v6=-6; grp=$GRP6; src=$SRC6; port=6000; }
+	if [ "$gso" = 1 ]; then
+		gsoopt=-G
+	else
+		cnt=1
+		tail=0
+	fi
+	local opts="-s $SEG -c $cnt -t $tail -n $NUM"
+
+	rm -f "$TMPD"/{recv,amtr,gw,send}.out
+
+	ip netns exec "$LISTENER" $AMT_GSO recv $v6 -p $port $opts -T 3 -S 15 \
+		>"$TMPD/recv.out" &
+	pid_r=$!
+	ip netns exec "$RELAY" $AMT_GSO sniff -I amtr -p $port -M "$AMTR_MTU" \
+		-o -T 20 >"$TMPD/amtr.out" &
+	pid_a=$!
+	ip netns exec "$GATEWAY" $AMT_GSO sniff -I gw_relay -p $PORT_AMT \
+		-M 1500 -i -a $v6 -T 20 >"$TMPD/gw.out" &
+	pid_g=$!
+	for f in recv amtr gw; do
+		busywait 5000 grep -q READY "$TMPD/$f.out"
+	done
+
+	DROP0=$(dev_stat "$RELAY" relay_gw tx_dropped)
+	TXP0=$(dev_stat "$RELAY" relay_gw tx_packets)
+	CSUM0=$(udp_mib "$GATEWAY" InCsumErrors)
+	GRX0=$(dev_stat "$GATEWAY" amtg rx_packets)
+	LRX0=$(dev_stat "$LISTENER" l_gw rx_packets)
+	GRXD0=$(dev_stat "$GATEWAY" amtg rx_dropped)
+
+	ip netns exec "$RELAY" $AMT_GSO send $v6 -I amtr -g $grp -b $src \
+		-p $port $opts $gsoopt -d 2000 >"$TMPD/send.out"
+
+	wait $pid_r
+	RECV_RC=$?
+	# The receiver is done, so every frame the captures will see is already
+	# queued on their sockets. Ask them to drain it and report.
+	kill -TERM $pid_a $pid_g
+	wait $pid_a $pid_g
+
+	DROPS=$(($(dev_stat "$RELAY" relay_gw tx_dropped) - DROP0))
+	TXP=$(($(dev_stat "$RELAY" relay_gw tx_packets) - TXP0))
+	CSUMERR=$(($(udp_mib "$GATEWAY" InCsumErrors) - CSUM0))
+	GRX=$(($(dev_stat "$GATEWAY" amtg rx_packets) - GRX0))
+	LRX=$(($(dev_stat "$LISTENER" l_gw rx_packets) - LRX0))
+	GRXD=$(($(dev_stat "$GATEWAY" amtg rx_dropped) - GRXD0))
+	EXPECT=$((NUM * (cnt + (tail ? 1 : 0))))
+}
+
+# run_case <name> <4|6> <tx on|off> <gso 0|1>
+run_case()
+{
+	local name=$1 fam=$2 tx=$3 gso=$4 big gwn stats
+
+	RET=0
+	retmsg=
+	ip netns exec "$RELAY" ethtool -K amtr tx "$tx" >/dev/null
+	if [ "$tx" = on ] && ! ip netns exec "$RELAY" ethtool -k amtr |
+	   grep -q '^tx-udp-segmentation: on'; then
+		log_test_skip "$name" "amtr cannot take tx-udp-segmentation"
+		return
+	fi
+
+	run_burst "$fam" "$gso"
+	big=$(field "$TMPD/amtr.out" over_mtu)
+
+	gwn=$(field "$TMPD/gw.out" amt_data)
+	log_info "$name: $(grep RECV "$TMPD/recv.out")"
+	log_info "$name: amtr tx $(grep SNIFF "$TMPD/amtr.out")"
+	log_info "$name: wire $(grep SNIFF "$TMPD/gw.out")"
+	stats="relay_gw tx +$TXP drop +$DROPS; gw csum_err +$CSUMERR"
+	stats="$stats; amtg rx +$GRX drop +$GRXD; listener rx +$LRX"
+	log_info "$name: $stats"
+
+	check_err $(($(field "$TMPD/amtr.out" lost) + \
+		     $(field "$TMPD/gw.out" lost) != 0)) \
+		"a packet capture lost frames, the verdict is unreliable"
+	check_err $RECV_RC "listener did not get every datagram intact"
+	check_err $((DROPS != 0)) "relay_gw dropped $DROPS packets"
+	check_err $((CSUMERR != 0)) "gateway counted $CSUMERR csum errors"
+	check_err $((gwn != EXPECT)) \
+		"gateway saw $gwn AMT data messages, expected $EXPECT"
+	check_err $(($(field "$TMPD/gw.out" over_mtu) != 0)) \
+		"frames larger than the MTU were put on the wire"
+
+	if [ "$tx" = on ] && [ "$gso" = 1 ]; then
+		# Without this the case proves nothing about the driver.
+		if [ "$big" = 0 ] && [ $RET -eq 0 ]; then
+			RET=$ksft_skip
+			retmsg="no GSO skb reached amt_dev_xmit()"
+		fi
+	else
+		check_err $((big != 0)) \
+			"a frame larger than the MTU reached amt_dev_xmit()"
+	fi
+
+	log_test "$name"
+}
+
+require_command ip
+require_command ethtool
+if [ ! -x $AMT_GSO ]; then
+	log_test_skip "amt_gso helper not built"
+	exit $EXIT_STATUS
+fi
+ip link help 2>&1 | grep -q amt || {
+	log_test_skip "iproute2 without amt support"
+	exit $EXIT_STATUS
+}
+
+setup_topology
+
+wait_tunnel 4 || { log_test_skip "IPv4 AMT tunnel did not come up"
+		   exit $EXIT_STATUS; }
+wait_tunnel 6 || { log_test_skip "IPv6 AMT tunnel did not come up"
+		   exit $EXIT_STATUS; }
+
+run_case "IPv4 UDP_SEGMENT burst, amt tx offload off (control)" 4 off 1
+run_case "IPv6 UDP_SEGMENT burst, amt tx offload off (control)" 6 off 1
+run_case "IPv4 UDP_SEGMENT burst, amt tx offload on" 4 on 1
+run_case "IPv6 UDP_SEGMENT burst, amt tx offload on" 6 on 1
+run_case "IPv4 plain datagrams, amt tx offload on" 4 on 0
+run_case "IPv6 plain datagrams, amt tx offload on" 6 on 0
+
+exit $EXIT_STATUS
-- 
2.43.0


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH net-next 1/2] amt: mark relay data as a UDP tunnel packet before sending it
  2026-10-01 17:10 ` [PATCH net-next 1/2] amt: mark relay data as a UDP tunnel packet before sending it Omar Ramadan
@ 2026-10-01 17:33   ` Eric Dumazet
  2026-10-01 18:26     ` Omar Ramadan
  0 siblings, 1 reply; 5+ messages in thread
From: Eric Dumazet @ 2026-10-01 17:33 UTC (permalink / raw)
  To: Omar Ramadan
  Cc: Taehee Yoo, Andrew Lunn, David S . Miller, Jakub Kicinski,
	Paolo Abeni, Shuah Khan, Simon Horman, netdev, linux-kselftest,
	linux-kernel

On Thu, Oct 1, 2026 at 7:10 PM Omar Ramadan <omar@blockcast.net> wrote:
>
> amt_send_multicast_data() copies the multicast packet, puts an AMT
> multicast data header and a UDP header in front of it, and sends it with
> udp_tunnel_xmit_skb(). Unlike the other UDP tunnels, it never calls
> udp_tunnel_handle_offloads(), so the copy has neither skb->encapsulation
> nor an SKB_GSO_UDP_TUNNEL* bit set. If the copy is a GSO skb, the lower
> layers see a plain UDP_L4 skb that has an outer UDP header in front of
> it.
>
> A GSO skb only reaches amt_dev_xmit() when tx checksum offload has been
> turned on for the amt device (it is off by default, in which case the
> core segments the packet before ndo_start_xmit), for example with a
> UDP_SEGMENT sender on the relay. The default configuration is not
> affected.
>
> Call udp_tunnel_handle_offloads() on the copy, as bareudp and geneve do.
> The AMT header and the UDP header are pushed after that. Two details
> need care:
>
>  - udp_csum is true, because udp_tunnel_xmit_skb() is called with
>    nocheck set to false. The GSO checksum of the outer UDP header is
>    then completed for every segment, which needs
>    SKB_GSO_UDP_TUNNEL_CSUM.
>
>  - amt is ARPHRD_ETHER (amt_link_setup() ends with ether_setup()), and
>    amt_dev_xmit() pulls the Ethernet header without moving the mac
>    header. skb_copy_expand() keeps the mac header relative to the data,
>    so, going by the code, in the copy it should sit 14 bytes before the
>    inner IP header. The tunnel segmentation derives the length of the
>    outer headers from inner_mac_header - transport_header, which would
>    then be negative. I did not measure either value; what was observed
>    is described below. Reset the mac header on the copy before the
>    inner headers are recorded, so that inner_mac_header is the inner IP
>    header, as it is for the other tunnels that have no link-layer
>    header.
>
> The call also changes what a plain, non-GSO datagram looks like when it
> leaves amt. iptunnel_handle_offloads() sets skb->encapsulation on every
> skb and clears it again only if ip_summed is not CHECKSUM_PARTIAL. A
> CHECKSUM_PARTIAL datagram, which is what the stack hands to the driver
> with tx checksum offload on, now has encapsulation set where it had
> none before, so netif_skb_features() limits the features available for
> it to those in hw_enc_features, and udp_set_csum() takes the local
> checksum offload branch, which leaves the inner checksum to the lower
> device. The other UDP tunnels do the same, but the selftest does not
> cover hardware checksumming of such a packet: the egress device in it
> has tx offload off, so skb_checksum_help() completes the checksum in
> software.
>
> This follows the suggestion made by Eric Dumazet on the earlier
> [PATCH net] "amt: do not offer software GSO on the amt device", which
> this replaces.
>
> The problem was found by an LLM-assisted code review of
> drivers/net/amt.c while developing an IPv6 outer transport for amt.
>
> Tested with the selftest in the next patch, in a KVM guest running
> net-next at commit eb0c18404c89 ("amt: pull the AMT header behind the
> transport header in amt_parse_type()"), x86_64, CONFIG_DEBUG_NET=y, AMT
> built in, eleven runs per kernel of the final selftest (22 guest boots,
> two at a time on a busy host). See the next patch for how stable the
> selftest itself has been. The sender is a local UDP_SEGMENT burst
> of eight 1200-byte segments plus a 100-byte tail, 100 bursts, IPv4 and
> IPv6 inner traffic, with "ethtool -K <amt relay dev> tx on" and the
> relay's egress device doing its segmentation in software:
>
>  - Without this patch, every GSO skb (9728 bytes for IPv4, 9748 for
>    IPv6) reached amt_dev_xmit(), none of the 900 datagrams arrived at
>    the listener, the tx_dropped counter of the relay's egress device
>    went up by 100 (one per GSO skb), and nothing was put on the wire.
>    A function-graph trace of one run showed __skb_gso_segment() on that
>    device failing with -EINVAL, from __udp_gso_segment() under
>    udp4_ufo_fragment(), and the skb being freed in validate_xmit_skb().
>
>  - With this patch, all 900 datagrams arrived intact in every run, one
>    AMT message per segment was seen on the wire, none of them larger
>    than the MTU, tx_dropped did not move, and the gateway counted no UDP
>    checksum errors. The trace showed skb_udp_tunnel_segment() doing the
>    outer segmentation.
>
>  - With this patch minus the skb_reset_mac_header() call (two runs, with
>    an earlier version of the selftest), the packets were dropped in the
>    same way as without the patch, and DEBUG_NET warned in
>    skb_udp_tunnel_segment() (pskb_may_pull() with a length above
>    INT_MAX). That fits a negative header length, but the value itself
>    was not printed.
>
>  - With tx offload off (the default), and with non-GSO datagrams with tx
>    on, everything arrived with and without the patch.
>
>  - The existing tools/testing/selftests/net/amt.sh passes with the patch
>    (discovery, IPv4 and IPv6 forwarding, and both torture tests).
>
> Not tested: hardware that offloads UDP tunnel segmentation or the
> checksum of a CHECKSUM_PARTIAL packet, a forwarded UDP GRO packet as the
> GSO source, NETIF_F_GSO_FRAGLIST, KASAN, and the udp_csum=false variant,
> so the choice of true rests on reading the code and on the patched runs
> above being clean, not on a failing false variant. sparse was not run,
> and the existing amt.sh was run only with the patch, not on the
> unpatched kernel. Only the IPv4 outer transport exists in this tree.
>
> Assisted-by: LLM
> Signed-off-by: Omar Ramadan <omar@blockcast.net>
> ---
>  drivers/net/amt.c | 11 +++++++++++
>  1 file changed, 11 insertions(+)
>
> diff --git a/drivers/net/amt.c b/drivers/net/amt.c
> index 0277e4cac..1f0afc11e 100644
> --- a/drivers/net/amt.c
> +++ b/drivers/net/amt.c
> @@ -1078,7 +1078,18 @@ static void amt_send_multicast_data(struct amt_dev *amt,
>         if (!skb)
>                 return;
>
> +       /* amt_dev_xmit() pulled the Ethernet header without moving the mac
> +        * header, so the copy's mac header sits 14 bytes before the inner IP
> +        * header. Make it coincide with it, as the inner segmentation code
> +        * expects for a device without a link-layer header.
> +        */
> +       skb_reset_mac_header(skb);
>         skb_reset_inner_headers(skb);
> +       if (udp_tunnel_handle_offloads(skb, true)) {
> +               kfree_skb(skb);
> +               return;
> +       }
> +

Okay, but the changelog is far far too long.

Changelogs are for humans (LLM do not care much) and should be shorter.
Also note the use of the 'Suggested-by:' tag.

Something like:

amt_send_multicast_data() encapsulates the multicast packet in
AMT + UDP + IP headers, but unlike other UDP tunnels it never calls
udp_tunnel_handle_offloads().

If tx checksum offload is enabled on the amt device (off by default),
GSO packets (e.g. from a UDP_SEGMENT sender) reach amt_dev_xmit()
unsegmented. They are then sent with neither skb->encapsulation nor
SKB_GSO_UDP_TUNNEL_CSUM set, and the lower device drops them:
__udp_gso_segment() fails because csum_start does not match the
(outer) transport header.

Call udp_tunnel_handle_offloads(skb, true), as other UDP tunnels do.
udp_csum is true because udp_tunnel_xmit_skb() is called with
nocheck == false.

Also reset the mac header of the copy before recording the inner
headers. amt_dev_xmit() pulled the Ethernet header without moving
mac_header, so inner_mac_header would point 14 bytes before the
inner IP header. That makes tnl_hlen negative in
__skb_udp_tunnel_segment().


Suggested-by: Eric Dumazet <edumazet@kernel.org>
Assisted-by: LLM
Signed-off-by: Omar Ramadan <omar@blockcast.net>
---
Testing notes (selftest results, untested configurations, ...) go here.

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH net-next 1/2] amt: mark relay data as a UDP tunnel packet before sending it
  2026-10-01 17:33   ` Eric Dumazet
@ 2026-10-01 18:26     ` Omar Ramadan
  0 siblings, 0 replies; 5+ messages in thread
From: Omar Ramadan @ 2026-10-01 18:26 UTC (permalink / raw)
  To: Eric Dumazet
  Cc: Taehee Yoo, Andrew Lunn, David S. Miller, Jakub Kicinski,
	Paolo Abeni, Simon Horman, Shuah Khan, netdev, linux-kselftest,
	linux-kernel

On Thu, Oct 1, 2026, Eric Dumazet wrote:
> Okay, but the changelog is far far too long.

Thanks, and sorry about the length. I'll use your changelog, add the
Suggested-by tag, and move the testing notes below the "---".

The netdev CI also flagged the selftest in patch 2 (shellcheck, and lines
over 80 columns). I'll fix those in v2 and shorten the patch 2 and cover
letter text the same way, and wait 24 hours for more comments first.

pw-bot: cr

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2026-10-01 18:27 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-10-01 17:10 [PATCH net-next 0/2] amt: mark relay data as a UDP tunnel packet, with a selftest Omar Ramadan
2026-10-01 17:10 ` [PATCH net-next 1/2] amt: mark relay data as a UDP tunnel packet before sending it Omar Ramadan
2026-10-01 17:33   ` Eric Dumazet
2026-10-01 18:26     ` Omar Ramadan
2026-10-01 17:10 ` [PATCH net-next 2/2] selftests: net: add an amt test for UDP_SEGMENT through the relay Omar Ramadan

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®