Bug 11749

Created attachment 18286 [details]
Disable MIB interrupts on ath5k for 2.6.27

ANI is not yet used in ath5k

Please try this patch.

I have tried the patch, it can not fix the issue.

I think the issue may come from the filter, at default filter receives all the multiple packages even station does not associate with any AP. 

We also track the bug a moblin bugzilla. see

http://bugzilla.moblin.org/show_bug.cgi?id=80

What?? You receive frames intended to go to another AP????

(In reply to comment #2)
> Please try this patch.
> 
This patch can't fix this issue, the result is still so.

Re #3, AFAICT the moblin.org bugzilla is talking about beacons, not data frames.  The filter is currently set to all-1s in ath5k_hw_set_associd so I wouldn't expect seeing data frames from other APs in sta mode.

Sorry, the above statements about Intel wireless driver is incorrect, number of interrupt per second should be 60 or more when it is in state "Not Associated", but that will be 1 to 5 after association is successful and AP is connected.

But for ath5k, that value doesn't change before and after association is done, it is about 40.

So now the problem is why there are so many interrupts before association is done, why does it not change after association is done.

Why does wireless driver respond all the beacon frames? Why "iwconfig wlan0 sens 90" can't work? Do we have an effective way to ignore beacon frames from those APs to which we don't want to connect?

Intel's wireless cards have a dedicated CPU as part of their hardware and they use firmware which runs on it. We don't have a CPU and we don't have firmware which is why it was *very* difficult for us to open up our drivers.

So you should see more load on the CPU on our hardware, its just the nature of our hardware. You however will see no proprietary firmware :)

Created attachment 18424 [details]
Fix handling of FIF_BCN_PRBRESP_PROMISC

At any rate I'm not convinced ath5k's handling of FIF_BCN_PRBRESP_PROMISC is correct, since it doesn't set the associd back to broadcast, and it enables beacons in STA mode regardless of whether the flag is set.  Could you try the attached patch which makes it behave similarly to ath9k?

Sorry, i missed this.

This patch is very good, it can reduce many interrupts, it is valid both before and after association is done, so strongly suggest to submit it to upstream kernel driver.

With this patch, number of interrupts of ath5k per second is 2~4  before association is done, after that number of interrupts of ath5k per second is 0.

