2008-10-21 10:31:57

by Johannes Berg

[permalink] [raw]
Subject: [PATCH] wireless: add regulatory_struct_hint

This adds a new function, regulatory_struct_hint, which acts
as a hint to the wireless core which regdomain a card thinks
the system is operating in, but given in terms of the actual
regdomain definition. Multiple hints are permitted when the
specified bands do not overlap.

Signed-off-by: Johannes Berg <[email protected]>
---
Entirely untested. Anyone want to give it a go in the dual-band scenario?

include/net/wireless.h | 14 +++
net/wireless/reg.c | 225 ++++++++++++++++++++++++++++++++-----------------
2 files changed, 161 insertions(+), 78 deletions(-)

--- everything.orig/include/net/wireless.h 2008-10-21 11:50:01.000000000 +0200
+++ everything/include/net/wireless.h 2008-10-21 11:50:16.000000000 +0200
@@ -355,4 +355,18 @@ ieee80211_get_channel(struct wiphy *wiph
* for a regulatory domain structure for the respective country.
*/
extern void regulatory_hint(const char *alpha2);
+
+/**
+ * regulatory_struct_hint - hint wireless core about regdomain
+ *
+ * @rd: regdomain structure containing the frequency ranges that are
+ * permitted for use.
+ * @bands: bitmask of bands this contains, use BIT(IEEE80211_BAND_...)
+ *
+ * This function informs the wireless core that the driver believes
+ * that the bands indicated are defined by the given structure in the
+ * regulatory domain the system is operating in.
+ */
+extern void regulatory_struct_hint(struct ieee80211_regdomain *rd,
+ u32 bands);
#endif /* __NET_WIRELESS_H */
--- everything.orig/net/wireless/reg.c 2008-10-21 11:50:14.000000000 +0200
+++ everything/net/wireless/reg.c 2008-10-21 12:26:08.000000000 +0200
@@ -45,9 +45,9 @@
/* wiphy is set if this request's initiator is REGDOM_SET_BY_COUNTRY_IE */
struct regulatory_request {
struct wiphy *wiphy;
- int granted;
enum reg_set_by initiator;
char alpha2[2];
+ u32 bands;
};

static struct regulatory_request *last_request;
@@ -296,82 +296,6 @@ static int call_crda(const char *alpha2)
return kobject_uevent_env(&reg_pdev->dev.kobj, KOBJ_CHANGE, envp);
}

-/* This has the logic which determines when a new request
- * should be ignored. */
-static int ignore_request(struct wiphy *wiphy, enum reg_set_by set_by,
- const char *alpha2)
-{
- /* All initial requests are respected */
- if (!last_request)
- return 0;
-
- switch (set_by) {
- case REGDOM_SET_BY_INIT:
- return -EINVAL;
- case REGDOM_SET_BY_CORE:
- /* Always respect new wireless core hints, should only
- * come in for updating the world regulatory domain at init
- * anyway */
- return 0;
- case REGDOM_SET_BY_COUNTRY_IE:
- if (last_request->initiator == REGDOM_SET_BY_COUNTRY_IE) {
- if (last_request->wiphy != wiphy) {
- /* Two cards with two APs claiming different
- * different Country IE alpha2s!
- * You're special!! */
- if (!alpha2_equal(last_request->alpha2,
- cfg80211_regdomain->alpha2)) {
- /* XXX: Deal with conflict, consider
- * building a new one out of the
- * intersection */
- WARN_ON(1);
- return -EOPNOTSUPP;
- }
- return -EALREADY;
- }
- /* Two consecutive Country IE hints on the same wiphy */
- if (!alpha2_equal(cfg80211_regdomain->alpha2, alpha2))
- return 0;
- return -EALREADY;
- }
- if (WARN_ON(!is_alpha2_set(alpha2) || !is_an_alpha2(alpha2)),
- "Invalid Country IE regulatory hint passed "
- "to the wireless core\n")
- return -EINVAL;
- /* We ignore Country IE hints for now, as we haven't yet
- * added the dot11MultiDomainCapabilityEnabled flag
- * for wiphys */
- return 1;
- case REGDOM_SET_BY_DRIVER:
- BUG_ON(!wiphy);
- if (last_request->initiator == REGDOM_SET_BY_DRIVER)
- return -EALREADY;
- if (last_request->initiator == REGDOM_SET_BY_CORE)
- return 0;
- /* XXX: Handle intersection, and add the
- * dot11MultiDomainCapabilityEnabled flag to wiphy. For now
- * we assume the driver has this set to false, following the
- * 802.11d dot11MultiDomainCapabilityEnabled documentation */
- if (last_request->initiator == REGDOM_SET_BY_COUNTRY_IE)
- return 0;
- return 0;
- case REGDOM_SET_BY_USER:
- if (last_request->initiator == REGDOM_SET_BY_USER ||
- last_request->initiator == REGDOM_SET_BY_CORE)
- return 0;
- /* Drivers can use their wiphy's reg_notifier()
- * to override any information */
- if (last_request->initiator == REGDOM_SET_BY_DRIVER)
- return 0;
- /* XXX: Handle intersection */
- if (last_request->initiator == REGDOM_SET_BY_COUNTRY_IE)
- return -EOPNOTSUPP;
- return 0;
- default:
- return -EINVAL;
- }
-}
-
/* Used by nl80211 before kmalloc'ing our regulatory domain */
bool reg_is_valid_request(const char *alpha2)
{
@@ -542,6 +466,83 @@ void wiphy_update_regulatory(struct wiph
}
}

