Message-ID: <3d76d0d033609fff4e72448a9fd7d63ad1365cb2.camel@sipsolutions.net>
Subject: Re: [RFC PATCH] mac80211/cfg80211: update bss channel on channel
 switch
From:   Johannes Berg <johannes@sipsolutions.net>
To:     Sergey Matyukevich <sergey.matyukevich.os@quantenna.com>,
        "linux-wireless@vger.kernel.org" <linux-wireless@vger.kernel.org>
Cc:     Igor Mitsyanko <igor.mitsyanko.os@quantenna.com>,
        Arend van Spriel <arend.vanspriel@broadcom.com>
Date:   Sat, 23 Mar 2019 13:32:47 +0100
In-Reply-To: <20190321151915.16826-1-sergey.matyukevich.os@quantenna.com> (sfid-20190321_162003_388890_DC9426EB)
References: <20190321151915.16826-1-sergey.matyukevich.os@quantenna.com>
         (sfid-20190321_162003_388890_DC9426EB)
Content-Type: text/plain; charset="UTF-8"
Mime-Version: 1.0
Content-Transfer-Encoding: 7bit
Sender: linux-wireless-owner@vger.kernel.org
Precedence: bulk

On Thu, 2019-03-21 at 15:19 +0000, Sergey Matyukevich wrote:
> FullMAC STAs have no way to update bss channel after CSA channel switch
> completion. As a result, user-space tools may provide inconsistent
> channel info. For instance, consider the following two commands:
> $ sudo iw dev wlan0 link
> $ sudo iw dev wlan0 info
> The latter command gets channel info from the hardware, so most probably
> its output will be correct. However the former command gets channel info
> from scan cache, so its output will contain outdated channel info.
> In fact, current bss channel info will not be updated until the
> next [re-]connect.
> 
> Note that mac80211 STAs have a workaround for this, but it requires
> access to internal cfg80211 data, see ieee80211_chswitch_work:
> 
> 	/* XXX: shouldn't really modify cfg80211-owned data! */
> 	ifmgd->associated->channel = sdata->csa_chandef.chan;
> 
> This patch suggests to convert mac80211 workaround to cfg80211 behavior
> and to update current bss channel in cfg80211_ch_switch_notify.
> 
> Signed-off-by: Sergey Matyukevich <sergey.matyukevich.os@quantenna.com>
> 
> ---
> 
> This issue has been observed for both qtnfmac and brcmfmac. Fix tested
> for qtnfmac and iwlwifi, to make sure there is no regression for mac80211.
> However this is not going to be enough to fix brcmfmac behavior as it
> does not use cfg80211_ch_switch_notify. This issue may also affect
> ath6kl and mwifiex, but I have no hardware to check.

Yeah, I think this makes sense.

> +	if (wdev->iftype == NL80211_IFTYPE_STATION) {
> +		if (wdev->current_bss)
> +			wdev->current_bss->pub.channel = chandef->chan;
> +	}

Maybe do

	if (wdev->iftype == ... &&
	    !WARN_ON(!wdev->current_bss))
		wdev->current_bss->... = ...

I worry slightly about locking too, since there's no protection on the
wdev->current_bss pointer here.

johannes