[PATCH 3.7 1/2] cfg80211: fix antenna gain handling

[PATCH 3.7 1/2] cfg80211: fix antenna gain handling

[Felix Fietkau]

When not intersecting, orig->chan_mag is initialized to zero. Because of
that, the regulatory antenna gain limit never gets set.

Signed-off-by: Felix Fietkau <nbd@openwrt.org>
Cc: stable@vger.kernel.org
---
 net/wireless/reg.c |    3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/net/wireless/reg.c b/net/wireless/reg.c
index 3b8cbbc..1a16f19 100644
--- a/net/wireless/reg.c
+++ b/net/wireless/reg.c
@@ -915,8 +915,7 @@ static void handle_channel(struct wiphy *wiphy,
 
 	chan->beacon_found = false;
 	chan->flags = flags | bw_flags | map_regdom_flags(reg_rule->flags);
-	chan->max_antenna_gain = min(chan->orig_mag,
-		(int) MBI_TO_DBI(power_rule->max_antenna_gain));
+	chan->max_antenna_gain = (int) MBI_TO_DBI(power_rule->max_antenna_gain);
 	chan->max_reg_power = (int) MBM_TO_DBM(power_rule->max_eirp);
 	if (chan->orig_mpwr) {
 		/*
-- 
1.7.9.6 (Apple Git-31.1)

[PATCH 3.7 2/2] cfg80211: fix initialization of chan->max_reg_power

[Felix Fietkau]

A few places touch chan->max_power based on updated tx power rules, but
forget to do the same to chan->max_reg_power.

Signed-off-by: Felix Fietkau <nbd@openwrt.org>
Cc: stable@vger.kernel.org
---
 net/wireless/reg.c |    5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/net/wireless/reg.c b/net/wireless/reg.c
index 1a16f19..7062475 100644
--- a/net/wireless/reg.c
+++ b/net/wireless/reg.c
@@ -908,7 +908,7 @@ static void handle_channel(struct wiphy *wiphy,
 			map_regdom_flags(reg_rule->flags) | bw_flags;
 		chan->max_antenna_gain = chan->orig_mag =
 			(int) MBI_TO_DBI(power_rule->max_antenna_gain);
-		chan->max_power = chan->orig_mpwr =
+		chan->max_reg_power = chan->max_power = chan->orig_mpwr =
 			(int) MBM_TO_DBM(power_rule->max_eirp);
 		return;
 	}
@@ -1330,7 +1330,8 @@ static void handle_channel_custom(struct wiphy *wiphy,
 
 	chan->flags |= map_regdom_flags(reg_rule->flags) | bw_flags;
 	chan->max_antenna_gain = (int) MBI_TO_DBI(power_rule->max_antenna_gain);
-	chan->max_power = (int) MBM_TO_DBM(power_rule->max_eirp);
+	chan->max_reg_power = chan->max_power =
+		(int) MBM_TO_DBM(power_rule->max_eirp);
 }
 
 static void handle_band_custom(struct wiphy *wiphy, enum ieee80211_band band,
-- 
1.7.9.6 (Apple Git-31.1)

Re: [PATCH 3.7 1/2] cfg80211: fix antenna gain handling

[John W. Linville]

Anyone object?  I suspect that Johannes may be a little more
busy/distracted than usual for the next few days/weeks...

John

On Wed, Oct 03, 2012 at 07:51:46PM +0200, Felix Fietkau wrote:
> When not intersecting, orig->chan_mag is initialized to zero. Because of
> that, the regulatory antenna gain limit never gets set.
> 
> Signed-off-by: Felix Fietkau <nbd@openwrt.org>
> Cc: stable@vger.kernel.org
> ---
>  net/wireless/reg.c |    3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/net/wireless/reg.c b/net/wireless/reg.c
> index 3b8cbbc..1a16f19 100644
> --- a/net/wireless/reg.c
> +++ b/net/wireless/reg.c
> @@ -915,8 +915,7 @@ static void handle_channel(struct wiphy *wiphy,
>  
>  	chan->beacon_found = false;
>  	chan->flags = flags | bw_flags | map_regdom_flags(reg_rule->flags);
> -	chan->max_antenna_gain = min(chan->orig_mag,
> -		(int) MBI_TO_DBI(power_rule->max_antenna_gain));
> +	chan->max_antenna_gain = (int) MBI_TO_DBI(power_rule->max_antenna_gain);
>  	chan->max_reg_power = (int) MBM_TO_DBM(power_rule->max_eirp);
>  	if (chan->orig_mpwr) {
>  		/*
> -- 
> 1.7.9.6 (Apple Git-31.1)
> 
> 

-- 
John W. Linville		Someday the world will need a hero, and you
linville@tuxdriver.com			might be all we have.  Be ready.

Re: [PATCH 3.7 2/2] cfg80211: fix initialization of chan->max_reg_power

[John W. Linville]

Same here -- anyone object?  I suspect that Johannes may be a little
more busy/distracted than usual for the next few days/weeks...

John

On Wed, Oct 03, 2012 at 07:51:47PM +0200, Felix Fietkau wrote:
> A few places touch chan->max_power based on updated tx power rules, but
> forget to do the same to chan->max_reg_power.
> 
> Signed-off-by: Felix Fietkau <nbd@openwrt.org>
> Cc: stable@vger.kernel.org
> ---
>  net/wireless/reg.c |    5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/net/wireless/reg.c b/net/wireless/reg.c
> index 1a16f19..7062475 100644
> --- a/net/wireless/reg.c
> +++ b/net/wireless/reg.c
> @@ -908,7 +908,7 @@ static void handle_channel(struct wiphy *wiphy,
>  			map_regdom_flags(reg_rule->flags) | bw_flags;
>  		chan->max_antenna_gain = chan->orig_mag =
>  			(int) MBI_TO_DBI(power_rule->max_antenna_gain);
> -		chan->max_power = chan->orig_mpwr =
> +		chan->max_reg_power = chan->max_power = chan->orig_mpwr =
>  			(int) MBM_TO_DBM(power_rule->max_eirp);
>  		return;
>  	}
> @@ -1330,7 +1330,8 @@ static void handle_channel_custom(struct wiphy *wiphy,
>  
>  	chan->flags |= map_regdom_flags(reg_rule->flags) | bw_flags;
>  	chan->max_antenna_gain = (int) MBI_TO_DBI(power_rule->max_antenna_gain);
> -	chan->max_power = (int) MBM_TO_DBM(power_rule->max_eirp);
> +	chan->max_reg_power = chan->max_power =
> +		(int) MBM_TO_DBM(power_rule->max_eirp);
>  }
>  
>  static void handle_band_custom(struct wiphy *wiphy, enum ieee80211_band band,
> -- 
> 1.7.9.6 (Apple Git-31.1)
> 
> 

-- 
John W. Linville		Someday the world will need a hero, and you
linville@tuxdriver.com			might be all we have.  Be ready.

Re: [PATCH 3.7 1/2] cfg80211: fix antenna gain handling

[Arend van Spriel]

On 10/05/2012 10:22 PM, John W. Linville wrote:
> Anyone object?  I suspect that Johannes may be a little more
> busy/distracted than usual for the next few days/weeks...
>
> John
>
> On Wed, Oct 03, 2012 at 07:51:46PM +0200, Felix Fietkau wrote:
>> When not intersecting, orig->chan_mag is initialized to zero. Because of
>> that, the regulatory antenna gain limit never gets set.
>>
>> Signed-off-by: Felix Fietkau <nbd@openwrt.org>
>> Cc: stable@vger.kernel.org
>> ---
>>   net/wireless/reg.c |    3 +--
>>   1 file changed, 1 insertion(+), 2 deletions(-)
>>
>> diff --git a/net/wireless/reg.c b/net/wireless/reg.c
>> index 3b8cbbc..1a16f19 100644
>> --- a/net/wireless/reg.c
>> +++ b/net/wireless/reg.c
>> @@ -915,8 +915,7 @@ static void handle_channel(struct wiphy *wiphy,
>>
>>   	chan->beacon_found = false;
>>   	chan->flags = flags | bw_flags | map_regdom_flags(reg_rule->flags);
>> -	chan->max_antenna_gain = min(chan->orig_mag,
>> -		(int) MBI_TO_DBI(power_rule->max_antenna_gain));

Just reading the commit message, I would think it better to initialize 
chan->orig_mag to INT_MAX. Just my 0.02$

Gr. AvS

>> +	chan->max_antenna_gain = (int) MBI_TO_DBI(power_rule->max_antenna_gain);
>>   	chan->max_reg_power = (int) MBM_TO_DBM(power_rule->max_eirp);
>>   	if (chan->orig_mpwr) {
>>   		/*
>> --
>> 1.7.9.6 (Apple Git-31.1)
>>
>>
>

Re: [PATCH 3.7 1/2] cfg80211: fix antenna gain handling

[Felix Fietkau]

On 2012-10-05 11:02 PM, Arend van Spriel wrote:
> On 10/05/2012 10:22 PM, John W. Linville wrote:
>> Anyone object?  I suspect that Johannes may be a little more
>> busy/distracted than usual for the next few days/weeks...
>>
>> John
>>
>> On Wed, Oct 03, 2012 at 07:51:46PM +0200, Felix Fietkau wrote:
>>> When not intersecting, orig->chan_mag is initialized to zero. Because of
>>> that, the regulatory antenna gain limit never gets set.
>>>
>>> Signed-off-by: Felix Fietkau <nbd@openwrt.org>
>>> Cc: stable@vger.kernel.org
>>> ---
>>>   net/wireless/reg.c |    3 +--
>>>   1 file changed, 1 insertion(+), 2 deletions(-)
>>>
>>> diff --git a/net/wireless/reg.c b/net/wireless/reg.c
>>> index 3b8cbbc..1a16f19 100644
>>> --- a/net/wireless/reg.c
>>> +++ b/net/wireless/reg.c
>>> @@ -915,8 +915,7 @@ static void handle_channel(struct wiphy *wiphy,
>>>
>>>   	chan->beacon_found = false;
>>>   	chan->flags = flags | bw_flags | map_regdom_flags(reg_rule->flags);
>>> -	chan->max_antenna_gain = min(chan->orig_mag,
>>> -		(int) MBI_TO_DBI(power_rule->max_antenna_gain));
> 
> Just reading the commit message, I would think it better to initialize 
> chan->orig_mag to INT_MAX. Just my 0.02$
I'm not sure it's useful for the driver to be able to add initial max
antenna gain restrictions that way (which IMHO is the only point to
using chan->orig_mag).
I think it's better (and more predictable) to just stick to the power
rules from the regulatory information.

- Felix

Re: [PATCH 3.7 1/2] cfg80211: fix antenna gain handling

[Arend van Spriel]

On 10/06/2012 01:15 AM, Felix Fietkau wrote:
> On 2012-10-05 11:02 PM, Arend van Spriel wrote:
>> On 10/05/2012 10:22 PM, John W. Linville wrote:
>>> Anyone object?  I suspect that Johannes may be a little more
>>> busy/distracted than usual for the next few days/weeks...
>>>
>>> John
>>>
>>> On Wed, Oct 03, 2012 at 07:51:46PM +0200, Felix Fietkau wrote:
>>>> When not intersecting, orig->chan_mag is initialized to zero. Because of
>>>> that, the regulatory antenna gain limit never gets set.
>>>>
>>>> Signed-off-by: Felix Fietkau <nbd@openwrt.org>
>>>> Cc: stable@vger.kernel.org
>>>> ---
>>>>    net/wireless/reg.c |    3 +--
>>>>    1 file changed, 1 insertion(+), 2 deletions(-)
>>>>
>>>> diff --git a/net/wireless/reg.c b/net/wireless/reg.c
>>>> index 3b8cbbc..1a16f19 100644
>>>> --- a/net/wireless/reg.c
>>>> +++ b/net/wireless/reg.c
>>>> @@ -915,8 +915,7 @@ static void handle_channel(struct wiphy *wiphy,
>>>>
>>>>    	chan->beacon_found = false;
>>>>    	chan->flags = flags | bw_flags | map_regdom_flags(reg_rule->flags);
>>>> -	chan->max_antenna_gain = min(chan->orig_mag,
>>>> -		(int) MBI_TO_DBI(power_rule->max_antenna_gain));
>>
>> Just reading the commit message, I would think it better to initialize
>> chan->orig_mag to INT_MAX. Just my 0.02$
> I'm not sure it's useful for the driver to be able to add initial max
> antenna gain restrictions that way (which IMHO is the only point to
> using chan->orig_mag).
> I think it's better (and more predictable) to just stick to the power
> rules from the regulatory information.
>
> - Felix
>

Ok,

I was not understanding the usage scenario here. Your patch will ignore 
any driver initialized max antenna gain, which does not sound right to 
me. What if chan->orig_mag is a sensible (non-zero) value, which is 
lower than the value in regulatory power rule? Just seems that the patch 
changes more than the commit message it saying.

Gr. AvS

Re: [PATCH 3.7 1/2] cfg80211: fix antenna gain handling

[Felix Fietkau]

On 2012-10-06 10:01 AM, Arend van Spriel wrote:
> On 10/06/2012 01:15 AM, Felix Fietkau wrote:
>> On 2012-10-05 11:02 PM, Arend van Spriel wrote:
>>> On 10/05/2012 10:22 PM, John W. Linville wrote:
>>>> Anyone object?  I suspect that Johannes may be a little more
>>>> busy/distracted than usual for the next few days/weeks...
>>>>
>>>> John
>>>>
>>>> On Wed, Oct 03, 2012 at 07:51:46PM +0200, Felix Fietkau wrote:
>>>>> When not intersecting, orig->chan_mag is initialized to zero. Because of
>>>>> that, the regulatory antenna gain limit never gets set.
>>>>>
>>>>> Signed-off-by: Felix Fietkau <nbd@openwrt.org>
>>>>> Cc: stable@vger.kernel.org
>>>>> ---
>>>>>    net/wireless/reg.c |    3 +--
>>>>>    1 file changed, 1 insertion(+), 2 deletions(-)
>>>>>
>>>>> diff --git a/net/wireless/reg.c b/net/wireless/reg.c
>>>>> index 3b8cbbc..1a16f19 100644
>>>>> --- a/net/wireless/reg.c
>>>>> +++ b/net/wireless/reg.c
>>>>> @@ -915,8 +915,7 @@ static void handle_channel(struct wiphy *wiphy,
>>>>>
>>>>>    	chan->beacon_found = false;
>>>>>    	chan->flags = flags | bw_flags | map_regdom_flags(reg_rule->flags);
>>>>> -	chan->max_antenna_gain = min(chan->orig_mag,
>>>>> -		(int) MBI_TO_DBI(power_rule->max_antenna_gain));
>>>
>>> Just reading the commit message, I would think it better to initialize
>>> chan->orig_mag to INT_MAX. Just my 0.02$
>> I'm not sure it's useful for the driver to be able to add initial max
>> antenna gain restrictions that way (which IMHO is the only point to
>> using chan->orig_mag).
>> I think it's better (and more predictable) to just stick to the power
>> rules from the regulatory information.
>>
>> - Felix
>>
> 
> Ok,
> 
> I was not understanding the usage scenario here. Your patch will ignore 
> any driver initialized max antenna gain, which does not sound right to 
> me. What if chan->orig_mag is a sensible (non-zero) value, which is 
> lower than the value in regulatory power rule? Just seems that the patch 
> changes more than the commit message it saying.
I did take another close look at the code and it seems you're right. I
will change the code so that it initializes chan->orig_mag to INT_MAX in
wiphy_register, and I will make the patch description a bit more verbose
as well.
That way the chan->max_antenna_gain value supplied by the driver gets
ignored, which is a good thing, considering that only one driver
actually uses max_antenna_gain, and no driver initializes it to
something useful.

- Felix

Re: [PATCH 3.7 1/2] cfg80211: fix antenna gain handling

[Johannes Berg]

On Sat, 2012-10-06 at 14:26 +0200, Felix Fietkau wrote:

> > I was not understanding the usage scenario here. Your patch will ignore 
> > any driver initialized max antenna gain, which does not sound right to 
> > me. What if chan->orig_mag is a sensible (non-zero) value, which is 
> > lower than the value in regulatory power rule? Just seems that the patch 
> > changes more than the commit message it saying.
> I did take another close look at the code and it seems you're right. I
> will change the code so that it initializes chan->orig_mag to INT_MAX in
> wiphy_register, and I will make the patch description a bit more verbose
> as well.
> That way the chan->max_antenna_gain value supplied by the driver gets
> ignored, which is a good thing, considering that only one driver
> actually uses max_antenna_gain, and no driver initializes it to
> something useful.

I know we need the "limit to driver value" behaviour for TX power for
sure (otherwise the firmware would assert if it goes over)

johannes

Re: [PATCH 3.7 1/2] cfg80211: fix antenna gain handling

[Felix Fietkau]

On 2012-10-06 4:47 PM, Johannes Berg wrote:
> On Sat, 2012-10-06 at 14:26 +0200, Felix Fietkau wrote:
> 
>> > I was not understanding the usage scenario here. Your patch will ignore 
>> > any driver initialized max antenna gain, which does not sound right to 
>> > me. What if chan->orig_mag is a sensible (non-zero) value, which is 
>> > lower than the value in regulatory power rule? Just seems that the patch 
>> > changes more than the commit message it saying.
>> I did take another close look at the code and it seems you're right. I
>> will change the code so that it initializes chan->orig_mag to INT_MAX in
>> wiphy_register, and I will make the patch description a bit more verbose
>> as well.
>> That way the chan->max_antenna_gain value supplied by the driver gets
>> ignored, which is a good thing, considering that only one driver
>> actually uses max_antenna_gain, and no driver initializes it to
>> something useful.
> 
> I know we need the "limit to driver value" behaviour for TX power for
> sure (otherwise the firmware would assert if it goes over)
Sure. This isn't really necessary for max_antenna_gain though, it's up
to the driver if it even wants to make use of this (mac80211 doesn't).
ath9k uses it to check if tx power needs to be reduced, when an
eeprom-specified antenna gain exceeds the regulatory max antenna gain.

I only noticed this, because I'm going to submit a patch soon to add a
configurable antenna gain to nl80211/mac80211. It is useful for setups
where you have a highly directional antenna with a big antenna gain and
want to make sure that the txpower stays within legal limits.

- Felix