2014-01-04 21:24:26

by Krishna Chaitanya

[permalink] [raw]
Subject: [PATCH] mac80211: SMPS: Fix the overwrite of rx_chains in the interface loop.

The interface loop identifies the no of static and dynamic
smps chains, but overwrites their values across the VIF's
(assuming there are more than one, in my case its AP and STA
loopback), so the driver might not intimated about this update
of SMPS mode, is the value is same.

So inform driver for every VIF.

For Ex: STA's SMPS state is changed, but in the loop AP appears
at the last, SMPS state of AP is unchanged hence not intimated
to the driver.

Signed-off-by: Chaitanya T K <[email protected]>
---

net/mac80211/chan.c | 34 +++++++++++++++++-----------------
1 file changed, 17 insertions(+), 17 deletions(-)

diff --git a/net/mac80211/chan.c b/net/mac80211/chan.c
index f43613a..e153a7e 100644
--- a/net/mac80211/chan.c
+++ b/net/mac80211/chan.c
@@ -483,26 +483,26 @@ void ieee80211_recalc_smps_chanctx(struct ieee80211_local *local,

rx_chains_static = max(rx_chains_static, needed_static);
rx_chains_dynamic = max(rx_chains_dynamic, needed_dynamic);
- }
- rcu_read_unlock();
+ if (!local->use_chanctx) {
+ if (rx_chains_static > 1)
+ local->smps_mode = IEEE80211_SMPS_OFF;
+ else if (rx_chains_dynamic > 1)
+ local->smps_mode = IEEE80211_SMPS_DYNAMIC;
+ else
+ local->smps_mode = IEEE80211_SMPS_STATIC;
+ ieee80211_hw_config(local, 0);
+ }

- if (!local->use_chanctx) {
- if (rx_chains_static > 1)
- local->smps_mode = IEEE80211_SMPS_OFF;
- else if (rx_chains_dynamic > 1)
- local->smps_mode = IEEE80211_SMPS_DYNAMIC;
- else
- local->smps_mode = IEEE80211_SMPS_STATIC;
- ieee80211_hw_config(local, 0);
- }
+ if (rx_chains_static == chanctx->conf.rx_chains_static &&
+ rx_chains_dynamic == chanctx->conf.rx_chains_dynamic)
+ return;

- if (rx_chains_static == chanctx->conf.rx_chains_static &&
- rx_chains_dynamic == chanctx->conf.rx_chains_dynamic)
- return;
+ chanctx->conf.rx_chains_static = rx_chains_static;
+ chanctx->conf.rx_chains_dynamic = rx_chains_dynamic;
+ drv_change_chanctx(local, chanctx, IEEE80211_CHANCTX_CHANGE_RX_CHAINS);
+ }
+ rcu_read_unlock();

- chanctx->conf.rx_chains_static = rx_chains_static;
- chanctx->conf.rx_chains_dynamic = rx_chains_dynamic;
- drv_change_chanctx(local, chanctx, IEEE80211_CHANCTX_CHANGE_RX_CHAINS);
}

