linux-wireless.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 00/19] cfg80211: regulatory updates
@ 2013-11-05 17:17 Luis R. Rodriguez
  2013-11-05 17:17 ` [PATCH 01/19] cfg80211: enforce disabling channels by custom or strict settings Luis R. Rodriguez
                   ` (19 more replies)
  0 siblings, 20 replies; 30+ messages in thread
From: Luis R. Rodriguez @ 2013-11-05 17:17 UTC (permalink / raw)
  To: johannes; +Cc: linux-wireless, Luis R. Rodriguez

This series deals with 3 things:

  a) Three enhancements for regulatory
  b) Make reg_process_hint() easier to follow
  c) Make set_regdom() easier to follow

The enhancements in a) are to help ensure a combination of
custom and strict regulatory settings are respected but also
address original design consideations but that we had overlooked.
The third enhancement helps avoid possible abuse on memory
pressure by userspace by invalidating requests early.

The work on b) and c) come from the realization that while the
existing code works its really hard to follow as you have to
consider two different state machine consideration at once.
I've decided to create a compromise on making the code easier
to follow by increasing code size by splitting up each case
into its own path, this lets the reader only focus on each
individual type of request individually. Code is still shared
when possible.

I've tested all these changes with mac80211_hwsim regtest for
all regtetest options before and after applying these changes.
I'm pretty confident on these changes, I suspect this actually
also ends up addressing a few corner cases previously not
covered.

Luis R. Rodriguez (19):
  cfg80211: enforce disabling channels by custom or strict settings
  cfg80211: force WIPHY_FLAG_CUSTOM_REGULATORY on
    wiphy_apply_custom_regulatory()
  cfg80211: check regulatory request alpha2 early
  cfg80211: remove second argument from reg_process_hint()
  cfg80211: processing core regulatory hints on its own
  cfg80211: process user regulatory requests on its own
  cfg80211: process driver regulatory requests on its own
  cfg80211: process country IE regulatory requests on their own
  cfg80211: process non country IE conflicting first
  cfg80211: add helper for kfree'ing last_request
  cfg80211: add helper for kfree'ing and assigning last_request
  cfg80211: add helper for calling CRDA
  cfg80211: allow only the core to request to update the world regdom
  cfg80211: move core reg_notfier() check to source
  cfg80211: pass the last_request to __set_regdom()
  cfg80211: set core regulatory updates on its own
  cfg80211: set user regulatory updates on its own
  cfg80211: set driver regulatory updates on its own
  cfg80211: rename __set_regdom() to reg_set_rd_country_ie()

 include/net/cfg80211.h |   7 +-
 net/wireless/nl80211.c |   3 +
 net/wireless/reg.c     | 681 ++++++++++++++++++++++++++++++-------------------
 net/wireless/reg.h     |   1 +
 4 files changed, 422 insertions(+), 270 deletions(-)

-- 
1.8.4.rc3


^ permalink raw reply	[flat|nested] 30+ messages in thread

* [PATCH 01/19] cfg80211: enforce disabling channels by custom or strict settings
  2013-11-05 17:17 [PATCH 00/19] cfg80211: regulatory updates Luis R. Rodriguez
@ 2013-11-05 17:17 ` Luis R. Rodriguez
  2013-11-06 10:15   ` Johannes Berg
  2013-11-05 17:18 ` [PATCH 02/19] cfg80211: force WIPHY_FLAG_CUSTOM_REGULATORY on wiphy_apply_custom_regulatory() Luis R. Rodriguez
                   ` (18 subsequent siblings)
  19 siblings, 1 reply; 30+ messages in thread
From: Luis R. Rodriguez @ 2013-11-05 17:17 UTC (permalink / raw)
  To: johannes; +Cc: linux-wireless, Luis R. Rodriguez

If a custom regulatory domain is passed and if a rule for a
channel indicates it should be disabled that channel should
always remain disabled as per its documentation and design.

Likewise if WIPHY_FLAG_STRICT_REGULATORY flag is set and a
regulatory_hint() is issued if a channel is disabled that
channel should remain disabled.

Without this change only drivers that set the _orig flags
appropriately on their own would ensure disallowed channels
remaind disabled. This helps drivers save code by relying on
the APIS provided to entrust channels that should not be enabled
be respected by only having to use wiphy_apply_custom_regulatory()
or regulatory_hint() with the WIPHY_FLAG_STRICT_REGULATORY set.

If wiphy_apply_custom_regulatory() is used together with
WIPHY_FLAG_STRICT_REGULATORY and a regulatory_hint() issued
later, the incoming regulatory domain can override previously
set _orig parameters from the initial custom regulatory
setting.

Signed-off-by: Luis R. Rodriguez <mcgrof@do-not-panic.com>
---
 net/wireless/reg.c | 16 +++++++++++++---
 1 file changed, 13 insertions(+), 3 deletions(-)

diff --git a/net/wireless/reg.c b/net/wireless/reg.c
index 89b4664..999b4bb7 100644
--- a/net/wireless/reg.c
+++ b/net/wireless/reg.c
@@ -847,8 +847,18 @@ static void handle_channel(struct wiphy *wiphy,
 		    PTR_ERR(reg_rule) == -ERANGE)
 			return;
 
-		REG_DBG_PRINT("Disabling freq %d MHz\n", chan->center_freq);
-		chan->flags |= IEEE80211_CHAN_DISABLED;
+		if (lr->initiator == NL80211_REGDOM_SET_BY_DRIVER &&
+		    request_wiphy && request_wiphy == wiphy &&
+		    request_wiphy->flags & WIPHY_FLAG_STRICT_REGULATORY) {
+			REG_DBG_PRINT("Disabling freq %d MHz for good\n",
+				      chan->center_freq);
+			chan->flags = chan->orig_flags |=
+				IEEE80211_CHAN_DISABLED;
+		} else {
+			REG_DBG_PRINT("Disabling freq %d MHz\n",
+				      chan->center_freq);
+			chan->flags |= IEEE80211_CHAN_DISABLED;
+		}
 		return;
 	}
 
