Re: [PATCH] scan: limit COLOCATED_6GHZ flag to 6ghz devices

[Denis Kenzior <denkenz@gmail.com>]

Hi James,

On 1/3/24 06:56, James Prestwood wrote:
> It was seen that this flag seems to cause issues when in AP mode on
> brcmfmac devices (e.g. the raspberry Pi 3). When in AP mode an a

Seriously?  Why would this flag do anything for non-6E devices?

> scan is issued clients will disconnect. After testing this behavior
> was isolated to the use of the COLOCATED_6GHZ flag.
> 
> Besides working around the problem on this specific hardware the
> patch itself makes sense as a non-6GHz capable device shouldn't use
> this flag anyways.

I guess if you're super paranoid and don't trust the kernel to do the right thing...

> 
> As stated in the patch comment, this isn't really a catch all
> workaround since the flag is still used for devices supporting 6GHz.
> If additional hardware exhibits this behavior we may need additional
> changes like a hardware blacklist or an explicit option to disable
> the flag.
> 
> Reported-By: Keith G <ys3al35l@gmail.com>
> ---
>   src/scan.c | 15 ++++++++++++++-
>   1 file changed, 14 insertions(+), 1 deletion(-)
> 
> diff --git a/src/scan.c b/src/scan.c
> index f48ffdef..8c6fdc08 100644
> --- a/src/scan.c
> +++ b/src/scan.c
> @@ -394,7 +394,20 @@ static struct l_genl_msg *scan_build_cmd(struct scan_context *sc,
>   	if (params->ap_scan)
>   		flags |= NL80211_SCAN_FLAG_AP;
>   
> -	flags |= NL80211_SCAN_FLAG_COLOCATED_6GHZ;
> +	/*
> +	 * TODO: This flag appears to cause some undesired behavior on brcmfmac
> +	 *       when the device is in AP mode, or has a secondary AP interface
> +	 *       running, causing clients to disconnect when a scan is issued.
> +	 *
> +	 *       Only using this flag for 6GHz capable devices will limit this
> +	 *       behavior to only 6GHz devices and in reality makes sense
> +	 *       because a non-6GHz device shouldn't use this flag anyways. If
> +	 *       more issues still are seen related to this we may need an
> +	 *       explicit workaround, either brcmfmac-specific or a disable
> +	 *       option.
> +	 */

I'd just drop this comment entirely if you still think this is needed.

> +	if (wiphy_band_is_disabled(sc->wiphy, BAND_FREQ_6_GHZ) != -ENOTSUP)
> +		flags |= NL80211_SCAN_FLAG_COLOCATED_6GHZ;
>   
>   	if (flags)
>   		l_genl_msg_append_attr(msg, NL80211_ATTR_SCAN_FLAGS, 4, &flags);

Regards,
-Denis