[PATCH v2,1/2] wifi: mac80211: Add utilities for converting op_class

[PATCH v2,1/2] wifi: mac80211: Add utilities for converting op_class

[Michael-CY Lee]

These utilities include converting op_class to nl80211 channel width and
center frequency.

Signed-off-by: Michael-CY Lee <michael-cy.lee@mediatek.com>
Signed-off-by: Money Wang <money.wang@mediatek.com>
---
v2: fix the build warning
---
 include/net/cfg80211.h |  25 ++++++++
 net/wireless/util.c    | 127 +++++++++++++++++++++++++++++++++++++++++
 2 files changed, 152 insertions(+)

diff --git a/include/net/cfg80211.h b/include/net/cfg80211.h
index b137a33a1b68..a226d1cae7f7 100644
--- a/include/net/cfg80211.h
+++ b/include/net/cfg80211.h
@@ -8669,6 +8669,31 @@ void cfg80211_ch_switch_started_notify(struct net_device *dev,
 bool ieee80211_operating_class_to_band(u8 operating_class,
 				       enum nl80211_band *band);
 
+/**
+ * ieee80211_operating_class_to_center_freq - convert operating class to
+ * center frequency
+ *
+ * @operating_class: the operating class to convert
+ * @chan: the ieee80211_channel to convert
+ * @center_freq1: cneter frequency 1 pointer to fill
+ * @center_freq2: cneter frequency 2 pointer to fill
+ *
+ * Returns %true if the conversion was successful, %false otherwise.
+ */
+bool ieee80211_operating_class_to_center_freq(u8 operating_class,
+					      struct ieee80211_channel *chan,
+					      u32 *center_freq1,
+					      u32 *center_freq2);
+
+/**
+ * ieee80211_operating_class_to_chan_width - convert operating class to
+ * nl80211 channel width
+ *
+ * @operating_class: the operating class to convert
+ */
+enum nl80211_chan_width
+ieee80211_operating_class_to_chan_width(u8 operating_class);
+
 /**
  * ieee80211_chandef_to_operating_class - convert chandef to operation class
  *
diff --git a/net/wireless/util.c b/net/wireless/util.c
index 626b858b4b35..383c9f0897ce 100644
--- a/net/wireless/util.c
+++ b/net/wireless/util.c
@@ -2017,6 +2017,133 @@ bool ieee80211_operating_class_to_band(u8 operating_class,
 }
 EXPORT_SYMBOL(ieee80211_operating_class_to_band);
 
+bool ieee80211_operating_class_to_center_freq(u8 operating_class,
+					      struct ieee80211_channel *chan,
+					      u32 *center_freq1,
+					      u32 *center_freq2)
+{
+	u32 control_freq, offset = 0;
+	enum nl80211_band band;
+
+	control_freq = chan->center_freq;
+	if (!ieee80211_operating_class_to_band(operating_class, &band))
+		return false;
+
+	if (band != chan->band)
+		return false;
+
+	if (control_freq >= 5955)
+		offset = control_freq - 5955;
+	else if (control_freq >= 5745)
+		offset = control_freq - 5745;
+	else if (control_freq >= 5180)
+		offset = control_freq - 5180;
+	offset /= 20;
+
+	*center_freq2 = 0;
+	switch (operating_class) {
+	case 81:  /* 2 GHz band; 20 MHz; channels 1..13 */
+	case 82:  /* 2 GHz band; 20 MHz; channel 14 */
+	case 115: /* 5 GHz band; 20 MHz; channels 36,40,44,48 */
+	case 118: /* 5 GHz band; 20 MHz; channels 52,56,60,64 */
+	case 121: /* 5 GHz band; 20 MHz; channels 100..144 */
+	case 124: /* 5 GHz band; 20 MHz; channels 149,153,157,161 */
+	case 125: /* 5 GHz band; 20 MHz; channels 149..177 */
+	case 131: /* 6 GHz band; 20 MHz; channels 1..233*/
+	case 136: /* 6 GHz band; 20 MHz; channel 2 */
+		*center_freq1 = control_freq;
+		return true;
+	case 83:  /* 2 GHz band; 40 MHz; channels 1..9 */
+	case 116: /* 5 GHz band; 40 MHz; channels 36,44 */
+	case 119: /* 5 GHz band; 40 MHz; channels 52,60 */
+	case 122: /* 5 GHz band; 40 MHz; channels 100,108,116,124,132,140 */
+	case 126: /* 5 GHz band; 40 MHz; channels 149,157,165,173 */
+		*center_freq1 = control_freq + 10;
+		return true;
+	case 84:  /* 2 GHz band; 40 MHz; channels 5..13 */
+	case 117: /* 5 GHz band; 40 MHz; channels 40,48 */
+	case 120: /* 5 GHz band; 40 MHz; channels 56,64 */
+	case 123: /* 5 GHz band; 40 MHz; channels 104,112,120,128,136,144 */
+	case 127: /* 5 GHz band; 40 MHz; channels 153,161,169,177 */
+		*center_freq1 = control_freq - 10;
+		return true;
+	case 132: /* 6 GHz band; 40 MHz; channels 1,5,..,229*/
+		*center_freq1 = control_freq + 10 - (offset & 1) * 20;
+		return true;
+	case 128: /* 5 GHz band; 80 MHz; channels 36..64,100..144,149..177 */
+		*center_freq1 = control_freq + 30 - (offset & 3) * 20;
+		return true;
+	case 130: /* 5 GHz band; 80+80 MHz; channels 36..64,100..144,149..177 */
+		/* TODO How to know the center_freq2 of 80+80 MHz?*/
+		*center_freq1 = 0;
+		return false;
+	case 133: /* 6 GHz band; 80 MHz; channels 1,5,..,229 */
+		*center_freq1 = control_freq + 30 - (offset & 3) * 20;
+		return true;
+	case 129: /* 5 GHz band; 160 MHz; channels 36..64,100..144,149..177 */
+		*center_freq1 = control_freq + 70 - (offset & 7) * 20;
+		return true;
+	case 134: /* 6 GHz band; 160 MHz; channels 1,5,..,229 */
+		*center_freq1 = control_freq + 70 - (offset & 7) * 20;
+		return true;
+	case 135: /* 6 GHz band; 80+80 MHz; channels 1,5,..,229 */
+		/* TODO How to know the center_freq2 of 80+80 MHz?*/
+		*center_freq1 = 0;
+		return false;
+	case 137: /* 6 GHz band; 320 MHz; channels 1,5,..,229 */
+		/* TODO it's 320 MHz-1 or 320 MHz-2 channelization? */
+		*center_freq1 = 0;
+		return false;
+	default:
+		return false;
+	}
+}
+EXPORT_SYMBOL(ieee80211_operating_class_to_center_freq);
+
+enum nl80211_chan_width
+ieee80211_operating_class_to_chan_width(u8 operating_class)
+{
+	switch (operating_class) {
+	case 81:  /* 2 GHz band; 20 MHz; channels 1..13 */
+	case 82:  /* 2 GHz band; 20 MHz; channel 14 */
+	case 115: /* 5 GHz band; 20 MHz; channels 36,40,44,48 */
+	case 118: /* 5 GHz band; 20 MHz; channels 52,56,60,64 */
+	case 121: /* 5 GHz band; 20 MHz; channels 100..144 */
+	case 124: /* 5 GHz band; 20 MHz; channels 149,153,157,161 */
+	case 125: /* 5 GHz band; 20 MHz; channels 149..177 */
+	case 131: /* 6 GHz band; 20 MHz; channels 1..233*/
+	case 136: /* 6 GHz band; 20 MHz; channel 2 */
+		return NL80211_CHAN_WIDTH_20;
+	case 83:  /* 2 GHz band; 40 MHz; channels 1..9 */
+	case 84:  /* 2 GHz band; 40 MHz; channels 5..13 */
+	case 116: /* 5 GHz band; 40 MHz; channels 36,44 */
+	case 117: /* 5 GHz band; 40 MHz; channels 40,48 */
+	case 119: /* 5 GHz band; 40 MHz; channels 52,60 */
+	case 120: /* 5 GHz band; 40 MHz; channels 56,64 */
+	case 122: /* 5 GHz band; 40 MHz; channels 100,108,116,124,132,140 */
+	case 123: /* 5 GHz band; 40 MHz; channels 104,112,120,128,136,144 */
+	case 126: /* 5 GHz band; 40 MHz; channels 149,157,165,173 */
+	case 127: /* 5 GHz band; 40 MHz; channels 153,161,169,177 */
+	case 132: /* 6 GHz band; 40 MHz; channels 1,5,..,229*/
+		return NL80211_CHAN_WIDTH_40;
+	case 128: /* 5 GHz band; 80 MHz; channels 36..64,100..144,149..177 */
+	case 133: /* 6 GHz band; 80 MHz; channels 1,5,..,229 */
+		return NL80211_CHAN_WIDTH_80;
+	case 130: /* 5 GHz band; 80+80 MHz; channels 36..64,100..144,149..177 */
+	case 135: /* 6 GHz band; 80+80 MHz; channels 1,5,..,229 */
+		return NL80211_CHAN_WIDTH_80P80;
+	case 129: /* 5 GHz band; 160 MHz; channels 36..64,100..144,149..177 */
+	case 134: /* 6 GHz band; 160 MHz; channels 1,5,..,229 */
+		return NL80211_CHAN_WIDTH_160;
+	case 137: /* 6 GHz band; 320 MHz; channels 1,5,..,229 */
+		return NL80211_CHAN_WIDTH_320;
+	default:
+		WARN_ON(1);
+		return NL80211_CHAN_WIDTH_20_NOHT;
+	}
+}
+EXPORT_SYMBOL(ieee80211_operating_class_to_chan_width);
+
 bool ieee80211_chandef_to_operating_class(struct cfg80211_chan_def *chandef,
 					  u8 *op_class)
 {
-- 
2.25.1

[PATCH v2,2/2] wifi: mac80211: Refactor STA CSA parsing flow

[Michael-CY Lee]

The Wi-Fi Standard (IEEE 802.11-2020 9.4.2.160) initially specified that
the Wide Bandwidth Channel Switch (WBCS) IE subfields have same definitions
as the S1G or VHT Operation Information according to the operating band.

However, it did not change the definitions in the amendment for 6 GHz
(IEEE 802.11ax-2021), so the logic remain the same for handling the WBCS
IE even if there is no VHT mode in 6 GHz.

Now the Wi-Fi Standard draft (IEEE P80211be D3.2 9.4.2.159) modifies the
defitions, making the WBCS IE subfields follow the definitions of S1G,
VHT and HE Operation Information in S1G, 5 GHz and 6 GHz band, respectively.

APs in 6 GHz band might use the VHT or HE Operation Information to build
a WBCS IE according to the Wi-Fi Standard they follow. Originally, the STA
just parsed the WBCS IE as VHT Operation Inforamtion, which was wrong if
the AP was actually build the IE by the HE Operation Information.

To avoid the ambiguity, STA should prefer the op_class in the Extended
Channel Switch Announcement (ECSA) IE rathen than the WBCS IE. If the ECSA
IE is not presented in a channel switch to 6 GHz, the STA should be aware
of the possible ambiguity when parsing the WBCS IE.

To derive the correct bandwidtin in use, the STA should check the
combination of new channel width, ccfs0 and ccfs1, see the following
table:

| width|          ccfs1|            ccfs2| new BW in MHz|
---------------------------------------------------------
|     0|    center freq|                0|            40|
|     1|    center freq|                0|            80|
|     1| primary 80 MHz| secondary 80 MHz|         80P80|
|     1| primary 80 MHz|      center freq|           160|
|     1|    center freq|                0|            40|
|     2|    center freq|                0|           160|
|     2|    center freq|                0|            80|
|     3| primary 80 MHz| secondary 80 MHz|         80P80|
|     3| primary 80 MHz|      center freq|           160|

This patch refactors the STA parsing CSA flow:

1. If the BWI IE is not presented while the ECSA IE is presented, this
   patch uses the subfield op_class to build the HT, VHT and HE operation
   in 2.4 GHz, 5 GHz and 6 GHz band, respectively.
2. If the BWI and ECSA IE are NOT presented while the WBCS IE is presented,
   the new channel width is at least 40 MHz.
   Therefore this patch first prepares a 40 MHz chandef, then builds the
   HT, VHT and HE operation in 2 GHz, 5 GHz and 6 GHz band, respectively.
   In the 6 GHz band, this patch checks the combination of new channel
   width, ccfs0 and ccfs1 to build the correct HE operation.
3. From the HT/VHT/HE operation created in step 1 or 2, this patch then
   creates the chandef and assigns it to csa_ie.

Signed-off-by: Michael-CY Lee <michael-cy.lee@mediatek.com>
Signed-off-by: Money Wang <money.wang@mediatek.com>
---
 net/mac80211/spectmgmt.c | 512 ++++++++++++++++++++++++++++++++++-----
 1 file changed, 456 insertions(+), 56 deletions(-)

diff --git a/net/mac80211/spectmgmt.c b/net/mac80211/spectmgmt.c
index 55959b0b24c5..39441a4955eb 100644
--- a/net/mac80211/spectmgmt.c
+++ b/net/mac80211/spectmgmt.c
@@ -19,6 +19,332 @@
 #include "sta_info.h"
 #include "wme.h"
 
+static inline void
+op_class_to_6ghz_he_eht_oper(u8 op_class, struct ieee80211_channel *chan,
+			     struct ieee80211_he_operation *he_oper,
+			     struct ieee80211_eht_operation *eht_oper)
+{
+	u8 new_chan_width;
+	u32 he_oper_params, center_freq1 = 0, center_freq2 = 0;
+	struct ieee80211_he_6ghz_oper *he_6ghz_oper;
+	struct ieee80211_eht_operation_info *eht_oper_info;
+
+	new_chan_width = ieee80211_operating_class_to_chan_width(op_class);
+	if (!ieee80211_operating_class_to_center_freq(op_class, chan,
+						      &center_freq1,
+						      &center_freq2)) {
+		new_chan_width = NL80211_CHAN_WIDTH_20;
+		center_freq1 = chan->center_freq;
+	}
+
+	he_oper_params =
+		u32_encode_bits(1, IEEE80211_HE_OPERATION_6GHZ_OP_INFO);
+	he_oper->he_oper_params = cpu_to_le32(he_oper_params);
+
+	he_6ghz_oper = (struct ieee80211_he_6ghz_oper *)he_oper->optional;
+	he_6ghz_oper->primary =
+		ieee80211_frequency_to_channel(chan->center_freq);
+	he_6ghz_oper->ccfs0 = ieee80211_frequency_to_channel(center_freq1);
+	he_6ghz_oper->ccfs1 = center_freq2 ?
+		ieee80211_frequency_to_channel(center_freq2) : 0;
+
+	switch (new_chan_width) {
+	case NL80211_CHAN_WIDTH_320:
+		/* Cannot derive center frequency of 320 MHZ from op_class
+		 * since it might be 320 Mhz-1 or 320 Mhz-2.
+		 */
+		WARN_ON(1);
+		break;
+	case NL80211_CHAN_WIDTH_160:
+		he_6ghz_oper->ccfs1 = he_6ghz_oper->ccfs0;
+		he_6ghz_oper->ccfs0 += chan->center_freq < center_freq1 ? -8 : 8;
+		fallthrough;
+	case NL80211_CHAN_WIDTH_80P80:
+		he_6ghz_oper->control =
+			IEEE80211_HE_6GHZ_OPER_CTRL_CHANWIDTH_160MHZ;
+		break;
+	case NL80211_CHAN_WIDTH_80:
+		he_6ghz_oper->control =
+			IEEE80211_HE_6GHZ_OPER_CTRL_CHANWIDTH_80MHZ;
+		break;
+	case NL80211_CHAN_WIDTH_40:
+		he_6ghz_oper->control =
+			IEEE80211_HE_6GHZ_OPER_CTRL_CHANWIDTH_40MHZ;
+		break;
+	default:
+		he_6ghz_oper->control =
+			IEEE80211_HE_6GHZ_OPER_CTRL_CHANWIDTH_20MHZ;
+		break;
+	}
+
+	eht_oper->params = IEEE80211_EHT_OPER_INFO_PRESENT;
+
+	eht_oper_info =
+		(struct ieee80211_eht_operation_info *)eht_oper->optional;
+	eht_oper_info->control = he_6ghz_oper->control;
+	eht_oper_info->ccfs0 = he_6ghz_oper->ccfs0;
+	eht_oper_info->ccfs1 = he_6ghz_oper->ccfs1;
+}
+
+static inline void
+wbcs_ie_to_6ghz_he_eht_oper(const struct ieee80211_wide_bw_chansw_ie *wbcs_ie,
+			    u8 new_chan_no,
+			    struct ieee80211_he_operation *he_oper,
+			    struct ieee80211_eht_operation *eht_oper)
+{
+	u32 he_oper_params;
+	struct ieee80211_he_6ghz_oper *he_6ghz_oper;
+	struct ieee80211_eht_operation_info *eht_oper_info;
+	bool fallback_20mhz;
+	u8 ccfs_diff;
+
+	he_oper_params =
+		u32_encode_bits(1, IEEE80211_HE_OPERATION_6GHZ_OP_INFO);
+	he_oper->he_oper_params = cpu_to_le32(he_oper_params);
+
+	he_6ghz_oper = (struct ieee80211_he_6ghz_oper *)he_oper->optional;
+	he_6ghz_oper->primary = new_chan_no;
+
+	/* The Wide Bandwidth Channel Switch IE in a 6 GHz BSS migth be
+	 * deprecated VHT operation, VHT operation (IEEE 802.11-2020 9.4.2.160)
+	 * or HE operation (IEEE P80211be D3.2 9.4.2.159).
+	 * Check the combination of width, ccfs0 and ccfs1 to build the correct
+	 * HE/EHT operation.
+	 */
+	he_6ghz_oper->ccfs0 = wbcs_ie->new_center_freq_seg0;
+	he_6ghz_oper->ccfs1 = wbcs_ie->new_center_freq_seg1;
+	switch (wbcs_ie->new_channel_width) {
+	case 0:
+		/* Must be [deprecated] VHT operation with 40 MHz bandwidth */
+		he_6ghz_oper->control =
+			IEEE80211_HE_6GHZ_OPER_CTRL_CHANWIDTH_40MHZ;
+		break;
+	case 1:
+		if (he_6ghz_oper->ccfs1) {
+			/* VHT operation with 160/80P80 MHz bandwidth */
+			he_6ghz_oper->control =
+				IEEE80211_HE_6GHZ_OPER_CTRL_CHANWIDTH_160MHZ;
+		} else if ((he_6ghz_oper->ccfs0 - 7) % 16 == 0) {
+			/* [deprecated] VHT operation with 80 MHz bandwidth */
+			he_6ghz_oper->control =
+				IEEE80211_HE_6GHZ_OPER_CTRL_CHANWIDTH_80MHZ;
+		} else {
+			/* HE operation with 40 MHz bandwidth */
+			he_6ghz_oper->control =
+				IEEE80211_HE_6GHZ_OPER_CTRL_CHANWIDTH_40MHZ;
+		}
+		break;
+	case 2:
+		if ((he_6ghz_oper->ccfs0 - 15) % 32 == 0) {
+			/* deprecated VHT operation with 160 MHz bandwidth */
+			he_6ghz_oper->control =
+				IEEE80211_HE_6GHZ_OPER_CTRL_CHANWIDTH_160MHZ;
+			he_6ghz_oper->ccfs1 = he_6ghz_oper->ccfs0;
+			he_6ghz_oper->ccfs0 +=
+				new_chan_no < he_6ghz_oper->ccfs0 ? -8 : 8;
+		} else {
+			/* HE operation with 80 MHz bandwidth */
+			he_6ghz_oper->control =
+				IEEE80211_HE_6GHZ_OPER_CTRL_CHANWIDTH_80MHZ;
+		}
+		break;
+	case 3:
+		/* Can be
+		 * 1. deprecated VHT operation with 80P80 MHz bandwidth
+		 * 2. HE operation with 160/80P80 MHz bandwidth
+		 */
+		he_6ghz_oper->control =
+			IEEE80211_HE_6GHZ_OPER_CTRL_CHANWIDTH_160MHZ;
+		break;
+	case 4:
+		/* 320 MHz bandwidth
+		 * TODO channel switch to 320 MHz bandwidth should be indiated
+		 * by Bandwidth Indication IE (IEEE P80211be D3.2 9.4.2.159)
+		 */
+		he_6ghz_oper->control = IEEE80211_EHT_OPER_CHAN_WIDTH_320MHZ;
+		break;
+	default:
+		/* Ignore invalid width */
+		break;
+	}
+
+	/* Validate the relationship between new channel width and center frequency
+	 * segments, and fallback to 20 MHz if the relationship is wrong.
+	 */
+	fallback_20mhz = false;
+	switch (he_6ghz_oper->control) {
+	case IEEE80211_HE_6GHZ_OPER_CTRL_CHANWIDTH_40MHZ:
+		if ((he_6ghz_oper->ccfs0 - 3) % 8 != 0)
+			fallback_20mhz = true;
+		break;
+	case IEEE80211_HE_6GHZ_OPER_CTRL_CHANWIDTH_80MHZ:
+		if ((he_6ghz_oper->ccfs0 - 7) % 16 != 0)
+			fallback_20mhz = true;
+		break;
+	case IEEE80211_HE_6GHZ_OPER_CTRL_CHANWIDTH_160MHZ:
+		ccfs_diff = abs(he_6ghz_oper->ccfs1 - he_6ghz_oper->ccfs0);
+		if ((ccfs_diff == 8 && (he_6ghz_oper->ccfs1 - 15) % 32 != 0) ||
+		    (ccfs_diff > 16 && ((he_6ghz_oper->ccfs0 - 7) % 16 != 0 ||
+		    (he_6ghz_oper->ccfs1 - 7) % 16 != 0)))
+			fallback_20mhz = true;
+		break;
+	case IEEE80211_EHT_OPER_CHAN_WIDTH_320MHZ:
+		if ((he_6ghz_oper->ccfs1 - 31) % 32 != 0)
+			fallback_20mhz = true;
+		break;
+	}
+
+	if (fallback_20mhz) {
+		he_6ghz_oper->control =
+			IEEE80211_HE_6GHZ_OPER_CTRL_CHANWIDTH_20MHZ;
+		he_6ghz_oper->ccfs0 = he_6ghz_oper->primary;
+		he_6ghz_oper->ccfs1 = 0;
+	}
+
+	eht_oper->params = IEEE80211_EHT_OPER_INFO_PRESENT;
+	eht_oper_info =
+		(struct ieee80211_eht_operation_info *)eht_oper->optional;
+	eht_oper_info->control = he_6ghz_oper->control;
+	eht_oper_info->ccfs0 = he_6ghz_oper->ccfs0;
+	eht_oper_info->ccfs1 = he_6ghz_oper->ccfs1;
+}
+
+static inline void
+op_class_to_ht_vht_oper(u8 op_class, struct ieee80211_channel *chan,
+			struct ieee80211_ht_operation *ht_oper,
+			struct ieee80211_vht_operation *vht_oper)
+{
+	u8 new_chan_width;
+	u32 center_freq1 = 0, center_freq2 = 0;
+
+	new_chan_width = ieee80211_operating_class_to_chan_width(op_class);
+	if (!ieee80211_operating_class_to_center_freq(op_class, chan,
+						      &center_freq1,
+						      &center_freq2)) {
+		new_chan_width = NL80211_CHAN_WIDTH_20;
+		center_freq1 = chan->center_freq;
+	}
+
+	vht_oper->center_freq_seg0_idx =
+		ieee80211_frequency_to_channel(center_freq1);
+	vht_oper->center_freq_seg1_idx = center_freq2 ?
+		ieee80211_frequency_to_channel(center_freq2) : 0;
+
+	ht_oper->ht_param = (chan->center_freq / 20) & 1 ?
+				IEEE80211_HT_PARAM_CHA_SEC_ABOVE :
+				IEEE80211_HT_PARAM_CHA_SEC_BELOW;
+
+	switch (new_chan_width) {
+	case NL80211_CHAN_WIDTH_320:
+		WARN_ON(1);
+		break;
+	case NL80211_CHAN_WIDTH_160:
+		vht_oper->chan_width = IEEE80211_VHT_CHANWIDTH_80MHZ;
+		vht_oper->center_freq_seg1_idx = vht_oper->center_freq_seg0_idx;
+		vht_oper->center_freq_seg0_idx +=
+			chan->center_freq < center_freq1 ? -8 : 8;
+		break;
+	case NL80211_CHAN_WIDTH_80P80:
+		vht_oper->chan_width = IEEE80211_VHT_CHANWIDTH_80MHZ;
+		break;
+	case NL80211_CHAN_WIDTH_80:
+		vht_oper->chan_width = IEEE80211_VHT_CHANWIDTH_80MHZ;
+		break;
+	default:
+		vht_oper->chan_width = IEEE80211_VHT_CHANWIDTH_USE_HT;
+		if (chan->center_freq != center_freq1)
+			ht_oper->ht_param = chan->center_freq > center_freq1 ?
+				IEEE80211_HT_PARAM_CHA_SEC_BELOW :
+				IEEE80211_HT_PARAM_CHA_SEC_ABOVE;
+		else
+			ht_oper->ht_param = IEEE80211_HT_PARAM_CHA_SEC_NONE;
+	}
+
+	ht_oper->operation_mode =
+		cpu_to_le16(vht_oper->center_freq_seg1_idx <<
+				IEEE80211_HT_OP_MODE_CCFS2_SHIFT);
+}
+
+static inline void
+wbcs_ie_to_ht_vht_oper(struct ieee80211_channel *chan,
+		       const struct ieee80211_wide_bw_chansw_ie *wbcs_ie,
+		       struct ieee80211_ht_operation *ht_oper,
+		       struct ieee80211_vht_operation *vht_oper)
+{
+	u8 new_chan_width, new_ccfs0, new_ccfs1;
+	bool fallback_20mhz;
+
+	new_chan_width = wbcs_ie->new_channel_width;
+	new_ccfs0 = wbcs_ie->new_center_freq_seg0;
+	new_ccfs1 = wbcs_ie->new_center_freq_seg1;
+
+	/* Validate the relationship between new channel width and center frequency
+	 * segments, and fallback to 20 MHz if the relationship is wrong.
+	 */
+	fallback_20mhz = false;
+	switch (new_chan_width) {
+	case IEEE80211_VHT_CHANWIDTH_USE_HT:
+		/* If the wide bandwidth channel switch IE is presented,
+		 * the new channel width is at least 40 MHz.
+		 */
+		if (!new_ccfs1) {
+			new_ccfs0 -= (new_ccfs0 >= 149) ? 151 : 38;
+			if (new_ccfs0 % 8 != 0)
+				fallback_20mhz = true;
+		} else {
+			fallback_20mhz = true;
+		}
+		break;
+	case IEEE80211_VHT_CHANWIDTH_80MHZ:
+		if (!new_ccfs1) {
+			new_ccfs0 -= (new_ccfs0 >= 149) ? 155 : 42;
+			if (new_ccfs0 % 16 != 0)
+				fallback_20mhz = true;
+			break;
+		} else if (abs(new_ccfs1 - new_ccfs0) == 8) {
+			new_ccfs0 = new_ccfs1;
+			new_ccfs1 = 0;
+		}
+		fallthrough;
+	case IEEE80211_VHT_CHANWIDTH_160MHZ:
+		if (!new_ccfs1) {
+			if (new_ccfs0 != 50 && new_ccfs0 != 114 && new_ccfs0 != 163)
+				fallback_20mhz = true;
+			break;
+		}
+		fallthrough;
+	case IEEE80211_VHT_CHANWIDTH_80P80MHZ:
+		new_ccfs0 -= (new_ccfs0 >= 149) ? 155 : 42;
+		new_ccfs1 -= (new_ccfs1 >= 149) ? 155 : 42;
+		if (new_ccfs0 % 16 != 0 || new_ccfs1 % 16 != 0)
+			fallback_20mhz = true;
+		break;
+	default:
+		fallback_20mhz = true;
+	}
+
+	if (fallback_20mhz) {
+		ht_oper->ht_param = IEEE80211_HT_PARAM_CHA_SEC_NONE;
+
+		vht_oper->chan_width = IEEE80211_VHT_CHANWIDTH_USE_HT;
+		vht_oper->center_freq_seg0_idx =
+			ieee80211_frequency_to_channel(chan->center_freq);
+		vht_oper->center_freq_seg1_idx = 0;
+
+	} else {
+		ht_oper->ht_param = (chan->center_freq / 20) & 1 ?
+					IEEE80211_HT_PARAM_CHA_SEC_ABOVE :
+					IEEE80211_HT_PARAM_CHA_SEC_BELOW;
+
+		vht_oper->chan_width = new_chan_width;
+		vht_oper->center_freq_seg0_idx = wbcs_ie->new_center_freq_seg0;
+		vht_oper->center_freq_seg1_idx = wbcs_ie->new_center_freq_seg1;
+	}
+
+	ht_oper->operation_mode = cpu_to_le16(vht_oper->center_freq_seg1_idx <<
+					      IEEE80211_HT_OP_MODE_CCFS2_SHIFT);
+}
+
 int ieee80211_parse_ch_switch_ie(struct ieee80211_sub_if_data *sdata,
 				 struct ieee802_11_elems *elems,
 				 enum nl80211_band current_band,
@@ -26,14 +352,21 @@ int ieee80211_parse_ch_switch_ie(struct ieee80211_sub_if_data *sdata,
 				 ieee80211_conn_flags_t conn_flags, u8 *bssid,
 				 struct ieee80211_csa_ie *csa_ie)
 {
+	struct ieee80211_supported_band *sband;
 	enum nl80211_band new_band = current_band;
-	int new_freq;
-	u8 new_chan_no;
+	int new_freq, size;
+	u8 new_chan_no = 0, new_op_class = 0;
 	struct ieee80211_channel *new_chan;
-	struct cfg80211_chan_def new_vht_chandef = {};
+	struct cfg80211_chan_def new_chandef = {};
 	const struct ieee80211_sec_chan_offs_ie *sec_chan_offs;
 	const struct ieee80211_wide_bw_chansw_ie *wide_bw_chansw_ie;
 	const struct ieee80211_bandwidth_indication *bwi;
+	const struct ieee80211_ext_chansw_ie *ext_chansw_ie;
+	struct ieee80211_ht_operation *ht_oper;
+	struct ieee80211_vht_operation *vht_oper;
+	struct ieee80211_he_operation *he_oper;
+	struct ieee80211_eht_operation *eht_oper;
+	struct ieee80211_sta_ht_cap sta_ht_cap;
 	int secondary_channel_offset = -1;
 
 	memset(csa_ie, 0, sizeof(*csa_ie));
@@ -41,6 +374,7 @@ int ieee80211_parse_ch_switch_ie(struct ieee80211_sub_if_data *sdata,
 	sec_chan_offs = elems->sec_chan_offs;
 	wide_bw_chansw_ie = elems->wide_bw_chansw_ie;
 	bwi = elems->bandwidth_indication;
+	ext_chansw_ie = elems->ext_chansw_ie;
 
 	if (conn_flags & (IEEE80211_CONN_DISABLE_HT |
 			  IEEE80211_CONN_DISABLE_40MHZ)) {
@@ -48,29 +382,30 @@ int ieee80211_parse_ch_switch_ie(struct ieee80211_sub_if_data *sdata,
 		wide_bw_chansw_ie = NULL;
 	}
 
-	if (conn_flags & IEEE80211_CONN_DISABLE_VHT)
-		wide_bw_chansw_ie = NULL;
-
-	if (elems->ext_chansw_ie) {
-		if (!ieee80211_operating_class_to_band(
-				elems->ext_chansw_ie->new_operating_class,
-				&new_band)) {
-			sdata_info(sdata,
-				   "cannot understand ECSA IE operating class, %d, ignoring\n",
-				   elems->ext_chansw_ie->new_operating_class);
+	if (ext_chansw_ie) {
+		new_op_class = ext_chansw_ie->new_operating_class;
+		if (!ieee80211_operating_class_to_band(new_op_class, &new_band)) {
+			new_op_class = 0;
+			sdata_info(sdata, "cannot understand ECSA IE "
+					  "operating class, %d, ignoring\n",
+				   ext_chansw_ie->new_operating_class);
+		} else {
+			new_chan_no = ext_chansw_ie->new_ch_num;
+			csa_ie->count = ext_chansw_ie->count;
+			csa_ie->mode = ext_chansw_ie->mode;
 		}
-		new_chan_no = elems->ext_chansw_ie->new_ch_num;
-		csa_ie->count = elems->ext_chansw_ie->count;
-		csa_ie->mode = elems->ext_chansw_ie->mode;
-	} else if (elems->ch_switch_ie) {
+	}
+
+	if (!new_op_class && elems->ch_switch_ie) {
 		new_chan_no = elems->ch_switch_ie->new_ch_num;
 		csa_ie->count = elems->ch_switch_ie->count;
 		csa_ie->mode = elems->ch_switch_ie->mode;
-	} else {
-		/* nothing here we understand */
-		return 1;
 	}
 
+	/* nothing here we understand */
+	if (!new_chan_no)
+		return 1;
+
 	/* Mesh Channel Switch Parameters Element */
 	if (elems->mesh_chansw_params_ie) {
 		csa_ie->ttl = elems->mesh_chansw_params_ie->mesh_ttl;
@@ -136,59 +471,124 @@ int ieee80211_parse_ch_switch_ie(struct ieee80211_sub_if_data *sdata,
 
 	if (bwi) {
 		/* start with the CSA one */
-		new_vht_chandef = csa_ie->chandef;
+		new_chandef = csa_ie->chandef;
 		/* and update the width accordingly */
 		/* FIXME: support 160/320 */
 		ieee80211_chandef_eht_oper(&bwi->info, true, true,
-					   &new_vht_chandef);
-	} else if (wide_bw_chansw_ie) {
-		u8 new_seg1 = wide_bw_chansw_ie->new_center_freq_seg1;
-		struct ieee80211_vht_operation vht_oper = {
-			.chan_width =
-				wide_bw_chansw_ie->new_channel_width,
-			.center_freq_seg0_idx =
-				wide_bw_chansw_ie->new_center_freq_seg0,
-			.center_freq_seg1_idx = new_seg1,
-			/* .basic_mcs_set doesn't matter */
-		};
-		struct ieee80211_ht_operation ht_oper = {
-			.operation_mode =
-				cpu_to_le16(new_seg1 <<
-					    IEEE80211_HT_OP_MODE_CCFS2_SHIFT),
-		};
-
-		/* default, for the case of IEEE80211_VHT_CHANWIDTH_USE_HT,
-		 * to the previously parsed chandef
-		 */
-		new_vht_chandef = csa_ie->chandef;
+					   &new_chandef);
+	} else if (new_band == NL80211_BAND_6GHZ) {
+		size = sizeof(struct ieee80211_he_operation) +
+		       sizeof(struct ieee80211_he_6ghz_oper);
+		he_oper = kzalloc(size, GFP_KERNEL);
+		if (!he_oper)
+			return -ENOMEM;
+
+		size = sizeof(struct ieee80211_eht_operation) +
+		       sizeof(struct ieee80211_eht_operation_info);
+		eht_oper = kzalloc(size, GFP_KERNEL);
+		if (!eht_oper) {
+			kfree(he_oper);
+			return -ENOMEM;
+		}
+
+		if (new_op_class && new_op_class != 135 && new_op_class != 137) {
+			/* There is no way to tell the ccfs1 for op_class 135
+			 * (80P80 MHz) and 137 (320 MHz).
+			 */
+			op_class_to_6ghz_he_eht_oper(new_op_class, new_chan,
+						     he_oper, eht_oper);
+		} else if (wide_bw_chansw_ie) {
+			wbcs_ie_to_6ghz_he_eht_oper(wide_bw_chansw_ie,
+						    new_chan_no, he_oper,
+						    eht_oper);
+		}
+
+		new_chandef = csa_ie->chandef;
 
 		/* ignore if parsing fails */
-		if (!ieee80211_chandef_vht_oper(&sdata->local->hw,
-						vht_cap_info,
-						&vht_oper, &ht_oper,
-						&new_vht_chandef))
-			new_vht_chandef.chan = NULL;
+		if (!ieee80211_chandef_he_6ghz_oper(sdata, he_oper, eht_oper,
+						    &new_chandef))
+			new_chandef.chan = NULL;
+
+		kfree(he_oper);
+		kfree(eht_oper);
+	} else {
+		sband = sdata->local->hw.wiphy->bands[new_band];
+		memcpy(&sta_ht_cap, &sband->ht_cap, sizeof(sta_ht_cap));
+		ieee80211_apply_htcap_overrides(sdata, &sta_ht_cap);
+
+		if (!sta_ht_cap.ht_supported ||
+		    !(sta_ht_cap.cap & IEEE80211_HT_CAP_SUP_WIDTH_20_40))
+			goto out;
+
+		ht_oper = kzalloc(sizeof(*ht_oper), GFP_KERNEL);
+		if (!ht_oper)
+			return -ENOMEM;
+
+		vht_oper = kzalloc(sizeof(*vht_oper), GFP_KERNEL);
+		if (!vht_oper) {
+			kfree(ht_oper);
+			return -ENOMEM;
+		}
+
+		if (new_op_class && new_op_class != 130) {
+			/* There is no way to tell the ccfs1 for op_class 130
+			 * (80P80 MHz).
+			 */
+			op_class_to_ht_vht_oper(new_op_class, new_chan, ht_oper,
+						vht_oper);
+		} else if (wide_bw_chansw_ie && new_band == NL80211_BAND_5GHZ &&
+			   sband->vht_cap.vht_supported) {
+			/* It is assumed that there is no WBCS IE in the beacon
+			 * from a 2 GHz BSS during a channel switch.
+			 */
+			wbcs_ie_to_ht_vht_oper(new_chan, wide_bw_chansw_ie,
+					       ht_oper, vht_oper);
+		} else {
+			kfree(ht_oper);
+			kfree(vht_oper);
+			goto out;
+		}
+
+		new_chandef = csa_ie->chandef;
+
+		ieee80211_chandef_ht_oper(ht_oper, &new_chandef);
+
+		/* ignore if parsing fails */
+		if (sband->vht_cap.vht_supported &&
+		    !ieee80211_chandef_vht_oper(&sdata->local->hw, vht_cap_info,
+						vht_oper, ht_oper, &new_chandef))
+			new_chandef.chan = NULL;
+
+		kfree(ht_oper);
+		kfree(vht_oper);
+	}
+
+	if (new_chandef.chan) {
+		if (conn_flags & IEEE80211_CONN_DISABLE_320MHZ &&
+		    new_chandef.width == NL80211_CHAN_WIDTH_320)
+			ieee80211_chandef_downgrade(&new_chandef);
 
 		if (conn_flags & IEEE80211_CONN_DISABLE_80P80MHZ &&
-		    new_vht_chandef.width == NL80211_CHAN_WIDTH_80P80)
-			ieee80211_chandef_downgrade(&new_vht_chandef);
+		    new_chandef.width == NL80211_CHAN_WIDTH_80P80)
+			ieee80211_chandef_downgrade(&new_chandef);
+
 		if (conn_flags & IEEE80211_CONN_DISABLE_160MHZ &&
-		    new_vht_chandef.width == NL80211_CHAN_WIDTH_160)
-			ieee80211_chandef_downgrade(&new_vht_chandef);
-	}
+		    new_chandef.width == NL80211_CHAN_WIDTH_160)
+			ieee80211_chandef_downgrade(&new_chandef);
 
-	/* if VHT data is there validate & use it */
-	if (new_vht_chandef.chan) {
-		if (!cfg80211_chandef_compatible(&new_vht_chandef,
+		if (!cfg80211_chandef_compatible(&new_chandef,
 						 &csa_ie->chandef)) {
 			sdata_info(sdata,
 				   "BSS %pM: CSA has inconsistent channel data, disconnecting\n",
 				   bssid);
 			return -EINVAL;
 		}
-		csa_ie->chandef = new_vht_chandef;
+
+		csa_ie->chandef = new_chandef;
 	}
 
+out:
 	if (elems->max_channel_switch_time)
 		csa_ie->max_switch_time =
 			(elems->max_channel_switch_time[0] << 0) |
-- 
2.25.1

Re: [PATCH v2,2/2] wifi: mac80211: Refactor STA CSA parsing flow

[Johannes Berg]

On Mon, 2023-11-13 at 10:11 +0800, Michael-CY Lee wrote:
> The Wi-Fi Standard (IEEE 802.11-2020 9.4.2.160) initially specified that
> the Wide Bandwidth Channel Switch (WBCS) IE subfields have same definitions
> as the S1G or VHT Operation Information according to the operating band.
> 
> However, it did not change the definitions in the amendment for 6 GHz
> (IEEE 802.11ax-2021), so the logic remain the same for handling the WBCS
> IE even if there is no VHT mode in 6 GHz.
> 
> Now the Wi-Fi Standard draft (IEEE P80211be D3.2 9.4.2.159) modifies the
> defitions, making the WBCS IE subfields follow the definitions of S1G,

type - definitions

> VHT and HE Operation Information in S1G, 5 GHz and 6 GHz band, respectively.
> 
> APs in 6 GHz band might use the VHT or HE Operation Information to build
> a WBCS IE according to the Wi-Fi Standard they follow. Originally, the STA

Probably should say "Element" in place of all those "IE" - the spec
stopped calling them "Information Elements" a long time ago :)

> just parsed the WBCS IE as VHT Operation Inforamtion, which was wrong if
> the AP was actually build the IE by the HE Operation Information.
> 
> To avoid the ambiguity, STA should prefer the op_class in the Extended
> Channel Switch Announcement (ECSA) IE rathen than the WBCS IE. If the ECSA

typo - rather

> IE is not presented in a channel switch to 6 GHz, the STA should be aware
> of the possible ambiguity when parsing the WBCS IE.
> 
> To derive the correct bandwidtin in use, the STA should check the

typo - bandwidth in

> +	case 4:
> +		/* 320 MHz bandwidth
> +		 * TODO channel switch to 320 MHz bandwidth should be indiated
> +		 * by Bandwidth Indication IE (IEEE P80211be D3.2 9.4.2.159)
> +		 */
> +		he_6ghz_oper->control = IEEE80211_EHT_OPER_CHAN_WIDTH_320MHZ;
> +		break;

I'm not sure what this TODO was meant to refer to, but I do know that
D4.1 made some changes here, maybe we should check those? I haven't even
checked what the changes are though.

In any case, checking with a newer draft and using that would seem
useful?

Haven't really read all the other things here yet, this just caught my
eye since I also just heard about D4.1 changes, but I don't have that or
even the old stuff all in my head right now.

johannes

Re: [PATCH v2,2/2] wifi: mac80211: Refactor STA CSA parsing flow

[Michael-cy Lee (李峻宇)]

Hi Johannes,

I will fix the typos and change all the “IE ” to “Element”. The TODO
for 320 MHz bandwidth is used for internal tests, I’ll remove it, too.
 
We had checked the D4.1, and unfortunately, the additional description
for WBCS Element in D3.2 9.4.2.159 is removed. In other words, D4.1
does not change the definitions of WBCS Element and WBCS Element should
be parsed as VHT operating information regardless of the BSS being VHT,
HE, or EHT.
 
After parsing the WBCS Element, the STA needs to check whether the new
channel fits its capabilities according to the operating mode
(VHT/HE/EHT).
However, the existing flow only checks the VHT capability after the
WBCS Element parsing, which is incorrect when the BSS is HE/EHT or the
band is 6 GHz.
 
In summary, I will refactor the WBCS Element parsing part of this
patch, along with other fixes.

Best,
Michael

On Mon, 2023-11-13 at 11:20 +0100, Johannes Berg wrote:
>  	 
> External email : Please do not click links or open attachments until
> you have verified the sender or the content.
>  On Mon, 2023-11-13 at 10:11 +0800, Michael-CY Lee wrote:
> > The Wi-Fi Standard (IEEE 802.11-2020 9.4.2.160) initially specified
> that
> > the Wide Bandwidth Channel Switch (WBCS) IE subfields have same
> definitions
> > as the S1G or VHT Operation Information according to the operating
> band.
> > 
> > However, it did not change the definitions in the amendment for 6
> GHz
> > (IEEE 802.11ax-2021), so the logic remain the same for handling the
> WBCS
> > IE even if there is no VHT mode in 6 GHz.
> > 
> > Now the Wi-Fi Standard draft (IEEE P80211be D3.2 9.4.2.159)
> modifies the
> > defitions, making the WBCS IE subfields follow the definitions of
> S1G,
> 
> type - definitions
> 
> > VHT and HE Operation Information in S1G, 5 GHz and 6 GHz band,
> respectively.
> > 
> > APs in 6 GHz band might use the VHT or HE Operation Information to
> build
> > a WBCS IE according to the Wi-Fi Standard they follow. Originally,
> the STA
> 
> Probably should say "Element" in place of all those "IE" - the spec
> stopped calling them "Information Elements" a long time ago :)
> 
> > just parsed the WBCS IE as VHT Operation Inforamtion, which was
> wrong if
> > the AP was actually build the IE by the HE Operation Information.
> > 
> > To avoid the ambiguity, STA should prefer the op_class in the
> Extended
> > Channel Switch Announcement (ECSA) IE rathen than the WBCS IE. If
> the ECSA
> 
> typo - rather
> 
> > IE is not presented in a channel switch to 6 GHz, the STA should be
> aware
> > of the possible ambiguity when parsing the WBCS IE.
> > 
> > To derive the correct bandwidtin in use, the STA should check the
> 
> typo - bandwidth in
> 
> > +case 4:
> > +/* 320 MHz bandwidth
> > + * TODO channel switch to 320 MHz bandwidth should be indiated
> > + * by Bandwidth Indication IE (IEEE P80211be D3.2 9.4.2.159)
> > + */
> > +he_6ghz_oper->control = IEEE80211_EHT_OPER_CHAN_WIDTH_320MHZ;
> > +break;
> 
> I'm not sure what this TODO was meant to refer to, but I do know that
> D4.1 made some changes here, maybe we should check those? I haven't
> even
> checked what the changes are though.
> 
> In any case, checking with a newer draft and using that would seem
> useful?
> 
> Haven't really read all the other things here yet, this just caught
> my
> eye since I also just heard about D4.1 changes, but I don't have that
> or
> even the old stuff all in my head right now.
> 
> johannes
> 

Re: [PATCH v2,2/2] wifi: mac80211: Refactor STA CSA parsing flow

[Johannes Berg]

Hi Michael,

> I will fix the typos and change all the “IE ” to “Element”. The TODO
> for 320 MHz bandwidth is used for internal tests, I’ll remove it, too.

Sounds good.

> We had checked the D4.1, and unfortunately, the additional description
> for WBCS Element in D3.2 9.4.2.159 is removed. In other words, D4.1
> does not change the definitions of WBCS Element and WBCS Element should
> be parsed as VHT operating information regardless of the BSS being VHT,
> HE, or EHT.

Right, I thought there were going to be some changes here.

> After parsing the WBCS Element, the STA needs to check whether the new
> channel fits its capabilities according to the operating mode
> (VHT/HE/EHT).
> However, the existing flow only checks the VHT capability after the
> WBCS Element parsing, which is incorrect when the BSS is HE/EHT or the
> band is 6 GHz.
>  
> In summary, I will refactor the WBCS Element parsing part of this
> patch, along with other fixes.

Sounds good.

We'll have to figure this out separately, but I'm also working on
something else that will certainly affect this code. I noticed very
recently that the whole STA_DISABLE_* flags are very messy, and am
working on some improvements along these lines. The current version
looks like this, but it doesn't even compile yet:

https://p.sipsolutions.net/ccb552810b020745.txt

No idea if this would help/hinder you in any way, but I figured since it
has some overlap I'd let you know. I don't think you need to worry about
it for now, it'll take me some more time to work on it.

johannes

Re: [PATCH v2,1/2] wifi: mac80211: Add utilities for converting op_class

[Johannes Berg]

Btw, I ended looking at this again...

On Mon, 2023-11-13 at 10:11 +0800, Michael-CY Lee wrote: 
> +/**
> + * ieee80211_operating_class_to_center_freq - convert operating class to
> + * center frequency
> + *
> + * @operating_class: the operating class to convert
> + * @chan: the ieee80211_channel to convert
> + * @center_freq1: cneter frequency 1 pointer to fill
> + * @center_freq2: cneter frequency 2 pointer to fill

typos here ("center")

But maybe it'd be better to fill (or update, we could pass the channel
pointer in it) a chandef struct? Then it could also be more easily
extended to understand more opclasses in the future, perhaps S1G or DMG?

> + *
> + * Returns %true if the conversion was successful, %false otherwise.
> + */
> +bool ieee80211_operating_class_to_center_freq(u8 operating_class,
> +					      struct ieee80211_channel *chan,
> +					      u32 *center_freq1,
> +					      u32 *center_freq2);
> +
> +/**
> + * ieee80211_operating_class_to_chan_width - convert operating class to
> + * nl80211 channel width
> + *
> + * @operating_class: the operating class to convert
> + */
> +enum nl80211_chan_width
> +ieee80211_operating_class_to_chan_width(u8 operating_class);

And you'd actually get both in one function call? The chan ->
center_freq anyway implies you know the width, no? Is this really needed
separately?

>  /**
>   * ieee8 0211_chandef_to_operating_class - convert chandef to operation class

This also converts the other way around, btw.

> +	case 135: /* 6 GHz band; 80+80 MHz; channels 1,5,..,229 */
> +		/* TODO How to know the center_freq2 of 80+80 MHz?*/
> +		*center_freq1 = 0;

Well, you don't, from this. I'm actually a bit surprised 80+80 exists in
6 GHz, I thought it was treated more or less as a dead end.

johannes

Re: [PATCH v2,2/2] wifi: mac80211: Refactor STA CSA parsing flow

[Johannes Berg]

On Mon, 2023-11-13 at 10:11 +0800, Michael-CY Lee wrote:
> 
> +	new_chan_width = ieee80211_operating_class_to_chan_width(op_class);
> +	if (!ieee80211_operating_class_to_center_freq(op_class, chan,
> +						      &center_freq1,
> +						      &center_freq2)) {

Unless I missed it, I see two places here calling it together, so seems
reasonable to fill in a chandef here instead?

> +	new_chan_width = ieee80211_operating_class_to_chan_width(op_class);
> +	if (!ieee80211_operating_class_to_center_freq(op_class, chan,
> +						      &center_freq1,
> +						      &center_freq2)) {


Here you have it too.


> +		new_chan_width = NL80211_CHAN_WIDTH_20;
> +		center_freq1 = chan->center_freq;

And actually you could just have a chandef created with

    cfg80211_chandef_create(&chandef, chan, NL80211_CHAN_WIDTH_20)

which mirrors the failure case here, and just not update it when
something like

    ieee80211_update_chandef_from_op_class(op_class, &chandef)

returned false (not that I necessarily think that name should be used.)

Or just pass the channel, and make it create one with WIDTH_20 in the
failure case?

    ieee80211_create_chandef_from_opclass(chan, op_class, &chandef);

which is is maybe even nicer?

I'm also not quite sure why you're converting to operation elements
first?

johannes

Re: [PATCH v2,1/2] wifi: mac80211: Add utilities for converting op_class

[Michael-cy Lee (李峻宇)]

Hi Johannes, 

Thanks for the suggestion. It's truely more reasonable to do the
conversion in one single function.

we'll modify it along with the typo in next version.

Best,
Michael

On Fri, 2023-11-24 at 20:25 +0100, Johannes Berg wrote:
>  	 
> External email : Please do not click links or open attachments until
> you have verified the sender or the content.
>  Btw, I ended looking at this again...
> 
> On Mon, 2023-11-13 at 10:11 +0800, Michael-CY Lee wrote: 
> > +/**
> > + * ieee80211_operating_class_to_center_freq - convert operating
> class to
> > + * center frequency
> > + *
> > + * @operating_class: the operating class to convert
> > + * @chan: the ieee80211_channel to convert
> > + * @center_freq1: cneter frequency 1 pointer to fill
> > + * @center_freq2: cneter frequency 2 pointer to fill
> 
> typos here ("center")
> 
> But maybe it'd be better to fill (or update, we could pass the
> channel
> pointer in it) a chandef struct? Then it could also be more easily
> extended to understand more opclasses in the future, perhaps S1G or
> DMG?
> 
> > + *
> > + * Returns %true if the conversion was successful, %false
> otherwise.
> > + */
> > +bool ieee80211_operating_class_to_center_freq(u8 operating_class,
> > +      struct ieee80211_channel *chan,
> > +      u32 *center_freq1,
> > +      u32 *center_freq2);
> > +
> > +/**
> > + * ieee80211_operating_class_to_chan_width - convert operating
> class to
> > + * nl80211 channel width
> > + *
> > + * @operating_class: the operating class to convert
> > + */
> > +enum nl80211_chan_width
> > +ieee80211_operating_class_to_chan_width(u8 operating_class);
> 
> And you'd actually get both in one function call? The chan ->
> center_freq anyway implies you know the width, no? Is this really
> needed
> separately?
> 
> >  /**
> >   * ieee8 0211_chandef_to_operating_class - convert chandef to
> operation class
> 
> This also converts the other way around, btw.
> 
> > +case 135: /* 6 GHz band; 80+80 MHz; channels 1,5,..,229 */
> > +/* TODO How to know the center_freq2 of 80+80 MHz?*/
> > +*center_freq1 = 0;
> 
> Well, you don't, from this. I'm actually a bit surprised 80+80 exists
> in
> 6 GHz, I thought it was treated more or less as a dead end.


> 
> johannes

Re: [PATCH v2,2/2] wifi: mac80211: Refactor STA CSA parsing flow

[Michael-cy Lee (李峻宇)]

Hi Johannes,

On Fri, 2023-11-24 at 20:31 +0100, Johannes Berg wrote:
>  	 
> External email : Please do not click links or open attachments until
> you have verified the sender or the content.
>  On Mon, 2023-11-13 at 10:11 +0800, Michael-CY Lee wrote:
> > 
> > +new_chan_width =
> ieee80211_operating_class_to_chan_width(op_class);
> > +if (!ieee80211_operating_class_to_center_freq(op_class, chan,
> > +      &center_freq1,
> > +      &center_freq2)) {
> 
> Unless I missed it, I see two places here calling it together, so
> seems
> reasonable to fill in a chandef here instead?
> 
> > +new_chan_width =
> ieee80211_operating_class_to_chan_width(op_class);
> > +if (!ieee80211_operating_class_to_center_freq(op_class, chan,
> > +      &center_freq1,
> > +      &center_freq2)) {
> 
> 
> Here you have it too.
> 
> 
> > +new_chan_width = NL80211_CHAN_WIDTH_20;
> > +center_freq1 = chan->center_freq;
> 
> And actually you could just have a chandef created with
> 
>     cfg80211_chandef_create(&chandef, chan, NL80211_CHAN_WIDTH_20)
> 
> which mirrors the failure case here, and just not update it when
> something like
> 
>     ieee80211_update_chandef_from_op_class(op_class, &chandef)
> 
> returned false (not that I necessarily think that name should be
> used.)
> 
> Or just pass the channel, and make it create one with WIDTH_20 in the
> failure case?
> 
>     ieee80211_create_chandef_from_opclass(chan, op_class, &chandef);
> 
> which is is maybe even nicer?

Good suggestion!
We'll decide which one is better.

> 
> I'm also not quite sure why you're converting to operation elements
> first?

The old flow also converted the Element to operation elements first,
then it used ieee80211_chandef_vht_oper() to build the new chandef from
operation elements.

We think it's necessary for the case that AP is trying to switch to a
160 MHz bandwidth, while the STA doesn't support the 160 MHz bandwidth.

Just like what had been done during the association,
ieee80211_chandef_vht_oper() checks the STA's capabilities and builds a
valid chandef for the STA. However, even if the STA doesn't support the
160 MHz bandwidth, ieee80211_chandef_vht_oper() doesn't mark the
conn_flags as IEEE80211_CONN_DISABLE_160MHZ, so the same check is
necessary when handling CSA.

Like we had discussed in previous mail, we expected the patch will be
simplified.
In summary, the steps for STA to handling CSA are, 
1. parse the new channel information from either operating class or
WBCS Element.
2. convert the channel information into corresponding operation Element
(HT/VHT in 5 GHz band and HE/EHT in 6 GHz band)
3. Build a valid chandef from the operation Element.

> 
> johannes

Best, 
Micahel
> 

Re: [PATCH v2,2/2] wifi: mac80211: Refactor STA CSA parsing flow

[Johannes Berg]

Hi,

> The old flow also converted the Element to operation elements first,
> then it used ieee80211_chandef_vht_oper() to build the new chandef from
> operation elements.
> 
> We think it's necessary for the case that AP is trying to switch to a
> 160 MHz bandwidth, while the STA doesn't support the 160 MHz bandwidth.

Yeah, you're right, it did before and I suppose it's still easier than
managing two conversions, since some formats are the same.

> Just like what had been done during the association,
> ieee80211_chandef_vht_oper() checks the STA's capabilities and builds a
> valid chandef for the STA. However, even if the STA doesn't support the
> 160 MHz bandwidth, ieee80211_chandef_vht_oper() doesn't mark the
> conn_flags as IEEE80211_CONN_DISABLE_160MHZ, so the same check is
> necessary when handling CSA.

Right.

> Like we had discussed in previous mail, we expected the patch will be
> simplified.

Yeah I just circled back to it for stupid reasons (I guess mostly forgot
to mark in patchwork that you were going to make changes) and looked
again after having looked at all the other code in the series I posted
late last week.

> In summary, the steps for STA to handling CSA are, 
> 1. parse the new channel information from either operating class or
> WBCS Element.
> 2. convert the channel information into corresponding operation Element
> (HT/VHT in 5 GHz band and HE/EHT in 6 GHz band)
> 3. Build a valid chandef from the operation Element.
> 

Sounds good.

johannes