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

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

[Julius Bairaktaris]

GRO sets SKB_GSO_TCP_FIXEDID for a TCP packet whose IPv4 IDs do not
change. Since commit 3271f19bf7b9 ("net: gso: restore ids of outer ip
headers correctly") the flag describes the outermost header and
SKB_GSO_TCP_FIXEDID_INNER the inner one, so every path that adds or
removes a header would have to move the flags. None does, and a TCP
stream that GRO merged and that crosses a tunnel boundary is segmented
with the fixed-ID state of the wrong header.

Patch 1 lets SKB_GSO_TCP_FIXEDID describe the innermost header again and
replaces the inner flag with SKB_GSO_TCP_FIXEDID_OUTER, as Alexei
suggested in review of v1. Patch 2 adds a selftest.

Tested in QEMU (KVM, KASAN and lockdep) with the selftest, 18 cases,
400 segments each:
- net (30c71476d842), 2 of 2 runs: 6 cases fail. Fixed inner IDs
  increment after IPIP over IPv4 and IPv6 and after the ESP tunnel (390
  of 400 segments carry an inner ID the sender did not use), the outer
  IDs repeat after GRE and VXLAN over IPv4 (10 distinct outer IDs for 400
  segments), and a VXLAN stream with fixed outer IDs keeps that state
  after decapsulation into IPIP (10 distinct outer IDs).
- this series, 2 of 2 runs: 18 of 18 pass, no KASAN or lockdep report.

With a tunnel inside a tunnel, such as GRE traffic that GRO merged and
that is routed into an IPIP tunnel without decapsulation, two flags still
describe three IPv4 headers: the new outermost header takes
SKB_GSO_TCP_FIXEDID_OUTER of the header below it, as it takes
SKB_GSO_TCP_FIXEDID on net. This series does not change that case; it
gets a separate fix.

Changes in v2:
- Keep SKB_GSO_TCP_FIXEDID on the innermost header and describe the
  outer header with SKB_GSO_TCP_FIXEDID_OUTER instead of moving the
  flag on encapsulation and decapsulation (Alexei). Encapsulation then
  needs no change, which also covers ESP tunnel mode with crypto
  offload, missed in v1 (Alexei, Sashiko), the Geneve GRO hint path,
  which sets skb->encapsulation again after iptunnel_pull_offloads()
  (Sashiko), and BPF encapsulation followed by an L3-only decapsulation
  (Sashiko). FOU, which pulls the offloads twice, clears the outer flag
  twice.
- Keep the encap_level reset in gre_gso_segment() and
  __skb_udp_tunnel_segment(): v1 undid commit 5197f3499c47 ("net: Reset
  encap_level to avoid resetting features on inner IP headers"), which
  with remote checksum offload masks NETIF_F_HW_CSUM out of the inner
  features on devices without it in hw_enc_features (Alexei, Sashiko).
- Drop the segment routing changes: the outer header there is IPv6
  (Alexei). This also drops the read of hdr->nexthdr after
  iptunnel_pull_offloads() in ipv6_srh_rcv() that Sashiko reported as a
  use-after-free; ipv6_srh_rcv() is unchanged.
- Convert mlx5 and sfc to the outer flag (Alexei).
- Selftest: add an ESP tunnel mode case offloaded to netdevsim; check
  the inner ID values, not only how many distinct IDs arrive; skip the
  decap cases when the IPIP device cannot be created; fail a case whose
  receiver reports no result; keep IPv6 link-local traffic off the
  tunnels, whose tx_packets decide the GRO skip; remove the temporary
  file on exit; sort the Makefile entry; wrap lines at 80 columns
  (Sashiko, NIPA).
- v1: https://lore.kernel.org/netdev/cover.1792857378.git.julius@bairaktaris.de/

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

 .../networking/segmentation-offloads.rst      |   4 +-
 .../net/ethernet/mellanox/mlx5/core/en_rx.c   |   8 +-
 drivers/net/ethernet/sfc/ef100_tx.c           |  12 +-
 include/linux/netdevice.h                     |   2 +-
 include/linux/skbuff.h                        |   4 +-
 include/net/ip_tunnels.h                      |   1 +
 net/core/filter.c                             |   1 +
 net/ipv4/af_inet.c                            |   8 +-
 net/ipv4/tcp_offload.c                        |   6 +-
 tools/testing/selftests/net/.gitignore        |   1 +
 tools/testing/selftests/net/Makefile          |   2 +
 tools/testing/selftests/net/tcp_fixedid.c     | 411 ++++++++++++++++++
 tools/testing/selftests/net/tcp_fixedid.sh    | 400 +++++++++++++++++
 13 files changed, 836 insertions(+), 24 deletions(-)
 create mode 100644 tools/testing/selftests/net/tcp_fixedid.c
 create mode 100755 tools/testing/selftests/net/tcp_fixedid.sh


base-commit: 30c71476d842b094651bd7712f6c8ee8716147d0
-- 
2.53.0

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

[Julius Bairaktaris]

Commit 3271f19bf7b9 ("net: gso: restore ids of outer ip headers
correctly") made SKB_GSO_TCP_FIXEDID describe the outermost IPv4 header
and added SKB_GSO_TCP_FIXEDID_INNER for the inner header of an
encapsulated packet. Every path that adds or removes a header then has to
move the flags, and none does. A TCP stream that GRO merged with fixed
IDs and that is forwarded into an IPIP tunnel, or into an ESP tunnel mode
SA with crypto offload, is segmented with one outer ID for all segments
and with incrementing inner IDs. Decapsulation leaves the state of the
removed outer header on the new outer one, and the inner header of a GRE
or UDP tunnel, which segmentation reads at encap_level 0, gets the state
of the outer header.

Let SKB_GSO_TCP_FIXEDID describe the innermost IPv4 header again, as it
did before that commit, and replace SKB_GSO_TCP_FIXEDID_INNER with
SKB_GSO_TCP_FIXEDID_OUTER for the outer header of an encapsulated
packet. A new outer header then has no flag and gets incrementing IDs,
so encapsulation needs no change, and decapsulation clears the outer
flag in iptunnel_pull_offloads() and bpf_skb_net_shrink(). Convert mlx5
and sfc, the two drivers that use the inner flag.

Fixes: 3271f19bf7b9 ("net: gso: restore ids of outer ip headers correctly")
Suggested-by: Alexei Starovoitov <alexei.starovoitov@gmail.com>
Signed-off-by: Julius Bairaktaris <julius@bairaktaris.de>
---
 Documentation/networking/segmentation-offloads.rst |  4 ++--
 drivers/net/ethernet/mellanox/mlx5/core/en_rx.c    |  8 ++------
 drivers/net/ethernet/sfc/ef100_tx.c                | 12 +++---------
 include/linux/netdevice.h                          |  2 +-
 include/linux/skbuff.h                             |  4 +++-
 include/net/ip_tunnels.h                           |  1 +
 net/core/filter.c                                  |  1 +
 net/ipv4/af_inet.c                                 |  8 +++++---
 net/ipv4/tcp_offload.c                             |  6 ++++--
 9 files changed, 22 insertions(+), 24 deletions(-)

diff --git a/Documentation/networking/segmentation-offloads.rst b/Documentation/networking/segmentation-offloads.rst
index 25a8b7eca847a..0f20bb643145d 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 22fea4739a2c9..b804a759710e0 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 03005757c060d..162cfa54f2d04 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 435be7880d7e5..7ecf0f9c5f655 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -5536,7 +5536,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 6787fcf6ac3e4..6eb913ebc7d53 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 5cccf4c0e6913..a8e1cd7021fcf 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 52e6ea6d50455..340b2e64d818e 100644
--- a/net/core/filter.c
+++ b/net/core/filter.c
@@ -3867,6 +3867,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 7677b0343abd4..d5769a5fdc314 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 b84fcd07c7230..79190044f6fe9 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;
-- 
2.53.0

Re: [PATCH net v2 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 4 potential
issue(s) to consider.

Critical: 0 · High: 2 · Medium: 1 · Low: 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] The commit message says the case 'Decapsulation leaves the state
  of the removed outer header on the new outer one' is fixed by clearing…
- [High] The patch says 'A new outer header then has no flag and gets
  incrementing IDs, so encapsulation needs no change', and the new…
- [Medium] inet_gso_segment() (net/ipv4/af_inet.c:1405-1410) now uses
  skb->encapsulation alone to decide that the header at encap_level 0 is…
- [Low] bpf_skb_net_shrink() (net/core/filter.c:3870) now clears
  SKB_GSO_TCP_FIXEDID_OUTER whenever a BPF_F_ADJ_ROOM_DECAP_L4_* or…

--

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

This patch makes SKB_GSO_TCP_FIXEDID describe the innermost IPv4 header
again. It replaces SKB_GSO_TCP_FIXEDID_INNER with
SKB_GSO_TCP_FIXEDID_OUTER for the outer header of an encapsulated packet.
Decapsulation clears the outer flag in iptunnel_pull_offloads() and
bpf_skb_net_shrink(), and mlx5 and sfc are converted to the new flag.

[ ... ]

> diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
> index 6787fcf6ac3e4..6eb913ebc7d53 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.

[Severity: High]

Is "nothing to do on encapsulation" still true when the skb is already
encapsulated?

Take a forwarded IPIP stream that a router GRO merges without
terminating the tunnel. ipip_gro_complete() sets skb->encapsulation = 1.
tcp4_gro_complete() then maps the old outer header's fixed-ID bit to
SKB_GSO_TCP_FIXEDID_OUTER.

If that skb is then routed into an IP tunnel,
ipip_tunnel_xmit()->iptunnel_handle_offloads() skips the reset block and
only ORs in the new tunnel type:

net/ipv4/ip_tunnel_core.c:iptunnel_handle_offloads() {
	if (likely(!skb->encapsulation)) {
		skb_reset_inner_headers(skb);
		skb->encapsulation = 1;
	}

	if (skb_is_gso(skb)) {
		...
		skb_shinfo(skb)->gso_type |= gso_type_mask;
		return 0;
	}
}

So SKB_GSO_TCP_FIXEDID_OUTER stays set. IPVS ip_vs_tunnel_xmit() goes
through the same helper.

In software GSO, inet_gso_segment() handles the newly created outermost
header at encap_level 0 with skb->encapsulation set. It reads
SKB_GSO_TCP_FIXEDID_OUTER, which was recorded for the old outer header.

Would every segment then share one ID on the header the tunnel just
built, even though iptunnel_xmit() reserved gso_segs IDs for it?

For IPIP-in-IPIP, ipip_gso_segment() reaches the old outer header (now
the middle one) at encap_level > 0. There it reads SKB_GSO_TCP_FIXEDID,
which holds the innermost header's state. With two flags and three IPv4
headers, the middle header's state can't be represented.

The ESP tunnel mode case doesn't look affected. GRO'd encapsulated skbs
have inner_protocol set, so xfrm_output() segments them through
xfrm_output_gso() before ESP adds its header.

The behaviour here is the same as before the patch. However, the commit
message says the "forwarded into an IPIP tunnel" case is fixed and that
"encapsulation needs no change".

Should the tunnel xmit path clear SKB_GSO_TCP_FIXEDID_OUTER when it adds
a header to an already-encapsulated skb? Or should the comment and the
commit message be narrowed?

>  	 */
>  	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 5cccf4c0e6913..a8e1cd7021fcf 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;

[Severity: High]

The commit message says decapsulation "clears the outer flag in
iptunnel_pull_offloads() and bpf_skb_net_shrink()". Does the netfilter
flowtable IPIP fast path need the same change?

nf_flow_encap_pop()->nf_flow_ip_tunnel_pop() removes the outer IPv4
header of a GRO'd IPIP skb:

net/netfilter/nf_flow_table_ip.c:nf_flow_ip_tunnel_pop() {
	...
	skb_pull(skb, ctx->tun.hdr_size);
	skb_reset_network_header(skb);

	if (ctx->tun.inner_proto == IPPROTO_IPIP)
		skb->protocol = htons(ETH_P_IP);
	...
}

It never touches gso_type or skb->encapsulation.

ipip_gro_complete() set skb->encapsulation = 1, and tcp4_gro_complete()
stored the removed outer header's state in SKB_GSO_TCP_FIXEDID_OUTER. On
transmit, software GSO runs inet_gso_segment() for the remaining header
at encap_level 0. It reads SKB_GSO_TCP_FIXEDID_OUTER and ignores
SKB_GSO_TCP_FIXEDID, which is the flag that describes this header.

Suppose the removed header had fixed IDs and the remaining one did not.
Wouldn't every forwarded segment get the same ID? inet_gro_flush() only
requires DF to match between packets, so that header may have DF clear.
In the reverse case, fixed IDs are rewritten as incrementing.

The result is the same as before the patch. Still, this is the
"Decapsulation leaves the state of the removed outer header on the new
outer one" case that the commit message says is fixed.

Should nf_flow_ip_tunnel_pop() also clear SKB_GSO_TCP_FIXEDID_OUTER, or
skb->encapsulation?

>  	}
>  
>  	skb->encapsulation = 0;
> diff --git a/net/core/filter.c b/net/core/filter.c
> index 52e6ea6d50455..340b2e64d818e 100644
> --- a/net/core/filter.c
> +++ b/net/core/filter.c
> @@ -3867,6 +3867,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;

[Severity: Low]

This isn't a bug, but the documentation no longer matches the code
here.

The new clear only runs when BPF_F_ADJ_ROOM_DECAP_L4_MASK or
BPF_F_ADJ_ROOM_DECAP_IPXIP_MASK flags are passed. The
bpf_skb_adjust_room() documentation in include/uapi/linux/bpf.h lists
which SKB_GSO_* flags each decap flag clears, and this one isn't
included. That documentation also says BPF_F_ADJ_ROOM_DECAP_L3_IPV4 and
BPF_F_ADJ_ROOM_DECAP_L3_IPV6 "only trigger a protocol change without
clearing any tunnel-specific GSO flags".

The new skbuff.h comment says, with no qualification, "Decapsulation
clears the outer flag". After a decap with flags = 0 or with only the
DECAP_L3 flags, both SKB_GSO_TCP_FIXEDID_OUTER and skb->encapsulation
stay set. inet_gso_segment() then applies the removed header's state to
the new outermost header.

Could the uapi documentation list SKB_GSO_TCP_FIXEDID_OUTER, and could
the skbuff.h comment be qualified to match?

>  			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 7677b0343abd4..d5769a5fdc314 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);
> +	}

