Linux Kernel Selftest development
 help / color / mirror / Atom feed
* [PATCH net v4 0/4] net: hsr: fix GRO/GSO super-packet handling
@ 2026-08-03 22:22 Xin Xie
  2026-08-03 22:22 ` [PATCH net v4 1/4] net: hsr: fix packet drops caused by GRO superpackets Xin Xie
                   ` (3 more replies)
  0 siblings, 4 replies; 7+ messages in thread
From: Xin Xie @ 2026-08-03 22:22 UTC (permalink / raw)
  To: netdev, linux-kselftest, linux-kernel
  Cc: davem, edumazet, kuba, pabeni, horms, andrew+netdev, shuah, kees,
	petr.wozniak, qingfang.deng, fmaurer, luka.gejak, bigeasy,
	xiaoliang.yang_1, skhawaja, stable, sdf.kernel, Xin Xie

HSR/PRP requires per-wire-frame tags/RCTs and sequence numbers, and
duplicate discard is per frame. RX GRO and TX GSO can present
multiple frames as one skb and violate that assumption: a super-skb
is either rejected by a constrained lower device, or forwarded
without valid per-frame trailers and sequence numbers.

Patch 1 adds netif_disable_gro()/dev_disable_gro() and disables
GRO and GRO_HW on lower devices at HSR/PRP enslavement time,
mirroring the existing LRO treatment.

Patch 2 shrinks hsr->seqnr_lock from whole hsr_forward_skb() calls
to the sequence counter updates, so the segmentation work of
patch 3 never runs under the global sequence lock. The outer lock
also incidentally serialized the tx statistics updates in
hsr_forward_skb(); those now use the atomic DEV_STATS_* helpers.

Patch 3 unfolds the remaining GSO super-packets at the forward
entry with the top-level GSO dispatch (__skb_gso_segment()), so
each wire frame gets its own tag and sequence number. The HSR
master also drops NETIF_F_GSO_MASK from its advertised features so
locally generated traffic is segmented as early as possible.

Patch 1 provides the normal setup-time default. Patch 3 is the
fail-safe when a privileged override or another source still
presents a GSO/GRO super-packet: plain-Ethernet super-packets from
the master or the untagged SAN side are segmented, while
super-packets from tagged LAN ingress are rejected by ingress-port
policy.

Patch 4 adds a kselftest covering the series.

Patch 1 is independent. Patch 3 depends on patch 2, and patches 2
and 3 are selected for stable only on 7.0 and newer, where
sparse-bitmap duplicate discard accepts out-of-order arrival.
Older branches need adapted backports.

Validation:
* the v4 kernel builds cleanly, and W=1 allmodconfig/allyesconfig
  base-vs-patched shows no new warnings;
* hsr_gro_superpacket passes on the v4 kernel and fails on the
  base kernel with the expected mechanism;
* hsr_ping, hsr_redbox, link_faults and prp_ping all pass;
* no kernel WARN/BUG/Oops, no leftover namespaces.

Note on the contest report: the contest conflict is with the
already merged PRP RedBox work.

- In send_prp_supervision_frame(), preserve the Type-30 RedBox-MAC
  TLV and EOT construction, release seqnr_lock immediately after
  the sequence-number update, and remove the later stale unlocks.
- Insert hsr_gro_superpacket.sh at the sorted Makefile position.

This composition builds and both the GRO/GSO and PRP RedBox
selftests pass on the tested net-next tree (69963a0678a3).

---

Changes in v4:
- Patches 2/3: the four tx statistics updates affected by the
  seqnr_lock shrink now use DEV_STATS_*, so they cannot lose
  increments on the concurrently callable forwarding paths.
- Patch 4: three namespace variables are explicitly initialized
  (empty), clearing the ShellCheck SC2154 findings.
- Patch 1 is payload-identical to v3 and carries Ali Ahmet
  Memis's Reviewed-by and Tested-by from the v3 thread; his
  Reviewed-by is not carried on the modified patches 2-4.

Previous postings (newest first):
v3: https://lore.kernel.org/netdev/20260731090224.18-1-xiexinet@gmail.com/
v2: https://lore.kernel.org/netdev/20260724161253.79-1-xiexinet@gmail.com/
v1: https://lore.kernel.org/netdev/20260722171836.196-1-xiexinet@gmail.com/

Xin Xie (4):
  net: hsr: fix packet drops caused by GRO superpackets
  net: hsr: shrink seqnr_lock to sequence counter updates
  net: hsr: unfold GSO super-packets at the forward entry
  selftests: net: hsr: add GRO super-packet forwarding test

 include/linux/netdevice.h                     |   2 +
 net/core/dev.c                                |  18 +
 net/core/dev_api.c                            |  16 +
 net/hsr/hsr_device.c                          |  17 +-
 net/hsr/hsr_forward.c                         |  59 ++-
 net/hsr/hsr_slave.c                           |  12 +-
 tools/testing/selftests/net/hsr/Makefile      |   1 +
 .../selftests/net/hsr/hsr_gro_superpacket.sh  | 465 ++++++++++++++++++
 8 files changed, 563 insertions(+), 27 deletions(-)
 create mode 100755 tools/testing/selftests/net/hsr/hsr_gro_superpacket.sh

-- 
2.43.0


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

* [PATCH net v4 1/4] net: hsr: fix packet drops caused by GRO superpackets
  2026-08-03 22:22 [PATCH net v4 0/4] net: hsr: fix GRO/GSO super-packet handling Xin Xie
@ 2026-08-03 22:22 ` Xin Xie
  2026-08-03 22:22 ` [PATCH net v4 2/4] net: hsr: shrink seqnr_lock to sequence counter updates Xin Xie
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 7+ messages in thread
From: Xin Xie @ 2026-08-03 22:22 UTC (permalink / raw)
  To: netdev, linux-kselftest, linux-kernel
  Cc: davem, edumazet, kuba, pabeni, horms, andrew+netdev, shuah, kees,
	petr.wozniak, qingfang.deng, fmaurer, luka.gejak, bigeasy,
	xiaoliang.yang_1, skhawaja, stable, sdf.kernel, Xin Xie,
	Ali Ahmet Memis

HSR/PRP append a 6-byte tag/RCT to every forwarded frame and process
each frame individually (sequence numbering, duplicate discard). When
a lower device aggregates received frames into a GRO super-packet --
in software, or in hardware on GRO_HW-capable NICs -- the HSR
receive/forward path sees a single skb instead of the individual
frames. Depending on the lower device, that super-skb is then either
rejected outright (for example when it exceeds the lower MTU at
egress), or forwarded without valid per-wire-frame HSR/PRP
processing: its single trailing tag/RCT cannot represent the
per-frame trailers and sequence numbers of the aggregated frames. On
memory-constrained devices, processing super-skbs in softirq context
can also pressure atomic memory allocation.

The HSR/PRP stack already disables LRO on enslaved devices for the
same reason. Extend that treatment to GRO: add netif_disable_gro()
and dev_disable_gro() mirroring netif_disable_lro()/dev_disable_lro(),
and call dev_disable_gro() from hsr_portdev_setup() so enslavement to
an HSR/PRP master automatically strips NETIF_F_GRO and NETIF_F_GRO_HW
on the lower device (recursively on its own lowers, as with LRO).

