Re: [PATCH v12 2/4] mac80211: MBSSID support in interface handling

[Jouni Malinen <j@w1.fi>]

On Wed, Sep 15, 2021 at 07:54:35PM -0700, Aloka Dixit wrote:
> Configure multiple BSSID and enhanced multi-BSSID advertisement (EMA)
> parameters in mac80211 for AP mode.
> 
> For each interface, 'mbssid_tx_vif' points to the transmitting interface of
> the MBSSID set. The pointer is set to NULL if MBSSID is disabled.
> 
> Function ieee80211_stop() is modified to always bring down all the
> non-transmitting interfaces first and the transmitting interface last.

This has already been applied, but this has some apparent issues that
are now showing up with mac80211_hwsim testing being available..

> diff --git a/net/mac80211/cfg.c b/net/mac80211/cfg.c
> +static int ieee80211_set_ap_mbssid_options(struct ieee80211_sub_if_data *sdata,
> +					   struct cfg80211_mbssid_config params)

While that does not really break behavior, why is that params argument
passed by value instead of by reference? I see no point in copying
struct cfg80211_mbssid_config members for this call since the function
is only reading the value.

> +	sdata->vif.mbssid_tx_vif = NULL;
> +	sdata->vif.bss_conf.bssid_index = 0;
> +	sdata->vif.bss_conf.nontransmitted = false;
> +	sdata->vif.bss_conf.ema_ap = false;

This cleanup is important, but it is done only here in this helper
function..

> @@ -1105,6 +1135,14 @@ static int ieee80211_start_ap(struct wiphy *wiphy, struct net_device *dev,
> +	if (sdata->vif.type == NL80211_IFTYPE_AP &&
> +	    params->mbssid_config.tx_wdev) {
> +		err = ieee80211_set_ap_mbssid_options(sdata,
> +						      params->mbssid_config);
> +		if (err)
> +			return err;
> +	}

And that is the only place where the help function is called and this
happens only under the params->mbssid_config.tx_wdev condition. In other
words, those bssid_index/nontransmitted/ema_ap values are not cleared in
all cases. This results in issue when the bss_conf (link_conf in the
current kernel snapshot) is left in the previous mbssid configuration.

As an example, this will make the following mac80211_hwsim test case
sequence fail:

hostap/tests/hwsim/vm$ ./vm-run.sh he_ap_ema p2p_group_cli_invalid

This happens because ema_ap is set to true in he_ap_ema and then it is
left set true for p2p_group_cli_invalid and that test case does not
actually end up sending Beacon frames.

This can be fixed by clearing something in the
!params->mbssid_config.tx_wdev case in ieee80211_start_ap(). I'm not
completely sure what is the correct way of doing this, but at least
ema_ap needs to be cleared to false and likely some other cleanup needs
to be done as well.

-- 
Jouni Malinen                                            PGP id EFC895FA