On Tue, Jul 24, 2012 at 5:34 PM, Johannes Berg
<johannes@xxxxxxxxxxxxxxxx> 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.
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
