Re: [PATCH v2 1/2] wifi: cfg80211: say why the auth/assoc BSS lookup failed

[Johannes Berg <johannes@sipsolutions.net>]

On Tue, 2026-07-21 at 20:11 +0200, Louis Kotze wrote:
> 
> -	if (!attrs[NL80211_ATTR_MAC] || !attrs[NL80211_ATTR_WIPHY_FREQ])
> +	if (!attrs[NL80211_ATTR_MAC] || !attrs[NL80211_ATTR_WIPHY_FREQ]) {
> +		NL_SET_ERR_MSG(info->extack, "BSSID or frequency missing");
> 
> 

could use the GENL versions and save some characters :)

>  		if (IS_ERR(links[link_id].bss)) {
>  			err = PTR_ERR(links[link_id].bss);
>  			links[link_id].bss = NULL;
> -			NL_SET_ERR_MSG_ATTR(info->extack, link,
> -					    "Error fetching BSS for link");
> +			NL_SET_BAD_ATTR(info->extack, link);

why remove the message, it's possible to have both?

>  
> -		req.bss = nl80211_assoc_bss(rdev, ssid, ssid_len, info->attrs,
> -					    -1, -1);
> +		req.bss = nl80211_assoc_bss(rdev, info, ssid, ssid_len,
> +					    info->attrs, -1, -1);

entirely unrelated (whitespace) change?

> +++ b/net/wireless/scan.c
> @@ -1609,10 +1609,12 @@ struct cfg80211_bss *__cfg80211_get_bss(struct wiphy *wiphy,
>  					const u8 *ssid, size_t ssid_len,
>  					enum ieee80211_bss_type bss_type,
>  					enum ieee80211_privacy privacy,
> -					u32 use_for)
> +					u32 use_for,
> +					struct netlink_ext_ack *extack)
>  {
>  	struct cfg80211_registered_device *rdev = wiphy_to_rdev(wiphy);
>  	struct cfg80211_internal_bss *bss, *res = NULL;
> +	bool expired = false, unusable = false;
>  	unsigned long now = jiffies;
>  	int bss_privacy;
>  
> @@ -1634,22 +1636,39 @@ struct cfg80211_bss *__cfg80211_get_bss(struct wiphy *wiphy,
>  			continue;
>  		if (!is_valid_ether_addr(bss->pub.bssid))
>  			continue;
> -		if ((bss->pub.use_for & use_for) != use_for)
> +		if (!is_bss(&bss->pub, bssid, ssid, ssid_len))
>  			continue;
> +
>  		/* Don't get expired BSS structs */
>  		if (time_after(now, bss->ts + IEEE80211_SCAN_RESULT_EXPIRE) &&
> -		    !atomic_read(&bss->hold))
> +		    !atomic_read(&bss->hold)) {
> +			expired = true;
> +			continue;
> +		}
> +
> +		if ((bss->pub.use_for & use_for) != use_for) {
> +			unusable = true;
>  			continue;
> -		if (is_bss(&bss->pub, bssid, ssid, ssid_len)) {
> -			res = bss;
> -			bss_ref_get(rdev, res);
> -			break;
>  		}
> +
> +		res = bss;
> +		bss_ref_get(rdev, res);
> +		break;

That code should probably have a comment that the is_bss() must come
first...

Also however, it could result in having *both* 'unusable' and 'expired'
set, and then

> -	if (!res)
> +	if (!res) {
> +		if (unusable)
> +			NL_SET_ERR_MSG(extack,
> +				       "BSS cannot be used for the requested operation");
> +		else if (expired)
> +			NL_SET_ERR_MSG(extack,
> +				       "BSS entry is expired, scan again");
> +		else
> +			NL_SET_ERR_MSG(extack,
> +				       "BSS not found in scan results");
>  		return NULL;

you prefer the 'unusable' message... not sure that makes sense?

I also don't think the "scan again" instruction makes any sense here -
this is meant to provide an error message, not instructions how to fix
it?

johannes