From: Richard Acayan <mailingradian@gmail.com>

The WCN3990 might split MSDUs among multiple "in-order" indications. The
driver needs information from previous indications to handle MPDUs that
are not started by the same indications that complete them. Move the
list that tracks unprocessed MSDUs to the driver state so the driver can
handle MPDUs that are split in this way and be less confused.

Fixes: c545070e404b ("ath10k: implement rx reorder support")
Signed-off-by: Richard Acayan <mailingradian@gmail.com>
Co-developed-by: David Heidelberg <david@ixit.cz>
Signed-off-by: David Heidelberg <david@ixit.cz>
---
This one we have in-tree forever.

Changes in v2:
- checkpatch & style. (Jeff)
- Improve comments.
- Link to v1: https://lore.kernel.org/linux-wireless/20260210021249.12132-2-mailingradian@gmail.com/
---
 drivers/net/wireless/ath/ath10k/htt.h    |  4 +++
 drivers/net/wireless/ath/ath10k/htt_rx.c | 50 +++++++++++++++++++++++++-------
 2 files changed, 43 insertions(+), 11 deletions(-)

diff --git a/drivers/net/wireless/ath/ath10k/htt.h b/drivers/net/wireless/ath/ath10k/htt.h
index 25c6b2e2f81c8..07c3dc088f07b 100644
--- a/drivers/net/wireless/ath/ath10k/htt.h
+++ b/drivers/net/wireless/ath/ath10k/htt.h
@@ -1924,16 +1924,20 @@ struct ath10k_htt {
 
 	bool tx_mem_allocated;
 	const struct ath10k_htt_tx_ops *tx_ops;
 	const struct ath10k_htt_rx_ops *rx_ops;
 	bool disable_tx_comp;
 	bool bundle_tx;
 	struct sk_buff_head tx_req_head;
 	struct sk_buff_head tx_complete_head;
+
+	u8 rx_in_ord_split_tid;
+	u16 rx_in_ord_split_peer_id;
+	struct sk_buff_head rx_in_ord_split;
 };
 
 struct ath10k_htt_tx_ops {
 	int (*htt_send_rx_ring_cfg)(struct ath10k_htt *htt);
 	int (*htt_send_frag_desc_bank_cfg)(struct ath10k_htt *htt);
 	int (*htt_alloc_frag_desc)(struct ath10k_htt *htt);
 	void (*htt_free_frag_desc)(struct ath10k_htt *htt);
 	int (*htt_tx)(struct ath10k_htt *htt, enum ath10k_hw_txrx_mode txmode,
diff --git a/drivers/net/wireless/ath/ath10k/htt_rx.c b/drivers/net/wireless/ath/ath10k/htt_rx.c
index ab2d373b4750d..732bd3a2f9992 100644
--- a/drivers/net/wireless/ath/ath10k/htt_rx.c
+++ b/drivers/net/wireless/ath/ath10k/htt_rx.c
@@ -291,16 +291,18 @@ void ath10k_htt_rx_free(struct ath10k_htt *htt)
 		return;
 
 	timer_delete_sync(&htt->rx_ring.refill_retry_timer);
 
 	skb_queue_purge(&htt->rx_msdus_q);
 	skb_queue_purge(&htt->rx_in_ord_compl_q);
 	skb_queue_purge(&htt->tx_fetch_ind_q);
 
+	skb_queue_purge(&htt->rx_in_ord_split);
+
 	spin_lock_bh(&htt->rx_ring.lock);
 	ath10k_htt_rx_ring_free(htt);
 	spin_unlock_bh(&htt->rx_ring.lock);
 
 	dma_free_coherent(htt->ar->dev,
 			  ath10k_htt_get_rx_ring_size(htt),
 			  ath10k_htt_get_vaddr_ring(htt),
 			  htt->rx_ring.base_paddr);
@@ -841,16 +843,18 @@ int ath10k_htt_rx_alloc(struct ath10k_htt *htt)
 	htt->rx_ring.sw_rd_idx.msdu_payld = 0;
 	hash_init(htt->rx_ring.skb_table);
 
 	skb_queue_head_init(&htt->rx_msdus_q);
 	skb_queue_head_init(&htt->rx_in_ord_compl_q);
 	skb_queue_head_init(&htt->tx_fetch_ind_q);
 	atomic_set(&htt->num_mpdus_ready, 0);
 
+	skb_queue_head_init(&htt->rx_in_ord_split);
+
 	ath10k_dbg(ar, ATH10K_DBG_BOOT, "htt rx ring size %d fill_level %d\n",
 		   htt->rx_ring.size, htt->rx_ring.fill_level);
 	return 0;
 
 err_dma_idx:
 	dma_free_coherent(htt->ar->dev,
 			  ath10k_htt_get_rx_ring_size(htt),
 			  vaddr_ring,
@@ -3156,16 +3160,20 @@ static int ath10k_htt_rx_extract_amsdu(struct ath10k_hw_params *hw,
 	struct rx_msdu_end_common *rxd_msdu_end_common;
 
 	if (skb_queue_empty(list))
 		return -ENOBUFS;
 
 	if (WARN_ON(!skb_queue_empty(amsdu)))
 		return -EINVAL;
 
+	msdu = skb_peek(list);
+	rxd = HTT_RX_BUF_TO_RX_DESC(hw,
+				    (void *)msdu->data - hw->rx_desc_ops->rx_desc_size);
+
 	while ((msdu = __skb_dequeue(list))) {
 		__skb_queue_tail(amsdu, msdu);
 
 		rxd = HTT_RX_BUF_TO_RX_DESC(hw,
 					    (void *)msdu->data -
 					    hw->rx_desc_ops->rx_desc_size);
 
 		rxd_msdu_end_common = ath10k_htt_rx_desc_get_msdu_end(hw, rxd);
@@ -3257,17 +3265,16 @@ static void ath10k_htt_rx_h_rx_offload(struct ath10k *ar,
 	}
 }
 
 static int ath10k_htt_rx_in_ord_ind(struct ath10k *ar, struct sk_buff *skb)
 {
 	struct ath10k_htt *htt = &ar->htt;
 	struct htt_resp *resp = (void *)skb->data;
 	struct ieee80211_rx_status *status = &htt->rx_status;
-	struct sk_buff_head list;
 	struct sk_buff_head amsdu;
 	u16 peer_id;
 	u16 msdu_count;
 	u8 vdev_id;
 	u8 tid;
 	bool offload;
 	bool frag;
 	int ret;
@@ -3292,64 +3299,85 @@ static int ath10k_htt_rx_in_ord_ind(struct ath10k *ar, struct sk_buff *skb)
 		   "htt rx in ord vdev %i peer %i tid %i offload %i frag %i msdu count %i\n",
 		   vdev_id, peer_id, tid, offload, frag, msdu_count);
 
 	if (skb->len < msdu_count * sizeof(*resp->rx_in_ord_ind.msdu_descs32)) {
 		ath10k_warn(ar, "dropping invalid in order rx indication\n");
 		return -EINVAL;
 	}
 
-	/* The event can deliver more than 1 A-MSDU. Each A-MSDU is later
-	 * extracted and processed.
+	if (!skb_queue_empty(&htt->rx_in_ord_split)) {
+		/*
+		 * It might still be possible to handle this case if there is
+		 * only one peer that splits at each given moment. We are
+		 * bailing out because we should have a test case for this
+		 * before trying to fix it.
+		 */
+		if (tid != htt->rx_in_ord_split_tid ||
+		    peer_id != htt->rx_in_ord_split_peer_id ||
+		    offload) {
+			ath10k_warn(ar, "split amsdu did not resume immediately\n");
+			htt->rx_confused = true;
+			ath10k_core_start_recovery(ar);
+			return -EIO;
+		}
+	}
+
+	/*
+	 * The event can deliver more than 1 A-MSDU or continue a previous one.
+	 * Each A-MSDU is later extracted and processed.
 	 */
-	__skb_queue_head_init(&list);
 	if (ar->hw_params.target_64bit)
 		ret = ath10k_htt_rx_pop_paddr64_list(htt, &resp->rx_in_ord_ind,
-						     &list);
+						     &htt->rx_in_ord_split);
 	else
 		ret = ath10k_htt_rx_pop_paddr32_list(htt, &resp->rx_in_ord_ind,
-						     &list);
+						     &htt->rx_in_ord_split);
 
 	if (ret < 0) {
 		ath10k_warn(ar, "failed to pop paddr list: %d\n", ret);
 		htt->rx_confused = true;
 		ath10k_core_start_recovery(ar);
 		return -EIO;
 	}
 
 	/* Offloaded frames are very different and need to be handled
 	 * separately.
 	 */
 	if (offload)
-		ath10k_htt_rx_h_rx_offload(ar, &list);
+		ath10k_htt_rx_h_rx_offload(ar, &htt->rx_in_ord_split);
 
-	while (!skb_queue_empty(&list)) {
+	while (!skb_queue_empty(&htt->rx_in_ord_split)) {
 		__skb_queue_head_init(&amsdu);
-		ret = ath10k_htt_rx_extract_amsdu(&ar->hw_params, &list, &amsdu);
+		ret = ath10k_htt_rx_extract_amsdu(&ar->hw_params,
+						  &htt->rx_in_ord_split, &amsdu);
 		switch (ret) {
 		case 0:
 			/* Note: The in-order indication may report interleaved
 			 * frames from different PPDUs meaning reported rx rate
 			 * to mac80211 isn't accurate/reliable. It's still
 			 * better to report something than nothing though. This
 			 * should still give an idea about rx rate to the user.
 			 */
 			ath10k_htt_rx_h_ppdu(ar, &amsdu, status, vdev_id);
 			ath10k_htt_rx_h_filter(ar, &amsdu, status, NULL);
 			ath10k_htt_rx_h_mpdu(ar, &amsdu, status, false, NULL,
 					     NULL, peer_id, frag);
 			ath10k_htt_rx_h_enqueue(ar, &amsdu, status);
 			break;
 		case -EAGAIN:
-			fallthrough;
+			htt->rx_in_ord_split_tid = tid;
+			htt->rx_in_ord_split_peer_id = peer_id;
+
+			return -EIO;
 		default:
 			/* Should not happen. */
 			ath10k_warn(ar, "failed to extract amsdu: %d\n", ret);
 			htt->rx_confused = true;
-			__skb_queue_purge(&list);
+			__skb_queue_purge(&htt->rx_in_ord_split);
 			ath10k_core_start_recovery(ar);
 			return -EIO;
 		}
 	}
 	return ret;
 }
 
 static void ath10k_htt_rx_tx_fetch_resp_id_confirm(struct ath10k *ar,

---
base-commit: 0718283ab28bc3907e10b61a6b4be6fefa1cbb2f
change-id: 20260719-ath10k-a-msdu-c49334eb091d

Best regards,
-- 
David Heidelberg <david@ixit.cz>

Sharing Sashiko review, at the first part of it was something I wondered before 
resending the v2.

I'm not sure what exactly is achieved with the msdu & rxd assignment, as it 
seems to be really immediately discarded.

Please excuse my lack of knowledge in the wireless field, it's just patch which 
we carry for long time and it seems helps others projects using WCN3990 too

 > - 0016: WCN3990 splits A-MSDUs across multiple HTT in-order
 >   indications; mainline dropped them ("failed to extract amsdu: -11",
 >   rx_confused), collapsing TCP under load (49 kB/s). Persist the
 >   in-order MSDU list in driver state.
https://github.com/commaai/vamOS/commit/12d80d09e61188895e90d9e4245d513ad4d07f25

Thanks
David

--- Patch [1]: [PATCH RFC v2] wifi: ath10k: make in-order rx amsdu buffers 
persistent ---
commit 10e41b4de22d818aaaa807183134d42a4da09825
Author: Richard Acayan <mailingradian@gmail.com>

wifi: ath10k: make in-order rx amsdu buffers persistent

This commit moves the list tracking unprocessed MSDUs to the driver state,
allowing it to handle MPDUs that are split among multiple in-order
indications.

Link: 
https://lore.kernel.org/linux-wireless/20260210021249.12132-2-mailingradian@gmail.com/

 > diff --git a/drivers/net/wireless/ath/ath10k/htt_rx.c 
b/drivers/net/wireless/ath/ath10k/htt_rx.c
 > index ab2d373b4750d..732bd3a2f9992 100644
 > --- a/drivers/net/wireless/ath/ath10k/htt_rx.c
 > +++ b/drivers/net/wireless/ath/ath10k/htt_rx.c

[ ... ]

 > @@ -3161,6 +3165,10 @@ static int ath10k_htt_rx_extract_amsdu(struct 
ath10k_hw_params *hw,
 >  	if (WARN_ON(!skb_queue_empty(amsdu)))
 >  		return -EINVAL;
 >
 > +	msdu = skb_peek(list);
 > +	rxd = HTT_RX_BUF_TO_RX_DESC(hw,
 > +				    (void *)msdu->data - hw->rx_desc_ops->rx_desc_size);
 > +

This isn't a bug, but are these assignments to msdu and rxd unused?

They appear to be unconditionally overwritten in the while loop condition and
the first statement of the loop body without ever being read.

 >  	while ((msdu = __skb_dequeue(list))) {
 >  		__skb_queue_tail(amsdu, msdu);

[ ... ]

 > @@ -3339,12 +3364,15 @@ static int ath10k_htt_rx_in_ord_ind(struct ath10k 
*ar, struct sk_buff *skb)
 >  			ath10k_htt_rx_h_enqueue(ar, &amsdu, status);
 >  			break;
 >  		case -EAGAIN:
 > -			fallthrough;
 > +			htt->rx_in_ord_split_tid = tid;
 > +			htt->rx_in_ord_split_peer_id = peer_id;
 > +
 > +			return -EIO;

Will returning -EIO here prematurely abort the NAPI polling loop?

When a split AMSDU is received from the hardware and
ath10k_htt_rx_extract_amsdu() returns -EAGAIN, this -EIO return value is
propagated up to ath10k_htt_txrx_compl_task(), which interprets it as a
signal to abort the rx_in_ord_compl_q loop and reschedule NAPI.

This seems to leave any subsequent queued packets, potentially including the
remainder of the split AMSDU, unprocessed until the next NAPI poll.

 >  		default:
 >  			/* Should not happen. */
 >  			ath10k_warn(ar, "failed to extract amsdu: %d\n", ret);