* [PATCH net v6 0/4] net: hsr: fix GRO/GSO super-packet handling
@ 2026-08-09 12:14 Xin Xie
2026-08-09 12:14 ` [PATCH 1/4] net: hsr: fix packet drops caused by GRO superpackets Xin Xie
` (3 more replies)
0 siblings, 4 replies; 6+ messages in thread
From: Xin Xie @ 2026-08-09 12:14 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, liuhangbin, 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. Disabling is explicitly
best-effort: a later privileged override can re-enable GRO, and
devices with fixed-on GRO_HW (for example a virtio-net device
negotiating guest TSO without VIRTIO_NET_F_CTRL_GUEST_OFFLOADS)
cannot be forced off, so enslavement succeeds whether or not the
feature could be cleared.
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 master/interlink tx statistics
updates in hsr_forward_skb(); those are now updated without the
outer lock, and consistent per-cpu/per-queue statistics for all HSR
paths (including hsr_deliver_master() and multicast) are handled in
a separate series.
Patch 3 unfolds GSO super-packets at the forward entry with the
top-level GSO dispatch (__skb_gso_segment()), so each wire frame
gets its own tag/RCT and sequence number. Admission is decided by a
content-based classifier, not by the ingress port: plain-Ethernet
aggregates are segmented on every ingress role (master, interlink,
and LAN slaves), while aggregates whose effective protocol is
ETH_P_HSR or ETH_P_PRP carry per-frame trailers that software
segmentation cannot reconstruct and are dropped. Aggregates with
unreadable net_iov (device-memory) fragments are also dropped: their
payload is not host-readable, and software segmentation would produce
segments with uninitialized payload that are silently dropped at
transmit or expose uninitialized data where netmem transmit is
enabled. The classifier unwraps accelerated VLAN and one in-band
VLAN level, and treats NETIF_F_HW_HSR_TAG_RM lowers as plain by
construction. The trailer-free premise of the plain-aggregate path is
proven for in-tree software GRO: its IPv4 and IPv6 length checks
reject frames with trailing bytes beyond the L3 length, so an
RCT-bearing PRP frame is not merged. Device-specific fixed-on GRO_HW
output is not claimed to be completely covered. Locally destined
segments are delivered to the host; the rest are forwarded per frame.
Patch 4 adds a kselftest covering the series, including a LAN-slave
plain-GSO regression: a plain SAN aggregate entering a PRP LAN slave
must be segmented, with local delivery and per-frame arrival proven
by independent counter oracles.
Patch 1 is a safe, partial, best-effort mitigation and is
independently stable-selectable; patch 3, named by subject in its
message, is the fallback for plain, trailer-free GSO aggregates.
Patch 3 depends on patch 2, and patches 2 and 3 are selected for
stable only where the sparse-bitmap duplicate discard is present,
which accepts out-of-order arrival. The stable tag names that
prerequisite, which exists only in 7.0 and newer.
Older branches need adapted backports.
Validation:
* the v6 kernel builds cleanly;
* LAN-slave plain-GSO regression on the v6 kernel: aggregates arrive
at the PRP master and are delivered per-frame (SAN TX avg ~38 KB
per frame; prp0 RX avg ~1476 B per frame);
* interlink super-packet test passes (DUT LAN legs avg ~1496 B per
frame with the SAN emitting ~38 KB super-packets);
* fixed-on GRO_HW: unchanged from v5 (patch 1 control flow
untouched); with a virtio guest negotiating TSO without
VIRTIO_NET_F_CTRL_GUEST_OFFLOADS, HSR setup succeeds without the
v4-era "failed to disable GRO!" splat (v5 evidence reused);
* the bounded server-readiness/reap waits pass on the v6 kernel;
* hsr_ping, hsr_redbox, link_faults and prp_ping all pass;
* checkpatch: 0 errors; shellcheck unchanged from v5 (0 errors);
* the series applies cleanly with plain git am on current net.
Note on the net-next conflict: this series conflicts with our own PRP
RedBox support now merged in net-next
(https://lore.kernel.org/netdev/20260717201457.54-1-xiexinet@gmail.com/).
On current net it applies cleanly (re-verified by a full-series
git am). For net-next the conflict is confined to two sites:
1. net/hsr/hsr_device.c, send_prp_supervision_frame(): net-next
added the PRP RedBox Type-30 TLV and EOT construction where
patch 2/4 shrinks the seqnr_lock critical section. Resolution:
keep the lock release immediately after the sup_sequence_nr
update (as in this patch), build the whole TLV chain (LifeCheck
payload, Type-30 RedBox-MAC TLV, EOT) unlocked, and drop the
two stale unlocks from the padding-error and normal-exit paths.
2. tools/testing/selftests/net/hsr/Makefile: insert
hsr_gro_superpacket.sh at its sorted position.
The resolution was independently proven by composition on the
current net-next (4fa4977a0d90): the composed tree preserves the
Type-30 RedBox-MAC TLV and EOT construction with one balanced lock
region. This series' conflict-relevant hunks are unchanged from v5,
which applied and validated the same resolution on an earlier
net-next (2fbade662450): the composed tree builds cleanly and
hsr_prp_redbox.sh passes on the composed kernel.
---
Changes in v6:
A confirmed defect is fixed: a GSO aggregate carrying unreadable
net_iov (device-memory) fragments could reach the forward-entry
segmentation, where the software copy path cannot read such fragments
and would leave the segment payload uninitialized — silently dropped
at transmit on most devices, or exposing uninitialized data where
netmem transmit is enabled. v6 rejects such aggregates before
segmentation (skb_frags_readable() gate) and patch 3 describes this
accurately.
The selftest is hardened: the server reap now polls the wrapper's
status file instead of a recycled-PID candidate, server-PID
validation retries across the fork/exec window, and server readiness
polls the listening socket instead of a fixed sleep; all waits stay
bounded.
Further changes:
- patch 1: the comment now names fixed-on GRO_HW only (plain GRO is
always changeable).
- patch 3: the prerequisite is named by subject and commit; the
hw_features wording states the unconditional GSO_MASK removal (only
GSO_MASK member types become fixed-off;
generic-segmentation-offload stays changeable and is cleared at
runtime); the classifier comment describes its own policy.
- patch 4: the message describes the new test file, the header
documents the LAN-slave case, and the master-RX bound has its own
named constant.
- maintainer feedback adopted: patch 2 mentions commit aae9d6b616b5
("hsr: Implement more robust duplicate discard for HSR"), uses the
commit-annotated stable tag (verified equivalent to the former
7.0.x floor: the prerequisite exists only in 7.0.y/7.1.y), carries
the syzbot Reported-by/Closes pair and the two Fixes: tags, and
moves all statistics work out of the series (plain increments
restored; consistent per-cpu/per-queue statistics are a separate
follow-up series).
The remaining review findings were assessed and left unchanged: the
one-way best-effort GRO disable mirrors dev_disable_lro() and is
ethtool-recoverable; the kernel-doc already names the re-enable path;
out-of-order emission is tolerated by the sparse-bitmap duplicate
discard on both ends under the stable floor; the remaining statistics
races are accounting-precision issues covered by the follow-up
series; the software-GRO RCT claim holds (IPv4/IPv6 length checks
reject trailing bytes); the generic-segmentation-offload selftest
oracle was verified empirically on the identical tree; test cleanup
re-validates PID identity before signalling and has a netns-scoped
fallback; in-place skb mutations during forwarding are pre-existing
and handled by a separate series.
Previous postings (newest first):
v5: https://lore.kernel.org/netdev/20260807140751.1351-1-xiexinet@gmail.com/
v4: https://lore.kernel.org/netdev/20260803222211.877-1-xiexinet@gmail.com/
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: cover GSO super-packets on PRP slave ingress
include/linux/netdevice.h | 2 +
net/core/dev.c | 15 +
net/core/dev_api.c | 21 +
net/hsr/hsr_device.c | 17 +-
net/hsr/hsr_forward.c | 109 ++-
net/hsr/hsr_slave.c | 16 +-
tools/testing/selftests/net/hsr/Makefile | 1 +
.../selftests/net/hsr/hsr_gro_superpacket.sh | 678 ++++++++++++++++++
8 files changed, 835 insertions(+), 24 deletions(-)
create mode 100755 tools/testing/selftests/net/hsr/hsr_gro_superpacket.sh
--
2.43.0
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH 1/4] net: hsr: fix packet drops caused by GRO superpackets
2026-08-09 12:14 [PATCH net v6 0/4] net: hsr: fix GRO/GSO super-packet handling Xin Xie
@ 2026-08-09 12:14 ` Xin Xie
2026-08-09 12:14 ` [PATCH 2/4] net: hsr: shrink seqnr_lock to sequence counter updates Xin Xie
` (2 subsequent siblings)
3 siblings, 0 replies; 6+ messages in thread
From: Xin Xie @ 2026-08-09 12:14 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, liuhangbin, stable, sdf.kernel,
Xin Xie
HSR/PRP process each wire frame separately for tagging and duplicate
discard. GRO on a lower device hides multiple frames in one skb, which
cannot be forwarded with valid per-frame metadata.
Disable GRO and GRO_HW when a lower device is enslaved, matching the
existing LRO handling.
This is best effort because GRO may be re-enabled and some devices cannot
disable GRO_HW. The forward-entry segmentation fix handles plain,
trailer-free GSO skbs that still arrive; device-specific fixed-on GRO_HW
output is outside this guarantee.
Fixes: f421436a591d ("net/hsr: Add support for the High-availability Seamless Redundancy protocol (HSRv0)")
Cc: stable@vger.kernel.org
Signed-off-by: Xin Xie <xiexinet@gmail.com>
---
include/linux/netdevice.h | 2 ++
net/core/dev.c | 15 +++++++++++++++
net/core/dev_api.c | 21 +++++++++++++++++++++
net/hsr/hsr_slave.c | 5 +++++
4 files changed, 43 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..f20d5ab0cf72 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -1840,6 +1840,21 @@ 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);
+
+ 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..3ca2515ad048 100644
--- a/net/core/dev_api.c
+++ b/net/core/dev_api.c
@@ -269,6 +269,27 @@ 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
+ *
+ * Best-effort disable of 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.
+ *
+ * The disable is best-effort: a device with a fixed-on feature (for
+ * example GRO_HW on a virtio-net device negotiated without
+ * VIRTIO_NET_F_CTRL_GUEST_OFFLOADS) keeps it enabled. Callers that
+ * need a hard guarantee must inspect the resulting feature state.
+ */
+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..bb2182a169a3 100644
--- a/net/hsr/hsr_slave.c
+++ b/net/hsr/hsr_slave.c
@@ -171,6 +171,11 @@ static int hsr_portdev_setup(struct hsr_priv *hsr, struct net_device *dev,
goto fail_rx_handler;
dev_disable_lro(dev);
+ /* GRO disabling is best-effort: fixed-on GRO_HW cannot be
+ * forced off, and GRO may be re-enabled later via ethtool.
+ */
+ dev_disable_gro(dev);
+
return 0;
fail_rx_handler:
--
2.43.0
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH 2/4] net: hsr: shrink seqnr_lock to sequence counter updates
2026-08-09 12:14 [PATCH net v6 0/4] net: hsr: fix GRO/GSO super-packet handling Xin Xie
2026-08-09 12:14 ` [PATCH 1/4] net: hsr: fix packet drops caused by GRO superpackets Xin Xie
@ 2026-08-09 12:14 ` Xin Xie
2026-08-10 2:23 ` Hangbin Liu
2026-08-09 12:14 ` [PATCH 3/4] net: hsr: unfold GSO super-packets at the forward entry Xin Xie
2026-08-09 12:14 ` [PATCH 4/4] selftests: net: hsr: cover GSO super-packets on PRP slave ingress Xin Xie
3 siblings, 1 reply; 6+ messages in thread
From: Xin Xie @ 2026-08-09 12:14 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, liuhangbin, stable, sdf.kernel,
Xin Xie, syzbot+fbf74291c3b7e753b481
Commit 06afd2c31d33 ("hsr: Synchronize sending frames to have always
incremented outgoing seq nr.") and commit 430d67bdcb04 ("net: hsr: Use
the seqnr lock for frames received via interlink port.") hold
seqnr_lock across the whole forwarding path. Transmitting while
holding the lock can create lock-dependency issues when HSR devices
are stacked with other net devices, up to a real deadlock (see
Reported-by/Closes).
Since commit aae9d6b616b5 ("hsr: Implement more robust duplicate
discard for HSR"), duplicate discard is order-independent (sparse
bitmaps), so only the sequence counter updates need serialization.
Limit seqnr_lock to the counter updates in handle_std_frame(); the
master TX path (hsr_dev_xmit()) and the interlink RX path
(hsr_handle_frame()) drop their outer lock, and the supervision frame
builders release it right after their counter update.
Concurrent forwarding may emit frames out of allocation order, which
sparse-bitmap discard tolerates. The master/interlink tx statistics are
no longer serialized; consistent per-cpu/per-queue statistics for all
HSR paths are handled in a separate series.
Fixes: 06afd2c31d33 ("hsr: Synchronize sending frames to have always incremented outgoing seq nr.")
Fixes: 430d67bdcb04 ("net: hsr: Use the seqnr lock for frames received via interlink port.")
Reported-by: syzbot+fbf74291c3b7e753b481@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=fbf74291c3b7e753b481
Cc: <stable@vger.kernel.org> # aae9d6b616b5: hsr: Implement more robust duplicate discard for HSR
Signed-off-by: Xin Xie <xiexinet@gmail.com>
---
net/hsr/hsr_device.c | 15 ++++-----------
net/hsr/hsr_forward.c | 3 ++-
net/hsr/hsr_slave.c | 11 +----------
3 files changed, 7 insertions(+), 22 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..8e4158a9b57c 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);
}
}
diff --git a/net/hsr/hsr_slave.c b/net/hsr/hsr_slave.c
index bb2182a169a3..8b96eafe15b3 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] 6+ messages in thread
* [PATCH 3/4] net: hsr: unfold GSO super-packets at the forward entry
2026-08-09 12:14 [PATCH net v6 0/4] net: hsr: fix GRO/GSO super-packet handling Xin Xie
2026-08-09 12:14 ` [PATCH 1/4] net: hsr: fix packet drops caused by GRO superpackets Xin Xie
2026-08-09 12:14 ` [PATCH 2/4] net: hsr: shrink seqnr_lock to sequence counter updates Xin Xie
@ 2026-08-09 12:14 ` Xin Xie
2026-08-09 12:14 ` [PATCH 4/4] selftests: net: hsr: cover GSO super-packets on PRP slave ingress Xin Xie
3 siblings, 0 replies; 6+ messages in thread
From: Xin Xie @ 2026-08-09 12:14 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, liuhangbin, stable, sdf.kernel,
Xin Xie
HSR/PRP require per-wire-frame tags and sequence numbers; treating a
GSO skb as one frame breaks those semantics.
Classify GSO skbs at the forward entry by effective protocol rather
than ingress port. Segment plain aggregates and process each segment
normally, preserving local delivery and forwarding. Drop ETH_P_HSR and
ETH_P_PRP aggregates (per-frame metadata cannot be rebuilt),
classification failures (unreadable header, stacked or S-tag tagging),
and aggregates with unreadable net_iov (device-memory) fragments:
their payload is not host-readable, and segmentation would leave the
segment payload uninitialized, silently dropped at transmit or exposed
where netmem transmit is enabled.
In-tree software GRO does not merge PRP RCT frames (its IPv4/IPv6
length checks reject trailing bytes); fixed-on GRO_HW output is
outside this guarantee.
Also remove the GSO_MASK member types from the HSR master's
hw_features: local traffic is segmented by the core before the forward
path, so the master funnel branch sees single frames, and TSO and the
other GSO_MASK offloads can no longer be enabled on the master;
generic-segmentation-offload stays changeable and is cleared at
runtime.
This patch depends on patch 2 ("net: hsr: shrink seqnr_lock to
sequence counter updates") and the sparse-bitmap duplicate discard
introduced by commit aae9d6b616b5 ("hsr: Implement more robust
duplicate discard for HSR"); older trees require an adapted backport.
Fixes: f421436a591d ("net/hsr: Add support for the High-availability Seamless Redundancy protocol (HSRv0)")
Cc: <stable@vger.kernel.org> # aae9d6b616b5: hsr: Implement more robust duplicate discard for HSR
Signed-off-by: Xin Xie <xiexinet@gmail.com>
---
net/hsr/hsr_device.c | 2 +-
net/hsr/hsr_forward.c | 106 +++++++++++++++++++++++++++++++++++++++++-
2 files changed, 106 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 8e4158a9b57c..ae49c2d74ace 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,106 @@ void hsr_forward_skb(struct sk_buff *skb, struct hsr_port *port)
port->dev->stats.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.
+ */
+/* Effective frame protocol of a (possibly VLAN-tagged) skb, or 0 when
+ * it cannot be determined or the tagging exceeds what HSR supports.
+ * HSR supports one 802.1Q C-tag only: an accelerated tag must be a
+ * C-tag with a non-VLAN inner protocol; an in-band tag is unwrapped
+ * exactly once and a residual VLAN EtherType is rejected. Read-only;
+ * no state is kept beyond the immediate protocol value.
+ */
+static __be16 hsr_gso_effective_proto(const struct sk_buff *skb)
+{
+ struct ethhdr eh;
+ struct vlan_hdr vh;
+ const struct ethhdr *eth;
+ const struct vlan_hdr *vhdr;
+ __be16 proto;
+
+ if (skb_vlan_tag_present(skb)) {
+ /* HSR supports one 802.1Q C-tag only. */
+ if (skb->vlan_proto != htons(ETH_P_8021Q))
+ return 0;
+ if (eth_type_vlan(skb->protocol))
+ return 0;
+ return skb->protocol;
+ }
+
+ eth = skb_header_pointer(skb, 0, sizeof(eh), &eh);
+ if (!eth)
+ return 0;
+
+ proto = eth->h_proto;
+ if (!eth_type_vlan(proto))
+ return proto;
+ if (proto != htons(ETH_P_8021Q))
+ return 0;
+
+ vhdr = skb_header_pointer(skb, ETH_HLEN, sizeof(vh), &vh);
+ if (!vhdr)
+ return 0;
+
+ proto = vhdr->h_vlan_encapsulated_proto;
+ if (eth_type_vlan(proto))
+ return 0;
+
+ return proto;
+}
+
+void hsr_forward_skb(struct sk_buff *skb, struct hsr_port *port)
+{
+ struct sk_buff *segs, *next;
+ __be16 proto;
+
+ if (likely(!skb_is_gso(skb))) {
+ hsr_forward_skb_one(skb, port);
+ return;
+ }
+
+ /* Conforming plain-protocol GSO super-packets carry trailer-free
+ * sender payload and are segmented here: each segment is delivered
+ * or forwarded as its own wire frame, on any ingress role.
+ *
+ * The gate is content-based, not port-based. An aggregate whose
+ * effective protocol is ETH_P_HSR or ETH_P_PRP cannot be safely
+ * segmented and is dropped, as is any skb whose header cannot be
+ * read, whose tagging exceeds the single 802.1Q C-tag HSR
+ * supports, or whose fragments are unreadable net_iov
+ * (device-memory) pages: software segmentation cannot read their
+ * payload, so each segment would inherit the unreadable flag and
+ * carry uninitialized data. With NETIF_F_HW_HSR_TAG_RM the lower
+ * has already stripped the tag, so such aggregates arrive plain
+ * and are segmented.
+ */
+ proto = hsr_gso_effective_proto(skb);
+ if (!proto)
+ goto drop_gso; /* classification failure, fail-safe */
+ if (proto == htons(ETH_P_HSR) || proto == htons(ETH_P_PRP))
+ goto drop_gso;
+ if (!skb_frags_readable(skb))
+ goto drop_gso; /* net_iov (device-memory) frags are not host-readable */
+
+ /* 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:
+ port->dev->stats.tx_dropped++;
+ kfree_skb(skb);
+}
--
2.43.0
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH 4/4] selftests: net: hsr: cover GSO super-packets on PRP slave ingress
2026-08-09 12:14 [PATCH net v6 0/4] net: hsr: fix GRO/GSO super-packet handling Xin Xie
` (2 preceding siblings ...)
2026-08-09 12:14 ` [PATCH 3/4] net: hsr: unfold GSO super-packets at the forward entry Xin Xie
@ 2026-08-09 12:14 ` Xin Xie
3 siblings, 0 replies; 6+ messages in thread
From: Xin Xie @ 2026-08-09 12:14 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, liuhangbin, stable, sdf.kernel,
Xin Xie
Add a new always-run HSR/PRP kselftest (hsr_gro_superpacket.sh)
covering the GRO/GRO_HW state on enslaved devices, the HSR master's
GSO/TSO feature advertisement, a TSO stream through the forward path,
and a PRP LAN-slave plain-GSO regression in which aggregates are
segmented at the forward entry and delivered locally to the master.
The test asserts that plain aggregates are unfolded per-frame on any
ingress role and that local delivery and forwarding are preserved;
interface-counter deltas are the evidence. Server readiness and reap
waits are bounded, and the tool dependencies (ethtool, iperf3,
timeout) are enforced via check_tool().
Signed-off-by: Xin Xie <xiexinet@gmail.com>
---
tools/testing/selftests/net/hsr/Makefile | 1 +
.../selftests/net/hsr/hsr_gro_superpacket.sh | 678 ++++++++++++++++++
2 files changed, 679 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..d2b34f6f37a7
--- /dev/null
+++ b/tools/testing/selftests/net/hsr/hsr_gro_superpacket.sh
@@ -0,0 +1,678 @@
+#!/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.
+# 4. PRP LAN-slave plain-GSO regression: a plain SAN aggregate
+# entering a PRP LAN slave (ns_ls over the d_pa/ls_a and d_pb/ls_p
+# veth pairs) is segmented at the forward entry and delivered
+# locally to the PRP master (prp0), not dropped. Oracles: the SAN
+# TX average proves aggregates left the SAN; the prp0 RX volume and
+# per-frame average prove local delivery of the segmented stream.
+#
+# 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
+# Master-RX per-frame bound for the LAN-slave test: prp0 RX counts
+# recv_len after skb_pull(ETH_HLEN), so per-frame payload is at most
+# 1500. Same guard-with-margin shape as LAN_AVG_MAX, but a different
+# quantity (see do_lansan_gso_test).
+MASTER_RX_AVG_MAX=1514
+
+iperf_pid=""
+server_wrapper=""
+active_srv_ns=""
+workdir=""
+pidfile=""
+ns_dut=""
+ns_san=""
+ns_peer=""
+ns_ls=""
+rcfile=""
+
+# Delete the per-server private work directory and reset its
+# variables. Called after a successful reap and from the EXIT
+# trap (all normal failure paths; runner-timeout INT/TERM signals
+# exit via the same trap).
+cleanup_workdir()
+{
+ # remove only the known non-empty private directory
+ if [ -n "${workdir}" ] && [ -d "${workdir}" ]; then
+ rm -rf "${workdir}"
+ fi
+ workdir=""
+ pidfile=""
+ rcfile=""
+}
+
+cleanup()
+{
+ # Server cleanup targets only the namespace of the currently
+ # active server (recorded by start_iperf_server); after a
+ # successful reap nothing is active.
+ if [ -n "${active_srv_ns}" ]; then
+ # exact-PID kill only after RE-validating identity (guards
+ # against PID reuse between publication and cleanup)
+ if [ -n "${iperf_pid}" ] && valid_server_pid "${active_srv_ns}" "${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 active server 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 "${active_srv_ns}" 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 "${active_srv_ns}" 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 "${active_srv_ns}" 2>/dev/null); do
+ if is_iperf3_pid "$_p"; then
+ kill -9 "$_p" 2>/dev/null
+ fi
+ done
+ fi
+ active_srv_ns=""
+ cleanup_workdir
+ cleanup_all_ns
+}
+
+trap cleanup EXIT
+# INT/TERM (e.g. a runner timeout) must not leave the workdir,
+# server or namespaces behind: exit 143 (128+SIGTERM) via the EXIT
+# trap so cleanup runs exactly once. Do not use a shared
+# 'trap cleanup EXIT INT TERM': the trap would return and the
+# script would keep running after cleanup.
+trap 'exit 143' INT TERM
+
+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" ]
+}
+
+# Bounded reap of the one-shot server: wait at most 5s for it to
+# exit, then reap the wrapper and REQUIRE the rcfile with its real
+# status. A stuck server can never hang the script. The wrapper writes
+# the rcfile only after iperf3 has exited, so polling the rcfile has no
+# PID-reuse ambiguity (a kill -0 poll on the non-child PID could
+# spuriously match a recycled PID).
+reap_iperf_server()
+{
+ local server_rc
+
+ for _ in $(seq 1 50); do
+ [ -s "${rcfile}" ] && break
+ sleep 0.1
+ done
+ if [ ! -s "${rcfile}" ]; then
+ echo "FAIL: iperf3 server did not exit within 5s" 1>&2
+ ret=1
+ return 1
+ fi
+ wait "${server_wrapper}"
+ server_wrapper=""
+ if [ ! -s "${rcfile}" ]; then
+ echo "FAIL: iperf3 server status file missing (${rcfile})" 1>&2
+ ret=1
+ return 1
+ 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 1
+ fi
+ iperf_pid=""
+ cleanup_workdir
+ active_srv_ns=""
+ return 0
+}
+
+# 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-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 given netns.
+valid_server_pid()
+{
+ local pns="$1" p="$2"
+
+ [[ "$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 "$pns" 2>/dev/null | grep -qx "$p"
+}
+
+start_iperf_server()
+{
+ local srv_ns="$1"
+ local srv_ip="${2:-}"
+ 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
+ # record the server namespace for cleanup() before
+ # anything can fail with the server running
+ active_srv_ns="$srv_ns"
+ ( nsx "$srv_ns" "iperf3 -s -1 ${srv_ip:+-B $srv_ip} > /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}")
+ # The pidfile is written between fork() and execve(), when comm is
+ # still "bash". Retry for up to 5s so that transient state cannot
+ # fail a server that is starting normally; a genuinely dead or
+ # never-execed server still fails at expiry.
+ for _ in $(seq 1 50); do
+ valid_server_pid "$srv_ns" "${candidate_pid}" && break
+ sleep 0.1
+ done
+ if ! valid_server_pid "$srv_ns" "${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}"
+ # Bounded listen() readiness poll: the pidfile proves the process
+ # started, not that bind()+listen() completed; on a loaded CI the
+ # client can otherwise hit "Connection refused" while the kernel
+ # behaves correctly. The server netns is freshly created and
+ # private, so a :5201 listener there is ours. If ss is unavailable
+ # in a minimal environment, keep the old fixed wait as last resort.
+ if command -v ss > /dev/null 2>&1; then
+ for _ in $(seq 1 50); do
+ nsx "$srv_ns" "ss -ltn" | grep -q ':5201' && break
+ sleep 0.1
+ done
+ if ! nsx "$srv_ns" "ss -ltn" | grep -q ':5201'; then
+ echo "FAIL: iperf3 server did not listen on :5201 within 5s" 1>&2
+ ret=1
+ return 1
+ fi
+ else
+ sleep 1
+ fi
+ 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
+}
+
+# Print "<bytes> <packets>" for exactly one RX record of ns/dev; the
+# same single-record discipline as read_tx_counters.
+read_rx_counters()
+{
+ local ns="$1" dev="$2"
+ local out cnt
+
+ out=$(nsx "$ns" "ip -s link show $dev" | \
+ awk '/^ +RX:/{getline; print $1, $2}')
+ cnt=$(echo "$out" | grep -c '^[0-9]* [0-9]*$')
+ if [ "$cnt" -ne 1 ]; then
+ echo "FAIL: cannot parse RX 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
+}
+
+# LAN-slave plain-GSO regression: a plain SAN aggregate entering a PRP
+# LAN slave must be segmented at the forward entry, not dropped. PRP
+# drops slave-to-slave forwarding by design (prp_drop_frame), so the
+# consumer of LAN-slave SAN traffic is local delivery to the master.
+# Two independent oracles: the SAN-side proof that aggregates really
+# arrived at the DUT, and the volume + per-frame shape of the local
+# delivery. On the broken gate the aggregates are dropped and TCP
+# crawls on retransmitted single segments, separating the kernels by
+# an order of magnitude in delivered bytes.
+do_lansan_gso_test()
+{
+ local prp_ip="10.99.1.1" ls_ip="10.99.1.10"
+ local out san_b0 san_p0 san_b1 san_p1
+ local a_b0 a_p0 a_b1 a_p1 r_b0 r_p0 r_b1 r_p1
+
+ setup_ns ns_ls || exit $?
+
+ ip link add d_pa netns "$ns_dut" type veth peer name ls_a netns "$ns_ls"
+ ip link add d_pb netns "$ns_dut" type veth peer name ls_p netns "$ns_ls"
+
+ nsx "$ns_dut" "ip link set d_pa mtu 1600 up"
+ nsx "$ns_dut" "ip link set d_pb mtu 1600 up"
+ nsx "$ns_ls" "ip link set ls_a mtu 1600 up; \
+ ip link set ls_p mtu 1600 up; ip addr add $ls_ip/24 dev ls_a"
+
+ # PRP DUT with the SAN on a LAN slave: plain SAN aggregates are
+ # valid traffic on a PRP LAN and must not be dropped.
+ nsx "$ns_dut" "ip link add prp0 type hsr \
+ slave1 d_pa slave2 d_pb proto 1; \
+ ip link set prp0 up; ip addr add $prp_ip/24 dev prp0"
+
+ # Let supervision frames converge.
+ sleep 2
+
+ echo "INFO: Enabling TSO/GSO on the LAN-side SAN interface."
+ nsx "$ns_ls" "ethtool -K ls_a tso on gso on"
+ check_feature "$ns_ls" ls_a tcp-segmentation-offload on
+ stop_if_error "Could not enable TSO on the LAN-side SAN interface."
+
+ start_iperf_server "$ns_dut" "$prp_ip" || return
+
+ read -r r_b0 r_p0 <<EOF
+$(read_rx_counters "$ns_dut" prp0)
+EOF
+ read -r san_b0 san_p0 <<EOF
+$(read_tx_counters "$ns_ls" ls_a)
+EOF
+ if ! valid_decimals "$r_b0" "$r_p0" "$san_b0" "$san_p0"; then
+ echo "FAIL: baseline counter snapshot invalid" 1>&2
+ ret=1
+ return 1
+ fi
+
+ if ! out=$(nsx "$ns_ls" "timeout 60 iperf3 -c $prp_ip -M 1446 \
+ -b 2G -t 10" 2>&1); then
+ echo "FAIL: LAN-slave GSO local-delivery stream failed:" 1>&2
+ echo "$out" 1>&2
+ ret=1
+ return
+ fi
+
+ read -r r_b1 r_p1 <<EOF
+$(read_rx_counters "$ns_dut" prp0)
+EOF
+ read -r san_b1 san_p1 <<EOF
+$(read_tx_counters "$ns_ls" ls_a)
+EOF
+ if ! valid_decimals "$r_b1" "$r_p1" "$san_b1" "$san_p1"; then
+ echo "FAIL: final counter snapshot invalid" 1>&2
+ ret=1
+ return 1
+ fi
+
+ # Oracle 1 - aggregates really arrived: the SAN emitted
+ # super-packets toward the DUT.
+ eval_counter_delta "SAN ls_a TX" "$san_b0" "$san_p0" \
+ "$san_b1" "$san_p1" gt "$SAN_AVG_MIN"
+ [ "${ret:-0}" -eq 0 ] || return
+
+ # Oracle 2 - local delivery of those aggregates: volume and
+ # per-frame shape on the PRP master. The volume gate separates
+ # full delivery from the retransmit crawl the broken gate leaves;
+ # the average gate proves the bytes arrived per-frame.
+ if [ $((r_b1 - r_b0)) -lt 100000000 ]; then
+ echo "FAIL: LAN-slave local delivery degraded" \
+ "(prp0 RX delta $((r_b1 - r_b0)) bytes < 100000000)" 1>&2
+ ret=1
+ return
+ fi
+ if [ $((r_b1 - r_b0)) -gt $(( (r_p1 - r_p0) * MASTER_RX_AVG_MAX )) ]; then
+ echo "FAIL: local delivery not per-frame" \
+ "(avg $(( (r_b1 - r_b0) / (r_p1 - r_p0) )) > $MASTER_RX_AVG_MAX)" 1>&2
+ ret=1
+ return
+ fi
+ echo "INFO: LAN-slave local delivery prp0 RX delta" \
+ "$((r_b1 - r_b0)) bytes / $((r_p1 - r_p0)) pkts [ OK ]"
+ reap_iperf_server || return
+ echo "INFO: LAN-slave plain-GSO regression [ OK ]"
+}
+
+do_tso_stream_test()
+{
+ local out sender_retr
+ 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 "$ns_peer" || 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: bounded reap with the server's real status
+ reap_iperf_server || return
+
+ # 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."
+
+do_lansan_gso_test
+stop_if_error "LAN-slave plain-GSO regression failed."
+
+echo "INFO: All good."
+cleanup
+exit $ret
--
2.43.0
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH 2/4] net: hsr: shrink seqnr_lock to sequence counter updates
2026-08-09 12:14 ` [PATCH 2/4] net: hsr: shrink seqnr_lock to sequence counter updates Xin Xie
@ 2026-08-10 2:23 ` Hangbin Liu
0 siblings, 0 replies; 6+ messages in thread
From: Hangbin Liu @ 2026-08-10 2:23 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, syzbot+fbf74291c3b7e753b481
On Sun, Aug 09, 2026 at 02:14:52PM +0200, Xin Xie wrote:
> Commit 06afd2c31d33 ("hsr: Synchronize sending frames to have always
> incremented outgoing seq nr.") and commit 430d67bdcb04 ("net: hsr: Use
> the seqnr lock for frames received via interlink port.") hold
> seqnr_lock across the whole forwarding path. Transmitting while
> holding the lock can create lock-dependency issues when HSR devices
> are stacked with other net devices, up to a real deadlock (see
> Reported-by/Closes).
>
> Since commit aae9d6b616b5 ("hsr: Implement more robust duplicate
> discard for HSR"), duplicate discard is order-independent (sparse
> bitmaps), so only the sequence counter updates need serialization.
> Limit seqnr_lock to the counter updates in handle_std_frame(); the
> master TX path (hsr_dev_xmit()) and the interlink RX path
> (hsr_handle_frame()) drop their outer lock, and the supervision frame
> builders release it right after their counter update.
>
> Concurrent forwarding may emit frames out of allocation order, which
> sparse-bitmap discard tolerates. The master/interlink tx statistics are
> no longer serialized; consistent per-cpu/per-queue statistics for all
> HSR paths are handled in a separate series.
>
> Fixes: 06afd2c31d33 ("hsr: Synchronize sending frames to have always incremented outgoing seq nr.")
> Fixes: 430d67bdcb04 ("net: hsr: Use the seqnr lock for frames received via interlink port.")
> Reported-by: syzbot+fbf74291c3b7e753b481@syzkaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=fbf74291c3b7e753b481
> Cc: <stable@vger.kernel.org> # aae9d6b616b5: hsr: Implement more robust duplicate discard for HSR
> Signed-off-by: Xin Xie <xiexinet@gmail.com>
> ---
> net/hsr/hsr_device.c | 15 ++++-----------
> net/hsr/hsr_forward.c | 3 ++-
> net/hsr/hsr_slave.c | 11 +----------
> 3 files changed, 7 insertions(+), 22 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..8e4158a9b57c 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);
> }
> }
>
> diff --git a/net/hsr/hsr_slave.c b/net/hsr/hsr_slave.c
> index bb2182a169a3..8b96eafe15b3 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
>
Thanks for the fix.
Reviewed-by: Hangbin Liu <liuhangbin@kylinos.cn>
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2026-08-10 2:23 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-08-09 12:14 [PATCH net v6 0/4] net: hsr: fix GRO/GSO super-packet handling Xin Xie
2026-08-09 12:14 ` [PATCH 1/4] net: hsr: fix packet drops caused by GRO superpackets Xin Xie
2026-08-09 12:14 ` [PATCH 2/4] net: hsr: shrink seqnr_lock to sequence counter updates Xin Xie
2026-08-10 2:23 ` Hangbin Liu
2026-08-09 12:14 ` [PATCH 3/4] net: hsr: unfold GSO super-packets at the forward entry Xin Xie
2026-08-09 12:14 ` [PATCH 4/4] selftests: net: hsr: cover GSO super-packets on PRP slave ingress Xin Xie
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox