linux-kernel - Re: [PATCH 2/2] wifi: ath10k: Flush only requested txq in ath10k

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <753a6661-9d52-93da-23a4-d33dab4a396b@quicinc.com>
Date: Fri, 18 Oct 2024 21:48:50 +0530
From: Vasanthakumar Thiagarajan <quic_vthiagar@...cinc.com>
To: Remi Pommarel <repk@...plefau.lt>, <ath10k@...ts.infradead.org>,
        <linux-wireless@...r.kernel.org>, <linux-kernel@...r.kernel.org>
CC: Kalle Valo <kvalo@...nel.org>, Jeff Johnson <jjohnson@...nel.org>,
        Cedric
 Veilleux <veilleux.cedric@...il.com>
Subject: Re: [PATCH 2/2] wifi: ath10k: Flush only requested txq in
 ath10k_flush()



On 10/12/2024 7:43 PM, 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>
> 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    | 33 +++++++++++++++++-------
>   drivers/net/wireless/ath/ath10k/txrx.c   |  2 +-
>   4 files changed, 44 insertions(+), 16 deletions(-)
> 
> diff --git a/drivers/net/wireless/ath/ath10k/htt.h b/drivers/net/wireless/ath/ath10k/htt.h
> index d150f9330941..33054fc4d9fb 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 pending_per_queue[IEEE80211_MAX_QUEUES];

Something like num_pending_per_queue[] to align with the existing
packet counter looks better?

Vasanth