[RFC PATCH v1 0/3] mac80211: beacon filtering

[RFC PATCH v1 0/3] mac80211: beacon filtering

[Kalle Valo]

Here's my suggestion how to implement beacon filtering in mac80211.
The basic idea is simple, the driver enables filtering is whenever
mac80211 enables power save. Driver calls ieee80211_beacon_loss()
whenever hardware informs about beacon loss.

I have tested this with stlc45xx and based on simple tests this seems
to work. For reference I'll send stlc45xx patch as a followup.

TODO: 

o API documentation
o test with different hardwares (without beacon filtering) to 
  avoid regressions

---

Kalle Valo (3):
      mac80211: add beacon filtering support
      mac80211: track beacons separately from the rx path activity
      mac80211: decrease execution of the associated timer


 include/net/mac80211.h     |    2 +
 net/mac80211/ieee80211_i.h |    4 ++
 net/mac80211/iface.c       |    3 +
 net/mac80211/mlme.c        |  114 ++++++++++++++++++++++++++++++++------------
 net/mac80211/rx.c          |    9 +++
 5 files changed, 99 insertions(+), 33 deletions(-)

[RFC PATCH v1 1/3] mac80211: decrease execution of the associated timer

[Kalle Valo]

Currently the timer is triggering every two seconds
(IEEE80211_MONITORING_INTERVAL). Decrease the timer to only trigger when
nothing is received to avoid waking up CPU.

Now there's a functional change that probe requests are sent only when the
data path is idle, earlier they were sent also while there was activity
on the data path.

This is also preparation for beacon filtering support. Thanks to Johannes
Berg for the idea.

Signed-off-by: Kalle Valo <kalle.valo@nokia.com>
---

 net/mac80211/ieee80211_i.h |    1 +
 net/mac80211/mlme.c        |    7 +++++++
 net/mac80211/rx.c          |    3 +++
 3 files changed, 11 insertions(+), 0 deletions(-)

diff --git a/net/mac80211/ieee80211_i.h b/net/mac80211/ieee80211_i.h
index d06c757..7851135 100644
--- a/net/mac80211/ieee80211_i.h
+++ b/net/mac80211/ieee80211_i.h
@@ -1078,6 +1078,7 @@ void ieee80211_dynamic_ps_timer(unsigned long data);
 void ieee80211_send_nullfunc(struct ieee80211_local *local,
 			     struct ieee80211_sub_if_data *sdata,
 			     int powersave);
+void ieee80211_rx_trigger(struct ieee80211_sub_if_data *sdata);
 
 void ieee80211_wake_queues_by_reason(struct ieee80211_hw *hw,
 				     enum queue_stop_reason reason);
diff --git a/net/mac80211/mlme.c b/net/mac80211/mlme.c
index 5a49779..b3caac0 100644
--- a/net/mac80211/mlme.c
+++ b/net/mac80211/mlme.c
@@ -911,6 +911,13 @@ static void ieee80211_associate(struct ieee80211_sub_if_data *sdata)
 	mod_timer(&ifmgd->timer, jiffies + IEEE80211_ASSOC_TIMEOUT);
 }
 
+void ieee80211_rx_trigger(struct ieee80211_sub_if_data *sdata)
+{
+	if (sdata->vif.type == NL80211_IFTYPE_STATION) {
+		mod_timer(&sdata->u.mgd.timer,
+			  jiffies + IEEE80211_MONITORING_INTERVAL);
+	}
+}
 
 static void ieee80211_associated(struct ieee80211_sub_if_data *sdata)
 {
diff --git a/net/mac80211/rx.c b/net/mac80211/rx.c
index 66f7ecf..4afb639 100644
--- a/net/mac80211/rx.c
+++ b/net/mac80211/rx.c
@@ -855,6 +855,9 @@ ieee80211_rx_h_sta_process(struct ieee80211_rx_data *rx)
 	if (!(rx->flags & IEEE80211_RX_RA_MATCH))
 		return RX_CONTINUE;
 
+	if (!is_multicast_ether_addr(hdr->addr1))
+		ieee80211_rx_trigger(rx->sdata);
+
 	sta->rx_fragments++;
 	sta->rx_bytes += rx->skb->len;
 	sta->last_signal = rx->status->signal;

Re: [RFC PATCH v1 1/3] mac80211: decrease execution of the associated timer

[Johannes Berg]

[-- Attachment #1: Type: text/plain, Size: 2188 bytes --]

On Mon, 2009-02-23 at 18:37 +0200, Kalle Valo wrote:
> Currently the timer is triggering every two seconds
> (IEEE80211_MONITORING_INTERVAL). Decrease the timer to only trigger when
> nothing is received to avoid waking up CPU.
> 
> Now there's a functional change that probe requests are sent only when the
> data path is idle, earlier they were sent also while there was activity
> on the data path.
> 
> This is also preparation for beacon filtering support. Thanks to Johannes
> Berg for the idea.
> 
> Signed-off-by: Kalle Valo <kalle.valo@nokia.com>
> ---
> 
>  net/mac80211/ieee80211_i.h |    1 +
>  net/mac80211/mlme.c        |    7 +++++++
>  net/mac80211/rx.c          |    3 +++
>  3 files changed, 11 insertions(+), 0 deletions(-)
> 
> diff --git a/net/mac80211/ieee80211_i.h b/net/mac80211/ieee80211_i.h
> index d06c757..7851135 100644
> --- a/net/mac80211/ieee80211_i.h
> +++ b/net/mac80211/ieee80211_i.h
> @@ -1078,6 +1078,7 @@ void ieee80211_dynamic_ps_timer(unsigned long data);
>  void ieee80211_send_nullfunc(struct ieee80211_local *local,
>  			     struct ieee80211_sub_if_data *sdata,
>  			     int powersave);
> +void ieee80211_rx_trigger(struct ieee80211_sub_if_data *sdata);

I think that could have a better name maybe? _sta_rx_notify?

>  void ieee80211_wake_queues_by_reason(struct ieee80211_hw *hw,
>  				     enum queue_stop_reason reason);
> diff --git a/net/mac80211/mlme.c b/net/mac80211/mlme.c
> index 5a49779..b3caac0 100644
> --- a/net/mac80211/mlme.c
> +++ b/net/mac80211/mlme.c
> @@ -911,6 +911,13 @@ static void ieee80211_associate(struct ieee80211_sub_if_data *sdata)
>  	mod_timer(&ifmgd->timer, jiffies + IEEE80211_ASSOC_TIMEOUT);
>  }
>  
> +void ieee80211_rx_trigger(struct ieee80211_sub_if_data *sdata)
> +{
> +	if (sdata->vif.type == NL80211_IFTYPE_STATION) {
> +		mod_timer(&sdata->u.mgd.timer,
> +			  jiffies + IEEE80211_MONITORING_INTERVAL);
> +	}

Should we have the sta check outside the function? Seems a little odd to
have a check in a file that only contains STATION mode code.

Other than that, looks good to me. I'd have thought this would be more
complicated :)

johannes

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [RFC PATCH v1 1/3] mac80211: decrease execution of the associated timer

[Kalle Valo]

Johannes Berg <johannes@sipsolutions.net> writes:

>> --- a/net/mac80211/ieee80211_i.h
>> +++ b/net/mac80211/ieee80211_i.h
>> @@ -1078,6 +1078,7 @@ void ieee80211_dynamic_ps_timer(unsigned long data);
>>  void ieee80211_send_nullfunc(struct ieee80211_local *local,
>>  			     struct ieee80211_sub_if_data *sdata,
>>  			     int powersave);
>> +void ieee80211_rx_trigger(struct ieee80211_sub_if_data *sdata);
>
> I think that could have a better name maybe? _sta_rx_notify?

That's a lot better name, I'll rename it.

>> +void ieee80211_rx_trigger(struct ieee80211_sub_if_data *sdata)
>> +{
>> +	if (sdata->vif.type == NL80211_IFTYPE_STATION) {
>> +		mod_timer(&sdata->u.mgd.timer,
>> +			  jiffies + IEEE80211_MONITORING_INTERVAL);
>> +	}
>
> Should we have the sta check outside the function? Seems a little odd to
> have a check in a file that only contains STATION mode code.

Actually I first had the check in rx.c, then moved it to mlme.c just
to have less changes in rx.c. But you have a point, I'll move the
check back to rx.c.

> Other than that, looks good to me. I'd have thought this would be more
> complicated :)

Luckily it wasn't, I prefer having simple patches :)

-- 
Kalle Valo

[RFC PATCH v1 2/3] mac80211: track beacons separately from the rx path activity

[Kalle Valo]

Separate beacon and rx path tracking in preparation for the beacon filtering
support. At the same time change ieee80211_associated() to look a bit simpler.

Probe requests are now sent only after IEEE80211_PROBE_IDLE_TIME, which
is now set to 60 seconds.

Signed-off-by: Kalle Valo <kalle.valo@nokia.com>
---

 net/mac80211/ieee80211_i.h |    1 +
 net/mac80211/mlme.c        |   75 +++++++++++++++++++++++++-------------------
 net/mac80211/rx.c          |    6 +++-
 3 files changed, 49 insertions(+), 33 deletions(-)

diff --git a/net/mac80211/ieee80211_i.h b/net/mac80211/ieee80211_i.h
index 7851135..2a19e4c 100644
--- a/net/mac80211/ieee80211_i.h
+++ b/net/mac80211/ieee80211_i.h
@@ -305,6 +305,7 @@ struct ieee80211_if_managed {
 	unsigned long request;
 
 	unsigned long last_probe;
+	unsigned long last_beacon;
 
 	unsigned int flags;
 
diff --git a/net/mac80211/mlme.c b/net/mac80211/mlme.c
index b3caac0..d2cc1e0 100644
--- a/net/mac80211/mlme.c
+++ b/net/mac80211/mlme.c
@@ -30,7 +30,7 @@
 #define IEEE80211_ASSOC_TIMEOUT (HZ / 5)
 #define IEEE80211_ASSOC_MAX_TRIES 3
 #define IEEE80211_MONITORING_INTERVAL (2 * HZ)
-#define IEEE80211_PROBE_INTERVAL (60 * HZ)
+#define IEEE80211_PROBE_IDLE_TIME (60 * HZ)
 #define IEEE80211_RETRY_AUTH_INTERVAL (1 * HZ)
 
 /* utils */
@@ -924,7 +924,7 @@ static void ieee80211_associated(struct ieee80211_sub_if_data *sdata)
 	struct ieee80211_if_managed *ifmgd = &sdata->u.mgd;
 	struct ieee80211_local *local = sdata->local;
 	struct sta_info *sta;
-	int disassoc;
+	bool disassoc = false;
 
 	/* TODO: start monitoring current AP signal quality and number of
 	 * missed beacons. Scan other channels every now and then and search
@@ -939,36 +939,39 @@ static void ieee80211_associated(struct ieee80211_sub_if_data *sdata)
 	if (!sta) {
 		printk(KERN_DEBUG "%s: No STA entry for own AP %pM\n",
 		       sdata->dev->name, ifmgd->bssid);
-		disassoc = 1;
-	} else {
-		disassoc = 0;
-		if (time_after(jiffies,
-			       sta->last_rx + IEEE80211_MONITORING_INTERVAL)) {
-			if (ifmgd->flags & IEEE80211_STA_PROBEREQ_POLL) {
-				printk(KERN_DEBUG "%s: No ProbeResp from "
-				       "current AP %pM - assume out of "
-				       "range\n",
-				       sdata->dev->name, ifmgd->bssid);
-				disassoc = 1;
-			} else
-				ieee80211_send_probe_req(sdata, ifmgd->bssid,
-							 ifmgd->ssid,
-							 ifmgd->ssid_len,
-							 NULL, 0);
-			ifmgd->flags ^= IEEE80211_STA_PROBEREQ_POLL;
-		} else {
-			ifmgd->flags &= ~IEEE80211_STA_PROBEREQ_POLL;
-			if (time_after(jiffies, ifmgd->last_probe +
-				       IEEE80211_PROBE_INTERVAL)) {
-				ifmgd->last_probe = jiffies;
-				ieee80211_send_probe_req(sdata, ifmgd->bssid,
-							 ifmgd->ssid,
-							 ifmgd->ssid_len,
-							 NULL, 0);
-			}
-		}
+		disassoc = true;
+		goto unlock;
+	}
+
+	if ((ifmgd->flags & IEEE80211_STA_PROBEREQ_POLL) &&
+	    time_after(jiffies, sta->last_rx + IEEE80211_MONITORING_INTERVAL)) {
+		printk(KERN_DEBUG "%s: no probe response from AP %pM "
+		       "- disassociating\n",
+		       sdata->dev->name, ifmgd->bssid);
+		disassoc = true;
+		ifmgd->flags &= ~IEEE80211_STA_PROBEREQ_POLL;
+		goto unlock;
+	}
+
+	if (time_after(jiffies,
+		       ifmgd->last_beacon + IEEE80211_MONITORING_INTERVAL)) {
+		printk(KERN_DEBUG "%s: beacon loss from AP %pM "
+		       "- sending probe request\n",
+		       sdata->dev->name, ifmgd->bssid);
+		ifmgd->flags |= IEEE80211_STA_PROBEREQ_POLL;
+		ieee80211_send_probe_req(sdata, ifmgd->bssid, ifmgd->ssid,
+					 ifmgd->ssid_len, NULL, 0);
+		goto unlock;
+
 	}
 
+	if (time_after(jiffies, sta->last_rx + IEEE80211_PROBE_IDLE_TIME)) {
+		ifmgd->flags |= IEEE80211_STA_PROBEREQ_POLL;
+		ieee80211_send_probe_req(sdata, ifmgd->bssid, ifmgd->ssid,
+					 ifmgd->ssid_len, NULL, 0);
+	}
+
+ unlock:
 	rcu_read_unlock();
 
 	if (disassoc)
@@ -976,7 +979,7 @@ static void ieee80211_associated(struct ieee80211_sub_if_data *sdata)
 					WLAN_REASON_PREV_AUTH_NOT_VALID);
 	else
 		mod_timer(&ifmgd->timer, jiffies +
-				      IEEE80211_MONITORING_INTERVAL);
+			  IEEE80211_MONITORING_INTERVAL);
 }
 
 
@@ -1356,6 +1359,8 @@ static void ieee80211_rx_mgmt_assoc_resp(struct ieee80211_sub_if_data *sdata,
 	bss_conf->assoc_capability = capab_info;
 	ieee80211_set_associated(sdata, changed);
 
+	ifmgd->last_beacon = jiffies;
+
 	ieee80211_associated(sdata);
 }
 
@@ -1403,9 +1408,12 @@ static void ieee80211_rx_mgmt_probe_resp(struct ieee80211_sub_if_data *sdata,
 					 size_t len,
 					 struct ieee80211_rx_status *rx_status)
 {
+	struct ieee80211_if_managed *ifmgd;
 	size_t baselen;
 	struct ieee802_11_elems elems;
 
+	ifmgd = &sdata->u.mgd;
+
 	if (memcmp(mgmt->da, sdata->dev->dev_addr, ETH_ALEN))
 		return; /* ignore ProbeResp to foreign address */
 
@@ -1420,11 +1428,14 @@ static void ieee80211_rx_mgmt_probe_resp(struct ieee80211_sub_if_data *sdata,
 
 	/* direct probe may be part of the association flow */
 	if (test_and_clear_bit(IEEE80211_STA_REQ_DIRECT_PROBE,
-	    &sdata->u.mgd.request)) {
+			       &ifmgd->request)) {
 		printk(KERN_DEBUG "%s direct probe responded\n",
 		       sdata->dev->name);
 		ieee80211_authenticate(sdata);
 	}
+
+	if (ifmgd->flags & IEEE80211_STA_PROBEREQ_POLL)
+		ifmgd->flags &= ~IEEE80211_STA_PROBEREQ_POLL;
 }
 
 static void ieee80211_rx_mgmt_beacon(struct ieee80211_sub_if_data *sdata,
diff --git a/net/mac80211/rx.c b/net/mac80211/rx.c
index 4afb639..ce58e02 100644
--- a/net/mac80211/rx.c
+++ b/net/mac80211/rx.c
@@ -849,7 +849,11 @@ ieee80211_rx_h_sta_process(struct ieee80211_rx_data *rx)
 		 * Mesh beacons will update last_rx when if they are found to
 		 * match the current local configuration when processed.
 		 */
-		sta->last_rx = jiffies;
+		if (rx->sdata->vif.type == NL80211_IFTYPE_STATION &&
+		    ieee80211_is_beacon(hdr->frame_control)) {
+			rx->sdata->u.mgd.last_beacon = jiffies;
+		} else
+			sta->last_rx = jiffies;
 	}
 
 	if (!(rx->flags & IEEE80211_RX_RA_MATCH))

Re: [RFC PATCH v1 2/3] mac80211: track beacons separately from the rx path activity

[Johannes Berg]

[-- Attachment #1: Type: text/plain, Size: 796 bytes --]

On Mon, 2009-02-23 at 18:37 +0200, Kalle Valo wrote:

> @@ -1356,6 +1359,8 @@ static void ieee80211_rx_mgmt_assoc_resp(struct ieee80211_sub_if_data *sdata,
>  	bss_conf->assoc_capability = capab_info;
>  	ieee80211_set_associated(sdata, changed);
>  
> +	ifmgd->last_beacon = jiffies;
> +

That looks a little misplaced; I think it's actually intended anyway?
Maybe add a comment?

> -		sta->last_rx = jiffies;
> +		if (rx->sdata->vif.type == NL80211_IFTYPE_STATION &&
> +		    ieee80211_is_beacon(hdr->frame_control)) {
> +			rx->sdata->u.mgd.last_beacon = jiffies;
> +		} else
> +			sta->last_rx = jiffies;

would it be more appropriate to do that in the function that processes
the beacon in mlme.c? Or does that not work because of the workqueue and
timing?

johannes

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [RFC PATCH v1 2/3] mac80211: track beacons separately from the rx path activity

[Kalle Valo]

Johannes Berg <johannes@sipsolutions.net> writes:

> On Mon, 2009-02-23 at 18:37 +0200, Kalle Valo wrote:
>
>> @@ -1356,6 +1359,8 @@ static void ieee80211_rx_mgmt_assoc_resp(struct ieee80211_sub_if_data *sdata,
>>  	bss_conf->assoc_capability = capab_info;
>>  	ieee80211_set_associated(sdata, changed);
>>  
>> +	ifmgd->last_beacon = jiffies;
>> +
>
> That looks a little misplaced; I think it's actually intended
> anyway?

Yes, it's intended. We need to initialise the variable after a
successful association, otherwise this test in ieee80211_associated()
will trigger immediately:

	if (time_after(jiffies,
		       ifmgd->last_beacon + IEEE80211_MONITORING_INTERVAL)) {
		printk(KERN_DEBUG "%s: beacon loss from AP %pM "
		       "- sending probe request\n",
		       sdata->dev->name, ifmgd->bssid);

> Maybe add a comment?

I will.

>> -		sta->last_rx = jiffies;
>> +		if (rx->sdata->vif.type == NL80211_IFTYPE_STATION &&
>> +		    ieee80211_is_beacon(hdr->frame_control)) {
>> +			rx->sdata->u.mgd.last_beacon = jiffies;
>> +		} else
>> +			sta->last_rx = jiffies;
>
> would it be more appropriate to do that in the function that processes
> the beacon in mlme.c? Or does that not work because of the workqueue and
> timing?

The test is there because I didn't want to include beacons to
sta->last_rx timestamp. That way I can send probe requests only during
data path idle period:

	if (time_after(jiffies, sta->last_rx + IEEE80211_PROBE_IDLE_TIME)) {
		ifmgd->flags |= IEEE80211_STA_PROBEREQ_POLL;
		ieee80211_send_probe_req(sdata, ifmgd->bssid, ifmgd->ssid,
					 ifmgd->ssid_len, NULL, 0);
	}

sta->last_rx is used in master mode as well, so I might have to do
some trickery if I move the code to mlme.c. But I do share your
view of having mlme related code in mlme.c, it makes life a lot
easier. I need to think this a bit more.

-- 
Kalle Valo

[RFC PATCH v1 3/3] mac80211: add beacon filtering support

[Kalle Valo]

Add IEEE80211_HW_BEACON_FILTERING flag so that driver inform that it supports
beacon filtering. Drivers need to call the new function
ieee80211_beacon_loss() to notify about beacon loss.

Signed-off-by: Kalle Valo <kalle.valo@nokia.com>
---

 include/net/mac80211.h     |    2 ++
 net/mac80211/ieee80211_i.h |    2 ++
 net/mac80211/iface.c       |    3 +++
 net/mac80211/mlme.c        |   34 +++++++++++++++++++++++++++++++++-
 4 files changed, 40 insertions(+), 1 deletions(-)

diff --git a/include/net/mac80211.h b/include/net/mac80211.h
index e01c63a..f74bada 100644
--- a/include/net/mac80211.h
+++ b/include/net/mac80211.h
@@ -903,6 +903,7 @@ enum ieee80211_hw_flags {
 	IEEE80211_HW_PS_NULLFUNC_STACK			= 1<<11,
 	IEEE80211_HW_SUPPORTS_DYNAMIC_PS		= 1<<12,
 	IEEE80211_HW_MFP_CAPABLE			= 1<<13,
+	IEEE80211_HW_BEACON_FILTERING			= 1<<14,
 };
 
 /**
@@ -1971,6 +1972,7 @@ void ieee80211_stop_tx_ba_cb_irqsafe(struct ieee80211_hw *hw, const u8 *ra,
 struct ieee80211_sta *ieee80211_find_sta(struct ieee80211_hw *hw,
 					 const u8 *addr);
 
+void ieee80211_beacon_loss(struct ieee80211_hw *hw);
 
 /* Rate control API */
 
diff --git a/net/mac80211/ieee80211_i.h b/net/mac80211/ieee80211_i.h
index 2a19e4c..5cd6296 100644
--- a/net/mac80211/ieee80211_i.h
+++ b/net/mac80211/ieee80211_i.h
@@ -272,6 +272,7 @@ struct ieee80211_if_managed {
 	struct timer_list chswitch_timer;
 	struct work_struct work;
 	struct work_struct chswitch_work;
+	struct work_struct beacon_loss_work;
 
 	u8 bssid[ETH_ALEN], prev_bssid[ETH_ALEN];
 
@@ -1080,6 +1081,7 @@ void ieee80211_send_nullfunc(struct ieee80211_local *local,
 			     struct ieee80211_sub_if_data *sdata,
 			     int powersave);
 void ieee80211_rx_trigger(struct ieee80211_sub_if_data *sdata);
+void ieee80211_beacon_loss_work(struct work_struct *work);
 
 void ieee80211_wake_queues_by_reason(struct ieee80211_hw *hw,
 				     enum queue_stop_reason reason);
diff --git a/net/mac80211/iface.c b/net/mac80211/iface.c
index 2acc416..e0fa20f 100644
--- a/net/mac80211/iface.c
+++ b/net/mac80211/iface.c
@@ -470,6 +470,9 @@ static int ieee80211_stop(struct net_device *dev)
 		 */
 		cancel_work_sync(&sdata->u.mgd.work);
 		cancel_work_sync(&sdata->u.mgd.chswitch_work);
+
+		cancel_work_sync(&sdata->u.mgd.beacon_loss_work);
+
 		/*
 		 * When we get here, the interface is marked down.
 		 * Call synchronize_rcu() to wait for the RX path
diff --git a/net/mac80211/mlme.c b/net/mac80211/mlme.c
index d2cc1e0..22cc8e4 100644
--- a/net/mac80211/mlme.c
+++ b/net/mac80211/mlme.c
@@ -919,6 +919,36 @@ void ieee80211_rx_trigger(struct ieee80211_sub_if_data *sdata)
 	}
 }
 
+void ieee80211_beacon_loss_work(struct work_struct *work)
+{
+	struct ieee80211_sub_if_data *sdata =
+		container_of(work, struct ieee80211_sub_if_data,
+			     u.mgd.beacon_loss_work);
+
+	printk(KERN_DEBUG "%s: beacon loss from AP %pM "
+	       "- disassociating\n", sdata->dev->name, sdata->u.mgd.bssid);
+
+	ieee80211_set_disassoc(sdata, true, true,
+			       WLAN_REASON_PREV_AUTH_NOT_VALID);
+}
+
+void ieee80211_beacon_loss(struct ieee80211_hw *hw)
+{
+	struct ieee80211_local *local = hw_to_local(hw);
+	struct ieee80211_sub_if_data *sdata;
+
+	rcu_read_lock();
+	list_for_each_entry(sdata, &local->interfaces, list) {
+		if (sdata->vif.type != NL80211_IFTYPE_STATION)
+			continue;
+
+		queue_work(local->hw.workqueue,
+			   &sdata->u.mgd.beacon_loss_work);
+	}
+	rcu_read_unlock();
+}
+EXPORT_SYMBOL(ieee80211_beacon_loss);
+
 static void ieee80211_associated(struct ieee80211_sub_if_data *sdata)
 {
 	struct ieee80211_if_managed *ifmgd = &sdata->u.mgd;
@@ -953,7 +983,8 @@ static void ieee80211_associated(struct ieee80211_sub_if_data *sdata)
 		goto unlock;
 	}
 
-	if (time_after(jiffies,
+	if (!(local->hw.flags & IEEE80211_HW_BEACON_FILTERING) &&
+	    time_after(jiffies,
 		       ifmgd->last_beacon + IEEE80211_MONITORING_INTERVAL)) {
 		printk(KERN_DEBUG "%s: beacon loss from AP %pM "
 		       "- sending probe request\n",
@@ -1834,6 +1865,7 @@ void ieee80211_sta_setup_sdata(struct ieee80211_sub_if_data *sdata)
 	ifmgd = &sdata->u.mgd;
 	INIT_WORK(&ifmgd->work, ieee80211_sta_work);
 	INIT_WORK(&ifmgd->chswitch_work, ieee80211_chswitch_work);
+	INIT_WORK(&ifmgd->beacon_loss_work, ieee80211_beacon_loss_work);
 	setup_timer(&ifmgd->timer, ieee80211_sta_timer,
 		    (unsigned long) sdata);
 	setup_timer(&ifmgd->chswitch_timer, ieee80211_chswitch_timer,

Re: [RFC PATCH v1 3/3] mac80211: add beacon filtering support

[Luis R. Rodriguez]

On Mon, Feb 23, 2009 at 8:37 AM, Kalle Valo <kalle.valo@nokia.com> wrote:
> Add IEEE80211_HW_BEACON_FILTERING flag so that driver inform that it supports
> beacon filtering. Drivers need to call the new function
> ieee80211_beacon_loss() to notify about beacon loss.
>
> Signed-off-by: Kalle Valo <kalle.valo@nokia.com>
> ---
>
>  include/net/mac80211.h     |    2 ++
>  net/mac80211/ieee80211_i.h |    2 ++
>  net/mac80211/iface.c       |    3 +++
>  net/mac80211/mlme.c        |   34 +++++++++++++++++++++++++++++++++-
>  4 files changed, 40 insertions(+), 1 deletions(-)
>
> diff --git a/include/net/mac80211.h b/include/net/mac80211.h
> index e01c63a..f74bada 100644
> --- a/include/net/mac80211.h
> +++ b/include/net/mac80211.h
> @@ -903,6 +903,7 @@ enum ieee80211_hw_flags {
>        IEEE80211_HW_PS_NULLFUNC_STACK                  = 1<<11,
>        IEEE80211_HW_SUPPORTS_DYNAMIC_PS                = 1<<12,
>        IEEE80211_HW_MFP_CAPABLE                        = 1<<13,
> +       IEEE80211_HW_BEACON_FILTERING                   = 1<<14,

Not sure this conveys what it means, how about BEACON_MISS ?

>  };
>
>  /**
> @@ -1971,6 +1972,7 @@ void ieee80211_stop_tx_ba_cb_irqsafe(struct ieee80211_hw *hw, const u8 *ra,
>  struct ieee80211_sta *ieee80211_find_sta(struct ieee80211_hw *hw,
>                                         const u8 *addr);
>
> +void ieee80211_beacon_loss(struct ieee80211_hw *hw);
>
>  /* Rate control API */
>

Some kdoc would be nice for this. Also I suspect you designed this
with 2 GHz legacy (802.11bg) single band cards in mind. For the case
or cards capable of 5 GHz or of HT I believe we should consider the
cases of dealing with DFS or HT channel switch both of which can occur
dynamically while the STAs are associated. If hardware is capable of
differentiating this (through beacon checksums) then that's an
enhancement but for now I suspect that is not the case for most
hardware that implements this and as such for the case when on DFS or
using HT we can simply have mac80211 disable this. Not sure...
Thoughts?

  Luis

Re: [RFC PATCH v1 3/3] mac80211: add beacon filtering support

[Kalle Valo]

"Luis R. Rodriguez" <mcgrof@gmail.com> writes:

> On Mon, Feb 23, 2009 at 8:37 AM, Kalle Valo <kalle.valo@nokia.com> wrote:
>> --- a/include/net/mac80211.h
>> +++ b/include/net/mac80211.h
>> @@ -903,6 +903,7 @@ enum ieee80211_hw_flags {
>>        IEEE80211_HW_PS_NULLFUNC_STACK                  = 1<<11,
>>        IEEE80211_HW_SUPPORTS_DYNAMIC_PS                = 1<<12,
>>        IEEE80211_HW_MFP_CAPABLE                        = 1<<13,
>> +       IEEE80211_HW_BEACON_FILTERING                   = 1<<14,
>
> Not sure this conveys what it means, how about BEACON_MISS ?

To me the name "beacon filtering" sounds better. The idea of the
feature is to avoid sending beacons to the host, that is "filter
beacons" in firmware. Beacon miss is an action of that feature. Am I
making any sense? :)

Also my assumption here is that ieee80211_beacon_loss() should be
called only after certain number of consecutive beacon misses. While
testing these patches on stlc45xx I used number 10. Can ath9k handle
anything like this? Or will it just report each beacon miss
individually?

>> +void ieee80211_beacon_loss(struct ieee80211_hw *hw);
>>
>>  /* Rate control API */
>>
>
> Some kdoc would be nice for this.

Definitely. It's on my TODO list.

> Also I suspect you designed this with 2 GHz legacy (802.11bg) single
> band cards in mind.

Good guess :) Yes, I haven't thought about 5 GHz band.

> For the case or cards capable of 5 GHz or of HT I believe we should
> consider the cases of dealing with DFS or HT channel switch both of
> which can occur dynamically while the STAs are associated. If
> hardware is capable of differentiating this (through beacon
> checksums) then that's an enhancement but for now I suspect that is
> not the case for most hardware that implements this and as such for
> the case when on DFS or using HT we can simply have mac80211 disable
> this. Not sure... Thoughts?

I don't see a problem. Like you said, such hardware should have beacon
checksumming support. Whenever the checksum has changed, the hardware
should pass the beacon to the host and mac80211 would receive the
beacon just like without beacon filtering. 

Beacon filtering can be thought like filtering unrelevant beacons, but
passing through the beacons which have new information. For example,
stlc45xx already has beacon checksum support even though it doesn't
support 5 GHz band. Unfortunately I haven't managed to find the time
to test it yet.

If there is hardware using 5 GHz band and does not support beacon
checksumming, then the driver should not even enable beacon filtering.

-- 
Kalle Valo

Re: [RFC PATCH v1 3/3] mac80211: add beacon filtering support

[Luis R. Rodriguez]

On Mon, Feb 23, 2009 at 11:06 AM, Kalle Valo <kalle.valo@iki.fi> wrote:
> "Luis R. Rodriguez" <mcgrof@gmail.com> writes:
>
>> On Mon, Feb 23, 2009 at 8:37 AM, Kalle Valo <kalle.valo@nokia.com> wrote:
>>> --- a/include/net/mac80211.h
>>> +++ b/include/net/mac80211.h
>>> @@ -903,6 +903,7 @@ enum ieee80211_hw_flags {
>>>        IEEE80211_HW_PS_NULLFUNC_STACK                  = 1<<11,
>>>        IEEE80211_HW_SUPPORTS_DYNAMIC_PS                = 1<<12,
>>>        IEEE80211_HW_MFP_CAPABLE                        = 1<<13,
>>> +       IEEE80211_HW_BEACON_FILTERING                   = 1<<14,
>>
>> Not sure this conveys what it means, how about BEACON_MISS ?
>
> To me the name "beacon filtering" sounds better. The idea of the
> feature is to avoid sending beacons to the host, that is "filter
> beacons" in firmware. Beacon miss is an action of that feature. Am I
> making any sense? :)

Nah you're right, I was thinking our own "action" heh.

> Also my assumption here is that ieee80211_beacon_loss() should be
> called only after certain number of consecutive beacon misses. While
> testing these patches on stlc45xx I used number 10. Can ath9k handle
> anything like this? Or will it just report each beacon miss
> individually?

Will have to check, I haven't read how this works in detail yet.

>>> +void ieee80211_beacon_loss(struct ieee80211_hw *hw);
>>>
>>>  /* Rate control API */
>>>
>>
>> Some kdoc would be nice for this.
>
> Definitely. It's on my TODO list.
>
>> Also I suspect you designed this with 2 GHz legacy (802.11bg) single
>> band cards in mind.
>
> Good guess :) Yes, I haven't thought about 5 GHz band.
>
>> For the case or cards capable of 5 GHz or of HT I believe we should
>> consider the cases of dealing with DFS or HT channel switch both of
>> which can occur dynamically while the STAs are associated. If
>> hardware is capable of differentiating this (through beacon
>> checksums) then that's an enhancement but for now I suspect that is
>> not the case for most hardware that implements this and as such for
>> the case when on DFS or using HT we can simply have mac80211 disable
>> this. Not sure... Thoughts?
>
> I don't see a problem. Like you said, such hardware should have beacon
> checksumming support. Whenever the checksum has changed, the hardware
> should pass the beacon to the host and mac80211 would receive the
> beacon just like without beacon filtering.

_should_ -- but I don't think this is yet implemented.

> Beacon filtering can be thought like filtering unrelevant beacons, but
> passing through the beacons which have new information. For example,
> stlc45xx already has beacon checksum support even though it doesn't
> support 5 GHz band. Unfortunately I haven't managed to find the time
> to test it yet.

As IEEE-802.11 advances how does the hardware know which IEs to
checksum or not for? :)

> If there is hardware using 5 GHz band and does not support beacon
> checksumming, then the driver should not even enable beacon filtering.

Heh.. umm, I'd like us to add this as a feature eventually too to be
able to save power but I think we do not checksum and hence my
comments.

  Luis

Re: [RFC PATCH v1 3/3] mac80211: add beacon filtering support

[Kalle Valo]

"Luis R. Rodriguez" <mcgrof@gmail.com> writes:

> On Mon, Feb 23, 2009 at 11:06 AM, Kalle Valo <kalle.valo@iki.fi> wrote:
>>
>>> For the case or cards capable of 5 GHz or of HT I believe we should
>>> consider the cases of dealing with DFS or HT channel switch both of
>>> which can occur dynamically while the STAs are associated. If
>>> hardware is capable of differentiating this (through beacon
>>> checksums) then that's an enhancement but for now I suspect that is
>>> not the case for most hardware that implements this and as such for
>>> the case when on DFS or using HT we can simply have mac80211 disable
>>> this. Not sure... Thoughts?
>>
>> I don't see a problem. Like you said, such hardware should have beacon
>> checksumming support. Whenever the checksum has changed, the hardware
>> should pass the beacon to the host and mac80211 would receive the
>> beacon just like without beacon filtering.
>
> _should_ -- but I don't think this is yet implemented.

This is transparent for mac80211, if that's what you are asking. Even
though the beacon filtering feature is enabled, the hardware can still
send as much as beacons it wants. The most important is that the
driver will call ieee80211_beacon_loss() whenever beacons are lost
because mac80211 will not be following them.

>> Beacon filtering can be thought like filtering unrelevant beacons, but
>> passing through the beacons which have new information. For example,
>> stlc45xx already has beacon checksum support even though it doesn't
>> support 5 GHz band. Unfortunately I haven't managed to find the time
>> to test it yet.
>
> As IEEE-802.11 advances how does the hardware know which IEs to
> checksum or not for? :)

This is what is documented about stlc45xx firmware beacon filtering:

"The LM_PSM_CHECKSUM flag configures the LMAC to calculate a checksum
over the beacon frame body. The checksum is calculated over the
frame-body, starting after the timestamp element. Excluded from the
checksum calculation are all flexible elements, with a corresponding
element ID in the exclude array structure member. The beacon is
forwarded to the application, if the checksum changes from the
previous received beacon."

So the host can just provide the element ids which need to be excluded.

>> If there is hardware using 5 GHz band and does not support beacon
>> checksumming, then the driver should not even enable beacon filtering.
>
> Heh.. umm, I'd like us to add this as a feature eventually too to be
> able to save power but I think we do not checksum and hence my
> comments.

Other option is that the hardware just periodically passes a beacon to
the host, for example every 10 seconds. Not very elegant, but I guess
good enough for certain use cases.

-- 
Kalle Valo

Re: [RFC PATCH v1 3/3] mac80211: add beacon filtering support

[Luis R. Rodriguez]

On Mon, Feb 23, 2009 at 11:31 AM, Kalle Valo <kalle.valo@iki.fi> wrote:
> "Luis R. Rodriguez" <mcgrof@gmail.com> writes:
>
>> On Mon, Feb 23, 2009 at 11:06 AM, Kalle Valo <kalle.valo@iki.fi> wrote:
>>>
>>>> For the case or cards capable of 5 GHz or of HT I believe we should
>>>> consider the cases of dealing with DFS or HT channel switch both of
>>>> which can occur dynamically while the STAs are associated. If
>>>> hardware is capable of differentiating this (through beacon
>>>> checksums) then that's an enhancement but for now I suspect that is
>>>> not the case for most hardware that implements this and as such for
>>>> the case when on DFS or using HT we can simply have mac80211 disable
>>>> this. Not sure... Thoughts?
>>>
>>> I don't see a problem. Like you said, such hardware should have beacon
>>> checksumming support. Whenever the checksum has changed, the hardware
>>> should pass the beacon to the host and mac80211 would receive the
>>> beacon just like without beacon filtering.
>>
>> _should_ -- but I don't think this is yet implemented.
>
> This is transparent for mac80211, if that's what you are asking. Even
> though the beacon filtering feature is enabled, the hardware can still
> send as much as beacons it wants. The most important is that the
> driver will call ieee80211_beacon_loss() whenever beacons are lost
> because mac80211 will not be following them.

Sure. My concerns was if drivers should enable/disable the flag
dynamically based on configuration. If hardware supports checksumming
as the one you are working with does then you are set, however if it
is not then essentially we either do not support this flag for those
type of devices that do not support checksumming _or_ we consider the
cases where this can be disabled dynamically depending on the
configuration (DFS, HT).

If we do support dynamic tuning of this variable due to different type
of connections to APs (DFS or HT) it might make sense to just have a
separate flag to indicate this and let mac80211 figure things out for
you. This is what I was getting at.

>>> Beacon filtering can be thought like filtering unrelevant beacons, but
>>> passing through the beacons which have new information. For example,
>>> stlc45xx already has beacon checksum support even though it doesn't
>>> support 5 GHz band. Unfortunately I haven't managed to find the time
>>> to test it yet.
>>
>> As IEEE-802.11 advances how does the hardware know which IEs to
>> checksum or not for? :)
>
> This is what is documented about stlc45xx firmware beacon filtering:
>
> "The LM_PSM_CHECKSUM flag configures the LMAC to calculate a checksum
> over the beacon frame body. The checksum is calculated over the
> frame-body, starting after the timestamp element. Excluded from the
> checksum calculation are all flexible elements, with a corresponding
> element ID in the exclude array structure member. The beacon is
> forwarded to the application, if the checksum changes from the
> previous received beacon."
>
> So the host can just provide the element ids which need to be excluded.

This is nice :)

>>> If there is hardware using 5 GHz band and does not support beacon
>>> checksumming, then the driver should not even enable beacon filtering.
>>
>> Heh.. umm, I'd like us to add this as a feature eventually too to be
>> able to save power but I think we do not checksum and hence my
>> comments.
>
> Other option is that the hardware just periodically passes a beacon to
> the host, for example every 10 seconds. Not very elegant, but I guess
> good enough for certain use cases.

Not good enough for DFS and not sure about HT as per the spec. I think
its easier to not enable this when on DFS channels or connecting using
11n.

  Luis

Re: [RFC PATCH v1 3/3] mac80211: add beacon filtering support

[Luis R. Rodriguez]

On Mon, Feb 23, 2009 at 11:31 AM, Kalle Valo <kalle.valo@iki.fi> wrote:
> "Luis R. Rodriguez" <mcgrof@gmail.com> writes:
>
>> On Mon, Feb 23, 2009 at 11:06 AM, Kalle Valo <kalle.valo@iki.fi> wrote:
>>>
>>>> For the case or cards capable of 5 GHz or of HT I believe we should
>>>> consider the cases of dealing with DFS or HT channel switch both of
>>>> which can occur dynamically while the STAs are associated. If
>>>> hardware is capable of differentiating this (through beacon
>>>> checksums) then that's an enhancement but for now I suspect that is
>>>> not the case for most hardware that implements this and as such for
>>>> the case when on DFS or using HT we can simply have mac80211 disable
>>>> this. Not sure... Thoughts?
>>>
>>> I don't see a problem. Like you said, such hardware should have beacon
>>> checksumming support. Whenever the checksum has changed, the hardware
>>> should pass the beacon to the host and mac80211 would receive the
>>> beacon just like without beacon filtering.
>>
>> _should_ -- but I don't think this is yet implemented.
>
> This is transparent for mac80211, if that's what you are asking. Even
> though the beacon filtering feature is enabled, the hardware can still
> send as much as beacons it wants. The most important is that the
> driver will call ieee80211_beacon_loss() whenever beacons are lost
> because mac80211 will not be following them.
>
>>> Beacon filtering can be thought like filtering unrelevant beacons, but
>>> passing through the beacons which have new information. For example,
>>> stlc45xx already has beacon checksum support even though it doesn't
>>> support 5 GHz band. Unfortunately I haven't managed to find the time
>>> to test it yet.
>>
>> As IEEE-802.11 advances how does the hardware know which IEs to
>> checksum or not for? :)
>
> This is what is documented about stlc45xx firmware beacon filtering:
>
> "The LM_PSM_CHECKSUM flag configures the LMAC to calculate a checksum
> over the beacon frame body. The checksum is calculated over the
> frame-body, starting after the timestamp element. Excluded from the
> checksum calculation are all flexible elements, with a corresponding
> element ID in the exclude array structure member. The beacon is
> forwarded to the application, if the checksum changes from the
> previous received beacon."
>
> So the host can just provide the element ids which need to be excluded.

How does the hardware deal with IEs that may change order? I am not
sure how common this is but I do wonder.

  Luis

Re: [RFC PATCH v1 3/3] mac80211: add beacon filtering support

[Johannes Berg]

[-- Attachment #1: Type: text/plain, Size: 319 bytes --]

On Mon, 2009-02-23 at 20:46 -0800, Luis R. Rodriguez wrote:

> How does the hardware deal with IEs that may change order? I am not
> sure how common this is but I do wonder.

Uh, it would be extraordinarily stupid to change IE order in every
beacon! I guess it won't deal and we lose the benefits...

johannes

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [RFC PATCH v1 3/3] mac80211: add beacon filtering support

[Jouni Malinen]

On Mon, Feb 23, 2009 at 08:46:59PM -0800, Luis R. Rodriguez wrote:

> How does the hardware deal with IEs that may change order? I am not
> sure how common this is but I do wonder.

There are no such IEs; there is an explicit requirement for the order of
IEs in the Beacon frames. Or well, the last slot is for vendor specific
stuff, but anyway, I would agree with Johannes on someone changing the
order of IEs in Beacons to be quite idiotic.

-- 
Jouni Malinen                                            PGP id EFC895FA

Re: [RFC PATCH v1 3/3] mac80211: add beacon filtering support

[Johannes Berg]

[-- Attachment #1: Type: text/plain, Size: 1420 bytes --]

On Mon, 2009-02-23 at 21:06 +0200, Kalle Valo wrote:

> Also my assumption here is that ieee80211_beacon_loss() should be
> called only after certain number of consecutive beacon misses. While
> testing these patches on stlc45xx I used number 10. Can ath9k handle
> anything like this? Or will it just report each beacon miss
> individually?

Should the number be configurable? The beacon interval might vary so it
might be useful to set it so that misses * interval is constant?

> I don't see a problem. Like you said, such hardware should have beacon
> checksumming support. Whenever the checksum has changed, the hardware
> should pass the beacon to the host and mac80211 would receive the
> beacon just like without beacon filtering. 
> 
> Beacon filtering can be thought like filtering unrelevant beacons, but
> passing through the beacons which have new information. For example,
> stlc45xx already has beacon checksum support even though it doesn't
> support 5 GHz band. Unfortunately I haven't managed to find the time
> to test it yet.
> 
> If there is hardware using 5 GHz band and does not support beacon
> checksumming, then the driver should not even enable beacon filtering.

Should the flag be per-band in that case? Or do we need checksum support
anyway? (Actually, we shouldn't call that checksum support, but 'beacon
change notification' or something, I guess)

johannes

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [RFC PATCH v1 3/3] mac80211: add beacon filtering support

[Kalle Valo]

Johannes Berg <johannes@sipsolutions.net> writes:

> On Mon, 2009-02-23 at 21:06 +0200, Kalle Valo wrote:
>
>> Also my assumption here is that ieee80211_beacon_loss() should be
>> called only after certain number of consecutive beacon misses. While
>> testing these patches on stlc45xx I used number 10. Can ath9k handle
>> anything like this? Or will it just report each beacon miss
>> individually?
>
> Should the number be configurable? The beacon interval might vary so it
> might be useful to set it so that misses * interval is constant?

Yes, I think so. I decided to omit this part for now and add it later
when we know more how different hardware support it.

>> I don't see a problem. Like you said, such hardware should have beacon
>> checksumming support. Whenever the checksum has changed, the hardware
>> should pass the beacon to the host and mac80211 would receive the
>> beacon just like without beacon filtering. 
>> 
>> Beacon filtering can be thought like filtering unrelevant beacons, but
>> passing through the beacons which have new information. For example,
>> stlc45xx already has beacon checksum support even though it doesn't
>> support 5 GHz band. Unfortunately I haven't managed to find the time
>> to test it yet.
>> 
>> If there is hardware using 5 GHz band and does not support beacon
>> checksumming, then the driver should not even enable beacon filtering.
>
> Should the flag be per-band in that case? 

I would like to see such (in my opinion broken) hardware design first.

> Or do we need checksum support anyway?

My current thinking is that to have proper beacon filtering the
hardware needs to support checksumming. Beacon filtering might work
without checksum support in most cases so that the users won't notice
anything. But from engineer's point of view I don't consider it as
good enough, at least ERP protection changes would end up unnoticed,
and I'm sure there are also other cases.

I don't know how beacon filtering works in wl12xx, I need to test that
soon. It might give some hints how other vendors have implemented
this.

> (Actually, we shouldn't call that checksum support, but 'beacon
> change notification' or something, I guess)

Yes. But I don't think we need to add any checksum support to
mac80211. Apart from mentioning it in the documentation, of course.

-- 
Kalle Valo

Re: [RFC PATCH v1 3/3] mac80211: add beacon filtering support

[Johannes Berg]

[-- Attachment #1: Type: text/plain, Size: 975 bytes --]

On Mon, 2009-02-23 at 18:37 +0200, Kalle Valo wrote:

> +void ieee80211_beacon_loss_work(struct work_struct *work)
> +{
> +	struct ieee80211_sub_if_data *sdata =
> +		container_of(work, struct ieee80211_sub_if_data,
> +			     u.mgd.beacon_loss_work);
> +
> +	printk(KERN_DEBUG "%s: beacon loss from AP %pM "
> +	       "- disassociating\n", sdata->dev->name, sdata->u.mgd.bssid);
> +
> +	ieee80211_set_disassoc(sdata, true, true,
> +			       WLAN_REASON_PREV_AUTH_NOT_VALID);
> +}

We used to go through a probe request cycle once to make sure, but I'm
not sure there's a point in that. Just pointing out the change here.

The code looks pretty good, but this will lead to an interesting
situation where "iwlist wlan1 scan last" ("iw dev wlan1 scan dump") will
not show _any_ BSS, which will probably trip up NM; this happens because
the BSS will not be updated and expire after 10 seconds. I think we need
a way to "hold on" to the BSS.

johannes

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [RFC PATCH v1 3/3] mac80211: add beacon filtering support

[Jouni Malinen]

On Mon, Feb 23, 2009 at 06:20:58PM -0800, Johannes Berg wrote:

> We used to go through a probe request cycle once to make sure, but I'm
> not sure there's a point in that. Just pointing out the change here.

Having seen some hardware implementations of Beacon miss getting out of
sync with the AP timing and causing way too frequent disconnections, I
would think that there may well be good reasons to check in software
that report was indeed correct, e.g., by sending the probe request
before giving up on the AP.

> The code looks pretty good, but this will lead to an interesting
> situation where "iwlist wlan1 scan last" ("iw dev wlan1 scan dump") will
> not show _any_ BSS, which will probably trip up NM; this happens because
> the BSS will not be updated and expire after 10 seconds. I think we need
> a way to "hold on" to the BSS.

Dropping the BSS is not good.. The scan results should always include
the current BSS so that information from it is available for things like
4-way handshake (WPA/RSN IE is needed from Beacon/ProbeResp).

-- 
Jouni Malinen                                            PGP id EFC895FA

Re: [RFC PATCH v1 3/3] mac80211: add beacon filtering support

[Kalle Valo]

Jouni Malinen <j@w1.fi> writes:

> On Mon, Feb 23, 2009 at 06:20:58PM -0800, Johannes Berg wrote:
>
>> We used to go through a probe request cycle once to make sure, but I'm
>> not sure there's a point in that. Just pointing out the change here.
>
> Having seen some hardware implementations of Beacon miss getting out of
> sync with the AP timing and causing way too frequent disconnections, I
> would think that there may well be good reasons to check in software
> that report was indeed correct, e.g., by sending the probe request
> before giving up on the AP.

I see your point, I also have had to suffer because of buggy APs
having unstable TBTT. But the cost for this is high, currently it
increases two seconds the time to signal a lost connection to the user
space. I would hope to have something faster.

>> The code looks pretty good, but this will lead to an interesting
>> situation where "iwlist wlan1 scan last" ("iw dev wlan1 scan dump") will
>> not show _any_ BSS, which will probably trip up NM; this happens because
>> the BSS will not be updated and expire after 10 seconds. I think we need
>> a way to "hold on" to the BSS.
>
> Dropping the BSS is not good.. The scan results should always include
> the current BSS so that information from it is available for things like
> 4-way handshake (WPA/RSN IE is needed from Beacon/ProbeResp).

Understood. Thanks for bringing this up.

-- 
Kalle Valo

Re: [RFC PATCH v1 3/3] mac80211: add beacon filtering support

[Jouni Malinen]

On Tue, Feb 24, 2009 at 08:36:03PM +0200, Kalle Valo wrote:
> I see your point, I also have had to suffer because of buggy APs
> having unstable TBTT. But the cost for this is high, currently it
> increases two seconds the time to signal a lost connection to the user
> space. I would hope to have something faster.

How can that take two seconds? All that would be needed here is to send
out a unicast Probe Request to the current AP and wait for a Probe
Response for a short timeout, say 20 ms. That shouldn't take more than
50 ms or so at most.. If the current implementation uses two seconds for
this, there is certainly room for improvement there and I would rather
see that done than lose this extra protection against buggy
implementations (either AP or STA for that matter).

-- 
Jouni Malinen                                            PGP id EFC895FA

Re: [RFC PATCH v1 3/3] mac80211: add beacon filtering support

[Kalle Valo]

Jouni Malinen <j@w1.fi> writes:

> On Tue, Feb 24, 2009 at 08:36:03PM +0200, Kalle Valo wrote:
>> I see your point, I also have had to suffer because of buggy APs
>> having unstable TBTT. But the cost for this is high, currently it
>> increases two seconds the time to signal a lost connection to the user
>> space. I would hope to have something faster.
>
> How can that take two seconds?

I was just referring to the current implementation.

> All that would be needed here is to send out a unicast Probe Request
> to the current AP and wait for a Probe Response for a short timeout,
> say 20 ms. That shouldn't take more than 50 ms or so at most.. 

I agree. Or sending a null frame and waiting for ack is even faster,
but I think mac80211 doesn't yet have infrastructure to check the
acks. And not all hardware can support this, unfortunately.

> If the current implementation uses two seconds for this, there is
> certainly room for improvement there and I would rather see that
> done than lose this extra protection against buggy implementations
> (either AP or STA for that matter).

Sure. But the thing is that there are a lot of buggy APs which are
broken is so many different ways that I would like to have a limit for
workarounds we create. But I guess this workaround might still be
useful. I'll revisit this whenever I find time to work on roaming
improvements, hopefully after few weeks.

-- 
Kalle Valo

Re: [RFC PATCH v1 3/3] mac80211: add beacon filtering support

[Kalle Valo]

Johannes Berg <johannes@sipsolutions.net> writes:

> On Mon, 2009-02-23 at 18:37 +0200, Kalle Valo wrote:
>
>> +void ieee80211_beacon_loss_work(struct work_struct *work)
>> +{
>> +	struct ieee80211_sub_if_data *sdata =
>> +		container_of(work, struct ieee80211_sub_if_data,
>> +			     u.mgd.beacon_loss_work);
>> +
>> +	printk(KERN_DEBUG "%s: beacon loss from AP %pM "
>> +	       "- disassociating\n", sdata->dev->name, sdata->u.mgd.bssid);
>> +
>> +	ieee80211_set_disassoc(sdata, true, true,
>> +			       WLAN_REASON_PREV_AUTH_NOT_VALID);
>> +}
>
> We used to go through a probe request cycle once to make sure, but I'm
> not sure there's a point in that. Just pointing out the change here.

Good catch, I'll fix this in v2.

I'm also not sure if it's good idea to send a probe request when we
have lost beacons. It slows down the AP lost case quite a lot. But
this can be considered in a separate patch, for now I want to have the
same functionality. Most probably I will revisit when I'll start
working with roaming improvements.

> The code looks pretty good, but this will lead to an interesting
> situation where "iwlist wlan1 scan last" ("iw dev wlan1 scan dump") will
> not show _any_ BSS, which will probably trip up NM; this happens because
> the BSS will not be updated and expire after 10 seconds. I think we need
> a way to "hold on" to the BSS.

I'll take a look at this and try to come up with something. 

Thanks for reviewing the patches.

-- 
Kalle Valo

[PATCH] stlc45xx: add beacon filtering support

[Kalle Valo]

Experimental patch.

Signed-off-by: Kalle Valo <kalle.valo@nokia.com>
---

 stlc45xx.c |   38 +++++++++++++++++++++++++++++++++-----
 1 files changed, 33 insertions(+), 5 deletions(-)

diff --git a/stlc45xx.c b/stlc45xx.c
index 7d6f909..276339f 100644
--- a/stlc45xx.c
+++ b/stlc45xx.c
@@ -1217,6 +1217,28 @@ static int stlc45xx_rx_txack(struct stlc45xx *stlc, struct sk_buff *skb)
 	return 0;
 }
 
+static int stlc45xx_rx_trap(struct stlc45xx *stlc, struct sk_buff *skb)
+{
+	struct s_lm_control *control;
+	struct s_lmo_trap *trap;
+
+	stlc45xx_debug(DEBUG_FUNC, "%s", __func__);
+
+	control = (struct s_lm_control *) skb->data;
+	trap = (struct s_lmo_trap *) (control + 1);
+
+	switch (trap->event) {
+	case LM_TRAP_NO_BEACON:
+		ieee80211_beacon_loss(stlc->hw);	
+		break;
+	default:
+		stlc45xx_warning("unhandled trap: %d\n", trap->event);
+		break;
+	}
+	
+	return 0;
+}
+
 static int stlc45xx_rx_control(struct stlc45xx *stlc, struct sk_buff *skb)
 {
 	struct s_lm_control *control;
@@ -1230,9 +1252,11 @@ static int stlc45xx_rx_control(struct stlc45xx *stlc, struct sk_buff *skb)
 	case LM_OID_TX:
 		ret = stlc45xx_rx_txack(stlc, skb);
 		break;
+	case LM_OID_TRAP:
+		ret = stlc45xx_rx_trap(stlc, skb);
+		break;
 	case LM_OID_SETUP:
 	case LM_OID_SCAN:
-	case LM_OID_TRAP:
 	case LM_OID_EDCF:
 	case LM_OID_KEYCACHE:
 	case LM_OID_PSM:
@@ -1587,7 +1611,7 @@ static void stlc45xx_tx_setup(struct stlc45xx *stlc)
 	setup->rx_buffer = FIRMWARE_RXBUFFER_START;
 	setup->rx_mtu = FIRMWARE_MTU;
 	setup->frontend = 5;
-	setup->timeout = 0;
+	setup->timeout = 2;
 	setup->truncate = 48896;
 	setup->bratemask = 0xffffffff;
 	setup->ref_clock = 644245094;
@@ -1662,11 +1686,13 @@ static void stlc45xx_tx_psm(struct stlc45xx *stlc, bool enable)
 	control->oid = LM_OID_PSM;
 
 	if (enable)
-		psm->flags |= LM_PSM;
+		psm->flags |= LM_PSM | LM_PSM_MCBC |
+			LM_PSM_CHECKSUM | LM_PSM_BEACON_TIMEOUT;
 
 	psm->aid = stlc->aid;
 
 	psm->beacon_rcpi_skip_max = 60;
+	psm->rcpi_delta_threshold = 0;
 
 	psm->intervals[0].interval = 1;
 	psm->intervals[0].periods = 1;
@@ -1677,7 +1703,7 @@ static void stlc45xx_tx_psm(struct stlc45xx *stlc, bool enable)
 	psm->intervals[3].interval = 1;
 	psm->intervals[3].periods = 1;
 
-	psm->nr = 0;
+	psm->nr = 10;
 	psm->exclude[0] = 0;
 
 	stlc45xx_debug(DEBUG_PSM, "sending LM_OID_PSM (aid %d, interval %d)",
@@ -2107,7 +2133,9 @@ static int __devinit stlc45xx_probe(struct spi_device *spi)
 		IEEE80211_HW_SIGNAL_DBM |
 		IEEE80211_HW_NOISE_DBM |
 		IEEE80211_HW_SUPPORTS_PS |
-		IEEE80211_HW_PS_NULLFUNC_STACK;
+		IEEE80211_HW_PS_NULLFUNC_STACK |
+		IEEE80211_HW_BEACON_FILTERING;
+
 	/* four bytes for padding */
 	hw->extra_tx_headroom = sizeof(struct s_lm_data_out) + 4;