[PATCH net 0/2] net: keep the TCP fixed-ID flags on the header they describe

[PATCH net 0/2] net: keep the TCP fixed-ID flags on the header they describe

[Julius Bairaktaris]

GRO sets SKB_GSO_TCP_FIXEDID for the outer IPv4 header of a TCP packet
whose IDs do not change, and SKB_GSO_TCP_FIXEDID_INNER for the inner
header of an encapsulated one, since commit 3271f19bf7b9 ("net: gso:
restore ids of outer ip headers correctly"). Encapsulation,
decapsulation and the GRE and UDP tunnel segmentation paths do not keep
the flags on the header they describe, so a TCP stream that GRO merged
and that crosses a tunnel boundary is segmented with the fixed-ID state
of the wrong header. Sashiko found this in review of an ipmr series:
https://lore.kernel.org/netdev/cover.1792293419.git.julius@bairaktaris.de/

Patch 1 moves the flag on encapsulation and decapsulation and keeps the
encapsulation level below GRE and UDP tunnels. The three changes are one
patch because each alone loses fixed IDs in a case that works today.
Patch 2 adds a selftest.

Tested in QEMU (KASAN) with the selftest, 17 cases, 400 segments each:
- net (7076f684d513): 5 cases fail: fixed inner IDs incremented after
  IPIP over IPv4 and IPv6 (40 distinct inner IDs), outer IDs repeated
  after GRE and VXLAN over IPv4 (10 distinct outer IDs for 400
  segments), and a VXLAN stream with fixed outer IDs whose outer state
  survives decapsulation into IPIP. The same 5 failed in 3 runs of an
  earlier version of the test.
- this series: 17 of 17 pass, 3 of 3 runs.
- with only the decapsulation part, or only the encapsulation and
  segmentation parts: 8 and 5 cases fail, among them every
  encapsulation followed by decapsulation without segmentation, which
  passes on net.
- fou_gre_gro.sh and bareudp.sh pass on net and with the series;
  gre_gso.sh and udpgro_fwd.sh (3 paired runs) show the same failures
  on both, which come from the slow emulated machine.

I used Claude (Anthropic) to help write these patches.

Julius Bairaktaris (2):
  net: keep the TCP fixed-ID flags on the header they describe
  selftests: net: check TCP IP IDs across tunnels after GRO and GSO

 include/linux/skbuff.h                     |  30 ++
 include/net/ip_tunnels.h                   |   2 +
 net/core/filter.c                          |  11 +-
 net/core/lwt_bpf.c                         |   1 +
 net/ipv4/gre_offload.c                     |   1 -
 net/ipv4/ip_tunnel_core.c                  |   5 +-
 net/ipv4/udp_offload.c                     |   1 -
 net/ipv6/exthdrs.c                         |  11 +-
 tools/testing/selftests/net/.gitignore     |   1 +
 tools/testing/selftests/net/Makefile       |   2 +
 tools/testing/selftests/net/tcp_fixedid.c  | 396 +++++++++++++++++++++
 tools/testing/selftests/net/tcp_fixedid.sh | 291 +++++++++++++++
 12 files changed, 744 insertions(+), 8 deletions(-)
 create mode 100644 tools/testing/selftests/net/tcp_fixedid.c
 create mode 100755 tools/testing/selftests/net/tcp_fixedid.sh


base-commit: cb284914e749388eef402938bdb3115fe1f6d337
-- 
2.53.0

[PATCH net 1/2] net: keep the TCP fixed-ID flags on the header they describe

[Julius Bairaktaris]

GRO sets SKB_GSO_TCP_FIXEDID for the outer IPv4 header of a TCP packet
and SKB_GSO_TCP_FIXEDID_INNER for the inner header of an encapsulated
one, and segmentation reads them the same way. Three paths do not keep
the flags on the header they describe:

- Encapsulation: iptunnel_handle_offloads(), bpf_skb_net_grow() and
  bpf_lwt_push_ip_encap() make the IPv4 header of a GSO packet the
  inner one but leave SKB_GSO_TCP_FIXEDID. A TCP stream that GRO merged
  with fixed IDs and that is forwarded into a tunnel gets one outer ID
  for all segments, also with DF clear, and incrementing inner IDs.
- Decapsulation: iptunnel_pull_offloads(), bpf_skb_net_shrink() and
  the segment routing decapsulation in ipv6_srh_rcv() and
  ipv6_rpl_srh_rcv() leave both flags, so the new outer header is
  segmented with the state of the removed one: inner IDs that
  incremented come out fixed when the outer IDs were fixed, and the
  reverse.
- Segmentation below GRE and UDP tunnels: inet_gso_segment() tells the
  inner header from the outer one by SKB_GSO_CB(skb)->encap_level,
  which gre_gso_segment() and __skb_udp_tunnel_segment() reset before
  they segment the inner packet. The inner IPv4 header of a GRE, VXLAN,
  GENEVE, FOU, GUE or bareudp packet is segmented with the outer flag.

Add skb_gso_fixedid_encap() and skb_gso_fixedid_decap(), which move the
flag to the inner or the outer bit, and call them where a GSO packet
becomes encapsulated or loses its outer header. The segment routing
paths use iptunnel_pull_offloads() for this, which also clears the
tunnel GSO types of the removed header. Keep encap_level below GRE and
UDP tunnels, as ipip_gso_segment() and the IPv6 tunnel handlers do; the
inner pass then also masks the features with hw_enc_features, which the
tunnel handlers have done already, and resets the inner headers of each
segment.

The three changes go together: with any one of them alone, fixed IDs
are lost in a case that works today, such as a packet encapsulated and
decapsulated again without segmentation, or one forwarded into an IPv6
GRE tunnel.

Fixes: 3271f19bf7b9 ("net: gso: restore ids of outer ip headers correctly")
Signed-off-by: Julius Bairaktaris <julius@bairaktaris.de>
---
 include/linux/skbuff.h    | 30 ++++++++++++++++++++++++++++++
 include/net/ip_tunnels.h  |  2 ++
 net/core/filter.c         | 11 ++++++++---
 net/core/lwt_bpf.c        |  1 +
 net/ipv4/gre_offload.c    |  1 -
 net/ipv4/ip_tunnel_core.c |  5 ++++-
 net/ipv4/udp_offload.c    |  1 -
 net/ipv6/exthdrs.c        | 11 +++++++++--
 8 files changed, 54 insertions(+), 8 deletions(-)

diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
index 6787fcf6ac3e4..00df57cbc05c2 100644
--- a/include/linux/skbuff.h
+++ b/include/linux/skbuff.h
@@ -5337,6 +5337,36 @@ static inline void skb_decrease_gso_size(struct skb_shared_info *shinfo,
 	shinfo->gso_size -= decrement;
 }
 
+/**
+ * skb_gso_fixedid_encap - move the TCP fixed-ID flag to the inner header
+ * @shinfo: shared info of a GSO packet that becomes encapsulated
+ *
+ * The IPv4 header that SKB_GSO_TCP_FIXEDID describes becomes the inner one,
+ * and the new outer header gets an incrementing ID.
+ */
+static inline void skb_gso_fixedid_encap(struct skb_shared_info *shinfo)
+{
+	unsigned int fixedid = shinfo->gso_type & SKB_GSO_TCP_FIXEDID;
+
+	shinfo->gso_type &= ~(SKB_GSO_TCP_FIXEDID | SKB_GSO_TCP_FIXEDID_INNER);
+	shinfo->gso_type |= fixedid << 1;
+}
+
+/**
+ * skb_gso_fixedid_decap - move the TCP fixed-ID flag to the new outer header
+ * @shinfo: shared info of an encapsulated GSO packet losing its outer header
+ *
+ * The inner IPv4 header, which SKB_GSO_TCP_FIXEDID_INNER describes, becomes
+ * the outer one, and the state of the removed outer header is dropped.
+ */
+static inline void skb_gso_fixedid_decap(struct skb_shared_info *shinfo)
+{
+	unsigned int fixedid = shinfo->gso_type & SKB_GSO_TCP_FIXEDID_INNER;
+
+	shinfo->gso_type &= ~(SKB_GSO_TCP_FIXEDID | SKB_GSO_TCP_FIXEDID_INNER);
+	shinfo->gso_type |= fixedid >> 1;
+}
+
 void __skb_warn_lro_forwarding(const struct sk_buff *skb);
 
 static __always_inline bool skb_warn_if_lro(const struct sk_buff *skb)
diff --git a/include/net/ip_tunnels.h b/include/net/ip_tunnels.h
index 5cccf4c0e6913..c2ffda6029a06 100644
--- a/include/net/ip_tunnels.h
+++ b/include/net/ip_tunnels.h
@@ -665,6 +665,8 @@ static inline int iptunnel_pull_offloads(struct sk_buff *skb)
 		err = skb_unclone(skb, GFP_ATOMIC);
 		if (unlikely(err))
 			return err;
+		if (skb->encapsulation)
+			skb_gso_fixedid_decap(skb_shinfo(skb));
 		skb_shinfo(skb)->gso_type &= ~(NETIF_F_GSO_ENCAP_ALL >>
 					       NETIF_F_GSO_SHIFT);
 	}
diff --git a/net/core/filter.c b/net/core/filter.c
index 70dc621672f2e..70f41e7597bf3 100644
--- a/net/core/filter.c
+++ b/net/core/filter.c
@@ -3691,6 +3691,9 @@ static int bpf_skb_net_grow(struct sk_buff *skb, u32 off, u32 len_diff,
 	if (skb_is_gso(skb)) {
 		struct skb_shared_info *shinfo = skb_shinfo(skb);
 
+		if (encap)
+			skb_gso_fixedid_encap(shinfo);
+
 		/* Header must be checked, and gso_segs recomputed. */
 		shinfo->gso_type |= gso_type;
 		shinfo->gso_segs = 0;
@@ -3782,9 +3785,11 @@ static int bpf_skb_net_shrink(struct sk_buff *skb, u32 off, u32 len_diff,
 						  SKB_GSO_GRE_CSUM |
 						  SKB_GSO_IPXIP4 |
 						  SKB_GSO_IPXIP6 |
-						  SKB_GSO_ESP)))
-				if (skb->encapsulation)
-					skb->encapsulation = 0;
+						  SKB_GSO_ESP)) &&
+			    skb->encapsulation) {
+				skb_gso_fixedid_decap(shinfo);
+				skb->encapsulation = 0;
+			}
 		}
 
 		/* Header must be checked, and gso_segs recomputed. */
