[RFC 1/3] mac80211: Support sw_scan_start_cur

[RFC 1/3] mac80211: Support sw_scan_start_cur

[greearb]

From: Ben Greear <greearb@candelatech.com>

This method is called when driver can support
scanning the currect active channel without otherwise
impeding traffic on that channel.  The mac80211 scan
logic may call this when we are only scanning on the
active channel and thus do not need to go off channel.

Signed-off-by: Ben Greear <greearb@candelatech.com>
---
:100644 100644 d6b0045... 3e89ae7... M	include/net/mac80211.h
 include/net/mac80211.h |    8 ++++++++
 1 files changed, 8 insertions(+), 0 deletions(-)

diff --git a/include/net/mac80211.h b/include/net/mac80211.h
index d6b0045..3e89ae7 100644
--- a/include/net/mac80211.h
+++ b/include/net/mac80211.h
@@ -1672,6 +1672,12 @@ enum ieee80211_ampdu_mlme_action {
  *	is started. Can be NULL, if the driver doesn't need this notification.
  *	The callback can sleep.
  *
+ * @sw_scan_start_cur: Notifier function that is called just before a software
+ *	scan on only the current channel is started. If NULL, sw_scan_start
+ *      will be used instead.  sw_scan_start_cur with second argument set to
+ *      false should be treated identically to sw_scan_start.
+ *	The callback can sleep.
+ *
  * @sw_scan_complete: Notifier function that is called just after a
  *	software scan finished. Can be NULL, if the driver doesn't need
  *	this notification.
@@ -1820,6 +1826,8 @@ struct ieee80211_ops {
 	int (*hw_scan)(struct ieee80211_hw *hw, struct ieee80211_vif *vif,
 		       struct cfg80211_scan_request *req);
 	void (*sw_scan_start)(struct ieee80211_hw *hw);
+	void (*sw_scan_start_cur)(struct ieee80211_hw *hw,
+				  bool cur_channel_only);
 	void (*sw_scan_complete)(struct ieee80211_hw *hw);
 	int (*get_stats)(struct ieee80211_hw *hw,
 			 struct ieee80211_low_level_stats *stats);
-- 
1.7.2.3

[RFC 2/3] mac80211: Support scanning only current active channel.

[greearb]

From: Ben Greear <greearb@candelatech.com>

This allows user-space to request scan on only the current
channel and have that not interfere with other traffic.

Especially useful when using virtual stations that must
all run on the same channel anyway.

Signed-off-by: Ben Greear <greearb@candelatech.com>
---
:100644 100644 78af32d... 7cd7af8... M	net/mac80211/driver-ops.h
:100644 100644 fbabbc2... 8c6d253... M	net/mac80211/driver-trace.h
:100644 100644 c47d7c0... 388db0e... M	net/mac80211/ieee80211_i.h
:100644 100644 1236710... b1767a5... M	net/mac80211/rx.c
:100644 100644 fb274db... b42e5ad... M	net/mac80211/scan.c
 net/mac80211/driver-ops.h   |   12 ++++++++
 net/mac80211/driver-trace.h |   18 ++++++++++++
 net/mac80211/ieee80211_i.h  |    3 ++
 net/mac80211/rx.c           |    1 +
 net/mac80211/scan.c         |   61 +++++++++++++++++++++++++++++++-----------
 5 files changed, 79 insertions(+), 16 deletions(-)

diff --git a/net/mac80211/driver-ops.h b/net/mac80211/driver-ops.h
index 78af32d..7cd7af8 100644
--- a/net/mac80211/driver-ops.h
+++ b/net/mac80211/driver-ops.h
@@ -201,6 +201,18 @@ static inline void drv_sw_scan_start(struct ieee80211_local *local)
 	trace_drv_return_void(local);
 }
 
+static inline void drv_sw_scan_start_cur(struct ieee80211_local *local,
+					 bool cur_channel_only)
+{
+	might_sleep();
+
+	trace_drv_sw_scan_start_cur(local);
+	if (local->ops->sw_scan_start_cur)
+		local->ops->sw_scan_start_cur(&local->hw,
+					      cur_channel_only);
+	trace_drv_return_void(local);
+}
+
 static inline void drv_sw_scan_complete(struct ieee80211_local *local)
 {
 	might_sleep();
diff --git a/net/mac80211/driver-trace.h b/net/mac80211/driver-trace.h
index fbabbc2..8c6d253 100644
--- a/net/mac80211/driver-trace.h
+++ b/net/mac80211/driver-trace.h
@@ -457,6 +457,24 @@ TRACE_EVENT(drv_sw_scan_start,
 	)
 );
 
+TRACE_EVENT(drv_sw_scan_start_cur,
+	TP_PROTO(struct ieee80211_local *local),
+
+	TP_ARGS(local),
+
+	TP_STRUCT__entry(
+		LOCAL_ENTRY
+	),
+
+	TP_fast_assign(
+		LOCAL_ASSIGN;
+	),
+
+	TP_printk(
+		LOCAL_PR_FMT, LOCAL_PR_ARG
+	)
+);
+
 TRACE_EVENT(drv_sw_scan_complete,
 	TP_PROTO(struct ieee80211_local *local),
 
diff --git a/net/mac80211/ieee80211_i.h b/net/mac80211/ieee80211_i.h
index c47d7c0..388db0e 100644
--- a/net/mac80211/ieee80211_i.h
+++ b/net/mac80211/ieee80211_i.h
@@ -660,6 +660,8 @@ 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_ON_CUR_CHANNEL:  Set when we are scanning only on the current
+ *      channel.  This means no off/on-channel logic needs to be run.
  */
 enum {
 	SCAN_SW_SCANNING,
@@ -667,6 +669,7 @@ enum {
 	SCAN_OFF_CHANNEL,
 	SCAN_COMPLETED,
 	SCAN_ABORTED,
+	SCAN_ON_CUR_CHANNEL,
 };
 
 /**
diff --git a/net/mac80211/rx.c b/net/mac80211/rx.c
index 1236710..b1767a5 100644
--- a/net/mac80211/rx.c
+++ b/net/mac80211/rx.c
@@ -2749,6 +2749,7 @@ static void __ieee80211_rx_handle_packet(struct ieee80211_hw *hw,
 		local->dot11ReceivedFragmentCount++;
 
 	if (unlikely(test_bit(SCAN_HW_SCANNING, &local->scanning) ||
+		     test_bit(SCAN_ON_CUR_CHANNEL, &local->scanning) ||
 		     test_bit(SCAN_OFF_CHANNEL, &local->scanning)))
 		status->rx_flags |= IEEE80211_RX_IN_SCAN;
 
diff --git a/net/mac80211/scan.c b/net/mac80211/scan.c
index fb274db..b42e5ad 100644
--- a/net/mac80211/scan.c
+++ b/net/mac80211/scan.c
@@ -293,11 +293,17 @@ 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 (!test_bit(SCAN_ON_CUR_CHANNEL, &local->scanning))
+		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_ON_CUR_CHANNEL, &local->scanning))
+			/* Don't call this if we never left the channel. */
+			ieee80211_offchannel_return(local, true);
+		else
+			__clear_bit(SCAN_ON_CUR_CHANNEL, &local->scanning);
 	}
 
 	mutex_lock(&local->mtx);
@@ -338,15 +344,28 @@ 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);
 
-	ieee80211_offchannel_stop_beaconing(local);
+	if (local->ops->sw_scan_start_cur &&
+	    local->scan_req->n_channels == 1 &&
+	    local->scan_req->channels[0] == local->hw.conf.channel) {
+		__set_bit(SCAN_ON_CUR_CHANNEL, &local->scanning);
+		drv_sw_scan_start_cur(local, true);
+	} else
+		drv_sw_scan_start(local);
+
+	/* If we are scanning one channel, and only our own channel
+	 * then we don't need to call the off-channel logic.
+	 */
+	if (!test_bit(SCAN_ON_CUR_CHANNEL, &local->scanning)) {
+		ieee80211_offchannel_stop_beaconing(local);
+		local->leave_oper_channel_time = 0;
+	}
 
-	local->leave_oper_channel_time = 0;
 	local->next_scan_state = SCAN_DECISION;
 	local->scan_channel_idx = 0;
 
-	drv_flush(local, false);
+	if (!test_bit(SCAN_ON_CUR_CHANNEL, &local->scanning))
+		drv_flush(local, false);
 
 	ieee80211_configure_filter(local);
 
@@ -522,10 +541,14 @@ static void 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 only if we are probing more than the current
+		 * channel.
 		 */
-		local->next_scan_state = SCAN_LEAVE_OPER_CHANNEL;
+		if (test_bit(SCAN_ON_CUR_CHANNEL, &local->scanning))
+			local->next_scan_state = SCAN_SET_CHANNEL;
+		else
+			local->next_scan_state = SCAN_LEAVE_OPER_CHANNEL;
 	}
 
 	*next_delay = 0;