+/* This has the logic which determines when a new request
+ * should be ignored. */
+static int ignore_request(struct wiphy *wiphy, enum reg_set_by set_by,
+ const char *alpha2)
+{
+ /* All initial requests are respected */
+ if (!last_request)
+ return 0;
+
+ switch (set_by) {
+ case REGDOM_SET_BY_INIT:
+ return -EINVAL;
+ case REGDOM_SET_BY_CORE:
+ /* Always respect new wireless core hints, should only
+ * come in for updating the world regulatory domain at init
+ * anyway */
+ return 0;
+ case REGDOM_SET_BY_COUNTRY_IE:
+ if (last_request->initiator == REGDOM_SET_BY_COUNTRY_IE) {
+ if (last_request->wiphy != wiphy) {
+ /* Two cards with two APs claiming different
+ * different Country IE alpha2s!
+ * You're special!! */
+ if (!alpha2_equal(last_request->alpha2,
+ cfg80211_regdomain->alpha2)) {
+ /* XXX: Deal with conflict, consider
+ * building a new one out of the
+ * intersection */
+ WARN_ON(1);
+ return -EOPNOTSUPP;
+ }
+ return -EALREADY;
+ }
+ /* Two consecutive Country IE hints on the same wiphy */
+ if (!alpha2_equal(cfg80211_regdomain->alpha2, alpha2))
+ return 0;
+ return -EALREADY;
+ }
+ if (WARN_ON(!is_alpha2_set(alpha2) || !is_an_alpha2(alpha2)),
+ "Invalid Country IE regulatory hint passed "
+ "to the wireless core\n")
+ return -EINVAL;
+ /* We ignore Country IE hints for now, as we haven't yet
+ * added the dot11MultiDomainCapabilityEnabled flag
+ * for wiphys */
+ return 1;
+ case REGDOM_SET_BY_DRIVER:
+ BUG_ON(!wiphy);
+ switch (last_request->initiator) {
+ case REGDOM_SET_BY_DRIVER:
+ return -EALREADY;
+ case REGDOM_SET_BY_INIT:
+ case REGDOM_SET_BY_CORE:
+ case REGDOM_SET_BY_USER:
+ case REGDOM_SET_BY_COUNTRY_IE:
+ return 0;
+ default:
+ break;
+ }
+ return 0;
+ case REGDOM_SET_BY_USER:
+ if (last_request->initiator == REGDOM_SET_BY_USER ||
+ last_request->initiator == REGDOM_SET_BY_CORE)
+ return 0;
+ /* Drivers can use their wiphy's reg_notifier()
+ * to override any information */
+ if (last_request->initiator == REGDOM_SET_BY_DRIVER)
+ return 0;
+ /* XXX: Handle intersection */
+ if (last_request->initiator == REGDOM_SET_BY_COUNTRY_IE)
+ return -EOPNOTSUPP;
+ return 0;
+ default:
+ return -EINVAL;
+ }
+}
+
/* Caller must hold &cfg80211_drv_mutex */
int __regulatory_hint(struct wiphy *wiphy, enum reg_set_by set_by,
const char *alpha2)
@@ -567,6 +568,7 @@ int __regulatory_hint(struct wiphy *wiph
request->alpha2[1] = alpha2[1];
request->initiator = set_by;
request->wiphy = wiphy;
+ request->bands = ~0;

kfree(last_request);
last_request = request;
@@ -594,6 +596,74 @@ void regulatory_hint(const char *alpha2)
}
EXPORT_SYMBOL(regulatory_hint);

+void regulatory_struct_hint(struct ieee80211_regdomain *rd, u32 bands)
+{
+ const struct ieee80211_regdomain *orig = NULL;
+ struct ieee80211_regdomain *new = NULL;
+ int origrules;
+
+ BUG_ON(!rd);
+ BUG_ON(!bands);
+
+ mutex_lock(&cfg80211_drv_mutex);
+
+ /*
+ * ignore hint if anything else set it or if the given
+ * bands overlap already defined bands
+ */
+ if (last_request) {
+ switch (last_request->initiator) {
+ case REGDOM_SET_BY_DRIVER:
+ if (last_request->bands & bands)
+ goto out;
+ break;
+ case REGDOM_SET_BY_CORE:
+ break;
+ default:
+ goto out;
+ }
+
+ /* modify the currently set regdom */
+ orig = cfg80211_regdomain;
+ origrules = orig->n_reg_rules;
+ } else {
+ last_request = kzalloc(sizeof(struct regulatory_request),
+ GFP_KERNEL);
+ if (!last_request)
+ goto out;
+
+ last_request->alpha2[0] = '9';
+ last_request->alpha2[1] = '9';
+ last_request->initiator = REGDOM_SET_BY_DRIVER;
+
+ origrules = 0;
+ }
+
+ last_request->bands |= bands;
+
+ new = krealloc(orig,
+ sizeof(struct ieee80211_regdomain) +
+ sizeof(struct ieee80211_reg_rule) * origrules +
+ sizeof(struct ieee80211_reg_rule) * rd->n_reg_rules,
+ GFP_KERNEL);
+ if (!new)
+ goto out;
+
+ new->alpha2[0] = '9';
+ new->alpha2[1] = '9';
+ new->n_reg_rules = origrules + rd->n_reg_rules;
+ /* original rules still intact */
+ memcpy(&new->reg_rules[origrules],
+ rd->reg_rules,
+ sizeof(struct ieee80211_reg_rule) * rd->n_reg_rules);
+
+ set_regdom(new);
+ kfree(rd);
+
+ out:
+ mutex_unlock(&cfg80211_drv_mutex);
+}
+