@@ -1251,7 +1261,7 @@ static void handle_channel_custom(struct wiphy *wiphy,
 	if (IS_ERR(reg_rule)) {
 		REG_DBG_PRINT("Disabling freq %d MHz as custom regd has no rule that fits it\n",
 			      chan->center_freq);
-		chan->flags = IEEE80211_CHAN_DISABLED;
+		chan->flags = chan->orig_flags |= IEEE80211_CHAN_DISABLED;
 		return;
 	}
 
-- 
1.8.4.rc3


^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [PATCH 02/19] cfg80211: force WIPHY_FLAG_CUSTOM_REGULATORY on wiphy_apply_custom_regulatory()
  2013-11-05 17:17 [PATCH 00/19] cfg80211: regulatory updates Luis R. Rodriguez
  2013-11-05 17:17 ` [PATCH 01/19] cfg80211: enforce disabling channels by custom or strict settings Luis R. Rodriguez
@ 2013-11-05 17:18 ` Luis R. Rodriguez
  2013-11-05 17:18 ` [PATCH 03/19] cfg80211: check regulatory request alpha2 early Luis R. Rodriguez
                   ` (17 subsequent siblings)
  19 siblings, 0 replies; 30+ messages in thread
From: Luis R. Rodriguez @ 2013-11-05 17:18 UTC (permalink / raw)
  To: johannes; +Cc: linux-wireless, Luis R. Rodriguez

wiphy_apply_custom_regulatory() implies WIPHY_FLAG_CUSTOM_REGULATORY
but we never enforced it, do that now and warn if the driver
didn't set it. All drivers should be following this today already.

Having WIPHY_FLAG_CUSTOM_REGULATORY does not however mean you will
use wiphy_apply_custom_regulatory() though, you may have your own
_orig value set up tools / helpers. The intel drivers are examples
of this type of driver.

Signed-off-by: Luis R. Rodriguez <mcgrof@do-not-panic.com>
---
 include/net/cfg80211.h | 7 ++++++-
 net/wireless/reg.c     | 4 ++++
 2 files changed, 10 insertions(+), 1 deletion(-)

diff --git a/include/net/cfg80211.h b/include/net/cfg80211.h
index b5cee04..638fd67 100644
--- a/include/net/cfg80211.h
+++ b/include/net/cfg80211.h
@@ -2433,7 +2433,9 @@ struct cfg80211_ops {
  * 	has its own custom regulatory domain and cannot identify the
  * 	ISO / IEC 3166 alpha2 it belongs to. When this is enabled
  * 	we will disregard the first regulatory hint (when the
- * 	initiator is %REGDOM_SET_BY_CORE).
+ * 	initiator is %REGDOM_SET_BY_CORE). Drivers that use
+ * 	wiphy_apply_custom_regulatory() should have this flag set
+ * 	or the regulatory core will set it for wiphy.
  * @WIPHY_FLAG_STRICT_REGULATORY: tells us the driver for this device will
  *	ignore regulatory domain settings until it gets its own regulatory
  *	domain via its regulatory_hint() unless the regulatory hint is
@@ -3461,6 +3463,9 @@ int regulatory_hint(struct wiphy *wiphy, const char *alpha2);
  * custom regulatory domain will be trusted completely and as such previous
  * default channel settings will be disregarded. If no rule is found for a
  * channel on the regulatory domain the channel will be disabled.
+ * Drivers using this for a wiphy should also set the wiphy flag
+ * WIPHY_FLAG_CUSTOM_REGULATORY or cfg80211 will set it for the wiphy
+ * that called this helper.
  */
 void wiphy_apply_custom_regulatory(struct wiphy *wiphy,
 				   const struct ieee80211_regdomain *regd);
diff --git a/net/wireless/reg.c b/net/wireless/reg.c
index 999b4bb7..6183c90 100644
--- a/net/wireless/reg.c
+++ b/net/wireless/reg.c
@@ -1303,6 +1303,10 @@ void wiphy_apply_custom_regulatory(struct wiphy *wiphy,
 	enum ieee80211_band band;
 	unsigned int bands_set = 0;
 
+	WARN(!(wiphy->flags & WIPHY_FLAG_CUSTOM_REGULATORY),
+	     "wiphy should have WIPHY_FLAG_CUSTOM_REGULATORY\n");
+	wiphy->flags |= WIPHY_FLAG_CUSTOM_REGULATORY;
+
 	for (band = 0; band < IEEE80211_NUM_BANDS; band++) {
 		if (!wiphy->bands[band])
 			continue;
-- 
1.8.4.rc3


^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [PATCH 03/19] cfg80211: check regulatory request alpha2 early
  2013-11-05 17:17 [PATCH 00/19] cfg80211: regulatory updates Luis R. Rodriguez
  2013-11-05 17:17 ` [PATCH 01/19] cfg80211: enforce disabling channels by custom or strict settings Luis R. Rodriguez
  2013-11-05 17:18 ` [PATCH 02/19] cfg80211: force WIPHY_FLAG_CUSTOM_REGULATORY on wiphy_apply_custom_regulatory() Luis R. Rodriguez
@ 2013-11-05 17:18 ` Luis R. Rodriguez
  2013-11-06 10:17   ` Johannes Berg
  2013-11-05 17:18 ` [PATCH 04/19] cfg80211: remove second argument from reg_process_hint() Luis R. Rodriguez
                   ` (16 subsequent siblings)
  19 siblings, 1 reply; 30+ messages in thread
From: Luis R. Rodriguez @ 2013-11-05 17:18 UTC (permalink / raw)
  To: johannes; +Cc: linux-wireless, Luis R. Rodriguez

Currently nl80211 allows userspace to send the kernel
a bogus regulatory domain with at most 32 rules set
and it won't reject it until after its allocated
memory. Let's be smart about it and take advantage
that the last_request is now available under RCU
and check if the alpha2 matches an expected request
and reject any bogus userspace requests prior to
hitting the memory allocator.

Signed-off-by: Luis R. Rodriguez <mcgrof@do-not-panic.com>
---
 net/wireless/nl80211.c | 3 +++
 net/wireless/reg.c     | 2 +-
 net/wireless/reg.h     | 1 +
 3 files changed, 5 insertions(+), 1 deletion(-)

diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c
index cc5d106..476d32c 100644
--- a/net/wireless/nl80211.c
+++ b/net/wireless/nl80211.c
@@ -5053,6 +5053,9 @@ static int nl80211_set_reg(struct sk_buff *skb, struct genl_info *info)
 			return -EINVAL;
 	}
 
+	if (!reg_is_valid_request(alpha2))
+		return -EINVAL;
+
 	size_of_regd = sizeof(struct ieee80211_regdomain) +
 		       num_rules * sizeof(struct ieee80211_reg_rule);
 
diff --git a/net/wireless/reg.c b/net/wireless/reg.c
index 6183c90..ff595d1 100644
--- a/net/wireless/reg.c
+++ b/net/wireless/reg.c
@@ -450,7 +450,7 @@ static int call_crda(const char *alpha2)
 	return kobject_uevent(&reg_pdev->dev.kobj, KOBJ_CHANGE);
 }
 
-static bool reg_is_valid_request(const char *alpha2)
+bool reg_is_valid_request(const char *alpha2)
 {
 	struct regulatory_request *lr = get_last_request();
 
diff --git a/net/wireless/reg.h b/net/wireless/reg.h
index 9677e3c..b4076ba 100644
--- a/net/wireless/reg.h
+++ b/net/wireless/reg.h
@@ -18,6 +18,7 @@
 
 extern const struct ieee80211_regdomain __rcu *cfg80211_regdomain;
 
+bool reg_is_valid_request(const char *alpha2);
 bool is_world_regdom(const char *alpha2);
 bool reg_supported_dfs_region(u8 dfs_region);
 
-- 
1.8.4.rc3


^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [PATCH 04/19] cfg80211: remove second argument from reg_process_hint()
  2013-11-05 17:17 [PATCH 00/19] cfg80211: regulatory updates Luis R. Rodriguez
                   ` (2 preceding siblings ...)
  2013-11-05 17:18 ` [PATCH 03/19] cfg80211: check regulatory request alpha2 early Luis R. Rodriguez
@ 2013-11-05 17:18 ` Luis R. Rodriguez
  2013-11-05 17:18 ` [PATCH 05/19] cfg80211: processing core regulatory hints on its own Luis R. Rodriguez
                   ` (15 subsequent siblings)
  19 siblings, 0 replies; 30+ messages in thread
From: Luis R. Rodriguez @ 2013-11-05 17:18 UTC (permalink / raw)
  To: johannes; +Cc: linux-wireless, Luis R. Rodriguez

The iniator is already available to us, so use it.

Signed-off-by: Luis R. Rodriguez <mcgrof@do-not-panic.com>
---
 net/wireless/reg.c | 11 +++++------
 1 file changed, 5 insertions(+), 6 deletions(-)

diff --git a/net/wireless/reg.c b/net/wireless/reg.c
index ff595d1..dd2ff68 100644
--- a/net/wireless/reg.c
+++ b/net/wireless/reg.c
@@ -1537,8 +1537,7 @@ new_request:
 }
 
 /* This processes *all* regulatory hints */
-static void reg_process_hint(struct regulatory_request *reg_request,
-			     enum nl80211_reg_initiator reg_initiator)
+static void reg_process_hint(struct regulatory_request *reg_request)
 {
 	struct wiphy *wiphy = NULL;
 
@@ -1548,7 +1547,7 @@ static void reg_process_hint(struct regulatory_request *reg_request,
 	if (reg_request->wiphy_idx != WIPHY_IDX_INVALID)
 		wiphy = wiphy_idx_to_wiphy(reg_request->wiphy_idx);
 
-	if (reg_initiator == NL80211_REGDOM_SET_BY_DRIVER && !wiphy) {
+	if (reg_request->initiator == NL80211_REGDOM_SET_BY_DRIVER && !wiphy) {
 		kfree(reg_request);
 		return;
 	}
@@ -1557,10 +1556,10 @@ static void reg_process_hint(struct regulatory_request *reg_request,
 	case REG_REQ_ALREADY_SET:
 		/* This is required so that the orig_* parameters are saved */
 		if (wiphy && wiphy->flags & WIPHY_FLAG_STRICT_REGULATORY)
-			wiphy_update_regulatory(wiphy, reg_initiator);
+			wiphy_update_regulatory(wiphy, reg_request->initiator);
 		break;
 	default:
-		if (reg_initiator == NL80211_REGDOM_SET_BY_USER)
+		if (reg_request->initiator == NL80211_REGDOM_SET_BY_USER)
 			schedule_delayed_work(&reg_timeout,
 					      msecs_to_jiffies(3142));
 		break;
@@ -1598,7 +1597,7 @@ static void reg_process_pending_hints(void)
 
 	spin_unlock(&reg_requests_lock);
 
-	reg_process_hint(reg_request, reg_request->initiator);
+	reg_process_hint(reg_request);
 }
 
 /* Processes beacon hints -- this has nothing to do with country IEs */
-- 
1.8.4.rc3


^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [PATCH 05/19] cfg80211: processing core regulatory hints on its own
  2013-11-05 17:17 [PATCH 00/19] cfg80211: regulatory updates Luis R. Rodriguez
                   ` (3 preceding siblings ...)
  2013-11-05 17:18 ` [PATCH 04/19] cfg80211: remove second argument from reg_process_hint() Luis R. Rodriguez
@ 2013-11-05 17:18 ` Luis R. Rodriguez
  2013-11-06 10:24   ` Johannes Berg
  2013-11-05 17:18 ` [PATCH 06/19] cfg80211: process user regulatory requests " Luis R. Rodriguez
                   ` (14 subsequent siblings)
  19 siblings, 1 reply; 30+ messages in thread
From: Luis R. Rodriguez @ 2013-11-05 17:18 UTC (permalink / raw)
  To: johannes; +Cc: linux-wireless, Luis R. Rodriguez

This makes the code path easier to read for the core case.

Signed-off-by: Luis R. Rodriguez <mcgrof@do-not-panic.com>
---
 net/wireless/reg.c | 43 +++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 41 insertions(+), 2 deletions(-)

diff --git a/net/wireless/reg.c b/net/wireless/reg.c
index dd2ff68..d242af0 100644
--- a/net/wireless/reg.c
+++ b/net/wireless/reg.c
@@ -1337,7 +1337,7 @@ get_reg_request_treatment(struct wiphy *wiphy,
 
 	switch (pending_request->initiator) {
 	case NL80211_REGDOM_SET_BY_CORE:
-		return REG_REQ_OK;
+		return REG_REQ_IGNORE;
 	case NL80211_REGDOM_SET_BY_COUNTRY_IE:
 		if (reg_request_cell_base(lr)) {
 			/* Trust a Cell base station over the AP's country IE */
@@ -1443,6 +1443,33 @@ static void reg_set_request_processed(void)
 }
 
 /**
+ * reg_process_hint_core - process core regulatory requests
+ * @pending_request: a pending core regulatory request
+ *
+ * The wireless subsystem can use this function to process
+ * a regulatory request issued by the regulatory core.
+ *
+ * Returns one of the different reg request treatment values.
+ */
+static enum reg_request_treatment
+reg_process_hint_core(struct regulatory_request *core_request)
+{
+	struct regulatory_request *lr;
+
+	lr = get_last_request();
+	if (lr != &core_request_world && lr)
+		kfree_rcu(lr, rcu_head);
+
+	core_request->intersect = false;
+	core_request->processed = false;
+	rcu_assign_pointer(last_request, core_request);
+
+	if (call_crda(core_request->alpha2))
+		return REG_REQ_IGNORE;
+	return REG_REQ_OK;
+}
+
+/**
  * __regulatory_hint - hint to the wireless core a regulatory domain
  * @wiphy: if the hint comes from country information from an AP, this
  *	is required to be set to the wiphy that received the information
@@ -1540,6 +1567,7 @@ new_request:
 static void reg_process_hint(struct regulatory_request *reg_request)
 {
 	struct wiphy *wiphy = NULL;
+	enum reg_request_treatment treatment;
 
 	if (WARN_ON(!reg_request->alpha2))
 		return;
@@ -1552,7 +1580,18 @@ static void reg_process_hint(struct regulatory_request *reg_request)
 		return;
 	}
 
-	switch (__regulatory_hint(wiphy, reg_request)) {
+	switch (reg_request->initiator) {
+	case NL80211_REGDOM_SET_BY_CORE:
+		reg_process_hint_core(reg_request);
+		return;
+	case NL80211_REGDOM_SET_BY_USER:
+	case NL80211_REGDOM_SET_BY_DRIVER:
+	case NL80211_REGDOM_SET_BY_COUNTRY_IE:
+		treatment = __regulatory_hint(wiphy, reg_request);
+		break;
+	}
+
+	switch (treatment) {
 	case REG_REQ_ALREADY_SET:
 		/* This is required so that the orig_* parameters are saved */
 		if (wiphy && wiphy->flags & WIPHY_FLAG_STRICT_REGULATORY)
-- 
1.8.4.rc3


^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [PATCH 06/19] cfg80211: process user regulatory requests on its own
  2013-11-05 17:17 [PATCH 00/19] cfg80211: regulatory updates Luis R. Rodriguez
                   ` (4 preceding siblings ...)
  2013-11-05 17:18 ` [PATCH 05/19] cfg80211: processing core regulatory hints on its own Luis R. Rodriguez
@ 2013-11-05 17:18 ` Luis R. Rodriguez
  2013-11-05 17:18 ` [PATCH 07/19] cfg80211: process driver " Luis R. Rodriguez
                   ` (13 subsequent siblings)
  19 siblings, 0 replies; 30+ messages in thread
From: Luis R. Rodriguez @ 2013-11-05 17:18 UTC (permalink / raw)
  To: johannes; +Cc: linux-wireless, Luis R. Rodriguez

This makes the code path easier to read and lets us
split out some functionality that is only user specific,
that makes it easier to read the other types of requests.

Signed-off-by: Luis R. Rodriguez <mcgrof@do-not-panic.com>
---
 net/wireless/reg.c | 114 +++++++++++++++++++++++++++++++++++++----------------
 1 file changed, 81 insertions(+), 33 deletions(-)

diff --git a/net/wireless/reg.c b/net/wireless/reg.c
index d242af0..ed893ab 100644
--- a/net/wireless/reg.c
+++ b/net/wireless/reg.c
@@ -1337,6 +1337,7 @@ get_reg_request_treatment(struct wiphy *wiphy,
 
 	switch (pending_request->initiator) {
 	case NL80211_REGDOM_SET_BY_CORE:
+	case NL80211_REGDOM_SET_BY_USER:
 		return REG_REQ_IGNORE;
 	case NL80211_REGDOM_SET_BY_COUNTRY_IE:
 		if (reg_request_cell_base(lr)) {
@@ -1388,36 +1389,6 @@ get_reg_request_treatment(struct wiphy *wiphy,
 			return REG_REQ_ALREADY_SET;
 
 		return REG_REQ_INTERSECT;
-	case NL80211_REGDOM_SET_BY_USER:
-		if (reg_request_cell_base(pending_request))
-			return reg_ignore_cell_hint(pending_request);
-
-		if (reg_request_cell_base(lr))
-			return REG_REQ_IGNORE;
-
-		if (lr->initiator == NL80211_REGDOM_SET_BY_COUNTRY_IE)
-			return REG_REQ_INTERSECT;
-		/*
-		 * If the user knows better the user should set the regdom
-		 * to their country before the IE is picked up
-		 */
-		if (lr->initiator == NL80211_REGDOM_SET_BY_USER &&
-		    lr->intersect)
-			return REG_REQ_IGNORE;
-		/*
-		 * Process user requests only after previous user/driver/core
-		 * requests have been processed
-		 */
-		if ((lr->initiator == NL80211_REGDOM_SET_BY_CORE ||
-		     lr->initiator == NL80211_REGDOM_SET_BY_DRIVER ||
-		     lr->initiator == NL80211_REGDOM_SET_BY_USER) &&
-		    regdom_changes(lr->alpha2))
-			return REG_REQ_IGNORE;
-
-		if (!regdom_changes(pending_request->alpha2))
-			return REG_REQ_ALREADY_SET;
-
-		return REG_REQ_OK;
 	}
 
 	return REG_REQ_IGNORE;
@@ -1469,6 +1440,80 @@ reg_process_hint_core(struct regulatory_request *core_request)
 	return REG_REQ_OK;
 }
 
+static enum reg_request_treatment
+__reg_process_hint_user(struct regulatory_request *user_request)
+{
+	struct regulatory_request *lr = get_last_request();
+
+	if (reg_request_cell_base(user_request))
+		return reg_ignore_cell_hint(user_request);
+
+	if (reg_request_cell_base(lr))
+		return REG_REQ_IGNORE;
+
+	if (lr->initiator == NL80211_REGDOM_SET_BY_COUNTRY_IE)
+		return REG_REQ_INTERSECT;
+	/*
+	 * If the user knows better the user should set the regdom
+	 * to their country before the IE is picked up
+	 */
+	if (lr->initiator == NL80211_REGDOM_SET_BY_USER &&
+	    lr->intersect)
+		return REG_REQ_IGNORE;
+	/*
+	 * Process user requests only after previous user/driver/core
+	 * requests have been processed
+	 */
+	if ((lr->initiator == NL80211_REGDOM_SET_BY_CORE ||
+	     lr->initiator == NL80211_REGDOM_SET_BY_DRIVER ||
+	     lr->initiator == NL80211_REGDOM_SET_BY_USER) &&
+	    regdom_changes(lr->alpha2))
+		return REG_REQ_IGNORE;
+
+	if (!regdom_changes(user_request->alpha2))
+		return REG_REQ_ALREADY_SET;
+
+	return REG_REQ_OK;
+}
+
+/**
+ * reg_process_hint_user - process user regulatory requests
+ * @user_request: a pending user regulatory request
+ *
+ * The wireless subsystem can use this function to process
+ * a regulatory request initiated by userspace.
+ *
+ * Returns one of the different reg request treatment values.
+ */
+static enum reg_request_treatment
+reg_process_hint_user(struct regulatory_request *user_request)
+{
+	enum reg_request_treatment treatment;
+	struct regulatory_request *lr;
+
+	treatment = __reg_process_hint_user(user_request);
+	if (treatment == REG_REQ_IGNORE ||
+	    treatment == REG_REQ_ALREADY_SET) {
+		kfree(user_request);
+		return treatment;
+	}
+
+	lr = get_last_request();
+	if (lr != &core_request_world && lr)
+		kfree_rcu(lr, rcu_head);
+
+	user_request->intersect = treatment == REG_REQ_INTERSECT;
+	user_request->processed = false;
+	rcu_assign_pointer(last_request, user_request);
+
+	user_alpha2[0] = user_request->alpha2[0];
+	user_alpha2[1] = user_request->alpha2[1];
+
+	if (call_crda(user_request->alpha2))
+		return REG_REQ_IGNORE;
+	return REG_REQ_OK;
+}
+
 /**
  * __regulatory_hint - hint to the wireless core a regulatory domain
  * @wiphy: if the hint comes from country information from an AP, this
@@ -1585,6 +1630,12 @@ static void reg_process_hint(struct regulatory_request *reg_request)
 		reg_process_hint_core(reg_request);
 		return;
 	case NL80211_REGDOM_SET_BY_USER:
+		treatment = reg_process_hint_user(reg_request);
+		if (treatment == REG_REQ_OK ||
+		    treatment == REG_REQ_ALREADY_SET)
+			return;
+		schedule_delayed_work(&reg_timeout, msecs_to_jiffies(3142));
+		return;
 	case NL80211_REGDOM_SET_BY_DRIVER:
 	case NL80211_REGDOM_SET_BY_COUNTRY_IE:
 		treatment = __regulatory_hint(wiphy, reg_request);
@@ -1598,9 +1649,6 @@ static void reg_process_hint(struct regulatory_request *reg_request)
 			wiphy_update_regulatory(wiphy, reg_request->initiator);
 		break;
 	default:
-		if (reg_request->initiator == NL80211_REGDOM_SET_BY_USER)
-			schedule_delayed_work(&reg_timeout,
-					      msecs_to_jiffies(3142));
 		break;
 	}
 }
-- 
1.8.4.rc3


^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [PATCH 07/19] cfg80211: process driver regulatory requests on its own
  2013-11-05 17:17 [PATCH 00/19] cfg80211: regulatory updates Luis R. Rodriguez
                   ` (5 preceding siblings ...)
  2013-11-05 17:18 ` [PATCH 06/19] cfg80211: process user regulatory requests " Luis R. Rodriguez
@ 2013-11-05 17:18 ` Luis R. Rodriguez
  2013-11-05 17:18 ` [PATCH 08/19] cfg80211: process country IE regulatory requests on their own Luis R. Rodriguez
                   ` (12 subsequent siblings)
  19 siblings, 0 replies; 30+ messages in thread
From: Luis R. Rodriguez @ 2013-11-05 17:18 UTC (permalink / raw)
  To: johannes; +Cc: linux-wireless, Luis R. Rodriguez

This makes the code easier to read and follow.

Signed-off-by: Luis R. Rodriguez <mcgrof@do-not-panic.com>
---
 net/wireless/reg.c | 103 ++++++++++++++++++++++++++++++++++++++++++++---------
 1 file changed, 86 insertions(+), 17 deletions(-)

diff --git a/net/wireless/reg.c b/net/wireless/reg.c
index ed893ab..bf2ef37 100644
--- a/net/wireless/reg.c
+++ b/net/wireless/reg.c
@@ -1338,6 +1338,7 @@ get_reg_request_treatment(struct wiphy *wiphy,
 	switch (pending_request->initiator) {
 	case NL80211_REGDOM_SET_BY_CORE:
 	case NL80211_REGDOM_SET_BY_USER:
+	case NL80211_REGDOM_SET_BY_DRIVER:
 		return REG_REQ_IGNORE;
 	case NL80211_REGDOM_SET_BY_COUNTRY_IE:
 		if (reg_request_cell_base(lr)) {
@@ -1372,23 +1373,6 @@ get_reg_request_treatment(struct wiphy *wiphy,
 			return REG_REQ_ALREADY_SET;
 		}
 		return REG_REQ_OK;
-	case NL80211_REGDOM_SET_BY_DRIVER:
-		if (lr->initiator == NL80211_REGDOM_SET_BY_CORE) {
-			if (regdom_changes(pending_request->alpha2))
-				return REG_REQ_OK;
-			return REG_REQ_ALREADY_SET;
-		}
-
-		/*
-		 * This would happen if you unplug and plug your card
-		 * back in or if you add a new device for which the previously
-		 * loaded card also agrees on the regulatory domain.
-		 */
-		if (lr->initiator == NL80211_REGDOM_SET_BY_DRIVER &&
-		    !regdom_changes(pending_request->alpha2))
-			return REG_REQ_ALREADY_SET;
-
-		return REG_REQ_INTERSECT;
 	}
 
 	return REG_REQ_IGNORE;
@@ -1514,6 +1498,89 @@ reg_process_hint_user(struct regulatory_request *user_request)
 	return REG_REQ_OK;
 }
 
+static enum reg_request_treatment
+__reg_process_hint_driver(struct regulatory_request *driver_request)
+{
+	struct regulatory_request *lr = get_last_request();
+
+	if (lr->initiator == NL80211_REGDOM_SET_BY_CORE) {
+		if (regdom_changes(driver_request->alpha2))
+			return REG_REQ_OK;
+		return REG_REQ_ALREADY_SET;
+	}
+
+	/*
+	 * This would happen if you unplug and plug your card
+	 * back in or if you add a new device for which the previously
+	 * loaded card also agrees on the regulatory domain.
+	 */
+	if (lr->initiator == NL80211_REGDOM_SET_BY_DRIVER &&
+	    !regdom_changes(driver_request->alpha2))
+		return REG_REQ_ALREADY_SET;
+
+	return REG_REQ_INTERSECT;
+}
+
+/**
+ * reg_process_hint_driver - process driver regulatory requests
+ * @driver_request: a pending driver regulatory request
+ *
+ * The wireless subsystem can use this function to process
+ * a regulatory request issued by an 802.11 driver.
+ *
+ * Returns one of the different reg request treatment values.
+ */
+static enum reg_request_treatment
+reg_process_hint_driver(struct wiphy *wiphy,
+			struct regulatory_request *driver_request)
+{
+	const struct ieee80211_regdomain *regd;
+	enum reg_request_treatment treatment;
+	struct regulatory_request *lr;
+
+	treatment = __reg_process_hint_driver(driver_request);
+
+	switch (treatment) {
+	case REG_REQ_OK:
+		break;
+	case REG_REQ_IGNORE:
+		kfree(driver_request);
+		return treatment;
+	case REG_REQ_INTERSECT:
+		/* fall through */
+	case REG_REQ_ALREADY_SET:
+		regd = reg_copy_regd(get_cfg80211_regdom());
+		if (IS_ERR(regd)) {
+			kfree(driver_request);
+			return REG_REQ_IGNORE;
+		}
+		rcu_assign_pointer(wiphy->regd, regd);
+	}
+
+	lr = get_last_request();
+	if (lr != &core_request_world && lr)
+		kfree_rcu(lr, rcu_head);
+
+	driver_request->intersect = treatment == REG_REQ_INTERSECT;
+	driver_request->processed = false;
+	rcu_assign_pointer(last_request, driver_request);
+
+	/*
+	 * Since CRDA will not be called in this case as we already
+	 * have applied the requested regulatory domain before we just
+	 * inform userspace we have processed the request
+	 */
+	if (treatment == REG_REQ_ALREADY_SET) {
+		nl80211_send_reg_change_event(driver_request);
+		reg_set_request_processed();
+		return treatment;
+	}
+
+	if (call_crda(driver_request->alpha2))
+		return REG_REQ_IGNORE;
+	return REG_REQ_OK;
+}
+
 /**
  * __regulatory_hint - hint to the wireless core a regulatory domain
  * @wiphy: if the hint comes from country information from an AP, this
@@ -1637,6 +1704,8 @@ static void reg_process_hint(struct regulatory_request *reg_request)
 		schedule_delayed_work(&reg_timeout, msecs_to_jiffies(3142));
 		return;
 	case NL80211_REGDOM_SET_BY_DRIVER:
+		treatment = reg_process_hint_driver(wiphy, reg_request);
+		break;
 	case NL80211_REGDOM_SET_BY_COUNTRY_IE:
 		treatment = __regulatory_hint(wiphy, reg_request);
 		break;
-- 
1.8.4.rc3


^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [PATCH 08/19] cfg80211: process country IE regulatory requests on their own
  2013-11-05 17:17 [PATCH 00/19] cfg80211: regulatory updates Luis R. Rodriguez
                   ` (6 preceding siblings ...)
  2013-11-05 17:18 ` [PATCH 07/19] cfg80211: process driver " Luis R. Rodriguez
@ 2013-11-05 17:18 ` Luis R. Rodriguez
  2013-11-05 17:18 ` [PATCH 09/19] cfg80211: process non country IE conflicting first Luis R. Rodriguez
                   ` (11 subsequent siblings)
  19 siblings, 0 replies; 30+ messages in thread
From: Luis R. Rodriguez @ 2013-11-05 17:18 UTC (permalink / raw)
  To: johannes; +Cc: linux-wireless, Luis R. Rodriguez

This is the last split up of the old unified __regultory_hint()
processing set of functionality, it moves the country IE processing
all on its own. This makes it easier to follow and read what exactly
is going on for the case of processing country IEs.

Signed-off-by: Luis R. Rodriguez <mcgrof@do-not-panic.com>
---
 net/wireless/reg.c | 201 ++++++++++++++++++-----------------------------------
 1 file changed, 68 insertions(+), 133 deletions(-)

diff --git a/net/wireless/reg.c b/net/wireless/reg.c
index bf2ef37..20e64bd 100644
--- a/net/wireless/reg.c
+++ b/net/wireless/reg.c
@@ -1322,62 +1322,6 @@ void wiphy_apply_custom_regulatory(struct wiphy *wiphy,
 }
 EXPORT_SYMBOL(wiphy_apply_custom_regulatory);
 
-/* This has the logic which determines when a new request
- * should be ignored. */
-static enum reg_request_treatment
-get_reg_request_treatment(struct wiphy *wiphy,
-			  struct regulatory_request *pending_request)
-{
-	struct wiphy *last_wiphy = NULL;
-	struct regulatory_request *lr = get_last_request();
-
-	/* All initial requests are respected */
-	if (!lr)
-		return REG_REQ_OK;
-
-	switch (pending_request->initiator) {
-	case NL80211_REGDOM_SET_BY_CORE:
-	case NL80211_REGDOM_SET_BY_USER:
-	case NL80211_REGDOM_SET_BY_DRIVER:
-		return REG_REQ_IGNORE;
-	case NL80211_REGDOM_SET_BY_COUNTRY_IE:
-		if (reg_request_cell_base(lr)) {
-			/* Trust a Cell base station over the AP's country IE */
-			if (regdom_changes(pending_request->alpha2))
-				return REG_REQ_IGNORE;
-			return REG_REQ_ALREADY_SET;
-		}
-
-		last_wiphy = wiphy_idx_to_wiphy(lr->wiphy_idx);
-
-		if (unlikely(!is_an_alpha2(pending_request->alpha2)))
-			return -EINVAL;
-		if (lr->initiator == NL80211_REGDOM_SET_BY_COUNTRY_IE) {
-			if (last_wiphy != wiphy) {
-				/*
-				 * Two cards with two APs claiming different
-				 * Country IE alpha2s. We could
-				 * intersect them, but that seems unlikely
-				 * to be correct. Reject second one for now.
-				 */
-				if (regdom_changes(pending_request->alpha2))
-					return REG_REQ_IGNORE;
-				return REG_REQ_ALREADY_SET;
-			}
-			/*
-			 * Two consecutive Country IE hints on the same wiphy.
-			 * This should be picked up early by the driver/stack
-			 */
-			if (WARN_ON(regdom_changes(pending_request->alpha2)))
-				return REG_REQ_OK;
-			return REG_REQ_ALREADY_SET;
-		}
-		return REG_REQ_OK;
-	}
-
-	return REG_REQ_IGNORE;
-}
-
 static void reg_set_request_processed(void)
 {
 	bool need_more_processing = false;
@@ -1581,96 +1525,92 @@ reg_process_hint_driver(struct wiphy *wiphy,
 	return REG_REQ_OK;
 }
 
+static enum reg_request_treatment
+__reg_process_hint_country_ie(struct wiphy *wiphy,
+			      struct regulatory_request *country_ie_request)
+{
+	struct wiphy *last_wiphy = NULL;
+	struct regulatory_request *lr = get_last_request();
+
+	if (reg_request_cell_base(lr)) {
+		/* Trust a Cell base station over the AP's country IE */
+		if (regdom_changes(country_ie_request->alpha2))
+			return REG_REQ_IGNORE;
+		return REG_REQ_ALREADY_SET;
+	}
+
+	last_wiphy = wiphy_idx_to_wiphy(lr->wiphy_idx);
+
+	if (unlikely(!is_an_alpha2(country_ie_request->alpha2)))
+		return -EINVAL;
+	if (lr->initiator == NL80211_REGDOM_SET_BY_COUNTRY_IE) {
+		if (last_wiphy != wiphy) {
+			/*
+			 * Two cards with two APs claiming different
+			 * Country IE alpha2s. We could
+			 * intersect them, but that seems unlikely
+			 * to be correct. Reject second one for now.
+			 */
+			if (regdom_changes(country_ie_request->alpha2))
+				return REG_REQ_IGNORE;
+			return REG_REQ_ALREADY_SET;
+		}
+		/*
+		 * Two consecutive Country IE hints on the same wiphy.
+		 * This should be picked up early by the driver/stack
+		 */
+		if (WARN_ON(regdom_changes(country_ie_request->alpha2)))
+			return REG_REQ_OK;
+		return REG_REQ_ALREADY_SET;
+	}
+	return REG_REQ_OK;
+}
+
 /**
- * __regulatory_hint - hint to the wireless core a regulatory domain
- * @wiphy: if the hint comes from country information from an AP, this
- *	is required to be set to the wiphy that received the information
- * @pending_request: the regulatory request currently being processed
+ * reg_process_hint_country_ie - process regulatory requests from country IEs
+ * @country_ie_request: a regulatory request from a country IE
  *
- * The Wireless subsystem can use this function to hint to the wireless core
- * what it believes should be the current regulatory domain.
+ * The wireless subsystem can use this function to process
+ * a regulatory request issued by a country Information Element.
  *
  * Returns one of the different reg request treatment values.
  */
 static enum reg_request_treatment
-__regulatory_hint(struct wiphy *wiphy,
-		  struct regulatory_request *pending_request)
+reg_process_hint_country_ie(struct wiphy *wiphy,
+			    struct regulatory_request *country_ie_request)
 {
-	const struct ieee80211_regdomain *regd;
-	bool intersect = false;
 	enum reg_request_treatment treatment;
 	struct regulatory_request *lr;
 
-	treatment = get_reg_request_treatment(wiphy, pending_request);
+	treatment = __reg_process_hint_country_ie(wiphy, country_ie_request);
 
 	switch (treatment) {
-	case REG_REQ_INTERSECT:
-		if (pending_request->initiator ==
-		    NL80211_REGDOM_SET_BY_DRIVER) {
-			regd = reg_copy_regd(get_cfg80211_regdom());
-			if (IS_ERR(regd)) {
-				kfree(pending_request);
-				return PTR_ERR(regd);
-			}
-			rcu_assign_pointer(wiphy->regd, regd);
-		}
-		intersect = true;
-		break;
 	case REG_REQ_OK:
 		break;
-	default:
+	case REG_REQ_IGNORE:
+		/* fall through */
+	case REG_REQ_ALREADY_SET:
+		kfree(country_ie_request);
+		return treatment;
+	case REG_REQ_INTERSECT:
+		kfree(country_ie_request);
 		/*
-		 * If the regulatory domain being requested by the
-		 * driver has already been set just copy it to the
-		 * wiphy
+		 * This doesn't happen yet, not sure we
+		 * ever want to support it for this case.
 		 */
-		if (treatment == REG_REQ_ALREADY_SET &&
-		    pending_request->initiator == NL80211_REGDOM_SET_BY_DRIVER) {
-			regd = reg_copy_regd(get_cfg80211_regdom());
-			if (IS_ERR(regd)) {
-				kfree(pending_request);
-				return REG_REQ_IGNORE;
-			}
-			treatment = REG_REQ_ALREADY_SET;
-			rcu_assign_pointer(wiphy->regd, regd);
-			goto new_request;
-		}
-		kfree(pending_request);
-		return treatment;
+		WARN_ONCE(1, "Unexpected intersection for country IEs");
+		return REG_REQ_IGNORE;
 	}
 
-new_request:
 	lr = get_last_request();
 	if (lr != &core_request_world && lr)
 		kfree_rcu(lr, rcu_head);
 
-	pending_request->intersect = intersect;
-	pending_request->processed = false;
-	rcu_assign_pointer(last_request, pending_request);
-	lr = pending_request;
-
-	pending_request = NULL;
+	country_ie_request->intersect = false;
+	country_ie_request->processed = false;
+	rcu_assign_pointer(last_request, country_ie_request);
 
-	if (lr->initiator == NL80211_REGDOM_SET_BY_USER) {
-		user_alpha2[0] = lr->alpha2[0];
-		user_alpha2[1] = lr->alpha2[1];
-	}
-
-	/* When r == REG_REQ_INTERSECT we do need to call CRDA */
-	if (treatment != REG_REQ_OK && treatment != REG_REQ_INTERSECT) {
-		/*
-		 * Since CRDA will not be called in this case as we already
-		 * have applied the requested regulatory domain before we just
-		 * inform userspace we have processed the request
-		 */
-		if (treatment == REG_REQ_ALREADY_SET) {
-			nl80211_send_reg_change_event(lr);
-			reg_set_request_processed();
-		}
-		return treatment;
-	}
-
-	if (call_crda(lr->alpha2))
+	if (call_crda(country_ie_request->alpha2))
 		return REG_REQ_IGNORE;
 	return REG_REQ_OK;
 }
@@ -1707,19 +1647,14 @@ static void reg_process_hint(struct regulatory_request *reg_request)
 		treatment = reg_process_hint_driver(wiphy, reg_request);
 		break;
 	case NL80211_REGDOM_SET_BY_COUNTRY_IE:
-		treatment = __regulatory_hint(wiphy, reg_request);
+		treatment = reg_process_hint_country_ie(wiphy, reg_request);
 		break;
 	}
 
-	switch (treatment) {
-	case REG_REQ_ALREADY_SET:
-		/* This is required so that the orig_* parameters are saved */
-		if (wiphy && wiphy->flags & WIPHY_FLAG_STRICT_REGULATORY)
-			wiphy_update_regulatory(wiphy, reg_request->initiator);
-		break;
-	default:
-		break;
-	}
+	/* This is required so that the orig_* parameters are saved */
+	if (treatment == REG_REQ_ALREADY_SET && wiphy &&
+	    wiphy->flags & WIPHY_FLAG_STRICT_REGULATORY)
+		wiphy_update_regulatory(wiphy, reg_request->initiator);
 }
 
 /*
-- 
1.8.4.rc3


^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [PATCH 09/19] cfg80211: process non country IE conflicting first
  2013-11-05 17:17 [PATCH 00/19] cfg80211: regulatory updates Luis R. Rodriguez
                   ` (7 preceding siblings ...)
  2013-11-05 17:18 ` [PATCH 08/19] cfg80211: process country IE regulatory requests on their own Luis R. Rodriguez
@ 2013-11-05 17:18 ` Luis R. Rodriguez
  2013-11-05 17:18 ` [PATCH 10/19] cfg80211: add helper for kfree'ing last_request Luis R. Rodriguez
                   ` (10 subsequent siblings)
  19 siblings, 0 replies; 30+ messages in thread
From: Luis R. Rodriguez @ 2013-11-05 17:18 UTC (permalink / raw)
  To: johannes; +Cc: linux-wireless, Luis R. Rodriguez

By dealing with non country IE conficts first we can shift
the code that deals with the conflict to the left. This has
no functional changes.

Signed-off-by: Luis R. Rodriguez <mcgrof@do-not-panic.com>
---
 net/wireless/reg.c | 39 ++++++++++++++++++++-------------------
 1 file changed, 20 insertions(+), 19 deletions(-)

diff --git a/net/wireless/reg.c b/net/wireless/reg.c
index 20e64bd..4d552d8 100644
--- a/net/wireless/reg.c
+++ b/net/wireless/reg.c
@@ -1539,31 +1539,32 @@ __reg_process_hint_country_ie(struct wiphy *wiphy,
 		return REG_REQ_ALREADY_SET;
 	}
 
-	last_wiphy = wiphy_idx_to_wiphy(lr->wiphy_idx);
-
 	if (unlikely(!is_an_alpha2(country_ie_request->alpha2)))
 		return -EINVAL;
-	if (lr->initiator == NL80211_REGDOM_SET_BY_COUNTRY_IE) {
-		if (last_wiphy != wiphy) {
-			/*
-			 * Two cards with two APs claiming different
-			 * Country IE alpha2s. We could
-			 * intersect them, but that seems unlikely
-			 * to be correct. Reject second one for now.
-			 */
-			if (regdom_changes(country_ie_request->alpha2))
-				return REG_REQ_IGNORE;
-			return REG_REQ_ALREADY_SET;
-		}
+
+	if (lr->initiator != NL80211_REGDOM_SET_BY_COUNTRY_IE)
+		return REG_REQ_OK;
+
+	last_wiphy = wiphy_idx_to_wiphy(lr->wiphy_idx);
+
+	if (last_wiphy != wiphy) {
 		/*
-		 * Two consecutive Country IE hints on the same wiphy.
-		 * This should be picked up early by the driver/stack
+		 * Two cards with two APs claiming different
+		 * Country IE alpha2s. We could
+		 * intersect them, but that seems unlikely
+		 * to be correct. Reject second one for now.
 		 */
-		if (WARN_ON(regdom_changes(country_ie_request->alpha2)))
-			return REG_REQ_OK;
+		if (regdom_changes(country_ie_request->alpha2))
+			return REG_REQ_IGNORE;
 		return REG_REQ_ALREADY_SET;
 	}
