Re: [PATCH 02/31] staging: wfx: do not send CAB while scanning

From: Kari Argillander <kari.argillander@gmail.com>
To: Jerome Pouiller <Jerome.Pouiller@silabs.com>
Cc: devel@driverdev.osuosl.org, linux-wireless@vger.kernel.org,
	netdev@vger.kernel.org, linux-kernel@vger.kernel.org,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	Kalle Valo <kvalo@codeaurora.org>,
	"David S . Miller" <davem@davemloft.net>
Subject: Re: [PATCH 02/31] staging: wfx: do not send CAB while scanning
Date: Fri, 10 Sep 2021 19:31:00 +0300	[thread overview]
Message-ID: <20210910163100.n6ltzn543f2mnggy@kari-VirtualBox> (raw)
In-Reply-To: <20210910160504.1794332-3-Jerome.Pouiller@silabs.com>

On Fri, Sep 10, 2021 at 06:04:35PM +0200, Jerome Pouiller wrote:
> From: Jérôme Pouiller <jerome.pouiller@silabs.com>
> 
> During the scan requests, the Tx traffic is suspended. This lock is
> shared by all the network interfaces. So, a scan request on one
> interface will block the traffic on a second interface. This causes
> trouble when the queued traffic contains CAB (Content After DTIM Beacon)
> since this traffic cannot be delayed.
> 
> It could be possible to make the lock local to each interface. But It
> would only push the problem further. The device won't be able to send
> the CAB before the end of the scan.
> 
> So, this patch just ignore the DTIM indication when a scan is in
> progress. The firmware will send another indication on the next DTIM and
> this time the system will be able to send the traffic just behind the
> beacon.
> 
> The only drawback of this solution is that the stations connected to
> the AP will wait for traffic after the DTIM for nothing. But since the
> case is really rare it is not a big deal.
> 
> Signed-off-by: Jérôme Pouiller <jerome.pouiller@silabs.com>
> ---
>  drivers/staging/wfx/sta.c | 10 ++++++++++
>  1 file changed, 10 insertions(+)
> 
> diff --git a/drivers/staging/wfx/sta.c b/drivers/staging/wfx/sta.c
> index a236e5bb6914..d901588237a4 100644
> --- a/drivers/staging/wfx/sta.c
> +++ b/drivers/staging/wfx/sta.c
> @@ -629,8 +629,18 @@ int wfx_set_tim(struct ieee80211_hw *hw, struct ieee80211_sta *sta, bool set)
>  
>  void wfx_suspend_resume_mc(struct wfx_vif *wvif, enum sta_notify_cmd notify_cmd)
>  {
> +	struct wfx_vif *wvif_it;
> +
>  	if (notify_cmd != STA_NOTIFY_AWAKE)
>  		return;
> +
> +	// Device won't be able to honor CAB if a scan is in progress on any
> +	// interface. Prefer to skip this DTIM and wait for the next one.

In one patch you drop // comments but you introduce some of your self.

> +	wvif_it = NULL;
> +	while ((wvif_it = wvif_iterate(wvif->wdev, wvif_it)) != NULL)
> +		if (mutex_is_locked(&wvif_it->scan_lock))
> +			return;
> +
>  	if (!wfx_tx_queues_has_cab(wvif) || wvif->after_dtim_tx_allowed)
>  		dev_warn(wvif->wdev->dev, "incorrect sequence (%d CAB in queue)",
>  			 wfx_tx_queues_has_cab(wvif));
> -- 
> 2.33.0
> 