static void print_rd_rules(const struct ieee80211_regdomain *rd)
{
@@ -710,7 +780,6 @@ static int __set_regdom(const struct iee

/* Tada! */
cfg80211_regdomain = rd;
- last_request->granted = 1;

return 0;
}




2008-10-23 19:53:44

by Marcel Holtmann

[permalink] [raw]
Subject: RE: [PATCH] wireless: add regulatory_struct_hint

Hi Luis,

> >Since this is only for wiphys this seems reasonable. I just keep in the
> >back of my mind leaving open the possibility for other wireless
> >subsystems to be able to make use of the currently set regulatory domain
> >and its regulatory rules, but this is in keeping with that as our
> >current requests are not changing the regulatory definitions, and just
> >as we have a wiphy for last_request we can add later struct
> >foo_new_wireless_type there too. I am curious if band definitions
> >should be shared between Bluetooth and 802.11 though. I don't think
> >BT devices have any notion of regulatory though nor are they capable of
> >exporting it though. Marcel is this correct? Inaky -- how about uwb, or
> >WiMax?
>
> UWB swipes over all the bands (from 3.1 to 10.6G), but keeping emission below FCCp15 limits (-41dBm, if memory serves) so it looks as interference to others. All the channel assignments are fixed and known, so in theory,
> it'd be possible to coordinate.

and Bluetooth uses the full 2.4 GHz band (split into 79 channels) and it
uses the full band equally and is allowed to. The regulatory efforts of
the Bluetooth SIG made it possible to use this world-wide. No regulatory
stuff is needed here.

Bluetooth will also use UWB in the future in the range of 6 GHz and it
will get the same world-wide regulatory effort.

The only part I am not sure is Bluetooth over 802.11 since they have
some weird stuff in there and the specification is not final yet.

Regards

Marcel



2008-10-27 03:08:58

by Zhu Yi

[permalink] [raw]
Subject: Re: [PATCH] wireless: add regulatory_struct_hint

On Fri, 2008-10-24 at 02:39 -0700, Johannes Berg wrote:
> On Fri, 2008-10-24 at 10:43 +0800, Zhu Yi wrote:
> >
> > The patch extends the current regulatory framework to support regulatory
> > enforcement by device hardware (firmware). If the regulatory check is
> > performed by hardware, the driver uses a special flag to indicate the
> > regulatory framework, so that the regulatory framework will bypass all
> > the regulatory checks for this device and delegate it to the hardware.
>
> The way I'm reading this, it's incorrect because it doesn't allow the
> user to override the hardware's idea of the regulatory domain. I think
> it's just a wrong description though.
>
> However, there is another major problem with this, if I use a USB device
> that has no regulatory information on a laptop that has this virtual
> regdomain configured because of a built-in Intel device, my USB device
> will wrongly enable all channels.
>
> But inspired by your patch, here's a different idea:
>
> * remove the struct regdomain hint thing
> * introduce a "hardware has regulatory check" flag, which means that
> - hardware will enforce regulatory compliance to whatever it thinks
> the regulatory domain is
> - the driver will, of course, still also enforce the information in
> wiphy->bands as it does now
> * if a wiphy has the "hw regulatory" flag set and the
> cfg80211_regdomain is the world regdomain (whether hard-coded or
> gotten from CRDA), then (and only then!) don't apply the
> cfg80211_regdomain to it
>
> This would have the following consequences:
> + much less code since all the hint stuff goes away
> + still works for users who move around with a hw-regulatory based
> laptop if they set the regdomain to something other than world
> manually
> - secondary hardware cannot benefit of the, now no longer given, hint
> which regdomain the laptop is in and will be restricted to world
> - some degree of confusion possible when one device can use channel 13
> (say iwl-agn hardware configured for Europe) and another cannot (say
> a USB device without regulatory information, leading to the world
> regdomain being the used one)

I totally agree.

Thanks,
-yi


2008-10-24 17:15:24

by Luis R. Rodriguez

[permalink] [raw]
Subject: Re: [PATCH] wireless: add regulatory_struct_hint

On Fri, Oct 24, 2008 at 02:39:36AM -0700, Johannes Berg wrote:
> On Fri, 2008-10-24 at 10:43 +0800, Zhu Yi wrote:
> >
> > The patch extends the current regulatory framework to support regulatory
> > enforcement by device hardware (firmware). If the regulatory check is
> > performed by hardware, the driver uses a special flag to indicate the
> > regulatory framework, so that the regulatory framework will bypass all
> > the regulatory checks for this device and delegate it to the hardware.
>
> The way I'm reading this, it's incorrect because it doesn't allow the
> user to override the hardware's idea of the regulatory domain. I think
> it's just a wrong description though.
>
> However, there is another major problem with this, if I use a USB device
> that has no regulatory information on a laptop that has this virtual
> regdomain configured because of a built-in Intel device, my USB device
> will wrongly enable all channels.
>
> But inspired by your patch, here's a different idea:
>
> * remove the struct regdomain hint thing
> * introduce a "hardware has regulatory check" flag, which means that
> - hardware will enforce regulatory compliance to whatever it thinks
> the regulatory domain is
> - the driver will, of course, still also enforce the information in
> wiphy->bands as it does now
> * if a wiphy has the "hw regulatory" flag set and the
> cfg80211_regdomain is the world regdomain (whether hard-coded or
> gotten from CRDA)

And if initiator == CORE

> , then (and only then!) don't apply the
> cfg80211_regdomain to it

I like this approach.

> This would have the following consequences:
> + much less code since all the hint stuff goes away
> + still works for users who move around with a hw-regulatory based
> laptop if they set the regdomain to something other than world
> manually
> - secondary hardware cannot benefit of the, now no longer given, hint
> which regdomain the laptop is in and will be restricted to world
> - some degree of confusion possible when one device can use channel 13
> (say iwl-agn hardware configured for Europe) and another cannot (say
> a USB device without regulatory information, leading to the world
> regdomain being the used one)

This will not be a problem if distributions end up asking to
set regdomain for a country for the user, which I think they should.

like_this_approach++

Luis

2008-10-24 02:42:44

by Zhu Yi

[permalink] [raw]
Subject: Re: [PATCH] wireless: add regulatory_struct_hint

On Tue, 2008-10-21 at 03:31 -0700, Johannes Berg wrote:
> This adds a new function, regulatory_struct_hint, which acts
> as a hint to the wireless core which regdomain a card thinks
> the system is operating in, but given in terms of the actual
> regdomain definition. Multiple hints are permitted when the
> specified bands do not overlap.
>
> Signed-off-by: Johannes Berg <[email protected]>

Thanks for the patch. I think this implemnted what we called the
per-band regulatory hint as we discussed. I'll test it.

It should solve the *real* problem. But in theory, it is still not
correct. I couldn't find a real example here. Let me just suppose a
non-exist device D, which is known to be broken for its channel 6. The
device excludes channel 6 on its EEPROM, thus the driver
regulatory_struct_hint() without channel 6. Then user insert the second
card. The second card works also without channel 6 support!

The real problem here is the mess up of device capabiliy vs. regulatory
rules. The channles a CARDx in a system with n wifi cards can be used
is:

(CARD1_REG & CARD2_REG & ... & CARDn_REG) & CARDx_CAPA

Given a device with the information of (CARDx_REG & CARDx_CAPA) in the
EEPROM, use it as the regulatory rules for the whole system is wrong. It
ends out CARDy is constrained with the capability of CARDx!

(CARD1_REG & ... & CARDx_REG & CARDx_CAPA & ...) & CARDy_CAPA

Although I couldn't find a real example in reality. And the valid dual
bands example is solved by this patch. So I don't have strong argument
on solving something not existed. But the logic is wrong!

I've attached my implmentation to this problem.

Thanks,
-yi


Attachments:
reg.patch (9.59 kB)

2008-10-24 18:18:50

by Marcel Holtmann

[permalink] [raw]
Subject: Re: [PATCH] wireless: add regulatory_struct_hint

Hi Johannes,

> However, there is another major problem with this, if I use a USB device
> that has no regulatory information on a laptop that has this virtual
> regdomain configured because of a built-in Intel device, my USB device
> will wrongly enable all channels.
>
> But inspired by your patch, here's a different idea:
>
> * remove the struct regdomain hint thing
> * introduce a "hardware has regulatory check" flag, which means that
> - hardware will enforce regulatory compliance to whatever it thinks
> the regulatory domain is
> - the driver will, of course, still also enforce the information in
> wiphy->bands as it does now
> * if a wiphy has the "hw regulatory" flag set and the
> cfg80211_regdomain is the world regdomain (whether hard-coded or
> gotten from CRDA), then (and only then!) don't apply the
> cfg80211_regdomain to it

we really only care about the case where we have no userspace capable of
setting the regulatory domain. So in only that case it makes sense to
fallback to the hardware regulatory support. So I would not make this
depend on world domain since that might not work good enough in the 5
GHz case. Can the mac80211 just track if userspace has set a regulatory
domain and then enforce it. Otherwise leave it to the hardware that
supports it or enforce the world domain to hardware that has no
regulatory support in hardware.

Can we also have a command that clears/resets the regulatory domain
setting so we get back to the initial state without rebooting. Would be
also good to have for testing.

> This would have the following consequences:
> + much less code since all the hint stuff goes away
> + still works for users who move around with a hw-regulatory based
> laptop if they set the regdomain to something other than world
> manually
> - secondary hardware cannot benefit of the, now no longer given, hint
> which regdomain the laptop is in and will be restricted to world
> - some degree of confusion possible when one device can use channel 13
> (say iwl-agn hardware configured for Europe) and another cannot (say
> a USB device without regulatory information, leading to the world
> regdomain being the used one)

This discussion started with having two adapters and disabling 5 GHz for
the second if the first one is BG only. This solution would make this
work when the second card sets the hardware regulatory flag. So it looks
good to me.

I don't see a big problem with restricting channels. Disabling a whole
band is an issue. However we could add printk's to tell the user when we
apply different regulatory domains to different devices, because one has
the hardware flag set.

Other than that, I think this idea makes a lot of sense. Having a much
more simpler logic is a good thing.

Regards

Marcel



2008-10-22 19:21:53

by Luis R. Rodriguez

[permalink] [raw]
Subject: Re: [PATCH] wireless: add regulatory_struct_hint

On Tue, Oct 21, 2008 at 03:31:50AM -0700, Johannes Berg wrote:
> This adds a new function, regulatory_struct_hint, which acts
> as a hint to the wireless core which regdomain a card thinks
> the system is operating in, but given in terms of the actual
> regdomain definition. Multiple hints are permitted when the
> specified bands do not overlap.
>
> Signed-off-by: Johannes Berg <[email protected]>
> ---
> Entirely untested. Anyone want to give it a go in the dual-band scenario?
>
> include/net/wireless.h | 14 +++
> net/wireless/reg.c | 225 ++++++++++++++++++++++++++++++++-----------------
> 2 files changed, 161 insertions(+), 78 deletions(-)
>
> --- everything.orig/include/net/wireless.h 2008-10-21 11:50:01.000000000 +0200
> +++ everything/include/net/wireless.h 2008-10-21 11:50:16.000000000 +0200
> @@ -355,4 +355,18 @@ ieee80211_get_channel(struct wiphy *wiph
> * for a regulatory domain structure for the respective country.
> */
> extern void regulatory_hint(const char *alpha2);
> +
> +/**
> + * regulatory_struct_hint - hint wireless core about regdomain
> + *
> + * @rd: regdomain structure containing the frequency ranges that are
> + * permitted for use.
> + * @bands: bitmask of bands this contains, use BIT(IEEE80211_BAND_...)

Since this is only for wiphys this seems reasonable. I just keep in the
back of my mind leaving open the possibility for other wireless
subsystems to be able to make use of the currently set regulatory domain
and its regulatory rules, but this is in keeping with that as our
current requests are not changing the regulatory definitions, and just
as we have a wiphy for last_request we can add later struct
foo_new_wireless_type there too. I am curious if band definitions
should be shared between Bluetooth and 802.11 though. I don't think
BT devices have any notion of regulatory though nor are they capable of
exporting it though. Marcel is this correct? Inaky -- how about uwb, or
WiMax?

For our purposes though this is OK though, just wanted to make that
note, so we keep in mind this *can* potentially be used by other
wireless foo.

> + *
> + * This function informs the wireless core that the driver believes
> + * that the bands indicated are defined by the given structure in the
> + * regulatory domain the system is operating in.
> + */
> +extern void regulatory_struct_hint(struct ieee80211_regdomain *rd,
> + u32 bands);
> #endif /* __NET_WIRELESS_H */
> --- everything.orig/net/wireless/reg.c 2008-10-21 11:50:14.000000000 +0200
> +++ everything/net/wireless/reg.c 2008-10-21 12:26:08.000000000 +0200
> @@ -45,9 +45,9 @@
> /* wiphy is set if this request's initiator is REGDOM_SET_BY_COUNTRY_IE */
> struct regulatory_request {
> struct wiphy *wiphy;
> - int granted;
> enum reg_set_by initiator;
> char alpha2[2];
> + u32 bands;
> };
>
> static struct regulatory_request *last_request;
> @@ -296,82 +296,6 @@ static int call_crda(const char *alpha2)
> return kobject_uevent_env(&reg_pdev->dev.kobj, KOBJ_CHANGE, envp);
> }
>
> -/* This has the logic which determines when a new request
> - * should be ignored. */
> -static int ignore_request(struct wiphy *wiphy, enum reg_set_by set_by,
> - const char *alpha2)
> -{

<-- snip -->

> -}
> -
> /* Used by nl80211 before kmalloc'ing our regulatory domain */
> bool reg_is_valid_request(const char *alpha2)
> {

<-- snip -->

> }
> }
>
> +/* This has the logic which determines when a new request
> + * should be ignored. */
> +static int ignore_request(struct wiphy *wiphy, enum reg_set_by set_by,
> + const char *alpha2)
> +{
> +}

