diff mbox

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

Message ID 5f8caab7e7cfb88072c2e221ea45063e@codeaurora.org (mailing list archive)
State Changes Requested
Delegated to: Johannes Berg
Headers show

Commit Message

Amar Singhal April 13, 2018, 8:32 p.m. UTC 
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 <asinghal@codeaurora.org>

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 <asinghal@codeaurora.org>
Signed-off-by: Kiran Kumar Lokere <klokere@codeaurora.org>
Signed-off-by: Jouni Malinen <jouni@codeaurora.org>
---
  net/wireless/reg.c | 31 +++++++++++++++++++++----------
  1 file changed, 21 insertions(+), 10 deletions(-)


  	/* 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);
  }

Comments

Johannes Berg April 19, 2018, 3:07 p.m. UTC | #1 
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:
> > 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
Amar Singhal April 30, 2018, 10:10 p.m. UTC | #2 
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:
>> 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
diff mbox

Patch

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;
+		}
  	}