Re: [PATCH net v7 2/4] net: hsr: preserve submission order without a forwarding lock
flat view
From: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
Date: 2026-10-14 13:47:22
Also in:
linux-kselftest, lkml, stable
On 2026-10-09 22:13:22 [+0200], Xin Xie wrote:
Holding seqnr_lock across dev_queue_xmit() serializes numbering and submission, but creates transmit-lock dependencies that can deadlock stacked HSR devices. Merely shrinking the lock to the counter lets one CPU send frame N after another has sent N+1; old HSR peers discard the late N as stale.
What are stacked HSR devices? Looking from the setup in the referenced report this is a bridge device attached to a HSR ring. An announce frame is sent and this goes to hsr_dev_xmit(). This does not look like a normal setup. Is this something you use or is this something the bot came up with?
Use one consumer for master TX, interlink RX and internally generated supervision frames. An idle caller processes its own input inline; contended inputs wait in a FIFO drained by one BH work item. Allocate sequence numbers only when the consumer executes a frame, and submit it to every lower before numbering the next frame. The short counter lock is released before forwarding. Limit queued inputs to 1024 jobs and 8 MiB, reserving capacity for internal supervision. Carry transmit recursion depth across deferred execution, cancel the worker and purge the queue on teardown, and count queue drops once through core per-CPU statistics. LAN A/B reception stays synchronous and tagged frames retain their received numbers. The ordering guarantee is per-lower submission, not physical wire order across multiple TX queues. On PREEMPT_RT, high-priority load preempted both the inline consumer and BH worker while another CPU continued to enqueue inputs. All 2118 inputs completed without drops; the finite data workload completed 4.07 seconds after the load ended.
This looks very complex and I am not a big fan of it. I would prefer to break lock chain early assuming the setup is valid/ sane (see above). The ID by itself can be assigned with an atomic_inc() but then you don't have the transmit ordering. So you introduce the workqueue for the ordering which is ugh. I'm not sure how this works if you get submissions from two CPUs given the lock is gone and the workqueue is per-CPU.
Reported-by: syzbot+fbf74291c3b7e753b481@syzkaller.appspotmail.com Link: https://syzkaller.appspot.com/bug?extid=fbf74291c3b7e753b481 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.") Signed-off-by: Xin Xie <redacted>
Sebastian