I take it reg_is_valid_request() was just shifted, no changes were made to it
here?

> +
> /* Caller must hold &cfg80211_drv_mutex */
> int __regulatory_hint(struct wiphy *wiphy, enum reg_set_by set_by,
> const char *alpha2)
> @@ -567,6 +568,7 @@ int __regulatory_hint(struct wiphy *wiph
> request->alpha2[1] = alpha2[1];
> request->initiator = set_by;
> request->wiphy = wiphy;
> + request->bands = ~0;
>
> kfree(last_request);
> last_request = request;
> @@ -594,6 +596,74 @@ void regulatory_hint(const char *alpha2)
> }
> EXPORT_SYMBOL(regulatory_hint);
>
> +void regulatory_struct_hint(struct ieee80211_regdomain *rd, u32 bands)

I see you changed the return value to void for both routines the driver
or a subsystem can use, can you elaborate on the kdoc why this is the
case?

> +{
> + const struct ieee80211_regdomain *orig = NULL;
> + struct ieee80211_regdomain *new = NULL;
> + int origrules;
> +
> + BUG_ON(!rd);
> + BUG_ON(!bands);
> +
> + mutex_lock(&cfg80211_drv_mutex);
> +
> + /*
> + * ignore hint if anything else set it or if the given
> + * bands overlap already defined bands

Is the assumption all along here that this is for hardware which registers
only the channels its hardware is legally capable of? If so can the
documentation clarify that?

Otherwise it would seem to me if USER already set it we should get the
intersection. For example when a user plugs in a USB wireless
card it would not have gotten the chance to have regulatory_hint'd
first so the user may have already set it and if the driver
didn't have a reg_notifier() and simply registered all the channels
upon mac80211 register then the channels the user set will be
allowed. Also what about when the COUNTRY_IE had set it (remember the
result of such country IE request may be in an intersection with the
first driver too, but its ok, we always can trust the result of the
structure of the last set regdomain)? Again, disregard this comment
if the answer to the above paragraph is yes.

> + */
> + if (last_request) {
> + switch (last_request->initiator) {
> + case REGDOM_SET_BY_DRIVER:
> + if (last_request->bands & bands)
> + goto out;
> + break;
> + case REGDOM_SET_BY_CORE:
> + break;
> + default:
> + goto out;
> + }
> +
> + /* modify the currently set regdom */
> + orig = cfg80211_regdomain;
> + origrules = orig->n_reg_rules;
> + } else {
> + last_request = kzalloc(sizeof(struct regulatory_request),
> + GFP_KERNEL);
> + if (!last_request)
> + goto out;
> +
> + last_request->alpha2[0] = '9';
> + last_request->alpha2[1] = '9';
> + last_request->initiator = REGDOM_SET_BY_DRIVER;
> +
> + origrules = 0;
> + }
> +
> + last_request->bands |= bands;
> +
> + new = krealloc(orig,
> + sizeof(struct ieee80211_regdomain) +
> + sizeof(struct ieee80211_reg_rule) * origrules +
> + sizeof(struct ieee80211_reg_rule) * rd->n_reg_rules,
> + GFP_KERNEL);

Very nice :)

