2008-11-03 00:36:55

by Luis R. Rodriguez

[permalink] [raw]
Subject: [PATCH] ath5k: enable RXing beacons

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 <[email protected]>
---
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



2008-11-04 19:19:12

by Luis R. Rodriguez

[permalink] [raw]
Subject: Re: [PATCH] ath5k: enable RXing beacons

On Tue, Nov 4, 2008 at 4:37 AM, Bob Copeland <[email protected]> wrote:
> On Mon, Nov 3, 2008 at 10:17 AM, Bob Copeland <[email protected]> wrote:
>> On Sun, Nov 2, 2008 at 11:52 PM, Luis R. Rodriguez <[email protected]> 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

2008-11-04 12:37:11

by Bob Copeland

[permalink] [raw]
Subject: Re: [PATCH] ath5k: enable RXing beacons

On Mon, Nov 3, 2008 at 10:17 AM, Bob Copeland <[email protected]> wrote:
> On Sun, Nov 2, 2008 at 11:52 PM, Luis R. Rodriguez <[email protected]> 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 %% http://www.bobcopeland.com

2008-11-03 04:52:44

by Luis R. Rodriguez

[permalink] [raw]
Subject: Re: [PATCH] ath5k: enable RXing beacons

On Sun, Nov 2, 2008 at 8:49 PM, Luis R. Rodriguez <[email protected]> wrote:
> On Sun, Nov 2, 2008 at 8:43 PM, Luis R. Rodriguez <[email protected]> wrote:
>> On Sun, Nov 2, 2008 at 6:10 PM, Bob Copeland <[email protected]> wrote:
>>> On Sun, Nov 2, 2008 at 8:39 PM, Dan Williams <[email protected]> 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 <[email protected]>
>>>>
>>>> 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

2008-11-03 15:17:05

by Bob Copeland

[permalink] [raw]
Subject: Re: [PATCH] ath5k: enable RXing beacons

On Sun, Nov 2, 2008 at 11:52 PM, Luis R. Rodriguez <[email protected]> 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 %% http://www.bobcopeland.com

2008-11-03 01:46:54

by Dan Williams

[permalink] [raw]
Subject: Re: [PATCH] ath5k: enable RXing beacons

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 <[email protected]>

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))


2008-11-05 16:32:41

by Nick Kossifidis

[permalink] [raw]
Subject: Re: [PATCH] ath5k: enable RXing beacons

2008/11/3 Bob Copeland <[email protected]>:
>
> 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

2008-11-03 10:17:08

by Felix Fietkau

[permalink] [raw]
Subject: Re: [PATCH] ath5k: enable RXing beacons

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

2008-11-03 04:43:28

by Luis R. Rodriguez

[permalink] [raw]
Subject: Re: [PATCH] ath5k: enable RXing beacons

On Sun, Nov 2, 2008 at 6:10 PM, Bob Copeland <[email protected]> wrote:
> On Sun, Nov 2, 2008 at 8:39 PM, Dan Williams <[email protected]> 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 <[email protected]>
>>
>> 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

2008-11-03 04:49:28

by Luis R. Rodriguez

[permalink] [raw]
Subject: Re: [PATCH] ath5k: enable RXing beacons

On Sun, Nov 2, 2008 at 8:43 PM, Luis R. Rodriguez <[email protected]> wrote:
> On Sun, Nov 2, 2008 at 6:10 PM, Bob Copeland <[email protected]> wrote:
>> On Sun, Nov 2, 2008 at 8:39 PM, Dan Williams <[email protected]> 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 <[email protected]>
>>>
>>> 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

2008-11-03 02:10:30

by Bob Copeland

[permalink] [raw]
Subject: Re: [PATCH] ath5k: enable RXing beacons

On Sun, Nov 2, 2008 at 8:39 PM, Dan Williams <[email protected]> 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 <[email protected]>
>
> 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 %% http://www.bobcopeland.com

2008-11-05 16:03:20

by Bob Copeland

[permalink] [raw]
Subject: Re: [PATCH] ath5k: enable RXing beacons

On Tue, Nov 4, 2008 at 2:19 PM, Luis R. Rodriguez <[email protected]> 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 %% http://www.bobcopeland.com