[Severity: Medium]

Does the else branch hold for ESP transport mode and BEET with crypto
offload?

xfrm_output() sets skb->encapsulation = 1 for every offloaded state. For
transport and BEET GSO skbs it then only adds SKB_GSO_ESP:

net/xfrm/xfrm_output.c:xfrm_output() {
	...
		xfrm_get_inner_ipproto(skb, x);
		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;
	...
}

No second IPv4 header is added.

The stack can fall back to software GSO when the device lacks GSO_ESP
or TSO_MANGLEID, or in validate_xmit_xfrm() after a reroute. In that
case inet_gso_segment() handles the only IPv4 header at encap_level 0
with skb->encapsulation == 1. It takes the else branch and reads
SKB_GSO_TCP_FIXEDID_OUTER.

esp4_gso_segment()->xfrm4_transport_gso_segment() and
xfrm4_beet_gso_segment() call the TCP handler directly, so
SKB_GSO_TCP_FIXEDID is never read on this path.

For a forwarded TCP stream that GRO merged without a tunnel,
tcp4_gro_complete() sets only SKB_GSO_TCP_FIXEDID, because
skb->encapsulation is 0 at GRO time.

Won't these segments now get incrementing IDs even though the stream had
fixed IDs? Before this patch, SKB_GSO_TCP_FIXEDID << encap with
encap == 0 read SKB_GSO_TCP_FIXEDID and the IDs were kept.

