| Message ID | CAGnO3drxMJTwh9uwKUQW5Rz2rfG74veJ7P9OzuBcpOopZjPcag@mail.gmail.com |
|---|---|
| State | Changes Requested |
| Headers | show |
On Wed, Feb 22, 2017 at 04:54:08PM +0000, Nick Lowe wrote: > Explicitly require WMM to be enabled if HT (11n) or VHT > (11ac) is enabled. This adds a check where VHT is enabled that was previously > missing. WMM would previously silently be enabled where HT was enabled. Could you please clarify why this would be a good thing to do? > diff --git a/src/ap/hostapd.c b/src/ap/hostapd.c > @@ -960,8 +960,10 @@ static int hostapd_setup_bss(struct hostapd_data > *hapd, int first) > - if (conf->wmm_enabled < 0) > - conf->wmm_enabled = hapd->iconf->ieee80211n; > + if (!conf->wmm_enabled && (hapd->iconf->ieee80211n || > hapd->iconf->ieee80211ac)) { > + wpa_printf(MSG_ERROR, "HT/VHT is enabled so WMM cannot be disabled"); > + return -1; > + } This looks quite broken. conf->wmm_enabled is initialized to -1 in hostapd_config_default_bss(). If there is no explicit wmm_enabled parameter in the configuration, this !conf->wmm_enabled would not be true and as such, the default value (-1) would be left in place and that would be interpreted as WMM being enabled by default regardless of whether HT or VHT are being used.
Hi Jouni, I forgot to include patch 4 which makes that very change. I have now sent that. Sorry. The 802.11n, ac and ax specs expect devices to support 802.11e QoS in order to use HT/VHT/HE rates. This is because the TID field is key to aggregation mechanisms, including block ACKs, that enable high throughput rates. Where WMM is configured to be disabled and HT/VHT/HE support are configured hostapd should fail to start therefore so that this explicitly fails and the configuration can be corrected. We could silently enable WMM where HT/VHT/HE support are configured and a WMM is not defined. I think, however, that it is cleaner and more logically consistent to require it to be explicitly enabled in that case. Thanks, Nick
On Sat, Feb 25, 2017 at 03:13:40PM +0000, Nick Lowe wrote: > The 802.11n, ac and ax specs expect devices to support 802.11e QoS in > order to use HT/VHT/HE rates. This is because the TID field is key to > aggregation mechanisms, including block ACKs, that enable high > throughput rates. WMM != 802.11e QoS > Where WMM is configured to be disabled and HT/VHT/HE support are > configured hostapd should fail to start therefore so that this > explicitly fails and the configuration can be corrected. > > We could silently enable WMM where HT/VHT/HE support are configured > and a WMM is not defined. I think, however, that it is cleaner and > more logically consistent to require it to be explicitly enabled in > that case. This would just make it more difficult for users without any clear benefit. At most, the check could verify that wmm_enabled=0 is not explicitly used with HT/VHT/HE. That said, I still do not see why hostapd would need to explicitly mandate WMM to be enabled. IEEE 802.11 QoS is fine for VHT/HE, no need for WMM there. In addition, if other HT/VHT/HE functionality works without QoS and only subset of functionality needs to be disable (e.g., everything related to aggregation), it is not clear why this possibility should be prevented. If for no other use, this could be of some use for testing purposes.
diff --git a/hostapd/hostapd.conf b/hostapd/hostapd.conf index c4eebff..89e61a1 100644 --- a/hostapd/hostapd.conf +++ b/hostapd/hostapd.conf @@ -516,7 +516,7 @@ wmm_ac_vo_acm=0 # ieee80211n: Whether IEEE 802.11n (HT) is enabled # 0 = disabled (default) # 1 = enabled -# Note: You will also need to enable WMM for full HT functionality. +# Note: You will also need to enable WMM for HT functionality. # Note: hw_mode=g (2.4 GHz) and hw_mode=a (5 GHz) is used to specify the band. #ieee80211n=1 @@ -570,7 +570,7 @@ wmm_ac_vo_acm=0 # ieee80211ac: Whether IEEE 802.11ac (VHT) is enabled # 0 = disabled (default) # 1 = enabled -# Note: You will also need to enable WMM for full VHT functionality. +# Note: You will also need to enable WMM for VHT functionality. # Note: hw_mode=a is used to specify that 5 GHz band is used with VHT. #ieee80211ac=1 diff --git a/src/ap/hostapd.c b/src/ap/hostapd.c index cf8a8cb..023c141 100644 --- a/src/ap/hostapd.c +++ b/src/ap/hostapd.c @@ -960,8 +960,10 @@ static int hostapd_setup_bss(struct hostapd_data *hapd, int first) os_memcpy(hapd->own_addr, if_addr, ETH_ALEN); } - if (conf->wmm_enabled < 0) - conf->wmm_enabled = hapd->iconf->ieee80211n; + if (!conf->wmm_enabled && (hapd->iconf->ieee80211n || hapd->iconf->ieee80211ac)) { + wpa_printf(MSG_ERROR, "HT/VHT is enabled so WMM cannot be disabled"); + return -1; + }
Explicitly require WMM to be enabled if HT (11n) or VHT (11ac) is enabled. This adds a check where VHT is enabled that was previously missing. WMM would previously silently be enabled where HT was enabled. Signed-off-by: Nick Lowe <nick.lowe@gmail.com> --- hostapd/hostapd.conf | 4 ++-- src/ap/hostapd.c | 6 ++++-- 2 files changed, 6 insertions(+), 4 deletions(-) #ifdef CONFIG_IEEE80211R_AP if (is_zero_ether_addr(conf->r1_key_holder))