On 2011-05-06 3:26 PM, Johannes Berg wrote:
On Fri, 2011-05-06 at 15:20 +0200, Felix Fietkau wrote:
On 2011-05-06 3:08 PM, Christian Lamparter wrote:
> On Friday 06 May 2011 02:41:14 Felix Fietkau wrote:
>> PS filtering is only useful in AP mode and by setting the flags in mac80211,
>> vif mode checks do not have to be added to every single driver supporting
>> this.
>> Fixes part of a regression introduced in commit 93ae2dd2
>> "ath9k: assign keycache slots to unencrypted stations"
>>
>> Signed-off-by: Felix Fietkau<nbd@xxxxxxxxxxx>
>> Reported-by: Ben Greear<greearb@xxxxxxxxxxxxxxx>
>> ---
>> net/mac80211/tx.c | 3 +++
>> 1 files changed, 3 insertions(+), 0 deletions(-)
>>
>> diff --git a/net/mac80211/tx.c b/net/mac80211/tx.c
>> index e1a39ed..b301216 100644
>> --- a/net/mac80211/tx.c
>> +++ b/net/mac80211/tx.c
>> @@ -1267,6 +1267,9 @@ ieee80211_tx_prepare(struct ieee80211_sub_if_data *sdata,
>> info->flags |= IEEE80211_TX_CTL_CLEAR_PS_FILT;
>> else if (test_and_clear_sta_flags(tx->sta, WLAN_STA_CLEAR_PS_FILT))
>> info->flags |= IEEE80211_TX_CTL_CLEAR_PS_FILT;
>> + else if (sdata->vif.type != NL80211_IFTYPE_AP&&
>> + sdata->vif.type != NL80211_IFTYPE_AP_VLAN)
>> + info->flags |= IEEE80211_TX_CTL_CLEAR_PS_FILT;
>>
>> hdrlen = ieee80211_hdrlen(hdr->frame_control);
>> if (skb->len> hdrlen + sizeof(rfc1042_header) + 2) {
>>
> uh, I think this will break p54* powersave in station mode.
> But why do we set it anyway? It's not like the AP is in PS mode, right?
PS filtering is not meant for station mode at all. It's meant for
suppressing lots of consecutive excessive retries when in AP mode
sending traffic to a STA that just entered powersave.
Actually, I think we should probably leave this patch out and fix it in
the driver instead.
You're right -- this is all intended for AP mode only, so it makes no
sense to set it in STA mode to begin with. I thought maybe it's
acceptable since drivers wouldn't use it in non-AP mode, but I can just
as well imagine that drivers would always check it and rely on it being
set only in AP mode.
Rely on what being set in AP mode only? The patch ensures that the flag
for *clearing* PS filtering gets set for all non-AP modes. This flag
already gets set whenever tx->sta is unset, so I think it also makes
sense to extend that for other cases where the hardware is not supposed
to enable any PS filtering on its own.
Your patch makes those drivers responsible for checking the mode, but
that makes less sense than having drivers that need to do some magic in
all modes, no?
How does my patch make drivers responsible for checking the mode?
- Felix
--
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
