2012-07-03 23:15:21

by Luis R. Rodriguez

[permalink] [raw]
Subject: [PATCH 0/4] wireless: certification onus and cell regulatory hint

From: "Luis R. Rodriguez" <[email protected]>

This is a second series on the wireless expert idea patches
with one additional patch added. As discussed the wireless expert
name really was not doing justice to the intent behind what we
wanted to convey and allow. The CONFIG_CFG80211_CERTIFICATION_ONUS
is what folks seemed to agree on, I've gone ahead and added some
lanaguage which I think represents the intent behind the option
clearly.

I've also added an extra patch which adds a new type of regulatory
hint which actually makes use of the new kernel configuration option
CONFIG_CFG80211_CERTIFICATION_ONUS. In this case the option is designed
to allow userspace to classify userspace regulatory hints either as
coming from a user or a cellular base station. If devices have been
tested with such a feature the driver could be annotated as such, this
typically may require a set of testing / perhaps some communication to
the firmware.

Open solutions obviously allow users to hack up their own code and
send random data to the kernel, however the intent behind the new
kernel option CONFIG_CFG80211_CERTIFICATION_ONUS is to allow a new
type of hint which we *do* want to treat differently in kernel space
and drivers. Linux distribtuions / system integrators can use this
new regulatory hint by classifying the regulatory hint using a new
attribute. Exactly how userspace propagates the cellular base station
hint to the kernel is outside the exact scope of this series, however,
I suspect userspace cell base station models could end up using dbus
signals to trigger an event to signal the respective regulatory hint.
Using something like geoclue would make sense.

An interesting side effect of supporting this type of new regulatory
hint is addressing which type of hints takes precedence: do we trust
the cell base station hint over an Access Point's country IE? In this
series that is what we do, we prefer the cell base station hint over
other hints mainly to also simplify the implementation and design.
This also has implications with as to what gets applied to the core
and to other drivers. For example the core will always trust the
cell base station hint if CONFIG_CFG80211_CERTIFICATION_ONUS was
enabled *but* a driver may wish to want to ignore these type of
hints. In such case then the core, with a cell base station hint
present, would not be passing along country IE hints. This soft of
corner case must be considered.

We must also consider what we do upon suspend / disconnect. We follow
the tradition of the existing implementation of how we handle
disconnect / suspend -- we reset the regulatory core to its default,
just as if we had booted a device for the first time. We do this given
that possible scenerio that you got last a cell base station hint
in Japan but resume the device in the US, in such cases you could
not initiate radiation on channel 13, for example.

The way this is implemented however is to disable this feature unless
both CONFIG_CFG80211_CERTIFICATION_ONUS *and* the driver explicitly
enable this feature. As such regressions should only be found by
those users using the new feature and willing to participate on
the development of the feature / idea or the cell base station
regulatory hint.

Luis R. Rodriguez (4):
cfg80211: add CONFIG_CFG80211_CERTIFICATION_ONUS
ath5k: replace modparam_all_channels with CONFIG_ATH5K_TEST_CHANNELS
ath9k: make CONFIG_ATH9K_DFS_CERTIFIED depend on
CFG80211_CERTIFICATION_ONUS
cfg80211: add cellular base station regulatory hint support

drivers/net/wireless/ath/ath5k/Kconfig | 8 +++
drivers/net/wireless/ath/ath5k/base.c | 17 +++---
drivers/net/wireless/ath/ath9k/Kconfig | 2 +-
include/linux/nl80211.h | 28 ++++++++++
include/net/regulatory.h | 4 ++
net/wireless/Kconfig | 21 ++++++++
net/wireless/nl80211.c | 15 +++++-
net/wireless/reg.c | 88 +++++++++++++++++++++++++++++---
net/wireless/reg.h | 4 +-
9 files changed, 171 insertions(+), 16 deletions(-)

--
1.7.10.rc1.22.gf5241



2012-07-03 23:15:49

by Luis R. Rodriguez

[permalink] [raw]
Subject: [PATCH 4/4] cfg80211: add cellular base station regulatory hint support

From: "Luis R. Rodriguez" <[email protected]>

Cellular base stations can provide hints to cfg80211 about
where they think we are. This can be done for example by on
a cell phone. To enable these hints we simply allow them
through as user regulatory hints but we allow userspace
to clasify the hint. This option is only available for
system integrators which are willing to enable
CONFIG_CFG80211_CERTIFICATION_ONUS.

Signed-off-by: Luis R. Rodriguez <[email protected]>
---
include/linux/nl80211.h | 28 +++++++++++++++
include/net/regulatory.h | 4 +++
net/wireless/nl80211.c | 15 +++++++-
net/wireless/reg.c | 88 ++++++++++++++++++++++++++++++++++++++++++----
net/wireless/reg.h | 4 ++-
5 files changed, 131 insertions(+), 8 deletions(-)