diff --git a/net/core/lwt_bpf.c b/net/core/lwt_bpf.c
index da49364ec63de..4d9faf2cc5de5 100644
--- a/net/core/lwt_bpf.c
+++ b/net/core/lwt_bpf.c
@@ -539,6 +539,7 @@ static int handle_gso_type(struct sk_buff *skb, unsigned int gso_type,
 {
 	struct skb_shared_info *shinfo = skb_shinfo(skb);
 
+	skb_gso_fixedid_encap(shinfo);
 	gso_type |= SKB_GSO_DODGY;
 	shinfo->gso_type |= gso_type;
 	skb_decrease_gso_size(shinfo, encap_len);
diff --git a/net/ipv4/gre_offload.c b/net/ipv4/gre_offload.c
index c8129782d39de..a8f9a0a0b06bd 100644
--- a/net/ipv4/gre_offload.c
+++ b/net/ipv4/gre_offload.c
@@ -35,7 +35,6 @@ static struct sk_buff *gre_gso_segment(struct sk_buff *skb,
 
 	/* setup inner skb. */
 	skb->encapsulation = 0;
-	SKB_GSO_CB(skb)->encap_level = 0;
 	__skb_pull(skb, tnl_hlen);
 	skb_reset_mac_header(skb);
 	skb_set_network_header(skb, skb_inner_network_offset(skb));
diff --git a/net/ipv4/ip_tunnel_core.c b/net/ipv4/ip_tunnel_core.c
index 6c2855adff289..59050229e3499 100644
--- a/net/ipv4/ip_tunnel_core.c
+++ b/net/ipv4/ip_tunnel_core.c
@@ -182,9 +182,10 @@ EXPORT_SYMBOL_GPL(iptunnel_metadata_reply);
 int iptunnel_handle_offloads(struct sk_buff *skb,
 			     int gso_type_mask)
 {
+	bool encap = !skb->encapsulation;
 	int err;
 
-	if (likely(!skb->encapsulation)) {
+	if (likely(encap)) {
 		skb_reset_inner_headers(skb);
 		skb->encapsulation = 1;
 	}
@@ -193,6 +194,8 @@ int iptunnel_handle_offloads(struct sk_buff *skb,
 		err = skb_header_unclone(skb, GFP_ATOMIC);
 		if (unlikely(err))
 			return err;
+		if (encap)
+			skb_gso_fixedid_encap(skb_shinfo(skb));
 		skb_shinfo(skb)->gso_type |= gso_type_mask;
 		return 0;
 	}
diff --git a/net/ipv4/udp_offload.c b/net/ipv4/udp_offload.c
index cf07c3c6611a2..edcabe1115a60 100644
--- a/net/ipv4/udp_offload.c
+++ b/net/ipv4/udp_offload.c
@@ -205,7 +205,6 @@ static struct sk_buff *__skb_udp_tunnel_segment(struct sk_buff *skb,
 
 	/* setup inner skb. */
 	skb->encapsulation = 0;
-	SKB_GSO_CB(skb)->encap_level = 0;
 	__skb_pull(skb, tnl_hlen);
 	skb_reset_mac_header(skb);
 	skb_set_network_header(skb, skb_inner_network_offset(skb));
diff --git a/net/ipv6/exthdrs.c b/net/ipv6/exthdrs.c
index 96c8402a27a54..126058772bc5a 100644
--- a/net/ipv6/exthdrs.c
+++ b/net/ipv6/exthdrs.c
@@ -53,6 +53,7 @@
 #include <linux/ioam6_genl.h>
 #include <net/ioam6.h>
 #include <net/dst_metadata.h>
+#include <net/ip_tunnels.h>
 
 #include <linux/uaccess.h>
 
@@ -406,7 +407,10 @@ static int ipv6_srh_rcv(struct sk_buff *skb, struct inet6_dev *idev)
 
 			skb_reset_network_header(skb);
 			skb_reset_transport_header(skb);
-			skb->encapsulation = 0;
+			if (iptunnel_pull_offloads(skb)) {
+				kfree_skb(skb);
+				return -1;
+			}
 			if (hdr->nexthdr == NEXTHDR_IPV4)
 				skb->protocol = htons(ETH_P_IP);
 			__skb_tunnel_rx(skb, skb->dev, net);
@@ -513,7 +517,10 @@ static int ipv6_rpl_srh_rcv(struct sk_buff *skb, struct inet6_dev *idev)
 
 			skb_reset_network_header(skb);
 			skb_reset_transport_header(skb);
-			skb->encapsulation = 0;
+			if (iptunnel_pull_offloads(skb)) {
+				kfree_skb(skb);
+				return -1;
+			}
 
 			__skb_tunnel_rx(skb, skb->dev, net);
 
-- 
2.53.0

[PATCH net 2/2] selftests: net: check TCP IP IDs across tunnels after GRO and GSO

[Julius Bairaktaris]

Send TCP segments with fixed or incrementing IPv4 IDs from a packet
socket, let GRO on a veth merge them, forward the merged packets into
IPIP, GRE and VXLAN tunnels over IPv4 and IPv6 with segmentation done in
software, and check that the inner IDs come out as they went in and the
IDs of a new outer IPv4 header are unique. Also forward a VXLAN stream
with one header's IDs fixed after decapsulation into IPIP, and send the
merged packets through a tunnel that the far end decapsulates without
segmenting them before it forwards them.

A test where GRO merged nothing is skipped.

Signed-off-by: Julius Bairaktaris <julius@bairaktaris.de>
---
 tools/testing/selftests/net/.gitignore     |   1 +
 tools/testing/selftests/net/Makefile       |   2 +
 tools/testing/selftests/net/tcp_fixedid.c  | 396 +++++++++++++++++++++
 tools/testing/selftests/net/tcp_fixedid.sh | 291 +++++++++++++++
 4 files changed, 690 insertions(+)
 create mode 100644 tools/testing/selftests/net/tcp_fixedid.c
 create mode 100755 tools/testing/selftests/net/tcp_fixedid.sh

diff --git a/tools/testing/selftests/net/.gitignore b/tools/testing/selftests/net/.gitignore
index 21412510ac26b..96263d5978258 100644
--- a/tools/testing/selftests/net/.gitignore
+++ b/tools/testing/selftests/net/.gitignore
@@ -46,6 +46,7 @@ so_rcv_listener
 stress_reuseport_listen
 tap
 tcp_fastopen_backup_key
+tcp_fixedid
 tcp_inq
 tcp_mmap
 tcp_port_share
diff --git a/tools/testing/selftests/net/Makefile b/tools/testing/selftests/net/Makefile
index 9180b9bec5155..dd402814bc748 100644
--- a/tools/testing/selftests/net/Makefile
+++ b/tools/testing/selftests/net/Makefile
@@ -106,6 +106,7 @@ TEST_PROGS := \
 	srv6_iptunnel_cache.sh \
 	stress_reuseport_listen.sh \
 	tcp_ecmp_failover.sh \
+	tcp_fixedid.sh \
 	tcp_fastopen_backup_key.sh \
 	test_bridge_backup_port.sh \
 	test_bridge_neigh_suppress.sh \
@@ -169,6 +170,7 @@ TEST_GEN_FILES := \
 	socket \
 	stress_reuseport_listen \
 	tcp_fastopen_backup_key \
+	tcp_fixedid \
 	tcp_inq \
 	tcp_mmap \
 	tfo \
diff --git a/tools/testing/selftests/net/tcp_fixedid.c b/tools/testing/selftests/net/tcp_fixedid.c
new file mode 100644
index 0000000000000..d507d12b46308
--- /dev/null
+++ b/tools/testing/selftests/net/tcp_fixedid.c
@@ -0,0 +1,396 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Send TCP segments with fixed or incrementing IPv4 IDs, optionally in
+ * VXLAN, and report the IP IDs of TCP segments received on a device.
+ *
+ * tx: tcp_fixedid -T -i dev -M dmac -s saddr -d daddr [-f] [-n segs]
+ *                 [-b bursts] [-l len] [-V osaddr,odaddr -m inner_dmac [-F]]
+ * rx: tcp_fixedid -R -i dev [-c segs] [-w ms]
+ *
+ * The receiver prints "ready" once it is bound, then the number of
+ * segments, of segments with an outer IPv4 header, and of distinct outer
+ * and inner IDs.
+ */
+
+#include <arpa/inet.h>
+#include <errno.h>
+#include <error.h>
+#include <linux/if_packet.h>
+#include <net/ethernet.h>
+#include <net/if.h>
+#include <netinet/ip.h>
+#include <netinet/ip6.h>
+#include <netinet/tcp.h>
+#include <netinet/udp.h>
+#include <poll.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>
+
+#define TEST_PORT	9000
+#define VXLAN_PORT	4789
+#define FIXED_ID	0x1234
+
+static const char *ifname;
+static bool fixed_id, fixed_outer;
+static int segs = 40, bursts = 10, len = 1000, wait_ms = 2000, stop;
+static struct in_addr saddr, daddr, osaddr, odaddr;
+static bool vxlan;
+static unsigned char dmac[ETH_ALEN], inner_dmac[ETH_ALEN];
+
+static void parse_mac(const char *s, unsigned char *mac)
+{
+	if (sscanf(s, "%hhx:%hhx:%hhx:%hhx:%hhx:%hhx", &mac[0], &mac[1],
+		   &mac[2], &mac[3], &mac[4], &mac[5]) != 6)
+		error(1, 0, "bad mac %s", s);
+}
+
+static void parse_addr(const char *s, struct in_addr *a)
+{
+	if (inet_pton(AF_INET, s, a) != 1)
+		error(1, 0, "bad address %s", s);
+}
+
+static uint32_t csum_add(uint32_t sum, const void *data, size_t n)
+{
+	const uint8_t *p = data;
+
+	while (n > 1) {
+		sum += (p[0] << 8) | p[1];
+		p += 2;
+		n -= 2;
+	}
+	if (n)
+		sum += p[0] << 8;
+	return sum;
+}
+
+static uint16_t csum_fold(uint32_t sum)
+{
+	while (sum >> 16)
+		sum = (sum & 0xffff) + (sum >> 16);
+	return htons(~sum & 0xffff);
+}
+
+static void build_ip(struct iphdr *iph, int proto, int payload, uint16_t id,
+		     struct in_addr s, struct in_addr d)
+{
+	memset(iph, 0, sizeof(*iph));
+	iph->version = 4;
+	iph->ihl = 5;
+	iph->tot_len = htons(sizeof(*iph) + payload);
+	iph->id = htons(id);
+	iph->frag_off = htons(IP_DF);
+	iph->ttl = 64;
+	iph->protocol = proto;
+	iph->saddr = s.s_addr;
+	iph->daddr = d.s_addr;
+	iph->check = csum_fold(csum_add(0, iph, sizeof(*iph)));
+}
+
+/* Ethernet, IPv4 and TCP header plus payload for segment @i at @p */
+static int build_tcp(unsigned char *p, const unsigned char *mac, int i,
+		     uint16_t id)
+{
+	struct ethhdr *eth = (struct ethhdr *)p;
+	struct iphdr *iph = (struct iphdr *)(eth + 1);
+	struct tcphdr *th = (struct tcphdr *)(iph + 1);
+	struct {
+		uint32_t s, d;
+		uint8_t zero, proto;
+		uint16_t len;
+	} ph;	/* 12 bytes, no padding */
+	uint32_t sum;
+
+	memcpy(eth->h_dest, mac, ETH_ALEN);
+	memset(eth->h_source, 0, ETH_ALEN);
+	eth->h_source[0] = 0x02;
+	eth->h_source[5] = 0x01;
+	eth->h_proto = htons(ETH_P_IP);
+	build_ip(iph, IPPROTO_TCP, sizeof(*th) + len, id, saddr, daddr);
+
+	memset(th, 0, sizeof(*th));
+	th->source = htons(TEST_PORT + 1);
+	th->dest = htons(TEST_PORT);
+	th->seq = htonl(1 + i * len);
+	th->ack_seq = htonl(1);
+	th->doff = 5;
+	th->ack = 1;
+	th->window = htons(65535);
+	memset(th + 1, 'a' + i % 26, len);
+
+	ph.s = iph->saddr;
+	ph.d = iph->daddr;
+	ph.zero = 0;
+	ph.proto = IPPROTO_TCP;
+	ph.len = htons(sizeof(*th) + len);
+	sum = csum_add(0, &ph, sizeof(ph));
+	th->check = csum_fold(csum_add(sum, th, sizeof(*th) + len));
+
+	return sizeof(*eth) + sizeof(*iph) + sizeof(*th) + len;
+}
+
+static int do_tx(int fd, struct sockaddr_ll *sll)
+{
+	unsigned char buf[ETH_FRAME_LEN + 128];
+	int b, s, i = 0, n;
+
+	for (b = 0; b < bursts; b++) {
+		for (s = 0; s < segs; s++, i++) {
+			uint16_t id = fixed_id ? FIXED_ID : 0x1000 + i;
+			unsigned char *p = buf;
+
+			if (vxlan) {
+				struct ethhdr *eth = (struct ethhdr *)p;
+				struct iphdr *iph = (struct iphdr *)(eth + 1);
+				struct udphdr *uh = (struct udphdr *)(iph + 1);
+				uint32_t *vx = (uint32_t *)(uh + 1);
+				int inner;
+
+				inner = build_tcp((unsigned char *)(vx + 2),
+						  inner_dmac, i, id);
+				memcpy(eth->h_dest, dmac, ETH_ALEN);
+				memset(eth->h_source, 0, ETH_ALEN);
+				eth->h_source[0] = 0x02;
+				eth->h_source[5] = 0x01;
+				eth->h_proto = htons(ETH_P_IP);
+				memset(uh, 0, sizeof(*uh));
+				uh->source = htons(VXLAN_PORT + 1);
+				uh->dest = htons(VXLAN_PORT);
+				uh->len = htons(sizeof(*uh) + 8 + inner);
+				vx[0] = htonl(0x08000000);
+				vx[1] = htonl(42 << 8);
+				build_ip(iph, IPPROTO_UDP,
+					 sizeof(*uh) + 8 + inner,
+					 fixed_outer ? FIXED_ID : 0x2000 + i,
+					 osaddr, odaddr);
+				n = sizeof(*eth) + sizeof(*iph) + sizeof(*uh) +
+				    8 + inner;
+			} else {
+				n = build_tcp(p, dmac, i, id);
+			}
+
+			if (sendto(fd, buf, n, 0, (struct sockaddr *)sll,
+				   sizeof(*sll)) != n)
+				error(1, errno, "sendto");
+		}
+		usleep(50000);
+	}
+	return 0;
+}
+
+/* Return the inner IPv4 header of a TCP segment to TEST_PORT, or NULL */
+static const struct iphdr *find_inner(const unsigned char *p, int n,
+				      const struct iphdr **outer4)
+{
+	const struct ethhdr *eth = (const struct ethhdr *)p;
+	const unsigned char *end = p + n;
+	const unsigned char *l4;
+	int proto, depth;
+
+	*outer4 = NULL;
+	if (n < (int)sizeof(*eth))
+		return NULL;
+	p += sizeof(*eth);
+
+	for (depth = 0; depth < 2; depth++) {
+		uint16_t etype = depth ? htons(ETH_P_IP) : eth->h_proto;
+
+		if (etype == htons(ETH_P_IP)) {
+			const struct iphdr *iph = (const struct iphdr *)p;
+
+			if (p + sizeof(*iph) > end)
+				return NULL;
+			proto = iph->protocol;
+			l4 = p + iph->ihl * 4;
+			if (proto == IPPROTO_TCP) {
+				const struct tcphdr *th;
+
+				th = (const struct tcphdr *)l4;
+				if (l4 + sizeof(*th) > end ||
+				    th->dest != htons(TEST_PORT))
+					return NULL;
+				return iph;
+			}
+			if (depth)
+				return NULL;
+			*outer4 = iph;
+		} else if (etype == htons(ETH_P_IPV6)) {
+			const struct ip6_hdr *ip6h = (const struct ip6_hdr *)p;
+
+			if (p + sizeof(*ip6h) > end)
+				return NULL;
+			proto = ip6h->ip6_nxt;
+			l4 = p + sizeof(*ip6h);
+		} else {
+			return NULL;
+		}
+
+		/* step over the tunnel header to the inner IPv4 header */
+		if (proto == IPPROTO_IPIP) {
+			p = l4;
+		} else if (proto == IPPROTO_GRE) {
+			uint16_t flags;
+
+			if (l4 + 4 > end)
+				return NULL;
+			flags = (l4[0] << 8) | l4[1];
+			if (((l4[2] << 8) | l4[3]) != ETH_P_IP)
+				return NULL;
+			p = l4 + 4 + 4 * (!!(flags & 0x8000) +
+					  !!(flags & 0x2000) +
+					  !!(flags & 0x1000));
+		} else if (proto == IPPROTO_UDP) {
+			const struct udphdr *uh = (const struct udphdr *)l4;
+
+			if (l4 + sizeof(*uh) + 8 + sizeof(*eth) > end ||
+			    uh->dest != htons(VXLAN_PORT))
+				return NULL;
+			p = l4 + sizeof(*uh) + 8 + sizeof(*eth);
+		} else {
+			return NULL;
+		}
+	}
+	return NULL;
+}
+
+static void count_id(uint8_t *map, unsigned int *distinct, uint16_t id)
+{
+	if (!(map[id / 8] & (1 << (id % 8)))) {
+		map[id / 8] |= 1 << (id % 8);
+		(*distinct)++;
+	}
+}
+
+static int do_rx(int fd)
+{
+	static uint8_t outer_map[8192], inner_map[8192];
+	unsigned int frames = 0, outer4 = 0, outer_ids = 0, inner_ids = 0;
+	unsigned char buf[65536];
+	struct pollfd pfd = { .fd = fd, .events = POLLIN };
+
+	printf("ready\n");
+	fflush(stdout);
+
+	/* wait longer for the first segment than between segments */
+	while (poll(&pfd, 1, frames ? wait_ms : 10 * wait_ms) > 0) {
+		const struct iphdr *iph, *oiph;
+		struct sockaddr_ll from;
+		socklen_t fl = sizeof(from);
+		int n;
+
+		n = recvfrom(fd, buf, sizeof(buf), 0, (struct sockaddr *)&from,
+			     &fl);
+		if (n < 0)
+			error(1, errno, "recvfrom");
+		if (from.sll_pkttype == PACKET_OUTGOING)
+			continue;
+		iph = find_inner(buf, n, &oiph);
+		if (!iph)
+			continue;
+		frames++;
+		count_id(inner_map, &inner_ids, ntohs(iph->id));
+		if (oiph) {
+			outer4++;
+			count_id(outer_map, &outer_ids, ntohs(oiph->id));
+		}
+		if (stop && frames == (unsigned int)stop)
+			break;
+	}
+
+	printf("frames %u outer4 %u outer_ids %u inner_ids %u\n", frames,
+	       outer4, outer_ids, inner_ids);
+	return 0;
+}
+
+int main(int argc, char **argv)
+{
+	struct sockaddr_ll sll = {};
+	bool tx = false, rx = false;
+	char *comma;
+	int c, fd;
+
+	while ((c = getopt(argc, argv, "TRi:M:s:d:fn:b:l:V:m:Fw:c:")) != -1) {
+		switch (c) {
+		case 'T':
+			tx = true;
+			break;
+		case 'R':
+			rx = true;
+			break;
+		case 'i':
+			ifname = optarg;
+			break;
+		case 'M':
+			parse_mac(optarg, dmac);
+			break;
+		case 's':
+			parse_addr(optarg, &saddr);
+			break;
+		case 'd':
+			parse_addr(optarg, &daddr);
+			break;
+		case 'f':
+			fixed_id = true;
+			break;
+		case 'n':
+			segs = atoi(optarg);
+			break;
+		case 'b':
+			bursts = atoi(optarg);
+			break;
+		case 'l':
+			len = atoi(optarg);
+			break;
+		case 'V':
+			comma = strchr(optarg, ',');
+			if (!comma)
+				error(1, 0, "-V needs outer saddr,daddr");
+			*comma = '\0';
+			parse_addr(optarg, &osaddr);
+			parse_addr(comma + 1, &odaddr);
+			vxlan = true;
+			break;
+		case 'm':
+			parse_mac(optarg, inner_dmac);
+			break;
+		case 'F':
+			fixed_outer = true;
+			break;
+		case 'w':
+			wait_ms = atoi(optarg);
+			break;
+		case 'c':
+			stop = atoi(optarg);
+			break;
+		default:
+			error(1, 0, "bad option");
+		}
+	}
+	if (tx == rx || !ifname || len < 1 || len > 1400 || segs < 1 ||
+	    bursts < 1)
+		error(1, 0, "usage: see the comment at the top of %s",
+		      __FILE__);
+
+	fd = socket(AF_PACKET, SOCK_RAW, htons(tx ? 0 : ETH_P_ALL));
+	if (fd < 0)
+		error(1, errno, "socket");
+	sll.sll_family = AF_PACKET;
+	sll.sll_protocol = htons(ETH_P_ALL);
+	sll.sll_ifindex = if_nametoindex(ifname);
+	if (!sll.sll_ifindex)
+		error(1, errno, "%s", ifname);
+	if (bind(fd, (struct sockaddr *)&sll, sizeof(sll)))
+		error(1, errno, "bind");
+	sll.sll_halen = ETH_ALEN;
+	memcpy(sll.sll_addr, dmac, ETH_ALEN);
+
+	c = tx ? do_tx(fd, &sll) : do_rx(fd);
+	close(fd);
+	return c;
+}
diff --git a/tools/testing/selftests/net/tcp_fixedid.sh b/tools/testing/selftests/net/tcp_fixedid.sh
new file mode 100755
index 0000000000000..88f9298174077
--- /dev/null
+++ b/tools/testing/selftests/net/tcp_fixedid.sh
@@ -0,0 +1,291 @@
+#!/bin/bash
+# SPDX-License-Identifier: GPL-2.0
+#
+# Check the IPv4 IDs of a TCP stream that GRO merged and software
+# segmentation split again across a tunnel. A sender in NSRC writes
+# segments with fixed or incrementing IDs onto a veth with GRO on in
+# NRTR, which forwards the merged packets:
+#
+#  encap:       into an IPIP, GRE or VXLAN tunnel over IPv4 or IPv6,
+#               segmented on the underlay towards NMID
+#  decap:       out of VXLAN, received from NSRC with one header fixed and
+#               the other incrementing, into an IPIP tunnel towards NMID
+#  encap+decap: into a tunnel that NMID ends without segmenting, and on
+#               to NDST, segmented there
+#
+# The inner IDs must come out as they went in, and the IDs of a new outer
+# IPv4 header must be unique. Packets that GRO did not merge are not
+# tested, so a run where nothing was merged is skipped.
+
+source lib.sh
+
+NSRC=""
+NRTR=""
+NMID=""
+NDST=""
+
+SEGS=40
+BURSTS=10
+TOTAL=$((SEGS * BURSTS))
+INNER_SRC=10.1.0.1
+INNER_DST=10.9.0.2
+VX_MAC=02:00:00:00:00:42
+TUN_MAC=02:00:00:00:00:99
+MID_MAC=02:00:00:00:00:98
+TX=./tcp_fixedid
+
+NOSEG="tso gso tx-gre-segmentation tx-gre-csum-segmentation
+       tx-ipxip4-segmentation tx-ipxip6-segmentation
+       tx-udp_tnl-segmentation tx-udp_tnl-csum-segmentation"
+
+cleanup() {
+	cleanup_all_ns
+}
+
+trap cleanup EXIT
+
+# NSRC s0 - r0 NRTR r1 - m1 NMID m2 - d2 NDST
+setup() {
+	setup_ns NSRC NRTR NMID NDST || return "$ksft_skip"
+
+	ip link add s0 netns "$NSRC" type veth peer name r0 netns "$NRTR"
+	ip link add r1 netns "$NRTR" type veth peer name m1 netns "$NMID"
+	ip link add m2 netns "$NMID" type veth peer name d2 netns "$NDST"
+
+	ip -n "$NRTR" addr add 10.1.0.2/24 dev r0
+	ip -n "$NRTR" addr add 10.2.0.1/24 dev r1
+	ip -n "$NRTR" addr add fd02::1/64 dev r1 nodad
+	ip -n "$NMID" addr add 10.2.0.2/24 dev m1
+	ip -n "$NMID" addr add fd02::2/64 dev m1 nodad
+	ip -n "$NMID" addr add 10.4.0.1/24 dev m2
+	ip -n "$NDST" addr add 10.4.0.2/24 dev d2
+	ip -n "$NSRC" link set s0 up
+	ip -n "$NRTR" link set r0 up
+	ip -n "$NRTR" link set r1 up
+	ip -n "$NMID" link set m1 up
+	ip -n "$NMID" link set m2 up
+	ip -n "$NDST" link set d2 up
+
+	ip netns exec "$NRTR" sysctl -qw net.ipv4.ip_forward=1
+	ip netns exec "$NMID" sysctl -qw net.ipv4.ip_forward=1
+	for ns in "$NRTR" "$NMID"; do
+		ip netns exec "$ns" sysctl -qw net.ipv4.conf.all.rp_filter=0 \
+			net.ipv4.conf.default.rp_filter=0
+	done
+
+	# veth passes packets to GRO only if the sender does not do TSO
+	ip netns exec "$NRTR" ethtool -K r0 gro on >/dev/null ||
+		return "$ksft_skip"
+	ip netns exec "$NSRC" ethtool -K s0 tso off >/dev/null ||
+		return "$ksft_skip"
+	# release the sender's segments in bursts for GRO to merge
+	tc -n "$NSRC" qdisc add dev s0 root netem slot 20ms 20ms \
+		packets "$SEGS" || return "$ksft_skip"
+}
+
+# tunnel <ns> <name> <type> <family> <local> <remote>
+tunnel() {
+	local ns=$1 name=$2 type=$3 family=$4 local=$5 remote=$6
+
+	case "$type" in
+	ipip)
+		if [ "$family" = 4 ]; then
+			ip -n "$ns" link add "$name" type ipip \
+				local "$local" remote "$remote"
+		else
+			ip -n "$ns" link add "$name" type ip6tnl mode ipip6 \
+				local "$local" remote "$remote" encaplimit none
+		fi ;;
+	gre)
+		if [ "$family" = 4 ]; then
+			ip -n "$ns" link add "$name" type gre \
+				local "$local" remote "$remote"
+		else
+			ip -n "$ns" link add "$name" type ip6gre \
+				local "$local" remote "$remote" encaplimit none
+		fi ;;
+	vxlan)
+		ip -n "$ns" link add "$name" address "$TUN_MAC" type vxlan \
+			id 7 dstport 4789 local "$local" remote "$remote" \
+			nolearning ;;
+	esac || return 1
+	ip -n "$ns" link set "$name" up
+}
+
+# route_into <ns> <dev>: send INNER_DST into <dev>
+route_into() {
+	ip -n "$1" route add "$INNER_DST/32" dev "$2"
+	ip -n "$1" neigh add "$INNER_DST" lladdr "$TUN_MAC" dev "$2" \
+		2>/dev/null
+	return 0
+}
+
+# noseg <ns> <dev>: segment everything in software before <dev>
+noseg() {
+	local f
+
+	for f in $NOSEG; do
+		ip netns exec "$1" ethtool -K "$2" "$f" off >/dev/null 2>&1
+	done
+}
+
+tx_packets() {
+	ip netns exec "$1" cat "/sys/class/net/$2/statistics/tx_packets"
+}
+
+# check <name> <result> <want: fixed|incr> <outer: 0|1> <merged-into-dev>
+check() {
+	local name=$1 res=$2 want=$3 outer=$4 sent=$5
+	local frames outer_ids inner_ids
+
+	read -r _ frames _ _ _ outer_ids _ inner_ids <<< "$res"
+	RET=0
+	check_err $((frames != TOTAL)) "$frames of $TOTAL segments received"
+	if [ "$want" = fixed ]; then
+		check_err $((inner_ids != 1)) "$inner_ids inner IDs, want 1"
+	else
+		check_err $((inner_ids != TOTAL)) \
+			"$inner_ids inner IDs, want $TOTAL"
+	fi
+	if [ "$outer" = 1 ]; then
+		check_err $((outer_ids != frames)) \
+			"$outer_ids outer IDs for $frames segments"
+	fi
+	if [ "$RET" -eq 0 ] && [ "$sent" -ge "$TOTAL" ]; then
+		ret_set_ksft_status "$ksft_skip" "GRO did not merge"
+	fi
+	log_test "$name"
+}
+
+# capture <ns> <dev>: start the receiver in the background, pid in CAP_PID,
+# and wait until it is bound
+capture() {
+	local i
+
+	: > "$RESULT"
+	ip netns exec "$1" "$TX" -R -i "$2" -c "$TOTAL" -w 3000 > "$RESULT" &
+	CAP_PID=$!
+	for i in $(seq 100); do
+		grep -q ready "$RESULT" && return
+		sleep 0.1
+	done
+}
+
+run_encap() {
+	local type=$1 family=$2 want=$3
+	local r0mac t0 laddr=10.2.0.1 raddr=10.2.0.2
+	local f=""
+
+	setup || { log_test_skip "encap $type IPv$family"; return; }
+	if [ "$family" = 6 ]; then
+		laddr=fd02::1
+		raddr=fd02::2
+	fi
+	tunnel "$NRTR" tun0 "$type" "$family" "$laddr" "$raddr" ||
+		{ log_test_skip "encap $type IPv$family"; cleanup_all_ns; return; }
+	route_into "$NRTR" tun0
+	noseg "$NRTR" r1
+
+	r0mac=$(ip -n "$NRTR" -br link show r0 | awk '{ print $3 }')
+	capture "$NMID" m1
+	t0=$(tx_packets "$NRTR" tun0)
+	[ "$want" = fixed ] && f=-f
+	ip netns exec "$NSRC" "$TX" -T -i s0 -M "$r0mac" -s "$INNER_SRC" \
+		-d "$INNER_DST" -n "$SEGS" -b "$BURSTS" $f
+	wait "$CAP_PID"
+	check "encap $type IPv$family, $want inner IDs" "$(tail -n 1 "$RESULT")" \
+		"$want" $((family == 4)) $(($(tx_packets "$NRTR" tun0) - t0))
+	cleanup_all_ns
+}
+
+# run_decap <fixed: inner|outer>: VXLAN from NSRC, decapsulated in NRTR
+run_decap() {
+	local fixed=$1 want=incr flags=-F
+	local r0mac t0
+
+	setup || { log_test_skip "decap"; return; }
+	ip -n "$NRTR" link add vx0 address "$VX_MAC" type vxlan id 42 \
+		dstport 4789 local 10.1.0.2 nolearning ||
+		{ log_test_skip "decap"; cleanup_all_ns; return; }
+	ip -n "$NRTR" addr add 10.3.0.2/24 dev vx0
+	ip -n "$NRTR" link set vx0 up
+	tunnel "$NRTR" tun0 ipip 4 10.2.0.1 10.2.0.2
+	route_into "$NRTR" tun0
+	noseg "$NRTR" r1
+
+	if [ "$fixed" = inner ]; then
+		want=fixed
+		flags=-f
+	fi
+	r0mac=$(ip -n "$NRTR" -br link show r0 | awk '{ print $3 }')
+	capture "$NMID" m1
+	t0=$(tx_packets "$NRTR" tun0)
+	ip netns exec "$NSRC" "$TX" -T -i s0 -M "$r0mac" -s 10.3.0.1 \
+		-d "$INNER_DST" -n "$SEGS" -b "$BURSTS" \
+		-V 10.1.0.1,10.1.0.2 -m "$VX_MAC" $flags
+	wait "$CAP_PID"
+	check "decap VXLAN with fixed $fixed IDs, into IPIP" \
+		"$(tail -n 1 "$RESULT")" "$want" 1 \
+		$(($(tx_packets "$NRTR" tun0) - t0))
+	cleanup_all_ns
+}
+
+# run_encap_decap <type>: NRTR encapsulates, NMID decapsulates the GSO
+# packet without segmenting it and forwards it to NDST
+run_encap_decap() {
+	local type=$1
+	local r0mac t0
+
+	setup || { log_test_skip "encap+decap $type"; return; }
+	if ! tunnel "$NRTR" tun0 "$type" 4 10.2.0.1 10.2.0.2 ||
+	   ! tunnel "$NMID" tun0 "$type" 4 10.2.0.2 10.2.0.1; then
+		log_test_skip "encap+decap $type"
+		cleanup_all_ns
+		return
+	fi
+	route_into "$NRTR" tun0
+	# VXLAN drops frames from its own address as loops
+	if [ "$type" = vxlan ]; then
+		ip -n "$NMID" link set tun0 address "$MID_MAC"
+		ip -n "$NRTR" neigh replace "$INNER_DST" lladdr "$MID_MAC" \
+			dev tun0
+	fi
+	ip -n "$NMID" route add "$INNER_DST/32" via 10.4.0.2
+	ip -n "$NDST" addr add "$INNER_DST/32" dev d2
+	noseg "$NMID" m2
+
+	r0mac=$(ip -n "$NRTR" -br link show r0 | awk '{ print $3 }')
+	capture "$NDST" d2
+	t0=$(tx_packets "$NRTR" tun0)
+	ip netns exec "$NSRC" "$TX" -T -i s0 -M "$r0mac" -s "$INNER_SRC" \
+		-d "$INNER_DST" -n "$SEGS" -b "$BURSTS" -f
+	wait "$CAP_PID"
+	check "encap+decap $type, fixed inner IDs" "$(tail -n 1 "$RESULT")" fixed 0 \
+		$(($(tx_packets "$NRTR" tun0) - t0))
+	cleanup_all_ns
+}
+
+require_command ethtool
+require_command tc
+
+if [ ! -x "$TX" ]; then
+	echo "SKIP: $TX not built"
+	exit "$ksft_skip"
+fi
+
+RESULT=$(mktemp)
+
+for type in ipip gre vxlan; do
+	for family in 4 6; do
+		run_encap "$type" "$family" fixed
+		run_encap "$type" "$family" incr
+	done
+done
+run_decap inner
+run_decap outer
+for type in ipip gre vxlan; do
+	run_encap_decap "$type"
+done
+
+rm -f "$RESULT"
+exit "$EXIT_STATUS"
-- 
2.53.0

Re: [PATCH net 1/2] net: keep the TCP fixed-ID flags on the header they describe

[Alexei Starovoitov]

On Sun, Nov 01, 2026 at 08:46 AM Julius Bairaktaris <julius@bairaktaris.de> wrote:
> GRO sets SKB_GSO_TCP_FIXEDID for the outer IPv4 header of a TCP packet
> and SKB_GSO_TCP_FIXEDID_INNER for the inner header of an encapsulated
> one, and segmentation reads them the same way. Three paths do not keep
> the flags on the header they describe:

When the same mistake is made in N places the interface is the problem.

Before commit 3271f19bf7b9 SKB_GSO_TCP_FIXEDID described the innermost
header. Encapsulation didn't have to know about it.
That commit made it describe the outermost header.
Now every site that adds or removes a header has to move the flag.

This patch changes 9 sites and it's not enough.
xfrm_output() sets skb->encapsulation and SKB_GSO_ESP.
xfrm4_tunnel_encap_add() adds the outer IPv4 header.
SKB_GSO_TCP_FIXEDID stays, so ESP tunnel mode has the same bug as IPIP.

> tunnel GSO types of the removed header. Keep encap_level below GRE and
> UDP tunnels, as ipip_gso_segment() and the IPv6 tunnel handlers do; the
> inner pass then also masks the features with hw_enc_features, which the
> tunnel handlers have done already, and resets the inner headers of each
> segment.

This undoes
commit 5197f3499c47 ("net: Reset encap_level to avoid resetting features on inner IP headers").
With remcsum __skb_udp_tunnel_segment() sets NETIF_F_HW_CSUM after its
mask. The mask in inet_gso_segment() clears it again on bnxt and mlx4.
Their hw_enc_features has NETIF_F_IP_CSUM and no NETIF_F_HW_CSUM.
Then skb_segment() checksums every segment in software.

> The three changes go together: with any one of them alone, fixed IDs
> are lost in a case that works today, such as a packet encapsulated and
> decapsulated again without segmentation, or one forwarded into an IPv6
> GRE tunnel.

Lets keep SKB_GSO_TCP_FIXEDID on the innermost header.
The second flag describes the outer header of an encapsulated packet.
Willem's intuition in the review of 3271f19bf7b9 was right:
https://lore.kernel.org/netdev/willemdebruijn.kernel.a8507becb441@gmail.com/

                               this patch          flag on the innermost
                                                   header
  encapsulation                move the flag       nothing to do
  ESP tunnel mode              missed              nothing to do
  decapsulation                move the flag       clear the outer flag
  SRH decapsulation            move the flag       nothing, outer is IPv6
  FOU pulls offloads twice     needs a check of    clears it twice
                               skb->encapsulation
  GRE, UDP tunnel GSO          keep encap_level    as-is
  sfc, mlx5                    as-is               less code

Then no need for skb_gso_fixedid_encap(), skb_gso_fixedid_decap(),
the bool in iptunnel_handle_offloads(), etc.

Something like this (compile tested only, the mlx5 hunk was not built):

diff --git a/Documentation/networking/segmentation-offloads.rst b/Documentation/networking/segmentation-offloads.rst
index 25a8b7e..0f20bb6 100644
--- a/Documentation/networking/segmentation-offloads.rst
+++ b/Documentation/networking/segmentation-offloads.rst
@@ -49,8 +49,8 @@ The default behavior is to increment the IP ID with every segment.  If the
 GSO type SKB_GSO_TCP_FIXEDID is specified then we will not increment the IP
 ID and all segments will use the same IP ID.

-For encapsulated packets, SKB_GSO_TCP_FIXEDID refers only to the outer header.
-SKB_GSO_TCP_FIXEDID_INNER can be used to specify the same for the inner header.
+For encapsulated packets, SKB_GSO_TCP_FIXEDID refers only to the inner header.
+SKB_GSO_TCP_FIXEDID_OUTER can be used to specify the same for the outer header.
 Any combination of these two GSO types is allowed.

 If a device has NETIF_F_TSO_MANGLEID set then the IP ID can be ignored when
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_rx.c b/drivers/net/ethernet/mellanox/mlx5/core/en_rx.c
index 22fea47..b804a75 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en_rx.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en_rx.c
@@ -1175,12 +1175,8 @@ static void mlx5e_shampo_update_ipv4_tcp_hdr(struct mlx5e_rq *rq, struct iphdr *
 	tcp->check = ~tcp_v4_check(skb->len - tcp_off, ipv4->saddr,
 				   ipv4->daddr, 0);
 	skb_shinfo(skb)->gso_type |= SKB_GSO_TCPV4;
-	if (ntohs(ipv4->id) == rq->hw_gro_data->second_ip_id) {
-		bool encap = rq->hw_gro_data->fk.control.flags & FLOW_DIS_ENCAPSULATION;
-
-		skb_shinfo(skb)->gso_type |= encap ? SKB_GSO_TCP_FIXEDID_INNER :
-						     SKB_GSO_TCP_FIXEDID;
-	}
+	if (ntohs(ipv4->id) == rq->hw_gro_data->second_ip_id)
+		skb_shinfo(skb)->gso_type |= SKB_GSO_TCP_FIXEDID;

 	skb->csum_start = (unsigned char *)tcp - skb->head;
 	skb->csum_offset = offsetof(struct tcphdr, check);
diff --git a/drivers/net/ethernet/sfc/ef100_tx.c b/drivers/net/ethernet/sfc/ef100_tx.c
index 0300575..162cfa5 100644
--- a/drivers/net/ethernet/sfc/ef100_tx.c
+++ b/drivers/net/ethernet/sfc/ef100_tx.c
@@ -201,16 +201,10 @@ static void ef100_make_tso_desc(struct efx_nic *efx,
 	bool outer_csum;
 	u32 paylen;

-	if (encap) {
-		if (skb_shinfo(skb)->gso_type & SKB_GSO_TCP_FIXEDID_INNER)
-			mangleid = ESE_GZ_TX_DESC_IP4_ID_NO_OP;
-		if (skb_shinfo(skb)->gso_type & SKB_GSO_TCP_FIXEDID)
-			mangleid_outer = ESE_GZ_TX_DESC_IP4_ID_NO_OP;
-	} else {
-		if (skb_shinfo(skb)->gso_type & SKB_GSO_TCP_FIXEDID)
-			mangleid = ESE_GZ_TX_DESC_IP4_ID_NO_OP;
+	if (skb_shinfo(skb)->gso_type & SKB_GSO_TCP_FIXEDID)
+		mangleid = ESE_GZ_TX_DESC_IP4_ID_NO_OP;
+	if (!encap || (skb_shinfo(skb)->gso_type & SKB_GSO_TCP_FIXEDID_OUTER))
 		mangleid_outer = ESE_GZ_TX_DESC_IP4_ID_NO_OP;
-	}

 	if (efx->net_dev->features & NETIF_F_HW_VLAN_CTAG_TX)
 		vlan_enable = skb_vlan_tag_present(skb);
diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index 6f7fec2..ac3f664 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -5535,7 +5535,7 @@ static inline bool net_gso_ok(netdev_features_t features, int gso_type)
 {
 	netdev_features_t feature;

-	if (gso_type & (SKB_GSO_TCP_FIXEDID | SKB_GSO_TCP_FIXEDID_INNER))
+	if (gso_type & (SKB_GSO_TCP_FIXEDID | SKB_GSO_TCP_FIXEDID_OUTER))
 		gso_type |= __SKB_GSO_TCP_FIXEDID;

 	feature = ((netdev_features_t)gso_type << NETIF_F_GSO_SHIFT) & NETIF_F_GSO_MASK;
diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
index 6787fcf..6eb913e 100644
--- a/include/linux/skbuff.h
+++ b/include/linux/skbuff.h
@@ -716,9 +716,11 @@ enum {

 	/* These indirectly map onto the same netdev feature.
 	 * If NETIF_F_TSO_MANGLEID is set it may mangle both inner and outer IDs.
+	 * SKB_GSO_TCP_FIXEDID stays with the innermost IPv4 header, so there is
+	 * nothing to do on encapsulation. Decapsulation clears the outer flag.
 	 */
 	SKB_GSO_TCP_FIXEDID = 1 << 30,
-	SKB_GSO_TCP_FIXEDID_INNER = 1 << 31,
+	SKB_GSO_TCP_FIXEDID_OUTER = 1 << 31,
 };

 #if BITS_PER_LONG > 32
diff --git a/include/net/ip_tunnels.h b/include/net/ip_tunnels.h
index 5cccf4c..a8e1cd7 100644
--- a/include/net/ip_tunnels.h
+++ b/include/net/ip_tunnels.h
@@ -667,6 +667,7 @@ static inline int iptunnel_pull_offloads(struct sk_buff *skb)
 			return err;
 		skb_shinfo(skb)->gso_type &= ~(NETIF_F_GSO_ENCAP_ALL >>
 					       NETIF_F_GSO_SHIFT);
+		skb_shinfo(skb)->gso_type &= ~SKB_GSO_TCP_FIXEDID_OUTER;
 	}

 	skb->encapsulation = 0;
diff --git a/net/core/filter.c b/net/core/filter.c
index 09c60a5..0b8d363 100644
--- a/net/core/filter.c
+++ b/net/core/filter.c
@@ -3803,6 +3803,7 @@ static int bpf_skb_net_shrink(struct sk_buff *skb, u32 off, u32 len_diff,
 		/* Clear encapsulation flag only when no tunnel GSO flags remain */
 		if (flags & (BPF_F_ADJ_ROOM_DECAP_L4_MASK |
 			     BPF_F_ADJ_ROOM_DECAP_IPXIP_MASK)) {
+			shinfo->gso_type &= ~SKB_GSO_TCP_FIXEDID_OUTER;
 			if (!(shinfo->gso_type & (SKB_GSO_UDP_TUNNEL |
 						  SKB_GSO_UDP_TUNNEL_CSUM |
 						  SKB_GSO_GRE |
diff --git a/net/ipv4/af_inet.c b/net/ipv4/af_inet.c
index 7677b03..d5769a5 100644
--- a/net/ipv4/af_inet.c
+++ b/net/ipv4/af_inet.c
@@ -1402,10 +1402,12 @@ struct sk_buff *inet_gso_segment(struct sk_buff *skb,

 	segs = ERR_PTR(-EPROTONOSUPPORT);

-	fixedid = !!(skb_shinfo(skb)->gso_type & (SKB_GSO_TCP_FIXEDID << encap));
-
-	if (!skb->encapsulation || encap)
+	if (!skb->encapsulation || encap) {
 		udpfrag = !!(skb_shinfo(skb)->gso_type & SKB_GSO_UDP);
+		fixedid = !!(skb_shinfo(skb)->gso_type & SKB_GSO_TCP_FIXEDID);
+	} else {
+		fixedid = !!(skb_shinfo(skb)->gso_type & SKB_GSO_TCP_FIXEDID_OUTER);
+	}

 	ops = rcu_dereference(inet_offloads[proto]);
 	if (likely(ops && ops->callbacks.gso_segment)) {
diff --git a/net/ipv4/tcp_offload.c b/net/ipv4/tcp_offload.c
index b84fcd0..7919004 100644
--- a/net/ipv4/tcp_offload.c
+++ b/net/ipv4/tcp_offload.c
@@ -459,6 +459,7 @@ INDIRECT_CALLABLE_SCOPE int tcp4_gro_complete(struct sk_buff *skb, int thoff)
 {
 	const u16 offset = NAPI_GRO_CB(skb)->network_offsets[skb->encapsulation];
 	const struct iphdr *iph = (struct iphdr *)(skb->data + offset);
+	const u8 fixedid = NAPI_GRO_CB(skb)->ip_fixedid;
 	struct tcphdr *th = tcp_hdr(skb);

 	if (unlikely(NAPI_GRO_CB(skb)->is_flist)) {
@@ -473,9 +474,10 @@ INDIRECT_CALLABLE_SCOPE int tcp4_gro_complete(struct sk_buff *skb, int thoff)
 	th->check = ~tcp_v4_check(skb->len - thoff, iph->saddr,
 				  iph->daddr, 0);

-	BUILD_BUG_ON(SKB_GSO_TCP_FIXEDID << 1 != SKB_GSO_TCP_FIXEDID_INNER);
+	/* Bit 0 of ip_fixedid is the outermost header, bit 1 is the inner one. */
 	skb_shinfo(skb)->gso_type |= SKB_GSO_TCPV4 |
-			(NAPI_GRO_CB(skb)->ip_fixedid * SKB_GSO_TCP_FIXEDID);
+		(((fixedid >> skb->encapsulation) & 1) * SKB_GSO_TCP_FIXEDID) |
+		((fixedid & skb->encapsulation) * SKB_GSO_TCP_FIXEDID_OUTER);

 	tcp_gro_complete(skb);
 	return 0;

Re: [PATCH net 1/2] net: keep the TCP fixed-ID flags on the header they describe

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [Critical] ipv6: use-after-free of hdr in ipv6_srh_rcv() after iptunnel_pull_offloads()
- [Medium] net: __skb_udp_tunnel_segment() loses remcsum NETIF_F_HW_CSUM when encap_level > 0
- [Medium] net: geneve_post_decap_hint() leaves fixed-ID flag on outer header after decap
--

commit f6a9b837efd7885691511746c504ec36f4d4eb89
Author: Julius Bairaktaris <julius@bairaktaris.de>

net: keep the TCP fixed-ID flags on the header they describe

This commit introduces skb_gso_fixedid_encap() and skb_gso_fixedid_decap() to
move the SKB_GSO_TCP_FIXEDID and SKB_GSO_TCP_FIXEDID_INNER flags between
headers during tunnel encapsulation and decapsulation. It also preserves
SKB_GSO_CB(skb)->encap_level in gre_gso_segment() and
__skb_udp_tunnel_segment() so inner IPv4 headers are segmented using the inner
fixed-ID flag.

[ ... ]

> diff --git a/include/net/ip_tunnels.h b/include/net/ip_tunnels.h
> index 5cccf4c0e6913..c2ffda6029a06 100644
> --- a/include/net/ip_tunnels.h
> +++ b/include/net/ip_tunnels.h
> @@ -665,6 +665,8 @@ static inline int iptunnel_pull_offloads(struct sk_buff *skb)
>  		err = skb_unclone(skb, GFP_ATOMIC);
>  		if (unlikely(err))
>  			return err;
> +		if (skb->encapsulation)
> +			skb_gso_fixedid_decap(skb_shinfo(skb));
>  		skb_shinfo(skb)->gso_type &= ~(NETIF_F_GSO_ENCAP_ALL >>
>  					       NETIF_F_GSO_SHIFT);
>  	}

[Severity: Medium]
When a double-encapsulated Geneve GSO packet carrying a GRO hint option is
decapsulated in geneve_udp_encap_recv(), iptunnel_pull_header() calls
iptunnel_pull_offloads(), which shifts SKB_GSO_TCP_FIXEDID_INNER into
SKB_GSO_TCP_FIXEDID and clears skb->encapsulation.

Immediately afterwards, geneve_udp_encap_recv() calls geneve_post_decap_hint()
to restore encapsulation state for the nested tunnel:

drivers/net/geneve.c:geneve_udp_encap_recv() {
    ...
	if (iptunnel_pull_header(skb, GENEVE_BASE_HLEN + opts_len, inner_proto,
				 !net_eq(geneve->net, dev_net(geneve->dev)))) {
		dev_dstats_rx_dropped(geneve->dev);
		goto drop;
	}
    ...
	if (geneve_post_decap_hint(sk, skb, sizeof(struct genevehdr) +
				   opts_len, &geneveh)) {
    ...
}

drivers/net/geneve.c:geneve_post_decap_hint() {
    ...
	skb->encapsulation = 1;
	skb_set_transport_header(skb, gro_hint->nested_tp_offset);
    ...
	if (uh->check) {
		...
		skb_shinfo(skb)->gso_type |= SKB_GSO_UDP_TUNNEL_CSUM;
		...
	} else {
		skb_shinfo(skb)->gso_type |= SKB_GSO_UDP_TUNNEL;
	}
    ...
}

Doesn't this leave the fixed-ID flag for the innermost TCP/IPv4 header in
SKB_GSO_TCP_FIXEDID instead of SKB_GSO_TCP_FIXEDID_INNER while
skb->encapsulation is 1, causing subsequent segmentation or decapsulation of
the nested tunnel to apply the inner TCP fixed-ID state to the nested tunnel's
outer header and lose the fixed-ID state for the innermost IPv4 header?

[ ... ]

> diff --git a/net/ipv4/udp_offload.c b/net/ipv4/udp_offload.c
> index cf07c3c6611a2..edcabe1115a60 100644
> --- a/net/ipv4/udp_offload.c
> +++ b/net/ipv4/udp_offload.c
> @@ -205,7 +205,6 @@ static struct sk_buff *__skb_udp_tunnel_segment(struct sk_buff *skb,
>  
>  	/* setup inner skb. */
>  	skb->encapsulation = 0;
> -	SKB_GSO_CB(skb)->encap_level = 0;
>  	__skb_pull(skb, tnl_hlen);
>  	skb_reset_mac_header(skb);
>  	skb_set_network_header(skb, skb_inner_network_offset(skb));

[Severity: Medium]
In __skb_udp_tunnel_segment(), when SKB_GSO_TUNNEL_REMCSUM is active, features
is first masked with skb->dev->hw_enc_features and then updated to strip
NETIF_F_CSUM_MASK and set NETIF_F_HW_CSUM before calling gso_inner_segment():

net/ipv4/udp_offload.c:__skb_udp_tunnel_segment() {
    ...
	features &= skb->dev->hw_enc_features;
	if (need_csum)
		features &= ~NETIF_F_SCTP_CRC;

	if (remcsum) {
		features &= ~NETIF_F_CSUM_MASK;
		if (!need_csum || offload_csum)
			features |= NETIF_F_HW_CSUM;
	}

	/* segment inner packet. */
	segs = gso_inner_segment(skb, features);
    ...
}

With SKB_GSO_CB(skb)->encap_level remaining non-zero, both inet_gso_segment()
and ipv6_gso_segment() see encap as true and mask features with
skb->dev->hw_enc_features a second time:

net/ipv4/af_inet.c:inet_gso_segment() {
    ...
	encap = SKB_GSO_CB(skb)->encap_level > 0;
	if (encap)
		features &= skb->dev->hw_enc_features;
    ...
}

Won't this second masking clear NETIF_F_HW_CSUM on any device where
hw_enc_features does not include NETIF_F_HW_CSUM (such as devices without
hardware tunnel checksum offload or NICs that only advertise
NETIF_F_IP_CSUM | NETIF_F_IPV6_CSUM in hw_enc_features), forcing software
segmentation to compute unnecessary inner payload checksums and disabling
GSO_PARTIAL and frag_list segmentation for remcsum packets?

> diff --git a/net/ipv6/exthdrs.c b/net/ipv6/exthdrs.c
> index 96c8402a27a54..126058772bc5a 100644
> --- a/net/ipv6/exthdrs.c
> +++ b/net/ipv6/exthdrs.c
> @@ -53,6 +53,7 @@
>  #include <linux/ioam6_genl.h>
>  #include <net/ioam6.h>
>  #include <net/dst_metadata.h>
> +#include <net/ip_tunnels.h>
>  
>  #include <linux/uaccess.h>
>  
> @@ -406,7 +407,10 @@ static int ipv6_srh_rcv(struct sk_buff *skb, struct inet6_dev *idev)
>  
>  			skb_reset_network_header(skb);
>  			skb_reset_transport_header(skb);
> -			skb->encapsulation = 0;
> +			if (iptunnel_pull_offloads(skb)) {
> +				kfree_skb(skb);
> +				return -1;
> +			}
>  			if (hdr->nexthdr == NEXTHDR_IPV4)
>  				skb->protocol = htons(ETH_P_IP);
>  			__skb_tunnel_rx(skb, skb->dev, net);

[Severity: Critical]
In ipv6_srh_rcv(), hdr points into skb->head via skb_transport_header(skb)
before entering the segments_left == 0 decapsulation block:

net/ipv6/exthdrs.c:ipv6_srh_rcv() {
	hdr = (struct ipv6_sr_hdr *)skb_transport_header(skb);
    ...
looped_back:
	if (hdr->segments_left == 0) {
		if (hdr->nexthdr == NEXTHDR_IPV6 || hdr->nexthdr == NEXTHDR_IPV4) {
			...
			if (iptunnel_pull_offloads(skb)) {
				kfree_skb(skb);
				return -1;
			}
			if (hdr->nexthdr == NEXTHDR_IPV4)
				skb->protocol = htons(ETH_P_IP);
    ...
}

When skb_is_gso(skb) && skb_cloned(skb) is true, iptunnel_pull_offloads()
calls skb_unclone(skb, GFP_ATOMIC) -> pskb_expand_head(), which reallocates
skb->head and frees the old buffer.

Doesn't reading hdr->nexthdr after iptunnel_pull_offloads(skb) cause a
use-after-free read from the old skb->head buffer?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/cover.1792857378.git.julius@bairaktaris.de?part=1

Re: [PATCH net 2/2] selftests: net: check TCP IP IDs across tunnels after GRO and GSO

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [Critical] ipv6: seg6: use-after-free of hdr in ipv6_srh_rcv() after iptunnel_pull_offloads()
- [Medium] selftests: net: unchecked tunnel() return status in run_decap()
- [Medium] udp: gso: inner hw_enc_features re-masking clobbers remcsum NETIF_F_HW_CSUM
--

commit e0a5534c2eabd4b1e53b2a69aaee94090aff3fec
Author: Julius Bairaktaris <julius@bairaktaris.de>

selftests: net: check TCP IP IDs across tunnels after GRO and GSO

This series updates GSO fixed-ID flag handling across tunnel encapsulation,
decapsulation, and segmentation paths, and adds a network selftest verifying
inner and outer IPv4 IDs across IPIP, GRE, and VXLAN tunnels.

> diff --git a/net/ipv4/udp_offload.c b/net/ipv4/udp_offload.c
> index cf07c3c6611a2..edcabe1115a60 100644
> --- a/net/ipv4/udp_offload.c
> +++ b/net/ipv4/udp_offload.c
> @@ -205,7 +205,6 @@ static struct sk_buff *__skb_udp_tunnel_segment(struct sk_buff *skb,
>  
>  	/* setup inner skb. */
>  	skb->encapsulation = 0;
> -	SKB_GSO_CB(skb)->encap_level = 0;
>  	__skb_pull(skb, tnl_hlen);
>  	skb_reset_mac_header(skb);
>  	skb_set_network_header(skb, skb_inner_network_offset(skb));

[Severity: Medium]
When __skb_udp_tunnel_segment() processes a packet with remote checksum
offload enabled, it masks features with skb->dev->hw_enc_features, strips
all checksum bits, and sets only the hardware checksum bit:

net/ipv4/udp_offload.c:__skb_udp_tunnel_segment() {
	...
	features &= skb->dev->hw_enc_features;
	if (need_csum)
		features &= ~NETIF_F_SCTP_CRC;

	if (remcsum) {
		features &= ~NETIF_F_CSUM_MASK;
		if (!need_csum || offload_csum)
			features |= NETIF_F_HW_CSUM;
	}

	/* segment inner packet. */
	segs = gso_inner_segment(skb, features);
	...
}

With SKB_GSO_CB(skb)->encap_level = 0 removed here, encap_level remains
greater than zero when gso_inner_segment() invokes inet_gso_segment() in
net/ipv4/af_inet.c or ipv6_gso_segment() in net/ipv6/ip6_offload.c:

	encap = SKB_GSO_CB(skb)->encap_level > 0;
	if (encap)
		features &= skb->dev->hw_enc_features;

Won't this second masking with skb->dev->hw_enc_features clear the
NETIF_F_HW_CSUM bit that __skb_udp_tunnel_segment() just set for remcsum on
any device where hw_enc_features does not include NETIF_F_HW_CSUM (such as
devices with hw_enc_features == 0 or devices only advertising protocol-
specific checksum flags), forcing skb_segment() into the software checksum
path?

> diff --git a/net/ipv6/exthdrs.c b/net/ipv6/exthdrs.c
> index 96c8402a27a54..126058772bc5a 100644
> --- a/net/ipv6/exthdrs.c
> +++ b/net/ipv6/exthdrs.c

[ ... ]

> @@ -406,7 +407,10 @@ static int ipv6_srh_rcv(struct sk_buff *skb, struct inet6_dev *idev)
>  
>  			skb_reset_network_header(skb);
>  			skb_reset_transport_header(skb);
> -			skb->encapsulation = 0;
> +			if (iptunnel_pull_offloads(skb)) {
> +				kfree_skb(skb);
> +				return -1;
> +			}
>  			if (hdr->nexthdr == NEXTHDR_IPV4)
>  				skb->protocol = htons(ETH_P_IP);
>  			__skb_tunnel_rx(skb, skb->dev, net);

[Severity: Critical]
In ipv6_srh_rcv(), hdr is initialized from skb_transport_header(skb) at the
start of the function:

net/ipv6/exthdrs.c:ipv6_srh_rcv() {
	...
	hdr = (struct ipv6_sr_hdr *)skb_transport_header(skb);

When hdr->segments_left == 0 on entry, the skb_cloned(skb) check later in
ipv6_srh_rcv() has not run. Calling iptunnel_pull_offloads(skb) on a cloned
GSO packet calls skb_unclone(skb, GFP_ATOMIC) -> pskb_expand_head(), which
allocates a new skb->head buffer and frees the old one:

include/net/ip_tunnels.h:iptunnel_pull_offloads() {
	if (skb_is_gso(skb)) {
		int err;

		err = skb_unclone(skb, GFP_ATOMIC);
		if (unlikely(err))
			return err;
	...
}

Can checking hdr->nexthdr after iptunnel_pull_offloads(skb) returns
dereference a dangling hdr pointer into the freed skb->head buffer?

> diff --git a/tools/testing/selftests/net/tcp_fixedid.sh b/tools/testing/selftests/net/tcp_fixedid.sh
> new file mode 100755
> index 0000000000000..88f9298174077
> --- /dev/null
> +++ b/tools/testing/selftests/net/tcp_fixedid.sh
> @@ -0,0 +1,291 @@

[ ... ]

> +# run_decap <fixed: inner|outer>: VXLAN from NSRC, decapsulated in NRTR
> +run_decap() {
> +	local fixed=$1 want=incr flags=-F
> +	local r0mac t0
> +
> +	setup || { log_test_skip "decap"; return; }
> +	ip -n "$NRTR" link add vx0 address "$VX_MAC" type vxlan id 42 \
> +		dstport 4789 local 10.1.0.2 nolearning ||
> +		{ log_test_skip "decap"; cleanup_all_ns; return; }
> +	ip -n "$NRTR" addr add 10.3.0.2/24 dev vx0
> +	ip -n "$NRTR" link set vx0 up
> +	tunnel "$NRTR" tun0 ipip 4 10.2.0.1 10.2.0.2
> +	route_into "$NRTR" tun0
> +	noseg "$NRTR" r1
> +
> +	if [ "$fixed" = inner ]; then
> +		want=fixed
> +		flags=-f
> +	fi
> +	r0mac=$(ip -n "$NRTR" -br link show r0 | awk '{ print $3 }')
> +	capture "$NMID" m1
> +	t0=$(tx_packets "$NRTR" tun0)
> +	ip netns exec "$NSRC" "$TX" -T -i s0 -M "$r0mac" -s 10.3.0.1 \
> +		-d "$INNER_DST" -n "$SEGS" -b "$BURSTS" \
> +		-V 10.1.0.1,10.1.0.2 -m "$VX_MAC" $flags
> +	wait "$CAP_PID"
> +	check "decap VXLAN with fixed $fixed IDs, into IPIP" \
> +		"$(tail -n 1 "$RESULT")" "$want" 1 \
> +		$(($(tx_packets "$NRTR" tun0) - t0))
> +	cleanup_all_ns
> +}

[Severity: Medium]
Unlike run_encap() and run_encap_decap(), run_decap() does not check the
return status of the tunnel helper when creating the ipip device tun0:

	tunnel "$NRTR" tun0 ipip 4 10.2.0.1 10.2.0.2

If the kernel supports vxlan but does not have ipip support enabled, won't
tun0 creation fail silently here, causing capture to wait for its full poll
timeout and tx_packets to output an empty string that triggers an arithmetic
syntax error in:

	$(($(tx_packets "$NRTR" tun0) - t0))

and fails the test instead of skipping it?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/cover.1792857378.git.julius@bairaktaris.de?part=2

Re: [PATCH net 1/2] net: keep the TCP fixed-ID flags on the header they describe

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found 6 potential
issue(s) to consider.

Critical: 0 · High: 2 · Medium: 4 (1) · Low: 0 (1)

If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).

- [High] Use-after-free read in ipv6_srh_rcv() (net/ipv6/exthdrs.c).
- [High] One encapsulation site was missed: IPsec tunnel mode with device
  ESP offload.
- [Medium] The patch removes `SKB_GSO_CB(skb)->encap_level = 0;` from
  __skb_udp_tunnel_segment() (and gre_gso_segment()).
- [Medium] iptunnel_handle_offloads() (net/ipv4/ip_tunnel_core.c) moves
  the fixed-ID flag only on the 0 -> 1 transition of skb->encapsulation…
- [Medium] BPF L3-only decapsulation now loses fixed IDs on a round trip
  that worked before.
- [Medium] net: geneve_post_decap_hint() leaves fixed-ID flag on outer
  header after decap

Pre-existing issues:
- [Medium] This problem wasn't introduced by this patch, but
  handle_gso_encap() (net/core/lwt_bpf.c) matches `case IPPROTO_IP:` (0)…
- [Low] This problem wasn't introduced by this patch, but
  bpf_lwt_seg6_action() SEG6_LOCAL_ACTION_END_DT6 (net/core/filter.c)…

--

net: keep the TCP fixed-ID flags on the header they describe

This patch adds skb_gso_fixedid_encap() and skb_gso_fixedid_decap(). They
move SKB_GSO_TCP_FIXEDID between the outer and inner bit when a GSO packet
gains or loses an outer header, and the IP tunnel, BPF and segment routing
encap/decap paths now call them. The patch also stops gre_gso_segment()
and __skb_udp_tunnel_segment() from resetting SKB_GSO_CB(skb)->encap_level.

> diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
> index 6787fcf6ac3e4..00df57cbc05c2 100644
> --- a/include/linux/skbuff.h
> +++ b/include/linux/skbuff.h
> @@ -5337,6 +5337,36 @@ static inline void skb_decrease_gso_size(struct skb_shared_info *shinfo,
>  	shinfo->gso_size -= decrement;
>  }
>  
> +/**
> + * skb_gso_fixedid_encap - move the TCP fixed-ID flag to the inner header
> + * @shinfo: shared info of a GSO packet that becomes encapsulated
> + *
> + * The IPv4 header that SKB_GSO_TCP_FIXEDID describes becomes the inner one,
> + * and the new outer header gets an incrementing ID.
> + */
> +static inline void skb_gso_fixedid_encap(struct skb_shared_info *shinfo)
> +{
> +	unsigned int fixedid = shinfo->gso_type & SKB_GSO_TCP_FIXEDID;
> +
> +	shinfo->gso_type &= ~(SKB_GSO_TCP_FIXEDID | SKB_GSO_TCP_FIXEDID_INNER);
> +	shinfo->gso_type |= fixedid << 1;
> +}

[Severity: High]
Does one more encapsulation site need this helper? IPsec tunnel mode with
ESP device offload seems to make a GSO packet encapsulated without moving
the flag.

In xfrm_output(), when xfrm_dev_offload_ok() succeeds for a GSO skb
without inner_protocol:

net/xfrm/xfrm_output.c:xfrm_output() {
    ...
	skb->encapsulation = 1;

	if (skb_is_gso(skb)) {
		if (skb->inner_protocol && x->props.mode == XFRM_MODE_TUNNEL)
			return xfrm_output_gso(net, sk, skb);

		skb_shinfo(skb)->gso_type |= SKB_GSO_ESP;
		goto out;
	}
    ...
}

xfrm4_tunnel_encap_add() then turns the original IPv4 header into the
inner one and builds a new outer header:

net/xfrm/xfrm_output.c:xfrm4_tunnel_encap_add() {
    ...
	top_iph->frag_off = (flags & XFRM_STATE_NOPMTUDISC) || small_ipv6 ?
		0 : (XFRM_MODE_SKB_CB(skb)->frag_off & htons(IP_DF));
    ...
	ip_select_ident(dev_net(dst->dev), skb, NULL);
    ...
}

Neither function calls skb_gso_fixedid_encap(), so SKB_GSO_TCP_FIXEDID
still describes the old header. With software segmentation, the outer
inet_gso_segment() pass runs with encap_level == 0 and reads:

	fixedid = !!(skb_shinfo(skb)->gso_type & (SKB_GSO_TCP_FIXEDID << encap));

Every segment would then get the same outer ID, possibly with DF clear.

The inner pass goes through
esp4_gso_segment()->xfrm4_tunnel_gso_segment()->skb_eth_gso_segment()
with encap_level > 0. It finds FIXEDID_INNER clear, so the inner IDs
increment.

The commit message describes this same result for
iptunnel_handle_offloads(). Should the xfrm tunnel output path also call
skb_gso_fixedid_encap()?

[ ... ]

> diff --git a/net/core/filter.c b/net/core/filter.c
> index 09c60a58c82ab..4f87a827b25be 100644
> --- a/net/core/filter.c
> +++ b/net/core/filter.c
> @@ -3718,6 +3718,9 @@ static int bpf_skb_net_grow(struct sk_buff *skb, u32 off, u32 len_diff,
>  	if (skb_is_gso(skb)) {
>  		struct skb_shared_info *shinfo = skb_shinfo(skb);
>  
> +		if (encap)
> +			skb_gso_fixedid_encap(shinfo);
> +
>  		/* Header must be checked, and gso_segs recomputed. */
>  		shinfo->gso_type |= gso_type;
>  		shinfo->gso_segs = 0;
> @@ -3809,9 +3812,11 @@ static int bpf_skb_net_shrink(struct sk_buff *skb, u32 off, u32 len_diff,
>  						  SKB_GSO_GRE_CSUM |
>  						  SKB_GSO_IPXIP4 |
>  						  SKB_GSO_IPXIP6 |
> -						  SKB_GSO_ESP)))
> -				if (skb->encapsulation)
> -					skb->encapsulation = 0;
> +						  SKB_GSO_ESP)) &&
> +			    skb->encapsulation) {
> +				skb_gso_fixedid_decap(shinfo);
> +				skb->encapsulation = 0;
> +			}
>  		}

[Severity: Medium]
Can a BPF encap/decap round trip now lose fixed IDs?

bpf_skb_net_grow() moves the flag to SKB_GSO_TCP_FIXEDID_INNER for any L3
encap. That includes BPF_F_ADJ_ROOM_ENCAP_L3_IPV6 or _IPV4 with no L4
flag.

In bpf_skb_net_shrink(), the flag is only moved back inside:

	if (flags & (BPF_F_ADJ_ROOM_DECAP_L4_MASK |
		     BPF_F_ADJ_ROOM_DECAP_IPXIP_MASK)) {

bpf_skb_adjust_room() still accepts a shrink with only
BPF_F_ADJ_ROOM_DECAP_L3_IPV4/IPV6 set, or with no decap flags at all.
Programs written before the L4/IPXIP decap flags existed do this.

After such a decap the bit stays in FIXEDID_INNER. The top-level
inet_gso_segment() then sees SKB_GSO_TCP_FIXEDID clear, and the segments
get incrementing IDs.

Before this patch, grow left SKB_GSO_TCP_FIXEDID where it was, so the
round trip kept fixed IDs. The commit message says the change avoids
losing fixed IDs for "a packet encapsulated and decapsulated again without
segmentation".

Should the fixed-ID move in bpf_skb_net_shrink() also cover L3-only
decaps?

[Severity: Low]
This isn't a bug introduced by this patch. The segment routing paths in
exthdrs.c are converted to iptunnel_pull_offloads(), but the
SEG6_LOCAL_ACTION_END_DT6 case in bpf_lwt_seg6_action() still
decapsulates IPv6-in-IPv6 by hand:

net/core/filter.c:bpf_lwt_seg6_action() {
    ...
		__skb_pull(skb, hdroff);
		skb_postpull_rcsum(skb, skb_network_header(skb), hdroff);
		skb_reset_network_header(skb);
		skb_reset_transport_header(skb);
		skb->encapsulation = 0;
    ...
}

Tunnel GSO types such as SKB_GSO_IPXIP6 stay set on a packet that is no
longer encapsulated. Both headers are IPv6, so the fixed-ID bits don't
matter here.

Could the leftover gso_type force software GSO on devices without
GSO_IPXIP6? Would iptunnel_pull_offloads() fit here as well?

> diff --git a/net/core/lwt_bpf.c b/net/core/lwt_bpf.c
> index da49364ec63de..4d9faf2cc5de5 100644
> --- a/net/core/lwt_bpf.c
> +++ b/net/core/lwt_bpf.c
> @@ -539,6 +539,7 @@ static int handle_gso_type(struct sk_buff *skb, unsigned int gso_type,
>  {
>  	struct skb_shared_info *shinfo = skb_shinfo(skb);
>  
> +	skb_gso_fixedid_encap(shinfo);
>  	gso_type |= SKB_GSO_DODGY;
>  	shinfo->gso_type |= gso_type;
>  	skb_decrease_gso_size(shinfo, encap_len);

[Severity: Medium]
This is a pre-existing issue, not one introduced by this patch.
handle_gso_encap() matches IPPROTO_IP (0) instead of IPPROTO_IPIP (4):

net/core/lwt_bpf.c:handle_gso_encap() {
    ...
	case IPPROTO_IP:
	case IPPROTO_IPV6:
		if (ipv4)
			return handle_gso_type(skb, SKB_GSO_IPXIP4, encap_len);
    ...
}

Take a TCP GSO packet pushed by bpf_lwt_push_ip_encap() with an outer IPv4
header whose protocol is IPPROTO_IPIP. Does it fall through to the default
case and fail with -EPROTONOSUPPORT, even though the same header works for
non-GSO packets?

Should this be case IPPROTO_IPIP?

[ ... ]

> diff --git a/net/ipv4/ip_tunnel_core.c b/net/ipv4/ip_tunnel_core.c
> index 6c2855adff289..59050229e3499 100644
> --- a/net/ipv4/ip_tunnel_core.c
> +++ b/net/ipv4/ip_tunnel_core.c
> @@ -182,9 +182,10 @@ EXPORT_SYMBOL_GPL(iptunnel_metadata_reply);
>  int iptunnel_handle_offloads(struct sk_buff *skb,
>  			     int gso_type_mask)
>  {
> +	bool encap = !skb->encapsulation;
>  	int err;
>  
> -	if (likely(!skb->encapsulation)) {
> +	if (likely(encap)) {
>  		skb_reset_inner_headers(skb);
>  		skb->encapsulation = 1;
>  	}
> @@ -193,6 +194,8 @@ int iptunnel_handle_offloads(struct sk_buff *skb,
>  		err = skb_header_unclone(skb, GFP_ATOMIC);
>  		if (unlikely(err))
>  			return err;
> +		if (encap)
> +			skb_gso_fixedid_encap(skb_shinfo(skb));
>  		skb_shinfo(skb)->gso_type |= gso_type_mask;
>  		return 0;
>  	}

[Severity: Medium]
What happens here when the GSO skb is already encapsulated?

Forwarded GRE or IPIP traffic merged by gre_gro_complete() or
ipip_gro_complete() arrives with skb->encapsulation = 1. Its
SKB_GSO_TCP_FIXEDID describes its outer header.

Suppose a router sends it into another IPv4 tunnel, for example through
ipip_tunnel_xmit(). Then encap is false here and the flag stays where it
is.

The new outermost header is segmented by inet_gso_segment() with
encap_level == 0, which reads the stale SKB_GSO_TCP_FIXEDID. Every segment
then gets the single ID from ip_select_ident(), possibly with DF clear.

That is the wire behaviour the commit message lists as fixed: "forwarded
into a tunnel gets one outer ID for all segments, also with DF clear".

Two bits can't describe three headers. Would clearing SKB_GSO_TCP_FIXEDID
for the newly pushed outer header in the nested case avoid this? If not,
could the commit message mention the limitation?

> diff --git a/net/ipv4/udp_offload.c b/net/ipv4/udp_offload.c
> index cf07c3c6611a2..edcabe1115a60 100644
> --- a/net/ipv4/udp_offload.c
> +++ b/net/ipv4/udp_offload.c
> @@ -205,7 +205,6 @@ static struct sk_buff *__skb_udp_tunnel_segment(struct sk_buff *skb,
>  
>  	/* setup inner skb. */
>  	skb->encapsulation = 0;
> -	SKB_GSO_CB(skb)->encap_level = 0;
>  	__skb_pull(skb, tnl_hlen);
>  	skb_reset_mac_header(skb);
>  	skb_set_network_header(skb, skb_inner_network_offset(skb));

[Severity: Medium]
Does keeping encap_level here undo the feature override for remote
checksum offload?

With SKB_GSO_TUNNEL_REMCSUM, __skb_udp_tunnel_segment() masks with
hw_enc_features and then adds NETIF_F_HW_CSUM on purpose for the inner
pass:

	features &= skb->dev->hw_enc_features;
	...
	if (remcsum) {
		features &= ~NETIF_F_CSUM_MASK;
		if (!need_csum || offload_csum)
			features |= NETIF_F_HW_CSUM;
	}

encap_level is no longer reset, so the inner inet_gso_segment() (and
ipv6_gso_segment()) masks a second time:

	encap = SKB_GSO_CB(skb)->encap_level > 0;
	if (encap)
		features &= skb->dev->hw_enc_features;

The second mask removes NETIF_F_HW_CSUM whenever hw_enc_features lacks
it. Examples are the register_netdevice() default and NICs that only
advertise IP_CSUM or IPV6_CSUM.

skb_segment() then runs with csum == false, which has three effects:

- It skips the GSO_PARTIAL path.
- It linearizes every segment with shared frags. If __skb_linearize()
  fails, the whole GSO packet is dropped.
- It computes a software checksum per segment. remcsum_offload keeps
  CHECKSUM_PARTIAL, so tcp_gso_segment() throws that checksum away in
  gso_reset_checksum().

The commit message says "the inner pass then also masks the features with
hw_enc_features, which the tunnel handlers have done already". Does that
still hold in the remcsum case?

> diff --git a/net/ipv6/exthdrs.c b/net/ipv6/exthdrs.c
> index 96c8402a27a54..126058772bc5a 100644
> --- a/net/ipv6/exthdrs.c
> +++ b/net/ipv6/exthdrs.c

[ ... ]

> @@ -406,7 +407,10 @@ static int ipv6_srh_rcv(struct sk_buff *skb, struct inet6_dev *idev)
>  
>  			skb_reset_network_header(skb);
>  			skb_reset_transport_header(skb);
> -			skb->encapsulation = 0;
> +			if (iptunnel_pull_offloads(skb)) {
> +				kfree_skb(skb);
> +				return -1;
> +			}
>  			if (hdr->nexthdr == NEXTHDR_IPV4)
                            ^^^^^^^^^^^^

[Severity: High]
Can hdr be stale at this point? It is set at the top of ipv6_srh_rcv():

	hdr = (struct ipv6_sr_hdr *)skb_transport_header(skb);

For a GSO skb, iptunnel_pull_offloads() calls skb_unclone(). If
skb_cloned() is true, that reaches pskb_expand_head(), which allocates a
new head, drops this skb's reference on the old one and switches
skb->head:

iptunnel_pull_offloads()
  skb_unclone(skb, GFP_ATOMIC)
    pskb_expand_head()
      skb_release_data(skb, SKB_CONSUMED)
      skb->head = data

After that, hdr->nexthdr reads a buffer that only the other clone holder
still references. For example, when a packet tap is active, an AF_PACKET
socket on another CPU may already have freed it.

The segments_left != 0 path in this function already reloads hdr after its
own pskb_expand_head().

Should hdr->nexthdr be saved before the iptunnel_pull_offloads() call, or
hdr reloaded after it? ipv6_rpl_srh_rcv() doesn't use hdr after the new
call, so it looks unaffected.

[ ... ]

> diff --git a/include/net/ip_tunnels.h b/include/net/ip_tunnels.h
> --- a/include/net/ip_tunnels.h
> +++ b/include/net/ip_tunnels.h
> @@ -665,6 +665,8 @@ static inline int iptunnel_pull_offloads(struct sk_buff *skb)
>  		err = skb_unclone(skb, GFP_ATOMIC);
>  		if (unlikely(err))
>  			return err;
> +		if (skb->encapsulation)
> +			skb_gso_fixedid_decap(skb_shinfo(skb));
>  		skb_shinfo(skb)->gso_type &= ~(NETIF_F_GSO_ENCAP_ALL >>
>  					       NETIF_F_GSO_SHIFT);
>  	}

[Severity: Medium]
Does the Geneve GRO hint path need a matching skb_gso_fixedid_encap()?

geneve_udp_encap_recv() calls iptunnel_pull_header() and then
geneve_post_decap_hint(). Take a GRO-merged packet that carries a GRO
hint for a nested UDP tunnel. GRO completed the innermost TCP/IPv4
header with encap_mark set, so that header's fixed-ID state sits in
SKB_GSO_TCP_FIXEDID_INNER.

With this patch, iptunnel_pull_offloads() sees skb->encapsulation set
and calls skb_gso_fixedid_decap(), which moves the bit down to
SKB_GSO_TCP_FIXEDID. geneve_post_decap_hint() then does two things:

- It sets skb->encapsulation = 1 again.
- It adds SKB_GSO_UDP_TUNNEL or SKB_GSO_UDP_TUNNEL_CSUM for the nested
  tunnel.

skb->inner_network_header still points at the innermost IPv4 header,
and nothing moves the bit back to SKB_GSO_TCP_FIXEDID_INNER.

When the packet is segmented, the top-level inet_gso_segment() pass
runs with encap_level == 0 on the nested tunnel's outer IPv4 header.
It reads SKB_GSO_TCP_FIXEDID, which really describes the innermost
header. If that outer header has DF set, all segments get the same
ID. If DF is clear, the fixed-ID check in inet_gso_segment() fails the
segmentation and the packet is dropped.

The inner pass then finds SKB_GSO_TCP_FIXEDID_INNER clear. The
innermost IDs that GRO saw as fixed come out incrementing.

If the nested tunnel is decapsulated later instead, the next
iptunnel_pull_offloads() shifts the now clear
SKB_GSO_TCP_FIXEDID_INNER down. The fixed-ID state of the innermost
header is then lost.

Should geneve_post_decap_hint() call skb_gso_fixedid_encap() where it
restores skb->encapsulation? That would put the bit back on the
innermost header and give the nested outer header incrementing IDs.
Or would you rather keep iptunnel_pull_offloads() from moving the flag
when a GRO hint is present?

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/cover.1792857378.git.julius%40bairaktaris.de