>  
>  	ops = rcu_dereference(inet_offloads[proto]);
>  	if (likely(ops && ops->callbacks.gso_segment)) {

[ ... ]

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

[PATCH net v2 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 and into an ESP tunnel
mode SA offloaded to netdevsim, 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, or for ESP increment. 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  | 411 +++++++++++++++++++++
 tools/testing/selftests/net/tcp_fixedid.sh | 400 ++++++++++++++++++++
 4 files changed, 814 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..755f2536a510f 100644
--- a/tools/testing/selftests/net/Makefile
+++ b/tools/testing/selftests/net/Makefile
@@ -107,6 +107,7 @@ TEST_PROGS := \
 	stress_reuseport_listen.sh \
 	tcp_ecmp_failover.sh \
 	tcp_fastopen_backup_key.sh \
+	tcp_fixedid.sh \
 	test_bridge_backup_port.sh \
 	test_bridge_neigh_suppress.sh \
 	test_ingress_egress_chaining.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..25c752beab4ed
--- /dev/null
+++ b/tools/testing/selftests/net/tcp_fixedid.c
@@ -0,0 +1,411 @@
+// 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,
+ * also inside IPIP, GRE, VXLAN or ESP.
+ *
+ * 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 [-f] [-c segs] [-w ms]
+ *
+ * The receiver prints "ready" once it is bound, then the number of
+ * segments, of segments with an outer IPv4 header, of distinct outer and
+ * inner IDs, and of inner IDs the sender did not use: other than the fixed
+ * ID with -f, outside the incrementing range of segs IDs without it.
+ */
+
+#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
+#define INCR_ID		0x1000
+
+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 : INCR_ID + 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 if (proto == IPPROTO_ESP) {
+			/* SPI, sequence number and the 8 byte IV of
+			 * rfc4106(gcm(aes)); netdevsim does not encrypt
+			 */
+			if (l4 + 16 > end)
+				return NULL;
+			p = l4 + 16;
+		} 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 int inner_bad = 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);
+		uint16_t id;
+		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++;
+		id = ntohs(iph->id);
+		count_id(inner_map, &inner_ids, id);
+		if (fixed_id ? id != FIXED_ID : (uint16_t)(id - INCR_ID) >= stop)
+			inner_bad++;
+		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 inner_bad %u\n",
+	       frames, outer4, outer_ids, inner_ids, inner_bad);
+	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..05ef7ec6b08e7
--- /dev/null
+++ b/tools/testing/selftests/net/tcp_fixedid.sh
@@ -0,0 +1,400 @@
+#!/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
+#  esp:         into an ESP tunnel mode SA offloaded to netdevsim, which
+#               segments the packet in software and does not encrypt it
+#
+# The inner IDs must come out as they went in, and the IDs of a new outer
+# IPv4 header must be unique, or for ESP, whose outer IDs start at 0 for
+# each packet with DF set, increment. A run in which GRO merged nothing 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
+	rm -f "$RESULT"
+}
+
+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
+	# no IPv6 link-local traffic in the tx_packets of the tunnel
+	ip netns exec "$ns" sysctl -qw "net.ipv6.conf.$name.disable_ipv6=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"
+}
+
+# fwd_packets <ns>: packets forwarded, a GSO packet counting once
+fwd_packets() {
+	ip netns exec "$1" awk '/^Ip: [0-9]/ { print $7 }' /proc/net/snmp
+}
+
+# check <name> <result> <want: fixed|incr> <outer: 0|unique|incr> <merged>
+check() {
+	local name=$1 res=$2 want=$3 outer=$4 sent=$5
+	local frames outer_ids inner_ids inner_bad
+
+	read -r _ frames _ _ _ outer_ids _ inner_ids _ inner_bad <<< "$res"
+	RET=0
+	if [ -z "$inner_bad" ]; then
+		check_err 1 "no result from the receiver"
+		log_test "$name"
+		return
+	fi
+	check_err $((frames != TOTAL)) "$frames of $TOTAL segments received"
+	check_err "$inner_bad" "$inner_bad inner IDs not sent"
+	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" = unique ]; then
+		check_err $((outer_ids != frames)) \
+			"$outer_ids outer IDs for $frames segments"
+	elif [ "$outer" = incr ]; then
+		check_err $((outer_ids < 2)) "fixed outer IDs"
+	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> <want: fixed|incr>: start the receiver in the
+# background, pid in CAP_PID, and wait until it is bound
+capture() {
+	local f="" i
+
+	[ "$3" = fixed ] && f=-f
+	: > "$RESULT"
+	ip netns exec "$1" "$TX" -R -i "$2" -c "$TOTAL" -w 3000 $f > "$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
+	if ! tunnel "$NRTR" tun0 "$type" "$family" "$laddr" "$raddr"; then
+		log_test_skip "encap $type IPv$family"
+		cleanup_all_ns
+		return
+	fi
+	route_into "$NRTR" tun0
+	noseg "$NRTR" r1
+
+	r0mac=$(ip -n "$NRTR" -br link show r0 | awk '{ print $3 }')
+	capture "$NMID" m1 "$want"
+	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 ] && echo unique || echo 0)" \
+		$(($(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
+	if ! tunnel "$NRTR" tun0 ipip 4 10.2.0.1 10.2.0.2; then
+		log_test_skip "decap"
+		cleanup_all_ns
+		return
+	fi
+	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 "$want"
+	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" unique \
+		$(($(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 fixed
+	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
+}
+
+NSIM_ID=$((RANDOM % 1024 + 1024))
+NSIM_BUS=/sys/bus/netdevsim
+
+# nsim_add <id> <ns>: create a netdevsim port in <ns>, print its name
+nsim_add() {
+	local dev
+
+	echo "$1 1" > "$NSIM_BUS/new_device" || return 1
+	for dev in "$NSIM_BUS/devices/netdevsim$1/net/"*; do
+		dev=${dev##*/}
+		ip link set "$dev" netns "$2" || return 1
+		echo "$dev"
+		return 0
+	done
+	return 1
+}
+
+nsim_del() {
+	echo "$NSIM_ID" > "$NSIM_BUS/del_device" 2>/dev/null
+	echo $((NSIM_ID + 1)) > "$NSIM_BUS/del_device" 2>/dev/null
+}
+
+# run_esp: NRTR forwards into an ESP tunnel mode SA offloaded to a
+# netdevsim port with TSO off, linked to a netdevsim port in NMID
+run_esp() {
+	local key=0x3132333435363738393031323334353664636261
+	local r0mac rdev mdev rfd mfd t0
+
+	setup || { log_test_skip "esp"; return; }
+	if ! modprobe -q netdevsim 2>/dev/null && [ ! -w "$NSIM_BUS/new_device" ]; then
+		log_test_skip "esp: no netdevsim"
+		cleanup_all_ns
+		return
+	fi
+	if ! rdev=$(nsim_add "$NSIM_ID" "$NRTR") ||
+	   ! mdev=$(nsim_add $((NSIM_ID + 1)) "$NMID"); then
+		log_test_skip "esp: netdevsim"
+		nsim_del
+		cleanup_all_ns
+		return
+	fi
+	exec {rfd}<"/run/netns/$NRTR" {mfd}<"/run/netns/$NMID"
+	echo "$rfd:$(ip netns exec "$NRTR" cat "/sys/class/net/$rdev/ifindex")" \
+	     "$mfd:$(ip netns exec "$NMID" cat "/sys/class/net/$mdev/ifindex")" \
+	     > "$NSIM_BUS/link_device"
+	exec {rfd}<&- {mfd}<&-
+
+	ip -n "$NRTR" addr add 10.5.0.1/24 dev "$rdev"
+	ip -n "$NMID" addr add 10.5.0.2/24 dev "$mdev"
+	ip -n "$NRTR" link set "$rdev" up
+	ip -n "$NMID" link set "$mdev" up
+	ip -n "$NRTR" neigh add 10.5.0.2 dev "$rdev" lladdr \
+		"$(ip -n "$NMID" -br link show "$mdev" | awk '{ print $3 }')"
+	ip -n "$NRTR" route add "$INNER_DST/32" via 10.5.0.2 dev "$rdev"
+	ip netns exec "$NRTR" ethtool -K "$rdev" tso off >/dev/null
+	if ! ip -n "$NRTR" xfrm state add src 10.5.0.1 dst 10.5.0.2 \
+		proto esp spi 0x100 reqid 1 mode tunnel \
+		aead 'rfc4106(gcm(aes))' "$key" 128 \
+		offload dev "$rdev" dir out ||
+	   ! ip -n "$NRTR" xfrm policy add dir out \
+		src "$INNER_SRC/32" dst "$INNER_DST/32" \
+		tmpl src 10.5.0.1 dst 10.5.0.2 proto esp reqid 1 mode tunnel; then
+		log_test_skip "esp: no ESP offload"
+		nsim_del
+		cleanup_all_ns
+		return
+	fi
+
+	r0mac=$(ip -n "$NRTR" -br link show r0 | awk '{ print $3 }')
+	capture "$NMID" "$mdev" fixed
+	t0=$(fwd_packets "$NRTR")
+	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 "esp tunnel offload, fixed inner IDs" "$(tail -n 1 "$RESULT")" \
+		fixed incr $(($(fwd_packets "$NRTR") - t0))
+	cleanup_all_ns
+	nsim_del
+}
+
+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
+run_esp
+
+exit "$EXIT_STATUS"
-- 
2.53.0

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

[netdev-bot+sashiko]

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

Critical: 0 · High: 0 · Medium: 2 · Low: 4

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).

