* [PATCH v2] mac80211: Optimize scans on current operating channel.
@ 2011-01-21 18:05 greearb
2011-01-23 9:18 ` Johannes Berg
2011-01-26 15:36 ` Johannes Berg
0 siblings, 2 replies; 13+ messages in thread
From: greearb @ 2011-01-21 18:05 UTC (permalink / raw)
To: linux-wireless; +Cc: Ben Greear
From: Ben Greear <greearb@candelatech.com>
This should decrease un-necessary flushes, on/off channel work,
and channel changes in cases where the only scanned channel is
the current operating channel.
Signed-off-by: Ben Greear <greearb@candelatech.com>
---
v2: Check channels instead of flag when determining if we should
do a channel change in scan_completed_finish.
:100644 100644 c47d7c0... 59fe5e7... M net/mac80211/ieee80211_i.h
:100644 100644 1236710... e6de0e7... M net/mac80211/rx.c
:100644 100644 3e660db... fa0aeb9... M net/mac80211/scan.c
net/mac80211/ieee80211_i.h | 5 +++++
net/mac80211/rx.c | 11 ++++++++---
net/mac80211/scan.c | 41 +++++++++++++++++++++++++----------------
3 files changed, 38 insertions(+), 19 deletions(-)
diff --git a/net/mac80211/ieee80211_i.h b/net/mac80211/ieee80211_i.h
index c47d7c0..59fe5e7 100644
--- a/net/mac80211/ieee80211_i.h
+++ b/net/mac80211/ieee80211_i.h
@@ -660,6 +660,10 @@ struct tpt_led_trigger {
* that the scan completed.
* @SCAN_ABORTED: Set for our scan work function when the driver reported
* a scan complete for an aborted scan.
+ * @SCAN_LEFT_OPER_CHANNEL: Set this flag if the scan process leaves the
+ * operating channel at any time. If scanning ONLY the current operating
+ * channel this flag should not be set, and this will allow fewer
+ * offchannel changes.
*/
enum {
SCAN_SW_SCANNING,
@@ -667,6 +671,7 @@ enum {
SCAN_OFF_CHANNEL,
SCAN_COMPLETED,
SCAN_ABORTED,
+ SCAN_LEFT_OPER_CHANNEL,
};
/**
diff --git a/net/mac80211/rx.c b/net/mac80211/rx.c
index 1236710..e6de0e7 100644
--- a/net/mac80211/rx.c
+++ b/net/mac80211/rx.c
@@ -388,6 +388,7 @@ ieee80211_rx_h_passive_scan(struct ieee80211_rx_data *rx)
struct ieee80211_local *local = rx->local;
struct ieee80211_rx_status *status = IEEE80211_SKB_RXCB(rx->skb);
struct sk_buff *skb = rx->skb;
+ int ret;
if (likely(!(status->rx_flags & IEEE80211_RX_IN_SCAN)))
return RX_CONTINUE;
@@ -396,10 +397,14 @@ ieee80211_rx_h_passive_scan(struct ieee80211_rx_data *rx)
return ieee80211_scan_rx(rx->sdata, skb);
if (test_bit(SCAN_SW_SCANNING, &local->scanning)) {
- /* drop all the other packets during a software scan anyway */
- if (ieee80211_scan_rx(rx->sdata, skb) != RX_QUEUED)
+ ret = ieee80211_scan_rx(rx->sdata, skb);
+ /* drop all the other packets while scanning off channel */
+ if (ret != RX_QUEUED &&
+ test_bit(SCAN_OFF_CHANNEL, &local->scanning)) {
dev_kfree_skb(skb);
- return RX_QUEUED;
+ return RX_QUEUED;
+ }
+ return ret;
}
/* scanning finished during invoking of handlers */
diff --git a/net/mac80211/scan.c b/net/mac80211/scan.c
index 3e660db..fa0aeb9 100644
--- a/net/mac80211/scan.c
+++ b/net/mac80211/scan.c
@@ -293,11 +293,14 @@ static void __ieee80211_scan_completed_finish(struct ieee80211_hw *hw,
{
struct ieee80211_local *local = hw_to_local(hw);
- ieee80211_hw_config(local, IEEE80211_CONF_CHANGE_CHANNEL);
+ if ((local->oper_channel != local->hw.conf.channel) || was_hw_scan)
+ ieee80211_hw_config(local, IEEE80211_CONF_CHANGE_CHANNEL);
+
if (!was_hw_scan) {
ieee80211_configure_filter(local);
drv_sw_scan_complete(local);
- ieee80211_offchannel_return(local, true);
+ if (test_bit(SCAN_LEFT_OPER_CHANNEL, &local->scanning))
+ ieee80211_offchannel_return(local, true);
}
mutex_lock(&local->mtx);
@@ -397,13 +400,10 @@ static int ieee80211_start_sw_scan(struct ieee80211_local *local)
drv_sw_scan_start(local);
- ieee80211_offchannel_stop_beaconing(local);
-
local->leave_oper_channel_time = 0;
local->next_scan_state = SCAN_DECISION;
local->scan_channel_idx = 0;
-
- drv_flush(local, false);
+ __clear_bit(SCAN_LEFT_OPER_CHANNEL, &local->scanning);
ieee80211_configure_filter(local);
@@ -543,7 +543,18 @@ static void ieee80211_scan_state_decision(struct ieee80211_local *local,
}
mutex_unlock(&local->iflist_mtx);
- if (local->scan_channel) {
+ next_chan = local->scan_req->channels[local->scan_channel_idx];
+
+ if (local->oper_channel == local->hw.conf.channel) {
+ if (next_chan == local->oper_channel)
+ local->next_scan_state = SCAN_SET_CHANNEL;
+ else
+ /*
+ * we're on the operating channel currently, let's
+ * leave that channel now to scan another one
+ */
+ local->next_scan_state = SCAN_LEAVE_OPER_CHANNEL;
+ } else {
/*
* we're currently scanning a different channel, let's
* see if we can scan another channel without interfering
@@ -559,7 +570,6 @@ static void ieee80211_scan_state_decision(struct ieee80211_local *local,
*
* Otherwise switch back to the operating channel.
*/
- next_chan = local->scan_req->channels[local->scan_channel_idx];
bad_latency = time_after(jiffies +
ieee80211_scan_get_channel_time(next_chan),
@@ -577,12 +587,6 @@ static void ieee80211_scan_state_decision(struct ieee80211_local *local,
local->next_scan_state = SCAN_ENTER_OPER_CHANNEL;
else
local->next_scan_state = SCAN_SET_CHANNEL;
- } else {
- /*
- * we're on the operating channel currently, let's
- * leave that channel now to scan another one
- */
- local->next_scan_state = SCAN_LEAVE_OPER_CHANNEL;
}
*next_delay = 0;
@@ -591,9 +595,12 @@ static void ieee80211_scan_state_decision(struct ieee80211_local *local,
static void ieee80211_scan_state_leave_oper_channel(struct ieee80211_local *local,
unsigned long *next_delay)
{
+ ieee80211_offchannel_stop_beaconing(local);
+
ieee80211_offchannel_stop_station(local);
__set_bit(SCAN_OFF_CHANNEL, &local->scanning);
+ __set_bit(SCAN_LEFT_OPER_CHANNEL, &local->scanning);
/*
* What if the nullfunc frames didn't arrive?
@@ -640,8 +647,10 @@ static void ieee80211_scan_state_set_channel(struct ieee80211_local *local,
chan = local->scan_req->channels[local->scan_channel_idx];
local->scan_channel = chan;
- if (ieee80211_hw_config(local, IEEE80211_CONF_CHANGE_CHANNEL))
- skip = 1;
+
+ if (chan != local->hw.conf.channel)
+ if (ieee80211_hw_config(local, IEEE80211_CONF_CHANGE_CHANNEL))
+ skip = 1;
/* advance state machine to next channel/band */
local->scan_channel_idx++;
--
1.7.2.3
^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH v2] mac80211: Optimize scans on current operating channel.
2011-01-21 18:05 [PATCH v2] mac80211: Optimize scans on current operating channel greearb
@ 2011-01-23 9:18 ` Johannes Berg
2011-01-23 15:49 ` Ben Greear
2011-01-24 22:12 ` Ben Greear
2011-01-26 15:36 ` Johannes Berg
1 sibling, 2 replies; 13+ messages in thread
From: Johannes Berg @ 2011-01-23 9:18 UTC (permalink / raw)
To: greearb; +Cc: linux-wireless
On Fri, 2011-01-21 at 10:05 -0800, greearb@candelatech.com wrote:
> + * @SCAN_LEFT_OPER_CHANNEL: Set this flag if the scan process leaves the
> + * operating channel at any time. If scanning ONLY the current operating
> + * channel this flag should not be set, and this will allow fewer
> + * offchannel changes.
Why does this make sense? In the code below, you do
> - ieee80211_hw_config(local, IEEE80211_CONF_CHANGE_CHANNEL);
> + if ((local->oper_channel != local->hw.conf.channel) || was_hw_scan)
> + ieee80211_hw_config(local, IEEE80211_CONF_CHANGE_CHANNEL);
> +
> if (!was_hw_scan) {
> ieee80211_configure_filter(local);
> drv_sw_scan_complete(local);
> - ieee80211_offchannel_return(local, true);
> + if (test_bit(SCAN_LEFT_OPER_CHANNEL, &local->scanning))
> + ieee80211_offchannel_return(local, true);
so you could just as well use a local variable (the first if you add is
exactly this test).
> static void ieee80211_scan_state_leave_oper_channel(struct ieee80211_local *local,
> unsigned long *next_delay)
> {
> + ieee80211_offchannel_stop_beaconing(local);
> +
Won't that confuse drivers that expect not to be beaconing between
sw_scan_start and sw_scan_end?
Also, are you sure the filter configuration works correctly this way? If
you can, I'd like to look at a binary trace of the commands going to the
device with this change: "trace-cmd record -e mac80211".
johannes
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v2] mac80211: Optimize scans on current operating channel.
2011-01-23 9:18 ` Johannes Berg
@ 2011-01-23 15:49 ` Ben Greear
2011-01-24 22:12 ` Ben Greear
1 sibling, 0 replies; 13+ messages in thread
From: Ben Greear @ 2011-01-23 15:49 UTC (permalink / raw)
To: Johannes Berg; +Cc: linux-wireless
On 01/23/2011 01:18 AM, Johannes Berg wrote:
> On Fri, 2011-01-21 at 10:05 -0800, greearb@candelatech.com wrote:
>
>> + * @SCAN_LEFT_OPER_CHANNEL: Set this flag if the scan process leaves the
>> + * operating channel at any time. If scanning ONLY the current operating
>> + * channel this flag should not be set, and this will allow fewer
>> + * offchannel changes.
>
> Why does this make sense? In the code below, you do
>
>
>> - ieee80211_hw_config(local, IEEE80211_CONF_CHANGE_CHANNEL);
>> + if ((local->oper_channel != local->hw.conf.channel) || was_hw_scan)
>> + ieee80211_hw_config(local, IEEE80211_CONF_CHANGE_CHANNEL);
>> +
>> if (!was_hw_scan) {
>> ieee80211_configure_filter(local);
>> drv_sw_scan_complete(local);
>> - ieee80211_offchannel_return(local, true);
>> + if (test_bit(SCAN_LEFT_OPER_CHANNEL,&local->scanning))
>> + ieee80211_offchannel_return(local, true);
>
> so you could just as well use a local variable (the first if you add is
> exactly this test).
I'm sorry, but I don't understand your comments. I need some way to track
whether we ever called the offchannel-stop-beaconing code so that it can
be selectively re-enabled in the code above. I was using a flag
instead of adding another boolean to the local struct.
The flag could be called 'SCAN_STOPPED_BEACONING' instead if that makes more
sense?
>> static void ieee80211_scan_state_leave_oper_channel(struct ieee80211_local *local,
>> unsigned long *next_delay)
>> {
>> + ieee80211_offchannel_stop_beaconing(local);
>> +
>
> Won't that confuse drivers that expect not to be beaconing between
> sw_scan_start and sw_scan_end?
I didn't know that any would care. Maybe we need a phy_features set of
flags and let each driver enable this optimization as it is tested/verified?
Seems to work fine for ath9k with multiple VIFs, for what that's worth.
> Also, are you sure the filter configuration works correctly this way? If
> you can, I'd like to look at a binary trace of the commands going to the
> device with this change: "trace-cmd record -e mac80211".
I'll try to run this soon and post the results.
Thanks,
Ben
--
Ben Greear <greearb@candelatech.com>
Candela Technologies Inc http://www.candelatech.com
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v2] mac80211: Optimize scans on current operating channel.
2011-01-23 9:18 ` Johannes Berg
2011-01-23 15:49 ` Ben Greear
@ 2011-01-24 22:12 ` Ben Greear
2011-01-26 15:22 ` Johannes Berg
1 sibling, 1 reply; 13+ messages in thread
From: Ben Greear @ 2011-01-24 22:12 UTC (permalink / raw)
To: Johannes Berg; +Cc: linux-wireless
On 01/23/2011 01:18 AM, Johannes Berg wrote:
> Also, are you sure the filter configuration works correctly this way? If
> you can, I'd like to look at a binary trace of the commands going to the
> device with this change: "trace-cmd record -e mac80211".
I uploaded two trace files. One that scans the single operating
channel, and a second that scans all channels. This is with my -v2
patch applied, using ath9k as the driver. There are two virtual
STA interfaces configured on this system for this test.
http://www.candelatech.com/~greearb/trace-scan-all.dat.bz2
http://www.candelatech.com/~greearb/trace-scan-one.dat.bz2
Please let me know what you think.
Thanks,
Ben
--
Ben Greear <greearb@candelatech.com>
Candela Technologies Inc http://www.candelatech.com
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v2] mac80211: Optimize scans on current operating channel.
2011-01-24 22:12 ` Ben Greear
@ 2011-01-26 15:22 ` Johannes Berg
2011-01-26 17:28 ` Ben Greear
0 siblings, 1 reply; 13+ messages in thread
From: Johannes Berg @ 2011-01-26 15:22 UTC (permalink / raw)
To: Ben Greear; +Cc: linux-wireless
On Mon, 2011-01-24 at 14:12 -0800, Ben Greear wrote:
> On 01/23/2011 01:18 AM, Johannes Berg wrote:
>
> > Also, are you sure the filter configuration works correctly this way? If
> > you can, I'd like to look at a binary trace of the commands going to the
> > device with this change: "trace-cmd record -e mac80211".
>
> I uploaded two trace files. One that scans the single operating
> channel, and a second that scans all channels. This is with my -v2
> patch applied, using ath9k as the driver. There are two virtual
> STA interfaces configured on this system for this test.
>
>
> http://www.candelatech.com/~greearb/trace-scan-all.dat.bz2
> http://www.candelatech.com/~greearb/trace-scan-one.dat.bz2
>
> Please let me know what you think.
Thanks. I don't think the -one one really is just a single scan? But
anyway, looking at it now.
johannes
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v2] mac80211: Optimize scans on current operating channel.
2011-01-21 18:05 [PATCH v2] mac80211: Optimize scans on current operating channel greearb
2011-01-23 9:18 ` Johannes Berg
@ 2011-01-26 15:36 ` Johannes Berg
2011-01-26 18:10 ` Ben Greear
2011-01-26 18:39 ` Ben Greear
1 sibling, 2 replies; 13+ messages in thread
From: Johannes Berg @ 2011-01-26 15:36 UTC (permalink / raw)
To: greearb; +Cc: linux-wireless
On Fri, 2011-01-21 at 10:05 -0800, greearb@candelatech.com wrote:
> From: Ben Greear <greearb@candelatech.com>
>
> This should decrease un-necessary flushes, on/off channel work,
> and channel changes in cases where the only scanned channel is
> the current operating channel.
>
> Signed-off-by: Ben Greear <greearb@candelatech.com>
> ---
>
> v2: Check channels instead of flag when determining if we should
> do a channel change in scan_completed_finish.
Can you look at work.c -- where we call
ieee80211_offchannel_stop_beaconing etc.
In this patch, you're moving the call to
ieee80211_offchannel_stop_beaconing next to
ieee80211_offchannel_stop_station in scan.c.
Therefore, you can combine those two into one function.
However, I guess it'd also be nice to also address the TODO in work.c
and move the "don't do so much if not really off-channel" logic into
that function? You must run into that when trying to associate on one
vif and the others get stopped etc. unnecessarily to do that.
johannes
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v2] mac80211: Optimize scans on current operating channel.
2011-01-26 15:22 ` Johannes Berg
@ 2011-01-26 17:28 ` Ben Greear
0 siblings, 0 replies; 13+ messages in thread
From: Ben Greear @ 2011-01-26 17:28 UTC (permalink / raw)
To: Johannes Berg; +Cc: linux-wireless
On 01/26/2011 07:22 AM, Johannes Berg wrote:
> On Mon, 2011-01-24 at 14:12 -0800, Ben Greear wrote:
>> On 01/23/2011 01:18 AM, Johannes Berg wrote:
>>
>>> Also, are you sure the filter configuration works correctly this way? If
>>> you can, I'd like to look at a binary trace of the commands going to the
>>> device with this change: "trace-cmd record -e mac80211".
>>
>> I uploaded two trace files. One that scans the single operating
>> channel, and a second that scans all channels. This is with my -v2
>> patch applied, using ath9k as the driver. There are two virtual
>> STA interfaces configured on this system for this test.
>>
>>
>> http://www.candelatech.com/~greearb/trace-scan-all.dat.bz2
>> http://www.candelatech.com/~greearb/trace-scan-one.dat.bz2
>>
>> Please let me know what you think.
>
> Thanks. I don't think the -one one really is just a single scan? But
> anyway, looking at it now.
No, sorry..it's probably several scans and other things..but it should
not scan more than one (operating) channel.
Also, I'm seeing problems where sometimes wpa_supplicant doesn't
see scan results when scanning a single operating channel. I think this is
probably a bug in my mac80211 patch. I plan to look at that in
detail today.
Thanks,
Ben
>
> johannes
--
Ben Greear <greearb@candelatech.com>
Candela Technologies Inc http://www.candelatech.com
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v2] mac80211: Optimize scans on current operating channel.
2011-01-26 15:36 ` Johannes Berg
@ 2011-01-26 18:10 ` Ben Greear
2011-01-27 13:56 ` Johannes Berg
2011-01-26 18:39 ` Ben Greear
1 sibling, 1 reply; 13+ messages in thread
From: Ben Greear @ 2011-01-26 18:10 UTC (permalink / raw)
To: Johannes Berg; +Cc: linux-wireless
On 01/26/2011 07:36 AM, Johannes Berg wrote:
> On Fri, 2011-01-21 at 10:05 -0800, greearb@candelatech.com wrote:
>> From: Ben Greear<greearb@candelatech.com>
>>
>> This should decrease un-necessary flushes, on/off channel work,
>> and channel changes in cases where the only scanned channel is
>> the current operating channel.
>>
>> Signed-off-by: Ben Greear<greearb@candelatech.com>
>> ---
>>
>> v2: Check channels instead of flag when determining if we should
>> do a channel change in scan_completed_finish.
>
> Can you look at work.c -- where we call
> ieee80211_offchannel_stop_beaconing etc.
>
> In this patch, you're moving the call to
> ieee80211_offchannel_stop_beaconing next to
> ieee80211_offchannel_stop_station in scan.c.
>
> Therefore, you can combine those two into one function.
>
> However, I guess it'd also be nice to also address the TODO in work.c
> and move the "don't do so much if not really off-channel" logic into
> that function? You must run into that when trying to associate on one
> vif and the others get stopped etc. unnecessarily to do that.
I think you are right about this, but due to the tricky nature
of this code, maybe save that for a second patch?
Thanks,
Ben
>
> johannes
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
Ben Greear <greearb@candelatech.com>
Candela Technologies Inc http://www.candelatech.com
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v2] mac80211: Optimize scans on current operating channel.
2011-01-26 15:36 ` Johannes Berg
2011-01-26 18:10 ` Ben Greear
@ 2011-01-26 18:39 ` Ben Greear
2011-01-27 13:55 ` Johannes Berg
1 sibling, 1 reply; 13+ messages in thread
From: Ben Greear @ 2011-01-26 18:39 UTC (permalink / raw)
To: Johannes Berg; +Cc: linux-wireless
On 01/26/2011 07:36 AM, Johannes Berg wrote:
> On Fri, 2011-01-21 at 10:05 -0800, greearb@candelatech.com wrote:
>> From: Ben Greear<greearb@candelatech.com>
>>
>> This should decrease un-necessary flushes, on/off channel work,
>> and channel changes in cases where the only scanned channel is
>> the current operating channel.
>>
>> Signed-off-by: Ben Greear<greearb@candelatech.com>
>> ---
>>
>> v2: Check channels instead of flag when determining if we should
>> do a channel change in scan_completed_finish.
>
> Can you look at work.c -- where we call
> ieee80211_offchannel_stop_beaconing etc.
>
> In this patch, you're moving the call to
> ieee80211_offchannel_stop_beaconing next to
> ieee80211_offchannel_stop_station in scan.c.
The offchannel_stop_beaconing doesn't actually change
state, it just calls
ieee80211_bss_info_change_notify(
sdata, BSS_CHANGED_BEACON_ENABLED);
That method will disable beaconing if SCAN_SW_SCANNING is set,
regardless of current channel.
So, if that work.c logic is happening during a scan, we
will always disable scanning, but if we are not SW scanning,
then that offchannel_stop_beaconing won't actually do
anything useful.
My first inclination is to add another check in bss_info_change_notify
to test if we are really off-channel:
if (local->quiescing || !ieee80211_sdata_running(sdata) ||
(test_bit(SCAN_SW_SCANNING, &local->scanning) &&
test_bit(SCAN_OFF_CHANNEL, &local->scanning)) {
sdata->vif.bss_conf.enable_beacon = false;
And either set the SCAN_OFF_CHANNEL flag before calling offchannel_stop_beaconing,
or perhaps set that flag in offchannel_stop_beaconing. The first might be
less risky, but that leaves the work.c logic funky in my opinion, since if
we are not actually scanning, beacons will still be enabled.
I could use some suggestions on how to proceed. I am overly tempted to
start changing everything in sight, but that is likely to cause all sorts
of bugs, subtle and otherwise....
Thanks,
Ben
--
Ben Greear <greearb@candelatech.com>
Candela Technologies Inc http://www.candelatech.com
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v2] mac80211: Optimize scans on current operating channel.
2011-01-26 18:39 ` Ben Greear
@ 2011-01-27 13:55 ` Johannes Berg
0 siblings, 0 replies; 13+ messages in thread
From: Johannes Berg @ 2011-01-27 13:55 UTC (permalink / raw)
To: Ben Greear; +Cc: linux-wireless
On Wed, 2011-01-26 at 10:39 -0800, Ben Greear wrote:
> The offchannel_stop_beaconing doesn't actually change
> state, it just calls
>
> ieee80211_bss_info_change_notify(
> sdata, BSS_CHANGED_BEACON_ENABLED);
>
> That method will disable beaconing if SCAN_SW_SCANNING is set,
> regardless of current channel.
>
> So, if that work.c logic is happening during a scan, we
> will always disable scanning, but if we are not SW scanning,
> then that offchannel_stop_beaconing won't actually do
> anything useful.
> My first inclination is to add another check in bss_info_change_notify
> to test if we are really off-channel:
>
> if (local->quiescing || !ieee80211_sdata_running(sdata) ||
> (test_bit(SCAN_SW_SCANNING, &local->scanning) &&
> test_bit(SCAN_OFF_CHANNEL, &local->scanning)) {
> sdata->vif.bss_conf.enable_beacon = false;
>
> And either set the SCAN_OFF_CHANNEL flag before calling offchannel_stop_beaconing,
> or perhaps set that flag in offchannel_stop_beaconing. The first might be
> less risky, but that leaves the work.c logic funky in my opinion, since if
> we are not actually scanning, beacons will still be enabled.
Yes, I'm a bit concerned about that too -- that bit really shouldn't be
in the scan bits, and then we can set it in offchannel_stop_beaconing or
whatever the combination ends up being called.
> I could use some suggestions on how to proceed. I am overly tempted to
> start changing everything in sight, but that is likely to cause all sorts
> of bugs, subtle and otherwise....
:)
I've long been tempted to rewrite scanning in terms of off-channel work
items ... Not really sure what to do.
johannes
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v2] mac80211: Optimize scans on current operating channel.
2011-01-26 18:10 ` Ben Greear
@ 2011-01-27 13:56 ` Johannes Berg
0 siblings, 0 replies; 13+ messages in thread
From: Johannes Berg @ 2011-01-27 13:56 UTC (permalink / raw)
To: Ben Greear; +Cc: linux-wireless
On Wed, 2011-01-26 at 10:10 -0800, Ben Greear wrote:
> > However, I guess it'd also be nice to also address the TODO in work.c
> > and move the "don't do so much if not really off-channel" logic into
> > that function? You must run into that when trying to associate on one
> > vif and the others get stopped etc. unnecessarily to do that.
>
> I think you are right about this, but due to the tricky nature
> of this code, maybe save that for a second patch?
Yeah, I think that might be good -- but maybe prepare in your current
version by not having the offchannel bit in the scan?
In fact -- I'm not even sure why you have two off-channel bits there,
but please reply to my comments on v3 rather than here.
johannes
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v2] mac80211: Optimize scans on current operating channel.
2011-01-21 18:03 greearb
@ 2011-01-21 18:06 ` Ben Greear
0 siblings, 0 replies; 13+ messages in thread
From: Ben Greear @ 2011-01-21 18:06 UTC (permalink / raw)
To: netdev
On 01/21/2011 10:03 AM, greearb@candelatech.com wrote:
> From: Ben Greear<greearb@candelatech.com>
>
> This should decrease un-necessary flushes, on/off channel work,
> and channel changes in cases where the only scanned channel is
> the current operating channel.
Sorry..meant to send this to linux-wireless....
Ben
--
Ben Greear <greearb@candelatech.com>
Candela Technologies Inc http://www.candelatech.com
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH v2] mac80211: Optimize scans on current operating channel.
@ 2011-01-21 18:03 greearb
2011-01-21 18:06 ` Ben Greear
0 siblings, 1 reply; 13+ messages in thread
From: greearb @ 2011-01-21 18:03 UTC (permalink / raw)
To: netdev; +Cc: Ben Greear
From: Ben Greear <greearb@candelatech.com>
This should decrease un-necessary flushes, on/off channel work,
and channel changes in cases where the only scanned channel is
the current operating channel.
Signed-off-by: Ben Greear <greearb@candelatech.com>
---
v2: Check channels instead of flag when determining if we should
do a channel change in scan_completed_finish.
:100644 100644 c47d7c0... 59fe5e7... M net/mac80211/ieee80211_i.h
:100644 100644 1236710... e6de0e7... M net/mac80211/rx.c
:100644 100644 3e660db... fa0aeb9... M net/mac80211/scan.c
net/mac80211/ieee80211_i.h | 5 +++++
net/mac80211/rx.c | 11 ++++++++---
net/mac80211/scan.c | 41 +++++++++++++++++++++++++----------------
3 files changed, 38 insertions(+), 19 deletions(-)
diff --git a/net/mac80211/ieee80211_i.h b/net/mac80211/ieee80211_i.h
index c47d7c0..59fe5e7 100644
--- a/net/mac80211/ieee80211_i.h
+++ b/net/mac80211/ieee80211_i.h
@@ -660,6 +660,10 @@ struct tpt_led_trigger {
* that the scan completed.
* @SCAN_ABORTED: Set for our scan work function when the driver reported
* a scan complete for an aborted scan.
+ * @SCAN_LEFT_OPER_CHANNEL: Set this flag if the scan process leaves the
+ * operating channel at any time. If scanning ONLY the current operating
+ * channel this flag should not be set, and this will allow fewer
+ * offchannel changes.
*/
enum {
SCAN_SW_SCANNING,
@@ -667,6 +671,7 @@ enum {
SCAN_OFF_CHANNEL,
SCAN_COMPLETED,
SCAN_ABORTED,
+ SCAN_LEFT_OPER_CHANNEL,
};
/**
diff --git a/net/mac80211/rx.c b/net/mac80211/rx.c
index 1236710..e6de0e7 100644
--- a/net/mac80211/rx.c
+++ b/net/mac80211/rx.c
@@ -388,6 +388,7 @@ ieee80211_rx_h_passive_scan(struct ieee80211_rx_data *rx)
struct ieee80211_local *local = rx->local;
struct ieee80211_rx_status *status = IEEE80211_SKB_RXCB(rx->skb);
struct sk_buff *skb = rx->skb;
+ int ret;
if (likely(!(status->rx_flags & IEEE80211_RX_IN_SCAN)))
return RX_CONTINUE;
@@ -396,10 +397,14 @@ ieee80211_rx_h_passive_scan(struct ieee80211_rx_data *rx)
return ieee80211_scan_rx(rx->sdata, skb);
if (test_bit(SCAN_SW_SCANNING, &local->scanning)) {
- /* drop all the other packets during a software scan anyway */
- if (ieee80211_scan_rx(rx->sdata, skb) != RX_QUEUED)
+ ret = ieee80211_scan_rx(rx->sdata, skb);
+ /* drop all the other packets while scanning off channel */
+ if (ret != RX_QUEUED &&
+ test_bit(SCAN_OFF_CHANNEL, &local->scanning)) {
dev_kfree_skb(skb);
- return RX_QUEUED;
+ return RX_QUEUED;
+ }
+ return ret;
}
/* scanning finished during invoking of handlers */
diff --git a/net/mac80211/scan.c b/net/mac80211/scan.c
index 3e660db..fa0aeb9 100644
--- a/net/mac80211/scan.c
+++ b/net/mac80211/scan.c
@@ -293,11 +293,14 @@ static void __ieee80211_scan_completed_finish(struct ieee80211_hw *hw,
{
struct ieee80211_local *local = hw_to_local(hw);
- ieee80211_hw_config(local, IEEE80211_CONF_CHANGE_CHANNEL);
+ if ((local->oper_channel != local->hw.conf.channel) || was_hw_scan)
+ ieee80211_hw_config(local, IEEE80211_CONF_CHANGE_CHANNEL);
+
if (!was_hw_scan) {
ieee80211_configure_filter(local);
drv_sw_scan_complete(local);
- ieee80211_offchannel_return(local, true);
+ if (test_bit(SCAN_LEFT_OPER_CHANNEL, &local->scanning))
+ ieee80211_offchannel_return(local, true);
}
mutex_lock(&local->mtx);
@@ -397,13 +400,10 @@ static int ieee80211_start_sw_scan(struct ieee80211_local *local)
drv_sw_scan_start(local);
- ieee80211_offchannel_stop_beaconing(local);
-
local->leave_oper_channel_time = 0;
local->next_scan_state = SCAN_DECISION;
local->scan_channel_idx = 0;
-
- drv_flush(local, false);
+ __clear_bit(SCAN_LEFT_OPER_CHANNEL, &local->scanning);
ieee80211_configure_filter(local);
@@ -543,7 +543,18 @@ static void ieee80211_scan_state_decision(struct ieee80211_local *local,
}
mutex_unlock(&local->iflist_mtx);
- if (local->scan_channel) {
+ next_chan = local->scan_req->channels[local->scan_channel_idx];
+
+ if (local->oper_channel == local->hw.conf.channel) {
+ if (next_chan == local->oper_channel)
+ local->next_scan_state = SCAN_SET_CHANNEL;
+ else
+ /*
+ * we're on the operating channel currently, let's
+ * leave that channel now to scan another one
+ */
+ local->next_scan_state = SCAN_LEAVE_OPER_CHANNEL;
+ } else {
/*
* we're currently scanning a different channel, let's
* see if we can scan another channel without interfering
@@ -559,7 +570,6 @@ static void ieee80211_scan_state_decision(struct ieee80211_local *local,
*
* Otherwise switch back to the operating channel.
*/
- next_chan = local->scan_req->channels[local->scan_channel_idx];
bad_latency = time_after(jiffies +
ieee80211_scan_get_channel_time(next_chan),
@@ -577,12 +587,6 @@ static void ieee80211_scan_state_decision(struct ieee80211_local *local,
local->next_scan_state = SCAN_ENTER_OPER_CHANNEL;
else
local->next_scan_state = SCAN_SET_CHANNEL;
- } else {
- /*
- * we're on the operating channel currently, let's
- * leave that channel now to scan another one
- */
- local->next_scan_state = SCAN_LEAVE_OPER_CHANNEL;
}
*next_delay = 0;
@@ -591,9 +595,12 @@ static void ieee80211_scan_state_decision(struct ieee80211_local *local,
static void ieee80211_scan_state_leave_oper_channel(struct ieee80211_local *local,
unsigned long *next_delay)
{
+ ieee80211_offchannel_stop_beaconing(local);
+
ieee80211_offchannel_stop_station(local);
__set_bit(SCAN_OFF_CHANNEL, &local->scanning);
+ __set_bit(SCAN_LEFT_OPER_CHANNEL, &local->scanning);
/*
* What if the nullfunc frames didn't arrive?
@@ -640,8 +647,10 @@ static void ieee80211_scan_state_set_channel(struct ieee80211_local *local,
chan = local->scan_req->channels[local->scan_channel_idx];
local->scan_channel = chan;
- if (ieee80211_hw_config(local, IEEE80211_CONF_CHANGE_CHANNEL))
- skip = 1;
+
+ if (chan != local->hw.conf.channel)
+ if (ieee80211_hw_config(local, IEEE80211_CONF_CHANGE_CHANNEL))
+ skip = 1;
/* advance state machine to next channel/band */
local->scan_channel_idx++;
--
1.7.2.3
^ permalink raw reply related [flat|nested] 13+ messages in thread
end of thread, other threads:[~2011-01-27 13:56 UTC | newest]
Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-01-21 18:05 [PATCH v2] mac80211: Optimize scans on current operating channel greearb
2011-01-23 9:18 ` Johannes Berg
2011-01-23 15:49 ` Ben Greear
2011-01-24 22:12 ` Ben Greear
2011-01-26 15:22 ` Johannes Berg
2011-01-26 17:28 ` Ben Greear
2011-01-26 15:36 ` Johannes Berg
2011-01-26 18:10 ` Ben Greear
2011-01-27 13:56 ` Johannes Berg
2011-01-26 18:39 ` Ben Greear
2011-01-27 13:55 ` Johannes Berg
-- strict thread matches above, loose matches on Subject: below --
2011-01-21 18:03 greearb
2011-01-21 18:06 ` Ben Greear
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.