r8169 changes against 2.6.23-rc3
Subsystems (2)
Quoted 9 lines · Dirk
Quoted 5 lines
So it seems that when the driver tries to queue a packet while the controller is busy processing the queue, the newly queued packet does not get noticed by the controller (until further packet activity occurs). Perhaps there is a problem with the memory barriers when adding to the TX queue, but I'm a newbie on linux kernel memory barriers.
One thing I noticed a while ago (march) is that floodpinging (ping -f) the r8169 host from an external system also increases performance without changing code.
Yes, I just tried this and saw the same result. Makes perfect sense - if the TX queue is normally getting stuck until TCP retransmits, then keeping the TX queue busy keeps the queue from remaining stuck. I think this is a good demonstration that the underlying problem is a stuck TX queue as suggested.
Quoted 2 lines
I ended up (until now perhaps :-) with disabling the onboard nic and adding an e1000 card.
Yes, ditching the realtek interface and going with an ad-on nic seems to be what everyone has been doing to get around this problem. Perhaps you'd like to try the busy-wait workaround with ndelay(10)? It has saved me from buying an e1000 card as well.
Speaking of the e1000, I notice that its TX queue processing code for that driver includes spin_lock_irqsave()/spin_unlock_irqrestore() protection on access to the queue. The r8169 driver seems to be missing equivalent code. Last time I dealt with kernel locking bugs was in the old days of splnet()/splx(), so I could use some help here, but I suspect this could be fixed with more careful locking.
So I did some experimenting with locking, but eventually found that this chunk:
@@ -2677,10 +2681,18 @@ static void rtl8169_tx_interrupt(struct net_device *dev,
if (tp->dirty_tx != dirty_tx) {
tp->dirty_tx = dirty_tx;- - smp_wmb();
- - if (netif_queue_stopped(dev) &&
- - (TX_BUFFS_AVAIL(tp) >= MAX_SKB_FRAGS)) {
- - netif_wake_queue(dev);
+ smp_mb();
+ if (unlikely(netif_queue_stopped(dev))) {
+ netif_tx_lock(dev);
+ if (TX_BUFFS_AVAIL(tp) >= MAX_SKB_FRAGS)
+ netif_wake_queue(dev);
+ if (dirty_tx != tp->cur_tx)
+ RTL_W8(TxPoll, NPQ);
+ netif_tx_unlock(dev);
+ } else if (dirty_tx != tp->cur_tx) {
+ netif_tx_lock(dev);
+ RTL_W8(TxPoll, NPQ);
+ netif_tx_unlock(dev);
}
}
}from the patch in http://www.spinics.net/lists/netdev/msg33960.html
was sufficient to fix the stuck TX queue bug without the busy-wait. Actually just the else portion of the above chunk was sufficient in my testing, without the barrier change or the if statement change.
David Gundersen pointed me to this potential fix days ago, but I didn't consider it first since the change had (presumably intentionally) been dropped from the set of diffs Francois pointed me to. Given that I had reported the same problem as David Gundersen (and Dirk, and other samba users...) I thought this patch had been ruled out. Apparently not. Hopefully this can be dusted off and made into a fairly high priority fix as it has been biting realtek users since last year at least.