- [Medium] The header comment of tcp_fixedid.sh (lines 18-21) and the
  commit message both say that a run in which GRO merged nothing is…
- [Medium] When run_esp() hits an error, it calls nsim_del() (lines 334
  and 360).
- [Low] In do_rx() in tcp_fixedid.c, the check for incrementing IDs is
  `(uint16_t)(id - INCR_ID) >= stop` (line 311).
- [Low] The EXIT trap handler cleanup() in tcp_fixedid.sh (lines 44-47)
  only runs cleanup_all_ns and removes $RESULT.
- [Low] On the success path, run_esp() runs cleanup_all_ns (line 373)
  before nsim_del (line 374), which is the reverse of setup order.
- [Low] selftests: net: tcp_fixedid.sh duplicates lib.sh netdevsim and
  wait helpers

--

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

This adds a packet socket helper and a script. The script sends TCP
segments with fixed or incrementing IPv4 IDs through veth GRO into IPIP,
GRE, VXLAN and netdevsim-offloaded ESP tunnels. It then checks that inner
IDs are preserved and that outer IDs are unique or increment, and it
skips runs in which GRO merged nothing.

> diff --git a/tools/testing/selftests/net/tcp_fixedid.c b/tools/testing/selftests/net/tcp_fixedid.c
> new file mode 100644
> index 0000000000000..25c752beab4ed
> --- /dev/null
> +++ b/tools/testing/selftests/net/tcp_fixedid.c

