2011-09-22 15:49:36

by Johannes Berg

[permalink] [raw]
Subject: [RFC 00/15] mac80211 uAPSD support

Here's a new set of my uAPSD support patches.
New is:
* ps-poll and uAPSD service periods may not
happen at the same time, this simplifies
things and hopefully stations won't be that
stupid ...
* fixes a bunch of issues
* thought about GO absence periods
* added documentation

johannes



2011-09-23 08:59:07

by Johannes Berg

[permalink] [raw]
Subject: Re: [RFC 00/15] mac80211 uAPSD support

On Thu, 2011-09-22 at 16:25 -0700, Luis R. Rodriguez wrote:

> Awesome :) I started to review this but tried applying the entire
> series [1] to several different master tags on wireless-testing but it
> doesn't apply, any chance you could rebase? I'll try to finish my
> review through though.

It seems to apply to wireless-testing for me, not sure ...

johannes


2011-09-23 18:14:17

by Johannes Berg

[permalink] [raw]
Subject: Re: [RFC 00/15] mac80211 uAPSD support

Hi,

Thanks for looking.

> I think it should be like below (NL80211_ATTR_QOS_INFO and
> NL80211_ATTR_UAPSD_SUPPORTED only for backward-compatibility)

Is the change below on top of my changes? I'm not sure I understand it
completely. See inline.

> --- a/include/linux/ieee80211.h
> +++ b/include/linux/ieee80211.h
> @@ -117,6 +117,7 @@
> #define IEEE80211_MAX_MESH_ID_LEN 32
>
> #define IEEE80211_QOS_CTL_LEN 2
> +#define IEEE80211_QOS_CTL_EOSP 0x0010
> #define IEEE80211_QOS_CTL_TID_MASK 0x000F
> #define IEEE80211_QOS_CTL_TAG1D_MASK 0x0007

This is already in the kernel for me...?

