| Message ID | 20240430074313.885807-2-quic_kathirve@quicinc.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 5453bbd6fef4ca2fea3b4b338fc715d7135afc6f |
| Delegated to: | Kalle Valo |
| Headers | show |
| Series | wifi: ath12k: drop failed transmitted frames from metric calculation | expand |
On 4/30/2024 12:43 AM, Karthikeyan Kathirvel wrote: > From: Sven Eckelmann <sven@narfation.org> > > When a station idles for a long time, hostapd will try to send > a QoS Null frame to the station as "poll". NL80211_CMD_PROBE_CLIENT > is used for this purpose. > And the skb will be added to ack_status_frame - waiting for a > completion via ieee80211_report_ack_skb(). > > But when the peer was already removed before the tx_complete arrives, > the peer will be missing. And when using dev_kfree_skb_any (instead > of going through mac80211), the entry will stay inside > ack_status_frames thus not clean up related information in its > internal data structures. This IDR will therefore run full after > 8K request were generated for such clients. > At this point, the access point will then just stall and not allow > any new clients because idr_alloc() for ack_status_frame will fail. > > ieee80211_free_txskb() on the other hand will (when required) call > ieee80211_report_ack_skb() and make sure that (when required) remove > the entry from the ack_status_frame and clean up related > information in its internal data structures. > > Tested-on: QCN9274 hw2.0 PCI WLAN.WBE.1.0.1-00029-QCAHKSWPL_SILICONZ-1 > > Fixes: d889913205cf ("wifi: ath12k: driver for Qualcomm Wi-Fi 7 devices") > Signed-off-by: Sven Eckelmann <sven@narfation.org> > Signed-off-by: Sarika Sharma <quic_sarishar@quicinc.com> > Signed-off-by: Karthikeyan Kathirvel <quic_kathirve@quicinc.com> > Link: https://lore.kernel.org/r/20230802-ath11k-ack_status_leak-v2-1-c0af729d6229@narfation.org Acked-by: Jeff Johnson <quic_jjohnson@quicinc.com>
Karthikeyan Kathirvel <quic_kathirve@quicinc.com> wrote: > When a station idles for a long time, hostapd will try to send > a QoS Null frame to the station as "poll". NL80211_CMD_PROBE_CLIENT > is used for this purpose. > And the skb will be added to ack_status_frame - waiting for a > completion via ieee80211_report_ack_skb(). > > But when the peer was already removed before the tx_complete arrives, > the peer will be missing. And when using dev_kfree_skb_any (instead > of going through mac80211), the entry will stay inside > ack_status_frames thus not clean up related information in its > internal data structures. This IDR will therefore run full after > 8K request were generated for such clients. > At this point, the access point will then just stall and not allow > any new clients because idr_alloc() for ack_status_frame will fail. > > ieee80211_free_txskb() on the other hand will (when required) call > ieee80211_report_ack_skb() and make sure that (when required) remove > the entry from the ack_status_frame and clean up related > information in its internal data structures. > > Tested-on: QCN9274 hw2.0 PCI WLAN.WBE.1.0.1-00029-QCAHKSWPL_SILICONZ-1 > > Fixes: d889913205cf ("wifi: ath12k: driver for Qualcomm Wi-Fi 7 devices") > Signed-off-by: Sven Eckelmann <sven@narfation.org> > Signed-off-by: Sarika Sharma <quic_sarishar@quicinc.com> > Signed-off-by: Karthikeyan Kathirvel <quic_kathirve@quicinc.com> > Link: https://lore.kernel.org/r/20230802-ath11k-ack_status_leak-v2-1-c0af729d6229@narfation.org > Acked-by: Jeff Johnson <quic_jjohnson@quicinc.com> > Signed-off-by: Kalle Valo <quic_kvalo@quicinc.com> 2 patches applied to ath-next branch of ath.git, thanks. 5453bbd6fef4 wifi: ath12k: Don't drop tx_status in failure case 50971dc6694c wifi: ath12k: drop failed transmitted frames from metric calculation.
diff --git a/drivers/net/wireless/ath/ath12k/dp_tx.c b/drivers/net/wireless/ath/ath12k/dp_tx.c index 9b6d7d72f57c..81a85d5946f5 100644 --- a/drivers/net/wireless/ath/ath12k/dp_tx.c +++ b/drivers/net/wireless/ath/ath12k/dp_tx.c @@ -352,15 +352,15 @@ static void ath12k_dp_tx_free_txbuf(struct ath12k_base *ab, u8 pdev_id = ath12k_hw_mac_id_to_pdev_id(ab->hw_params, mac_id); skb_cb = ATH12K_SKB_CB(msdu); + ar = ab->pdevs[pdev_id].ar; dma_unmap_single(ab->dev, skb_cb->paddr, msdu->len, DMA_TO_DEVICE); if (skb_cb->paddr_ext_desc) dma_unmap_single(ab->dev, skb_cb->paddr_ext_desc, sizeof(struct hal_tx_msdu_ext_desc), DMA_TO_DEVICE); - dev_kfree_skb_any(msdu); + ieee80211_free_txskb(ar->ah->hw, msdu); - ar = ab->pdevs[pdev_id].ar; if (atomic_dec_and_test(&ar->dp.num_tx_pending)) wake_up(&ar->dp.tx_empty_waitq); } @@ -448,6 +448,7 @@ static void ath12k_dp_tx_complete_msdu(struct ath12k *ar, struct hal_tx_status *ts) { struct ath12k_base *ab = ar->ab; + struct ath12k_hw *ah = ar->ah; struct ieee80211_tx_info *info; struct ath12k_skb_cb *skb_cb; @@ -466,12 +467,12 @@ static void ath12k_dp_tx_complete_msdu(struct ath12k *ar, rcu_read_lock(); if (!rcu_dereference(ab->pdevs_active[ar->pdev_idx])) { - dev_kfree_skb_any(msdu); + ieee80211_free_txskb(ah->hw, msdu); goto exit; } if (!skb_cb->vif) { - dev_kfree_skb_any(msdu); + ieee80211_free_txskb(ah->hw, msdu); goto exit; }