> + if (!new)
> + goto out;
> +
> + new->alpha2[0] = '9';
> + new->alpha2[1] = '9';

What about cases where the alpha2 can be determined? I know we have no
such hardware yet and doubt we will though. Or are we just going to
point those to use the alpha2 call instead?

> + new->n_reg_rules = origrules + rd->n_reg_rules;
> + /* original rules still intact */
> + memcpy(&new->reg_rules[origrules],
> + rd->reg_rules,
> + sizeof(struct ieee80211_reg_rule) * rd->n_reg_rules);

So did this work ? :) Zhu Yi, did you get to test?

> +
> + set_regdom(new);
> + kfree(rd);

Very nice indeed.

> +
> + out:
> + mutex_unlock(&cfg80211_drv_mutex);
> +}
> +
>
> static void print_rd_rules(const struct ieee80211_regdomain *rd)
> {
> @@ -710,7 +780,6 @@ static int __set_regdom(const struct iee
>
> /* Tada! */
> cfg80211_regdomain = rd;
> - last_request->granted = 1;

What's wrong with this?

Luis

2008-10-23 19:45:50

by Perez-Gonzalez, Inaky

[permalink] [raw]
Subject: RE: [PATCH] wireless: add regulatory_struct_hint

>From: Luis R. Rodriguez [mailto:[email protected]]
>
>Since this is only for wiphys this seems reasonable. I just keep in the
>back of my mind leaving open the possibility for other wireless
>subsystems to be able to make use of the currently set regulatory domain
>and its regulatory rules, but this is in keeping with that as our
>current requests are not changing the regulatory definitions, and just
>as we have a wiphy for last_request we can add later struct
>foo_new_wireless_type there too. I am curious if band definitions
>should be shared between Bluetooth and 802.11 though. I don't think
>BT devices have any notion of regulatory though nor are they capable of
>exporting it though. Marcel is this correct? Inaky -- how about uwb, or
>WiMax?

UWB swipes over all the bands (from 3.1 to 10.6G), but keeping emission below FCCp15 limits (-41dBm, if memory serves) so it looks as interference to others. All the channel assignments are fixed and known, so in theory,
it'd be possible to coordinate.

On WiMAX the bands are allocated per country and per operator, so if the
device can tell us what it supports or what the operator is telling it to
use, it should be possible for it to report it to some band controller for coordination, but I don't know if it'll make any sense to ask the device to use only bands so and so, because the knowledge of what's allowed is in the network side.

I might be missing the crux of the question though :)


2008-10-24 18:27:09

by Johannes Berg

[permalink] [raw]
Subject: Re: [PATCH] wireless: add regulatory_struct_hint

On Fri, 2008-10-24 at 03:15 -0700, Luis R. Rodriguez wrote:

> > * if a wiphy has the "hw regulatory" flag set and the
> > cfg80211_regdomain is the world regdomain (whether hard-coded or
> > gotten from CRDA)
>
> And if initiator == CORE

Makes sense, yeah.


> This will not be a problem if distributions end up asking to
> set regdomain for a country for the user, which I think they should.

Right.

I'll post the patches that rework the current stuff and remove the
struct hint, and then we can implement this on top.

johannes


Attachments:
signature.asc (836.00 B)
This is a digitally signed message part

2008-10-24 18:33:31

by Luis R. Rodriguez

[permalink] [raw]
Subject: Re: [PATCH] wireless: add regulatory_struct_hint

On Fri, Oct 24, 2008 at 11:26 AM, Johannes Berg
<[email protected]> wrote:
> On Fri, 2008-10-24 at 03:15 -0700, Luis R. Rodriguez wrote:
>
>> > * if a wiphy has the "hw regulatory" flag set and the
>> > cfg80211_regdomain is the world regdomain (whether hard-coded or
>> > gotten from CRDA)
>>
>> And if initiator == CORE
>
> Makes sense, yeah.

Now that i think about it, this would suffice for the check, no need
to check for world reg. Thoughts?

Luis

2008-10-23 19:54:43

by Perez-Gonzalez, Inaky

[permalink] [raw]
Subject: RE: [PATCH] wireless: add regulatory_struct_hint

>From: [email protected] [mailto:[email protected]] On Behalf Of Luis R.
>
>Actually that helps a lot. As you had mentioned before though we are
>not sure if *all* WiMAX devices will operate in the same way but its
>good to know what yours are doing. I was wondering specifically if
>other wireless technology can contribute to the regulatory rules
>collection we have in the future. It seems it *might* be more than
>anything we need coexistence technology.

You bet -- I just need the time to implement the proof of concept
we presented in the summit :)

