2010-09-28 00:07:41

by Ben Greear

[permalink] [raw]
Subject: [PATCH 1/2] ath9k: Check for identical channels when changing channels.

From: Ben Greear <[email protected]>

Don't do un-needed work. Should be useful when scanning on
only the working channel.

Signed-off-by: Ben Greear <[email protected]>
---
:100644 100644 a133878... 4d34ba5... M drivers/net/wireless/ath/ath9k/main.c
drivers/net/wireless/ath/ath9k/main.c | 5 +++++
1 files changed, 5 insertions(+), 0 deletions(-)

diff --git a/drivers/net/wireless/ath/ath9k/main.c b/drivers/net/wireless/ath/ath9k/main.c
index a133878..4d34ba5 100644
--- a/drivers/net/wireless/ath/ath9k/main.c
+++ b/drivers/net/wireless/ath/ath9k/main.c
@@ -1602,6 +1602,11 @@ static int ath9k_config(struct ieee80211_hw *hw, u32 changed)
struct ieee80211_channel *curchan = hw->conf.channel;
int pos = curchan->hw_value;

+ /* If channels are the same, then don't actually do anything.
+ */
+ if (sc->sc_ah->curchan == &sc->sc_ah->channels[pos])
+ goto skip_chan_change;
+
aphy->chan_idx = pos;
aphy->chan_is_ht = conf_is_ht(conf);
if (hw->conf.flags & IEEE80211_CONF_OFFCHANNEL)
--
1.7.2.2



2010-09-29 05:48:48

by Jouni Malinen

[permalink] [raw]
Subject: Re: [PATCH 2/2] mac80211: Allow scanning single channel if other VIF is associated.

On Tue, Sep 28, 2010 at 08:17:04AM -0700, Ben Greear wrote:
> On 09/28/2010 06:57 AM, Johannes Berg wrote:
> >You can run all networks in a single wpa_s instance, I believe, and then
> >it'd know about all this, right?
>
> But then, if you want to add an additional interface, you have to restart
> everything.

We would you restart everything? You can add/remove interface
dynamically..

> I was thinking that maybe I could keep the logic in wpa_s, but instead of
> all the special casing, just locate the current associated channel for the
> phy in question and populate the scan-req with that single channel.

wpa_supplicant already stores the operating channel: wpa_s->assoc_freq.
You can iterate over the configured interfaces and fetch the current
operating channel(s) using that without adding anything new to the
kernel. With that, you could make some wpa_supplicant scans (e.g.,
bgscan or scan for new networks if only a single network block is
enabled) restrict the list of channels to scan. Still, some other scan
requests (e.g., user initiated full scan or P2P Search) would need to be
able to specify more channels even if one of the interfaces is
associated.

I would like to get rid of the one-channel-per-phy restriction, so I
don't think I would like to see new code added for that.

--
Jouni Malinen PGP id EFC895FA

2010-09-28 07:22:48

by Johannes Berg

[permalink] [raw]
Subject: Re: [PATCH 2/2] mac80211: Allow scanning single channel if other VIF is associated.

On Mon, 2010-09-27 at 17:07 -0700, [email protected] wrote:
> From: Ben Greear <[email protected]>
>
> This patch aims to decrease channel switching when there is at least one
> interface associated. This should help multiple station interfaces co-exist
> on the same hardware, especially in WPA mode.

Totally NACK. The code is ugly, complex, and unnecessary.

You can make wpa_s scan the channels one by one, or you can improve the
off-channel behaviour, but this is a workaround that just isn't
appropriate.

johannes


2010-09-29 15:28:07

by Ben Greear

[permalink] [raw]
Subject: Re: [PATCH 2/2] mac80211: Allow scanning single channel if other VIF is associated.

On 09/28/2010 10:48 PM, Jouni Malinen wrote:
> On Tue, Sep 28, 2010 at 08:17:04AM -0700, Ben Greear wrote:
>> On 09/28/2010 06:57 AM, Johannes Berg wrote:
>>> You can run all networks in a single wpa_s instance, I believe, and then
>>> it'd know about all this, right?
>>
>> But then, if you want to add an additional interface, you have to restart
>> everything.
>
> We would you restart everything? You can add/remove interface
> dynamically..

I prefer to run one supplicant process per interface..just makes
everything easier in my environment.

