mac80211: add stop/start logic for software TXQs

Sometimes, it is required to stop the transmissions momentarily and resume it later; stopping the txqs becomes very critical in scenarios where the packet transmission has to be ceased completely. For example, during the hardware restart, during off channel operations,
when initiating CSA(upon detecting a radar on the DFS channel), etc.

The TX queue stop/start logic in mac80211 works well in stopping the TX when drivers make use of netdev queues, i.e, when Qdiscs in network layer take care of traffic scheduling. Since the devices implementing wake_tx_queue run without Qdiscs, packets will be handed to mac80211 directly without queueing them in the netdev queues. Also, mac80211 does not invoke any of the netif_stop_*/netif_wake_* APIs to stop the transmissions and this might very well lead to undesirabled things.

For example,
During hardware restart, we stop the netdev queues so that packets are not sent to the driver. Since ath10k implements wake_tx_queue, TX queues will not be stopped and packets might reach the hardware while it is restarting; this can make hardware unresponsive and can be recovered only by rebooting the system.

There is another problem to this, it is observed that the packets were sent on the DFS channel for a prolonged duration after radar detection impacting the channel closing times.

We can still invoke netif stop/wake APIs when wake_tx_queue is implemented but this could lead to packet drops in network layer; adding stop/start logic for software TXQs in mac80211 instead makes more sense; the change proposed adds the same in mac80211.

+++ b/net/mac80211/agg-tx.c

net/mac80211/agg-tx.c +0 -0

@@ -213,11 +213,15 @@ static void
 ieee80211_agg_start_txq(struct sta_info *sta, int tid, bool enable)
 {
 	struct ieee80211_txq *txq = sta->sta.txq[tid];

Quoted 11 lines

+++ b/net/mac80211/agg-tx.c

net/mac80211/agg-tx.c +1 -0

@@ -213,11 +213,15 @@ static void
  ieee80211_agg_start_txq(struct sta_info *sta, int tid, bool enable)
  {
  	struct ieee80211_txq *txq = sta->sta.txq[tid];
+	struct ieee80211_local *local = sta->local;
  	struct txq_info *txqi;
 
  	if (!txq)
  		return;
 

net/mac80211/agg-tx.c static void · line 225

Quoted 4 lines

net/mac80211/agg-tx.c +3 -0

+	if (local->txqs_stopped)
+		return;
+
  	txqi = to_txq_info(txq);

This doesn't seem right, all the logic that clears the TXQ_STOP etc. still needs to be invoked, otherwise your TXQ will get stuck completely since you'll never run this code again.

+++ b/net/mac80211/sta_info.c

net/mac80211/sta_info.c +3 -0

@@ -1244,6 +1244,9 @@ void ieee80211_sta_ps_deliver_wakeup(struct sta_info *sta)
  			if (!txq_has_queue(sta->sta.txq[i]))
  				continue;
 
+			if (local->txqs_stopped)
+				continue;
+
  			drv_wake_tx_queue(local, to_txq_info(sta->sta.txq[i]));
  		}

That seems like it's in an odd place, why bother going through all the logic first?

Also, you have the same problem as above - you never re-run this code so
you'd get stuck completely.

net/mac80211/tx.c static bool ieee80211_queue_skb(struct ieee80211_local *local, · line 1567

Quoted 11 lines

net/mac80211/tx.c +4 -0

+++ b/net/mac80211/tx.c

@@ -1559,6 +1559,9 @@ static bool ieee80211_queue_skb(struct ieee80211_local *local,
  	    sdata->vif.type == NL80211_IFTYPE_MONITOR)
  		return false;
 
+	if (local->txqs_stopped)
+		return false;
+
  	if (sdata->vif.type == NL80211_IFTYPE_AP_VLAN)
  		sdata = container_of(sdata->bss,
  				     struct ieee80211_sub_if_data, u.ap);

That also seems too early.

net/mac80211/util.c static void __ieee80211_wake_queue(struct ieee80211_hw *hw, int queue, · line 302

Quoted 10 lines

net/mac80211/util.c +2 -0

index 2d82c88..da7ae8b 100644
@@ -298,6 +298,9 @@ static void __ieee80211_wake_queue(struct ieee80211_hw *hw, int queue,
  	if (local->q_stop_reasons[queue][reason] == 0)
  		__clear_bit(reason, &local->queue_stop_reasons[queue]);
 
+	if (local->ops->wake_tx_queue)
+		__clear_bit(reason, &local->txqs_stopped);

Ah, never mind, you do use it as a bitmap.

But I think your approach is wrong, somehow you have to make sure you restart.

Perhaps a good approach would be to have a check when the driver pulls an SKB, and then record the queue it was pulled. Then, when the stop bitmap goes to 0, you can drv_wake_tx_queue() all the queues that were marked as "pulled while stopped" to recover.

Quoted 4 lines

This doesn't seem right, all the logic that clears the TXQ_STOP etc. still needs to be invoked, otherwise your TXQ will get stuck completely since you'll never run this code again.

If the queues are blocked more than the block ack timeout then the traffic should be sent only in the next BA session.
I am not really sure whether the queues will be blocked more than the block ack timeout value unless the queues are stopped because of multiple reasons.
In any case, traffic will be pushed out in subsequent BA sessions, no?

Quoted 2 lines

Also, you have the same problem as above - you never re-run this code so
you'd get stuck completely.

I didn't get your point here. By the time the queues gets started again, there could be possibility that the station might have been back to sleep. In this case, it is better not to send the traffic, no? Anyways, station would receive the traffic in the next cycle when it is out of sleep. Considering codel logic, there could be frame drops though; maybe I am missing something?

Quoted 9 lines · Manikanta Pubbisetty

Quoted 3 lines

This doesn't seem right, all the logic that clears the TXQ_STOP etc. still needs to be invoked, otherwise your TXQ will get stuck completely since you'll never run this code again.

If the queues are blocked more than the block ack timeout then the traffic should be sent only in the next BA session.
I am not really sure whether the queues will be blocked more than the block ack timeout value unless the queues are stopped because of multiple reasons.
In any case, traffic will be pushed out in subsequent BA sessions, no?

I'm not really sure what you're saying, but it sounds almost like you're confusing a "BA session" with a single A-MPDU? The session will get stuck if you do the code this way, I think.

Quoted 8 lines

Quoted 2 lines

Also, you have the same problem as above - you never re-run this code so
you'd get stuck completely.

I didn't get your point here. By the time the queues gets started again, there could be possibility that the station might have been back to sleep. In this case, it is better not to send the traffic, no? Anyways, station would receive the traffic in the next cycle when it is out of sleep. Considering codel logic, there could be frame drops though; maybe I am missing something?

But this is still the old code before cycling, so you never get here during the TX cycle you're thinking of?