-	return REG_REQ_OK;
+	/*
+	 * Two consecutive Country IE hints on the same wiphy.
+	 * This should be picked up early by the driver/stack
+	 */
+	if (WARN_ON(regdom_changes(country_ie_request->alpha2)))
+		return REG_REQ_OK;
+	return REG_REQ_ALREADY_SET;
 }
 
 /**
-- 
1.8.4.rc3


^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [PATCH 10/19] cfg80211: add helper for kfree'ing last_request
  2013-11-05 17:17 [PATCH 00/19] cfg80211: regulatory updates Luis R. Rodriguez
                   ` (8 preceding siblings ...)
  2013-11-05 17:18 ` [PATCH 09/19] cfg80211: process non country IE conflicting first Luis R. Rodriguez
@ 2013-11-05 17:18 ` Luis R. Rodriguez
  2013-11-05 17:18 ` [PATCH 11/19] cfg80211: add helper for kfree'ing and assigning last_request Luis R. Rodriguez
                   ` (9 subsequent siblings)
  19 siblings, 0 replies; 30+ messages in thread
From: Luis R. Rodriguez @ 2013-11-05 17:18 UTC (permalink / raw)
  To: johannes; +Cc: linux-wireless, Luis R. Rodriguez

This is common code, this reduces the chance of making
a mistake of how we free it.

Signed-off-by: Luis R. Rodriguez <mcgrof@do-not-panic.com>
---
 net/wireless/reg.c | 42 +++++++++++++++++++-----------------------
 1 file changed, 19 insertions(+), 23 deletions(-)

diff --git a/net/wireless/reg.c b/net/wireless/reg.c
index 4d552d8..30e813f 100644
--- a/net/wireless/reg.c
+++ b/net/wireless/reg.c
@@ -202,11 +202,20 @@ static char user_alpha2[2];
 module_param(ieee80211_regdom, charp, 0444);
 MODULE_PARM_DESC(ieee80211_regdom, "IEEE 802.11 regulatory domain code");
 
+static void reg_kfree_last_request(void)
+{
+	struct regulatory_request *lr;
+
+	lr = get_last_request();
+
+	if (lr != &core_request_world && lr)
+		kfree_rcu(lr, rcu_head);
+}
+
 static void reset_regdomains(bool full_reset,
 			     const struct ieee80211_regdomain *new_regdom)
 {
 	const struct ieee80211_regdomain *r;
-	struct regulatory_request *lr;
 
 	ASSERT_RTNL();
 
@@ -229,9 +238,7 @@ static void reset_regdomains(bool full_reset,
 	if (!full_reset)
 		return;
 
-	lr = get_last_request();
-	if (lr != &core_request_world && lr)
-		kfree_rcu(lr, rcu_head);
+	reg_kfree_last_request();
 	rcu_assign_pointer(last_request, &core_request_world);
 }
 
@@ -1353,14 +1360,11 @@ static void reg_set_request_processed(void)
 static enum reg_request_treatment
 reg_process_hint_core(struct regulatory_request *core_request)
 {
-	struct regulatory_request *lr;
-
-	lr = get_last_request();
-	if (lr != &core_request_world && lr)
-		kfree_rcu(lr, rcu_head);
 
 	core_request->intersect = false;
 	core_request->processed = false;
+
+	reg_kfree_last_request();
 	rcu_assign_pointer(last_request, core_request);
 
 	if (call_crda(core_request->alpha2))
@@ -1417,7 +1421,6 @@ static enum reg_request_treatment
 reg_process_hint_user(struct regulatory_request *user_request)
 {
 	enum reg_request_treatment treatment;
-	struct regulatory_request *lr;
 
 	treatment = __reg_process_hint_user(user_request);
 	if (treatment == REG_REQ_IGNORE ||
@@ -1426,12 +1429,10 @@ reg_process_hint_user(struct regulatory_request *user_request)
 		return treatment;
 	}
 
-	lr = get_last_request();
-	if (lr != &core_request_world && lr)
-		kfree_rcu(lr, rcu_head);
-
 	user_request->intersect = treatment == REG_REQ_INTERSECT;
 	user_request->processed = false;
+
+	reg_kfree_last_request();
 	rcu_assign_pointer(last_request, user_request);
 
 	user_alpha2[0] = user_request->alpha2[0];
@@ -1480,7 +1481,6 @@ reg_process_hint_driver(struct wiphy *wiphy,
 {
 	const struct ieee80211_regdomain *regd;
 	enum reg_request_treatment treatment;
-	struct regulatory_request *lr;
 
 	treatment = __reg_process_hint_driver(driver_request);
 
@@ -1501,12 +1501,11 @@ reg_process_hint_driver(struct wiphy *wiphy,
 		rcu_assign_pointer(wiphy->regd, regd);
 	}
 
-	lr = get_last_request();
-	if (lr != &core_request_world && lr)
-		kfree_rcu(lr, rcu_head);
 
 	driver_request->intersect = treatment == REG_REQ_INTERSECT;
 	driver_request->processed = false;
+
+	reg_kfree_last_request();
 	rcu_assign_pointer(last_request, driver_request);
 
 	/*
@@ -1581,7 +1580,6 @@ reg_process_hint_country_ie(struct wiphy *wiphy,
 			    struct regulatory_request *country_ie_request)
 {
 	enum reg_request_treatment treatment;
-	struct regulatory_request *lr;
 
 	treatment = __reg_process_hint_country_ie(wiphy, country_ie_request);
 
@@ -1603,12 +1601,10 @@ reg_process_hint_country_ie(struct wiphy *wiphy,
 		return REG_REQ_IGNORE;
 	}
 
-	lr = get_last_request();
-	if (lr != &core_request_world && lr)
-		kfree_rcu(lr, rcu_head);
-
 	country_ie_request->intersect = false;
 	country_ie_request->processed = false;
+
+	reg_kfree_last_request();
 	rcu_assign_pointer(last_request, country_ie_request);
 
 	if (call_crda(country_ie_request->alpha2))
-- 
1.8.4.rc3


^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [PATCH 11/19] cfg80211: add helper for kfree'ing and assigning last_request
  2013-11-05 17:17 [PATCH 00/19] cfg80211: regulatory updates Luis R. Rodriguez
                   ` (9 preceding siblings ...)
  2013-11-05 17:18 ` [PATCH 10/19] cfg80211: add helper for kfree'ing last_request Luis R. Rodriguez
@ 2013-11-05 17:18 ` Luis R. Rodriguez
  2013-11-05 17:18 ` [PATCH 12/19] cfg80211: add helper for calling CRDA Luis R. Rodriguez
                   ` (8 subsequent siblings)
  19 siblings, 0 replies; 30+ messages in thread
From: Luis R. Rodriguez @ 2013-11-05 17:18 UTC (permalink / raw)
  To: johannes; +Cc: linux-wireless, Luis R. Rodriguez

This enforces proper RCU APIs accross the code.

Signed-off-by: Luis R. Rodriguez <mcgrof@do-not-panic.com>
---
 net/wireless/reg.c | 21 +++++++++++----------
 1 file changed, 11 insertions(+), 10 deletions(-)

diff --git a/net/wireless/reg.c b/net/wireless/reg.c
index 30e813f..ddccc9a 100644
--- a/net/wireless/reg.c
+++ b/net/wireless/reg.c
@@ -212,6 +212,12 @@ static void reg_kfree_last_request(void)
 		kfree_rcu(lr, rcu_head);
 }
 
+static void reg_update_last_request(struct regulatory_request *request)
+{
+	reg_kfree_last_request();
+	rcu_assign_pointer(last_request, request);
+}
+
 static void reset_regdomains(bool full_reset,
 			     const struct ieee80211_regdomain *new_regdom)
 {
@@ -238,8 +244,7 @@ static void reset_regdomains(bool full_reset,
 	if (!full_reset)
 		return;
 
-	reg_kfree_last_request();
-	rcu_assign_pointer(last_request, &core_request_world);
+	reg_update_last_request(&core_request_world);
 }
 
 /*
@@ -1364,8 +1369,7 @@ reg_process_hint_core(struct regulatory_request *core_request)
 	core_request->intersect = false;
 	core_request->processed = false;
 
-	reg_kfree_last_request();
-	rcu_assign_pointer(last_request, core_request);
+	reg_update_last_request(core_request);
 
 	if (call_crda(core_request->alpha2))
 		return REG_REQ_IGNORE;
@@ -1432,8 +1436,7 @@ reg_process_hint_user(struct regulatory_request *user_request)
 	user_request->intersect = treatment == REG_REQ_INTERSECT;
 	user_request->processed = false;
 
-	reg_kfree_last_request();
-	rcu_assign_pointer(last_request, user_request);
+	reg_update_last_request(user_request);
 
 	user_alpha2[0] = user_request->alpha2[0];
 	user_alpha2[1] = user_request->alpha2[1];
@@ -1505,8 +1508,7 @@ reg_process_hint_driver(struct wiphy *wiphy,
 	driver_request->intersect = treatment == REG_REQ_INTERSECT;
 	driver_request->processed = false;
 
-	reg_kfree_last_request();
-	rcu_assign_pointer(last_request, driver_request);
+	reg_update_last_request(driver_request);
 
 	/*
 	 * Since CRDA will not be called in this case as we already
@@ -1604,8 +1606,7 @@ reg_process_hint_country_ie(struct wiphy *wiphy,
 	country_ie_request->intersect = false;
 	country_ie_request->processed = false;
 
-	reg_kfree_last_request();
-	rcu_assign_pointer(last_request, country_ie_request);
+	reg_update_last_request(country_ie_request);
 
 	if (call_crda(country_ie_request->alpha2))
 		return REG_REQ_IGNORE;
-- 
1.8.4.rc3


^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [PATCH 12/19] cfg80211: add helper for calling CRDA
  2013-11-05 17:17 [PATCH 00/19] cfg80211: regulatory updates Luis R. Rodriguez
                   ` (10 preceding siblings ...)
  2013-11-05 17:18 ` [PATCH 11/19] cfg80211: add helper for kfree'ing and assigning last_request Luis R. Rodriguez
@ 2013-11-05 17:18 ` Luis R. Rodriguez
  2013-11-05 17:18 ` [PATCH 13/19] cfg80211: allow only the core to request to update the world regdom Luis R. Rodriguez
                   ` (7 subsequent siblings)
  19 siblings, 0 replies; 30+ messages in thread
From: Luis R. Rodriguez @ 2013-11-05 17:18 UTC (permalink / raw)
  To: johannes; +Cc: linux-wireless, Luis R. Rodriguez

All the regulatory request process routines use the
same pattern.

Signed-off-by: Luis R. Rodriguez <mcgrof@do-not-panic.com>
---
 net/wireless/reg.c | 24 ++++++++++++------------
 1 file changed, 12 insertions(+), 12 deletions(-)

diff --git a/net/wireless/reg.c b/net/wireless/reg.c
index ddccc9a..4619c74 100644
--- a/net/wireless/reg.c
+++ b/net/wireless/reg.c
@@ -462,6 +462,14 @@ static int call_crda(const char *alpha2)
 	return kobject_uevent(&reg_pdev->dev.kobj, KOBJ_CHANGE);
 }
 
+static enum reg_request_treatment
+reg_call_crda(struct regulatory_request *request)
+{
+	if (call_crda(request->alpha2))
+		return REG_REQ_IGNORE;
+	return REG_REQ_OK;
+}
+
 bool reg_is_valid_request(const char *alpha2)
 {
 	struct regulatory_request *lr = get_last_request();
@@ -1371,9 +1379,7 @@ reg_process_hint_core(struct regulatory_request *core_request)
 
 	reg_update_last_request(core_request);
 
-	if (call_crda(core_request->alpha2))
-		return REG_REQ_IGNORE;
-	return REG_REQ_OK;
+	return reg_call_crda(core_request);
 }
 
 static enum reg_request_treatment
@@ -1441,9 +1447,7 @@ reg_process_hint_user(struct regulatory_request *user_request)
 	user_alpha2[0] = user_request->alpha2[0];
 	user_alpha2[1] = user_request->alpha2[1];
 
-	if (call_crda(user_request->alpha2))
-		return REG_REQ_IGNORE;
-	return REG_REQ_OK;
+	return reg_call_crda(user_request);
 }
 
 static enum reg_request_treatment
@@ -1521,9 +1525,7 @@ reg_process_hint_driver(struct wiphy *wiphy,
 		return treatment;
 	}
 
-	if (call_crda(driver_request->alpha2))
-		return REG_REQ_IGNORE;
-	return REG_REQ_OK;
+	return reg_call_crda(driver_request);
 }
 
 static enum reg_request_treatment
@@ -1608,9 +1610,7 @@ reg_process_hint_country_ie(struct wiphy *wiphy,
 
 	reg_update_last_request(country_ie_request);
 
-	if (call_crda(country_ie_request->alpha2))
-		return REG_REQ_IGNORE;
-	return REG_REQ_OK;
+	return reg_call_crda(country_ie_request);
 }
 
 /* This processes *all* regulatory hints */