This is a setup-time default, not an immutable feature policy: a
later privileged feature override, or a lower newly attached below a
stacked slave, can re-enable GRO without re-walking the HSR
enslavement path. Patch 3 is the fail-safe for that case: a GSO skb
that nevertheless reaches the forward entry is segmented on the
plain master/interlink paths or rejected on the LAN/tagged paths, so
invalid aggregates are never forwarded as-is -- though a later
override can still cost traffic on a LAN ingress.

Fixes: f421436a591d ("net/hsr: Add support for the High-availability Seamless Redundancy protocol (HSRv0)")
Cc: stable@vger.kernel.org
Reviewed-by: Ali Ahmet Memis <ali@iusegentoo.com>
Tested-by: Ali Ahmet Memis <ali@iusegentoo.com>
Signed-off-by: Xin Xie <xiexinet@gmail.com>
---
 include/linux/netdevice.h |  2 ++
 net/core/dev.c            | 18 ++++++++++++++++++
 net/core/dev_api.c        | 16 ++++++++++++++++
 net/hsr/hsr_slave.c       |  1 +
 4 files changed, 37 insertions(+)

diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index 9981d637f8b5..eba2c26a49ba 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -3434,6 +3434,8 @@ void dev_close(struct net_device *dev);
 void netif_close_many(struct list_head *head, bool unlink);
 void netif_disable_lro(struct net_device *dev);
 void dev_disable_lro(struct net_device *dev);
+void netif_disable_gro(struct net_device *dev);
+void dev_disable_gro(struct net_device *dev);
 int dev_loopback_xmit(struct net *net, struct sock *sk, struct sk_buff *newskb);
 u16 dev_pick_tx_zero(struct net_device *dev, struct sk_buff *skb,
 		     struct net_device *sb_dev);
diff --git a/net/core/dev.c b/net/core/dev.c
index 5933c5dab09e..a6cf2adc8625 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -1840,6 +1840,24 @@ void netif_disable_lro(struct net_device *dev)
 	}
 }
 
+void netif_disable_gro(struct net_device *dev)
+{
+	struct net_device *lower_dev;
+	struct list_head *iter;
+
+	dev->wanted_features &= ~(NETIF_F_GRO | NETIF_F_GRO_HW);
+	netdev_update_features(dev);
+
+	if (unlikely(dev->features & (NETIF_F_GRO | NETIF_F_GRO_HW)))
+		netdev_WARN(dev, "failed to disable GRO!\n");
+
+	netdev_for_each_lower_dev(dev, lower_dev, iter) {
+		netdev_lock_ops(lower_dev);
+		netif_disable_gro(lower_dev);
+		netdev_unlock_ops(lower_dev);
+	}
+}
+
 /**
  *	dev_disable_gro_hw - disable HW Generic Receive Offload on a device
  *	@dev: device
diff --git a/net/core/dev_api.c b/net/core/dev_api.c
index 437947dd08ed..02fb21629512 100644
--- a/net/core/dev_api.c
+++ b/net/core/dev_api.c
@@ -269,6 +269,22 @@ void dev_disable_lro(struct net_device *dev)
 }
 EXPORT_SYMBOL(dev_disable_lro);
 
+/**
+ * dev_disable_gro() - disable Generic Receive Offload on a device
+ * @dev: device
+ *
+ * Disable Generic Receive Offload (GRO) on a net device.  Must be
+ * called under RTNL.  This is needed if received packets may be
+ * forwarded to another interface.
+ */
+void dev_disable_gro(struct net_device *dev)
+{
+	netdev_lock_ops(dev);
+	netif_disable_gro(dev);
+	netdev_unlock_ops(dev);
+}
+EXPORT_SYMBOL(dev_disable_gro);
+
 /**
  * dev_set_promiscuity() - update promiscuity count on a device
  * @dev: device
diff --git a/net/hsr/hsr_slave.c b/net/hsr/hsr_slave.c
index 01c73b4b50dd..da06b21cdf51 100644
--- a/net/hsr/hsr_slave.c
+++ b/net/hsr/hsr_slave.c
@@ -170,6 +170,7 @@ static int hsr_portdev_setup(struct hsr_priv *hsr, struct net_device *dev,
 	if (res)
 		goto fail_rx_handler;
 	dev_disable_lro(dev);
+	dev_disable_gro(dev);
 
 	return 0;
 
-- 
2.43.0


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

* [PATCH net v4 2/4] net: hsr: shrink seqnr_lock to sequence counter updates
  2026-08-03 22:22 [PATCH net v4 0/4] net: hsr: fix GRO/GSO super-packet handling Xin Xie
  2026-08-03 22:22 ` [PATCH net v4 1/4] net: hsr: fix packet drops caused by GRO superpackets Xin Xie
@ 2026-08-03 22:22 ` Xin Xie
  2026-08-07 14:27   ` Hangbin Liu
  2026-08-03 22:22 ` [PATCH net v4 3/4] net: hsr: unfold GSO super-packets at the forward entry Xin Xie
  2026-08-03 22:22 ` [PATCH net v4 4/4] selftests: net: hsr: add GRO super-packet forwarding test Xin Xie
  3 siblings, 1 reply; 7+ messages in thread
From: Xin Xie @ 2026-08-03 22:22 UTC (permalink / raw)
  To: netdev, linux-kselftest, linux-kernel
  Cc: davem, edumazet, kuba, pabeni, horms, andrew+netdev, shuah, kees,
	petr.wozniak, qingfang.deng, fmaurer, luka.gejak, bigeasy,
	xiaoliang.yang_1, skhawaja, stable, sdf.kernel, Xin Xie

hsr->seqnr_lock is currently held across entire hsr_forward_skb()
calls: master TX (hsr_dev_xmit()), interlink RX (hsr_handle_frame()),
and both supervision frame builders hold it while frames are built,
classified, duplicated and forwarded on every port. The only state
that actually needs the lock is the sequence counters themselves
(hsr->sequence_nr / hsr->sup_sequence_nr).

Shrink the locking to the individual counter updates:
handle_std_frame() now takes the lock around its sequence number
allocation (replacing the lockdep assertion), the master TX and
interlink RX paths drop their outer lock, and the supervision
builders release the lock right after updating their counter instead
of holding it across frame construction and forwarding.

Statistics: the outer seqnr_lock also incidentally serialized the
tx_packets/tx_bytes/tx_dropped updates in hsr_forward_skb(), which are
reachable from the concurrently callable master-TX, supervision and
interlink-RX paths. Those updates now use DEV_STATS_INC()/
DEV_STATS_ADD(), the atomic legacy-stat helpers, so narrowing the lock
does not turn them into lossy plain read-modify-writes.

Ordering: with IFF_NO_QUEUE and dev->lltx, hsr_dev_xmit() is
concurrently callable, and this change removes the old
allocation-through-forward ordering guarantee there: master-TX frames
may now be emitted out of sequence-allocation order. On the current
tree this is safe because duplicate discard tracks individual
sequence numbers in sparse bitmaps (commit aae9d6b616b5 ("hsr:
Implement more robust duplicate discard for HSR") and
commit 415e6367512b ("hsr: Implement more robust duplicate discard
for PRP")) rather than requiring monotonic arrival. Sequence numbers
remain unique and monotonically allocated per counter.

This is a latency/critical-section prerequisite for unfolding GSO
super-packets at the forward entry (not a functional prerequisite):
the segmentation work should not extend the global sequence lock's
critical section. Patch 3 depends on this change; their automatic
stable selection is limited to 7.0 and newer, where sparse-bitmap
duplicate discard accepts out-of-order arrival. Older stable branches
require an adapted backport.

History of the lock being narrowed: it was introduced by
commit 06afd2c31d33 ("hsr: Synchronize sending frames to have always
incremented outgoing seq nr.") and briefly removed by
commit b3c9e65eb227 ("net: hsr: remove seqnr_lock") in net. Merge
commit 46ae4d0a4897 ("Merge git://git.kernel.org/pub/scm/linux/kernel/git/netdev/net")
reverted that removal because
commit 430d67bdcb04 ("net: hsr: Use the seqnr lock for frames
received via interlink port.") in net-next had already superseded it
by adding locking for the interlink RX path. All sequence counter
updates remain protected; only the forwarding work moves out of the
critical section.

Cc: <stable@vger.kernel.org> # 7.0.x
Signed-off-by: Xin Xie <xiexinet@gmail.com>
---
 net/hsr/hsr_device.c  | 15 ++++-----------
 net/hsr/hsr_forward.c |  9 +++++----
 net/hsr/hsr_slave.c   | 11 +----------
 3 files changed, 10 insertions(+), 25 deletions(-)

diff --git a/net/hsr/hsr_device.c b/net/hsr/hsr_device.c
index 5555b71ab19b..3fd1762d8916 100644
--- a/net/hsr/hsr_device.c
+++ b/net/hsr/hsr_device.c
@@ -232,9 +232,7 @@ static netdev_tx_t hsr_dev_xmit(struct sk_buff *skb, struct net_device *dev)
 		skb->dev = master->dev;
 		skb_reset_mac_header(skb);
 		skb_reset_mac_len(skb);
-		spin_lock_bh(&hsr->seqnr_lock);
 		hsr_forward_skb(skb, master);
-		spin_unlock_bh(&hsr->seqnr_lock);
 	} else {
 		dev_core_stats_tx_dropped_inc(dev);
 		dev_kfree_skb_any(skb);
@@ -335,6 +333,7 @@ static void send_hsr_supervision_frame(struct hsr_port *port,
 		hsr_stag->sequence_nr = htons(hsr->sequence_nr);
 		hsr->sequence_nr++;
 	}
+	spin_unlock_bh(&hsr->seqnr_lock);
 
 	hsr_stag->tlv.HSR_TLV_type = type;
 	/* HSRv0 has 6 unused bytes after the MAC */
