From: Amar Singhal <[email protected]>
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 <[email protected]>
Signed-off-by: Kiran Kumar Lokere <[email protected]>
Signed-off-by: Jouni Malinen <[email protected]>
---
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(®_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
From: Amar Singhal <[email protected]>
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 <[email protected]>
Signed-off-by: Kiran Kumar Lokere <[email protected]>
Signed-off-by: Jouni Malinen <[email protected]>
---
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
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
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 <[email protected]>
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 <[email protected]>
Signed-off-by: Kiran Kumar Lokere <[email protected]>
Signed-off-by: Jouni Malinen <[email protected]>
---
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(®_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:
> >hw->wiphy->regulatory_flags |= REGULATORY_WIPHY_SELF_MANAGED;
above driver does not register reg_notifier callback
> drivers/net/wireless/quantenna/qtnfmac/cfg80211.c:
> >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
On Fri, 2018-04-13 at 13:32 -0700, [email protected] wrote:
> 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:
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(...)
> @@ -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
hi Johannes,
replies inline:
On 2018-04-19 08:07, Johannes Berg wrote:
> On Fri, 2018-04-13 at 13:32 -0700, [email protected] wrote:
>> 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:
>
> 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.
>> 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(...)
This has been addressed in the new patch with a separate notify
function.
>
>
>> @@ -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