[PATCH 05/13] mbssid: add multiple BSSID elements

[Jouni Malinen]

On Wed, Mar 02, 2022 at 02:26:26PM -0800, Aloka Dixit wrote:
> Add data as per IEEE Std 802.11-2020 9.4.2.45 Multiple BSSID element.
> Include this element in beacons and probe response frames when mbssid
> is enabled. Split the BSSes into multiple elements if the data
> does not fit in 255 bytes allowed for a single element.
> 
> Add support for enhanced multiple BSSID advertisements (EMA) by
> sending offsets to the start of each MBSSID element to nl80211.
> Mac80211 generates different EMA beacons by including only one
> multiple BSSID element in each beacon frame.

What are those nl80211/mac80211 comments doing here when there are no
changes to any nl80211 functionality in this patch?

> diff --git a/src/ap/beacon.c b/src/ap/beacon.c
> @@ -447,9 +450,54 @@ static void hostapd_set_mbssid_beacon(struct hostapd_data *hapd,
> -	params->mbssid_tx_iface = hostapd_mbssid_get_tx_bss(hapd)->conf->iface;
> +	tx_bss = hostapd_mbssid_get_tx_bss(hapd);
> +	params->mbssid_tx_iface = tx_bss->conf->iface;
>  	params->mbssid_index = hostapd_mbssid_get_bss_index(hapd);
>  	params->mbssid_count = iface->num_bss;
> +
> +	len = hostapd_eid_mbssid_len(tx_bss, WLAN_FC_STYPE_BEACON, &num_mbssid);
> +	if (hapd->iconf->ema) {
> +		if (!iface->ema_max_periodicity) {
> +			wpa_printf(MSG_DEBUG,
> +				   "MBSSID: Driver doesn't support enhanced multiple BSSID advertisements");
> +			return;
> +		}
> +		if (num_mbssid > iface->ema_max_periodicity) {
> +			wpa_printf(MSG_DEBUG,
> +				   "MBSSID: Driver supports maximum %u EMA profile periodicity",
> +				   iface->ema_max_periodicity);
> +			return;
> +		}
> +		params->ema = 1;
> +	}

Can those no-driver-support cases happen in practice? If so, it would
seem much better to not allow things get this far in the first place
instead of ignoring missing capability and ending up doing something
else than what the configuration explicitly asked for.

> @@ -462,8 +510,11 @@ static u8 * hostapd_gen_probe_resp(struct hostapd_data *hapd,
> +	buflen += hostapd_eid_mbssid_len(hapd, WLAN_FC_STYPE_PROBE_RESP, NULL);
...
> +	pos = hostapd_eid_mbssid(hapd, pos, epos, WLAN_FC_STYPE_PROBE_RESP, 0,
> +				 NULL);

Is there something preventing the case of this Probe Response frame
ending up being too long to fit within the maximum MMPDU length if there
is a large number of nontransmitted BSSIDs?

> @@ -1100,6 +1154,9 @@ void handle_probe_req(struct hostapd_data *hapd,
> +	if (hapd != hostapd_mbssid_get_tx_bss(hapd) && res != EXACT_SSID_MATCH)
> +		return;

It would be nice to include a comment above this explaining what this
special case is about.

> diff --git a/src/ap/ieee802_11.c b/src/ap/ieee802_11.c
> +u8 hostapd_max_bssid_indicator(struct hostapd_data *hapd)

static u8 ...

> @@ -7450,4 +7467,199 @@ u8 * hostapd_eid_rnr(struct hostapd_data *hapd, u8 *eid, u32 type)
>  	return eid;
>  }
>  
> +
> +static size_t hostapd_eid_mbssid_elem_len(struct hostapd_data *hapd,
> +					  u32 frame_type, size_t *bss_index)
> +{
> +	size_t len = 3, i;
> +
> +	for (i = *bss_index; i < hapd->iface->num_bss; i++) {
> +		struct hostapd_data *bss = hapd->iface->bss[i];
> +		const u8 *auth, *rsn, *rsnx;
> +		size_t nontx_profile_len, auth_len;
> +
> +		if (!bss || !bss->conf || !bss->started)
> +			continue;
> +
> +		/*
> +		 * Sublement ID: 1 byte
> +		 * Length: 1 byte
> +		 * Nontransmitted capabilities: 4 bytes
> +		 * SSID element: 2 + variable
> +		 * Multiple BSSID Index Element: 3 bytes (+2 bytes in beacons)
> +		 * Fixed length = 1 + 1 + 4 + 2 + 3 = 11
> +		 */
> +		nontx_profile_len = 11 + bss->conf->ssid.ssid_len;
> +
> +		if (frame_type == WLAN_FC_STYPE_BEACON)
> +			nontx_profile_len += 2;
> +
> +		auth = wpa_auth_get_wpa_ie(bss->wpa_auth, &auth_len);
> +		if (auth) {
> +			rsn = get_ie(auth, auth_len, WLAN_EID_RSN);
> +			if (rsn)
> +				nontx_profile_len += (2 + rsn[1]);
> +
> +			rsnx = get_ie(auth, auth_len, WLAN_EID_RSNX);
> +			if (rsnx)
> +				nontx_profile_len += (2 + rsnx[1]);
> +		}

Are those all the elements that are needed here? There seems to be a lot
of other things that could differ between the nontransmitted BSSIDs and
the transmitted BSSID. The same would obviously apply to
hostapd_eid_mbssid_elem() below.

> diff --git a/src/drivers/driver.h b/src/drivers/driver.h
> +	/**
> +	 * mbssid_elem - Buffer containing all multiple BSSID elements
> +	 */
> +	u8 *mbssid_elem;

Should that be const u8 * instead?

> +	/**
> +	 * mbssid_elem_count - The number of multiple bssid elements

" bssid " --> " BSSID "

> +	/**
> +	 * mbssid_elem_offset - Offsets to elements in mbssid_elem.
> +	 * Kernel will use these offsets to generate multiple BSSID beacons.
> +	 */
> +	u8 **mbssid_elem_offset;

const?

> +	/**
> +	 * ema - Enhanced multi-bssid advertisements support.
> +	 */
> +	u8 ema;

bool instead of u8

> +
>  };

No empty line at the end of the block.

-- 
Jouni Malinen                                            PGP id EFC895FA