-- 
1.8.4.rc3


^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [PATCH 13/19] cfg80211: allow only the core to request to update the world regdom
  2013-11-05 17:17 [PATCH 00/19] cfg80211: regulatory updates Luis R. Rodriguez
                   ` (11 preceding siblings ...)
  2013-11-05 17:18 ` [PATCH 12/19] cfg80211: add helper for calling CRDA Luis R. Rodriguez
@ 2013-11-05 17:18 ` Luis R. Rodriguez
  2013-11-05 17:18 ` [PATCH 14/19] cfg80211: move core reg_notfier() check to source Luis R. Rodriguez
                   ` (6 subsequent siblings)
  19 siblings, 0 replies; 30+ messages in thread
From: Luis R. Rodriguez @ 2013-11-05 17:18 UTC (permalink / raw)
  To: johannes; +Cc: linux-wireless, Luis R. Rodriguez

It seems some out of tree drivers were using a regulatory_hint("00")
to trigger off the wiphy regulatory notifier, for those cases just
setting the WIPHY_FLAG_CUSTOM_REGULATORY would suffice to call
the reg_notifier() for a world regulatory domain update. If drivers
find other needs for calling the reg_notifier() a proper implemenation
is preferred.

Signed-off-by: Luis R. Rodriguez <mcgrof@do-not-panic.com>
---
 net/wireless/reg.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/net/wireless/reg.c b/net/wireless/reg.c
index 4619c74..a5b3b40 100644
--- a/net/wireless/reg.c
+++ b/net/wireless/reg.c
@@ -2197,6 +2197,8 @@ static int __set_regdom(const struct ieee80211_regdomain *rd)
 		return -EINVAL;
 
 	if (is_world_regdom(rd->alpha2)) {
+		if (lr->initiator != NL80211_REGDOM_SET_BY_CORE)
+			return -EINVAL;
 		update_world_regdomain(rd);
 		return 0;
 	}
-- 
1.8.4.rc3


^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [PATCH 14/19] cfg80211: move core reg_notfier() check to source
  2013-11-05 17:17 [PATCH 00/19] cfg80211: regulatory updates Luis R. Rodriguez
                   ` (12 preceding siblings ...)
  2013-11-05 17:18 ` [PATCH 13/19] cfg80211: allow only the core to request to update the world regdom Luis R. Rodriguez
@ 2013-11-05 17:18 ` Luis R. Rodriguez
  2013-11-05 17:18 ` [PATCH 15/19] cfg80211: pass the last_request to __set_regdom() Luis R. Rodriguez
                   ` (5 subsequent siblings)
  19 siblings, 0 replies; 30+ messages in thread
