Hello Samuel,
Thanks for the quick review.
> David Bidner, le mer. 07 oct. 2026 06:11:02 +0200, a ecrit:
>> Ethernet receive threads and loopback sends can enqueue packets at the
>> same time: they hold different locks,
>
> ? They don't?
>
> loopback_xmit() assumes that net_bh_lock() is already taken, and
> ethernet_demuxer takes it.
ethernet_demuxer does take it, but the loopback send path does not.
start_bh_atomic() is a no-op in this tree
(pfinet/glue-include/linux/interrupt.h, the net_bh_lock version is commented
out), so dev_queue_xmit() does not take net_bh_lock before calling
loopback_xmit(). An application send runs
S_io_write -> tcp_sendmsg -> tcp_transmit_skb -> ip_queue_xmit
-> dev_queue_xmit -> loopback_xmit -> netif_rx
holding only global_lock, and tcp_retransmit_timer runs the same path from the
timer thread. I checked a running translator: at loopback_xmit the RPC path
has global_lock held and net_bh_lock free, while ethernet_demuxer has
net_bh_lock held and global_lock free. So the two enqueues into backlog are
not mutually exclusive.
>> and the skb queue spinlocks are no-ops on Hurd.
>
> Yes, because we essentially run the whole tcp/ip stack single-threaded
> from net_bh_worker().
Agreed, except for the enqueue itself: the receiver thread calls netif_rx
under net_bh_lock while an RPC or timer thread can call netif_rx under
global_lock. Those two locks are independent, and the skb_queue_tail()/qlen
updates are not atomic.
>> Concurrent updates can corrupt the receive backlog, causing packet
>> loss and TCP timeouts.
>
> I have never observed such a thing. Did you really observe it? How?
Yes. With a read-only probe of the live translator (proc task port and
vm_read, no modification), during a loopback TCP stress the backlog list was
empty while qlen had leaked to about 290-300, netdev_rx_dropped grew to
roughly 1.5-1.8e5, and a stalled loopback connection had unacknowledged
segments with a retransmit timer pending while the timer thread was still
healthy.
>> Add backlog_lock to protect the queue and netdev_dropping in netif_rx(),
>> net_bh() and dev_clear_backlog().
>
> “I had a problem, so I added a lock.
> Now I have two problems: a deadlock and a race condition.”
>
> More seriously, net_bh_lock should already be the mutex that you are
> looking for, that protects the backlog without blocking RPCs.
net_bh_lock protects the backlog from the receiver side, but the loopback
sender does not hold it. Taking it in loopback_xmit() would deadlock two
ways: net_bh_worker holds net_bh_lock and then takes global_lock while the RPC
and timer paths already hold global_lock; and net_bh itself reaches
dev_queue_xmit while holding net_bh_lock, so it would re-lock the same
non-recursive mutex on the same thread. That is why I used a separate lock
taken innermost. I am not claiming the skb spinlocks are fixed in general.
backlog_lock only serializes backlog and netdev_dropping.
David