>> I was thinking that maybe I could keep the logic in wpa_s, but instead of
>> all the special casing, just locate the current associated channel for the
>> phy in question and populate the scan-req with that single channel.
>
> wpa_supplicant already stores the operating channel: wpa_s->assoc_freq.
> You can iterate over the configured interfaces and fetch the current
> operating channel(s) using that without adding anything new to the
> kernel. With that, you could make some wpa_supplicant scans (e.g.,
> bgscan or scan for new networks if only a single network block is
> enabled) restrict the list of channels to scan. Still, some other scan
> requests (e.g., user initiated full scan or P2P Search) would need to be
> able to specify more channels even if one of the interfaces is
> associated.

Even if wpa_supplicant can work around this in user-space, the kernel
scanning logic for non-supplicant controlled interfaces still needs
the kernel portion.

That said, it looks like the patch is DOA as Johannes doesn't like
it at all, so I'll quite submitting it :)

> I would like to get rid of the one-channel-per-phy restriction, so I
> don't think I would like to see new code added for that.

If that ever happened, then the can_scan_one logic could simply indicate
you want to scan only on associated channels, not all of them. It
would still help by not scanning on channels you have no intention
of using because other VIFs are already associated on channels you do
wish to use.

Thanks,
Ben


--
Ben Greear <[email protected]>
Candela Technologies Inc http://www.candelatech.com

2010-09-28 14:06:06

by Felix Fietkau

[permalink] [raw]
Subject: Re: [PATCH 1/2] ath9k: Check for identical channels when changing channels.

On 2010-09-28 3:37 PM, Ben Greear wrote:
> On 09/28/2010 01:51 AM, Felix Fietkau wrote:
>> On 2010-09-28 2:07 AM, [email protected] wrote:
>>> From: Ben Greear<[email protected]>
>>>
>>> Don't do un-needed work. Should be useful when scanning on
>>> only the working channel.
>>>
>>> Signed-off-by: Ben Greear<[email protected]>
>>> ---
>>> :100644 100644 a133878... 4d34ba5... M drivers/net/wireless/ath/ath9k/main.c
>>> drivers/net/wireless/ath/ath9k/main.c | 5 +++++
>>> 1 files changed, 5 insertions(+), 0 deletions(-)
>>>
>>> diff --git a/drivers/net/wireless/ath/ath9k/main.c b/drivers/net/wireless/ath/ath9k/main.c
>>> index a133878..4d34ba5 100644
>>> --- a/drivers/net/wireless/ath/ath9k/main.c
>>> +++ b/drivers/net/wireless/ath/ath9k/main.c
>>> @@ -1602,6 +1602,11 @@ static int ath9k_config(struct ieee80211_hw *hw, u32 changed)
>>> struct ieee80211_channel *curchan = hw->conf.channel;
>>> int pos = curchan->hw_value;
>>>
>>> + /* If channels are the same, then don't actually do anything.
>>> + */
>>> + if (sc->sc_ah->curchan ==&sc->sc_ah->channels[pos])
>>> + goto skip_chan_change;
>>> +
>> NACK. This ignores off-channel flag changes, which are important for
>> calibration.
>
> Is it enough to just add a check to make sure no changes were made to
> the SC_OP_OFFCHANNEL flag?
Shouldn't this check be done in mac80211 then?

- Felix

2010-09-28 15:18:40

by Johannes Berg

[permalink] [raw]
Subject: Re: [PATCH 2/2] mac80211: Allow scanning single channel if other VIF is associated.

On Tue, 2010-09-28 at 08:17 -0700, Ben Greear wrote:

> > You can run all networks in a single wpa_s instance, I believe, and then
> > it'd know about all this, right?
>
> But then, if you want to add an additional interface, you have to restart
> everything.

Well, you can add it dynamically over dbus or the unix socket control
interface.

> I was thinking that maybe I could keep the logic in wpa_s, but instead of
> all the special casing, just locate the current associated channel for the
> phy in question and populate the scan-req with that single channel.
>
> Then, I think I wouldn't have to muck with much of the scan logic..just
> a bit of code on the entry point to select the proper channel.
>
> Does that sound like a possible solution to you?

I don't care all that much, but I really don't like the in-kernel
workaround. For that, you'd have to iterate over "iw <wlanX> link" I
suppose.

johannes


2010-09-28 15:22:26

