diff mbox series

[09/13] mbssid: set extended capabilities

Message ID 20220302222634.22185-10-quic_alokad@quicinc.com
State Changes Requested
Headers show
Series hostapd: MBSSID and EMA support | expand

Commit Message

Aloka Dixit March 2, 2022, 10:26 p.m. UTC 
Set extended capabilities as described in IEEE Std 802.11-2020
section 9.4.2.26 Extended Capabilities element.

Co-developed-by: John Crispin <john@phrozen.org>
Signed-off-by: John Crispin <john@phrozen.org>
Signed-off-by: Aloka Dixit <quic_alokad@quicinc.com>
---
 src/ap/beacon.c | 38 ++++++++++++++++++++++++++++++++++++--
 1 file changed, 36 insertions(+), 2 deletions(-)

Comments

Jouni Malinen April 7, 2022, 8:44 p.m. UTC | #1 
On Wed, Mar 02, 2022 at 02:26:30PM -0800, Aloka Dixit wrote:
> Set extended capabilities as described in IEEE Std 802.11-2020
> section 9.4.2.26 Extended Capabilities element.

EMA is not defined in IEEE Std 802.11-2020, so this should also include
IEEE Std 802.11ax-2021.

> diff --git a/src/ap/beacon.c b/src/ap/beacon.c
> +static u8 * hostapd_ext_capab_mbssid(struct hostapd_data *hapd, u8 *eid,
> +				     u8 *eid_ext_cap, u8 ema_periodicity)
> +{
> +	size_t len;
> +
> +	if (eid == eid_ext_cap)
> +		return eid;
> +
> +	len = eid_ext_cap[1];
> +	eid_ext_cap += 2;
> +
> +	if (!hapd->iconf->mbssid && len >= 3)
> +		eid_ext_cap[2] &= ~0x40;
> +
> +	if (!hapd->iconf->ema && len >= 11)
> +		eid_ext_cap[10] &= ~0x08;
...

This looks really confusing. Why would this separate function be used to
modify what hostapd_eid_ext_capab() wrote instead of modifying
hostapd_eid_ext_capab() with these extensions?

> +	if (hapd->iconf->mbssid && ema_periodicity <= 1) {
> +		while (len < 11) {
> +			*eid++ = 0x00;
> +			len++;
> +		}

And what exactly is this trying to do? Clear a lot of unrelated bits?!

>  static u8 * hostapd_gen_probe_resp(struct hostapd_data *hapd,
> +	ext_cap_pos = pos;
>  	pos = hostapd_eid_ext_capab(hapd, pos);
> +	pos = hostapd_ext_capab_mbssid(hapd, pos, ext_cap_pos, 1);

> @@ -1656,7 +1687,10 @@ int ieee802_11_build_ap_params(struct hostapd_data *hapd,
> +	ext_cap_pos = tailpos;
>  	tailpos = hostapd_eid_ext_capab(hapd, tailpos);
> +	tailpos = hostapd_ext_capab_mbssid(hapd, tailpos, ext_cap_pos,
> +					   params->mbssid_elem_count);

What about other frames than Beacon and Probe Response frames?
hostapd_eid_ext_capab() is used for (Re)Association Response frames as
well. This would not modify hostapd_build_ap_extra_ies() either.
Aloka Dixit May 5, 2022, 6:52 p.m. UTC | #2 
On 4/7/2022 1:44 PM, Jouni Malinen wrote:
> On Wed, Mar 02, 2022 at 02:26:30PM -0800, Aloka Dixit wrote:
>> Set extended capabilities as described in IEEE Std 802.11-2020
>> section 9.4.2.26 Extended Capabilities element.
> 
> EMA is not defined in IEEE Std 802.11-2020, so this should also include
> IEEE Std 802.11ax-2021.
> 
>> diff --git a/src/ap/beacon.c b/src/ap/beacon.c
>> +static u8 * hostapd_ext_capab_mbssid(struct hostapd_data *hapd, u8 *eid,
>> +				     u8 *eid_ext_cap, u8 ema_periodicity)
>> +{
>> +	size_t len;
>> +
>> +	if (eid == eid_ext_cap)
>> +		return eid;
>> +
>> +	len = eid_ext_cap[1];
>> +	eid_ext_cap += 2;
>> +
>> +	if (!hapd->iconf->mbssid && len >= 3)
>> +		eid_ext_cap[2] &= ~0x40;
>> +
>> +	if (!hapd->iconf->ema && len >= 11)
>> +		eid_ext_cap[10] &= ~0x08;
> ...
> 
> This looks really confusing. Why would this separate function be used to
> modify what hostapd_eid_ext_capab() wrote instead of modifying
> hostapd_eid_ext_capab() with these extensions?
> 
Will try to add the code in the existing function itself.

>> +	if (hapd->iconf->mbssid && ema_periodicity <= 1) {
>> +		while (len < 11) {
>> +			*eid++ = 0x00;
>> +			len++;
>> +		}
> 
> And what exactly is this trying to do? Clear a lot of unrelated bits?!
> 
Set the "complete list of non-tx profiles bit when applicable

The second part which clears bunch of bits is because we saw issues 
during inter-op with some specific client devices which failed in 
association if MBSSID/EMA extended capabilities bit were set to '1' when 
MBSSID was not actually in use.


>>   static u8 * hostapd_gen_probe_resp(struct hostapd_data *hapd,
>> +	ext_cap_pos = pos;
>>   	pos = hostapd_eid_ext_capab(hapd, pos);
>> +	pos = hostapd_ext_capab_mbssid(hapd, pos, ext_cap_pos, 1);
> 
>> @@ -1656,7 +1687,10 @@ int ieee802_11_build_ap_params(struct hostapd_data *hapd,
>> +	ext_cap_pos = tailpos;
>>   	tailpos = hostapd_eid_ext_capab(hapd, tailpos);
>> +	tailpos = hostapd_ext_capab_mbssid(hapd, tailpos, ext_cap_pos,
>> +					   params->mbssid_elem_count);
> 
> What about other frames than Beacon and Probe Response frames?
> hostapd_eid_ext_capab() is used for (Re)Association Response frames as
> well. This would not modify hostapd_build_ap_extra_ies() either.
> 

Got it, will fix this.

Thanks.
diff mbox series

Patch

diff --git a/src/ap/beacon.c b/src/ap/beacon.c
index b2b2bfa8babe..be0f0658155a 100644
--- a/src/ap/beacon.c
+++ b/src/ap/beacon.c
@@ -511,12 +511,41 @@  static u8 * hostapd_set_mbssid_beacon(struct hostapd_data *hapd,
 }
 
 
+static u8 * hostapd_ext_capab_mbssid(struct hostapd_data *hapd, u8 *eid,
+				     u8 *eid_ext_cap, u8 ema_periodicity)
+{
+	size_t len;
+
+	if (eid == eid_ext_cap)
+		return eid;
+
+	len = eid_ext_cap[1];
+	eid_ext_cap += 2;
+
+	if (!hapd->iconf->mbssid && len >= 3)
+		eid_ext_cap[2] &= ~0x40;
+
+	if (!hapd->iconf->ema && len >= 11)
+		eid_ext_cap[10] &= ~0x08;
+
+	if (hapd->iconf->mbssid && ema_periodicity <= 1) {
+		while (len < 11) {
+			*eid++ = 0x00;
+			len++;
+		}
+		eid_ext_cap[10] |= 0x01;
+	}
+
+	return eid;
+}
+
+
 static u8 * hostapd_gen_probe_resp(struct hostapd_data *hapd,
 				   const struct ieee80211_mgmt *req,
 				   int is_p2p, size_t *resp_len)
 {
 	struct ieee80211_mgmt *resp;
-	u8 *pos, *epos, *csa_pos;
+	u8 *pos, *epos, *csa_pos, *ext_cap_pos;
 	size_t buflen;
 
 	hapd = hostapd_mbssid_get_tx_bss(hapd);
@@ -627,7 +656,9 @@  static u8 * hostapd_gen_probe_resp(struct hostapd_data *hapd,
 	pos = hostapd_eid_mbssid(hapd, pos, epos, WLAN_FC_STYPE_PROBE_RESP, 0,
 				 NULL);
 
+	ext_cap_pos = pos;
 	pos = hostapd_eid_ext_capab(hapd, pos);
+	pos = hostapd_ext_capab_mbssid(hapd, pos, ext_cap_pos, 1);
 
 	pos = hostapd_eid_time_adv(hapd, pos);
 	pos = hostapd_eid_time_zone(hapd, pos);
@@ -1523,7 +1554,7 @@  int ieee802_11_build_ap_params(struct hostapd_data *hapd,
 	size_t resp_len = 0;
 #ifdef NEED_AP_MLME
 	u16 capab_info;
-	u8 *pos, *tailpos, *tailend, *csa_pos;
+	u8 *pos, *tailpos, *tailend, *csa_pos, *ext_cap_pos;
 #endif /* NEED_AP_MLME */
 
 	os_memset(params, 0, sizeof(*params));
@@ -1656,7 +1687,10 @@  int ieee802_11_build_ap_params(struct hostapd_data *hapd,
 	tailpos = hostapd_eid_ht_operation(hapd, tailpos);
 	tailpos = hostapd_set_mbssid_beacon(hapd, params, tailpos);
 
+	ext_cap_pos = tailpos;
 	tailpos = hostapd_eid_ext_capab(hapd, tailpos);
+	tailpos = hostapd_ext_capab_mbssid(hapd, tailpos, ext_cap_pos,
+					   params->mbssid_elem_count);
 
 	/*
 	 * TODO: Time Advertisement element should only be included in some