2013-09-29 12:48:06

by Felix Fietkau

[permalink] [raw]
Subject: [PATCH 3.12] mac80211: fix a tx power handling regression

commit 1ea6f9c0d48b11b6ec3ec4b5579ec74fc3951cf8
"mac80211: handle TX power per virtual interface"

This commit added support for tracking tx power configuration for
multiple interfaces, however instead of using the maximum value of all
virtual interfaces, it uses the minimum.

This causes the configured tx power to be reset to the absolute minimum
for all virtual interfaces, whenever an interface is created and destroyed
immediately afterwards.

Cc: [email protected]
Signed-off-by: Felix Fietkau <[email protected]>
---
net/mac80211/main.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/net/mac80211/main.c b/net/mac80211/main.c
index 21d5d44..87c5509 100644
--- a/net/mac80211/main.c
+++ b/net/mac80211/main.c
@@ -97,7 +97,7 @@ static u32 ieee80211_hw_conf_chan(struct ieee80211_local *local)
struct ieee80211_sub_if_data *sdata;
struct cfg80211_chan_def chandef = {};
u32 changed = 0;
- int power;
+ int power = 0;
u32 offchannel_flag;

offchannel_flag = local->hw.conf.flags & IEEE80211_CONF_OFFCHANNEL;
@@ -142,16 +142,16 @@ static u32 ieee80211_hw_conf_chan(struct ieee80211_local *local)
changed |= IEEE80211_CONF_CHANGE_SMPS;
}

- power = ieee80211_chandef_max_power(&chandef);
-
rcu_read_lock();
list_for_each_entry_rcu(sdata, &local->interfaces, list) {
if (!rcu_access_pointer(sdata->vif.chanctx_conf))
continue;
- power = min(power, sdata->vif.bss_conf.txpower);
+ power = max(power, sdata->vif.bss_conf.txpower);
}
rcu_read_unlock();

+ power = min(power, ieee80211_chandef_max_power(&chandef));
+
if (local->hw.conf.power_level != power) {
changed |= IEEE80211_CONF_CHANGE_POWER;
local->hw.conf.power_level = power;
--
1.8.0.2



2013-09-30 09:09:22

by Johannes Berg

[permalink] [raw]
Subject: Re: [PATCH 3.12] mac80211: fix a tx power handling regression

On Sun, 2013-09-29 at 14:48 +0200, Felix Fietkau wrote:
> commit 1ea6f9c0d48b11b6ec3ec4b5579ec74fc3951cf8
> "mac80211: handle TX power per virtual interface"
>
> This commit added support for tracking tx power configuration for
> multiple interfaces, however instead of using the maximum value of all
> virtual interfaces, it uses the minimum.

I'm not sure it should be using the maximum? What if the AP required
lowering TX power by way of TPC for example?

johannes


2013-09-30 10:38:21

by Johannes Berg

[permalink] [raw]
Subject: Re: [PATCH 3.12] mac80211: fix a tx power handling regression

On Mon, 2013-09-30 at 11:43 +0200, Felix Fietkau wrote:
> On 2013-09-30 11:09 AM, Johannes Berg wrote:
> > On Sun, 2013-09-29 at 14:48 +0200, Felix Fietkau wrote:
> >> commit 1ea6f9c0d48b11b6ec3ec4b5579ec74fc3951cf8
> >> "mac80211: handle TX power per virtual interface"
> >>
> >> This commit added support for tracking tx power configuration for
> >> multiple interfaces, however instead of using the maximum value of all
> >> virtual interfaces, it uses the minimum.
> >
> > I'm not sure it should be using the maximum? What if the AP required
> > lowering TX power by way of TPC for example?
> Shouldn't that only affect the virtual interface that is connected to
> that AP?

Yes, but not all drivers support per-interface TX power I guess?

> If there's a regulatory requirement to use lower tx power, it should be
> tracked as a limit somewhere else instead of implicitly being handled
> via vif tx power configuration.

Not sure I see why? It's an absolute value after we do the calculations
in that interface that has the TPC.

johannes


2013-09-30 10:51:06

by Felix Fietkau

[permalink] [raw]
Subject: Re: [PATCH 3.12] mac80211: fix a tx power handling regression

On 2013-09-30 12:38 PM, Johannes Berg wrote:
> On Mon, 2013-09-30 at 11:43 +0200, Felix Fietkau wrote:
>> On 2013-09-30 11:09 AM, Johannes Berg wrote:
>> > On Sun, 2013-09-29 at 14:48 +0200, Felix Fietkau wrote:
>> >> commit 1ea6f9c0d48b11b6ec3ec4b5579ec74fc3951cf8
>> >> "mac80211: handle TX power per virtual interface"
>> >>
>> >> This commit added support for tracking tx power configuration for
>> >> multiple interfaces, however instead of using the maximum value of all
>> >> virtual interfaces, it uses the minimum.
>> >
>> > I'm not sure it should be using the maximum? What if the AP required
>> > lowering TX power by way of TPC for example?
>> Shouldn't that only affect the virtual interface that is connected to
>> that AP?
> Yes, but not all drivers support per-interface TX power I guess?
>
>> If there's a regulatory requirement to use lower tx power, it should be
>> tracked as a limit somewhere else instead of implicitly being handled
>> via vif tx power configuration.
>
> Not sure I see why? It's an absolute value after we do the calculations
> in that interface that has the TPC.
Maybe we need to rework this somehow, but in the mean time, this patch
fixes a serious regression that I've been looking into for a while now.
I haven't worked out the exact conditions that trigger this yet, but
often when an AP VLAN gets destroyed and recreated, or when a new
temporary interface is brought up and then down again, the tx power for
*all* interfaces gets reset to the lowest possible level.

- Felix

2013-09-30 09:43:30

by Felix Fietkau

[permalink] [raw]
Subject: Re: [PATCH 3.12] mac80211: fix a tx power handling regression

On 2013-09-30 11:09 AM, Johannes Berg wrote:
> On Sun, 2013-09-29 at 14:48 +0200, Felix Fietkau wrote:
>> commit 1ea6f9c0d48b11b6ec3ec4b5579ec74fc3951cf8
>> "mac80211: handle TX power per virtual interface"
>>
>> This commit added support for tracking tx power configuration for
>> multiple interfaces, however instead of using the maximum value of all
>> virtual interfaces, it uses the minimum.
>
> I'm not sure it should be using the maximum? What if the AP required
> lowering TX power by way of TPC for example?
Shouldn't that only affect the virtual interface that is connected to
that AP?
If there's a regulatory requirement to use lower tx power, it should be
tracked as a limit somewhere else instead of implicitly being handled
via vif tx power configuration.

- Felix


2013-10-01 12:32:09

by Johannes Berg

[permalink] [raw]
Subject: Re: [PATCH 3.12] mac80211: fix a tx power handling regression

On Mon, 2013-09-30 at 12:51 +0200, Felix Fietkau wrote:

> > Not sure I see why? It's an absolute value after we do the calculations
> > in that interface that has the TPC.
> Maybe we need to rework this somehow, but in the mean time, this patch
> fixes a serious regression that I've been looking into for a while now.
> I haven't worked out the exact conditions that trigger this yet, but
> often when an AP VLAN gets destroyed and recreated, or when a new
> temporary interface is brought up and then down again, the tx power for
> *all* interfaces gets reset to the lowest possible level.

I think we just need to skip VLANs in the loop instead, they don't
really matter here. Can you try that? Using the max would effectively
ignore them since they might always have 0 for bss_conf.txpower.

johannes