| Message ID | 20240403162225.3096228-1-quic_rrchinan@quicinc.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | wifi: nl80211/mac80211 Handle BSS critical update | expand |
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. > > 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. > > 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
On 5/3/2024 2:57 PM, Johannes Berg wrote: > 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. >> >> 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. >> >> 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. > In case of MBSSID configuration, hostapd has to provide multiple offsets to update these counters on BMLE per-STA profile for each non-TX MBSSID profile when CSA/BCCA triggered on any of partner links. To avoid providing multiple offsets, this approach is used to pass the counter values to hostapd and use it while generating the probe/assoc response frame. > 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 Yes, firmware is handling some extra cases and maintain synchronization among partner links and it maintains BPCC value and set/clear critical flag based on TBTT timer. Hence we are passing these values also to user space. > 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. > Sure, will check this. > 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