[ ... ]

> +static int do_rx(int fd)
> +{

[ ... ]

> +		frames++;
> +		id = ntohs(iph->id);
> +		count_id(inner_map, &inner_ids, id);
> +		if (fixed_id ? id != FIXED_ID : (uint16_t)(id - INCR_ID) >= stop)
> +			inner_bad++;

[Severity: Low]
What happens here when the receiver runs without -c? Only -c sets stop,
so it defaults to 0:

static int segs = 40, bursts = 10, len = 1000, wait_ms = 2000, stop;

With stop == 0 and no -f, (uint16_t)(id - INCR_ID) >= 0 is always true.
Every frame then counts as inner_bad, even when the IDs are correct.

The usage comment lists [-c segs] as optional for rx. capture() in
tcp_fixedid.sh always passes -c "$TOTAL", so this only affects running
the helper by hand as documented.

Should rx without -f require -c, or fall back to some other range?

[ ... ]

> diff --git a/tools/testing/selftests/net/tcp_fixedid.sh b/tools/testing/selftests/net/tcp_fixedid.sh
> new file mode 100755
> index 0000000000000..05ef7ec6b08e7
> --- /dev/null
> +++ b/tools/testing/selftests/net/tcp_fixedid.sh

[ ... ]

> +cleanup() {
> +	cleanup_all_ns
> +	rm -f "$RESULT"
> +}
> +
> +trap cleanup EXIT

[Severity: Low]
Can this EXIT trap leave behind the netdevsim devices that run_esp()
creates? Only run_esp()'s own return paths call nsim_del(). If the script
is killed during run_esp(), for example by a runner timeout or Ctrl-C,
the trap only deletes the namespaces.

netdevsim ports have no rtnl_link_ops, so default_device_exit_net() moves
them into init_net instead of destroying them:

net/core/dev.c:default_device_exit_net() {
    ...
		/* Leave virtual devices for the generic cleanup */
		if (dev->rtnl_link_ops && !dev->rtnl_link_ops->netns_refund)
			continue;

		/* Push remaining network devices to init_net */
    ...
}

That leaves two netdevsim bus devices and their netdevs in the host
namespace. Could cleanup() also call nsim_del?

[ ... ]

> +# check <name> <result> <want: fixed|incr> <outer: 0|unique|incr> <merged>
> +check() {

[ ... ]

> +	if [ "$outer" = unique ]; then
> +		check_err $((outer_ids != frames)) \
> +			"$outer_ids outer IDs for $frames segments"
> +	elif [ "$outer" = incr ]; then
> +		check_err $((outer_ids < 2)) "fixed outer IDs"
> +	fi
> +	if [ "$RET" -eq 0 ] && [ "$sent" -ge "$TOTAL" ]; then
> +		ret_set_ksft_status "$ksft_skip" "GRO did not merge"
> +	fi
> +	log_test "$name"
> +}

[Severity: Medium]
Does the ESP case report FAIL instead of SKIP when GRO merged nothing?
The commit message says:

  A test where GRO merged nothing is skipped.

The header comment says the same.

Without merging, each forwarded skb carries one segment.
xfrm4_tunnel_encap_add() copies DF from the inner header and calls
ip_select_ident(dev_net(dst->dev), skb, NULL). ip_select_ident_segs()
then does:

	if ((iph->frag_off & htons(IP_DF)) && !skb->ignore_df) {
		iph->id = 0;
		return;
	}

Every ESP outer ID is therefore 0 and outer_ids is 1. The outer=incr
check sets RET=1 before the skip test, so the RET -eq 0 condition never
lets run_esp() skip. The unique and 0 outer modes do skip correctly in
the same situation.

Also, the header comment says the ESP outer IDs must "increment".
outer_ids < 2 only checks that at least two distinct outer IDs were
seen. Is a stricter check intended here?

[ ... ]

> +nsim_del() {
> +	echo "$NSIM_ID" > "$NSIM_BUS/del_device" 2>/dev/null
> +	echo $((NSIM_ID + 1)) > "$NSIM_BUS/del_device" 2>/dev/null
> +}

[ ... ]

> +	if ! rdev=$(nsim_add "$NSIM_ID" "$NRTR") ||
> +	   ! mdev=$(nsim_add $((NSIM_ID + 1)) "$NMID"); then
> +		log_test_skip "esp: netdevsim"
> +		nsim_del
> +		cleanup_all_ns
> +		return
> +	fi

[Severity: Medium]
Can this delete a netdevsim device that another test or the user
created? NSIM_ID is picked at random from 1024..2047.
nsim_bus_dev_new() allocates the ID with ida_alloc_range(id, id), so
new_device fails if that ID is already in use.

In that case nsim_add() returns 1, and nsim_del() writes both NSIM_ID and
NSIM_ID + 1 to del_device, whether or not this run created them.

The same applies to NSIM_ID + 1 if the second nsim_add() collides. It
also applies if the first nsim_add() creates NSIM_ID and then ip link
set netns fails.

Could nsim_del() remove only the IDs that nsim_add() actually created?

[ ... ]

> +	check "esp tunnel offload, fixed inner IDs" "$(tail -n 1 "$RESULT")" \
> +		fixed incr $(($(fwd_packets "$NRTR") - t0))
> +	cleanup_all_ns
> +	nsim_del
> +}

