[PATCH 0/4] mac80211 powersave work

[PATCH 0/4] mac80211 powersave work

[Johannes Berg]

This is pretty much unchanged from my previous v2 RFCs,
except that I added a default of 500ms for the dynamic
powersave timer.

The fourth patch is new, it adds beacon filtering to
mac80211 -- for the reasons explained in that patch.

For 2.6.31, we should make MAC80211_DEFAULT_PS default
off instead. John, how do you want to handle that?

johannes

[PATCH 1/4] mac80211: improve powersave implementation

[Johannes Berg]

When you have multiple virtual interfaces the current
implementation requires setting them up properly from
userspace, which is undesirable when we want to default
to power save mode. Keep track of powersave requested
from userspace per managed mode interface, and only
enable powersave globally when exactly one managed mode
interface is active and has powersave turned on.

Second, only start the dynPS timer when PS is turned
on, and properly turn it off when PS is turned off.

Third, fix the scan_sdata abuse in the dynps code.

Finally, also reorder the code and refactor the code
that enables PS or the dynps timer instead of having
it copied in two places.

Signed-off-by: Johannes Berg <johannes@sipsolutions.net>
---
v2: * don't disable PS when adding monitor interfaces
    * fix scan_sdata use for powersave timer bug by
      keeping track of which interface is _the_ managed
      interface (can only be one)

 net/mac80211/ieee80211_i.h |    9 +
 net/mac80211/iface.c       |    4 
 net/mac80211/mlme.c        |  228 ++++++++++++++++++++++++++++-----------------
 net/mac80211/scan.c        |    2 
 net/mac80211/wext.c        |   43 +-------
 5 files changed, 165 insertions(+), 121 deletions(-)

