It was noticed that APs stopped to accept clients after a while. With QCA's ath11k fork, it even printed some additional information:
attach ack fail -28
when new clients tried to connect. hostapd was then usually showing a message like "deauthenticated due to inactivity (timer DEAUTH/REMOVE)".
While debugging this, it was noticed that this happened when a peer was no longer known by ath11k but an NL80211_CMD_PROBE_CLIENT triggered TX was just "finished" for it. In that case, ath11k was just throwing the skb away and left some information in various data structures in mac80211.
And after Felix pointed out ieee80211_free_txskb(), it is also clear that dev_kfree_skb_any() in these functions should also be calls to ieee80211_free_txskb() - but for these, I have nothing to trigger this error case. Still, a patch is provided as part of this patch series.
Signed-off-by: Sven Eckelmann sven@narfation.org --- Changes in v2: - Simply switch to ieee80211_free_txskb() as recommended by Felix Fietkau
+ ieee80211_free_txskb calls ieee80211_report_used_skb + ieee80211_report_used_skb calls ieee80211_report_ack_skb (when ack_frame_id is set and !IEEE80211_TX_INTFL_MLME_CONN_TX) + ieee80211_report_ack_skb will remove skb from ack_status_frames
- Add second patch which handles similar situations in the previously patched functions - Link to v1: https://lore.kernel.org/r/20230801-ath11k-ack_status_leak-v1-1-539cb72c55bc@...
--- Sven Eckelmann (2): ath11k: Don't drop tx_status when peer cannot be found ath11k: Cleanup mac80211 references on failure during tx_complete
drivers/net/wireless/ath/ath11k/dp_tx.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) --- base-commit: 1d7dd5aa35474e553b8671b58579e0749b560779 change-id: 20230801-ath11k-ack_status_leak-70a7a30e5d9f
Best regards,
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. 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.
Tested-on: IPQ6018 hw1.0 WLAN.HK.2.5.0.1-01100-QCAHKSWPL_SILICONZ-1
Fixes: 6257c702264c ("wifi: ath11k: fix tx status reporting in encap offload mode") Fixes: 94739d45c388 ("ath11k: switch to using ieee80211_tx_status_ext()") Cc: stable@vger.kernel.org Signed-off-by: Sven Eckelmann sven@narfation.org --- drivers/net/wireless/ath/ath11k/dp_tx.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/net/wireless/ath/ath11k/dp_tx.c b/drivers/net/wireless/ath/ath11k/dp_tx.c index a34833de7c67..27c976f52c7a 100644 --- a/drivers/net/wireless/ath/ath11k/dp_tx.c +++ b/drivers/net/wireless/ath/ath11k/dp_tx.c @@ -369,7 +369,7 @@ ath11k_dp_tx_htt_tx_complete_buf(struct ath11k_base *ab, "dp_tx: failed to find the peer with peer_id %d\n", ts->peer_id); spin_unlock_bh(&ab->base_lock); - dev_kfree_skb_any(msdu); + ieee80211_free_txskb(ar->hw, msdu); return; } spin_unlock_bh(&ab->base_lock); @@ -624,7 +624,7 @@ static void ath11k_dp_tx_complete_msdu(struct ath11k *ar, "dp_tx: failed to find the peer with peer_id %d\n", ts->peer_id); spin_unlock_bh(&ab->base_lock); - dev_kfree_skb_any(msdu); + ieee80211_free_txskb(ar->hw, msdu); return; } arsta = (struct ath11k_sta *)peer->sta->drv_priv;
Sven Eckelmann sven@narfation.org 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. 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.
Tested-on: IPQ6018 hw1.0 WLAN.HK.2.5.0.1-01100-QCAHKSWPL_SILICONZ-1
Fixes: 6257c702264c ("wifi: ath11k: fix tx status reporting in encap offload mode") Fixes: 94739d45c388 ("ath11k: switch to using ieee80211_tx_status_ext()") Cc: stable@vger.kernel.org Signed-off-by: Sven Eckelmann sven@narfation.org Signed-off-by: Kalle Valo quic_kvalo@quicinc.com
2 patches applied to ath-next branch of ath.git, thanks.
400ece6c7f34 wifi: ath11k: Don't drop tx_status when peer cannot be found 29d15589f084 wifi: ath11k: Cleanup mac80211 references on failure during tx_complete
When a function is using functions from mac80211 to free an skb then it should do it consistently and not switch to the generic dev_kfree_skb_any (or similar functions). Otherwise (like in the error handlers), mac80211 will will not be aware of the freed skb and thus not clean up related information in its internal data structures.
Not doing so lead in the past to filled up structure which then prevented new clients to connect.
Fixes: d5c65159f289 ("ath11k: driver for Qualcomm IEEE 802.11ax devices") Fixes: 6257c702264c ("wifi: ath11k: fix tx status reporting in encap offload mode") Cc: stable@vger.kernel.org Signed-off-by: Sven Eckelmann sven@narfation.org --- drivers/net/wireless/ath/ath11k/dp_tx.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/net/wireless/ath/ath11k/dp_tx.c b/drivers/net/wireless/ath/ath11k/dp_tx.c index 27c976f52c7a..b85a4a03b37a 100644 --- a/drivers/net/wireless/ath/ath11k/dp_tx.c +++ b/drivers/net/wireless/ath/ath11k/dp_tx.c @@ -344,7 +344,7 @@ ath11k_dp_tx_htt_tx_complete_buf(struct ath11k_base *ab, dma_unmap_single(ab->dev, skb_cb->paddr, msdu->len, DMA_TO_DEVICE);
if (!skb_cb->vif) { - dev_kfree_skb_any(msdu); + ieee80211_free_txskb(ar->hw, msdu); return; }
@@ -566,12 +566,12 @@ static void ath11k_dp_tx_complete_msdu(struct ath11k *ar, dma_unmap_single(ab->dev, skb_cb->paddr, msdu->len, DMA_TO_DEVICE);
if (unlikely(!rcu_access_pointer(ab->pdevs_active[ar->pdev_idx]))) { - dev_kfree_skb_any(msdu); + ieee80211_free_txskb(ar->hw, msdu); return; }
if (unlikely(!skb_cb->vif)) { - dev_kfree_skb_any(msdu); + ieee80211_free_txskb(ar->hw, msdu); return; }
linux-stable-mirror@lists.linaro.org