diff mbox series

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

Message ID 20231113021107.13110-1-michael-cy.lee@mediatek.com
State New
Headers show
Series [v2,1/2] wifi: mac80211: Add utilities for converting op_class | expand

Commit Message

Michael-CY Lee Nov. 13, 2023, 2:11 a.m. UTC
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(+)

Comments

Johannes Berg Nov. 13, 2023, 10:20 a.m. UTC | #1
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
Johannes Berg Nov. 20, 2023, 10:07 a.m. UTC | #2
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
Johannes Berg Nov. 24, 2023, 7:25 p.m. UTC | #3
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
Michael-CY Lee Nov. 27, 2023, 2:49 a.m. UTC | #4
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
diff mbox series

Patch

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)
 {