--- wireless-testing.orig/net/mac80211/ieee80211_i.h	2009-04-16 02:15:54.000000000 +0200
+++ wireless-testing/net/mac80211/ieee80211_i.h	2009-04-16 02:18:22.000000000 +0200
@@ -295,6 +295,8 @@ struct ieee80211_if_managed {
 	int auth_tries; /* retries for auth req */
 	int assoc_tries; /* retries for assoc req */
 
+	bool powersave; /* powersave requested for this iface */
+
 	unsigned long request;
 
 	unsigned long last_probe;
@@ -739,8 +741,12 @@ struct ieee80211_local {
 	int wifi_wme_noack_test;
 	unsigned int wmm_acm; /* bit field of ACM bits (BIT(802.1D tag)) */
 
-	bool powersave;
 	bool pspolling;
+	/*
+	 * PS can only be enabled when we have exactly one managed
+	 * interface (and monitors) in PS, this then points there.
+	 */
+	struct ieee80211_sub_if_data *ps_sdata;
 	struct work_struct dynamic_ps_enable_work;
 	struct work_struct dynamic_ps_disable_work;
 	struct timer_list dynamic_ps_timer;
@@ -932,6 +938,7 @@ int ieee80211_sta_deauthenticate(struct 
 int ieee80211_sta_disassociate(struct ieee80211_sub_if_data *sdata, u16 reason);
 void ieee80211_send_pspoll(struct ieee80211_local *local,
 			   struct ieee80211_sub_if_data *sdata);
+void ieee80211_recalc_ps(struct ieee80211_local *local);
 
 /* IBSS code */
 int ieee80211_ibss_commit(struct ieee80211_sub_if_data *sdata);
--- wireless-testing.orig/net/mac80211/iface.c	2009-04-16 02:15:54.000000000 +0200
+++ wireless-testing/net/mac80211/iface.c	2009-04-16 02:18:22.000000000 +0200
@@ -317,6 +317,8 @@ static int ieee80211_open(struct net_dev
 		ieee80211_set_wmm_default(sdata);
 	}
 
+	ieee80211_recalc_ps(local);
+
 	/*
 	 * ieee80211_sta_work is disabled while network interface
 	 * is down. Therefore, some configuration changes may not
@@ -572,6 +574,8 @@ static int ieee80211_stop(struct net_dev
 		hw_reconf_flags = 0;
 	}
 
+	ieee80211_recalc_ps(local);
+
 	/* do after stop to avoid reconfiguring when we stop anyway */
 	if (hw_reconf_flags)
 		ieee80211_hw_config(local, hw_reconf_flags);
--- wireless-testing.orig/net/mac80211/wext.c	2009-04-16 02:15:54.000000000 +0200
+++ wireless-testing/net/mac80211/wext.c	2009-04-16 02:18:22.000000000 +0200
@@ -747,7 +747,7 @@ static int ieee80211_ioctl_siwpower(stru
 	struct ieee80211_sub_if_data *sdata = IEEE80211_DEV_TO_SUB_IF(dev);
 	struct ieee80211_local *local = wdev_priv(dev->ieee80211_ptr);
 	struct ieee80211_conf *conf = &local->hw.conf;
-	int ret = 0, timeout = 0;
+	int timeout = 0;
 	bool ps;
 
 	if (!(local->hw.flags & IEEE80211_HW_SUPPORTS_PS))
@@ -779,42 +779,19 @@ static int ieee80211_ioctl_siwpower(stru
 		timeout = wrq->value / 1000;
 
  set:
-	if (ps == local->powersave && timeout == conf->dynamic_ps_timeout)
-		return ret;
+	if (ps == sdata->u.mgd.powersave && timeout == conf->dynamic_ps_timeout)
+		return 0;
 
-	local->powersave = ps;
+	sdata->u.mgd.powersave = ps;
 	conf->dynamic_ps_timeout = timeout;
 
 	if (local->hw.flags & IEEE80211_HW_SUPPORTS_DYNAMIC_PS)
-		ret = ieee80211_hw_config(local,
-					  IEEE80211_CONF_CHANGE_DYNPS_TIMEOUT);
+		ieee80211_hw_config(local,
+				    IEEE80211_CONF_CHANGE_DYNPS_TIMEOUT);
 
-	if (!(sdata->u.mgd.flags & IEEE80211_STA_ASSOCIATED))
-		return ret;
+	ieee80211_recalc_ps(local);
 
-	if (conf->dynamic_ps_timeout > 0 &&
-	    !(local->hw.flags & IEEE80211_HW_SUPPORTS_DYNAMIC_PS)) {
-		mod_timer(&local->dynamic_ps_timer, jiffies +
-			  msecs_to_jiffies(conf->dynamic_ps_timeout));
-	} else {
-		if (local->powersave) {
-			if (local->hw.flags & IEEE80211_HW_PS_NULLFUNC_STACK)
-				ieee80211_send_nullfunc(local, sdata, 1);
-			conf->flags |= IEEE80211_CONF_PS;
-			ret = ieee80211_hw_config(local,
-					IEEE80211_CONF_CHANGE_PS);
-		} else {
-			conf->flags &= ~IEEE80211_CONF_PS;
-			ret = ieee80211_hw_config(local,
-					IEEE80211_CONF_CHANGE_PS);
-			if (local->hw.flags & IEEE80211_HW_PS_NULLFUNC_STACK)
-				ieee80211_send_nullfunc(local, sdata, 0);
-			del_timer_sync(&local->dynamic_ps_timer);
-			cancel_work_sync(&local->dynamic_ps_enable_work);
-		}
-	}
-
-	return ret;
+	return 0;
 }
 
 static int ieee80211_ioctl_giwpower(struct net_device *dev,
@@ -822,9 +799,9 @@ static int ieee80211_ioctl_giwpower(stru
 				    union iwreq_data *wrqu,
 				    char *extra)
 {
-	struct ieee80211_local *local = wdev_priv(dev->ieee80211_ptr);
+	struct ieee80211_sub_if_data *sdata = IEEE80211_DEV_TO_SUB_IF(dev);
 
-	wrqu->power.disabled = !local->powersave;
+	wrqu->power.disabled = !sdata->u.mgd.powersave;
 
 	return 0;
 }
--- wireless-testing.orig/net/mac80211/mlme.c	2009-04-16 02:18:22.000000000 +0200
+++ wireless-testing/net/mac80211/mlme.c	2009-04-16 02:18:22.000000000 +0200
@@ -446,6 +446,145 @@ void ieee80211_send_pspoll(struct ieee80
 	ieee80211_tx_skb(sdata, skb, 0);
 }
 
+void ieee80211_send_nullfunc(struct ieee80211_local *local,
+			     struct ieee80211_sub_if_data *sdata,
+			     int powersave)
+{
+	struct sk_buff *skb;
+	struct ieee80211_hdr *nullfunc;
+	__le16 fc;
+
+	if (WARN_ON(sdata->vif.type != NL80211_IFTYPE_STATION))
+		return;
+
+	skb = dev_alloc_skb(local->hw.extra_tx_headroom + 24);
+	if (!skb) {
+		printk(KERN_DEBUG "%s: failed to allocate buffer for nullfunc "
+		       "frame\n", sdata->dev->name);
+		return;
+	}
+	skb_reserve(skb, local->hw.extra_tx_headroom);
+
+	nullfunc = (struct ieee80211_hdr *) skb_put(skb, 24);
+	memset(nullfunc, 0, 24);
+	fc = cpu_to_le16(IEEE80211_FTYPE_DATA | IEEE80211_STYPE_NULLFUNC |
+			 IEEE80211_FCTL_TODS);
+	if (powersave)
+		fc |= cpu_to_le16(IEEE80211_FCTL_PM);
+	nullfunc->frame_control = fc;
+	memcpy(nullfunc->addr1, sdata->u.mgd.bssid, ETH_ALEN);
+	memcpy(nullfunc->addr2, sdata->dev->dev_addr, ETH_ALEN);
+	memcpy(nullfunc->addr3, sdata->u.mgd.bssid, ETH_ALEN);
+
+	ieee80211_tx_skb(sdata, skb, 0);
+}
+
+/* powersave */
+static void ieee80211_enable_ps(struct ieee80211_local *local,
+				struct ieee80211_sub_if_data *sdata)
+{
+	struct ieee80211_conf *conf = &local->hw.conf;
+
+	if (conf->dynamic_ps_timeout > 0 &&
+	    !(local->hw.flags & IEEE80211_HW_SUPPORTS_DYNAMIC_PS)) {
+		mod_timer(&local->dynamic_ps_timer, jiffies +
+			  msecs_to_jiffies(conf->dynamic_ps_timeout));
+	} else {
+		if (local->hw.flags & IEEE80211_HW_PS_NULLFUNC_STACK)
+			ieee80211_send_nullfunc(local, sdata, 1);
+		conf->flags |= IEEE80211_CONF_PS;
+		ieee80211_hw_config(local, IEEE80211_CONF_CHANGE_PS);
+	}
+}
+
+static void ieee80211_change_ps(struct ieee80211_local *local)
+{
+	struct ieee80211_conf *conf = &local->hw.conf;
+
+	if (local->ps_sdata) {
+		if (!(local->ps_sdata->u.mgd.flags & IEEE80211_STA_ASSOCIATED))
+			return;
+
+		ieee80211_enable_ps(local, local->ps_sdata);
+	} else if (conf->flags & IEEE80211_CONF_PS) {
+		conf->flags &= ~IEEE80211_CONF_PS;
+		ieee80211_hw_config(local, IEEE80211_CONF_CHANGE_PS);
+		del_timer_sync(&local->dynamic_ps_timer);
+		cancel_work_sync(&local->dynamic_ps_enable_work);
+	}
+}
+
+/* need to hold RTNL or interface lock */
+void ieee80211_recalc_ps(struct ieee80211_local *local)
+{
+	struct ieee80211_sub_if_data *sdata, *found = NULL;
+	int count = 0;
+
+	if (!(local->hw.flags & IEEE80211_HW_SUPPORTS_PS)) {
+		local->ps_sdata = NULL;
+		return;
+	}
+
+	list_for_each_entry(sdata, &local->interfaces, list) {
+		if (!netif_running(sdata->dev))
+			continue;
+		if (sdata->vif.type != NL80211_IFTYPE_STATION)
+			continue;
+		found = sdata;
+		count++;
+	}
+
+	if (count == 1 && found->u.mgd.powersave)
+		local->ps_sdata = found;
+	else
+		local->ps_sdata = NULL;
+
+	ieee80211_change_ps(local);
+}
+
+void ieee80211_dynamic_ps_disable_work(struct work_struct *work)
+{
+	struct ieee80211_local *local =
+		container_of(work, struct ieee80211_local,
+			     dynamic_ps_disable_work);
+
+	if (local->hw.conf.flags & IEEE80211_CONF_PS) {
+		local->hw.conf.flags &= ~IEEE80211_CONF_PS;
+		ieee80211_hw_config(local, IEEE80211_CONF_CHANGE_PS);
+	}
+
+	ieee80211_wake_queues_by_reason(&local->hw,
+					IEEE80211_QUEUE_STOP_REASON_PS);
+}
+
+void ieee80211_dynamic_ps_enable_work(struct work_struct *work)
+{
+	struct ieee80211_local *local =
+		container_of(work, struct ieee80211_local,
+			     dynamic_ps_enable_work);
+	struct ieee80211_sub_if_data *sdata = local->ps_sdata;
+
+	/* can only happen when PS was just disabled anyway */
+	if (!sdata)
+		return;
+
+	if (local->hw.conf.flags & IEEE80211_CONF_PS)
+		return;
+
+	if (local->hw.flags & IEEE80211_HW_PS_NULLFUNC_STACK)
+		ieee80211_send_nullfunc(local, sdata, 1);
+
+	local->hw.conf.flags |= IEEE80211_CONF_PS;
+	ieee80211_hw_config(local, IEEE80211_CONF_CHANGE_PS);
+}
+
+void ieee80211_dynamic_ps_timer(unsigned long data)
+{
+	struct ieee80211_local *local = (void *) data;
+
+	queue_work(local->hw.workqueue, &local->dynamic_ps_enable_work);
+}
+
 /* MLME */
 static void ieee80211_sta_wmm_params(struct ieee80211_local *local,
 				     struct ieee80211_if_managed *ifmgd,
@@ -718,19 +857,9 @@ static void ieee80211_set_associated(str
 	bss_info_changed |= BSS_CHANGED_BASIC_RATES;
 	ieee80211_bss_info_change_notify(sdata, bss_info_changed);
 
-	if (local->powersave) {
-		if (!(local->hw.flags & IEEE80211_HW_SUPPORTS_DYNAMIC_PS) &&
-		    local->hw.conf.dynamic_ps_timeout > 0) {
-			mod_timer(&local->dynamic_ps_timer, jiffies +
-				  msecs_to_jiffies(
-					local->hw.conf.dynamic_ps_timeout));
-		} else {
-			if (local->hw.flags & IEEE80211_HW_PS_NULLFUNC_STACK)
-				ieee80211_send_nullfunc(local, sdata, 1);
-			conf->flags |= IEEE80211_CONF_PS;
-			ieee80211_hw_config(local, IEEE80211_CONF_CHANGE_PS);
-		}
-	}
+	/* will be same as sdata */
+	if (local->ps_sdata)
+		ieee80211_enable_ps(local, sdata);
 
 	netif_tx_start_all_queues(sdata->dev);
 	netif_carrier_on(sdata->dev);
@@ -2184,76 +2313,3 @@ void ieee80211_mlme_notify_scan_complete
 		ieee80211_restart_sta_timer(sdata);
 	rcu_read_unlock();
 }
-
-void ieee80211_dynamic_ps_disable_work(struct work_struct *work)
-{
-	struct ieee80211_local *local =
-		container_of(work, struct ieee80211_local,
-			     dynamic_ps_disable_work);
-
-	if (local->hw.conf.flags & IEEE80211_CONF_PS) {
-		local->hw.conf.flags &= ~IEEE80211_CONF_PS;
-		ieee80211_hw_config(local, IEEE80211_CONF_CHANGE_PS);
-	}
-
-	ieee80211_wake_queues_by_reason(&local->hw,
-					IEEE80211_QUEUE_STOP_REASON_PS);
-}
-
-void ieee80211_dynamic_ps_enable_work(struct work_struct *work)
-{
-	struct ieee80211_local *local =
-		container_of(work, struct ieee80211_local,
-			     dynamic_ps_enable_work);
-	/* XXX: using scan_sdata is completely broken! */
-	struct ieee80211_sub_if_data *sdata = local->scan_sdata;
-
-	if (local->hw.conf.flags & IEEE80211_CONF_PS)
-		return;
-
-	if (local->hw.flags & IEEE80211_HW_PS_NULLFUNC_STACK && sdata)
-		ieee80211_send_nullfunc(local, sdata, 1);
-
-	local->hw.conf.flags |= IEEE80211_CONF_PS;
-	ieee80211_hw_config(local, IEEE80211_CONF_CHANGE_PS);
-}
-
-void ieee80211_dynamic_ps_timer(unsigned long data)
-{
-	struct ieee80211_local *local = (void *) data;
-
-	queue_work(local->hw.workqueue, &local->dynamic_ps_enable_work);
-}
-
-void ieee80211_send_nullfunc(struct ieee80211_local *local,
-			     struct ieee80211_sub_if_data *sdata,
-			     int powersave)
-{
-	struct sk_buff *skb;
-	struct ieee80211_hdr *nullfunc;
-	__le16 fc;
-
-	if (WARN_ON(sdata->vif.type != NL80211_IFTYPE_STATION))
-		return;
-
-	skb = dev_alloc_skb(local->hw.extra_tx_headroom + 24);
-	if (!skb) {
-		printk(KERN_DEBUG "%s: failed to allocate buffer for nullfunc "
-		       "frame\n", sdata->dev->name);
-		return;
-	}
-	skb_reserve(skb, local->hw.extra_tx_headroom);
-
-	nullfunc = (struct ieee80211_hdr *) skb_put(skb, 24);
-	memset(nullfunc, 0, 24);
-	fc = cpu_to_le16(IEEE80211_FTYPE_DATA | IEEE80211_STYPE_NULLFUNC |
-			 IEEE80211_FCTL_TODS);
-	if (powersave)
-		fc |= cpu_to_le16(IEEE80211_FCTL_PM);
-	nullfunc->frame_control = fc;
-	memcpy(nullfunc->addr1, sdata->u.mgd.bssid, ETH_ALEN);
-	memcpy(nullfunc->addr2, sdata->dev->dev_addr, ETH_ALEN);
-	memcpy(nullfunc->addr3, sdata->u.mgd.bssid, ETH_ALEN);
-
-	ieee80211_tx_skb(sdata, skb, 0);
-}
--- wireless-testing.orig/net/mac80211/scan.c	2009-04-16 02:15:54.000000000 +0200
+++ wireless-testing/net/mac80211/scan.c	2009-04-16 02:18:22.000000000 +0200
@@ -253,7 +253,7 @@ static void ieee80211_scan_ps_disable(st
 {
 	struct ieee80211_local *local = sdata->local;
 
-	if (!local->powersave)
+	if (!local->ps_sdata)
 		ieee80211_send_nullfunc(local, sdata, 0);
 	else {
 		/*

-- 

[PATCH 2/4] mac80211: disable powersave if pm_qos asks for low latency

[Johannes Berg]

When an application asks for a latency lower than the beacon interval
there's nothing we can do -- we need to stay awake and not have the
AP buffer frames for us. Add code to automatically calculate this
constraint in mac80211 so drivers need not concern themselves with it.

Signed-off-by: Johannes Berg <johannes@sipsolutions.net>
---
v2: rebased

 include/linux/ieee80211.h  |    9 +++++++++
 net/mac80211/ieee80211_i.h |    5 ++++-
 net/mac80211/iface.c       |    4 ++--
 net/mac80211/main.c        |   31 ++++++++++++++++++++++++-------
 net/mac80211/mlme.c        |   36 ++++++++++++++++++++++++++++++++----
 net/mac80211/wext.c        |    2 +-
 6 files changed, 72 insertions(+), 15 deletions(-)

--- wireless-testing.orig/net/mac80211/mlme.c	2009-04-16 02:18:22.000000000 +0200
+++ wireless-testing/net/mac80211/mlme.c	2009-04-16 02:18:23.000000000 +0200
@@ -17,6 +17,7 @@
 #include <linux/if_arp.h>
 #include <linux/etherdevice.h>
 #include <linux/rtnetlink.h>
+#include <linux/pm_qos_params.h>
 #include <net/mac80211.h>
 #include <asm/unaligned.h>
 
@@ -515,7 +516,7 @@ static void ieee80211_change_ps(struct i
 }
 
 /* need to hold RTNL or interface lock */
-void ieee80211_recalc_ps(struct ieee80211_local *local)
+void ieee80211_recalc_ps(struct ieee80211_local *local, s32 latency)
 {
 	struct ieee80211_sub_if_data *sdata, *found = NULL;
 	int count = 0;
@@ -534,10 +535,22 @@ void ieee80211_recalc_ps(struct ieee8021
 		count++;
 	}
 
-	if (count == 1 && found->u.mgd.powersave)
-		local->ps_sdata = found;
-	else
+	if (count == 1 && found->u.mgd.powersave) {
+		s32 beaconint_us;
+
+		if (latency < 0)
+			latency = pm_qos_requirement(PM_QOS_NETWORK_LATENCY);
+
+		beaconint_us = ieee80211_tu_to_usec(
+					found->vif.bss_conf.beacon_int);
+
+		if (beaconint_us > latency)
+			local->ps_sdata = NULL;
+		else
+			local->ps_sdata = found;
+	} else {
 		local->ps_sdata = NULL;
+	}
 
 	ieee80211_change_ps(local);
 }
@@ -2313,3 +2326,18 @@ void ieee80211_mlme_notify_scan_complete
 		ieee80211_restart_sta_timer(sdata);
 	rcu_read_unlock();
 }
+
+int ieee80211_max_network_latency(struct notifier_block *nb,
+				  unsigned long data, void *dummy)
+{
+	s32 latency_usec = (s32) data;
+	struct ieee80211_local *local =
+		container_of(nb, struct ieee80211_local,
+			     network_latency_notifier);
+
+	mutex_lock(&local->iflist_mtx);
+	ieee80211_recalc_ps(local, latency_usec);
+	mutex_unlock(&local->iflist_mtx);
+
+	return 0;
+}
--- wireless-testing.orig/net/mac80211/ieee80211_i.h	2009-04-16 02:18:22.000000000 +0200
+++ wireless-testing/net/mac80211/ieee80211_i.h	2009-04-16 02:18:23.000000000 +0200
@@ -750,6 +750,7 @@ struct ieee80211_local {
 	struct work_struct dynamic_ps_enable_work;
 	struct work_struct dynamic_ps_disable_work;
 	struct timer_list dynamic_ps_timer;
+	struct notifier_block network_latency_notifier;
 
 	int user_power_level; /* in dBm */
 	int power_constr_level; /* in dBm */
@@ -938,7 +939,9 @@ int ieee80211_sta_deauthenticate(struct 
 int ieee80211_sta_disassociate(struct ieee80211_sub_if_data *sdata, u16 reason);
 void ieee80211_send_pspoll(struct ieee80211_local *local,
 			   struct ieee80211_sub_if_data *sdata);
-void ieee80211_recalc_ps(struct ieee80211_local *local);
+void ieee80211_recalc_ps(struct ieee80211_local *local, s32 latency);
+int ieee80211_max_network_latency(struct notifier_block *nb,
+				  unsigned long data, void *dummy);
 
 /* IBSS code */
 int ieee80211_ibss_commit(struct ieee80211_sub_if_data *sdata);
--- wireless-testing.orig/net/mac80211/main.c	2009-04-16 02:15:54.000000000 +0200
+++ wireless-testing/net/mac80211/main.c	2009-04-16 02:18:23.000000000 +0200
@@ -21,6 +21,7 @@
 #include <linux/wireless.h>
 #include <linux/rtnetlink.h>
 #include <linux/bitmap.h>
+#include <linux/pm_qos_params.h>
 #include <net/net_namespace.h>
 #include <net/cfg80211.h>
 
@@ -1038,25 +1039,38 @@ int ieee80211_register_hw(struct ieee802
 		}
 	}
 
+	local->network_latency_notifier.notifier_call =
+		ieee80211_max_network_latency;
+	result = pm_qos_add_notifier(PM_QOS_NETWORK_LATENCY,
+				     &local->network_latency_notifier);
+
+	if (result) {
+		rtnl_lock();
+		goto fail_pm_qos;
+	}
+
 	return 0;
 
-fail_wep:
+ fail_pm_qos:
+	ieee80211_led_exit(local);
+	ieee80211_remove_interfaces(local);
+ fail_wep:
 	rate_control_deinitialize(local);
-fail_rate:
+ fail_rate:
 	unregister_netdevice(local->mdev);
 	local->mdev = NULL;
-fail_dev:
+ fail_dev:
 	rtnl_unlock();
 	sta_info_stop(local);
-fail_sta_info:
+ fail_sta_info:
 	debugfs_hw_del(local);
 	destroy_workqueue(local->hw.workqueue);
-fail_workqueue:
+ fail_workqueue:
 	if (local->mdev)
 		free_netdev(local->mdev);
-fail_mdev_alloc:
+ fail_mdev_alloc:
 	wiphy_unregister(local->hw.wiphy);
-fail_wiphy_register:
+ fail_wiphy_register:
 	kfree(local->int_scan_req.channels);
 	return result;
 }
@@ -1069,6 +1083,9 @@ void ieee80211_unregister_hw(struct ieee
 	tasklet_kill(&local->tx_pending_tasklet);
 	tasklet_kill(&local->tasklet);
 
+	pm_qos_remove_notifier(PM_QOS_NETWORK_LATENCY,
+			       &local->network_latency_notifier);
+
 	rtnl_lock();
 
 	/*
--- wireless-testing.orig/net/mac80211/iface.c	2009-04-16 02:18:22.000000000 +0200
+++ wireless-testing/net/mac80211/iface.c	2009-04-16 02:18:23.000000000 +0200
@@ -317,7 +317,7 @@ static int ieee80211_open(struct net_dev
 		ieee80211_set_wmm_default(sdata);
 	}
 
-	ieee80211_recalc_ps(local);
+	ieee80211_recalc_ps(local, -1);
 
 	/*
 	 * ieee80211_sta_work is disabled while network interface
@@ -574,7 +574,7 @@ static int ieee80211_stop(struct net_dev
 		hw_reconf_flags = 0;
 	}
 
-	ieee80211_recalc_ps(local);
+	ieee80211_recalc_ps(local, -1);
 
 	/* do after stop to avoid reconfiguring when we stop anyway */
 	if (hw_reconf_flags)
--- wireless-testing.orig/net/mac80211/wext.c	2009-04-16 02:18:22.000000000 +0200
+++ wireless-testing/net/mac80211/wext.c	2009-04-16 02:18:23.000000000 +0200
@@ -789,7 +789,7 @@ static int ieee80211_ioctl_siwpower(stru
 		ieee80211_hw_config(local,
 				    IEEE80211_CONF_CHANGE_DYNPS_TIMEOUT);
 
-	ieee80211_recalc_ps(local);
+	ieee80211_recalc_ps(local, -1);
 
 	return 0;
 }
--- wireless-testing.orig/include/linux/ieee80211.h	2009-04-16 02:15:54.000000000 +0200
+++ wireless-testing/include/linux/ieee80211.h	2009-04-16 02:18:23.000000000 +0200
@@ -1383,4 +1383,13 @@ static inline int ieee80211_freq_to_ofdm
 		return -1;
 }
 
+/**
+ * ieee80211_tu_to_usec - convert time units (TU) to microseconds
+ * @tu: the TUs
+ */
+static inline unsigned long ieee80211_tu_to_usec(unsigned long tu)
+{
+	return 1024 * tu;
+}
+
 #endif /* LINUX_IEEE80211_H */

-- 

[PATCH 3/4] mac80211: implement beacon filtering in software

[Johannes Berg]

Regardless of whether the hardware implements beacon filtering,
there's no need to process all beacons in software all the time
throughout the stack (mac80211 does a lot, then cfg80211, then
in the future possibly userspace).

This patch implements the "best possible" beacon filtering in
mac80211. "Best possible" means that it can look for changes in
all requested information elements, and distinguish vendor IEs
by their OUI.

In the future, we will add nl80211 API for userspace to request
information elements and vendor IE OUIs to watch -- drivers can
then implement the best they can do while software implements
it fully.

It is unclear whether or not this actually saves CPU time, but
the data is all in the cache already so it should be fairly
cheap. The additional _testing_, however, has great benefit;
Without this, and on hardware that doesn't implement beacon
filtering, wrong assumptions about, for example, scan result
updates could quickly creep into code.

Signed-off-by: Johannes Berg <johannes@sipsolutions.net>
---
 net/mac80211/ieee80211_i.h |    5 ++++
 net/mac80211/mlme.c        |   52 ++++++++++++++++++++++++++++++++++-----------
 net/mac80211/util.c        |   23 ++++++++++++++++++-
 3 files changed, 66 insertions(+), 14 deletions(-)

--- wireless-testing.orig/net/mac80211/ieee80211_i.h	2009-04-16 04:51:12.000000000 +0200
+++ wireless-testing/net/mac80211/ieee80211_i.h	2009-04-16 05:01:05.000000000 +0200
@@ -308,6 +308,8 @@ struct ieee80211_if_managed {
 	int auth_alg; /* currently used IEEE 802.11 authentication algorithm */
 	int auth_transaction;
 
+	u32 beacon_crc;
+
 	enum {
 		IEEE80211_MFP_DISABLED,
 		IEEE80211_MFP_OPTIONAL,
@@ -1085,6 +1087,9 @@ void ieee80211_tx_skb(struct ieee80211_s
 		      int encrypt);
 void ieee802_11_parse_elems(u8 *start, size_t len,
 			    struct ieee802_11_elems *elems);
+u32 ieee802_11_parse_elems_crc(u8 *start, size_t len,
+			       struct ieee802_11_elems *elems,
+			       u64 filter, u32 crc);
 int ieee80211_set_freq(struct ieee80211_sub_if_data *sdata, int freq);
 u32 ieee80211_mandatory_rates(struct ieee80211_local *local,
 			      enum ieee80211_band band);
--- wireless-testing.orig/net/mac80211/mlme.c	2009-04-16 04:51:12.000000000 +0200
+++ wireless-testing/net/mac80211/mlme.c	2009-04-16 12:12:08.000000000 +0200
@@ -18,6 +18,7 @@
 #include <linux/etherdevice.h>
 #include <linux/rtnetlink.h>
 #include <linux/pm_qos_params.h>
+#include <linux/crc32.h>
 #include <net/mac80211.h>
 #include <asm/unaligned.h>
 
@@ -1749,46 +1750,73 @@ static void ieee80211_rx_mgmt_probe_resp
 		ifmgd->flags &= ~IEEE80211_STA_PROBEREQ_POLL;
 }
 
+/*
+ * This is the canonical list of information elements we care about,
+ * the filter code also gives us all changes to the Microsoft OUI
+ * (00:50:F2) vendor IE which is used for WMM which we need to track.
+ *
+ * We implement beacon filtering in software since that means we can
+ * avoid processing the frame here and in cfg80211, and userspace
+ * will not be able to tell whether the hardware supports it or not.
+ *
+ * XXX: This list needs to be dynamic -- userspace needs to be able to
+ *	add items it requires. It also needs to be able to tell us to
+ *	look out for other vendor IEs.
+ */
+static const u64 care_about_ies =
+	BIT(WLAN_EID_COUNTRY) |
+	BIT(WLAN_EID_ERP_INFO) |
+	BIT(WLAN_EID_CHANNEL_SWITCH) |
+	BIT(WLAN_EID_PWR_CONSTRAINT) |
+	BIT(WLAN_EID_HT_CAPABILITY) |
+	BIT(WLAN_EID_HT_INFORMATION);
+
 static void ieee80211_rx_mgmt_beacon(struct ieee80211_sub_if_data *sdata,
 				     struct ieee80211_mgmt *mgmt,
 				     size_t len,
 				     struct ieee80211_rx_status *rx_status)
 {
-	struct ieee80211_if_managed *ifmgd;
+	struct ieee80211_if_managed *ifmgd = &sdata->u.mgd;
 	size_t baselen;
 	struct ieee802_11_elems elems;
 	struct ieee80211_local *local = sdata->local;
 	u32 changed = 0;
-	bool erp_valid, directed_tim;
+	bool erp_valid, directed_tim = false;
 	u8 erp_value = 0;
+	u32 ncrc;
 
 	/* Process beacon from the current BSS */
 	baselen = (u8 *) mgmt->u.beacon.variable - (u8 *) mgmt;
 	if (baselen > len)
 		return;
 
-	ieee802_11_parse_elems(mgmt->u.beacon.variable, len - baselen, &elems);
-
-	ieee80211_rx_bss_info(sdata, mgmt, len, rx_status, &elems, true);
-
-	if (sdata->vif.type != NL80211_IFTYPE_STATION)
+	if (rx_status->freq != local->hw.conf.channel->center_freq)
 		return;
 
-	ifmgd = &sdata->u.mgd;
-
 	if (!(ifmgd->flags & IEEE80211_STA_ASSOCIATED) ||
 	    memcmp(ifmgd->bssid, mgmt->bssid, ETH_ALEN) != 0)
 		return;
 
-	if (rx_status->freq != local->hw.conf.channel->center_freq)
+	ncrc = crc32_be(0, (void *)&mgmt->u.beacon.beacon_int, 4);
+	ncrc = ieee802_11_parse_elems_crc(mgmt->u.beacon.variable,
+					  len - baselen, &elems,
+					  care_about_ies, ncrc);
+
+	if (local->hw.flags & IEEE80211_HW_PS_NULLFUNC_STACK)
+		directed_tim = ieee80211_check_tim(&elems, ifmgd->aid);
+
+	ncrc = crc32_be(ncrc, (void *)&directed_tim, sizeof(directed_tim));
+
+	if (ncrc == ifmgd->beacon_crc)
 		return;
+	ifmgd->beacon_crc = ncrc;
+
+	ieee80211_rx_bss_info(sdata, mgmt, len, rx_status, &elems, true);
 
 	ieee80211_sta_wmm_params(local, ifmgd, elems.wmm_param,
 				 elems.wmm_param_len);
 
 	if (local->hw.flags & IEEE80211_HW_PS_NULLFUNC_STACK) {
-		directed_tim = ieee80211_check_tim(&elems, ifmgd->aid);
-
 		if (directed_tim) {
 			if (local->hw.conf.dynamic_ps_timeout > 0) {
 				local->hw.conf.flags &= ~IEEE80211_CONF_PS;
--- wireless-testing.orig/net/mac80211/util.c	2009-04-16 04:51:12.000000000 +0200
+++ wireless-testing/net/mac80211/util.c	2009-04-16 12:19:45.000000000 +0200
@@ -20,6 +20,7 @@
 #include <linux/if_arp.h>
 #include <linux/wireless.h>
 #include <linux/bitmap.h>
+#include <linux/crc32.h>
 #include <net/net_namespace.h>
 #include <net/cfg80211.h>
 #include <net/rtnetlink.h>
@@ -537,8 +538,16 @@ EXPORT_SYMBOL_GPL(ieee80211_iterate_acti
 void ieee802_11_parse_elems(u8 *start, size_t len,
 			    struct ieee802_11_elems *elems)
 {
+	ieee802_11_parse_elems_crc(start, len, elems, 0, 0);
+}
+
+u32 ieee802_11_parse_elems_crc(u8 *start, size_t len,
+			       struct ieee802_11_elems *elems,
+			       u64 filter, u32 crc)
+{
 	size_t left = len;
 	u8 *pos = start;
+	bool calc_crc = filter != 0;
 
 	memset(elems, 0, sizeof(*elems));
 	elems->ie_start = start;
@@ -552,7 +561,10 @@ void ieee802_11_parse_elems(u8 *start, s
 		left -= 2;
 
 		if (elen > left)
-			return;
+			break;
+
+		if (calc_crc && id < 64 && (filter & BIT(id)))
+			crc = crc32_be(crc, pos - 2, elen + 2);
 
 		switch (id) {
 		case WLAN_EID_SSID:
@@ -587,15 +599,20 @@ void ieee802_11_parse_elems(u8 *start, s
 			elems->challenge = pos;
 			elems->challenge_len = elen;
 			break;
-		case WLAN_EID_WPA:
+		case WLAN_EID_VENDOR_SPECIFIC:
 			if (elen >= 4 && pos[0] == 0x00 && pos[1] == 0x50 &&
 			    pos[2] == 0xf2) {
 				/* Microsoft OUI (00:50:F2) */
+
+				if (calc_crc)
+					crc = crc32_be(crc, pos - 2, elen + 2);
+
 				if (pos[3] == 1) {
 					/* OUI Type 1 - WPA IE */
 					elems->wpa = pos;
 					elems->wpa_len = elen;
 				} else if (elen >= 5 && pos[3] == 2) {
+					/* OUI Type 2 - WMM IE */
 					if (pos[4] == 0) {
 						elems->wmm_info = pos;
 						elems->wmm_info_len = elen;
@@ -680,6 +697,8 @@ void ieee802_11_parse_elems(u8 *start, s
 		left -= elen;
 		pos += elen;
 	}
+
+	return crc;
 }
 
 void ieee80211_set_wmm_default(struct ieee80211_sub_if_data *sdata)

-- 

[PATCH 4/4] mac80211: enable PS by default

[Johannes Berg]

Enable PS by default (depending on Kconfig) -- rely on drivers
to control the level using pm_qos. Due to the previous patch
we turn off PS when necessary due to latency requirements.

This has a Kconfig symbol so people can, if they really want,
configure the default in their kernels. We may want to keep it
at "default y" only in wireless-testing for a while.

Signed-off-by: Johannes Berg <johannes@sipsolutions.net>
---
 net/mac80211/Kconfig |   16 ++++++++++++++++
 net/mac80211/mlme.c  |    8 ++++++++
 2 files changed, 24 insertions(+)

--- wireless-testing.orig/net/mac80211/mlme.c	2009-04-16 13:07:02.000000000 +0200
+++ wireless-testing/net/mac80211/mlme.c	2009-04-16 13:15:16.000000000 +0200
@@ -2181,6 +2181,7 @@ static void ieee80211_restart_sta_timer(
 void ieee80211_sta_setup_sdata(struct ieee80211_sub_if_data *sdata)
 {
 	struct ieee80211_if_managed *ifmgd;
+	u32 hw_flags;
 
 	ifmgd = &sdata->u.mgd;
 	INIT_WORK(&ifmgd->work, ieee80211_sta_work);
@@ -2200,6 +2201,13 @@ void ieee80211_sta_setup_sdata(struct ie
 		IEEE80211_STA_AUTO_CHANNEL_SEL;
 	if (sdata->local->hw.queues >= 4)
 		ifmgd->flags |= IEEE80211_STA_WMM_ENABLED;
+
+	hw_flags = sdata->local->hw.flags;
+
+	if (hw_flags & IEEE80211_HW_SUPPORTS_PS) {
+		ifmgd->powersave = CONFIG_MAC80211_DEFAULT_PS_VALUE;
+		local->hw.conf.dynamic_ps_timeout = 500;
+	}
 }
 
 /* configuration hooks */
--- wireless-testing.orig/net/mac80211/Kconfig	2009-04-16 13:06:59.000000000 +0200
+++ wireless-testing/net/mac80211/Kconfig	2009-04-16 13:07:02.000000000 +0200
@@ -11,6 +11,22 @@ config MAC80211
 	  This option enables the hardware independent IEEE 802.11
 	  networking stack.
 
+config MAC80211_DEFAULT_PS
+	bool "enable powersave by default"
+	depends on MAC80211
+	default y
+	help
+	  This option enables powersave mode by default.
+
+	  If this causes your applications to misbehave you should fix your
+	  applications instead -- they need to register their network
+	  latency requirement, see Documentation/power/pm_qos_interface.txt.
+
+config MAC80211_DEFAULT_PS_VALUE
+	int
+	default 1 if MAC80211_DEFAULT_PS
+	default 0
+
 menu "Rate control algorithm selection"
 	depends on MAC80211 != n
 

-- 

[PATCH 4/4 v2] mac80211: enable PS by default

[Johannes Berg]

Enable PS by default (depending on Kconfig) -- rely on drivers
to control the level using pm_qos. Due to the previous patch
we turn off PS when necessary due to latency requirements.

This has a Kconfig symbol so people can, if they really want,
configure the default in their kernels. We may want to keep it
at "default y" only in wireless-testing for a while.

Signed-off-by: Johannes Berg <johannes@sipsolutions.net>
---
v2: oops. forgot to refresh -- compile error

 net/mac80211/Kconfig |   16 ++++++++++++++++
 net/mac80211/mlme.c  |    8 ++++++++
 2 files changed, 24 insertions(+)

--- wireless-testing.orig/net/mac80211/mlme.c	2009-04-16 13:07:02.000000000 +0200
+++ wireless-testing/net/mac80211/mlme.c	2009-04-16 13:26:52.000000000 +0200
@@ -2181,6 +2181,7 @@ static void ieee80211_restart_sta_timer(
 void ieee80211_sta_setup_sdata(struct ieee80211_sub_if_data *sdata)
 {
 	struct ieee80211_if_managed *ifmgd;
+	u32 hw_flags;
 
 	ifmgd = &sdata->u.mgd;
 	INIT_WORK(&ifmgd->work, ieee80211_sta_work);
@@ -2200,6 +2201,13 @@ void ieee80211_sta_setup_sdata(struct ie
 		IEEE80211_STA_AUTO_CHANNEL_SEL;
 	if (sdata->local->hw.queues >= 4)
 		ifmgd->flags |= IEEE80211_STA_WMM_ENABLED;
+
+	hw_flags = sdata->local->hw.flags;
+
+	if (hw_flags & IEEE80211_HW_SUPPORTS_PS) {
+		ifmgd->powersave = CONFIG_MAC80211_DEFAULT_PS_VALUE;
+		sdata->local->hw.conf.dynamic_ps_timeout = 500;
+	}
 }
 
 /* configuration hooks */
--- wireless-testing.orig/net/mac80211/Kconfig	2009-04-16 13:06:59.000000000 +0200
+++ wireless-testing/net/mac80211/Kconfig	2009-04-16 13:07:02.000000000 +0200
@@ -11,6 +11,22 @@ config MAC80211
 	  This option enables the hardware independent IEEE 802.11
 	  networking stack.
 
+config MAC80211_DEFAULT_PS
+	bool "enable powersave by default"
+	depends on MAC80211
+	default y
+	help
+	  This option enables powersave mode by default.
+
+	  If this causes your applications to misbehave you should fix your
+	  applications instead -- they need to register their network
+	  latency requirement, see Documentation/power/pm_qos_interface.txt.
+
+config MAC80211_DEFAULT_PS_VALUE
+	int
+	default 1 if MAC80211_DEFAULT_PS
+	default 0
+
 menu "Rate control algorithm selection"
 	depends on MAC80211 != n
 

Re: [PATCH 0/4] mac80211 powersave work

[John W. Linville]

On Thu, Apr 16, 2009 at 01:17:23PM +0200, Johannes Berg wrote:
> This is pretty much unchanged from my previous v2 RFCs,
> except that I added a default of 500ms for the dynamic
> powersave timer.
> 
> The fourth patch is new, it adds beacon filtering to
> mac80211 -- for the reasons explained in that patch.
> 
> For 2.6.31, we should make MAC80211_DEFAULT_PS default
> off instead. John, how do you want to handle that?

I'm not sure I understand.  Your patch (which would go to 2.6.31)
has it turned-on by default.  Which way do you want it?

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

Re: [PATCH 0/4] mac80211 powersave work

[Johannes Berg]

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

On Mon, 2009-04-20 at 15:44 -0400, John W. Linville wrote:
> On Thu, Apr 16, 2009 at 01:17:23PM +0200, Johannes Berg wrote:
> > This is pretty much unchanged from my previous v2 RFCs,
> > except that I added a default of 500ms for the dynamic
> > powersave timer.
> > 
> > The fourth patch is new, it adds beacon filtering to
> > mac80211 -- for the reasons explained in that patch.
> > 
> > For 2.6.31, we should make MAC80211_DEFAULT_PS default
> > off instead. John, how do you want to handle that?
> 
> I'm not sure I understand.  Your patch (which would go to 2.6.31)
> has it turned-on by default.  Which way do you want it?

I want it turned on by default, at least in wireless-testing; Kalle
thinks that we should turn it off for a .31 release.

I would think that since an easy workaround is available ("iwconfig
wlan0 power off") and it doesn't affect most hardware yet anyway (only
those supporting powersave) we should turn it on by default anyway,
since otherwise we won't find any bugs -- but maybe that's just me.

johannes

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

Re: [PATCH 0/4] mac80211 powersave work

[Davide Pesavento]

On Mon, Apr 20, 2009 at 21:53, Johannes Berg <johannes@sipsolutions.net=
> wrote:
> On Mon, 2009-04-20 at 15:44 -0400, John W. Linville wrote:
>> On Thu, Apr 16, 2009 at 01:17:23PM +0200, Johannes Berg wrote:
>> > This is pretty much unchanged from my previous v2 RFCs,
>> > except that I added a default of 500ms for the dynamic
>> > powersave timer.
>> >
>> > The fourth patch is new, it adds beacon filtering to
>> > mac80211 -- for the reasons explained in that patch.
>> >
>> > For 2.6.31, we should make MAC80211_DEFAULT_PS default
>> > off instead. John, how do you want to handle that?
>>
>> I'm not sure I understand. =C2=A0Your patch (which would go to 2.6.3=
1)
>> has it turned-on by default. =C2=A0Which way do you want it?
>
> I want it turned on by default, at least in wireless-testing; Kalle
> thinks that we should turn it off for a .31 release.
>
> I would think that since an easy workaround is available ("iwconfig
> wlan0 power off") and it doesn't affect most hardware yet anyway (onl=
y
> those supporting powersave) we should turn it on by default anyway,
> since otherwise we won't find any bugs -- but maybe that's just me.
>

As I've already reported [1][2], ath9k is currently broken when power
saving is enabled, at least on AR5418 hardware. So please don't turn
it on by default yet.

Thanks,
Davide

[1] http://marc.info/?l=3Dlinux-wireless&m=3D123816406913599&w=3D2
[2] http://marc.info/?l=3Dlinux-wireless&m=3D123999864507104&w=3D2
--
To unsubscribe from this list: send the line "unsubscribe linux-wireles=
s" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 0/4] mac80211 powersave work

[Johannes Berg]

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

On Mon, 2009-04-20 at 22:26 +0200, Davide Pesavento wrote:

> As I've already reported [1][2], ath9k is currently broken when power
> saving is enabled, at least on AR5418 hardware. So please don't turn
> it on by default yet.

But if that's just a hardware/driver bug shouldn't the driver disable PS
for that combination? It seems wrong to disable PS in the stack by
default just because one driver has a problem with it, when that driver
could just unset the PS support bit.

johannes

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

Re: [PATCH 0/4] mac80211 powersave work

[Davide Pesavento]

On Mon, Apr 20, 2009 at 22:30, Johannes Berg <johannes@sipsolutions.net> wrote:
> On Mon, 2009-04-20 at 22:26 +0200, Davide Pesavento wrote:
>
>> As I've already reported [1][2], ath9k is currently broken when power
>> saving is enabled, at least on AR5418 hardware. So please don't turn
>> it on by default yet.
>
> But if that's just a hardware/driver bug shouldn't the driver disable PS
> for that combination? It seems wrong to disable PS in the stack by
> default just because one driver has a problem with it, when that driver
> could just unset the PS support bit.
>

You're right. It makes perfect sense.
Unfortunately I don't know how to do it... :-(

Regards,
Davide

Re: [PATCH 0/4] mac80211 powersave work

[Johannes Berg]

On Mon, 2009-04-20 at 23:19 +0200, Davide Pesavento wrote:

> >> As I've already reported [1][2], ath9k is currently broken when power
> >> saving is enabled, at least on AR5418 hardware. So please don't turn
> >> it on by default yet.
> >
> > But if that's just a hardware/driver bug shouldn't the driver disable PS
> > for that combination? It seems wrong to disable PS in the stack by
> > default just because one driver has a problem with it, when that driver
> > could just unset the PS support bit.
> >
> 
> You're right. It makes perfect sense.
> Unfortunately I don't know how to do it... :-(

Something like this.

johannes

--- wireless-testing.orig/drivers/net/wireless/ath/ath9k/main.c	2009-04-20 23:27:15.000000000 +0200
+++ wireless-testing/drivers/net/wireless/ath/ath9k/main.c	2009-04-20 23:27:54.000000000 +0200
@@ -1537,8 +1537,6 @@ void ath_set_hw_capab(struct ath_softc *
 		IEEE80211_HW_HOST_BROADCAST_PS_BUFFERING |
 		IEEE80211_HW_SIGNAL_DBM |
 		IEEE80211_HW_AMPDU_AGGREGATION |
-		IEEE80211_HW_SUPPORTS_PS |
-		IEEE80211_HW_PS_NULLFUNC_STACK |
 		IEEE80211_HW_SPECTRUM_MGMT;
 
 	if (AR_SREV_9160_10_OR_LATER(sc->sc_ah) || modparam_nohwcrypt)

Re: [PATCH 0/4] mac80211 powersave work

[Fabio Rossi]

On Monday 20 April 2009, Johannes Berg wrote:

> But if that's just a hardware/driver bug shouldn't the driver disable PS
> for that combination? It seems wrong to disable PS in the stack by
> default just because one driver has a problem with it, when that driver
> could just unset the PS support bit.

I think that a printk notice message could be useful to state that the 
powersave management is active (of course with DEBUG on, at least during the 
first larger-user-base test period).

Fabio

Re: [PATCH 0/4] mac80211 powersave work

[John W. Linville]

On Mon, Apr 20, 2009 at 09:53:08PM +0200, Johannes Berg wrote:
> On Mon, 2009-04-20 at 15:44 -0400, John W. Linville wrote:
> > On Thu, Apr 16, 2009 at 01:17:23PM +0200, Johannes Berg wrote:
> > > This is pretty much unchanged from my previous v2 RFCs,
> > > except that I added a default of 500ms for the dynamic
> > > powersave timer.
> > > 
> > > The fourth patch is new, it adds beacon filtering to
> > > mac80211 -- for the reasons explained in that patch.
> > > 
> > > For 2.6.31, we should make MAC80211_DEFAULT_PS default
> > > off instead. John, how do you want to handle that?
> > 
> > I'm not sure I understand.  Your patch (which would go to 2.6.31)
> > has it turned-on by default.  Which way do you want it?
> 
> I want it turned on by default, at least in wireless-testing; Kalle
> thinks that we should turn it off for a .31 release.
> 
> I would think that since an easy workaround is available ("iwconfig
> wlan0 power off") and it doesn't affect most hardware yet anyway (only
> those supporting powersave) we should turn it on by default anyway,
> since otherwise we won't find any bugs -- but maybe that's just me.

Ah, OK -- let's leave it on for now and if it is a problem we can
turn it off in 2.6.31 during/after 2.6.31-rc1.

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

Re: [PATCH 0/4] mac80211 powersave work

[Kalle Valo]

Johannes Berg <johannes@sipsolutions.net> writes:

>> I'm not sure I understand.  Your patch (which would go to 2.6.31)
>> has it turned-on by default.  Which way do you want it?
>
> I want it turned on by default, at least in wireless-testing; Kalle
> thinks that we should turn it off for a .31 release.
>
> I would think that since an easy workaround is available ("iwconfig
> wlan0 power off") and it doesn't affect most hardware yet anyway (only
> those supporting powersave) we should turn it on by default anyway,
> since otherwise we won't find any bugs -- but maybe that's just me.

Yes, in my opinion the power save should not be enabled by default for
the big masses, at least not yet. My understanding is that only few
people have tested it and I would like to see more testing first. 

But if you (Johannes&John) think it's ready for the prime time, I'm not
going to complain :)

-- 
Kalle Valo

Re: [PATCH 0/4] mac80211 powersave work

[John W. Linville]

On Tue, Apr 21, 2009 at 08:24:28AM +0300, Kalle Valo wrote:
> Johannes Berg <johannes@sipsolutions.net> writes:
> 
> >> I'm not sure I understand.  Your patch (which would go to 2.6.31)
> >> has it turned-on by default.  Which way do you want it?
> >
> > I want it turned on by default, at least in wireless-testing; Kalle
> > thinks that we should turn it off for a .31 release.
> >
> > I would think that since an easy workaround is available ("iwconfig
> > wlan0 power off") and it doesn't affect most hardware yet anyway (only
> > those supporting powersave) we should turn it on by default anyway,
> > since otherwise we won't find any bugs -- but maybe that's just me.
> 
> Yes, in my opinion the power save should not be enabled by default for
> the big masses, at least not yet. My understanding is that only few
> people have tested it and I would like to see more testing first. 
> 
> But if you (Johannes&John) think it's ready for the prime time, I'm not
> going to complain :)

Well there is always a trade-off.  But by the time 2.6.31 come around
the code will have been in wireless-testing, net-next, and linux-next
for a reasonably long time.  Hopefully we will have good confidence
in it by then.  If not, then we can disable it by default later.

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

Re: [PATCH 0/4] mac80211 powersave work

[Kalle Valo]

"John W. Linville" <linville@tuxdriver.com> writes:

>> Yes, in my opinion the power save should not be enabled by default for
>> the big masses, at least not yet. My understanding is that only few
>> people have tested it and I would like to see more testing first. 
>> 
>> But if you (Johannes&John) think it's ready for the prime time, I'm not
>> going to complain :)
>
> Well there is always a trade-off.  But by the time 2.6.31 come around
> the code will have been in wireless-testing, net-next, and linux-next
> for a reasonably long time.  Hopefully we will have good confidence
> in it by then.  If not, then we can disable it by default later.

That sounds good. With those three trees there should be quite a lot of
testers.

-- 
Kalle Valo