[PATCH 0/9] cfg80211/mac80211: Add support for 6GHZ STA for various modes : LPI, SP and VLP

[PATCH 0/9] cfg80211/mac80211: Add support for 6GHZ STA for various modes : LPI, SP and VLP

[Wen Gong]

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 (9):
  cfg80211: add power type definition for 6G Hz
  mac80211: add definition of regulatory info in 6G Hz operation
    information
  mac80211: add parse regulatory info in 6G Hz operation information
  cfg80211: add definition for 6G 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: add transmit power envelope element and power constraint in
    bss_conf
  mac80211: save transmit power envelope element and power constraint

 include/linux/ieee80211.h    | 35 ++++++++++++++++++++++++++++++++++-
 include/net/cfg80211.h       |  7 +++++++
 include/net/mac80211.h       |  6 ++++++
 include/net/regulatory.h     |  1 +
 include/uapi/linux/nl80211.h | 32 ++++++++++++++++++++++++++++++++
 net/mac80211/ieee80211_i.h   |  3 +++
 net/mac80211/mlme.c          | 21 +++++++++++++++++++++
 net/mac80211/util.c          | 18 ++++++++++++++++++
 net/wireless/reg.c           | 14 ++++++++++++++
 9 files changed, 136 insertions(+), 1 deletion(-)

-- 
2.31.1

[PATCH 1/9] cfg80211: add power type definition for 6G Hz

[Wen Gong]

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

Signed-off-by: Wen Gong <wgong@codeaurora.org>
---
 include/net/cfg80211.h       |  2 ++
 include/uapi/linux/nl80211.h | 30 ++++++++++++++++++++++++++++++
 2 files changed, 32 insertions(+)

diff --git a/include/net/cfg80211.h b/include/net/cfg80211.h
index 911fae42b0c0..13d92c643794 100644
--- a/include/net/cfg80211.h
+++ b/include/net/cfg80211.h
@@ -675,6 +675,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 6G
  */
 struct cfg80211_chan_def {
 	struct ieee80211_channel *chan;
@@ -683,6 +684,7 @@ struct cfg80211_chan_def {
 	u32 center_freq2;
 	struct ieee80211_edmg edmg;
 	u16 freq1_offset;
+	enum nl80211_ap_reg_power power_type;
 };
 
 /*
diff --git a/include/uapi/linux/nl80211.h b/include/uapi/linux/nl80211.h
index ac78da99fccd..9f8e9e49a16a 100644
--- a/include/uapi/linux/nl80211.h
+++ b/include/uapi/linux/nl80211.h
@@ -4085,6 +4085,36 @@ 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_MAX_AP_TYPE = 3,
+};
+
+/**
+ * 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_MAX_CLIENT_TYPE = 2,
+};
+
 /**
  * enum nl80211_user_reg_hint_type - type of user regulatory hint
  *
-- 
2.31.1

Re: [PATCH 1/9] cfg80211: add power type definition for 6G Hz

[Johannes Berg]

Hi,

Sorry it took me so long to look at this. I started earlier, but then
found some questions and then ... sorry.

> 
> +/**
> + * 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_MAX_AP_TYPE = 3,

That last one is missing docs. Also, why should it be numbered
explicitly? Better add something like

	NUM_NL80211_REG_POWER_TYPE,
	NL80211_REG_MAX_TYPE = NUM_NL80211_REG_POWER_TYPE - 1

or something?


> +enum nl80211_client_reg_power {
> +	NL80211_REG_UNSET_CLIENT,
> +	NL80211_REG_DEFAULT_CLIENT,
> +	NL80211_REG_SUBORDINATE_CLIENT,
> +	NL80211_REG_MAX_CLIENT_TYPE = 2,

same here.

johannes

[PATCH 2/9] mac80211: add definition of regulatory info in 6G Hz operation information

[Wen Gong]

IEEE P802.11ax™/D8.0 added regulatory info subfield in HE operation
element, this patch is to add it in mac80211 definition.

Signed-off-by: Wen Gong <wgong@codeaurora.org>
---
 include/linux/ieee80211.h | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/include/linux/ieee80211.h b/include/linux/ieee80211.h
index 3d8cf579745d..894a2c4d7cb7 100644
--- a/include/linux/ieee80211.h
+++ b/include/linux/ieee80211.h
@@ -2266,6 +2266,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
@@ -2282,6 +2285,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;
-- 
2.31.1

[PATCH 3/9] mac80211: add parse regulatory info in 6G Hz operation information

[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.

Signed-off-by: Wen Gong <wgong@codeaurora.org>
---
 net/mac80211/util.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/net/mac80211/util.c b/net/mac80211/util.c
index f080fcf60e45..f58136e844a7 100644
--- a/net/mac80211/util.c
+++ b/net/mac80211/util.c
@@ -3405,6 +3405,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:
-- 
2.31.1

Re: [PATCH 3/9] mac80211: add parse regulatory info in 6G Hz operation information

[Johannes Berg]

> 6G Hz
(subject)

You have a few different ways of spelling "6 GHz" in this patchset,
sometimes only 6G, sometimes like this - I'd prefer it be consistently
spelled as "6 GHz" :)

johannes

[PATCH 4/9] cfg80211: add definition for 6G power spectral density(psd)

[Wen Gong]

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

Signed-off-by: Wen Gong <wgong@codeaurora.org>
---
 include/net/cfg80211.h       | 5 +++++
 include/net/regulatory.h     | 1 +
 include/uapi/linux/nl80211.h | 2 ++
 3 files changed, 8 insertions(+)

diff --git a/include/net/cfg80211.h b/include/net/cfg80211.h
index 13d92c643794..2f1769412fd6 100644
--- a/include/net/cfg80211.h
+++ b/include/net/cfg80211.h
@@ -107,6 +107,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 {
@@ -129,6 +131,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 \
@@ -162,6 +165,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;
@@ -178,6 +182,7 @@ struct ieee80211_channel {
 	enum nl80211_dfs_state dfs_state;
 	unsigned long dfs_state_entered;
 	unsigned int dfs_cac_ms;
+	s8 psd;
 };
 
 /**
diff --git a/include/net/regulatory.h b/include/net/regulatory.h
index 47f06f6f5a67..ed20004fb6a9 100644
--- a/include/net/regulatory.h
+++ b/include/net/regulatory.h
@@ -221,6 +221,7 @@ struct ieee80211_reg_rule {
 	u32 flags;
 	u32 dfs_cac_ms;
 	bool has_wmm;
+	s8 psd;
 };
 
 struct ieee80211_regdomain {
diff --git a/include/uapi/linux/nl80211.h b/include/uapi/linux/nl80211.h
index 9f8e9e49a16a..b843ba0afad2 100644
--- a/include/uapi/linux/nl80211.h
+++ b/include/uapi/linux/nl80211.h
@@ -4040,6 +4040,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,
@@ -4058,6 +4059,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
-- 
2.31.1

Re: [PATCH 4/9] cfg80211: add definition for 6G power spectral density(psd)

[Johannes Berg]

On Mon, 2021-05-17 at 16:19 -0400, Wen Gong wrote:
> 
> + * @IEEE80211_CHAN_PSD: power spectral density (in dBm)
> + *	on this channel.

Do we need that? Which really is just another way of asking

> + * @psd: power spectral density (in dBm)

whether or not 0 is a valid value for this?

> +++ b/include/uapi/linux/nl80211.h
> @@ -4040,6 +4040,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

It doesn't seem like we need this, after all, there must be some kind of
attribute for the PSD, and then its presence/absence already indicates
this?

johannes

Re: [PATCH 4/9] cfg80211: add definition for 6G power spectral density(psd)

[Wen Gong]

On 2021-07-23 17:24, Johannes Berg wrote:
> On Mon, 2021-05-17 at 16:19 -0400, Wen Gong wrote:
>> 
>> + * @IEEE80211_CHAN_PSD: power spectral density (in dBm)
>> + *	on this channel.
> 
> Do we need that? Which really is just another way of asking
> 
>> + * @psd: power spectral density (in dBm)
> 
> whether or not 0 is a valid value for this?
yes, 0 is also a valid value.
It also have negative, such as -1dBm.
> 
>> +++ b/include/uapi/linux/nl80211.h
>> @@ -4040,6 +4040,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
> 
> It doesn't seem like we need this, after all, there must be some kind 
> of
> attribute for the PSD, and then its presence/absence already indicates
> this?
Now the psd value 0 is also a valid value, so now we can not consider 
the
psd 0 as NON-PSD, so we need this flag to check it is psd or NON-psd.
> 
> johannes

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

[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.

Signed-off-by: Wen Gong <wgong@codeaurora.org>
---
 net/wireless/reg.c | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/net/wireless/reg.c b/net/wireless/reg.c
index 21536c48deec..270be66a8d3f 100644
--- a/net/wireless/reg.c
+++ b/net/wireless/reg.c
@@ -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;
 }
 
-- 
2.31.1

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

[Johannes Berg]

On Mon, 2021-05-17 at 16:19 -0400, Wen Gong wrote:
> 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.

It seems to me you should also add provisions to allow regdb file from
userspace to have the PSD value?

johannes

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

[Wen Gong]

On 2021-07-23 17:27, Johannes Berg wrote:
> On Mon, 2021-05-17 at 16:19 -0400, Wen Gong wrote:
>> 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.
> 
> It seems to me you should also add provisions to allow regdb file from
> userspace to have the PSD value?
> 
> johannes
This can be added in future.
Also anyone who need the interface can add it :)

[PATCH 6/9] mac80211: add definition for transmit power envelope element

[Wen Gong]

IEEE P802.11ax™/D8.0 have some change for  transmit power envelope
element. This patch to add the definition of it.

Signed-off-by: Wen Gong <wgong@codeaurora.org>
---
 include/linux/ieee80211.h | 31 ++++++++++++++++++++++++++++++-
 1 file changed, 30 insertions(+), 1 deletion(-)

diff --git a/include/linux/ieee80211.h b/include/linux/ieee80211.h
index 894a2c4d7cb7..a39b5fc1dffc 100644
--- a/include/linux/ieee80211.h
+++ b/include/linux/ieee80211.h
@@ -2292,6 +2292,35 @@ struct ieee80211_he_6ghz_oper {
 	u8 minrate;
 } __packed;
 
+#define IEEE80211_TPE_MAX_IE_COUNT	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 TX_PWR_ENV_INFO_COUNT	GENMASK(2, 0)
+#define TX_PWR_ENV_INFO_INTERPRET	GENMASK(5, 3)
+#define TX_PWR_ENV_INFO_CATEGORY	GENMASK(7, 6)
+
+#define GET_TX_PWR_ENV_COUNT(fv) FIELD_GET(TX_PWR_ENV_INFO_COUNT, fv)
+#define GET_TX_PWR_ENV_INTERPRET(fv) FIELD_GET(TX_PWR_ENV_INFO_INTERPRET, fv)
+#define GET_TX_PWR_ENV_CATEGORY(fv) FIELD_GET(TX_PWR_ENV_INFO_CATEGORY, fv)
+
 /*
  * 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
@@ -2873,7 +2902,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,
-- 
2.31.1

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

[Johannes Berg]

On Mon, 2021-05-17 at 16:19 -0400, Wen Gong wrote:
> 
> +#define TX_PWR_ENV_INFO_COUNT	GENMASK(2, 0)
> +#define TX_PWR_ENV_INFO_INTERPRET	GENMASK(5, 3)
> +#define TX_PWR_ENV_INFO_CATEGORY	GENMASK(7, 6)

Personally, I'm not a big fan of GENMASK(), seems more complicated to me
than
			0x0007
			0x0038
			0x00c0

but YMMV :)

We haven't really used GENMASK here anywhere else, have we?


> +#define GET_TX_PWR_ENV_COUNT(fv) FIELD_GET(TX_PWR_ENV_INFO_COUNT, fv)
> +#define GET_TX_PWR_ENV_INTERPRET(fv) FIELD_GET(TX_PWR_ENV_INFO_INTERPRET, fv)
> +#define GET_TX_PWR_ENV_CATEGORY(fv) FIELD_GET(TX_PWR_ENV_INFO_CATEGORY, fv)

I don't think we really need these, and we should be using u*_get_bits()
anyway rather than FIELD_GET.

johannes

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

[Johannes Berg]

On Mon, 2021-05-17 at 16:19 -0400, Wen Gong wrote:
> 
> 
> +#define IEEE80211_TPE_MAX_IE_COUNT	8

Is this actually a spec limit?

johannes

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

[Wen Gong]

On 2021-07-23 17:31, Johannes Berg wrote:
> On Mon, 2021-05-17 at 16:19 -0400, Wen Gong wrote:
>> 
>> 
>> +#define IEEE80211_TPE_MAX_IE_COUNT	8
> 
> Is this actually a spec limit?
> 
> johannes

Yes,
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 8.

[PATCH 7/9] mac80211: add parse transmit power envelope element

[Wen Gong]

This patch is to add the transmit power envelope element parse in
_ieee802_11_parse_elems_crc(), it maybe have more than one transmit
power envelope element in a beacon.

Signed-off-by: Wen Gong <wgong@codeaurora.org>
---
 net/mac80211/ieee80211_i.h | 3 +++
 net/mac80211/util.c        | 8 ++++++++
 2 files changed, 11 insertions(+)

diff --git a/net/mac80211/ieee80211_i.h b/net/mac80211/ieee80211_i.h
index ecda126a7026..22220e8dfdc7 100644
--- a/net/mac80211/ieee80211_i.h
+++ b/net/mac80211/ieee80211_i.h
@@ -1507,6 +1507,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;
@@ -1557,6 +1558,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;
diff --git a/net/mac80211/util.c b/net/mac80211/util.c
index f58136e844a7..585d2eeb4470 100644
--- a/net/mac80211/util.c
+++ b/net/mac80211/util.c
@@ -1344,6 +1344,14 @@ _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))
+				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,
-- 
2.31.1

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

[Johannes Berg]

On Mon, 2021-05-17 at 16:19 -0400, Wen Gong wrote:
> This patch is to add the transmit power envelope element parse in
> _ieee802_11_parse_elems_crc(), it maybe have more than one transmit
> power envelope element in a beacon.

This is really hard to read.

I'm sure you're aware of
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches#commit_messages

Also, FWIW, "This patch" language is pointless. We all know we're
talking about a patch. Or maybe even not, we may be reading the commit
later on.

> +		case WLAN_EID_TX_POWER_ENVELOPE:
> +			if (elems->tx_pwr_env_num >=
> ARRAY_SIZE(elems->tx_pwr_env))
> +				break;
> 

Seems to me this should do some validation on the actual element? It at
least has to have _one_ octet, afaict?

johannes

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

[Wen Gong]

On 2021-07-23 17:33, Johannes Berg wrote:
> On Mon, 2021-05-17 at 16:19 -0400, Wen Gong wrote:
>> This patch is to add the transmit power envelope element parse in
>> _ieee802_11_parse_elems_crc(), it maybe have more than one transmit
>> power envelope element in a beacon.
> 
> This is really hard to read.
> 
> I'm sure you're aware of
> https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches#commit_messages
> 
> Also, FWIW, "This patch" language is pointless. We all know we're
> talking about a patch. Or maybe even not, we may be reading the commit
> later on.

will change it.
> 
>> +		case WLAN_EID_TX_POWER_ENVELOPE:
>> +			if (elems->tx_pwr_env_num >=
>> ARRAY_SIZE(elems->tx_pwr_env))
>> +				break;
>> 
> 
> Seems to me this should do some validation on the actual element? It at
> least has to have _one_ octet, afaict?
> 

will change it.
> johannes

[PATCH 8/9] mac80211: add transmit power envelope element and power constraint in bss_conf

[Wen Gong]

This patch is to add definition of transmit power envelope element and
power constraint in struct ieee80211_bss_conf for 6GHz.

Signed-off-by: Wen Gong <wgong@codeaurora.org>
---
 include/net/mac80211.h | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/include/net/mac80211.h b/include/net/mac80211.h
index 2d1d629e5d14..1e9d3650fbc4 100644
--- a/include/net/mac80211.h
+++ b/include/net/mac80211.h
@@ -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;
 };
 
 /**
-- 
2.31.1

Re: [PATCH 8/9] mac80211: add transmit power envelope element and power constraint in bss_conf

[Johannes Berg]

On Mon, 2021-05-17 at 16:19 -0400, Wen Gong wrote:
> This patch is to add definition of transmit power envelope element and
> power constraint in struct ieee80211_bss_conf for 6GHz.

This does nothing, please squash with the next patch.

johannes

[PATCH 9/9] mac80211: save transmit power envelope element and power constraint

[Wen Gong]

This patch is to save the transmit power envelope element and power
constraint in struct ieee80211_bss_conf for 6GHz.

Signed-off-by: Wen Gong <wgong@codeaurora.org>
---
 net/mac80211/mlme.c | 21 +++++++++++++++++++++
 1 file changed, 21 insertions(+)

diff --git a/net/mac80211/mlme.c b/net/mac80211/mlme.c
index 2e33a1263518..5b02d78bd934 100644
--- a/net/mac80211/mlme.c
+++ b/net/mac80211/mlme.c
@@ -5076,6 +5076,27 @@ static int ieee80211_prep_channel(struct ieee80211_sub_if_data *sdata,
 		else
 			he_oper = NULL;
 
+		if (is_6ghz) {
+			struct ieee802_11_elems elems;
+			struct ieee80211_bss_conf *bss_conf;
+			u8 i, n;
+
+			ieee802_11_parse_elems(ies->data, ies->len, false, &elems,
+					       NULL, NULL);
+			bss_conf = &sdata->vif.bss_conf;
+			bss_conf->pwr_reduction = 0;
+			if (elems.pwr_constr_elem)
+				bss_conf->pwr_reduction = *elems.pwr_constr_elem;
+
+			memset(bss_conf->tx_pwr_env, 0, sizeof(bss_conf->tx_pwr_env));
+			bss_conf->tx_pwr_env_num = elems.tx_pwr_env_num;
+			n = min_t(u8, elems.tx_pwr_env_num,
+				  ARRAY_SIZE(elems.tx_pwr_env));
+			for (i = 0; i < n; i++)
+				memcpy(&bss_conf->tx_pwr_env[i], elems.tx_pwr_env[i],
+				       elems.tx_pwr_env_len[i]);
+		}
+
 		if (!ieee80211_verify_sta_he_mcs_support(sband, he_oper))
 			ifmgd->flags |= IEEE80211_STA_DISABLE_HE;
 	}
-- 
2.31.1

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

[Johannes Berg]

On Mon, 2021-05-17 at 16:19 -0400, Wen Gong wrote:
> 
> +		if (is_6ghz) {
> +			struct ieee802_11_elems elems;
> +			struct ieee80211_bss_conf *bss_conf;
> +			u8 i, n;
> +
> +			ieee802_11_parse_elems(ies->data, ies->len, false, &elems,
> +					       NULL, NULL);
> +			bss_conf = &sdata->vif.bss_conf;
> +			bss_conf->pwr_reduction = 0;
> +			if (elems.pwr_constr_elem)
> +				bss_conf->pwr_reduction = *elems.pwr_constr_elem;
> +
> +			memset(bss_conf->tx_pwr_env, 0, sizeof(bss_conf->tx_pwr_env));
> +			bss_conf->tx_pwr_env_num = elems.tx_pwr_env_num;
> +			n = min_t(u8, elems.tx_pwr_env_num,
> +				  ARRAY_SIZE(elems.tx_pwr_env));

If anything, that min_t would make sense only if you were actually using
ARRAY_SIZE(bss_conf->tx_pwr_env), but like this it's quite pointless,
just checking again if the element parsing was internally consistent?

I'd probably remove it and throw in a

	BUILD_BUG_ON(ARRAY_SIZE(bss_conf->tx_pwr_env) !=
                     ARRAY_SIZE(elems.tx_pwr_env));

instead.

> +			for (i = 0; i < n; i++)
> +				memcpy(&bss_conf->tx_pwr_env[i], elems.tx_pwr_env[i],
> +				       elems.tx_pwr_env_len[i]);

You also never validated that the element wasn't too long!


If you connect to 6 Ghz with this, and then again to another AP that
doesn't, you'll have it stuck at the old values. You need to reset at
some point (during disconnect).

And then two more questions:

1) Could this information change? Should we track it in beacons?

2) Should we at least check it again from the protected beacon or such
after association, so we don't blindly trust the probe response or
beacon (received during scan, not validated) at least when BIGTK is in
use?

johannes

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

[Wen Gong]

On 2021-07-23 17:38, Johannes Berg wrote:
> On Mon, 2021-05-17 at 16:19 -0400, Wen Gong wrote:
>> 
>> +		if (is_6ghz) {
>> +			struct ieee802_11_elems elems;
>> +			struct ieee80211_bss_conf *bss_conf;
>> +			u8 i, n;
>> +
>> +			ieee802_11_parse_elems(ies->data, ies->len, false, &elems,
>> +					       NULL, NULL);
>> +			bss_conf = &sdata->vif.bss_conf;
>> +			bss_conf->pwr_reduction = 0;
>> +			if (elems.pwr_constr_elem)
>> +				bss_conf->pwr_reduction = *elems.pwr_constr_elem;
>> +
>> +			memset(bss_conf->tx_pwr_env, 0, sizeof(bss_conf->tx_pwr_env));
>> +			bss_conf->tx_pwr_env_num = elems.tx_pwr_env_num;
>> +			n = min_t(u8, elems.tx_pwr_env_num,
>> +				  ARRAY_SIZE(elems.tx_pwr_env));
> 
> If anything, that min_t would make sense only if you were actually 
> using
> ARRAY_SIZE(bss_conf->tx_pwr_env), but like this it's quite pointless,
> just checking again if the element parsing was internally consistent?
> 
> I'd probably remove it and throw in a
> 
> 	BUILD_BUG_ON(ARRAY_SIZE(bss_conf->tx_pwr_env) !=
>                      ARRAY_SIZE(elems.tx_pwr_env));
> 
> instead.
> 
>> +			for (i = 0; i < n; i++)
>> +				memcpy(&bss_conf->tx_pwr_env[i], elems.tx_pwr_env[i],
>> +				       elems.tx_pwr_env_len[i]);
> 
> You also never validated that the element wasn't too long!
> 
will change it.
> 
> If you connect to 6 Ghz with this, and then again to another AP that
> doesn't, you'll have it stuck at the old values. You need to reset at
> some point (during disconnect).
> 
will change to reset it in ieee80211_prep_channel outside is_6ghz{}.
Then it will be reset for each connection.
> And then two more questions:
> 
> 1) Could this information change? Should we track it in beacons?
> 

The information is from AP side, it should be not changed untill the AP 
restart.
If someone want to change configure of AP, the AP should restart and 
then take effect by my understand.
Is it have some case for this information change?


> 2) Should we at least check it again from the protected beacon or such
> after association, so we don't blindly trust the probe response or
> beacon (received during scan, not validated) at least when BIGTK is in
> use?

May we add support for BIGTK in future with another patch?
The info(pwr_reduction and tx_pwr_env) is used by lower driver such as 
ath11k.
If the info changed after association, then how to notify lower driver?
Do it like below in ieee80211_rx_mgmt_beacon()?
And use BSS_CHANGED_TXPOWER or a new enum in ieee80211_bss_change?

ieee80211_rx_mgmt_beacon{
	changed |= ieee80211_handle_pwr_constr(sdata, chan, mgmt,
					       elems.country_elem,
					       elems.country_elem_len,
					       elems.pwr_constr_elem,
					       elems.cisco_dtpc_elem);

	ieee80211_bss_info_change_notify(sdata, changed);
}

> 
> johannes

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

[Wen Gong]

Hi johannes,

Could you see my answer below?
please feel free to point out the mistakes :)

On 2021-07-30 18:47, Wen Gong wrote:
> On 2021-07-23 17:38, Johannes Berg wrote:
>> On Mon, 2021-05-17 at 16:19 -0400, Wen Gong wrote:
>>> 
>>> +		if (is_6ghz) {
>>> +			struct ieee802_11_elems elems;
>>> +			struct ieee80211_bss_conf *bss_conf;
>>> +			u8 i, n;
>>> +
>>> +			ieee802_11_parse_elems(ies->data, ies->len, false, &elems,
>>> +					       NULL, NULL);
>>> +			bss_conf = &sdata->vif.bss_conf;
>>> +			bss_conf->pwr_reduction = 0;
>>> +			if (elems.pwr_constr_elem)
>>> +				bss_conf->pwr_reduction = *elems.pwr_constr_elem;
>>> +
>>> +			memset(bss_conf->tx_pwr_env, 0, sizeof(bss_conf->tx_pwr_env));
>>> +			bss_conf->tx_pwr_env_num = elems.tx_pwr_env_num;
>>> +			n = min_t(u8, elems.tx_pwr_env_num,
>>> +				  ARRAY_SIZE(elems.tx_pwr_env));
>> 
>> If anything, that min_t would make sense only if you were actually 
>> using
>> ARRAY_SIZE(bss_conf->tx_pwr_env), but like this it's quite pointless,
>> just checking again if the element parsing was internally consistent?
>> 
>> I'd probably remove it and throw in a
>> 
>> 	BUILD_BUG_ON(ARRAY_SIZE(bss_conf->tx_pwr_env) !=
>>                      ARRAY_SIZE(elems.tx_pwr_env));
>> 
>> instead.
>> 
>>> +			for (i = 0; i < n; i++)
>>> +				memcpy(&bss_conf->tx_pwr_env[i], elems.tx_pwr_env[i],
>>> +				       elems.tx_pwr_env_len[i]);
>> 
>> You also never validated that the element wasn't too long!
>> 
> will change it.
>> 
>> If you connect to 6 Ghz with this, and then again to another AP that
>> doesn't, you'll have it stuck at the old values. You need to reset at
>> some point (during disconnect).
>> 
> will change to reset it in ieee80211_prep_channel outside is_6ghz{}.
> Then it will be reset for each connection.
>> And then two more questions:
>> 
>> 1) Could this information change? Should we track it in beacons?
>> 
> 
> The information is from AP side, it should be not changed untill the AP 
> restart.
> If someone want to change configure of AP, the AP should restart and
> then take effect by my understand.
> Is it have some case for this information change?
> 
> 
>> 2) Should we at least check it again from the protected beacon or such
>> after association, so we don't blindly trust the probe response or
>> beacon (received during scan, not validated) at least when BIGTK is in
>> use?
> 
> May we add support for BIGTK in future with another patch?
> The info(pwr_reduction and tx_pwr_env) is used by lower driver such as 
> ath11k.
> If the info changed after association, then how to notify lower driver?
> Do it like below in ieee80211_rx_mgmt_beacon()?
> And use BSS_CHANGED_TXPOWER or a new enum in ieee80211_bss_change?
> 
> ieee80211_rx_mgmt_beacon{
> 	changed |= ieee80211_handle_pwr_constr(sdata, chan, mgmt,
> 					       elems.country_elem,
> 					       elems.country_elem_len,
> 					       elems.pwr_constr_elem,
> 					       elems.cisco_dtpc_elem);
> 
> 	ieee80211_bss_info_change_notify(sdata, changed);
> }
> 
>> 
>> johannes

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

[Johannes Berg]

On Fri, 2021-07-30 at 18:47 +0800, Wen Gong wrote:
> 
> > And then two more questions:
> > 
> > 1) Could this information change? Should we track it in beacons?
> > 
> 
> The information is from AP side, it should be not changed untill the AP 
> restart.
> If someone want to change configure of AP, the AP should restart and 
> then take effect by my understand.
> Is it have some case for this information change?

No, I guess that's fine then, I just didn't know.

> > 2) Should we at least check it again from the protected beacon or such
> > after association, so we don't blindly trust the probe response or
> > beacon (received during scan, not validated) at least when BIGTK is in
> > use?
> 
> May we add support for BIGTK in future with another patch?

We already have BIGTK support in mac80211, so if we don't do that now
we're almost certainly not going to do it, so I'd really prefer if you
did it here, or if a separate patch still did it now.

> The info(pwr_reduction and tx_pwr_env) is used by lower driver such as 
> ath11k.

Sure.

> If the info changed after association, then how to notify lower driver?
> Do it like below in ieee80211_rx_mgmt_beacon()?
> And use BSS_CHANGED_TXPOWER or a new enum in ieee80211_bss_change?

Yeah, dunno. Are the drivers assuming now it's set once you get to
associated state?

johannes

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

[Johannes Berg]

On Fri, 2021-08-13 at 09:19 +0200, Johannes Berg wrote:
> 
> > > 2) Should we at least check it again from the protected beacon or such
> > > after association, so we don't blindly trust the probe response or
> > > beacon (received during scan, not validated) at least when BIGTK is in
> > > use?
> > 
> > May we add support for BIGTK in future with another patch?
> 
> We already have BIGTK support in mac80211, so if we don't do that now
> we're almost certainly not going to do it, so I'd really prefer if you
> did it here, or if a separate patch still did it now.

Actually, I should say though - the question was more whether we even
need/want that, rather than whether we can do it later or not.

If we should protect this data/information then IMHO we should do it
now, but it's not clear to me that we should, given that we also don't
have encrypted association response and we still take information from
there too, etc.

johannes

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

[Wen Gong]

On 2021-08-13 15:25, Johannes Berg wrote:
> On Fri, 2021-08-13 at 09:19 +0200, Johannes Berg wrote:
>> 
>> > > 2) Should we at least check it again from the protected beacon or such
>> > > after association, so we don't blindly trust the probe response or
>> > > beacon (received during scan, not validated) at least when BIGTK is in
>> > > use?
>> >
>> > May we add support for BIGTK in future with another patch?
>> 
>> We already have BIGTK support in mac80211, so if we don't do that now
>> we're almost certainly not going to do it, so I'd really prefer if you
>> did it here, or if a separate patch still did it now.
> 
> Actually, I should say though - the question was more whether we even
> need/want that, rather than whether we can do it later or not.
> 
> If we should protect this data/information then IMHO we should do it
> now, but it's not clear to me that we should, given that we also don't
> have encrypted association response and we still take information from
> there too, etc.
> 
> johannes
I prefer to add a new enum(not use BSS_CHANGED_TXPOWER),e.g, 
BSS_CHANGED_PWR_ENV.
And add check in ieee80211_rx_mgmt_beacon() as well as 
ieee80211_handle_pwr_constr(),
when the value of pwr_reduction or content of elems.tx_pwr_env changed,
save the pwr_reduction and elems.tx_pwr_env to ieee80211_bss_conf, and 
notify lower
driver with BSS_CHANGED_PWR_ENV, then lower driver will do next action.

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

[Johannes Berg]

On Fri, 2021-08-13 at 16:47 +0800, Wen Gong wrote:
> > > > > 2) Should we at least check it again from the protected beacon or such
> > > > > after association, so we don't blindly trust the probe response or
> > > > > beacon (received during scan, not validated) at least when BIGTK is in
> > > > > use?
> > > > 
> > > > May we add support for BIGTK in future with another patch?
> > > 
> > > We already have BIGTK support in mac80211, so if we don't do that now
> > > we're almost certainly not going to do it, so I'd really prefer if you
> > > did it here, or if a separate patch still did it now.
> > 
> > Actually, I should say though - the question was more whether we even
> > need/want that, rather than whether we can do it later or not.
> > 
> > If we should protect this data/information then IMHO we should do it
> > now, but it's not clear to me that we should, given that we also don't
> > have encrypted association response and we still take information from
> > there too, etc.
> > 
> > johannes
> I prefer to add a new enum(not use BSS_CHANGED_TXPOWER),e.g, 
> BSS_CHANGED_PWR_ENV.
> And add check in ieee80211_rx_mgmt_beacon() as well as 
> ieee80211_handle_pwr_constr(),
> when the value of pwr_reduction or content of elems.tx_pwr_env changed,
> save the pwr_reduction and elems.tx_pwr_env to ieee80211_bss_conf, and 
> notify lower
> driver with BSS_CHANGED_PWR_ENV, then lower driver will do next action.
> 
I don't really have any objection to this, but OTOH it feels like
drivers will probably not really listen to this if it can only happen
due to BIGTK?

And if we always defer this until the first beacon, that also feels
wrong and bad?

I'm not sure what the right answer here is, TBH.

Maybe the right answer is to indeed ignore beacon protection for this,
and do exactly what you did here, and say that the TX power envelope
thing is just not meant to be protected, because the protection is meant
to protect the connection etc. and not the performance (and regulatory?)

Do we get this *only* in the beacon, or also in the association
response? If it's also in the association response we could use the data
from *there*, and basically say that the association response might need
some protection (later) anyway?

johannes

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

[Wen Gong]

On 2021-08-13 16:53, Johannes Berg wrote:
> On Fri, 2021-08-13 at 16:47 +0800, Wen Gong wrote:
>> > > > > 2) Should we at least check it again from the protected beacon or such
>> > > > > after association, so we don't blindly trust the probe response or
>> > > > > beacon (received during scan, not validated) at least when BIGTK is in
>> > > > > use?
>> > > >
>> > > > May we add support for BIGTK in future with another patch?
>> > >
>> > > We already have BIGTK support in mac80211, so if we don't do that now
>> > > we're almost certainly not going to do it, so I'd really prefer if you
>> > > did it here, or if a separate patch still did it now.
>> >
>> > Actually, I should say though - the question was more whether we even
>> > need/want that, rather than whether we can do it later or not.
>> >
>> > If we should protect this data/information then IMHO we should do it
>> > now, but it's not clear to me that we should, given that we also don't
>> > have encrypted association response and we still take information from
>> > there too, etc.
>> >
>> > johannes
>> I prefer to add a new enum(not use BSS_CHANGED_TXPOWER),e.g,
>> BSS_CHANGED_PWR_ENV.
>> And add check in ieee80211_rx_mgmt_beacon() as well as
>> ieee80211_handle_pwr_constr(),
>> when the value of pwr_reduction or content of elems.tx_pwr_env 
>> changed,
>> save the pwr_reduction and elems.tx_pwr_env to ieee80211_bss_conf, and
>> notify lower
>> driver with BSS_CHANGED_PWR_ENV, then lower driver will do next 
>> action.
>> 
> I don't really have any objection to this, but OTOH it feels like
> drivers will probably not really listen to this if it can only happen
> due to BIGTK?
yes, it should have some flag/logic to check whether it is BIGTK.
If you know it, you can tell me. :)
> 
> And if we always defer this until the first beacon, that also feels
> wrong and bad?
It can not defer this untill the 1st beacon which pass BIGTK verify.
Lower driver need this info to set power before TX data include EAPOL.
> 
> I'm not sure what the right answer here is, TBH.
> 
> Maybe the right answer is to indeed ignore beacon protection for this,
> and do exactly what you did here, and say that the TX power envelope
> thing is just not meant to be protected, because the protection is 
> meant
> to protect the connection etc. and not the performance (and 
> regulatory?)
Yes, the lower driver also have the max power limit itself. If power 
calulated
from the fake beacon is bigger than the max power limit, then it will be
ignored.
> 
> Do we get this *only* in the beacon, or also in the association
> response? If it's also in the association response we could use the 
> data
> from *there*, and basically say that the association response might 
> need
> some protection (later) anyway?
> 
The Transmit Power Envelope is not existed in the assoc response, it is 
existed
in beacon. So it can not use assoc response.

beacon:
IEEE 802.11 wireless LAN
     Fixed parameters (12 bytes)
         Timestamp: 0x0000005070684036
         Beacon Interval: 0.102400 [Seconds]
         Capabilities Information: 0x0511
     Tagged parameters (264 bytes)
         Tag: SSID parameter set: Renhui-6G
         Tag: Supported Rates and BSS Membership Selectors 6.0(B), 9, 
12.0(B), 18, 24(B), 36, 48, 54, [Mbit/sec]
         Tag: Traffic Indication Map (TIM): DTIM 0 of
         Tag: Country Information: Country Code US, Environment Unknown 
(0x04)
         Tag: Power Constraint: 3
         Tag: TPC Report Transmit Power: 17, Link Margin: 0
         Tag: Extended Supported Rates and BSS Membership Selectors BSS 
requires support for direct hashing to elements in SAE, [Mbit/sec]
         Tag: RSN Information
         Tag: Extended Capabilities (11 octets)
         Tag: Transmit Power Envelope
         Tag: Transmit Power Envelope
         Ext Tag: Reserved (55)
         Ext Tag: HE Capabilities (IEEE Std 802.11ax/D2.0)
         Ext Tag: HE Operation (IEEE Std 802.11ax/D2.0)
         Ext Tag: Spatial Reuse Parameter Set
         Ext Tag: MU EDCA Parameter Set
         Ext Tag: 6GHz Band Capabilities

assoc response:
IEEE 802.11 wireless LAN
     Fixed parameters (6 bytes)
         Capabilities Information: 0x0511
         Status code: Successful (0x0000)
         ..00 0000 0001 0001 = Association ID: 0x0011
     Tagged parameters (169 bytes)
         Tag: Supported Rates and BSS Membership Selectors 6.0(B), 9, 
12.0(B), 18, 24(B), 36, 48, 54, [Mbit/sec]
         Tag: Extended Supported Rates and BSS Membership Selectors BSS 
requires support for direct hashing to elements in SAE, [Mbit/sec]
         Tag: Extended Capabilities (11 octets)
         Ext Tag: HE Capabilities (IEEE Std 802.11ax/D2.0)
         Ext Tag: HE Operation (IEEE Std 802.11ax/D2.0)
         Ext Tag: Spatial Reuse Parameter Set
         Ext Tag: MU EDCA Parameter Set
         Ext Tag: 6GHz Band Capabilities

> johannes

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

[Johannes Berg]

On Fri, 2021-08-13 at 17:16 +0800, Wen Gong wrote:
> 
> yes, it should have some flag/logic to check whether it is BIGTK.
> If you know it, you can tell me. :)

Uh, actually, we don't have a secure indication of BIGTK getting used
until after the 4-way-HS.

> > 
> Yes, the lower driver also have the max power limit itself. If power 
> calulated
> from the fake beacon is bigger than the max power limit, then it will be
> ignored.

Right.

> > 
> The Transmit Power Envelope is not existed in the assoc response, it is 
> existed
> in beacon. So it can not use assoc response.

Right.


Given this discussion, I think we should just leave it as is, and simply
not assume that the TPE is protected by beacon protection or such. There
are a number of other similar parameters, and doing some real protection
at this level would likely require further spec changes.

johannes

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

[Wen Gong]

On 2021-08-13 18:11, Johannes Berg wrote:
> On Fri, 2021-08-13 at 17:16 +0800, Wen Gong wrote:
>> 
>> yes, it should have some flag/logic to check whether it is BIGTK.
>> If you know it, you can tell me. :)
> 
> Uh, actually, we don't have a secure indication of BIGTK getting used
> until after the 4-way-HS.
> 
>> >
>> Yes, the lower driver also have the max power limit itself. If power
>> calulated
>> from the fake beacon is bigger than the max power limit, then it will 
>> be
>> ignored.
> 
> Right.
> 
>> >
>> The Transmit Power Envelope is not existed in the assoc response, it 
>> is
>> existed
>> in beacon. So it can not use assoc response.
> 
> Right.
> 
> 
> Given this discussion, I think we should just leave it as is, and 
> simply
> not assume that the TPE is protected by beacon protection or such. 
> There
> are a number of other similar parameters, and doing some real 
> protection
> at this level would likely require further spec changes.
> 
Thanks.
I will leave it as is without change for BIGTK.
I will change others patch and send new version.
> johannes

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

[Wen Gong]

On 2021-08-13 15:19, Johannes Berg wrote:
> On Fri, 2021-07-30 at 18:47 +0800, Wen Gong wrote:
>> 
>> > And then two more questions:
>> >
>> > 1) Could this information change? Should we track it in beacons?
>> >
>> 
>> The information is from AP side, it should be not changed untill the 
>> AP
>> restart.
>> If someone want to change configure of AP, the AP should restart and
>> then take effect by my understand.
>> Is it have some case for this information change?
> 
> No, I guess that's fine then, I just didn't know.
> 
>> > 2) Should we at least check it again from the protected beacon or such
>> > after association, so we don't blindly trust the probe response or
>> > beacon (received during scan, not validated) at least when BIGTK is in
>> > use?
>> 
>> May we add support for BIGTK in future with another patch?
> 
> We already have BIGTK support in mac80211, so if we don't do that now
> we're almost certainly not going to do it, so I'd really prefer if you
> did it here, or if a separate patch still did it now.
> 
>> The info(pwr_reduction and tx_pwr_env) is used by lower driver such as
>> ath11k.
> 
> Sure.
> 
>> If the info changed after association, then how to notify lower 
>> driver?
>> Do it like below in ieee80211_rx_mgmt_beacon()?
>> And use BSS_CHANGED_TXPOWER or a new enum in ieee80211_bss_change?
> 
> Yeah, dunno. Are the drivers assuming now it's set once you get to
> associated state?

yes, driver need this info while associate process.

> 
> johannes

Re: [PATCH 0/9] cfg80211/mac80211: Add support for 6GHZ STA for various modes : LPI, SP and VLP

[Wen Gong]

On 2021-05-18 04:19, Wen Gong wrote:
> 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 (9):
>   cfg80211: add power type definition for 6G Hz
>   mac80211: add definition of regulatory info in 6G Hz operation
>     information
>   mac80211: add parse regulatory info in 6G Hz operation information
>   cfg80211: add definition for 6G 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: add transmit power envelope element and power constraint in
>     bss_conf
>   mac80211: save transmit power envelope element and power constraint
> 
...
Nobody have comments?

Re: [PATCH 0/9] cfg80211/mac80211: Add support for 6GHZ STA for various modes : LPI, SP and VLP

[Wen Gong]

On 2021-05-25 18:04, Wen Gong wrote:
> On 2021-05-18 04:19, Wen Gong wrote:
>> 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 (9):
>>   cfg80211: add power type definition for 6G Hz
>>   mac80211: add definition of regulatory info in 6G Hz operation
>>     information
>>   mac80211: add parse regulatory info in 6G Hz operation information
>>   cfg80211: add definition for 6G 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: add transmit power envelope element and power constraint 
>> in
>>     bss_conf
>>   mac80211: save transmit power envelope element and power constraint
>> 
> ...
> Nobody have comments?
Hi Johannes Berg,

Do you have comments?