diff mbox

[v2,4/4] cfg80211: Allow usermode to query wiphy specific regd info

Message ID 1415895219-19848-4-git-send-email-arik@wizery.com (mailing list archive)
State Not Applicable, archived
Headers show

Commit Message

Arik Nemtsov Nov. 13, 2014, 4:13 p.m. UTC
From: Jonathan Doron <jond@wizery.com>

Allow usermode to query wiphy-specific regd info, for drivers that use
wiphy-specific regulatory management.

Use the existing API for sending regdomain info to usermode, but return
the wiphy-specific regd in case wiphy index is provided and the driver
employs wiphy-specific management. This implies user and kernel-mode
support for the feature and is backward compatible.

Signed-off-by: Jonathan Doron <jonathanx.doron@intel.com>
Signed-off-by: Arik Nemtsov <arikx.nemtsov@intel.com>
---
 include/uapi/linux/nl80211.h |  6 +++++
 net/wireless/nl80211.c       | 54 +++++++++++++++++++++++++++++++++++---------
 net/wireless/reg.c           |  2 +-
 net/wireless/reg.h           |  1 +
 4 files changed, 51 insertions(+), 12 deletions(-)

Comments

Luis Chamberlain Nov. 13, 2014, 11:13 p.m. UTC | #1
On Thu, Nov 13, 2014 at 06:13:39PM +0200, Arik Nemtsov wrote:
> From: Jonathan Doron <jond@wizery.com>
> 
> Allow usermode to query wiphy-specific regd info, for drivers that use
> wiphy-specific regulatory management.
> 
> Use the existing API for sending regdomain info to usermode, but return
> the wiphy-specific regd in case wiphy index is provided and the driver
> employs wiphy-specific management. This implies user and kernel-mode
> support for the feature and is backward compatible.

This patch does not address my feedback about making this generic
to any wiphy->regd.

  Luis
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Arik Nemtsov Nov. 16, 2014, 11:06 a.m. UTC | #2
On Fri, Nov 14, 2014 at 1:13 AM, Luis R. Rodriguez <mcgrof@suse.com> wrote:
> On Thu, Nov 13, 2014 at 06:13:39PM +0200, Arik Nemtsov wrote:
>> From: Jonathan Doron <jond@wizery.com>
>>
>> Allow usermode to query wiphy-specific regd info, for drivers that use
>> wiphy-specific regulatory management.
>>
>> Use the existing API for sending regdomain info to usermode, but return
>> the wiphy-specific regd in case wiphy index is provided and the driver
>> employs wiphy-specific management. This implies user and kernel-mode
>> support for the feature and is backward compatible.
>
> This patch does not address my feedback about making this generic
> to any wiphy->regd.

Copy-pasting my previous reply:

Well always sending wiphy->regd whenever it is set is easy, but it
might be problematic I guess:

We intend to add a patch to wpa_s to always add the wiphy_idx to
NL80211_CMD_GET_REG. With the current approach only drivers with
SELF_MANAGED_REG will get wiphy->regd back. This is ok since these are
new drivers, which are familiar with this API.

But if we use your suggestion and always return wiphy->regd, then some
driver like ath9k that uses regulatory_hint() will now get it's
private regd returned to the wpa_s that manages it. I'm not saying
it's necessarily bad, but it's different than what was returned
before. The cfg80211 regdomain is intersected with wiphy->regd, so now
ath9k will start getting more permissive channels in usermode.

So we thought it's best to enable the new behavior only if the driver
explicitly wants it, using a new regulatory flag.

Arik
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Johannes Berg Nov. 20, 2014, 3:22 p.m. UTC | #3
On Sun, 2014-11-16 at 13:06 +0200, Arik Nemtsov wrote:

> We intend to add a patch to wpa_s to always add the wiphy_idx to
> NL80211_CMD_GET_REG. With the current approach only drivers with
> SELF_MANAGED_REG will get wiphy->regd back. This is ok since these are
> new drivers, which are familiar with this API.
> 
> But if we use your suggestion and always return wiphy->regd, then some
> driver like ath9k that uses regulatory_hint() will now get it's
> private regd returned to the wpa_s that manages it. I'm not saying
> it's necessarily bad, but it's different than what was returned
> before. The cfg80211 regdomain is intersected with wiphy->regd, so now
> ath9k will start getting more permissive channels in usermode.
> 
> So we thought it's best to enable the new behavior only if the driver
> explicitly wants it, using a new regulatory flag.