by Ben Greear

[permalink] [raw]
Subject: Re: [PATCH 1/2] ath9k: Check for identical channels when changing channels.

On 09/28/2010 07:05 AM, Felix Fietkau wrote:
> On 2010-09-28 3:37 PM, Ben Greear wrote:
>> On 09/28/2010 01:51 AM, Felix Fietkau wrote:
>>> On 2010-09-28 2:07 AM, [email protected] wrote:
>>>> From: Ben Greear<[email protected]>
>>>>
>>>> Don't do un-needed work. Should be useful when scanning on
>>>> only the working channel.
>>>>
>>>> Signed-off-by: Ben Greear<[email protected]>
>>>> ---
>>>> :100644 100644 a133878... 4d34ba5... M drivers/net/wireless/ath/ath9k/main.c
>>>> drivers/net/wireless/ath/ath9k/main.c | 5 +++++
>>>> 1 files changed, 5 insertions(+), 0 deletions(-)
>>>>
>>>> diff --git a/drivers/net/wireless/ath/ath9k/main.c b/drivers/net/wireless/ath/ath9k/main.c
>>>> index a133878..4d34ba5 100644
>>>> --- a/drivers/net/wireless/ath/ath9k/main.c
>>>> +++ b/drivers/net/wireless/ath/ath9k/main.c
>>>> @@ -1602,6 +1602,11 @@ static int ath9k_config(struct ieee80211_hw *hw, u32 changed)
>>>> struct ieee80211_channel *curchan = hw->conf.channel;
>>>> int pos = curchan->hw_value;
>>>>
>>>> + /* If channels are the same, then don't actually do anything.
>>>> + */
>>>> + if (sc->sc_ah->curchan ==&sc->sc_ah->channels[pos])
>>>> + goto skip_chan_change;
>>>> +
>>> NACK. This ignores off-channel flag changes, which are important for
>>> calibration.
>>
>> Is it enough to just add a check to make sure no changes were made to
>> the SC_OP_OFFCHANNEL flag?
> Shouldn't this check be done in mac80211 then?

Maybe so...I was curious why the check wasn't already in place,
and assumed that maybe some drivers needed it, or maybe the calling
code doesn't always know the driver's current channel?

Thanks,
Ben

>
> - Felix


--
Ben Greear <[email protected]>
Candela Technologies Inc http://www.candelatech.com

2010-09-28 08:51:38

by Felix Fietkau

[permalink] [raw]
Subject: Re: [PATCH 1/2] ath9k: Check for identical channels when changing channels.

On 2010-09-28 2:07 AM, [email protected] wrote:
> From: Ben Greear <[email protected]>
>
> Don't do un-needed work. Should be useful when scanning on
> only the working channel.
>
> Signed-off-by: Ben Greear <[email protected]>
> ---
> :100644 100644 a133878... 4d34ba5... M drivers/net/wireless/ath/ath9k/main.c
> drivers/net/wireless/ath/ath9k/main.c | 5 +++++
> 1 files changed, 5 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/net/wireless/ath/ath9k/main.c b/drivers/net/wireless/ath/ath9k/main.c
> index a133878..4d34ba5 100644
> --- a/drivers/net/wireless/ath/ath9k/main.c
> +++ b/drivers/net/wireless/ath/ath9k/main.c
> @@ -1602,6 +1602,11 @@ static int ath9k_config(struct ieee80211_hw *hw, u32 changed)
> struct ieee80211_channel *curchan = hw->conf.channel;
> int pos = curchan->hw_value;
>
> + /* If channels are the same, then don't actually do anything.
> + */
> + if (sc->sc_ah->curchan == &sc->sc_ah->channels[pos])
> + goto skip_chan_change;
> +
NACK. This ignores off-channel flag changes, which are important for
calibration.

- Felix

2010-09-28 15:17:09

by Ben Greear

[permalink] [raw]
Subject: Re: [PATCH 2/2] mac80211: Allow scanning single channel if other VIF is associated.