Glad it helped,


2008-10-24 09:56:29

by Johannes Berg

[permalink] [raw]
Subject: Re: [PATCH] wireless: add regulatory_struct_hint

On Fri, 2008-10-24 at 10:43 +0800, Zhu Yi wrote:
>
> The patch extends the current regulatory framework to support regulatory
> enforcement by device hardware (firmware). If the regulatory check is
> performed by hardware, the driver uses a special flag to indicate the
> regulatory framework, so that the regulatory framework will bypass all
> the regulatory checks for this device and delegate it to the hardware.

The way I'm reading this, it's incorrect because it doesn't allow the
user to override the hardware's idea of the regulatory domain. I think
it's just a wrong description though.

However, there is another major problem with this, if I use a USB device
that has no regulatory information on a laptop that has this virtual
regdomain configured because of a built-in Intel device, my USB device
will wrongly enable all channels.

But inspired by your patch, here's a different idea:

* remove the struct regdomain hint thing
* introduce a "hardware has regulatory check" flag, which means that
- hardware will enforce regulatory compliance to whatever it thinks
the regulatory domain is
- the driver will, of course, still also enforce the information in
wiphy->bands as it does now
* if a wiphy has the "hw regulatory" flag set and the
cfg80211_regdomain is the world regdomain (whether hard-coded or
gotten from CRDA), then (and only then!) don't apply the
cfg80211_regdomain to it

