On Jun 12, 2011, at 10:19 AM, Michael Büsch wrote: > On Sun, 12 Jun 2011 07:28:10 +0200 > francesco.gring...@ing.unibs.it wrote: > >> On Jun 12, 2011, at 1:19 AM, Michael Büsch wrote: >> >>> On Sun, 12 Jun 2011 00:55:58 +0200 >>> francesco.gring...@ing.unibs.it wrote: >>> >>>> Anyway if you put some printk in b43_tx_work you will see that some TCP >>>> segments are discarded by it trying to send packets to the dma even if the >>>> rings are full. This prevents packets to queue up correctly in the qdisc, >>>> so shaping traffic through a b43 interface becomes really difficult. >>> >>> So this doesn't sound like a b43 problem to me. >>> The driver disables the software queues, if the hardware queue is full. >>> If the stack continues to push packets to the driver while queues are >>> stopped, >>> the only thing the driver can do is discard them. >> I agree with you, and this was perfectly implemented up to 2.6.31. After the >> introduction of the asynchronous worker, the behavior you are talking about >> has been almost broken: if b43_dma_tx stops the queue, the stack does not >> send any more packet to the driver. It is b43_tx_work that keeps sending >> packets to the hardware (b43_dma_tx). Rather weird :-) >> >> Try to explain better: imagine the worker finished its previous run and gave >> up because the mac queue was empty. No problem up to now, but suppose the >> dma ring has only one more descriptor free and that hundreds of stations are >> competing for the channel so that the firmware is not dequeuing packets for >> a long time. Now a bunch of packets is injected by the stack: b43_op_tx >> pushes them to the mac queue and finally it add works to the work queue. >> >> Now b43_tx_work is run again by the kernel, it pops the first packet, send >> it to the hardware (b43_dma_tx) that ultimately stops the queues because, as >> I said before, only one more position was free in the queue. From here on >> the stack stops sending packets to b43_op_tx (maybe one more packet can >> pass?), it is b43_tx_work that continues to send all the remaining packets >> in the mac queue to the hardware that can only drop them. >> >> Take a look at b43_dma_tx, and focus on the comment >> >> /* We get here only because of a bug in mac80211. >> * Because of a race, one packet may be queued after >> * the queue is stopped, thus we got called when we shouldn't. >> * For now, just refuse the transmit. */ >> >> It's not true anymore after 2.6.32 but the code has been left as if it were. >> It is not "one packet may be queued" because of the mac80211, several >> instead can be passed here, but only by b43_tx_work, not the mac. > > Ok, thanks. I see the bug now. > This should be pretty easy to fix, though. Just don't unqueue the packet > from the b43_tx_work queue, if b43_dma_tx failed to transmit it, due to full > ring. > And then poke the TX work again, if the queues are unblocked. Hi Michael,
thanks, I see. I can work on it. I think also Rafał comment is right and we should move the ring check code directly in b43_tx_work which is now the only piece of code to trigger the dma (or pio) engine for transmission. BR, -Francesco _______________________________________________ b43-dev mailing list b43-dev@lists.infradead.org http://lists.infradead.org/mailman/listinfo/b43-dev