On 09/28/2010 06:57 AM, Johannes Berg wrote:
> On Tue, 2010-09-28 at 06:50 -0700, Ben Greear wrote:
>
>> At best, it would be a race for wpa_s to determine if any other interface
>> on it's interface's hardware is associated, so I don't see a good way to
>> do this in user-space.
>>
>> When at least one is associated, I don't want the NIC to go offchannel at all,
>> at least not for wpa_s scanning. Anything else is going to interrupt other
>> stations's traffic, and for no good reason that I can see, since the scanning
>> interface must associate on the same channel as the rest of the stations
>> anyway.
>>
>> If you have any suggestions for how to accomplish this, please let me know..
>> otherwise, I can just carry this patch in my tree.
>
> You can run all networks in a single wpa_s instance, I believe, and then
> it'd know about all this, right?

But then, if you want to add an additional interface, you have to restart
everything.

I was thinking that maybe I could keep the logic in wpa_s, but instead of
all the special casing, just locate the current associated channel for the
phy in question and populate the scan-req with that single channel.

Then, I think I wouldn't have to muck with much of the scan logic..just
a bit of code on the entry point to select the proper channel.

Does that sound like a possible solution to you?

Thanks,
Ben

>
> johannes


--
Ben Greear <[email protected]>
Candela Technologies Inc http://www.candelatech.com

2010-09-28 13:37:30

by Ben Greear

[permalink] [raw]
Subject: Re: [PATCH 1/2] ath9k: Check for identical channels when changing channels.

On 09/28/2010 01:51 AM, Felix Fietkau wrote:
> On 2010-09-28 2:07 AM, [email protected] wrote:
>> From: Ben Greear<[email protected]>
>>
>> Don't do un-needed work. Should be useful when scanning on
>> only the working channel.
>>
>> Signed-off-by: Ben Greear<[email protected]>
>> ---
>> :100644 100644 a133878... 4d34ba5... M drivers/net/wireless/ath/ath9k/main.c
>> drivers/net/wireless/ath/ath9k/main.c | 5 +++++
>> 1 files changed, 5 insertions(+), 0 deletions(-)
>>
>> diff --git a/drivers/net/wireless/ath/ath9k/main.c b/drivers/net/wireless/ath/ath9k/main.c
>> index a133878..4d34ba5 100644
>> --- a/drivers/net/wireless/ath/ath9k/main.c
>> +++ b/drivers/net/wireless/ath/ath9k/main.c
>> @@ -1602,6 +1602,11 @@ static int ath9k_config(struct ieee80211_hw *hw, u32 changed)
>> struct ieee80211_channel *curchan = hw->conf.channel;
>> int pos = curchan->hw_value;
>>
>> + /* If channels are the same, then don't actually do anything.
>> + */
>> + if (sc->sc_ah->curchan ==&sc->sc_ah->channels[pos])
>> + goto skip_chan_change;
>> +
> NACK. This ignores off-channel flag changes, which are important for
> calibration.

Is it enough to just add a check to make sure no changes were made to
the SC_OP_OFFCHANNEL flag?

Thanks,
Ben

>
> - Felix


--
Ben Greear <[email protected]>
Candela Technologies Inc http://www.candelatech.com

2010-09-28 00:07:50

by Ben Greear

[permalink] [raw]
Subject: [PATCH 2/2] mac80211: Allow scanning single channel if other VIF is associated.

From: Ben Greear <[email protected]>

This patch aims to decrease channel switching when there is at least one
interface associated. This should help multiple station interfaces co-exist
on the same hardware, especially in WPA mode.

Signed-off-by: Ben Greear <[email protected]>
---
:100644 100644 f0518b0... 7ecf8b0... M include/linux/nl80211.h
:100644 100644 a0613ff... f355b8c... M include/net/cfg80211.h
:100644 100644 945fbf2... ac720d2... M net/mac80211/ieee80211_i.h
:100644 100644 8b733cf... 6323954... M net/mac80211/mlme.c
:100644 100644 0b0e83e... 318b2de... M net/mac80211/rx.c
:100644 100644 57b5e66... 59d5925... M net/mac80211/scan.c
:100644 100644 ae344d1... 1bfc1e0... M net/mac80211/work.c
:100644 100644 4ff827e... b45646e... M net/wireless/nl80211.c
:100644 100644 f161b98... 87b0cd7... M net/wireless/sme.c
include/linux/nl80211.h | 2 +
include/net/cfg80211.h | 3 +
net/mac80211/ieee80211_i.h | 2 +
net/mac80211/mlme.c | 14 +++--
net/mac80211/rx.c | 2 +-
net/mac80211/scan.c | 110 +++++++++++++++++++++++++++++++++-----------
net/mac80211/work.c | 17 ++++--
net/wireless/nl80211.c | 6 ++
net/wireless/sme.c | 5 ++
9 files changed, 121 insertions(+), 40 deletions(-)

