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 <[email protected]>
---
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/[email protected]
---
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,
--
Sven Eckelmann <[email protected]>
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: [email protected]
Signed-off-by: Sven Eckelmann <[email protected]>
---
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;
}
--
2.39.2
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: [email protected]
Signed-off-by: Sven Eckelmann <[email protected]>
---
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;
--
2.39.2
Sven Eckelmann <[email protected]> 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: [email protected]
> Signed-off-by: Sven Eckelmann <[email protected]>
> Signed-off-by: Kalle Valo <[email protected]>
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
--
https://patchwork.kernel.org/project/linux-wireless/patch/[email protected]/
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches