Skip to contentSkip to conversation

b43: Do not return TX_BUSY from op_tx

PATCH In review Michael Buesch 3 participants 4 messages Jul 2, 2008 – Jul 3, 2008

lore
Share & tools
lore

Michael Buesch

author
#0 lore raw

PATCH 1/1 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]>
drivers/net/wireless/b43/main.c +7 -2
Index: linux-2.6.25.6/drivers/net/wireless/b43/main.c
===================================================================
@@ -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)
#1 lore raw

drivers/net/wireless/b43/main.c static int b43_op_tx(struct ieee80211_hw · line 2628

Quoted 4 lines · Michael Buesch
drivers/net/wireless/b43/main.c +3 -0
+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.

Signature · 1 line
Kalle Valo
author
#2 lore raw
Quoted 9 lines · On Thursday 03 July 2008 08:35:23 Kalle Valo wrote:

drivers/net/wireless/b43/main.c static int b43_op_tx(struct ieee80211_hw · line 2628

Quoted 4 lines · Michael Buesch
drivers/net/wireless/b43/main.c +3 -0
+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.

Signature · 1 line
Greetings Michael.
#3 lore raw
Quoted 11 lines · Michael Buesch
Quoted 9 lines · On Thursday 03 July 2008 08:35:23 Kalle Valo wrote:

drivers/net/wireless/b43/main.c static int b43_op_tx(struct ieee80211_hw · line 2628

Quoted 4 lines · Michael Buesch
drivers/net/wireless/b43/main.c +3 -0
+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.

Quoted 1 line

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