public inbox for b43-dev@lists.infradead.org
 help / color / mirror / Atom feed
From: "Michael Büsch" <m@bues.ch>
To: b43-dev@lists.infradead.org
Subject: problems with b43 and greedy traffic
Date: Sun, 12 Jun 2011 10:19:44 +0200	[thread overview]
Message-ID: <20110612101944.4b6b55c5@maggie> (raw)
In-Reply-To: <5E9810AB-73AC-4A86-99BC-93771C7DE655@ing.unibs.it>

On Sun, 12 Jun 2011 07:28:10 +0200
francesco.gringoli at 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.gringoli at 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.

  reply	other threads:[~2011-06-12  8:19 UTC|newest]

Thread overview: 27+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-04-21 18:31 problems with b43 and greedy traffic francesco.gringoli at ing.unibs.it
2011-04-21 18:35 ` Michael Büsch
2011-04-21 21:08 ` Larry Finger
2011-04-22 10:09   ` francesco.gringoli at ing.unibs.it
2011-04-22 19:37   ` francesco.gringoli at ing.unibs.it
2011-04-22 19:58     ` Francesco Gringoli
2011-04-22 21:07       ` Tom Gundersen
2011-04-22 23:37         ` francesco.gringoli at ing.unibs.it
2011-04-26 10:11   ` francesco.gringoli at ing.unibs.it
2011-04-26 14:53     ` Rafał Miłecki
2011-04-26 15:21       ` problems with b43 and greedy (UDP) traffic Larry Finger
2011-04-30 17:03         ` francesco.gringoli at ing.unibs.it
2011-04-30 15:54       ` problems with b43 and greedy traffic francesco.gringoli at ing.unibs.it
2011-04-30 16:01       ` francesco.gringoli at ing.unibs.it
2011-06-11 20:28 ` Rafał Miłecki
2011-06-11 20:34   ` Michael Büsch
2011-06-11 20:48     ` Rafał Miłecki
2011-06-11 21:17       ` Rafał Miłecki
2011-06-11 21:46         ` Michael Büsch
2011-06-11 22:55           ` francesco.gringoli at ing.unibs.it
2011-06-11 23:19             ` Michael Büsch
2011-06-12  5:28               ` francesco.gringoli at ing.unibs.it
2011-06-12  8:19                 ` Michael Büsch [this message]
2011-06-12 12:53                   ` Rafał Miłecki
2011-06-12 15:33                   ` francesco.gringoli at ing.unibs.it
2011-06-12 14:45           ` Rafał Miłecki
2011-06-11 22:48         ` francesco.gringoli at ing.unibs.it

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20110612101944.4b6b55c5@maggie \
    --to=m@bues.ch \
    --cc=b43-dev@lists.infradead.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox