[RFC 0/3] mac80211: new API for handling broadcast / multicast on sw scan

[RFC 0/3] mac80211: new API for handling broadcast / multicast on sw scan

[Luis R. Rodriguez]

mac80211's sw scan doesn't play nice with buffered broadcast and
multicast frames. Since drivers are required to keep track of the DTIM
count this series provides some new basic functionality to let drivers
help mac80211 make more prudent decisions for sw scan.

Tested with an AP with a beacon interval of 100 and then two dtim
intervals: 1, 2.

I'll send out the PS fix separately, its an RFC because I am not aware
of the impact of enabling PS after 2.6.34... did we *really* go on testing
without PS since then? The good news is that at least with these patches
my AR9003 device works well with PS enabled. Haven't tested the AR9002
family. You can force enable PS to test it by using iw:

	iw dev wlan0 set power_save on

I believe we can expand on this to move more of the code ath9k does into
mac80211 and just provide basic APIs for informating mac80211 what it
should do.

I'd appreciate if others would test, specially those without ath9k
and using sw scan.

Luis R. Rodriguez (3):
  ath9k: fix regression which disabled ps on ath9k on all cards
  mac80211: allow drivers to specify sw scan wait constraints
  ath9k: implement the sw scan wait constraints

 drivers/net/wireless/ath/ath9k/ath9k.h |    1 +
 drivers/net/wireless/ath/ath9k/init.c  |    3 +-
 drivers/net/wireless/ath/ath9k/main.c  |   69 ++++++++++++++++++++++++++++++++
 include/net/mac80211.h                 |   12 ++++++
 net/mac80211/driver-ops.h              |   16 +++++++
 net/mac80211/driver-trace.h            |   18 ++++++++
 net/mac80211/ieee80211_i.h             |    2 +
 net/mac80211/scan.c                    |   42 +++++++++++++++++--
 8 files changed, 158 insertions(+), 5 deletions(-)

[RFC 1/3] ath9k: fix regression which disabled ps on ath9k on all cards

[Luis R. Rodriguez]

The patch titled "ath9k: Add new file init.c" shuffled some code
around but in dong so for some reason also removed the revision
check for disablign power save. Add this revision check again
so we can get power save re-enabled again by default on cards
newer than AR5416 and AR5418.

$ git describe --contains 556242049cc3992d0ee625e9f15c4b00ea4baac8
v2.6.34-rc1~233^2~49^2~343

Before on all cards:

$ iw dev wlan39 get power_save
Power save: off

Cc: Amod Bodas <amod.bodas@atheros.com>
Signed-off-by: Luis R. Rodriguez <lrodriguez@atheros.com>
---
 drivers/net/wireless/ath/ath9k/init.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/drivers/net/wireless/ath/ath9k/init.c b/drivers/net/wireless/ath/ath9k/init.c
index 243c177..4129390 100644
--- a/drivers/net/wireless/ath/ath9k/init.c
+++ b/drivers/net/wireless/ath/ath9k/init.c
@@ -641,7 +641,8 @@ void ath9k_set_hw_capab(struct ath_softc *sc, struct ieee80211_hw *hw)
 		BIT(NL80211_IFTYPE_ADHOC) |
 		BIT(NL80211_IFTYPE_MESH_POINT);
 
-	hw->wiphy->flags &= ~WIPHY_FLAG_PS_ON_BY_DEFAULT;
+	if (AR_SREV_5416(sc->sc_ah))
+		hw->wiphy->flags &= ~WIPHY_FLAG_PS_ON_BY_DEFAULT;
 
 	hw->queues = 4;
 	hw->max_rates = 4;
-- 
1.7.0.4

[RFC 2/3] mac80211: allow drivers to specify sw scan wait constraints

[Luis R. Rodriguez]

mac80211's scan implementation puts the burden of processing the DTIM
count, ensuring all the device has recevied all broadcast and multicast
frames, and ensuring the AP has received or nullfunc frame prior to
allowing the driver to go to scan. This patch enables drivers to inform
mac80211 of any of these time constraints prior to scanning on the next
channel. If a time constraint is reached mac80211 will move to the home
operating channel and delay scanning on the next channel until the time
constraints are removed.

We handle the nullfunc frame specially on the allowed callback. If
we know that the nullfunc frame has not yet been ACK'd by the AP
we can inform mac80211 so it can do a software retry on it. After
a few failed retries we simply give up and move on with the scan.

Cc: Amod Bodas <amod.bodas@atheros.com>
Signed-off-by: Luis R. Rodriguez <lrodriguez@atheros.com>
---
 include/net/mac80211.h      |   12 ++++++++++++
 net/mac80211/driver-ops.h   |   16 ++++++++++++++++
 net/mac80211/driver-trace.h |   18 ++++++++++++++++++
 net/mac80211/ieee80211_i.h  |    2 ++
 net/mac80211/scan.c         |   42 ++++++++++++++++++++++++++++++++++++++----
 5 files changed, 86 insertions(+), 4 deletions(-)

diff --git a/include/net/mac80211.h b/include/net/mac80211.h
index dcc8c2b..fa455d6 100644
--- a/include/net/mac80211.h
+++ b/include/net/mac80211.h
@@ -1600,6 +1600,17 @@ enum ieee80211_ampdu_mlme_action {
  *	is started. Can be NULL, if the driver doesn't need this notification.
  *	The callback can sleep.
  *
+ * @sw_scan_wait_constraints: used by mac80211 to query for scan wait constraints.
+ *	The sw scan implementation in mac80211 assumes drivers take care of
+ *	processing an associated station's DTIM count, ensuring we received all
+ *	buffered broadcast and multicast frames, and ensuring the we have received
+ *	an ACK from the AP that that we are going into power save. Drivers can
+ *	implement this callback to let mac80211 query for these wait constraints
+ *	on scan prior to moving on to the next channel in the scan list. The
+ *	callback returns 0 if there are no wait constraints. If non zero is
+ *	returned mac80211 will return to the home channel and delay the scan
+ *	on the next channel some more. This callback is optional and 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.
@@ -1719,6 +1730,7 @@ 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);
+	int (*sw_scan_wait_constraints)(struct ieee80211_hw *hw);
 	void (*sw_scan_complete)(struct ieee80211_hw *hw);
 	int (*get_stats)(struct ieee80211_hw *hw,
 			 struct ieee80211_low_level_stats *stats);
diff --git a/net/mac80211/driver-ops.h b/net/mac80211/driver-ops.h
index 14123dc..5025950 100644
--- a/net/mac80211/driver-ops.h
+++ b/net/mac80211/driver-ops.h
@@ -187,6 +187,22 @@ static inline void drv_sw_scan_start(struct ieee80211_local *local)
 	trace_drv_return_void(local);
 }
 
+static inline int drv_sw_scan_wait_constraints(struct ieee80211_local *local)
+{
+	bool wait_constraints = false;
+
+	might_sleep();
+
+	trace_drv_sw_scan_wait_constraints(local);
+	if (local->ops->sw_scan_start)
+		wait_constraints =
+			local->ops->sw_scan_wait_constraints(&local->hw);
+	trace_drv_return_int(local, wait_constraints);
+
+	return wait_constraints;
+}
+
+
 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 b5a9558..f822f0f 100644
--- a/net/mac80211/driver-trace.h
+++ b/net/mac80211/driver-trace.h
@@ -427,6 +427,24 @@ TRACE_EVENT(drv_sw_scan_start,
 	)
 );
 
+TRACE_EVENT(drv_sw_scan_wait_constraints,
+	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 e73ae51..b50e4b5 100644
--- a/net/mac80211/ieee80211_i.h
+++ b/net/mac80211/ieee80211_i.h
@@ -757,6 +757,8 @@ struct ieee80211_local {
 
 	/* Scanning and BSS list */
 	unsigned long scanning;
+	u32 scan_nullfunc_retries;
+	u32 scan_oper_chan_waits;
 	struct cfg80211_ssid scan_ssid;
 	struct cfg80211_scan_request *int_scan_req;
 	struct cfg80211_scan_request *scan_req, *hw_scan_req;
diff --git a/net/mac80211/scan.c b/net/mac80211/scan.c
index 2c7e376..1d24002 100644
--- a/net/mac80211/scan.c
+++ b/net/mac80211/scan.c
@@ -512,7 +512,8 @@ static int ieee80211_scan_state_decision(struct ieee80211_local *local,
 				local->hw.conf.listen_interval);
 
 		if (associated && ( !tx_empty || bad_latency ||
-		    listen_int_exceeded))
+		    listen_int_exceeded ||
+		    drv_sw_scan_wait_constraints(local)))
 			local->next_scan_state = SCAN_ENTER_OPER_CHANNEL;
 		else
 			local->next_scan_state = SCAN_SET_CHANNEL;
@@ -520,23 +521,38 @@ static int ieee80211_scan_state_decision(struct ieee80211_local *local,
 		/*
 		 * we're on the operating channel currently, let's
 		 * leave that channel now to scan another one
+		 * unless the driver has other wait time constraints
 		 */
-		local->next_scan_state = SCAN_LEAVE_OPER_CHANNEL;
+		if (drv_sw_scan_wait_constraints(local) &&
+		    local->scan_oper_chan_waits < 2) {
+			local->next_scan_state = SCAN_DECISION;
+			local->scan_oper_chan_waits++;
+		} else {
+			local->next_scan_state = SCAN_LEAVE_OPER_CHANNEL;
+			local->scan_oper_chan_waits = 0;
+		}
 	}
 
-	*next_delay = 0;
+	if (local->next_scan_state == SCAN_DECISION)
+		*next_delay = HZ / 5;
+	else
+		*next_delay = 0;
+
 	return 0;
 }
 
 static void ieee80211_scan_state_leave_oper_channel(struct ieee80211_local *local,
 						    unsigned long *next_delay)
 {
+	int r;
+
 	ieee80211_offchannel_stop_station(local);
 
 	__set_bit(SCAN_OFF_CHANNEL, &local->scanning);
 
 	/*
-	 * What if the nullfunc frames didn't arrive?
+	 * Drivers are responsible for ensuring their nullfunc frame
+	 * was ACKed by the AP.
 	 */
 	drv_flush(local, false);
 	if (local->ops->flush)
@@ -544,8 +560,26 @@ static void ieee80211_scan_state_leave_oper_channel(struct ieee80211_local *loca
 	else
 		*next_delay = HZ / 10;
 
+	/*
+	 * If we're not getting ACKs for our nullfuncs we're likely in bad
+	 * shape so we should not care about missed data as we can't even
+	 * hear the AP. We want to help roam away if we can so just go
+	 * ahead and scan. Try getting the ACK just 3 times.
+	 */
+	r = drv_sw_scan_wait_constraints(local);
+	if (r == -EAGAIN) {
+		local->scan_nullfunc_retries++;
+		*next_delay = HZ / 10;
+
+		if (local->scan_nullfunc_retries < 3) {
+			local->next_scan_state = SCAN_LEAVE_OPER_CHANNEL;
+			return;
+		}
+	}
+
 	/* remember when we left the operating channel */
 	local->leave_oper_channel_time = jiffies;
+	local->scan_nullfunc_retries = 0;
 
 	/* advance to the next channel to be scanned */
 	local->next_scan_state = SCAN_SET_CHANNEL;
-- 
1.7.0.4

Re: [RFC 2/3] mac80211: allow drivers to specify sw scan wait constraints

[Luis R. Rodriguez]

On Sat, Aug 28, 2010 at 12:13 AM, Luis R. Rodriguez
<lrodriguez@atheros.com> wrote:

> @@ -520,23 +521,38 @@ static int ieee80211_scan_state_decision(struct ieee80211_local *local,
>                /*
>                 * we're on the operating channel currently, let's
>                 * leave that channel now to scan another one
> +                * unless the driver has other wait time constraints
>                 */
> -               local->next_scan_state = SCAN_LEAVE_OPER_CHANNEL;
> +               if (drv_sw_scan_wait_constraints(local) &&
> +                   local->scan_oper_chan_waits < 2) {
> +                       local->next_scan_state = SCAN_DECISION;
> +                       local->scan_oper_chan_waits++;
> +               } else {
> +                       local->next_scan_state = SCAN_LEAVE_OPER_CHANNEL;
> +                       local->scan_oper_chan_waits = 0;
> +               }
>        }
>
> -       *next_delay = 0;
> +       if (local->next_scan_state == SCAN_DECISION)
> +               *next_delay = HZ / 5;

This needs some good review as well. As its implemented here we'll
essentially stop all TX except null data frames and probe requests as
local->scanning is set by SCAN_SW_SCANNING through
__ieee80211_start_scan(). One possible alternative is to do the
drv_sw_scan_wait_constraints() check prior to calling the sw scan and
fail the __ieee80211_start_scan() call if we need to wait. I didn't
think this was the best thing to do at first though because the user
will just get a scan busy error from userspace. It seemed more
rational to queue the request in since we just have to wait until the
driver clears its wait constraints -- or we wait too long (just a loop
counter now on the operating channel).

> @@ -544,8 +560,26 @@ static void ieee80211_scan_state_leave_oper_channel(struct ieee80211_local *loca
>        else
>                *next_delay = HZ / 10;
>
> +       /*
> +        * If we're not getting ACKs for our nullfuncs we're likely in bad
> +        * shape so we should not care about missed data as we can't even
> +        * hear the AP. We want to help roam away if we can so just go
> +        * ahead and scan. Try getting the ACK just 3 times.
> +        */
> +       r = drv_sw_scan_wait_constraints(local);
> +       if (r == -EAGAIN) {
> +               local->scan_nullfunc_retries++;
> +               *next_delay = HZ / 10;

After some review I believe we need to set here
local->offchannel_ps_enabled otherwise we won't resend the nullfunc.
Also we may want to reconsider the amount of time we wait for a resend
as in that duration we will be only letting through nullfuncs and
probe requests.

  Luis

Re: [RFC 2/3] mac80211: allow drivers to specify sw scan wait constraints

[Luis R. Rodriguez]

On Sun, Aug 29, 2010 at 12:55 PM, Luis R. Rodriguez
<lrodriguez@atheros.com> wrote:
> On Sat, Aug 28, 2010 at 12:13 AM, Luis R. Rodriguez
> <lrodriguez@atheros.com> wrote:
>
>> @@ -520,23 +521,38 @@ static int ieee80211_scan_state_decision(struct ieee80211_local *local,
>>                /*
>>                 * we're on the operating channel currently, let's
>>                 * leave that channel now to scan another one
>> +                * unless the driver has other wait time constraints
>>                 */
>> -               local->next_scan_state = SCAN_LEAVE_OPER_CHANNEL;
>> +               if (drv_sw_scan_wait_constraints(local) &&
>> +                   local->scan_oper_chan_waits < 2) {
>> +                       local->next_scan_state = SCAN_DECISION;
>> +                       local->scan_oper_chan_waits++;
>> +               } else {
>> +                       local->next_scan_state = SCAN_LEAVE_OPER_CHANNEL;
>> +                       local->scan_oper_chan_waits = 0;
>> +               }
>>        }
>>
>> -       *next_delay = 0;
>> +       if (local->next_scan_state == SCAN_DECISION)
>> +               *next_delay = HZ / 5;
>
> This needs some good review as well. As its implemented here we'll
> essentially stop all TX except null data frames and probe requests as
> local->scanning is set by SCAN_SW_SCANNING through
> __ieee80211_start_scan(). One possible alternative is to do the
> drv_sw_scan_wait_constraints() check prior to calling the sw scan and
> fail the __ieee80211_start_scan() call if we need to wait. I didn't
> think this was the best thing to do at first though because the user
> will just get a scan busy error from userspace. It seemed more
> rational to queue the request in since we just have to wait until the
> driver clears its wait constraints -- or we wait too long (just a loop
> counter now on the operating channel).

After some testing I notice that these are actually very rare if
called prior to scan. If you think about it it makes sense, there are
only several wait constraints that ath9k would use. The nullfunc wait
that is not yet handled in mac80211 is from the off channel operation,
but that can eventually be handled internally within mac80211 as well.
There are other things though that are a bit more rare, like the
beacon synch requirement which would happen when our TSF gets off
track from the AP's. It seems reasonable to let drivers notify
mac80211 about these and let mac80211 do the work. This is a lot of
work though and I would prefer to start out knocking one wait
constraint at a time. Since the wait constraints are rare at least for
ath9k prior to starting a scan then it seems reasonable to me to at
least skip the userspace requested scan if we have wait constraints.
Later I think though our goal should be to WARN_ON() the wait
constraints prior to initiating the scan as all wait constraints would
ideally be handled within mac80211.

>> @@ -544,8 +560,26 @@ static void ieee80211_scan_state_leave_oper_channel(struct ieee80211_local *loca
>>        else
>>                *next_delay = HZ / 10;
>>
>> +       /*
>> +        * If we're not getting ACKs for our nullfuncs we're likely in bad
>> +        * shape so we should not care about missed data as we can't even
>> +        * hear the AP. We want to help roam away if we can so just go
>> +        * ahead and scan. Try getting the ACK just 3 times.
>> +        */
>> +       r = drv_sw_scan_wait_constraints(local);
>> +       if (r == -EAGAIN) {
>> +               local->scan_nullfunc_retries++;
>> +               *next_delay = HZ / 10;
>
> After some review I believe we need to set here
> local->offchannel_ps_enabled otherwise we won't resend the nullfunc.
> Also we may want to reconsider the amount of time we wait for a resend
> as in that duration we will be only letting through nullfuncs and
> probe requests.

Tested this and think its the right thing to do. I'm now toying with
something like this:

diff --git a/net/mac80211/ieee80211_i.h b/net/mac80211/ieee80211_i.h
index b50e4b5..77d7539 100644
--- a/net/mac80211/ieee80211_i.h
+++ b/net/mac80211/ieee80211_i.h
@@ -322,6 +322,12 @@ enum ieee80211_sta_flags {
        IEEE80211_STA_RESET_SIGNAL_AVE  = BIT(9),
 };

+enum ieee80211_nullfunc_type {
+       IEEE80211_NULLFUNC_DYN_PS       = BIT(0),
+       IEEE80211_NULLFUNC_OFFCHAN      = BIT(1),
+       IEEE80211_NULLFUNC_PRIVATE      = BIT(2),
+};
+
 struct ieee80211_if_managed {
        struct timer_list timer;
        struct timer_list conn_mon_timer;
@@ -350,6 +356,7 @@ struct ieee80211_if_managed {
        struct work_struct request_smps_work;

        unsigned int flags;
+       enum ieee80211_nullfunc_type nullfunc_type;

        u32 beacon_crc;

diff --git a/net/mac80211/mlme.c b/net/mac80211/mlme.c
index 5282ac1..ba4a622 100644
--- a/net/mac80211/mlme.c
+++ b/net/mac80211/mlme.c
@@ -2181,6 +2181,7 @@ int ieee80211_mgd_assoc(struct
ieee80211_sub_if_data *sdata,

        ifmgd->flags &= ~IEEE80211_STA_DISABLE_11N;
        ifmgd->flags &= ~IEEE80211_STA_NULLFUNC_ACKED;
+       ifmgd->nullfunc_type = 0;

        for (i = 0; i < req->crypto.n_ciphers_pairwise; i++)
                if (req->crypto.ciphers_pairwise[i] ==
WLAN_CIPHER_SUITE_WEP40 ||
diff --git a/net/mac80211/status.c b/net/mac80211/status.c
index 571b32b..acfc3d1 100644
--- a/net/mac80211/status.c
+++ b/net/mac80211/status.c
@@ -280,18 +280,50 @@ void ieee80211_tx_status(struct ieee80211_hw
*hw, struct sk_buff *skb)
                        local->dot11FailedCount++;
        }

+       /*
+        * We perform a nullfunc ACK check on two separate code paths:
+        *      - dynamic ps
+        *      - when going off channel during sw scan
+        *
+        * Both nullfunc retries are handled separately.
+        */
        if (ieee80211_is_nullfunc(fc) && ieee80211_has_pm(fc) &&
            (local->hw.flags & IEEE80211_HW_REPORTS_TX_ACK_STATUS) &&
            !(info->flags & IEEE80211_TX_CTL_INJECTED) &&
-           local->ps_sdata && !(local->scanning)) {
-               if (info->flags & IEEE80211_TX_STAT_ACK) {
-                       local->ps_sdata->u.mgd.flags |=
+           local->ps_sdata) {
+               if (!local->scanning) {
+                       if (info->flags & IEEE80211_TX_STAT_ACK) {
+                               printk("NULLFUNCK ACK: dynamic PS: %d,
last type: %d\n",
+                                      IEEE80211_NULLFUNC_DYN_PS;
+                                      local->ps_sdata->u.mgd.nullfunc_type);
+                               local->ps_sdata->u.mgd.flags |=
                                        IEEE80211_STA_NULLFUNC_ACKED;
-                       ieee80211_queue_work(&local->hw,
+                               local->ps_sdata->u.mgd.nullfunc_type =
+                                       IEEE80211_NULLFUNC_DYN_PS;
+                               ieee80211_queue_work(&local->hw,
                                        &local->dynamic_ps_enable_work);
-               } else
-                       mod_timer(&local->dynamic_ps_timer, jiffies +
-                                       msecs_to_jiffies(10));
+                       } else
+                               mod_timer(&local->dynamic_ps_timer, jiffies +
+                                         msecs_to_jiffies(10));
+               } else if (test_bit(SCAN_SW_SCANNING, &local->scanning)) {
+                       printk("NULLFUNCK ACK: offchannel: %d, last type: %d\n",
+                              IEEE80211_NULLFUNC_OFFCHAN,
+                              local->ps_sdata->u.mgd.nullfunc_type);
+                       local->ps_sdata->u.mgd.flags |=
+                               IEEE80211_STA_NULLFUNC_ACKED;
+                       local->ps_sdata->u.mgd.nullfunc_type =
+                               IEEE80211_NULLFUNC_OFFCHAN;
+               } else {
+                       /*
+                        * other cases might happen but mac80211 currently does
+                        * does not care for them.
+                        */
+                       printk("NULLFUNCK ACK: private: %d, last type: %d\n",
+                              IEEE80211_NULLFUNC_PRIVATE,
+                              local->ps_sdata->u.mgd.nullfunc_type);
+                       local->ps_sdata->u.mgd.null_func_type =
+                               IEEE80211_NULLFUNC_PRIVATE;
+               }
        }

        if (info->flags & IEEE80211_TX_INTFL_NL80211_FRAME_TX)


In hopes of covering the whole reason why ath9k has its own ACK check
for the nullfunc. Hope is to get rid of it. I think the reason we
couldn't was that mac80211 *did* not do the nullfunc ACK check for the
offchannel operation, it only did it for the dynamic PS work. So the
above categorizes the nullfuncs and would expect mac80211 to do the
right thing for each case. The exception here obviously ath9k's
virtial wiphjy stuff so hence the private type... but hopefully
that'll all be removed eventually and replaced by using internal
virtual interfaces that can operate on separate bands

I do wonder -- why do we only check for the nullfunc ACK when PS is
set, why not when we come back ? A *good* reason to check for the
nullfunc when we are going to sleep is we want to ensure we are in PS
mode on the AP side, but it seems equally reasonable to except the AP
to know we're awake otherwise if that nullfunc gets dropped the AP
would still buffer our frames for us and we wouldn't wake up for them,
creating a desynchronization between the two. Or am I missing
something?

 Luis

Re: [RFC 2/3] mac80211: allow drivers to specify sw scan wait constraints

[John W. Linville]

On Sat, Aug 28, 2010 at 03:13:09AM -0400, Luis R. Rodriguez wrote:

> diff --git a/net/mac80211/driver-ops.h b/net/mac80211/driver-ops.h
> index 14123dc..5025950 100644
> --- a/net/mac80211/driver-ops.h
> +++ b/net/mac80211/driver-ops.h
> @@ -187,6 +187,22 @@ static inline void drv_sw_scan_start(struct ieee80211_local *local)
>  	trace_drv_return_void(local);
>  }
>  
> +static inline int drv_sw_scan_wait_constraints(struct ieee80211_local *local)
> +{
> +	bool wait_constraints = false;
> +
> +	might_sleep();
> +
> +	trace_drv_sw_scan_wait_constraints(local);
> +	if (local->ops->sw_scan_start)

Are we checking the right member here?

> +		wait_constraints =
> +			local->ops->sw_scan_wait_constraints(&local->hw);
> +	trace_drv_return_int(local, wait_constraints);
> +
> +	return wait_constraints;
> +}
> +
> +
>  static inline void drv_sw_scan_complete(struct ieee80211_local *local)
>  {
>  	might_sleep();

-- 
John W. Linville		Someday the world will need a hero, and you
linville@tuxdriver.com			might be all we have.  Be ready.

Re: [RFC 2/3] mac80211: allow drivers to specify sw scan wait constraints

[Luis R. Rodriguez]

On Mon, Aug 30, 2010 at 7:17 AM, John W. Linville
<linville@tuxdriver.com> wrote:
> On Sat, Aug 28, 2010 at 03:13:09AM -0400, Luis R. Rodriguez wrote:
>
>> diff --git a/net/mac80211/driver-ops.h b/net/mac80211/driver-ops.h
>> index 14123dc..5025950 100644
>> --- a/net/mac80211/driver-ops.h
>> +++ b/net/mac80211/driver-ops.h
>> @@ -187,6 +187,22 @@ static inline void drv_sw_scan_start(struct ieee80211_local *local)
>>       trace_drv_return_void(local);
>>  }
>>
>> +static inline int drv_sw_scan_wait_constraints(struct ieee80211_local *local)
>> +{
>> +     bool wait_constraints = false;
>> +
>> +     might_sleep();
>> +
>> +     trace_drv_sw_scan_wait_constraints(local);
>> +     if (local->ops->sw_scan_start)
>
> Are we checking the right member here?

Nope, thanks, will fix.

  Luis

[RFC 3/3] ath9k: implement the sw scan wait constraints

[Luis R. Rodriguez]

When mac80211 sw scan is triggered this will inform mac80211
to take us to the home channel if we are aware of some
time constraints for scanning. This ensures ath9k will not
loose buffered broadcast and multicast frames when scanning.

Cc: Amod Bodas <amod.bodas@atheros.com>
Signed-off-by: Luis R. Rodriguez <lrodriguez@atheros.com>
---
 drivers/net/wireless/ath/ath9k/ath9k.h |    1 +
 drivers/net/wireless/ath/ath9k/main.c  |   69 ++++++++++++++++++++++++++++++++
 2 files changed, 70 insertions(+), 0 deletions(-)

diff --git a/drivers/net/wireless/ath/ath9k/ath9k.h b/drivers/net/wireless/ath/ath9k/ath9k.h
index f0197a6..89bbcd0 100644
--- a/drivers/net/wireless/ath/ath9k/ath9k.h
+++ b/drivers/net/wireless/ath/ath9k/ath9k.h
@@ -588,6 +588,7 @@ struct ath_softc {
 	int led_off_cnt;
 
 	int beacon_interval;
+	u32 scan_wait_counter;
 
 #ifdef CONFIG_ATH9K_DEBUGFS
 	struct ath9k_debug debug;
diff --git a/drivers/net/wireless/ath/ath9k/main.c b/drivers/net/wireless/ath/ath9k/main.c
index 1165f90..181edea 100644
--- a/drivers/net/wireless/ath/ath9k/main.c
+++ b/drivers/net/wireless/ath/ath9k/main.c
@@ -2043,6 +2043,74 @@ static void ath9k_sw_scan_start(struct ieee80211_hw *hw)
 	mutex_unlock(&sc->mutex);
 }
 
+static void ath9k_ps_flags_dbg(struct ath_softc *sc)
+{
+#define ATH9K_WAIT_DBG(_reason) do { \
+	if (sc->ps_flags & _reason) \
+		ath_print(common, ATH_DBG_PS, \
+			  "  %s\n", #_reason); \
+	} while (0)
+
+	struct ath_common *common = ath9k_hw_common(sc->sc_ah);
+
+	ath_print(common, ATH_DBG_PS, "PS flags:\n");
+
+	ATH9K_WAIT_DBG(PS_WAIT_FOR_BEACON);
+	ATH9K_WAIT_DBG(PS_WAIT_FOR_CAB);
+	ATH9K_WAIT_DBG(PS_WAIT_FOR_PSPOLL_DATA);
+	ATH9K_WAIT_DBG(PS_WAIT_FOR_TX_ACK);
+	ATH9K_WAIT_DBG(PS_BEACON_SYNC);
+	ATH9K_WAIT_DBG(PS_NULLFUNC_COMPLETED);
+	ATH9K_WAIT_DBG(PS_ENABLED);
+
+#undef ATH9K_WAIT_DBG
+}
+
+/*
+ * We expect to be disassociated if the counter gets too high.
+ */
+static int ath9k_sw_scan_wait_constraints(struct ieee80211_hw *hw)
+{
+	struct ath_wiphy *aphy = hw->priv;
+	struct ath_softc *sc = aphy->sc;
+	struct ath_common *common = ath9k_hw_common(sc->sc_ah);
+	int r = 0;
+	u32 wait_for;
+
+	mutex_lock(&sc->mutex);
+
+	wait_for = PS_WAIT_FOR_BEACON |
+		   PS_BEACON_SYNC |
+		   PS_WAIT_FOR_CAB |
+		   PS_WAIT_FOR_PSPOLL_DATA |
+		   PS_WAIT_FOR_TX_ACK |
+		   PS_NULLFUNC_COMPLETED;
+
+	if (sc->ps_flags & wait_for) {
+		sc->scan_wait_counter++;
+		ath_print(common, ATH_DBG_PS,
+			  "Holding on channel change due to a "
+			  "wait constraint, count: %d, ps_flags: 0x%04x\n",
+			  sc->scan_wait_counter,
+			  sc->ps_flags);
+		ath9k_ps_flags_dbg(sc);
+		if (sc->ps_flags & PS_NULLFUNC_COMPLETED)
+			r = -EAGAIN;
+		else
+			r = -EBUSY;
+		goto out;
+	}
+
+	sc->scan_wait_counter = 0;
+	ath_print(common, ATH_DBG_PS,
+		  "No scan wait contraints found, ps_flags: 0x%04x\n",
+		   sc->ps_flags);
+out:
+	mutex_unlock(&sc->mutex);
+
+	return r;
+}
+
 /*
  * XXX: this requires a revisit after the driver
  * scan_complete gets moved to another place/removed in mac80211.
@@ -2089,6 +2157,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_wait_constraints = ath9k_sw_scan_wait_constraints,
 	.sw_scan_complete   = ath9k_sw_scan_complete,
 	.rfkill_poll        = ath9k_rfkill_poll_state,
 	.set_coverage_class = ath9k_set_coverage_class,
-- 
1.7.0.4

Re: [RFC 0/3] mac80211: new API for handling broadcast / multicast on sw scan

[Johannes Berg]

On Sat, 2010-08-28 at 03:13 -0400, Luis R. Rodriguez wrote:
> mac80211's sw scan doesn't play nice with buffered broadcast and
> multicast frames. Since drivers are required to keep track of the DTIM
> count 

I said they were required for powersave, not necessarily for scanning.

> this series provides some new basic functionality to let drivers
> help mac80211 make more prudent decisions for sw scan.

I'm not convinced of this. Why does this need to be so complicated?
Also, some of your comments seem like you're confused about being an AP
vs. being associated to one?

What are you actually trying to achieve? It seems mac80211 can, no
matter what the driver is
 - wait for DTIM beacons, and the end of mcast traffic, before going off
   channel (should be relevant for p2p as well)
 - attempt to do better wrt. scheduling between DTIM, by scheduling
   closer (but if DTIM period is 1, this may fail)
 - subject to the driver advertising guaranteed TX status, it can also
   wait for the nullfunc ACK -- you should also implement flush which
   will probably improve things by itself for ath9k

The other things in your patch set I don't understand, but those I think
should be done more generically?

johannes

Re: [RFC 0/3] mac80211: new API for handling broadcast / multicast on sw scan

[Luis R. Rodriguez]

On Mon, Aug 30, 2010 at 7:19 AM, Johannes Berg
<johannes@sipsolutions.net> wrote:
> On Sat, 2010-08-28 at 03:13 -0400, Luis R. Rodriguez wrote:
>> mac80211's sw scan doesn't play nice with buffered broadcast and
>> multicast frames. Since drivers are required to keep track of the DTIM
>> count
>
> I said they were required for powersave, not necessarily for scanning.

Its also the case for scanning. We currently don't even check for the
ACK for the scanning nullfunc. It something my patches will address as
well.

>> this series provides some new basic functionality to let drivers
>> help mac80211 make more prudent decisions for sw scan.
>
> I'm not convinced of this. Why does this need to be so complicated?

See below for more details. Apart from the DTIM synch wait constraints
for drivers which do sw scan will vary, you either need something like
a wait constraint callback or as I see it you move all the
synchronization to mac80211. The later requires quite a bit of work
and I see it as necessary for the long run.

> Also, some of your comments seem like you're confused about being an AP
> vs. being associated to one?

Like?

> What are you actually trying to achieve?

The current sw code simply follows the listen interval, it in no way
respect the DTIM. What this means is you loose the buffered multicast
and broadcast frames the AP saves up for you after the DTIM beacon.
Apart from ensuring you awake at the DTIM count you then also have to
ensure you stay awake for all the multicast / broadcast data after the
DTIM. It was my understanding you were postulating that all this is a
requirement for the drivers to do properly given that mac80211 is too
slow to do it properly. ath9k already has code to do exactly this.
What you then need is way for ath9k to communicate to mac80211 it
cannot go into a sw scan as it may be waiting for the DTIM beacon, and
other reasons like a beacon synch to synchronize the TSF.

> It seems mac80211 can, no
> matter what the driver is
>  - wait for DTIM beacons, and the end of mcast traffic, before going off
>   channel (should be relevant for p2p as well)

I do not deny this is not possible, I was following your statements
about mac80211 being too slow to handle this, so was relying more on
the driver to achieve this. I actually believe this should go into
mac80211 but its not there yet. ath9k also has its own nullfunc
requirements handled for the ath9k vritual wiphy stuff so either way
you still have some private uses for the nullfunc frames. I rather
take this up in steps in mac80211.

>  - attempt to do better wrt. scheduling between DTIM, by scheduling
>   closer (but if DTIM period is 1, this may fail)

To do this you still need to handle all the different sorts of code
paths for nullfuncs and ensure they are ACK'd. You have the private
ath9k virtual wiphy too.. which I rather see removed but we have no
alternative to replace it yet do we?

>  - subject to the driver advertising guaranteed TX status, it can also
>   wait for the nullfunc ACK -- you should also implement flush which
>   will probably improve things by itself for ath9k

Sure, I have some patches for that as well. The nullfunc ACK wait for
the offchannel operation *can* be handled indeed by mac80211, as my
latest comments indicate. But note we'd have to categorize the
different types of uses for the nullfuncs. We also have some private
driver uses of nullfuncs such as the ath9k virtual wiphy.

Then -- you also have other synch wait considerations to address which
are also currently only private to the driver. ath9k has these:

+       wait_for = PS_WAIT_FOR_BEACON |
+                  PS_BEACON_SYNC |
+                  PS_WAIT_FOR_CAB |
+                  PS_WAIT_FOR_PSPOLL_DATA |
+                  PS_WAIT_FOR_TX_ACK |
+                  PS_NULLFUNC_COMPLETED;

Of those I see an easy way to move to mac80211 the
PS_NULLFUNC_COMPLETED except for the private driver usage for
ath9k_send_nullfunc(). The others require either some new API or as I
put it, a driver callback for wait constraints. As I see it, I think
these all of these *should* be moved to mac80211, but I don't expect
to do this all in one shot and think its better to do address them one
at a time.

> The other things in your patch set I don't understand, but those I think
> should be done more generically?

That is the goal.

  Luis

Re: [RFC 0/3] mac80211: new API for handling broadcast / multicast on sw scan

[Johannes Berg]

On Mon, 2010-08-30 at 08:37 -0700, Luis R. Rodriguez wrote:

> > Also, some of your comments seem like you're confused about being an AP
> > vs. being associated to one?
> 
> Like?

Like doing CAB stuff, or saying that drivers need to keep track of DTIM
count, that seems more AP related to me, but maybe I'm not understanding
you correctly.

> > What are you actually trying to achieve?
> 
> The current sw code simply follows the listen interval, it in no way
> respect the DTIM. What this means is you loose the buffered multicast
> and broadcast frames the AP saves up for you after the DTIM beacon.

Right, I don't deny that this is an issue.

> Apart from ensuring you awake at the DTIM count you then also have to
> ensure you stay awake for all the multicast / broadcast data after the
> DTIM. It was my understanding you were postulating that all this is a
> requirement for the drivers to do properly given that mac80211 is too
> slow to do it properly.

No, I was mostly speaking about the powersave case, where for maximum
power saving and correctness you need to wake up right before the
beacon. Going offchannel only _after_ a DTIM beacon and associated
multicast traffic seems unrelated to that?

> > It seems mac80211 can, no
> > matter what the driver is
> >  - wait for DTIM beacons, and the end of mcast traffic, before going off
> >   channel (should be relevant for p2p as well)
> 
> I do not deny this is not possible, I was following your statements
> about mac80211 being too slow to handle this, so was relying more on
> the driver to achieve this.

I believe we just misunderstood each other in that other thread, which
is probably mostly my fault since I started talking about powersave when
you were concerned about scan only.

>  I actually believe this should go into
> mac80211 but its not there yet. ath9k also has its own nullfunc
> requirements handled for the ath9k vritual wiphy stuff so either way
> you still have some private uses for the nullfunc frames. I rather
> take this up in steps in mac80211.

Right, I agree, let's tackle the issue step by step :) I guess we
disagree on what the steps should be and what order they should be in,
though.

> >  - attempt to do better wrt. scheduling between DTIM, by scheduling
> >   closer (but if DTIM period is 1, this may fail)
> 
> To do this you still need to handle all the different sorts of code
> paths for nullfuncs and ensure they are ACK'd. You have the private
> ath9k virtual wiphy too.. which I rather see removed but we have no
> alternative to replace it yet do we?

We're going to discuss multi-channel operation next week, and I believe
then we can remove it. We're mostly there, I believe, just some mac80211
internal re-design will be needed (with driver changes, obviously).

> >  - subject to the driver advertising guaranteed TX status, it can also
> >   wait for the nullfunc ACK -- you should also implement flush which
> >   will probably improve things by itself for ath9k
> 
> Sure, I have some patches for that as well. The nullfunc ACK wait for
> the offchannel operation *can* be handled indeed by mac80211, as my
> latest comments indicate. But note we'd have to categorize the
> different types of uses for the nullfuncs. We also have some private
> driver uses of nullfuncs such as the ath9k virtual wiphy.

Well those seem like a corner case though, let's not let the design be
impacted by something we're going to get rid of in the fairly short
term.

> Then -- you also have other synch wait considerations to address which
> are also currently only private to the driver. ath9k has these:
> 
> +       wait_for = PS_WAIT_FOR_BEACON |
> +                  PS_BEACON_SYNC |
> +                  PS_WAIT_FOR_CAB |
> +                  PS_WAIT_FOR_PSPOLL_DATA |
> +                  PS_WAIT_FOR_TX_ACK |
> +                  PS_NULLFUNC_COMPLETED;
> 
> Of those I see an easy way to move to mac80211 the
> PS_NULLFUNC_COMPLETED except for the private driver usage for
> ath9k_send_nullfunc().

 - wait for beacon: can be moved, if that's waiting for DTIM + traffic?
 - beacon sync:     no idea what this is?
 - wait for cab:    seems AP related? we turn off beaconing when
                    scanning on APs
 - pspoll:          seems like it could be done in mac80211, though
                    I guess it shouldn't matter if our nullfunc was
                    ACKed
 - tx ack:          no idea what this is about?
 - nullfunc:        can be moved

> The others require either some new API or as I
> put it, a driver callback for wait constraints. As I see it, I think
> these all of these *should* be moved to mac80211, but I don't expect
> to do this all in one shot and think its better to do address them one
> at a time.

Well, the other major issue with this is that you basically poll the
driver until those constraints no longer exist, and the polling interval
of 200ms is rather strange. So I suspect this really needs to be more of
an event based API.

johannes

Re: [RFC 0/3] mac80211: new API for handling broadcast / multicast on sw scan

[Luis R. Rodriguez]

On Mon, Aug 30, 2010 at 8:47 AM, Johannes Berg
<johannes@sipsolutions.net> wrote:
> On Mon, 2010-08-30 at 08:37 -0700, Luis R. Rodriguez wrote:
>
>> > Also, some of your comments seem like you're confused about being an AP
>> > vs. being associated to one?
>>
>> Like?
>
> Like doing CAB stuff, or saying that drivers need to keep track of DTIM
> count, that seems more AP related to me, but maybe I'm not understanding
> you correctly.

Well say you want to go to sleep for a sw scan. You not only have to
wait for the DTIM beacon prior to going to sleep but you also have to
wait so that all multicast and broadcast traffice get spit out by the
AP, right?

>> > What are you actually trying to achieve?
>>
>> The current sw code simply follows the listen interval, it in no way
>> respect the DTIM. What this means is you loose the buffered multicast
>> and broadcast frames the AP saves up for you after the DTIM beacon.
>
> Right, I don't deny that this is an issue.

OK what I'm saying is I realized that simply taking into consideration
the DTIM interval is not enough. We also have to ensure we receive all
the multicast and broadcast data prior to going to sleep. Then, there
are the other wait constraints, and handling of the different types of
nullfuncs.

>> Apart from ensuring you awake at the DTIM count you then also have to
>> ensure you stay awake for all the multicast / broadcast data after the
>> DTIM. It was my understanding you were postulating that all this is a
>> requirement for the drivers to do properly given that mac80211 is too
>> slow to do it properly.
>
> No, I was mostly speaking about the powersave case, where for maximum
> power saving and correctness you need to wake up right before the
> beacon.

Ah, I see.

> Going offchannel only _after_ a DTIM beacon and associated
> multicast traffic seems unrelated to that?

Yes. What you note seems to be another challenge.

>> > It seems mac80211 can, no
>> > matter what the driver is
>> >  - wait for DTIM beacons, and the end of mcast traffic, before going off
>> >   channel (should be relevant for p2p as well)
>>
>> I do not deny this is not possible, I was following your statements
>> about mac80211 being too slow to handle this, so was relying more on
>> the driver to achieve this.
>
> I believe we just misunderstood each other in that other thread, which
> is probably mostly my fault since I started talking about powersave when
> you were concerned about scan only.

OK -- I do also think it would be nice then to ensure mac80211 keeps
us up until DTIM / mcast/bcast frames are RX'd, but think this can be
done in steps. More on this below.

>>  I actually believe this should go into
>> mac80211 but its not there yet. ath9k also has its own nullfunc
>> requirements handled for the ath9k vritual wiphy stuff so either way
>> you still have some private uses for the nullfunc frames. I rather
>> take this up in steps in mac80211.
>
> Right, I agree, let's tackle the issue step by step :) I guess we
> disagree on what the steps should be and what order they should be in,
> though.

I don't think we disagree, I think we just need to review this
carefully and decide what is best.

>> >  - attempt to do better wrt. scheduling between DTIM, by scheduling
>> >   closer (but if DTIM period is 1, this may fail)
>>
>> To do this you still need to handle all the different sorts of code
>> paths for nullfuncs and ensure they are ACK'd. You have the private
>> ath9k virtual wiphy too.. which I rather see removed but we have no
>> alternative to replace it yet do we?
>
> We're going to discuss multi-channel operation next week, and I believe
> then we can remove it. We're mostly there, I believe, just some mac80211
> internal re-design will be needed (with driver changes, obviously).

offchannel.c will eventually be removed?

>> >  - subject to the driver advertising guaranteed TX status, it can also
>> >   wait for the nullfunc ACK -- you should also implement flush which
>> >   will probably improve things by itself for ath9k
>>
>> Sure, I have some patches for that as well. The nullfunc ACK wait for
>> the offchannel operation *can* be handled indeed by mac80211, as my
>> latest comments indicate. But note we'd have to categorize the
>> different types of uses for the nullfuncs. We also have some private
>> driver uses of nullfuncs such as the ath9k virtual wiphy.
>
> Well those seem like a corner case though, let's not let the design be
> impacted by something we're going to get rid of in the fairly short
> term.

Sure :) but we still do need to take into consideration the other
valid uses of nulfuncs in mac80211. The offchannel case is not handled
properly right now.

>> Then -- you also have other synch wait considerations to address which
>> are also currently only private to the driver. ath9k has these:
>>
>> +       wait_for = PS_WAIT_FOR_BEACON |
>> +                  PS_BEACON_SYNC |
>> +                  PS_WAIT_FOR_CAB |
>> +                  PS_WAIT_FOR_PSPOLL_DATA |
>> +                  PS_WAIT_FOR_TX_ACK |
>> +                  PS_NULLFUNC_COMPLETED;
>>
>> Of those I see an easy way to move to mac80211 the
>> PS_NULLFUNC_COMPLETED except for the private driver usage for
>> ath9k_send_nullfunc().
>
>  - wait for beacon: can be moved, if that's waiting for DTIM + traffic?

This is used for:

a. Sync TSF if it does not look correct against the AP's TSF, so we
wait until the next AP's beacon prior to going to sleep. We ask the
driver to wait for the next beacon but also set the beacon synch flag
so we can sync up the TSF.

b. When we get a DTIM and it notes we have mcast/bcast (CAB) crap to
wait for. What this does is it makes us wait until the next beacon in
case we miss the last broadcast / mcast frame. The next beacon will
serve as a backup trigger for returning to to network sleep mode.

>  - beacon sync:     no idea what this is?

This is used in combination with the above. It forces us to
reconfigure our beacon timers, essentially we run
ath_beacon_config(sc, NULL); on the next beacon we get from the AP.
This sets up the beacon timers according to the timestamp of the last
received beacon and the current TSF, configures PCF, DTIM handling,
programs the sleep registers so the hardware will wake up in time to
receive beacons, and configures the beacon miss interrupt to alert us
when we've stop seeing beacons from the AP we have associated with.

>  - wait for cab:    seems AP related? we turn off beaconing when
>                    scanning on APs

No, we use this to annotate to the driver it needs to sit and wait
until all mcast / bcast frames have been received. We do this in
combination with the beacon wait flag in case we miss the last bcast /
mcast frame or if the AP fails to send a frame indicating that all CAB
frames have been delivered.

>  - pspoll:          seems like it could be done in mac80211, though
>                    I guess it shouldn't matter if our nullfunc was
>                    ACKed

Right.

>  - tx ack:          no idea what this is about?

This is used by ath9k to avoid putting the chip to sleep if we are
waking up and want to wait for the first ACK from the AP. When we wake
up from sleep mac80211 will typically send a ps-poll to the AP so we
can pick up any buffered frames, but there are other cases when we may
not send the ps-poll, so we need at least an ACK from the AP to ensure
we are communicating with it. I am not sure of the history of this,
and will need to print here to see when this occurs exactly.

>  - nullfunc:        can be moved

Heh... not yet. That was done because we ended up looping trying to
send nullfuncs because we never waited for the nullfunc to be ACK'd. I
believe the issue was caused by the fact that mac80211 only keeps
track of nullfunc ACKs for PS but not for when we go off channel. I
could be wrong though. Anyway, if we remove considerations for the
ath9k virtual wiphy then I think we can move this into mac80211 as I
noted in my last set of hunks, the code that checks for the type of
nullfunc is still missing and obviously this would need testing.

>> The others require either some new API or as I
>> put it, a driver callback for wait constraints. As I see it, I think
>> these all of these *should* be moved to mac80211, but I don't expect
>> to do this all in one shot and think its better to do address them one
>> at a time.
>
> Well, the other major issue with this is that you basically poll the
> driver until those constraints no longer exist, and the polling interval
> of 200ms is rather strange.

Agreed -- I noted this needs review :)

> So I suspect this really needs to be more of
> an event based API.

Agreed, also I believe the wait constraint state machine should be
handled in mac80211 for drivers which require sw scan. But I'd like to
start off with the simple items and fix the driver with as little work
as possible first. Then slowly move crap over to mac80211 and replace
ath9k code with its mac80211 equivalent.

  Luis

Re: [RFC 0/3] mac80211: new API for handling broadcast / multicast on sw scan

[Johannes Berg]

On Mon, 2010-08-30 at 11:00 -0700, Luis R. Rodriguez wrote:

> > Like doing CAB stuff, or saying that drivers need to keep track of DTIM
> > count, that seems more AP related to me, but maybe I'm not understanding
> > you correctly.
> 
> Well say you want to go to sleep for a sw scan. You not only have to
> wait for the DTIM beacon prior to going to sleep but you also have to
> wait so that all multicast and broadcast traffice get spit out by the
> AP, right?

Yeah, ok, I was thinking of these terms (CAB, DTIM count) as being AP
specific -- the client just waits for the right beacon, it doesn't
really need to count etc. But ok, just a misunderstanding then.

> > Going offchannel only _after_ a DTIM beacon and associated
> > multicast traffic seems unrelated to that?
> 
> Yes. What you note seems to be another challenge.

I thought you set out to solve that bit. Now I'm confused.

> > We're going to discuss multi-channel operation next week, and I believe
> > then we can remove it. We're mostly there, I believe, just some mac80211
> > internal re-design will be needed (with driver changes, obviously).
> 
> offchannel.c will eventually be removed?

I don't think so, why? Oh, no, I meant virtual wiphys can be removed.

> >> Then -- you also have other synch wait considerations to address which
> >> are also currently only private to the driver. ath9k has these:
> >>
> >> +       wait_for = PS_WAIT_FOR_BEACON |
> >> +                  PS_BEACON_SYNC |
> >> +                  PS_WAIT_FOR_CAB |
> >> +                  PS_WAIT_FOR_PSPOLL_DATA |
> >> +                  PS_WAIT_FOR_TX_ACK |
> >> +                  PS_NULLFUNC_COMPLETED;
> >>
> >> Of those I see an easy way to move to mac80211 the
> >> PS_NULLFUNC_COMPLETED except for the private driver usage for
> >> ath9k_send_nullfunc().
> >
> >  - wait for beacon: can be moved, if that's waiting for DTIM + traffic?
> 
> This is used for:
> 
> a. Sync TSF if it does not look correct against the AP's TSF, so we
> wait until the next AP's beacon prior to going to sleep. We ask the
> driver to wait for the next beacon but also set the beacon synch flag
> so we can sync up the TSF.

But if we're doing a sleep for scan, we don't really care all that much
about TSF sync, since we're going to be awake anyway.

> b. When we get a DTIM and it notes we have mcast/bcast (CAB) crap to
> wait for. What this does is it makes us wait until the next beacon in
> case we miss the last broadcast / mcast frame. The next beacon will
> serve as a backup trigger for returning to to network sleep mode.

Ok, this is OK I guess, it seems to be an artifact of your powersave
implementation. I was thinking it was AP related.

> >  - beacon sync:     no idea what this is?
> 
> This is used in combination with the above. It forces us to
> reconfigure our beacon timers, essentially we run
> ath_beacon_config(sc, NULL); on the next beacon we get from the AP.
> This sets up the beacon timers according to the timestamp of the last
> received beacon and the current TSF, configures PCF, DTIM handling,
> programs the sleep registers so the hardware will wake up in time to
> receive beacons, and configures the beacon miss interrupt to alert us
> when we've stop seeing beacons from the AP we have associated with.

Ok but then why do we care for a scan case?

> >  - wait for cab:    seems AP related? we turn off beaconing when
> >                    scanning on APs
> 
> No, we use this to annotate to the driver it needs to sit and wait
> until all mcast / bcast frames have been received. We do this in
> combination with the beacon wait flag in case we miss the last bcast /
> mcast frame or if the AP fails to send a frame indicating that all CAB
> frames have been delivered.

Gotcha. But that's easy to implement in mac80211.

> >  - tx ack:          no idea what this is about?
> 
> This is used by ath9k to avoid putting the chip to sleep if we are
> waking up and want to wait for the first ACK from the AP. When we wake
> up from sleep mac80211 will typically send a ps-poll to the AP so we
> can pick up any buffered frames, but there are other cases when we may
> not send the ps-poll, so we need at least an ACK from the AP to ensure
> we are communicating with it. I am not sure of the history of this,
> and will need to print here to see when this occurs exactly.

Ok, but if it is as you say, then we don't need this for scan?

> >  - nullfunc:        can be moved
> 
> Heh... not yet. That was done because we ended up looping trying to
> send nullfuncs because we never waited for the nullfunc to be ACK'd. I
> believe the issue was caused by the fact that mac80211 only keeps
> track of nullfunc ACKs for PS but not for when we go off channel. I
> could be wrong though. Anyway, if we remove considerations for the
> ath9k virtual wiphy then I think we can move this into mac80211 as I
> noted in my last set of hunks, the code that checks for the type of
> nullfunc is still missing and obviously this would need testing.

Right ... the virtual wiphy case is a special case, but I'd rather
disregard it for a while. If you implement flush(), then waiting for the
nullfunc (and potentially retrying, which we don't do right now since
it's a unicast frame and really shouldn't get lost) should be simple.

> Agreed, also I believe the wait constraint state machine should be
> handled in mac80211 for drivers which require sw scan. But I'd like to
> start off with the simple items and fix the driver with as little work
> as possible first. Then slowly move crap over to mac80211 and replace
> ath9k code with its mac80211 equivalent.

I just don't like the need to have this API. Nothing here so far seems
to really need new API between the driver and mac80211, with the
possible exception of the driver saying that it has reliable TX status.
Or so I believe?

johannes

Re: [RFC 0/3] mac80211: new API for handling broadcast / multicast on sw scan

[Luis R. Rodriguez]

Note: this e-mail is on a public mailing list.

On Mon, Aug 30, 2010 at 11:10 AM, Johannes Berg
<johannes@sipsolutions.net> wrote:
> On Mon, 2010-08-30 at 11:00 -0700, Luis R. Rodriguez wrote:
>
>> > Like doing CAB stuff, or saying that drivers need to keep track of DTIM
>> > count, that seems more AP related to me, but maybe I'm not understanding
>> > you correctly.
>>
>> Well say you want to go to sleep for a sw scan. You not only have to
>> wait for the DTIM beacon prior to going to sleep but you also have to
>> wait so that all multicast and broadcast traffice get spit out by the
>> AP, right?
>
> Yeah, ok, I was thinking of these terms (CAB, DTIM count) as being AP
> specific -- the client just waits for the right beacon, it doesn't
> really need to count etc. But ok, just a misunderstanding then.
>
>> > Going offchannel only _after_ a DTIM beacon and associated
>> > multicast traffic seems unrelated to that?
>>
>> Yes. What you note seems to be another challenge.
>
> I thought you set out to solve that bit. Now I'm confused.

Hm yeah this came out wrong in this context, I meant about you
pointing out we want to *wake* up at the right time. What my patches
do is prevent us from going to sleep at the wrong times, it does not
address we ensure we *wake* up at the right time. The existing code
addresses the waking up part by ensuring we don't go off channel for
more than the listen interval. My additions push this further to
ensure we don't also going to sleep when waiting for DTIM / CAB crap /
PS-Poll / first TX ACK from AP.

I haven't yet considered how we can guarantee we will be awake at the
right time though. If you think about it the driver wait constraint
callback I provide simply prevents us from scanning off channel when
our driver knows we should not soon. That soon is obviously relative
to where and how the ath9k wait flags are set.

In practice my patches are working swell though but I do believe we
need to consider both requirements to make this work perfectly:
prevent going off channel for wait constraints, and ensuring we wake
up. I see we program beacon timers for this on ath9k though but not
sure what happens when we go off channel.

>> > We're going to discuss multi-channel operation next week, and I believe
>> > then we can remove it. We're mostly there, I believe, just some mac80211
>> > internal re-design will be needed (with driver changes, obviously).
>>
>> offchannel.c will eventually be removed?
>
> I don't think so, why? Oh, no, I meant virtual wiphys can be removed.

Ah OK.

>> >> Then -- you also have other synch wait considerations to address which
>> >> are also currently only private to the driver. ath9k has these:
>> >>
>> >> +       wait_for = PS_WAIT_FOR_BEACON |
>> >> +                  PS_BEACON_SYNC |
>> >> +                  PS_WAIT_FOR_CAB |
>> >> +                  PS_WAIT_FOR_PSPOLL_DATA |
>> >> +                  PS_WAIT_FOR_TX_ACK |
>> >> +                  PS_NULLFUNC_COMPLETED;
>> >>
>> >> Of those I see an easy way to move to mac80211 the
>> >> PS_NULLFUNC_COMPLETED except for the private driver usage for
>> >> ath9k_send_nullfunc().
>> >
>> >  - wait for beacon: can be moved, if that's waiting for DTIM + traffic?
>>
>> This is used for:
>>
>> a. Sync TSF if it does not look correct against the AP's TSF, so we
>> wait until the next AP's beacon prior to going to sleep. We ask the
>> driver to wait for the next beacon but also set the beacon synch flag
>> so we can sync up the TSF.
>
> But if we're doing a sleep for scan, we don't really care all that much
> about TSF sync, since we're going to be awake anyway.

The TSF synch will help us more accurately set the other flags though,
if we ignore it by the time we come back to our home channel we likely
would be out of synch with the AP's beacons. It makes sense to me to
wait to be in synch with the AP's beacons before going off channel,
otherwise our other wait constraints would be off. Let alone
calculating the time we do need to be awake for.

>> b. When we get a DTIM and it notes we have mcast/bcast (CAB) crap to
>> wait for. What this does is it makes us wait until the next beacon in
>> case we miss the last broadcast / mcast frame. The next beacon will
>> serve as a backup trigger for returning to to network sleep mode.
>
> Ok, this is OK I guess, it seems to be an artifact of your powersave
> implementation. I was thinking it was AP related.

Right -- its all software, I guess hardware could be improved to
handle this but then it means more registers and state machines. I'll
check for future hardware.

>> >  - beacon sync:     no idea what this is?
>>
>> This is used in combination with the above. It forces us to
>> reconfigure our beacon timers, essentially we run
>> ath_beacon_config(sc, NULL); on the next beacon we get from the AP.
>> This sets up the beacon timers according to the timestamp of the last
>> received beacon and the current TSF, configures PCF, DTIM handling,
>> programs the sleep registers so the hardware will wake up in time to
>> receive beacons, and configures the beacon miss interrupt to alert us
>> when we've stop seeing beacons from the AP we have associated with.
>
> Ok but then why do we care for a scan case?

To wake up at the right time, to be in synch with the AP's beacons
after we have gone off channel, and come back home. To be honest the
ath_beacon_config_sta() seems a little fishy to me. The beacon config
is picked up from &sc->cur_beacon_conf but I do not see that ever
being set from beacon data. The only use I see for
ath_beacon_config_sta() is the TSF sync and computing our next wake up
time, and beacon miss timers.

>> >  - wait for cab:    seems AP related? we turn off beaconing when
>> >                    scanning on APs
>>
>> No, we use this to annotate to the driver it needs to sit and wait
>> until all mcast / bcast frames have been received. We do this in
>> combination with the beacon wait flag in case we miss the last bcast /
>> mcast frame or if the AP fails to send a frame indicating that all CAB
>> frames have been delivered.
>
> Gotcha. But that's easy to implement in mac80211.

Sure. Agreed.

>> >  - tx ack:          no idea what this is about?
>>
>> This is used by ath9k to avoid putting the chip to sleep if we are
>> waking up and want to wait for the first ACK from the AP. When we wake
>> up from sleep mac80211 will typically send a ps-poll to the AP so we
>> can pick up any buffered frames, but there are other cases when we may
>> not send the ps-poll, so we need at least an ACK from the AP to ensure
>> we are communicating with it. I am not sure of the history of this,
>> and will need to print here to see when this occurs exactly.
>
> Ok, but if it is as you say, then we don't need this for scan?

It is a good question, can we be relying on on ACK from the AP other
than PS-poll data after coming out of sleep? I am not sure.

>> >  - nullfunc:        can be moved
>>
>> Heh... not yet. That was done because we ended up looping trying to
>> send nullfuncs because we never waited for the nullfunc to be ACK'd. I
>> believe the issue was caused by the fact that mac80211 only keeps
>> track of nullfunc ACKs for PS but not for when we go off channel. I
>> could be wrong though. Anyway, if we remove considerations for the
>> ath9k virtual wiphy then I think we can move this into mac80211 as I
>> noted in my last set of hunks, the code that checks for the type of
>> nullfunc is still missing and obviously this would need testing.
>
> Right ... the virtual wiphy case is a special case, but I'd rather
> disregard it for a while. If you implement flush(), then waiting for the
> nullfunc (and potentially retrying, which we don't do right now since
> it's a unicast frame and really shouldn't get lost) should be simple.

Agreed, in theory. We'll see in practice.

>> Agreed, also I believe the wait constraint state machine should be
>> handled in mac80211 for drivers which require sw scan. But I'd like to
>> start off with the simple items and fix the driver with as little work
>> as possible first. Then slowly move crap over to mac80211 and replace
>> ath9k code with its mac80211 equivalent.
>
> I just don't like the need to have this API. Nothing here so far seems
> to really need new API between the driver and mac80211, with the
> possible exception of the driver saying that it has reliable TX status.
> Or so I believe?

Truth be told I think all this is just hackery to get this stuff
working, the real work needed is to move more checks from ath9k into
mac80211, but as I see it, this should be done in steps and I
currently cannot guarantee proper functionality without this API since
drivers which do sw can *are* dealing with these synch issues. The
problem lies in that until we don't get *all* of them dealt with in
mac80211 we'll need this temporary API. I am 100% reluctant to add
bloat to mac80211 but I am also more reluctant to prevent proper
functionality with only a high requirement on code changes.

Not sure how else to deal with this. Please let me know if you can
think of a better way.

  Luis

Re: [RFC 0/3] mac80211: new API for handling broadcast / multicast on sw scan

[Johannes Berg]

On Mon, 2010-08-30 at 12:20 -0700, Luis R. Rodriguez wrote:

> >> > Going offchannel only _after_ a DTIM beacon and associated
> >> > multicast traffic seems unrelated to that?
> >>
> >> Yes. What you note seems to be another challenge.
> >
> > I thought you set out to solve that bit. Now I'm confused.
> 
> Hm yeah this came out wrong in this context, I meant about you
> pointing out we want to *wake* up at the right time. What my patches
> do is prevent us from going to sleep at the wrong times, it does not
> address we ensure we *wake* up at the right time. 

We're not even really going to sleep, when we scan. It would be
interesting to try to be back on the channel in time, but I believe we
can either make it easily, or for passive scanning basically always miss
it if the DTIM period is 1.

> The existing code
> addresses the waking up part by ensuring we don't go off channel for
> more than the listen interval. My additions push this further to
> ensure we don't also going to sleep when waiting for DTIM / CAB crap /
> PS-Poll / first TX ACK from AP.

Go offchannel, not to sleep.

> I haven't yet considered how we can guarantee we will be awake at the
> right time though. If you think about it the driver wait constraint
> callback I provide simply prevents us from scanning off channel when
> our driver knows we should not soon. That soon is obviously relative
> to where and how the ath9k wait flags are set.

I still don't quite believe that. The wait flags seem to be mostly
related to powersave, but for scan we really only have to wait for DTIM
+traffic plus all the other stuff we talked about.

> In practice my patches are working swell though but I do believe we
> need to consider both requirements to make this work perfectly:
> prevent going off channel for wait constraints

Yes, but we don't need ath9k's wait constraints for this. All of the
stuff you've really implemented, apart from virtual wiphys, can also be
implemented in mac80211.

> >> a. Sync TSF if it does not look correct against the AP's TSF, so we
> >> wait until the next AP's beacon prior to going to sleep. We ask the
> >> driver to wait for the next beacon but also set the beacon synch flag
> >> so we can sync up the TSF.
> >
> > But if we're doing a sleep for scan, we don't really care all that much
> > about TSF sync, since we're going to be awake anyway.
> 
> The TSF synch will help us more accurately set the other flags though,
> if we ignore it by the time we come back to our home channel we likely
> would be out of synch with the AP's beacons. It makes sense to me to
> wait to be in synch with the AP's beacons before going off channel,
> otherwise our other wait constraints would be off. Let alone
> calculating the time we do need to be awake for.

I don't think I can parse this. But does it matter? If we're going to
wait for a DTIM beacon in mac80211, we should have TSF sync anyway. And
after you go back to the operating channel, it seems likely that you'd
want to re-sync anyway.

> >> >  - tx ack:          no idea what this is about?
> >>
> >> This is used by ath9k to avoid putting the chip to sleep if we are
> >> waking up and want to wait for the first ACK from the AP. When we wake
> >> up from sleep mac80211 will typically send a ps-poll to the AP so we
> >> can pick up any buffered frames, but there are other cases when we may
> >> not send the ps-poll, so we need at least an ACK from the AP to ensure
> >> we are communicating with it. I am not sure of the history of this,
> >> and will need to print here to see when this occurs exactly.
> >
> > Ok, but if it is as you say, then we don't need this for scan?
> 
> It is a good question, can we be relying on on ACK from the AP other
> than PS-poll data after coming out of sleep? I am not sure.

?
The AP will ACK the nullfunc frame when you wake up, that should be
sufficient to know that the AP knows you've woken up. Or returned to the
operating channel, in the context of this discussion.

> Truth be told I think all this is just hackery to get this stuff
> working, the real work needed is to move more checks from ath9k into
> mac80211, but as I see it, this should be done in steps and I
> currently cannot guarantee proper functionality without this API since
> drivers which do sw can *are* dealing with these synch issues. The
> problem lies in that until we don't get *all* of them dealt with in
> mac80211 we'll need this temporary API. I am 100% reluctant to add
> bloat to mac80211 but I am also more reluctant to prevent proper
> functionality with only a high requirement on code changes.
> 
> Not sure how else to deal with this. Please let me know if you can
> think of a better way.

I dunno, why do you need this solved yesterday if you didn't care about
it all along? :-)

I guess I'd suggest to just start doing it in mac80211 where it helps
all other drivers more? Like for example just implementing waiting for
DTIM (traffic), that should be simple -- at the beginning of scan you
just wake up the HW, wait for a (any) beacon, calculate if you can fit
off-channel time before DTIM, and either wait for DTIM beacon+traffic or
squeeze off-channel time before it... Seems simple enough?

johannes

Re: [RFC 0/3] mac80211: new API for handling broadcast / multicast on sw scan

[Luis R. Rodriguez]

On Tue, Aug 31, 2010 at 7:36 AM, Johannes Berg
<johannes@sipsolutions.net> wrote:
> On Mon, 2010-08-30 at 12:20 -0700, Luis R. Rodriguez wrote:
>
>> >> > Going offchannel only _after_ a DTIM beacon and associated
>> >> > multicast traffic seems unrelated to that?
>> >>
>> >> Yes. What you note seems to be another challenge.
>> >
>> > I thought you set out to solve that bit. Now I'm confused.
>>
>> Hm yeah this came out wrong in this context, I meant about you
>> pointing out we want to *wake* up at the right time. What my patches
>> do is prevent us from going to sleep at the wrong times, it does not
>> address we ensure we *wake* up at the right time.
>
> We're not even really going to sleep, when we scan. It would be
> interesting to try to be back on the channel in time, but I believe we
> can either make it easily, or for passive scanning basically always miss
> it if the DTIM period is 1.

What do you mean by always miss if the DTIM period is 1?

>> The existing code
>> addresses the waking up part by ensuring we don't go off channel for
>> more than the listen interval. My additions push this further to
>> ensure we don't also going to sleep when waiting for DTIM / CAB crap /
>> PS-Poll / first TX ACK from AP.
>
> Go offchannel, not to sleep.

Sure, now re-read the above with "go offchannel".

>> I haven't yet considered how we can guarantee we will be awake at the
>> right time though. If you think about it the driver wait constraint
>> callback I provide simply prevents us from scanning off channel when
>> our driver knows we should not soon. That soon is obviously relative
>> to where and how the ath9k wait flags are set.
>
> I still don't quite believe that. The wait flags seem to be mostly
> related to powersave,

This is true, but why would they not be important for going off
channel ? The TSF sync seems just as important as waiting for the ACK
for the nullfunc from the AP.

> but for scan we really only have to wait for DTIM
> +traffic plus all the other stuff we talked about.

Can you be more specific, we spoke about quite a few things.
Specifically I mentioned all possible wait constraints.

>> In practice my patches are working swell though but I do believe we
>> need to consider both requirements to make this work perfectly:
>> prevent going off channel for wait constraints
>
> Yes, but we don't need ath9k's wait constraints for this. All of the
> stuff you've really implemented, apart from virtual wiphys, can also be
> implemented in mac80211.

And I agree... however its a good chunk of work to get it all done in
mac80211 in one shot so I am looking for a way to put us on that path.

>> >> a. Sync TSF if it does not look correct against the AP's TSF, so we
>> >> wait until the next AP's beacon prior to going to sleep. We ask the
>> >> driver to wait for the next beacon but also set the beacon synch flag
>> >> so we can sync up the TSF.
>> >
>> > But if we're doing a sleep for scan, we don't really care all that much
>> > about TSF sync, since we're going to be awake anyway.
>>
>> The TSF synch will help us more accurately set the other flags though,
>> if we ignore it by the time we come back to our home channel we likely
>> would be out of synch with the AP's beacons. It makes sense to me to
>> wait to be in synch with the AP's beacons before going off channel,
>> otherwise our other wait constraints would be off. Let alone
>> calculating the time we do need to be awake for.
>
> I don't think I can parse this. But does it matter? If we're going to
> wait for a DTIM beacon in mac80211, we should have TSF sync anyway. And
> after you go back to the operating channel, it seems likely that you'd
> want to re-sync anyway.

I don't understand, you first say it doesn't matter and then you end
up suggesting to actually do a TSF sync prior to going off channel and
then when coming back. It seems to me you do get it.

>> >> >  - tx ack:          no idea what this is about?
>> >>
>> >> This is used by ath9k to avoid putting the chip to sleep if we are
>> >> waking up and want to wait for the first ACK from the AP. When we wake
>> >> up from sleep mac80211 will typically send a ps-poll to the AP so we
>> >> can pick up any buffered frames, but there are other cases when we may
>> >> not send the ps-poll, so we need at least an ACK from the AP to ensure
>> >> we are communicating with it. I am not sure of the history of this,
>> >> and will need to print here to see when this occurs exactly.
>> >
>> > Ok, but if it is as you say, then we don't need this for scan?
>>
>> It is a good question, can we be relying on on ACK from the AP other
>> than PS-poll data after coming out of sleep? I am not sure.
>
> ?
> The AP will ACK the nullfunc frame when you wake up, that should be
> sufficient to know that the AP knows you've woken up. Or returned to the
> operating channel, in the context of this discussion.

Yes, but my point was that right now we don't check for this in
mac80211 but that code does exist in ath9k to wait for an ACK from the
AP. I was explaining when I see PS_WAIT_FOR_TX_ACK used, I noted that
ath9k has code that checks for an ACK from the AP prior to letting us
go to sleep, we force this upon ourselves when first frame we send out
to the AP is not a PS-POLL. I was wondering when this could possibly
happen.

>> Truth be told I think all this is just hackery to get this stuff
>> working, the real work needed is to move more checks from ath9k into
>> mac80211, but as I see it, this should be done in steps and I
>> currently cannot guarantee proper functionality without this API since
>> drivers which do sw can *are* dealing with these synch issues. The
>> problem lies in that until we don't get *all* of them dealt with in
>> mac80211 we'll need this temporary API. I am 100% reluctant to add
>> bloat to mac80211 but I am also more reluctant to prevent proper
>> functionality with only a high requirement on code changes.
>>
>> Not sure how else to deal with this. Please let me know if you can
>> think of a better way.
>
> I dunno, why do you need this solved yesterday if you didn't care about
> it all along? :-)

Our users who care for bcast / mcast do want it solved.

> I guess I'd suggest to just start doing it in mac80211 where it helps
> all other drivers more?

And what is exactly what I'm trying to do. I provided a path to move
code from ath9k to mac80211.

> Like for example just implementing waiting for
> DTIM (traffic), that should be simple -- at the beginning of scan you
> just wake up the HW, wait for a (any) beacon, calculate if you can fit
> off-channel time before DTIM, and either wait for DTIM beacon+traffic or
> squeeze off-channel time before it... Seems simple enough?

And I'm saying its not. To wait for the DTIM you need to know the DTIM
count, and you could also miss a few DTIM beacons. Then you also need
to keep track of the mcast / bcast data after the DTIM beacon. I noted
how ath9k already does all this and we use flags to annotate these
things to help avoid putting our chip to sleep. My patches make use of
these flags from ath9k to prevent us from going offchannel, and what
I'd like to do is to start offloading some of these flags one by one
to mac80211 to make them generic for all mac80211 drivers that use sw
scan.

  Luis

Re: [RFC 0/3] mac80211: new API for handling broadcast / multicast on sw scan

[Johannes Berg]

On Tue, 2010-08-31 at 08:30 -0700, Luis R. Rodriguez wrote:

> > We're not even really going to sleep, when we scan. It would be
> > interesting to try to be back on the channel in time, but I believe we
> > can either make it easily, or for passive scanning basically always miss
> > it if the DTIM period is 1.
> 
> What do you mean by always miss if the DTIM period is 1?

Well if the DTIM period is 1, and the beacon interval is, say, 100, then
there's no way you can do a passive scan without missing a DTIM beacon,
I'd say.

> >> I haven't yet considered how we can guarantee we will be awake at the
> >> right time though. If you think about it the driver wait constraint
> >> callback I provide simply prevents us from scanning off channel when
> >> our driver knows we should not soon. That soon is obviously relative
> >> to where and how the ath9k wait flags are set.
> >
> > I still don't quite believe that. The wait flags seem to be mostly
> > related to powersave,
> 
> This is true, but why would they not be important for going off
> channel ? The TSF sync seems just as important as waiting for the ACK
> for the nullfunc from the AP.

Why? It doesn't seem important to me?

> >> The TSF synch will help us more accurately set the other flags though,
> >> if we ignore it by the time we come back to our home channel we likely
> >> would be out of synch with the AP's beacons. It makes sense to me to
> >> wait to be in synch with the AP's beacons before going off channel,
> >> otherwise our other wait constraints would be off. Let alone
> >> calculating the time we do need to be awake for.
> >
> > I don't think I can parse this. But does it matter? If we're going to
> > wait for a DTIM beacon in mac80211, we should have TSF sync anyway. And
> > after you go back to the operating channel, it seems likely that you'd
> > want to re-sync anyway.
> 
> I don't understand, you first say it doesn't matter and then you end
> up suggesting to actually do a TSF sync prior to going off channel and
> then when coming back. It seems to me you do get it.

No, I don't understand the need for a TSF sync. And the time we need to
be awake for doesn't seem like it's determined by the wait constraints
or the beacon ... you're not going to get it perfect anyway.

> >> It is a good question, can we be relying on on ACK from the AP other
> >> than PS-poll data after coming out of sleep? I am not sure.
> >
> > ?
> > The AP will ACK the nullfunc frame when you wake up, that should be
> > sufficient to know that the AP knows you've woken up. Or returned to the
> > operating channel, in the context of this discussion.
> 
> Yes, but my point was that right now we don't check for this in
> mac80211 but that code does exist in ath9k to wait for an ACK from the
> AP. I was explaining when I see PS_WAIT_FOR_TX_ACK used, I noted that
> ath9k has code that checks for an ACK from the AP prior to letting us
> go to sleep, we force this upon ourselves when first frame we send out
> to the AP is not a PS-POLL. I was wondering when this could possibly
> happen.

Oh, dunno.

> > Like for example just implementing waiting for
> > DTIM (traffic), that should be simple -- at the beginning of scan you
> > just wake up the HW, wait for a (any) beacon, calculate if you can fit
> > off-channel time before DTIM, and either wait for DTIM beacon+traffic or
> > squeeze off-channel time before it... Seems simple enough?
> 
> And I'm saying its not. To wait for the DTIM you need to know the DTIM
> count, and you could also miss a few DTIM beacons. 

What do you mean by "need to know the DTIM count"? You just need to look
at every received beacon.

> Then you also need
> to keep track of the mcast / bcast data after the DTIM beacon. I noted
> how ath9k already does all this and we use flags to annotate these
> things to help avoid putting our chip to sleep. My patches make use of
> these flags from ath9k to prevent us from going offchannel, and what
> I'd like to do is to start offloading some of these flags one by one
> to mac80211 to make them generic for all mac80211 drivers that use sw
> scan.

Right right, I understand, but I guess I believe that the problem isn't
hard enough or pressing enough to warrant the additional churn needed,
since it's not just as you did it now, but it really needs to be
asynchronous with event callbacks, and error handling is also more
complex in the mac80211/driver interaction case (what if the driver
never returns the event in the timeout? then mac80211 asks for the next
channel, but the driver _then_ returns the event? etc.)

johannes

Re: [RFC 0/3] mac80211: new API for handling broadcast / multicast on sw scan

[Luis R. Rodriguez]

On Tue, Aug 31, 2010 at 08:54:52AM -0700, Johannes Berg wrote:
> On Tue, 2010-08-31 at 08:30 -0700, Luis R. Rodriguez wrote:
> 
> > > We're not even really going to sleep, when we scan. It would be
> > > interesting to try to be back on the channel in time, but I believe we
> > > can either make it easily, or for passive scanning basically always miss
> > > it if the DTIM period is 1.
> > 
> > What do you mean by always miss if the DTIM period is 1?
> 
> Well if the DTIM period is 1, and the beacon interval is, say, 100,

And this seems to be the standard configuration I find being used on APs.

> then there's no way you can do a passive scan without missing a DTIM beacon,
> I'd say.

Ah, yes, I see. Good point.

> > >> I haven't yet considered how we can guarantee we will be awake at the
> > >> right time though. If you think about it the driver wait constraint
> > >> callback I provide simply prevents us from scanning off channel when
> > >> our driver knows we should not soon. That soon is obviously relative
> > >> to where and how the ath9k wait flags are set.
> > >
> > > I still don't quite believe that. The wait flags seem to be mostly
> > > related to powersave,
> > 
> > This is true, but why would they not be important for going off
> > channel ? The TSF sync seems just as important as waiting for the ACK
> > for the nullfunc from the AP.
> 
> Why? It doesn't seem important to me?

How else could you possibly do a reliable calculation for the computation of the
next DTIM?

> > >> The TSF synch will help us more accurately set the other flags though,
> > >> if we ignore it by the time we come back to our home channel we likely
> > >> would be out of synch with the AP's beacons. It makes sense to me to
> > >> wait to be in synch with the AP's beacons before going off channel,
> > >> otherwise our other wait constraints would be off. Let alone
> > >> calculating the time we do need to be awake for.
> > >
> > > I don't think I can parse this. But does it matter? If we're going to
> > > wait for a DTIM beacon in mac80211, we should have TSF sync anyway. And
> > > after you go back to the operating channel, it seems likely that you'd
> > > want to re-sync anyway.
> > 
> > I don't understand, you first say it doesn't matter and then you end
> > up suggesting to actually do a TSF sync prior to going off channel and
> > then when coming back. It seems to me you do get it.
> 
> No, I don't understand the need for a TSF sync.

        /*
         * Pull nexttbtt forward to reflect the current
         * TSF and calculate dtim+cfp state for the result.
         */
        tsf = ath9k_hw_gettsf64(ah);
        tsftu = TSF_TO_TU(tsf>>32, tsf) + FUDGE;

        num_beacons = tsftu / intval + 1;
        offset = tsftu % intval;
        nexttbtt = tsftu - offset;
        if (offset)
                nexttbtt += intval;

        /* DTIM Beacon every dtimperiod Beacon */
        dtim_dec_count = num_beacons % dtimperiod;
        /* CFP every cfpperiod DTIM Beacon */
        cfp_dec_count = (num_beacons / dtimperiod) % cfpperiod;
        if (dtim_dec_count)
                cfp_dec_count++;

        dtimcount -= dtim_dec_count;
        if (dtimcount < 0)
                dtimcount += dtimperiod;

        cfpcount -= cfp_dec_count;
        if (cfpcount < 0)
                cfpcount += cfpperiod;

        bs.bs_intval = intval;
        bs.bs_nexttbtt = nexttbtt;
        bs.bs_dtimperiod = dtimperiod*intval;
        bs.bs_nextdtim = bs.bs_nexttbtt + dtimcount*intval;
        bs.bs_cfpperiod = cfpperiod*bs.bs_dtimperiod;
        bs.bs_cfpnext = bs.bs_nextdtim + cfpcount*bs.bs_dtimperiod;
        bs.bs_cfpmaxduration = 0;

We then set the beacon miss stuff:

        /*
         * Calculate the number of consecutive beacons to miss* before taking
         * a BMISS interrupt. The configuration is specified in TU so we only
         * need calculate based on the beacon interval.  Note that we clamp the
         * result to at most 15 beacons.
         */
        if (sleepduration > intval) {
                bs.bs_bmissthreshold = conf->listen_interval *
                        ATH_DEFAULT_BMISS_LIMIT / 2;
        } else {
                bs.bs_bmissthreshold = DIV_ROUND_UP(conf->bmiss_timeout, intval);
                if (bs.bs_bmissthreshold > 15)
                        bs.bs_bmissthreshold = 15;
                else if (bs.bs_bmissthreshold <= 0)
                        bs.bs_bmissthreshold = 1;
        }

For more details see ath_beacon_config_sta().

So we do the TSF synch to more reliably know when we will get a DTIM and also
adjust our beacon miss interrupt. For offchannel operation know the DTIM
more accurately will help in our computation when going off channel.

> And the time we need to
> be awake for doesn't seem like it's determined by the wait constraints
> or the beacon ... you're not going to get it perfect anyway.

Correct, only that waiting for proper DTIM calculation is important though,
and there are wait constraints that relate to ensuring we update the DTIM
calculation more accurately.

The ath9k wait constraint are checked for prior go going to power save and since
we use power save for offchannel operation we should also avoid going to power
save for the same wait constraints. That was the logic I followed. Granted I
do believe more of this should go into mac80211 but until then these wait
constraints are the only thing I have to resolve these issues.

> > >> It is a good question, can we be relying on on ACK from the AP other
> > >> than PS-poll data after coming out of sleep? I am not sure.
> > >
> > > ?
> > > The AP will ACK the nullfunc frame when you wake up, that should be
> > > sufficient to know that the AP knows you've woken up. Or returned to the
> > > operating channel, in the context of this discussion.
> > 
> > Yes, but my point was that right now we don't check for this in
> > mac80211 but that code does exist in ath9k to wait for an ACK from the
> > AP. I was explaining when I see PS_WAIT_FOR_TX_ACK used, I noted that
> > ath9k has code that checks for an ACK from the AP prior to letting us
> > go to sleep, we force this upon ourselves when first frame we send out
> > to the AP is not a PS-POLL. I was wondering when this could possibly
> > happen.
> 
> Oh, dunno.

I'll check.. Wonder if this is just leftoever cruft code.

> > > Like for example just implementing waiting for
> > > DTIM (traffic), that should be simple -- at the beginning of scan you
> > > just wake up the HW, wait for a (any) beacon, calculate if you can fit
> > > off-channel time before DTIM, and either wait for DTIM beacon+traffic or
> > > squeeze off-channel time before it... Seems simple enough?
> > 
> > And I'm saying its not. To wait for the DTIM you need to know the DTIM
> > count, and you could also miss a few DTIM beacons. 
> 
> What do you mean by "need to know the DTIM count"? You just need to look
> at every received beacon.

The DTIM count on the TIM information element tells stations how many beacons
must be transmitted before receiving the next DTIM. The DTIM count will be 0
when we've reached a DTIM.

http://wireless.kernel.org/en/developers/Documentation/ieee80211/power-savings

If you do not know the DTIM count at which beacon will we get the DTIM?

> > Then you also need
> > to keep track of the mcast / bcast data after the DTIM beacon. I noted
> > how ath9k already does all this and we use flags to annotate these
> > things to help avoid putting our chip to sleep. My patches make use of
> > these flags from ath9k to prevent us from going offchannel, and what
> > I'd like to do is to start offloading some of these flags one by one
> > to mac80211 to make them generic for all mac80211 drivers that use sw
> > scan.
> 
> Right right, I understand, but I guess I believe that the problem isn't
> hard enough or pressing enough to warrant the additional churn needed,

It is a subjective matter, I agree, it depends how reliable you want
to claim mac80211 is for broadcast and multicast traffic when drivers
use sw scan. One key item that is more important though is if we are
only trying to be on our home channel when doing offchannel operation
for bg scanning then we must also send a PS-POLL to get our own buffered
unicast frames after DTIM. I am not sure if we take care of this right
now and the wait constraints in ath9k might actually be helping with
this.

> since it's not just as you did it now, but it really needs to be
> asynchronous with event callbacks,

In case of firmware timeouts? Or what?

> and error handling is also more
> complex in the mac80211/driver interaction case (what if the driver
> never returns the event in the timeout? then mac80211 asks for the next
> channel, but the driver _then_ returns the event? etc.)

So I am not sure what you are referring to here. Are you referring to
timeouts for calls we make on the driver for wait constraints or
are you referring to possible endless wait constraints?

If the first, then I do not follow.

If the later, then I think it shouldn't happen often enough *if* we
actually end up dealing with most constraints properly in mac80211.

Or maybe I just completely misunderstood this last point.

Worst case scenerio we will just have to review this in more detail next week
in person on Thursday or Friday during the wireless summit.

  Luis

Re: [RFC 0/3] mac80211: new API for handling broadcast / multicast on sw scan

[Johannes Berg]

On Tue, 2010-08-31 at 09:59 -0700, Luis R. Rodriguez wrote:

> > > This is true, but why would they not be important for going off
> > > channel ? The TSF sync seems just as important as waiting for the ACK
> > > for the nullfunc from the AP.
> > 
> > Why? It doesn't seem important to me?
> 
> How else could you possibly do a reliable calculation for the computation of the
> next DTIM?

Dunno what your TSF sync does, but a single beacon frame has all the
relevant information for computing this.

> So we do the TSF synch to more reliably know when we will get a DTIM and also
> adjust our beacon miss interrupt. For offchannel operation know the DTIM
> more accurately will help in our computation when going off channel.

Yeah but when _going_ offchannel we don't need to know exactly when it
will happen in advance, we can just wait for the relevant thing to end
and then go offchannel ... what am I missing?

> > > And I'm saying its not. To wait for the DTIM you need to know the DTIM
> > > count, and you could also miss a few DTIM beacons. 
> > 
> > What do you mean by "need to know the DTIM count"? You just need to look
> > at every received beacon.
> 
> The DTIM count on the TIM information element tells stations how many beacons
> must be transmitted before receiving the next DTIM. The DTIM count will be 0
> when we've reached a DTIM.
> 
> http://wireless.kernel.org/en/developers/Documentation/ieee80211/power-savings
> 
> If you do not know the DTIM count at which beacon will we get the DTIM?

Each beacon contains the count and period ... we're just going around in
circles. You just need a single beacon frame.

> It is a subjective matter, I agree, it depends how reliable you want
> to claim mac80211 is for broadcast and multicast traffic when drivers
> use sw scan. One key item that is more important though is if we are
> only trying to be on our home channel when doing offchannel operation
> for bg scanning then we must also send a PS-POLL to get our own buffered
> unicast frames after DTIM.

No, we don't need to send PS-poll as we send a wakeup. I think, anyway.

> > since it's not just as you did it now, but it really needs to be
> > asynchronous with event callbacks,
> 
> In case of firmware timeouts? Or what?

No, just the polling you have now doesn't seem adequate.

> > and error handling is also more
> > complex in the mac80211/driver interaction case (what if the driver
> > never returns the event in the timeout? then mac80211 asks for the next
> > channel, but the driver _then_ returns the event? etc.)
> 
> So I am not sure what you are referring to here. Are you referring to
> timeouts for calls we make on the driver for wait constraints or
> are you referring to possible endless wait constraints?
> 
> If the first, then I do not follow.
> 
> If the later, then I think it shouldn't happen often enough *if* we
> actually end up dealing with most constraints properly in mac80211.
> 
> Or maybe I just completely misunderstood this last point.

No, I'm referring to the fact that when we don't poll, we'll be waiting
for the driver to tell us when we can go offchannel. But what if the
driver for some reason doesn't tell us within an acceptable timeout
period? Say it has endless constraints. Do we require drivers to
_always_ tell us? What then if the driver's buggy? etc.

> Worst case scenerio we will just have to review this in more detail next week
> in person on Thursday or Friday during the wireless summit.

Actually let's do that, since anything that isn't purely implemented in
mac80211 should probably take into account multi-channel virtual
interface operation as well, which we'll probably want to offload to the
driver for scheduling between interfaces.

johannes

Re: [RFC 0/3] mac80211: new API for handling broadcast / multicast on sw scan

[Luis R. Rodriguez]

On Tue, Aug 31, 2010 at 11:12:20AM -0700, Johannes Berg wrote:
> On Tue, 2010-08-31 at 09:59 -0700, Luis R. Rodriguez wrote:
> 
> > > > This is true, but why would they not be important for going off
> > > > channel ? The TSF sync seems just as important as waiting for the ACK
> > > > for the nullfunc from the AP.
> > > 
> > > Why? It doesn't seem important to me?
> > 
> > How else could you possibly do a reliable calculation for the computation of the
> > next DTIM?
> 
> Dunno what your TSF sync does, but a single beacon frame has all the
> relevant information for computing this.

Ok you have a point about the DTIM period and count being available on all
beacons but more on the TSF sync below.

> > So we do the TSF synch to more reliably know when we will get a DTIM and also
> > adjust our beacon miss interrupt. For offchannel operation know the DTIM
> > more accurately will help in our computation when going off channel.
> 
> Yeah but when _going_ offchannel we don't need to know exactly when it
> will happen in advance, we can just wait for the relevant thing to end
> and then go offchannel 

And how do we do this currently wait for the relevant things to end
in mac802111 prior to going offchannel ? We agree we don't, right. But
what we're trying to get more agreement on is what items we do need
to wait on. Let me clarify what I believe these are and show their
dependency graph:

  * ACK for nullfunc for offchannel operation - not handled yet
  * Do not go offchannel when we would expect the DTIM -- not handled yet

That's it! But the second one require some more consideration. Lets consider
the worst case scenerio. After we get the DTIM we will get all buffered
broadcast and multicast buffered frames from the AP. This will ultimately
vary depending on the size of your BSS, so you cannot easily compute this
as a station, unless you add some guess fudge factor and that will
ultimately be very subjective. So I argue you cannot calculate this
and instead it is easier to monitor for the broadcast / multicast data
sent by the AP and check when we get the last one (the more bit is off).

Do you agree with that? If not how else do you suggest we calculate this
on mac80211?

Now, lets consider an even harier situation. Say we use the above mechanism
but you missed the last frame that indicates that there are no more broadcast
and multicast traffic, your next best bet is to timeout on the next beacon
and assume no more broadcast / multicast traffic is pending. Consider this
situation now if our DTIM is 1 though and we happen to have a lot of noise
and happen to loose the last broadcast / multicast frame with the with the
more bit off... We just won't be able to go offchannel unless we are willing
to drop frames in that worst case scenerio and we really value our broadcast
and multicast frames. You could also deal with only going offchannel if
you do get this more bit off. If DTIM period is > 1 though then we have
more flexibility and can rely on the next beacon for a timeout indication
to know we can then go off channel.

Now, all this is sensitive to timing constraints lead by the DTIM count.
As the DTIM period increases it means we are able to skip more beacons for
going offchannel. Consider a DTIM period (also called DTIM interval on my
E3000 it seems) of 100. Granted, *today* on 802.11 you won't likely go
offchannel for more than:

  100 * beacon_interval * TU

But if we start doing more complex things while off channel (*cough*) or
consider a few years with new possible bands other than 2.4 GHz and 5 GHz,
you should be able to calculate with more accuracy what is the maximum time
you are allowed to go offchannel. You'll only get one shot at this calculation:
at the end of the next DTIM if you're already in power save mode. So if our
TSF is off from the last beacon you received from the AP then when going
offchannel you will likely not make the right computation.

> ... what am I missing?

Not sure if this helps. Please let me know.

> > > > And I'm saying its not. To wait for the DTIM you need to know the DTIM
> > > > count, and you could also miss a few DTIM beacons. 
> > > 
> > > What do you mean by "need to know the DTIM count"? You just need to look
> > > at every received beacon.
> > 
> > The DTIM count on the TIM information element tells stations how many beacons
> > must be transmitted before receiving the next DTIM. The DTIM count will be 0
> > when we've reached a DTIM.
> > 
> > http://wireless.kernel.org/en/developers/Documentation/ieee80211/power-savings
> > 
> > If you do not know the DTIM count at which beacon will we get the DTIM?
> 
> Each beacon contains the count and period ... we're just going around in
> circles. You just need a single beacon frame.

OK sure.

> > It is a subjective matter, I agree, it depends how reliable you want
> > to claim mac80211 is for broadcast and multicast traffic when drivers
> > use sw scan. One key item that is more important though is if we are
> > only trying to be on our home channel when doing offchannel operation
> > for bg scanning then we must also send a PS-POLL to get our own buffered
> > unicast frames after DTIM.
> 
> No, we don't need to send PS-poll as we send a wakeup. I think, anyway.

How else would we get buffered unicast data from the AP?

> > > since it's not just as you did it now, but it really needs to be
> > > asynchronous with event callbacks,
> > 
> > In case of firmware timeouts? Or what?
> 
> No, just the polling you have now doesn't seem adequate.

Heh yeah, I agree with that completely. I just can't figure out
a better way yet. I suspect things would be a lot easier if all the
work ath9k did would be in mac80211 already.

> > > and error handling is also more
> > > complex in the mac80211/driver interaction case (what if the driver
> > > never returns the event in the timeout? then mac80211 asks for the next
> > > channel, but the driver _then_ returns the event? etc.)
> > 
> > So I am not sure what you are referring to here. Are you referring to
> > timeouts for calls we make on the driver for wait constraints or
> > are you referring to possible endless wait constraints?
> > 
> > If the first, then I do not follow.
> > 
> > If the later, then I think it shouldn't happen often enough *if* we
> > actually end up dealing with most constraints properly in mac80211.
> > 
> > Or maybe I just completely misunderstood this last point.
> 
> No, I'm referring to the fact that when we don't poll, we'll be waiting
> for the driver to tell us when we can go offchannel. But what if the
> driver for some reason doesn't tell us within an acceptable timeout
> period? Say it has endless constraints. Do we require drivers to
> _always_ tell us? What then if the driver's buggy? etc.

If we have all the work within mac80211 I don't suspect we'll need even
events, we'd just know immediately when going offchannel of the status
of the hardware. If we don't have all the wait constraints addressed in
mac80211 then yeah, things get fishy and we either need polling which we
both don't like or events.

Relying on events is kind of odd for scanning. Say a user issues a scan
while associated. The command will sit there and wait until the driver
sends an event for when its ready -- or we fail. I suppose that is not
so unreasonable..

> > Worst case scenerio we will just have to review this in more detail next week
> > in person on Thursday or Friday during the wireless summit.
> 
> Actually let's do that, since anything that isn't purely implemented in
> mac80211 should probably take into account multi-channel virtual
> interface operation as well, which we'll probably want to offload to the
> driver for scheduling between interfaces.

OK sure, but I may still need some sort of solution in the interim,
we'll see!

  Luis

Re: [RFC 0/3] mac80211: new API for handling broadcast / multicast on sw scan

[Johannes Berg]

On Tue, 2010-08-31 at 19:14 -0700, Luis R. Rodriguez wrote:

> > > So we do the TSF synch to more reliably know when we will get a DTIM and also
> > > adjust our beacon miss interrupt. For offchannel operation know the DTIM
> > > more accurately will help in our computation when going off channel.
> > 
> > Yeah but when _going_ offchannel we don't need to know exactly when it
> > will happen in advance, we can just wait for the relevant thing to end
> > and then go offchannel 
> 
> And how do we do this currently wait for the relevant things to end
> in mac802111 prior to going offchannel ? We agree we don't, right. 

Yes, we don't, right now.

> But
> what we're trying to get more agreement on is what items we do need
> to wait on. Let me clarify what I believe these are and show their
> dependency graph:
> 
>   * ACK for nullfunc for offchannel operation - not handled yet
>   * Do not go offchannel when we would expect the DTIM -- not handled yet
> 
> That's it!

Agreed.

> But the second one require some more consideration. Lets consider
> the worst case scenerio. After we get the DTIM we will get all buffered
> broadcast and multicast buffered frames from the AP. This will ultimately
> vary depending on the size of your BSS, so you cannot easily compute this
> as a station, unless you add some guess fudge factor and that will
> ultimately be very subjective. 

Yeah but you don't need to compute it... you can just wait for it.

> So I argue you cannot calculate this
> and instead it is easier to monitor for the broadcast / multicast data
> sent by the AP and check when we get the last one (the more bit is off).

Right.

> Do you agree with that? If not how else do you suggest we calculate this
> on mac80211?

Yes, I agree, we don't need to compute it beforehand.

> Now, lets consider an even harier situation. Say we use the above mechanism
> but you missed the last frame that indicates that there are no more broadcast
> and multicast traffic, your next best bet is to timeout on the next beacon
> and assume no more broadcast / multicast traffic is pending. Consider this
> situation now if our DTIM is 1 though and we happen to have a lot of noise
> and happen to loose the last broadcast / multicast frame with the with the
> more bit off... We just won't be able to go offchannel unless we are willing
> to drop frames in that worst case scenerio and we really value our broadcast
> and multicast frames. You could also deal with only going offchannel if
> you do get this more bit off. If DTIM period is > 1 though then we have
> more flexibility and can rely on the next beacon for a timeout indication
> to know we can then go off channel.

Yeah, but if you value your multicast traffic _that_ much, you'd better
know up front and never actually ask mac80211 to scan. If it's asked to
scan, the only sensible thing to do is to actually do a scan, even if
that might mean dropping some multicast traffic.

> Now, all this is sensitive to timing constraints lead by the DTIM count.
> As the DTIM period increases it means we are able to skip more beacons for
> going offchannel. Consider a DTIM period (also called DTIM interval on my
> E3000 it seems) of 100.

I realise you're doing this for illustration purposes, but a DTIM period
of 100 is insane, even with a short beacon interval.

> But if we start doing more complex things while off channel (*cough*) or
> consider a few years with new possible bands other than 2.4 GHz and 5 GHz,
> you should be able to calculate with more accuracy what is the maximum time
> you are allowed to go offchannel.

Hopefully, by then, 802.11v will be deployed in access points ...

> You'll only get one shot at this calculation:
> at the end of the next DTIM if you're already in power save mode. So if our
> TSF is off from the last beacon you received from the AP then when going
> offchannel you will likely not make the right computation.

Yet still, I see this as the other way around. With powersave, you're
limiting your saved power by all these factors, and you return for
multicast traffic etc. But when you ask for a scan, where's the
trade-off? Should we really make the scan basically useless in order to
not lose multicast frames? I say no, if you ask for a scan you've got to
consider the possibility it may cause dropping multicast frames.

> > > It is a subjective matter, I agree, it depends how reliable you want
> > > to claim mac80211 is for broadcast and multicast traffic when drivers
> > > use sw scan. One key item that is more important though is if we are
> > > only trying to be on our home channel when doing offchannel operation
> > > for bg scanning then we must also send a PS-POLL to get our own buffered
> > > unicast frames after DTIM.
> > 
> > No, we don't need to send PS-poll as we send a wakeup. I think, anyway.
> 
> How else would we get buffered unicast data from the AP?

We send a nullfunc wakeup to it, and it just flushes all the frames to
us.

> If we have all the work within mac80211 I don't suspect we'll need even
> events, we'd just know immediately when going offchannel of the status
> of the hardware. If we don't have all the wait constraints addressed in
> mac80211 then yeah, things get fishy and we either need polling which we
> both don't like or events.

Yeah, but the way I see it it's not so bad that we can't fairly easily
implement it in mac80211 now. You've stated before that we really only
need to wait for two things (I'll repeat them):
 * ACK for nullfunc for offchannel operation - not handled yet
 * Do not go offchannel when we would expect the DTIM -- not handled yet

ACK for nullfunc is pretty easy, really, but I suspect once you
implement flush it will no longer matter since we can just do this:
 1) stop sdata traffic
 2) drv_flush()
 3) queue up nullfunc frame on VO
 4) drv_flush() again
 5) do channel switch etc.

which will make sure the nullfunc frame went out _last_ and it actually
went out before we channel switch


DTIM handling is slightly more complicated, but all it really needs to
do is turn off powersave, and wait for the beacon. That doesn't seem so
complex to implement -- probably easier, in fact, than doing the stuff
you did properly without polling.

(of course, we should probably also re-implement software scanning in
terms of off-channel work, so we don't have to do everything twice)

johannes