2011-11-23 15:41:03

by Luis R. Rodriguez

[permalink] [raw]
Subject: [RFC 0/5] wireless: applying country IE and reg changes immediately

The first patch in this series still needs some work as noted by Rajkumar Manoharan,
but it also made me think more how we are applying power setting and regulatory
settings. Turns out that the reason why the power settings will take effect
*soonish* is that we do things like scanning, and other hardware operations
that trigger applying the settings to hardware. But -- if we want to be even
more efficient we may want to apply the power settings.. say from the country IE
if we are allowed for more power ASAP. The gain is small though.. but this
series adds some junk to where I think we'd need to apply settings and here's
one way of doing it. I am not a fan of it but its one way of doing it.

Not sure if we have alternatives. Apart from power settings consider the
different types of channe types and if for whatever reason a new channel
type that we are on gets disabled. I don't think this is gong to happen...
because considering that if you connect to an AP -- chances are it is
allowed to use that channel on the same channel type you associated to it
with :) The only use case would be if the user manually set the regulatory
domain to override and help compliance.

OK I'm inclined to leave this as-is without all this junk code. But
will send out just as a brain excercise and to see if anyone else has
some thoughts. If we are fine living with the power settings from
the country IE not taking effect immediatley then great.. otherwise
we need something like this. The other thing that I'm not too happy
about is the usage of NL80211_TX_POWER_REG as API and userspace
would simply ignore that -- unless I guesss we send that as an event?
I can't find a better place for this though.

Luis R. Rodriguez (5):
cfg80211: allow following country IE power for custom regdom cards
cfg80211: add support to immediately apply reg settings
mac80211: add support for NL80211_TX_POWER_REG
ath6kl: add support for NL80211_TX_POWER_REG
wireless: annotate drivers that do not support NL80211_TX_POWER_REG

drivers/net/wireless/ath/ath6kl/cfg80211.c | 6 ++++
.../net/wireless/brcm80211/brcmfmac/wl_cfg80211.c | 2 +
drivers/net/wireless/iwmc3200wifi/cfg80211.c | 2 +
drivers/net/wireless/mwifiex/cfg80211.c | 3 ++
include/linux/nl80211.h | 4 +++
include/net/cfg80211.h | 4 ++-
net/mac80211/cfg.c | 6 ++++
net/wireless/core.c | 23 ++++++++++++++++
net/wireless/core.h | 2 +
net/wireless/nl80211.c | 5 +++
net/wireless/reg.c | 28 +++++++++++++++++---
11 files changed, 80 insertions(+), 5 deletions(-)

--
1.7.4.15.g7811d



2011-11-23 15:41:07

by Luis R. Rodriguez

[permalink] [raw]
Subject: [RFC 1/5] cfg80211: allow following country IE power for custom regdom cards

By definition WIPHY_FLAG_STRICT_REGULATORY was intended to allow the
wiphy to adjust itself to the country IE power information if the
card had no regulatory data but we had no way to tell cfg80211 that if
the card also had its own custom regulatory domain (these are typically
custom world regulatory domains) that we want to follow the country IE's
noted values for power for each channel. We add support for this and
document it.

This is not a critical fix but a performance optimization for cards
with custom regulatory domains that associate to an AP with sends
out country IEs with a higher EIRP than the one on the custom
regulatory domain. In practice the only driver affected right now
are the Atheros drivers as they are the only drivers using both
WIPHY_FLAG_STRICT_REGULATORY and WIPHY_FLAG_CUSTOM_REGULATORY --
used on cards that have an Atheros world regulatory domain. Cards
that have been programmed to follow a country specifically will not
follow the country IE power. So although not a stable fix distributions
should consider cherry picking this.

Please do consider though that although this does update the channel
max power, the hardware will not trigger that setting until another
hardware operation will be performed to kick the hardware. This
varies by driver. For mac80211 this means until we change channel
for example, or apply wiphy attributes, etc, anything that calls
hw_config().