@@ -356,14 +355,10 @@ static void send_hsr_supervision_frame(struct hsr_port *port,
 		ether_addr_copy(hsr_sp->macaddress_A, hsr->macaddress_redbox);
 	}
 
-	if (skb_put_padto(skb, ETH_ZLEN)) {
-		spin_unlock_bh(&hsr->seqnr_lock);
+	if (skb_put_padto(skb, ETH_ZLEN))
 		return;
-	}
 
 	hsr_forward_skb(skb, port);
-	spin_unlock_bh(&hsr->seqnr_lock);
-	return;
 }
 
 static void send_prp_supervision_frame(struct hsr_port *master,
@@ -390,6 +385,7 @@ static void send_prp_supervision_frame(struct hsr_port *master,
 	spin_lock_bh(&hsr->seqnr_lock);
 	hsr_stag->sequence_nr = htons(hsr->sup_sequence_nr);
 	hsr->sup_sequence_nr++;
+	spin_unlock_bh(&hsr->seqnr_lock);
 	hsr_stag->tlv.HSR_TLV_type = PRP_TLV_LIFE_CHECK_DD;
 	hsr_stag->tlv.HSR_TLV_length = sizeof(struct hsr_sup_payload);
 
@@ -397,13 +393,10 @@ static void send_prp_supervision_frame(struct hsr_port *master,
 	hsr_sp = skb_put(skb, sizeof(struct hsr_sup_payload));
 	ether_addr_copy(hsr_sp->macaddress_A, master->dev->dev_addr);
 
-	if (skb_put_padto(skb, ETH_ZLEN)) {
-		spin_unlock_bh(&hsr->seqnr_lock);
+	if (skb_put_padto(skb, ETH_ZLEN))
 		return;
-	}
 
 	hsr_forward_skb(skb, master);
-	spin_unlock_bh(&hsr->seqnr_lock);
 }
 
 /* Announce (supervision frame) timer function
diff --git a/net/hsr/hsr_forward.c b/net/hsr/hsr_forward.c
index 0774981a65c1..87cd72a1dc65 100644
--- a/net/hsr/hsr_forward.c
+++ b/net/hsr/hsr_forward.c
@@ -621,9 +621,10 @@ static void handle_std_frame(struct sk_buff *skb,
 	if (port->type == HSR_PT_MASTER ||
 	    port->type == HSR_PT_INTERLINK) {
 		/* Sequence nr for the master/interlink node */
-		lockdep_assert_held(&hsr->seqnr_lock);
+		spin_lock_bh(&hsr->seqnr_lock);
 		frame->sequence_nr = hsr->sequence_nr;
 		hsr->sequence_nr++;
+		spin_unlock_bh(&hsr->seqnr_lock);
 	}
 }
 
@@ -746,8 +747,8 @@ void hsr_forward_skb(struct sk_buff *skb, struct hsr_port *port)
 	 * So check and increment stats for master port only here.
 	 */
 	if (port->type == HSR_PT_MASTER || port->type == HSR_PT_INTERLINK) {
-		port->dev->stats.tx_packets++;
-		port->dev->stats.tx_bytes += skb->len;
+		DEV_STATS_INC(port->dev, tx_packets);
+		DEV_STATS_ADD(port->dev, tx_bytes, skb->len);
 	}
 
 	kfree_skb(frame.skb_hsr);
@@ -757,6 +758,6 @@ void hsr_forward_skb(struct sk_buff *skb, struct hsr_port *port)
 
 out_drop:
 	rcu_read_unlock();
-	port->dev->stats.tx_dropped++;
+	DEV_STATS_INC(port->dev, tx_dropped);
 	kfree_skb(skb);
 }
diff --git a/net/hsr/hsr_slave.c b/net/hsr/hsr_slave.c
index da06b21cdf51..0ca55d9323c5 100644
--- a/net/hsr/hsr_slave.c
+++ b/net/hsr/hsr_slave.c
@@ -73,16 +73,7 @@ static rx_handler_result_t hsr_handle_frame(struct sk_buff **pskb)
 	}
 	skb_reset_mac_len(skb);
 
-	/* Only the frames received over the interlink port will assign a
-	 * sequence number and require synchronisation vs other sender.
-	 */
-	if (port->type == HSR_PT_INTERLINK) {
-		spin_lock_bh(&hsr->seqnr_lock);
-		hsr_forward_skb(skb, port);
-		spin_unlock_bh(&hsr->seqnr_lock);
-	} else {
-		hsr_forward_skb(skb, port);
-	}
+	hsr_forward_skb(skb, port);
 
 finish_consume:
 	return RX_HANDLER_CONSUMED;
-- 
2.43.0


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