@@ -559,14 +582,19 @@ 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.
+	/* We only return if we ever left, and should never leave if
+	 * scanning single channel that is also the operating channel.
 	 */
-	ieee80211_offchannel_return(local, false);
+	if (!test_bit(SCAN_ON_CUR_CHANNEL, &local->scanning)) {
+		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;
@@ -583,8 +611,9 @@ 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 (local->hw.conf.channel != chan)
+		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

[RFC 3/3] ath9k: Support scanning on current channel.

[greearb]

From: Ben Greear <greearb@candelatech.com>

This adds support for scanning on only the current
channel.  We do not need to flush xmit queues or
otherwise impede traffic in this scenario.

Signed-off-by: Ben Greear <greearb@candelatech.com>
---
:100644 100644 dab0271... fa5bd0d... M	drivers/net/wireless/ath/ath9k/ath9k.h
:100644 100644 333486d... ef182d1... M	drivers/net/wireless/ath/ath9k/debug.c
:100644 100644 f01de0e... a2454ba... M	drivers/net/wireless/ath/ath9k/main.c
:100644 100644 d205c66... 261a68a... M	drivers/net/wireless/ath/ath9k/virtual.c
 drivers/net/wireless/ath/ath9k/ath9k.h   |    1 +
 drivers/net/wireless/ath/ath9k/debug.c   |    2 ++
 drivers/net/wireless/ath/ath9k/main.c    |   22 ++++++++++++++++++----
 drivers/net/wireless/ath/ath9k/virtual.c |    6 ++++--
 4 files changed, 25 insertions(+), 6 deletions(-)

diff --git a/drivers/net/wireless/ath/ath9k/ath9k.h b/drivers/net/wireless/ath/ath9k/ath9k.h
index dab0271..fa5bd0d 100644
--- a/drivers/net/wireless/ath/ath9k/ath9k.h
+++ b/drivers/net/wireless/ath/ath9k/ath9k.h
@@ -672,6 +672,7 @@ struct ath_wiphy {
 		ATH_WIPHY_PAUSING,
 		ATH_WIPHY_PAUSED,
 		ATH_WIPHY_SCAN,
+		ATH_WIPHY_SCAN_CUR,
 	} state;
 	bool idle;
 	int chan_idx;
diff --git a/drivers/net/wireless/ath/ath9k/debug.c b/drivers/net/wireless/ath/ath9k/debug.c
index 333486d..ef182d1 100644
--- a/drivers/net/wireless/ath/ath9k/debug.c
+++ b/drivers/net/wireless/ath/ath9k/debug.c
@@ -394,6 +394,8 @@ static const char * ath_wiphy_state_str(enum ath_wiphy_state state)
 		return "PAUSED";
 	case ATH_WIPHY_SCAN:
 		return "SCAN";
+	case ATH_WIPHY_SCAN_CUR:
+		return "SCAN_CUR";
 	}
 	return "?";
 }