diff --git a/include/linux/nl80211.h b/include/linux/nl80211.h
index 74cc55c..2235533 100644
--- a/include/linux/nl80211.h
+++ b/include/linux/nl80211.h
@@ -1242,6 +1242,12 @@ enum nl80211_commands {
* @NL80211_ATTR_BG_SCAN_PERIOD: Background scan period in seconds
* or 0 to disable background scan.
*
+ * @NL80211_ATTR_USER_REG_HINT_TYPE: type of regulatory hint passed from
+ * userspace. If unset it is assumed the hint comes directly from
+ * a user. If set code could specify exactly what type of source
+ * was used to provide the hint. For the different types of
+ * allowed user regulatory hints see nl80211_user_reg_hint_type.
+ *
* @NL80211_ATTR_MAX: highest attribute number currently defined
* @__NL80211_ATTR_AFTER_LAST: internal use
*/
@@ -1493,6 +1499,8 @@ enum nl80211_attrs {

NL80211_ATTR_BG_SCAN_PERIOD,

+ NL80211_ATTR_USER_REG_HINT_TYPE,
+
/* add attributes here, update the policy in nl80211.c */

__NL80211_ATTR_AFTER_LAST,
@@ -2045,6 +2053,22 @@ enum nl80211_dfs_regions {
};

/**
+ * enum nl80211_user_reg_hint_type - type of user regulatory hint
+ *
+ * @NL80211_USER_REG_HINT_USER: a user sent the hint. This is always
+ * assumed if the attribute is not set.
+ * @NL80211_USER_REG_HINT_CELL_BASE: the hint comes from a cellular
+ * base station. Device drivers that have been tested to work
+ * properly to support this type of hint can enable these hints
+ * by setting the NL80211_FEATURE_CELL_BASE_REG_HINTS feature
+ * capability on the struct wiphy.
+ */
+enum nl80211_user_reg_hint_type {
+ NL80211_USER_REG_HINT_USER = 0,
+ NL80211_USER_REG_HINT_CELL_BASE = 1,
+};
+
+/**
* enum nl80211_survey_info - survey information
*
* These attribute types are used with %NL80211_ATTR_SURVEY_INFO
@@ -2933,11 +2957,15 @@ enum nl80211_ap_sme_features {
* @NL80211_FEATURE_HT_IBSS: This driver supports IBSS with HT datarates.
* @NL80211_FEATURE_INACTIVITY_TIMER: This driver takes care of freeing up
* the connected inactive stations in AP mode.
+ * @NL80211_FEATURE_CELL_BASE_REG_HINTS: This driver has been tested
+ * to work properly to suppport receiving regulatory hints from
+ * cellular base stations.
*/
enum nl80211_feature_flags {
NL80211_FEATURE_SK_TX_STATUS = 1 << 0,
NL80211_FEATURE_HT_IBSS = 1 << 1,
NL80211_FEATURE_INACTIVITY_TIMER = 1 << 2,
+ NL80211_FEATURE_CELL_BASE_REG_HINTS = 1 << 3,
};

/**
diff --git a/include/net/regulatory.h b/include/net/regulatory.h
index a5f7993..61c394a 100644
--- a/include/net/regulatory.h
+++ b/include/net/regulatory.h
@@ -52,6 +52,9 @@ enum environment_cap {
* DFS master operation on a known DFS region (NL80211_DFS_*),
* dfs_region represents that region. Drivers can use this and the
* @alpha2 to adjust their device's DFS parameters as required.
+ * @user_reg_hint_type: if the @initiator was of type
+ * %NL80211_REGDOM_SET_BY_USER, this clasifies the type
+ * of hint passed. This could be any of the %NL80211_USER_REG_HINT_*
* @intersect: indicates whether the wireless core should intersect
* the requested regulatory domain with the presently set regulatory
* domain.
@@ -70,6 +73,7 @@ enum environment_cap {
struct regulatory_request {
int wiphy_idx;
enum nl80211_reg_initiator initiator;
+ enum nl80211_user_reg_hint_type user_reg_hint_type;
char alpha2[2];
u8 dfs_region;
bool intersect;
diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c
index 77102e6..037cc57a 100644
--- a/net/wireless/nl80211.c
+++ b/net/wireless/nl80211.c
@@ -294,6 +294,7 @@ static const struct nla_policy nl80211_policy[NL80211_ATTR_MAX+1] = {
[NL80211_ATTR_NOACK_MAP] = { .type = NLA_U16 },
[NL80211_ATTR_INACTIVITY_TIMEOUT] = { .type = NLA_U16 },
[NL80211_ATTR_BG_SCAN_PERIOD] = { .type = NLA_U16 },
+ [NL80211_ATTR_USER_REG_HINT_TYPE] = { .type = NLA_U8 },
};

/* policy for the key attributes */
@@ -3480,6 +3481,7 @@ static int nl80211_req_set_reg(struct sk_buff *skb, struct genl_info *info)
{
int r;
char *data = NULL;
+ enum nl80211_user_reg_hint_type user_reg_hint_type;

/*
* You should only get this when cfg80211 hasn't yet initialized
@@ -3499,7 +3501,13 @@ static int nl80211_req_set_reg(struct sk_buff *skb, struct genl_info *info)

data = nla_data(info->attrs[NL80211_ATTR_REG_ALPHA2]);

- r = regulatory_hint_user(data);
+ if (info->attrs[NL80211_ATTR_USER_REG_HINT_TYPE])
+ user_reg_hint_type =
+ nla_get_u8(info->attrs[NL80211_ATTR_USER_REG_HINT_TYPE]);
+ else
+ user_reg_hint_type = NL80211_USER_REG_HINT_USER;
+
+ r = regulatory_hint_user(data, user_reg_hint_type);

return r;
}
@@ -3869,6 +3877,11 @@ static int nl80211_get_reg(struct sk_buff *skb, struct genl_info *info)
cfg80211_regdomain->dfs_region)))
goto nla_put_failure;

+ if (reg_last_request_cell_base() &&
+ nla_put_u8(msg, NL80211_ATTR_USER_REG_HINT_TYPE,
+ NL80211_USER_REG_HINT_CELL_BASE))
+ goto nla_put_failure;
+
nl_reg_rules = nla_nest_start(msg, NL80211_ATTR_REG_RULES);
if (!nl_reg_rules)
goto nla_put_failure;
diff --git a/net/wireless/reg.c b/net/wireless/reg.c
index b2b3222..3644159 100644
--- a/net/wireless/reg.c
+++ b/net/wireless/reg.c
@@ -911,6 +911,56 @@ static void handle_band(struct wiphy *wiphy,
handle_channel(wiphy, initiator, band, i);
}

+static bool reg_request_cell_base(struct regulatory_request *request)
+{
+ if (request->initiator != NL80211_REGDOM_SET_BY_USER)
+ return false;
+ if (request->user_reg_hint_type != NL80211_USER_REG_HINT_CELL_BASE)
+ return false;
+ return true;
+}
+
+bool reg_last_request_cell_base(void)
+{
+ assert_cfg80211_lock();
+
+ mutex_lock(&reg_mutex);
+ return reg_request_cell_base(last_request);
+ mutex_unlock(&reg_mutex);
+}
+
+#ifdef CONFIG_CFG80211_CERTIFICATION_ONUS
+
+/* Core specific check */
+static int reg_ignore_cell_hint(struct regulatory_request *pending_request)
+{
+ if (reg_request_cell_base(last_request)) {
+ if (!regdom_changes(pending_request->alpha2))
+ return -EALREADY;
+ return 0;
+ }
+ return 0;
+}
+
+/* Device specific check */
+static bool reg_dev_ignore_cell_hint(struct wiphy *wiphy)
+{
+ if (!(wiphy->features & NL80211_FEATURE_CELL_BASE_REG_HINTS))
+ return true;
+ return false;
+}
+#else
+static int reg_ignore_cell_hint(struct regulatory_request *pending_request)
+{
+ return -EOPNOTSUPP;
+}
+static int reg_dev_ignore_cell_hint(struct wiphy *wiphy)
+{
+ return true;
+}
+#endif
+
+
static bool ignore_reg_update(struct wiphy *wiphy,
enum nl80211_reg_initiator initiator)
{
@@ -944,6 +994,9 @@ static bool ignore_reg_update(struct wiphy *wiphy,
return true;
}

+ if (reg_request_cell_base(last_request))
+ return reg_dev_ignore_cell_hint(wiphy);
+
return false;
}

@@ -1307,6 +1360,13 @@ static int ignore_request(struct wiphy *wiphy,
return 0;
case NL80211_REGDOM_SET_BY_COUNTRY_IE:

+ if (reg_request_cell_base(last_request)) {
+ /* Trust a Cell base station over the AP's country IE */
+ if (regdom_changes(pending_request->alpha2))
+ return -EOPNOTSUPP;
+ return -EALREADY;
+ }
+
last_wiphy = wiphy_idx_to_wiphy(last_request->wiphy_idx);

if (unlikely(!is_an_alpha2(pending_request->alpha2)))
@@ -1351,6 +1411,12 @@ static int ignore_request(struct wiphy *wiphy,

return REG_INTERSECT;
case NL80211_REGDOM_SET_BY_USER:
+ if (reg_request_cell_base(pending_request))
+ return reg_ignore_cell_hint(pending_request);
+
+ if (reg_request_cell_base(last_request))
+ return -EOPNOTSUPP;
+
if (last_request->initiator == NL80211_REGDOM_SET_BY_COUNTRY_IE)
return REG_INTERSECT;
/*
@@ -1640,7 +1706,8 @@ static int regulatory_hint_core(const char *alpha2)
}

/* User hints */
-int regulatory_hint_user(const char *alpha2)
+int regulatory_hint_user(const char *alpha2,
+ enum nl80211_user_reg_hint_type user_reg_hint_type)
{
struct regulatory_request *request;

@@ -1654,6 +1721,7 @@ int regulatory_hint_user(const char *alpha2)
request->alpha2[0] = alpha2[0];
request->alpha2[1] = alpha2[1];
request->initiator = NL80211_REGDOM_SET_BY_USER;
+ request->user_reg_hint_type = user_reg_hint_type;

queue_regulatory_request(request);

@@ -1906,7 +1974,7 @@ static void restore_regulatory_settings(bool reset_user)
* settings, user regulatory settings takes precedence.
*/
if (is_an_alpha2(alpha2))
- regulatory_hint_user(user_alpha2);
+ regulatory_hint_user(user_alpha2, NL80211_USER_REG_HINT_USER);

if (list_empty(&tmp_reg_req_list))
return;
@@ -2081,9 +2149,16 @@ static void print_regdomain(const struct ieee80211_regdomain *rd)
else {
if (is_unknown_alpha2(rd->alpha2))
pr_info("Regulatory domain changed to driver built-in settings (unknown country)\n");
- else
- pr_info("Regulatory domain changed to country: %c%c\n",
- rd->alpha2[0], rd->alpha2[1]);
+ else {
+ if (reg_request_cell_base(last_request))
+ pr_info("Regulatory domain changed "
+ "to country: %c%c by Cell Station\n",
+ rd->alpha2[0], rd->alpha2[1]);
+ else
+ pr_info("Regulatory domain changed "
+ "to country: %c%c\n",
+ rd->alpha2[0], rd->alpha2[1]);
+ }
}
print_dfs_region(rd->dfs_region);
print_rd_rules(rd);
@@ -2364,7 +2439,8 @@ int __init regulatory_init(void)
* as a user hint.
*/
if (!is_world_regdom(ieee80211_regdom))
- regulatory_hint_user(ieee80211_regdom);
+ regulatory_hint_user(ieee80211_regdom,
+ NL80211_USER_REG_HINT_USER);

return 0;
}
diff --git a/net/wireless/reg.h b/net/wireless/reg.h
index e2aaaf5..ba1097e 100644
--- a/net/wireless/reg.h
+++ b/net/wireless/reg.h
@@ -22,7 +22,8 @@ bool is_world_regdom(const char *alpha2);
bool reg_is_valid_request(const char *alpha2);
bool reg_supported_dfs_region(u8 dfs_region);

-int regulatory_hint_user(const char *alpha2);
+int regulatory_hint_user(const char *alpha2,
+ enum nl80211_user_reg_hint_type user_reg_hint_type);

int reg_device_uevent(struct device *dev, struct kobj_uevent_env *env);
void reg_device_remove(struct wiphy *wiphy);
@@ -33,6 +34,7 @@ void regulatory_exit(void);
int set_regdom(const struct ieee80211_regdomain *rd);

void regulatory_update(struct wiphy *wiphy, enum nl80211_reg_initiator setby);
+bool reg_last_request_cell_base(void);

/**
* regulatory_hint_found_beacon - hints a beacon was found on a channel
--
1.7.10.rc1.22.gf5241


2012-07-06 16:15:54

by Johannes Berg

[permalink] [raw]
Subject: Re: [PATCH 4/4] cfg80211: add cellular base station regulatory hint support

On Fri, 2012-07-06 at 09:07 -0700, Luis R. Rodriguez wrote:

> > What I really wanted to know though is this: If the core code is
> > enabled, what effect does receiving a "blessed" hint have on devices
> > that don't set the flag? If there's no effect vs. an "unblessed" hint,
> > then I think we don't need another option?
>
> Ah, OK so yes, as it is right now if the core accepts the hint but
> devices do not listen to it we do have an effect right now which I
> alluded to in my cover letter: country IE hints won't be listened to
> after a base station hint gets accepted. Whether or not we want to
> process country IE hints *after* a cell base station hint gets
> accepted is debatable -- I decided that its easier to implement to
> ignore country IE hints after a cell base station hint and I also
> decided cell base station hints are likely something you'd prefer to
> review over country IE hints. We can certainly change this and I'm
> open to it.
>
> So yes, as it is right now if you enable onus but your device does not
> support he feature you may detect a regression: country IE hints would
> not be processed if you did receive a base station hint that the core
> processed.

It seems this might be a problem, in the case that I mentioned before:
you do take all the necessary steps to enable this, but then somebody
plugs in a USB device (say this is a tablet-like device?)

It seems it would be better to make the base station/country IE hint
also depend on the feature flag?

johannes


2012-07-06 16:53:32

by Luis R. Rodriguez

[permalink] [raw]
Subject: Re: [PATCH 4/4] cfg80211: add cellular base station regulatory hint support

On Fri, Jul 6, 2012 at 9:31 AM, Luis R. Rodriguez
<[email protected]> wrote:
> upon wiphy registration we could detect if
> a device has the feature enabled and peg core_base_hint_enabled++ or
> whatever and then allow them through. Upon deregistration we'd
> core_base_hint_enabled-- and only enable the feature if its > 0.
>
> Seems doable.

Yeah.. I like this -- it means we wouldn't have to add a kconfig for
the feature, instead each driver could define a kconfig of its own for
it. I'll respin, thanks!

Luis

2012-07-06 14:06:06

by Luis R. Rodriguez

[permalink] [raw]
Subject: Re: [PATCH 4/4] cfg80211: add cellular base station regulatory hint support

On Thu, Jul 5, 2012 at 11:39 PM, Johannes Berg
<[email protected]> wrote:
> Oh, that explains the extra flag there, I pretty much thought it was
> useless. I guess that could be useful for when you ship some device with
> builtin wireless but allow plugging in other wireless?

That, but also it means there are two hops for a device to use the
cell base station hint: onus option, and then the feature flag, which
could be wrapped itself around a driver specific kconfig option.
Reason for the feature flag is devices may need a firmware update in
order for them to work properly.

As for the onus and the cell base station hint though, it is true
though that the onus option would alone enable the core cell base
station hints, so the onus option itself would be flipping a switch
for a feature. I could extend the patch to include a core specific
base station hint kconfig option, because as you say there is no
specific switch to select the feature right now for the core.

That would mean at most 3 options that would need to be enabled for
the feature for a specific device. Thoughts ?

Luis

2012-07-06 16:31:20

by Luis R. Rodriguez

[permalink] [raw]
Subject: Re: [PATCH 4/4] cfg80211: add cellular base station regulatory hint support

On Fri, Jul 6, 2012 at 9:15 AM, Johannes Berg <[email protected]> wrote:
> On Fri, 2012-07-06 at 09:07 -0700, Luis R. Rodriguez wrote:
>
>> > What I really wanted to know though is this: If the core code is
>> > enabled, what effect does receiving a "blessed" hint have on devices
>> > that don't set the flag? If there's no effect vs. an "unblessed" hint,
>> > then I think we don't need another option?
>>
>> Ah, OK so yes, as it is right now if the core accepts the hint but
>> devices do not listen to it we do have an effect right now which I
>> alluded to in my cover letter: country IE hints won't be listened to
>> after a base station hint gets accepted. Whether or not we want to
>> process country IE hints *after* a cell base station hint gets
>> accepted is debatable -- I decided that its easier to implement to
>> ignore country IE hints after a cell base station hint and I also
>> decided cell base station hints are likely something you'd prefer to
>> review over country IE hints. We can certainly change this and I'm
>> open to it.
>>
>> So yes, as it is right now if you enable onus but your device does not
>> support he feature you may detect a regression: country IE hints would
>> not be processed if you did receive a base station hint that the core
>> processed.
>
> It seems this might be a problem, in the case that I mentioned before:
> you do take all the necessary steps to enable this, but then somebody
> plugs in a USB device (say this is a tablet-like device?)

Agreed however see below.

> It seems it would be better to make the base station/country IE hint
> also depend on the feature flag?

Agreed, at first I figured this'd be complex to figure out dynamically
but it does not have to be: upon wiphy registration we could detect if
a device has the feature enabled and peg core_base_hint_enabled++ or
whatever and then allow them through. Upon deregistration we'd
core_base_hint_enabled-- and only enable the feature if its > 0.

Seems doable.

Luis

2012-07-04 09:42:28

by Johannes Berg

[permalink] [raw]
Subject: Re: [PATCH 4/4] cfg80211: add cellular base station regulatory hint support

Hi Luis,

Please split this series -- I won't apply the Atheros driver patches :-)


> + * @NL80211_ATTR_USER_REG_HINT_TYPE: type of regulatory hint passed from
> + * userspace. If unset it is assumed the hint comes directly from
> + * a user. If set code could specify exactly what type of source
> + * was used to provide the hint. For the different types of
> + * allowed user regulatory hints see nl80211_user_reg_hint_type.

Please mention what the data type is, but see below


> + * @NL80211_FEATURE_CELL_BASE_REG_HINTS: This driver has been tested
> + * to work properly to suppport receiving regulatory hints from
> + * cellular base stations.

Typo "support", but I don't understand the reasoning behind this flag?
First you hide away behind the certification Kconfig thing ...? Maybe
you just need to explain better exactly what the cell-base-station
regulatory hint does.


> + [NL80211_ATTR_USER_REG_HINT_TYPE] = { .type = NLA_U8 },

Not much point in using a u8, I think you should just use a u32.

> - r = regulatory_hint_user(data);
> + if (info->attrs[NL80211_ATTR_USER_REG_HINT_TYPE])
> + user_reg_hint_type =
> + nla_get_u8(info->attrs[NL80211_ATTR_USER_REG_HINT_TYPE]);

Need to verify that it's a valid number, I think, otherwise you can pass
random junk like 0x42 into the regulatory framework.

> @@ -3869,6 +3877,11 @@ static int nl80211_get_reg(struct sk_buff *skb, struct genl_info *info)
> cfg80211_regdomain->dfs_region)))
> goto nla_put_failure;
>
> + if (reg_last_request_cell_base() &&

Is that really worthwhile as a function call like this rather than some
sort of _get_hint_type()?

> +#ifdef CONFIG_CFG80211_CERTIFICATION_ONUS

So I'm not really convinced about this. It seems this Kconfig should
better be a Kconfig that enables other Kconfig only, not enabling other
features. How else would anyone be able to do due diligence and check
what exactly this enables that they need to test?

johannes


2012-07-06 14:12:04

by Johannes Berg

[permalink] [raw]
Subject: Re: [PATCH 4/4] cfg80211: add cellular base station regulatory hint support

On Fri, 2012-07-06 at 07:05 -0700, Luis R. Rodriguez wrote:

> That, but also it means there are two hops for a device to use the
> cell base station hint: onus option, and then the feature flag, which
> could be wrapped itself around a driver specific kconfig option.
> Reason for the feature flag is devices may need a firmware update in
> order for them to work properly.
>
> As for the onus and the cell base station hint though, it is true
> though that the onus option would alone enable the core cell base
> station hints, so the onus option itself would be flipping a switch
> for a feature. I could extend the patch to include a core specific
> base station hint kconfig option, because as you say there is no
> specific switch to select the feature right now for the core.

I guess the question is -- what does that code really enable, if the
driver doesn't also set the feature flag? If it doesn't mean anything
without the flag I think we don't need the extra core option?

johannes


2012-07-06 06:39:52

by Johannes Berg

[permalink] [raw]
Subject: Re: [PATCH 4/4] cfg80211: add cellular base station regulatory hint support

On Thu, 2012-07-05 at 09:48 -0700, Luis R. Rodriguez wrote:
> On Thu, Jul 5, 2012 at 12:45 AM, Johannes Berg
> <[email protected]> wrote:
> > On Wed, 2012-07-04 at 09:26 -0700, Luis R. Rodriguez wrote:
> >> I've skipped the other comments as I can address those in a new series.
> >>
> >> On Wed, Jul 4, 2012 at 2:42 AM, Johannes Berg <[email protected]> wrote:
> >> > So I'm not really convinced about this. It seems this Kconfig should
> >> > better be a Kconfig that enables other Kconfig only, not enabling other
> >> > features. How else would anyone be able to do due diligence and check
> >> > what exactly this enables that they need to test?
> >>
> >> Makes sense, so would we then have CONFIG_REG_HINT_CELL_BASE_STATION ?
> >
> > I don't know how fine-grained it should be? Maybe it should be more
> > generic and be a config for all (future) kinds of user hints?
>
> Well so in this case the cell base station hint support gets used and
> trusted on the wireless core if CONFIG_REG_HINT_CELL_BASE_STATION is
> set. Whether or not a *driver* trusts and uses it as well will depend
> on whether or not they set the NL80211_FEATURE_CELL_BASE_REG_HINTS
> feature on their wiphy->features. The way I was thinking about drivers
> going about enabling / disabling was to let the driver have its own
> kconfig option for this. The reason is that firmware may require some
> implementation / changes / testing to ensure that a device won't poop
> out if this is used.

Oh, that explains the extra flag there, I pretty much thought it was
useless. I guess that could be useful for when you ship some device with
builtin wireless but allow plugging in other wireless?

johannes


2012-07-05 16:48:38

by Luis R. Rodriguez

[permalink] [raw]
Subject: Re: [PATCH 4/4] cfg80211: add cellular base station regulatory hint support

On Thu, Jul 5, 2012 at 12:45 AM, Johannes Berg
<[email protected]> wrote:
> On Wed, 2012-07-04 at 09:26 -0700, Luis R. Rodriguez wrote:
>> I've skipped the other comments as I can address those in a new series.
>>
>> On Wed, Jul 4, 2012 at 2:42 AM, Johannes Berg <[email protected]> wrote:
>> > So I'm not really convinced about this. It seems this Kconfig should
>> > better be a Kconfig that enables other Kconfig only, not enabling other
>> > features. How else would anyone be able to do due diligence and check
>> > what exactly this enables that they need to test?
>>
>> Makes sense, so would we then have CONFIG_REG_HINT_CELL_BASE_STATION ?
>
> I don't know how fine-grained it should be? Maybe it should be more
> generic and be a config for all (future) kinds of user hints?

Well so in this case the cell base station hint support gets used and
trusted on the wireless core if CONFIG_REG_HINT_CELL_BASE_STATION is
set. Whether or not a *driver* trusts and uses it as well will depend
on whether or not they set the NL80211_FEATURE_CELL_BASE_REG_HINTS
feature on their wiphy->features. The way I was thinking about drivers
going about enabling / disabling was to let the driver have its own
kconfig option for this. The reason is that firmware may require some
implementation / changes / testing to ensure that a device won't poop
out if this is used.

Luis

2012-07-04 16:26:39

by Luis R. Rodriguez

[permalink] [raw]
Subject: Re: [PATCH 4/4] cfg80211: add cellular base station regulatory hint support

I've skipped the other comments as I can address those in a new series.

On Wed, Jul 4, 2012 at 2:42 AM, Johannes Berg <[email protected]> wrote:
> So I'm not really convinced about this. It seems this Kconfig should
> better be a Kconfig that enables other Kconfig only, not enabling other
> features. How else would anyone be able to do due diligence and check
> what exactly this enables that they need to test?

Makes sense, so would we then have CONFIG_REG_HINT_CELL_BASE_STATION ?

Luis

2012-07-05 07:45:09

by Johannes Berg

[permalink] [raw]
Subject: Re: [PATCH 4/4] cfg80211: add cellular base station regulatory hint support

On Wed, 2012-07-04 at 09:26 -0700, Luis R. Rodriguez wrote:
> I've skipped the other comments as I can address those in a new series.
>
> On Wed, Jul 4, 2012 at 2:42 AM, Johannes Berg <[email protected]> wrote:
> > So I'm not really convinced about this. It seems this Kconfig should
> > better be a Kconfig that enables other Kconfig only, not enabling other
> > features. How else would anyone be able to do due diligence and check
> > what exactly this enables that they need to test?
>
> Makes sense, so would we then have CONFIG_REG_HINT_CELL_BASE_STATION ?

I don't know how fine-grained it should be? Maybe it should be more
generic and be a config for all (future) kinds of user hints?

johannes


2012-07-06 16:07:32

by Luis R. Rodriguez

[permalink] [raw]
Subject: Re: [PATCH 4/4] cfg80211: add cellular base station regulatory hint support

On Fri, Jul 6, 2012 at 8:51 AM, Johannes Berg <[email protected]> wrote:
> On Fri, 2012-07-06 at 08:47 -0700, Luis R. Rodriguez wrote:
>
>> > I guess the question is -- what does that code really enable, if the
>> > driver doesn't also set the feature flag? If it doesn't mean anything
>> > without the flag I think we don't need the extra core option?
>>
>> Ah -- good question. It'd just be the core. The core can support the
>> base station hint or not but apart from the core we could technically
>> also have a scenario today where perhaps one 802.11 card does support
>> the station hint another does not. It is true that with no 802.11 card
>> present having the core makes no sense but in practice we would, or at
>> the very least we'd have the mac80211_hwsim.
>>
>> So as I designed this I realized we really did have to consider the
>> core getting this Vs a card getting this hint. The core getting the
>> hint would only happen as-is with the onus option enabled, so if we
>> want the onus option only to be a gate to other options then
>> technically the feature would be for the core, while drivers would
>> have the options to enable / disable the same feature given that this
>> requires testing / maybe some changes in the firmware.
>
> I'm not sure I really understand what you're saying :-)
>
> What I really wanted to know though is this: If the core code is
> enabled, what effect does receiving a "blessed" hint have on devices
> that don't set the flag? If there's no effect vs. an "unblessed" hint,
> then I think we don't need another option?

Ah, OK so yes, as it is right now if the core accepts the hint but
devices do not listen to it we do have an effect right now which I
alluded to in my cover letter: country IE hints won't be listened to
after a base station hint gets accepted. Whether or not we want to
process country IE hints *after* a cell base station hint gets
accepted is debatable -- I decided that its easier to implement to
ignore country IE hints after a cell base station hint and I also
decided cell base station hints are likely something you'd prefer to
review over country IE hints. We can certainly change this and I'm
open to it.

So yes, as it is right now if you enable onus but your device does not
support he feature you may detect a regression: country IE hints would
not be processed if you did receive a base station hint that the core
processed.

Luis

2012-07-03 23:15:34

by Luis R. Rodriguez

[permalink] [raw]
Subject: [PATCH 2/4] ath5k: replace modparam_all_channels with CONFIG_ATH5K_TEST_CHANNELS

From: "Luis R. Rodriguez" <[email protected]>

This stashes away this feature from standard kernel builds.

Signed-off-by: Luis R. Rodriguez <[email protected]>
---
drivers/net/wireless/ath/ath5k/Kconfig | 8 ++++++++
drivers/net/wireless/ath/ath5k/base.c | 17 ++++++++++-------
2 files changed, 18 insertions(+), 7 deletions(-)

diff --git a/drivers/net/wireless/ath/ath5k/Kconfig b/drivers/net/wireless/ath/ath5k/Kconfig
index e18a9aa..338c5c4 100644
--- a/drivers/net/wireless/ath/ath5k/Kconfig
+++ b/drivers/net/wireless/ath/ath5k/Kconfig
@@ -64,3 +64,11 @@ config ATH5K_PCI
---help---
This adds support for PCI type chipsets of the 5xxx Atheros
family.
+
+config ATH5K_TEST_CHANNELS
+ bool "Enables testing channels on ath5k"
+ depends on ATH5K && CFG80211_CERTIFICATION_ONUS
+ ---help---
+ This enables non-standard IEEE 802.11 channels on ath5k, which
+ can be used for research purposes. This option should be disabled
+ unless doing research.
diff --git a/drivers/net/wireless/ath/ath5k/base.c b/drivers/net/wireless/ath/ath5k/base.c
index 44ad6fe..8c4c040 100644
--- a/drivers/net/wireless/ath/ath5k/base.c
+++ b/drivers/net/wireless/ath/ath5k/base.c
@@ -74,10 +74,6 @@ bool ath5k_modparam_nohwcrypt;
module_param_named(nohwcrypt, ath5k_modparam_nohwcrypt, bool, S_IRUGO);
MODULE_PARM_DESC(nohwcrypt, "Disable hardware encryption.");

-static bool modparam_all_channels;
-module_param_named(all_channels, modparam_all_channels, bool, S_IRUGO);
-MODULE_PARM_DESC(all_channels, "Expose all channels the device can use.");
-
static bool modparam_fastchanswitch;
module_param_named(fastchanswitch, modparam_fastchanswitch, bool, S_IRUGO);
MODULE_PARM_DESC(fastchanswitch, "Enable fast channel switching for AR2413/AR5413 radios.");
@@ -258,8 +254,15 @@ static int ath5k_reg_notifier(struct wiphy *wiphy, struct regulatory_request *re
\********************/

/*
- * Returns true for the channel numbers used without all_channels modparam.
+ * Returns true for the channel numbers used.
*/
+#ifdef CONFIG_ATH5K_TEST_CHANNELS
+static bool ath5k_is_standard_channel(short chan, enum ieee80211_band band)
+{
+ return true;
+}
+
+#else
static bool ath5k_is_standard_channel(short chan, enum ieee80211_band band)
{
if (band == IEEE80211_BAND_2GHZ && chan <= 14)
@@ -276,6 +279,7 @@ static bool ath5k_is_standard_channel(short chan, enum ieee80211_band band)
/* 802.11j 4.9GHz (20MHz) */
(chan == 184 || chan == 188 || chan == 192 || chan == 196));
}
+#endif

static unsigned int
ath5k_setup_channels(struct ath5k_hw *ah, struct ieee80211_channel *channels,
@@ -316,8 +320,7 @@ ath5k_setup_channels(struct ath5k_hw *ah, struct ieee80211_channel *channels,
if (!ath5k_channel_ok(ah, &channels[count]))
continue;

- if (!modparam_all_channels &&
- !ath5k_is_standard_channel(ch, band))
+ if (!ath5k_is_standard_channel(ch, band))
continue;

count++;
--
1.7.10.rc1.22.gf5241


2012-07-03 23:15:40

by Luis R. Rodriguez

[permalink] [raw]
Subject: [PATCH 3/4] ath9k: make CONFIG_ATH9K_DFS_CERTIFIED depend on CFG80211_CERTIFICATION_ONUS

From: "Luis R. Rodriguez" <[email protected]>

Turns out every most standard Linux distributions enable
CONFIG_EXPERT, so use the shiny new CFG80211_CERTIFICATION_ONUS
which is meant by design to not be enabled by all Linux
distributions.

Signed-off-by: Luis R. Rodriguez <[email protected]>
---
drivers/net/wireless/ath/ath9k/Kconfig | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/wireless/ath/ath9k/Kconfig b/drivers/net/wireless/ath/ath9k/Kconfig
index e507e78..c7aa664 100644
--- a/drivers/net/wireless/ath/ath9k/Kconfig
+++ b/drivers/net/wireless/ath/ath9k/Kconfig
@@ -64,7 +64,7 @@ config ATH9K_DEBUGFS

config ATH9K_DFS_CERTIFIED
bool "Atheros DFS support for certified platforms"
- depends on ATH9K && EXPERT
+ depends on ATH9K && CFG80211_CERTIFICATION_ONUS
default n
---help---
This option enables DFS support for initiating radiation on
--
1.7.10.rc1.22.gf5241


2012-07-06 15:51:24

by Johannes Berg

[permalink] [raw]
Subject: Re: [PATCH 4/4] cfg80211: add cellular base station regulatory hint support

On Fri, 2012-07-06 at 08:47 -0700, Luis R. Rodriguez wrote:

> > I guess the question is -- what does that code really enable, if the
> > driver doesn't also set the feature flag? If it doesn't mean anything
> > without the flag I think we don't need the extra core option?
>
> Ah -- good question. It'd just be the core. The core can support the
> base station hint or not but apart from the core we could technically
> also have a scenario today where perhaps one 802.11 card does support
> the station hint another does not. It is true that with no 802.11 card
> present having the core makes no sense but in practice we would, or at
> the very least we'd have the mac80211_hwsim.
>
> So as I designed this I realized we really did have to consider the
> core getting this Vs a card getting this hint. The core getting the
> hint would only happen as-is with the onus option enabled, so if we
> want the onus option only to be a gate to other options then
> technically the feature would be for the core, while drivers would
> have the options to enable / disable the same feature given that this
> requires testing / maybe some changes in the firmware.

I'm not sure I really understand what you're saying :-)

What I really wanted to know though is this: If the core code is
enabled, what effect does receiving a "blessed" hint have on devices
that don't set the flag? If there's no effect vs. an "unblessed" hint,
then I think we don't need another option?

johannes


2012-07-03 23:15:27

by Luis R. Rodriguez

[permalink] [raw]
Subject: [PATCH 1/4] cfg80211: add CONFIG_CFG80211_CERTIFICATION_ONUS

From: "Luis R. Rodriguez" <[email protected]>

This adds CONFIG_CFG80211_CERTIFICATION_ONUS which is to
be used for features / code which require a bit of work on
the system integrator's part to ensure that the system will
still pass 802.11 regulatory certification. This option is
also usable for researchers and experimenters looking to add
code in the kernel without impacting compliant code.

We'd use CONFIG_EXPERT alone but it seems that most standard
Linux distributions are enabling CONFIG_EXPERT already. This
allows us to define 802.11 specific kernel features under a
flag that is intended by design to be disabled by standard
Linux distributions, and only enabled by system integrators
or distributions that have done work to ensure regulatory
certification on the system with the enabled features.

Signed-off-by: Luis R. Rodriguez <[email protected]>
---
net/wireless/Kconfig | 21 +++++++++++++++++++++
1 file changed, 21 insertions(+)

diff --git a/net/wireless/Kconfig b/net/wireless/Kconfig
index 4d2b1ec..fe4adb1 100644
--- a/net/wireless/Kconfig
+++ b/net/wireless/Kconfig
@@ -74,6 +74,27 @@ config CFG80211_REG_DEBUG

If unsure, say N.

+config CFG80211_CERTIFICATION_ONUS
+ bool "cfg80211 certification onus"
+ depends on CFG80211 && EXPERT
+ default n
+ ---help---
+ You should disable this option unless you are both capable
+ and willing to ensure your system will remain regulatory
+ compliant with the features available under this option.
+ Some options may still be under heavy development and
+ for whatever reason regulatory compliance has not or
+ cannot yet be verified. Regulatory verification may at
+ times only be possible until you have the final system
+ in place.
+
+ This option should only be enabled by system integrators
+ or distributions that have done work necessary to ensure
+ regulatory certification on the system with the enabled
+ features. Alternatively you can enable this option if
+ you are a wireless researcher and are working in a controlled
+ and approved environment by your local regulatory agency.
+
config CFG80211_DEFAULT_PS
bool "enable powersave by default"
depends on CFG80211
--
1.7.10.rc1.22.gf5241


2012-07-06 15:48:07

by Luis R. Rodriguez

[permalink] [raw]
Subject: Re: [PATCH 4/4] cfg80211: add cellular base station regulatory hint support

On Fri, Jul 6, 2012 at 7:11 AM, Johannes Berg <[email protected]> wrote:
> On Fri, 2012-07-06 at 07:05 -0700, Luis R. Rodriguez wrote:
>
>> That, but also it means there are two hops for a device to use the
>> cell base station hint: onus option, and then the feature flag, which
>> could be wrapped itself around a driver specific kconfig option.
>> Reason for the feature flag is devices may need a firmware update in
>> order for them to work properly.
>>
>> As for the onus and the cell base station hint though, it is true
>> though that the onus option would alone enable the core cell base
>> station hints, so the onus option itself would be flipping a switch
>> for a feature. I could extend the patch to include a core specific
>> base station hint kconfig option, because as you say there is no
>> specific switch to select the feature right now for the core.
>
> I guess the question is -- what does that code really enable, if the
> driver doesn't also set the feature flag? If it doesn't mean anything
> without the flag I think we don't need the extra core option?

Ah -- good question. It'd just be the core. The core can support the
base station hint or not but apart from the core we could technically
also have a scenario today where perhaps one 802.11 card does support
the station hint another does not. It is true that with no 802.11 card
present having the core makes no sense but in practice we would, or at
the very least we'd have the mac80211_hwsim.

So as I designed this I realized we really did have to consider the
core getting this Vs a card getting this hint. The core getting the
hint would only happen as-is with the onus option enabled, so if we
want the onus option only to be a gate to other options then
technically the feature would be for the core, while drivers would
have the options to enable / disable the same feature given that this
requires testing / maybe some changes in the firmware.

Luis

2012-07-06 22:09:15

by Luis R. Rodriguez

[permalink] [raw]
Subject: Re: [PATCH 4/4] cfg80211: add cellular base station regulatory hint support

FWIW, here's the delta I plan on adding that adds a device
count for number of devices that support the base station
hints and disregards the hints unless the core has 1 device
that supports this. I'll roll this into a new series and
later send ath5k / ath9k patches separately as I see now
that cfg80211 changes go through you and driver updates
through linville.

diff --git a/net/wireless/core.c b/net/wireless/core.c
index e13365f..153f7dc 100644
--- a/net/wireless/core.c
+++ b/net/wireless/core.c
@@ -537,6 +537,7 @@ int wiphy_register(struct wiphy *wiphy)
}

/* set up regulatory info */
+ wiphy_regulatory_register(wiphy);
regulatory_update(wiphy, NL80211_REGDOM_SET_BY_CORE);

list_add_rcu(&rdev->list, &cfg80211_rdev_list);
diff --git a/net/wireless/reg.c b/net/wireless/reg.c
index 3644159..05af62d 100644
--- a/net/wireless/reg.c
+++ b/net/wireless/reg.c
@@ -97,9 +97,16 @@ const struct ieee80211_regdomain *cfg80211_regdomain;
* - cfg80211_world_regdom
* - cfg80211_regdom
* - last_request
+ * - reg_num_devs_support_basehint
*/
static DEFINE_MUTEX(reg_mutex);

+/*
+ * Number of devices that registered to the core
+ * that support cellular base station regulatory hints
+ */
+static int reg_num_devs_support_basehint;
+
static inline void assert_reg_lock(void)
{
lockdep_assert_held(&reg_mutex);
@@ -934,6 +941,9 @@ bool reg_last_request_cell_base(void)
/* Core specific check */
static int reg_ignore_cell_hint(struct regulatory_request *pending_request)
{
+ if (!reg_num_devs_support_basehint)
+ return -EOPNOTSUPP;
+
if (reg_request_cell_base(last_request)) {
if (!regdom_changes(pending_request->alpha2))
return -EALREADY;
@@ -2365,6 +2375,18 @@ int reg_device_uevent(struct device *dev, struct kobj_uevent_env *env)
}
#endif /* CONFIG_HOTPLUG */

+void wiphy_regulatory_register(struct wiphy *wiphy)
+{
+ assert_cfg80211_lock();
+
+ mutex_lock(&reg_mutex);
+
+ if (!reg_dev_ignore_cell_hint(wiphy))
+ reg_num_devs_support_basehint++;
+
+ mutex_unlock(&reg_mutex);
+}
+
/* Caller must hold cfg80211_mutex */
void reg_device_remove(struct wiphy *wiphy)
{
@@ -2374,6 +2396,9 @@ void reg_device_remove(struct wiphy *wiphy)

mutex_lock(&reg_mutex);

+ if (!reg_dev_ignore_cell_hint(wiphy))
+ reg_num_devs_support_basehint--;
+
kfree(wiphy->regd);

if (last_request)
diff --git a/net/wireless/reg.h b/net/wireless/reg.h
index ba1097e..519492f 100644
--- a/net/wireless/reg.h
+++ b/net/wireless/reg.h
@@ -26,6 +26,7 @@ int regulatory_hint_user(const char *alpha2,
enum nl80211_user_reg_hint_type user_reg_hint_type);

int reg_device_uevent(struct device *dev, struct kobj_uevent_env *env);
+void wiphy_regulatory_register(struct wiphy *wiphy);
void reg_device_remove(struct wiphy *wiphy);

int __init regulatory_init(void);