From: Luis R. Rodriguez @ 2013-11-05 17:18 UTC (permalink / raw)
  To: johannes; +Cc: linux-wireless, Luis R. Rodriguez

Drivers that set the WIPHY_FLAG_CUSTOM_REGULATORY skip
the core world regulatory domain updates, but do want
their reg_notifier() called. Move the check for this
closer to the source of the check that detected skipped
was required and while at it add a helper for the notifier
calling. This has no functional changes. This brings together
the place where we call the reg_notifier() will be called.

Signed-off-by: Luis R. Rodriguez <mcgrof@do-not-panic.com>
---
 net/wireless/reg.c | 31 ++++++++++++++++++-------------
 1 file changed, 18 insertions(+), 13 deletions(-)

diff --git a/net/wireless/reg.c b/net/wireless/reg.c
index a5b3b40..e77ae8d 100644
--- a/net/wireless/reg.c
+++ b/net/wireless/reg.c
@@ -1223,14 +1223,30 @@ static void reg_process_ht_flags(struct wiphy *wiphy)
 		reg_process_ht_flags_band(wiphy, wiphy->bands[band]);
 }
 
+static void reg_call_notifier(struct wiphy *wiphy,
+			      struct regulatory_request *request)
+{
+	if (wiphy->reg_notifier)
+		wiphy->reg_notifier(wiphy, request);
+}
+
 static void wiphy_update_regulatory(struct wiphy *wiphy,
 				    enum nl80211_reg_initiator initiator)
 {
 	enum ieee80211_band band;
 	struct regulatory_request *lr = get_last_request();
 
-	if (ignore_reg_update(wiphy, initiator))
+	if (ignore_reg_update(wiphy, initiator)) {
+		/*
+		 * Regulatory updates set by CORE are ignored for custom
+		 * regulatory cards. Let us notify the changes to the driver,
+		 * as some drivers used this to restore its orig_* reg domain.
+		 */
+		if (initiator == NL80211_REGDOM_SET_BY_CORE &&
+		    wiphy->flags & WIPHY_FLAG_CUSTOM_REGULATORY)
+			reg_call_notifier(wiphy, lr);
 		return;
+	}
 
 	lr->dfs_region = get_cfg80211_regdom()->dfs_region;
 
@@ -1239,9 +1255,7 @@ static void wiphy_update_regulatory(struct wiphy *wiphy,
 
 	reg_process_beacons(wiphy);
 	reg_process_ht_flags(wiphy);
-
-	if (wiphy->reg_notifier)
-		wiphy->reg_notifier(wiphy, lr);
+	reg_call_notifier(wiphy, lr);
 }
 
 static void update_all_wiphy_regulatory(enum nl80211_reg_initiator initiator)
@@ -1254,15 +1268,6 @@ static void update_all_wiphy_regulatory(enum nl80211_reg_initiator initiator)
 	list_for_each_entry(rdev, &cfg80211_rdev_list, list) {
 		wiphy = &rdev->wiphy;
 		wiphy_update_regulatory(wiphy, initiator);
-		/*
-		 * Regulatory updates set by CORE are ignored for custom
-		 * regulatory cards. Let us notify the changes to the driver,
-		 * as some drivers used this to restore its orig_* reg domain.
-		 */
-		if (initiator == NL80211_REGDOM_SET_BY_CORE &&
-		    wiphy->flags & WIPHY_FLAG_CUSTOM_REGULATORY &&
-		    wiphy->reg_notifier)
-			wiphy->reg_notifier(wiphy, get_last_request());
 	}
 }
 