* [PATCH net v4 3/4] net: hsr: unfold GSO super-packets at the forward entry
  2026-08-03 22:22 [PATCH net v4 0/4] net: hsr: fix GRO/GSO super-packet handling Xin Xie
  2026-08-03 22:22 ` [PATCH net v4 1/4] net: hsr: fix packet drops caused by GRO superpackets Xin Xie
  2026-08-03 22:22 ` [PATCH net v4 2/4] net: hsr: shrink seqnr_lock to sequence counter updates Xin Xie
@ 2026-08-03 22:22 ` Xin Xie
  2026-08-03 22:22 ` [PATCH net v4 4/4] selftests: net: hsr: add GRO super-packet forwarding test Xin Xie
  3 siblings, 0 replies; 7+ messages in thread
From: Xin Xie @ 2026-08-03 22:22 UTC (permalink / raw)
  To: netdev, linux-kselftest, linux-kernel
  Cc: davem, edumazet, kuba, pabeni, horms, andrew+netdev, shuah, kees,
	petr.wozniak, qingfang.deng, fmaurer, luka.gejak, bigeasy,
	xiaoliang.yang_1, skhawaja, stable, sdf.kernel, Xin Xie

HSR/PRP forward frames one by one: each wire frame gets its own tag
and sequence number, and duplicate discard is per frame. A GSO
super-packet reaching hsr_forward_skb() breaks that per-frame
semantics: it would be tagged and forwarded as a single frame.

Unfold such super-packets at the forward entry with the top-level GSO
dispatch: __skb_gso_segment() initializes SKB_GSO_CB() and performs
the L2->L3->L4 protocol dispatch (calling the low-level skb_segment()
helper directly is not allowed here -- it reads SKB_GSO_CB(skb) state
that only __skb_gso_segment() sets up). features = 0 requests full
software segmentation; tx_path is selected by ingress port (master =
locally generated TX, interlink = RX) to get the right checksum
semantics. Each segment then runs through the existing per-frame
path, which is split out as hsr_forward_skb_one() so that no GSO skb
can reach it.

Segmentation is only offered for the ingress roles whose frames are
known to be plain Ethernet: the master (locally generated) and the
interlink (SAN side, untagged). A super-packet received from a LAN
slave may carry per-frame HSR tags or PRP RCT trailers that software
segmentation cannot recover, and an already-tagged HSR/PRP
super-packet violates per-frame wire semantics; both are rejected by
ingress-port policy. (ETH_P_PRP identifies supervision traffic only;
a PRP data frame keeps its payload EtherType, so RCT carriage cannot
be tested by protocol.)

Also drop NETIF_F_GSO_MASK from the HSR master's hw_features so
locally generated traffic is segmented before reaching the forward
path whenever possible.

Patch 2 (seqnr_lock shrink) is a latency/critical-section
prerequisite for this change; their automatic stable selection is
limited to 7.0 and newer. Older stable branches require an adapted
backport.

The GSO-drop accounting uses the same atomic legacy-stat helper
because the forwarding entry is concurrently callable.

Fixes: f421436a591d ("net/hsr: Add support for the High-availability Seamless Redundancy protocol (HSRv0)")
Cc: <stable@vger.kernel.org> # 7.0.x
Signed-off-by: Xin Xie <xiexinet@gmail.com>
---
 net/hsr/hsr_device.c  |  2 +-
 net/hsr/hsr_forward.c | 50 ++++++++++++++++++++++++++++++++++++++++++-
 2 files changed, 50 insertions(+), 2 deletions(-)

diff --git a/net/hsr/hsr_device.c b/net/hsr/hsr_device.c
index 3fd1762d8916..248cbb142e21 100644
--- a/net/hsr/hsr_device.c
+++ b/net/hsr/hsr_device.c
@@ -652,7 +652,7 @@ void hsr_dev_setup(struct net_device *dev)
 	dev->needs_free_netdev = true;
 
 	dev->hw_features = NETIF_F_SG | NETIF_F_FRAGLIST | NETIF_F_HIGHDMA |
-			   NETIF_F_GSO_MASK | NETIF_F_HW_CSUM |
+			   NETIF_F_HW_CSUM |
 			   NETIF_F_HW_VLAN_CTAG_TX |
 			   NETIF_F_HW_VLAN_CTAG_FILTER;
 
diff --git a/net/hsr/hsr_forward.c b/net/hsr/hsr_forward.c
index 87cd72a1dc65..b1b75a25a01a 100644
--- a/net/hsr/hsr_forward.c
+++ b/net/hsr/hsr_forward.c
@@ -12,6 +12,7 @@
 #include <linux/skbuff.h>
 #include <linux/etherdevice.h>
 #include <linux/if_vlan.h>
+#include <net/gso.h>
 #include "hsr_main.h"
 #include "hsr_framereg.h"
 
@@ -732,7 +733,7 @@ static int fill_frame_info(struct hsr_frame_info *frame,
 }
 
 /* Must be called holding rcu read lock (because of the port parameter) */
-void hsr_forward_skb(struct sk_buff *skb, struct hsr_port *port)
+static void hsr_forward_skb_one(struct sk_buff *skb, struct hsr_port *port)
 {
 	struct hsr_frame_info frame;
 
@@ -761,3 +762,50 @@ void hsr_forward_skb(struct sk_buff *skb, struct hsr_port *port)
 	DEV_STATS_INC(port->dev, tx_dropped);
 	kfree_skb(skb);
 }
+
+/* GSO fan-out funnel: unfold super-packets before per-frame processing so
+ * each wire frame gets its own HSR/PRP tag and sequence number.
+ */
+void hsr_forward_skb(struct sk_buff *skb, struct hsr_port *port)
+{
+	struct sk_buff *segs, *next;
+
+	if (likely(!skb_is_gso(skb))) {
+		hsr_forward_skb_one(skb, port);
+		return;
+	}
+
+	/* Unfold only plain-Ethernet GSO super-packets: locally generated
+	 * on the master, or arriving untagged from the SAN side on the
+	 * interlink. A super-packet from a LAN slave may carry per-frame
+	 * HSR tags / PRP RCT trailers that software segmentation cannot
+	 * recover; an already-tagged HSR/PRP super-packet violates
+	 * per-frame wire semantics. Drop both.
+	 */
+	if (port->type != HSR_PT_MASTER && port->type != HSR_PT_INTERLINK)
+		goto drop_gso;
+	if (skb->protocol == htons(ETH_P_HSR) ||
+	    skb->protocol == htons(ETH_P_PRP))
+		goto drop_gso;
+
+	/* features = 0: request full software segmentation. tx_path is true
+	 * only for locally generated traffic on the master; ingress from
+	 * the interlink follows RX checksum semantics.
+	 */
+	segs = __skb_gso_segment(skb, 0, port->type == HSR_PT_MASTER);
+	if (IS_ERR(segs) || unlikely(!segs))
+		goto drop_gso;
+
+	consume_skb(skb);
+	while (segs) {
+		next = segs->next;
+		segs->next = NULL;
+		hsr_forward_skb_one(segs, port);
+		segs = next;
+	}
+	return;
+
+drop_gso:
+	DEV_STATS_INC(port->dev, tx_dropped);
+	kfree_skb(skb);
+}
-- 
2.43.0


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

* [PATCH net v4 4/4] selftests: net: hsr: add GRO super-packet forwarding test
  2026-08-03 22:22 [PATCH net v4 0/4] net: hsr: fix GRO/GSO super-packet handling Xin Xie
                   ` (2 preceding siblings ...)
  2026-08-03 22:22 ` [PATCH net v4 3/4] net: hsr: unfold GSO super-packets at the forward entry Xin Xie
