Received-SPF: pass (google.com: domain of linux-wireless+bounces-7135-linux.lists.archive=gmail.com@vger.kernel.org designates 139.178.88.99 as permitted sender) client-ip=139.178.88.99;
Message-ID: <226dad2f0736d9b5c3313943dce93f57cb6f4b2a.camel@sipsolutions.net>
Subject: Re: [RFC v2 0/2] wifi: nl80211/mac80211 Handle BSS critical update
From: Johannes Berg <johannes@sipsolutions.net>
To: Rathees Kumar R Chinannan <quic_rrchinan@quicinc.com>
Cc: linux-wireless@vger.kernel.org
Date: Fri, 03 May 2024 11:27:10 +0200
In-Reply-To: <20240403162225.3096228-1-quic_rrchinan@quicinc.com>
References: <20240403162225.3096228-1-quic_rrchinan@quicinc.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, 2024-04-03 at 21:52 +0530, Rathees Kumar R Chinannan wrote:
> When a critical update occurs to any of elements inside beacon frame, AP
> shall increment BSS Parameters Change Count(BPCC) subfield and set the
> Critical Update flag subfield of the Capability Information to notify
> client that the critical update occurred on AP. Refer section "35.3.10
> BSS parameter critical update procedure" on IEEE P802.11be D4.0 for
> details.
>=20
> On beacon offload case, change in CU parameters should be sent to user
> space either before or along with probe or assoc request frame receive
> to ensure that user space uses latest CU values and BPCC while generating
> response to the received frames. So, add the critical update parameters
> as a new attribute to existing NL80211_CMD_FRAME command instead of
> sending this on a separate NL80211 event.
>=20
> Add an ieee80211_critical_update() API to send the parameters to cfg80211
> and call it when event received from firmware to update critical
> parameters to user space.

Somewhat more conceptually, I wonder if we should really handle this
hybrid approach? You're offloading the beacon updates, why not offload
the probe/assoc response cases as well? Are they really _that_ much more
complex? What does the hostapd code for this look like?


Also, as we already discussed, this is fundamentally racy today, and
that cannot be fixed unless you really put it all into the firmware,
directly in the TX path, which is probably never going to happen.

So under the assumption that it already *is* racy, I'm not entirely sure
I see where this is needed at all?

You're basically handling two (kinds of) values here:

 1a) CSA counters: these are today handled in mac80211, and we've
     already decided that we need to handle them also in mac80211 (and
     get offsets to the partner links from hostapd a la
     NL80211_ATTR_CSA_C_OFFSETS_TX).
     Yeah this is still racy, but you can't fix that without offloading
     it all anyway.

 1b) BCCA counters: these are missing today, but that should be fixed,
     and then it's just the same as 1a.

 2) BSS parameter change count and critical update flag: are these
    really actually completely by the firmware? I'm not sure how that's
    necessary, since hostapd initiates all the relevant operations? So
    not sure why you need these at all, couldn't hostapd track this and
    you just copy it across to the other links?

So for 1a/1b I don't even think we should push this to hostapd, it's not
necessary and it will just cause more API fragmentation, because already
*know* that we have to do things like NL80211_ATTR_CSA_C_OFFSETS_TX for
other devices (and hwsim) for this, just more complex.

Thus, I don't think this is right. We can handle your case the same way,
and you don't even need the NL80211_EXT_FEATURE_CRITICAL_UPDATE_OFFLOAD
flag right now, by just pushing the offsets into the kernel and handling
the values coming from the firmware by filling in at the right offsets.
Could perhaps get the offsets down into the driver and do it there, or
just handle it in mac80211, not sure.

But this then doesn't fragment the API here, because hostapd will just
give all the offsets, and the below stack (mac80211/driver/fw/hwsim)
will fill the values.

Not sure about the BPCC and critical update flag, but let's think about
how that would be handled on other hardware? It feels to me right now
that hostapd should already be in control and know this, and not need
any indications (it does control all links), but maybe I'm wrong (or
there's just some extra case the firmware might do) and it doesn't, but
then let's also consider how that would be handled in other hardware (or
think about hwsim instead), and find API that doesn't fragment so much.

Yes we'll always fragment whether or not the partner link beacons need
updating, but it shouldn't need all those different paths for everything
else too.

johannes