2008-07-02 23:05:41

by Michael Büsch

[permalink] [raw]
Subject: [PATCH stable] b43: Do not return TX_BUSY from op_tx

Never return TX_BUSY from op_tx. It doesn't make sense to return
TX_BUSY, if we can not transmit the packet.
Drop the packet and return TX_OK.
This will fix the resume hang.

Upstream commit is
66193a7cef2239bfd1b9b96e304770facf7a49c7

Signed-off-by: Michael Buesch <[email protected]>


Index: linux-2.6.25.6/drivers/net/wireless/b43/main.c
===================================================================
--- linux-2.6.25.6.orig/drivers/net/wireless/b43/main.c 2008-06-14 22:47:31.000000000 +0200
+++ linux-2.6.25.6/drivers/net/wireless/b43/main.c 2008-07-03 00:50:53.000000000 +0200
@@ -2604,25 +2604,30 @@ static int b43_op_tx(struct ieee80211_hw
struct b43_wl *wl = hw_to_b43_wl(hw);
struct b43_wldev *dev = wl->current_dev;
unsigned long flags;
int err;

if (unlikely(!dev))
- return NETDEV_TX_BUSY;
+ goto drop_packet;

/* Transmissions on seperate queues can run concurrently. */
read_lock_irqsave(&wl->tx_lock, flags);

err = -ENODEV;
if (likely(b43_status(dev) >= B43_STAT_STARTED))
err = b43_dma_tx(dev, skb, ctl);

read_unlock_irqrestore(&wl->tx_lock, flags);

if (unlikely(err))
- return NETDEV_TX_BUSY;
+ goto drop_packet;
+ return NETDEV_TX_OK;
+
+drop_packet:
+ /* We can not transmit this packet. Drop it. */
+ dev_kfree_skb_any(skb);
return NETDEV_TX_OK;
}

static int b43_op_conf_tx(struct ieee80211_hw *hw,
int queue,
const struct ieee80211_tx_queue_params *params)


2008-07-03 06:36:05

by Kalle Valo

[permalink] [raw]
Subject: Re: [PATCH stable] b43: Do not return TX_BUSY from op_tx

Michael Buesch <[email protected]> writes:

> +drop_packet:
> + /* We can not transmit this packet. Drop it. */
> + dev_kfree_skb_any(skb);
> return NETDEV_TX_OK;

So no need to call ieee80211_tx_status() in this case? I'm just
curious about this, nothing else.

--
Kalle Valo

2008-07-03 12:25:05

by Johannes Berg

[permalink] [raw]
Subject: Re: [PATCH stable] b43: Do not return TX_BUSY from op_tx

On Thu, 2008-07-03 at 10:31 +0200, Michael Buesch wrote:
> On Thursday 03 July 2008 08:35:23 Kalle Valo wrote:
> > Michael Buesch <[email protected]> writes:
> >
> > > +drop_packet:
> > > + /* We can not transmit this packet. Drop it. */
> > > + dev_kfree_skb_any(skb);
> > > return NETDEV_TX_OK;
> >
> > So no need to call ieee80211_tx_status() in this case? I'm just
> > curious about this, nothing else.
>
> I don't think we must call tx_status for dropped or lost packets.

That's a bit of a corner case really. If you call tx-status then the
rate control algorithm will adjust to something that hasn't happened,
and if you don't then the packet won't be on the monitor interface.

In practice, it doesn't really matter because so far this only happens
during resume and that'll hopefully get fixed.

johannes


Attachments:
signature.asc (836.00 B)
This is a digitally signed message part

2008-07-03 12:12:16

by Michael Büsch

[permalink] [raw]
Subject: Re: [PATCH stable] b43: Do not return TX_BUSY from op_tx

On Thursday 03 July 2008 08:35:23 Kalle Valo wrote:
> Michael Buesch <[email protected]> writes:
>
> > +drop_packet:
> > + /* We can not transmit this packet. Drop it. */
> > + dev_kfree_skb_any(skb);
> > return NETDEV_TX_OK;
>
> So no need to call ieee80211_tx_status() in this case? I'm just
> curious about this, nothing else.

I don't think we must call tx_status for dropped or lost packets.

--
Greetings Michael.