2010-02-17 15:58:29

by Kalle Valo

[permalink] [raw]
Subject: [PATCH v2] nl80211: add power save commands

From: Kalle Valo <[email protected]>

The most needed command from nl80211, which Wireless Extensions had,
is support for power save mode. Add a simple command to make it possible
to enable and disable power save via nl80211.

I was also planning about extending the interface, for example adding the
timeout value, but after thinking more about this I decided not to do it.
Basically there were three reasons:

Firstly, the parameters for power save are very much hardware dependent.
Trying to find a unified interface which would work with all hardware, and
still make sense to users, will be very difficult.

Secondly, IEEE 802.11 power save implementation in Linux is still in state
of flux. We have a long way to still to go and there is no way to predict
what kind of implementation we will have after few years. And because we
need to support nl80211 interface a long time, practically forever, adding
now parameters to nl80211 might create maintenance problems later on.

Third issue are the users. Power save parameters are mostly used for
debugging, so debugfs is better, more flexible, interface for this.
For example, wpa_supplicant currently doesn't configure anything related
to power save mode. It's better to strive that kernel can automatically
optimise the power save parameters, like with help of pm qos network
and other traffic parameters.

Later on, when we have better understanding of power save, we can extend
this command with more features, if there's a need for that.

Signed-off-by: Kalle Valo <[email protected]>
---
include/linux/nl80211.h | 10 +++
include/net/cfg80211.h | 7 +-
net/wireless/core.c | 16 +++--
net/wireless/nl80211.c | 131 ++++++++++++++++++++++++++++++++++++++++++++
net/wireless/wext-compat.c | 10 ++-
5 files changed, 159 insertions(+), 15 deletions(-)

