2021-08-20 12:22:04

by Wen Gong

[permalink] [raw]
Subject: [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.

Signed-off-by: Wen Gong <[email protected]>
---
include/net/cfg80211.h | 2 ++
include/uapi/linux/nl80211.h | 34 ++++++++++++++++++++++++++++++++++
2 files changed, 36 insertions(+)

diff --git a/include/net/cfg80211.h b/include/net/cfg80211.h
index 58c2cd417e89..f17a4d1202fc 100644
--- a/include/net/cfg80211.h
+++ b/include/net/cfg80211.h
@@ -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;
};

/*
diff --git a/include/uapi/linux/nl80211.h b/include/uapi/linux/nl80211.h
index f962c06e9818..ab1d4aa85272 100644
--- a/include/uapi/linux/nl80211.h
+++ b/include/uapi/linux/nl80211.h
@@ -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
*
--
2.31.1


2021-08-26 08:23:15

by Johannes Berg

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

On Thu, 2021-08-26 at 10:20 +0200, Johannes Berg wrote:
> >  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;
>
> 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

2021-08-26 08:24:06

by Johannes Berg

[permalink] [raw]
Subject: Re: [PATCH v2 1/8] cfg80211: add power type definition 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;

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.

> +/**
> + * 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,
> +};

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

johannes

2021-08-26 10:57:42

by Wen Gong

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

On 2021-08-26 16:20, Johannes Berg wrote:
>>  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;
>
> 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?
>> +/**
>> + * 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,
>> +};
>
> 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?
> johannes

2021-08-26 10:59:32

by Johannes Berg

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

On Thu, 2021-08-26 at 18:57 +0800, Wen Gong wrote:
> On 2021-08-26 16:20, Johannes Berg wrote:
> > >  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;
> >
> > 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?
> > > +/**
> > > + * enum nl80211_ap_reg_power - regulatory power for a Access Point
[...]
> >
> 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
>

2021-08-26 11:03:00

by Wen Gong

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

On 2021-08-26 18:59, Johannes Berg wrote:
> On Thu, 2021-08-26 at 18:57 +0800, Wen Gong wrote:
>> On 2021-08-26 16:20, Johannes Berg wrote:
>> > >  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;
>> >
>> > 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?
>> > > +/**
>> > > + * enum nl80211_ap_reg_power - regulatory power for a Access Point
> [...]
>> >
>> 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?
yes, I also want to move it to struct ieee80211_bss_conf.
>
> johannes
>>

2021-08-26 11:08:56

by Wen Gong

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

On 2021-08-26 16:22, Johannes Berg wrote:
> On Thu, 2021-08-26 at 10:20 +0200, Johannes Berg wrote:
>> >  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;
>>
>> 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)?
>
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.
> johannes

2021-08-26 11:11:50

by Johannes Berg

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

On Thu, 2021-08-26 at 19:02 +0800, Wen Gong wrote:
> On 2021-08-26 16:22, Johannes Berg wrote:
> > On Thu, 2021-08-26 at 10:20 +0200, Johannes Berg wrote:
> > > >  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;
> > >
> > > 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)?
> >
> 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