Skip to contentSkip to conversation

cfg80211/mac80211: Add support for 6GHZ STA for various modes : LPI, SP and VLP

PATCH Superseded Wen Gong 3 participants 43 messages Aug 20, 2021 – Jul 19, 2023

Subsystems (4)

v2 of 4 Superseded Latest is v4 →

lore
Share & tools
lore

Wen Gong

author
#0 lore raw

v2 cover cfg80211/mac80211: Add support for 6GHZ STA for various modes : LPI, SP and VLP

v2: change per comments of johannes.
including code style, code logic, patch merge, commit log...

It introduced some new concept:
power type of AP(STANDARD_POWER_AP, INDOOR_AP, VERY_LOW_POWER_AP) power type of STATION(DEFAULT_CLIENT, SUBORDINATE_CLIENT) power spectral density(psd)

This patchset for cfg80211/mac80211 is to add the definition of new concept of 6G and add basic parse of IE(transmit power envelope element) in beacon and save power spectral density(psd) reported by lower-driver for 6G channel, the info will be passed to lower driver when connecting to 6G AP.

Wen Gong (8):
cfg80211: add power type definition for 6 GHz
mac80211: add definition of regulatory info in 6 GHz operation
information
mac80211: add parse regulatory info in 6 GHz operation information
cfg80211: add definition for 6 GHz power spectral density(psd)
cfg80211: save power spectral density(psd) of regulatory rule
mac80211: add definition for transmit power envelope element
mac80211: add parse transmit power envelope element
mac80211: save transmit power envelope element and power constraint

 include/linux/ieee80211.h    | 43 +++++++++++++++++++++++++++++++++++-
 include/net/cfg80211.h       |  7 ++++++
 include/net/mac80211.h       |  6 +++++
 include/net/regulatory.h     |  1 +
 include/uapi/linux/nl80211.h | 36 ++++++++++++++++++++++++++++++
 net/mac80211/chan.c          |  9 ++++++++
 net/mac80211/ieee80211_i.h   |  3 +++
 net/mac80211/mlme.c          | 26 ++++++++++++++++++++++
 net/mac80211/util.c          | 19 ++++++++++++++++
 net/wireless/reg.c           | 14 ++++++++++++
 10 files changed, 163 insertions(+), 1 deletion(-)
Signature · 1 line
2.31.1
author
#1 lore raw

PATCH v2 1/8 cfg80211: add power type definition for 6 GHz

6 GHz regulatory domains introduces different modes for 6 GHz AP operations Low Power Indoor(LPI), Standard Power(SP) and Very Low Power(VLP). 6 GHz STAs could be operated as either Regular or Subordinate clients. This patch is define the flags for power type of AP and STATION mode.

---
 include/net/cfg80211.h       |  2 ++
 include/uapi/linux/nl80211.h | 34 ++++++++++++++++++++++++++++++++++
 2 files changed, 36 insertions(+)