How does this work the other way around - i.e. a newer wpa_s requesting
per-wiphy information but it not being present?

It seems to me that either way what the kernel should return is the
information that will actually be applied when validated, which is of
course not possible when there are wiphy-specific regdomains and a
global one is requested (unless there's just one wiphy, which might be
something to consider making work?)

I also don't actually see a driver regulatory flag in this patch, as
expected, so not sure what exactly you're talking about above?

johannes

--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Arik Nemtsov Nov. 20, 2014, 4:47 p.m. UTC | #4
On Thu, Nov 20, 2014 at 5:22 PM, Johannes Berg
<johannes@sipsolutions.net> wrote:
> On Sun, 2014-11-16 at 13:06 +0200, Arik Nemtsov wrote:
>
>> We intend to add a patch to wpa_s to always add the wiphy_idx to
>> NL80211_CMD_GET_REG. With the current approach only drivers with
>> SELF_MANAGED_REG will get wiphy->regd back. This is ok since these are
>> new drivers, which are familiar with this API.
>>
>> But if we use your suggestion and always return wiphy->regd, then some
>> driver like ath9k that uses regulatory_hint() will now get it's
>> private regd returned to the wpa_s that manages it. I'm not saying
>> it's necessarily bad, but it's different than what was returned
>> before. The cfg80211 regdomain is intersected with wiphy->regd, so now
>> ath9k will start getting more permissive channels in usermode.
>>
>> So we thought it's best to enable the new behavior only if the driver
>> explicitly wants it, using a new regulatory flag.
>
> How does this work the other way around - i.e. a newer wpa_s requesting
> per-wiphy information but it not being present?

Then it gets the global one, and it knows it via a wiphy attribute:

      (wiphy->regulatory_flags & REGULATORY_WIPHY_SELF_MANAGED) &&
+                   (nla_put_flag(msg, NL80211_ATTR_WIPHY_SELF_MANAGED_REG)))
+                       goto nla_put_failure;

(we won't do this put_flag if it's global)

The supplicant patches are not up yet, but I think for now we will
just act according to the global one. Anyway it has the option to
change policy, since it knows.

>
> It seems to me that either way what the kernel should return is the
> information that will actually be applied when validated, which is of
> course not possible when there are wiphy-specific regdomains and a
> global one is requested (unless there's just one wiphy, which might be
> something to consider making work?)

Well the cfg80211 regdomain is always intersected with all wiphy->regd
in the system, so the global one is always less permissive.
This is of course true before REGULATORY_WIPHY_SELF_MANAGED - in this
case wiphy->regd will not affect the global regd.

With CUSTOM_REG we have a special case, since the regulatory code
doesn't set wiphy->regd, it just applies a user supplied regd to the
wiphy channels (which get validated).
If the user also happens to set  wiphy->regd himself with CUSTOM_REG,
then it makes sense to return the per-wiphy one. The global regd is
meaningless anyway for CUSTOM_REG, and the per-wiphy one might be too.
But I guess we can return it if it's there?

>
> I also don't actually see a driver regulatory flag in this patch, as
> expected, so not sure what exactly you're talking about above?

The regulatory flag is in the previous patch and is called
REGULATORY_WIPHY_SELF_MANAGED. The relevant code here is:

+               if (wiphy->regulatory_flags & REGULATORY_WIPHY_SELF_MANAGED)
+                       regdom = get_wiphy_regdom(wiphy);

Arik
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Luis Chamberlain Nov. 20, 2014, 8:54 p.m. UTC | #5
On Thu, Nov 20, 2014 at 06:47:24PM +0200, Arik Nemtsov wrote:
> On Thu, Nov 20, 2014 at 5:22 PM, Johannes Berg
> <johannes@sipsolutions.net> wrote:
> > On Sun, 2014-11-16 at 13:06 +0200, Arik Nemtsov wrote:
> >
> >> We intend to add a patch to wpa_s to always add the wiphy_idx to
> >> NL80211_CMD_GET_REG. With the current approach only drivers with
> >> SELF_MANAGED_REG will get wiphy->regd back. This is ok since these are
> >> new drivers, which are familiar with this API.
> >>
> >> But if we use your suggestion and always return wiphy->regd, then some
> >> driver like ath9k that uses regulatory_hint() will now get it's
> >> private regd returned to the wpa_s that manages it. I'm not saying
> >> it's necessarily bad, but it's different than what was returned
> >> before. The cfg80211 regdomain is intersected with wiphy->regd, so now
> >> ath9k will start getting more permissive channels in usermode.
> >>
> >> So we thought it's best to enable the new behavior only if the driver
> >> explicitly wants it, using a new regulatory flag.
> >
> > How does this work the other way around - i.e. a newer wpa_s requesting
> > per-wiphy information but it not being present?
> 
> Then it gets the global one, and it knows it via a wiphy attribute:
> 
>       (wiphy->regulatory_flags & REGULATORY_WIPHY_SELF_MANAGED) &&
> +                   (nla_put_flag(msg, NL80211_ATTR_WIPHY_SELF_MANAGED_REG)))
> +                       goto nla_put_failure;
> 
> (we won't do this put_flag if it's global)

You can still follow this on wpa_s for REGULATORY_WIPHY_SELF_MANAGED, and
the other type that uses wiphy->regd would still follow the global regdomain.
The other flag I'm looking for is more informational for userspace in particular
'iw reg get' (for central and all devices) or 'iw reg get dev wlan0'.

> The supplicant patches are not up yet, but I think for now we will
> just act according to the global one. Anyway it has the option to
> change policy, since it knows.
> 
> >
> > It seems to me that either way what the kernel should return is the
> > information that will actually be applied when validated, which is of
> > course not possible when there are wiphy-specific regdomains and a
> > global one is requested (unless there's just one wiphy, which might be
> > something to consider making work?)
> 
> Well the cfg80211 regdomain is always intersected with all wiphy->regd
> in the system, so the global one is always less permissive.
> This is of course true before REGULATORY_WIPHY_SELF_MANAGED - in this
> case wiphy->regd will not affect the global regd.

The discrepancy is a long term issue when device combing but it seems folks
dealing with REGULATORY_WIPHY_SELF_MANAGED are OK with supporting whatever
issues come up.

> With CUSTOM_REG we have a special case, since the regulatory code
> doesn't set wiphy->regd, it just applies a user supplied regd to the
> wiphy channels (which get validated).

Its a custom world regdom, its a minimum base set. New information is
always welcomed to help it comply further, and that's what happens.

> If the user also happens to set  wiphy->regd himself with CUSTOM_REG,
> then it makes sense to return the per-wiphy one. The global regd is
> meaningless anyway for CUSTOM_REG, and the per-wiphy one might be too.
> But I guess we can return it if it's there?

When CUSTOM_REG is used we already have code that deals with what is
allowed or now given new hints from different sources, wpa_s can
and should rely on the channel information state it is in already.
All the intersection logic is already dealt for the device. The only
thing wpa_s could probably care more about from the regulatory data is
the actual ISO3166 alpha2 used for country IEs.

  Luis
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Arik Nemtsov Nov. 21, 2014, 9:33 a.m. UTC | #6
On Thu, Nov 20, 2014 at 10:54 PM, Luis R. Rodriguez <mcgrof@suse.com> wrote:
>>
>> Then it gets the global one, and it knows it via a wiphy attribute:
>>
>>       (wiphy->regulatory_flags & REGULATORY_WIPHY_SELF_MANAGED) &&
>> +                   (nla_put_flag(msg, NL80211_ATTR_WIPHY_SELF_MANAGED_REG)))
>> +                       goto nla_put_failure;
>>
>> (we won't do this put_flag if it's global)
>
> You can still follow this on wpa_s for REGULATORY_WIPHY_SELF_MANAGED, and
> the other type that uses wiphy->regd would still follow the global regdomain.
> The other flag I'm looking for is more informational for userspace in particular
> 'iw reg get' (for central and all devices) or 'iw reg get dev wlan0'.

I'm not sure why another flag is needed for userspace. If we have
SELF_MANAGED, we'll return the per-wiphy one (if a wiphy_idx is given
to us of course). For validation purposes, the global one is used in
other cases. And for CUSTOM_REG, we'll return the global one (we don't
have the per-wiphy information in cfg80211).

Arik
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Luis Chamberlain Nov. 21, 2014, 11:38 p.m. UTC | #7
On Fri, Nov 21, 2014 at 11:33:43AM +0200, Arik Nemtsov wrote:
> On Thu, Nov 20, 2014 at 10:54 PM, Luis R. Rodriguez <mcgrof@suse.com> wrote:
> >>
> >> Then it gets the global one, and it knows it via a wiphy attribute:
> >>
> >>       (wiphy->regulatory_flags & REGULATORY_WIPHY_SELF_MANAGED) &&
> >> +                   (nla_put_flag(msg, NL80211_ATTR_WIPHY_SELF_MANAGED_REG)))
> >> +                       goto nla_put_failure;
> >>
> >> (we won't do this put_flag if it's global)
> >
> > You can still follow this on wpa_s for REGULATORY_WIPHY_SELF_MANAGED, and
> > the other type that uses wiphy->regd would still follow the global regdomain.
> > The other flag I'm looking for is more informational for userspace in particular
> > 'iw reg get' (for central and all devices) or 'iw reg get dev wlan0'.
> 
> I'm not sure why another flag is needed for userspace.

wiphy->regd will be used for REGULATORY_WIPHY_SELF_MANAGED and also on
devices that regulatory_hint() is called. Even though users of regulatory_hint()
will still abide by an intersection its useful to display that device's
regulatory domain in userspace, for that you'd need to distinguish it from
REGULATORY_WIPHY_SELF_MANAGED so another flag is needed. Showing that
regulatory domain will be informational.

> If we have
> SELF_MANAGED, we'll return the per-wiphy one (if a wiphy_idx is given
> to us of course). For validation purposes, the global one is used in
> other cases. And for CUSTOM_REG, we'll return the global one (we don't
> have the per-wiphy information in cfg80211).

Atheros cards always use CUSTOM_REG first, then later if they determine
the card was programmed with an alpha2 regulatory_hint() is called and
the CUSTOM_REG flag cleared. Regardless if wiphy->regd is set it would
be useful to have that information available.

  Luis
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/include/uapi/linux/nl80211.h b/include/uapi/linux/nl80211.h
index f050dcb..9c996bc 100644
--- a/include/uapi/linux/nl80211.h
+++ b/include/uapi/linux/nl80211.h
@@ -1674,6 +1674,10 @@  enum nl80211_commands {
  * @NL80211_ATTR_SMPS_MODE: SMPS mode to use (ap mode). see
  *	&enum nl80211_smps_mode.
  *
+ * @NL80211_ATTR_WIPHY_SELF_MANAGED_REG: flag attribute indicating the
+ *	regulatory information came from the driver and not from the global
+ *	cfg80211 regulatory domain information.
+ *
  * @NL80211_ATTR_MAX: highest attribute number currently defined
  * @__NL80211_ATTR_AFTER_LAST: internal use
  */
@@ -2026,6 +2030,8 @@  enum nl80211_attrs {
 
 	NL80211_ATTR_SMPS_MODE,
 
+	NL80211_ATTR_WIPHY_SELF_MANAGED_REG,
+
 	/* add attributes here, update the policy in nl80211.c */
 
 	__NL80211_ATTR_AFTER_LAST,
diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c
index 3a422e6..f1f632c 100644
--- a/net/wireless/nl80211.c
+++ b/net/wireless/nl80211.c
@@ -395,6 +395,7 @@  static const struct nla_policy nl80211_policy[NL80211_ATTR_MAX+1] = {
 	[NL80211_ATTR_USER_PRIO] = { .type = NLA_U8 },
 	[NL80211_ATTR_ADMITTED_TIME] = { .type = NLA_U16 },
 	[NL80211_ATTR_SMPS_MODE] = { .type = NLA_U8 },
+	[NL80211_ATTR_WIPHY_SELF_MANAGED_REG] = { .type = NLA_FLAG },
 };
 
 /* policy for the key attributes */
@@ -5287,14 +5288,24 @@  static int nl80211_update_mesh_config(struct sk_buff *skb,
 
 static int nl80211_get_reg(struct sk_buff *skb, struct genl_info *info)
 {
-	const struct ieee80211_regdomain *regdom;
+	const struct ieee80211_regdomain *regdom = NULL;
+	struct cfg80211_registered_device *rdev;
 	struct sk_buff *msg;
 	void *hdr = NULL;
 	struct nlattr *nl_reg_rules;
 	unsigned int i;
 
-	if (!cfg80211_regdomain)
-		return -EINVAL;
+	if (info->attrs[NL80211_ATTR_WIPHY] != NULL) {
+		struct wiphy *wiphy;
+
+		rdev = cfg80211_get_dev_from_info(genl_info_net(info), info);
+		if (IS_ERR(rdev))
+			return PTR_ERR(rdev);
+
+		wiphy = &rdev->wiphy;
+		if (wiphy->regulatory_flags & REGULATORY_WIPHY_SELF_MANAGED)
+			regdom = get_wiphy_regdom(wiphy);
+	}
 
 	msg = nlmsg_new(NLMSG_DEFAULT_SIZE, GFP_KERNEL);
 	if (!msg)
@@ -5305,13 +5316,27 @@  static int nl80211_get_reg(struct sk_buff *skb, struct genl_info *info)
 	if (!hdr)
 		goto put_failure;
 
-	if (reg_last_request_cell_base() &&
-	    nla_put_u32(msg, NL80211_ATTR_USER_REG_HINT_TYPE,
-			NL80211_USER_REG_HINT_CELL_BASE))
-		goto nla_put_failure;
+	if (!regdom) {
+		if (!cfg80211_regdomain) {
+			nlmsg_free(msg);
+			return -EINVAL;
+		}
+
+		if (reg_last_request_cell_base() &&
+		    nla_put_u32(msg, NL80211_ATTR_USER_REG_HINT_TYPE,
+				NL80211_USER_REG_HINT_CELL_BASE))
+			goto nla_put_failure;
+	} else {
+		if (nla_put_flag(msg, NL80211_ATTR_WIPHY_SELF_MANAGED_REG))
+			goto nla_put_failure;
+		if (nla_put_u32(msg, NL80211_ATTR_WIPHY, rdev->wiphy_idx))
+			goto nla_put_failure;
+	}
 
 	rcu_read_lock();
-	regdom = rcu_dereference(cfg80211_regdomain);
+
+	if (regdom == NULL)
+		regdom = rcu_dereference(cfg80211_regdomain);
 
 	if (nla_put_string(msg, NL80211_ATTR_REG_ALPHA2, regdom->alpha2) ||
 	    (regdom->dfs_region &&
@@ -10687,9 +10712,16 @@  static bool nl80211_reg_change_event_fill(struct sk_buff *msg,
 			goto nla_put_failure;
 	}
 
-	if (request->wiphy_idx != WIPHY_IDX_INVALID &&
-	    nla_put_u32(msg, NL80211_ATTR_WIPHY, request->wiphy_idx))
-		goto nla_put_failure;
+	if (request->wiphy_idx != WIPHY_IDX_INVALID) {
+		struct wiphy *wiphy;
+
+		wiphy = wiphy_idx_to_wiphy(request->wiphy_idx);
+		if ((wiphy != NULL) &&
+		    nla_put_u32(msg, NL80211_ATTR_WIPHY, request->wiphy_idx) &&
+		    (wiphy->regulatory_flags & REGULATORY_WIPHY_SELF_MANAGED) &&
+		    (nla_put_flag(msg, NL80211_ATTR_WIPHY_SELF_MANAGED_REG)))
+			goto nla_put_failure;
+	}
 
 	return true;
 
diff --git a/net/wireless/reg.c b/net/wireless/reg.c
index 4ee282d..a5f32d5 100644
--- a/net/wireless/reg.c
+++ b/net/wireless/reg.c
@@ -142,7 +142,7 @@  static const struct ieee80211_regdomain *get_cfg80211_regdom(void)
 	return rtnl_dereference(cfg80211_regdomain);
 }
 
-static const struct ieee80211_regdomain *get_wiphy_regdom(struct wiphy *wiphy)
+const struct ieee80211_regdomain *get_wiphy_regdom(struct wiphy *wiphy)
 {
 	return rtnl_dereference(wiphy->regd);
 }
diff --git a/net/wireless/reg.h b/net/wireless/reg.h
index 5e48031..4b45d6e 100644
--- a/net/wireless/reg.h
+++ b/net/wireless/reg.h
@@ -38,6 +38,7 @@  unsigned int reg_get_max_bandwidth(const struct ieee80211_regdomain *rd,
 				   const struct ieee80211_reg_rule *rule);
 
 bool reg_last_request_cell_base(void);
+const struct ieee80211_regdomain *get_wiphy_regdom(struct wiphy *wiphy);
 
 /**
  * regulatory_hint_found_beacon - hints a beacon was found on a channel