Re: [PATCH v2] wifi: mac80211: Update bssid indicator with real BSS numbers

[Johannes Berg <johannes@sipsolutions.net>]

Hi,

We should have Lorenzo here, he wrote the original code.

On Fri, 2023-12-08 at 14:38 +0800, Allen Ye wrote:
> The cnt member in mbssid is the count of total number of MBSSID elements
> instead of BSSID. Therefore, we fix this by reading the MaxBSSID Indicator
> field directly.

I'll say I don't understand this much ...

Are you trying to have BSSIDs that are hidden from the kernel? Or not
contiguous in the MBSSID set? Not sure how the two can be not
equivalent?

> Co-developed-by: Evelyn Tsai <evelyn.tsai@mediatek.com>
> Signed-off-by: Evelyn Tsai <evelyn.tsai@mediatek.com>
> Co-developed-by: Money Wang <money.wang@mediatek.com>
> Signed-off-by: Money Wang <money.wang@mediatek.com>
> Signed-off-by: Allen Ye <allen.ye@mediatek.com>

I have to admit that I chuckled a bit about this for a 5 line patch :-)

> diff --git a/net/mac80211/cfg.c b/net/mac80211/cfg.c
> index 606b1b2e4123..f90bcd59f85a 100644
> --- a/net/mac80211/cfg.c
> +++ b/net/mac80211/cfg.c
> @@ -1164,9 +1164,11 @@ ieee80211_assign_beacon(struct ieee80211_sub_if_data *sdata,
>  	/* copy in optional mbssid_ies */
>  	if (mbssid) {
>  		u8 *pos = new->tail + new->tail_len;
> +		const struct element *mbssid_elem;
>  
>  		new->mbssid_ies = (void *)pos;
>  		pos += struct_size(new->mbssid_ies, elem, mbssid->cnt);
> +		mbssid_elem = (const struct element *)pos;
>  		pos += ieee80211_copy_mbssid_beacon(pos, new->mbssid_ies,
>  						    mbssid);
>  		if (rnr) {
> @@ -1175,8 +1177,7 @@ ieee80211_assign_beacon(struct ieee80211_sub_if_data *sdata,
>  			ieee80211_copy_rnr_beacon(pos, new->rnr_ies, rnr);
>  		}
>  		/* update bssid_indicator */
> -		link_conf->bssid_indicator =
> -			ilog2(__roundup_pow_of_two(mbssid->cnt + 1));
> +		link_conf->bssid_indicator = mbssid_elem->data[0];

But this seems fishy to me, if you look into the element itself, you're
going to have to do some validation on it? And what about fragmentation?

johannes