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

[Johannes Berg <johannes@sipsolutions.net>]

On Wed, 2023-12-27 at 09:38 +0000, Allen Ye (葉芷勳) wrote:
> > 
> > We should have Lorenzo here, he wrote the original code.

Actually John/Aloka at least also did ... so whatever.


All this only _really_ matters I think when you have EMA, right?


> The number of BSS is not equal to MBSSID element count plus 1 because
> there might be multiple nontransmitted BSSID profile subelements in one
> MBSSID element.

Yes.

Actually it's even more confusing:

> Also, one nontransmitted BSSID profile subelement might
> be fragmented across two MBSSID elements if the length of the
> subelement exceeds 255 octets.

True; however, in this case, a single entry in the
NL80211_ATTR_MBSSID_ELEMS array (and thus in mbssid_ies) must contain
both MBSSID elements together, so it's only counted once still. More
importantly, otherwise we would split them across two frames with EMA,
which is wrong.

So really mbssid_ies / NL80211_ATTR_MBSSID_ELEMS is *not* the list of
MBSSID elements as they should appear in the frame, individually, but an
array of *sets* of MBSSID elements.

For example:

mbssid_ies[0] = mbssid_elem(profile1, profile2), mbssid_elem(profile3)
mbssid_ies[1] = mbssid_elem(profile4_part1), mbssid_elem(profile4_part2)

when you have EMA with periodicity 2, and 4 non-transmitted BSSes where
the first two are small and fit into one element, number 3 is bigger and
needs its own, and number 4 needs to be split ...


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

I stand by this though.

> > And what about fragmentation?

But not this :)

> Whether the subelement is aggregated or fragmented, the MaxBSSID
> Indicator field would be the same for the multiple BSSID set.

Right.

> Therefore, we directly retrieve this field from the element.

Yeah, makes sense.

> For example, there are five BSSes in one multiple BSSID set, one
> transmitted and four non-transmitted BSSes. We might use just two
> MBSSID elements to store all the non-transmitted BSS information. Here
> the MaxBSSID Indicator is 3 in both MBSSID elements. However, the
> element cnt is 2 and if we use the original method to calculate the
> BSSID Indicator we will get 2 which is wrong.

Right.

Anyway, I think I agree, but can you please add some validation of this
to cfg80211 as a first patch, and also document all this better in the
commit message? Optional, but some additional nl80211 documention would
be very nice.

Thanks,
johannes