Received-SPF: pass (google.com: domain of linux-wireless-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
From:   Johannes Berg <johannes@sipsolutions.net>
To:     linux-wireless@vger.kernel.org
Cc:     Johannes Berg <johannes.berg@intel.com>
Subject: [PATCH] mac80211: properly process TXQ management frames
Date:   Fri, 16 Apr 2021 13:47:04 +0200
Message-Id: <20210416134702.ef8486a64293.If0a9025b39c71bb91b11dd6ac45547aba682df34@changeid>
MIME-Version: 1.0
Content-Transfer-Encoding: 8bit
Precedence: bulk

From: Johannes Berg <johannes.berg@intel.com>

My previous commit to not apply flow control to management frames
that are going over TXQs (which is currently only the case for
iwlwifi, I think) broke things, with iwlwifi firmware crashing on
certain frames. As it turns out, that was due to the frame being
too short: space for the MIC wasn't added at the end of encrypted
management frames.

Clearly, this is due to using the 'frags' queue - this is meant
only for frames that have already been processed for TX, and the
code in ieee80211_tx_dequeue() just returns them. This caused all
management frames to now not get any TX processing.

To fix this, use IEEE80211_TX_INTCFL_NEED_TXPROCESSING (which is
currently used only in other circumstances) to indicate that the
frames need processing, and clear it immediately after so that,
at least in theory, MMPDUs can be fragmented.

Fixes: 73bc9e0af594 ("mac80211: don't apply flow control on management frames")
Signed-off-by: Johannes Berg <johannes.berg@intel.com>
---
 net/mac80211/tx.c | 19 ++++++++++++++-----
 1 file changed, 14 insertions(+), 5 deletions(-)

diff --git a/net/mac80211/tx.c b/net/mac80211/tx.c
index 3d6c5b8ec0c2..8aeb60182f10 100644
--- a/net/mac80211/tx.c
+++ b/net/mac80211/tx.c
@@ -1394,11 +1394,14 @@ static void ieee80211_txq_enqueue(struct ieee80211_local *local,
 	 * want to simplify the driver API by having them on the
 	 * txqi.
 	 */
-	if (unlikely(txqi->txq.tid == IEEE80211_NUM_TIDS))
+	if (unlikely(txqi->txq.tid == IEEE80211_NUM_TIDS)) {
+		IEEE80211_SKB_CB(skb)->control.flags |=
+			IEEE80211_TX_INTCFL_NEED_TXPROCESSING;
 		__skb_queue_tail(&txqi->frags, skb);
-	else
+	} else {
 		fq_tin_enqueue(fq, tin, flow_idx, skb,
 			       fq_skb_free_func);
+	}
 	spin_unlock_bh(&fq->lock);
 }
 
@@ -3587,10 +3590,16 @@ struct sk_buff *ieee80211_tx_dequeue(struct ieee80211_hw *hw,
 
 	/* Make sure fragments stay together. */
 	skb = __skb_dequeue(&txqi->frags);
-	if (skb)
-		goto out;
+	if (unlikely(skb)) {
+		if (!(IEEE80211_SKB_CB(skb)->control.flags &
+				IEEE80211_TX_INTCFL_NEED_TXPROCESSING))
+			goto out;
+		IEEE80211_SKB_CB(skb)->control.flags &=
+			~IEEE80211_TX_INTCFL_NEED_TXPROCESSING;
+	} else {
+		skb = fq_tin_dequeue(fq, tin, fq_tin_dequeue_func);
+	}
 
-	skb = fq_tin_dequeue(fq, tin, fq_tin_dequeue_func);
 	if (!skb)
 		goto out;
 
-- 
2.30.2