This would have the following consequences:
+ much less code since all the hint stuff goes away
+ still works for users who move around with a hw-regulatory based
laptop if they set the regdomain to something other than world
manually
- secondary hardware cannot benefit of the, now no longer given, hint
which regdomain the laptop is in and will be restricted to world
- some degree of confusion possible when one device can use channel 13
(say iwl-agn hardware configured for Europe) and another cannot (say
a USB device without regulatory information, leading to the world
regdomain being the used one)

johannes


Attachments:
signature.asc (836.00 B)
This is a digitally signed message part

2008-10-22 19:31:50

by Johannes Berg

[permalink] [raw]
Subject: Re: [PATCH] wireless: add regulatory_struct_hint

On Wed, 2008-10-22 at 05:27 -0700, Luis R. Rodriguez wrote:
> On Tue, Oct 21, 2008 at 03:31:50AM -0700, Johannes Berg wrote:
> > +extern void regulatory_struct_hint(struct ieee80211_regdomain *rd,
> > + u32 bands);
> > #endif /* __NET_WIRELESS_H */
> > --- everything.orig/net/wireless/reg.c 2008-10-21 11:50:14.000000000 +0200
> > +++ everything/net/wireless/reg.c 2008-10-21 12:26:08.000000000 +0200
> > EXPORT_SYMBOL(regulatory_hint);
> >
> > +void regulatory_struct_hint(struct ieee80211_regdomain *rd, u32 bands)
> > +{
>
> <-- snip -->
>
> > +}
> > +
>
> Oh and forgot the EXPORT_SYMBOL(regulatory_struct_hint) though.

Oh, good point. Maybe I'll just let those who test this stuff add it
though :)

johannes


Attachments:
signature.asc (836.00 B)
This is a digitally signed message part

2008-10-23 21:21:20

by Tomas Winkler

[permalink] [raw]
Subject: Re: [PATCH] wireless: add regulatory_struct_hint

On Thu, Oct 23, 2008 at 9:54 PM, Marcel Holtmann
<[email protected]> wrote:
> Hi Luis,
>
>> >Since this is only for wiphys this seems reasonable. I just keep in the
>> >back of my mind leaving open the possibility for other wireless
>> >subsystems to be able to make use of the currently set regulatory domain
>> >and its regulatory rules, but this is in keeping with that as our
>> >current requests are not changing the regulatory definitions, and just
>> >as we have a wiphy for last_request we can add later struct
>> >foo_new_wireless_type there too. I am curious if band definitions
>> >should be shared between Bluetooth and 802.11 though. I don't think
>> >BT devices have any notion of regulatory though nor are they capable of
>> >exporting it though. Marcel is this correct? Inaky -- how about uwb, or
>> >WiMax?
>>
>> UWB swipes over all the bands (from 3.1 to 10.6G), but keeping emission below FCCp15 limits (-41dBm, if memory serves) so it looks as interference to others. All the channel assignments are fixed and known, so in theory,
>> it'd be possible to coordinate.
>
> and Bluetooth uses the full 2.4 GHz band (split into 79 channels) and it
> uses the full band equally and is allowed to. The regulatory efforts of
> the Bluetooth SIG made it possible to use this world-wide. No regulatory
> stuff is needed here.
>
> Bluetooth will also use UWB in the future in the range of 6 GHz and it
> will get the same world-wide regulatory effort.
>
> The only part I am not sure is Bluetooth over 802.11 since they have
> some weird stuff in there and the specification is not final yet.
>
> Regards
>
> Marcel
>
There is a similar effort going on in WiFi but it probably will take
some time, till this regulator nonsense is removed
Tomas

2008-10-22 19:27:10

by Luis R. Rodriguez

[permalink] [raw]
Subject: Re: [PATCH] wireless: add regulatory_struct_hint

On Tue, Oct 21, 2008 at 03:31:50AM -0700, Johannes Berg wrote:
> +extern void regulatory_struct_hint(struct ieee80211_regdomain *rd,
> + u32 bands);
> #endif /* __NET_WIRELESS_H */
> --- everything.orig/net/wireless/reg.c 2008-10-21 11:50:14.000000000 +0200
> +++ everything/net/wireless/reg.c 2008-10-21 12:26:08.000000000 +0200
> EXPORT_SYMBOL(regulatory_hint);
>
> +void regulatory_struct_hint(struct ieee80211_regdomain *rd, u32 bands)
> +{

<-- snip -->

> +}
> +

Oh and forgot the EXPORT_SYMBOL(regulatory_struct_hint) though.

Luis

2008-10-24 18:24:16

by Luis R. Rodriguez

[permalink] [raw]
Subject: Re: [PATCH] wireless: add regulatory_struct_hint

On Fri, Oct 24, 2008 at 11:18 AM, Marcel Holtmann <[email protected]> wrote:
> Can the mac80211 just track if userspace has set a regulatory
> domain and then enforce it.

yeap, when initiator == CORE

Luis

2008-10-24 18:36:37

by Johannes Berg

[permalink] [raw]
Subject: Re: [PATCH] wireless: add regulatory_struct_hint

On Fri, 2008-10-24 at 11:33 -0700, Luis R. Rodriguez wrote:
> On Fri, Oct 24, 2008 at 11:26 AM, Johannes Berg
> <[email protected]> wrote:
> > On Fri, 2008-10-24 at 03:15 -0700, Luis R. Rodriguez wrote:
> >
> >> > * if a wiphy has the "hw regulatory" flag set and the
> >> > cfg80211_regdomain is the world regdomain (whether hard-coded or
> >> > gotten from CRDA)
> >>
> >> And if initiator == CORE
> >
> > Makes sense, yeah.
>
> Now that i think about it, this would suffice for the check, no need
> to check for world reg. Thoughts?