[Severity: Low]
Is this teardown order intended? The success path runs cleanup_all_ns
before nsim_del, but the error paths in run_esp() run nsim_del first.

Because the namespaces are deleted first, default_device_exit_net()
moves the netdevsim ports into init_net. They briefly appear as host
interfaces until nsim_del removes them.

Would calling nsim_del before cleanup_all_ns here match the error paths?

[ ... ]

> @@ -0,0 +1,400 @@
> +#!/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
> +#  esp:         into an ESP tunnel mode SA offloaded to netdevsim, which
> +#               segments the packet in software and does not encrypt it
> +#
> +# The inner IDs must come out as they went in, and the IDs of a new outer
> +# IPv4 header must be unique, or for ESP, whose outer IDs start at 0 for
> +# each packet with DF set, increment. A run in which GRO merged nothing 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
> +	rm -f "$RESULT"
> +}
> +
> +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
> +	# no IPv6 link-local traffic in the tx_packets of the tunnel
> +	ip netns exec "$ns" sysctl -qw "net.ipv6.conf.$name.disable_ipv6=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"
> +}
> +
> +# fwd_packets <ns>: packets forwarded, a GSO packet counting once
> +fwd_packets() {
> +	ip netns exec "$1" awk '/^Ip: [0-9]/ { print $7 }' /proc/net/snmp
> +}
> +
> +# check <name> <result> <want: fixed|incr> <outer: 0|unique|incr> <merged>
> +check() {
> +	local name=$1 res=$2 want=$3 outer=$4 sent=$5
> +	local frames outer_ids inner_ids inner_bad
> +
> +	read -r _ frames _ _ _ outer_ids _ inner_ids _ inner_bad <<< "$res"
> +	RET=0
> +	if [ -z "$inner_bad" ]; then
> +		check_err 1 "no result from the receiver"
> +		log_test "$name"
> +		return
> +	fi
> +	check_err $((frames != TOTAL)) "$frames of $TOTAL segments received"
> +	check_err "$inner_bad" "$inner_bad inner IDs not sent"
> +	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" = unique ]; then
> +		check_err $((outer_ids != frames)) \
> +			"$outer_ids outer IDs for $frames segments"
> +	elif [ "$outer" = incr ]; then
> +		check_err $((outer_ids < 2)) "fixed outer IDs"
> +	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> <want: fixed|incr>: start the receiver in the
> +# background, pid in CAP_PID, and wait until it is bound
> +capture() {
> +	local f="" i
> +
> +	[ "$3" = fixed ] && f=-f
> +	: > "$RESULT"
> +	ip netns exec "$1" "$TX" -R -i "$2" -c "$TOTAL" -w 3000 $f > "$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
> +	if ! tunnel "$NRTR" tun0 "$type" "$family" "$laddr" "$raddr"; then
> +		log_test_skip "encap $type IPv$family"
> +		cleanup_all_ns
> +		return
> +	fi
> +	route_into "$NRTR" tun0
> +	noseg "$NRTR" r1
> +
> +	r0mac=$(ip -n "$NRTR" -br link show r0 | awk '{ print $3 }')
> +	capture "$NMID" m1 "$want"
> +	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 ] && echo unique || echo 0)" \
> +		$(($(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
> +	if ! tunnel "$NRTR" tun0 ipip 4 10.2.0.1 10.2.0.2; then
> +		log_test_skip "decap"
> +		cleanup_all_ns
> +		return
> +	fi
> +	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 "$want"
> +	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" unique \
> +		$(($(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 fixed
> +	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
> +}
> +
> +NSIM_ID=$((RANDOM % 1024 + 1024))
> +NSIM_BUS=/sys/bus/netdevsim
> +
> +# nsim_add <id> <ns>: create a netdevsim port in <ns>, print its name
> +nsim_add() {
> +	local dev
> +
> +	echo "$1 1" > "$NSIM_BUS/new_device" || return 1
> +	for dev in "$NSIM_BUS/devices/netdevsim$1/net/"*; do
> +		dev=${dev##*/}
> +		ip link set "$dev" netns "$2" || return 1
> +		echo "$dev"
> +		return 0
> +	done
> +	return 1
> +}
> +
> +nsim_del() {
> +	echo "$NSIM_ID" > "$NSIM_BUS/del_device" 2>/dev/null
> +	echo $((NSIM_ID + 1)) > "$NSIM_BUS/del_device" 2>/dev/null
> +}
> +
> +# run_esp: NRTR forwards into an ESP tunnel mode SA offloaded to a
> +# netdevsim port with TSO off, linked to a netdevsim port in NMID
> +run_esp() {
> +	local key=0x3132333435363738393031323334353664636261
> +	local r0mac rdev mdev rfd mfd t0
> +
> +	setup || { log_test_skip "esp"; return; }
> +	if ! modprobe -q netdevsim 2>/dev/null && [ ! -w "$NSIM_BUS/new_device" ]; then
> +		log_test_skip "esp: no netdevsim"
> +		cleanup_all_ns
> +		return
> +	fi
> +	if ! rdev=$(nsim_add "$NSIM_ID" "$NRTR") ||
> +	   ! mdev=$(nsim_add $((NSIM_ID + 1)) "$NMID"); then
> +		log_test_skip "esp: netdevsim"
> +		nsim_del
> +		cleanup_all_ns
> +		return
> +	fi
> +	exec {rfd}<"/run/netns/$NRTR" {mfd}<"/run/netns/$NMID"
> +	echo "$rfd:$(ip netns exec "$NRTR" cat "/sys/class/net/$rdev/ifindex")" \
> +	     "$mfd:$(ip netns exec "$NMID" cat "/sys/class/net/$mdev/ifindex")" \
> +	     > "$NSIM_BUS/link_device"
> +	exec {rfd}<&- {mfd}<&-
> +
> +	ip -n "$NRTR" addr add 10.5.0.1/24 dev "$rdev"
> +	ip -n "$NMID" addr add 10.5.0.2/24 dev "$mdev"
> +	ip -n "$NRTR" link set "$rdev" up
> +	ip -n "$NMID" link set "$mdev" up
> +	ip -n "$NRTR" neigh add 10.5.0.2 dev "$rdev" lladdr \
> +		"$(ip -n "$NMID" -br link show "$mdev" | awk '{ print $3 }')"
> +	ip -n "$NRTR" route add "$INNER_DST/32" via 10.5.0.2 dev "$rdev"
> +	ip netns exec "$NRTR" ethtool -K "$rdev" tso off >/dev/null
> +	if ! ip -n "$NRTR" xfrm state add src 10.5.0.1 dst 10.5.0.2 \
> +		proto esp spi 0x100 reqid 1 mode tunnel \
> +		aead 'rfc4106(gcm(aes))' "$key" 128 \
> +		offload dev "$rdev" dir out ||
> +	   ! ip -n "$NRTR" xfrm policy add dir out \
> +		src "$INNER_SRC/32" dst "$INNER_DST/32" \
> +		tmpl src 10.5.0.1 dst 10.5.0.2 proto esp reqid 1 mode tunnel; then
> +		log_test_skip "esp: no ESP offload"
> +		nsim_del
> +		cleanup_all_ns
> +		return
> +	fi
> +
> +	r0mac=$(ip -n "$NRTR" -br link show r0 | awk '{ print $3 }')
> +	capture "$NMID" "$mdev" fixed
> +	t0=$(fwd_packets "$NRTR")
> +	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 "esp tunnel offload, fixed inner IDs" "$(tail -n 1 "$RESULT")" \
> +		fixed incr $(($(fwd_packets "$NRTR") - t0))
> +	cleanup_all_ns
> +	nsim_del
> +}
> +
> +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
> +run_esp
> +
> +exit "$EXIT_STATUS"

[Severity: Low]
The script already sources lib.sh, and lib.sh has helpers for most
of what is open-coded here:

  - create_netdevsim() and cleanup_netdevsim() cover nsim_add() and
    nsim_del()
  - slowwait() covers the readiness loop in capture()
  - check_gen_prog() covers the [ ! -x "$TX" ] test before the main
    loop

The netdevsim case is not just style. nsim_add() writes new_device in
the initial namespace. It then runs ip link set netns on whatever name
the glob finds under devices/netdevsim$1/net/.

On a host running udevd, the interface can be renamed between the glob
and the ip link call. The move then fails, or nsim_add() prints a
stale name. Either way run_esp() skips, or later commands act on the
wrong name.

create_netdevsim() avoids this. It runs udevadm settle and writes
new_device from inside the target namespace via ip netns exec, so the
port never appears in init_net. cleanup_netdevsim() also checks that
the device exists before writing del_device.

Could nsim_add(), nsim_del(), the capture() wait and the TX check use
the lib.sh helpers instead? create_netdevsim() takes the ID and the
namespace as arguments, so run_esp() should still know both IDs for
the link_device write. Is there a reason the lib.sh versions do not
fit here?

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

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

[netdev-bot+sinfo]

Hi!

This is an automated message. This series looks like a fix, but its
commit messages seem to be missing some information:

 - How the issue was discovered, e.g. hit in production, hit during
   development, syzbot report, manual code inspection, LLM or static
   analysis tool scan.

Please do not repost the series just to address the above. Instead,
reply to this email with the missing information, so that reviewers
can take it into account.

*IMPORTANT*: If you do repost (for other reasons) - please include
the relevant info in the cover letter or commit messages.
-- 
The evaluation is done by an LLM so it may be wrong, if you think
that is the case please reply and explain.

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

[Julius Bairaktaris]

On Sat, 14 Nov 2026 18:33:19 +0000, netdev-bot+sinfo@kernel.org wrote:
> - How the issue was discovered, e.g. hit in production, hit during
>   development, syzbot report, manual code inspection, LLM or static
>   analysis tool scan.

Reported by the hosted Sashiko AI review of v2 2/3 of my "ipmr: forward
GSO multicast packets" series:
https://lore.kernel.org/netdev/179182559332.434549.18294510093742639467@kernel.org/
I then reproduced it with the selftest in patch 2: on net, 6 of its 18
cases fail in 2 of 2 runs.

Julius

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

[Alexei Starovoitov]

On Sat, Nov 14, 2026 at 10:27 AM Julius Bairaktaris <julius@bairaktaris.de> wrote:
> Patch 1 lets SKB_GSO_TCP_FIXEDID describe the innermost header again and
> replaces the inner flag with SKB_GSO_TCP_FIXEDID_OUTER, as Alexei
> suggested in review of v1. Patch 2 adds a selftest.

Thx. lgtm.