@ 2026-08-03 22:22 ` Xin Xie
  3 siblings, 0 replies; 7+ messages in thread
From: Xin Xie @ 2026-08-03 22:22 UTC (permalink / raw)
  To: netdev, linux-kselftest, linux-kernel
  Cc: davem, edumazet, kuba, pabeni, horms, andrew+netdev, shuah, kees,
	petr.wozniak, qingfang.deng, fmaurer, luka.gejak, bigeasy,
	xiaoliang.yang_1, skhawaja, stable, sdf.kernel, Xin Xie

Add a test exercising the HSR forward path with GSO super-packets:
a TSO-enabled SAN behind the interlink streams TCP through an HSR
DUT to a peer node. The test verifies that:

* enslaved devices have GRO and HW-GRO disabled automatically,
* the HSR master does not advertise GSO/TSO features (including
  tx-tcp6/udp/gso-list, to catch future hw_features leaks),
* super-packets really leave the SAN (TX average frame size above a
  fixed threshold) while bulk output on the DUT's LAN legs stays at
  per-frame size — aggregate counter evidence that GSO enters the
  forward path and is unfolded at the forward entry. Zero TCP
  retransmits is reported as a secondary health signal.

The one-shot iperf3 server lives in a private mktemp -d workdir
(mode 0700): its exact PID is retained only after numeric, alive,
comm==iperf3 and netns-membership checks, its real exit status is
propagated through an rc file, and cleanup kills by exact PID with a
bounded wrapper reap plus a namespace-scoped iperf3 sweep. Server
startup failure, client failure and a never-published PID are all
bounded exits with no process or directory leaks. Environments whose
iproute2 lacks the HSR interlink syntax are skipped with ksft_skip.

Without this series the feature checks fail, and on drivers that hand
GRO super-packets to the HSR receive path the stream degrades or
stalls.

Signed-off-by: Xin Xie <xiexinet@gmail.com>
---
 tools/testing/selftests/net/hsr/Makefile      |   1 +
 .../selftests/net/hsr/hsr_gro_superpacket.sh  | 465 ++++++++++++++++++
 2 files changed, 466 insertions(+)
 create mode 100755 tools/testing/selftests/net/hsr/hsr_gro_superpacket.sh

diff --git a/tools/testing/selftests/net/hsr/Makefile b/tools/testing/selftests/net/hsr/Makefile
index 31fb9326cf53..0d105476e7c5 100644
--- a/tools/testing/selftests/net/hsr/Makefile
+++ b/tools/testing/selftests/net/hsr/Makefile
@@ -3,6 +3,7 @@
 top_srcdir = ../../../../..
 
 TEST_PROGS := \
+	hsr_gro_superpacket.sh \
 	hsr_ping.sh \
 	hsr_redbox.sh \
 	link_faults.sh \
diff --git a/tools/testing/selftests/net/hsr/hsr_gro_superpacket.sh b/tools/testing/selftests/net/hsr/hsr_gro_superpacket.sh
new file mode 100755
index 000000000000..ec05754274c8
--- /dev/null
+++ b/tools/testing/selftests/net/hsr/hsr_gro_superpacket.sh
@@ -0,0 +1,465 @@
+#!/bin/bash
+# SPDX-License-Identifier: GPL-2.0
+#
+# Test HSR handling of GRO/GSO super-packets:
+#
+#  1. Enslaving a device to an HSR master disables GRO on it
+#     (dev_disable_gro()).
+#  2. The HSR master does not advertise GSO/TSO features.
+#  3. A TCP stream from a TSO-enabled SAN (which therefore emits GSO
+#     super-packets) is unfolded at the HSR forward entry. Evidence:
+#     interface-counter deltas show super-packet-sized frames leaving
+#     the SAN and per-frame-sized traffic leaving the DUT's LAN ports.
+#
+# Topology (100.64.0.0/24):
+#
+#   ns_san                    ns_dut                      ns_peer
+#  +-----------+  interlink  +---------------+  LAN A/B  +-----------+
+#  | s0 [0.1]  |-------------| d_il   hsr0   |-----------| hsr1 [0.3]|
+#  +-----------+             | d_a / d_b     |           | p_a / p_b |
+#                            +---------------+           +-----------+
+#
+# SAN traffic reaches ns_peer only through hsr0's forward path
+# (interlink RX -> LAN A/B TX), so every SAN frame is tagged and
+# forwarded by the DUT.
+
+source ./hsr_common.sh
+
+san_ip="100.64.0.1"
+peer_ip="100.64.0.3"
+
+# Aggregate counter thresholds for the stream test (bytes/packets):
+# SAN_AVG_MIN proves GSO super-packets left the SAN; LAN_AVG_MAX is a
+# guard with margin, not the protocol maximum (see do_tso_stream_test).
+SAN_AVG_MIN=2048
+LAN_AVG_MAX=1514
+
+iperf_pid=""
+server_wrapper=""
+workdir=""
+pidfile=""
+ns_dut=""
+ns_san=""
+ns_peer=""
+rcfile=""
+
+cleanup()
+{
+	# exact-PID kill only after RE-validating identity (guards against
+	# PID reuse between publication and cleanup)
+	if [ -n "${iperf_pid}" ] && valid_server_pid "${iperf_pid}"; then
+		kill "${iperf_pid}" 2>/dev/null
+	fi
+	iperf_pid=""
+	if [ -n "${server_wrapper}" ]; then
+		# the wrapper waits on the server; reap it with a 5s bound so a
+		# live-but-unpublished server can never hang cleanup
+		for _ in $(seq 1 50); do
+			kill -0 "${server_wrapper}" 2>/dev/null || break
+			sleep 0.1
+		done
+		kill "${server_wrapper}" 2>/dev/null
+		wait "${server_wrapper}" 2>/dev/null
+		server_wrapper=""
+	fi
+	# last resort, namespace-scoped only: TERM the iperf3 processes that
+	# actually live in the peer netns, poll for bounded exit, then
+	# SIGKILL any survivor before touching the namespace name. A blind
+	# pkill would scan the host PID space and hit unrelated tests.
+	local _p _still
+	for _p in $(ip netns pids "$ns_peer" 2>/dev/null); do
+		if is_iperf3_pid "$_p"; then
+			kill "$_p" 2>/dev/null
+		fi
+	done
+	for _ in $(seq 1 50); do
+		_still=0
+		for _p in $(ip netns pids "$ns_peer" 2>/dev/null); do
+			if is_iperf3_pid "$_p"; then
+				_still=1
+				break
+			fi
+		done
+		[ "$_still" -eq 0 ] && break
+		sleep 0.1
+	done
+	for _p in $(ip netns pids "$ns_peer" 2>/dev/null); do
+		if is_iperf3_pid "$_p"; then
+			kill -9 "$_p" 2>/dev/null
+		fi
+	done
+	# remove only the known non-empty private directory
+	if [ -n "${workdir}" ] && [ -d "${workdir}" ]; then
+		rm -rf "${workdir}"
+	fi
+	workdir=""
+	pidfile=""
+	rcfile=""
+	cleanup_all_ns
+}
+
+trap cleanup EXIT
+
+check_tool()
+{
+	if ! command -v "$1" > /dev/null 2>&1; then
+		echo "SKIP: Could not run test without $1"
+		exit $ksft_skip
+	fi
+}
+
+nsx()
+{
+	ip netns exec "$1" bash -c "$2"
+}
+
+is_iperf3_pid()
+{
+	[ "$(cat /proc/"$1"/comm 2>/dev/null)" = "iperf3" ]
+}
+
+# Decimal-counter validation for the snapshot blocks: every value must
+# be a plain decimal number. A parse failure in read_tx_counters yields
+# empty/garbled fields, which this check turns into an immediate FAIL.
+valid_decimals()
+{
+	local v
+
+	for v in "$@"; do
+		[[ "$v" =~ ^[0-9]+$ ]] || return 1
+	done
+	return 0
+}
+
+setup_topo()
+{
+	setup_ns ns_dut ns_san ns_peer || exit $?
+
+	ip link add d_a netns "$ns_dut" type veth peer name p_a netns "$ns_peer"
+	ip link add d_b netns "$ns_dut" type veth peer name p_b netns "$ns_peer"
+	ip link add d_il netns "$ns_dut" type veth peer name s0 netns "$ns_san"
+
+	# HSR tags add 6 bytes per frame; give the LAN legs headroom.
+	for iface in d_a d_b; do
+		nsx "$ns_dut" "ip link set $iface mtu 1600; \
+			ip link set $iface up"
+	done
+	for iface in p_a p_b; do
+		nsx "$ns_peer" "ip link set $iface mtu 1600; \
+			ip link set $iface up"
+	done
+
+	nsx "$ns_dut" "ip link set d_il up"
+	nsx "$ns_san" "ip link set s0 up; ip addr add $san_ip/24 dev s0"
+
+	nsx "$ns_dut" "ip link add hsr0 type hsr \
+		slave1 d_a slave2 d_b interlink d_il proto 0; \
+		ip link set hsr0 up"
+	nsx "$ns_peer" "ip link add hsr1 type hsr \
+		slave1 p_a slave2 p_b proto 0; \
+		ip link set hsr1 up; ip addr add $peer_ip/24 dev hsr1"
+
+	# Let the nodes see each other's supervision frames.
+	sleep 2
+}
+
+check_feature()
+{
+	local ns="$1"
+	local iface="$2"
+	local feature="$3"
+	local want="$4"
+
+	if nsx "$ns" "ethtool -k $iface" | grep -q "^$feature: $want"; then
+		echo "INFO: $ns/$iface $feature is $want [ OK ]"
+	else
+		echo "FAIL: $ns/$iface $feature is not $want" 1>&2
+		ret=1
+	fi
+}
+
+# Off-or-absent variant: fails only when the feature is present AND on,
+# so devices that simply do not list the feature do not fail it.
+check_feature_not_on()
+{
+	local ns="$1"
+	local iface="$2"
+	local feature="$3"
+
+	if nsx "$ns" "ethtool -k $iface" | grep -q "^$feature: on"; then
+		echo "FAIL: $ns/$iface $feature is on" 1>&2
+		ret=1
+	else
+		echo "INFO: $ns/$iface $feature not on [ OK ]"
+	fi
+}
+
+do_gro_feature_checks()
+{
+	echo "INFO: Checking that enslavement disabled GRO."
+	check_feature "$ns_dut" d_a generic-receive-offload off
+	check_feature "$ns_dut" d_b generic-receive-offload off
+	check_feature "$ns_dut" d_il generic-receive-offload off
+	stop_if_error "GRO not disabled on enslaved devices."
+
+	echo "INFO: Checking that enslavement disabled HW-GRO."
+	check_feature "$ns_dut" d_a rx-gro-hw off
+	check_feature "$ns_dut" d_b rx-gro-hw off
+	check_feature "$ns_dut" d_il rx-gro-hw off
+	stop_if_error "HW-GRO not disabled on enslaved devices."
+
+	echo "INFO: Checking that the HSR master does not advertise GSO/TSO."
+	check_feature "$ns_dut" hsr0 generic-segmentation-offload off
+	check_feature "$ns_dut" hsr0 tcp-segmentation-offload off
+	check_feature_not_on "$ns_dut" hsr0 tx-tcp6-segmentation
+	check_feature_not_on "$ns_dut" hsr0 tx-udp-segmentation
+	check_feature_not_on "$ns_dut" hsr0 tx-gso-list
+	stop_if_error "HSR master still advertises GSO-family features."
+}
+
+alloc_workdir()
+{
+	# Allocated only here, long after the initial topology cleanup, so
+	# cleanup() at setup_topo() time can never remove it. mktemp failure
+	# is a hard test failure.
+	workdir=$(mktemp -d /tmp/hsr_gro_test.XXXXXX) || {
+		echo "FAIL: mktemp -d failed" 1>&2
+		exit 1
+	}
+	chmod 700 "${workdir}"
+	pidfile="${workdir}/iperf.pid"
+	rcfile="${workdir}/iperf.rc"
+}
+
+# Numeric, alive, comm == iperf3, and really owned by the peer netns.
+valid_server_pid()
+{
+	local p="$1"
+
+	[[ "$p" =~ ^[0-9]+$ ]] || return 1
+	kill -0 "$p" 2>/dev/null || return 1
+	[ "$(cat /proc/"$p"/comm 2>/dev/null)" = "iperf3" ] || return 1
+	ip netns pids "$ns_peer" 2>/dev/null | grep -qx "$p"
+}
+
+start_iperf_server()
+{
+	local candidate_pid
+
+	# One-shot server, no -D: the wrapper records its exact PID and its
+	# real exit status (netns shares the PID namespace and the host fs).
+	alloc_workdir
+	( nsx "$ns_peer" "iperf3 -s -1 > /dev/null 2>&1 & \
+		echo \$! > ${pidfile}; \
+		wait \$!; \
+		echo \$? > ${rcfile}" ) &
+	server_wrapper=$!
+	# the wrapper writes the pidfile asynchronously; wait for it to
+	# appear instead of racing the read
+	for _ in $(seq 1 50); do
+		[ -s "${pidfile}" ] && break
+		sleep 0.1
+	done
+	if [ ! -s "${pidfile}" ]; then
+		echo "FAIL: iperf3 server did not publish a pid" \
+			"(no pidfile)" 1>&2
+		ret=1
+		return 1
+	fi
+	candidate_pid=$(<"${pidfile}")
+	if ! valid_server_pid "${candidate_pid}"; then
+		echo "FAIL: iperf3 server pid '${candidate_pid}'" \
+			"failed validation" 1>&2
+		ret=1
+		return 1
+	fi
+	# publish only after full validation
+	iperf_pid="${candidate_pid}"
+	sleep 1
+	return 0
+}
+
+# Print "<bytes> <packets>" for exactly one TX record of ns/dev; anything
+# else (missing, duplicated, non-numeric) is a hard FAIL.
+read_tx_counters()
+{
+	local ns="$1" dev="$2"
+	local out cnt
+
+	out=$(nsx "$ns" "ip -s link show $dev" | \
+		awk '/^ +TX:/{getline; print $1, $2}')
+	cnt=$(echo "$out" | grep -c '^[0-9]* [0-9]*$')
+	if [ "$cnt" -ne 1 ]; then
+		echo "FAIL: cannot parse TX counters of $ns/$dev" \
+			"(records=$cnt)" 1>&2
+		return 1
+	fi
+	echo "$out"
+	return 0
+}
+
+eval_counter_delta()
+{
+	local name="$1" b0="$2" p0="$3" b1="$4" p1="$5" op="$6" limit="$7"
+	local bd pd
+
+	if ! [[ "$b0" =~ ^[0-9]+$ && "$b1" =~ ^[0-9]+$ && \
+		"$p0" =~ ^[0-9]+$ && "$p1" =~ ^[0-9]+$ ]]; then
+		echo "FAIL: non-numeric counter input for $name" 1>&2
+		ret=1
+		return 1
+	fi
+	bd=$((b1 - b0))
+	pd=$((p1 - p0))
+	if [ "$bd" -lt 0 ] || [ "$pd" -le 0 ]; then
+		echo "FAIL: counter delta invalid for $name" \
+			"(bytes=$bd pkts=$pd)" 1>&2
+		ret=1
+		return 1
+	fi
+	if [ "$op" = "gt" ]; then
+		if [ "$bd" -le $((pd * limit)) ]; then
+			echo "FAIL: $name bytes/packets $bd/$pd <= $limit" 1>&2
+			ret=1
+			return 1
+		fi
+	else
+		if [ "$bd" -gt $((pd * limit)) ]; then
+			echo "FAIL: $name bytes/packets $bd/$pd > $limit" 1>&2
+			ret=1
+			return 1
+		fi
+	fi
+	echo "INFO: $name counter delta bytes=$bd packets=$pd" \
+		"(op $op limit $limit) [ OK ]"
+	return 0
+}
+
+do_tso_stream_test()
+{
+	local out sender_retr server_rc
+	local san_b0 san_p0 san_b1 san_p1
+	local a_b0 a_p0 a_b1 a_p1 b_b0 b_p0 b_b1 b_p1
+
+	echo "INFO: Enabling TSO/GSO on the SAN interface."
+	nsx "$ns_san" "ethtool -K s0 tso on gso on"
+	check_feature "$ns_san" s0 tcp-segmentation-offload on
+	stop_if_error "Could not enable TSO on the SAN interface."
+
+	echo "INFO: Running 10s TCP stream SAN -> peer through the HSR DUT."
+	start_iperf_server || return
+
+	# Counter snapshots around the stream window. The SAN-side average
+	# must exceed SAN_AVG_MIN (aggregate proof that GSO super-packets
+	# really left the SAN); each DUT LAN leg must stay under LAN_AVG_MAX
+	# (aggregate proof that bulk output was segmented per-frame). These
+	# are aggregate discriminators, not a per-frame maximum proof.
+	san_b0=0; san_p0=0; a_b0=0; a_p0=0; b_b0=0; b_p0=0
+	read -r san_b0 san_p0 <<EOF
+$(read_tx_counters "$ns_san" s0)
+EOF
+	read -r a_b0 a_p0 <<EOF
+$(read_tx_counters "$ns_dut" d_a)
+EOF
+	read -r b_b0 b_p0 <<EOF
+$(read_tx_counters "$ns_dut" d_b)
+EOF
+	if ! valid_decimals "$san_b0" "$san_p0" "$a_b0" "$a_p0" \
+		"$b_b0" "$b_p0"; then
+		echo "FAIL: baseline TX counter snapshot invalid" 1>&2
+		ret=1
+		return 1
+	fi
+
+	# rate-capped: the PRIMARY discriminator is the counter inequality
+	# above, not max throughput; retransmits are informational only.
+	# Uncapped runs flap at VM/CI edge rates without indicating a
+	# functional problem.
+	if ! out=$(nsx "$ns_san" "timeout 60 iperf3 -c $peer_ip -M 1446 \
+		-b 2G -t 10" 2>&1); then
+		echo "FAIL: iperf3 client failed:" 1>&2
+		echo "$out" 1>&2
+		ret=1
+		return
+	fi
+
+	read -r san_b1 san_p1 <<EOF
+$(read_tx_counters "$ns_san" s0)
+EOF
+	read -r a_b1 a_p1 <<EOF
+$(read_tx_counters "$ns_dut" d_a)
+EOF
+	read -r b_b1 b_p1 <<EOF
+$(read_tx_counters "$ns_dut" d_b)
+EOF
+	if ! valid_decimals "$san_b1" "$san_p1" "$a_b1" "$a_p1" \
+		"$b_b1" "$b_p1"; then
+		echo "FAIL: final TX counter snapshot invalid" 1>&2
+		ret=1
+		return 1
+	fi
+
+	eval_counter_delta "SAN s0 TX" "$san_b0" "$san_p0" "$san_b1" "$san_p1" \
+		gt "$SAN_AVG_MIN"
+	eval_counter_delta "DUT d_a TX" "$a_b0" "$a_p0" "$a_b1" "$a_p1" \
+		le "$LAN_AVG_MAX"
+	eval_counter_delta "DUT d_b TX" "$b_b0" "$b_p0" "$b_b1" "$b_p1" \
+		le "$LAN_AVG_MAX"
+	[ "${ret:-0}" -eq 0 ] || return
+
+	# success path: the one-shot server exits by itself; reap the
+	# wrapper, then REQUIRE the rcfile with the server's real status
+	wait "${server_wrapper}"
+	server_wrapper=""
+	if [ ! -s "${rcfile}" ]; then
+		echo "FAIL: iperf3 server status file missing (${rcfile})" 1>&2
+		ret=1
+		return
+	fi
+	server_rc=$(cat "${rcfile}")
+	if ! [[ "$server_rc" =~ ^[0-9]+$ ]] || [ "$server_rc" -ne 0 ]; then
+		echo "FAIL: iperf3 server exited with rc='${server_rc}'" 1>&2
+		ret=1
+		return
+	fi
+	iperf_pid=""
+
+	# secondary health signal only: anchored, single-match, numeric —
+	# any parse anomaly is a loud FAIL, but the value itself no longer
+	# gates (the counter inequalities above are the primary evidence).
+	sender_retr=$(echo "$out" | awk '/sec .* sender$/ {print $(NF-1)}')
+	if [ "$(echo "$sender_retr" | grep -Ec '^[0-9]+$')" -ne 1 ]; then
+		echo "FAIL: cannot parse sender retransmits reliably" 1>&2
+		echo "$out" 1>&2
+		ret=1
+		return
+	fi
+	echo "INFO: TCP stream done;" \
+		"sender retransmits=$sender_retr (secondary signal)"
+	echo "$out" | grep -E "sender|receiver"
+}
+
+check_prerequisites
+check_tool ethtool
+check_tool iperf3
+check_tool timeout
+
+# iproute2 must know the HSR interlink syntax.
+if ! ip link help hsr 2>&1 | grep -qi interlink; then
+	echo "SKIP: iproute2 has no HSR interlink support"
+	exit $ksft_skip
+fi
+
+setup_topo
+
+echo "INFO: Initial validation ping (SAN -> peer through the DUT)."
+do_ping "$ns_san" "$peer_ip"
+stop_if_error "Initial validation failed."
+
+do_gro_feature_checks
+do_tso_stream_test
+stop_if_error "GSO super-packet stream test failed."
+
+echo "INFO: All good."
+cleanup
+exit $ret
-- 
2.43.0


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

* Re: [PATCH net v4 2/4] net: hsr: shrink seqnr_lock to sequence counter updates
  2026-08-03 22:22 ` [PATCH net v4 2/4] net: hsr: shrink seqnr_lock to sequence counter updates Xin Xie
