Hi Johannes,
Newer Intel cards store regulatory settings in firmware.
This patch series adds support for a wiphy callback, allowing
the regulatory core to fetch driver/firmware defined redgomain
data whenever a country code change is requested.
Along with that, there is also a small fix from Eliad.
Arik Nemtsov (6):
cfg80211: don't set reg timeout for user-handled hint
cfg80211: introduce regulatory flags controlling bw
cfg80211: allow drivers to provide regulatory settings
cfg80211: treat the special "unknown" alpha2 as valid
cfg80211: accept world/same regdom from driver/user hints
cfg80211: leave invalid channels on regdomain change
Eliad Peller (1):
regulatory: add NULL to alpha2
include/net/cfg80211.h | 17 ++++
include/net/regulatory.h | 2 +-
include/uapi/linux/nl80211.h | 12 +++
net/wireless/reg.c | 195 ++++++++++++++++++++++++++++++++++++++-----
4 files changed, 205 insertions(+), 21 deletions(-)
--
1.8.3.2
On Sun, 2014-05-11 at 11:50 +0300, Emmanuel Grumbach wrote:
> From: Arik Nemtsov <[email protected]>
>
> Otherwise every "indoor" setting by usermode will cause a regdomain reset.
Applied this one :)
johannes
On Tue, May 20, 2014 at 2:25 AM, Arik Nemtsov <[email protected]> wrote:
>
> Why won't old regdb rules work? The NL80211_RRF_NO_160MHZ for instance
> is not used anywhere in old or new regdbs.
> So all the new code in reg_get_max_bandwidth is ignored in current or
> older crda/regdb flows.
>
> What am I missing?
It will also be ignored on newer kernels using old wireless-regdb.
Luis
On Tue, May 20, 2014 at 11:51 AM, Luis R. Rodriguez
<[email protected]> wrote:
> On Tue, May 20, 2014 at 1:33 AM, Arik Nemtsov <[email protected]> wrote:
>> On Tue, May 20, 2014 at 11:26 AM, Luis R. Rodriguez
>> <[email protected]> wrote:
>>> On Sun, May 11, 2014 at 1:50 AM, Emmanuel Grumbach
>>> <[email protected]> wrote:
>>>> From: Arik Nemtsov <[email protected]>
>>>>
>>>> Allow setting bandwidth related regulatory flags. These flags are mapped
>>>> to the corresponding channel flags in the specified range.
>>>> Make sure the new flags are consulted when calculating the maximum
>>>> bandwidth allowed by a regulatory-rule.
>>>>
>>>> Also allow propagating the GO_CONCURRENT modifier from a reg-rule to a
>>>> channel.
>>>>
>>>> Change-Id: I1b0506ab332cdd268cbf4b02e03574f5c30ba5c0
>>>> Signed-off-by: Arik Nemtsov <[email protected]>
>>>> Signed-off-by: Emmanuel Grumbach <[email protected]>
>>>> ---
>>>> include/uapi/linux/nl80211.h | 12 ++++++++++++
>>>> net/wireless/reg.c | 30 ++++++++++++++++++++++++++++--
>>>> 2 files changed, 40 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git a/include/uapi/linux/nl80211.h b/include/uapi/linux/nl80211.h
>>>> index 406010d..f332231 100644
>>>> --- a/include/uapi/linux/nl80211.h
>>>> +++ b/include/uapi/linux/nl80211.h
>>>> @@ -2558,6 +2558,11 @@ enum nl80211_sched_scan_match_attr {
>>>> * @NL80211_RRF_AUTO_BW: maximum available bandwidth should be calculated
>>>> * base on contiguous rules and wider channels will be allowed to cross
>>>> * multiple contiguous/overlapping frequency ranges.
>>>> + * @NL80211_RRF_GO_CONCURRENT: See &NL80211_FREQUENCY_ATTR_GO_CONCURRENT
>>>> + * @NL80211_RRF_NO_HT40MINUS: channels can't be used in HT40- operation
>>>> + * @NL80211_RRF_NO_HT40PLUS: channels can't be used in HT40+ operation
>>>> + * @NL80211_RRF_NO_80MHZ: 80MHz operation not allowed
>>>> + * @NL80211_RRF_NO_160MHZ: 160MHz operation not allowed
>>>> */
>>>> enum nl80211_reg_rule_flags {
>>>> NL80211_RRF_NO_OFDM = 1<<0,
>>>> @@ -2570,11 +2575,18 @@ enum nl80211_reg_rule_flags {
>>>> NL80211_RRF_NO_IR = 1<<7,
>>>> __NL80211_RRF_NO_IBSS = 1<<8,
>>>> NL80211_RRF_AUTO_BW = 1<<11,
>>>> + NL80211_RRF_GO_CONCURRENT = 1<<12,
>>>> + NL80211_RRF_NO_HT40MINUS = 1<<13,
>>>> + NL80211_RRF_NO_HT40PLUS = 1<<14,
>>>> + NL80211_RRF_NO_80MHZ = 1<<15,
>>>> + NL80211_RRF_NO_160MHZ = 1<<16,
>>>> };
>>>
>>> Automatic calculation on max bandwidth scales better, I'd much prefer
>>> these only be used and properly documented to only be used in cases
>>> where we want to be explicit about this, I don't think this should be
>>> the automatic behavior, or at least your patch in no way explains any
>>> justification as to why the move is being done, so I cannot guess what
>>> the logic was here.
>>
>> We're not going to use CRDA/wireless-regdb as the regulatory data
>> source.
>
> Why not?
There's no good reason I can point to. It's just a decision by Intel
regulatory. This is not only for Linux, but for everything.
>
>> These flags fit the reg-data storage format of the intel FW.
>> It's just a different way of doing things.
>
> You can covert things, we have tons of drivers doing that already.
Again, it's not my call to do this.
>
>>>> #define NL80211_RRF_PASSIVE_SCAN NL80211_RRF_NO_IR
>>>> #define NL80211_RRF_NO_IBSS NL80211_RRF_NO_IR
>>>> #define NL80211_RRF_NO_IR NL80211_RRF_NO_IR
>>>> +#define NL80211_RRF_NO_HT40 (NL80211_RRF_NO_HT40MINUS |\
>>>> + NL80211_RRF_NO_HT40PLUS)
>>>>
>>>> /* For backport compatibility with older userspace */
>>>> #define NL80211_RRF_NO_IR_ALL (NL80211_RRF_NO_IR | __NL80211_RRF_NO_IBSS)
>>>> diff --git a/net/wireless/reg.c b/net/wireless/reg.c
>>>> index 558b0e3..efd6d0d 100644
>>>> --- a/net/wireless/reg.c
>>>> +++ b/net/wireless/reg.c
>>>> @@ -572,8 +572,9 @@ static const struct ieee80211_regdomain *reg_get_regdomain(struct wiphy *wiphy)
>>>> return get_cfg80211_regdom();
>>>> }
>>>>
>>>> -unsigned int reg_get_max_bandwidth(const struct ieee80211_regdomain *rd,
>>>> - const struct ieee80211_reg_rule *rule)
>>>> +static unsigned int
>>>> +reg_get_max_bandwidth_from_range(const struct ieee80211_regdomain *rd,
>>>> + const struct ieee80211_reg_rule *rule)
>>>
>>> This change is renaming a routine we used heavily to something tucked
>>> in the closet and then introducing the usage of the new flags, which
>>> have no justifications nor proper regdb updates, this also doesn't
>>> even account for the old regdb situations so I'll have to provide a
>>> huge NACK for all these reasons.
>>>
>>> You want to consider wireless-regdb / CRDA when making these changes.
>>
>> I can rename things any way you'd like.
>
> You missed my point, you just did not rename something you went and
> then made this new unused flag you introduced be used in tons of
> locations, that'd introduce tons of regressions.
This is on purpose - I want this code to consider the new flags in all
existing locations.
I'm not sure I follow regarding regressions. The new flags are not set
in any existing regulatory database.
>
>> Not sure how this can hurt old
>> regdb situations though. The new flags are not used in any rules
>> there.
>
> Exactly, you can't expect the rules to work then.
Why won't old regdb rules work? The NL80211_RRF_NO_160MHZ for instance
is not used anywhere in old or new regdbs.
So all the new code in reg_get_max_bandwidth is ignored in current or
older crda/regdb flows.
What am I missing?
Arik
>
> Emmanuel Grumbach <[email protected]> writes:
>
> > From: Arik Nemtsov <[email protected]>
> >
> > Otherwise every "indoor" setting by usermode will cause a regdomain
> reset.
> >
> > Change-Id: I3225325d9fb956174458d386d4dd0f4e069deb58
> > Signed-off-by: Arik Nemtsov <[email protected]>
> > Signed-off-by: Emmanuel Grumbach <[email protected]>
>
> All the patches in the series have the useless Change-Id tag.
>
Yeah - I know - I thought I had removed them, but then I had to rebase the patches and they re-appeared. Anyway - I removed the hook from the directory, so that should be the last time I'll do that mistake. Thank you for pointing that out.
Johannes, do you want me to re-submit, or you'll remove them by yourself?
>> The wiphy_apply_custom_regulatory() option is to be used before
>> registering the wiphy. We want to be able to accept country code
>> changes at runtime, with the driver supplying the regdomain.
>
> For which cases exactly at run time? This is already being handled on
> other drivers without changing APIs, so its unclear why you need this
> and to expose this to cfg80211. To be clear, a few drivers other than
> Intel already had this strategy and they managed to just use the
> reg_notifier() and do what it needs by using the flag that ignores
> country IEs, and doing everything else on the driver side. Please
> explore this avenue.
The problem is not propagating the country setting the FW. I agree
this can be done via the notifier. The problem is propagating the
regulatory data from FW to userspace.
For instance we want P2P to be able to use 5Ghz channels in the US.
For this, wpa_s must have up to date information from the regulatory
DB for country=US. For Intel, the regulatory DB is in FW.
>
>> As for why this was chosen - I think you're barking up the wrong tree :)
>> The regulatory folks at Intel decided to store the data in FW,
>
> This has been done for a long time but the main reason why this was
> done that way was that Intel had no need to have tons of regulatory
> domains, and instead had only 4 world regulatory domains, that's all,
> if things have changed it'd be good to understand this and also the
> reasons why things are being done.
This is no longer true. Some variants will now contain settings per county.
>
>> I don't have any say here. I think this is more legal than technology reasons.
>
> Asking these questions, understanding them, and addressing concerns
> are the questions that need to be asked to help advance wireless on
> Linux, it was not asking these questions that got us into trouble in
> the first place, we don't want to go back to that. So even if it is
> non technical and purely regulatory we obviously should ask why.
>
I'm actually an advocate of the CRDA/regdb approach. Less work for us. :)
We presented it but ultimately the decision was theirs, not mine.
I believe the main motivations were security and uniformity across
different OSes. For instance, one might replace the regdb and break
regulatory. So the FW has a regulatory checker that verifies correct
settings. Which in turn means it will hold the regulatory DB anyway.
The practical thing to do is use the same settings as the actual
regdb.
Regards,
Arik
On Sun, May 11, 2014 at 1:50 AM, Emmanuel Grumbach
<[email protected]> wrote:
> Hi Johannes,
>
> Newer Intel cards store regulatory settings in firmware.
> This patch series adds support for a wiphy callback, allowing
> the regulatory core to fetch driver/firmware defined redgomain
> data whenever a country code change is requested.
>
> Along with that, there is also a small fix from Eliad.
>
> Arik Nemtsov (6):
> cfg80211: don't set reg timeout for user-handled hint
> cfg80211: introduce regulatory flags controlling bw
> cfg80211: allow drivers to provide regulatory settings
> cfg80211: treat the special "unknown" alpha2 as valid
> cfg80211: accept world/same regdom from driver/user hints
> cfg80211: leave invalid channels on regdomain change
Please Cc me on regulatory patches. I've noted issues with the wiphy
callback addition, but other than place if you can please resend
patches that do not depend on that, that'd be appreciated, I'll take a
look at those in the morning.
Luis
On Tue, May 20, 2014 at 11:44 AM, Luis R. Rodriguez
<[email protected]> wrote:
> On Sun, May 11, 2014 at 1:50 AM, Emmanuel Grumbach
> <[email protected]> wrote:
>> From: Arik Nemtsov <[email protected]>
>>
>> Define a new wiphy callback allowing drivers to provide regulatory
>> settings.
>>
>> Only The first wiphy registered with this callback will be able to provide
>> regulatory domain info. If such a wiphy exists, it takes precedence over
>> other data sources.
>>
>> Change-Id: I7c7e368e200c1414b53e3a86e131de24adc62b93
>> Signed-off-by: Arik Nemtsov <[email protected]>
>> Signed-off-by: Emmanuel Grumbach <[email protected]>
>> ---
>> include/net/cfg80211.h | 17 +++++++++++++
>> net/wireless/reg.c | 65 +++++++++++++++++++++++++++++++++++++++++++++-----
>> 2 files changed, 76 insertions(+), 6 deletions(-)
>>
>> diff --git a/include/net/cfg80211.h b/include/net/cfg80211.h
>> index f2c3186..3c96b62 100644
>> --- a/include/net/cfg80211.h
>> +++ b/include/net/cfg80211.h
>> @@ -3008,6 +3008,23 @@ struct wiphy {
>> void (*reg_notifier)(struct wiphy *wiphy,
>> struct regulatory_request *request);
>>
>> + /*
>> + * Indicates this wiphy can provide regulatory information.
>> + * Must be set before the wiphy is registered. Only the first
>> + * wiphy with this callback will be called to provide a regdomain
>> + * on country-code changes. The alpha2 in the returned regdomain
>> + * information can be different from the one given via argument,
>> + * if the argument contains the "99" alpha2, meaning unknown.
>> + * If an ERR_PTR is returned the regulatory core will consult other
>> + * sources for the regdomain info (internal regdb and CRDA).
>> + * Returning NULL will cause the regdomain to remain the same.
>> + * The callee will return a struct allocated with kmalloc(). After
>> + * the struct is returned, the regulatory core is responsible
>> + * for freeing it.
>> + */
>
> Use kdoc instead of this long documentation blob.
Sure.
>
>> + struct ieee80211_regdomain * (*get_regd)(struct wiphy *wiphy,
>> + const char *alpha2);
>> +
>> /* fields below are read-only, assigned by cfg80211 */
>>
>> const struct ieee80211_regdomain __rcu *regd;
>
> The driver should be able to dump a regdomain it needs when it needs
> it and use the internal callbacks wiphy_apply_custom_regulatory()
> which tons of drivers already use when it needs to upon
> initialization. As for country IE data you can ignore country IEs,
> there's a flag for this, and do what you want on firmware just as that
> crappy non-upstream vendor qualcomm driver does. Apart from that its
> unclear in this patch why instead any delta observed on wireless-regdb
> is addressed publicly I'd like for this to be considered as a sane
> alternative. Additionally keeping regulatory data in firmware is very
> bug prone and it also doesn't let us grow mature the architecture on
> cfg80211, Intel's firmware has historically only had a few world
> regulatory domains, and historically this is why not much
> contributions from Intel have gone into wireless-regdb, if there is a
> need to support specific countries now on firmware I'd encourage the
> firmware bloat strategy to be seriously reconsidered in light of the
> issues that could arise.
The wiphy_apply_custom_regulatory() option is to be used before
registering the wiphy. We want to be able to accept country code
changes at runtime, with the driver supplying the regdomain.
As for why this was chosen - I think you're barking up the wrong tree :)
The regulatory folks at Intel decided to store the data in FW, I don't
have any say here. I think this is more legal than technology reasons.
Would this patch be acceptable with the documentation changed to kdoc format?
Again, this is a different way of doing things, but it's largely
compatible to the way reg.c already works.
Arik
From: Arik Nemtsov <[email protected]>
Allow setting bandwidth related regulatory flags. These flags are mapped
to the corresponding channel flags in the specified range.
Make sure the new flags are consulted when calculating the maximum
bandwidth allowed by a regulatory-rule.
Also allow propagating the GO_CONCURRENT modifier from a reg-rule to a
channel.
Change-Id: I1b0506ab332cdd268cbf4b02e03574f5c30ba5c0
Signed-off-by: Arik Nemtsov <[email protected]>
Signed-off-by: Emmanuel Grumbach <[email protected]>
---
include/uapi/linux/nl80211.h | 12 ++++++++++++
net/wireless/reg.c | 30 ++++++++++++++++++++++++++++--
2 files changed, 40 insertions(+), 2 deletions(-)
diff --git a/include/uapi/linux/nl80211.h b/include/uapi/linux/nl80211.h
index 406010d..f332231 100644
--- a/include/uapi/linux/nl80211.h
+++ b/include/uapi/linux/nl80211.h
@@ -2558,6 +2558,11 @@ enum nl80211_sched_scan_match_attr {
* @NL80211_RRF_AUTO_BW: maximum available bandwidth should be calculated
* base on contiguous rules and wider channels will be allowed to cross
* multiple contiguous/overlapping frequency ranges.
+ * @NL80211_RRF_GO_CONCURRENT: See &NL80211_FREQUENCY_ATTR_GO_CONCURRENT
+ * @NL80211_RRF_NO_HT40MINUS: channels can't be used in HT40- operation
+ * @NL80211_RRF_NO_HT40PLUS: channels can't be used in HT40+ operation
+ * @NL80211_RRF_NO_80MHZ: 80MHz operation not allowed
+ * @NL80211_RRF_NO_160MHZ: 160MHz operation not allowed
*/
enum nl80211_reg_rule_flags {
NL80211_RRF_NO_OFDM = 1<<0,
@@ -2570,11 +2575,18 @@ enum nl80211_reg_rule_flags {
NL80211_RRF_NO_IR = 1<<7,
__NL80211_RRF_NO_IBSS = 1<<8,
NL80211_RRF_AUTO_BW = 1<<11,
+ NL80211_RRF_GO_CONCURRENT = 1<<12,
+ NL80211_RRF_NO_HT40MINUS = 1<<13,
+ NL80211_RRF_NO_HT40PLUS = 1<<14,
+ NL80211_RRF_NO_80MHZ = 1<<15,
+ NL80211_RRF_NO_160MHZ = 1<<16,
};
#define NL80211_RRF_PASSIVE_SCAN NL80211_RRF_NO_IR
#define NL80211_RRF_NO_IBSS NL80211_RRF_NO_IR
#define NL80211_RRF_NO_IR NL80211_RRF_NO_IR
+#define NL80211_RRF_NO_HT40 (NL80211_RRF_NO_HT40MINUS |\
+ NL80211_RRF_NO_HT40PLUS)
/* For backport compatibility with older userspace */
#define NL80211_RRF_NO_IR_ALL (NL80211_RRF_NO_IR | __NL80211_RRF_NO_IBSS)
diff --git a/net/wireless/reg.c b/net/wireless/reg.c
index 558b0e3..efd6d0d 100644
--- a/net/wireless/reg.c
+++ b/net/wireless/reg.c
@@ -572,8 +572,9 @@ static const struct ieee80211_regdomain *reg_get_regdomain(struct wiphy *wiphy)
return get_cfg80211_regdom();
}
-unsigned int reg_get_max_bandwidth(const struct ieee80211_regdomain *rd,
- const struct ieee80211_reg_rule *rule)
+static unsigned int
+reg_get_max_bandwidth_from_range(const struct ieee80211_regdomain *rd,
+ const struct ieee80211_reg_rule *rule)
{
const struct ieee80211_freq_range *freq_range = &rule->freq_range;
const struct ieee80211_freq_range *freq_range_tmp;
@@ -621,6 +622,21 @@ unsigned int reg_get_max_bandwidth(const struct ieee80211_regdomain *rd,
return end_freq - start_freq;
}
+unsigned int reg_get_max_bandwidth(const struct ieee80211_regdomain *rd,
+ const struct ieee80211_reg_rule *rule)
+{
+ unsigned int bw = reg_get_max_bandwidth_from_range(rd, rule);
+
+ if (rule->flags & NL80211_RRF_NO_160MHZ)
+ bw = min_t(unsigned int, bw, MHZ_TO_KHZ(80));
+ if (rule->flags & NL80211_RRF_NO_80MHZ)
+ bw = min_t(unsigned int, bw, MHZ_TO_KHZ(40));
+ if (rule->flags & NL80211_RRF_NO_HT40)
+ bw = min_t(unsigned int, bw, MHZ_TO_KHZ(20));
+
+ return bw;
+}
+
/* Sanity check on a regulatory rule */
static bool is_valid_reg_rule(const struct ieee80211_reg_rule *rule)
{
@@ -906,6 +922,16 @@ static u32 map_regdom_flags(u32 rd_flags)
channel_flags |= IEEE80211_CHAN_NO_OFDM;
if (rd_flags & NL80211_RRF_NO_OUTDOOR)
channel_flags |= IEEE80211_CHAN_INDOOR_ONLY;
+ if (rd_flags & NL80211_RRF_GO_CONCURRENT)
+ channel_flags |= IEEE80211_CHAN_GO_CONCURRENT;
+ if (rd_flags & NL80211_RRF_NO_HT40MINUS)
+ channel_flags |= IEEE80211_CHAN_NO_HT40MINUS;
+ if (rd_flags & NL80211_RRF_NO_HT40PLUS)
+ channel_flags |= IEEE80211_CHAN_NO_HT40PLUS;
+ if (rd_flags & NL80211_RRF_NO_80MHZ)
+ channel_flags |= IEEE80211_CHAN_NO_80MHZ;
+ if (rd_flags & NL80211_RRF_NO_160MHZ)
+ channel_flags |= IEEE80211_CHAN_NO_160MHZ;
return channel_flags;
}
--
1.8.3.2
From: Arik Nemtsov <[email protected]>
Otherwise every "indoor" setting by usermode will cause a regdomain reset.
Change-Id: I3225325d9fb956174458d386d4dd0f4e069deb58
Signed-off-by: Arik Nemtsov <[email protected]>
Signed-off-by: Emmanuel Grumbach <[email protected]>
---
net/wireless/reg.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/net/wireless/reg.c b/net/wireless/reg.c
index e78f532..558b0e3 100644
--- a/net/wireless/reg.c
+++ b/net/wireless/reg.c
@@ -1876,7 +1876,8 @@ static void reg_process_hint(struct regulatory_request *reg_request)
case NL80211_REGDOM_SET_BY_USER:
treatment = reg_process_hint_user(reg_request);
if (treatment == REG_REQ_IGNORE ||
- treatment == REG_REQ_ALREADY_SET)
+ treatment == REG_REQ_ALREADY_SET ||
+ treatment == REG_REQ_USER_HINT_HANDLED)
return;
queue_delayed_work(system_power_efficient_wq,
®_timeout, msecs_to_jiffies(3142));
--
1.8.3.2
On Tue, May 20, 2014 at 2:12 AM, Arik Nemtsov <[email protected]> wrote:
> On Tue, May 20, 2014 at 11:44 AM, Luis R. Rodriguez
> <[email protected]> wrote:
>> On Sun, May 11, 2014 at 1:50 AM, Emmanuel Grumbach
>> <[email protected]> wrote:
>>> From: Arik Nemtsov <[email protected]>
>>> + struct ieee80211_regdomain * (*get_regd)(struct wiphy *wiphy,
>>> + const char *alpha2);
>>> +
>>> /* fields below are read-only, assigned by cfg80211 */
>>>
>>> const struct ieee80211_regdomain __rcu *regd;
>>
>> The driver should be able to dump a regdomain it needs when it needs
>> it and use the internal callbacks wiphy_apply_custom_regulatory()
>> which tons of drivers already use when it needs to upon
>> initialization. As for country IE data you can ignore country IEs,
>> there's a flag for this, and do what you want on firmware just as that
>> crappy non-upstream vendor qualcomm driver does. Apart from that its
>> unclear in this patch why instead any delta observed on wireless-regdb
>> is addressed publicly I'd like for this to be considered as a sane
>> alternative. Additionally keeping regulatory data in firmware is very
>> bug prone and it also doesn't let us grow mature the architecture on
>> cfg80211, Intel's firmware has historically only had a few world
>> regulatory domains, and historically this is why not much
>> contributions from Intel have gone into wireless-regdb, if there is a
>> need to support specific countries now on firmware I'd encourage the
>> firmware bloat strategy to be seriously reconsidered in light of the
>> issues that could arise.
>
> The wiphy_apply_custom_regulatory() option is to be used before
> registering the wiphy. We want to be able to accept country code
> changes at runtime, with the driver supplying the regdomain.
For which cases exactly at run time? This is already being handled on
other drivers without changing APIs, so its unclear why you need this
and to expose this to cfg80211. To be clear, a few drivers other than
Intel already had this strategy and they managed to just use the
reg_notifier() and do what it needs by using the flag that ignores
country IEs, and doing everything else on the driver side. Please
explore this avenue.
> As for why this was chosen - I think you're barking up the wrong tree :)
> The regulatory folks at Intel decided to store the data in FW,
This has been done for a long time but the main reason why this was
done that way was that Intel had no need to have tons of regulatory
domains, and instead had only 4 world regulatory domains, that's all,
if things have changed it'd be good to understand this and also the
reasons why things are being done.
> I don't have any say here. I think this is more legal than technology reasons.
Asking these questions, understanding them, and addressing concerns
are the questions that need to be asked to help advance wireless on
Linux, it was not asking these questions that got us into trouble in
the first place, we don't want to go back to that. So even if it is
non technical and purely regulatory we obviously should ask why.
> Would this patch be acceptable with the documentation changed to kdoc format?
> Again, this is a different way of doing things, but it's largely
> compatible to the way reg.c already works.
Not yet, you have to explain if you've explored all other avenues
which others have before you have with similar architecture.
Luis
On Sun, May 11, 2014 at 1:50 AM, Emmanuel Grumbach
<[email protected]> wrote:
> From: Arik Nemtsov <[email protected]>
>
> Define a new wiphy callback allowing drivers to provide regulatory
> settings.
>
> Only The first wiphy registered with this callback will be able to provide
> regulatory domain info. If such a wiphy exists, it takes precedence over
> other data sources.
>
> Change-Id: I7c7e368e200c1414b53e3a86e131de24adc62b93
> Signed-off-by: Arik Nemtsov <[email protected]>
> Signed-off-by: Emmanuel Grumbach <[email protected]>
> ---
> include/net/cfg80211.h | 17 +++++++++++++
> net/wireless/reg.c | 65 +++++++++++++++++++++++++++++++++++++++++++++-----
> 2 files changed, 76 insertions(+), 6 deletions(-)
>
> diff --git a/include/net/cfg80211.h b/include/net/cfg80211.h
> index f2c3186..3c96b62 100644
> --- a/include/net/cfg80211.h
> +++ b/include/net/cfg80211.h
> @@ -3008,6 +3008,23 @@ struct wiphy {
> void (*reg_notifier)(struct wiphy *wiphy,
> struct regulatory_request *request);
>
> + /*
> + * Indicates this wiphy can provide regulatory information.
> + * Must be set before the wiphy is registered. Only the first
> + * wiphy with this callback will be called to provide a regdomain
> + * on country-code changes. The alpha2 in the returned regdomain
> + * information can be different from the one given via argument,
> + * if the argument contains the "99" alpha2, meaning unknown.
> + * If an ERR_PTR is returned the regulatory core will consult other
> + * sources for the regdomain info (internal regdb and CRDA).
> + * Returning NULL will cause the regdomain to remain the same.
> + * The callee will return a struct allocated with kmalloc(). After
> + * the struct is returned, the regulatory core is responsible
> + * for freeing it.
> + */
Use kdoc instead of this long documentation blob.
> + struct ieee80211_regdomain * (*get_regd)(struct wiphy *wiphy,
> + const char *alpha2);
> +
> /* fields below are read-only, assigned by cfg80211 */
>
> const struct ieee80211_regdomain __rcu *regd;
The driver should be able to dump a regdomain it needs when it needs
it and use the internal callbacks wiphy_apply_custom_regulatory()
which tons of drivers already use when it needs to upon
initialization. As for country IE data you can ignore country IEs,
there's a flag for this, and do what you want on firmware just as that
crappy non-upstream vendor qualcomm driver does. Apart from that its
unclear in this patch why instead any delta observed on wireless-regdb
is addressed publicly I'd like for this to be considered as a sane
alternative. Additionally keeping regulatory data in firmware is very
bug prone and it also doesn't let us grow mature the architecture on
cfg80211, Intel's firmware has historically only had a few world
regulatory domains, and historically this is why not much
contributions from Intel have gone into wireless-regdb, if there is a
need to support specific countries now on firmware I'd encourage the
firmware bloat strategy to be seriously reconsidered in light of the
issues that could arise.
Luis
On Tue, May 20, 2014 at 12:54 PM, Luis R. Rodriguez
<[email protected]> wrote:
> On Tue, May 20, 2014 at 2:25 AM, Arik Nemtsov <[email protected]> wrote:
>>
>> Why won't old regdb rules work? The NL80211_RRF_NO_160MHZ for instance
>> is not used anywhere in old or new regdbs.
>> So all the new code in reg_get_max_bandwidth is ignored in current or
>> older crda/regdb flows.
>>
>> What am I missing?
>
> It will also be ignored on newer kernels using old wireless-regdb.
Is that a problem?
Note that the new flags don't permit more things, but only narrow down
the range. So if VHT80 was blocked due to the range, it will still be
blocked.
Don't really see a reason to use them in newer regdbs either. Like you
said - range only is more scalable.
These flags are very useful for translating the Intel FW regulatory
format to reg.c format. We don't have ranges there, only flags per
channel. This allows for seamless interop, with per-channel rules.
Regards,
Arik
On Mon, 2014-05-12 at 06:49 +0000, Grumbach, Emmanuel wrote:
> > All the patches in the series have the useless Change-Id tag.
> Johannes, do you want me to re-submit, or you'll remove them by yourself?
I'll deal with it.
johannes
Emmanuel Grumbach <[email protected]> writes:
> From: Arik Nemtsov <[email protected]>
>
> Otherwise every "indoor" setting by usermode will cause a regdomain reset.
>
> Change-Id: I3225325d9fb956174458d386d4dd0f4e069deb58
> Signed-off-by: Arik Nemtsov <[email protected]>
> Signed-off-by: Emmanuel Grumbach <[email protected]>
All the patches in the series have the useless Change-Id tag.
--
Kalle Valo
On Sun, May 11, 2014 at 1:50 AM, Emmanuel Grumbach
<[email protected]> wrote:
> From: Arik Nemtsov <[email protected]>
>
> Otherwise every "indoor" setting by usermode will cause a regdomain reset.
>
> Change-Id: I3225325d9fb956174458d386d4dd0f4e069deb58
> Signed-off-by: Arik Nemtsov <[email protected]>
> Signed-off-by: Emmanuel Grumbach <[email protected]>
> ---
> net/wireless/reg.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/net/wireless/reg.c b/net/wireless/reg.c
> index e78f532..558b0e3 100644
> --- a/net/wireless/reg.c
> +++ b/net/wireless/reg.c
> @@ -1876,7 +1876,8 @@ static void reg_process_hint(struct regulatory_request *reg_request)
> case NL80211_REGDOM_SET_BY_USER:
> treatment = reg_process_hint_user(reg_request);
> if (treatment == REG_REQ_IGNORE ||
> - treatment == REG_REQ_ALREADY_SET)
> + treatment == REG_REQ_ALREADY_SET ||
> + treatment == REG_REQ_USER_HINT_HANDLED)
> return;
> queue_delayed_work(system_power_efficient_wq,
> ®_timeout, msecs_to_jiffies(3142));
This patch would create a conflict with Janusz's patch, but otherwise
it looks fine to me.
Acked-by: Luis R. Rodriguez <[email protected]>
Luis
On Sun, May 11, 2014 at 1:50 AM, Emmanuel Grumbach
<[email protected]> wrote:
> From: Arik Nemtsov <[email protected]>
>
> Allow setting bandwidth related regulatory flags. These flags are mapped
> to the corresponding channel flags in the specified range.
> Make sure the new flags are consulted when calculating the maximum
> bandwidth allowed by a regulatory-rule.
>
> Also allow propagating the GO_CONCURRENT modifier from a reg-rule to a
> channel.
>
> Change-Id: I1b0506ab332cdd268cbf4b02e03574f5c30ba5c0
> Signed-off-by: Arik Nemtsov <[email protected]>
> Signed-off-by: Emmanuel Grumbach <[email protected]>
> ---
> include/uapi/linux/nl80211.h | 12 ++++++++++++
> net/wireless/reg.c | 30 ++++++++++++++++++++++++++++--
> 2 files changed, 40 insertions(+), 2 deletions(-)
>
> diff --git a/include/uapi/linux/nl80211.h b/include/uapi/linux/nl80211.h
> index 406010d..f332231 100644
> --- a/include/uapi/linux/nl80211.h
> +++ b/include/uapi/linux/nl80211.h
> @@ -2558,6 +2558,11 @@ enum nl80211_sched_scan_match_attr {
> * @NL80211_RRF_AUTO_BW: maximum available bandwidth should be calculated
> * base on contiguous rules and wider channels will be allowed to cross
> * multiple contiguous/overlapping frequency ranges.
> + * @NL80211_RRF_GO_CONCURRENT: See &NL80211_FREQUENCY_ATTR_GO_CONCURRENT
> + * @NL80211_RRF_NO_HT40MINUS: channels can't be used in HT40- operation
> + * @NL80211_RRF_NO_HT40PLUS: channels can't be used in HT40+ operation
> + * @NL80211_RRF_NO_80MHZ: 80MHz operation not allowed
> + * @NL80211_RRF_NO_160MHZ: 160MHz operation not allowed
> */
> enum nl80211_reg_rule_flags {
> NL80211_RRF_NO_OFDM = 1<<0,
> @@ -2570,11 +2575,18 @@ enum nl80211_reg_rule_flags {
> NL80211_RRF_NO_IR = 1<<7,
> __NL80211_RRF_NO_IBSS = 1<<8,
> NL80211_RRF_AUTO_BW = 1<<11,
> + NL80211_RRF_GO_CONCURRENT = 1<<12,
> + NL80211_RRF_NO_HT40MINUS = 1<<13,
> + NL80211_RRF_NO_HT40PLUS = 1<<14,
> + NL80211_RRF_NO_80MHZ = 1<<15,
> + NL80211_RRF_NO_160MHZ = 1<<16,
> };
Automatic calculation on max bandwidth scales better, I'd much prefer
these only be used and properly documented to only be used in cases
where we want to be explicit about this, I don't think this should be
the automatic behavior, or at least your patch in no way explains any
justification as to why the move is being done, so I cannot guess what
the logic was here.
> #define NL80211_RRF_PASSIVE_SCAN NL80211_RRF_NO_IR
> #define NL80211_RRF_NO_IBSS NL80211_RRF_NO_IR
> #define NL80211_RRF_NO_IR NL80211_RRF_NO_IR
> +#define NL80211_RRF_NO_HT40 (NL80211_RRF_NO_HT40MINUS |\
> + NL80211_RRF_NO_HT40PLUS)
>
> /* For backport compatibility with older userspace */
> #define NL80211_RRF_NO_IR_ALL (NL80211_RRF_NO_IR | __NL80211_RRF_NO_IBSS)
> diff --git a/net/wireless/reg.c b/net/wireless/reg.c
> index 558b0e3..efd6d0d 100644
> --- a/net/wireless/reg.c
> +++ b/net/wireless/reg.c
> @@ -572,8 +572,9 @@ static const struct ieee80211_regdomain *reg_get_regdomain(struct wiphy *wiphy)
> return get_cfg80211_regdom();
> }
>
> -unsigned int reg_get_max_bandwidth(const struct ieee80211_regdomain *rd,
> - const struct ieee80211_reg_rule *rule)
> +static unsigned int
> +reg_get_max_bandwidth_from_range(const struct ieee80211_regdomain *rd,
> + const struct ieee80211_reg_rule *rule)
This change is renaming a routine we used heavily to something tucked
in the closet and then introducing the usage of the new flags, which
have no justifications nor proper regdb updates, this also doesn't
even account for the old regdb situations so I'll have to provide a
huge NACK for all these reasons.
You want to consider wireless-regdb / CRDA when making these changes.
Luis
From: Arik Nemtsov <[email protected]>
Define a new wiphy callback allowing drivers to provide regulatory
settings.
Only The first wiphy registered with this callback will be able to provide
regulatory domain info. If such a wiphy exists, it takes precedence over
other data sources.
Change-Id: I7c7e368e200c1414b53e3a86e131de24adc62b93
Signed-off-by: Arik Nemtsov <[email protected]>
Signed-off-by: Emmanuel Grumbach <[email protected]>
---
include/net/cfg80211.h | 17 +++++++++++++
net/wireless/reg.c | 65 +++++++++++++++++++++++++++++++++++++++++++++-----
2 files changed, 76 insertions(+), 6 deletions(-)
diff --git a/include/net/cfg80211.h b/include/net/cfg80211.h
index f2c3186..3c96b62 100644
--- a/include/net/cfg80211.h
+++ b/include/net/cfg80211.h
@@ -3008,6 +3008,23 @@ struct wiphy {
void (*reg_notifier)(struct wiphy *wiphy,
struct regulatory_request *request);
+ /*
+ * Indicates this wiphy can provide regulatory information.
+ * Must be set before the wiphy is registered. Only the first
+ * wiphy with this callback will be called to provide a regdomain
+ * on country-code changes. The alpha2 in the returned regdomain
+ * information can be different from the one given via argument,
+ * if the argument contains the "99" alpha2, meaning unknown.
+ * If an ERR_PTR is returned the regulatory core will consult other
+ * sources for the regdomain info (internal regdb and CRDA).
+ * Returning NULL will cause the regdomain to remain the same.
+ * The callee will return a struct allocated with kmalloc(). After
+ * the struct is returned, the regulatory core is responsible
+ * for freeing it.
+ */
+ struct ieee80211_regdomain * (*get_regd)(struct wiphy *wiphy,
+ const char *alpha2);
+
/* fields below are read-only, assigned by cfg80211 */
const struct ieee80211_regdomain __rcu *regd;
diff --git a/net/wireless/reg.c b/net/wireless/reg.c
index efd6d0d..e2f33d7 100644
--- a/net/wireless/reg.c
+++ b/net/wireless/reg.c
@@ -78,6 +78,8 @@
* further processing is required, i.e., not need to update last_request
* etc. This should be used for user hints that do not provide an alpha2
* but some other type of regulatory hint, i.e., indoor operation.
+ * @REG_REQ_HANDLED: a request was handled synchronously. No need to set
+ * timeouts and potentially revert to the default settings.
*/
enum reg_request_treatment {
REG_REQ_OK,
@@ -85,6 +87,7 @@ enum reg_request_treatment {
REG_REQ_INTERSECT,
REG_REQ_ALREADY_SET,
REG_REQ_USER_HINT_HANDLED,
+ REG_REQ_HANDLED,
};
static struct regulatory_request core_request_world = {
@@ -129,6 +132,15 @@ static int reg_num_devs_support_basehint;
*/
static bool reg_is_indoor;
+/*
+ * Wiphy with a get_regd() callback that can provide regulatory information
+ * when the country code changes. Only the first wiphy registered with the
+ * get_regd callback will be called to provide a regdomain on country-code
+ * changes.
+ * (protected by RTNL)
+ */
+static struct wiphy *regd_info_wiphy;
+
static const struct ieee80211_regdomain *get_cfg80211_regdom(void)
{
return rtnl_dereference(cfg80211_regdomain);
@@ -538,9 +550,39 @@ static int call_crda(const char *alpha2)
return kobject_uevent_env(®_pdev->dev.kobj, KOBJ_CHANGE, env);
}
+static int call_wiphy_regd_info(const char *alpha2)
+{
+ struct ieee80211_regdomain *regd;
+
+ if (!regd_info_wiphy)
+ return -ENOENT;
+
+ /* can happen if the driver removes the callback at runtime */
+ if (WARN_ON(!regd_info_wiphy->get_regd))
+ return -EINVAL;
+
+ regd = regd_info_wiphy->get_regd(regd_info_wiphy, alpha2);
+ if (IS_ERR(regd))
+ return -EIO;
+
+ if (regd)
+ set_regdom(regd);
+
+ return 0;
+}
+
static enum reg_request_treatment
-reg_call_crda(struct regulatory_request *request)
+reg_get_regdom_data(struct regulatory_request *request)
{
+ ASSERT_RTNL();
+
+ /*
+ * A wiphy wishing to set the regdomain takes precedence. Note the
+ * regdomain setting happens synchronously inside.
+ */
+ if (!call_wiphy_regd_info(request->alpha2))
+ return REG_REQ_HANDLED;
+
if (call_crda(request->alpha2))
return REG_REQ_IGNORE;
return REG_REQ_OK;
@@ -1641,7 +1683,7 @@ reg_process_hint_core(struct regulatory_request *core_request)
reg_update_last_request(core_request);
- return reg_call_crda(core_request);
+ return reg_get_regdom_data(core_request);
}
static enum reg_request_treatment
@@ -1715,7 +1757,7 @@ reg_process_hint_user(struct regulatory_request *user_request)
user_alpha2[0] = user_request->alpha2[0];
user_alpha2[1] = user_request->alpha2[1];
- return reg_call_crda(user_request);
+ return reg_get_regdom_data(user_request);
}
static enum reg_request_treatment
@@ -1764,6 +1806,7 @@ reg_process_hint_driver(struct wiphy *wiphy,
break;
case REG_REQ_IGNORE:
case REG_REQ_USER_HINT_HANDLED:
+ case REG_REQ_HANDLED:
reg_free_request(driver_request);
return treatment;
case REG_REQ_INTERSECT:
@@ -1794,7 +1837,7 @@ reg_process_hint_driver(struct wiphy *wiphy,
return treatment;
}
- return reg_call_crda(driver_request);
+ return reg_get_regdom_data(driver_request);
}
static enum reg_request_treatment
@@ -1864,6 +1907,7 @@ reg_process_hint_country_ie(struct wiphy *wiphy,
break;
case REG_REQ_IGNORE:
case REG_REQ_USER_HINT_HANDLED:
+ case REG_REQ_HANDLED:
/* fall through */
case REG_REQ_ALREADY_SET:
reg_free_request(country_ie_request);
@@ -1883,7 +1927,7 @@ reg_process_hint_country_ie(struct wiphy *wiphy,
reg_update_last_request(country_ie_request);
- return reg_call_crda(country_ie_request);
+ return reg_get_regdom_data(country_ie_request);
}
/* This processes *all* regulatory hints */
@@ -1903,7 +1947,8 @@ static void reg_process_hint(struct regulatory_request *reg_request)
treatment = reg_process_hint_user(reg_request);
if (treatment == REG_REQ_IGNORE ||
treatment == REG_REQ_ALREADY_SET ||
- treatment == REG_REQ_USER_HINT_HANDLED)
+ treatment == REG_REQ_USER_HINT_HANDLED ||
+ treatment == REG_REQ_HANDLED)
return;
queue_delayed_work(system_power_efficient_wq,
®_timeout, msecs_to_jiffies(3142));
@@ -2684,6 +2729,9 @@ void wiphy_regulatory_register(struct wiphy *wiphy)
{
struct regulatory_request *lr;
+ if (wiphy->get_regd && !regd_info_wiphy)
+ regd_info_wiphy = wiphy;
+
if (!reg_dev_ignore_cell_hint(wiphy))
reg_num_devs_support_basehint++;
@@ -2696,6 +2744,8 @@ void wiphy_regulatory_deregister(struct wiphy *wiphy)
struct wiphy *request_wiphy = NULL;
struct regulatory_request *lr;
+ ASSERT_RTNL();
+
lr = get_last_request();
if (!reg_dev_ignore_cell_hint(wiphy))
@@ -2704,6 +2754,9 @@ void wiphy_regulatory_deregister(struct wiphy *wiphy)
rcu_free_regdom(get_wiphy_regdom(wiphy));
RCU_INIT_POINTER(wiphy->regd, NULL);
+ if (wiphy == regd_info_wiphy)
+ regd_info_wiphy = NULL;
+
if (lr)
request_wiphy = wiphy_idx_to_wiphy(lr->wiphy_idx);
--
1.8.3.2
From: Arik Nemtsov <[email protected]>
When the regulatory settings change, some channels might become invalid.
Disconnect interfaces acting on these channels, after giving userspace
code a grace period to leave them.
Because of cfg80211 channel tracking limitations, only disconnect
STA/P2P_CL interfaces if their current center channel became disabled.
Change-Id: I366a6172b4f3982412e2b47b32fe8cd7ec7d93f4
Signed-off-by: Arik Nemtsov <[email protected]>
Signed-off-by: Emmanuel Grumbach <[email protected]>
---
net/wireless/reg.c | 84 +++++++++++++++++++++++++++++++++++++++++++++++++++++-
1 file changed, 83 insertions(+), 1 deletion(-)
diff --git a/net/wireless/reg.c b/net/wireless/reg.c
index d6b83f9..db942f2 100644
--- a/net/wireless/reg.c
+++ b/net/wireless/reg.c
@@ -221,6 +221,9 @@ struct reg_beacon {
struct ieee80211_channel chan;
};
+static void reg_check_chans_work(struct work_struct *work);
+static DECLARE_DELAYED_WORK(reg_check_chans, reg_check_chans_work);
+
static void reg_todo(struct work_struct *work);
static DECLARE_WORK(reg_work, reg_todo);
@@ -1520,6 +1523,80 @@ static void reg_call_notifier(struct wiphy *wiphy,
wiphy->reg_notifier(wiphy, request);
}
+static bool reg_wdev_chan_valid(struct wiphy *wiphy, struct wireless_dev *wdev)
+{
+ struct ieee80211_channel *ch;
+ bool ret = true;
+
+ wdev_lock(wdev);
+
+ if (!wdev->netdev || !netif_running(wdev->netdev))
+ goto out;
+
+ switch (wdev->iftype) {
+ case NL80211_IFTYPE_AP:
+ case NL80211_IFTYPE_P2P_GO:
+ if (!wdev->beacon_interval)
+ goto out;
+
+ ret = cfg80211_reg_can_beacon(wiphy,
+ &wdev->chandef, wdev->iftype);
+ break;
+ case NL80211_IFTYPE_STATION:
+ case NL80211_IFTYPE_P2P_CLIENT:
+ if (!wdev->current_bss ||
+ !wdev->current_bss->pub.channel)
+ goto out;
+
+ /* TODO: more elaborate tracking for wide channels */
+ ch = wdev->current_bss->pub.channel;
+ ret = !(ch->flags & IEEE80211_CHAN_DISABLED);
+ break;
+ default:
+ /* others not implemented for now */
+ break;
+ }
+
+out:
+ wdev_unlock(wdev);
+ return ret;
+}
+
+static void reg_leave_invalid_chans(struct wiphy *wiphy)
+{
+ struct wireless_dev *wdev;
+ struct cfg80211_registered_device *rdev = wiphy_to_rdev(wiphy);
+
+ ASSERT_RTNL();
+
+ list_for_each_entry(wdev, &rdev->wdev_list, list)
+ if (!reg_wdev_chan_valid(wiphy, wdev))
+ cfg80211_leave(rdev, wdev);
+}
+
+static void reg_check_chans_work(struct work_struct *work)
+{
+ struct cfg80211_registered_device *rdev;
+
+ REG_DBG_PRINT("Verifying active interfaces after reg change\n");
+ rtnl_lock();
+
+ list_for_each_entry(rdev, &cfg80211_rdev_list, list)
+ reg_leave_invalid_chans(&rdev->wiphy);
+
+ rtnl_unlock();
+}
+
+static void reg_check_channels(void)
+{
+ /*
+ * Give usermode a chance to do something nicer (move to another
+ * channel, orderly disconnection), before forcing a disconnection.
+ */
+ mod_delayed_work(system_power_efficient_wq,
+ ®_check_chans, msecs_to_jiffies(60000));
+}
+
static void wiphy_update_regulatory(struct wiphy *wiphy,
enum nl80211_reg_initiator initiator)
{
@@ -1559,6 +1636,8 @@ static void update_all_wiphy_regulatory(enum nl80211_reg_initiator initiator)
wiphy = &rdev->wiphy;
wiphy_update_regulatory(wiphy, initiator);
}
+
+ reg_check_channels();
}
static void handle_channel_custom(struct wiphy *wiphy,
@@ -1972,8 +2051,10 @@ static void reg_process_hint(struct regulatory_request *reg_request)
/* This is required so that the orig_* parameters are saved */
if (treatment == REG_REQ_ALREADY_SET && wiphy &&
- wiphy->regulatory_flags & REGULATORY_STRICT_REG)
+ wiphy->regulatory_flags & REGULATORY_STRICT_REG) {
wiphy_update_regulatory(wiphy, reg_request->initiator);
+ reg_check_channels();
+ }
return;
@@ -2853,6 +2934,7 @@ void regulatory_exit(void)
cancel_work_sync(®_work);
cancel_delayed_work_sync(®_timeout);
+ cancel_delayed_work_sync(®_check_chans);
/* Lock to suppress warnings */
rtnl_lock();
--
1.8.3.2
From: Arik Nemtsov <[email protected]>
Allow driver and user hints to set the "world" regulatory domain, and
also allow them to set the same reg-domain. This is useful when sending
an unknown-country ("99") request to a wiphy-specific regulatory source,
requesting it to provide the current alpha2. It may return an updated
regdomain for the "world" alpha2.
Note there's a check for regdom change at the request level, so the case
where the same alpha2 is set repeatedly is still optimized.
Change-Id: Iff04537b26a4c4153b78232ea33a5525512f902a
Signed-off-by: Arik Nemtsov <[email protected]>
Signed-off-by: Emmanuel Grumbach <[email protected]>
---
net/wireless/reg.c | 9 ---------
1 file changed, 9 deletions(-)
diff --git a/net/wireless/reg.c b/net/wireless/reg.c
index c429ec5..d6b83f9 100644
--- a/net/wireless/reg.c
+++ b/net/wireless/reg.c
@@ -2539,9 +2539,6 @@ static int reg_set_rd_user(const struct ieee80211_regdomain *rd,
{
const struct ieee80211_regdomain *intersected_rd = NULL;
- if (!regdom_changes(rd->alpha2))
- return -EALREADY;
-
if (!is_valid_rd(rd)) {
pr_err("Invalid regulatory domain detected:\n");
print_regdomain_info(rd);
@@ -2572,12 +2569,6 @@ static int reg_set_rd_driver(const struct ieee80211_regdomain *rd,
const struct ieee80211_regdomain *tmp;
struct wiphy *request_wiphy;
- if (is_world_regdom(rd->alpha2))
- return -EINVAL;
-
- if (!regdom_changes(rd->alpha2))
- return -EALREADY;
-
if (!is_valid_rd(rd)) {
pr_err("Invalid regulatory domain detected:\n");
print_regdomain_info(rd);
--
1.8.3.2
From: Eliad Peller <[email protected]>
alpha2 is defined as 2-chars array, but is used in multiple
places as string (e.g. with nla_put_string calls), which
might leak kernel data.
Solve it by simply adding an extra char for the NULL
terminator, making such operations safe.
Change-Id: Ib31639dc65deab62953923cab3cdf169cb1d0d32
Signed-off-by: Eliad Peller <[email protected]>
Signed-off-by: Emmanuel Grumbach <[email protected]>
---
include/net/regulatory.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/include/net/regulatory.h b/include/net/regulatory.h
index 2599924..dad7ab2 100644
--- a/include/net/regulatory.h
+++ b/include/net/regulatory.h
@@ -167,7 +167,7 @@ struct ieee80211_reg_rule {
struct ieee80211_regdomain {
struct rcu_head rcu_head;
u32 n_reg_rules;
- char alpha2[2];
+ char alpha2[3];
enum nl80211_dfs_regions dfs_region;
struct ieee80211_reg_rule reg_rules[];
};
--
1.8.3.2
On Tue, May 20, 2014 at 11:26 AM, Luis R. Rodriguez
<[email protected]> wrote:
> On Sun, May 11, 2014 at 1:50 AM, Emmanuel Grumbach
> <[email protected]> wrote:
>> From: Arik Nemtsov <[email protected]>
>>
>> Allow setting bandwidth related regulatory flags. These flags are mapped
>> to the corresponding channel flags in the specified range.
>> Make sure the new flags are consulted when calculating the maximum
>> bandwidth allowed by a regulatory-rule.
>>
>> Also allow propagating the GO_CONCURRENT modifier from a reg-rule to a
>> channel.
>>
>> Change-Id: I1b0506ab332cdd268cbf4b02e03574f5c30ba5c0
>> Signed-off-by: Arik Nemtsov <[email protected]>
>> Signed-off-by: Emmanuel Grumbach <[email protected]>
>> ---
>> include/uapi/linux/nl80211.h | 12 ++++++++++++
>> net/wireless/reg.c | 30 ++++++++++++++++++++++++++++--
>> 2 files changed, 40 insertions(+), 2 deletions(-)
>>
>> diff --git a/include/uapi/linux/nl80211.h b/include/uapi/linux/nl80211.h
>> index 406010d..f332231 100644
>> --- a/include/uapi/linux/nl80211.h
>> +++ b/include/uapi/linux/nl80211.h
>> @@ -2558,6 +2558,11 @@ enum nl80211_sched_scan_match_attr {
>> * @NL80211_RRF_AUTO_BW: maximum available bandwidth should be calculated
>> * base on contiguous rules and wider channels will be allowed to cross
>> * multiple contiguous/overlapping frequency ranges.
>> + * @NL80211_RRF_GO_CONCURRENT: See &NL80211_FREQUENCY_ATTR_GO_CONCURRENT
>> + * @NL80211_RRF_NO_HT40MINUS: channels can't be used in HT40- operation
>> + * @NL80211_RRF_NO_HT40PLUS: channels can't be used in HT40+ operation
>> + * @NL80211_RRF_NO_80MHZ: 80MHz operation not allowed
>> + * @NL80211_RRF_NO_160MHZ: 160MHz operation not allowed
>> */
>> enum nl80211_reg_rule_flags {
>> NL80211_RRF_NO_OFDM = 1<<0,
>> @@ -2570,11 +2575,18 @@ enum nl80211_reg_rule_flags {
>> NL80211_RRF_NO_IR = 1<<7,
>> __NL80211_RRF_NO_IBSS = 1<<8,
>> NL80211_RRF_AUTO_BW = 1<<11,
>> + NL80211_RRF_GO_CONCURRENT = 1<<12,
>> + NL80211_RRF_NO_HT40MINUS = 1<<13,
>> + NL80211_RRF_NO_HT40PLUS = 1<<14,
>> + NL80211_RRF_NO_80MHZ = 1<<15,
>> + NL80211_RRF_NO_160MHZ = 1<<16,
>> };
>
> Automatic calculation on max bandwidth scales better, I'd much prefer
> these only be used and properly documented to only be used in cases
> where we want to be explicit about this, I don't think this should be
> the automatic behavior, or at least your patch in no way explains any
> justification as to why the move is being done, so I cannot guess what
> the logic was here.
We're not going to use CRDA/wireless-regdb as the regulatory data
source. These flags fit the reg-data storage format of the intel FW.
It's just a different way of doing things.
>
>> #define NL80211_RRF_PASSIVE_SCAN NL80211_RRF_NO_IR
>> #define NL80211_RRF_NO_IBSS NL80211_RRF_NO_IR
>> #define NL80211_RRF_NO_IR NL80211_RRF_NO_IR
>> +#define NL80211_RRF_NO_HT40 (NL80211_RRF_NO_HT40MINUS |\
>> + NL80211_RRF_NO_HT40PLUS)
>>
>> /* For backport compatibility with older userspace */
>> #define NL80211_RRF_NO_IR_ALL (NL80211_RRF_NO_IR | __NL80211_RRF_NO_IBSS)
>> diff --git a/net/wireless/reg.c b/net/wireless/reg.c
>> index 558b0e3..efd6d0d 100644
>> --- a/net/wireless/reg.c
>> +++ b/net/wireless/reg.c
>> @@ -572,8 +572,9 @@ static const struct ieee80211_regdomain *reg_get_regdomain(struct wiphy *wiphy)
>> return get_cfg80211_regdom();
>> }
>>
>> -unsigned int reg_get_max_bandwidth(const struct ieee80211_regdomain *rd,
>> - const struct ieee80211_reg_rule *rule)
>> +static unsigned int
>> +reg_get_max_bandwidth_from_range(const struct ieee80211_regdomain *rd,
>> + const struct ieee80211_reg_rule *rule)
>
> This change is renaming a routine we used heavily to something tucked
> in the closet and then introducing the usage of the new flags, which
> have no justifications nor proper regdb updates, this also doesn't
> even account for the old regdb situations so I'll have to provide a
> huge NACK for all these reasons.
>
> You want to consider wireless-regdb / CRDA when making these changes.
I can rename things any way you'd like. Not sure how this can hurt old
regdb situations though. The new flags are not used in any rules
there.
Arik
On Tue, May 20, 2014 at 1:33 AM, Arik Nemtsov <[email protected]> wrote:
> On Tue, May 20, 2014 at 11:26 AM, Luis R. Rodriguez
> <[email protected]> wrote:
>> On Sun, May 11, 2014 at 1:50 AM, Emmanuel Grumbach
>> <[email protected]> wrote:
>>> From: Arik Nemtsov <[email protected]>
>>>
>>> Allow setting bandwidth related regulatory flags. These flags are mapped
>>> to the corresponding channel flags in the specified range.
>>> Make sure the new flags are consulted when calculating the maximum
>>> bandwidth allowed by a regulatory-rule.
>>>
>>> Also allow propagating the GO_CONCURRENT modifier from a reg-rule to a
>>> channel.
>>>
>>> Change-Id: I1b0506ab332cdd268cbf4b02e03574f5c30ba5c0
>>> Signed-off-by: Arik Nemtsov <[email protected]>
>>> Signed-off-by: Emmanuel Grumbach <[email protected]>
>>> ---
>>> include/uapi/linux/nl80211.h | 12 ++++++++++++
>>> net/wireless/reg.c | 30 ++++++++++++++++++++++++++++--
>>> 2 files changed, 40 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/include/uapi/linux/nl80211.h b/include/uapi/linux/nl80211.h
>>> index 406010d..f332231 100644
>>> --- a/include/uapi/linux/nl80211.h
>>> +++ b/include/uapi/linux/nl80211.h
>>> @@ -2558,6 +2558,11 @@ enum nl80211_sched_scan_match_attr {
>>> * @NL80211_RRF_AUTO_BW: maximum available bandwidth should be calculated
>>> * base on contiguous rules and wider channels will be allowed to cross
>>> * multiple contiguous/overlapping frequency ranges.
>>> + * @NL80211_RRF_GO_CONCURRENT: See &NL80211_FREQUENCY_ATTR_GO_CONCURRENT
>>> + * @NL80211_RRF_NO_HT40MINUS: channels can't be used in HT40- operation
>>> + * @NL80211_RRF_NO_HT40PLUS: channels can't be used in HT40+ operation
>>> + * @NL80211_RRF_NO_80MHZ: 80MHz operation not allowed
>>> + * @NL80211_RRF_NO_160MHZ: 160MHz operation not allowed
>>> */
>>> enum nl80211_reg_rule_flags {
>>> NL80211_RRF_NO_OFDM = 1<<0,
>>> @@ -2570,11 +2575,18 @@ enum nl80211_reg_rule_flags {
>>> NL80211_RRF_NO_IR = 1<<7,
>>> __NL80211_RRF_NO_IBSS = 1<<8,
>>> NL80211_RRF_AUTO_BW = 1<<11,
>>> + NL80211_RRF_GO_CONCURRENT = 1<<12,
>>> + NL80211_RRF_NO_HT40MINUS = 1<<13,
>>> + NL80211_RRF_NO_HT40PLUS = 1<<14,
>>> + NL80211_RRF_NO_80MHZ = 1<<15,
>>> + NL80211_RRF_NO_160MHZ = 1<<16,
>>> };
>>
>> Automatic calculation on max bandwidth scales better, I'd much prefer
>> these only be used and properly documented to only be used in cases
>> where we want to be explicit about this, I don't think this should be
>> the automatic behavior, or at least your patch in no way explains any
>> justification as to why the move is being done, so I cannot guess what
>> the logic was here.
>
> We're not going to use CRDA/wireless-regdb as the regulatory data
> source.
Why not?
> These flags fit the reg-data storage format of the intel FW.
> It's just a different way of doing things.
You can covert things, we have tons of drivers doing that already.
>>> #define NL80211_RRF_PASSIVE_SCAN NL80211_RRF_NO_IR
>>> #define NL80211_RRF_NO_IBSS NL80211_RRF_NO_IR
>>> #define NL80211_RRF_NO_IR NL80211_RRF_NO_IR
>>> +#define NL80211_RRF_NO_HT40 (NL80211_RRF_NO_HT40MINUS |\
>>> + NL80211_RRF_NO_HT40PLUS)
>>>
>>> /* For backport compatibility with older userspace */
>>> #define NL80211_RRF_NO_IR_ALL (NL80211_RRF_NO_IR | __NL80211_RRF_NO_IBSS)
>>> diff --git a/net/wireless/reg.c b/net/wireless/reg.c
>>> index 558b0e3..efd6d0d 100644
>>> --- a/net/wireless/reg.c
>>> +++ b/net/wireless/reg.c
>>> @@ -572,8 +572,9 @@ static const struct ieee80211_regdomain *reg_get_regdomain(struct wiphy *wiphy)
>>> return get_cfg80211_regdom();
>>> }
>>>
>>> -unsigned int reg_get_max_bandwidth(const struct ieee80211_regdomain *rd,
>>> - const struct ieee80211_reg_rule *rule)
>>> +static unsigned int
>>> +reg_get_max_bandwidth_from_range(const struct ieee80211_regdomain *rd,
>>> + const struct ieee80211_reg_rule *rule)
>>
>> This change is renaming a routine we used heavily to something tucked
>> in the closet and then introducing the usage of the new flags, which
>> have no justifications nor proper regdb updates, this also doesn't
>> even account for the old regdb situations so I'll have to provide a
>> huge NACK for all these reasons.
>>
>> You want to consider wireless-regdb / CRDA when making these changes.
>
> I can rename things any way you'd like.
You missed my point, you just did not rename something you went and
then made this new unused flag you introduced be used in tons of
locations, that'd introduce tons of regressions.
> Not sure how this can hurt old
> regdb situations though. The new flags are not used in any rules
> there.
Exactly, you can't expect the rules to work then.
Luis
From: Arik Nemtsov <[email protected]>
If the regulatory request contains the special "99" unknown-country code,
allow a different alpha2 as response.
This special alpha2 is used when the real alpha2 is unknown and should
be provided by the driver via its get_regd() wiphy callback, as part of
the regdomain info.
Change-Id: Ia667ebd6b018bce1a32da7da7c5143a5b8a656a7
Signed-off-by: Arik Nemtsov <[email protected]>
Signed-off-by: Emmanuel Grumbach <[email protected]>
---
net/wireless/reg.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/net/wireless/reg.c b/net/wireless/reg.c
index e2f33d7..c429ec5 100644
--- a/net/wireless/reg.c
+++ b/net/wireless/reg.c
@@ -418,7 +418,8 @@ static bool is_user_regdom_saved(void)
return false;
/* This would indicate a mistake on the design */
- if (WARN(!is_world_regdom(user_alpha2) && !is_an_alpha2(user_alpha2),
+ if (WARN(!is_world_regdom(user_alpha2) && !is_an_alpha2(user_alpha2) &&
+ !is_unknown_alpha2(user_alpha2),
"Unexpected user alpha2: %c%c\n",
user_alpha2[0], user_alpha2[1]))
return false;
@@ -595,7 +596,8 @@ bool reg_is_valid_request(const char *alpha2)
if (!lr || lr->processed)
return false;
- return alpha2_equal(lr->alpha2, alpha2);
+ return alpha2_equal(lr->alpha2, alpha2) ||
+ is_unknown_alpha2(lr->alpha2);
}
static const struct ieee80211_regdomain *reg_get_regdomain(struct wiphy *wiphy)
--
1.8.3.2
On Tue, May 20, 2014 at 1:48 AM, Luis R. Rodriguez
<[email protected]> wrote:
> On Sun, May 11, 2014 at 1:50 AM, Emmanuel Grumbach
> <[email protected]> wrote:
>> Hi Johannes,
>>
>> Newer Intel cards store regulatory settings in firmware.
>> This patch series adds support for a wiphy callback, allowing
>> the regulatory core to fetch driver/firmware defined redgomain
>> data whenever a country code change is requested.
>>
>> Along with that, there is also a small fix from Eliad.
>>
>> Arik Nemtsov (6):
>> cfg80211: don't set reg timeout for user-handled hint
>> cfg80211: introduce regulatory flags controlling bw
>> cfg80211: allow drivers to provide regulatory settings
>> cfg80211: treat the special "unknown" alpha2 as valid
>> cfg80211: accept world/same regdom from driver/user hints
>> cfg80211: leave invalid channels on regdomain change
>
> Please Cc me on regulatory patches. I've noted issues with the wiphy
> callback addition, but other than place if you can please resend
> patches that do not depend on that, that'd be appreciated, I'll take a
> look at those in the morning.
I haven't seen these patches, please split them up, I think a few
already were accepted, but separating them between the new
functionality being added is key to speedy review. I'll take a second
stab at review of the fw API now as I'm going comfortalbe with the
idea of letting folks shoot themselves on the foot.
Luis
On Wed, Jun 11, 2014 at 12:43 AM, Luis R. Rodriguez
<[email protected]> wrote:
> On Tue, May 20, 2014 at 5:00 AM, Arik Nemtsov <[email protected]> wrote:
>>>> The wiphy_apply_custom_regulatory() option is to be used before
>>>> registering the wiphy. We want to be able to accept country code
>>>> changes at runtime, with the driver supplying the regdomain.
>>>
>>> For which cases exactly at run time? This is already being handled on
>>> other drivers without changing APIs, so its unclear why you need this
>>> and to expose this to cfg80211. To be clear, a few drivers other than
>>> Intel already had this strategy and they managed to just use the
>>> reg_notifier() and do what it needs by using the flag that ignores
>>> country IEs, and doing everything else on the driver side. Please
>>> explore this avenue.
>>
>> The problem is not propagating the country setting the FW. I agree
>> this can be done via the notifier. The problem is propagating the
>> regulatory data from FW to userspace.
>> For instance we want P2P to be able to use 5Ghz channels in the US.
>> For this, wpa_s must have up to date information from the regulatory
>> DB for country=US. For Intel, the regulatory DB is in FW.
>
> Doesn't the custom regulatory flag allow you to do what you want
> already? Is the issue that the custom flag does not let dynamic run
> time updates and that's what you need?
Yes - we need runtime updates for platforms that can roam to different
countries.
The reg.c code explicitly assumes there will be no regd change if the
custom-regulatory flags is set..
The proposed patch seemed like a clean way to have regulatory data
come from the FW - register the wiphy normally, just replace crda with
a driver callback.
>>
>> This is no longer true. Some variants will now contain settings per county.
>
> OK thanks, I suppose there is more need for regulatory folks at
> companies to be talking.
I'm all for preventing duplicated labor.
>
>>>> I don't have any say here. I think this is more legal than technology reasons.
>>>
>>> Asking these questions, understanding them, and addressing concerns
>>> are the questions that need to be asked to help advance wireless on
>>> Linux, it was not asking these questions that got us into trouble in
>>> the first place, we don't want to go back to that. So even if it is
>>> non technical and purely regulatory we obviously should ask why.
>>>
>>
>> I'm actually an advocate of the CRDA/regdb approach. Less work for us. :)
>> We presented it but ultimately the decision was theirs, not mine.
>
> I'm starting to think that having an organized group that does this
> for the community would be good, having different companies do this
> and come up with different conclusions seems rather a waste of time,
> energy and resources.
Agree. This would need to a be group of legal/regulatory folks, in
addition to developers.
>
>> For instance, one might replace the regdb and break
>> regulatory. So the FW has a regulatory checker that verifies correct
>> settings. Which in turn means it will hold the regulatory DB anyway.
>
> That's just brain dead, we're reverse engineered firmware before,
> firmware is no safe haven from modifications, what we should be
> striving for, and what I do need your help with is arguing back up to
> PHBs on the architectural issues with the firmware design, the fact
> that its proven over and over to have issues, and that it doesn't
> fucking scale.
Reverse-engineering the FW is one thing, changing the regulatory
settings inside it and signing it with an unknown private key is
entirely different. The verification of FW integrity can be done by
HW.
Of course we could verify the driver/kernel etc using TPM, but that's
mostly unfeasible. Therefore I tend to accept the argument the FW
regulatory data is more secure.
I've tried to make the counter-argument that CRDA/wireless-regdb is
good enough for platforms where the user doesn't have root access, but
it was rejected on the grounds that the solution needs to work on
everything.
>
> That said, if your PHBs want to shoot themselves on the foot we can
> let them, but we should at the very least be able to extract *all* the
> regulatory db from the damn firmware so that we can then properly
> implement a common solution upstream for all 802.11 drivers as we have
> been doing.
I guess the code I've already implemented should be used to extract
the data. Just a simple script iterating over countries vs. a live
card.
Asking for the data in other forms would likely bring the wrath of the
Intel legal folks upon us. :)
Arik
2014-06-11 7:24 GMT+02:00 Arik Nemtsov <[email protected]>:
> On Wed, Jun 11, 2014 at 12:28 AM, Luis R. Rodriguez
> <[email protected]> wrote:
>> On Tue, May 20, 2014 at 5:31 AM, Arik Nemtsov <[email protected]> wrote:
>>> On Tue, May 20, 2014 at 12:54 PM, Luis R. Rodriguez
>>> <[email protected]> wrote:
>>>> On Tue, May 20, 2014 at 2:25 AM, Arik Nemtsov <[email protected]> wrote:
>>>>>
>>>>> Why won't old regdb rules work? The NL80211_RRF_NO_160MHZ for instance
>>>>> is not used anywhere in old or new regdbs.
>>>>> So all the new code in reg_get_max_bandwidth is ignored in current or
>>>>> older crda/regdb flows.
>>>>>
>>>>> What am I missing?
>>>>
>>>> It will also be ignored on newer kernels using old wireless-regdb.
>>>
>>> Is that a problem?
>>
>> I would have not brought it up otherwise.
>>
>>> Note that the new flags don't permit more things, but only narrow down
>>> the range. So if VHT80 was blocked due to the range, it will still be
>>> blocked.
>>> Don't really see a reason to use them in newer regdbs either. Like you
>>> said - range only is more scalable.
>>
>> You can keep all those bells and whistles provided you respect old
>> userspace and old behavior first.
>
> I guess I'm waiting for some direction on what need to be changed.
> AFAICT, these flags don't hurt old userspace and/or new kernels using
> an old wireless-regdb.
> Can you propose a scenario where the new flags harm something older?
>
The flag NL80211_RRF_NO_80MHZ could be usefull I think. eg to fix
world regd veryfication issue we have:
Current failing line:
(2457 - 2482 @ 40), (20), NO-IR --> 2482 - 2457 = 25 < 40
Fixed line could be:
(2457 - 2482 @ 40), (20), NO-IR, AUTO-BW, NO-80MHZ
Without NO-80MHZ - AUTO-BW will setup BW=80MHz - I am not sure this is
OK for 2.4?
But setting NO-80MHZ and AUTO-BW flags we will get what expect?
BR
Janusz
On Wed, Jun 11, 2014 at 12:28 AM, Luis R. Rodriguez
<[email protected]> wrote:
> On Tue, May 20, 2014 at 5:31 AM, Arik Nemtsov <[email protected]> wrote:
>> On Tue, May 20, 2014 at 12:54 PM, Luis R. Rodriguez
>> <[email protected]> wrote:
>>> On Tue, May 20, 2014 at 2:25 AM, Arik Nemtsov <[email protected]> wrote:
>>>>
>>>> Why won't old regdb rules work? The NL80211_RRF_NO_160MHZ for instance
>>>> is not used anywhere in old or new regdbs.
>>>> So all the new code in reg_get_max_bandwidth is ignored in current or
>>>> older crda/regdb flows.
>>>>
>>>> What am I missing?
>>>
>>> It will also be ignored on newer kernels using old wireless-regdb.
>>
>> Is that a problem?
>
> I would have not brought it up otherwise.
>
>> Note that the new flags don't permit more things, but only narrow down
>> the range. So if VHT80 was blocked due to the range, it will still be
>> blocked.
>> Don't really see a reason to use them in newer regdbs either. Like you
>> said - range only is more scalable.
>
> You can keep all those bells and whistles provided you respect old
> userspace and old behavior first.
I guess I'm waiting for some direction on what need to be changed.
AFAICT, these flags don't hurt old userspace and/or new kernels using
an old wireless-regdb.
Can you propose a scenario where the new flags harm something older?
Thanks,
Arik
On Tue, May 20, 2014 at 5:31 AM, Arik Nemtsov <[email protected]> wrote:
> On Tue, May 20, 2014 at 12:54 PM, Luis R. Rodriguez
> <[email protected]> wrote:
>> On Tue, May 20, 2014 at 2:25 AM, Arik Nemtsov <[email protected]> wrote:
>>>
>>> Why won't old regdb rules work? The NL80211_RRF_NO_160MHZ for instance
>>> is not used anywhere in old or new regdbs.
>>> So all the new code in reg_get_max_bandwidth is ignored in current or
>>> older crda/regdb flows.
>>>
>>> What am I missing?
>>
>> It will also be ignored on newer kernels using old wireless-regdb.
>
> Is that a problem?
I would have not brought it up otherwise.
> Note that the new flags don't permit more things, but only narrow down
> the range. So if VHT80 was blocked due to the range, it will still be
> blocked.
> Don't really see a reason to use them in newer regdbs either. Like you
> said - range only is more scalable.
You can keep all those bells and whistles provided you respect old
userspace and old behavior first.
> These flags are very useful for translating the Intel FW regulatory
> format to reg.c format. We don't have ranges there, only flags per
> channel. This allows for seamless interop, with per-channel rules.
I get it, its all fine but just address ensuring that old behavior is
respected first, then you can add whatever on top of it.
Luis
On Tue, Jun 10, 2014 at 10:24 PM, Arik Nemtsov <[email protected]> wrote:
> Can you propose a scenario where the new flags harm something older?
Using old userspace that does not have these flags set, the old
mechanisms are supposed to still work.
Luis
On Wed, Jun 11, 2014 at 12:44 AM, Luis R. Rodriguez
<[email protected]> wrote:
>
> On Tue, May 20, 2014 at 1:48 AM, Luis R. Rodriguez
> <[email protected]> wrote:
> > On Sun, May 11, 2014 at 1:50 AM, Emmanuel Grumbach
> > <[email protected]> wrote:
> >> Hi Johannes,
> >>
> >> Newer Intel cards store regulatory settings in firmware.
> >> This patch series adds support for a wiphy callback, allowing
> >> the regulatory core to fetch driver/firmware defined redgomain
> >> data whenever a country code change is requested.
> >>
> >> Along with that, there is also a small fix from Eliad.
> >>
> >> Arik Nemtsov (6):
> >> cfg80211: don't set reg timeout for user-handled hint
> >> cfg80211: introduce regulatory flags controlling bw
> >> cfg80211: allow drivers to provide regulatory settings
> >> cfg80211: treat the special "unknown" alpha2 as valid
> >> cfg80211: accept world/same regdom from driver/user hints
> >> cfg80211: leave invalid channels on regdomain change
> >
> > Please Cc me on regulatory patches. I've noted issues with the wiphy
> > callback addition, but other than place if you can please resend
> > patches that do not depend on that, that'd be appreciated, I'll take a
> > look at those in the morning.
>
> I haven't seen these patches, please split them up, I think a few
> already were accepted, but separating them between the new
> functionality being added is key to speedy review. I'll take a second
> stab at review of the fw API now as I'm going comfortalbe with the
> idea of letting folks shoot themselves on the foot.
This one is in, it's an obvious fix:
4d3df54 cfg80211: don't set reg timeout for user-handled hint
The others are patches supporting the new Intel regulatory solution,
so none of them are in. I'll make sure to resend them today.
Arik
On Wed, Jun 11, 2014 at 9:21 AM, Janusz Dziedzic
<[email protected]> wrote:
> 2014-06-11 7:24 GMT+02:00 Arik Nemtsov <[email protected]>:
>> On Wed, Jun 11, 2014 at 12:28 AM, Luis R. Rodriguez
>> <[email protected]> wrote:
>>> On Tue, May 20, 2014 at 5:31 AM, Arik Nemtsov <[email protected]> wrote:
>>>> On Tue, May 20, 2014 at 12:54 PM, Luis R. Rodriguez
>>>> <[email protected]> wrote:
>>>>> On Tue, May 20, 2014 at 2:25 AM, Arik Nemtsov <[email protected]> wrote:
>>>>>>
>>>>>> Why won't old regdb rules work? The NL80211_RRF_NO_160MHZ for instance
>>>>>> is not used anywhere in old or new regdbs.
>>>>>> So all the new code in reg_get_max_bandwidth is ignored in current or
>>>>>> older crda/regdb flows.
>>>>>>
>>>>>> What am I missing?
>>>>>
>>>>> It will also be ignored on newer kernels using old wireless-regdb.
>>>>
>>>> Is that a problem?
>>>
>>> I would have not brought it up otherwise.
>>>
>>>> Note that the new flags don't permit more things, but only narrow down
>>>> the range. So if VHT80 was blocked due to the range, it will still be
>>>> blocked.
>>>> Don't really see a reason to use them in newer regdbs either. Like you
>>>> said - range only is more scalable.
>>>
>>> You can keep all those bells and whistles provided you respect old
>>> userspace and old behavior first.
>>
>> I guess I'm waiting for some direction on what need to be changed.
>> AFAICT, these flags don't hurt old userspace and/or new kernels using
>> an old wireless-regdb.
>> Can you propose a scenario where the new flags harm something older?
>>
>
> The flag NL80211_RRF_NO_80MHZ could be usefull I think. eg to fix
> world regd veryfication issue we have:
>
> Current failing line:
> (2457 - 2482 @ 40), (20), NO-IR --> 2482 - 2457 = 25 < 40
>
> Fixed line could be:
> (2457 - 2482 @ 40), (20), NO-IR, AUTO-BW, NO-80MHZ
>
> Without NO-80MHZ - AUTO-BW will setup BW=80MHz - I am not sure this is
> OK for 2.4?
> But setting NO-80MHZ and AUTO-BW flags we will get what expect?
Yea it should do the right thing. This is how Intel is using it.
Arik
On Tue, May 20, 2014 at 5:00 AM, Arik Nemtsov <[email protected]> wrote:
>>> The wiphy_apply_custom_regulatory() option is to be used before
>>> registering the wiphy. We want to be able to accept country code
>>> changes at runtime, with the driver supplying the regdomain.
>>
>> For which cases exactly at run time? This is already being handled on
>> other drivers without changing APIs, so its unclear why you need this
>> and to expose this to cfg80211. To be clear, a few drivers other than
>> Intel already had this strategy and they managed to just use the
>> reg_notifier() and do what it needs by using the flag that ignores
>> country IEs, and doing everything else on the driver side. Please
>> explore this avenue.
>
> The problem is not propagating the country setting the FW. I agree
> this can be done via the notifier. The problem is propagating the
> regulatory data from FW to userspace.
> For instance we want P2P to be able to use 5Ghz channels in the US.
> For this, wpa_s must have up to date information from the regulatory
> DB for country=US. For Intel, the regulatory DB is in FW.
Doesn't the custom regulatory flag allow you to do what you want
already? Is the issue that the custom flag does not let dynamic run
time updates and that's what you need?
>>> As for why this was chosen - I think you're barking up the wrong tree :)
>>> The regulatory folks at Intel decided to store the data in FW,
>>
>> This has been done for a long time but the main reason why this was
>> done that way was that Intel had no need to have tons of regulatory
>> domains, and instead had only 4 world regulatory domains, that's all,
>> if things have changed it'd be good to understand this and also the
>> reasons why things are being done.
>
> This is no longer true. Some variants will now contain settings per county.
OK thanks, I suppose there is more need for regulatory folks at
companies to be talking.
>>> I don't have any say here. I think this is more legal than technology reasons.
>>
>> Asking these questions, understanding them, and addressing concerns
>> are the questions that need to be asked to help advance wireless on
>> Linux, it was not asking these questions that got us into trouble in
>> the first place, we don't want to go back to that. So even if it is
>> non technical and purely regulatory we obviously should ask why.
>>
>
> I'm actually an advocate of the CRDA/regdb approach. Less work for us. :)
> We presented it but ultimately the decision was theirs, not mine.
I'm starting to think that having an organized group that does this
for the community would be good, having different companies do this
and come up with different conclusions seems rather a waste of time,
energy and resources.
> I believe the main motivations were security and uniformity across
> different OSes.
Having loose interpretations over what is generally accepted is
understood, specially for corner cases of new breeds of technology
like P2P and dealing with nagging customers who insist on things or
are pushing the envelope on what regulatory bodies have or have not
made explicit. However making it the norm seems rather counter
productive in the long run. I'm starting to think that having a clean
API to extract the regulatory data from FW to allow even dynamic
changes at run time might be good given that we can at least extract
that information and deduce updates from companies for the general
public wireless-regdb. That is, the mathematics on wireless-regdb /
CRDA can be used to create the intersections of what is accepted for
the case where no regulatory information is available. The technical
assumption however that one should be stuffing firmware with
regulatory data is brain dead given that it doesn't scale, prone to
regulatory issues, and bugs. The flexibility of having these updates
generalized and only using firmware for out of norm situations should
be strongly encouraged.
> For instance, one might replace the regdb and break
> regulatory. So the FW has a regulatory checker that verifies correct
> settings. Which in turn means it will hold the regulatory DB anyway.
That's just brain dead, we're reverse engineered firmware before,
firmware is no safe haven from modifications, what we should be
striving for, and what I do need your help with is arguing back up to
PHBs on the architectural issues with the firmware design, the fact
that its proven over and over to have issues, and that it doesn't
fucking scale.
That said, if your PHBs want to shoot themselves on the foot we can
let them, but we should at the very least be able to extract *all* the
regulatory db from the damn firmware so that we can then properly
implement a common solution upstream for all 802.11 drivers as we have
been doing.
Luis