| Message ID | 1424944447-10988-2-git-send-email-ilan.peer@intel.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Johannes Berg |
| Headers | show |
On Thu, Feb 26, 2015 at 04:54:06AM -0500, Ilan Peer wrote: > Previously, the indoor setting configuration assumed that as > long as a station interface is connected, the indoor environment > setting does not change. However, this assumption is problematic > as: > > - It is possible that a station interface is connected to a mobile > AP, e.g., softAP or a P2P GO, where it is possible that both the > station and the mobile AP move out of the indoor environment making > the indoor setting invalid. In such a case, user space has no way to > invalidate the setting. > - A station interface disconnection does not necessarily imply that > the device is no longer operating in an indoor environment, e.g., > it is possible that the station interface is roaming but is still > stays indoor. > > To handle the above, extend the indoor configuration API to allow > user space to indicate a change of indoor settings, and allow it to > indicate weather it controls the indoor setting, such that: > > 1. If the user space process explicitly indicates that it is going > to control the indoor setting, do not clear the indoor setting > internally, unless the socket is released. The user space process > should use the NL80211_ATTR_SOCKET_OWNER attribute in the command > to state that it is going to control the indoor setting. > 2. Reset the indoor setting when restoring the regulatory settings in > case it is not owned by a user space process. > > Based on the above, a user space tool that continuously monitors the > indoor settings, i.e., tracking power setting, location etc., can > indicate environment changes to the regulatory core. > > Currently, in case a received country IE includes environment > information (indicates that the country IE applies only to > indoor operation or only to outdoor operation) it is currently not > used. Since I spotted an issue and you have to remake this patch please also change this to: It should be noted that currently userspace is the only provided mechanism used to hint to the regulatory core over the indoor/outdoor environment -- while the country IEs do have an environment setting this has been completely ignored by the regulatory core by design for a while now since country IEs typically can contain bogus data. > @@ -4984,7 +4988,15 @@ static int nl80211_req_set_reg(struct sk_buff *skb, struct genl_info *info) > data = nla_data(info->attrs[NL80211_ATTR_REG_ALPHA2]); > return regulatory_hint_user(data, user_reg_hint_type); > case NL80211_USER_REG_HINT_INDOOR: > - return regulatory_hint_indoor_user(); > + if (info->attrs[NL80211_ATTR_SOCKET_OWNER]) { > + owner_nlportid = info->snd_portid; > + is_indoor = !!info->attrs[NL80211_ATTR_REG_INDOOR]; > + } else { > + owner_nlportid = 0; > + is_indoor = true; > + } > + > + return regulatory_hint_indoor(is_indoor, owner_nlportid); Good, this looks better now. But you are missing the chance that a secondary random interface can come up and override that environmental information without checking for previous users. I'll illustrate below. > default: > @@ -2288,15 +2291,51 @@ int regulatory_hint_user(const char *alpha2, > return 0; > } > > -int regulatory_hint_indoor_user(void) > +int regulatory_hint_indoor(bool is_indoor, u32 portid) > { > + spin_lock(®_indoor_lock); > > + /* > + * Process only if there is a real change, so the original port ID is > + * saved (to handle cases that several processes try to change the > + * indoor setting). > + */ > + if (reg_is_indoor == is_indoor) { > + spin_unlock(®_indoor_lock); > + return 0; > + } > > - reg_is_indoor = true; > + reg_is_indoor = is_indoor; > + if (reg_is_indoor) > + reg_is_indoor_portid = portid; > + else > + reg_is_indoor_portid = 0; Why do we ignore port id if we go outdoor? Also we seem to allow interfaces to disagree on the setting, how are you handling that here? Don't tell me that cannot happen, I want you to think about that and handle it. 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
Hi Luis > -----Original Message----- > From: linux-wireless-owner@vger.kernel.org [mailto:linux-wireless- > owner@vger.kernel.org] On Behalf Of Luis R. Rodriguez > Sent: Thursday, February 26, 2015 19:42 > To: Peer, Ilan > Cc: linux-wireless@vger.kernel.org; ArikX Nemtsov > Subject: Re: [PATCH v8 2/3] cfg80211: Add API to change the indoor > regulatory setting > > On Thu, Feb 26, 2015 at 04:54:06AM -0500, Ilan Peer wrote: > > Previously, the indoor setting configuration assumed that as long as a > > station interface is connected, the indoor environment setting does > > not change. However, this assumption is problematic > > as: > > > > - It is possible that a station interface is connected to a mobile > > AP, e.g., softAP or a P2P GO, where it is possible that both the > > station and the mobile AP move out of the indoor environment making > > the indoor setting invalid. In such a case, user space has no way to > > invalidate the setting. > > - A station interface disconnection does not necessarily imply that > > the device is no longer operating in an indoor environment, e.g., > > it is possible that the station interface is roaming but is still > > stays indoor. > > > > To handle the above, extend the indoor configuration API to allow user > > space to indicate a change of indoor settings, and allow it to > > indicate weather it controls the indoor setting, such that: > > > > 1. If the user space process explicitly indicates that it is going > > to control the indoor setting, do not clear the indoor setting > > internally, unless the socket is released. The user space process > > should use the NL80211_ATTR_SOCKET_OWNER attribute in the command > > to state that it is going to control the indoor setting. > > 2. Reset the indoor setting when restoring the regulatory settings in > > case it is not owned by a user space process. > > > > Based on the above, a user space tool that continuously monitors the > > indoor settings, i.e., tracking power setting, location etc., can > > indicate environment changes to the regulatory core. > > > > Currently, in case a received country IE includes environment > > information (indicates that the country IE applies only to indoor > > operation or only to outdoor operation) it is currently not used. > > Since I spotted an issue and you have to remake this patch please also change > this to: > > It should be noted that currently userspace is the only provided mechanism > used to hint to the regulatory core over the indoor/outdoor environment -- > while the country IEs do have an environment setting this has been > completely ignored by the regulatory core by design for a while now since > country IEs typically can contain bogus data. > Sure. Done. > > @@ -4984,7 +4988,15 @@ static int nl80211_req_set_reg(struct sk_buff > *skb, struct genl_info *info) > > data = nla_data(info->attrs[NL80211_ATTR_REG_ALPHA2]); > > return regulatory_hint_user(data, user_reg_hint_type); > > case NL80211_USER_REG_HINT_INDOOR: > > - return regulatory_hint_indoor_user(); > > + if (info->attrs[NL80211_ATTR_SOCKET_OWNER]) { > > + owner_nlportid = info->snd_portid; > > + is_indoor = !!info- > >attrs[NL80211_ATTR_REG_INDOOR]; > > + } else { > > + owner_nlportid = 0; > > + is_indoor = true; > > + } > > + > > + return regulatory_hint_indoor(is_indoor, owner_nlportid); > > Good, this looks better now. But you are missing the chance that a secondary > random interface can come up and override that environmental information > without checking for previous users. I'll illustrate below. > > > default: > > @@ -2288,15 +2291,51 @@ int regulatory_hint_user(const char *alpha2, > > return 0; > > } > > > > -int regulatory_hint_indoor_user(void) > > +int regulatory_hint_indoor(bool is_indoor, u32 portid) > > { > > + spin_lock(®_indoor_lock); > > > > + /* > > + * Process only if there is a real change, so the original port ID is > > + * saved (to handle cases that several processes try to change the > > + * indoor setting). > > + */ > > + if (reg_is_indoor == is_indoor) { > > + spin_unlock(®_indoor_lock); > > + return 0; > > + } > > > > - reg_is_indoor = true; > > + reg_is_indoor = is_indoor; > > + if (reg_is_indoor) > > + reg_is_indoor_portid = portid; > > + else > > + reg_is_indoor_portid = 0; > > Why do we ignore port id if we go outdoor? Also we seem to allow interfaces This is back to the default, where the regulatory core does not assume indoor operation so it does not care about the owner in this case. Generally, the hint is an indoor hint, which states that the device is operating in indoor environment, clearing it does not imply that it is operating outdoor, but indicates there is no conclusive indication that the device is operating in indoor environment. > to disagree on the setting, how are you handling that here? > Don't tell me that cannot happen, I want you to think about that and handle it. > Handled this so only the original owner that set the indoor indication would be able to clear it. Thanks, Ilan. -- 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 2dcf9bb..e59ea18 100644 --- a/include/uapi/linux/nl80211.h +++ b/include/uapi/linux/nl80211.h @@ -1684,6 +1684,10 @@ enum nl80211_commands { * If set during scheduled scan start then the new scan req will be * owned by the netlink socket that created it and the scheduled scan will * be stopped when the socket is closed. + * If set during configuration of regulatory indoor operation then the + * regulatory indoor configuration would be owned by the netlink socket + * that configured the indoor setting, and the indoor operation would be + * cleared when the socket is closed. * * @NL80211_ATTR_TDLS_INITIATOR: flag attribute indicating the current end is * the TDLS link initiator. @@ -1739,6 +1743,9 @@ enum nl80211_commands { * * @NL80211_ATTR_SCHED_SCAN_DELAY: delay before a scheduled scan (or a * WoWLAN net-detect scan) is started, u32 in seconds. + + * @NL80211_ATTR_REG_INDOOR: flag attribute, if set indicates that the device + * is operating in an indoor environment. * * @NUM_NL80211_ATTR: total number of nl80211_attrs available * @NL80211_ATTR_MAX: highest attribute number currently defined @@ -2107,6 +2114,8 @@ enum nl80211_attrs { NL80211_ATTR_SCHED_SCAN_DELAY, + NL80211_ATTR_REG_INDOOR, + /* 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 9c6e23e..aa221a3 100644 --- a/net/wireless/nl80211.c +++ b/net/wireless/nl80211.c @@ -399,6 +399,7 @@ static const struct nla_policy nl80211_policy[NUM_NL80211_ATTR] = { [NL80211_ATTR_WIPHY_SELF_MANAGED_REG] = { .type = NLA_FLAG }, [NL80211_ATTR_NETNS_FD] = { .type = NLA_U32 }, [NL80211_ATTR_SCHED_SCAN_DELAY] = { .type = NLA_U32 }, + [NL80211_ATTR_REG_INDOOR] = { .type = NLA_FLAG }, }; /* policy for the key attributes */ @@ -4958,7 +4959,10 @@ static int parse_reg_rule(struct nlattr *tb[], static int nl80211_req_set_reg(struct sk_buff *skb, struct genl_info *info) { char *data = NULL; + bool is_indoor; enum nl80211_user_reg_hint_type user_reg_hint_type; + u32 owner_nlportid; + /* * You should only get this when cfg80211 hasn't yet initialized @@ -4984,7 +4988,15 @@ static int nl80211_req_set_reg(struct sk_buff *skb, struct genl_info *info) data = nla_data(info->attrs[NL80211_ATTR_REG_ALPHA2]); return regulatory_hint_user(data, user_reg_hint_type); case NL80211_USER_REG_HINT_INDOOR: - return regulatory_hint_indoor_user(); + if (info->attrs[NL80211_ATTR_SOCKET_OWNER]) { + owner_nlportid = info->snd_portid; + is_indoor = !!info->attrs[NL80211_ATTR_REG_INDOOR]; + } else { + owner_nlportid = 0; + is_indoor = true; + } + + return regulatory_hint_indoor(is_indoor, owner_nlportid); default: return -EINVAL; } @@ -12767,6 +12779,11 @@ static int nl80211_netlink_notify(struct notifier_block * nb, rcu_read_unlock(); + /* + * It is possible that the user space process that is controlling the + * indoor setting disappeared, so notify the regulatory core. + */ + regulatory_netlink_notify(notify->portid); return NOTIFY_OK; } diff --git a/net/wireless/reg.c b/net/wireless/reg.c index c24c8bf..4ccf563 100644 --- a/net/wireless/reg.c +++ b/net/wireless/reg.c @@ -128,9 +128,12 @@ static int reg_num_devs_support_basehint; * State variable indicating if the platform on which the devices * are attached is operating in an indoor environment. The state variable * is relevant for all registered devices. - * (protected by RTNL) */ static bool reg_is_indoor; +static spinlock_t reg_indoor_lock; + +/* Used to track the userspace process controlling the indoor setting */ +static u32 reg_is_indoor_portid; static const struct ieee80211_regdomain *get_cfg80211_regdom(void) { @@ -2288,15 +2291,51 @@ int regulatory_hint_user(const char *alpha2, return 0; } -int regulatory_hint_indoor_user(void) +int regulatory_hint_indoor(bool is_indoor, u32 portid) { + spin_lock(®_indoor_lock); + /* + * Process only if there is a real change, so the original port ID is + * saved (to handle cases that several processes try to change the + * indoor setting). + */ + if (reg_is_indoor == is_indoor) { + spin_unlock(®_indoor_lock); + return 0; + } - reg_is_indoor = true; + reg_is_indoor = is_indoor; + if (reg_is_indoor) + reg_is_indoor_portid = portid; + else + reg_is_indoor_portid = 0; + + spin_unlock(®_indoor_lock); + + if (!is_indoor) + reg_check_channels(); return 0; } +void regulatory_netlink_notify(u32 portid) +{ + spin_lock(®_indoor_lock); + + if (reg_is_indoor_portid != portid) { + spin_unlock(®_indoor_lock); + return; + } + + reg_is_indoor = false; + reg_is_indoor_portid = 0; + + spin_unlock(®_indoor_lock); + + reg_check_channels(); +} + /* Driver hints */ int regulatory_hint(struct wiphy *wiphy, const char *alpha2) { @@ -2464,7 +2503,17 @@ static void restore_regulatory_settings(bool reset_user) ASSERT_RTNL(); - reg_is_indoor = false; + /* + * Clear the indoor setting in case that it is not controlled by user + * space, as otherwise there is no guarantee that the device is still + * operating in an indoor environment. + */ + spin_lock(®_indoor_lock); + if (reg_is_indoor && !reg_is_indoor_portid) { + reg_is_indoor = false; + reg_check_channels(); + } + spin_unlock(®_indoor_lock); reset_regdomains(true, &world_regdom); restore_alpha2(alpha2, reset_user); @@ -3061,6 +3110,7 @@ int __init regulatory_init(void) spin_lock_init(®_requests_lock); spin_lock_init(®_pending_beacons_lock); + spin_lock_init(®_indoor_lock); reg_regdb_size_check(); diff --git a/net/wireless/reg.h b/net/wireless/reg.h index 4b45d6e..a2c4e16 100644 --- a/net/wireless/reg.h +++ b/net/wireless/reg.h @@ -25,7 +25,20 @@ enum nl80211_dfs_regions reg_get_dfs_region(struct wiphy *wiphy); int regulatory_hint_user(const char *alpha2, enum nl80211_user_reg_hint_type user_reg_hint_type); -int regulatory_hint_indoor_user(void); + +/** + * regulatory_hint_indoor - hint operation in indoor env. or not + * @is_indoor: if true indicates that user space thinks that the + * device is operating in an indoor environment. + * @portid: the netlink port ID on which the hint was given. + */ +int regulatory_hint_indoor(bool is_indoor, u32 portid); + +/** + * regulatory_netlink_notify - notify on released netlink socket + * @portid: the netlink socket port ID + */ +void regulatory_netlink_notify(u32 portid); void wiphy_regulatory_register(struct wiphy *wiphy); void wiphy_regulatory_deregister(struct wiphy *wiphy);