[PATCH 1/2] cfg80211: Enhance semantics for self-managed hints

Subsystems: 802.11 (including cfg80211/nl80211), networking [general], the rest

STALE3015d

6 messages, 3 authors, 2018-04-30 · open the first message on its own page

[PATCH 1/2] cfg80211: Enhance semantics for self-managed hints

From: Jouni Malinen <hidden>
Date: 2018-03-03 19:40:33

From: Amar Singhal <redacted>

Currently, kernel ignores regulatory_hint_user for a wiphy
with flag REGULATORY_WIPHY_SELF_MANAGED set. Wiphy with
REGULATORY_WIPHY_SELF_MANAGED set would ignore beacon hints and country
IE hints and updates from other wiphys in the system, but it may want
to know regulatory setting originating from user-space
(NL80211_REGDOM_SET_BY_USER) that can be trusted
(NL80211_USER_REG_HINT_CELL_BASE). Therefore, conditionally call the
regulatory notifier for a self managed wiphy.

Signed-off-by: Amar Singhal <redacted>
Signed-off-by: Kiran Kumar Lokere <redacted>
Signed-off-by: Jouni Malinen <redacted>
---
 include/net/regulatory.h |  7 ++++---
 net/wireless/reg.c       | 14 ++++++++++----
 2 files changed, 14 insertions(+), 7 deletions(-)
diff --git a/include/net/regulatory.h b/include/net/regulatory.h
index f83cacc..25241c7 100644
--- a/include/net/regulatory.h
+++ b/include/net/regulatory.h
@@ -156,9 +156,10 @@ struct regulatory_request {
  *	system. Conversely, a self-managed wiphy does not share its regulatory
  *	hints with other devices in the system. If a system contains several
  *	devices, one or more of which are self-managed, there might be
- *	contradictory regulatory settings between them. Usage of flag is
- *	generally discouraged. Only use it if the FW/driver is incompatible
- *	with non-locally originated hints.
+ *	contradictory regulatory settings between them. Regulatory information
+ *	from trusted user space sources can still be passed to self-managed
+ *	wiphy. Usage of this flag is generally discouraged. Only use it if the
+ *	FW/driver is incompatible with non-locally originated hints.
  *	This flag is incompatible with the flags: %REGULATORY_CUSTOM_REG,
  *	%REGULATORY_STRICT_REG, %REGULATORY_COUNTRY_IE_FOLLOW_POWER,
  *	%REGULATORY_COUNTRY_IE_IGNORE and %REGULATORY_DISABLE_BEACON_HINTS.
diff --git a/net/wireless/reg.c b/net/wireless/reg.c
index 7b42f0b..4f25a11b 100644
--- a/net/wireless/reg.c
+++ b/net/wireless/reg.c
@@ -2580,7 +2580,7 @@ static void reg_process_hint(struct regulatory_request *reg_request)
 	reg_free_request(reg_request);
 }
 