diff --git a/drivers/net/wireless/ath/ath9k/main.c b/drivers/net/wireless/ath/ath9k/main.c
index f01de0e..a2454ba 100644
--- a/drivers/net/wireless/ath/ath9k/main.c
+++ b/drivers/net/wireless/ath/ath9k/main.c
@@ -1220,7 +1220,9 @@ static int ath9k_tx(struct ieee80211_hw *hw,
 	struct ath_tx_control txctl;
 	struct ieee80211_hdr *hdr = (struct ieee80211_hdr *) skb->data;
 
-	if (aphy->state != ATH_WIPHY_ACTIVE && aphy->state != ATH_WIPHY_SCAN) {
+	if (aphy->state != ATH_WIPHY_ACTIVE &&
+	    aphy->state != ATH_WIPHY_SCAN_CUR &&
+	    aphy->state != ATH_WIPHY_SCAN) {
 		ath_dbg(common, ATH_DBG_XMIT,
 			"ath9k: %s: TX in unexpected wiphy state %d\n",
 			wiphy_name(hw->wiphy), aphy->state);
@@ -1803,6 +1805,7 @@ static int ath9k_config(struct ieee80211_hw *hw, u32 changed)
 			sc->sc_flags &= ~SC_OP_OFFCHANNEL;
 
 		if (aphy->state == ATH_WIPHY_SCAN ||
+		    aphy->state == ATH_WIPHY_SCAN_CUR ||
 		    aphy->state == ATH_WIPHY_ACTIVE)
 			ath9k_wiphy_pause_all_forced(sc, aphy);
 		else {
@@ -2262,7 +2265,8 @@ static int ath9k_get_survey(struct ieee80211_hw *hw, int idx,
 	return 0;
 }
 
-static void ath9k_sw_scan_start(struct ieee80211_hw *hw)
+static void ath9k_sw_scan_start_cur(struct ieee80211_hw *hw,
+				    bool cur_only)
 {
 	struct ath_wiphy *aphy = hw->priv;
 	struct ath_softc *sc = aphy->sc;
@@ -2280,11 +2284,20 @@ static void ath9k_sw_scan_start(struct ieee80211_hw *hw)
 		return;
 	}
 
-	aphy->state = ATH_WIPHY_SCAN;
-	ath9k_wiphy_pause_all_forced(sc, aphy);
+	if (cur_only)
+		aphy->state = ATH_WIPHY_SCAN_CUR;
+	else {
+		aphy->state = ATH_WIPHY_SCAN;
+		ath9k_wiphy_pause_all_forced(sc, aphy);
+	}
 	mutex_unlock(&sc->mutex);
 }
 
+static void ath9k_sw_scan_start(struct ieee80211_hw *hw)
+{
+	ath9k_sw_scan_start_cur(hw, false);
+}
+
 /*
  * XXX: this requires a revisit after the driver
  * scan_complete gets moved to another place/removed in mac80211.
@@ -2331,6 +2344,7 @@ struct ieee80211_ops ath9k_ops = {
 	.ampdu_action       = ath9k_ampdu_action,
 	.get_survey	    = ath9k_get_survey,
 	.sw_scan_start      = ath9k_sw_scan_start,
+	.sw_scan_start_cur  = ath9k_sw_scan_start_cur,
 	.sw_scan_complete   = ath9k_sw_scan_complete,
 	.rfkill_poll        = ath9k_rfkill_poll_state,
 	.set_coverage_class = ath9k_set_coverage_class,
diff --git a/drivers/net/wireless/ath/ath9k/virtual.c b/drivers/net/wireless/ath/ath9k/virtual.c
index d205c66..261a68a 100644
--- a/drivers/net/wireless/ath/ath9k/virtual.c
+++ b/drivers/net/wireless/ath/ath9k/virtual.c
@@ -176,11 +176,13 @@ static bool ath9k_wiphy_pausing(struct ath_softc *sc)
 static bool __ath9k_wiphy_scanning(struct ath_softc *sc)
 {
 	int i;
-	if (sc->pri_wiphy->state == ATH_WIPHY_SCAN)
+	if (sc->pri_wiphy->state == ATH_WIPHY_SCAN ||
+	    sc->pri_wiphy->state == ATH_WIPHY_SCAN_CUR)
 		return true;
 	for (i = 0; i < sc->num_sec_wiphy; i++) {
 		if (sc->sec_wiphy[i] &&
-		    sc->sec_wiphy[i]->state == ATH_WIPHY_SCAN)
+		    (sc->sec_wiphy[i]->state == ATH_WIPHY_SCAN ||
+		     sc->sec_wiphy[i]->state == ATH_WIPHY_SCAN_CUR))
 			return true;
 	}
 	return false;
-- 
1.7.2.3

Re: [RFC 1/3] mac80211: Support sw_scan_start_cur

[Johannes Berg]

On Thu, 2011-01-20 at 09:32 -0800, greearb@candelatech.com wrote:
> From: Ben Greear <greearb@candelatech.com>
> 
> This method is called when driver can support
> scanning the currect active channel without otherwise
> impeding traffic on that channel.  The mac80211 scan
> logic may call this when we are only scanning on the
> active channel and thus do not need to go off channel.

I frankly don't see any point telling the driver about this. Looking at
the ath9k patch you sent, it seems to avoid some things like flushing --
but the flushing should be controlled by mac80211 already (or converted
to be) so that it's not necessary to have this.

johannes

Re: [RFC 2/3] mac80211: Support scanning only current active channel.

[Johannes Berg]

On Thu, 2011-01-20 at 09:32 -0800, greearb@candelatech.com wrote:

> -	ieee80211_offchannel_stop_beaconing(local);
> +	if (local->ops->sw_scan_start_cur &&
> +	    local->scan_req->n_channels == 1 &&
> +	    local->scan_req->channels[0] == local->hw.conf.channel) {
> +		__set_bit(SCAN_ON_CUR_CHANNEL, &local->scanning);
> +		drv_sw_scan_start_cur(local, true);
> +	} else
> +		drv_sw_scan_start(local);

This doesn't seem to make much sense either -- even if we do a scan over
multiple channels we should be able to optimise the part on the current
channel (maybe put it at the beginning or end too).

johannes

Re: [RFC 1/3] mac80211: Support sw_scan_start_cur

[Ben Greear]

On 01/20/2011 09:37 AM, Johannes Berg wrote:
> On Thu, 2011-01-20 at 09:32 -0800, greearb@candelatech.com wrote:
>> From: Ben Greear<greearb@candelatech.com>
>>
>> This method is called when driver can support
>> scanning the currect active channel without otherwise
>> impeding traffic on that channel.  The mac80211 scan
>> logic may call this when we are only scanning on the
>> active channel and thus do not need to go off channel.
>
> I frankly don't see any point telling the driver about this. Looking at
> the ath9k patch you sent, it seems to avoid some things like flushing --
> but the flushing should be controlled by mac80211 already (or converted
> to be) so that it's not necessary to have this.

mac80211 + ath9k is a house of cards, and this seemed the least
invasive approach.  It also lets us not change behaviour for
the rest of the drivers until they are audited and potentially
updated.  At best I can test/hack on ath5k and ath9k.  These are
touchy enough, and other drivers may be even more twitchy...

Thanks,
Ben

-- 
Ben Greear <greearb@candelatech.com>
Candela Technologies Inc  http://www.candelatech.com

Re: [RFC 2/3] mac80211: Support scanning only current active channel.

[Ben Greear]

On 01/20/2011 09:39 AM, Johannes Berg wrote:
> On Thu, 2011-01-20 at 09:32 -0800, greearb@candelatech.com wrote:
>
>> -	ieee80211_offchannel_stop_beaconing(local);
>> +	if (local->ops->sw_scan_start_cur&&
>> +	    local->scan_req->n_channels == 1&&
>> +	    local->scan_req->channels[0] == local->hw.conf.channel) {
>> +		__set_bit(SCAN_ON_CUR_CHANNEL,&local->scanning);
>> +		drv_sw_scan_start_cur(local, true);
>> +	} else
>> +		drv_sw_scan_start(local);
>
> This doesn't seem to make much sense either -- even if we do a scan over
> multiple channels we should be able to optimise the part on the current
> channel (maybe put it at the beginning or end too).

Maybe..but user-space can specify the channels it wants to scan,
and I assume that means what order to scan them in.  I have no
idea if changing that order would confuse some application.

It seems to me that it would take quite a bit of re-work of the
mac80211 scanning logic to deal with scanning on the current
channel w/out affecting other tx/rx packets (as my patch attempts
to do), without setting some explicit flag before you enter
the scan state machine.

Thanks,
Ben

>
> johannes


-- 
Ben Greear <greearb@candelatech.com>
Candela Technologies Inc  http://www.candelatech.com

Re: [RFC 2/3] mac80211: Support scanning only current active channel.

[Johannes Berg]

On Thu, 2011-01-20 at 10:06 -0800, Ben Greear wrote:

> > This doesn't seem to make much sense either -- even if we do a scan over
> > multiple channels we should be able to optimise the part on the current
> > channel (maybe put it at the beginning or end too).
> 
> Maybe..but user-space can specify the channels it wants to scan,
> and I assume that means what order to scan them in.  I have no
> idea if changing that order would confuse some application.

No, it can't actually determine the order -- we sort them in cfg80211
anyway to de-duplicate them.

> It seems to me that it would take quite a bit of re-work of the
> mac80211 scanning logic to deal with scanning on the current
> channel w/out affecting other tx/rx packets (as my patch attempts
> to do), without setting some explicit flag before you enter
> the scan state machine.

Yeah, so maybe it needs some re-work, but I think what you're doing is a
pretty strange hack.

johannes

Re: [RFC 2/3] mac80211: Support scanning only current active channel.

[Ben Greear]

On 01/20/2011 10:17 AM, Johannes Berg wrote:
> On Thu, 2011-01-20 at 10:06 -0800, Ben Greear wrote:
>
>>> This doesn't seem to make much sense either -- even if we do a scan over
>>> multiple channels we should be able to optimise the part on the current
>>> channel (maybe put it at the beginning or end too).
>>
>> Maybe..but user-space can specify the channels it wants to scan,
>> and I assume that means what order to scan them in.  I have no
>> idea if changing that order would confuse some application.
>
> No, it can't actually determine the order -- we sort them in cfg80211
> anyway to de-duplicate them.

Ok, so assuming we re-work scanning across the board, maybe the first
thing is to sort them such that the current channel is always first
(if it's in the list at all)?

>> It seems to me that it would take quite a bit of re-work of the
>> mac80211 scanning logic to deal with scanning on the current
>> channel w/out affecting other tx/rx packets (as my patch attempts
>> to do), without setting some explicit flag before you enter
>> the scan state machine.
>
> Yeah, so maybe it needs some re-work, but I think what you're doing is a
> pretty strange hack.

If you have time to write some patches, I'll be happy to test them on
our ath9k and ath5k systems.

If you don't, then I can make an attempt.  Suggestions for an acceptable
way to go about doing this would be welcome.

Thanks,
Ben


-- 
Ben Greear <greearb@candelatech.com>
Candela Technologies Inc  http://www.candelatech.com

Re: [RFC 2/3] mac80211: Support scanning only current active channel.

[Johannes Berg]

On Thu, 2011-01-20 at 10:21 -0800, Ben Greear wrote:

> > No, it can't actually determine the order -- we sort them in cfg80211
> > anyway to de-duplicate them.
> 
> Ok, so assuming we re-work scanning across the board, maybe the first
> thing is to sort them such that the current channel is always first
> (if it's in the list at all)?

Yeah I guess mac80211 could do that -- I wouldn't necessarily do it in
cfg80211. However, I'm not convinced it's necessary.

> >> It seems to me that it would take quite a bit of re-work of the
> >> mac80211 scanning logic to deal with scanning on the current
> >> channel w/out affecting other tx/rx packets (as my patch attempts
> >> to do), without setting some explicit flag before you enter
> >> the scan state machine.
> >
> > Yeah, so maybe it needs some re-work, but I think what you're doing is a
> > pretty strange hack.
> 
> If you have time to write some patches, I'll be happy to test them on
> our ath9k and ath5k systems.

I don't, unfortunately, at least not now.

> If you don't, then I can make an attempt.  Suggestions for an acceptable
> way to go about doing this would be welcome.

Well I'd start by looking at the offchannel code and seeing if it has
the flushing in all the right places. I don't think it does, so drivers
attempt to work around it by flushing on channel changes. That could be
improved first.

Then I'd again look at the offchannel code and see how it can behave
better when it's not actually going offchannel -- not stop beaconing
etc., just reprogram the filters or so. That way, I'm not even sure if
modifying much of the scan code will even be necessary, and I think you
wouldn't even have to sort the channel list differently.

johannes

Re: [RFC 2/3] mac80211: Support scanning only current active channel.

[Helmut Schaa]

Am Donnerstag, 20. Januar 2011 schrieb Ben Greear:
> On 01/20/2011 10:17 AM, Johannes Berg wrote:
> > Yeah, so maybe it needs some re-work, but I think what you're doing is a
> > pretty strange hack.
> 
> If you have time to write some patches, I'll be happy to test them on
> our ath9k and ath5k systems.

Try this, I only ran a quick test with iwlagn (disable_hw_scan=1), seems to
work fine. However, I did not think much about it yet ;)

Helmut


From: Helmut Schaa <helmut.schaa@googlemail.com>
Date: Thu, 20 Jan 2011 20:11:51 +0100
Subject: [PATCH] mac80211: Scan operating channel without enabling PS

Signed-off-by: Helmut Schaa <helmut.schaa@googlemail.com>
---
 net/mac80211/rx.c   |   13 +++++++++----
 net/mac80211/scan.c |   43 ++++++++++++++++++++++++++++---------------
 2 files changed, 37 insertions(+), 19 deletions(-)

diff --git a/net/mac80211/rx.c b/net/mac80211/rx.c
index a6701ed..d710149 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 */
@@ -2730,7 +2735,7 @@ static void __ieee80211_rx_handle_packet(struct ieee80211_hw *hw,
 		local->dot11ReceivedFragmentCount++;
 
 	if (unlikely(test_bit(SCAN_HW_SCANNING, &local->scanning) ||
-		     test_bit(SCAN_OFF_CHANNEL, &local->scanning)))
+		     test_bit(SCAN_SW_SCANNING, &local->scanning)))
 		status->rx_flags |= IEEE80211_RX_IN_SCAN;
 
 	if (ieee80211_is_mgmt(fc))
diff --git a/net/mac80211/scan.c b/net/mac80211/scan.c
index fb274db..bcc8a3e 100644
--- a/net/mac80211/scan.c
+++ b/net/mac80211/scan.c
@@ -486,7 +486,15 @@ static void ieee80211_scan_state_decision(struct ieee80211_local *local,
 	}
 	mutex_unlock(&local->iflist_mtx);
 
-	if (local->scan_channel) {
+	if (local->scan_channel == local->oper_channel ||
+	    local->scan_channel == NULL) {
+		/*
+		 * 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
@@ -520,12 +528,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;
@@ -534,6 +536,21 @@ 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)
 {
+	struct ieee80211_channel *chan;
+	chan = local->scan_req->channels[local->scan_channel_idx];
+
+	/* remember when we left the operating channel */
+	local->leave_oper_channel_time = jiffies;
+
+	/* advance to the next channel to be scanned */
+	local->next_scan_state = SCAN_SET_CHANNEL;
+
+	/* Scanning operating channel, take the shortcut */
+	if (chan == local->oper_channel) {
+		*next_delay = 0;
+		return;
+	}
+
 	ieee80211_offchannel_stop_station(local);
 
 	__set_bit(SCAN_OFF_CHANNEL, &local->scanning);
@@ -546,12 +563,6 @@ static void ieee80211_scan_state_leave_oper_channel(struct ieee80211_local *loca
 		*next_delay = 0;
 	else
 		*next_delay = HZ / 10;
-
-	/* remember when we left the operating channel */
-	local->leave_oper_channel_time = jiffies;
-
-	/* advance to the next channel to be scanned */
-	local->next_scan_state = SCAN_SET_CHANNEL;
 }
 
 static void ieee80211_scan_state_enter_oper_channel(struct ieee80211_local *local,
@@ -583,8 +594,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->oper_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.1

Re: [RFC 2/3] mac80211: Support scanning only current active channel.

[Ben Greear]

On 01/20/2011 11:14 AM, Helmut Schaa wrote:
> Am Donnerstag, 20. Januar 2011 schrieb Ben Greear:
>> On 01/20/2011 10:17 AM, Johannes Berg wrote:
>>> Yeah, so maybe it needs some re-work, but I think what you're doing is a
>>> pretty strange hack.
>>
>> If you have time to write some patches, I'll be happy to test them on
>> our ath9k and ath5k systems.
>
> Try this, I only ran a quick test with iwlagn (disable_hw_scan=1), seems to
> work fine. However, I did not think much about it yet ;)

I think it has a few issues, but might be moving in the right direction.

First, it doesn't deal with not calling change-channel on scan completion
if we never left the operating channel.  And doesn't mitigate the off-channel
call when scanning starts.

> @@ -534,6 +536,21 @@ 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)
>   {
> +	struct ieee80211_channel *chan;
> +	chan = local->scan_req->channels[local->scan_channel_idx];
> +
> +	/* remember when we left the operating channel */
> +	local->leave_oper_channel_time = jiffies;
> +
> +	/* advance to the next channel to be scanned */
> +	local->next_scan_state = SCAN_SET_CHANNEL;
> +
> +	/* Scanning operating channel, take the shortcut */
> +	if (chan == local->oper_channel) {
> +		*next_delay = 0;
> +		return;
> +	}

I think here you might need to compare against the current channel, which could be
one we are scanning on before we scan the operating channel, not the oper_channel.


>   static void ieee80211_scan_state_enter_oper_channel(struct ieee80211_local *local,
> @@ -583,8 +594,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->oper_channel)
> +		if (ieee80211_hw_config(local, IEEE80211_CONF_CHANGE_CHANNEL))
> +			skip = 1;

Same problem here I think.

I may be mis-understanding what oper_channel implies, however.

Thanks,
Ben


-- 
Ben Greear <greearb@candelatech.com>
Candela Technologies Inc  http://www.candelatech.com

Re: [RFC 2/3] mac80211: Support scanning only current active channel.

[Ben Greear]

On 01/20/2011 11:14 AM, Helmut Schaa wrote:
> Am Donnerstag, 20. Januar 2011 schrieb Ben Greear:
>> On 01/20/2011 10:17 AM, Johannes Berg wrote:
>>> Yeah, so maybe it needs some re-work, but I think what you're doing is a
>>> pretty strange hack.
>>
>> If you have time to write some patches, I'll be happy to test them on
>> our ath9k and ath5k systems.
>
> Try this, I only ran a quick test with iwlagn (disable_hw_scan=1), seems to
> work fine. However, I did not think much about it yet ;)

I re-worked your patch, made it a bit more like my original
one, and re-posted it.  A quick test on ath9k with 128 stations
seems to be positive, but I haven't added tracing yet to make
sure it really skips going on/off channel when it's supposed to.

If you get a chance to try it out, please let me know how it goes.

Thanks,
Ben

-- 
Ben Greear <greearb@candelatech.com>
Candela Technologies Inc  http://www.candelatech.com