-- 
1.8.4.rc3


^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [PATCH 15/19] cfg80211: pass the last_request to __set_regdom()
  2013-11-05 17:17 [PATCH 00/19] cfg80211: regulatory updates Luis R. Rodriguez
                   ` (13 preceding siblings ...)
  2013-11-05 17:18 ` [PATCH 14/19] cfg80211: move core reg_notfier() check to source Luis R. Rodriguez
@ 2013-11-05 17:18 ` Luis R. Rodriguez
  2013-11-05 17:18 ` [PATCH 16/19] cfg80211: set core regulatory updates on its own Luis R. Rodriguez
                   ` (4 subsequent siblings)
  19 siblings, 0 replies; 30+ messages in thread
From: Luis R. Rodriguez @ 2013-11-05 17:18 UTC (permalink / raw)
  To: johannes; +Cc: linux-wireless, Luis R. Rodriguez

last_request is RCU protected, since we're getting it
on set_regdom() we might as well pass it to ensure the
same request is being processed, otherwise there is a
small race it could have changed. This makes processing
of the request atomic.

Signed-off-by: Luis R. Rodriguez <mcgrof@do-not-panic.com>
---
 net/wireless/reg.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/net/wireless/reg.c b/net/wireless/reg.c
index e77ae8d..dc99f3e 100644
--- a/net/wireless/reg.c
+++ b/net/wireless/reg.c
@@ -2189,12 +2189,12 @@ static void print_regdomain_info(const struct ieee80211_regdomain *rd)
 }
 
 /* Takes ownership of rd only if it doesn't fail */
-static int __set_regdom(const struct ieee80211_regdomain *rd)
+static int __set_regdom(const struct ieee80211_regdomain *rd,
+			struct regulatory_request *lr)
 {
 	const struct ieee80211_regdomain *regd;
 	const struct ieee80211_regdomain *intersected_rd = NULL;
 	struct wiphy *request_wiphy;
-	struct regulatory_request *lr = get_last_request();
 
 	/* Some basic sanity checks first */
 
@@ -2320,7 +2320,7 @@ int set_regdom(const struct ieee80211_regdomain *rd)
 	lr = get_last_request();
 
 	/* Note that this doesn't update the wiphys, this is done below */
-	r = __set_regdom(rd);
+	r = __set_regdom(rd, lr);
 	if (r) {
 		if (r == -EALREADY)
 			reg_set_request_processed();
-- 
1.8.4.rc3


^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [PATCH 16/19] cfg80211: set core regulatory updates on its own
  2013-11-05 17:17 [PATCH 00/19] cfg80211: regulatory updates Luis R. Rodriguez
                   ` (14 preceding siblings ...)
  2013-11-05 17:18 ` [PATCH 15/19] cfg80211: pass the last_request to __set_regdom() Luis R. Rodriguez
@ 2013-11-05 17:18 ` Luis R. Rodriguez
  2013-11-05 17:18 ` [PATCH 17/19] cfg80211: set user " Luis R. Rodriguez
                   ` (3 subsequent siblings)
  19 siblings, 0 replies; 30+ messages in thread
From: Luis R. Rodriguez @ 2013-11-05 17:18 UTC (permalink / raw)
  To: johannes; +Cc: linux-wireless, Luis R. Rodriguez

This splits up the core regulatory update to be
set on its own helper. This should make it easier
to read exactly what type of requests should be
expected there. In this case its clear that
NL80211_REGDOM_SET_BY_CORE is only used by the
core for updating the world regulatory domain.
This is consistant with the nl80211.h documentation.

Signed-off-by: Luis R. Rodriguez <mcgrof@do-not-panic.com>
---
 net/wireless/reg.c | 37 ++++++++++++++++++++++++-------------
 1 file changed, 24 insertions(+), 13 deletions(-)

diff --git a/net/wireless/reg.c b/net/wireless/reg.c
index dc99f3e..cb20ccc 100644
--- a/net/wireless/reg.c
+++ b/net/wireless/reg.c
@@ -2188,6 +2188,14 @@ static void print_regdomain_info(const struct ieee80211_regdomain *rd)
 	print_rd_rules(rd);
 }
 
+static int reg_set_rd_core(const struct ieee80211_regdomain *rd)
+{
+	if (!is_world_regdom(rd->alpha2))
+		return -EINVAL;
+	update_world_regdomain(rd);
+	return 0;
+}
+
 /* Takes ownership of rd only if it doesn't fail */
 static int __set_regdom(const struct ieee80211_regdomain *rd,
 			struct regulatory_request *lr)
