Re: [PATCH] wifi: nl80211: allow MBSSID Tx VAP bringup without MBSSID IEs

[Sowmiya Sree Elavalagan <quic_ssreeela@xxxxxxxxxxx>]

On 6/26/2024 5:45 PM, Johannes Berg wrote:
> On Fri, 2024-06-21 at 13:10 +0530, Sowmiya Sree Elavalagan wrote:
>> From: Rameshkumar Sundaram <quic_ramess@xxxxxxxxxxx>
>>
>> Current implementation of MBSSID configuration parsing mandates
>> MBSSID elements for Tx BSS (index 0). However with ML link addition
>> it is possible that Non-Tx BSS'es can be added at a later point in
>> time after Tx BSS is brought up. Hence allow bring up of MBSSID Tx
>> BSS even if no Non-Tx BSS are present at that time. Later when new
>> Non-TX BSS are added TX BSS beacon can be updated with MBSSID IEs.
> 
> nit: I tend to think we should mostly use "element" instead of "IE"
> since the spec changed (subject and text), except where historically we
> have variable names etc.
> 
> I'm also not convinced this actually works without further changes down
> the stack? Think ath11k/mac80211 for example, where
> ieee80211_beacon_get_template_ema_list() is called but would now return
> NULL because 
> 
>                 if (ema_beacons) {
>                         *ema_beacons =
>                                 ieee80211_beacon_get_ap_ema_list(hw, vif, link,
> 
> but the list is empty.
> 
> But you can still set NL80211_MBSSID_CONFIG_ATTR_EMA so it would be an
> EMA AP, and have config->tx_wdev set ...
> 
> So I don't think this can be correct?
> 
> johannes

Hi Johannes,
 
I agree, but based on the current hostapd implementation, this flag NL80211_MBSSID_CONFIG_ATTR_EMA is set only when num_bss > 1. This flag will not be set when we do not have any non Tx BSS.  
Even if this NL80211_MBSSID_CONFIG_ATTR_EMA is set when no TX BSS is present, can just fill beacon template in 0th index of ieee80211_ema_beacons structure, if mbssid_ies are not present.  
Shall we handle this in mac80211 layer in ieee80211_beacon_get_ap_ema_list function like below
 
ieee80211_beacon_get_ap_ema_list(struct ieee80211_hw *hw,
                                 struct ieee80211_chanctx_conf *chanctx_conf)
{
         ...

-       if (!beacon->mbssid_ies || !beacon->mbssid_ies->cnt)
-               return NULL;
-
-       ema = kzalloc(struct_size(ema, bcn, beacon->mbssid_ies->cnt),
+       if (!beacon->mbssid_ies || !beacon->mbssid_ies->cnt) {
+               ema = kzalloc(struct_size(ema, bcn, 1), GFP_ATOMIC);
+               total_beacons = 1;
+
+       } else {
+               ema = kzalloc(struct_size(ema, bcn, beacon->mbssid_ies->cnt),
                      GFP_ATOMIC);
+               total_beacons = beacon->mbssid_ies->cnt;
+       }
+
        if (!ema)
                return NULL;
 
-       for (ema->cnt = 0; ema->cnt < beacon->mbssid_ies->cnt; ema->cnt++) {
+       for (ema->cnt = 0; ema->cnt < total_beacons; ema->cnt++) {
           .....


Thanks,
Sowmiya Sree