2015-02-02 20:57:07

by Peer, Ilan

Subject: [PATCH v4 1/2] cfg80211: Add API to change the indoor regulatory setting

As the operating environment of a device can change, add
API for user space to indicate a change of indoor settings.
In addition modify the handling of the indoor processing as
follows:

1. Directly update the indoor setting without wrapping it as a
regulatory request.
2. Track the socket on which the indoor hint is issued, and reset
indoor setting if the socket was released. The motivation here is to
force a user space process that sets the indoor setting to constantly
monitor this setting, and be responsible to correctly toggling it,
so indoor operation will not be enabled uncontrolled.
3. Do not reset the indoor setting when restoring the regulatory
settings as it has nothing to do with CRDA or interface
disconnection.

Signed-off-by: Ilan Peer <[email protected]>
Signed-off-by: ArikX Nemtsov <[email protected]>
---
include/uapi/linux/nl80211.h | 5 +++
net/wireless/nl80211.c | 10 +++++-
net/wireless/reg.c | 80 +++++++++++++++++++++++++-------------------
net/wireless/reg.h | 15 ++++++++-
4 files changed, 73 insertions(+), 37 deletions(-)

diff --git a/include/uapi/linux/nl80211.h b/include/uapi/linux/nl80211.h
index 68b294e..d80edcc 100644
--- a/include/uapi/linux/nl80211.h
+++ b/include/uapi/linux/nl80211.h
@@ -1739,6 +1739,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 +2110,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 454d7a0..e78b096 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 */
@@ -4955,6 +4956,7 @@ 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;

/*
@@ -4981,7 +4983,8 @@ 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();
+ is_indoor = !!info->attrs[NL80211_ATTR_REG_INDOOR];
+ return regulatory_hint_indoor(is_indoor, info->snd_portid);
default:
return -EINVAL;
}
@@ -12759,6 +12762,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 886cc7c..e96251f 100644
--- a/net/wireless/reg.c
+++ b/net/wireless/reg.c
@@ -82,17 +82,12 @@
* be intersected with the current one.
* @REG_REQ_ALREADY_SET: the regulatory request will not change the current
* regulatory settings, and no further processing is required.
- * @REG_REQ_USER_HINT_HANDLED: a non alpha2 user hint was handled and no
- * further processing is required, i.e., not need to update last_request
- * etc. This should be used for user hints that do not provide an alpha2
- * but some other type of regulatory hint, i.e., indoor operation.
*/
enum reg_request_treatment {
REG_REQ_OK,
REG_REQ_IGNORE,
REG_REQ_INTERSECT,
REG_REQ_ALREADY_SET,
- REG_REQ_USER_HINT_HANDLED,
};

static struct regulatory_request core_request_world = {
@@ -133,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)
{
@@ -1248,13 +1246,6 @@ static bool reg_request_cell_base(struct regulatory_request *request)
return request->user_reg_hint_type == NL80211_USER_REG_HINT_CELL_BASE;
}