> diff --git a/include/linux/nl80211.h b/include/linux/nl80211.h
> index 2636c01..d831c15 100644
> --- a/include/linux/nl80211.h
> +++ b/include/linux/nl80211.h
> @@ -610,8 +610,6 @@ enum nl80211_commands {
> NL80211_CMD_SET_NOA,
> NL80211_CMD_SET_P2P_POWER_SAVE,
>
> - NL80211_ATTR_STA_WME,
> -
> /* add new commands above here */

How did you get the ATTR_STA_WME as part of the commands? It's not like
that in my kernel.

> /* used to define NL80211_CMD_MAX below */
> @@ -1216,6 +1214,10 @@ enum nl80211_attrs {
> NL80211_ATTR_P2P_PS_CTWINDOW,
> NL80211_ATTR_UAPSD,
>
> + NL80211_ATTR_STA_WME,
> + NL80211_ATTR_UAPSD_SUPPORTED, /* OBSOLETE */
> + NL80211_ATTR_QOS_INFO, /* OBSOLETE */

Where do you get PS_CTWINDOW? ATTR_UAPSD? I'm getting the impression
that you're using a pretty hacked up nl80211 here.

> +++ b/net/mac80211/sta_info.c
> @@ -625,7 +625,7 @@ static bool
> sta_info_cleanup_expire_buffered_ac(struct ieee80211_local *local,
> unsigned long flags;
> struct sk_buff *skb;
>
> - if (skb_queue_empty(&sta->ps_tx_buf))
> + if (skb_queue_empty(&sta->ps_tx_buf[ac]))
> return false;

If that kind of change was necessary on top of my patches, they wouldn't
even compile, so what does this mean?


I'm confused.


johannes


2011-09-22 15:52:20

by Johannes Berg

[permalink] [raw]
Subject: Re: [RFC 00/15] mac80211 uAPSD support

Oops, forgot the total diffstat:

Documentation/DocBook/80211.tmpl | 11
drivers/net/wireless/ath/ath9k/ath9k.h | 3
drivers/net/wireless/ath/ath9k/main.c | 3
drivers/net/wireless/ath/ath9k/xmit.c | 14
drivers/net/wireless/iwlegacy/iwl-4965-tx.c | 2
drivers/net/wireless/iwlwifi/iwl-agn-tx.c | 2
drivers/net/wireless/p54/txrx.c | 2
include/net/mac80211.h | 225 +++++++
net/mac80211/agg-rx.c | 2
net/mac80211/agg-tx.c | 2
net/mac80211/cfg.c | 33 -
net/mac80211/debugfs_sta.c | 54 +
net/mac80211/driver-ops.h | 30 +
net/mac80211/driver-trace.h | 57 +
net/mac80211/ht.c | 2
net/mac80211/ibss.c | 4
net/mac80211/ieee80211_i.h | 16
net/mac80211/iface.c | 4
net/mac80211/key.c | 4
net/mac80211/main.c | 14
net/mac80211/mesh_plink.c | 8
net/mac80211/mlme.c | 22
net/mac80211/pm.c | 2
net/mac80211/rx.c | 127 ++--
net/mac80211/sta_info.c | 816 ++++++++++++++++++++--------
net/mac80211/sta_info.h | 137 +---
net/mac80211/status.c | 60 +-
net/mac80211/tx.c | 111 +--
net/mac80211/util.c | 19
net/mac80211/wme.c | 4
30 files changed, 1268 insertions(+), 522 deletions(-)

johannes


2011-09-22 23:26:01

by Luis R. Rodriguez

[permalink] [raw]
Subject: Re: [RFC 00/15] mac80211 uAPSD support

T24gVGh1LCBTZXAgMjIsIDIwMTEgYXQgODo1MiBBTSwgSm9oYW5uZXMgQmVyZwo8am9oYW5uZXNA
c2lwc29sdXRpb25zLm5ldD4gd3JvdGU6Cj4gT29wcywgZm9yZ290IHRoZSB0b3RhbCBkaWZmc3Rh
dDoKPgo+IMKgRG9jdW1lbnRhdGlvbi9Eb2NCb29rLzgwMjExLnRtcGwgwqAgwqAgwqAgwqAgwqAg
wqB8IMKgIDExCj4gwqBkcml2ZXJzL25ldC93aXJlbGVzcy9hdGgvYXRoOWsvYXRoOWsuaCDCoCDC
oCDCoHwgwqAgwqAzCj4gwqBkcml2ZXJzL25ldC93aXJlbGVzcy9hdGgvYXRoOWsvbWFpbi5jIMKg
IMKgIMKgIHwgwqAgwqAzCj4gwqBkcml2ZXJzL25ldC93aXJlbGVzcy9hdGgvYXRoOWsveG1pdC5j
IMKgIMKgIMKgIHwgwqAgMTQKPiDCoGRyaXZlcnMvbmV0L3dpcmVsZXNzL2l3bGVnYWN5L2l3bC00
OTY1LXR4LmMgfCDCoCDCoDIKPiDCoGRyaXZlcnMvbmV0L3dpcmVsZXNzL2l3bHdpZmkvaXdsLWFn
bi10eC5jIMKgIHwgwqAgwqAyCj4gwqBkcml2ZXJzL25ldC93aXJlbGVzcy9wNTQvdHhyeC5jIMKg
IMKgIMKgIMKgIMKgIMKgIHwgwqAgwqAyCj4gwqBpbmNsdWRlL25ldC9tYWM4MDIxMS5oIMKgIMKg
IMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgfCDCoDIyNSArKysrKysrCj4gwqBuZXQvbWFjODAy
MTEvYWdnLXJ4LmMgwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAgfCDCoCDCoDIKPiDC
oG5ldC9tYWM4MDIxMS9hZ2ctdHguYyDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCB8
IMKgIMKgMgo+IMKgbmV0L21hYzgwMjExL2NmZy5jIMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKg
IMKgIMKgIMKgIMKgfCDCoCAzMyAtCj4gwqBuZXQvbWFjODAyMTEvZGVidWdmc19zdGEuYyDCoCDC
oCDCoCDCoCDCoCDCoCDCoCDCoCDCoHwgwqAgNTQgKwo+IMKgbmV0L21hYzgwMjExL2RyaXZlci1v
cHMuaCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCB8IMKgIDMwICsKPiDCoG5ldC9tYWM4MDIx
MS9kcml2ZXItdHJhY2UuaCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCB8IMKgIDU3ICsKPiDCoG5l
dC9tYWM4MDIxMS9odC5jIMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgIHwg
wqAgwqAyCj4gwqBuZXQvbWFjODAyMTEvaWJzcy5jIMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKg
IMKgIMKgIMKgIHwgwqAgwqA0Cj4gwqBuZXQvbWFjODAyMTEvaWVlZTgwMjExX2kuaCDCoCDCoCDC
oCDCoCDCoCDCoCDCoCDCoCDCoHwgwqAgMTYKPiDCoG5ldC9tYWM4MDIxMS9pZmFjZS5jIMKgIMKg
IMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgfCDCoCDCoDQKPiDCoG5ldC9tYWM4MDIxMS9r
ZXkuYyDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoHwgwqAgwqA0Cj4gwqBu
ZXQvbWFjODAyMTEvbWFpbi5jIMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgIHwg
wqAgMTQKPiDCoG5ldC9tYWM4MDIxMS9tZXNoX3BsaW5rLmMgwqAgwqAgwqAgwqAgwqAgwqAgwqAg
wqAgwqAgfCDCoCDCoDgKPiDCoG5ldC9tYWM4MDIxMS9tbG1lLmMgwqAgwqAgwqAgwqAgwqAgwqAg
wqAgwqAgwqAgwqAgwqAgwqAgfCDCoCAyMgo+IMKgbmV0L21hYzgwMjExL3BtLmMgwqAgwqAgwqAg
wqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAgfCDCoCDCoDIKPiDCoG5ldC9tYWM4MDIxMS9y
eC5jIMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgIHwgwqAxMjcgKystLQo+
IMKgbmV0L21hYzgwMjExL3N0YV9pbmZvLmMgwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAg
fCDCoDgxNiArKysrKysrKysrKysrKysrKysrKy0tLS0tLS0tCj4gwqBuZXQvbWFjODAyMTEvc3Rh
X2luZm8uaCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCB8IMKgMTM3ICstLS0KPiDCoG5l
dC9tYWM4MDIxMS9zdGF0dXMuYyDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCB8IMKg
IDYwICstCj4gwqBuZXQvbWFjODAyMTEvdHguYyDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDC
oCDCoCDCoCDCoCB8IMKgMTExICstLQo+IMKgbmV0L21hYzgwMjExL3V0aWwuYyDCoCDCoCDCoCDC
oCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCB8IMKgIDE5Cj4gwqBuZXQvbWFjODAyMTEvd21lLmMg
wqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqB8IMKgIMKgNAo+IMKgMzAgZmls
ZXMgY2hhbmdlZCwgMTI2OCBpbnNlcnRpb25zKCspLCA1MjIgZGVsZXRpb25zKC0pCgpBd2Vzb21l
IDopIEkgc3RhcnRlZCB0byByZXZpZXcgdGhpcyBidXQgdHJpZWQgYXBwbHlpbmcgdGhlIGVudGly
ZQpzZXJpZXMgWzFdIHRvIHNldmVyYWwgZGlmZmVyZW50IG1hc3RlciB0YWdzIG9uIHdpcmVsZXNz
LXRlc3RpbmcgYnV0IGl0CmRvZXNuJ3QgYXBwbHksIGFueSBjaGFuY2UgeW91IGNvdWxkIHJlYmFz
ZT8gSSdsbCB0cnkgdG8gZmluaXNoIG15CnJldmlldyB0aHJvdWdoIHRob3VnaC4KClsxXSBodHRw
Oi8vYm9tYmFkaWwuaW5mcmFkZWFkLm9yZy9+bWNncm9mL3BhdGNoZXMvMjAxMS8wOS8yMi9yZmMt
dWFwc2QucGF0Y2gKCiAgTHVpcwo=

2011-09-23 18:06:05

by Dmitry Tarnyagin

[permalink] [raw]
Subject: Re: [RFC 00/15] mac80211 uAPSD support

Hi Johannes,

I think it should be like below (NL80211_ATTR_QOS_INFO and
NL80211_ATTR_UAPSD_SUPPORTED only for backward-compatibility)

With best regards,
Dmitry

diff --git a/include/linux/ieee80211.h b/include/linux/ieee80211.h
index 69cd37d..f627621 100644
--- a/include/linux/ieee80211.h
+++ b/include/linux/ieee80211.h
@@ -117,6 +117,7 @@
#define IEEE80211_MAX_MESH_ID_LEN 32

#define IEEE80211_QOS_CTL_LEN 2
+#define IEEE80211_QOS_CTL_EOSP 0x0010
#define IEEE80211_QOS_CTL_TID_MASK 0x000F
#define IEEE80211_QOS_CTL_TAG1D_MASK 0x0007

diff --git a/include/linux/nl80211.h b/include/linux/nl80211.h
index 2636c01..d831c15 100644
--- a/include/linux/nl80211.h
+++ b/include/linux/nl80211.h
@@ -610,8 +610,6 @@ enum nl80211_commands {
NL80211_CMD_SET_NOA,
NL80211_CMD_SET_P2P_POWER_SAVE,

- NL80211_ATTR_STA_WME,
-
/* add new commands above here */

/* used to define NL80211_CMD_MAX below */
@@ -1216,6 +1214,10 @@ enum nl80211_attrs {
NL80211_ATTR_P2P_PS_CTWINDOW,
NL80211_ATTR_UAPSD,

+ NL80211_ATTR_STA_WME,
+ NL80211_ATTR_UAPSD_SUPPORTED, /* OBSOLETE */
+ NL80211_ATTR_QOS_INFO, /* OBSOLETE */
+
/* add attributes here, update the policy in nl80211.c */

__NL80211_ATTR_AFTER_LAST,
diff --git a/net/mac80211/sta_info.c b/net/mac80211/sta_info.c
index 47bcfee..80ece31 100644
--- a/net/mac80211/sta_info.c
+++ b/net/mac80211/sta_info.c
@@ -625,7 +625,7 @@ static bool
sta_info_cleanup_expire_buffered_ac(struct ieee80211_local *local,
unsigned long flags;
struct sk_buff *skb;

- if (skb_queue_empty(&sta->ps_tx_buf))
+ if (skb_queue_empty(&sta->ps_tx_buf[ac]))
return false;

/*
diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c
index b98f18d..791c0e4 100644
--- a/net/wireless/nl80211.c
+++ b/net/wireless/nl80211.c
@@ -176,6 +176,7 @@ static const struct nla_policy
nl80211_policy[NL80211_ATTR_MAX+1] = {
[NL80211_ATTR_WOWLAN_TRIGGERS] = { .type = NLA_NESTED },
[NL80211_ATTR_STA_PLINK_STATE] = { .type = NLA_U8 },
[NL80211_ATTR_SCHED_SCAN_INTERVAL] = { .type = NLA_U32 },
+ [NL80211_ATTR_QOS_INFO] = { .type = NLA_U8 },
[NL80211_ATTR_P2P_PS_NOA] = { .type = NLA_BINARY,
.len = 32 },
[NL80211_ATTR_P2P_PS_NOA_COUNT] = { .type = NLA_U8 },
@@ -952,6 +953,9 @@ static int nl80211_send_wiphy(struct sk_buff *msg,
u32 pid, u32 seq, int flags,
if (nl80211_put_iface_combinations(&dev->wiphy, msg))
goto nla_put_failure;

+ if (dev->wiphy.flags & WIPHY_FLAG_AP_UAPSD)
+ NLA_PUT_FLAG(msg, NL80211_ATTR_UAPSD_SUPPORTED);
+
return genlmsg_end(msg, hdr);

nla_put_failure: