| Message ID | 1417074298-12254-2-git-send-email-arik@wizery.com (mailing list archive) |
|---|---|
| State | Not Applicable, archived |
| Headers | show |
On Thu, 2014-11-27 at 09:44 +0200, Arik Nemtsov wrote: > If a wiphy-idx is specified, the kernel will return the wiphy specific > regdomain, if such exists. Otherwise return the global regdom. > > When no wiphy-idx is specified, return the global regdomain as well as > all wiphy-specific regulatory domains in the system, via a new nested > list of attributes. Is that really a good idea? Seems rather easy to overrun the message size with that, in which case your current code will not return anything at all... that'll cause strange errors if somebody plugs in a few devices or has hwsim open as well or so ... > Add a new attribute for each wiphy-specific regdomain, for usermode to > identify it as such. Shouldn't userspace also *request* this for backward compatibility? Otherwise older userspace might assume that a returned regd applies to everything, when it doesn't really? 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
On Fri, Nov 28, 2014 at 02:46:27PM +0100, Johannes Berg wrote: > On Thu, 2014-11-27 at 09:44 +0200, Arik Nemtsov wrote: > > If a wiphy-idx is specified, the kernel will return the wiphy specific > > regdomain, if such exists. Otherwise return the global regdom. > > > > When no wiphy-idx is specified, return the global regdomain as well as > > all wiphy-specific regulatory domains in the system, via a new nested > > list of attributes. > > Is that really a good idea? Seems rather easy to overrun the message > size with that, in which case your current code will not return anything > at all... that'll cause strange errors if somebody plugs in a few > devices or has hwsim open as well or so ... Good point, perhaps require 'iw reg get --all' for all listing then? This would mean requiring an new optional flag passed on reg get too then. > > Add a new attribute for each wiphy-specific regdomain, for usermode to > > identify it as such. > > Shouldn't userspace also *request* this for backward compatibility? > Otherwise older userspace might assume that a returned regd applies to > everything, when it doesn't really? If the flag --all is used and passed then I see no issue. 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
On Thu, Nov 27, 2014 at 09:44:56AM +0200, Arik Nemtsov wrote: > If a wiphy-idx is specified, the kernel will return the wiphy specific > regdomain, if such exists. Otherwise return the global regdom. > > When no wiphy-idx is specified, return the global regdomain as well as > all wiphy-specific regulatory domains in the system, via a new nested > list of attributes. > > Add a new attribute for each wiphy-specific regdomain, for usermode to > identify it as such. > > Signed-off-by: Arik Nemtsov <arikx.nemtsov@intel.com> > --- > v5: don't return all regdomains if a specific wiphy is requested > > include/uapi/linux/nl80211.h | 16 +++++- > net/wireless/nl80211.c | 127 +++++++++++++++++++++++++++++++++---------- > net/wireless/reg.c | 2 +- > net/wireless/reg.h | 1 + > 4 files changed, 115 insertions(+), 31 deletions(-) > > diff --git a/include/uapi/linux/nl80211.h b/include/uapi/linux/nl80211.h > index d775245..1f2f7d6 100644 > --- a/include/uapi/linux/nl80211.h > +++ b/include/uapi/linux/nl80211.h > @@ -252,7 +252,9 @@ > * %NL80211_ATTR_IFINDEX. > * > * @NL80211_CMD_GET_REG: ask the wireless core to send us its currently set > - * regulatory domain. > + * regulatory domain. If %NL80211_ATTR_WIPHY is specified and the device > + * has a private regulatory domain, it will be returned. Otherwise, the > + * global regdomain will be returned. > * @NL80211_CMD_SET_REG: Set current regulatory domain. CRDA sends this command > * after being queried by the kernel. CRDA replies by sending a regulatory > * domain structure which consists of %NL80211_ATTR_REG_ALPHA set to our > @@ -1688,6 +1690,14 @@ enum nl80211_commands { > * > * @NL80211_ATTR_MAC_MASK: MAC address mask > * > + * @NL80211_ATTR_WIPHY_PRIV_REG: flag attribute indicating the regulatory > + * information was obtained from the device's wiphy. This can happen > + * when the driver uses the regulatory_hint() API for setting the device's > + * regulatory domain. Can you clarify here that even if a driver used regulatory_hint() its device will still have some settings further restricted by consenus with other regulatory data gathered by cfg80211, the main cfg80211 regulatory domain is reflective of what the wiphy is really allowed, the wiphy->regd in this case would be reflective of the regulatory domain that the device originally wanted. Other than that I think we need a flag to let nl80211 pass all regdomains, to address Johannes' concerns. 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
On Sat, Nov 29, 2014 at 12:00 AM, Luis R. Rodriguez <mcgrof@suse.com> wrote: > On Fri, Nov 28, 2014 at 02:46:27PM +0100, Johannes Berg wrote: >> On Thu, 2014-11-27 at 09:44 +0200, Arik Nemtsov wrote: >> > If a wiphy-idx is specified, the kernel will return the wiphy specific >> > regdomain, if such exists. Otherwise return the global regdom. >> > >> > When no wiphy-idx is specified, return the global regdomain as well as >> > all wiphy-specific regulatory domains in the system, via a new nested >> > list of attributes. >> >> Is that really a good idea? Seems rather easy to overrun the message >> size with that, in which case your current code will not return anything >> at all... that'll cause strange errors if somebody plugs in a few >> devices or has hwsim open as well or so ... > > Good point, perhaps require 'iw reg get --all' for all listing then? > This would mean requiring an new optional flag passed on reg get too > then. I think Johannes' point was that it's easy to overrun the message size if there are a lot of wiphys. So I'll simply add an iterator over all wiphys in "iw reg get" and kernel-mode will only return a single regdomain in each GET_REG invocation. About the "--all" suggestion - I think it's fine to not have backward compatibility in the output of "iw reg get"? So we can just output the global first, and then output private regdoms for all wiphys that have them. Does that sound ok? > >> > Add a new attribute for each wiphy-specific regdomain, for usermode to >> > identify it as such. >> >> Shouldn't userspace also *request* this for backward compatibility? >> Otherwise older userspace might assume that a returned regd applies to >> everything, when it doesn't really? > > If the flag --all is used and passed then I see no issue. Well you have to give a wiphy-idx in order to get a private regdom in the first place. And only new userspace will add a wiphy-idx in the first place.. Also, when a global regdom is returned for a given wiphy-idx instead of a private one, it is valid, since the global one is being used for this wiphy. If there is a private regdom (from regulatory_hint()) then we'll return it to usermode. This is less restrictive than the global one, and that's the one being used for channel verification for this 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
On Sun, 2014-11-30 at 14:26 +0200, Arik Nemtsov wrote: > I think Johannes' point was that it's easy to overrun the message size > if there are a lot of wiphys. Yes. > So I'll simply add an iterator over all wiphys in "iw reg get" and > kernel-mode will only return a single regdomain in each GET_REG > invocation. Err, there's such a thing built into netlink already - just support dumpit instead of doit :) iw will have to fall back to doit for older kernels though I guess. > About the "--all" suggestion - I think it's fine to not have backward > compatibility in the output of "iw reg get"? So we can just output the > global first, and then output private regdoms for all wiphys that have > them. > > Does that sound ok? Yeah I wasn't taking about the iw display, and adding --all there doesn't help for what I was concerned about. > Well you have to give a wiphy-idx in order to get a private regdom in > the first place. And only new userspace will add a wiphy-idx in the > first place.. Are you sure about the last part though? wpa_supplicant often passed a netdev index instead of a wiphy index for example, so I could imagine it passing a wiphy index here even though it was previously ignored? If it didn't though then I think there's no problem, there shouldn't really be any userspace other than wpa_s and iw for this I guess/hope. 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
On Sun, Nov 30, 2014 at 2:32 PM, Johannes Berg <johannes@sipsolutions.net> wrote: > On Sun, 2014-11-30 at 14:26 +0200, Arik Nemtsov wrote: > >> I think Johannes' point was that it's easy to overrun the message size >> if there are a lot of wiphys. > > Yes. > >> So I'll simply add an iterator over all wiphys in "iw reg get" and >> kernel-mode will only return a single regdomain in each GET_REG >> invocation. > > Err, there's such a thing built into netlink already - just support > dumpit instead of doit :) That's cool. I'll use it then :) > iw will have to fall back to doit for older kernels though I guess. Well older kernels don't have this feature anyway, so it's fine to leave the doit as is. For backports, we are bringing this code with us anyway right? > >> About the "--all" suggestion - I think it's fine to not have backward >> compatibility in the output of "iw reg get"? So we can just output the >> global first, and then output private regdoms for all wiphys that have >> them. >> >> Does that sound ok? > > Yeah I wasn't taking about the iw display, and adding --all there > doesn't help for what I was concerned about. > >> Well you have to give a wiphy-idx in order to get a private regdom in >> the first place. And only new userspace will add a wiphy-idx in the >> first place.. > > Are you sure about the last part though? wpa_supplicant often passed a > netdev index instead of a wiphy index for example, so I could imagine it > passing a wiphy index here even though it was previously ignored? > > If it didn't though then I think there's no problem, there shouldn't > really be any userspace other than wpa_s and iw for this I guess/hope. It definitely doesn't pass it now. Otherwise we wouldn't have to change it :) 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
diff --git a/include/uapi/linux/nl80211.h b/include/uapi/linux/nl80211.h index d775245..1f2f7d6 100644 --- a/include/uapi/linux/nl80211.h +++ b/include/uapi/linux/nl80211.h @@ -252,7 +252,9 @@ * %NL80211_ATTR_IFINDEX. * * @NL80211_CMD_GET_REG: ask the wireless core to send us its currently set - * regulatory domain. + * regulatory domain. If %NL80211_ATTR_WIPHY is specified and the device + * has a private regulatory domain, it will be returned. Otherwise, the + * global regdomain will be returned. * @NL80211_CMD_SET_REG: Set current regulatory domain. CRDA sends this command * after being queried by the kernel. CRDA replies by sending a regulatory * domain structure which consists of %NL80211_ATTR_REG_ALPHA set to our @@ -1688,6 +1690,14 @@ enum nl80211_commands { * * @NL80211_ATTR_MAC_MASK: MAC address mask * + * @NL80211_ATTR_WIPHY_PRIV_REG: flag attribute indicating the regulatory + * information was obtained from the device's wiphy. This can happen + * when the driver uses the regulatory_hint() API for setting the device's + * regulatory domain. + * + * @NL80211_ATTR_WIPHY_REGDOM_LIST: Nested set of attributes containing + * a list of wiphy specific regdomains. + * * @NUM_NL80211_ATTR: total number of nl80211_attrs available * @NL80211_ATTR_MAX: highest attribute number currently defined * @__NL80211_ATTR_AFTER_LAST: internal use @@ -2045,6 +2055,10 @@ enum nl80211_attrs { NL80211_ATTR_MAC_MASK, + NL80211_ATTR_WIPHY_PRIV_REG, + + NL80211_ATTR_WIPHY_REGDOM_LIST, + /* 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 b5e3c48..ddee3ba 100644 --- a/net/wireless/nl80211.c +++ b/net/wireless/nl80211.c @@ -396,6 +396,8 @@ static const struct nla_policy nl80211_policy[NUM_NL80211_ATTR] = { [NL80211_ATTR_ADMITTED_TIME] = { .type = NLA_U16 }, [NL80211_ATTR_SMPS_MODE] = { .type = NLA_U8 }, [NL80211_ATTR_MAC_MASK] = { .len = ETH_ALEN }, + [NL80211_ATTR_WIPHY_PRIV_REG] = { .type = NLA_FLAG }, + [NL80211_ATTR_WIPHY_REGDOM_LIST] = { .type = NLA_NESTED }, }; /* policy for the key attributes */ @@ -5326,42 +5328,20 @@ static int nl80211_update_mesh_config(struct sk_buff *skb, return err; } -static int nl80211_get_reg(struct sk_buff *skb, struct genl_info *info) +static int nl80211_put_regdom(const struct ieee80211_regdomain *regdom, + struct sk_buff *msg) { - const struct ieee80211_regdomain *regdom; - struct sk_buff *msg; - void *hdr = NULL; struct nlattr *nl_reg_rules; unsigned int i; - if (!cfg80211_regdomain) - return -EINVAL; - - msg = nlmsg_new(NLMSG_DEFAULT_SIZE, GFP_KERNEL); - if (!msg) - return -ENOBUFS; - - hdr = nl80211hdr_put(msg, info->snd_portid, info->snd_seq, 0, - NL80211_CMD_GET_REG); - 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; - - rcu_read_lock(); - regdom = rcu_dereference(cfg80211_regdomain); - if (nla_put_string(msg, NL80211_ATTR_REG_ALPHA2, regdom->alpha2) || (regdom->dfs_region && nla_put_u8(msg, NL80211_ATTR_DFS_REGION, regdom->dfs_region))) - goto nla_put_failure_rcu; + goto nla_put_failure; nl_reg_rules = nla_nest_start(msg, NL80211_ATTR_REG_RULES); if (!nl_reg_rules) - goto nla_put_failure_rcu; + goto nla_put_failure; for (i = 0; i < regdom->n_reg_rules; i++) { struct nlattr *nl_reg_rule; @@ -5376,7 +5356,7 @@ static int nl80211_get_reg(struct sk_buff *skb, struct genl_info *info) nl_reg_rule = nla_nest_start(msg, i); if (!nl_reg_rule) - goto nla_put_failure_rcu; + goto nla_put_failure; max_bandwidth_khz = freq_range->max_bandwidth_khz; if (!max_bandwidth_khz) @@ -5397,13 +5377,102 @@ static int nl80211_get_reg(struct sk_buff *skb, struct genl_info *info) power_rule->max_eirp) || nla_put_u32(msg, NL80211_ATTR_DFS_CAC_TIME, reg_rule->dfs_cac_ms)) - goto nla_put_failure_rcu; + goto nla_put_failure; nla_nest_end(msg, nl_reg_rule); } - rcu_read_unlock(); nla_nest_end(msg, nl_reg_rules); + return 0; + +nla_put_failure: + return -EMSGSIZE; +} + +static int nl80211_get_reg(struct sk_buff *skb, struct genl_info *info) +{ + const struct ieee80211_regdomain *regdom = NULL; + struct cfg80211_registered_device *rdev; + struct wiphy *wiphy; + struct sk_buff *msg; + struct nlattr *nl_priv_regdoms, *nl_priv_regdom; + void *hdr = NULL; + int i; + + msg = nlmsg_new(NLMSG_DEFAULT_SIZE, GFP_KERNEL); + if (!msg) + return -ENOBUFS; + + hdr = nl80211hdr_put(msg, info->snd_portid, info->snd_seq, 0, + NL80211_CMD_GET_REG); + if (!hdr) + goto put_failure; + + if (info->attrs[NL80211_ATTR_WIPHY]) { + rdev = cfg80211_get_dev_from_info(genl_info_net(info), info); + if (IS_ERR(rdev)) { + nlmsg_free(msg); + return PTR_ERR(rdev); + } + + wiphy = &rdev->wiphy; + regdom = get_wiphy_regdom(wiphy); + if (regdom && + nla_put_u32(msg, NL80211_ATTR_WIPHY, rdev->wiphy_idx)) + 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; + } + + rcu_read_lock(); + + if (!regdom) + regdom = rcu_dereference(cfg80211_regdomain); + + if (nl80211_put_regdom(regdom, msg)) + goto nla_put_failure_rcu; + + /* bail here if a specific wiphy was requested */ + if (info->attrs[NL80211_ATTR_WIPHY]) + goto end; + + nl_priv_regdoms = nla_nest_start(msg, NL80211_ATTR_WIPHY_REGDOM_LIST); + if (!nl_priv_regdoms) + goto nla_put_failure_rcu; + + i = 0; + list_for_each_entry(rdev, &cfg80211_rdev_list, list) { + wiphy = &rdev->wiphy; + regdom = get_wiphy_regdom(wiphy); + if (!regdom) + continue; + + nl_priv_regdom = nla_nest_start(msg, i); + if (!nl_priv_regdom) + goto nla_put_failure_rcu; + + if (nl80211_put_regdom(regdom, msg)) + goto nla_put_failure_rcu; + + if (nla_put_flag(msg, NL80211_ATTR_WIPHY_PRIV_REG)) + goto nla_put_failure_rcu; + + nla_nest_end(msg, nl_priv_regdom); + i++; + } + nla_nest_end(msg, nl_priv_regdoms); +end: + rcu_read_unlock(); genlmsg_end(msg, hdr); return genlmsg_reply(msg, info); diff --git a/net/wireless/reg.c b/net/wireless/reg.c index 47be616..48d90da 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
If a wiphy-idx is specified, the kernel will return the wiphy specific regdomain, if such exists. Otherwise return the global regdom. When no wiphy-idx is specified, return the global regdomain as well as all wiphy-specific regulatory domains in the system, via a new nested list of attributes. Add a new attribute for each wiphy-specific regdomain, for usermode to identify it as such. Signed-off-by: Arik Nemtsov <arikx.nemtsov@intel.com> --- v5: don't return all regdomains if a specific wiphy is requested include/uapi/linux/nl80211.h | 16 +++++- net/wireless/nl80211.c | 127 +++++++++++++++++++++++++++++++++---------- net/wireless/reg.c | 2 +- net/wireless/reg.h | 1 + 4 files changed, 115 insertions(+), 31 deletions(-)