[PATCH net] hsr: Avoid holding seqnr_lock while transmitting packets

[PATCH net] hsr: Avoid holding seqnr_lock while transmitting packets

[Hangbin Liu]

From: Hangbin Liu <liuhangbin@kylinos.cn>

Commit 06afd2c31d33 ("hsr: Synchronize sending frames to have always
incremented outgoing seq nr.") added spin lock around the whole
hsr_forward_skb() path to synchronize outgoing sequence number handling.
Commit 430d67bdcb04 ("net: hsr: Use the seqnr lock for frames received
via interlink port.") also use the same approach.

However, holding seqnr_lock while transmitting packets can cause lock
dependency issues when HSR devices are stacked with other net devices.
For example, when an HSR device is enslaved to a bridge and the bridge is
enslaved to another HSR device, hsr_dev_xmit() can be called recursively
through the networking stack. Although the two seqnr_lock instances are
different in this case, the same lock class can trigger a lockdep warning,
and more complex stacking topologies could lead to a real deadlock.

Since commit aae9d6b616b5 ("hsr: Implement more robust duplicate discard
for HSR"), sequence numbers are stored in an array and no longer require
linear comparison. Protecting only the sequence number update is
sufficient. There is no need to hold seqnr_lock during the whole
forwarding operation.

Revert 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.") to avoid holding
seqnr_lock while transmitting packets.

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
Signed-off-by: Hangbin Liu <liuhangbin@kylinos.cn>
---
I'm not sure if I should add these 2 fixes as this patch is depend on
Felix's aae9d6b616b5 ("hsr: Implement more robust duplicate discard for
HSR"). Please correct me if I need a change.
---
 net/hsr/hsr_device.c  | 12 +++++-------
 net/hsr/hsr_forward.c |  3 ++-
 net/hsr/hsr_slave.c   | 11 +----------
 3 files changed, 8 insertions(+), 18 deletions(-)

diff --git a/net/hsr/hsr_device.c b/net/hsr/hsr_device.c
index 5555b71ab19b..14e8d0676229 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,13 +355,11 @@ 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;
 }
 
@@ -402,8 +399,9 @@ static void send_prp_supervision_frame(struct hsr_port *master,
 		return;
 	}
 
-	hsr_forward_skb(skb, master);
 	spin_unlock_bh(&hsr->seqnr_lock);
+
+	hsr_forward_skb(skb, master);
 }
 
 /* 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 01c73b4b50dd..267ffbd3c838 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;

---
base-commit: 2a33516f9ef59ad11844d4fc152f889449b5daf3
change-id: 20260804-hsr_deadlock-40fd40b36154

Best regards,
-- 
Hangbin Liu <liuhangbin@kylinos.cn>

Re: [PATCH net] hsr: Avoid holding seqnr_lock while transmitting packets

[Simon Horman]

This is an AI-generated review of your patch. The human sending this
email has considered the AI review valid, or at least plausible.
Full review at: https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260805-hsr_deadlock-v1-1-d8a6f06fc5a8@kylinos.cn?part=1
---
--- Patch [1]: [PATCH net] hsr: Avoid holding seqnr_lock while transmitting packets ---
commit 1d9b203ac401a6e7717dcd0d27015da8d13c979b
Author: Hangbin Liu <liuhangbin@kylinos.cn>

hsr: Avoid holding seqnr_lock while transmitting packets

This narrows hsr->seqnr_lock so that it only covers the outgoing sequence
number update in handle_std_frame(), and removes it from the whole
hsr_forward_skb() path in hsr_dev_xmit(), hsr_handle_frame() and the
supervision frame senders.

Closes: https://syzkaller.appspot.com/bug?extid=fbf74291c3b7e753b481

Before the diff, a question about the changelog and the Fixes: tags:

> Since commit aae9d6b616b5 ("hsr: Implement more robust duplicate discard
> for HSR"), sequence numbers are stored in an array and no longer require
> linear comparison. Protecting only the sequence number update is
> sufficient.
>
> 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.")

[Severity: Medium]
[Finding: main-1-0]
[Sources: opus-5]
The correctness argument here depends on aae9d6b616b5, which is much newer
than both commits named in the Fixes: tags (06afd2c31d33 is in the tree
since v6.2, 430d67bdcb04 since v6.12).

In a tree that has those two commits but not aae9d6b616b5,
hsr_register_frame_out() still does the last-seen/greater-than comparison,
so shrinking the lock to the counter bump in handle_std_frame() and dropping
it from hsr_dev_xmit() would reintroduce exactly the local drop that
06afd2c31d33 describes.

Should the changelog carry an explicit dependency note, or should the tags
be scoped (for example "Depends-on:" or a note that this is not suitable for
stable before aae9d6b616b5)? The doubt about the tags is currently written
below the --- separator, so it is dropped from the committed message.

Simon says:

I don't think the advice on how to annotate this is correct

Tags trump freeform text, but Depends-on is a not a standard tag and as
such should not be used.

I believe that the correct approach would be a tag like this:

  Cc: <stable@vger.kernel.org> # aae9d6b616b5: hsr: Implement more robust duplicate discard for HSR

Link: https://docs.kernel.org/process/stable-kernel-rules.html#option-1

> diff --git a/net/hsr/hsr_device.c b/net/hsr/hsr_device.c
> index 5555b71ab19b5..14e8d06762294 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);

[Severity: Medium]
[Finding: main-2-3]
[Sources: opus-5]
Can this lose statistics updates? The tail of hsr_forward_skb() does plain
read-modify-write on struct net_device_stats for the master and interlink
ports:

net/hsr/hsr_forward.c:hsr_forward_skb() {
	...
	if (port->type == HSR_PT_MASTER || port->type == HSR_PT_INTERLINK) {
		port->dev->stats.tx_packets++;
		port->dev->stats.tx_bytes += skb->len;
	}
	...
out_drop:
	rcu_read_unlock();
	port->dev->stats.tx_dropped++;
}

Before this change every caller reaching those increments for
master/interlink held hsr->seqnr_lock across all of hsr_forward_skb(), which
made them mutually exclusive as a side effect.

hsr_dev_setup() sets dev->lltx = true ("Prevent recursive tx locking") and
the master has no qdisc, so __dev_queue_xmit() calls hsr_dev_xmit() without
the txq lock. Two CPUs can then run tx_packets++ concurrently and drop an
increment, and tx_bytes += skb->len can tear on 32-bit. The interlink RX
path, hsr_handle_frame() -> hsr_forward_skb(), now races the same counters.

Would switching these to dev_sw_netstats or otherwise per-CPU counters be
preferable while the lock goes away?

[ ... ]

> @@ -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,13 +355,11 @@ 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;
>  }
>  
> @@ -402,8 +399,9 @@ static void send_prp_supervision_frame(struct hsr_port *master,
>  		return;
>  	}
>  
> -	hsr_forward_skb(skb, master);
>  	spin_unlock_bh(&hsr->seqnr_lock);
> +
> +	hsr_forward_skb(skb, master);
>  }

[Severity: Low]
[Finding: main-2-2]
[Sources: opus-5]
This isn't a bug, but the two implementers of the same
hsr_proto_ops->send_sv_frame callback now follow different rules for
seqnr_lock.

send_hsr_supervision_frame() was converted to the minimal scope the patch
establishes in handle_std_frame(): lock, bump sup_sequence_nr/sequence_nr,
unlock.

send_prp_supervision_frame() still takes seqnr_lock before the sequence
number bump and holds it across skb_put(), ether_addr_copy() and the
possibly reallocating skb_put_padto(), which is also why it keeps the
duplicated unlock in the error path and just before hsr_forward_skb():

net/hsr/hsr_device.c:send_prp_supervision_frame() {
	spin_lock_bh(&hsr->seqnr_lock);
	hsr_stag->sequence_nr = htons(hsr->sup_sequence_nr);
	hsr->sup_sequence_nr++;
	...
	if (skb_put_padto(skb, ETH_ZLEN)) {
		spin_unlock_bh(&hsr->seqnr_lock);
		return;
	}

	spin_unlock_bh(&hsr->seqnr_lock);

	hsr_forward_skb(skb, master);
}

Nothing in the extended region re-acquires seqnr_lock and the two unlock
sites are on mutually exclusive paths, so this is not an unbalanced unlock.
Could the PRP side be shrunk the same way so both callbacks obey one
convention and the duplicated unlock disappears?

> diff --git a/net/hsr/hsr_forward.c b/net/hsr/hsr_forward.c
> index 0774981a65c16..8e4158a9b57c8 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);
>  	}
>  }

[Severity: Medium]
[Finding: main-1-1]
[Sources: opus-5]
With the lock released here, allocation of the sequence number and the
actual transmit are no longer atomic with respect to each other. Can frames
now leave the ring out of sequence order?

CPU0 handle_std_frame()
	spin_lock_bh(&hsr->seqnr_lock);
	frame->sequence_nr = 100;
	hsr->sequence_nr = 101;
	spin_unlock_bh(&hsr->seqnr_lock);

CPU1 handle_std_frame()
	spin_lock_bh(&hsr->seqnr_lock);
	frame->sequence_nr = 101;
	hsr->sequence_nr = 102;
	spin_unlock_bh(&hsr->seqnr_lock);

CPU1 then reaches hsr_forward_do() -> hsr_xmit() -> dev_queue_xmit() on the
slaves before CPU0 does, so 101 goes on the wire before 100. hsr_dev_setup()
sets dev->lltx = true and the master has no queue, so nothing re-establishes
ordering between the two.

Local receive is fine, since hsr_check_duplicate() uses the per-node bitmap
under node->seq_out_lock. The changelog only justifies the relaxation with
this kernel's own array based discard, which is the point 06afd2c31d33
raised for the peer side:

	"if the higher sequence number leaves on wire before the lower does
	 and the destination receives them in that order ... it will drop the
	 packet with the lower sequence number and never inject into the
	 stack"

How is a remote DANH, RedBox or hardware peer that still uses last-seq or
window based duplicate discard expected to behave here, including a Linux
peer older than aae9d6b616b5? If out-of-order egress is considered
acceptable, could the changelog say so explicitly?

[ ... ]

Re: [PATCH net] hsr: Avoid holding seqnr_lock while transmitting packets

[Hangbin Liu]

Hi Simon,
On Fri, Aug 07, 2026 at 10:43:25AM +0100, Simon Horman wrote:
[ ... ]
> Simon says:
> 
> I don't think the advice on how to annotate this is correct
> 
> Tags trump freeform text, but Depends-on is a not a standard tag and as
> such should not be used.
> 
> I believe that the correct approach would be a tag like this:
> 
>   Cc: <stable@vger.kernel.org> # aae9d6b616b5: hsr: Implement more robust duplicate discard for HSR
> 
> Link: https://docs.kernel.org/process/stable-kernel-rules.html#option-1

Thanks, I will update with this.

> 
> > diff --git a/net/hsr/hsr_device.c b/net/hsr/hsr_device.c
> > index 5555b71ab19b5..14e8d06762294 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);
> 
> [Severity: Medium]
> [Finding: main-2-3]
> [Sources: opus-5]
> Can this lose statistics updates? The tail of hsr_forward_skb() does plain
> read-modify-write on struct net_device_stats for the master and interlink
> ports:
> 
> net/hsr/hsr_forward.c:hsr_forward_skb() {
> 	...
> 	if (port->type == HSR_PT_MASTER || port->type == HSR_PT_INTERLINK) {
> 		port->dev->stats.tx_packets++;
> 		port->dev->stats.tx_bytes += skb->len;
> 	}
> 	...
> out_drop:
> 	rcu_read_unlock();
> 	port->dev->stats.tx_dropped++;
> }
> 
> Before this change every caller reaching those increments for
> master/interlink held hsr->seqnr_lock across all of hsr_forward_skb(), which
> made them mutually exclusive as a side effect.
> 
> hsr_dev_setup() sets dev->lltx = true ("Prevent recursive tx locking") and
> the master has no qdisc, so __dev_queue_xmit() calls hsr_dev_xmit() without
> the txq lock. Two CPUs can then run tx_packets++ concurrently and drop an
> increment, and tx_bytes += skb->len can tear on 32-bit. The interlink RX
> path, hsr_handle_frame() -> hsr_forward_skb(), now races the same counters.
> 
> Would switching these to dev_sw_netstats or otherwise per-CPU counters be
> preferable while the lock goes away?

Using dev_dstats_* looks good since it has drop count. But hsr also counts
dev->stats.multicast. Do you have any advice other than re-implement a
per-CPU counter for multicast specifically?


[ ... ]

> [Severity: Low]
> [Finding: main-2-2]
> [Sources: opus-5]
> This isn't a bug, but the two implementers of the same
> hsr_proto_ops->send_sv_frame callback now follow different rules for
> seqnr_lock.
> 
> send_hsr_supervision_frame() was converted to the minimal scope the patch
> establishes in handle_std_frame(): lock, bump sup_sequence_nr/sequence_nr,
> unlock.
> 
> send_prp_supervision_frame() still takes seqnr_lock before the sequence
> number bump and holds it across skb_put(), ether_addr_copy() and the
> possibly reallocating skb_put_padto(), which is also why it keeps the
> duplicated unlock in the error path and just before hsr_forward_skb():
> 
> net/hsr/hsr_device.c:send_prp_supervision_frame() {
> 	spin_lock_bh(&hsr->seqnr_lock);
> 	hsr_stag->sequence_nr = htons(hsr->sup_sequence_nr);
> 	hsr->sup_sequence_nr++;
> 	...
> 	if (skb_put_padto(skb, ETH_ZLEN)) {
> 		spin_unlock_bh(&hsr->seqnr_lock);
> 		return;
> 	}
> 
> 	spin_unlock_bh(&hsr->seqnr_lock);
> 
> 	hsr_forward_skb(skb, master);
> }
> 
> Nothing in the extended region re-acquires seqnr_lock and the two unlock
> sites are on mutually exclusive paths, so this is not an unbalanced unlock.
> Could the PRP side be shrunk the same way so both callbacks obey one
> convention and the duplicated unlock disappears?

OK, I will update the prp path.

> 
> > diff --git a/net/hsr/hsr_forward.c b/net/hsr/hsr_forward.c
> > index 0774981a65c16..8e4158a9b57c8 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);
> >  	}
> >  }
> 
> [Severity: Medium]
> [Finding: main-1-1]
> [Sources: opus-5]
> With the lock released here, allocation of the sequence number and the
> actual transmit are no longer atomic with respect to each other. Can frames
> now leave the ring out of sequence order?
> 
> CPU0 handle_std_frame()
> 	spin_lock_bh(&hsr->seqnr_lock);
> 	frame->sequence_nr = 100;
> 	hsr->sequence_nr = 101;
> 	spin_unlock_bh(&hsr->seqnr_lock);
> 
> CPU1 handle_std_frame()
> 	spin_lock_bh(&hsr->seqnr_lock);
> 	frame->sequence_nr = 101;
> 	hsr->sequence_nr = 102;
> 	spin_unlock_bh(&hsr->seqnr_lock);
> 
> CPU1 then reaches hsr_forward_do() -> hsr_xmit() -> dev_queue_xmit() on the
> slaves before CPU0 does, so 101 goes on the wire before 100. hsr_dev_setup()
> sets dev->lltx = true and the master has no queue, so nothing re-establishes
> ordering between the two.
> 
> Local receive is fine, since hsr_check_duplicate() uses the per-node bitmap
> under node->seq_out_lock. The changelog only justifies the relaxation with
> this kernel's own array based discard, which is the point 06afd2c31d33
> raised for the peer side:
> 
> 	"if the higher sequence number leaves on wire before the lower does
> 	 and the destination receives them in that order ... it will drop the
> 	 packet with the lower sequence number and never inject into the
> 	 stack"
> 
> How is a remote DANH, RedBox or hardware peer that still uses last-seq or
> window based duplicate discard expected to behave here, including a Linux
> peer older than aae9d6b616b5? If out-of-order egress is considered
> acceptable, could the changelog say so explicitly?

I think this should not be a problem. The devices running HSR should be in
the same network. It is seldom that one device is upgraded while another
one is left with an old kernel. If the packets go through the internet
via interlink, then no one could promise that the packets are in order.

Thanks
Hangbin

Re: [PATCH net] hsr: Avoid holding seqnr_lock while transmitting packets

[Felix Maurer]

Hi Hangbin, hi Simon,

On Fri, Aug 07, 2026 at 08:42:30PM +0800, Hangbin Liu wrote:
> Hi Simon,
> On Fri, Aug 07, 2026 at 10:43:25AM +0100, Simon Horman wrote:
> [ ... ]
> > Simon says:
> >
> > I don't think the advice on how to annotate this is correct
> >
> > Tags trump freeform text, but Depends-on is a not a standard tag and as
> > such should not be used.
> >
> > I believe that the correct approach would be a tag like this:
> >
> >   Cc: <stable@vger.kernel.org> # aae9d6b616b5: hsr: Implement more robust duplicate discard for HSR
> >
> > Link: https://docs.kernel.org/process/stable-kernel-rules.html#option-1
>
> Thanks, I will update with this.

Thank you for the patch! Just to let you know, there is a patch
addressing the same thing in an in-flight patchset [1]. Unfortunately, I
didn't have a lot of time to review in the last weeks, so I'll just
leave this note here so you can coordinate if necessary :)

Thanks,
   Felix


[1]: https://lore.kernel.org/netdev/20260803222211.877-3-xiexinet@gmail.com/

Re: [PATCH net] hsr: Avoid holding seqnr_lock while transmitting packets

[Hangbin Liu]

On Fri, Aug 07, 2026 at 03:16:36PM +0200, Felix Maurer wrote:
> Hi Hangbin, hi Simon,
> 
> On Fri, Aug 07, 2026 at 08:42:30PM +0800, Hangbin Liu wrote:
> > Hi Simon,
> > On Fri, Aug 07, 2026 at 10:43:25AM +0100, Simon Horman wrote:
> > [ ... ]
> > > Simon says:
> > >
> > > I don't think the advice on how to annotate this is correct
> > >
> > > Tags trump freeform text, but Depends-on is a not a standard tag and as
> > > such should not be used.
> > >
> > > I believe that the correct approach would be a tag like this:
> > >
> > >   Cc: <stable@vger.kernel.org> # aae9d6b616b5: hsr: Implement more robust duplicate discard for HSR
> > >
> > > Link: https://docs.kernel.org/process/stable-kernel-rules.html#option-1
> >
> > Thanks, I will update with this.
> 
> Thank you for the patch! Just to let you know, there is a patch
> addressing the same thing in an in-flight patchset [1]. Unfortunately, I
> didn't have a lot of time to review in the last weeks, so I'll just
> leave this note here so you can coordinate if necessary :)
> 
> Thanks,
>    Felix
> 
> 
> [1]: https://lore.kernel.org/netdev/20260803222211.877-3-xiexinet@gmail.com/
> 

Oh, interesting. I just came back to work this week and found this issue
when reviewing the syzbot bug list. Since Xie Xin has worked on this,
let's drop my patch and fix it in his patch set.

Cheers
Hangbin

Re: [PATCH net] hsr: Avoid holding seqnr_lock while transmitting packets

[Xin Xie]

On 07/08/2026 16:12, Hangbin Liu wrote:
> On Fri, Aug 07, 2026 at 03:16:36PM +0200, Felix Maurer wrote:
> 
> Oh, interesting. I just came back to work this week and found this issue
> when reviewing the syzbot bug list. Since Xie Xin has worked on this,
> let's drop my patch and fix it in his patch set.
> 
> Cheers
> Hangbin

Hi Hangbin,

Thanks for coordinating. I have just posted v5:

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

Patch 2 contains the overlapping seqnr_lock fix, including the affected TX statistics and the PRP supervision path.

Thanks,
-- 
Xin