Ok, patch without the broadcast associd (since it didn't seem to matter) was posted on linux-wireless and committed as ba7273afa2db548ba556d6b2325c36d9c91fad95 in the wireless-testing tree.

Here's the thread:
http://marc.info/?l=linux-wireless&m=122528356211367&w=2

As Luis discovered, the patch alone won't work - it needs help from the beacon miss interrupt or else all beacons are simply discarded and consequently we will lose associations without probe responses.

And additionally in order to fix this we need to rely on the beacon miss interrupt but in order to use that we need to inform mac80211 about it which requires adding a small API for it so mac80211 doesn't *always* expect beacons.

Can we just let station ignore/filter all the beacons when it is not associated with any AP. That can resolve the the main issue of the bug.  

Created attachment 18742 [details]
stop hw when bringing an interface down

Good point. Please try this patch.

I think this should work as we call remove_interface() callback of the driver from mac80211 when we call ieee80211_stop() the net_device's stop(). This occurs for IBSS, STA and Mesh type interfaces.

ath5k_stop_locked() is also correct as its under the sc->lock.

Well sorry this will just stop the filter/hw/etc when we bring the interface down, which is still correct IMHO. What you want is to disable the beacon filter when you are not connected to an AP??

Well how will you connect to an AP then? Mind you to resolve the issue *properly* we just need to add API to mac80211 to make it posssible for it not to require beacons so we can use the beacon miss interrupt instead of always allowing all the beacons through.

I suppose we can disable the beacon filter if we're not associated *and* not scanning.. Have to think of where to put that. Anyone with ideas?

I think mac80211 already sets PRBRESP_PROMISC whenever it starts an active scan.

Regarding the other patch - shouldn't balancing ->start() and ->stop() be enough?  I would worry about calling stop if we haven't called start already (e.g. we never brought the interface up, and then stop() wants to dispose of things only created in start())

the key issue is to let ath5k driver know the change of the associate state so the beacon filter can be disable/enable.
I tried to use bss_info_changed(), but it can only work when set/disable essid through iwconfig command; and can not handle other scenario. 

Someone know how to info the associate state changing to ath5k driver?

Bob so the stop thing I put in the suggested patch doesn't kfree() anything but just stops the radio and disables the RX filter and turns off the leds so I think it should be safe on first glance.

So the reason we need the beacon filter *on* is mac80211 expects beacons. In this case Yi is asking for us to disable it when we are not associated, I just did it when we bring the interface down though..

Not sure where to poke yet to enable this where we are not associated yet, it would probably come in from mac80211 -- take a look at ieee80211_set_associated() on mlme.c in mac80211.

But I think it'd be nicer to add functionality to mac80211 to allow us to use the beacon miss interrupt as Felix was suggesting, which is what ath9k was using. To do that I think we will need maybe only enable the timer if the beacon miss interrupt is triggered but we would need a way to communicate that to mac80211.

Created attachment 18786 [details]
Add bss_info_change() to ath5k, disable radio when not associated


This adds the bss_info_change() callback to mac80211 and disables the radio, rx filter and leds when not associated. Let me know what you think.

With the other attachment 18742 [details] this also disables the RX filter when the interface is down.

(In reply to comment #21)
> Created an attachment (id=18786) [details]
> Add bss_info_change() to ath5k, disable radio when not associated
> 
> 
> This adds the bss_info_change() callback to mac80211 and disables the radio,
> rx
> filter and leds when not associated. Let me know what you think.
> 
> With the other attachment 18742 [details] this also disables the RX filter
> when the
> interface is down.
> 

I think the patch has some issues.
1. when disassociate it will power down the device totally, how can the station be powered on when it want to associate again?
2. basing on my observation, bss_info_change() can not notify the associate state changing all the times. (Although I originally thought it should)
see below scenario:
associate to one AP named Guest (iwconfig wlan0 essid Guest)
reassociated to another AP name tt which is not existing. (iwconfig wlan0 tt )
now iwconfig wlan0 you will find that station is not associated. HOWEVER, the changing of the associate state is not reported to ath5k driver. 

In fact I have also worked out a patch basing on bss_info_change(), in the patch I just disable/enable beacon filter. It can work fine except that bss_info_change issue mentioned above. 

(In reply to comment #20)
> Bob so the stop thing I put in the suggested patch doesn't kfree() anything
> but
> just stops the radio and disables the RX filter and turns off the leds so I
> think it should be safe on first glance.
> 
> So the reason we need the beacon filter *oissn* is mac80211 expects beacons.
> In
> this case Yi is asking for us to disable it when we are not associated, I
> just
> did it when we bring the interface down though..n
> e
To the current ath5k driver,when we bring the interface down, the issue does not exists any more. No interrupts will take place. 
BTW: I am Martin not Yi  :-)

Created attachment 18794 [details]
disable beacon filter when disassocated 

Please review the patch

I like it, except you just forgot the mutex_unlock(&sc->lock) on bss change callback. With the mutex_unlock() does it work for you?

I do not think mutex_unlock is useful here. I have verified the patch again, and find that it can work fine.

(In reply to comment #26)
> I do not think mutex_unlock is useful here. I have verified the patch again,
> and find that it can work fine.
> 
This mutex is necessary, sc must be accessed with sc->lock holden.

Definitely agree it needs balanced locking, also it would be nice if it compiled :)  But the idea is good...

Yang Yi, can you help us close the bug? Thanks!

commit cb99cf4fc881989464f2ba597cdcd276f80bc08e
Author: Martin Xu <martin.xu@intel.com>
Date:   Mon Nov 24 10:49:27 2008 +0800

    ath5k: disable beacon filter when station is not associated
    
    Ath5k driver has too many interrupts per second at idle
    http://bugzilla.kernel.org/show_bug.cgi?id=11749
    
    Signed-off-by: Martin Xu <martin.xu@intel.com>
    Signed-off-by: John W. Linville <linville@tuxdriver.com>