-static bool reg_only_self_managed_wiphys(void)
+static bool reg_only_self_managed_wiphys(struct regulatory_request *reg_request)
 {
 	struct cfg80211_registered_device *rdev;
 	struct wiphy *wiphy;
@@ -2590,10 +2590,16 @@ static bool reg_only_self_managed_wiphys(void)
 
 	list_for_each_entry(rdev, &cfg80211_rdev_list, list) {
 		wiphy = &rdev->wiphy;
-		if (wiphy->regulatory_flags & REGULATORY_WIPHY_SELF_MANAGED)
+		if (wiphy->regulatory_flags & REGULATORY_WIPHY_SELF_MANAGED) {
 			self_managed_found = true;
-		else
+			if (reg_request->initiator ==
+			    NL80211_REGDOM_SET_BY_USER &&
+			    reg_request->user_reg_hint_type ==
+			    NL80211_USER_REG_HINT_CELL_BASE)
+				reg_call_notifier(wiphy, reg_request);
+		} else {
 			return false;
+		}
 	}
 
 	/* make sure at least one self-managed wiphy exists */
@@ -2631,7 +2637,7 @@ static void reg_process_pending_hints(void)
 
 	spin_unlock(&reg_requests_lock);
 
-	if (reg_only_self_managed_wiphys()) {
+	if (reg_only_self_managed_wiphys(reg_request)) {
 		reg_free_request(reg_request);
 		return;
 	}
-- 
2.7.4

[PATCH 2/2] cfg80211: Modify wiphy registration semantics for self managed hints

From: Jouni Malinen <hidden>
Date: 2018-03-03 19:40:39

From: Amar Singhal <redacted>

Call regulatory notifier conditionally for self managed hints during
wiphy registration. Call when regulatory hint source is
NL80211_REGDOM_SET_BY_USER and sub-type is
NL80211_USER_REG_HINT_CELL_BASE. Call the notifier with last
reg-domain request. This is needed to allow trusted regulatory domain
changes to be processed by the driver.

Signed-off-by: Amar Singhal <redacted>
Signed-off-by: Kiran Kumar Lokere <redacted>
Signed-off-by: Jouni Malinen <redacted>
---
 net/wireless/reg.c | 12 +++++++++---
 1 file changed, 9 insertions(+), 3 deletions(-)
diff --git a/net/wireless/reg.c b/net/wireless/reg.c
index 4f25a11b..48eaa8d 100644
--- a/net/wireless/reg.c
+++ b/net/wireless/reg.c
@@ -3518,15 +3518,21 @@ void wiphy_regulatory_register(struct wiphy *wiphy)
 {
 	struct regulatory_request *lr;
 
-	/* self-managed devices ignore external hints */
-	if (wiphy->regulatory_flags & REGULATORY_WIPHY_SELF_MANAGED)
+	lr = get_last_request();
+
+	/* self-managed devices ignore beacon hints and 11d IE */
+	if (wiphy->regulatory_flags & REGULATORY_WIPHY_SELF_MANAGED) {
 		wiphy->regulatory_flags |= REGULATORY_DISABLE_BEACON_HINTS |
 					   REGULATORY_COUNTRY_IE_IGNORE;
 
+		if (lr->initiator == NL80211_REGDOM_SET_BY_USER &&
+		    lr->user_reg_hint_type == NL80211_USER_REG_HINT_CELL_BASE)
+			reg_call_notifier(wiphy, lr);
+	}
+
 	if (!reg_dev_ignore_cell_hint(wiphy))
 		reg_num_devs_support_basehint++;
 
-	lr = get_last_request();
 	wiphy_update_regulatory(wiphy, lr->initiator);
 	wiphy_all_share_dfs_chan_state(wiphy);
 }
-- 
2.7.4

Re: [PATCH 2/2] cfg80211: Modify wiphy registration semantics for self managed hints

From: Johannes Berg <johannes@sipsolutions.net>
Date: 2018-03-21 10:15:54

So I really think this should just be one patch - it's not about
"registration semantics" but about which types of requests get passed
to reg_notifier(), and if you do it in one place you'd better also do
it in the other.

Secondly, this changes behaviour - not only for ath which presumably is
what you want, but also for
 * brcmfmac
 * brcmsmac
 * libertas
 * mwifiex
 * mt76
 * qtnfmac
 * rtlwifi (& its staging cousin)
 * rsi
 * wlcore
 * rtl8723bs (staging)

So I'm not really convinced we should do this unconditionally.

johannes

Re: [PATCH 2/2] cfg80211: Modify wiphy registration semantics for self managed hints

From: <hidden>
Date: 2018-04-13 20:32:08

hi Johannes,
             please fine some replies inline:

On 2018-03-21 03:15, Johannes Berg wrote:
So I really think this should just be one patch - it's not about
"registration semantics" but about which types of requests get passed
to reg_notifier(), and if you do it in one place you'd better also do
it in the other.
Sure, I have combined the two patches in one patch now:

 From: Amar Singhal [off-list ref]

Call the regulatory notifier for self managed hints only if
initiator is NL80211_REGDOM_SET_BY_USER and hint type is
NL80211_USER_REG_HINT_CELL_BASE. Also call regulatory
notifier when wiphy is registered under similar conditions.

Signed-off-by: Amar Singhal <redacted>
Signed-off-by: Kiran Kumar Lokere <redacted>
Signed-off-by: Jouni Malinen <redacted>
---
  net/wireless/reg.c | 31 +++++++++++++++++++++----------
  1 file changed, 21 insertions(+), 10 deletions(-)
diff --git a/net/wireless/reg.c b/net/wireless/reg.c
index 16c7e4e..d74de76 100644
--- a/net/wireless/reg.c
+++ b/net/wireless/reg.c
@@ -2768,20 +2768,25 @@ static void reg_process_hint(struct 
regulatory_request *reg_request)
  	reg_free_request(reg_request);
  }

-static bool reg_only_self_managed_wiphys(void)
+static bool reg_only_self_managed_wiphys(struct regulatory_request 
*request)
  {
  	struct cfg80211_registered_device *rdev;
  	struct wiphy *wiphy;
-	bool self_managed_found = false;
+	bool self_managed_found = true;

  	ASSERT_RTNL();

  	list_for_each_entry(rdev, &cfg80211_rdev_list, list) {
  		wiphy = &rdev->wiphy;
-		if (wiphy->regulatory_flags & REGULATORY_WIPHY_SELF_MANAGED)
+		if (wiphy->regulatory_flags & REGULATORY_WIPHY_SELF_MANAGED) {
  			self_managed_found = true;
-		else
-			return false;
+			if (request->initiator == NL80211_REGDOM_SET_BY_USER &&
+			    request->user_reg_hint_type ==
+			    NL80211_USER_REG_HINT_CELL_BASE)
+				reg_call_notifier(wiphy, request);
+		} else {
+			self_managed_found = false;
+		}
  	}

  	/* make sure at least one self-managed wiphy exists */
@@ -2819,7 +2824,7 @@ static void reg_process_pending_hints(void)

  	spin_unlock(&reg_requests_lock);

-	if (reg_only_self_managed_wiphys()) {
+	if (reg_only_self_managed_wiphys(reg_request)) {
  		reg_free_request(reg_request);
  		return;
  	}
@@ -3700,15 +3705,21 @@ void wiphy_regulatory_register(struct wiphy 
*wiphy)
  {
  	struct regulatory_request *lr;

-	/* self-managed devices ignore external hints */
-	if (wiphy->regulatory_flags & REGULATORY_WIPHY_SELF_MANAGED)
+	lr = get_last_request();
+
+	/* self-managed devices ignore beacon hints and 11d IE */
+	if (wiphy->regulatory_flags & REGULATORY_WIPHY_SELF_MANAGED) {
  		wiphy->regulatory_flags |= REGULATORY_DISABLE_BEACON_HINTS |
-					   REGULATORY_COUNTRY_IE_IGNORE;
+			REGULATORY_COUNTRY_IE_IGNORE;
+
+		if (lr->initiator == NL80211_REGDOM_SET_BY_USER &&
+		    lr->user_reg_hint_type == NL80211_USER_REG_HINT_CELL_BASE)
+			reg_call_notifier(wiphy, lr);
+	}

  	if (!reg_dev_ignore_cell_hint(wiphy))
  		reg_num_devs_support_basehint++;

-	lr = get_last_request();
  	wiphy_update_regulatory(wiphy, lr->initiator);
  	wiphy_all_share_dfs_chan_state(wiphy);
  }
-- 
1.9.1
Secondly, this changes behaviour - not only for ath which presumably is
what you want, but also for
 * brcmfmac
 * brcmsmac
 * libertas
 * mwifiex
 * mt76
 * qtnfmac
 * rtlwifi (& its staging cousin)
 * rsi
 * wlcore
 * rtl8723bs (staging)

So I'm not really convinced we should do this unconditionally.
I am calling the callback conditionally.
Only couple of drivers are registering regulatory flag 
REGULATORY_WIPHY_SELF_MANAGED:
drivers/net/wireless/intel/iwlwifi/mvm/mac80211.c:              
quoted
hw->wiphy->regulatory_flags |= REGULATORY_WIPHY_SELF_MANAGED;
above driver does not register reg_notifier callback
drivers/net/wireless/quantenna/qtnfmac/cfg80211.c:              
quoted
wiphy->regulatory_flags |= REGULATORY_WIPHY_SELF_MANAGED;
drivers/net/wireless/quantenna/qtnfmac/cfg80211.c:
(wiphy->regulatory_flags & REGULATORY_WIPHY_SELF_MANAGED)
Above driver registers reg_notifier callback only when flag 
REGULATORY_WIPHY_SELF_MANAGED is not defined.

Also, in this change, when REGULATORY_WIPHY_SELF_MANAGED is defined, the 
callback is conditional on request initiator being 
NL80211_REGDOM_SET_BY_USER and user_reg_hint_type being 
NL80211_USER_REG_HINT_CELL_BASE.
Johannes
regards,
Amar

Re: [PATCH 2/2] cfg80211: Modify wiphy registration semantics for self managed hints

From: Johannes Berg <johannes@sipsolutions.net>
Date: 2018-04-19 15:07:11

On Fri, 2018-04-13 at 13:32 -0700, asinghal@codeaurora.org wrote:
hi Johannes,
             please fine some replies inline:

On 2018-03-21 03:15, Johannes Berg wrote:
quoted
So I really think this should just be one patch - it's not about
"registration semantics" but about which types of requests get passed
to reg_notifier(), and if you do it in one place you'd better also do
it in the other.
Sure, I have combined the two patches in one patch now:
So now you should probably resend it properly, with a new subject that
explains it better? Just "modify" doesn't really seem all that
appropriate - what's the modification?

Patchwork also lost half the patch for some reason, probably you
copy/pasted it and lost some whitespace at an empty line.
Call the regulatory notifier for self managed hints only if
initiator is NL80211_REGDOM_SET_BY_USER and hint type is
NL80211_USER_REG_HINT_CELL_BASE. Also call regulatory
notifier when wiphy is registered under similar conditions.
I guess this should say why.
  	list_for_each_entry(rdev, &cfg80211_rdev_list, list) {
  		wiphy = &rdev->wiphy;
-		if (wiphy->regulatory_flags & REGULATORY_WIPHY_SELF_MANAGED)
+		if (wiphy->regulatory_flags & REGULATORY_WIPHY_SELF_MANAGED) {
  			self_managed_found = true;
-		else
-			return false;
+			if (request->initiator == NL80211_REGDOM_SET_BY_USER &&
+			    request->user_reg_hint_type ==
+			    NL80211_USER_REG_HINT_CELL_BASE)
+				reg_call_notifier(wiphy, request);
+		} else {
+			self_managed_found = false;
+		}
  	}
This is awkward now - how about

self_managed_found = regulatory_flags & SELF_MANAGED;
if (self_managed_found &&
    request->initiator == ... &&
    ...)
        reg_call_notifier(...)

quoted hunk
@@ -3700,15 +3705,21 @@ void wiphy_regulatory_register(struct wiphy 
*wiphy)
  {
  	struct regulatory_request *lr;

-	/* self-managed devices ignore external hints */
-	if (wiphy->regulatory_flags & REGULATORY_WIPHY_SELF_MANAGED)
+	lr = get_last_request();
+
+	/* self-managed devices ignore beacon hints and 11d IE */
+	if (wiphy->regulatory_flags & REGULATORY_WIPHY_SELF_MANAGED) {
  		wiphy->regulatory_flags |= REGULATORY_DISABLE_BEACON_HINTS |
-					   REGULATORY_COUNTRY_IE_IGNORE;
+			REGULATORY_COUNTRY_IE_IGNORE;
no need to change the indentation here

johannes

Re: [PATCH 2/2] cfg80211: Modify wiphy registration semantics for self managed hints

From: <hidden>
Date: 2018-04-30 22:10:32

hi Johannes,
             replies inline:

On 2018-04-19 08:07, Johannes Berg wrote:
On Fri, 2018-04-13 at 13:32 -0700, asinghal@codeaurora.org wrote:
quoted
hi Johannes,
             please fine some replies inline:

On 2018-03-21 03:15, Johannes Berg wrote:
quoted
So I really think this should just be one patch - it's not about
"registration semantics" but about which types of requests get passed
to reg_notifier(), and if you do it in one place you'd better also do
it in the other.
Sure, I have combined the two patches in one patch now:
So now you should probably resend it properly, with a new subject that
explains it better? Just "modify" doesn't really seem all that
appropriate - what's the modification?

Patchwork also lost half the patch for some reason, probably you
copy/pasted it and lost some whitespace at an empty line.
We have posted a new patch-set with better commit title and message.
quoted
Call the regulatory notifier for self managed hints only if
initiator is NL80211_REGDOM_SET_BY_USER and hint type is
NL80211_USER_REG_HINT_CELL_BASE. Also call regulatory
notifier when wiphy is registered under similar conditions.
I guess this should say why.
quoted
  	list_for_each_entry(rdev, &cfg80211_rdev_list, list) {
  		wiphy = &rdev->wiphy;
-		if (wiphy->regulatory_flags & REGULATORY_WIPHY_SELF_MANAGED)
+		if (wiphy->regulatory_flags & REGULATORY_WIPHY_SELF_MANAGED) {
  			self_managed_found = true;
-		else
-			return false;
+			if (request->initiator == NL80211_REGDOM_SET_BY_USER &&
+			    request->user_reg_hint_type ==
+			    NL80211_USER_REG_HINT_CELL_BASE)
+				reg_call_notifier(wiphy, request);
+		} else {
+			self_managed_found = false;
+		}
  	}
This is awkward now - how about

self_managed_found = regulatory_flags & SELF_MANAGED;
if (self_managed_found &&
    request->initiator == ... &&
    ...)
        reg_call_notifier(...)
This has been addressed in the new patch with a separate notify 
function.
quoted
@@ -3700,15 +3705,21 @@ void wiphy_regulatory_register(struct wiphy
*wiphy)
  {
  	struct regulatory_request *lr;

-	/* self-managed devices ignore external hints */
-	if (wiphy->regulatory_flags & REGULATORY_WIPHY_SELF_MANAGED)
+	lr = get_last_request();
+
+	/* self-managed devices ignore beacon hints and 11d IE */
+	if (wiphy->regulatory_flags & REGULATORY_WIPHY_SELF_MANAGED) {
  		wiphy->regulatory_flags |= REGULATORY_DISABLE_BEACON_HINTS |
-					   REGULATORY_COUNTRY_IE_IGNORE;
+			REGULATORY_COUNTRY_IE_IGNORE;
no need to change the indentation here
has been addressed in new patch.
thanks,
amar
johannes
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help