| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <c5ec36c2692184a154ad07000c15492c4f37f141.camel@fh-aachen.de>
Date: Fri, 25 Jul 2025 21:57:59 +0000
From: "Maurer, Florian" <maurer@...aachen.de>
To: "linux-wireless@...r.kernel.org" <linux-wireless@...r.kernel.org>,
"repk@...plefau.lt" <repk@...plefau.lt>, "ath10k@...ts.infradead.org"
<ath10k@...ts.infradead.org>, "linux-kernel@...r.kernel.org"
<linux-kernel@...r.kernel.org>
CC: "kvalo@...nel.org" <kvalo@...nel.org>, "veilleux.cedric@...il.com"
<veilleux.cedric@...il.com>, "jjohnson@...nel.org" <jjohnson@...nel.org>,
"quic_vthiagar@...cinc.com" <quic_vthiagar@...cinc.com>
Subject: Re: [RESEND PATCH v3 2/2] wifi: ath10k: Flush only requested txq in
ath10k_flush()
Hi everyone,
we tested this Patch in Freifunk Aachen using Gluon/OpenWrt.
We had the issue that the Extreme Networks WS-AP3825i did have a high
load in a busy environment, making the node unresponsive.
This issue is resolved with this patch, improving the wifi stability
noticeably.
Thanks,
Florian Maurer
Tested-by: Florian Maurer <maurer@...aachen.de>
On Fri, 2024-11-22 at 17:48 +0100, Remi Pommarel wrote:
> The ieee80211 flush callback can be called to flush only part of all hw
> queues. The ath10k's flush callback implementation (i.e. ath10k_flush())
> was waiting for all pending frames of all queues to be flushed ignoring
> the queue parameter. Because only the queues to be flushed are stopped
> by mac80211, skb can still be queued to other queues meanwhile. Thus
> ath10k_flush() could fail (and wait 5sec holding ar->conf lock) even if
> the requested queues are flushed correctly.
>
> A way to reproduce the issue is to use two different APs because
> each vdev has its own hw queue in ath10k. Connect STA0 to AP0 and STA1
> to AP1. Then generate traffic from AP0 to STA0 and kill STA0 without
> clean disassociation frame (e.g. unplug power cable, reboot -f, ...).
> Now if we were to flush AP1's queue, ath10k_flush() would fail (and
> effectively block 5 seconds with ar->conf or even wiphy's lock held)
> with the following warning:
>
> ath10k_pci 0000:01:00.0: failed to flush transmit queue (skip 0 ar-state 2): 0
>
> Wait only for pending frames of the requested queues to be flushed in
> ath10k_flush() to avoid that long blocking.
>
> Reported-by: Cedric Veilleux <veilleux.cedric@...il.com>
> Closes: https://lore.kernel.org/all/CA+Xfe4FjUmzM5mvPxGbpJsF3SvSdE5_wgxvgFJ0bsdrKODVXCQ@mail.gmail.com/
> Signed-off-by: Remi Pommarel <repk@...plefau.lt>
> ---
> drivers/net/wireless/ath/ath10k/htt.h | 7 +++--
> drivers/net/wireless/ath/ath10k/htt_tx.c | 18 ++++++++++--
> drivers/net/wireless/ath/ath10k/mac.c | 35 ++++++++++++++++--------
> drivers/net/wireless/ath/ath10k/txrx.c | 2 +-
> 4 files changed, 45 insertions(+), 17 deletions(-)
>
> diff --git a/drivers/net/wireless/ath/ath10k/htt.h b/drivers/net/wireless/ath/ath10k/htt.h
> index d150f9330941..ca8bf3b6766d 100644
> --- a/drivers/net/wireless/ath/ath10k/htt.h
> +++ b/drivers/net/wireless/ath/ath10k/htt.h
> @@ -1870,6 +1870,7 @@ struct ath10k_htt {
> spinlock_t tx_lock;
> int max_num_pending_tx;
> int num_pending_tx;
> + int num_pending_per_queue[IEEE80211_MAX_QUEUES];
> int num_pending_mgmt_tx;
> struct idr pending_tx;
> wait_queue_head_t empty_tx_wq;
> @@ -2447,8 +2448,10 @@ void ath10k_htt_tx_txq_update(struct ieee80211_hw *hw,
> void ath10k_htt_tx_txq_recalc(struct ieee80211_hw *hw,
> struct ieee80211_txq *txq);
> void ath10k_htt_tx_txq_sync(struct ath10k *ar);
> -void ath10k_htt_tx_dec_pending(struct ath10k_htt *htt);
> -int ath10k_htt_tx_inc_pending(struct ath10k_htt *htt);
> +void ath10k_htt_tx_dec_pending(struct ath10k_htt *htt,
> + struct ieee80211_txq *txq);
> +int ath10k_htt_tx_inc_pending(struct ath10k_htt *htt,
> + struct ieee80211_txq *txq);
> void ath10k_htt_tx_mgmt_dec_pending(struct ath10k_htt *htt);
> int ath10k_htt_tx_mgmt_inc_pending(struct ath10k_htt *htt, bool is_mgmt,
> bool is_presp);
> diff --git a/drivers/net/wireless/ath/ath10k/htt_tx.c b/drivers/net/wireless/ath/ath10k/htt_tx.c
> index 211752bd0f65..ef5a992e8cce 100644
> --- a/drivers/net/wireless/ath/ath10k/htt_tx.c
> +++ b/drivers/net/wireless/ath/ath10k/htt_tx.c
> @@ -140,19 +140,26 @@ void ath10k_htt_tx_txq_update(struct ieee80211_hw *hw,
> spin_unlock_bh(&ar->htt.tx_lock);
> }
>
> -void ath10k_htt_tx_dec_pending(struct ath10k_htt *htt)
> +void ath10k_htt_tx_dec_pending(struct ath10k_htt *htt,
> + struct ieee80211_txq *txq)
> {
> + int qnr = -1;
> +
> lockdep_assert_held(&htt->tx_lock);
>
> htt->num_pending_tx--;
> if (htt->num_pending_tx == htt->max_num_pending_tx - 1)
> ath10k_mac_tx_unlock(htt->ar, ATH10K_TX_PAUSE_Q_FULL);
>
> - if (htt->num_pending_tx == 0)
> + if (txq)
> + qnr = --htt->num_pending_per_queue[txq->vif->hw_queue[txq->ac]];
> +
> + if (htt->num_pending_tx == 0 || qnr == 0)
> wake_up(&htt->empty_tx_wq);
> }
>
> -int ath10k_htt_tx_inc_pending(struct ath10k_htt *htt)
> +int ath10k_htt_tx_inc_pending(struct ath10k_htt *htt,
> + struct ieee80211_txq *txq)
> {
> lockdep_assert_held(&htt->tx_lock);
>
> @@ -163,6 +170,11 @@ int ath10k_htt_tx_inc_pending(struct ath10k_htt *htt)
> if (htt->num_pending_tx == htt->max_num_pending_tx)
> ath10k_mac_tx_lock(htt->ar, ATH10K_TX_PAUSE_Q_FULL);
>
> + if (!txq)
> + return 0;
> +
> + htt->num_pending_per_queue[txq->vif->hw_queue[txq->ac]]++;
> +
> return 0;
> }
>
> diff --git a/drivers/net/wireless/ath/ath10k/mac.c b/drivers/net/wireless/ath/ath10k/mac.c
> index 8b8d4f24e5fb..7c5f95f2858f 100644
> --- a/drivers/net/wireless/ath/ath10k/mac.c
> +++ b/drivers/net/wireless/ath/ath10k/mac.c
> @@ -4385,7 +4385,7 @@ int ath10k_mac_tx_push_txq(struct ieee80211_hw *hw,
> u16 airtime;
>
> spin_lock_bh(&ar->htt.tx_lock);
> - ret = ath10k_htt_tx_inc_pending(htt);
> + ret = ath10k_htt_tx_inc_pending(htt, txq);
> spin_unlock_bh(&ar->htt.tx_lock);
>
> if (ret)
> @@ -4394,7 +4394,7 @@ int ath10k_mac_tx_push_txq(struct ieee80211_hw *hw,
> skb = ieee80211_tx_dequeue_ni(hw, txq);
> if (!skb) {
> spin_lock_bh(&ar->htt.tx_lock);
> - ath10k_htt_tx_dec_pending(htt);
> + ath10k_htt_tx_dec_pending(htt, txq);
> spin_unlock_bh(&ar->htt.tx_lock);
>
> return -ENOENT;
> @@ -4416,7 +4416,7 @@ int ath10k_mac_tx_push_txq(struct ieee80211_hw *hw,
> ret = ath10k_htt_tx_mgmt_inc_pending(htt, is_mgmt, is_presp);
>
> if (ret) {
> - ath10k_htt_tx_dec_pending(htt);
> + ath10k_htt_tx_dec_pending(htt, txq);
> spin_unlock_bh(&ar->htt.tx_lock);
> return ret;
> }
> @@ -4430,7 +4430,7 @@ int ath10k_mac_tx_push_txq(struct ieee80211_hw *hw,
> ath10k_warn(ar, "failed to push frame: %d\n", ret);
>
> spin_lock_bh(&ar->htt.tx_lock);
> - ath10k_htt_tx_dec_pending(htt);
> + ath10k_htt_tx_dec_pending(htt, txq);
> if (is_mgmt)
> ath10k_htt_tx_mgmt_dec_pending(htt);
> spin_unlock_bh(&ar->htt.tx_lock);
> @@ -4693,7 +4693,7 @@ static void ath10k_mac_op_tx(struct ieee80211_hw *hw,
> is_presp = ieee80211_is_probe_resp(hdr->frame_control);
> }
>
> - ret = ath10k_htt_tx_inc_pending(htt);
> + ret = ath10k_htt_tx_inc_pending(htt, txq);
> if (ret) {
> ath10k_warn(ar, "failed to increase tx pending count: %d, dropping\n",
> ret);
> @@ -4706,7 +4706,7 @@ static void ath10k_mac_op_tx(struct ieee80211_hw *hw,
> if (ret) {
> ath10k_dbg(ar, ATH10K_DBG_MAC, "failed to increase tx mgmt pending count: %d, dropping\n",
> ret);
> - ath10k_htt_tx_dec_pending(htt);
> + ath10k_htt_tx_dec_pending(htt, txq);
> spin_unlock_bh(&ar->htt.tx_lock);
> ieee80211_free_txskb(ar->hw, skb);
> return;
> @@ -4719,7 +4719,7 @@ static void ath10k_mac_op_tx(struct ieee80211_hw *hw,
> ath10k_warn(ar, "failed to transmit frame: %d\n", ret);
> if (is_htt) {
> spin_lock_bh(&ar->htt.tx_lock);
> - ath10k_htt_tx_dec_pending(htt);
> + ath10k_htt_tx_dec_pending(htt, txq);
> if (is_mgmt)
> ath10k_htt_tx_mgmt_dec_pending(htt);
> spin_unlock_bh(&ar->htt.tx_lock);
> @@ -8059,10 +8059,12 @@ static int ath10k_mac_op_set_frag_threshold(struct ieee80211_hw *hw, u32 value)
> return -EOPNOTSUPP;
> }
>
> -void ath10k_mac_wait_tx_complete(struct ath10k *ar)
> +static void _ath10k_mac_wait_tx_complete(struct ath10k *ar,
> + unsigned long queues)
> {
> bool skip;
> long time_left;
> + unsigned int q;
>
> /* mac80211 doesn't care if we really xmit queued frames or not
> * we'll collect those frames either way if we stop/delete vdevs
> @@ -8072,10 +8074,14 @@ void ath10k_mac_wait_tx_complete(struct ath10k *ar)
> return;
>
> time_left = wait_event_timeout(ar->htt.empty_tx_wq, ({
> - bool empty;
> + bool empty = true;
>
> spin_lock_bh(&ar->htt.tx_lock);
> - empty = (ar->htt.num_pending_tx == 0);
> + for_each_set_bit(q, &queues, ar->hw->queues) {
> + empty = (ar->htt.num_pending_per_queue[q] == 0);
> + if (!empty)
> + break;
> + }
> spin_unlock_bh(&ar->htt.tx_lock);
>
> skip = (ar->state == ATH10K_STATE_WEDGED) ||
> @@ -8090,6 +8096,13 @@ void ath10k_mac_wait_tx_complete(struct ath10k *ar)
> skip, ar->state, time_left);
> }
>
> +void ath10k_mac_wait_tx_complete(struct ath10k *ar)
> +{
> + unsigned int queues = GENMASK(ar->hw->queues - 1, 0);
> +
> + _ath10k_mac_wait_tx_complete(ar, queues);
> +}
> +
> static void ath10k_flush(struct ieee80211_hw *hw, struct ieee80211_vif *vif,
> u32 queues, bool drop)
> {
> @@ -8111,7 +8124,7 @@ static void ath10k_flush(struct ieee80211_hw *hw, struct ieee80211_vif *vif,
> }
>
> mutex_lock(&ar->conf_mutex);
> - ath10k_mac_wait_tx_complete(ar);
> + _ath10k_mac_wait_tx_complete(ar, queues);
> mutex_unlock(&ar->conf_mutex);
> }
>
> diff --git a/drivers/net/wireless/ath/ath10k/txrx.c b/drivers/net/wireless/ath/ath10k/txrx.c
> index 5b6750ef7d19..b848962fc8fb 100644
> --- a/drivers/net/wireless/ath/ath10k/txrx.c
> +++ b/drivers/net/wireless/ath/ath10k/txrx.c
> @@ -82,7 +82,7 @@ int ath10k_txrx_tx_unref(struct ath10k_htt *htt,
>
> flags = skb_cb->flags;
> ath10k_htt_tx_free_msdu_id(htt, tx_done->msdu_id);
> - ath10k_htt_tx_dec_pending(htt);
> + ath10k_htt_tx_dec_pending(htt, txq);
> spin_unlock_bh(&htt->tx_lock);
>
> rcu_read_lock();
Powered by blists - more mailing lists