[PATCH] wifi: ath9k: track PS-Poll responses in the BlockAck window

[Evan Severson <eseverson@gmail.com>]

ath_tx_setup_buffer() takes a sequence number from the TID for every
QoS data frame, but a frame only enters the BlockAck window when it
carries IEEE80211_TX_CTL_AMPDU. Since commit 592234e941f1 ("wifi:
mac80211: Fix iTXQ AMPDU fragmentation handling"), mac80211 sets that
flag only when dequeuing from a TXQ. Frames that reach ath_tx_start()
directly while a session is running, such as PS-Poll responses for a
dozing station, now use up session sequence numbers without being
tracked.

Once seq_next is more than baw_size ahead of seq_start, every frame
the TXQ hands out is outside the window. ath_tx_get_tid_subframe()
puts it back on the retry queue and returns -EINPROGRESS on every
attempt, nothing ever moves the window, and the AP stops sending
unicast data to the station until the session is torn down or the
station reassociates.

Seen on a QCA9563 AP (OpenWrt) with a station that stayed in power
save for several seconds under downlink load: the window was empty
and seq_next was about 2170 ahead of seq_start. With this change, a
90 minute run saw a run of more than 3800 such frames and no stall.

Flag unfragmented QoS data frames of an active session as A-MPDU where
the sequence number is assigned, so they are tracked like any other
session frame. Fragments stay unflagged, as mac80211 intends. A frame
from ath_tx_start() cannot wait for the window, so if it is already
outside it, send it untracked as before rather than set a bit beyond
the window.

A PS-Poll response is sent on the UAPSD queue, and ath_tx_start() held
that queue's lock while it took the TID's sequence number. The sequence
counter and the window belong to the lock of the TID's own queue, which
the TXQ path holds when it adds to or advances the window, so take that
lock for the TID work and the UAPSD queue's lock only to hand the frame
to the hardware, as ath9k_release_buffered_frames() already does.

Fixes: 592234e941f1 ("wifi: mac80211: Fix iTXQ AMPDU fragmentation handling")
Cc: stable@vger.kernel.org
Signed-off-by: Evan Severson <eseverson@gmail.com>
---

Notes:
    An alternative would be to restore IEEE80211_TX_CTL_AMPDU on
    non-fragmented PS-Poll responses in mac80211, which would also cover
    other drivers that track the BA window in software. Happy to rework it
    that way if that's preferred.

 drivers/net/wireless/ath/ath9k/xmit.c | 59 +++++++++++++++++++--------
 1 file changed, 41 insertions(+), 18 deletions(-)

diff --git a/drivers/net/wireless/ath/ath9k/xmit.c b/drivers/net/wireless/ath/ath9k/xmit.c
index 3413bb4906d..7af9fa7f97c 100644
--- a/drivers/net/wireless/ath/ath9k/xmit.c
+++ b/drivers/net/wireless/ath/ath9k/xmit.c
@@ -54,7 +54,7 @@ static u16 bits_per_symbol[][2] = {
 };
 
 static void ath_tx_send_normal(struct ath_softc *sc, struct ath_txq *txq,
-			       struct ath_atx_tid *tid, struct sk_buff *skb);
+			       struct sk_buff *skb);
 static void ath_tx_complete(struct ath_softc *sc, struct sk_buff *skb,
 			    int tx_flags, struct ath_txq *txq,
 			    struct ieee80211_sta *sta);
