Thread (17 messages) 17 messages, 3 authors, 2d ago

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
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help