AR6000 netif_queue_stop non stop, Bug?
Rask Ingemann Lambertsen
rask at sygehus.dk
Sat Mar 28 15:58:11 CET 2009
On Fri, Mar 27, 2009 at 10:05:36PM +0300, Ivan Petrov wrote:
> Clean patch attached.
> If posible, please apply it to repo.
> Regards, Ivan.
> @@ -1278,6 +1280,7 @@
> connect.EpCallbacks.EpRecv = ar6000_rx;
> connect.EpCallbacks.EpRecvRefill = ar6000_rx_refill;
> connect.EpCallbacks.EpSendFull = ar6000_tx_queue_full;
> + connect.EpCallbacks.EpSendAvail = ar6000_tx_queue_avail;
> /* set the max queue depth so that our ar6000_tx_queue_full handler gets called.
> * Linux has the peculiarity of not providing flow control between the
> * NIC and the network stack. There is no API to indicate that a TX packet
[code continues like this]
* was sent which could provide some back pressure to the network stack.
* Under linux you would have to wait till the network stack consumed all sk_buffs
* before any back-flow kicked in. Which isn't very friendly.
* So we have to manage this ourselves */
connect.MaxSendQueueDepth = 32;
Not a comment about you patch, but since you're hacking on the driver, I
think I'll mention it here. You're not supposed to have a lot of TX packets
queued at the driver level. You should only queue enough that you have time
to prepare a new packet for the hardware while the hardware is still busy
sending the previous packet. If you queue many packets at the driver level,
the network stack has no chance of reordering the packets. It also won't
know that your hardware is already fully occupied after 2-3 calls of
dev->hard_start_xmit(). To me, it looks from the comment above as if the
ar6000 driver is shooting itself in the foot by maintaining a large queue,
because doing so causes exactly the problem that the comment complains
about. netif_stop_queue()/netif_wake_queue() _is_ the flow control.
If you want a large TX queue, look at dev->tx_queue_len. You can check
the value with ifconfig. Example:
$ /sbin/ifconfig usb0
usb0 Link encap:Ethernet HWaddr 06:e2:8a:da:03:a9
inet addr:192.168.0.202 Bcast:192.168.0.255 Mask:255.255.255.0
inet6 addr: fe80::4e2:8aff:feda:3a9/64 Scope:Link
UP BROADCAST RUNNING MULTICAST MTU:1500 Metric:1
RX packets:13006 errors:0 dropped:0 overruns:0 frame:0
TX packets:12510 errors:0 dropped:0 overruns:0 carrier:0
RX bytes:1044602 (1020.1 KiB) TX bytes:4824200 (4.6 MiB)
The default dev->tx_queue_len was raised from 100 to 1000 in 2003-2004
FWIW, when I wrote the driver for the NE3200 10 Mbps Ethernet card, I
found that a TX buffer large enough to hold three full size packets was
enough to keep the number of queued, unsent packet bytes at least as large
as the number of packet bytes being transmitted by the chip.
> @@ -2317,7 +2315,7 @@
> /* flush data queues */
> - netif_wake_queue(ar->arNetDev);
> + netif_start_queue(ar->arNetDev);
> if ((OPEN_AUTH == ar->arDot11AuthMode) &&
> (NONE_AUTH == ar->arAuthMode) &&
Out of curiousity, what does this change do?
Rask Ingemann Lambertsen
Danish law requires addresses in e-mail to be logged and stored for a year
More information about the openmoko-kernel