| Message ID | 1485492199-8182-1-git-send-email-masashi.honma@gmail.com |
|---|---|
| State | Accepted |
| Headers | show |
On Fri, Jan 27, 2017 at 01:43:19PM +0900, Masashi Honma wrote: > The remote mesh STA which had configuration disable_ht40=1 could have > HT Capabilities IE which includes Supported Channel Width Set = 1 (both > 20 MHz and 40 MHz operation is supported) even though it had HT > Operation IE which includes STA Channel Width = 0 (20 MHz channel width > only). Previously, local peer recognized such a remote peer as 40MHz > band width enabled STA because local peer only checked HT Capabilities > IE. This could cause disconnection between disable_ht40=1 mesh STA and > disable_ht40=0 mesh STA (they could be established but could not ping > with ath9k_htc device). This patch fixes the issue by refering HT > Operation IE. Thanks, applied with some cleanup and fixes. I split this into two commits to keep the actual change in mesh_mpm.c clearer. It looks like there are other issues with HT/VHT overrides. I fixed local channel configuration for VHT capable hardware with disable_vht=1. However, even with all such issues fixed in wpa_supplicant, the Beacon frames from mesh STAs seemed to be going out with HT/VHT capabilities based on hardware/driver capabilities instead of constraint capabilities after the HT/VHT overrides are applied. I guess this would be a mac80211 change (though, didn't look at any details there) to update or remove the HT/VHT elements from Beacon frames in some cases. > +void set_disable_ht40(struct ieee80211_ht_capabilities *htcaps, > + int disabled); > diff --git a/wpa_supplicant/wpa_supplicant.c b/wpa_supplicant/wpa_supplicant.c > @@ -3857,18 +3857,10 @@ static int wpa_set_disable_ht40(struct wpa_supplicant *wpa_s, > - /* Masking these out disables HT40 */ > - le16 msk = host_to_le16(HT_CAP_INFO_SUPP_CHANNEL_WIDTH_SET | > - HT_CAP_INFO_SHORT_GI40MHZ); > - > wpa_msg(wpa_s, MSG_DEBUG, "set_disable_ht40: %d", disabled); > > - if (disabled) > - htcaps->ht_capabilities_info &= ~msk; > - else > - htcaps->ht_capabilities_info |= msk; > - > - htcaps_mask->ht_capabilities_info |= msk; > + set_disable_ht40(htcaps, disabled); > + set_disable_ht40(htcaps_mask, 1); This does not look correct for htcaps_mask. That disabled = 1 would get the mask cleared, not added which is this old "|= msk" behavior here. I changed that to use disabled = 0 instead to maintain previous behavior of wpa_set_disable_ht40().
On 2017/01/30 09:05, Jouni Malinen wrote: > > Thanks, applied with some cleanup and fixes. I split this into two > commits to keep the actual change in mesh_mpm.c clearer. Thanks ! > > It looks like there are other issues with HT/VHT overrides. I fixed > local channel configuration for VHT capable hardware with disable_vht=1. > However, even with all such issues fixed in wpa_supplicant, the Beacon > frames from mesh STAs seemed to be going out with HT/VHT capabilities > based on hardware/driver capabilities instead of constraint capabilities > after the HT/VHT overrides are applied. I guess this would be a mac80211 > change (though, didn't look at any details there) to update or remove > the HT/VHT elements from Beacon frames in some cases. Yes, I am trying to sanitize these... > >> +void set_disable_ht40(struct ieee80211_ht_capabilities *htcaps, >> + int disabled); > >> diff --git a/wpa_supplicant/wpa_supplicant.c b/wpa_supplicant/wpa_supplicant.c >> @@ -3857,18 +3857,10 @@ static int wpa_set_disable_ht40(struct wpa_supplicant *wpa_s, > >> - /* Masking these out disables HT40 */ >> - le16 msk = host_to_le16(HT_CAP_INFO_SUPP_CHANNEL_WIDTH_SET | >> - HT_CAP_INFO_SHORT_GI40MHZ); >> - >> wpa_msg(wpa_s, MSG_DEBUG, "set_disable_ht40: %d", disabled); >> >> - if (disabled) >> - htcaps->ht_capabilities_info &= ~msk; >> - else >> - htcaps->ht_capabilities_info |= msk; >> - >> - htcaps_mask->ht_capabilities_info |= msk; >> + set_disable_ht40(htcaps, disabled); >> + set_disable_ht40(htcaps_mask, 1); > > This does not look correct for htcaps_mask. That disabled = 1 would get > the mask cleared, not added which is this old "|= msk" behavior here. I > changed that to use disabled = 0 instead to maintain previous behavior > of wpa_set_disable_ht40(). Yes, this is a just my mistake ! Thanks ! Masashi Honma.
diff --git a/src/common/hw_features_common.c b/src/common/hw_features_common.c index 9c37ea6..b1eb995 100644 --- a/src/common/hw_features_common.c +++ b/src/common/hw_features_common.c @@ -453,3 +453,16 @@ int hostapd_set_freq_params(struct hostapd_freq_params *data, return 0; } + +void set_disable_ht40(struct ieee80211_ht_capabilities *htcaps, + int disabled) +{ + /* Masking these out disables HT40 */ + le16 msk = host_to_le16(HT_CAP_INFO_SUPP_CHANNEL_WIDTH_SET | + HT_CAP_INFO_SHORT_GI40MHZ); + + if (disabled) + htcaps->ht_capabilities_info &= ~msk; + else + htcaps->ht_capabilities_info |= msk; +} diff --git a/src/common/hw_features_common.h b/src/common/hw_features_common.h index 7360b4e..234b7bf 100644 --- a/src/common/hw_features_common.h +++ b/src/common/hw_features_common.h @@ -35,5 +35,7 @@ int hostapd_set_freq_params(struct hostapd_freq_params *data, int vht_enabled, int sec_channel_offset, int vht_oper_chwidth, int center_segment0, int center_segment1, u32 vht_caps); +void set_disable_ht40(struct ieee80211_ht_capabilities *htcaps, + int disabled); #endif /* HW_FEATURES_COMMON_H */ diff --git a/wpa_supplicant/mesh_mpm.c b/wpa_supplicant/mesh_mpm.c index 6c3fa14..39a4ee8 100644 --- a/wpa_supplicant/mesh_mpm.c +++ b/wpa_supplicant/mesh_mpm.c @@ -11,6 +11,7 @@ #include "utils/common.h" #include "utils/eloop.h" #include "common/ieee802_11_defs.h" +#include "common/hw_features_common.h" #include "ap/hostapd.h" #include "ap/sta_info.h" #include "ap/ieee802_11.h" @@ -646,6 +647,9 @@ static struct sta_info * mesh_mpm_add_peer(struct wpa_supplicant *wpa_s, struct mesh_conf *conf = wpa_s->ifmsh->mconf; struct hostapd_data *data = wpa_s->ifmsh->bss[0]; struct sta_info *sta; +#ifdef CONFIG_IEEE80211N + struct ieee80211_ht_operation *oper; +#endif /* CONFIG_IEEE80211N */ int ret; if (elems->mesh_config_len >= 7 && @@ -677,6 +681,16 @@ static struct sta_info * mesh_mpm_add_peer(struct wpa_supplicant *wpa_s, #ifdef CONFIG_IEEE80211N copy_sta_ht_capab(data, sta, elems->ht_capabilities); + + oper = (struct ieee80211_ht_operation *)elems->ht_operation; + if (oper && + !(oper->ht_param & HT_INFO_HT_PARAM_STA_CHNL_WIDTH)) { + wpa_msg(wpa_s, MSG_DEBUG, MACSTR + " does not supports 40MHz band width", + MAC2STR(sta->addr)); + set_disable_ht40(sta->ht_capabilities, 1); + } + update_ht_state(data, sta); #endif /* CONFIG_IEEE80211N */ diff --git a/wpa_supplicant/wpa_supplicant.c b/wpa_supplicant/wpa_supplicant.c index bc59c67..6a3e02a 100644 --- a/wpa_supplicant/wpa_supplicant.c +++ b/wpa_supplicant/wpa_supplicant.c @@ -3857,18 +3857,10 @@ static int wpa_set_disable_ht40(struct wpa_supplicant *wpa_s, struct ieee80211_ht_capabilities *htcaps_mask, int disabled) { - /* Masking these out disables HT40 */ - le16 msk = host_to_le16(HT_CAP_INFO_SUPP_CHANNEL_WIDTH_SET | - HT_CAP_INFO_SHORT_GI40MHZ); - wpa_msg(wpa_s, MSG_DEBUG, "set_disable_ht40: %d", disabled); - if (disabled) - htcaps->ht_capabilities_info &= ~msk; - else - htcaps->ht_capabilities_info |= msk; - - htcaps_mask->ht_capabilities_info |= msk; + set_disable_ht40(htcaps, disabled); + set_disable_ht40(htcaps_mask, 1); return 0; }
The remote mesh STA which had configuration disable_ht40=1 could have HT Capabilities IE which includes Supported Channel Width Set = 1 (both 20 MHz and 40 MHz operation is supported) even though it had HT Operation IE which includes STA Channel Width = 0 (20 MHz channel width only). Previously, local peer recognized such a remote peer as 40MHz band width enabled STA because local peer only checked HT Capabilities IE. This could cause disconnection between disable_ht40=1 mesh STA and disable_ht40=0 mesh STA (they could be established but could not ping with ath9k_htc device). This patch fixes the issue by refering HT Operation IE. Signed-off-by: Masashi Honma <masashi.honma@gmail.com> --- src/common/hw_features_common.c | 13 +++++++++++++ src/common/hw_features_common.h | 2 ++ wpa_supplicant/mesh_mpm.c | 14 ++++++++++++++ wpa_supplicant/wpa_supplicant.c | 12 ++---------- 4 files changed, 31 insertions(+), 10 deletions(-)