[PATCH v2 1/3] cfg80211: add helper reg_get_regdomain() function

[PATCH v2 1/3] cfg80211: add helper reg_get_regdomain() function

[Janusz Dziedzic]

Add helper function that will return regdomain.
Follow the driver's regulatory domain, if present,
unless a country IE has been processed or a user
wants to help complaince further.

Signed-off-by: Janusz Dziedzic <janusz.dziedzi@tieto.com>
---
 net/wireless/reg.c |   31 ++++++++++++++++++++-----------
 1 file changed, 20 insertions(+), 11 deletions(-)

diff --git a/net/wireless/reg.c b/net/wireless/reg.c
index 9b897fc..d58a09c 100644
--- a/net/wireless/reg.c
+++ b/net/wireless/reg.c
@@ -522,6 +522,25 @@ bool reg_is_valid_request(const char *alpha2)
 	return alpha2_equal(lr->alpha2, alpha2);
 }
 
+static const struct ieee80211_regdomain *reg_get_regdomain(struct wiphy *wiphy)
+{
+	const struct ieee80211_regdomain *regd;
+	struct regulatory_request *lr = get_last_request();
+
+	/*
+	 * Follow the driver's regulatory domain, if present, unless a country
+	 * IE has been processed or a user wants to help complaince further
+	 */
+	if (lr->initiator != NL80211_REGDOM_SET_BY_COUNTRY_IE &&
+	    lr->initiator != NL80211_REGDOM_SET_BY_USER &&
+	    wiphy->regd)
+		regd = get_wiphy_regdom(wiphy);
+	else
+		regd = get_cfg80211_regdom();
+
+	return regd;
+}
+
 /* Sanity check on a regulatory rule */
 static bool is_valid_reg_rule(const struct ieee80211_reg_rule *rule)
 {
@@ -821,18 +840,8 @@ const struct ieee80211_reg_rule *freq_reg_info(struct wiphy *wiphy,
 					       u32 center_freq)
 {
 	const struct ieee80211_regdomain *regd;
-	struct regulatory_request *lr = get_last_request();
 
-	/*
-	 * Follow the driver's regulatory domain, if present, unless a country
-	 * IE has been processed or a user wants to help complaince further
-	 */
-	if (lr->initiator != NL80211_REGDOM_SET_BY_COUNTRY_IE &&
-	    lr->initiator != NL80211_REGDOM_SET_BY_USER &&
-	    wiphy->regd)
-		regd = get_wiphy_regdom(wiphy);
-	else
-		regd = get_cfg80211_regdom();
+	regd = reg_get_regdomain(wiphy);
 
 	return freq_reg_info_regd(wiphy, center_freq, regd);
 }
-- 
1.7.9.5

[PATCH v2 2/3] cfg80211: introduce regulatory wide bandwidth flag

[Janusz Dziedzic]

Introduce regulatory flags field, NL80211_REG_WIDE_BW
country flag and new attribute NL80211_ATTR_REG_FLAGS.
If NL80211_REG_WIDE_BW is set, check rules and calculate
maximum bandwidth base on all contiguous regulatory rules.
If unset get maximum badwitdh from regulatory rule.
This patch is backward compatible with current CRDA/db.txt
implementation.

Base on calculated maximum bandwidth set channels
flags (eg. IEEE80211_CHAN_NO_80MHZ).

In case we will check two regulatory rules as below:
        (5170 - 5250 @ 80), (N/A, 20)
        (5250 - 5330 @ 80), (N/A, 20), DFS

If we set WIDE_BW country flag, we will calulate
maximum available bandwidth as 160MHz, while
two reg rules are frequency contiguous and frequency
diff is 5330 - 5170 = 160.

If we don't set WIDE_BW flag we will get maximum
bandwidth from regulatory database - 80MHz in example
above.

Signed-off-by: Janusz Dziedzic <janusz.dziedzic@tieto.com>
---
 include/net/regulatory.h     |    1 +
 include/uapi/linux/nl80211.h |   14 ++++++
 net/wireless/nl80211.c       |   11 ++++-
 net/wireless/reg.c           |  102 ++++++++++++++++++++++++++++++++++++------
 4 files changed, 112 insertions(+), 16 deletions(-)

diff --git a/include/net/regulatory.h b/include/net/regulatory.h
index b07cdc9..7b938cd 100644
--- a/include/net/regulatory.h
+++ b/include/net/regulatory.h
@@ -161,6 +161,7 @@ struct ieee80211_regdomain {
 	struct rcu_head rcu_head;
 	u32 n_reg_rules;
 	char alpha2[2];
+	u32 flags;
 	enum nl80211_dfs_regions dfs_region;
 	struct ieee80211_reg_rule reg_rules[];
 };
diff --git a/include/uapi/linux/nl80211.h b/include/uapi/linux/nl80211.h
index 91054fd..8546293 100644
--- a/include/uapi/linux/nl80211.h
+++ b/include/uapi/linux/nl80211.h
@@ -1555,6 +1555,7 @@ enum nl80211_commands {
  *	data is in the format defined for the payload of the QoS Map Set element
  *	in IEEE Std 802.11-2012, 8.4.2.97.
  *
+ * @NL80211_ATTR_REG_FLAGS: Regulatory flags, see &enum nl80211_reg_flags
  * @NL80211_ATTR_MAX: highest attribute number currently defined
  * @__NL80211_ATTR_AFTER_LAST: internal use
  */
@@ -1883,6 +1884,8 @@ enum nl80211_attrs {
 
 	NL80211_ATTR_QOS_MAP,
 
+	NL80211_ATTR_REG_FLAGS,
+
 	/* add attributes here, update the policy in nl80211.c */
 
 	__NL80211_ATTR_AFTER_LAST,
@@ -2465,6 +2468,17 @@ enum nl80211_sched_scan_match_attr {
 #define NL80211_ATTR_SCHED_SCAN_MATCH_SSID NL80211_SCHED_SCAN_MATCH_ATTR_SSID
 
 /**
+ * enum nl80211_reg_flags - regulatory flags
+ * @NL80211_REG_FLAG_WIDE_BW: Calculate maximum available bandwidth
+ *	base on contiguous regulatory rules instead of using
+ *	maximum bandwidth from regulatory database.
+ *
+ */
+enum nl80211_reg_flags {
+	NL80211_REG_FLAG_WIDE_BW	= 1<<0,
+};
+
+/**
  * enum nl80211_reg_rule_flags - regulatory rule flags
  *
  * @NL80211_RRF_NO_OFDM: OFDM modulation not allowed
diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c
index d0afd82..fbdb9bb 100644
--- a/net/wireless/nl80211.c
+++ b/net/wireless/nl80211.c
@@ -384,6 +384,7 @@ static const struct nla_policy nl80211_policy[NL80211_ATTR_MAX+1] = {
 	[NL80211_ATTR_VENDOR_DATA] = { .type = NLA_BINARY },
 	[NL80211_ATTR_QOS_MAP] = { .type = NLA_BINARY,
 				   .len = IEEE80211_QOS_MAP_LEN_MAX },
+	[NL80211_ATTR_REG_FLAGS] = { .type = NLA_U32 },
 };
 
 /* policy for the key attributes */
@@ -5101,7 +5102,8 @@ static int nl80211_get_reg(struct sk_buff *skb, struct genl_info *info)
 
 	if (nla_put_string(msg, NL80211_ATTR_REG_ALPHA2, regdom->alpha2) ||
 	    (regdom->dfs_region &&
-	     nla_put_u8(msg, NL80211_ATTR_DFS_REGION, regdom->dfs_region)))
+	     nla_put_u8(msg, NL80211_ATTR_DFS_REGION, regdom->dfs_region)) ||
+	    nla_put_u32(msg, NL80211_ATTR_REG_FLAGS, regdom->flags))
 		goto nla_put_failure_rcu;
 
 	nl_reg_rules = nla_nest_start(msg, NL80211_ATTR_REG_RULES);
@@ -5160,7 +5162,7 @@ static int nl80211_set_reg(struct sk_buff *skb, struct genl_info *info)
 	struct nlattr *nl_reg_rule;
 	char *alpha2 = NULL;
 	int rem_reg_rules = 0, r = 0;
-	u32 num_rules = 0, rule_idx = 0, size_of_regd;
+	u32 num_rules = 0, rule_idx = 0, size_of_regd, flags = 0;
 	enum nl80211_dfs_regions dfs_region = NL80211_DFS_UNSET;
 	struct ieee80211_regdomain *rd = NULL;
 
@@ -5175,6 +5177,9 @@ static int nl80211_set_reg(struct sk_buff *skb, struct genl_info *info)
 	if (info->attrs[NL80211_ATTR_DFS_REGION])
 		dfs_region = nla_get_u8(info->attrs[NL80211_ATTR_DFS_REGION]);
 
+	if (info->attrs[NL80211_ATTR_REG_FLAGS])
+		flags = nla_get_u32(info->attrs[NL80211_ATTR_REG_FLAGS]);
+
 	nla_for_each_nested(nl_reg_rule, info->attrs[NL80211_ATTR_REG_RULES],
 			    rem_reg_rules) {
 		num_rules++;
@@ -5203,6 +5208,8 @@ static int nl80211_set_reg(struct sk_buff *skb, struct genl_info *info)
 	if (reg_supported_dfs_region(dfs_region))
 		rd->dfs_region = dfs_region;
 
+	rd->flags = flags;
+
 	nla_for_each_nested(nl_reg_rule, info->attrs[NL80211_ATTR_REG_RULES],
 			    rem_reg_rules) {
 		nla_parse(tb, NL80211_REG_RULE_ATTR_MAX,
diff --git a/net/wireless/reg.c b/net/wireless/reg.c
index d58a09c..33767db 100644
--- a/net/wireless/reg.c
+++ b/net/wireless/reg.c
@@ -541,8 +541,63 @@ static const struct ieee80211_regdomain *reg_get_regdomain(struct wiphy *wiphy)
 	return regd;
 }
 
+static unsigned int reg_get_max_bandwidth(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;
+	const struct ieee80211_reg_rule *tmp;
+	u32 start_freq, end_freq, idx, no;
+
+	/* First check if strict regulatory database bandwidth calculation */
+	if (!(rd->flags & NL80211_REG_FLAG_WIDE_BW))
+		return freq_range->end_freq_khz - freq_range->start_freq_khz;
+
+	/* Next check all contiguous rules */
+	for (idx = 0; idx < rd->n_reg_rules; idx++)
+		if (rule == &rd->reg_rules[idx])
+			break;
+
+	if (idx == rd->n_reg_rules)
+		return 0;
+
+	/* get start_freq */
+	no = idx;
+
+	while (no) {
+		tmp = &rd->reg_rules[--no];
+		freq_range_tmp = &tmp->freq_range;
+
+		if (freq_range_tmp->end_freq_khz < freq_range->start_freq_khz)
+			break;
+
+		freq_range = freq_range_tmp;
+	};
+
+	start_freq = freq_range->start_freq_khz;
+
+	/* get end_freq */
+	freq_range = &rule->freq_range;
+	no = idx;
+
+	while (no < rd->n_reg_rules - 1) {
+		tmp = &rd->reg_rules[++no];
+		freq_range_tmp = &tmp->freq_range;
+
+		if (freq_range_tmp->start_freq_khz > freq_range->end_freq_khz)
+			break;
+
+		freq_range = freq_range_tmp;
+	}
+
+	end_freq = freq_range->end_freq_khz;
+
+	return end_freq - start_freq;
+}
+
 /* Sanity check on a regulatory rule */
-static bool is_valid_reg_rule(const struct ieee80211_reg_rule *rule)
+static bool is_valid_reg_rule(const struct ieee80211_regdomain *rd,
+			      const struct ieee80211_reg_rule *rule)
 {
 	const struct ieee80211_freq_range *freq_range = &rule->freq_range;
 	u32 freq_diff;
@@ -553,10 +608,12 @@ static bool is_valid_reg_rule(const struct ieee80211_reg_rule *rule)
 	if (freq_range->start_freq_khz > freq_range->end_freq_khz)
 		return false;
 
-	freq_diff = freq_range->end_freq_khz - freq_range->start_freq_khz;
+	if (freq_range->end_freq_khz <= freq_range->start_freq_khz)
+		return false;
+
+	freq_diff = reg_get_max_bandwidth(rd, rule);
 
-	if (freq_range->end_freq_khz <= freq_range->start_freq_khz ||
-	    freq_range->max_bandwidth_khz > freq_diff)
+	if (freq_range->max_bandwidth_khz > freq_diff)
 		return false;
 
 	return true;
@@ -575,7 +632,7 @@ static bool is_valid_rd(const struct ieee80211_regdomain *rd)
 
 	for (i = 0; i < rd->n_reg_rules; i++) {
 		reg_rule = &rd->reg_rules[i];
-		if (!is_valid_reg_rule(reg_rule))
+		if (!is_valid_reg_rule(rd, reg_rule))
 			return false;
 	}
 
@@ -645,11 +702,17 @@ reg_intersect_dfs_region(const enum nl80211_dfs_regions dfs_region1,
 	return dfs_region1;
 }
 
+static u32 reg_intersect_flags(const u32 flags1, const u32 flags2)
+{
+	return flags1 | flags2;
+}
+
 /*
  * Helper for regdom_intersect(), this does the real
  * mathematical intersection fun
  */
-static int reg_rules_intersect(const struct ieee80211_reg_rule *rule1,
+static int reg_rules_intersect(const struct ieee80211_regdomain *rd,
+			       const struct ieee80211_reg_rule *rule1,
 			       const struct ieee80211_reg_rule *rule2,
 			       struct ieee80211_reg_rule *intersected_rule)
 {
@@ -674,7 +737,7 @@ static int reg_rules_intersect(const struct ieee80211_reg_rule *rule1,
 	freq_range->max_bandwidth_khz = min(freq_range1->max_bandwidth_khz,
 					    freq_range2->max_bandwidth_khz);
 
-	freq_diff = freq_range->end_freq_khz - freq_range->start_freq_khz;
+	freq_diff = reg_get_max_bandwidth(rd, intersected_rule);
 	if (freq_range->max_bandwidth_khz > freq_diff)
 		freq_range->max_bandwidth_khz = freq_diff;
 
@@ -685,7 +748,7 @@ static int reg_rules_intersect(const struct ieee80211_reg_rule *rule1,
 
 	intersected_rule->flags = rule1->flags | rule2->flags;
 
-	if (!is_valid_reg_rule(intersected_rule))
+	if (!is_valid_reg_rule(rd, intersected_rule))
 		return -EINVAL;
 
 	return 0;
@@ -713,7 +776,7 @@ regdom_intersect(const struct ieee80211_regdomain *rd1,
 	unsigned int num_rules = 0, rule_idx = 0;
 	const struct ieee80211_reg_rule *rule1, *rule2;
 	struct ieee80211_reg_rule *intersected_rule;
-	struct ieee80211_regdomain *rd;
+	struct ieee80211_regdomain *rd, dummy_rd;
 	/* This is just a dummy holder to help us count */
 	struct ieee80211_reg_rule dummy_rule;
 
@@ -728,11 +791,14 @@ regdom_intersect(const struct ieee80211_regdomain *rd1,
 	 * All rules that do check out OK are valid.
 	 */
 
+	dummy_rd.flags = reg_intersect_flags(rd1->flags, rd2->flags);
+
 	for (x = 0; x < rd1->n_reg_rules; x++) {
 		rule1 = &rd1->reg_rules[x];
 		for (y = 0; y < rd2->n_reg_rules; y++) {
 			rule2 = &rd2->reg_rules[y];
-			if (!reg_rules_intersect(rule1, rule2, &dummy_rule))
+			if (!reg_rules_intersect(&dummy_rd, rule1, rule2,
+						 &dummy_rule))
 				num_rules++;
 		}
 	}
@@ -747,6 +813,8 @@ regdom_intersect(const struct ieee80211_regdomain *rd1,
 	if (!rd)
 		return NULL;
 
+	rd->flags = reg_intersect_flags(rd1->flags, rd2->flags);
+
 	for (x = 0; x < rd1->n_reg_rules && rule_idx < num_rules; x++) {
 		rule1 = &rd1->reg_rules[x];
 		for (y = 0; y < rd2->n_reg_rules && rule_idx < num_rules; y++) {
@@ -757,7 +825,8 @@ regdom_intersect(const struct ieee80211_regdomain *rd1,
 			 * a memcpy()
 			 */
 			intersected_rule = &rd->reg_rules[rule_idx];
-			r = reg_rules_intersect(rule1, rule2, intersected_rule);
+			r = reg_rules_intersect(rd, rule1, rule2,
+						intersected_rule);
 			/*
 			 * No need to memset here the intersected rule here as
 			 * we're not using the stack anymore
@@ -912,6 +981,8 @@ static void handle_channel(struct wiphy *wiphy,
 	const struct ieee80211_freq_range *freq_range = NULL;
 	struct wiphy *request_wiphy = NULL;
 	struct regulatory_request *lr = get_last_request();
+	const struct ieee80211_regdomain *regd;
+	u32 max_bandwidth_khz;
 
 	request_wiphy = wiphy_idx_to_wiphy(lr->wiphy_idx);
 
@@ -953,11 +1024,14 @@ static void handle_channel(struct wiphy *wiphy,
 	power_rule = &reg_rule->power_rule;
 	freq_range = &reg_rule->freq_range;
 
-	if (freq_range->max_bandwidth_khz < MHZ_TO_KHZ(40))
+	regd = reg_get_regdomain(wiphy);
+	max_bandwidth_khz = reg_get_max_bandwidth(regd, reg_rule);
+
+	if (max_bandwidth_khz < MHZ_TO_KHZ(40))
 		bw_flags = IEEE80211_CHAN_NO_HT40;
-	if (freq_range->max_bandwidth_khz < MHZ_TO_KHZ(80))
+	if (max_bandwidth_khz < MHZ_TO_KHZ(80))
 		bw_flags |= IEEE80211_CHAN_NO_80MHZ;
-	if (freq_range->max_bandwidth_khz < MHZ_TO_KHZ(160))
+	if (max_bandwidth_khz < MHZ_TO_KHZ(160))
 		bw_flags |= IEEE80211_CHAN_NO_160MHZ;
 
 	if (lr->initiator == NL80211_REGDOM_SET_BY_DRIVER &&
-- 
1.7.9.5

Re: [PATCH v2 2/3] cfg80211: introduce regulatory wide bandwidth flag

[Johannes Berg]

On Wed, 2014-01-22 at 13:43 +0100, Janusz Dziedzic wrote:
> Introduce regulatory flags field, NL80211_REG_WIDE_BW
> country flag and new attribute NL80211_ATTR_REG_FLAGS.
> If NL80211_REG_WIDE_BW is set, check rules and calculate
> maximum bandwidth base on all contiguous regulatory rules.
> If unset get maximum badwitdh from regulatory rule.
> This patch is backward compatible with current CRDA/db.txt
> implementation.

This seems reasonable, thanks. Maybe we should require the bandwidth to
not be set at all or something? At least maybe in the regdb parser - it
makes very little sense to have @20 and then ignore it completely?

Or maybe the userspace code could just not expose the flag, but rather
set the new "wide_bw" flag when all the rules are marked as @N/A (and
treat a combination of @number and @N/A as a bug)?

> + * @NL80211_ATTR_REG_FLAGS: Regulatory flags, see &enum nl80211_reg_flags

should probably say it's a u32 attribute

>  /**
> + * enum nl80211_reg_flags - regulatory flags
> + * @NL80211_REG_FLAG_WIDE_BW: Calculate maximum available bandwidth
> + *	base on contiguous regulatory rules instead of using
> + *	maximum bandwidth from regulatory database.

And that should probably not refer to the database but rather the
nl80211 attribute that contains the number.

> @@ -5203,6 +5208,8 @@ static int nl80211_set_reg(struct sk_buff *skb, struct genl_info *info)
>  	if (reg_supported_dfs_region(dfs_region))
>  		rd->dfs_region = dfs_region;
>  
> +	rd->flags = flags;

Why bother with the temporary flags variable?

>  	nla_for_each_nested(nl_reg_rule, info->attrs[NL80211_ATTR_REG_RULES],
>  			    rem_reg_rules) {
>  		nla_parse(tb, NL80211_REG_RULE_ATTR_MAX,

As per above, maybe check that no bandwidth is actually in the rules?

> +static unsigned int reg_get_max_bandwidth(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;
> +	const struct ieee80211_reg_rule *tmp;
> +	u32 start_freq, end_freq, idx, no;
> +
> +	/* First check if strict regulatory database bandwidth calculation */
> +	if (!(rd->flags & NL80211_REG_FLAG_WIDE_BW))
> +		return freq_range->end_freq_khz - freq_range->start_freq_khz;

shouldn't that check the range's @bandwidth (as well)?

or this is for sanity check only I guess.

> +	freq_diff = reg_get_max_bandwidth(rd, rule);
>  
> -	if (freq_range->end_freq_khz <= freq_range->start_freq_khz ||
> -	    freq_range->max_bandwidth_khz > freq_diff)
> +	if (freq_range->max_bandwidth_khz > freq_diff)
>  		return false;

Err, wait, this won't work as advertised, would it?
 
> @@ -645,11 +702,17 @@ reg_intersect_dfs_region(const enum nl80211_dfs_regions dfs_region1,
>  	return dfs_region1;
>  }
>  
> +static u32 reg_intersect_flags(const u32 flags1, const u32 flags2)
> +{
> +	return flags1 | flags2;
> +}

Wow, ok, this is getting tricky. But I think it should be the other way
around? I mean, it seems WIDE_BW flag should be unset if it's unset in
either of them?

But we might also have to set max_bandwidth_khz to a proper calculated
wide_bw value before intersecting...

johannes

Re: [PATCH v2 2/3] cfg80211: introduce regulatory wide bandwidth flag

[Luis R. Rodriguez]

On Thu, Jan 23, 2014 at 04:57:00PM +0100, Johannes Berg wrote:
> On Wed, 2014-01-22 at 13:43 +0100, Janusz Dziedzic wrote:
> > Introduce regulatory flags field, NL80211_REG_WIDE_BW
> > country flag and new attribute NL80211_ATTR_REG_FLAGS.
> > If NL80211_REG_WIDE_BW is set, check rules and calculate
> > maximum bandwidth base on all contiguous regulatory rules.
> > If unset get maximum badwitdh from regulatory rule.
> > This patch is backward compatible with current CRDA/db.txt
> > implementation.
> 
> This seems reasonable, thanks. Maybe we should require the bandwidth to
> not be set at all or something? At least maybe in the regdb parser - it
> makes very little sense to have @20 and then ignore it completely?
> 
> Or maybe the userspace code could just not expose the flag, but rather
> set the new "wide_bw" flag when all the rules are marked as @N/A (and
> treat a combination of @number and @N/A as a bug)?

The optimizer code I added to CRDA does all this for us, so technically,
unless I'm missing something, this could be dealt with magically in
userspace. Its also unclear why we'd define this as a regulatory
parameter -- this just seems to make sense.

I'd look at extending CRDA binary to use the optimizer on the regulatory
domain prior to sending it to the kernel. All of a sudden you get full
support for this for free on any kernel.

  Luis

Re: [PATCH v2 2/3] cfg80211: introduce regulatory wide bandwidth flag

[Luis R. Rodriguez]

On Fri, Jan 24, 2014 at 4:32 PM, Luis R. Rodriguez
<mcgrof@do-not-panic.com> wrote:
> On Thu, Jan 23, 2014 at 04:57:00PM +0100, Johannes Berg wrote:
>> On Wed, 2014-01-22 at 13:43 +0100, Janusz Dziedzic wrote:
>> > Introduce regulatory flags field, NL80211_REG_WIDE_BW
>> > country flag and new attribute NL80211_ATTR_REG_FLAGS.
>> > If NL80211_REG_WIDE_BW is set, check rules and calculate
>> > maximum bandwidth base on all contiguous regulatory rules.
>> > If unset get maximum badwitdh from regulatory rule.
>> > This patch is backward compatible with current CRDA/db.txt
>> > implementation.
>>
>> This seems reasonable, thanks. Maybe we should require the bandwidth to
>> not be set at all or something? At least maybe in the regdb parser - it
>> makes very little sense to have @20 and then ignore it completely?
>>
>> Or maybe the userspace code could just not expose the flag, but rather
>> set the new "wide_bw" flag when all the rules are marked as @N/A (and
>> treat a combination of @number and @N/A as a bug)?
>
> The optimizer code I added to CRDA does all this for us, so technically,
> unless I'm missing something, this could be dealt with magically in
> userspace. Its also unclear why we'd define this as a regulatory
> parameter -- this just seems to make sense.
>
> I'd look at extending CRDA binary to use the optimizer on the regulatory
> domain prior to sending it to the kernel. All of a sudden you get full
> support for this for free on any kernel.

And as for the users of the internal regdb -- have them use the
optimizer to optimize their db prior to compiling the kernel ;)

  Luis

Re: [PATCH v2 2/3] cfg80211: introduce regulatory wide bandwidth flag

[Janusz Dziedzic]

2014/1/25 Luis R. Rodriguez <mcgrof@do-not-panic.com>:
> On Fri, Jan 24, 2014 at 4:32 PM, Luis R. Rodriguez
> <mcgrof@do-not-panic.com> wrote:
>> On Thu, Jan 23, 2014 at 04:57:00PM +0100, Johannes Berg wrote:
>>> On Wed, 2014-01-22 at 13:43 +0100, Janusz Dziedzic wrote:
>>> > Introduce regulatory flags field, NL80211_REG_WIDE_BW
>>> > country flag and new attribute NL80211_ATTR_REG_FLAGS.
>>> > If NL80211_REG_WIDE_BW is set, check rules and calculate
>>> > maximum bandwidth base on all contiguous regulatory rules.
>>> > If unset get maximum badwitdh from regulatory rule.
>>> > This patch is backward compatible with current CRDA/db.txt
>>> > implementation.
>>>
>>> This seems reasonable, thanks. Maybe we should require the bandwidth to
>>> not be set at all or something? At least maybe in the regdb parser - it
>>> makes very little sense to have @20 and then ignore it completely?
>>>
>>> Or maybe the userspace code could just not expose the flag, but rather
>>> set the new "wide_bw" flag when all the rules are marked as @N/A (and
>>> treat a combination of @number and @N/A as a bug)?
>>
>> The optimizer code I added to CRDA does all this for us, so technically,
>> unless I'm missing something, this could be dealt with magically in
>> userspace. Its also unclear why we'd define this as a regulatory
>> parameter -- this just seems to make sense.
>>
>> I'd look at extending CRDA binary to use the optimizer on the regulatory
>> domain prior to sending it to the kernel. All of a sudden you get full
>> support for this for free on any kernel.
>
> And as for the users of the internal regdb -- have them use the
> optimizer to optimize their db prior to compiling the kernel ;)
>

Sounds good. This mean we have to remove from kernel:

freq_diff = freq_range->end_freq_khz - freq_range->start_freq_khz;
if (freq_range->max_bandwidth_khz > freq_diff)
    return false;

For internal regdb/also my case this should be enough, while I can set
correct BW manually

Anyway
I send new version with changes Johannes suggest.

-- 
Janusz Dziedzic

Re: [PATCH v2 2/3] cfg80211: introduce regulatory wide bandwidth flag

[Johannes Berg]

On Fri, 2014-01-24 at 16:32 -0800, Luis R. Rodriguez wrote:

> > This seems reasonable, thanks. Maybe we should require the bandwidth to
> > not be set at all or something? At least maybe in the regdb parser - it
> > makes very little sense to have @20 and then ignore it completely?
> > 
> > Or maybe the userspace code could just not expose the flag, but rather
> > set the new "wide_bw" flag when all the rules are marked as @N/A (and
> > treat a combination of @number and @N/A as a bug)?
> 
> The optimizer code I added to CRDA does all this for us, so technically,
> unless I'm missing something, this could be dealt with magically in
> userspace. Its also unclear why we'd define this as a regulatory
> parameter -- this just seems to make sense.
> 
> I'd look at extending CRDA binary to use the optimizer on the regulatory
> domain prior to sending it to the kernel. All of a sudden you get full
> support for this for free on any kernel.

I'm not sure I get it. Whatever 'optimisation' you're doing surely can't
be changing the semantics of the rules, which are currently defined to
be separate and channels can't cross different rules. This may not be
all that interesting for most country entries today, but there are a few
that have overlapping rules and regardless, it's some form of API/ABI.

In any case, no optimiser can actually do what Janusz needs, since he
needs the ability to split up existing frequency ranges due to different
flags in parts of those, while keeping the ability to use channels that
use multiple ranges.

The only thing I could think of an optimiser doing is combine ranges,
but then it would lose flags which can't be right. And in fact combining
rules would be breaking the current db.txt format. I won't let you get
away with that on the kernel/userspace API (hence this patch), but if
you can get away with breaking it in CRDA I'm only going to roll my eyes
a bit :-)

johannes

Re: [PATCH v2 2/3] cfg80211: introduce regulatory wide bandwidth flag

[Luis R. Rodriguez]

On Wed, Jan 29, 2014 at 5:10 AM, Johannes Berg
<johannes@sipsolutions.net> wrote:
> On Fri, 2014-01-24 at 16:32 -0800, Luis R. Rodriguez wrote:
>
>> > This seems reasonable, thanks. Maybe we should require the bandwidth to
>> > not be set at all or something? At least maybe in the regdb parser - it
>> > makes very little sense to have @20 and then ignore it completely?
>> >
>> > Or maybe the userspace code could just not expose the flag, but rather
>> > set the new "wide_bw" flag when all the rules are marked as @N/A (and
>> > treat a combination of @number and @N/A as a bug)?
>>
>> The optimizer code I added to CRDA does all this for us, so technically,
>> unless I'm missing something, this could be dealt with magically in
>> userspace. Its also unclear why we'd define this as a regulatory
>> parameter -- this just seems to make sense.
>>
>> I'd look at extending CRDA binary to use the optimizer on the regulatory
>> domain prior to sending it to the kernel. All of a sudden you get full
>> support for this for free on any kernel.
>
> I'm not sure I get it. Whatever 'optimisation' you're doing surely can't
> be changing the semantics of the rules, which are currently defined to
> be separate and channels can't cross different rules. This may not be
> all that interesting for most country entries today, but there are a few
> that have overlapping rules and regardless, it's some form of API/ABI.

Sure.

> In any case, no optimiser can actually do what Janusz needs, since he
> needs the ability to split up existing frequency ranges due to different
> flags in parts of those, while keeping the ability to use channels that
> use multiple ranges.

Ah I see.

> The only thing I could think of an optimiser doing is combine ranges,
> but then it would lose flags which can't be right.

One thing is in-kernel rules, another is wireless-regdb. If you are
extending rules in wireless-regdb you may at times run into a
situation where two rules are contiguous and you know you can combine
them and want to, combination for example makes sense if your source
for the db has a set of contiguous frequency rules with the same
power, max eirp, and flags. That's what the optimizer available as a
library does in reglib, part of CRDA.

> And in fact combining rules would be breaking the current db.txt format.  I won't let you get
> away with that on the kernel/userspace API (hence this patch), but if
> you can get away with breaking it in CRDA I'm only going to roll my eyes
> a bit :-)

CRDA binary doesn't make use of the optimizer yet but it can.

  Luis

Re: [PATCH v2 2/3] cfg80211: introduce regulatory wide bandwidth flag

[Johannes Berg]

On Wed, 2014-01-29 at 16:41 -0800, Luis R. Rodriguez wrote:

> > The only thing I could think of an optimiser doing is combine ranges,
> > but then it would lose flags which can't be right.
> 
> One thing is in-kernel rules, another is wireless-regdb. If you are
> extending rules in wireless-regdb you may at times run into a
> situation where two rules are contiguous and you know you can combine
> them and want to, combination for example makes sense if your source
> for the db has a set of contiguous frequency rules with the same
> power, max eirp, and flags. That's what the optimizer available as a
> library does in reglib, part of CRDA.

My point mostly is that due to how the rules are interpreted (a single
channel must fit into a single frequency range), this isn't just an
optimisation, it actually has impact on the behaviour. So calling it an
'optimizer' is misleading, that implies that it's actually always
desired to do this, which isn't necessarily the case (it's quite likely
the case, and IMHO the old interpretation rules are stupid, but those
are what we're stuck with.)

johannes

[PATCH v2 3/3] cfg80211: parse WIDE-BW flag for internal regdb option

[Janusz Dziedzic]

Add support for parsing WIDE-BW country flag when
internal regulatory database is used.

Signed-off-by: Janusz Dziedzic <janusz.dziedzic@tieto.com>
---
 net/wireless/genregdb.awk |   26 ++++++++++++++++++++------
 1 file changed, 20 insertions(+), 6 deletions(-)

diff --git a/net/wireless/genregdb.awk b/net/wireless/genregdb.awk
index 9a8217d..22ffada 100644
--- a/net/wireless/genregdb.awk
+++ b/net/wireless/genregdb.awk
@@ -38,12 +38,26 @@ function parse_country_head() {
 	sub(/:/, "", country)
 	printf "static const struct ieee80211_regdomain regdom_%s = {\n", country
 	printf "\t.alpha2 = \"%s\",\n", country
-	if ($NF ~ /DFS-ETSI/)
-		printf "\t.dfs_region = NL80211_DFS_ETSI,\n"
-	else if ($NF ~ /DFS-FCC/)
-		printf "\t.dfs_region = NL80211_DFS_FCC,\n"
-	else if ($NF ~ /DFS-JP/)
-		printf "\t.dfs_region = NL80211_DFS_JP,\n"
+
+	flagstr = ""
+	dfs_region = ""
+	for (i=3; i<=NF; i++)
+		flagstr = flagstr $i
+	split(flagstr, flagarray, ",")
+	for (arg in flagarray) {
+		if (flagarray[arg] == "WIDE-BW")
+			printf "\t.flags = NL80211_REG_FLAG_WIDE_BW,\n"
+		if (flagarray[arg] == "DFS-ETSI")
+			dfs_region = "NL80211_DFS_ETSI"
+		if (flagarray[arg] == "DFS-FCC")
+			dfs_region = "NL80211_DFS_FCC"
+		if (flagarray[arg] == "DFS-JP")
+			dfs_region = "NL80211_DFS_JP"
+	}
+
+	if (dfs_region != "")
+		printf "\t.dfs_region = %s,\n", dfs_region
+
 	printf "\t.reg_rules = {\n"
 	active = 1
 	regdb = regdb "\t&regdom_" country ",\n"
-- 
1.7.9.5

Re: [PATCH v2 1/3] cfg80211: add helper reg_get_regdomain() function

[Luis R. Rodriguez]

On Wed, Jan 22, 2014 at 01:43:42PM +0100, Janusz Dziedzic wrote:
> Add helper function that will return regdomain.
> Follow the driver's regulatory domain, if present,
> unless a country IE has been processed or a user
> wants to help complaince further.
> 
> Signed-off-by: Janusz Dziedzic <janusz.dziedzi@tieto.com>
> ---
>  net/wireless/reg.c |   31 ++++++++++++++++++++-----------
>  1 file changed, 20 insertions(+), 11 deletions(-)
> 
> diff --git a/net/wireless/reg.c b/net/wireless/reg.c
> index 9b897fc..d58a09c 100644
> --- a/net/wireless/reg.c
> +++ b/net/wireless/reg.c
> @@ -522,6 +522,25 @@ bool reg_is_valid_request(const char *alpha2)
>  	return alpha2_equal(lr->alpha2, alpha2);
>  }
>  
> +static const struct ieee80211_regdomain *reg_get_regdomain(struct wiphy *wiphy)
> +{
> +	const struct ieee80211_regdomain *regd;
> +	struct regulatory_request *lr = get_last_request();
> +
> +	/*
> +	 * Follow the driver's regulatory domain, if present, unless a country
> +	 * IE has been processed or a user wants to help complaince further
> +	 */
> +	if (lr->initiator != NL80211_REGDOM_SET_BY_COUNTRY_IE &&
> +	    lr->initiator != NL80211_REGDOM_SET_BY_USER &&
> +	    wiphy->regd)
> +		regd = get_wiphy_regdom(wiphy);
> +	else
> +		regd = get_cfg80211_regdom();
> +
> +	return regd;
> +}
> +
>  /* Sanity check on a regulatory rule */
>  static bool is_valid_reg_rule(const struct ieee80211_reg_rule *rule)
>  {
> @@ -821,18 +840,8 @@ const struct ieee80211_reg_rule *freq_reg_info(struct wiphy *wiphy,
>  					       u32 center_freq)
>  {
>  	const struct ieee80211_regdomain *regd;
> -	struct regulatory_request *lr = get_last_request();
>  
> -	/*
> -	 * Follow the driver's regulatory domain, if present, unless a country
> -	 * IE has been processed or a user wants to help complaince further
> -	 */
> -	if (lr->initiator != NL80211_REGDOM_SET_BY_COUNTRY_IE &&
> -	    lr->initiator != NL80211_REGDOM_SET_BY_USER &&
> -	    wiphy->regd)
> -		regd = get_wiphy_regdom(wiphy);
> -	else
> -		regd = get_cfg80211_regdom();
> +	regd = reg_get_regdomain(wiphy);

Nice! I welcome this -- can you also add nl80211 API to expose when
a wiphy has its own wiphy->regd and send that to userspace too?

  Luis

Re: [PATCH v2 1/3] cfg80211: add helper reg_get_regdomain() function

[Janusz Dziedzic]

On 25 January 2014 01:34, Luis R. Rodriguez <mcgrof@do-not-panic.com> wrote:
> On Wed, Jan 22, 2014 at 01:43:42PM +0100, Janusz Dziedzic wrote:
>> Add helper function that will return regdomain.
>> Follow the driver's regulatory domain, if present,
>> unless a country IE has been processed or a user
>> wants to help complaince further.
>>
>> Signed-off-by: Janusz Dziedzic <janusz.dziedzi@tieto.com>
>> ---
>>  net/wireless/reg.c |   31 ++++++++++++++++++++-----------
>>  1 file changed, 20 insertions(+), 11 deletions(-)
>>
>> diff --git a/net/wireless/reg.c b/net/wireless/reg.c
>> index 9b897fc..d58a09c 100644
>> --- a/net/wireless/reg.c
>> +++ b/net/wireless/reg.c
>> @@ -522,6 +522,25 @@ bool reg_is_valid_request(const char *alpha2)
>>       return alpha2_equal(lr->alpha2, alpha2);
>>  }
>>
>> +static const struct ieee80211_regdomain *reg_get_regdomain(struct wiphy *wiphy)
>> +{
>> +     const struct ieee80211_regdomain *regd;
>> +     struct regulatory_request *lr = get_last_request();
>> +
>> +     /*
>> +      * Follow the driver's regulatory domain, if present, unless a country
>> +      * IE has been processed or a user wants to help complaince further
>> +      */
>> +     if (lr->initiator != NL80211_REGDOM_SET_BY_COUNTRY_IE &&
>> +         lr->initiator != NL80211_REGDOM_SET_BY_USER &&
>> +         wiphy->regd)
>> +             regd = get_wiphy_regdom(wiphy);
>> +     else
>> +             regd = get_cfg80211_regdom();
>> +
>> +     return regd;
>> +}
>> +
>>  /* Sanity check on a regulatory rule */
>>  static bool is_valid_reg_rule(const struct ieee80211_reg_rule *rule)
>>  {
>> @@ -821,18 +840,8 @@ const struct ieee80211_reg_rule *freq_reg_info(struct wiphy *wiphy,
>>                                              u32 center_freq)
>>  {
>>       const struct ieee80211_regdomain *regd;
>> -     struct regulatory_request *lr = get_last_request();
>>
>> -     /*
>> -      * Follow the driver's regulatory domain, if present, unless a country
>> -      * IE has been processed or a user wants to help complaince further
>> -      */
>> -     if (lr->initiator != NL80211_REGDOM_SET_BY_COUNTRY_IE &&
>> -         lr->initiator != NL80211_REGDOM_SET_BY_USER &&
>> -         wiphy->regd)
>> -             regd = get_wiphy_regdom(wiphy);
>> -     else
>> -             regd = get_cfg80211_regdom();
>> +     regd = reg_get_regdomain(wiphy);
>
> Nice! I welcome this -- can you also add nl80211 API to expose when
> a wiphy has its own wiphy->regd and send that to userspace too?
>
You mean we should introduce:
iw wlanX reg get?
while in last_request we have only last wiphy(lr->wiphy_idx).


BR
Janusz

Re: [PATCH v2 1/3] cfg80211: add helper reg_get_regdomain() function

[Luis R. Rodriguez]

On Tue, Jan 28, 2014 at 11:23 AM, Janusz Dziedzic
<janusz.dziedzic@tieto.com> wrote:
> On 25 January 2014 01:34, Luis R. Rodriguez <mcgrof@do-not-panic.com> wrote:
>> On Wed, Jan 22, 2014 at 01:43:42PM +0100, Janusz Dziedzic wrote:
>>> Add helper function that will return regdomain.
>>>
>>> +     regd = reg_get_regdomain(wiphy);
>>
>> Nice! I welcome this -- can you also add nl80211 API to expose when
>> a wiphy has its own wiphy->regd and send that to userspace too?
>>
> You mean we should introduce:
> iw wlanX reg get?

Sure, and iw dev reg get can also display what it does now but then
also iterate over all wiphys and also display all wiphy's own regd if
they have any.

> while in last_request we have only last wiphy(lr->wiphy_idx).

Right.

 Luis

Re: [PATCH v2 1/3] cfg80211: add helper reg_get_regdomain() function

[Johannes Berg]

On Tue, 2014-01-28 at 11:42 -0800, Luis R. Rodriguez wrote:

> > You mean we should introduce:
> > iw wlanX reg get?
> 
> Sure, and iw dev reg get can also display what it does now but then
> also iterate over all wiphys and also display all wiphy's own regd if
> they have any.

I'm not really sure I see much point in all this - what userspace
eventually cares about isn't the reg data but the channel list that can
be obtained with (the equivalent of) iw list.

johannes

Re: [PATCH v2 1/3] cfg80211: add helper reg_get_regdomain() function

[Luis R. Rodriguez]

On Wed, Jan 29, 2014 at 12:17 AM, Johannes Berg
<johannes@sipsolutions.net> wrote:
> On Tue, 2014-01-28 at 11:42 -0800, Luis R. Rodriguez wrote:
>
>> > You mean we should introduce:
>> > iw wlanX reg get?
>>
>> Sure, and iw dev reg get can also display what it does now but then
>> also iterate over all wiphys and also display all wiphy's own regd if
>> they have any.
>
> I'm not really sure I see much point in all this - what userspace
> eventually cares about isn't the reg data but the channel list that can
> be obtained with (the equivalent of) iw list.

That's true if you don't consider important the data that applied the
rules. This can be useful and if not exposed by userspace API I can at
least see this useful for debugfs. As for userspace API -- the data
reflects *calibrated* phy data, I see no reason why it shouldn't be
exposed just as other attributes of the phy, but I can see this
growing to other subjective areas, but I think we have things down
with a general usable generic regulatory domain data structure.

I can't say I care though :D just my $0.02 from a parking distance.

  Luis

Re: [PATCH v2 1/3] cfg80211: add helper reg_get_regdomain() function

[Luis R. Rodriguez]

  janusz.dziedzi@tieto.com bounces BTW

 Luis

On Wed, Jan 29, 2014 at 1:44 AM, Luis R. Rodriguez
<mcgrof@do-not-panic.com> wrote:
> On Wed, Jan 29, 2014 at 12:17 AM, Johannes Berg
> <johannes@sipsolutions.net> wrote:
>> On Tue, 2014-01-28 at 11:42 -0800, Luis R. Rodriguez wrote:
>>
>>> > You mean we should introduce:
>>> > iw wlanX reg get?
>>>
>>> Sure, and iw dev reg get can also display what it does now but then
>>> also iterate over all wiphys and also display all wiphy's own regd if
>>> they have any.
>>
>> I'm not really sure I see much point in all this - what userspace
>> eventually cares about isn't the reg data but the channel list that can
>> be obtained with (the equivalent of) iw list.
>
> That's true if you don't consider important the data that applied the
> rules. This can be useful and if not exposed by userspace API I can at
> least see this useful for debugfs. As for userspace API -- the data
> reflects *calibrated* phy data, I see no reason why it shouldn't be
> exposed just as other attributes of the phy, but I can see this
> growing to other subjective areas, but I think we have things down
> with a general usable generic regulatory domain data structure.
>
> I can't say I care though :D just my $0.02 from a parking distance.
>
>   Luis

Re: [PATCH v2 1/3] cfg80211: add helper reg_get_regdomain() function

[Johannes Berg]

On Wed, 2014-01-29 at 01:46 -0800, Luis R. Rodriguez wrote:
>   janusz.dziedzi@tieto.com bounces BTW

that's just because there's a missing 'c' in there :)

johannes