diff --git a/include/linux/nl80211.h b/include/linux/nl80211.h
index f0518b0..7ecf8b0 100644
--- a/include/linux/nl80211.h
+++ b/include/linux/nl80211.h
@@ -965,6 +965,8 @@ enum nl80211_attrs {
NL80211_ATTR_CONTROL_PORT_ETHERTYPE,
NL80211_ATTR_CONTROL_PORT_NO_ENCRYPT,

+ NL80211_ATTR_SCAN_ONE_IF_ASSOC,
+
/* add attributes here, update the policy in nl80211.c */

__NL80211_ATTR_AFTER_LAST,
diff --git a/include/net/cfg80211.h b/include/net/cfg80211.h
index a0613ff..f355b8c 100644
--- a/include/net/cfg80211.h
+++ b/include/net/cfg80211.h
@@ -667,6 +667,8 @@ struct cfg80211_ssid {
* @wiphy: the wiphy this was for
* @dev: the interface
* @aborted: (internal) scan request was notified as aborted
+ * @can_scan_one: If true, only scan active channel if at least one
+ * vif is already associated.
*/
struct cfg80211_scan_request {
struct cfg80211_ssid *ssids;
@@ -679,6 +681,7 @@ struct cfg80211_scan_request {
struct wiphy *wiphy;
struct net_device *dev;
bool aborted;
+ bool can_scan_one;

/* keep last */
struct ieee80211_channel *channels[0];
diff --git a/net/mac80211/ieee80211_i.h b/net/mac80211/ieee80211_i.h
index 945fbf2..ac720d2 100644
--- a/net/mac80211/ieee80211_i.h
+++ b/net/mac80211/ieee80211_i.h
@@ -815,6 +815,8 @@ struct ieee80211_local {
enum ieee80211_band hw_scan_band;
int scan_channel_idx;
int scan_ies_len;
+ int scanned_count; /* how many channels scanned so far in this scan */
+ bool scan_probe_once; /* if true, scan only the current channel. */

unsigned long leave_oper_channel_time;
enum mac80211_scan_state next_scan_state;
diff --git a/net/mac80211/mlme.c b/net/mac80211/mlme.c
index 8b733cf..6323954 100644
--- a/net/mac80211/mlme.c
+++ b/net/mac80211/mlme.c
@@ -1863,10 +1863,11 @@ void ieee80211_sta_work(struct ieee80211_sub_if_data *sdata)

else if (ifmgd->probe_send_count < IEEE80211_MAX_PROBE_TRIES) {
#ifdef CONFIG_MAC80211_VERBOSE_DEBUG
- printk(KERN_DEBUG "No probe response from AP %pM"
- " after %dms, try %d\n", bssid,
+ printk(KERN_DEBUG "%s: No probe response from AP %pM"
+ " after %dms, try %d flags: 0x%x\n",
+ sdata->dev->name, bssid,
(1000 * IEEE80211_PROBE_WAIT)/HZ,
- ifmgd->probe_send_count);
+ ifmgd->probe_send_count, ifmgd->flags);
#endif
ieee80211_mgd_probe_ap_send(sdata);
} else {
@@ -1876,9 +1877,10 @@ void ieee80211_sta_work(struct ieee80211_sub_if_data *sdata)
*/
ifmgd->flags &= ~(IEEE80211_STA_CONNECTION_POLL |
IEEE80211_STA_BEACON_POLL);
- printk(KERN_DEBUG "No probe response from AP %pM"
- " after %dms, disconnecting.\n",
- bssid, (1000 * IEEE80211_PROBE_WAIT)/HZ);
+ printk(KERN_DEBUG "%s: No probe response from AP %pM"
+ " after %dms, disconnecting, flags: 0x%x\n",
+ sdata->dev->name, bssid,
+ (1000 * IEEE80211_PROBE_WAIT)/HZ, ifmgd->flags);
ieee80211_set_disassoc(sdata, true);
mutex_unlock(&ifmgd->mtx);
mutex_lock(&local->mtx);
diff --git a/net/mac80211/rx.c b/net/mac80211/rx.c
index 0b0e83e..318b2de 100644
--- a/net/mac80211/rx.c
+++ b/net/mac80211/rx.c
@@ -2407,7 +2407,7 @@ static void ieee80211_rx_handlers(struct ieee80211_rx_data *rx,
do { \
res = rxh(rx); \
if (res != RX_CONTINUE) \
- goto rxh_next; \
+ goto rxh_next; \
} while (0);

while ((skb = __skb_dequeue(frames))) {
diff --git a/net/mac80211/scan.c b/net/mac80211/scan.c
index 57b5e66..59d5925 100644
--- a/net/mac80211/scan.c
+++ b/net/mac80211/scan.c
@@ -293,7 +293,9 @@ static void __ieee80211_scan_completed(struct ieee80211_hw *hw, bool aborted)
/* we only have to protect scan_req and hw/sw scan */
mutex_unlock(&local->mtx);

- ieee80211_hw_config(local, IEEE80211_CONF_CHANGE_CHANNEL);
+ if (!local->scan_probe_once)
+ ieee80211_hw_config(local, IEEE80211_CONF_CHANGE_CHANNEL);
+
if (was_hw_scan)
goto done;

@@ -301,7 +303,8 @@ static void __ieee80211_scan_completed(struct ieee80211_hw *hw, bool aborted)

drv_sw_scan_complete(local);

- ieee80211_offchannel_return(local, true);
+ if (!local->scan_probe_once)
+ ieee80211_offchannel_return(local, true);

done:
mutex_lock(&local->mtx);
@@ -341,13 +344,53 @@ static int ieee80211_start_sw_scan(struct ieee80211_local *local)
* nullfunc frames and probe requests will be dropped in
* ieee80211_tx_h_check_assoc().
*/
- drv_sw_scan_start(local);
+ int avifs = 0;
+ int svifs = 0;
+ struct ieee80211_sub_if_data *sdata;
+
+ local->scan_probe_once = false;
+ if (local->scan_req->can_scan_one) {
+ struct sta_info *sta;
+ mutex_lock(&local->iflist_mtx);
+ list_for_each_entry(sdata, &local->interfaces, list) {
+ if (!ieee80211_sdata_running(sdata))
+ continue;
+
+ if (sdata->vif.type != NL80211_IFTYPE_STATION)
+ avifs++;
+ else
+ svifs++;
+ }
+ mutex_unlock(&local->iflist_mtx);
+
+ rcu_read_lock();
+ list_for_each_entry_rcu(sta, &local->sta_list, list) {
+ if (!ieee80211_sdata_running(sta->sdata))
+ continue;
+ if (sta->sdata->vif.type != NL80211_IFTYPE_STATION)
+ continue;
+ if (test_sta_flags(sta, WLAN_STA_ASSOC))
+ avifs++;
+ }
+ rcu_read_unlock();
+
+ /* If one sta is associated, we don't want another to start
+ * scanning on all channels, as that will interfere with the
+ * one already associated.
+ */
+ if ((avifs > 1) || ((avifs == 1) && (svifs > 1)))
+ local->scan_probe_once = true;
+ }

- ieee80211_offchannel_stop_beaconing(local);
+ drv_sw_scan_start(local);

- local->leave_oper_channel_time = 0;
+ if (!local->scan_probe_once) {
+ ieee80211_offchannel_stop_beaconing(local);
+ local->leave_oper_channel_time = 0;
+ local->scan_channel_idx = 0;
+ }
+ local->scanned_count = 0;
local->next_scan_state = SCAN_DECISION;
- local->scan_channel_idx = 0;

drv_flush(local, false);

@@ -460,7 +503,8 @@ static int ieee80211_scan_state_decision(struct ieee80211_local *local,
struct ieee80211_channel *next_chan;

/* if no more bands/channels left, complete scan and advance to the idle state */
- if (local->scan_channel_idx >= local->scan_req->n_channels) {
+ if ((local->scan_channel_idx >= local->scan_req->n_channels) ||
+ (local->scanned_count && local->scan_probe_once)) {
__ieee80211_scan_completed(&local->hw, false);
return 1;
}
@@ -529,10 +573,13 @@ static int ieee80211_scan_state_decision(struct ieee80211_local *local,
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
+ * we're on the operating channel currently, Leave that
+ * channel if we are not probing the single channel.
*/
- local->next_scan_state = SCAN_LEAVE_OPER_CHANNEL;
+ if (local->scan_probe_once)
+ local->next_scan_state = SCAN_SET_CHANNEL;
+ else
+ local->next_scan_state = SCAN_LEAVE_OPER_CHANNEL;
}

*next_delay = 0;
@@ -567,14 +614,15 @@ static void ieee80211_scan_state_enter_oper_channel(struct ieee80211_local *loca
{
/* switch back to the operating channel */
local->scan_channel = NULL;
- ieee80211_hw_config(local, IEEE80211_CONF_CHANGE_CHANNEL);
-
- /*
- * Only re-enable station mode interface now; beaconing will be
- * re-enabled once the full scan has been completed.
- */
- ieee80211_offchannel_return(local, false);
+ if (!local->scan_probe_once) {
+ ieee80211_hw_config(local, IEEE80211_CONF_CHANGE_CHANNEL);

+ /*
+ * Only re-enable station mode interface now; beaconing will be
+ * re-enabled once the full scan has been completed.
+ */
+ ieee80211_offchannel_return(local, false);
+ }
__clear_bit(SCAN_OFF_CHANNEL, &local->scanning);

*next_delay = HZ / 5;
@@ -588,19 +636,25 @@ static void ieee80211_scan_state_set_channel(struct ieee80211_local *local,
struct ieee80211_channel *chan;

skip = 0;
- chan = local->scan_req->channels[local->scan_channel_idx];
+ if (local->scan_probe_once) {
+ chan = local->oper_channel;
+ local->scan_channel = chan;
+ } else {
+ chan = local->scan_req->channels[local->scan_channel_idx];
+ local->scan_channel = chan;

- local->scan_channel = chan;
- if (ieee80211_hw_config(local, IEEE80211_CONF_CHANGE_CHANNEL))
- skip = 1;
+ if (ieee80211_hw_config(local, IEEE80211_CONF_CHANGE_CHANNEL))
+ skip = 1;

- /* advance state machine to next channel/band */
- local->scan_channel_idx++;
+ /* advance state machine to next channel/band */
+ local->scan_channel_idx++;

- if (skip) {
- /* if we skip this channel return to the decision state */
- local->next_scan_state = SCAN_DECISION;
- return;
+ if (skip) {
+ /* if we skip this channel return to the decision
+ * state */
+ local->next_scan_state = SCAN_DECISION;
+ return;
+ }
}

/*
@@ -616,6 +670,7 @@ static void ieee80211_scan_state_set_channel(struct ieee80211_local *local,
if (chan->flags & IEEE80211_CHAN_PASSIVE_SCAN ||
!local->scan_req->n_ssids) {
*next_delay = IEEE80211_PASSIVE_CHANNEL_TIME;
+ local->scanned_count++;
local->next_scan_state = SCAN_DECISION;
return;
}
@@ -638,6 +693,7 @@ static void ieee80211_scan_state_send_probe(struct ieee80211_local *local,
local->scan_req->ssids[i].ssid_len,
local->scan_req->ie, local->scan_req->ie_len);

+ local->scanned_count++;
/*
* After sending probe requests, wait for probe responses
* on the channel.
diff --git a/net/mac80211/work.c b/net/mac80211/work.c
index ae344d1..1bfc1e0 100644
--- a/net/mac80211/work.c
+++ b/net/mac80211/work.c
@@ -910,12 +910,17 @@ static void ieee80211_work_work(struct work_struct *work)
* happen to be on the same channel as
* the requested channel
*/
- ieee80211_offchannel_stop_beaconing(local);
- ieee80211_offchannel_stop_station(local);
-
- local->tmp_channel = wk->chan;
- local->tmp_channel_type = wk->chan_type;
- ieee80211_hw_config(local, 0);
+ if (!(wk->chan == local->scan_channel ||
+ (wk->chan == local->oper_channel &&
+ !local->scan_channel))) {
+ /* Only change channels if we need to */
+ ieee80211_offchannel_stop_beaconing(local);
+ ieee80211_offchannel_stop_station(local);
+
+ local->tmp_channel = wk->chan;
+ local->tmp_channel_type = wk->chan_type;
+ ieee80211_hw_config(local, 0);
+ }
started = true;
wk->timeout = jiffies;
}
diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c
index 4ff827e..b45646e 100644
--- a/net/wireless/nl80211.c
+++ b/net/wireless/nl80211.c
@@ -138,6 +138,7 @@ static const struct nla_policy nl80211_policy[NL80211_ATTR_MAX+1] = {
[NL80211_ATTR_CONTROL_PORT] = { .type = NLA_FLAG },
[NL80211_ATTR_CONTROL_PORT_ETHERTYPE] = { .type = NLA_U16 },
[NL80211_ATTR_CONTROL_PORT_NO_ENCRYPT] = { .type = NLA_FLAG },
+ [NL80211_ATTR_SCAN_ONE_IF_ASSOC] = { .type = NLA_FLAG },
[NL80211_ATTR_PRIVACY] = { .type = NLA_FLAG },
[NL80211_ATTR_CIPHER_SUITE_GROUP] = { .type = NLA_U32 },
[NL80211_ATTR_WPA_VERSIONS] = { .type = NLA_U32 },
@@ -3217,6 +3218,11 @@ static int nl80211_trigger_scan(struct sk_buff *skb, struct genl_info *info)
goto out;
}

+ if (info->attrs[NL80211_ATTR_SCAN_ONE_IF_ASSOC])
+ request->can_scan_one = true;
+ else
+ request->can_scan_one = false;
+
if (n_ssids)
request->ssids = (void *)&request->channels[n_channels];
request->n_ssids = n_ssids;
diff --git a/net/wireless/sme.c b/net/wireless/sme.c
index f161b98..87b0cd7 100644
--- a/net/wireless/sme.c
+++ b/net/wireless/sme.c
@@ -105,6 +105,11 @@ static int cfg80211_conn_scan(struct wireless_dev *wdev)
if (!request)
return -ENOMEM;

+ /* If at least one VIF on this hardware is already associated, then
+ * only scan on the active channel.
+ */
+ request->can_scan_one = true;
+
if (wdev->conn->params.channel)
request->channels[0] = wdev->conn->params.channel;
else {
--
1.7.2.2


2010-09-28 13:57:05

by Johannes Berg

[permalink] [raw]
Subject: Re: [PATCH 2/2] mac80211: Allow scanning single channel if other VIF is associated.

On Tue, 2010-09-28 at 06:50 -0700, Ben Greear wrote:

> At best, it would be a race for wpa_s to determine if any other interface
> on it's interface's hardware is associated, so I don't see a good way to
> do this in user-space.
>
> When at least one is associated, I don't want the NIC to go offchannel at all,
> at least not for wpa_s scanning. Anything else is going to interrupt other
> stations's traffic, and for no good reason that I can see, since the scanning
> interface must associate on the same channel as the rest of the stations
> anyway.
>
> If you have any suggestions for how to accomplish this, please let me know..
> otherwise, I can just carry this patch in my tree.

You can run all networks in a single wpa_s instance, I believe, and then
it'd know about all this, right?

johannes


2010-09-28 13:50:34

by Ben Greear

[permalink] [raw]
Subject: Re: [PATCH 2/2] mac80211: Allow scanning single channel if other VIF is associated.

On 09/28/2010 12:22 AM, Johannes Berg wrote:
> On Mon, 2010-09-27 at 17:07 -0700, [email protected] wrote:
>> From: Ben Greear<[email protected]>
>>
>> This patch aims to decrease channel switching when there is at least one
>> interface associated. This should help multiple station interfaces co-exist
>> on the same hardware, especially in WPA mode.
>
> Totally NACK. The code is ugly, complex, and unnecessary.
>
> You can make wpa_s scan the channels one by one, or you can improve the
> off-channel behaviour, but this is a workaround that just isn't
> appropriate.

At best, it would be a race for wpa_s to determine if any other interface
on it's interface's hardware is associated, so I don't see a good way to
do this in user-space.

When at least one is associated, I don't want the NIC to go offchannel at all,
at least not for wpa_s scanning. Anything else is going to interrupt other
stations's traffic, and for no good reason that I can see, since the scanning
interface must associate on the same channel as the rest of the stations
anyway.

If you have any suggestions for how to accomplish this, please let me know..
otherwise, I can just carry this patch in my tree.

Thanks,
Ben

>
> johannes


--
Ben Greear <[email protected]>
Candela Technologies Inc http://www.candelatech.com