-static bool reg_request_indoor(struct regulatory_request *request)
-{
- if (request->initiator != NL80211_REGDOM_SET_BY_USER)
- return false;
- return request->user_reg_hint_type == NL80211_USER_REG_HINT_INDOOR;
-}
-
bool reg_last_request_cell_base(void)
{
return reg_request_cell_base(get_last_request());
@@ -1821,11 +1812,6 @@ __reg_process_hint_user(struct regulatory_request *user_request)
{
struct regulatory_request *lr = get_last_request();

- if (reg_request_indoor(user_request)) {
- reg_is_indoor = true;
- return REG_REQ_USER_HINT_HANDLED;
- }
-
if (reg_request_cell_base(user_request))
return reg_ignore_cell_hint(user_request);

@@ -1873,8 +1859,7 @@ reg_process_hint_user(struct regulatory_request *user_request)

treatment = __reg_process_hint_user(user_request);
if (treatment == REG_REQ_IGNORE ||
- treatment == REG_REQ_ALREADY_SET ||
- treatment == REG_REQ_USER_HINT_HANDLED) {
+ treatment == REG_REQ_ALREADY_SET) {
reg_free_request(user_request);
return treatment;
}
@@ -1935,7 +1920,6 @@ reg_process_hint_driver(struct wiphy *wiphy,
case REG_REQ_OK:
break;
case REG_REQ_IGNORE:
- case REG_REQ_USER_HINT_HANDLED:
reg_free_request(driver_request);
return treatment;
case REG_REQ_INTERSECT:
@@ -2035,7 +2019,6 @@ reg_process_hint_country_ie(struct wiphy *wiphy,
case REG_REQ_OK:
break;
case REG_REQ_IGNORE:
- case REG_REQ_USER_HINT_HANDLED:
/* fall through */
case REG_REQ_ALREADY_SET:
reg_free_request(country_ie_request);
@@ -2074,8 +2057,7 @@ static void reg_process_hint(struct regulatory_request *reg_request)
case NL80211_REGDOM_SET_BY_USER:
treatment = reg_process_hint_user(reg_request);
if (treatment == REG_REQ_IGNORE ||
- treatment == REG_REQ_ALREADY_SET ||
- treatment == REG_REQ_USER_HINT_HANDLED)
+ treatment == REG_REQ_ALREADY_SET)
return;
queue_delayed_work(system_power_efficient_wq,
&reg_timeout, msecs_to_jiffies(3142));
@@ -2297,22 +2279,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)
{
- struct regulatory_request *request;
+ spin_lock(&reg_indoor_lock);

- request = kzalloc(sizeof(struct regulatory_request), GFP_KERNEL);
- if (!request)
- return -ENOMEM;
+ /*
+ * 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(&reg_indoor_lock);
+ return 0;
+ }

- request->wiphy_idx = WIPHY_IDX_INVALID;
- request->initiator = NL80211_REGDOM_SET_BY_USER;
- request->user_reg_hint_type = NL80211_USER_REG_HINT_INDOOR;
- queue_regulatory_request(request);
+ reg_is_indoor = is_indoor;
+ if (reg_is_indoor)
+ reg_is_indoor_portid = portid;
+ else
+ reg_is_indoor_portid = 0;
+
+ spin_unlock(&reg_indoor_lock);
+
+ if (!is_indoor)
+ reg_check_channels();

return 0;
}

+void regulatory_netlink_notify(u32 portid)
+{
+ spin_lock(&reg_indoor_lock);
+
+ if (reg_is_indoor_portid != portid) {
+ spin_unlock(&reg_indoor_lock);
+ return;
+ }
+
+ reg_is_indoor = false;
+ reg_is_indoor_portid = 0;
+
+ spin_unlock(&reg_indoor_lock);
+
+ reg_check_channels();
+}
+
/* Driver hints */
int regulatory_hint(struct wiphy *wiphy, const char *alpha2)
{
@@ -2480,8 +2491,6 @@ static void restore_regulatory_settings(bool reset_user)

ASSERT_RTNL();

- reg_is_indoor = false;
-
reset_regdomains(true, &world_regdom);
restore_alpha2(alpha2, reset_user);

@@ -3077,6 +3086,7 @@ int __init regulatory_init(void)

spin_lock_init(&reg_requests_lock);
spin_lock_init(&reg_pending_beacons_lock);
+ spin_lock_init(&reg_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);
--
1.8.3.2

2015-02-06 23:59:52

by Luis Chamberlain

Subject: Re: [PATCH v4 2/2] cfg80211: Schedule timeout for all CRDA calls

On Mon, Feb 02, 2015 at 09:59:26AM -0500, Ilan Peer wrote:
> Timeout was scheduled only in case CRDA was called due to user hints,
> but was not scheduled for other cases. This can result in regulatory
> hint processing getting stuck in case that there is no CRDA configured.
>
> Change this by scheduling a timeout every time CRDA is called. In
> addition, in restore_regulatory_settings() all pending requests are
> restored (and not only the user ones).
>
> Signed-off-by: Ilan Peer <[email protected]>

Reviewed-by: Luis R. Rodriguez <[email protected]>

Luis

2015-02-02 20:57:07

by Peer, Ilan

Subject: [PATCH v4 2/2] cfg80211: Schedule timeout for all CRDA calls

Timeout was scheduled only in case CRDA was called due to user hints,
but was not scheduled for other cases. This can result in regulatory
hint processing getting stuck in case that there is no CRDA configured.

Change this by scheduling a timeout every time CRDA is called. In
addition, in restore_regulatory_settings() all pending requests are
restored (and not only the user ones).

Signed-off-by: Ilan Peer <[email protected]>
---
net/wireless/reg.c | 15 +++++----------
1 file changed, 5 insertions(+), 10 deletions(-)

diff --git a/net/wireless/reg.c b/net/wireless/reg.c
index e96251f..a34a534 100644
--- a/net/wireless/reg.c
+++ b/net/wireless/reg.c
@@ -552,6 +552,9 @@ reg_call_crda(struct regulatory_request *request)
{
if (call_crda(request->alpha2))
return REG_REQ_IGNORE;
+
+ queue_delayed_work(system_power_efficient_wq,
+ &reg_timeout, msecs_to_jiffies(3142));
return REG_REQ_OK;
}

@@ -1779,8 +1782,7 @@ static void reg_set_request_processed(void)
need_more_processing = true;
spin_unlock(&reg_requests_lock);

- if (lr->initiator == NL80211_REGDOM_SET_BY_USER)
- cancel_delayed_work(&reg_timeout);
+ cancel_delayed_work(&reg_timeout);

if (need_more_processing)
schedule_work(&reg_work);
@@ -2059,8 +2061,6 @@ static void reg_process_hint(struct regulatory_request *reg_request)
if (treatment == REG_REQ_IGNORE ||
treatment == REG_REQ_ALREADY_SET)
return;
- queue_delayed_work(system_power_efficient_wq,
- &reg_timeout, msecs_to_jiffies(3142));
return;
case NL80211_REGDOM_SET_BY_DRIVER:
if (!wiphy)
@@ -2485,7 +2485,6 @@ static void restore_regulatory_settings(bool reset_user)
char alpha2[2];
char world_alpha2[2];
struct reg_beacon *reg_beacon, *btmp;
- struct regulatory_request *reg_request, *tmp;
LIST_HEAD(tmp_reg_req_list);
struct cfg80211_registered_device *rdev;

@@ -2501,11 +2500,7 @@ static void restore_regulatory_settings(bool reset_user)
* settings.
*/
spin_lock(&reg_requests_lock);
- list_for_each_entry_safe(reg_request, tmp, &reg_requests_list, list) {
- if (reg_request->initiator != NL80211_REGDOM_SET_BY_USER)
- continue;
- list_move_tail(&reg_request->list, &tmp_reg_req_list);
- }
+ list_splice_tail_init(&reg_requests_list, &tmp_reg_req_list);
spin_unlock(&reg_requests_lock);

/* Clear beacon hints */
--
1.8.3.2

2015-02-15 12:26:10

by Peer, Ilan

Subject: RE: [PATCH v4 1/2] cfg80211: Add API to change the indoor regulatory setting

Hi Luis,

> >
> > This differs from the country setting that would potentially require
> > user space interaction and might invoke more complex flows. The flow
> > in this case is immediate, and does not require the somewhat complex
> > handling of country settings (it even complicates the flow
> > unnecessarily, with the REG_REQ_USER_HINT_HANDLED etc.).
>
> There's two things you should address then:
>
> 0) Try to mitigate the issue with the old userspace API if possible.
> This will enable old userspace to continue to work with the
> old API but also mitigate the issue you have described for which
> you are providing a new optimized solution for but that requires
> a new API.
>

Not sure I have a good solution for this. The problem here is that with the current API, the indoor setting will stick as long as a station interface is connected, although the indoor setting might no longer be true. For example when a station interface is connected to P2P GO (or a soft AP) and both devices are moving out of the indoor environment. The motivation for this patch was to move the full control of this setting to user space.

Any suggestion are welcomed.

> 1) With the new API have userspace be able to send to the kernel that
> userspace will do socket monitoring and because of this and the
> reasons you mentioned it will have more control over the environment
> boolean.
>

Ok. Will add such an API.

> > > > 2. Track the socket on which the indoor hint is issued, and reset
> > > > indoor setting if the socket was released. The motivation here is to
> > > > force a user space process that sets the indoor setting to constantly
> > > > monitor this setting, and be responsible to correctly toggling it,
> > > > so indoor operation will not be enabled uncontrolled.
> > >
> > > That seems to imply a new requirement for something that used to
> > > work, what having an option to set this requirement?
> >
> > (Sadly) I would not consider the previous implementation as working as
> > it would leave the regulatory core in a state that it considers to be
> > indoor although it is no longer true.
>
> Let's review the current implementation for indoor thing.
>
> We assume we're not indoor unless userspace sends a
> regulatory_hint_indoor_user(), this is with the user reg hint type set to
> NL80211_USER_REG_HINT_INDOOR.
> For country IEs we never trust the country IE data since it may contain bogus
> data, but we also end up ignoring the environment aspect too.
>
> If we disconnect we should be reseting the indoor setting to false.
> I just checked and restore_regulatory_settings() does set reg_is_indoor =
> false so if we are keeping the indoor setting I am missing something here, and
> it does indeed rather an issue that should be fixed. Where is the indoor
> setting being upkept?
>

This is generally true, but is not fully compatible with moving APs/P2P GOs, where you can be indoor, connect to an AP/P2P GO, and then move out of the indoor environment, while connected. Point is the being connected does not guarantee that the indoor setting is kept.

> > > > 3. Do not reset the indoor setting when restoring the regulatory
> > > > settings as it has nothing to do with CRDA or interface
> > > > disconnection.
> > >
> > > I disagree, if we disconnect we want the more restrictive setting
> > > and if we put the indoor setting out of general regulatory requests
> > > then we do want to reset this no?
> > >
> >
> > I do not think so. This setting is in the responsibility of the user
> > space daemon, so it should be the one controlling it.
>
> Right now the API requires sending the userspace regulatory hint and the
> kenrel should indeed reset to non-indoor upon disconnect, the later is an
> issue which should be fixed and what you introduce seems rather complex for
> something that should be fixed within the existing API.
>

The kernel does not have enough information to deduce indoor environment or not (as pointed above), only user space has the full information in this case, and should be responsible for controlling the setting.

> > A disconnection of the
> > station interface does not imply that we are no longer operating in an
> > indoor environment.
>
> I am confused you seem to be disagreeing with your above statement that
> says otherwise where you said that we leave the indoor setting in place if we
> disconnect. Can you clarify what you mean?
>

What I meant is that a disconnection of the station interface does not imply that we are no longer operating in an indoor environment. For example, in an enterprise environment, disconnections/reconnection would happen all the time due to roaming etc., but in all this cases that device indoor setting would continue to be true.

> Do you mean that you don't wish to fix it so that upon disconnect we never
> get an indoor setting and instead prefer this to be a matter of socket
> monitoring?
>

What I mean is that I want to make it the responsibility of the user space and not be depended on kernel wifi flows.

> > This also related to #2 above that tightens the responsibility of the
> > user space daemon controlling this setting.
>
> This smells like an optimization feature.
>
> > > > @@ -4981,7 +4983,8 @@ 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();
> > > > + is_indoor = !!info->attrs[NL80211_ATTR_REG_INDOOR];
> > > > + return regulatory_hint_indoor(is_indoor, info->snd_portid);
> > >
> > > So we break old functionality ? No bueno.
> > >
> >
> > no bueno, pero necesario.
> >
> > Previous functionality was not good in terms of regulatory compliance
> > (once indoor was set there was no way to undo it ...),
>
> As I noted restore_regulatory_settings() does set reg_is_indoor = false so
> what is missing?
>

Hope I clarified this above.

> > and lacked proper tracking of this setting by user space.
>
> This seems like an optimization that you want and if you do wish to add that I'd
> recommend NL80211_USER_REG_HINT_INDOOR_SOCK_MONITOR
> and document as a feature which allows the interpretation you noted but you
> must first address the issue with the existing API as I'm failing to see where we
> fail to reset the regulatory setting for indoor.
>

Sure. I'll extend this with requested flag or similar.

Thanks again Luis (and sorry for the delayed response).

Ilan.

2015-02-20 01:03:16

by Valdis Klētnieks

Subject: Re: [PATCH v4 1/2] cfg80211: Add API to change the indoor regulatory setting

On Fri, 20 Feb 2015 01:53:44 +0100, "Luis R. Rodriguez" said:
> Wider community:
>
> anyone aware of any *need* in the kernel to know whether one is indoor or
> not on a device running Linux other than wifi? Clearly it should be something
> that might be of interest to at least other RF devices, so that is at least
> one possibilty to consider already, but what else?

I can think of a lot of reasons for the kernel to make indoor/outdoor
status available to userspace, but am coming up empty why the kernel itself
should care....

Attachments:

2015-02-06 23:58:05

by Luis Chamberlain

Subject: Re: [PATCH v4 1/2] cfg80211: Add API to change the indoor regulatory setting

On Mon, Feb 02, 2015 at 09:59:25AM -0500, Ilan Peer wrote:
> As the operating environment of a device can change, add
> API for user space to indicate a change of indoor settings.
> In addition modify the handling of the indoor processing as
> follows:
>
> 1. Directly update the indoor setting without wrapping it as a
> regulatory request.

Why? We have this present as part of the request as it was part
of the country IE, that's all. I can see for instance then that
it is wise to require the supplicant for instance to be still
connected to the AP and the socket tracking as solution to the
problem. Does that summarize the logic ?

> 2. Track the socket on which the indoor hint is issued, and reset
> indoor setting if the socket was released. The motivation here is to
> force a user space process that sets the indoor setting to constantly
> monitor this setting, and be responsible to correctly toggling it,
> so indoor operation will not be enabled uncontrolled.

That seems to imply a new requirement for something that used to work,
what having an option to set this requirement?

> 3. Do not reset the indoor setting when restoring the regulatory
> settings as it has nothing to do with CRDA or interface
> disconnection.

I disagree, if we disconnect we want the more restrictive setting
and if we put the indoor setting out of general regulatory requests
then we do want to reset this no?

> Signed-off-by: Ilan Peer <[email protected]>
> Signed-off-by: ArikX Nemtsov <[email protected]>
> ---
> include/uapi/linux/nl80211.h | 5 +++
> net/wireless/nl80211.c | 10 +++++-
> net/wireless/reg.c | 80 +++++++++++++++++++++++++-------------------
> net/wireless/reg.h | 15 ++++++++-
> 4 files changed, 73 insertions(+), 37 deletions(-)
>
> diff --git a/include/uapi/linux/nl80211.h b/include/uapi/linux/nl80211.h
> index 68b294e..d80edcc 100644
> --- a/include/uapi/linux/nl80211.h
> +++ b/include/uapi/linux/nl80211.h
> @@ -1739,6 +1739,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 +2110,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 454d7a0..e78b096 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 */
> @@ -4955,6 +4956,7 @@ 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;
>
> /*
> @@ -4981,7 +4983,8 @@ 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();
> + is_indoor = !!info->attrs[NL80211_ATTR_REG_INDOOR];
> + return regulatory_hint_indoor(is_indoor, info->snd_portid);

So we break old functionality ? No bueno.

Luis

2015-02-22 07:57:14

by Peer, Ilan

Subject: RE: [PATCH v4 1/2] cfg80211: Add API to change the indoor regulatory setting

Hi Luis,

Can you please have a look at version 5 of the patch? I think that I addressed some concerns there.
Regardless, I'll send another version that clarifies the optimization in the commit message.

Thanks again,

Ilan.

> -----Original Message-----
> From: Luis R. Rodriguez [mailto:[email protected]]
> Sent: Friday, February 20, 2015 02:54
> To: Peer, Ilan
> Cc: [email protected]; ArikX Nemtsov; linux-
> [email protected]
> Subject: Re: [PATCH v4 1/2] cfg80211: Add API to change the indoor
> regulatory setting
>
> Wider community:
>
> anyone aware of any *need* in the kernel to know whether one is indoor or
> not on a device running Linux other than wifi? Clearly it should be something
> that might be of interest to at least other RF devices, so that is at least one
> possibilty to consider already, but what else?
>
> On Sun, Feb 15, 2015 at 12:26:04PM +0000, Peer, Ilan wrote:
> > Hi Luis,
> >
> > > >
> > > > This differs from the country setting that would potentially
> > > > require user space interaction and might invoke more complex
> > > > flows. The flow in this case is immediate, and does not require
> > > > the somewhat complex handling of country settings (it even
> > > > complicates the flow unnecessarily, with the
> REG_REQ_USER_HINT_HANDLED etc.).
> > >
> > > There's two things you should address then:
> > >
> > > 0) Try to mitigate the issue with the old userspace API if possible.
> > > This will enable old userspace to continue to work with the
> > > old API but also mitigate the issue you have described for which
> > > you are providing a new optimized solution for but that requires
> > > a new API.
> > >
> >
> > Not sure I have a good solution for this. The problem here is that
> > with the current API, the indoor setting will stick as long as a
> > station interface is connected, although the indoor setting might no
> > longer be true. For example when a station interface is connected to
> > P2P GO (or a soft AP) and both devices are moving out of the indoor
> > environment. The motivation for this patch was to move the full control of
> this setting to user space.
> >
> > Any suggestion are welcomed.
>
> Why not just clear it upon disconnect for the old case? It seems that would be
> better if that is better regulatory wise.
>

Yep. This is what I did in version 5. Did you get a chance to look at it?

> >
> > > 1) With the new API have userspace be able to send to the kernel that
> > > userspace will do socket monitoring and because of this and the
> > > reasons you mentioned it will have more control over the environment
> > > boolean.
> > >
> >
> > Ok. Will add such an API.
>
> Great.
>
> > > > > > 2. Track the socket on which the indoor hint is issued, and reset
> > > > > > indoor setting if the socket was released. The motivation here is to
> > > > > > force a user space process that sets the indoor setting to
> constantly
> > > > > > monitor this setting, and be responsible to correctly toggling it,
> > > > > > so indoor operation will not be enabled uncontrolled.
> > > > >
> > > > > That seems to imply a new requirement for something that used to
> > > > > work, what having an option to set this requirement?
> > > >
> > > > (Sadly) I would not consider the previous implementation as
> > > > working as it would leave the regulatory core in a state that it
> > > > considers to be indoor although it is no longer true.
> > >
> > > Let's review the current implementation for indoor thing.
> > >
> > > We assume we're not indoor unless userspace sends a
> > > regulatory_hint_indoor_user(), this is with the user reg hint type
> > > set to NL80211_USER_REG_HINT_INDOOR.
> > > For country IEs we never trust the country IE data since it may
> > > contain bogus data, but we also end up ignoring the environment aspect
> too.
> > >
> > > If we disconnect we should be reseting the indoor setting to false.
> > > I just checked and restore_regulatory_settings() does set
> > > reg_is_indoor = false so if we are keeping the indoor setting I am
> > > missing something here, and it does indeed rather an issue that
> > > should be fixed. Where is the indoor setting being upkept?
> > >
> >
> > This is generally true, but is not fully compatible with moving
> > APs/P2P GOs, where you can be indoor, connect to an AP/P2P GO, and
> > then move out of the indoor environment, while connected. Point is the
> > being connected does not guarantee that the indoor setting is kept.
>
> Ah so then the above description does satisfy the concern you stated.
> What you are describing *now* is a dynamic environment new requirement.
>

Not really a new requirement, but one that I missed in the original patch. Sorry.

> That's a very different picture than what you originally stated and your commit
> message does not clarify what worked and what really are the issue well. As it
> stands your changes will be an ammendment to help with dynamic
> environment setting, and it will also enable environment options to remain
> post disconnect.
>

I can clarify this in the commit message.

> > > > > > 3. Do not reset the indoor setting when restoring the regulatory
> > > > > > settings as it has nothing to do with CRDA or interface
> > > > > > disconnection.
> > > > >
> > > > > I disagree, if we disconnect we want the more restrictive
> > > > > setting and if we put the indoor setting out of general
> > > > > regulatory requests then we do want to reset this no?
> > > > >
> > > >
> > > > I do not think so. This setting is in the responsibility of the
> > > > user space daemon, so it should be the one controlling it.
> > >
> > > Right now the API requires sending the userspace regulatory hint and
> > > the kenrel should indeed reset to non-indoor upon disconnect, the
> > > later is an issue which should be fixed and what you introduce seems
> > > rather complex for something that should be fixed within the existing API.
> > >
> >
> > The kernel does not have enough information to deduce indoor
> > environment or not (as pointed above), only user space has the full
> > information in this case, and should be responsible for controlling the
> setting.
>
> Do you forsee other sources to override the socket information? What about
> a disconnect, will that kill the socket setting too? What about a future feature,
> say something generic in the kernel?
>

I modified this in version 5 of the patch, so unless user space clearly states that it owns the socket,
the indoor setting would be cleared upon disconnection. Otherwise, as long as the socket is connected,
the reg core assumes that the setting is controlled by user space and does not alter it.

> > > > A disconnection of the
> > > > station interface does not imply that we are no longer operating
> > > > in an indoor environment.
> > >
> > > I am confused you seem to be disagreeing with your above statement
> > > that says otherwise where you said that we leave the indoor setting
> > > in place if we disconnect. Can you clarify what you mean?
> > >
> >
> > What I meant is that a disconnection of the station interface does not
> > imply that we are no longer operating in an indoor environment.
>
> Sure.
>
> > For example, in an
> > enterprise environment, disconnections/reconnection would happen all
> > the time due to roaming etc., but in all this cases that device indoor
> > setting would continue to be true.
>
> Sure.
>
> > > Do you mean that you don't wish to fix it so that upon disconnect we
> > > never get an indoor setting and instead prefer this to be a matter
> > > of socket monitoring?
> > >
> >
> > What I mean is that I want to make it the responsibility of the user
> > space and not be depended on kernel wifi flows.
>
> OK
>
> > > > This also related to #2 above that tightens the responsibility of
> > > > the user space daemon controlling this setting.
> > >
> > > This smells like an optimization feature.
> > >
> > > > > > @@ -4981,7 +4983,8 @@ 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();
> > > > > > + is_indoor = !!info-
> >attrs[NL80211_ATTR_REG_INDOOR];
> > > > > > + return regulatory_hint_indoor(is_indoor, info-
> >snd_portid);
> > > > >
> > > > > So we break old functionality ? No bueno.
> > > > >
> > > >
> > > > no bueno, pero necesario.
> > > >
> > > > Previous functionality was not good in terms of regulatory
> > > > compliance (once indoor was set there was no way to undo it ...),
> > >
> > > As I noted restore_regulatory_settings() does set reg_is_indoor =
> > > false so what is missing?
> > >
> >
> > Hope I clarified this above.
>
> You did and it confirms my suspicion that this is an optimization.
> You need to sell it as such and most importantly document this very well.
>

Will do.

> > > > and lacked proper tracking of this setting by user space.
> > >
> > > This seems like an optimization that you want and if you do wish to
> > > add that I'd recommend
> NL80211_USER_REG_HINT_INDOOR_SOCK_MONITOR
> > > and document as a feature which allows the interpretation you noted
> > > but you must first address the issue with the existing API as I'm
> > > failing to see where we fail to reset the regulatory setting for indoor.
> > >
> >
> > Sure. I'll extend this with requested flag or similar.
> >
> > Thanks again Luis (and sorry for the delayed response).
>
> Likewise,

:)

>
> Luis

2015-02-08 09:10:07

by Peer, Ilan

Subject: RE: [PATCH v4 1/2] cfg80211: Add API to change the indoor regulatory setting

Hi Luis,

> -----Original Message-----
> From: Luis R. Rodriguez [mailto:[email protected]]
> Sent: Saturday, February 07, 2015 01:58
> To: Peer, Ilan
> Cc: [email protected]; ArikX Nemtsov
> Subject: Re: [PATCH v4 1/2] cfg80211: Add API to change the indoor
> regulatory setting
>
> On Mon, Feb 02, 2015 at 09:59:25AM -0500, Ilan Peer wrote:
> > As the operating environment of a device can change, add API for user
> > space to indicate a change of indoor settings.
> > In addition modify the handling of the indoor processing as
> > follows:
> >
> > 1. Directly update the indoor setting without wrapping it as a
> > regulatory request.
>
> Why? We have this present as part of the request as it was part of the
> country IE, that's all. I can see for instance then that it is wise to require the
> supplicant for instance to be still connected to the AP and the socket tracking
> as solution to the problem. Does that summarize the logic ?
>

This differs from the country setting that would potentially require user space interaction and might invoke more complex flows. The flow in this case is immediate, and does not require the somewhat complex handling of country settings (it even complicates the flow unnecessarily, with the REG_REQ_USER_HINT_HANDLED etc.).

> > 2. Track the socket on which the indoor hint is issued, and reset
> > indoor setting if the socket was released. The motivation here is to
> > force a user space process that sets the indoor setting to constantly
> > monitor this setting, and be responsible to correctly toggling it,
> > so indoor operation will not be enabled uncontrolled.
>
> That seems to imply a new requirement for something that used to work,
> what having an option to set this requirement?

(Sadly) I would not consider the previous implementation as working as it would leave the regulatory core in a state that it considers to be indoor although it is no longer true.

>
> > 3. Do not reset the indoor setting when restoring the regulatory
> > settings as it has nothing to do with CRDA or interface
> > disconnection.
>
> I disagree, if we disconnect we want the more restrictive setting and if we
> put the indoor setting out of general regulatory requests then we do want to
> reset this no?
>

I do not think so. This setting is in the responsibility of the user space daemon, so it should be the one controlling it. A disconnection of the station interface does not imply that we are no longer operating in an indoor environment. This also related to #2 above that tightens the responsibility of the user space daemon controlling this setting.

> > Signed-off-by: Ilan Peer <[email protected]>
> > Signed-off-by: ArikX Nemtsov <[email protected]>
> > ---
> > include/uapi/linux/nl80211.h | 5 +++
> > net/wireless/nl80211.c | 10 +++++-
> > net/wireless/reg.c | 80 +++++++++++++++++++++++++----------------
> ---
> > net/wireless/reg.h | 15 ++++++++-
> > 4 files changed, 73 insertions(+), 37 deletions(-)
> >
> > diff --git a/include/uapi/linux/nl80211.h
> > b/include/uapi/linux/nl80211.h index 68b294e..d80edcc 100644
> > --- a/include/uapi/linux/nl80211.h
> > +++ b/include/uapi/linux/nl80211.h
> > @@ -1739,6 +1739,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 +2110,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
> > 454d7a0..e78b096 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 */
> > @@ -4955,6 +4956,7 @@ 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;
> >
> > /*
> > @@ -4981,7 +4983,8 @@ 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();
> > + is_indoor = !!info->attrs[NL80211_ATTR_REG_INDOOR];
> > + return regulatory_hint_indoor(is_indoor, info->snd_portid);
>
> So we break old functionality ? No bueno.
>

no bueno, pero necesario.

Previous functionality was not good in terms of regulatory compliance (once indoor was set there was no way to undo it ...), and lacked proper tracking of this setting by user space.

Regards,

Ilan.

2015-02-10 20:00:30

by Luis Chamberlain

Subject: Re: [PATCH v4 1/2] cfg80211: Add API to change the indoor regulatory setting

On Sun, Feb 08, 2015 at 09:10:01AM +0000, Peer, Ilan wrote:
> Hi Luis,
>
> > -----Original Message-----
> > From: Luis R. Rodriguez [mailto:[email protected]]
> > Sent: Saturday, February 07, 2015 01:58
> > To: Peer, Ilan
> > Cc: [email protected]; ArikX Nemtsov
> > Subject: Re: [PATCH v4 1/2] cfg80211: Add API to change the indoor
> > regulatory setting
> >
> > On Mon, Feb 02, 2015 at 09:59:25AM -0500, Ilan Peer wrote:
> > > As the operating environment of a device can change, add API for user
> > > space to indicate a change of indoor settings.
> > > In addition modify the handling of the indoor processing as
> > > follows:
> > >
> > > 1. Directly update the indoor setting without wrapping it as a
> > > regulatory request.
> >
> > Why? We have this present as part of the request as it was part of the
> > country IE, that's all. I can see for instance then that it is wise to require the
> > supplicant for instance to be still connected to the AP and the socket tracking
> > as solution to the problem. Does that summarize the logic ?
> >
>
> This differs from the country setting that would potentially require user
> space interaction and might invoke more complex flows. The flow in this case
> is immediate, and does not require the somewhat complex handling of country
> settings (it even complicates the flow unnecessarily, with the
> REG_REQ_USER_HINT_HANDLED etc.).

There's two things you should address then:

0) Try to mitigate the issue with the old userspace API if possible.
This will enable old userspace to continue to work with the
old API but also mitigate the issue you have described for which
you are providing a new optimized solution for but that requires
a new API.

1) With the new API have userspace be able to send to the kernel that
userspace will do socket monitoring and because of this and the
reasons you mentioned it will have more control over the environment
boolean.

> > > 2. Track the socket on which the indoor hint is issued, and reset
> > > indoor setting if the socket was released. The motivation here is to
> > > force a user space process that sets the indoor setting to constantly
> > > monitor this setting, and be responsible to correctly toggling it,
> > > so indoor operation will not be enabled uncontrolled.
> >
> > That seems to imply a new requirement for something that used to work,
> > what having an option to set this requirement?
>
> (Sadly) I would not consider the previous implementation as working as it
> would leave the regulatory core in a state that it considers to be indoor
> although it is no longer true.

Let's review the current implementation for indoor thing.

We assume we're not indoor unless userspace sends a
regulatory_hint_indoor_user(), this is with the user
reg hint type set to NL80211_USER_REG_HINT_INDOOR.
For country IEs we never trust the country IE data
since it may contain bogus data, but we also end up
ignoring the environment aspect too.

If we disconnect we should be reseting the indoor setting to false.
I just checked and restore_regulatory_settings() does set reg_is_indoor = false
so if we are keeping the indoor setting I am missing something here,
and it does indeed rather an issue that should be fixed. Where is
the indoor setting being upkept?

> > > 3. Do not reset the indoor setting when restoring the regulatory
> > > settings as it has nothing to do with CRDA or interface
> > > disconnection.
> >
> > I disagree, if we disconnect we want the more restrictive setting and if we
> > put the indoor setting out of general regulatory requests then we do want to
> > reset this no?
> >
>
> I do not think so. This setting is in the responsibility of the user space
> daemon, so it should be the one controlling it.

Right now the API requires sending the userspace regulatory hint and the
kenrel should indeed reset to non-indoor upon disconnect, the later is an issue
which should be fixed and what you introduce seems rather complex for
something that should be fixed within the existing API.

> A disconnection of the
> station interface does not imply that we are no longer operating in an indoor
> environment.

I am confused you seem to be disagreeing with your above statement that says
otherwise where you said that we leave the indoor setting in place if we
disconnect. Can you clarify what you mean?

Do you mean that you don't wish to fix it so that upon disconnect we
never get an indoor setting and instead prefer this to be a matter of
socket monitoring?

> This also related to #2 above that tightens the responsibility
> of the user space daemon controlling this setting.

This smells like an optimization feature.

> > > @@ -4981,7 +4983,8 @@ 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();
> > > + is_indoor = !!info->attrs[NL80211_ATTR_REG_INDOOR];
> > > + return regulatory_hint_indoor(is_indoor, info->snd_portid);
> >
> > So we break old functionality ? No bueno.
> >
>
> no bueno, pero necesario.
>
> Previous functionality was not good in terms of regulatory compliance (once
> indoor was set there was no way to undo it ...),

As I noted restore_regulatory_settings() does set reg_is_indoor = false
so what is missing?

> and lacked proper tracking of this setting by user space.

This seems like an optimization that you want and if you do wish
to add that I'd recommend NL80211_USER_REG_HINT_INDOOR_SOCK_MONITOR
and document as a feature which allows the interpretation you noted
but you must first address the issue with the existing API as I'm
failing to see where we fail to reset the regulatory setting for
indoor.

Luis

2015-02-20 15:02:42

by Jonathan Bither

Subject: Re: [PATCH v4 1/2] cfg80211: Add API to change the indoor regulatory setting

On 02/19/2015 08:03 PM, [email protected] wrote:
> On Fri, 20 Feb 2015 01:53:44 +0100, "Luis R. Rodriguez" said:
>> Wider community:
>>
>> anyone aware of any *need* in the kernel to know whether one is indoor or
>> not on a device running Linux other than wifi? Clearly it should be something
>> that might be of interest to at least other RF devices, so that is at least
>> one possibilty to consider already, but what else?
>
> I can think of a lot of reasons for the kernel to make indoor/outdoor
> status available to userspace, but am coming up empty why the kernel itself
> should care....
>
That made me try to think of the possible uses for such a variable but I
imagine that they can all be handled by userspace as well.
Powering down GPS chips for energy consumption.
Applying filters to cameras for fluorescent lights.

2015-02-20 00:53:47

by Luis Chamberlain

Subject: Re: [PATCH v4 1/2] cfg80211: Add API to change the indoor regulatory setting

Wider community:

anyone aware of any *need* in the kernel to know whether one is indoor or
not on a device running Linux other than wifi? Clearly it should be something
that might be of interest to at least other RF devices, so that is at least
one possibilty to consider already, but what else?

On Sun, Feb 15, 2015 at 12:26:04PM +0000, Peer, Ilan wrote:
> Hi Luis,
>
> > >
> > > This differs from the country setting that would potentially require
> > > user space interaction and might invoke more complex flows. The flow
> > > in this case is immediate, and does not require the somewhat complex
> > > handling of country settings (it even complicates the flow
> > > unnecessarily, with the REG_REQ_USER_HINT_HANDLED etc.).
> >
> > There's two things you should address then:
> >
> > 0) Try to mitigate the issue with the old userspace API if possible.
> > This will enable old userspace to continue to work with the
> > old API but also mitigate the issue you have described for which
> > you are providing a new optimized solution for but that requires
> > a new API.
> >
>
> Not sure I have a good solution for this. The problem here is that with the
> current API, the indoor setting will stick as long as a station interface is
> connected, although the indoor setting might no longer be true. For example
> when a station interface is connected to P2P GO (or a soft AP) and both
> devices are moving out of the indoor environment. The motivation for this
> patch was to move the full control of this setting to user space.
>
> Any suggestion are welcomed.

Why not just clear it upon disconnect for the old case? It seems that would
be better if that is better regulatory wise.

>
> > 1) With the new API have userspace be able to send to the kernel that
> > userspace will do socket monitoring and because of this and the
> > reasons you mentioned it will have more control over the environment
> > boolean.
> >
>
> Ok. Will add such an API.

Great.

> > > > > 2. Track the socket on which the indoor hint is issued, and reset
> > > > > indoor setting if the socket was released. The motivation here is to
> > > > > force a user space process that sets the indoor setting to constantly
> > > > > monitor this setting, and be responsible to correctly toggling it,
> > > > > so indoor operation will not be enabled uncontrolled.
> > > >
> > > > That seems to imply a new requirement for something that used to
> > > > work, what having an option to set this requirement?
> > >
> > > (Sadly) I would not consider the previous implementation as working as
> > > it would leave the regulatory core in a state that it considers to be
> > > indoor although it is no longer true.
> >
> > Let's review the current implementation for indoor thing.
> >
> > We assume we're not indoor unless userspace sends a
> > regulatory_hint_indoor_user(), this is with the user reg hint type set to
> > NL80211_USER_REG_HINT_INDOOR.
> > For country IEs we never trust the country IE data since it may contain bogus
> > data, but we also end up ignoring the environment aspect too.
> >
> > If we disconnect we should be reseting the indoor setting to false.
> > I just checked and restore_regulatory_settings() does set reg_is_indoor =
> > false so if we are keeping the indoor setting I am missing something here, and
> > it does indeed rather an issue that should be fixed. Where is the indoor
> > setting being upkept?
> >
>
> This is generally true, but is not fully compatible with moving APs/P2P GOs,
> where you can be indoor, connect to an AP/P2P GO, and then move out of the
> indoor environment, while connected. Point is the being connected does not
> guarantee that the indoor setting is kept.

Ah so then the above description does satisfy the concern you stated.
What you are describing *now* is a dynamic environment new requirement.

That's a very different picture than what you originally stated and
your commit message does not clarify what worked and what really are
the issue well. As it stands your changes will be an ammendment to
help with dynamic environment setting, and it will also enable environment
options to remain post disconnect.

> > > > > 3. Do not reset the indoor setting when restoring the regulatory
> > > > > settings as it has nothing to do with CRDA or interface
> > > > > disconnection.
> > > >
> > > > I disagree, if we disconnect we want the more restrictive setting
> > > > and if we put the indoor setting out of general regulatory requests
> > > > then we do want to reset this no?
> > > >
> > >
> > > I do not think so. This setting is in the responsibility of the user
> > > space daemon, so it should be the one controlling it.
> >
> > Right now the API requires sending the userspace regulatory hint and the
> > kenrel should indeed reset to non-indoor upon disconnect, the later is an
> > issue which should be fixed and what you introduce seems rather complex for
> > something that should be fixed within the existing API.
> >
>
> The kernel does not have enough information to deduce indoor environment or
> not (as pointed above), only user space has the full information in this
> case, and should be responsible for controlling the setting.

Do you forsee other sources to override the socket information? What about
a disconnect, will that kill the socket setting too? What about a future
feature, say something generic in the kernel?

> > > A disconnection of the
> > > station interface does not imply that we are no longer operating in an
> > > indoor environment.
> >
> > I am confused you seem to be disagreeing with your above statement that
> > says otherwise where you said that we leave the indoor setting in place if we
> > disconnect. Can you clarify what you mean?
> >
>
> What I meant is that a disconnection of the station interface does not imply
> that we are no longer operating in an indoor environment.

Sure.

> For example, in an
> enterprise environment, disconnections/reconnection would happen all the time
> due to roaming etc., but in all this cases that device indoor setting would
> continue to be true.

Sure.

> > Do you mean that you don't wish to fix it so that upon disconnect we never
> > get an indoor setting and instead prefer this to be a matter of socket
> > monitoring?
> >
>
> What I mean is that I want to make it the responsibility of the user space
> and not be depended on kernel wifi flows.

OK

> > > This also related to #2 above that tightens the responsibility of the
> > > user space daemon controlling this setting.
> >
> > This smells like an optimization feature.
> >
> > > > > @@ -4981,7 +4983,8 @@ 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();
> > > > > + is_indoor = !!info->attrs[NL80211_ATTR_REG_INDOOR];
> > > > > + return regulatory_hint_indoor(is_indoor, info->snd_portid);
> > > >
> > > > So we break old functionality ? No bueno.
> > > >
> > >
> > > no bueno, pero necesario.
> > >
> > > Previous functionality was not good in terms of regulatory compliance
> > > (once indoor was set there was no way to undo it ...),
> >
> > As I noted restore_regulatory_settings() does set reg_is_indoor = false so
> > what is missing?
> >
>
> Hope I clarified this above.

You did and it confirms my suspicion that this is an optimization.
You need to sell it as such and most importantly document this
very well.

> > > and lacked proper tracking of this setting by user space.
> >
> > This seems like an optimization that you want and if you do wish to add that I'd
> > recommend NL80211_USER_REG_HINT_INDOOR_SOCK_MONITOR
> > and document as a feature which allows the interpretation you noted but you
> > must first address the issue with the existing API as I'm failing to see where we
> > fail to reset the regulatory setting for indoor.
> >
>
> Sure. I'll extend this with requested flag or similar.
>
> Thanks again Luis (and sorry for the delayed response).

Likewise,

Luis

2015-02-20 15:58:01

by Luis Chamberlain

Subject: Re: [PATCH v4 1/2] cfg80211: Add API to change the indoor regulatory setting

On Fri, Feb 20, 2015 at 7:02 AM, Jonathan Bither <[email protected]> wrote:
> On 02/19/2015 08:03 PM, [email protected] wrote:
>>
>> On Fri, 20 Feb 2015 01:53:44 +0100, "Luis R. Rodriguez" said:
>>>
>>> Wider community:
>>>
>>> anyone aware of any *need* in the kernel to know whether one is indoor or
>>> not on a device running Linux other than wifi? Clearly it should be
>>> something
>>> that might be of interest to at least other RF devices, so that is at
>>> least
>>> one possibilty to consider already, but what else?
>>
>>
>> I can think of a lot of reasons for the kernel to make indoor/outdoor
>> status available to userspace, but am coming up empty why the kernel
>> itself
>> should care....
>>
> That made me try to think of the possible uses for such a variable but I
> imagine that they can all be handled by userspace as well.
> Powering down GPS chips for energy consumption.
> Applying filters to cameras for fluorescent lights.

These days such devices are bundled with all these devices typically
aiming towards single-chip, ie one devices with multiple functions and
sharing as much as possible, as such it should be possible
architecturally to share this. So far then it seems this can just live
under its hood under cfg80211 then.

Luis

2015-02-20 02:09:27

by Luis Chamberlain

Subject: Re: [PATCH v4 1/2] cfg80211: Add API to change the indoor regulatory setting

On Thu, Feb 19, 2015 at 5:03 PM, <[email protected]> wrote:
> On Fri, 20 Feb 2015 01:53:44 +0100, "Luis R. Rodriguez" said:
>> Wider community:
>>
>> anyone aware of any *need* in the kernel to know whether one is indoor or
>> not on a device running Linux other than wifi? Clearly it should be something
>> that might be of interest to at least other RF devices, so that is at least
>> one possibilty to consider already, but what else?
>
> I can think of a lot of reasons for the kernel to make indoor/outdoor
> status available to userspace, but am coming up empty why the kernel itself
> should care....

Some devices enable regulatory control through the kernel, in such
cases the wireless-regdb is used and there is an INDOOR flag that
enables / disables specific functionality.

Luis