| Message ID | VE1PR04MB6431889C6886D23AE655549D8D579@VE1PR04MB6431.eurprd04.prod.outlook.com |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | [EXT,RESEND] Issue: Non-HE capable interface advertising HE | expand |
On Thu, Sep 29, 2022 at 10:41:53AM +0000, Deepti Panchal wrote: > This mail describes the purpose of the attached "hostapd" patch. > The patch avoids issue when there is mismatch between user's 11ax config and DRV capability > > Behavior (without the patch): > ========================== > Non-HE capable interface advertising HE IEs/Capabilities in Beacon/Probe Response/Association Response frames if "ieee80211ax=1" configuration is used > > Patch Description: > ================ > 1. The HE capability for an AP iface was read from the driver, but was not used to update the "drv_flags" for the interface > 2. Change is now added to update the HE capability in the "drv_flags" of the interface > 3. If the interface does not support HE capability, the "iface->conf->ieee80211ax" flag is set to 0 > 4. Now Beacons/Probe response/Association Response will advertise HE capabilities ONLY if the interface is HE capable So this would make hostapd ignore what the configuration is trying it to do? That does not sound correct behavior, i.e., I would have expected the configuration to be rejected instead of being silently ignored. In any case, I cannot really consider applying this patch without the Signed-off-by: line in the commit message as described in the CONTRIBUTIONS file.
Hi Jouni, My patch will override the user config in case the hardware does not support HE configuration. An error message will also be shown in the debug logs indicating the hardware is not HE supported. Can you please elaborate on what you mean by: "I would have expected the configuration to be rejected instead of being silently ignored" Please find the modified patch with the "Signed-off" line: ====================================================================================== From 1c6e7696df5664b86a5ce16631e04801fe5907a6 Mon Sep 17 00:00:00 2001 From: Deepti Panchal <deepti.panchal@nxp.com> Date: Mon, 18 Jul 2022 15:59:10 +0530 Subject: [PATCH] Issue: Non-HE capable interface advertising HE IEs/Capabilities in Beacon/Probe Response/Association Response frames if "ieee80211ax=1" configuration is used Change Description: 1. The HE capability for an AP iface was read from the driver, but was not used to update the "drv_flags" for the iface 2. Change is now added to update the HE capability in the "drv_flags" of the interface 3. If the interface does not support HE capability, the "iface->conf->ieee80211ax" flag is set to 0 4. Hence now Beacons/Probe response/Association Response will advertise HE capabilities ONLY if the interface is HE capable Signed-off-by: Deepti Panchal <deepti.panchal@nxp.com> --- src/ap/hw_features.c | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/src/ap/hw_features.c b/src/ap/hw_features.c index ed5ff41d3..87d7b52bd 100644 --- a/src/ap/hw_features.c +++ b/src/ap/hw_features.c @@ -679,7 +679,19 @@ static int ieee80211ac_supported_vht_capab(struct hostapd_iface *iface) #ifdef CONFIG_IEEE80211AX static int ieee80211ax_supported_he_capab(struct hostapd_iface *iface) { - return 1; + struct hostapd_hw_modes *mode = iface->current_mode; + struct he_capabilities *he_cap = &(mode->he_capab[IEEE80211_MODE_AP]); + + /* Check iface HE capability, update drv_flags and HE config setting */ + if (he_cap && he_cap->he_supported) { + wpa_printf(MSG_DEBUG, "iface hw he_supported: %d", he_cap->he_supported); + iface->drv_flags |= WPA_DRIVER_FLAGS_HE_CAPABILITIES; + } else { + wpa_printf(MSG_DEBUG, "iface does not support HE capability"); + iface->conf->ieee80211ax = 0; + } + + return 1; } #endif /* CONFIG_IEEE80211AX */ ================================================================================== Thanks, Deepti -----Original Message----- From: Jouni Malinen <j@w1.fi> Sent: Saturday, November 5, 2022 19:57 To: Deepti Panchal <deepti.panchal@nxp.com> Cc: hostap@lists.infradead.org Subject: Re: [EXT] [RESEND PATCH] Issue: Non-HE capable interface advertising HE Caution: EXT Email On Thu, Sep 29, 2022 at 10:41:53AM +0000, Deepti Panchal wrote: > This mail describes the purpose of the attached "hostapd" patch. > The patch avoids issue when there is mismatch between user's 11ax > config and DRV capability > > Behavior (without the patch): > ========================== > Non-HE capable interface advertising HE IEs/Capabilities in > Beacon/Probe Response/Association Response frames if "ieee80211ax=1" > configuration is used > > Patch Description: > ================ > 1. The HE capability for an AP iface was read from the driver, but was not used to update the "drv_flags" for the interface > 2. Change is now added to update the HE capability in the "drv_flags" of the interface > 3. If the interface does not support HE capability, the "iface->conf->ieee80211ax" flag is set to 0 > 4. Now Beacons/Probe response/Association Response will advertise > HE capabilities ONLY if the interface is HE capable So this would make hostapd ignore what the configuration is trying it to do? That does not sound correct behavior, i.e., I would have expected the configuration to be rejected instead of being silently ignored. In any case, I cannot really consider applying this patch without the Signed-off-by: line in the commit message as described in the CONTRIBUTIONS file. -- Jouni Malinen PGP id EFC895FA
Hi Jouni, Can you please review my comments below ? Thanks, Deepti -----Original Message----- From: Hostap <hostap-bounces@lists.infradead.org> On Behalf Of Deepti Panchal Sent: Monday, November 14, 2022 18:04 To: Jouni Malinen <j@w1.fi> Cc: hostap@lists.infradead.org Subject: RE: [EXT] [RESEND PATCH] Issue: Non-HE capable interface advertising HE Caution: EXT Email Hi Jouni, My patch will override the user config in case the hardware does not support HE configuration. An error message will also be shown in the debug logs indicating the hardware is not HE supported. Can you please elaborate on what you mean by: "I would have expected the configuration to be rejected instead of being silently ignored" Please find the modified patch with the "Signed-off" line: ====================================================================================== From 1c6e7696df5664b86a5ce16631e04801fe5907a6 Mon Sep 17 00:00:00 2001 From: Deepti Panchal <deepti.panchal@nxp.com> Date: Mon, 18 Jul 2022 15:59:10 +0530 Subject: [PATCH] Issue: Non-HE capable interface advertising HE IEs/Capabilities in Beacon/Probe Response/Association Response frames if "ieee80211ax=1" configuration is used Change Description: 1. The HE capability for an AP iface was read from the driver, but was not used to update the "drv_flags" for the iface 2. Change is now added to update the HE capability in the "drv_flags" of the interface 3. If the interface does not support HE capability, the "iface->conf->ieee80211ax" flag is set to 0 4. Hence now Beacons/Probe response/Association Response will advertise HE capabilities ONLY if the interface is HE capable Signed-off-by: Deepti Panchal <deepti.panchal@nxp.com> --- src/ap/hw_features.c | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/src/ap/hw_features.c b/src/ap/hw_features.c index ed5ff41d3..87d7b52bd 100644 --- a/src/ap/hw_features.c +++ b/src/ap/hw_features.c @@ -679,7 +679,19 @@ static int ieee80211ac_supported_vht_capab(struct hostapd_iface *iface) #ifdef CONFIG_IEEE80211AX static int ieee80211ax_supported_he_capab(struct hostapd_iface *iface) { - return 1; + struct hostapd_hw_modes *mode = iface->current_mode; + struct he_capabilities *he_cap = + &(mode->he_capab[IEEE80211_MODE_AP]); + + /* Check iface HE capability, update drv_flags and HE config setting */ + if (he_cap && he_cap->he_supported) { + wpa_printf(MSG_DEBUG, "iface hw he_supported: %d", he_cap->he_supported); + iface->drv_flags |= WPA_DRIVER_FLAGS_HE_CAPABILITIES; + } else { + wpa_printf(MSG_DEBUG, "iface does not support HE capability"); + iface->conf->ieee80211ax = 0; + } + + return 1; } #endif /* CONFIG_IEEE80211AX */ ================================================================================== Thanks, Deepti -----Original Message----- From: Jouni Malinen <j@w1.fi> Sent: Saturday, November 5, 2022 19:57 To: Deepti Panchal <deepti.panchal@nxp.com> Cc: hostap@lists.infradead.org Subject: Re: [EXT] [RESEND PATCH] Issue: Non-HE capable interface advertising HE Caution: EXT Email On Thu, Sep 29, 2022 at 10:41:53AM +0000, Deepti Panchal wrote: > This mail describes the purpose of the attached "hostapd" patch. > The patch avoids issue when there is mismatch between user's 11ax > config and DRV capability > > Behavior (without the patch): > ========================== > Non-HE capable interface advertising HE IEs/Capabilities in > Beacon/Probe Response/Association Response frames if "ieee80211ax=1" > configuration is used > > Patch Description: > ================ > 1. The HE capability for an AP iface was read from the driver, but was not used to update the "drv_flags" for the interface > 2. Change is now added to update the HE capability in the "drv_flags" of the interface > 3. If the interface does not support HE capability, the "iface->conf->ieee80211ax" flag is set to 0 > 4. Now Beacons/Probe response/Association Response will advertise > HE capabilities ONLY if the interface is HE capable So this would make hostapd ignore what the configuration is trying it to do? That does not sound correct behavior, i.e., I would have expected the configuration to be rejected instead of being silently ignored. In any case, I cannot really consider applying this patch without the Signed-off-by: line in the commit message as described in the CONTRIBUTIONS file. -- Jouni Malinen PGP id EFC895FA
On Mon, Nov 14, 2022 at 12:33:38PM +0000, Deepti Panchal wrote: > My patch will override the user config in case the hardware does not support HE configuration. > An error message will also be shown in the debug logs indicating the hardware is not HE supported. > > Can you please elaborate on what you mean by: > "I would have expected the configuration to be rejected instead of being silently ignored" I would expect hostapd to refuse to start AP operation with invalid configuration instead of modifying the configuration in this manner to ignore that the configuration requested HE to be enabled.
Hi Jouni, Rather than refusing to start the AP, my hostapd patch would throw an error message indicating that HE configuration is not supported and would instead start the AP in HT/VHT configuration. This would result in a seamless end user experience. Does this approach seem to be fair ? Thanks, Deepti -----Original Message----- From: Jouni Malinen <j@w1.fi> Sent: Monday, December 19, 2022 00:09 To: Deepti Panchal <deepti.panchal@nxp.com> Cc: hostap@lists.infradead.org Subject: Re: [EXT] [RESEND PATCH] Issue: Non-HE capable interface advertising HE Caution: EXT Email On Mon, Nov 14, 2022 at 12:33:38PM +0000, Deepti Panchal wrote: > My patch will override the user config in case the hardware does not support HE configuration. > An error message will also be shown in the debug logs indicating the hardware is not HE supported. > > Can you please elaborate on what you mean by: > "I would have expected the configuration to be rejected instead of being silently ignored" I would expect hostapd to refuse to start AP operation with invalid configuration instead of modifying the configuration in this manner to ignore that the configuration requested HE to be enabled. -- Jouni Malinen PGP id EFC895FA
On 25.12.22 17:27, Deepti Panchal wrote: > Hi Jouni, > > Rather than refusing to start the AP, my hostapd patch would throw an error message indicating that HE configuration is not supported and would instead start the AP in HT/VHT configuration. > This would result in a seamless end user experience. > > Does this approach seem to be fair ? > > Thanks, > Deepti > Hi I personally and professionally would not like something like that. When i configure something i expect it to be exactly what i configured. Auto-magically changing what is running to something that is not specified just leads to unexpected surprises later on... BR Matthias PS: If you want to hide config errors from your endusers, why don't you adjust the code that generated the config for hostapd to auto-magically mangle the hostapd-config?
diff --git a/src/ap/hw_features.c b/src/ap/hw_features.c index ed5ff41d3..87d7b52bd 100644 --- a/src/ap/hw_features.c +++ b/src/ap/hw_features.c @@ -679,7 +679,19 @@ static int ieee80211ac_supported_vht_capab(struct hostapd_iface *iface) #ifdef CONFIG_IEEE80211AX static int ieee80211ax_supported_he_capab(struct hostapd_iface *iface) { - return 1; + struct hostapd_hw_modes *mode = iface->current_mode; + struct he_capabilities *he_cap = + &(mode->he_capab[IEEE80211_MODE_AP]); + + /* Check iface HE capability, update drv_flags and HE config setting */ + if (he_cap && he_cap->he_supported) { + wpa_printf(MSG_DEBUG, "iface hw he_supported: %d", he_cap->he_supported); + iface->drv_flags |= WPA_DRIVER_FLAGS_HE_CAPABILITIES; + } else { + wpa_printf(MSG_DEBUG, "iface does not support HE capability"); + iface->conf->ieee80211ax = 0; + } + + return 1; } #endif /* CONFIG_IEEE80211AX */ Thanks, Deepti
This mail describes the purpose of the attached "hostapd" patch. The patch avoids issue when there is mismatch between user's 11ax config and DRV capability Behavior (without the patch): ========================== Non-HE capable interface advertising HE IEs/Capabilities in Beacon/Probe Response/Association Response frames if "ieee80211ax=1" configuration is used Patch Description: ================ 1. The HE capability for an AP iface was read from the driver, but was not used to update the "drv_flags" for the interface 2. Change is now added to update the HE capability in the "drv_flags" of the interface 3. If the interface does not support HE capability, the "iface->conf->ieee80211ax" flag is set to 0 4. Now Beacons/Probe response/Association Response will advertise HE capabilities ONLY if the interface is HE capable Patch Verification: ================= 1. For Intel and NXP non-11ax/HE capable chip/interface, when "ieee80211ax=1" is used in the config file, NO HE IEs are added in the Beacon/Probe Response/Association Response frames 2. For NXP 11ax/HE capable chips, the HE IEs get added correctly to the required MGMT frames ============================================================================================================== From e19bf318ce2a27ccc03f72023b2fc7f1bf6cd496 Mon Sep 17 00:00:00 2001 From: Deepti Panchal <deepti.panchal@nxp.com> Date: Mon, 18 Jul 2022 15:59:10 +0530 Subject: [PATCH] Issue: Non-HE capable interface advertising HE IEs/Capabilities in Beacon/Probe Response/Association Response frames if "ieee80211ax=1" configuration is used Change Description: 1. The HE capability for an AP iface was read from the driver, but was not used to update the "drv_flags" for the iface 2. Change is now added to update the HE capability in the "drv_flags" of the interface 3. If the interface does not support HE capability, the "iface->conf->ieee80211ax" flag is set to 0 4. Hence now Beacons/Probe response/Association Response will advertise HE capabilities ONLY if the interface is HE capable --- src/ap/hw_features.c | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-)