Received-SPF: pass (google.com: domain of linux-wireless+bounces-6714-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.199.223 as permitted sender) client-ip=147.75.199.223;
Message-ID: <757f417d63a750e84ab883b500becc2773e9ff17.camel@sipsolutions.net>
Subject: Re: [PATCH v2] wifi: mac80211: Update bssid indicator with real BSS
 numbers
From: Johannes Berg <johannes@sipsolutions.net>
To: Allen Ye =?UTF-8?Q?=28=E8=91=89=E8=8A=B7=E5=8B=B3=29?=
 <Allen.Ye@mediatek.com>, "linux-wireless@vger.kernel.org"
 <linux-wireless@vger.kernel.org>, "lorenzo@kernel.org" <lorenzo@kernel.org>
Cc: "lorenzo.bianconi@redhat.com" <lorenzo.bianconi@redhat.com>, 
	"nbd@nbd.name"
	 <nbd@nbd.name>, Evelyn Tsai =?UTF-8?Q?=28=E8=94=A1=E7=8F=8A=E9=88=BA=29?=
	 <Evelyn.Tsai@mediatek.com>, Money Wang
	=?UTF-8?Q?=28=E7=8E=8B=E4=BF=A1=E5=AE=89=29?=
	 <Money.Wang@mediatek.com>, "linux-mediatek@lists.infradead.org"
	 <linux-mediatek@lists.infradead.org>
Date: Tue, 23 Apr 2024 13:21:02 +0200
In-Reply-To: <c23373d71eff81ea043783edff0345502bb2e299.camel@mediatek.com>
References: <20231208063820.25983-1-allen.ye@mediatek.com>
	 <65d21336e8f7e180352403a3e894aeaf27a22cab.camel@sipsolutions.net>
	 <c23373d71eff81ea043783edff0345502bb2e299.camel@mediatek.com>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
User-Agent: Evolution 3.50.4 (3.50.4-1.fc39) 
Precedence: bulk
MIME-Version: 1.0

On Wed, 2023-12-27 at 09:38 +0000, Allen Ye (=E8=91=89=E8=8A=B7=E5=8B=B3) w=
rote:
> >=20
> > 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] =3D mbssid_elem(profile1, profile2), mbssid_elem(profile3)
mbssid_ies[1] =3D 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