int ieee80211_vif_use_channel(struct ieee80211_sub_if_data *sdata,


2014-01-05 07:33:57

by Emmanuel Grumbach

[permalink] [raw]
Subject: Re: [PATCH] mac80211: SMPS: Fix the overwrite of rx_chains in the interface loop.

Emmanuel Grumbach
[email protected]


On Sun, Jan 5, 2014 at 9:21 AM, Krishna Chaitanya
<[email protected]> wrote:
> On Sun, Jan 5, 2014 at 12:12 PM, Emmanuel Grumbach <[email protected]> wrote:
>>
>> On Sat, Jan 4, 2014 at 11:24 PM, Chaitanya T K <[email protected]> wrote:
>> > The interface loop identifies the no of static and dynamic
>> > smps chains, but overwrites their values across the VIF's
>> > (assuming there are more than one, in my case its AP and STA
>> > loopback), so the driver might not intimated about this update
>> > of SMPS mode, is the value is same.
>> >
>> > So inform driver for every VIF.
>> >
>> > For Ex: STA's SMPS state is changed, but in the loop AP appears
>> > at the last, SMPS state of AP is unchanged hence not intimated
>> > to the driver.
>> >
>> > Signed-off-by: Chaitanya T K <[email protected]>
>> > ---
>>
>> But the dynamic / static chains number is a property of the channel
>> context. If you have several vifs on the same channel context, the
>> maximum number of dynamic / static chains will be assigned to the
>> channel context. In short, I think that mac80211 already does the job
>> of taking all the parameters into account and putting the right value
>> into the channel context?
>> I don't really see what bug you are trying to solve - could you please
>> tell us exactly what is your configuration?
>> What SMPS do you have for each interface and what dynamic / static
>> chains settings does mac80211 end up with?
>
> Agree this looks good in channel context angle.
>
> Lets say we have two VIF's one AP and one STA and the driver doesn't
> support channel context's so in that case the config will happen
> through hw_config but the problem is hw_config is out side the loop
> so the config will happen with the parameters of the last VIF, which
> is incorrect.

Same applies if the driver doesn't support channel contexts. hw_config
will be called with the correct local->smps_mode based on the number
of channels required by each one of the vifs?
I don't see why only the last vif will be taken into account: the loop
computes the maximum number of antenna requires by *all* the vifs.
What am I missing here?

2014-01-05 06:42:22

by Emmanuel Grumbach

[permalink] [raw]
Subject: Re: [PATCH] mac80211: SMPS: Fix the overwrite of rx_chains in the interface loop.

On Sat, Jan 4, 2014 at 11:24 PM, Chaitanya T K <[email protected]> wrote:
> The interface loop identifies the no of static and dynamic
> smps chains, but overwrites their values across the VIF's
> (assuming there are more than one, in my case its AP and STA
> loopback), so the driver might not intimated about this update
> of SMPS mode, is the value is same.
>
> So inform driver for every VIF.
>
> For Ex: STA's SMPS state is changed, but in the loop AP appears
> at the last, SMPS state of AP is unchanged hence not intimated
> to the driver.
>
> Signed-off-by: Chaitanya T K <[email protected]>
> ---

But the dynamic / static chains number is a property of the channel
context. If you have several vifs on the same channel context, the
maximum number of dynamic / static chains will be assigned to the
channel context. In short, I think that mac80211 already does the job
of taking all the parameters into account and putting the right value
into the channel context?
I don't really see what bug you are trying to solve - could you please
tell us exactly what is your configuration?
What SMPS do you have for each interface and what dynamic / static
chains settings does mac80211 end up with?

2014-01-05 07:43:47

by Krishna Chaitanya

[permalink] [raw]
Subject: Re: [PATCH] mac80211: SMPS: Fix the overwrite of rx_chains in the interface loop.

>> Same applies if the driver doesn't support channel contexts. hw_config
>> will be called with the correct local->smps_mode based on the number
>> of channels required by each one of the vifs?
>> I don't see why only the last vif will be taken into account: the loop
>> computes the maximum number of antenna requires by *all* the vifs.
>> What am I missing here?

Got it. Ours was a special loopback case where 2 VIF's talk to each other
(one AP and one STA), so we had a problem there.

Please ignore this patch. Thanks Emmanuel.

2014-01-05 07:21:40

by Krishna Chaitanya

[permalink] [raw]
Subject: Re: [PATCH] mac80211: SMPS: Fix the overwrite of rx_chains in the interface loop.

On Sun, Jan 5, 2014 at 12:12 PM, Emmanuel Grumbach <[email protected]> wrote:
>
> On Sat, Jan 4, 2014 at 11:24 PM, Chaitanya T K <[email protected]> wrote:
> > The interface loop identifies the no of static and dynamic
> > smps chains, but overwrites their values across the VIF's
> > (assuming there are more than one, in my case its AP and STA
> > loopback), so the driver might not intimated about this update
> > of SMPS mode, is the value is same.
> >
> > So inform driver for every VIF.
> >
> > For Ex: STA's SMPS state is changed, but in the loop AP appears
> > at the last, SMPS state of AP is unchanged hence not intimated
> > to the driver.
> >
> > Signed-off-by: Chaitanya T K <[email protected]>
> > ---
>
> But the dynamic / static chains number is a property of the channel
> context. If you have several vifs on the same channel context, the
> maximum number of dynamic / static chains will be assigned to the
> channel context. In short, I think that mac80211 already does the job
> of taking all the parameters into account and putting the right value
> into the channel context?
> I don't really see what bug you are trying to solve - could you please
> tell us exactly what is your configuration?
> What SMPS do you have for each interface and what dynamic / static
> chains settings does mac80211 end up with?

Agree this looks good in channel context angle.

Lets say we have two VIF's one AP and one STA and the driver doesn't
support channel context's so in that case the config will happen
through hw_config but the problem is hw_config is out side the loop
so the config will happen with the parameters of the last VIF, which
is incorrect.

2014-01-05 07:37:05

by Emmanuel Grumbach

[permalink] [raw]
Subject: Re: [PATCH] mac80211: SMPS: Fix the overwrite of rx_chains in the interface loop.

> On Sun, Jan 5, 2014 at 9:21 AM, Krishna Chaitanya
> <[email protected]> wrote:
>> On Sun, Jan 5, 2014 at 12:12 PM, Emmanuel Grumbach <[email protected]> wrote:
>>>
>>> On Sat, Jan 4, 2014 at 11:24 PM, Chaitanya T K <[email protected]> wrote:
>>> > The interface loop identifies the no of static and dynamic
>>> > smps chains, but overwrites their values across the VIF's
>>> > (assuming there are more than one, in my case its AP and STA
>>> > loopback), so the driver might not intimated about this update
>>> > of SMPS mode, is the value is same.
>>> >
>>> > So inform driver for every VIF.
>>> >
>>> > For Ex: STA's SMPS state is changed, but in the loop AP appears
>>> > at the last, SMPS state of AP is unchanged hence not intimated
>>> > to the driver.
>>> >
>>> > Signed-off-by: Chaitanya T K <[email protected]>
>>> > ---
>>>
>>> But the dynamic / static chains number is a property of the channel
>>> context. If you have several vifs on the same channel context, the
>>> maximum number of dynamic / static chains will be assigned to the
>>> channel context. In short, I think that mac80211 already does the job
>>> of taking all the parameters into account and putting the right value
>>> into the channel context?
>>> I don't really see what bug you are trying to solve - could you please
>>> tell us exactly what is your configuration?
>>> What SMPS do you have for each interface and what dynamic / static
>>> chains settings does mac80211 end up with?
>>
>> Agree this looks good in channel context angle.
>>
>> Lets say we have two VIF's one AP and one STA and the driver doesn't
>> support channel context's so in that case the config will happen
>> through hw_config but the problem is hw_config is out side the loop
>> so the config will happen with the parameters of the last VIF, which
>> is incorrect.
>
> Same applies if the driver doesn't support channel contexts. hw_config
> will be called with the correct local->smps_mode based on the number
> of channels required by each one of the vifs?

s/channels/chains/

> I don't see why only the last vif will be taken into account: the loop
> computes the maximum number of antenna requires by *all* the vifs.
> What am I missing here?