@@ -2104,20 +2104,14 @@ static void ath_tx_txqaddbuf(struct ath_softc *sc, struct ath_txq *txq,
 }
 
 static void ath_tx_send_normal(struct ath_softc *sc, struct ath_txq *txq,
-			       struct ath_atx_tid *tid, struct sk_buff *skb)
+			       struct sk_buff *skb)
 {
-	struct ieee80211_tx_info *tx_info = IEEE80211_SKB_CB(skb);
 	struct ath_frame_info *fi = get_frame_info(skb);
 	struct list_head bf_head;
 	struct ath_buf *bf = fi->bf;
 
 	INIT_LIST_HEAD(&bf_head);
 	list_add_tail(&bf->list, &bf_head);
-	bf->bf_state.bf_type = 0;
-	if (tid && (tx_info->flags & IEEE80211_TX_CTL_AMPDU)) {
-		bf->bf_state.bf_type = BUF_AMPDU;
-		ath_tx_addto_baw(tid, bf);
-	}
 
 	bf->bf_next = NULL;
 	bf->bf_lastbf = bf;
@@ -2240,6 +2234,15 @@ static struct ath_buf *ath_tx_setup_buffer(struct ath_softc *sc,
 			INCR(tid->seq_next, IEEE80211_SEQ_MAX);
 
 		bf->bf_state.seqno = seqno;
+
+		/* Frames that bypass the TXQs (e.g. PS-Poll responses) lack
+		 * IEEE80211_TX_CTL_AMPDU but still use a session sequence
+		 * number; track them in the BA window too.
+		 */
+		if (tid->active && !fragno &&
+		    !ieee80211_has_morefrags(hdr->frame_control) &&
+		    ieee80211_is_data_qos(hdr->frame_control))
+			IEEE80211_SKB_CB(skb)->flags |= IEEE80211_TX_CTL_AMPDU;
 	}
 
 	bf->bf_mpdu = skb;
@@ -2356,20 +2359,36 @@ int ath_tx_start(struct ieee80211_hw *hw, struct sk_buff *skb,
 		tid = ath_get_skb_tid(an, skb);
 	}
 
-	ath_txq_lock(sc, txq);
-	if (txq == sc->tx.txq_map[q]) {
-		fi->txq = q;
-		++txq->pending_frames;
-	}
+	/*
+	 * The TID's sequence counter and BlockAck window are protected by
+	 * the lock of the TID's own queue, which is not txq for a PS-Poll
+	 * response. As in ath9k_release_buffered_frames(), do the TID work
+	 * under that lock first and queue to the hardware under txq's lock.
+	 */
+	if (tid)
+		ath_txq_lock(sc, tid->txq);
 
 	bf = ath_tx_setup_buffer(sc, txq, tid, skb);
+	if (bf && tid && (info->flags & IEEE80211_TX_CTL_AMPDU)) {
+		if (BAW_WITHIN(tid->seq_start, tid->baw_size,
+			       bf->bf_state.seqno)) {
+			bf->bf_state.bf_type = BUF_AMPDU;
+			ath_tx_addto_baw(tid, bf);
+		} else {
+			/* Can't wait for the window; send it untracked */
+			info->flags &= ~IEEE80211_TX_CTL_AMPDU;
+		}
+	}
+
+	if (tid)
+		ath_txq_unlock(sc, tid->txq);
+
 	if (!bf) {
-		ath_txq_skb_done(sc, skb);
 		if (txctl->paprd)
 			dev_kfree_skb_any(skb);
 		else
 			ieee80211_free_txskb(sc->hw, skb);
-		goto out;
+		return 0;
 	}
 
 	bf->bf_state.bfs_paprd = txctl->paprd;
@@ -2378,9 +2397,13 @@ int ath_tx_start(struct ieee80211_hw *hw, struct sk_buff *skb,
 		bf->bf_state.bfs_paprd_timestamp = jiffies;
 
 	ath_set_rates(vif, sta, bf);
-	ath_tx_send_normal(sc, txq, tid, skb);
 
-out:
+	ath_txq_lock(sc, txq);
+	if (txq == sc->tx.txq_map[q]) {
+		fi->txq = q;
+		++txq->pending_frames;
+	}
+	ath_tx_send_normal(sc, txq, skb);
 	ath_txq_unlock(sc, txq);
 
 	return 0;
@@ -2949,7 +2972,7 @@ int ath9k_tx99_send(struct ath_softc *sc, struct sk_buff *skb,
 	ath9k_hw_set_desc_link(sc->sc_ah, bf->bf_desc, bf->bf_daddr);
 	ath9k_hw_tx99_start(sc->sc_ah, txctl->txq->axq_qnum);
 
-	ath_tx_send_normal(sc, txctl->txq, NULL, skb);
+	ath_tx_send_normal(sc, txctl->txq, skb);
 
 	return 0;
 }
-- 
2.54.0