[PATCH] ath5k: enable RXing beacons

[PATCH] ath5k: enable RXing beacons

[Luis R. Rodriguez]

Prior to this we would rely only on probe responses
from the AP to keep associated. We now receive beacons
on ath5k. This should fix sporadic disassociations.

Signed-off-by: Luis R. Rodriguez <lrodriguez@atheros.com>
---
 drivers/net/wireless/ath5k/base.c |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/drivers/net/wireless/ath5k/base.c b/drivers/net/wireless/ath5k/base.c
index f5f46fe..5505f45 100644
--- a/drivers/net/wireless/ath5k/base.c
+++ b/drivers/net/wireless/ath5k/base.c
@@ -2948,6 +2948,8 @@ static void ath5k_configure_filter(struct ieee80211_hw *hw,
 			AR5K_RX_FILTER_PROBEREQ | AR5K_RX_FILTER_PROM;
 	if (sc->opmode != NL80211_IFTYPE_STATION)
 		rfilt |= AR5K_RX_FILTER_PROBEREQ;
+	if (sc->opmode == NL80211_IFTYPE_STATION)
+		rfilt |= AR5K_RX_FILTER_BEACON;
 	if (sc->opmode != NL80211_IFTYPE_AP &&
 		sc->opmode != NL80211_IFTYPE_MESH_POINT &&
 		test_bit(ATH_STAT_PROMISC, sc->status))
-- 
1.5.6.rc2.15.g457bb.dirty

Re: [PATCH] ath5k: enable RXing beacons

[Dan Williams]

On Sun, 2008-11-02 at 16:36 -0800, Luis R. Rodriguez wrote:
> Prior to this we would rely only on probe responses
> from the AP to keep associated. We now receive beacons
> on ath5k. This should fix sporadic disassociations.
> 
> Signed-off-by: Luis R. Rodriguez <lrodriguez@atheros.com>

Is this also a candidate for -stable?

> ---
>  drivers/net/wireless/ath5k/base.c |    2 ++
>  1 files changed, 2 insertions(+), 0 deletions(-)
> 
> diff --git a/drivers/net/wireless/ath5k/base.c b/drivers/net/wireless/ath5k/base.c
> index f5f46fe..5505f45 100644
> --- a/drivers/net/wireless/ath5k/base.c
> +++ b/drivers/net/wireless/ath5k/base.c
> @@ -2948,6 +2948,8 @@ static void ath5k_configure_filter(struct ieee80211_hw *hw,
>  			AR5K_RX_FILTER_PROBEREQ | AR5K_RX_FILTER_PROM;
>  	if (sc->opmode != NL80211_IFTYPE_STATION)
>  		rfilt |= AR5K_RX_FILTER_PROBEREQ;
> +	if (sc->opmode == NL80211_IFTYPE_STATION)
> +		rfilt |= AR5K_RX_FILTER_BEACON;
>  	if (sc->opmode != NL80211_IFTYPE_AP &&
>  		sc->opmode != NL80211_IFTYPE_MESH_POINT &&
>  		test_bit(ATH_STAT_PROMISC, sc->status))

Re: [PATCH] ath5k: enable RXing beacons

[Bob Copeland]

On Sun, Nov 2, 2008 at 8:39 PM, Dan Williams <dcbw@redhat.com> wrote:
> On Sun, 2008-11-02 at 16:36 -0800, Luis R. Rodriguez wrote:
>> Prior to this we would rely only on probe responses
>> from the AP to keep associated. We now receive beacons
>> on ath5k. This should fix sporadic disassociations.
>>
>> Signed-off-by: Luis R. Rodriguez <lrodriguez@atheros.com>
>
> Is this also a candidate for -stable?

Don't think so, see below...

>
>> ---
>>  drivers/net/wireless/ath5k/base.c |    2 ++
>>  1 files changed, 2 insertions(+), 0 deletions(-)
>>
>> diff --git a/drivers/net/wireless/ath5k/base.c b/drivers/net/wireless/ath5k/base.c
>> index f5f46fe..5505f45 100644
>> --- a/drivers/net/wireless/ath5k/base.c
>> +++ b/drivers/net/wireless/ath5k/base.c
>> @@ -2948,6 +2948,8 @@ static void ath5k_configure_filter(struct ieee80211_hw *hw,
>>                       AR5K_RX_FILTER_PROBEREQ | AR5K_RX_FILTER_PROM;
>>       if (sc->opmode != NL80211_IFTYPE_STATION)
>>               rfilt |= AR5K_RX_FILTER_PROBEREQ;
>> +     if (sc->opmode == NL80211_IFTYPE_STATION)
>> +             rfilt |= AR5K_RX_FILTER_BEACON;
>>       if (sc->opmode != NL80211_IFTYPE_AP &&
>>               sc->opmode != NL80211_IFTYPE_MESH_POINT &&
>>               test_bit(ATH_STAT_PROMISC, sc->status))

Sorry, my fault, this was a direct result of
60c7e22196fb4230b76db1f5fb283e811b8f3fb3 "ath5k: honor
FIF_BCN_PRBRESP_PROMISC."  The problem remains that we have too many
interrupts though if PRBRESP_PROMISC is not set.  IIRC my patch made
ath5k behave same as ath9k.  Any ideas on a proper fix?

-- 
Bob Copeland %% www.bobcopeland.com

Re: [PATCH] ath5k: enable RXing beacons

[Luis R. Rodriguez]

On Sun, Nov 2, 2008 at 6:10 PM, Bob Copeland <me@bobcopeland.com> wrote:
> On Sun, Nov 2, 2008 at 8:39 PM, Dan Williams <dcbw@redhat.com> wrote:
>> On Sun, 2008-11-02 at 16:36 -0800, Luis R. Rodriguez wrote:
>>> Prior to this we would rely only on probe responses
>>> from the AP to keep associated. We now receive beacons
>>> on ath5k. This should fix sporadic disassociations.
>>>
>>> Signed-off-by: Luis R. Rodriguez <lrodriguez@atheros.com>
>>
>> Is this also a candidate for -stable?
>
> Don't think so, see below...
>
>>
>>> ---
>>>  drivers/net/wireless/ath5k/base.c |    2 ++
>>>  1 files changed, 2 insertions(+), 0 deletions(-)
>>>
>>> diff --git a/drivers/net/wireless/ath5k/base.c b/drivers/net/wireless/ath5k/base.c
>>> index f5f46fe..5505f45 100644
>>> --- a/drivers/net/wireless/ath5k/base.c
>>> +++ b/drivers/net/wireless/ath5k/base.c
>>> @@ -2948,6 +2948,8 @@ static void ath5k_configure_filter(struct ieee80211_hw *hw,
>>>                       AR5K_RX_FILTER_PROBEREQ | AR5K_RX_FILTER_PROM;
>>>       if (sc->opmode != NL80211_IFTYPE_STATION)
>>>               rfilt |= AR5K_RX_FILTER_PROBEREQ;
>>> +     if (sc->opmode == NL80211_IFTYPE_STATION)
>>> +             rfilt |= AR5K_RX_FILTER_BEACON;
>>>       if (sc->opmode != NL80211_IFTYPE_AP &&
>>>               sc->opmode != NL80211_IFTYPE_MESH_POINT &&
>>>               test_bit(ATH_STAT_PROMISC, sc->status))
>
> Sorry, my fault, this was a direct result of
> 60c7e22196fb4230b76db1f5fb283e811b8f3fb3 "ath5k: honor
> FIF_BCN_PRBRESP_PROMISC."  The problem remains that we have too many
> interrupts though if PRBRESP_PROMISC is not set.  IIRC my patch made
> ath5k behave same as ath9k.  Any ideas on a proper fix?

If we don't have beacons coming in on ath9k its also an issue and may
explain the same exact issue we see there.

  Luis

Re: [PATCH] ath5k: enable RXing beacons

[Luis R. Rodriguez]

On Sun, Nov 2, 2008 at 8:43 PM, Luis R. Rodriguez <mcgrof@gmail.com> wrote:
> On Sun, Nov 2, 2008 at 6:10 PM, Bob Copeland <me@bobcopeland.com> wrote:
>> On Sun, Nov 2, 2008 at 8:39 PM, Dan Williams <dcbw@redhat.com> wrote:
>>> On Sun, 2008-11-02 at 16:36 -0800, Luis R. Rodriguez wrote:
>>>> Prior to this we would rely only on probe responses
>>>> from the AP to keep associated. We now receive beacons
>>>> on ath5k. This should fix sporadic disassociations.
>>>>
>>>> Signed-off-by: Luis R. Rodriguez <lrodriguez@atheros.com>
>>>
>>> Is this also a candidate for -stable?
>>
>> Don't think so, see below...
>>
>>>
>>>> ---
>>>>  drivers/net/wireless/ath5k/base.c |    2 ++
>>>>  1 files changed, 2 insertions(+), 0 deletions(-)
>>>>
>>>> diff --git a/drivers/net/wireless/ath5k/base.c b/drivers/net/wireless/ath5k/base.c
>>>> index f5f46fe..5505f45 100644
>>>> --- a/drivers/net/wireless/ath5k/base.c
>>>> +++ b/drivers/net/wireless/ath5k/base.c
>>>> @@ -2948,6 +2948,8 @@ static void ath5k_configure_filter(struct ieee80211_hw *hw,
>>>>                       AR5K_RX_FILTER_PROBEREQ | AR5K_RX_FILTER_PROM;
>>>>       if (sc->opmode != NL80211_IFTYPE_STATION)
>>>>               rfilt |= AR5K_RX_FILTER_PROBEREQ;
>>>> +     if (sc->opmode == NL80211_IFTYPE_STATION)
>>>> +             rfilt |= AR5K_RX_FILTER_BEACON;
>>>>       if (sc->opmode != NL80211_IFTYPE_AP &&
>>>>               sc->opmode != NL80211_IFTYPE_MESH_POINT &&
>>>>               test_bit(ATH_STAT_PROMISC, sc->status))
>>
>> Sorry, my fault, this was a direct result of
>> 60c7e22196fb4230b76db1f5fb283e811b8f3fb3 "ath5k: honor
>> FIF_BCN_PRBRESP_PROMISC."  The problem remains that we have too many
>> interrupts though if PRBRESP_PROMISC is not set.  IIRC my patch made
>> ath5k behave same as ath9k.  Any ideas on a proper fix?
>
> If we don't have beacons coming in on ath9k its also an issue and may
> explain the same exact issue we see there.

I see what you saw now too.. hmmm, either we handle that wrong right
now too or there is something else that should enable the beacons to
come through. Something is fishy for sure. Will get back to you.

  Luis

Re: [PATCH] ath5k: enable RXing beacons

[Luis R. Rodriguez]

On Sun, Nov 2, 2008 at 8:49 PM, Luis R. Rodriguez <mcgrof@gmail.com> wrote:
> On Sun, Nov 2, 2008 at 8:43 PM, Luis R. Rodriguez <mcgrof@gmail.com> wrote:
>> On Sun, Nov 2, 2008 at 6:10 PM, Bob Copeland <me@bobcopeland.com> wrote:
>>> On Sun, Nov 2, 2008 at 8:39 PM, Dan Williams <dcbw@redhat.com> wrote:
>>>> On Sun, 2008-11-02 at 16:36 -0800, Luis R. Rodriguez wrote:
>>>>> Prior to this we would rely only on probe responses
>>>>> from the AP to keep associated. We now receive beacons
>>>>> on ath5k. This should fix sporadic disassociations.
>>>>>
>>>>> Signed-off-by: Luis R. Rodriguez <lrodriguez@atheros.com>
>>>>
>>>> Is this also a candidate for -stable?
>>>
>>> Don't think so, see below...
>>>
>>>>
>>>>> ---
>>>>>  drivers/net/wireless/ath5k/base.c |    2 ++
>>>>>  1 files changed, 2 insertions(+), 0 deletions(-)
>>>>>
>>>>> diff --git a/drivers/net/wireless/ath5k/base.c b/drivers/net/wireless/ath5k/base.c
>>>>> index f5f46fe..5505f45 100644
>>>>> --- a/drivers/net/wireless/ath5k/base.c
>>>>> +++ b/drivers/net/wireless/ath5k/base.c
>>>>> @@ -2948,6 +2948,8 @@ static void ath5k_configure_filter(struct ieee80211_hw *hw,
>>>>>                       AR5K_RX_FILTER_PROBEREQ | AR5K_RX_FILTER_PROM;
>>>>>       if (sc->opmode != NL80211_IFTYPE_STATION)
>>>>>               rfilt |= AR5K_RX_FILTER_PROBEREQ;
>>>>> +     if (sc->opmode == NL80211_IFTYPE_STATION)
>>>>> +             rfilt |= AR5K_RX_FILTER_BEACON;
>>>>>       if (sc->opmode != NL80211_IFTYPE_AP &&
>>>>>               sc->opmode != NL80211_IFTYPE_MESH_POINT &&
>>>>>               test_bit(ATH_STAT_PROMISC, sc->status))
>>>
>>> Sorry, my fault, this was a direct result of
>>> 60c7e22196fb4230b76db1f5fb283e811b8f3fb3 "ath5k: honor
>>> FIF_BCN_PRBRESP_PROMISC."  The problem remains that we have too many
>>> interrupts though if PRBRESP_PROMISC is not set.  IIRC my patch made
>>> ath5k behave same as ath9k.  Any ideas on a proper fix?
>>
>> If we don't have beacons coming in on ath9k its also an issue and may
>> explain the same exact issue we see there.
>
> I see what you saw now too.. hmmm, either we handle that wrong right
> now too or there is something else that should enable the beacons to
> come through. Something is fishy for sure. Will get back to you.

OK Vasanth confirmed this is correct, and ath9k actually has incorrect
behavior so we have to fix that there as well. If stable got the ath5k
changes to the filter then yes we should push it there too.

  Luis

Re: [PATCH] ath5k: enable RXing beacons

[Felix Fietkau]

Luis R. Rodriguez wrote:
> Prior to this we would rely only on probe responses
> from the AP to keep associated. We now receive beacons
> on ath5k. This should fix sporadic disassociations.
IMHO beacons should be received in AP mode as well, at least
in 11g mode (for automatically enabling protection mode).

- Felix

Re: [PATCH] ath5k: enable RXing beacons

[Bob Copeland]

On Sun, Nov 2, 2008 at 11:52 PM, Luis R. Rodriguez <mcgrof@gmail.com> wrote:
>>>> Sorry, my fault, this was a direct result of
>>>> 60c7e22196fb4230b76db1f5fb283e811b8f3fb3 "ath5k: honor
[...]
> OK Vasanth confirmed this is correct, and ath9k actually has incorrect
> behavior so we have to fix that there as well. If stable got the ath5k
> changes to the filter then yes we should push it there too.

Heh, yeah I thought the hardware was doing magic on our behalf too; I
guess I should've asked about ath9k rather than assume it was right
and ath5k was wrong.

I don't think the change got submitted for stable and it only went for
rc3 a day or two ago.  John, can you revert
60c7e22196fb4230b76db1f5fb283e811b8f3fb3 or take Luis' patch which
effectively does the same?

-- 
Bob Copeland %% www.bobcopeland.com

Re: [PATCH] ath5k: enable RXing beacons

[Bob Copeland]

On Mon, Nov 3, 2008 at 10:17 AM, Bob Copeland <me@bobcopeland.com> wrote:
> On Sun, Nov 2, 2008 at 11:52 PM, Luis R. Rodriguez <mcgrof@gmail.com> wrote:
>>>>> Sorry, my fault, this was a direct result of
>>>>> 60c7e22196fb4230b76db1f5fb283e811b8f3fb3 "ath5k: honor
> [...]
>> OK Vasanth confirmed this is correct, and ath9k actually has incorrect
>> behavior so we have to fix that there as well. If stable got the ath5k
>> changes to the filter then yes we should push it there too.

BTW perhaps we should still look at the filter for ath5k since, once
associated, we still get beacons from every AP in the world instead of
just those with our BSSID.

     FIF_BCN_PRBRESP_PROMISC

     This flag is set during scanning to indicate to the hardware
     that it should not filter beacons or probe responses by BSSID.
     Filtering them can greatly reduce the amount of processing
     mac80211 needs to do and the amount of CPU wakeups, so
     you should honour this flag if possible.

Right now with ath5k there's basically no difference between having or
not having that flag in station mode, even though we do set the bssid
in ath5k_hw_set_associd AFAICT.

-- 
Bob Copeland %% www.bobcopeland.com

Re: [PATCH] ath5k: enable RXing beacons

[Luis R. Rodriguez]

On Tue, Nov 4, 2008 at 4:37 AM, Bob Copeland <me@bobcopeland.com> wrote:
> On Mon, Nov 3, 2008 at 10:17 AM, Bob Copeland <me@bobcopeland.com> wrote:
>> On Sun, Nov 2, 2008 at 11:52 PM, Luis R. Rodriguez <mcgrof@gmail.com> wrote:
>>>>>> Sorry, my fault, this was a direct result of
>>>>>> 60c7e22196fb4230b76db1f5fb283e811b8f3fb3 "ath5k: honor
>> [...]
>>> OK Vasanth confirmed this is correct, and ath9k actually has incorrect
>>> behavior so we have to fix that there as well. If stable got the ath5k
>>> changes to the filter then yes we should push it there too.
>
> BTW perhaps we should still look at the filter for ath5k since, once
> associated, we still get beacons from every AP in the world instead of
> just those with our BSSID.
>
>     FIF_BCN_PRBRESP_PROMISC
>
>     This flag is set during scanning to indicate to the hardware
>     that it should not filter beacons or probe responses by BSSID.
>     Filtering them can greatly reduce the amount of processing
>     mac80211 needs to do and the amount of CPU wakeups, so
>     you should honour this flag if possible.
>
> Right now with ath5k there's basically no difference between having or
> not having that flag in station mode, even though we do set the bssid
> in ath5k_hw_set_associd AFAICT.

Agreed, what we would need is as Johannes has been suggesting a way to
implement this properly in mac80211. Come to think of it mac80211 also
isn't aware of all the other filter flags ath5k/ath9k devices are
capable of, I'd be inclined to move all that to mac80211 that way
mac80211 *always* tells  us the exact filter flags needed and we don't
get some strange internal mode checks as we do now. I posted a
strategy we can take in the other thread to handle the beacon filter
in mac80211, let me know what you think.

  Luis

Re: [PATCH] ath5k: enable RXing beacons

[Bob Copeland]

On Tue, Nov 4, 2008 at 2:19 PM, Luis R. Rodriguez <mcgrof@gmail.com> wrote:
> get some strange internal mode checks as we do now. I posted a
> strategy we can take in the other thread to handle the beacon filter
> in mac80211, let me know what you think.

Yup, it sounds reasonable.  It would definitely be nice to be able to
drop the mode checks in the drivers' configure_filter callbacks.

-- 
Bob Copeland %% www.bobcopeland.com

Re: [PATCH] ath5k: enable RXing beacons

[Nick Kossifidis]

2008/11/3 Bob Copeland <me@bobcopeland.com>:
>
> Sorry, my fault, this was a direct result of
> 60c7e22196fb4230b76db1f5fb283e811b8f3fb3 "ath5k: honor
> FIF_BCN_PRBRESP_PROMISC."  The problem remains that we have too many
> interrupts though if PRBRESP_PROMISC is not set.  IIRC my patch made
> ath5k behave same as ath9k.  Any ideas on a proper fix?
>

How about setting enhanced sleep timers on AR5212+ chips ?
Right now we don't handle this on beacon_init...

1602 /*
1603  * First enhanced sleep register
1604  */
1605 #define AR5K_SLEEP0                     0x80d4
/* Register Address */
1606 #define AR5K_SLEEP0_NEXT_DTIM           0x0007ffff      /* Mask
for next DTIM (?) */
1607 #define AR5K_SLEEP0_NEXT_DTIM_S         0
1608 #define AR5K_SLEEP0_ASSUME_DTIM         0x00080000      /* Assume DTIM */
1609 #define AR5K_SLEEP0_ENH_SLEEP_EN        0x00100000      /* Enable
enchanced sleep control */
1610 #define AR5K_SLEEP0_CABTO               0xff000000      /* Mask
for CAB Time Out */
1611 #define AR5K_SLEEP0_CABTO_S             24
1612
1613 /*
1614  * Second enhanced sleep register
1615  */
1616 #define AR5K_SLEEP1                     0x80d8
/* Register Address */
1617 #define AR5K_SLEEP1_NEXT_TIM            0x0007ffff      /* Mask
for next TIM (?) */
1618 #define AR5K_SLEEP1_NEXT_TIM_S          0
1619 #define AR5K_SLEEP1_BEACON_TO           0xff000000      /* Mask
for Beacon Time Out */
1620 #define AR5K_SLEEP1_BEACON_TO_S         24
1621
1622 /*
1623  * Third enhanced sleep register
1624  */
1625 #define AR5K_SLEEP2                     0x80dc
/* Register Address */
1626 #define AR5K_SLEEP2_TIM_PER             0x0000ffff      /* Mask
for TIM period (?) */
1627 #define AR5K_SLEEP2_TIM_PER_S           0
1628 #define AR5K_SLEEP2_DTIM_PER            0xffff0000      /* Mask
for DTIM period (?) */
1629 #define AR5K_SLEEP2_DTIM_PER_S          16

-- 
GPG ID: 0xD21DB2DB
As you read this post global entropy rises. Have Fun ;-)
Nick