@@ -2196,18 +2204,6 @@ static int __set_regdom(const struct ieee80211_regdomain *rd,
 	const struct ieee80211_regdomain *intersected_rd = NULL;
 	struct wiphy *request_wiphy;
 
-	/* Some basic sanity checks first */
-
-	if (!reg_is_valid_request(rd->alpha2))
-		return -EINVAL;
-
-	if (is_world_regdom(rd->alpha2)) {
-		if (lr->initiator != NL80211_REGDOM_SET_BY_CORE)
-			return -EINVAL;
-		update_world_regdomain(rd);
-		return 0;
-	}
-
 	if (!is_alpha2_set(rd->alpha2) && !is_an_alpha2(rd->alpha2) &&
 	    !is_unknown_alpha2(rd->alpha2))
 		return -EINVAL;
@@ -2317,10 +2313,25 @@ int set_regdom(const struct ieee80211_regdomain *rd)
 	struct regulatory_request *lr;
 	int r;
 
+	if (!reg_is_valid_request(rd->alpha2)) {
+		kfree(rd);
+		return -EINVAL;
+	}
+
 	lr = get_last_request();
 
 	/* Note that this doesn't update the wiphys, this is done below */
-	r = __set_regdom(rd, lr);
+	switch (lr->initiator) {
+	case NL80211_REGDOM_SET_BY_CORE:
+		r = reg_set_rd_core(rd);
+		break;
+	case NL80211_REGDOM_SET_BY_USER:
+	case NL80211_REGDOM_SET_BY_DRIVER:
+	case NL80211_REGDOM_SET_BY_COUNTRY_IE:
+		r = __set_regdom(rd, lr);
+		break;
+	}
+
 	if (r) {
 		if (r == -EALREADY)
 			reg_set_request_processed();
-- 
1.8.4.rc3


^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [PATCH 17/19] cfg80211: set user regulatory updates on its own
  2013-11-05 17:17 [PATCH 00/19] cfg80211: regulatory updates Luis R. Rodriguez
                   ` (15 preceding siblings ...)
  2013-11-05 17:18 ` [PATCH 16/19] cfg80211: set core regulatory updates on its own Luis R. Rodriguez
@ 2013-11-05 17:18 ` Luis R. Rodriguez
  2013-11-05 17:18 ` [PATCH 18/19] cfg80211: set driver " Luis R. Rodriguez
                   ` (2 subsequent siblings)
  19 siblings, 0 replies; 30+ messages in thread
From: Luis R. Rodriguez @ 2013-11-05 17:18 UTC (permalink / raw)
  To: johannes; +Cc: linux-wireless, Luis R. Rodriguez

This splits out the user regulatory update on its
own, this helps simplify reading the case.

Signed-off-by: Luis R. Rodriguez <mcgrof@do-not-panic.com>
---
 net/wireless/reg.c | 35 +++++++++++++++++++++++++++++++++++
 1 file changed, 35 insertions(+)

diff --git a/net/wireless/reg.c b/net/wireless/reg.c
index cb20ccc..fa3bcbe 100644
--- a/net/wireless/reg.c
+++ b/net/wireless/reg.c
@@ -2196,6 +2196,39 @@ static int reg_set_rd_core(const struct ieee80211_regdomain *rd)
 	return 0;
 }
 
+static int reg_set_rd_user(const struct ieee80211_regdomain *rd,
+			   struct regulatory_request *user_request)
+{
+	const struct ieee80211_regdomain *intersected_rd = NULL;
+
+	if (is_world_regdom(rd->alpha2))
+		return -EINVAL;
+
+	if (!regdom_changes(rd->alpha2))
+		return -EALREADY;
+
+	if (!is_valid_rd(rd)) {
+		pr_err("Invalid regulatory domain detected:\n");
+		print_regdomain_info(rd);
+		return -EINVAL;
+	}
+
+	if (!user_request->intersect) {
+		reset_regdomains(false, rd);
+		return 0;
+	}
+
+	intersected_rd = regdom_intersect(rd, get_cfg80211_regdom());
+	if (!intersected_rd)
+		return -EINVAL;
+
+	kfree(rd);
+	rd = NULL;
+	reset_regdomains(false, intersected_rd);
+
+	return 0;
+}
+
 /* Takes ownership of rd only if it doesn't fail */
 static int __set_regdom(const struct ieee80211_regdomain *rd,
 			struct regulatory_request *lr)
@@ -2326,6 +2359,8 @@ int set_regdom(const struct ieee80211_regdomain *rd)
 		r = reg_set_rd_core(rd);
 		break;
 	case NL80211_REGDOM_SET_BY_USER:
+		r = reg_set_rd_user(rd, lr);
+		break;
 	case NL80211_REGDOM_SET_BY_DRIVER:
 	case NL80211_REGDOM_SET_BY_COUNTRY_IE:
 		r = __set_regdom(rd, lr);
-- 
1.8.4.rc3


^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [PATCH 18/19] cfg80211: set driver regulatory updates on its own
  2013-11-05 17:17 [PATCH 00/19] cfg80211: regulatory updates Luis R. Rodriguez
                   ` (16 preceding siblings ...)
  2013-11-05 17:18 ` [PATCH 17/19] cfg80211: set user " Luis R. Rodriguez
@ 2013-11-05 17:18 ` Luis R. Rodriguez
  2013-11-05 17:18 ` [PATCH 19/19] cfg80211: rename __set_regdom() to reg_set_rd_country_ie() Luis R. Rodriguez
  2013-11-11 14:50 ` [PATCH 00/19] cfg80211: regulatory updates Johannes Berg
  19 siblings, 0 replies; 30+ messages in thread
From: Luis R. Rodriguez @ 2013-11-05 17:18 UTC (permalink / raw)
  To: johannes; +Cc: linux-wireless, Luis R. Rodriguez

This splits up the driver regulatory update on its
own, this helps simplify the reading the case.

Signed-off-by: Luis R. Rodriguez <mcgrof@do-not-panic.com>
---
 net/wireless/reg.c | 123 ++++++++++++++++++++++++-----------------------------
 1 file changed, 56 insertions(+), 67 deletions(-)

diff --git a/net/wireless/reg.c b/net/wireless/reg.c
index fa3bcbe..2ecb0c7 100644
--- a/net/wireless/reg.c
+++ b/net/wireless/reg.c
@@ -2229,38 +2229,19 @@ static int reg_set_rd_user(const struct ieee80211_regdomain *rd,
 	return 0;
 }
 
-/* Takes ownership of rd only if it doesn't fail */
-static int __set_regdom(const struct ieee80211_regdomain *rd,
-			struct regulatory_request *lr)
+static int reg_set_rd_driver(const struct ieee80211_regdomain *rd,
+			     struct regulatory_request *driver_request)
 {
 	const struct ieee80211_regdomain *regd;
 	const struct ieee80211_regdomain *intersected_rd = NULL;
+	const struct ieee80211_regdomain *tmp;
 	struct wiphy *request_wiphy;
 
-	if (!is_alpha2_set(rd->alpha2) && !is_an_alpha2(rd->alpha2) &&
-	    !is_unknown_alpha2(rd->alpha2))
+	if (is_world_regdom(rd->alpha2))
 		return -EINVAL;
 
-	/*
-	 * Lets only bother proceeding on the same alpha2 if the current
-	 * rd is non static (it means CRDA was present and was used last)
-	 * and the pending request came in from a country IE
-	 */
-	if (lr->initiator != NL80211_REGDOM_SET_BY_COUNTRY_IE) {
-		/*
-		 * If someone else asked us to change the rd lets only bother
-		 * checking if the alpha2 changes if CRDA was already called
-		 */
-		if (!regdom_changes(rd->alpha2))
-			return -EALREADY;
-	}
-
-	/*
-	 * Now lets set the regulatory domain, update all driver channels
-	 * and finally inform them of what we have done, in case they want
-	 * to review or adjust their own settings based on their own
-	 * internal EEPROM data
-	 */
+	if (!regdom_changes(rd->alpha2))
+		return -EALREADY;
 
 	if (!is_valid_rd(rd)) {
 		pr_err("Invalid regulatory domain detected:\n");
@@ -2268,29 +2249,13 @@ static int __set_regdom(const struct ieee80211_regdomain *rd,
 		return -EINVAL;
 	}
 
-	request_wiphy = wiphy_idx_to_wiphy(lr->wiphy_idx);
-	if (!request_wiphy &&
-	    (lr->initiator == NL80211_REGDOM_SET_BY_DRIVER ||
-	     lr->initiator == NL80211_REGDOM_SET_BY_COUNTRY_IE)) {
+	request_wiphy = wiphy_idx_to_wiphy(driver_request->wiphy_idx);
+	if (!request_wiphy) {
 		schedule_delayed_work(&reg_timeout, 0);
 		return -ENODEV;
 	}
 
-	if (!lr->intersect) {
-		if (lr->initiator != NL80211_REGDOM_SET_BY_DRIVER) {
-			reset_regdomains(false, rd);
-			return 0;
-		}
-
-		/*
-		 * For a driver hint, lets copy the regulatory domain the
-		 * driver wanted to the wiphy to deal with conflicts
-		 */
-
-		/*
-		 * Userspace could have sent two replies with only
-		 * one kernel request.
-		 */
+	if (!driver_request->intersect) {
 		if (request_wiphy->regd)
 			return -EALREADY;
 
@@ -2303,38 +2268,60 @@ static int __set_regdom(const struct ieee80211_regdomain *rd,
 		return 0;
 	}
 
-	/* Intersection requires a bit more work */
+	intersected_rd = regdom_intersect(rd, get_cfg80211_regdom());
+	if (!intersected_rd)
+		return -EINVAL;
 
-	if (lr->initiator != NL80211_REGDOM_SET_BY_COUNTRY_IE) {
-		intersected_rd = regdom_intersect(rd, get_cfg80211_regdom());
-		if (!intersected_rd)
-			return -EINVAL;
+	/*
+	 * We can trash what CRDA provided now.
+	 * However if a driver requested this specific regulatory
+	 * domain we keep it for its private use
+	 */
+	tmp = get_wiphy_regdom(request_wiphy);
+	rcu_assign_pointer(request_wiphy->regd, rd);
+	rcu_free_regdom(tmp);
 
-		/*
-		 * We can trash what CRDA provided now.
-		 * However if a driver requested this specific regulatory
-		 * domain we keep it for its private use
-		 */
-		if (lr->initiator == NL80211_REGDOM_SET_BY_DRIVER) {
-			const struct ieee80211_regdomain *tmp;
+	rd = NULL;
 
-			tmp = get_wiphy_regdom(request_wiphy);
-			rcu_assign_pointer(request_wiphy->regd, rd);
-			rcu_free_regdom(tmp);
-		} else {
-			kfree(rd);
-		}
+	reset_regdomains(false, intersected_rd);
 
-		rd = NULL;
+	return 0;
+}
+
+/* Takes ownership of rd only if it doesn't fail */
+static int __set_regdom(const struct ieee80211_regdomain *rd,
+			struct regulatory_request *lr)
+{
+	struct wiphy *request_wiphy;
 
-		reset_regdomains(false, intersected_rd);
+	if (!is_alpha2_set(rd->alpha2) && !is_an_alpha2(rd->alpha2) &&
+	    !is_unknown_alpha2(rd->alpha2))
+		return -EINVAL;
 
-		return 0;
+	/*
+	 * Lets only bother proceeding on the same alpha2 if the current
+	 * rd is non static (it means CRDA was present and was used last)
+	 * and the pending request came in from a country IE
+	 */
+
+	if (!is_valid_rd(rd)) {
+		pr_err("Invalid regulatory domain detected:\n");
+		print_regdomain_info(rd);
+		return -EINVAL;
 	}
 
-	return -EINVAL;
-}
+	request_wiphy = wiphy_idx_to_wiphy(lr->wiphy_idx);
+	if (!request_wiphy) {
+		schedule_delayed_work(&reg_timeout, 0);
+		return -ENODEV;
+	}
 
+	if (lr->intersect)
+		return -EINVAL;
+
+	reset_regdomains(false, rd);
+	return 0;
+}
 
 /*
  * Use this call to set the current regulatory domain. Conflicts with
@@ -2362,6 +2349,8 @@ int set_regdom(const struct ieee80211_regdomain *rd)
 		r = reg_set_rd_user(rd, lr);
 		break;
 	case NL80211_REGDOM_SET_BY_DRIVER:
+		r = reg_set_rd_driver(rd, lr);
+		break;
 	case NL80211_REGDOM_SET_BY_COUNTRY_IE:
 		r = __set_regdom(rd, lr);
 		break;
-- 
1.8.4.rc3


^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [PATCH 19/19] cfg80211: rename __set_regdom() to reg_set_rd_country_ie()
  2013-11-05 17:17 [PATCH 00/19] cfg80211: regulatory updates Luis R. Rodriguez
                   ` (17 preceding siblings ...)
  2013-11-05 17:18 ` [PATCH 18/19] cfg80211: set driver " Luis R. Rodriguez
@ 2013-11-05 17:18 ` Luis R. Rodriguez
  2013-11-11 14:50 ` [PATCH 00/19] cfg80211: regulatory updates Johannes Berg
  19 siblings, 0 replies; 30+ messages in thread
From: Luis R. Rodriguez @ 2013-11-05 17:18 UTC (permalink / raw)
  To: johannes; +Cc: linux-wireless, Luis R. Rodriguez

This reflects that case is now completely separated.

Signed-off-by: Luis R. Rodriguez <mcgrof@do-not-panic.com>
---
 net/wireless/reg.c | 11 +++++------
 1 file changed, 5 insertions(+), 6 deletions(-)

diff --git a/net/wireless/reg.c b/net/wireless/reg.c
index 2ecb0c7..fb9a44d 100644
--- a/net/wireless/reg.c
+++ b/net/wireless/reg.c
@@ -2288,9 +2288,8 @@ static int reg_set_rd_driver(const struct ieee80211_regdomain *rd,
 	return 0;
 }
 
-/* Takes ownership of rd only if it doesn't fail */
-static int __set_regdom(const struct ieee80211_regdomain *rd,
-			struct regulatory_request *lr)
+static int reg_set_rd_country_ie(const struct ieee80211_regdomain *rd,
+				 struct regulatory_request *country_ie_request)
 {
 	struct wiphy *request_wiphy;
 
@@ -2310,13 +2309,13 @@ static int __set_regdom(const struct ieee80211_regdomain *rd,
 		return -EINVAL;
 	}
 
-	request_wiphy = wiphy_idx_to_wiphy(lr->wiphy_idx);
+	request_wiphy = wiphy_idx_to_wiphy(country_ie_request->wiphy_idx);
 	if (!request_wiphy) {
 		schedule_delayed_work(&reg_timeout, 0);
 		return -ENODEV;
 	}
 
-	if (lr->intersect)
+	if (country_ie_request->intersect)
 		return -EINVAL;
 
 	reset_regdomains(false, rd);
@@ -2352,7 +2351,7 @@ int set_regdom(const struct ieee80211_regdomain *rd)
 		r = reg_set_rd_driver(rd, lr);
 		break;
 	case NL80211_REGDOM_SET_BY_COUNTRY_IE:
-		r = __set_regdom(rd, lr);
+		r = reg_set_rd_country_ie(rd, lr);
 		break;
 	}
 
-- 
1.8.4.rc3


^ permalink raw reply related	[flat|nested] 30+ messages in thread

* Re: [PATCH 01/19] cfg80211: enforce disabling channels by custom or strict settings
  2013-11-05 17:17 ` [PATCH 01/19] cfg80211: enforce disabling channels by custom or strict settings Luis R. Rodriguez
@ 2013-11-06 10:15   ` Johannes Berg
  2013-11-06 16:54     ` Luis R. Rodriguez
  0 siblings, 1 reply; 30+ messages in thread
From: Johannes Berg @ 2013-11-06 10:15 UTC (permalink / raw)
  To: Luis R. Rodriguez; +Cc: linux-wireless

On Tue, 2013-11-05 at 09:17 -0800, Luis R. Rodriguez wrote:

> @@ -1251,7 +1261,7 @@ static void handle_channel_custom(struct wiphy *wiphy,
>  	if (IS_ERR(reg_rule)) {
>  		REG_DBG_PRINT("Disabling freq %d MHz as custom regd has no rule that fits it\n",
>  			      chan->center_freq);
> -		chan->flags = IEEE80211_CHAN_DISABLED;
> +		chan->flags = chan->orig_flags |= IEEE80211_CHAN_DISABLED;

Are you sure the |= is right?

johannes


^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH 03/19] cfg80211: check regulatory request alpha2 early
  2013-11-05 17:18 ` [PATCH 03/19] cfg80211: check regulatory request alpha2 early Luis R. Rodriguez
@ 2013-11-06 10:17   ` Johannes Berg
  2013-11-06 16:39     ` Luis R. Rodriguez
  0 siblings, 1 reply; 30+ messages in thread
From: Johannes Berg @ 2013-11-06 10:17 UTC (permalink / raw)
  To: Luis R. Rodriguez; +Cc: linux-wireless

On Tue, 2013-11-05 at 09:18 -0800, Luis R. Rodriguez wrote:
> Currently nl80211 allows userspace to send the kernel
> a bogus regulatory domain with at most 32 rules set
> and it won't reject it until after its allocated
> memory. Let's be smart about it and take advantage
> that the last_request is now available under RCU
                                               ^^^

I think you mean RTNL?

johannes


^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH 05/19] cfg80211: processing core regulatory hints on its own
  2013-11-05 17:18 ` [PATCH 05/19] cfg80211: processing core regulatory hints on its own Luis R. Rodriguez
@ 2013-11-06 10:24   ` Johannes Berg
  2013-11-06 10:26     ` Johannes Berg
  2013-11-06 16:35     ` Luis R. Rodriguez
  0 siblings, 2 replies; 30+ messages in thread
From: Johannes Berg @ 2013-11-06 10:24 UTC (permalink / raw)
  To: Luis R. Rodriguez; +Cc: linux-wireless

On Tue, 2013-11-05 at 09:18 -0800, Luis R. Rodriguez wrote:

>  	struct wiphy *wiphy = NULL;
> +	enum reg_request_treatment treatment;

This

>  	if (WARN_ON(!reg_request->alpha2))
>  		return;
> @@ -1552,7 +1580,18 @@ static void reg_process_hint(struct regulatory_request *reg_request)
>  		return;
>  	}
>  
> -	switch (__regulatory_hint(wiphy, reg_request)) {
> +	switch (reg_request->initiator) {
> +	case NL80211_REGDOM_SET_BY_CORE:
> +		reg_process_hint_core(reg_request);
> +		return;
> +	case NL80211_REGDOM_SET_BY_USER:
> +	case NL80211_REGDOM_SET_BY_DRIVER:
> +	case NL80211_REGDOM_SET_BY_COUNTRY_IE:
> +		treatment = __regulatory_hint(wiphy, reg_request);
> +		break;
> +	}
> +
> +	switch (treatment) {

is used uninitialized here

johannes


^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH 05/19] cfg80211: processing core regulatory hints on its own
  2013-11-06 10:24   ` Johannes Berg
@ 2013-11-06 10:26     ` Johannes Berg
  2013-11-06 16:35     ` Luis R. Rodriguez
  1 sibling, 0 replies; 30+ messages in thread
From: Johannes Berg @ 2013-11-06 10:26 UTC (permalink / raw)
  To: Luis R. Rodriguez; +Cc: linux-wireless


> > +	switch (treatment) {
> 
> is used uninitialized here

And dropping this patch pretty much stopped me from applying all others,
so I now only applied patch 2 and 4.

johannes


^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH 05/19] cfg80211: processing core regulatory hints on its own
  2013-11-06 10:24   ` Johannes Berg
  2013-11-06 10:26     ` Johannes Berg
@ 2013-11-06 16:35     ` Luis R. Rodriguez
  2013-11-07  0:47       ` Julian Calaby
  1 sibling, 1 reply; 30+ messages in thread
From: Luis R. Rodriguez @ 2013-11-06 16:35 UTC (permalink / raw)
  To: Johannes Berg; +Cc: linux-wireless

On Wed, Nov 6, 2013 at 2:24 AM, Johannes Berg <johannes@sipsolutions.net> wrote:
> On Tue, 2013-11-05 at 09:18 -0800, Luis R. Rodriguez wrote:
>
>>       struct wiphy *wiphy = NULL;
>> +     enum reg_request_treatment treatment;
>
> This
>
>>       if (WARN_ON(!reg_request->alpha2))
>>               return;
>> @@ -1552,7 +1580,18 @@ static void reg_process_hint(struct regulatory_request *reg_request)
>>               return;
>>       }
>>
>> -     switch (__regulatory_hint(wiphy, reg_request)) {
>> +     switch (reg_request->initiator) {
>> +     case NL80211_REGDOM_SET_BY_CORE:
>> +             reg_process_hint_core(reg_request);
>> +             return;

Note that for the core request we bail out early.

>> +     case NL80211_REGDOM_SET_BY_USER:
>> +     case NL80211_REGDOM_SET_BY_DRIVER:
>> +     case NL80211_REGDOM_SET_BY_COUNTRY_IE:
>> +             treatment = __regulatory_hint(wiphy, reg_request);
>> +             break;
>> +     }
>> +
>> +     switch (treatment) {
>
> is used uninitialized here

As I see it in the code we either bail early or its assigned to the
return value of __regulatory_hint() for the non-core case. Did I miss
something?

  Luis

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH 03/19] cfg80211: check regulatory request alpha2 early
  2013-11-06 10:17   ` Johannes Berg
@ 2013-11-06 16:39     ` Luis R. Rodriguez
  0 siblings, 0 replies; 30+ messages in thread
From: Luis R. Rodriguez @ 2013-11-06 16:39 UTC (permalink / raw)
  To: Johannes Berg; +Cc: linux-wireless

On Wed, Nov 6, 2013 at 2:17 AM, Johannes Berg <johannes@sipsolutions.net> wrote:
> On Tue, 2013-11-05 at 09:18 -0800, Luis R. Rodriguez wrote:
>> Currently nl80211 allows userspace to send the kernel
>> a bogus regulatory domain with at most 32 rules set
>> and it won't reject it until after its allocated
>> memory. Let's be smart about it and take advantage
>> that the last_request is now available under RCU
>                                                ^^^
>
> I think you mean RTNL?

I meant through RCU but yes it should also say under RTNL, the routine
already does have NL80211_FLAG_NEED_RTNL, as an internal flag set, so
this should be OK. Care to amend the commit log or want me to do it?

  Luis

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH 01/19] cfg80211: enforce disabling channels by custom or strict settings
  2013-11-06 10:15   ` Johannes Berg
@ 2013-11-06 16:54     ` Luis R. Rodriguez
  0 siblings, 0 replies; 30+ messages in thread
From: Luis R. Rodriguez @ 2013-11-06 16:54 UTC (permalink / raw)
  To: Johannes Berg; +Cc: linux-wireless

On Wed, Nov 6, 2013 at 2:15 AM, Johannes Berg <johannes@sipsolutions.net> wrote:
> On Tue, 2013-11-05 at 09:17 -0800, Luis R. Rodriguez wrote:
>
>> @@ -1251,7 +1261,7 @@ static void handle_channel_custom(struct wiphy *wiphy,
>>       if (IS_ERR(reg_rule)) {
>>               REG_DBG_PRINT("Disabling freq %d MHz as custom regd has no rule that fits it\n",
>>                             chan->center_freq);
>> -             chan->flags = IEEE80211_CHAN_DISABLED;
>> +             chan->flags = chan->orig_flags |= IEEE80211_CHAN_DISABLED;
>
> Are you sure the |= is right?

It works but I think its best to split this in retrospect to make it
clear, I'll send a v2 and I don't think the v2 would interfere with
the series.

  Luis

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH 05/19] cfg80211: processing core regulatory hints on its own
  2013-11-06 16:35     ` Luis R. Rodriguez
@ 2013-11-07  0:47       ` Julian Calaby
  2013-11-11 14:41         ` Johannes Berg
  0 siblings, 1 reply; 30+ messages in thread
From: Julian Calaby @ 2013-11-07  0:47 UTC (permalink / raw)
  To: Luis R. Rodriguez; +Cc: Johannes Berg, linux-wireless

Hi Luis,

On Thu, Nov 7, 2013 at 3:35 AM, Luis R. Rodriguez
<mcgrof@do-not-panic.com> wrote:
> On Wed, Nov 6, 2013 at 2:24 AM, Johannes Berg <johannes@sipsolutions.net> wrote:
>> On Tue, 2013-11-05 at 09:18 -0800, Luis R. Rodriguez wrote:
>>
>>>       struct wiphy *wiphy = NULL;
>>> +     enum reg_request_treatment treatment;
>>
>> This
>>
>>>       if (WARN_ON(!reg_request->alpha2))
>>>               return;
>>> @@ -1552,7 +1580,18 @@ static void reg_process_hint(struct regulatory_request *reg_request)
>>>               return;
>>>       }
>>>
>>> -     switch (__regulatory_hint(wiphy, reg_request)) {
>>> +     switch (reg_request->initiator) {
>>> +     case NL80211_REGDOM_SET_BY_CORE:
>>> +             reg_process_hint_core(reg_request);
>>> +             return;
>
> Note that for the core request we bail out early.
>
>>> +     case NL80211_REGDOM_SET_BY_USER:
>>> +     case NL80211_REGDOM_SET_BY_DRIVER:
>>> +     case NL80211_REGDOM_SET_BY_COUNTRY_IE:
>>> +             treatment = __regulatory_hint(wiphy, reg_request);
>>> +             break;
>>> +     }
>>> +
>>> +     switch (treatment) {
>>
>> is used uninitialized here
>
> As I see it in the code we either bail early or its assigned to the
> return value of __regulatory_hint() for the non-core case. Did I miss
> something?

I think Johannes just wants you to assign something to it so that
paranoid static checkers / compilers won't complain that it's unset in
the potential case where we make it through the switch without hitting
any of the cases. (Adding a default: case to the switch statement
would also work, but as you've been very very careful to spell out all
the cases here that'd tempt developers to remove all the careful
spelling out in some crazed cleanup binge.)

Of course this will never happen if reg_request->initiator is an enum
and all it's possible values are covered in the switch statement, but
some compilers (and developers like me) need it spelled out for them.

Thanks,

-- 
Julian Calaby

Email: julian.calaby@gmail.com
Profile: http://www.google.com/profiles/julian.calaby/
.Plan: http://sites.google.com/site/juliancalaby/

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH 05/19] cfg80211: processing core regulatory hints on its own
  2013-11-07  0:47       ` Julian Calaby
@ 2013-11-11 14:41         ` Johannes Berg
  0 siblings, 0 replies; 30+ messages in thread
From: Johannes Berg @ 2013-11-11 14:41 UTC (permalink / raw)
  To: Julian Calaby; +Cc: Luis R. Rodriguez, linux-wireless

On Thu, 2013-11-07 at 11:47 +1100, Julian Calaby wrote:

> >>> -     switch (__regulatory_hint(wiphy, reg_request)) {
> >>> +     switch (reg_request->initiator) {
> >>> +     case NL80211_REGDOM_SET_BY_CORE:
> >>> +             reg_process_hint_core(reg_request);
> >>> +             return;
> >
> > Note that for the core request we bail out early.
> >
> >>> +     case NL80211_REGDOM_SET_BY_USER:
> >>> +     case NL80211_REGDOM_SET_BY_DRIVER:
> >>> +     case NL80211_REGDOM_SET_BY_COUNTRY_IE:
> >>> +             treatment = __regulatory_hint(wiphy, reg_request);
> >>> +             break;
> >>> +     }
> >>> +
> >>> +     switch (treatment) {
> >>
> >> is used uninitialized here
> >
> > As I see it in the code we either bail early or its assigned to the
> > return value of __regulatory_hint() for the non-core case. Did I miss
> > something?
> 
> I think Johannes just wants you to assign something to it so that
> paranoid static checkers / compilers won't complain that it's unset in
> the potential case where we make it through the switch without hitting
> any of the cases. (Adding a default: case to the switch statement
> would also work, but as you've been very very careful to spell out all
> the cases here that'd tempt developers to remove all the careful
> spelling out in some crazed cleanup binge.)
> 
> Of course this will never happen if reg_request->initiator is an enum
> and all it's possible values are covered in the switch statement, but
> some compilers (and developers like me) need it spelled out for them.

Well actually I was just careless and didn't realize that it was
actually a false compiler warning :)

johannes


^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH 00/19] cfg80211: regulatory updates
  2013-11-05 17:17 [PATCH 00/19] cfg80211: regulatory updates Luis R. Rodriguez
                   ` (18 preceding siblings ...)
  2013-11-05 17:18 ` [PATCH 19/19] cfg80211: rename __set_regdom() to reg_set_rd_country_ie() Luis R. Rodriguez
@ 2013-11-11 14:50 ` Johannes Berg
  19 siblings, 0 replies; 30+ messages in thread
From: Johannes Berg @ 2013-11-11 14:50 UTC (permalink / raw)
  To: Luis R. Rodriguez; +Cc: linux-wireless

Ok, I've applied the remainder of these, substituting v2 for the first
patch and editing some to avoid compiler warnings.

johannes


^ permalink raw reply	[flat|nested] 30+ messages in thread

end of thread, other threads:[~2013-11-11 14:50 UTC | newest]

Thread overview: 30+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-11-05 17:17 [PATCH 00/19] cfg80211: regulatory updates Luis R. Rodriguez
2013-11-05 17:17 ` [PATCH 01/19] cfg80211: enforce disabling channels by custom or strict settings Luis R. Rodriguez
2013-11-06 10:15   ` Johannes Berg
2013-11-06 16:54     ` Luis R. Rodriguez
2013-11-05 17:18 ` [PATCH 02/19] cfg80211: force WIPHY_FLAG_CUSTOM_REGULATORY on wiphy_apply_custom_regulatory() Luis R. Rodriguez
2013-11-05 17:18 ` [PATCH 03/19] cfg80211: check regulatory request alpha2 early Luis R. Rodriguez
2013-11-06 10:17   ` Johannes Berg
2013-11-06 16:39     ` Luis R. Rodriguez
2013-11-05 17:18 ` [PATCH 04/19] cfg80211: remove second argument from reg_process_hint() Luis R. Rodriguez
2013-11-05 17:18 ` [PATCH 05/19] cfg80211: processing core regulatory hints on its own Luis R. Rodriguez
2013-11-06 10:24   ` Johannes Berg
2013-11-06 10:26     ` Johannes Berg
2013-11-06 16:35     ` Luis R. Rodriguez
2013-11-07  0:47       ` Julian Calaby
2013-11-11 14:41         ` Johannes Berg
2013-11-05 17:18 ` [PATCH 06/19] cfg80211: process user regulatory requests " Luis R. Rodriguez
2013-11-05 17:18 ` [PATCH 07/19] cfg80211: process driver " Luis R. Rodriguez
2013-11-05 17:18 ` [PATCH 08/19] cfg80211: process country IE regulatory requests on their own Luis R. Rodriguez
2013-11-05 17:18 ` [PATCH 09/19] cfg80211: process non country IE conflicting first Luis R. Rodriguez
2013-11-05 17:18 ` [PATCH 10/19] cfg80211: add helper for kfree'ing last_request Luis R. Rodriguez
2013-11-05 17:18 ` [PATCH 11/19] cfg80211: add helper for kfree'ing and assigning last_request Luis R. Rodriguez
2013-11-05 17:18 ` [PATCH 12/19] cfg80211: add helper for calling CRDA Luis R. Rodriguez
2013-11-05 17:18 ` [PATCH 13/19] cfg80211: allow only the core to request to update the world regdom Luis R. Rodriguez
2013-11-05 17:18 ` [PATCH 14/19] cfg80211: move core reg_notfier() check to source Luis R. Rodriguez
2013-11-05 17:18 ` [PATCH 15/19] cfg80211: pass the last_request to __set_regdom() Luis R. Rodriguez
2013-11-05 17:18 ` [PATCH 16/19] cfg80211: set core regulatory updates on its own Luis R. Rodriguez
2013-11-05 17:18 ` [PATCH 17/19] cfg80211: set user " Luis R. Rodriguez
2013-11-05 17:18 ` [PATCH 18/19] cfg80211: set driver " Luis R. Rodriguez
2013-11-05 17:18 ` [PATCH 19/19] cfg80211: rename __set_regdom() to reg_set_rd_country_ie() Luis R. Rodriguez
2013-11-11 14:50 ` [PATCH 00/19] cfg80211: regulatory updates Johannes Berg

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).