Re: [PATCH V4 0/8] HE: add MLME and channel management support

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Mon, May 20, 2019 at 09:55:03AM +0200, John Crispin wrote:
> This series
> * adds HE support for MLME
> * adds HE channel management
> * make the HE cap IE dynamically sized
> * add PPET support to the HE cap IE
> * fixes HE Beamforming bits
> 
> John Crispin (8):
>   HE: make the basic NSS/MCS configurable
>   HE: add MLME handling for HE stations
>   HE: remove VHT_ prefix from CHANWITDH_* define
>   HE: add helpers for getting the channel width parameters
>   HE: add HE channel management configuration options
>   HE: add HE to support to channel management
>   HE: various beacon fixes
>   HE: mask out the beamforming capabilities if they are not configured

Thanks, I applied most of this with significant cleanup and number of
fixes. Please note that it would be much nicer if the patches were split
into smaller parts (I ended up splitting those to 24 separate commits to
make reviewing easier) and especially, if the patches would not mix
together renaming of variables and adding new functionality.
Furthermore, the patches in the same series should not introduce issues
that are fixed in latter patches in the same series.

Most of the errors were copy-paste type of issues where seg0 was set to
value that was supposed to go to seg1 or the other way around, and one
issue was in deleting a line accidentally when replacing surrounding
lines with helper function calls (which showed up in a hwsim test case
regression).

The part that I could not fully understand and as such, could not fix,
is part of the "HE: add HE to support to channel management" patch. The
changes in hostapd_set_freq_params() cause regressions (e.g., break
ap_vht80_to_24g_ht and also he_open for that matter) by preventing
hostapd from starting AP mode.

Could you please clarify what the hostapd_set_freq_params() changes are
supposed to do and why do they change behavior for cases when HE is
disabled? The remaining changes (i.e., things that I did not apply in
some form) are as follows:

From: John Crispin <john@xxxxxxxxxxx>
Subject: [PATCH] HE: Operating frequency configuration

Verify that the chwidth/mode is supported by the hardware by checking
the capabilities.

Signed-off-by: Shashidhar Lakkavalli <slakkavalli@xxxxxxxxx>
Signed-off-by: John Crispin <john@xxxxxxxxxxx>
---
 src/common/hw_features_common.c | 69 +++++++++++++++++++++++++++------
 1 file changed, 58 insertions(+), 11 deletions(-)

diff --git a/src/common/hw_features_common.c b/src/common/hw_features_common.c
index 3fdbf893d..87f22ab6c 100644
--- a/src/common/hw_features_common.c
+++ b/src/common/hw_features_common.c
@@ -381,13 +381,44 @@ int hostapd_set_freq_params(struct hostapd_freq_params *data,
 	data->center_freq2 = 0;
 	data->bandwidth = sec_channel_offset ? 40 : 20;
 
-	if (data->vht_enabled) switch (oper_chwidth) {
+	if (data->he_enabled) switch (oper_chwidth) {
 	case CHANWIDTH_USE_HT:
-		if (center_segment1 ||
-		    (center_segment0 != 0 &&
-		     5000 + center_segment0 * 5 != data->center_freq1 &&
-		     2407 + center_segment0 * 5 != data->center_freq1))
+		if (mode < HOSTAPD_MODE_IEEE80211A) {
+			if (!(he_cap->phy_cap[HE_PHYCAP_CHANNEL_WIDTH_SET_IDX] &
+			      HE_PHYCAP_CHANNEL_WIDTH_SET_40MHZ_IN_2G)) {
+				wpa_printf(MSG_ERROR,
+					   "40 channel width is not supported!");
+				return -1;
+			}
+			break;
+		}
+		/* fall through */
+	case CHANWIDTH_80MHZ:
+		if (!(he_cap->phy_cap[HE_PHYCAP_CHANNEL_WIDTH_SET_IDX] &
+		      HE_PHYCAP_CHANNEL_WIDTH_SET_40MHZ_80MHZ_IN_5G)) {
+			wpa_printf(MSG_ERROR,
+				   "40/80 channel width is not supported!");
+			return -1;
+		}
+		break;
+	case CHANWIDTH_80P80MHZ:
+		if (!(he_cap->phy_cap[HE_PHYCAP_CHANNEL_WIDTH_SET_IDX] &
+		      HE_PHYCAP_CHANNEL_WIDTH_SET_80PLUS80MHZ_IN_5G)) {
+			wpa_printf(MSG_ERROR,
+				   "80+80 channel width is not supported!");
 			return -1;
+		}
+		break;
+	case CHANWIDTH_160MHZ:
+		if (!(he_cap->phy_cap[HE_PHYCAP_CHANNEL_WIDTH_SET_IDX] &
+		      HE_PHYCAP_CHANNEL_WIDTH_SET_160MHZ_IN_5G)) {
+			wpa_printf(MSG_ERROR,
+				   "160 channel width is not supported!");
+			return -1;
+		}
+		break;
+	} else if (data->vht_enabled) switch (oper_chwidth) {
+	case CHANWIDTH_USE_HT:
 		break;
 	case CHANWIDTH_80P80MHZ:
 		if (!(vht_caps & VHT_CAP_SUPP_CHAN_WIDTH_160_80PLUS80MHZ)) {
@@ -395,6 +426,28 @@ int hostapd_set_freq_params(struct hostapd_freq_params *data,
 				   "80+80 channel width is not supported!");
 			return -1;
 		}
+		/* fall through */
+	case CHANWIDTH_80MHZ:
+		break;
+	case CHANWIDTH_160MHZ:
+		if (!(vht_caps & (VHT_CAP_SUPP_CHAN_WIDTH_160MHZ |
+				  VHT_CAP_SUPP_CHAN_WIDTH_160_80PLUS80MHZ))) {
+			wpa_printf(MSG_ERROR,
+				   "160MHZ channel width is not supported!");
+			return -1;
+		}
+		break;
+	}
+
+	switch (oper_chwidth) {
+	case CHANWIDTH_USE_HT:
+		if (center_segment1 ||
+		    (center_segment0 != 0 &&
+		     5000 + center_segment0 * 5 != data->center_freq1 &&
+		     2407 + center_segment0 * 5 != data->center_freq1))
+			return -1;
+		break;
+	case CHANWIDTH_80P80MHZ:
 		if (center_segment1 == center_segment0 + 4 ||
 		    center_segment1 == center_segment0 - 4)
 			return -1;
@@ -439,12 +492,6 @@ int hostapd_set_freq_params(struct hostapd_freq_params *data,
 		break;
 	case CHANWIDTH_160MHZ:
 		data->bandwidth = 160;
-		if (!(vht_caps & (VHT_CAP_SUPP_CHAN_WIDTH_160MHZ |
-				  VHT_CAP_SUPP_CHAN_WIDTH_160_80PLUS80MHZ))) {
-			wpa_printf(MSG_ERROR,
-				   "160MHZ channel width is not supported!");
-			return -1;
-		}
 		if (center_segment1)
 			return -1;
 		if (!sec_channel_offset)
-- 
Jouni Malinen                                            PGP id EFC895FA

_______________________________________________
Hostap mailing list
Hostap@xxxxxxxxxxxxxxxxxxx
http://lists.infradead.org/mailman/listinfo/hostap



[Index of Archives]     [Linux Wireless]     [Linux Kernel]     [ATH6KL]     [Linux Bluetooth]     [Linux Netdev]     [Kernel Newbies]     [IDE]     [Security]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux ATA RAID]     [Samba]     [Device Mapper]

  Powered by Linux