[PATCH RFC v2] wifi: ath10k: make in-order rx amsdu buffers persistent

[PATCH RFC v2] wifi: ath10k: make in-order rx amsdu buffers persistent

[David Heidelberg via B4 Relay]

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>

Re: [PATCH RFC v2] wifi: ath10k: make in-order rx amsdu buffers persistent

[David Heidelberg]

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);

Re: [PATCH RFC v2] wifi: ath10k: make in-order rx amsdu buffers persistent

[Jeff Johnson]

On 7/19/2026 2:45 PM, David Heidelberg via B4 Relay wrote:
> 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.

I'm transcribing a few comments from my review agent (which may overlap
Sashiko). I have not vetted them for correctness. Hopefully I placed them at
the correct spots!

> 
> 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);

Dead rxd computation before the loop — VALID, MINOR

Lines 3168–3170 compute rxd via skb_peek(), but the while loop at 3172
immediately dequeues the same SKB and unconditionally recomputes rxd at lines
3175–3177. The pre-loop assignment is never read. It's dead code. The original
patch must have introduced this when restructuring (the pre-existing code
likely used rxd from before the loop). It should be removed.

> +
>  	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) {

Dead offload disjunct — VALID, MINOR

ath10k_htt_rx_h_rx_offload() at line 3346 drains rx_in_ord_split entirely (via
__skb_dequeue loop). After it runs, the while (!skb_queue_empty(...)) loop at
3348 never executes. Therefore a split (-EAGAIN from
ath10k_htt_rx_extract_amsdu) can never occur for offload frames. The ||
offload condition at line 3316 in the split-validation guard is unreachable.
It should be removed for clarity.


> +			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;

rx_in_ord_split not purged on pop_paddr error — VALID BUG

Lines 3335–3339: on pop_paddr error, htt->rx_confused = true and
ath10k_core_start_recovery() are called, but
__skb_queue_purge(&htt->rx_in_ord_split) is missing. If a split is already in
progress (queue is non-empty) when pop_paddr fails on the continuation event,
the existing SKBs are stranded. The default: path at lines 3371–3377 does
purge the queue, making this an inconsistency. This will leak DMA-mapped SKBs.

>  	}
>  
>  	/* 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;

frag flag not saved across -EAGAIN — VALID BUG

frag is a local extracted from the current event's header (line 3296). On
-EAGAIN (line 3366–3370), only tid and peer_id are saved. When the
continuation event arrives, ath10k_htt_rx_h_mpdu() at line 3362 is called with
the frag from the new event, not the original split event. If the original
said frag=1, the fragments will be misprocessed by mac80211 on continuation.
This needs rx_in_ord_split_frag added alongside _tid and _peer_id.

>  		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,

Re: [PATCH RFC v2] wifi: ath10k: make in-order rx amsdu buffers persistent

[Johannes Berg]

On Thu, 2026-07-30 at 19:24 -0700, Jeff Johnson wrote:
> On 7/19/2026 2:45 PM, David Heidelberg via B4 Relay wrote:
> > 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.

> I'm transcribing a few comments from my review agent (which may overlap
> Sashiko). I have not vetted them for correctness. Hopefully I placed them at
> the correct spots!

I think this is one of those cases where just doing LLMs isn't all that
helpful?

I'm not at all familiar with this, but why does this really need all the
complexity of hanging on to the entire MPDU etc. when "[the] driver
needs information"? Couldn't it just hang on to the relevant information
and reduce the complexity here?

Also, the entire point of this is for the loop, so when the LLM says:

> > +	msdu = skb_peek(list);
> > +	rxd = HTT_RX_BUF_TO_RX_DESC(hw,
> > +				    (void *)msdu->data - hw->rx_desc_ops->rx_desc_size);
> 
> Dead rxd computation before the loop — VALID, MINOR

I feel like it's probably missing the point entirely - the in-loop
version should be removed?

johannes

Re: [PATCH RFC v2] wifi: ath10k: make in-order rx amsdu buffers persistent

[Richard Acayan]

On Thu, Jul 30, 2026 at 07:24:00PM -0700, Jeff Johnson wrote:
> On 7/19/2026 2:45 PM, David Heidelberg via B4 Relay wrote:
> > 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.
> 
> I'm transcribing a few comments from my review agent (which may overlap
> Sashiko). I have not vetted them for correctness. Hopefully I placed them at
> the correct spots!
> 
> > 
> > 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);
> 
> Dead rxd computation before the loop — VALID, MINOR
> 
> Lines 3168–3170 compute rxd via skb_peek(), but the while loop at 3172
> immediately dequeues the same SKB and unconditionally recomputes rxd at lines
> 3175–3177. The pre-loop assignment is never read. It's dead code. The original
> patch must have introduced this when restructuring (the pre-existing code
> likely used rxd from before the loop). It should be removed.

This was removed on 20 January 2026 before v1 was submitted. It seems
this v2 is based on applying the commits to the pre-2026 patch.

Re: [PATCH RFC v2] wifi: ath10k: make in-order rx amsdu buffers persistent

[David Heidelberg]

On 31/07/2026 16:39, Richard Acayan wrote:
[...]

>>>   
>>> +	msdu = skb_peek(list);
>>> +	rxd = HTT_RX_BUF_TO_RX_DESC(hw,
>>> +				    (void *)msdu->data - hw->rx_desc_ops->rx_desc_size);
>>
>> Dead rxd computation before the loop — VALID, MINOR
>>
>> Lines 3168–3170 compute rxd via skb_peek(), but the while loop at 3172
>> immediately dequeues the same SKB and unconditionally recomputes rxd at lines
>> 3175–3177. The pre-loop assignment is never read. It's dead code. The original
>> patch must have introduced this when restructuring (the pre-existing code
>> likely used rxd from before the loop). It should be removed.
> 
> This was removed on 20 January 2026 before v1 was submitted. It seems
> this v2 is based on applying the commits to the pre-2026 patch.

I checked against what Comma.AI uses, and it's also gone there, so we've been 
using pre-v1 in sdm845-next.

Dropping it definitely make sense, and since Comma.AI already tested, I assume 
no regression.

Will be addressed in v3.

David