@ 2026-08-07 14:27   ` Hangbin Liu
  2026-08-07 14:49     ` Xin Xie
  0 siblings, 1 reply; 7+ messages in thread
From: Hangbin Liu @ 2026-08-07 14:27 UTC (permalink / raw)
  To: Xin Xie
  Cc: netdev, linux-kselftest, linux-kernel, davem, edumazet, kuba,
	pabeni, horms, andrew+netdev, shuah, kees, petr.wozniak,
	qingfang.deng, fmaurer, luka.gejak, bigeasy, xiaoliang.yang_1,
	skhawaja, stable, sdf.kernel

Hi Xie Xin,
On Tue, Aug 04, 2026 at 12:22:09AM +0200, Xin Xie wrote:
> hsr->seqnr_lock is currently held across entire hsr_forward_skb()
> calls: master TX (hsr_dev_xmit()), interlink RX (hsr_handle_frame()),
> and both supervision frame builders hold it while frames are built,
> classified, duplicated and forwarded on every port. The only state
> that actually needs the lock is the sequence counters themselves
> (hsr->sequence_nr / hsr->sup_sequence_nr).
> 
> Shrink the locking to the individual counter updates:
> handle_std_frame() now takes the lock around its sequence number
> allocation (replacing the lockdep assertion), the master TX and
> interlink RX paths drop their outer lock, and the supervision
> builders release the lock right after updating their counter instead
> of holding it across frame construction and forwarding.
> 
> Statistics: the outer seqnr_lock also incidentally serialized the
> tx_packets/tx_bytes/tx_dropped updates in hsr_forward_skb(), which are
> reachable from the concurrently callable master-TX, supervision and
> interlink-RX paths. Those updates now use DEV_STATS_INC()/
> DEV_STATS_ADD(), the atomic legacy-stat helpers, so narrowing the lock
> does not turn them into lossy plain read-modify-writes.
> 
> Ordering: with IFF_NO_QUEUE and dev->lltx, hsr_dev_xmit() is
> concurrently callable, and this change removes the old
> allocation-through-forward ordering guarantee there: master-TX frames
> may now be emitted out of sequence-allocation order. On the current
> tree this is safe because duplicate discard tracks individual
> sequence numbers in sparse bitmaps (commit aae9d6b616b5 ("hsr:
> Implement more robust duplicate discard for HSR") and
> commit 415e6367512b ("hsr: Implement more robust duplicate discard
> for PRP")) rather than requiring monotonic arrival. Sequence numbers
> remain unique and monotonically allocated per counter.
> 
> This is a latency/critical-section prerequisite for unfolding GSO
> super-packets at the forward entry (not a functional prerequisite):
> the segmentation work should not extend the global sequence lock's
> critical section. Patch 3 depends on this change; their automatic
> stable selection is limited to 7.0 and newer, where sparse-bitmap
> duplicate discard accepts out-of-order arrival. Older stable branches
> require an adapted backport.
> 
> History of the lock being narrowed: it was introduced by
> commit 06afd2c31d33 ("hsr: Synchronize sending frames to have always
> incremented outgoing seq nr.") and briefly removed by
> commit b3c9e65eb227 ("net: hsr: remove seqnr_lock") in net. Merge
> commit 46ae4d0a4897 ("Merge git://git.kernel.org/pub/scm/linux/kernel/git/netdev/net")
> reverted that removal because
> commit 430d67bdcb04 ("net: hsr: Use the seqnr lock for frames
> received via interlink port.") in net-next had already superseded it
> by adding locking for the interlink RX path. All sequence counter
> updates remain protected; only the forwarding work moves out of the
> critical section.
> 
> Cc: <stable@vger.kernel.org> # 7.0.x
> Signed-off-by: Xin Xie <xiexinet@gmail.com>

Maybe use a shorter commit description.

[ ... ]
> @@ -746,8 +747,8 @@ void hsr_forward_skb(struct sk_buff *skb, struct hsr_port *port)
>  	 * So check and increment stats for master port only here.
>  	 */
>  	if (port->type == HSR_PT_MASTER || port->type == HSR_PT_INTERLINK) {
> -		port->dev->stats.tx_packets++;
> -		port->dev->stats.tx_bytes += skb->len;
> +		DEV_STATS_INC(port->dev, tx_packets);
> +		DEV_STATS_ADD(port->dev, tx_bytes, skb->len);

Note: Avoid these macros in fast path, prefer per-cpu or per-queue counters.

And the counters in hsr_deliver_master() also need to protected. Especially
multicast.

Thanks
Hangbin

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

* Re: [PATCH net v4 2/4] net: hsr: shrink seqnr_lock to sequence counter updates
  2026-08-07 14:27   ` Hangbin Liu
@ 2026-08-07 14:49     ` Xin Xie
  0 siblings, 0 replies; 7+ messages in thread
From: Xin Xie @ 2026-08-07 14:49 UTC (permalink / raw)
  To: Hangbin Liu
  Cc: netdev, linux-kselftest, linux-kernel, davem, edumazet, kuba,
	pabeni, horms, andrew+netdev, shuah, kees, petr.wozniak,
	qingfang.deng, fmaurer, luka.gejak, bigeasy, xiaoliang.yang_1,
	skhawaja, stable, sdf.kernel

On 07/08/2026 16:27, Hangbin Liu wrote:
> 
> Maybe use a shorter commit description.
> 
> Note: Avoid these macros in fast path, prefer per-cpu or per-queue counters.
> 
> And the counters in hsr_deliver_master() also need to protected. Especially
> multicast.
> 
> Thanks
> Hangbin

Thanks.

The long commit description was from v4. I reworked and shortened it in v5:

https://lore.kernel.org/netdev/20260807140751.1351-3-xiexinet@gmail.com/

Regarding the statistics, Paolo previously suggested addressing the HSR dev stats races in a separate series, since there are already several occurrences:

https://lore.kernel.org/netdev/4fc3b9f1-4bef-4b34-ae7a-e89037cce829@redhat.com/

Would you prefer that I drop the DEV_STATS_* conversions from the next revision and address all HSR statistics consistently in a follow-up, including hsr_deliver_master() and multicast, rather than adding per-CPU accounting to this series?

-- 
Xin

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

end of thread, other threads:[~2026-08-07 14:49 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-08-03 22:22 [PATCH net v4 0/4] net: hsr: fix GRO/GSO super-packet handling Xin Xie
2026-08-03 22:22 ` [PATCH net v4 1/4] net: hsr: fix packet drops caused by GRO superpackets Xin Xie
2026-08-03 22:22 ` [PATCH net v4 2/4] net: hsr: shrink seqnr_lock to sequence counter updates Xin Xie
2026-08-07 14:27   ` Hangbin Liu
2026-08-07 14:49     ` Xin Xie
2026-08-03 22:22 ` [PATCH net v4 3/4] net: hsr: unfold GSO super-packets at the forward entry Xin Xie
2026-08-03 22:22 ` [PATCH net v4 4/4] selftests: net: hsr: add GRO super-packet forwarding test Xin Xie

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