diff --git a/include/linux/nl80211.h b/include/linux/nl80211.h
index 8e6384f..28ba20f 100644
--- a/include/linux/nl80211.h
+++ b/include/linux/nl80211.h
@@ -416,6 +416,9 @@ enum nl80211_commands {
NL80211_CMD_ACTION,
NL80211_CMD_ACTION_TX_STATUS,

+ NL80211_CMD_SET_POWER_SAVE,
+ NL80211_CMD_GET_POWER_SAVE,
+
/* add new commands above here */

/* used to define NL80211_CMD_MAX below */
@@ -837,6 +840,8 @@ enum nl80211_attrs {

NL80211_ATTR_ACK,

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

__NL80211_ATTR_AFTER_LAST,
@@ -1573,4 +1578,9 @@ enum nl80211_band {
NL80211_BAND_5GHZ,
};

+enum nl80211_ps_state {
+ NL80211_PS_DISABLED,
+ NL80211_PS_ENABLED,
+};
+
#endif /* __LINUX_NL80211_H */
diff --git a/include/net/cfg80211.h b/include/net/cfg80211.h
index 7188934..3d134a1 100644
--- a/include/net/cfg80211.h
+++ b/include/net/cfg80211.h
@@ -1150,7 +1150,6 @@ struct cfg80211_ops {
enum nl80211_channel_type channel_type,
const u8 *buf, size_t len, u64 *cookie);

- /* some temporary stuff to finish wext */
int (*set_power_mgmt)(struct wiphy *wiphy, struct net_device *dev,
bool enabled, int timeout);
};
@@ -1489,6 +1488,9 @@ struct wireless_dev {
struct cfg80211_internal_bss *auth_bsses[MAX_AUTH_BSSES];
struct cfg80211_internal_bss *current_bss; /* associated / joined */

+ bool ps;
+ int ps_timeout;
+
#ifdef CONFIG_CFG80211_WEXT
/* wext data */
struct {
@@ -1500,8 +1502,7 @@ struct wireless_dev {
u8 bssid[ETH_ALEN], prev_bssid[ETH_ALEN];
u8 ssid[IEEE80211_MAX_SSID_LEN];
s8 default_key, default_mgmt_key;
- bool ps, prev_bssid_valid;
- int ps_timeout;
+ bool prev_bssid_valid;
} wext;
#endif
};
diff --git a/net/wireless/core.c b/net/wireless/core.c
index 51908dc..7fdb940 100644
--- a/net/wireless/core.c
+++ b/net/wireless/core.c
@@ -698,19 +698,21 @@ static int cfg80211_netdev_notifier_call(struct notifier_block * nb,
wdev->wext.default_key = -1;
wdev->wext.default_mgmt_key = -1;
wdev->wext.connect.auth_type = NL80211_AUTHTYPE_AUTOMATIC;
+#endif
+
if (wdev->wiphy->flags & WIPHY_FLAG_PS_ON_BY_DEFAULT)
- wdev->wext.ps = true;
+ wdev->ps = true;
else
- wdev->wext.ps = false;
- wdev->wext.ps_timeout = 100;
+ wdev->ps = false;
+ wdev->ps_timeout = 100;
if (rdev->ops->set_power_mgmt)
if (rdev->ops->set_power_mgmt(wdev->wiphy, dev,
- wdev->wext.ps,
- wdev->wext.ps_timeout)) {
+ wdev->ps,
+ wdev->ps_timeout)) {
/* assume this means it's off */
- wdev->wext.ps = false;
+ wdev->ps = false;
}
-#endif
+
if (!dev->ethtool_ops)
dev->ethtool_ops = &cfg80211_ethtool_ops;

diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c
index 3281120..b0495a1 100644
--- a/net/wireless/nl80211.c
+++ b/net/wireless/nl80211.c
@@ -148,6 +148,7 @@ static struct nla_policy nl80211_policy[NL80211_ATTR_MAX+1] __read_mostly = {
[NL80211_ATTR_FRAME] = { .type = NLA_BINARY,
.len = IEEE80211_MAX_DATA_LEN },
[NL80211_ATTR_FRAME_MATCH] = { .type = NLA_BINARY, },
+ [NL80211_ATTR_PS_STATE] = { .type = NLA_U32 },
};

/* policy for the attributes */
@@ -4663,6 +4664,124 @@ unlock_rtnl:
return err;
}

+static int nl80211_set_power_save(struct sk_buff *skb, struct genl_info *info)
+{
+ struct cfg80211_registered_device *rdev;
+ struct wireless_dev *wdev;
+ struct net_device *dev;
+ u8 ps_state;
+ bool state;
+ int err;
+
+ if (!info->attrs[NL80211_ATTR_PS_STATE]) {
+ err = -EINVAL;
+ goto out;
+ }
+
+ ps_state = nla_get_u32(info->attrs[NL80211_ATTR_PS_STATE]);
+
+ if (ps_state != NL80211_PS_DISABLED && ps_state != NL80211_PS_ENABLED) {
+ err = -EINVAL;
+ goto out;
+ }
+
+ rtnl_lock();
+
+ err = get_rdev_dev_by_info_ifindex(info, &rdev, &dev);
+ if (err)
+ goto unlock_rdev;
+
+ wdev = dev->ieee80211_ptr;
+
+ if (!rdev->ops->set_power_mgmt) {
+ err = -EOPNOTSUPP;
+ goto unlock_rdev;
+ }
+
+ state = (ps_state == NL80211_PS_ENABLED) ? true : false;
+
+ if (state == wdev->ps)
+ goto unlock_rdev;
+
+ wdev->ps = state;
+
+ if (rdev->ops->set_power_mgmt(wdev->wiphy, dev, wdev->ps,
+ wdev->ps_timeout))
+ /* assume this means it's off */
+ wdev->ps = false;
+
+unlock_rdev:
+ cfg80211_unlock_rdev(rdev);
+ dev_put(dev);
+ rtnl_unlock();
+
+out:
+ return err;
+}
+
+static int nl80211_get_power_save(struct sk_buff *skb, struct genl_info *info)
+{
+ struct cfg80211_registered_device *rdev;
+ enum nl80211_ps_state ps_state;
+ struct wireless_dev *wdev;
+ struct net_device *dev;
+ struct sk_buff *msg;
+ void *hdr;
+ int err;
+
+ rtnl_lock();
+
+ err = get_rdev_dev_by_info_ifindex(info, &rdev, &dev);
+ if (err)
+ goto unlock_rtnl;
+
+ wdev = dev->ieee80211_ptr;
+
+ if (!rdev->ops->set_power_mgmt) {
+ err = -EOPNOTSUPP;
+ goto out;
+ }
+
+ msg = nlmsg_new(NLMSG_DEFAULT_SIZE, GFP_KERNEL);
+ if (!msg) {
+ err = -ENOMEM;
+ goto out;
+ }
+
+ hdr = nl80211hdr_put(msg, info->snd_pid, info->snd_seq, 0,
+ NL80211_CMD_GET_POWER_SAVE);
+ if (!hdr) {
+ err = -ENOMEM;
+ goto free_msg;
+ }
+
+ if (wdev->ps)
+ ps_state = NL80211_PS_ENABLED;
+ else
+ ps_state = NL80211_PS_DISABLED;
+
+ NLA_PUT_U32(msg, NL80211_ATTR_PS_STATE, ps_state);
+
+ genlmsg_end(msg, hdr);
+ err = genlmsg_reply(msg, info);
+ goto out;
+
+nla_put_failure:
+ err = -ENOBUFS;
+
+free_msg:
+ nlmsg_free(msg);
+
+out:
+ cfg80211_unlock_rdev(rdev);
+ dev_put(dev);
+
+unlock_rtnl:
+ rtnl_unlock();
+
+ return err;
+}
+
static struct genl_ops nl80211_ops[] = {
{
.cmd = NL80211_CMD_GET_WIPHY,
@@ -4955,6 +5074,18 @@ static struct genl_ops nl80211_ops[] = {
.policy = nl80211_policy,
.flags = GENL_ADMIN_PERM,
},
+ {
+ .cmd = NL80211_CMD_SET_POWER_SAVE,
+ .doit = nl80211_set_power_save,
+ .policy = nl80211_policy,
+ .flags = GENL_ADMIN_PERM,
+ },
+ {
+ .cmd = NL80211_CMD_GET_POWER_SAVE,
+ .doit = nl80211_get_power_save,
+ .policy = nl80211_policy,
+ /* can be retrieved by unprivileged users */
+ },
};

static struct genl_multicast_group nl80211_mlme_mcgrp = {
diff --git a/net/wireless/wext-compat.c b/net/wireless/wext-compat.c
index b17eeae..9ab5183 100644
--- a/net/wireless/wext-compat.c
+++ b/net/wireless/wext-compat.c
@@ -1099,8 +1099,8 @@ int cfg80211_wext_siwpower(struct net_device *dev,
{
struct wireless_dev *wdev = dev->ieee80211_ptr;
struct cfg80211_registered_device *rdev = wiphy_to_dev(wdev->wiphy);
- bool ps = wdev->wext.ps;
- int timeout = wdev->wext.ps_timeout;
+ bool ps = wdev->ps;
+ int timeout = wdev->ps_timeout;
int err;

if (wdev->iftype != NL80211_IFTYPE_STATION)
@@ -1133,8 +1133,8 @@ int cfg80211_wext_siwpower(struct net_device *dev,
if (err)
return err;

- wdev->wext.ps = ps;
- wdev->wext.ps_timeout = timeout;
+ wdev->ps = ps;
+ wdev->ps_timeout = timeout;

return 0;

@@ -1147,7 +1147,7 @@ int cfg80211_wext_giwpower(struct net_device *dev,
{
struct wireless_dev *wdev = dev->ieee80211_ptr;

- wrq->disabled = !wdev->wext.ps;
+ wrq->disabled = !wdev->ps;

return 0;
}



2010-02-17 16:06:05

by Kalle Valo

[permalink] [raw]
Subject: [PATCH v2 2/3] iw: update nl80211.h to include power save commands


---
nl80211.h | 10 ++++++++++
1 files changed, 10 insertions(+), 0 deletions(-)

diff --git a/nl80211.h b/nl80211.h
index 8e6384f..28ba20f 100644
--- a/nl80211.h
+++ b/nl80211.h
@@ -416,6 +416,9 @@ enum nl80211_commands {
NL80211_CMD_ACTION,
NL80211_CMD_ACTION_TX_STATUS,

+ NL80211_CMD_SET_POWER_SAVE,
+ NL80211_CMD_GET_POWER_SAVE,
+
/* add new commands above here */

/* used to define NL80211_CMD_MAX below */
@@ -837,6 +840,8 @@ enum nl80211_attrs {

NL80211_ATTR_ACK,

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

__NL80211_ATTR_AFTER_LAST,
@@ -1573,4 +1578,9 @@ enum nl80211_band {
NL80211_BAND_5GHZ,
};

+enum nl80211_ps_state {
+ NL80211_PS_DISABLED,
+ NL80211_PS_ENABLED,
+};
+
#endif /* __LINUX_NL80211_H */


2010-02-17 22:32:49

by Luis R. Rodriguez

[permalink] [raw]
Subject: Re: [PATCH v2] nl80211: add power save commands

T24gV2VkLCBGZWIgMTcsIDIwMTAgYXQgNzo1OCBBTSwgS2FsbGUgVmFsbyA8a2FsbGUudmFsb0Bp
a2kuZmk+IHdyb3RlOgo+ICsgwqAgwqAgwqAgewo+ICsgwqAgwqAgwqAgwqAgwqAgwqAgwqAgLmNt
ZCA9IE5MODAyMTFfQ01EX1NFVF9QT1dFUl9TQVZFLAo+ICsgwqAgwqAgwqAgwqAgwqAgwqAgwqAg
LmRvaXQgPSBubDgwMjExX3NldF9wb3dlcl9zYXZlLAo+ICsgwqAgwqAgwqAgwqAgwqAgwqAgwqAg
LnBvbGljeSA9IG5sODAyMTFfcG9saWN5LAo+ICsgwqAgwqAgwqAgwqAgwqAgwqAgwqAgLmZsYWdz
ID0gR0VOTF9BRE1JTl9QRVJNLAo+ICsgwqAgwqAgwqAgfSwKPiArIMKgIMKgIMKgIHsKPiArIMKg
IMKgIMKgIMKgIMKgIMKgIMKgIC5jbWQgPSBOTDgwMjExX0NNRF9HRVRfUE9XRVJfU0FWRSwKPiAr
IMKgIMKgIMKgIMKgIMKgIMKgIMKgIC5kb2l0ID0gbmw4MDIxMV9nZXRfcG93ZXJfc2F2ZSwKPiAr
IMKgIMKgIMKgIMKgIMKgIMKgIMKgIC5wb2xpY3kgPSBubDgwMjExX3BvbGljeSwKPiArIMKgIMKg
IMKgIMKgIMKgIMKgIMKgIC8qIGNhbiBiZSByZXRyaWV2ZWQgYnkgdW5wcml2aWxlZ2VkIHVzZXJz
ICovCj4gKyDCoCDCoCDCoCB9LAoKQW55IHJlYXNvbiB0aGlzIGNhbid0IGJlIHN0dWZmZWQgaW50
byB0aGUgc2V0L2dldCB3aXBoeSBjb21tYW5kPwoKICBMdWlzCg==

2010-02-17 16:06:05

by Kalle Valo

[permalink] [raw]
Subject: [PATCH v2 1/3] iw: update nl80211.h to include the new action commands


---
nl80211.h | 41 ++++++++++++++++++++++++++++++++++++++++-
1 files changed, 40 insertions(+), 1 deletions(-)

diff --git a/nl80211.h b/nl80211.h
index 127a730..8e6384f 100644
--- a/nl80211.h
+++ b/nl80211.h
@@ -3,7 +3,7 @@
/*
* 802.11 netlink interface public header
*
- * Copyright 2006, 2007, 2008 Johannes Berg <[email protected]>
+ * Copyright 2006-2010 Johannes Berg <[email protected]>
* Copyright 2008 Michael Wu <[email protected]>
* Copyright 2008 Luis Carlos Cobo <[email protected]>
* Copyright 2008 Michael Buesch <[email protected]>
@@ -299,6 +299,31 @@
* rate selection. %NL80211_ATTR_IFINDEX is used to specify the interface
* and @NL80211_ATTR_TX_RATES the set of allowed rates.
*
+ * @NL80211_CMD_REGISTER_ACTION: Register for receiving certain action frames
+ * (via @NL80211_CMD_ACTION) for processing in userspace. This command
+ * requires an interface index and a match attribute containing the first
+ * few bytes of the frame that should match, e.g. a single byte for only
+ * a category match or four bytes for vendor frames including the OUI.
+ * The registration cannot be dropped, but is removed automatically
+ * when the netlink socket is closed. Multiple registrations can be made.
+ * @NL80211_CMD_ACTION: Action frame TX request and RX notification. This
+ * command is used both as a request to transmit an Action frame and as an
+ * event indicating reception of an Action frame that was not processed in
+ * kernel code, but is for us (i.e., which may need to be processed in a
+ * user space application). %NL80211_ATTR_FRAME is used to specify the
+ * frame contents (including header). %NL80211_ATTR_WIPHY_FREQ (and
+ * optionally %NL80211_ATTR_WIPHY_CHANNEL_TYPE) is used to indicate on
+ * which channel the frame is to be transmitted or was received. This
+ * channel has to be the current channel (remain-on-channel or the
+ * operational channel). When called, this operation returns a cookie
+ * (%NL80211_ATTR_COOKIE) that will be included with the TX status event
+ * pertaining to the TX request.
+ * @NL80211_CMD_ACTION_TX_STATUS: Report TX status of an Action frame
+ * transmitted with %NL80211_CMD_ACTION. %NL80211_ATTR_COOKIE identifies
+ * the TX command and %NL80211_ATTR_FRAME includes the contents of the
+ * frame. %NL80211_ATTR_ACK flag is included if the recipient acknowledged
+ * the frame.
+ *
* @NL80211_CMD_MAX: highest used command number
* @__NL80211_CMD_AFTER_LAST: internal use
*/
@@ -387,6 +412,10 @@ enum nl80211_commands {

NL80211_CMD_SET_TX_BITRATE_MASK,

+ NL80211_CMD_REGISTER_ACTION,
+ NL80211_CMD_ACTION,
+ NL80211_CMD_ACTION_TX_STATUS,
+
/* add new commands above here */

/* used to define NL80211_CMD_MAX below */
@@ -653,6 +682,12 @@ enum nl80211_commands {
* rates based on negotiated supported rates information. This attribute
* is used with %NL80211_CMD_SET_TX_BITRATE_MASK.
*
+ * @NL80211_ATTR_FRAME_MATCH: A binary attribute which typically must contain
+ * at least one byte, currently used with @NL80211_CMD_REGISTER_ACTION.
+ *
+ * @NL80211_ATTR_ACK: Flag attribute indicating that the frame was
+ * acknowledged by the recipient.
+ *
* @NL80211_ATTR_MAX: highest attribute number currently defined
* @__NL80211_ATTR_AFTER_LAST: internal use
*/
@@ -798,6 +833,10 @@ enum nl80211_attrs {

NL80211_ATTR_TX_RATES,

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

__NL80211_ATTR_AFTER_LAST,


2010-02-18 08:11:38

by Johannes Berg

[permalink] [raw]
Subject: Re: [PATCH v2] nl80211: add power save commands

On Wed, 2010-02-17 at 14:32 -0800, Luis R. Rodriguez wrote:
> On Wed, Feb 17, 2010 at 7:58 AM, Kalle Valo <[email protected]> wrote:
> > + {
> > + .cmd = NL80211_CMD_SET_POWER_SAVE,
> > + .doit = nl80211_set_power_save,
> > + .policy = nl80211_policy,
> > + .flags = GENL_ADMIN_PERM,
> > + },
> > + {
> > + .cmd = NL80211_CMD_GET_POWER_SAVE,
> > + .doit = nl80211_get_power_save,
> > + .policy = nl80211_policy,
> > + /* can be retrieved by unprivileged users */
> > + },
>
> Any reason this can't be stuffed into the set/get wiphy command?

per netdev.

johannes


Attachments:
signature.asc (801.00 B)
This is a digitally signed message part

2010-02-17 16:06:02

by Kalle Valo

[permalink] [raw]
Subject: [PATCH v2 3/3] iw: add set/get power_save commands


---
Makefile | 2 +
ps.c | 83 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 84 insertions(+), 1 deletions(-)
create mode 100644 ps.c

diff --git a/Makefile b/Makefile
index bae1bfb..b0a4278 100644
--- a/Makefile
+++ b/Makefile
@@ -17,7 +17,7 @@ CFLAGS += -Wall -Wundef -Wstrict-prototypes -Wno-trigraphs -fno-strict-aliasing
OBJS = iw.o genl.o event.o info.o phy.o \
interface.o ibss.o station.o survey.o util.o \
mesh.o mpath.o scan.o reg.o version.o \
- reason.o status.o connect.o link.o offch.o
+ reason.o status.o connect.o link.o offch.o ps.o
OBJS += sections.o
ALL = iw

diff --git a/ps.c b/ps.c
new file mode 100644
index 0000000..6feeeb9
--- /dev/null
+++ b/ps.c
@@ -0,0 +1,83 @@
+#include <errno.h>
+#include <string.h>
+
+#include <netlink/genl/genl.h>
+#include <netlink/msg.h>
+#include <netlink/attr.h>
+
+#include "nl80211.h"
+#include "iw.h"
+
+static int set_power_save(struct nl80211_state *state,
+ struct nl_cb *cb,
+ struct nl_msg *msg,
+ int argc, char **argv)
+{
+ enum nl80211_ps_state ps_state;
+
+ if (argc != 1) {
+ printf("Invalid parameters!\n");
+ return 2;
+ }
+
+ if (strcmp(argv[0], "on") == 0)
+ ps_state = NL80211_PS_ENABLED;
+ else if (strcmp(argv[0], "off") == 0)
+ ps_state = NL80211_PS_DISABLED;
+ else {
+ printf("Invalid parameter: %s\n", argv[0]);
+ return 2;
+ }
+
+ NLA_PUT_U32(msg, NL80211_ATTR_PS_STATE, ps_state);
+
+ return 0;
+
+ nla_put_failure:
+ return -ENOBUFS;
+}
+
+COMMAND(set, power_save, "<on|off>",
+ NL80211_CMD_SET_POWER_SAVE, 0, CIB_NETDEV, set_power_save,
+ "Set power save state to on or off.");
+
+static int print_power_save_handler(struct nl_msg *msg, void *arg)
+{
+ struct nlattr *attrs[NL80211_ATTR_MAX + 1];
+ struct genlmsghdr *gnlh = nlmsg_data(nlmsg_hdr(msg));
+ const char *s;
+
+ nla_parse(attrs, NL80211_ATTR_MAX, genlmsg_attrdata(gnlh, 0),
+ genlmsg_attrlen(gnlh, 0), NULL);
+
+ if (!attrs[NL80211_ATTR_PS_STATE])
+ return NL_SKIP;
+
+ switch (nla_get_u32(attrs[NL80211_ATTR_PS_STATE])) {
+ case NL80211_PS_ENABLED:
+ s = "on";
+ break;
+ case NL80211_PS_DISABLED:
+ default:
+ s = "off";
+ break;
+ }
+
+ printf("Power save: %s\n", s);
+
+ return NL_SKIP;
+}
+
+static int get_power_save(struct nl80211_state *state,
+ struct nl_cb *cb,
+ struct nl_msg *msg,
+ int argc, char **argv)
+{
+ nl_cb_set(cb, NL_CB_VALID, NL_CB_CUSTOM,
+ print_power_save_handler, NULL);
+ return 0;
+}
+
+COMMAND(get, power_save, "<param>",
+ NL80211_CMD_GET_POWER_SAVE, 0, CIB_NETDEV, get_power_save,
+ "Retrieve power save state.");