Indeed, probably easier, CORE always only sets world.

johannes


Attachments:
signature.asc (836.00 B)
This is a digitally signed message part

2008-10-22 19:30:25

by Johannes Berg

[permalink] [raw]
Subject: Re: [PATCH] wireless: add regulatory_struct_hint


> For our purposes though this is OK though, just wanted to make that
> note, so we keep in mind this *can* potentially be used by other
> wireless foo.

True, but that hopefully won't even try to use this hint :)

> > +/* This has the logic which determines when a new request
> > + * should be ignored. */
> > +static int ignore_request(struct wiphy *wiphy, enum reg_set_by set_by,
> > + const char *alpha2)
> > +{
> > +}
>
> I take it reg_is_valid_request() was just shifted, no changes were made to it
> here?

Actually, I think I made a small change within, replacing a sequence of
if (...) with a switch().

> > +void regulatory_struct_hint(struct ieee80211_regdomain *rd, u32 bands)
>
> I see you changed the return value to void for both routines the driver
> or a subsystem can use, can you elaborate on the kdoc why this is the
> case?

Well, I don't think I should explain in kdoc why there is no return
value, but I think that the return value is pointless on a _hint_, as
we've seen in earlier discussions everybody seems to be confused by it.
Can you cite a use case? When would you ever care what the system did
with your hint?

> > + /*
> > + * ignore hint if anything else set it or if the given
> > + * bands overlap already defined bands
>
> Is the assumption all along here that this is for hardware which registers
> only the channels its hardware is legally capable of? If so can the
> documentation clarify that?
>
> Otherwise it would seem to me if USER already set it we should get the
> intersection. For example when a user plugs in a USB wireless
> card it would not have gotten the chance to have regulatory_hint'd
> first so the user may have already set it and if the driver
> didn't have a reg_notifier() and simply registered all the channels
> upon mac80211 register then the channels the user set will be
> allowed. Also what about when the COUNTRY_IE had set it (remember the
> result of such country IE request may be in an intersection with the
> first driver too, but its ok, we always can trust the result of the
> structure of the last set regdomain)? Again, disregard this comment
> if the answer to the above paragraph is yes.

I'm not sure I understand your concern. The point here is to allow a
driver to tell the core what it thinks the current operating domain is.
How do you define "[what] hardware is legally capable of"? The
assumption is that a driver will use this hint function if it thinks it
knows what the current operating domain is, in terms of frequency ranges
etc. I don't think there's any other assumption here.

> > + new->alpha2[0] = '9';
> > + new->alpha2[1] = '9';
>
> What about cases where the alpha2 can be determined? I know we have no
> such hardware yet and doubt we will though. Or are we just going to
> point those to use the alpha2 call instead?

Yeah, why would hardware tell us what the regdom is if it has alpha2?
This hypothetical hw could also do both, first hint the structs and then
the alpha2, and if crda is installed the alpha2 hint will call out and
update.

> > + new->n_reg_rules = origrules + rd->n_reg_rules;
> > + /* original rules still intact */
> > + memcpy(&new->reg_rules[origrules],
> > + rd->reg_rules,
> > + sizeof(struct ieee80211_reg_rule) * rd->n_reg_rules);
>
> So did this work ? :) Zhu Yi, did you get to test?

No idea :)

> > static void print_rd_rules(const struct ieee80211_regdomain *rd)
> > {
> > @@ -710,7 +780,6 @@ static int __set_regdom(const struct iee
> >
> > /* Tada! */
> > cfg80211_regdomain = rd;
> > - last_request->granted = 1;
>
> What's wrong with this?

Uh, I just removed the whole granted flag since it wasn't ever tested.
Side cleanup.

johannes


Attachments:
signature.asc (836.00 B)
This is a digitally signed message part

2008-10-23 19:51:14

by Luis R. Rodriguez

[permalink] [raw]
Subject: Re: [PATCH] wireless: add regulatory_struct_hint

On Thu, Oct 23, 2008 at 12:40 PM, Perez-Gonzalez, Inaky
<[email protected]> wrote:
>>From: Luis R. Rodriguez [mailto:[email protected]]
>>
>>Since this is only for wiphys this seems reasonable. I just keep in the
>>back of my mind leaving open the possibility for other wireless
>>subsystems to be able to make use of the currently set regulatory domain
>>and its regulatory rules, but this is in keeping with that as our
>>current requests are not changing the regulatory definitions, and just
>>as we have a wiphy for last_request we can add later struct
>>foo_new_wireless_type there too. I am curious if band definitions
>>should be shared between Bluetooth and 802.11 though. I don't think
>>BT devices have any notion of regulatory though nor are they capable of
>>exporting it though. Marcel is this correct? Inaky -- how about uwb, or
>>WiMax?
>
> UWB swipes over all the bands (from 3.1 to 10.6G), but keeping emission below FCCp15 limits (-41dBm, if memory serves) so it looks as interference to others. All the channel assignments are fixed and known, so in theory,
> it'd be possible to coordinate.
>
> On WiMAX the bands are allocated per country and per operator, so if the
> device can tell us what it supports or what the operator is telling it to
> use, it should be possible for it to report it to some band controller for coordination, but I don't know if it'll make any sense to ask the device to use only bands so and so, because the knowledge of what's allowed is in the network side.
>
> I might be missing the crux of the question though :)

Actually that helps a lot. As you had mentioned before though we are
not sure if *all* WiMAX devices will operate in the same way but its
good to know what yours are doing. I was wondering specifically if
other wireless technology can contribute to the regulatory rules
collection we have in the future. It seems it *might* be more than
anything we need coexistence technology.

Luis