include/net/cfg80211.h +2 -0
index 58c2cd417e89..f17a4d1202fc 100644
@@ -676,6 +676,7 @@ struct key_params {
  *	chan will define the primary channel and all other
  *	parameters are ignored.
  * @freq1_offset: offset from @center_freq1, in KHz
+ * @power_type: power type of BSS for 6 GHz
  */
 struct cfg80211_chan_def {
 	struct ieee80211_channel *chan;
@@ -684,6 +685,7 @@ struct cfg80211_chan_def {
 	u32 center_freq2;
 	struct ieee80211_edmg edmg;
 	u16 freq1_offset;
+	enum nl80211_ap_reg_power power_type;
 };
 
 /*
include/uapi/linux/nl80211.h +34 -0
index f962c06e9818..ab1d4aa85272 100644
@@ -4088,6 +4088,40 @@ enum nl80211_dfs_regions {
 	NL80211_DFS_JP		= 3,
 };
 
+/**
+ * enum nl80211_ap_reg_power - regulatory power for a Access Point
+ *
+ * @NL80211_REG_UNSET_AP: Access Point has no regulatory power mode
+ * @NL80211_REG_LPI: Indoor Access Point
+ * @NL80211_REG_SP: Standard power Access Point
+ * @NL80211_REG_VLP: Very low power Access Point
+ */
+enum nl80211_ap_reg_power {
+	NL80211_REG_UNSET_AP,
+	NL80211_REG_LPI_AP,
+	NL80211_REG_SP_AP,
+	NL80211_REG_VLP_AP,
+	NL80211_REG_AP_POWER_AFTER_LAST,
+	NL80211_REG_AP_POWER_MAX =
+		NL80211_REG_AP_POWER_AFTER_LAST - 1,
+};
+
+/**
+ * enum nl80211_client_reg_power - regulatory power for a client
+ *
+ * @NL80211_REG_UNSET_CLIENT: Client has no regulatory power mode
+ * @NL80211_REG_DEFAULT_CLIENT: Default Client
+ * @NL80211_REG_SUBORDINATE_CLIENT: Subordinate Client
+ */
+enum nl80211_client_reg_power {
+	NL80211_REG_UNSET_CLIENT,
+	NL80211_REG_DEFAULT_CLIENT,
+	NL80211_REG_SUBORDINATE_CLIENT,
+	NL80211_REG_CLIENT_POWER_AFTER_LAST,
+	NL80211_REG_CLIENT_POWER_MAX =
+		NL80211_REG_CLIENT_POWER_AFTER_LAST - 1,
+};
+
 /**
  * enum nl80211_user_reg_hint_type - type of user regulatory hint
  *
Signature · 1 line
2.31.1
maintainer
#2 lore raw

Re: [PATCH v2 1/8] cfg80211: add power type definition for 6 GHz

include/net/cfg80211.h struct cfg80211_chan_def { · line 688

Quoted 8 lines

 struct cfg80211_chan_def {
  struct ieee80211_channel *chan;

include/net/cfg80211.h +1 -0
@@ -684,6 +685,7 @@ struct cfg80211_chan_def {
  	u32 center_freq2;
  	struct ieee80211_edmg edmg;
  	u16 freq1_offset;
+	enum nl80211_ap_reg_power power_type;

I'm not sure why this should be in the chandef, there's no way that anything in cfg80211 is ever using it there, at least in your patches.

Quoted 7 lines
include/uapi/linux/nl80211.h +7 -0
+/**
+ * enum nl80211_ap_reg_power - regulatory power for a Access Point
+ *
+ * @NL80211_REG_UNSET_AP: Access Point has no regulatory power mode
+ * @NL80211_REG_LPI: Indoor Access Point
+ * @NL80211_REG_SP: Standard power Access Point
+ * @NL80211_REG_VLP: Very low power Access Point

include/uapi/linux/nl80211.h enum nl80211_dfs_regions { · line 4107

Quoted 10 lines
include/uapi/linux/nl80211.h +10 -0
+ */
+enum nl80211_ap_reg_power {
+	NL80211_REG_UNSET_AP,
+	NL80211_REG_LPI_AP,
+	NL80211_REG_SP_AP,
+	NL80211_REG_VLP_AP,
+	NL80211_REG_AP_POWER_AFTER_LAST,
+	NL80211_REG_AP_POWER_MAX =
+		NL80211_REG_AP_POWER_AFTER_LAST - 1,
+};

Similarly here (and the other enum), why is this in nl80211 if it's never used in nl80211?

johannes

maintainer
#3 lore raw

Re: [PATCH v2 1/8] cfg80211: add power type definition for 6 GHz

Quoted 10 lines · Johannes Berg

include/net/cfg80211.h struct cfg80211_chan_def { · line 688

Quoted 7 lines

 struct cfg80211_chan_def {
  struct ieee80211_channel *chan;

include/net/cfg80211.h +1 -0
@@ -684,6 +685,7 @@ struct cfg80211_chan_def {
  	u32 center_freq2;
  	struct ieee80211_edmg edmg;
  	u16 freq1_offset;
+	enum nl80211_ap_reg_power power_type;

I'm not sure why this should be in the chandef, there's no way that anything in cfg80211 is ever using it there, at least in your patches.

Does it even *apply* to a channel? What if I'm connecting to two APs on the same channel (two client interfaces)?

johannes

author
#4 lore raw

Re: [PATCH v2 1/8] cfg80211: add power type definition for 6 GHz

Quoted 12 lines · Johannes Berg
Quoted 10 lines · Johannes Berg

include/net/cfg80211.h struct cfg80211_chan_def { · line 688

Quoted 7 lines

 struct cfg80211_chan_def {
  struct ieee80211_channel *chan;

include/net/cfg80211.h +1 -0
@@ -684,6 +685,7 @@ struct cfg80211_chan_def {
  	u32 center_freq2;
  	struct ieee80211_edmg edmg;
  	u16 freq1_offset;
+	enum nl80211_ap_reg_power power_type;

I'm not sure why this should be in the chandef, there's no way that anything in cfg80211 is ever using it there, at least in your patches.

Quoted 3 lines

Does it even *apply* to a channel? What if I'm connecting to two APs on the same channel (two client interfaces)?

this is one copy for each connection, each client has its own cfg80211_chan_def. also it can be moved to struct ieee80211_bss_conf.

Quoted 1 line

johannes

Signature · 3 lines
ath11k mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/ath11k
maintainer
#5 lore raw

Re: [PATCH v2 1/8] cfg80211: add power type definition for 6 GHz

Quoted 13 lines · Wen Gong
Quoted 12 lines · Johannes Berg
Quoted 10 lines · Johannes Berg

include/net/cfg80211.h struct cfg80211_chan_def { · line 688

Quoted 7 lines

 struct cfg80211_chan_def {
  struct ieee80211_channel *chan;

include/net/cfg80211.h +1 -0
@@ -684,6 +685,7 @@ struct cfg80211_chan_def {
  	u32 center_freq2;
  	struct ieee80211_edmg edmg;
  	u16 freq1_offset;
+	enum nl80211_ap_reg_power power_type;

I'm not sure why this should be in the chandef, there's no way that anything in cfg80211 is ever using it there, at least in your patches.

Quoted 5 lines
Quoted 3 lines

Does it even *apply* to a channel? What if I'm connecting to two APs on the same channel (two client interfaces)?

this is one copy for each connection, each client has its own cfg80211_chan_def.

That depends on where you check it - but you're basically saying "use this only from vif->bss_conf.chandef (or something, didn't check now), but chandef shows up in many other places and you don't maintain it anywhere else.

johannes

author
#6 lore raw

Re: [PATCH v2 1/8] cfg80211: add power type definition for 6 GHz

In reply to Johannes Berg
Quoted 11 lines · Johannes Berg

include/net/cfg80211.h struct cfg80211_chan_def { · line 688

Quoted 7 lines

 struct cfg80211_chan_def {
  struct ieee80211_channel *chan;

include/net/cfg80211.h +1 -0
@@ -684,6 +685,7 @@ struct cfg80211_chan_def {
  	u32 center_freq2;
  	struct ieee80211_edmg edmg;
  	u16 freq1_offset;
+	enum nl80211_ap_reg_power power_type;

I'm not sure why this should be in the chandef, there's no way that anything in cfg80211 is ever using it there, at least in your patches.

It is used in mac80211 of [PATCH v2 3/8] mac80211: add parse regulatory info in 6 GHz operation information. should i move it to mac80211's .h file?

Quoted 12 lines
Quoted 12 lines
include/uapi/linux/nl80211.h +12 -0
+/**
+ * enum nl80211_ap_reg_power - regulatory power for a Access Point
+ *
+ * @NL80211_REG_UNSET_AP: Access Point has no regulatory power mode
+ * @NL80211_REG_LPI: Indoor Access Point
+ * @NL80211_REG_SP: Standard power Access Point
+ * @NL80211_REG_VLP: Very low power Access Point
+ */
+enum nl80211_ap_reg_power {
+	NL80211_REG_UNSET_AP,
+	NL80211_REG_LPI_AP,
+	NL80211_REG_SP_AP,
Quoted 9 lines

include/uapi/linux/nl80211.h enum nl80211_dfs_regions { · line 4107

Quoted 5 lines
include/uapi/linux/nl80211.h +5 -0
+	NL80211_REG_VLP_AP,
+	NL80211_REG_AP_POWER_AFTER_LAST,
+	NL80211_REG_AP_POWER_MAX =
+		NL80211_REG_AP_POWER_AFTER_LAST - 1,
+};

Similarly here (and the other enum), why is this in nl80211 if it's never used in nl80211?

It is used in mac80211 of [PATCH v2 3/8] mac80211: add parse regulatory info in 6 GHz operation information. should i move it to mac80211's .h file?

Quoted 1 line

johannes

maintainer
#7 lore raw

Re: [PATCH v2 1/8] cfg80211: add power type definition for 6 GHz

Quoted 17 lines · Wen Gong
Quoted 11 lines · Johannes Berg

include/net/cfg80211.h struct cfg80211_chan_def { · line 688

Quoted 7 lines

 struct cfg80211_chan_def {
  struct ieee80211_channel *chan;

include/net/cfg80211.h +1 -0
@@ -684,6 +685,7 @@ struct cfg80211_chan_def {
  	u32 center_freq2;
  	struct ieee80211_edmg edmg;
  	u16 freq1_offset;
+	enum nl80211_ap_reg_power power_type;

I'm not sure why this should be in the chandef, there's no way that anything in cfg80211 is ever using it there, at least in your patches.

It is used in mac80211 of [PATCH v2 3/8] mac80211: add parse regulatory info in 6 GHz operation information.
should i move it to mac80211's .h file?

Quoted 2 lines
Quoted 2 lines
include/uapi/linux/nl80211.h +2 -0
+/**
+ * enum nl80211_ap_reg_power - regulatory power for a Access Point

[...]

Quoted 4 lines
Quoted 1 line

It is used in mac80211 of [PATCH v2 3/8] mac80211: add parse regulatory info in 6 GHz operation information.
should i move it to mac80211's .h file?

Yeah I saw both of them are used, but why are they defined as nl80211 API? Do you have any intention to set them through nl80211?

And like I said, I'm not really convinced this belongs into struct cfg80211_chan_def either. Maybe it should be in bss_conf too?

johannes

Quoted 1 line
author
#8 lore raw

Re: [PATCH v2 1/8] cfg80211: add power type definition for 6 GHz

Quoted 24 lines · Johannes Berg
Quoted 17 lines · Wen Gong
Quoted 11 lines · Johannes Berg

include/net/cfg80211.h struct cfg80211_chan_def { · line 688

Quoted 7 lines

 struct cfg80211_chan_def {
  struct ieee80211_channel *chan;

include/net/cfg80211.h +1 -0
@@ -684,6 +685,7 @@ struct cfg80211_chan_def {
  	u32 center_freq2;
  	struct ieee80211_edmg edmg;
  	u16 freq1_offset;
+	enum nl80211_ap_reg_power power_type;

I'm not sure why this should be in the chandef, there's no way that anything in cfg80211 is ever using it there, at least in your patches.

It is used in mac80211 of [PATCH v2 3/8] mac80211: add parse regulatory info in 6 GHz operation information.
should i move it to mac80211's .h file?

Quoted 2 lines
Quoted 2 lines
include/uapi/linux/nl80211.h +2 -0
+/**
+ * enum nl80211_ap_reg_power - regulatory power for a Access Point

[...]

Quoted 4 lines
Quoted 1 line

It is used in mac80211 of [PATCH v2 3/8] mac80211: add parse regulatory info in 6 GHz operation information.
should i move it to mac80211's .h file?

Quoted 5 lines

Yeah I saw both of them are used, but why are they defined as nl80211 API? Do you have any intention to set them through nl80211?

And like I said, I'm not really convinced this belongs into struct cfg80211_chan_def either. Maybe it should be in bss_conf too?

yes, I also want to move it to struct ieee80211_bss_conf.

Quoted 3 lines

johannes

Quoted 1 line
Signature · 3 lines
ath11k mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/ath11k
author
#9 lore raw

PATCH v2 2/8 mac80211: add definition of regulatory info in 6 GHz operation information

In reply to Wen Gong

IEEE Std 802.11ax™-2021 added regulatory info subfield in HE operation element, this patch is to add it in mac80211 definition.

---
 include/linux/ieee80211.h | 4 ++++
 1 file changed, 4 insertions(+)
include/linux/ieee80211.h +4 -0
index 68d8b9cdd3b8..deb2f536d104 100644
@@ -2264,6 +2264,9 @@ ieee80211_he_ppe_size(u8 ppe_thres_hdr, const u8 *phy_cap_info)
 #define IEEE80211_HE_OPERATION_PARTIAL_BSS_COLOR		0x40000000
 #define IEEE80211_HE_OPERATION_BSS_COLOR_DISABLED		0x80000000
 
+#define IEEE80211_6GHZ_CTRL_REG_LPI_AP	0
+#define IEEE80211_6GHZ_CTRL_REG_SP_AP	1
+
 /**
  * ieee80211_he_6ghz_oper - HE 6 GHz operation Information field
  * @primary: primary channel
@@ -2280,6 +2283,7 @@ struct ieee80211_he_6ghz_oper {
 #define		IEEE80211_HE_6GHZ_OPER_CTRL_CHANWIDTH_80MHZ	2
 #define		IEEE80211_HE_6GHZ_OPER_CTRL_CHANWIDTH_160MHZ	3
 #define IEEE80211_HE_6GHZ_OPER_CTRL_DUP_BEACON	0x4
+#define IEEE80211_HE_6GHZ_OPER_CTRL_REG_INFO	0x38
 	u8 control;
 	u8 ccfs0;
 	u8 ccfs1;
Signature · 1 line
2.31.1
maintainer
#10 lore raw

Re: [PATCH v2 2/8] mac80211: add definition of regulatory info in 6 GHz operation information

Quoted 7 lines · Wen Gong

IEEE Std 802.11ax™-2021 added regulatory info subfield in HE operation element, this patch is to add it in mac80211 definition.

---
 include/linux/ieee80211.h | 4 ++++

 1 file changed, 4 insertions(+)

Not really mac80211 (subject), but I'll fix.

johannes

author
#11 lore raw

PATCH v2 3/8 mac80211: add parse regulatory info in 6 GHz operation information

In reply to Wen Gong

This patch is to convert the regulatory info subfield in HE operation element to power type and save in struct cfg80211_chan_def.

---
 net/mac80211/util.c | 10 ++++++++++
 1 file changed, 10 insertions(+)
net/mac80211/util.c +10 -0
index 93d96a4f9c3e..cb1c35d8ef48 100644
@@ -3395,6 +3395,16 @@ bool ieee80211_chandef_he_6ghz_oper(struct ieee80211_sub_if_data *sdata,
 					      NL80211_BAND_6GHZ);
 	he_chandef.chan = ieee80211_get_channel(sdata->local->hw.wiphy, freq);
 
+	switch (u8_get_bits(he_6ghz_oper->control,
+			    IEEE80211_HE_6GHZ_OPER_CTRL_REG_INFO)) {
+	case IEEE80211_6GHZ_CTRL_REG_LPI_AP:
+		he_chandef.power_type = NL80211_REG_LPI_AP;
+		break;
+	case IEEE80211_6GHZ_CTRL_REG_SP_AP:
+		he_chandef.power_type = NL80211_REG_SP_AP;
+		break;
+	}
+
 	switch (u8_get_bits(he_6ghz_oper->control,
 			    IEEE80211_HE_6GHZ_OPER_CTRL_CHANWIDTH)) {
 	case IEEE80211_HE_6GHZ_OPER_CTRL_CHANWIDTH_20MHZ:
Signature · 1 line
2.31.1
maintainer
#12 lore raw

Re: [PATCH v2 3/8] mac80211: add parse regulatory info in 6 GHz operation information

Quoted 17 lines · Wen Gong

This patch is to convert the regulatory info subfield in HE operation element to power type and save in struct cfg80211_chan_def.

---
 net/mac80211/util.c | 10 ++++++++++

 1 file changed, 10 insertions(+)
net/mac80211/util.c +0 -0
index 93d96a4f9c3e..cb1c35d8ef48 100644
@@ -3395,6 +3395,16 @@ bool ieee80211_chandef_he_6ghz_oper(struct ieee80211_sub_if_data *sdata,
  					      NL80211_BAND_6GHZ);
  	he_chandef.chan = ieee80211_get_channel(sdata->local->hw.wiphy, freq);

net/mac80211/util.c bool ieee80211_chandef_he_6ghz_oper(struct ieee80211_sub_if_data *sdata, · line 3406

Quoted 9 lines
net/mac80211/util.c +9 -0
+	switch (u8_get_bits(he_6ghz_oper->control,
+			    IEEE80211_HE_6GHZ_OPER_CTRL_REG_INFO)) {
+	case IEEE80211_6GHZ_CTRL_REG_LPI_AP:
+		he_chandef.power_type = NL80211_REG_LPI_AP;
+		break;
+	case IEEE80211_6GHZ_CTRL_REG_SP_AP:
+		he_chandef.power_type = NL80211_REG_SP_AP;
+		break;
+	}

Now here you put it in the chandef, but don't propagate it anywhere if channels are merged, etc. I don't think this can work correctly in all but the most trivial use cases.

johannes

author
#13 lore raw

PATCH v2 4/8 cfg80211: add definition for 6 GHz power spectral density(psd)

In reply to Wen Gong

6 GHz regulatory domains introduces power spectral density(psd). This patch is define the flags for psd.

---
 include/net/cfg80211.h       | 5 +++++
 include/net/regulatory.h     | 1 +
 include/uapi/linux/nl80211.h | 2 ++
 3 files changed, 8 insertions(+)
include/net/cfg80211.h +5 -0
index f17a4d1202fc..27326e5d5ff3 100644
@@ -108,6 +108,8 @@ struct wiphy;
  *	on this channel.
  * @IEEE80211_CHAN_16MHZ: 16 MHz bandwidth is permitted
  *	on this channel.
+ * @IEEE80211_CHAN_PSD: power spectral density (in dBm)
+ *	on this channel.
  *
  */
 enum ieee80211_channel_flags {
@@ -130,6 +132,7 @@ enum ieee80211_channel_flags {
 	IEEE80211_CHAN_4MHZ		= 1<<16,
 	IEEE80211_CHAN_8MHZ		= 1<<17,
 	IEEE80211_CHAN_16MHZ		= 1<<18,
+	IEEE80211_CHAN_PSD		= 1<<19,
 };
 
 #define IEEE80211_CHAN_NO_HT40 \
@@ -163,6 +166,7 @@ enum ieee80211_channel_flags {
  *	on this channel.
  * @dfs_state_entered: timestamp (jiffies) when the dfs state was entered.
  * @dfs_cac_ms: DFS CAC time in milliseconds, this is valid for DFS channels.
+ * @psd: power spectral density (in dBm)
  */
 struct ieee80211_channel {
 	enum nl80211_band band;
@@ -179,6 +183,7 @@ struct ieee80211_channel {
 	enum nl80211_dfs_state dfs_state;
 	unsigned long dfs_state_entered;
 	unsigned int dfs_cac_ms;
+	s8 psd;
 };
 
 /**
include/net/regulatory.h +1 -0
index 47f06f6f5a67..ed20004fb6a9 100644
@@ -221,6 +221,7 @@ struct ieee80211_reg_rule {
 	u32 flags;
 	u32 dfs_cac_ms;
 	bool has_wmm;
+	s8 psd;
 };
 
 struct ieee80211_regdomain {
include/uapi/linux/nl80211.h +2 -0
index ab1d4aa85272..9ac1dc551cbc 100644
@@ -4043,6 +4043,7 @@ enum nl80211_sched_scan_match_attr {
  * @NL80211_RRF_NO_80MHZ: 80MHz operation not allowed
  * @NL80211_RRF_NO_160MHZ: 160MHz operation not allowed
  * @NL80211_RRF_NO_HE: HE operation not allowed
+ * @NL80211_RRF_PSD: channels has power spectral density value
  */
 enum nl80211_reg_rule_flags {
 	NL80211_RRF_NO_OFDM		= 1<<0,
@@ -4061,6 +4062,7 @@ enum nl80211_reg_rule_flags {
 	NL80211_RRF_NO_80MHZ		= 1<<15,
 	NL80211_RRF_NO_160MHZ		= 1<<16,
 	NL80211_RRF_NO_HE		= 1<<17,
+	NL80211_RRF_PSD		= 1<<18,
 };
 
 #define NL80211_RRF_PASSIVE_SCAN	NL80211_RRF_NO_IR
Signature · 1 line
2.31.1
author
#14 lore raw

PATCH v2 5/8 cfg80211: save power spectral density(psd) of regulatory rule

In reply to Wen Gong

The power spectral density(psd) of regulatory rule should be take effect to the channels. This patch is to save the values to the channel which has psd value.

---
 net/wireless/reg.c | 14 ++++++++++++++
 1 file changed, 14 insertions(+)
net/wireless/reg.c +14 -0
index 0406ce7334fa..602d95e8bde6 100644
@@ -1583,6 +1583,8 @@ static u32 map_regdom_flags(u32 rd_flags)
 		channel_flags |= IEEE80211_CHAN_NO_160MHZ;
 	if (rd_flags & NL80211_RRF_NO_HE)
 		channel_flags |= IEEE80211_CHAN_NO_HE;
+	if (rd_flags & NL80211_RRF_PSD)
+		channel_flags |= IEEE80211_CHAN_PSD;
 	return channel_flags;
 }
 
@@ -1787,6 +1789,9 @@ static void handle_channel_single_rule(struct wiphy *wiphy,
 				chan->dfs_cac_ms = reg_rule->dfs_cac_ms;
 		}
 
+		if (chan->flags & IEEE80211_CHAN_PSD)
+			chan->psd = reg_rule->psd;
+
 		return;
 	}
 
@@ -1807,6 +1812,9 @@ static void handle_channel_single_rule(struct wiphy *wiphy,
 			chan->dfs_cac_ms = IEEE80211_DFS_MIN_CAC_TIME_MS;
 	}
 
+	if (chan->flags & IEEE80211_CHAN_PSD)
+		chan->psd = reg_rule->psd;
+
 	if (chan->orig_mpwr) {
 		/*
 		 * Devices that use REGULATORY_COUNTRY_IE_FOLLOW_POWER
@@ -1876,6 +1884,9 @@ static void handle_channel_adjacent_rules(struct wiphy *wiphy,
 							 rrule2->dfs_cac_ms);
 		}
 
+		if (chan->flags & IEEE80211_CHAN_PSD)
+			chan->psd = min_t(s8, rrule1->psd, rrule1->psd);
+
 		return;
 	}
 
@@ -2533,6 +2544,9 @@ static void handle_channel_custom(struct wiphy *wiphy,
 			chan->dfs_cac_ms = IEEE80211_DFS_MIN_CAC_TIME_MS;
 	}
 
+	if (chan->flags & IEEE80211_CHAN_PSD)
+		chan->psd = reg_rule->psd;
+
 	chan->max_power = chan->max_reg_power;
 }
 
Signature · 7 lines
2.31.1


-- 
ath11k mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/ath11k
maintainer
#15 lore raw

Re: [PATCH v2 5/8] cfg80211: save power spectral density(psd) of regulatory rule

I was going to apply this and patch 4 squashed, saying:

cfg80211: regulatory: handle 6 GHz power spectral density (PSD)

6 GHz regulatory domains introduce power spectral density (PSD). Allow wiphy-specific regulatory rules to specify these values.

but ...

net/wireless/reg.c static void handle_channel_adjacent_rules(struct wiphy *wiphy, · line 1889

Quoted 4 lines
net/wireless/reg.c +3 -0
+		if (chan->flags & IEEE80211_CHAN_PSD)
+			chan->psd = min_t(s8, rrule1->psd, rrule1->psd);
+

This is obviously wrong?

johannes

author
#16 lore raw

Re: [PATCH v2 5/8] cfg80211: save power spectral density(psd) of regulatory rule

Quoted 8 lines · Johannes Berg

I was going to apply this and patch 4 squashed, saying:

cfg80211: regulatory: handle 6 GHz power spectral density (PSD)

6 GHz regulatory domains introduce power spectral density (PSD). Allow wiphy-specific regulatory rules to specify these values.

Quoted 7 lines

but ...

net/wireless/reg.c static void handle_channel_adjacent_rules(struct wiphy *wiphy, · line 1889

Quoted 4 lines
net/wireless/reg.c +3 -0
+		if (chan->flags & IEEE80211_CHAN_PSD)
+			chan->psd = min_t(s8, rrule1->psd, rrule1->psd);
+

This is obviously wrong?

Yes it should change like this:

	if ((rrule1->flags & NL80211_RRF_PSD) && (rrule1->flags & 
NL80211_RRF_PSD))
		chan->psd = min_t(s8, rrule1->psd, rrule1->psd);
	else
		chan->flags &= ~NL80211_RRF_PSD;
Quoted 2 lines

johannes

maintainer
#17 lore raw

Re: [PATCH v2 5/8] cfg80211: save power spectral density(psd) of regulatory rule

Quoted 16 lines · Wen Gong
Quoted 8 lines · Johannes Berg

I was going to apply this and patch 4 squashed, saying:

    cfg80211: regulatory: handle 6 GHz power spectral density (PSD)

    6 GHz regulatory domains introduce power spectral density (PSD). Allow wiphy-specific regulatory rules to specify these values.

Quoted 7 lines

but ...

net/wireless/reg.c static void handle_channel_adjacent_rules(struct wiphy *wiphy, · line 1889

Quoted 4 lines
net/wireless/reg.c +3 -0
+		if (chan->flags & IEEE80211_CHAN_PSD)
+			chan->psd = min_t(s8, rrule1->psd, rrule1->psd);
+

This is obviously wrong?

Quoted 5 lines

Yes it should change like this:

	if ((rrule1->flags & NL80211_RRF_PSD) && (rrule1->flags & 
NL80211_RRF_PSD))
		chan->psd = min_t(s8, rrule1->psd, rrule1->psd);

One of those still should be rrule2 :)

johannes

author
#18 lore raw

Re: [PATCH v2 5/8] cfg80211: save power spectral density(psd) of regulatory rule

Quoted 17 lines · Johannes Berg
Quoted 16 lines · Wen Gong
Quoted 15 lines · Johannes Berg

I was going to apply this and patch 4 squashed, saying:

    cfg80211: regulatory: handle 6 GHz power spectral density (PSD)

    6 GHz regulatory domains introduce power spectral density (PSD). Allow wiphy-specific regulatory rules to specify these values.

but ...

net/wireless/reg.c static void handle_channel_adjacent_rules(struct wiphy *wiphy, · line 1889

Quoted 4 lines
net/wireless/reg.c +3 -0
+		if (chan->flags & IEEE80211_CHAN_PSD)
+			chan->psd = min_t(s8, rrule1->psd, rrule1->psd);
+

This is obviously wrong?

Quoted 8 lines
Quoted 5 lines

Yes it should change like this:

	if ((rrule1->flags & NL80211_RRF_PSD) && (rrule1->flags &
NL80211_RRF_PSD))
		chan->psd = min_t(s8, rrule1->psd, rrule1->psd);

One of those still should be rrule2 :)

yes

    if ((rrule1->flags & NL80211_RRF_PSD) && (rrule2->flags & 
NL80211_RRF_PSD))
        chan->psd = min_t(s8, rrule1->psd, rrule2->psd);
    else
        chan->flags &= ~NL80211_RRF_PSD;
Quoted 1 line

johannes

author
#19 lore raw

PATCH v2 6/8 mac80211: add definition for transmit power envelope element

In reply to Wen Gong

IEEE Std 802.11ax™-2021 have some change for transmit power envelope element. This patch to add the definition of it.

---
 include/linux/ieee80211.h | 39 ++++++++++++++++++++++++++++++++++++++-
 1 file changed, 38 insertions(+), 1 deletion(-)
include/linux/ieee80211.h +38 -1
index deb2f536d104..65f5100052df 100644
@@ -2290,6 +2290,43 @@ struct ieee80211_he_6ghz_oper {
 	u8 minrate;
 } __packed;
 
+/**
+ * In "9.4.2.161 Transmit Power Envelope element" of "IEEE Std 802.11ax-2021",
+ * it show 4 types in "Table 9-275a-Maximum Transmit Power Interpretation subfield encoding".
+ * And it has 2 category for each type in "Table E-12-Regulatory Info subfield encoding in the United States".
+ * So it it totally max 8 Transmit Power Envelope element.
+ */
+#define IEEE80211_TPE_MAX_IE_COUNT	8
+/**
+ * In "Table 9-277—Meaning of Maximum Transmit Power Count subfield"
+ * of "IEEE Std 802.11ax™‐2021", the max power level is 8.
+ */
+#define IEEE80211_MAX_NUM_PWR_LEVEL	8
+
+#define IEEE80211_TPE_MAX_POWER_COUNT	8
+
+/* transmit power interpretation type of transmit power envelope element*/
+enum ieee80211_tx_power_intrpt_type {
+	IEEE80211_TPE_LOCAL_EIRP,
+	IEEE80211_TPE_LOCAL_EIRP_PSD,
+	IEEE80211_TPE_REG_CLIENT_EIRP,
+	IEEE80211_TPE_REG_CLIENT_EIRP_PSD,
+};
+
+/**
+ * struct ieee80211_tx_pwr_env
+ *
+ * This structure represents the "Transmit Power Envelope element"
+ */
+struct ieee80211_tx_pwr_env {
+	u8 tx_power_info;
+	s8 tx_power[IEEE80211_TPE_MAX_POWER_COUNT];
+} __packed;
+
+#define IEEE80211_TX_PWR_ENV_INFO_COUNT 0x7
+#define IEEE80211_TX_PWR_ENV_INFO_INTERPRET 0x38
+#define IEEE80211_TX_PWR_ENV_INFO_CATEGORY 0xC0
+
 /*
  * ieee80211_he_oper_size - calculate 802.11ax HE Operations IE size
  * @he_oper_ie: byte data of the He Operations IE, stating from the byte
@@ -2871,7 +2908,7 @@ enum ieee80211_eid {
 	WLAN_EID_VHT_OPERATION = 192,
 	WLAN_EID_EXTENDED_BSS_LOAD = 193,
 	WLAN_EID_WIDE_BW_CHANNEL_SWITCH = 194,
-	WLAN_EID_VHT_TX_POWER_ENVELOPE = 195,
+	WLAN_EID_TX_POWER_ENVELOPE = 195,
 	WLAN_EID_CHANNEL_SWITCH_WRAPPER = 196,
 	WLAN_EID_AID = 197,
 	WLAN_EID_QUIET_CHANNEL = 198,
Signature · 1 line
2.31.1
maintainer
#20 lore raw

Re: [PATCH v2 6/8] mac80211: add definition for transmit power envelope element

Quoted 3 lines · Wen Gong

IEEE Std 802.11ax™-2021 have some change for transmit power envelope element. This patch to add the definition of it.

Generally: I'm sure Kalle has some page on this, but your commit messages could use some work - you don't need to say "this patch" for example, that's entirely obvious :)

Quoted 2 lines

+/**

not actually kernel-doc, but I can fix

Quoted 4 lines

+ * In "9.4.2.161 Transmit Power Envelope element" of "IEEE Std 802.11ax-2021",
+ * it show 4 types in "Table 9-275a-Maximum Transmit Power Interpretation subfield encoding".
+ * And it has 2 category for each type in "Table E-12-Regulatory Info subfield encoding in the United States".
+ * So it it totally max 8 Transmit Power Envelope element.

very long lines, will fix that too

johannes

author
#21 lore raw

PATCH v2 7/8 mac80211: add parse transmit power envelope element

In reply to Wen Gong

The transmit power envelope elements in beacon is used to calculate the power limit by lower driver, and sometimes it has more than one elements in a beacon frame.

This is to add parse the transmit power envelope elements, then it will be saved and transfer to lower driver to calculate the power limit.

---
 net/mac80211/ieee80211_i.h | 3 +++
 net/mac80211/util.c        | 9 +++++++++
 2 files changed, 12 insertions(+)
net/mac80211/ieee80211_i.h +3 -0
index 648696b49f89..bb62de5e3758 100644
@@ -1494,6 +1494,7 @@ struct ieee802_11_elems {
 	const struct ieee80211_he_spr *he_spr;
 	const struct ieee80211_mu_edca_param_set *mu_edca_param_set;
 	const struct ieee80211_he_6ghz_capa *he_6ghz_capa;
+	const struct ieee80211_tx_pwr_env *tx_pwr_env[IEEE80211_TPE_MAX_IE_COUNT];
 	const u8 *uora_element;
 	const u8 *mesh_id;
 	const u8 *peering;
@@ -1544,6 +1545,8 @@ struct ieee802_11_elems {
 	u8 perr_len;
 	u8 country_elem_len;
 	u8 bssid_index_len;
+	u8 tx_pwr_env_len[IEEE80211_TPE_MAX_IE_COUNT];
+	u8 tx_pwr_env_num;
 
 	/* whether a parse error occurred while retrieving these elements */
 	bool parse_error;
net/mac80211/util.c +9 -0
index cb1c35d8ef48..12c70cc30461 100644
@@ -1336,6 +1336,15 @@ _ieee802_11_parse_elems_crc(const u8 *start, size_t len, bool action,
 			elems->rsnx = pos;
 			elems->rsnx_len = elen;
 			break;
+		case WLAN_EID_TX_POWER_ENVELOPE:
+			if (elems->tx_pwr_env_num >= ARRAY_SIZE(elems->tx_pwr_env) ||
+			    elen < 1)
+				break;
+
+			elems->tx_pwr_env[elems->tx_pwr_env_num] = (void *)pos;
+			elems->tx_pwr_env_len[elems->tx_pwr_env_num] = elen;
+			elems->tx_pwr_env_num++;
+			break;
 		case WLAN_EID_EXTENSION:
 			ieee80211_parse_extension_element(calc_crc ?
 								&crc : NULL,
Signature · 1 line
2.31.1
maintainer
#22 lore raw

Re: [PATCH v2 7/8] mac80211: add parse transmit power envelope element

net/mac80211/util.c _ieee802_11_parse_elems_crc(const u8 *start, size_t len, bool action, · line 1342

Quoted 5 lines · Wen Gong
net/mac80211/util.c +4 -0
+		case WLAN_EID_TX_POWER_ENVELOPE:
+			if (elems->tx_pwr_env_num >= ARRAY_SIZE(elems->tx_pwr_env) ||
+			    elen < 1)
+				break;

IMHO this should also check the max length, but I'll fix that.

johannes

author
#23 lore raw

PATCH v2 8/8 mac80211: save transmit power envelope element and power constraint

In reply to Wen Gong

This is to save the transmit power envelope element and power constraint in struct ieee80211_bss_conf for 6 GHz. Lower driver will use this info to calculate the power limit.

---
 include/net/mac80211.h |  6 ++++++
 net/mac80211/chan.c    |  9 +++++++++
 net/mac80211/mlme.c    | 26 ++++++++++++++++++++++++++
 3 files changed, 41 insertions(+)
include/net/mac80211.h +6 -0
index e89530d0d9c6..6e11e122e364 100644
@@ -631,6 +631,9 @@ struct ieee80211_fils_discovery {
  * @s1g: BSS is S1G BSS (affects Association Request format).
  * @beacon_tx_rate: The configured beacon transmit rate that needs to be passed
  *	to driver when rate control is offloaded to firmware.
+ * @tx_pwr_env: transmit power envelope array of BSS.
+ * @tx_pwr_env_num: number of @tx_pwr_env.
+ * @pwr_reduction: power constraint of BSS.
  */
 struct ieee80211_bss_conf {
 	const u8 *bssid;
@@ -700,6 +703,9 @@ struct ieee80211_bss_conf {
 	u32 unsol_bcast_probe_resp_interval;
 	bool s1g;
 	struct cfg80211_bitrate_mask beacon_tx_rate;
+	struct ieee80211_tx_pwr_env tx_pwr_env[IEEE80211_TPE_MAX_IE_COUNT];
+	u8 tx_pwr_env_num;
+	u8 pwr_reduction;
 };
 
 /**
net/mac80211/chan.c +9 -0
index 907bb1f748a1..149d4ac798f6 100644
@@ -721,6 +721,15 @@ static int ieee80211_assign_vif_chanctx(struct ieee80211_sub_if_data *sdata,
 					 lockdep_is_held(&local->chanctx_mtx));
 
 	if (conf) {
+		if (conf->def.chan->band == NL80211_BAND_6GHZ) {
+			struct ieee80211_bss_conf *bss_conf;
+
+			bss_conf = &sdata->vif.bss_conf;
+			bss_conf->pwr_reduction = 0;
+			bss_conf->tx_pwr_env_num = 0;
+			memset(bss_conf->tx_pwr_env, 0, sizeof(bss_conf->tx_pwr_env));
+		}
+
 		curr_ctx = container_of(conf, struct ieee80211_chanctx, conf);
 
 		drv_unassign_vif_chanctx(local, sdata, curr_ctx);
net/mac80211/mlme.c +26 -0
index 2480bd0577bb..a6d66b4ad7ee 100644
@@ -5070,6 +5070,7 @@ static int ieee80211_prep_channel(struct ieee80211_sub_if_data *sdata,
 
 	if (!(ifmgd->flags & IEEE80211_STA_DISABLE_HE)) {
 		const struct cfg80211_bss_ies *ies;
+		struct ieee80211_bss_conf *bss_conf;
 		const u8 *he_oper_ie;
 
 		ies = rcu_dereference(cbss->ies);
@@ -5081,6 +5082,31 @@ static int ieee80211_prep_channel(struct ieee80211_sub_if_data *sdata,
 		else
 			he_oper = NULL;
 
+		if (is_6ghz) {
+			struct ieee802_11_elems elems;
+			u8 i, j = 0;
+
+			ieee802_11_parse_elems(ies->data, ies->len, false, &elems,
+					       NULL, NULL);
+
+			if (elems.pwr_constr_elem)
+				bss_conf->pwr_reduction = *elems.pwr_constr_elem;
+
+			BUILD_BUG_ON(ARRAY_SIZE(bss_conf->tx_pwr_env) !=
+				     ARRAY_SIZE(elems.tx_pwr_env));
+
+			for (i = 0; i < elems.tx_pwr_env_num; i++) {
+				if (elems.tx_pwr_env_len[i] >
+				    sizeof(bss_conf->tx_pwr_env[j]))
+					continue;
+
+				bss_conf->tx_pwr_env_num++;
+				memcpy(&bss_conf->tx_pwr_env[j], elems.tx_pwr_env[i],
+				       elems.tx_pwr_env_len[i]);
+				j++;
+			}
+		}
+
 		if (!ieee80211_verify_sta_he_mcs_support(sband, he_oper))
 			ifmgd->flags |= IEEE80211_STA_DISABLE_HE;
 	}
Signature · 7 lines
2.31.1


-- 
ath11k mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/ath11k
maintainer
#24 lore raw

Re: [PATCH v2 8/8] mac80211: save transmit power envelope element and power constraint

Review

Quoted 4 lines · Wen Gong

  if (!(ifmgd->flags & IEEE80211_STA_DISABLE_HE)) { const struct cfg80211_bss_ies *ies;

(quoted diff) +1 -0
+		struct ieee80211_bss_conf *bss_conf;

am I missing where you set this?

net/mac80211/mlme.c static int ieee80211_prep_channel(struct ieee80211_sub_if_data *sdata, · line 5086

Quoted 2 lines
net/mac80211/mlme.c +2 -0
+		if (is_6ghz) {
+			struct ieee802_11_elems elems;

This is pretty big, not sure we want it on the stack (causes warnings for me in build). Also, if we're doing this anyway, then we can change the code above (perhaps as a separate patch) to not do cfg80211_find_ext_ie() but rather take it out of the parsed.

net/mac80211/mlme.c static int ieee80211_prep_channel(struct ieee80211_sub_if_data *sdata, · line 5089

Quoted 3 lines
net/mac80211/mlme.c +3 -0
+			u8 i, j = 0;
+
+			ieee802_11_parse_elems(ies->data, ies->len, false, &elems,

(line too long)

net/mac80211/mlme.c static int ieee80211_prep_channel(struct ieee80211_sub_if_data *sdata, · line 5093

Quoted 4 lines
net/mac80211/mlme.c +4 -0
+					       NULL, NULL);
+
+			if (elems.pwr_constr_elem)
+				bss_conf->pwr_reduction = *elems.pwr_constr_elem;

before using it?

net/mac80211/mlme.c static int ieee80211_prep_channel(struct ieee80211_sub_if_data *sdata, · line 5101

Quoted 8 lines
net/mac80211/mlme.c +8 -0
+
+			BUILD_BUG_ON(ARRAY_SIZE(bss_conf->tx_pwr_env) !=
+				     ARRAY_SIZE(elems.tx_pwr_env));
+
+			for (i = 0; i < elems.tx_pwr_env_num; i++) {
+				if (elems.tx_pwr_env_len[i] >
+				    sizeof(bss_conf->tx_pwr_env[j]))
+					continue;

I did that in the parsing itself.

net/mac80211/mlme.c static int ieee80211_prep_channel(struct ieee80211_sub_if_data *sdata, · line 5105

Quoted 4 lines
net/mac80211/mlme.c +4 -0
+
+				bss_conf->tx_pwr_env_num++;
+				memcpy(&bss_conf->tx_pwr_env[j], elems.tx_pwr_env[i],
+				       elems.tx_pwr_env_len[i]);

You're never resetting any of this for the next connection (if it's not 6 GHz for example, or doesn't have any data) - should probably memset all the new members to 0 before the "if (is_6ghz)" or so?

johannes

author
#25 lore raw

Re: [PATCH v2 8/8] mac80211: save transmit power envelope element and power constraint

Quoted 7 lines · Johannes Berg

Review

Quoted 4 lines · Wen Gong

  if (!(ifmgd->flags & IEEE80211_STA_DISABLE_HE)) { const struct cfg80211_bss_ies *ies;

(quoted diff) +1 -0
+		struct ieee80211_bss_conf *bss_conf;

am I missing where you set this?

sorry, i lost it in v2 patch.
bss_conf = &sdata->vif.bss_conf;

Quoted 8 lines

Review

Quoted 3 lines
(quoted diff) +2 -0
+		if (is_6ghz) {
+			struct ieee802_11_elems elems;

This is pretty big, not sure we want it on the stack (causes warnings for me in build). Also, if we're doing this anyway, then we can change the code above (perhaps as a separate patch) to not do cfg80211_find_ext_ie() but rather take it out of the parsed.

will change it.

Quoted 25 lines

Review

Quoted 4 lines
(quoted diff) +3 -0
+			u8 i, j = 0;
+
+			ieee802_11_parse_elems(ies->data, ies->len, false, &elems,

(line too long)

Review

Quoted 4 lines
(quoted diff) +4 -0
+					       NULL, NULL);
+
+			if (elems.pwr_constr_elem)
+				bss_conf->pwr_reduction = *elems.pwr_constr_elem;

before using it?

Review

Quoted 8 lines
(quoted diff) +8 -0
+
+			BUILD_BUG_ON(ARRAY_SIZE(bss_conf->tx_pwr_env) !=
+				     ARRAY_SIZE(elems.tx_pwr_env));
+
+			for (i = 0; i < elems.tx_pwr_env_num; i++) {
+				if (elems.tx_pwr_env_len[i] >
+				    sizeof(bss_conf->tx_pwr_env[j]))
+					continue;

I did that in the parsing itself.

Quoted 9 lines

Review

Quoted 4 lines
(quoted diff) +4 -0
+
+				bss_conf->tx_pwr_env_num++;
+				memcpy(&bss_conf->tx_pwr_env[j], elems.tx_pwr_env[i],
+				       elems.tx_pwr_env_len[i]);

You're never resetting any of this for the next connection (if it's not 6 GHz for example, or doesn't have any data) - should probably memset all the new members to 0 before the "if (is_6ghz)" or so?

it is memset here i this patch:

net/mac80211/chan.c +0 -0
@@ -721,6 +721,15 @@ static int ieee80211_assign_vif_chanctx(struct ieee80211_sub_if_data *sdata,
                                         

lockdep_is_held(&local->chanctx_mtx));

        if (conf) {
+               if (conf->def.chan->band == NL80211_BAND_6GHZ) {
+                       struct ieee80211_bss_conf *bss_conf;
+
+                       bss_conf = &sdata->vif.bss_conf;
+                       bss_conf->pwr_reduction = 0;
+                       bss_conf->tx_pwr_env_num = 0;
+                       memset(bss_conf->tx_pwr_env, 0, sizeof(bss_conf->tx_pwr_env));

+ }
+
curr_ctx = container_of(conf, struct ieee80211_chanctx,
conf);

                drv_unassign_vif_chanctx(local, sdata, curr_ctx);
Quoted 1 line

johannes

Signature · 3 lines
ath11k mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/ath11k
maintainer
#26 lore raw

Re: [PATCH v2 8/8] mac80211: save transmit power envelope element and power constraint

Quoted 2 lines · Wen Gong
Quoted 1 line

it is memset here i this patch:

Oops, missed that.

But is that really a good place for it? Doesn't really seem to belong to assigning a channel context - maybe put it into set_disassoc()?

johannes

author
#27 lore raw

Re: [PATCH v2 8/8] mac80211: save transmit power envelope element and power constraint

Quoted 9 lines · Johannes Berg
Quoted 2 lines · Wen Gong
Quoted 1 line

it is memset here i this patch:

Oops, missed that.

But is that really a good place for it? Doesn't really seem to belong to assigning a channel context - maybe put it into set_disassoc()?

it is correct.
you can see it is place together with "drv_unassign_vif_chanctx(local, sdata, curr_ctx)" in ieee80211_assign_vif_chanctx(), it is for disconnect.

Quoted 1 line

johannes

Signature · 3 lines
ath11k mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/ath11k
maintainer
#28 lore raw

Re: [PATCH v2 8/8] mac80211: save transmit power envelope element and power constraint

Quoted 7 lines · Wen Gong
Quoted 6 lines · Johannes Berg
Quoted 2 lines · Wen Gong
Quoted 1 line

it is memset here i this patch:

Oops, missed that.

Quoted 8 lines
Quoted 4 lines

But is that really a good place for it? Doesn't really seem to belong to
assigning a channel context - maybe put it into set_disassoc()?

it is correct.
you can see it is place together with "drv_unassign_vif_chanctx(local, sdata, curr_ctx)"
in ieee80211_assign_vif_chanctx(), it is for disconnect.

Yes, I know it's *correct*, but that doesn't mean it's *good*?

Look at that code - it does nothing with bss_conf. Nobody is ever going to look there.

johannes

Quoted 1 line
author
#29 lore raw

Re: [PATCH v2 8/8] mac80211: save transmit power envelope element and power constraint

Quoted 17 lines · Johannes Berg
Quoted 7 lines · Wen Gong
Quoted 6 lines · Johannes Berg
Quoted 2 lines · Wen Gong
Quoted 1 line

it is memset here i this patch:

Oops, missed that.

Quoted 8 lines
Quoted 4 lines

But is that really a good place for it? Doesn't really seem to belong to
assigning a channel context - maybe put it into set_disassoc()?

it is correct.
you can see it is place together with "drv_unassign_vif_chanctx(local, sdata, curr_ctx)"
in ieee80211_assign_vif_chanctx(), it is for disconnect.

Quoted 5 lines

Yes, I know it's *correct*, but that doesn't mean it's *good*?

Look at that code - it does nothing with bss_conf. Nobody is ever going to look there.

Yes,
seems it is better do memset() in ieee80211_set_disassoc(). I will change it in next verion.

Quoted 2 lines

johannes

Quoted 1 line
#30 lore raw

Re: [PATCH v2 8/8] mac80211: save transmit power envelope element and power constraint

In reply to Johannes Berg

Hi Johannes,

Quoted 8 lines · Johannes Berg
Quoted 1 line · Wen Gong

it is memset here i this patch:

Oops, missed that.

But is that really a good place for it? Doesn't really seem to belong to assigning a channel context - maybe put it into set_disassoc()?

johannes

We hit buffer overflow issue while connecting to 6 GHz AP fail and fail. Will you fix it? Or do you have any suggestion to fix it?

[  227.539928] wlp90s0: authenticate with 02:03:7f:12:66:66
[  227.601846] wlp90s0: send auth to 02:03:7f:12:66:66 (try 1/3) [  227.633902] wlp90s0: authenticate with 02:03:7f:12:66:66
[  227.633906] wlp90s0: send auth to 02:03:7f:12:66:66 (try 1/3) [  227.657203] wlp90s0: 02:03:7f:12:66:66 denied authentication (status 1)
...
[  263.014661] wlp90s0: authenticate with 02:03:7f:12:66:66
[  263.075667] wlp90s0: send auth to 02:03:7f:12:66:66 (try 1/3) [  263.112427] wlp90s0: authenticate with 02:03:7f:12:66:66
[  263.112433] wlp90s0: send auth to 02:03:7f:12:66:66 (try 1/3) [  263.132507] wlp90s0: 02:03:7f:12:66:66 denied authentication (status 1) [  279.668551] wlp90s0: authenticate with 02:03:7f:12:66:66
[  279.728848] wlp90s0: send auth to 02:03:7f:12:66:66 (try 1/3) [  279.763685] wlp90s0: authenticate with 02:03:7f:12:66:66
[  279.763696] wlp90s0: send auth to 02:03:7f:12:66:66 (try 1/3) [  279.790867] wlp90s0: 02:03:7f:12:66:66 denied authentication (status 1)

for above fail, ieee80211_set_disassoc() is not called, so the bss_conf->tx_pwr_env_num never reset and it is increased each time in ieee80211_prep_channel().

Finally the bss_conf->tx_pwr_env_num arrived to 20+(it should be 1 for correct value), it has exceeded the max value IEEE80211_TPE_MAX_IE_COUNT(8), and lead access bss_conf->tx_pwr_env[IEEE80211_TPE_MAX_IE_COUNT] overflow.

[  327.391621] wlp90s0: authenticate with 02:03:7f:12:66:66
[  327.434036] BUG: kernel NULL pointer dereference, address: 0000000000000018
[  327.434039] #PF: supervisor read access in kernel mode
[  327.434040] #PF: error_code(0x0000) - not-present page
[  327.434042] PGD 0 P4D 0
[  327.434044] Oops: 0000 [#1] PREEMPT SMP NOPTI
[  327.434047] CPU: 2 PID: 804 Comm: wpa_supplicant Kdump: loaded Tainted: G        W          6.2.0-rc8-wt-ath+ #13 [  327.434050] Hardware name: Intel(R) Client Systems NUC11PHi7/NUC11PHBi7, BIOS PHTGL579.0063.2021.0707.1057 07/07/2021 [  327.434052] RIP: 0010:ath12k_mac_fill_reg_tpc_info+0x292/0x3b0 [ath12k]
[  327.434080] RSP: 0018:ffffb7330160f448 EFLAGS: 00010246
[  327.434081] RAX: 0000000000000000 RBX: 0000000000000006 RCX: 00000000005a8f98
[  327.434082] RDX: ffff9c7de1cb7f00 RSI: 00000000006cdf18 RDI: ffff9c7de2000508
[  327.434084] RBP: ffffb7330160f4b8 R08: 0000000000000000 R09: ffff9c7de1cb7f00
[  327.434084] R10: ffff9c7de2000508 R11: ffffb7330160f0e0 R12: ffff9c7dda376090
[  327.434085] R13: 0000000000000001 R14: 0000000000000010 R15: ffff9c7de2002080
[  327.434086] FS:  00007f8258ee9c00(0000) GS:ffff9c8170480000(0000) knlGS:0000000000000000
[  327.434087] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[  327.434088] CR2: 0000000000000018 CR3: 0000000108fec006 CR4: 0000000000770ee0
[  327.434090] PKRU: 55555554
[  327.434090] Call Trace:
[  327.434091]  <TASK>
[  327.434094]  ath12k_mac_vdev_start_restart+0x6c8/0x7f0 [ath12k]
[  327.434104]  ? crypto_alloc_tfm_node+0x60/0x130
[  327.434108]  ath12k_mac_station_add+0x163/0x440 [ath12k]
[  327.434117]  ath12k_mac_handle_link_sta_state.isra.79+0x76/0x420 [ath12k]
[  327.434126]  ath12k_mac_op_sta_state+0x19f/0x350 [ath12k]
[  327.434136]  drv_sta_state+0x89/0x5d0 [mac80211]
[  327.434160]  sta_info_insert_rcu+0x222/0x580 [mac80211]
[  327.434176]  sta_info_insert+0xf/0x20 [mac80211]
[  327.434190]  ieee80211_prep_connection+0x200/0x490 [mac80211]
[  327.434214]  ieee80211_mgd_auth+0x2aa/0x4f0 [mac80211]
[  327.434237]  ? __local_bh_enable_ip+0x3b/0x80
[  327.434239]  ? _raw_spin_unlock_bh+0x1d/0x30
[  327.434243]  ieee80211_auth+0x18/0x20 [mac80211]
[  327.434263]  cfg80211_mlme_auth+0x94/0x180 [cfg80211]
[  327.434297]  nl80211_authenticate+0x392/0x3f0 [cfg80211]
[  327.434315]  genl_family_rcv_msg_doit.isra.19+0xf4/0x120
[  327.434318]  genl_rcv_msg+0x1a5/0x2a0
[  327.434320]  ? __cfg80211_rdev_from_attrs+0x1f0/0x1f0 [cfg80211]
[  327.434336]  ? cfg80211_prepare_cqm.isra.79+0x170/0x170 [cfg80211]
[  327.434352]  ? nl80211_put_signal.part.56+0xd0/0xd0 [cfg80211]
[  327.434368]  ? genl_get_cmd_both+0x60/0x60
[  327.434370]  netlink_rcv_skb+0x5a/0x110
[  327.434372]  genl_rcv+0x28/0x40
[  327.434374]  netlink_unicast+0x1be/0x290
[  327.434375]  netlink_sendmsg+0x377/0x4e0
[  327.434377]  sock_sendmsg+0x9a/0xa0
[  327.434380]  ____sys_sendmsg+0x22b/0x2f0
[  327.434382]  ___sys_sendmsg+0x88/0xd0
[  327.434384]  ? dput+0x5f/0x2e0
[  327.434386]  ? __fsnotify_parent+0x109/0x350
[  327.434389]  __sys_sendmsg+0x6c/0xc0
[  327.434391]  ? __sys_sendmsg+0x6c/0xc0
[  327.434393]  __x64_sys_sendmsg+0x1f/0x30
[  327.434395]  do_syscall_64+0x37/0x90
[  327.434398]  entry_SYSCALL_64_after_hwframe+0x63/0xcd
[  327.434400] RIP: 0033:0x7f8258927b17

Quoted 4 lines
author
#31 lore raw

Re: [PATCH v2 8/8] mac80211: save transmit power envelope element and power constraint

In reply to Johannes Berg

On 2021-08-26 16:29, Johannes Berg wrote:
...

Quoted 8 lines

net/mac80211/mlme.c static int ieee80211_prep_channel(struct ieee80211_sub_if_data *sdata, · line 5086

Quoted 3 lines
net/mac80211/mlme.c +2 -0
+		if (is_6ghz) {
+			struct ieee802_11_elems elems;

This is pretty big, not sure we want it on the stack (causes warnings for me in build). Also, if we're doing this anyway, then we can change the code above (perhaps as a separate patch) to not do cfg80211_find_ext_ie() but rather take it out of the parsed.

do you mean NOT use cfg80211_find_ext_ie()/cfg80211_find_ie() and still use "struct ieee802_11_elems elems" here and move this code to a separate function/patch?
it has more than one tx_pwr_env in one beacon, if we use cfg80211_find_ext_ie()/cfg80211_find_ie(), it need add more logic.

Quoted 1 line

...

Quoted 2 lines

johannes

Signature · 3 lines
ath11k mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/ath11k
maintainer
#32 lore raw

Re: [PATCH v2 8/8] mac80211: save transmit power envelope element and power constraint

Quoted 6 lines · Wen Gong
Quoted 3 lines

Also, if we're doing this anyway, then we can change the code above (perhaps as a separate patch) to not do cfg80211_find_ext_ie() but rather take it out of the parsed.

do you mean NOT use cfg80211_find_ext_ie()/cfg80211_find_ie() and still use "struct ieee802_11_elems elems" here and
move this code to a separate function/patch?

Well, there's an existing place in this function that uses cfg80211_find_ext_ie(), and various uses of ieee80211_bss_get_ie(), so it feels like if we're going to do the full parsing, we should switch all the existing "look up an element" to also use the parsed data instead.

Not the other way around :)

johannes

author
#33 lore raw

Re: [PATCH v2 8/8] mac80211: save transmit power envelope element and power constraint

Quoted 8 lines · Johannes Berg
Quoted 6 lines · Wen Gong
Quoted 3 lines

Also, if we're doing this anyway, then we can change the code above (perhaps as a separate patch) to not do cfg80211_find_ext_ie() but rather take it out of the parsed.

do you mean NOT use cfg80211_find_ext_ie()/cfg80211_find_ie() and still use "struct ieee802_11_elems elems" here and
move this code to a separate function/patch?

Quoted 7 lines

Well, there's an existing place in this function that uses cfg80211_find_ext_ie(), and various uses of ieee80211_bss_get_ie(), so it feels like if we're going to do the full parsing, we should switch all the existing "look up an element" to also use the parsed data instead.

Not the other way around :)

ok.
so it ha 2 way to change, right?
1.
change ieee802_11_parse_elems() to ieee80211_bss_get_ie()

2.
still use ieee802_11_parse_elems(), and change others ieee80211_bss_get_ie()/cfg80211_find_ext_ie() to use the result of ieee802_11_parse_elems()

Quoted 2 lines

johannes

maintainer
#34 lore raw

Re: [PATCH v2 8/8] mac80211: save transmit power envelope element and power constraint

Quoted 11 lines · Wen Gong
Quoted 6 lines

Well, there's an existing place in this function that uses cfg80211_find_ext_ie(), and various uses of ieee80211_bss_get_ie(), so it feels like if we're going to do the full parsing, we should switch all the existing "look up an element" to also use the parsed data instead.

ok.
so it ha 2 way to change, right?
1.
change ieee802_11_parse_elems() to ieee80211_bss_get_ie()

No why?

I think we should make a first patch (that doesn't add TPE yet) that changes the function to ieee80211_parse_elems() and removes all the ieee80211_bss_get_ie() / cfg80211_find_ext_ie() calls in favour of just parsing once, and then looking at the elements there.

Then your TPE patch becomes trivial since the elems are already there?

Quoted 6 lines

2.
still use ieee802_11_parse_elems(), and change others ieee80211_bss_get_ie()/cfg80211_find_ext_ie() to use the result of ieee802_11_parse_elems()

Right!

johannes

author
#35 lore raw

Re: [PATCH v2 8/8] mac80211: save transmit power envelope element and power constraint

Quoted 13 lines · Johannes Berg
Quoted 11 lines · Wen Gong
Quoted 6 lines

Well, there's an existing place in this function that uses cfg80211_find_ext_ie(), and various uses of ieee80211_bss_get_ie(), so it feels like if we're going to do the full parsing, we should switch all the existing "look up an element" to also use the parsed data instead.

ok.
so it ha 2 way to change, right?
1.
change ieee802_11_parse_elems() to ieee80211_bss_get_ie()

Quoted 8 lines

No why?

I think we should make a first patch (that doesn't add TPE yet) that changes the function to ieee80211_parse_elems() and removes all the ieee80211_bss_get_ie() / cfg80211_find_ext_ie() calls in favour of just parsing once, and then looking at the elements there.

Then your TPE patch becomes trivial since the elems are already there?

this patch still needed, because the lower driver need the info. and this patch is save the info to "struct ieee80211_bss_conf *bss_conf" and pass it to lower driver.

Quoted 11 lines
Quoted 7 lines

2.
still use ieee802_11_parse_elems(), and change others ieee80211_bss_get_ie()/cfg80211_find_ext_ie() to use the result of ieee802_11_parse_elems()

Right!

johannes

maintainer
#36 lore raw

Re: [PATCH v2 8/8] mac80211: save transmit power envelope element and power constraint

Quoted 21 lines · Wen Gong
Quoted 20 lines · Johannes Berg
Quoted 11 lines · Wen Gong
Quoted 6 lines

Well, there's an existing place in this function that uses cfg80211_find_ext_ie(), and various uses of ieee80211_bss_get_ie(), so it feels like if we're going to do the full parsing, we should switch all the existing "look up an element" to also use the parsed data instead.

ok.
so it ha 2 way to change, right?
1.
change ieee802_11_parse_elems() to ieee80211_bss_get_ie()

No why?

I think we should make a first patch (that doesn't add TPE yet) that changes the function to ieee80211_parse_elems() and removes all the ieee80211_bss_get_ie() / cfg80211_find_ext_ie() calls in favour of just parsing once, and then looking at the elements there.

Quoted 5 lines
Quoted 1 line

Then your TPE patch becomes trivial since the elems are already there?

this patch still needed, because the lower driver need the info. and this patch is save the info to "struct ieee80211_bss_conf *bss_conf" and
pass it to lower driver.

Of course, but you don't have to deal with parsing etc. in that patch then.

johannes

Signature · 3 lines
ath11k mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/ath11k
author
#37 lore raw

Re: [PATCH v2 8/8] mac80211: save transmit power envelope element and power constraint

Quoted 22 lines · Johannes Berg
Quoted 21 lines · Wen Gong
Quoted 20 lines · Johannes Berg
Quoted 11 lines · Wen Gong
Quoted 6 lines

Well, there's an existing place in this function that uses cfg80211_find_ext_ie(), and various uses of ieee80211_bss_get_ie(), so it feels like if we're going to do the full parsing, we should switch all the existing "look up an element" to also use the parsed data instead.

ok.
so it ha 2 way to change, right?
1.
change ieee802_11_parse_elems() to ieee80211_bss_get_ie()

No why?

I think we should make a first patch (that doesn't add TPE yet) that changes the function to ieee80211_parse_elems() and removes all the ieee80211_bss_get_ie() / cfg80211_find_ext_ie() calls in favour of just parsing once, and then looking at the elements there.

Quoted 9 lines
Quoted 5 lines
Quoted 1 line

Then your TPE patch becomes trivial since the elems are already there?

this patch still needed, because the lower driver need the info. and this patch is save the info to "struct ieee80211_bss_conf *bss_conf" and
pass it to lower driver.

Of course, but you don't have to deal with parsing etc. in that patch then.

yes.
then should I use "struct ieee802_11_elems elems = {0}" or "struct ieee802_11_elems *elems = kzalloc(sizeof(*elems))" in the parsing patch?

Quoted 1 line

johannes

maintainer
#38 lore raw

Re: [PATCH v2 8/8] mac80211: save transmit power envelope element and power constraint

Quoted 4 lines · Wen Gong

then should I use "struct ieee802_11_elems elems = {0}" or "struct ieee802_11_elems *elems = kzalloc(sizeof(*elems))"
in the parsing patch?

Yeah, it's a good question ...

We keep adding stuff here, so it'll be safer to alloc it.

I get a fair number of stack size warnings on the build now (possibly due to the addition of the TPE fields?), and while they're probably fine for now (we get there from nl80211, so no deep stack), it's only going to increase - we have EHT patches already now, for example.

johannes

author
#39 lore raw

Re: [PATCH v2 8/8] mac80211: save transmit power envelope element and power constraint

Quoted 6 lines · Johannes Berg
Quoted 4 lines · Wen Gong

then should I use "struct ieee802_11_elems elems = {0}" or "struct ieee802_11_elems *elems = kzalloc(sizeof(*elems))"
in the parsing patch?

Quoted 9 lines

Yeah, it's a good question ...

We keep adding stuff here, so it'll be safer to alloc it.

I get a fair number of stack size warnings on the build now (possibly due to the addition of the TPE fields?), and while they're probably fine for now (we get there from nl80211, so no deep stack), it's only going to increase - we have EHT patches already now, for example.

the TPE is only 8 pointer in the struct ieee802_11_elems. so stack size warnings should not caused by TPE.

#define IEEE80211_TPE_MAX_IE_COUNT 8
const struct ieee80211_tx_pwr_env *tx_pwr_env[IEEE80211_TPE_MAX_IE_COUNT];

Quoted 1 line

johannes

maintainer
#40 lore raw

Re: [PATCH v2 8/8] mac80211: save transmit power envelope element and power constraint

Quoted 2 lines · Wen Gong

the TPE is only 8 pointer in the struct ieee802_11_elems.

I know, but it's showing _just_ above the threshold for the warning now, so if it was _just_ below the threshold before, adding 64 bytes could still do that. 64 bytes is a lot, after all, the threshold is only 512 I think.

johannes

Signature · 3 lines
ath11k mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/ath11k
author
#41 lore raw

Re: [PATCH v2 8/8] mac80211: save transmit power envelope element and power constraint

Quoted 10 lines · Johannes Berg
Quoted 2 lines · Wen Gong

the TPE is only 8 pointer in the struct ieee802_11_elems.

I know, but it's showing _just_ above the threshold for the warning now, so if it was _just_ below the threshold before, adding 64 bytes could still do that. 64 bytes is a lot, after all, the threshold is only 512 I think.

Yes, I will change to below in next version. "struct ieee802_11_elems *elems = kzalloc(sizeof(*elems))"

Quoted 1 line

johannes

author
#42 lore raw
In reply to Wen Gong

Hi johannes,

Could I get your comments on this v2 patches?

Quoted 36 lines · Wen Gong

v2: change per comments of johannes.
including code style, code logic, patch merge, commit log...

It introduced some new concept:
power type of AP(STANDARD_POWER_AP, INDOOR_AP, VERY_LOW_POWER_AP) power type of STATION(DEFAULT_CLIENT, SUBORDINATE_CLIENT) power spectral density(psd)

This patchset for cfg80211/mac80211 is to add the definition of new concept of 6G and add basic parse of IE(transmit power envelope element) in beacon and save power spectral density(psd) reported by lower-driver for 6G channel, the info will be passed to lower driver when connecting to 6G AP.

Wen Gong (8):
cfg80211: add power type definition for 6 GHz
mac80211: add definition of regulatory info in 6 GHz operation
information
mac80211: add parse regulatory info in 6 GHz operation information
cfg80211: add definition for 6 GHz power spectral density(psd)
cfg80211: save power spectral density(psd) of regulatory rule
mac80211: add definition for transmit power envelope element
mac80211: add parse transmit power envelope element
mac80211: save transmit power envelope element and power constraint

 include/linux/ieee80211.h    | 43 +++++++++++++++++++++++++++++++++++-
 include/net/cfg80211.h       |  7 ++++++
 include/net/mac80211.h       |  6 +++++
 include/net/regulatory.h     |  1 +
 include/uapi/linux/nl80211.h | 36 ++++++++++++++++++++++++++++++
 net/mac80211/chan.c          |  9 ++++++++
 net/mac80211/ieee80211_i.h   |  3 +++
 net/mac80211/mlme.c          | 26 ++++++++++++++++++++++
 net/mac80211/util.c          | 19 ++++++++++++++++
 net/wireless/reg.c           | 14 ++++++++++++
 10 files changed, 163 insertions(+), 1 deletion(-)
Signature · 3 lines
ath11k mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/ath11k