2015-06-11 11:06:39

by Matthias May

[permalink] [raw]
Subject: [PATCHv2 0/4] Handle minimum bandwidth for quarter and half rates

The goal of this patch series is to improve regulatory handling of quarter and half rates.

v2: Changes based on feedback by Johannes Berg.

Matthias May (4):
cfg80211: handle minimum bandwidth for quarter and half rate
ath: send minimum bandwidth to handle.
brcm80211: send minimum bandwidth to handle
rtlwifi: send minimum bandwidth to handle

drivers/net/wireless/ath/regd.c | 3 +-
drivers/net/wireless/brcm80211/brcmsmac/channel.c | 3 +-
drivers/net/wireless/rtlwifi/regd.c | 7 ++--
include/net/cfg80211.h | 3 +-
net/wireless/reg.c | 46 ++++++++++++++++++-----
5 files changed, 47 insertions(+), 15 deletions(-)

--
2.1.4



2015-06-11 11:06:39

by Matthias May

[permalink] [raw]
Subject: [PATCHv2 4/4] rtlwifi: send minimum bandwidth to handle

add 20MHz minimum bandwidth to the calls of freq_reg_info()

Signed-off-by: Matthias May <[email protected]>
---
drivers/net/wireless/rtlwifi/regd.c | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/net/wireless/rtlwifi/regd.c b/drivers/net/wireless/rtlwifi/regd.c
index a62bf0a..bbc3cb9 100644
--- a/drivers/net/wireless/rtlwifi/regd.c
+++ b/drivers/net/wireless/rtlwifi/regd.c
@@ -174,7 +174,8 @@ static void _rtl_reg_apply_beaconing_flags(struct wiphy *wiphy,
continue;
if (initiator == NL80211_REGDOM_SET_BY_COUNTRY_IE) {
reg_rule = freq_reg_info(wiphy,
- ch->center_freq);
+ ch->center_freq,
+ MHZ_TO_KHZ(20));
if (IS_ERR(reg_rule))
continue;
/*
@@ -236,7 +237,7 @@ static void _rtl_reg_apply_active_scan_flags(struct wiphy *wiphy,
*/

ch = &sband->channels[11]; /* CH 12 */
- reg_rule = freq_reg_info(wiphy, ch->center_freq);
+ reg_rule = freq_reg_info(wiphy, ch->center_freq, MHZ_TO_KHZ(20));
if (!IS_ERR(reg_rule)) {
if (!(reg_rule->flags & NL80211_RRF_PASSIVE_SCAN))
if (ch->flags & IEEE80211_CHAN_PASSIVE_SCAN)
@@ -244,7 +245,7 @@ static void _rtl_reg_apply_active_scan_flags(struct wiphy *wiphy,
}

ch = &sband->channels[12]; /* CH 13 */
- reg_rule = freq_reg_info(wiphy, ch->center_freq);
+ reg_rule = freq_reg_info(wiphy, ch->center_freq, MHZ_TO_KHZ(20));
if (!IS_ERR(reg_rule)) {
if (!(reg_rule->flags & NL80211_RRF_PASSIVE_SCAN))
if (ch->flags & IEEE80211_CHAN_PASSIVE_SCAN)
--
2.1.4


2015-06-11 11:06:39

by Matthias May

[permalink] [raw]
Subject: [PATCHv2 2/4] ath: send minimum bandwidth to handle.

add 20MHz minimum bandwidth to the calls of freq_reg_info()

Signed-off-by: Matthias May <[email protected]>
---
drivers/net/wireless/ath/regd.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/net/wireless/ath/regd.c b/drivers/net/wireless/ath/regd.c
index 06ea6cc..57b2e44 100644
--- a/drivers/net/wireless/ath/regd.c
+++ b/drivers/net/wireless/ath/regd.c
@@ -264,7 +264,8 @@ static void ath_force_clear_no_ir_chan(struct wiphy *wiphy,
{
const struct ieee80211_reg_rule *reg_rule;

- reg_rule = freq_reg_info(wiphy, MHZ_TO_KHZ(ch->center_freq));
+ reg_rule = freq_reg_info(wiphy, MHZ_TO_KHZ(ch->center_freq),
+ MHZ_TO_KHZ(20));
if (IS_ERR(reg_rule))
return;

--
2.1.4


2015-06-11 15:22:32

by Johannes Berg

[permalink] [raw]
Subject: Re: [PATCHv2 1/4] cfg80211: handle minimum bandwidth for quarter and half rates

On Thu, 2015-06-11 at 14:46 +0200, Matthias May wrote:

> Using the dummy country code:
> country XS: DFS-UNSET
> (5170 - 5180 @ 5), (N/A, 14), (N/A)
> (5175 - 5185 @ 10), (N/A, 17), (N/A)
> (5170 - 5190 @ 20), (N/A, 20), (N/A)
>
> For a center freq of 5175
> The first loop finds the first rule. --> we want this.
> The second loop doesn't find anything.
> The third loop doesn't find anything.
>
> However for a center freq of 5180
> The first loop find the first rule
> The second loop find the second rule
> The third loop find the third rule. --> We want this
>
> If there are different overlapping rules for the different bandwidths
> different results can be returned, especially at the border of a
> frequency range.

Hmmm. Can we just forbid those overlapping rules, and say you want
AUTO-BW?

I mean, do you see a reason for this otherwise?

johannes


2015-06-11 11:44:22

by Johannes Berg

[permalink] [raw]
Subject: Re: [PATCHv2 1/4] cfg80211: handle minimum bandwidth for quarter and half rates

On Thu, 2015-06-11 at 13:06 +0200, Matthias May wrote:

> const struct ieee80211_reg_rule *freq_reg_info(struct wiphy *wiphy,
> - u32 center_freq)
> + u32 center_freq, u32 min_bw)
> {
> const struct ieee80211_regdomain *regd;
> + const struct ieee80211_reg_rule *rr_tmp = NULL;
> + const struct ieee80211_reg_rule *reg_rule = NULL;
> + u32 bw;
>
> regd = reg_get_regdomain(wiphy);
>
> - return freq_reg_info_regd(wiphy, center_freq, regd);
> + for(bw=min_bw; bw <= MHZ_TO_KHZ(20); bw=bw*2) {
> + rr_tmp = freq_reg_info_regd(wiphy, center_freq, bw, regd);
> + if(!reg_rule || !IS_ERR(rr_tmp))
> + reg_rule = rr_tmp;
> + }
> +
> + return reg_rule;

This makes no sense to me.

You have the same code below, but why would you care? You can just look
at the reg_rule's max_bandwidth, no?

johannes


2015-06-11 12:46:27

by Matthias May

[permalink] [raw]
Subject: Re: [PATCHv2 1/4] cfg80211: handle minimum bandwidth for quarter and half rates

On 11/06/15 13:44, Johannes Berg wrote:
> On Thu, 2015-06-11 at 13:06 +0200, Matthias May wrote:
>
>> const struct ieee80211_reg_rule *freq_reg_info(struct wiphy *wiphy,
>> - u32 center_freq)
>> + u32 center_freq, u32 min_bw)
>> {
>> const struct ieee80211_regdomain *regd;
>> + const struct ieee80211_reg_rule *rr_tmp = NULL;
>> + const struct ieee80211_reg_rule *reg_rule = NULL;
>> + u32 bw;
>>
>> regd = reg_get_regdomain(wiphy);
>>
>> - return freq_reg_info_regd(wiphy, center_freq, regd);
>> + for(bw=min_bw; bw <= MHZ_TO_KHZ(20); bw=bw*2) {
>> + rr_tmp = freq_reg_info_regd(wiphy, center_freq, bw, regd);
>> + if(!reg_rule || !IS_ERR(rr_tmp))
>> + reg_rule = rr_tmp;
>> + }
>> +
>> + return reg_rule;
> This makes no sense to me.
>
> You have the same code below, but why would you care? You can just look
> at the reg_rule's max_bandwidth, no?
>
> johannes
>
Using the dummy country code:
country XS: DFS-UNSET
(5170 - 5180 @ 5), (N/A, 14), (N/A)
(5175 - 5185 @ 10), (N/A, 17), (N/A)
(5170 - 5190 @ 20), (N/A, 20), (N/A)

For a center freq of 5175
The first loop finds the first rule. --> we want this.
The second loop doesn't find anything.
The third loop doesn't find anything.

However for a center freq of 5180
The first loop find the first rule
The second loop find the second rule
The third loop find the third rule. --> We want this

If there are different overlapping rules for the different bandwidths
different results can be returned, especially at the border of a
frequency range.

A work colleague pointed out that it could be done more efficiently:

const struct ieee80211_regdomain *regd;
const struct ieee80211_reg_rule *reg_rule = NULL;
u32 bw;

regd = reg_get_regdomain(wiphy);

for (bw = MHZ_TO_KHZ(20); bw >= min_bw; bw = bw / 2) {
reg_rule = freq_reg_info_regd(wiphy, center_freq, bw, regd);
if (!IS_ERR(reg_rule))
return reg_rule;
}

return reg_rule;

However after some more discussion we noticed, that this again would
allow to start 20MHz wide operation at the border
This is because when setting the flags we only look at the max_bandwidth
of the returned rule.
I guess we would have to recheck if a certain width fits a center freq
when setting this flags.

Will have to think a bit more how to solve this without having to
recheck everything again...

Matthias

2015-06-11 11:33:30

by Johannes Berg

[permalink] [raw]
Subject: Re: [PATCHv2 1/4] cfg80211: handle minimum bandwidth for quarter and half rates

On Thu, 2015-06-11 at 13:06 +0200, Matthias May wrote:
> This patch changes the API of freq_freq_info() to take as additional argument
> the minimum bandwidth the caller can handle.
> If multiple rules match, the match with the widest bandwidth is returned.

You can't make a patch like this that breaks the compile - all these
need to be a single patch.

That said, you could get away with not changing the public
freq_reg_info() API since the callers thereof don't need it, you could
make a __freq_reg_info() that has the bandwidth and freq_reg_info() that
doesn't.

johannes


2015-06-11 11:06:39

by Matthias May

[permalink] [raw]
Subject: [PATCHv2 1/4] cfg80211: handle minimum bandwidth for quarter and half rates

This patch changes the API of freq_freq_info() to take as additional argument
the minimum bandwidth the caller can handle.
If multiple rules match, the match with the widest bandwidth is returned.

Signed-off-by: Matthias May <[email protected]>
---
include/net/cfg80211.h | 3 ++-
net/wireless/reg.c | 46 +++++++++++++++++++++++++++++++++++++---------
2 files changed, 39 insertions(+), 10 deletions(-)

diff --git a/include/net/cfg80211.h b/include/net/cfg80211.h
index a741678..ac5d8e8 100644
--- a/include/net/cfg80211.h
+++ b/include/net/cfg80211.h
@@ -3876,6 +3876,7 @@ void wiphy_apply_custom_regulatory(struct wiphy *wiphy,
* freq_reg_info - get regulatory information for the given frequency
* @wiphy: the wiphy for which we want to process this rule for
* @center_freq: Frequency in KHz for which we want regulatory information for
+ * @min_bw: The minimum bandwidth in KHz the caller can handle
*
* Use this function to get the regulatory rule for a specific frequency on
* a given wireless device. If the device has a specific regulatory domain
@@ -3891,7 +3892,7 @@ void wiphy_apply_custom_regulatory(struct wiphy *wiphy,
* purely subjective and right now it's 802.11 specific.
*/
const struct ieee80211_reg_rule *freq_reg_info(struct wiphy *wiphy,
- u32 center_freq);
+ u32 center_freq, u32 min_bw);

/**
* reg_initiator_name - map regulatory request initiator enum to name
diff --git a/net/wireless/reg.c b/net/wireless/reg.c
index d359e06..c4f85bb 100644
--- a/net/wireless/reg.c
+++ b/net/wireless/reg.c
@@ -1003,7 +1003,7 @@ static u32 map_regdom_flags(u32 rd_flags)
}

static const struct ieee80211_reg_rule *
-freq_reg_info_regd(struct wiphy *wiphy, u32 center_freq,
+freq_reg_info_regd(struct wiphy *wiphy, u32 center_freq, u32 bw,
const struct ieee80211_regdomain *regd)
{
int i;
@@ -1028,7 +1028,7 @@ freq_reg_info_regd(struct wiphy *wiphy, u32 center_freq,
if (!band_rule_found)
band_rule_found = freq_in_rule_band(fr, center_freq);

- bw_fits = reg_does_bw_fit(fr, center_freq, MHZ_TO_KHZ(20));
+ bw_fits = reg_does_bw_fit(fr, center_freq, bw);

if (band_rule_found && bw_fits)
return rr;
@@ -1041,13 +1041,22 @@ freq_reg_info_regd(struct wiphy *wiphy, u32 center_freq,
}

const struct ieee80211_reg_rule *freq_reg_info(struct wiphy *wiphy,
- u32 center_freq)
+ u32 center_freq, u32 min_bw)
{
const struct ieee80211_regdomain *regd;
+ const struct ieee80211_reg_rule *rr_tmp = NULL;
+ const struct ieee80211_reg_rule *reg_rule = NULL;
+ u32 bw;

regd = reg_get_regdomain(wiphy);

- return freq_reg_info_regd(wiphy, center_freq, regd);
+ for(bw=min_bw; bw <= MHZ_TO_KHZ(20); bw=bw*2) {
+ rr_tmp = freq_reg_info_regd(wiphy, center_freq, bw, regd);
+ if(!reg_rule || !IS_ERR(rr_tmp))
+ reg_rule = rr_tmp;
+ }
+
+ return reg_rule;
}
EXPORT_SYMBOL(freq_reg_info);

@@ -1134,7 +1143,8 @@ static void handle_channel(struct wiphy *wiphy,

flags = chan->orig_flags;

- reg_rule = freq_reg_info(wiphy, MHZ_TO_KHZ(chan->center_freq));
+ reg_rule = freq_reg_info(wiphy, MHZ_TO_KHZ(chan->center_freq),
+ MHZ_TO_KHZ(20));
if (IS_ERR(reg_rule)) {
/*
* We will disable all channels that do not match our
@@ -1176,8 +1186,12 @@ static void handle_channel(struct wiphy *wiphy,
if (reg_rule->flags & NL80211_RRF_AUTO_BW)
max_bandwidth_khz = reg_get_max_bandwidth(regd, reg_rule);

+ if (max_bandwidth_khz < MHZ_TO_KHZ(10))
+ bw_flags = IEEE80211_CHAN_NO_10MHZ;
+ if (max_bandwidth_khz < MHZ_TO_KHZ(20))
+ bw_flags |= IEEE80211_CHAN_NO_20MHZ;
if (max_bandwidth_khz < MHZ_TO_KHZ(40))
- bw_flags = IEEE80211_CHAN_NO_HT40;
+ bw_flags |= IEEE80211_CHAN_NO_HT40;
if (max_bandwidth_khz < MHZ_TO_KHZ(80))
bw_flags |= IEEE80211_CHAN_NO_80MHZ;
if (max_bandwidth_khz < MHZ_TO_KHZ(160))
@@ -1692,12 +1706,22 @@ static void handle_channel_custom(struct wiphy *wiphy,
{
u32 bw_flags = 0;
const struct ieee80211_reg_rule *reg_rule = NULL;
+ const struct ieee80211_reg_rule *rr_tmp = NULL;
const struct ieee80211_power_rule *power_rule = NULL;
const struct ieee80211_freq_range *freq_range = NULL;
u32 max_bandwidth_khz;
+ u32 bw;

- reg_rule = freq_reg_info_regd(wiphy, MHZ_TO_KHZ(chan->center_freq),
- regd);
+ /* Check if a frequency fits a rule for 5, 10 and 20MHz in that order.
+ * Use the rule with the widest bandwidth that fits.
+ */
+ for(bw=MHZ_TO_KHZ(5); bw <= MHZ_TO_KHZ(20); bw=bw*2) {
+ rr_tmp = freq_reg_info_regd(wiphy,
+ MHZ_TO_KHZ(chan->center_freq), bw,
+ regd);
+ if(!reg_rule || !IS_ERR(rr_tmp))
+ reg_rule = rr_tmp;
+ }

if (IS_ERR(reg_rule)) {
REG_DBG_PRINT("Disabling freq %d MHz as custom regd has no rule that fits it\n",
@@ -1721,8 +1745,12 @@ static void handle_channel_custom(struct wiphy *wiphy,
if (reg_rule->flags & NL80211_RRF_AUTO_BW)
max_bandwidth_khz = reg_get_max_bandwidth(regd, reg_rule);

+ if (max_bandwidth_khz < MHZ_TO_KHZ(10))
+ bw_flags = IEEE80211_CHAN_NO_10MHZ;
+ if (max_bandwidth_khz < MHZ_TO_KHZ(20))
+ bw_flags |= IEEE80211_CHAN_NO_20MHZ;
if (max_bandwidth_khz < MHZ_TO_KHZ(40))
- bw_flags = IEEE80211_CHAN_NO_HT40;
+ bw_flags |= IEEE80211_CHAN_NO_HT40;
if (max_bandwidth_khz < MHZ_TO_KHZ(80))
bw_flags |= IEEE80211_CHAN_NO_80MHZ;
if (max_bandwidth_khz < MHZ_TO_KHZ(160))
--
2.1.4


2015-06-11 11:06:39

by Matthias May

[permalink] [raw]
Subject: [PATCHv2 3/4] brcm80211: send minimum bandwidth to handle

add 20MHz minimum bandwidth to the calls of freq_reg_info()

Signed-off-by: Matthias May <[email protected]>
---
drivers/net/wireless/brcm80211/brcmsmac/channel.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/net/wireless/brcm80211/brcmsmac/channel.c b/drivers/net/wireless/brcm80211/brcmsmac/channel.c
index 635ae03..1de5129 100644
--- a/drivers/net/wireless/brcm80211/brcmsmac/channel.c
+++ b/drivers/net/wireless/brcm80211/brcmsmac/channel.c
@@ -680,7 +680,8 @@ brcms_reg_apply_beaconing_flags(struct wiphy *wiphy,

if (initiator == NL80211_REGDOM_SET_BY_COUNTRY_IE) {
rule = freq_reg_info(wiphy,
- MHZ_TO_KHZ(ch->center_freq));
+ MHZ_TO_KHZ(ch->center_freq),
+ MHZ_TO_KHZ(20));
if (IS_ERR(rule))
continue;

--
2.1.4