Re: [RFC] mac80211: add PS flag to bss_conf

[Eliad Peller <eliad@wizery.com>]

On Tue, Jul 24, 2012 at 5:34 PM, Johannes Berg
<johannes@sipsolutions.net> wrote:
> On Thu, 2012-07-19 at 16:11 +0300, Eliad Peller wrote:
>> Currently, ps mode is indicated per device (rather than
>> per interface), which doesn't make a lot of sense.
>>
>> Moreover, there are subtle bugs caused by the inability
>> to indicate ps change along with other changes
>> (e.g. when the AP deauth us, we'd like to indicate
>> CHANGED_PS | CHANGED_ASSOC, as changing PS before
>> notifying about disassociation will result in null-packets
>> being sent (if IEEE80211_HW_SUPPORTS_DYNAMIC_PS) while
>> the sta is already disconnected.)
>>
>> Keep the current per-device notifications, and add
>> additional per-vif notifications. In order to keep it
>> simple, these notifications are NOT called due to
>> (temporary) dynamic_ps/offchannel operations.
>
> So I think if we do this, it would make sense to not do it in recalc_ps,
> but like you did in disassoc:
>
>> @@ -1368,6 +1388,8 @@ static void ieee80211_set_disassoc(struct ieee80211_sub_if_data *sdata,
>>       if (local->hw.conf.flags & IEEE80211_CONF_PS) {
>>               local->hw.conf.flags &= ~IEEE80211_CONF_PS;
>>               ieee80211_hw_config(local, IEEE80211_CONF_CHANGE_PS);
>> +             sdata->vif.bss_conf.ps = false;
>> +             changed |= BSS_CHANGED_PS;
>>       }
>>       local->ps_sdata = NULL;
>
> set it directly in the relevant code.
>
>
> However, I'm not sure that we really should toggle it with association?
> For the original CONF_PS semantics where mac80211 actually woke up for
> dynamic PS etc. this made sense, but for smarter devices I think the PS
> flag could be solely mirroring the combination of
>  a) broken AP => no PS
>  b) user enable/disable
>
ok, i think i got your point.

> even when not associated?

we actually check for authorization, so i think it does make some
sense to leave this check.

thanks for the review.
i'll apply your comments and send it as a patch.

Eliad.