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

[asinghal@codeaurora.org]

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