Cc: [email protected]
Cc: Paul Stewart <[email protected]>
Cc: Rajkumar Manoharan <[email protected]>
Cc: Senthilkumar Balasubramanian <[email protected]>
Reported-by: Rajkumar Manoharan <[email protected]>
Signed-off-by: Luis R. Rodriguez <[email protected]>
---
include/net/cfg80211.h | 4 +++-
net/wireless/reg.c | 20 ++++++++++++++++----
2 files changed, 19 insertions(+), 5 deletions(-)

diff --git a/include/net/cfg80211.h b/include/net/cfg80211.h
index a992782..d21d764 100644
--- a/include/net/cfg80211.h
+++ b/include/net/cfg80211.h
@@ -1684,7 +1684,9 @@ struct cfg80211_ops {
* regulatory domain no user regulatory domain can enable these channels
* at a later time. This can be used for devices which do not have
* calibration information guaranteed for frequencies or settings
- * outside of its regulatory domain.
+ * outside of its regulatory domain. If used in combination with
+ * WIPHY_FLAG_CUSTOM_REGULATORY the inspected country IE power settings
+ * will be followed.
* @WIPHY_FLAG_DISABLE_BEACON_HINTS: enable this if your driver needs to ensure
* that passive scan flags and beaconing flags may not be lifted by
* cfg80211 due to regulatory beacon hints. For more information on beacon
diff --git a/net/wireless/reg.c b/net/wireless/reg.c
index 6049050..3f9f318 100644
--- a/net/wireless/reg.c
+++ b/net/wireless/reg.c
@@ -870,10 +870,22 @@ static void handle_channel(struct wiphy *wiphy,
chan->flags = flags | bw_flags | map_regdom_flags(reg_rule->flags);
chan->max_antenna_gain = min(chan->orig_mag,
(int) MBI_TO_DBI(power_rule->max_antenna_gain));
- if (chan->orig_mpwr)
- chan->max_power = min(chan->orig_mpwr,
- (int) MBM_TO_DBM(power_rule->max_eirp));
- else
+ if (chan->orig_mpwr) {
+ /*
+ * Devices that have their own custom regulatory domain
+ * but also use WIPHY_FLAG_STRICT_REGULATORY will follow the
+ * passed country IE power settings.
+ */
+ if (initiator == NL80211_REGDOM_SET_BY_COUNTRY_IE &&
+ wiphy->flags & WIPHY_FLAG_CUSTOM_REGULATORY &&
+ wiphy->flags & WIPHY_FLAG_STRICT_REGULATORY) {
+ chan->max_power =
+ MBM_TO_DBM(power_rule->max_eirp);
+ } else {
+ chan->max_power = min(chan->orig_mpwr,
+ (int) MBM_TO_DBM(power_rule->max_eirp));
+ }
+ } else
chan->max_power = (int) MBM_TO_DBM(power_rule->max_eirp);
}

--
1.7.4.15.g7811d


2011-11-23 15:41:19

by Luis R. Rodriguez

[permalink] [raw]
Subject: [RFC 5/5] wireless: annotate drivers that do not support NL80211_TX_POWER_REG

brcm80211, iwmc3200wifi and mwifiex lack support for
NL80211_TX_POWER_REG.

Signed-off-by: Luis R. Rodriguez <[email protected]>
---
.../net/wireless/brcm80211/brcmfmac/wl_cfg80211.c | 2 ++
drivers/net/wireless/iwmc3200wifi/cfg80211.c | 2 ++
drivers/net/wireless/mwifiex/cfg80211.c | 3 +++
3 files changed, 7 insertions(+), 0 deletions(-)

diff --git a/drivers/net/wireless/brcm80211/brcmfmac/wl_cfg80211.c b/drivers/net/wireless/brcm80211/brcmfmac/wl_cfg80211.c
index f23b0c3..678849e 100644
--- a/drivers/net/wireless/brcm80211/brcmfmac/wl_cfg80211.c
+++ b/drivers/net/wireless/brcm80211/brcmfmac/wl_cfg80211.c
@@ -1454,6 +1454,8 @@ brcmf_cfg80211_set_tx_power(struct wiphy *wiphy,
goto done;
}
break;
+ case NL80211_TX_POWER_REG:
+ return -EOPNOTSUPP;
}
/* Make sure radio is off or on as far as software is concerned */
disable = WL_RADIO_SW_DISABLE << 16;
diff --git a/drivers/net/wireless/iwmc3200wifi/cfg80211.c b/drivers/net/wireless/iwmc3200wifi/cfg80211.c
index 48e8218..d84390f 100644
--- a/drivers/net/wireless/iwmc3200wifi/cfg80211.c
+++ b/drivers/net/wireless/iwmc3200wifi/cfg80211.c
@@ -710,6 +710,8 @@ static int iwm_cfg80211_set_txpower(struct wiphy *wiphy,
return ret;

return iwm_tx_power_trigger(iwm);
+ case NL80211_TX_REG:
+ return -EOPNOTSUPP;
default:
IWM_ERR(iwm, "Unsupported power type: %d\n", type);
return -EOPNOTSUPP;
diff --git a/drivers/net/wireless/mwifiex/cfg80211.c b/drivers/net/wireless/mwifiex/cfg80211.c
index 0db97cc..136841b 100644
--- a/drivers/net/wireless/mwifiex/cfg80211.c
+++ b/drivers/net/wireless/mwifiex/cfg80211.c
@@ -126,6 +126,9 @@ mwifiex_cfg80211_set_tx_power(struct wiphy *wiphy,
struct mwifiex_power_cfg power_cfg;
int dbm = MBM_TO_DBM(mbm);

+ if (type == NL80211_TX_POWER_REG)
+ return -EOPNOTSUPP;
+
if (type == NL80211_TX_POWER_FIXED) {
power_cfg.is_power_auto = 0;
power_cfg.power_level = dbm;
--
1.7.4.15.g7811d


2011-11-24 16:50:24

by Luis R. Rodriguez

[permalink] [raw]
Subject: Re: [RFC 2/5] cfg80211: add support to immediately apply reg settings

On Thu, Nov 24, 2011 at 11:44 AM, Johannes Berg
<[email protected]> wrote:
> On Wed, 2011-11-23 at 10:40 -0500, Luis R. Rodriguez wrote:
>> It is useful to apply reguatory settings immediately. We already
>> handle applying regulatory hints immediatey for most hints but
>> for user hints and country IE hints we want to kick hardware
>> immediately to make these changes take effect. This adds supports
>> to let us kick hardware to make the changes take effect as soon
>> as possible.
>
> Can't say I like this -- wouldn't we rather call the regulatory update
> callback or so?

I'm not a fan either, at first I considered doing that on the
reg_notifier() but then that also means every driver must implement a
reg_notifier() callback, or a new reg_apply() callback. If we do it
within cfg80211 it means all drivers get this for free and that was
the tradeoff I wanted to review through these RFCs. Unfortunately
setting the tx power was the only thing I saw cfg80211 could do right
now.

I'm still "meh" about this too.

Luis

2011-11-23 15:41:13

by Luis R. Rodriguez

[permalink] [raw]
Subject: [RFC 3/5] mac80211: add support for NL80211_TX_POWER_REG

mac80211 already keeps track of user limits and uses the
regulatory limits properly, all we need to do is kick the
hw config routines so that we can kick the hardware.

Signed-off-by: Luis R. Rodriguez <[email protected]>
---
net/mac80211/cfg.c | 6 ++++++
1 files changed, 6 insertions(+), 0 deletions(-)

diff --git a/net/mac80211/cfg.c b/net/mac80211/cfg.c
index 2577c45..188cc7c 100644
--- a/net/mac80211/cfg.c
+++ b/net/mac80211/cfg.c
@@ -1666,6 +1666,12 @@ static int ieee80211_set_tx_power(struct wiphy *wiphy,
return -EINVAL;
local->user_power_level = MBM_TO_DBM(mbm);
break;
+ case NL80211_TX_POWER_REG:
+ /*
+ * ieee80211_hw_config() already has the proper logic
+ * to deal with this.
+ */
+ break;
}

ieee80211_hw_config(local, changes);
--
1.7.4.15.g7811d


2011-11-28 20:57:02

by Luis R. Rodriguez

[permalink] [raw]
Subject: Re: [RFC 5/5] wireless: annotate drivers that do not support NL80211_TX_POWER_REG

On Thu, Nov 24, 2011 at 4:02 PM, Jouni Malinen <[email protected]> wrote:
> On Wed, Nov 23, 2011 at 10:40:58AM -0500, Luis R. Rodriguez wrote:
>> brcm80211, iwmc3200wifi and mwifiex lack support for
>> NL80211_TX_POWER_REG.
>
>> diff --git a/drivers/net/wireless/iwmc3200wifi/cfg80211.c b/drivers/net/wireless/iwmc3200wifi/cfg80211.c
>> +     case NL80211_TX_REG:
>
> That would benefit from a test run with a compiler..

Thought I had.. sorry, dropping this in favor for a reg_notifier()
approach instead.

Luis

2011-11-23 15:41:16

by Luis R. Rodriguez

[permalink] [raw]
Subject: [RFC 4/5] ath6kl: add support for NL80211_TX_POWER_REG

Signed-off-by: Luis R. Rodriguez <[email protected]>
---
drivers/net/wireless/ath/ath6kl/cfg80211.c | 6 ++++++
1 files changed, 6 insertions(+), 0 deletions(-)

diff --git a/drivers/net/wireless/ath/ath6kl/cfg80211.c b/drivers/net/wireless/ath/ath6kl/cfg80211.c
index 9f3d3f0..983c3e9 100644
--- a/drivers/net/wireless/ath/ath6kl/cfg80211.c
+++ b/drivers/net/wireless/ath/ath6kl/cfg80211.c
@@ -1187,6 +1187,7 @@ static int ath6kl_cfg80211_set_txpower(struct wiphy *wiphy,
struct ath6kl_vif *vif;
u8 ath6kl_dbm;
int dbm = MBM_TO_DBM(mbm);
+ struct ieee80211_channel *chan;

ath6kl_dbg(ATH6KL_DBG_WLAN_CFG, "%s: type 0x%x, dbm %d\n", __func__,
type, dbm);
@@ -1204,6 +1205,11 @@ static int ath6kl_cfg80211_set_txpower(struct wiphy *wiphy,
case NL80211_TX_POWER_LIMITED:
ar->tx_pwr = ath6kl_dbm = dbm;
break;
+ case NL80211_TX_POWER_REG:
+ chan = ieee80211_get_channel(wiphy, vif->next_chan);
+ if (!chan)
+ return -EIO;
+ ar->tx_pwr = ath6kl_dbm = chan->max_power;
default:
ath6kl_dbg(ATH6KL_DBG_WLAN_CFG, "%s: type 0x%x not supported\n",
__func__, type);
--
1.7.4.15.g7811d


2011-11-24 21:02:29

by Jouni Malinen

[permalink] [raw]
Subject: Re: [RFC 5/5] wireless: annotate drivers that do not support NL80211_TX_POWER_REG

On Wed, Nov 23, 2011 at 10:40:58AM -0500, Luis R. Rodriguez wrote:
> brcm80211, iwmc3200wifi and mwifiex lack support for
> NL80211_TX_POWER_REG.

> diff --git a/drivers/net/wireless/iwmc3200wifi/cfg80211.c b/drivers/net/wireless/iwmc3200wifi/cfg80211.c
> + case NL80211_TX_REG:

That would benefit from a test run with a compiler..

--
Jouni Malinen PGP id EFC895FA

2011-11-24 16:44:48

by Johannes Berg

[permalink] [raw]
Subject: Re: [RFC 2/5] cfg80211: add support to immediately apply reg settings

On Wed, 2011-11-23 at 10:40 -0500, Luis R. Rodriguez wrote:
> It is useful to apply reguatory settings immediately. We already
> handle applying regulatory hints immediatey for most hints but
> for user hints and country IE hints we want to kick hardware
> immediately to make these changes take effect. This adds supports
> to let us kick hardware to make the changes take effect as soon
> as possible.

Can't say I like this -- wouldn't we rather call the regulatory update
callback or so?

johannes


2011-11-23 15:41:10

by Luis R. Rodriguez

[permalink] [raw]
Subject: [RFC 2/5] cfg80211: add support to immediately apply reg settings

It is useful to apply reguatory settings immediately. We already
handle applying regulatory hints immediatey for most hints but
for user hints and country IE hints we want to kick hardware
immediately to make these changes take effect. This adds supports
to let us kick hardware to make the changes take effect as soon
as possible.

Signed-off-by: Luis R. Rodriguez <[email protected]>
---
include/linux/nl80211.h | 4 ++++
net/wireless/core.c | 23 +++++++++++++++++++++++
net/wireless/core.h | 2 ++
net/wireless/nl80211.c | 5 +++++
net/wireless/reg.c | 8 ++++++++
5 files changed, 42 insertions(+), 0 deletions(-)

diff --git a/include/linux/nl80211.h b/include/linux/nl80211.h
index 97bfebf..5f68f60 100644
--- a/include/linux/nl80211.h
+++ b/include/linux/nl80211.h
@@ -2452,11 +2452,15 @@ enum nl80211_cqm_rssi_threshold_event {
* @NL80211_TX_POWER_AUTOMATIC: automatically determine transmit power
* @NL80211_TX_POWER_LIMITED: limit TX power by the mBm parameter
* @NL80211_TX_POWER_FIXED: fix TX power to the mBm parameter
+ * @NL80211_TX_POWER_REG: adjust tx power based on the latest
+ * regulatory update. The driver must inspect the chan->max_power,
+ * the mBm power passed will always be 0 and can be ignored.
*/
enum nl80211_tx_power_setting {
NL80211_TX_POWER_AUTOMATIC,
NL80211_TX_POWER_LIMITED,
NL80211_TX_POWER_FIXED,
+ NL80211_TX_POWER_REG,
};

/**
diff --git a/net/wireless/core.c b/net/wireless/core.c
index ccdfed8..9cac8c3 100644
--- a/net/wireless/core.c
+++ b/net/wireless/core.c
@@ -478,6 +478,29 @@ static int wiphy_verify_combinations(struct wiphy *wiphy)
return 0;
}

+/*
+ * Kicks the hardware to apply immediately any needed regulatory
+ * changes. This is an optimization to ensure values are propagated
+ * to hardware right away. Most regulatory hints are applied immediately
+ * but for country IE hints and user hints this will ensure we apply the
+ * changes immediately.
+ */
+int wiphy_reg_config_update(struct wiphy *wiphy)
+{
+ int r;
+ struct cfg80211_registered_device *rdev = wiphy_to_dev(wiphy);
+
+ if (!rdev || !rdev->ops->set_tx_power)
+ return -EINVAL;
+ r = rdev->ops->set_tx_power(wiphy, NL80211_TX_POWER_REG, 0);
+ /*
+ * XXX: trigger disconnect if operating channel / channel type
+ * is not allowed due to a new regulatory update. What do we
+ * want to do here?
+ */
+ return r;
+}
+
int wiphy_register(struct wiphy *wiphy)
{
struct cfg80211_registered_device *rdev = wiphy_to_dev(wiphy);
diff --git a/net/wireless/core.h b/net/wireless/core.h
index fb08c28..6c852d3 100644
--- a/net/wireless/core.h
+++ b/net/wireless/core.h
@@ -462,6 +462,8 @@ int ieee80211_get_ratemask(struct ieee80211_supported_band *sband,
int cfg80211_validate_beacon_int(struct cfg80211_registered_device *rdev,
u32 beacon_int);

+int wiphy_reg_config_update(struct wiphy *wiphy);
+
#ifdef CONFIG_CFG80211_DEVELOPER_WARNINGS
#define CFG80211_DEV_WARN_ON(cond) WARN_ON(cond)
#else
diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c
index a1cabde..3dd2e91 100644
--- a/net/wireless/nl80211.c
+++ b/net/wireless/nl80211.c
@@ -1329,6 +1329,11 @@ static int nl80211_set_wiphy(struct sk_buff *skb, struct genl_info *info)
idx = NL80211_ATTR_WIPHY_TX_POWER_SETTING;
type = nla_get_u32(info->attrs[idx]);

+ if (type == NL80211_TX_POWER_REG) {
+ result = -EOPNOTSUPP;
+ goto bad_res;
+ }
+
if (!info->attrs[NL80211_ATTR_WIPHY_TX_POWER_LEVEL] &&
(type != NL80211_TX_POWER_AUTOMATIC)) {
result = -EINVAL;
diff --git a/net/wireless/reg.c b/net/wireless/reg.c
index 3f9f318..38b1214 100644
--- a/net/wireless/reg.c
+++ b/net/wireless/reg.c
@@ -1142,6 +1142,7 @@ static void wiphy_update_regulatory(struct wiphy *wiphy,
enum nl80211_reg_initiator initiator)
{
enum ieee80211_band band;
+ int r;

assert_reg_lock();

@@ -1159,6 +1160,13 @@ static void wiphy_update_regulatory(struct wiphy *wiphy,
reg_process_ht_flags(wiphy);
if (wiphy->reg_notifier)
wiphy->reg_notifier(wiphy, last_request);
+
+ r = wiphy_reg_config_update(wiphy);
+ if (!r) {
+ REG_DBG_PRINT("Unable to immediately apply "
+ "regulatory updates on %s\n",
+ wiphy_name(wiphy));
+ }
}

void regulatory_update(struct wiphy *wiphy,
--
1.7.4.15.g7811d


2011-11-24 16:52:37

by Luis R. Rodriguez

[permalink] [raw]
Subject: Re: [RFC 2/5] cfg80211: add support to immediately apply reg settings

On Thu, Nov 24, 2011 at 11:50 AM, Luis R. Rodriguez
<[email protected]> wrote:
> On Thu, Nov 24, 2011 at 11:44 AM, Johannes Berg
> <[email protected]> wrote:
>> On Wed, 2011-11-23 at 10:40 -0500, Luis R. Rodriguez wrote:
>>> It is useful to apply reguatory settings immediately. We already
>>> handle applying regulatory hints immediatey for most hints but
>>> for user hints and country IE hints we want to kick hardware
>>> immediately to make these changes take effect. This adds supports
>>> to let us kick hardware to make the changes take effect as soon
>>> as possible.
>>
>> Can't say I like this -- wouldn't we rather call the regulatory update
>> callback or so?
>
> I'm not a fan either, at first I considered doing that on the
> reg_notifier() but then that also means every driver must implement a
> reg_notifier() callback, or a new reg_apply() callback. If we do it
> within cfg80211 it means all drivers get this for free and that was
> the tradeoff I wanted to review through these RFCs. Unfortunately
> setting the tx power was the only thing I saw cfg80211 could do right
> now.
>
> I'm still "meh" about this too.

I'm willing to give up on the complexity of this given that we *will*
eventually apply the settings after a scan, etc. The gains seem to be
small for what we would need to do. If we're happy to live without the
optimization I can just document it to ensure everyone knows that the
power stuff won't apply *immediately* but it will happen eventually
and typically this is soon enough.

Luis