2011-11-15 23:51:39

by Simon Wunderlich

[permalink] [raw]
Subject: [PATCH] mac80211: Add NoAck per WMM Queue Support

This patch adds support for NoAck per WMM Queue. The Unicast QoS
Header is adapted accordingly for each outgoing frame.
The support is turned on and off through nl80211 by extending
the WMM TX Queue Parameters, but can be triggered separately.

I have tested this feature on ath9k as well as ath5k devices. There is
an iw patch as well to make use of this feature.

It should apply well on the latest wireless-testing kernel.

Signed-off-by: Simon Wunderlich <[email protected]>
Signed-off-by: Mathias Kretschmer <[email protected]>
---
include/linux/nl80211.h | 2 ++
include/net/cfg80211.h | 7 +++++++
include/net/mac80211.h | 2 ++
net/mac80211/cfg.c | 21 +++++++++++++++++++++
net/mac80211/tx.c | 5 +++++
net/mac80211/util.c | 1 +
net/mac80211/wme.c | 5 ++++-
net/wireless/nl80211.c | 34 +++++++++++++++++++++++++++++-----
8 files changed, 71 insertions(+), 6 deletions(-)

diff --git a/include/linux/nl80211.h b/include/linux/nl80211.h
index f9261c2..6b0ecb4 100644
--- a/include/linux/nl80211.h
+++ b/include/linux/nl80211.h
@@ -2137,6 +2137,7 @@ enum nl80211_mesh_setup_params {
* @NL80211_TXQ_ATTR_CWMAX: Maximum contention window [a value of the form
* 2^n-1 in the range 1..32767]
* @NL80211_TXQ_ATTR_AIFS: Arbitration interframe space [0..255]
+ * @NL80211_TXQ_ATTR_NOACK: NoAck Mode, 0 meaning normal Ack operation
* @__NL80211_TXQ_ATTR_AFTER_LAST: Internal
* @NL80211_TXQ_ATTR_MAX: Maximum TXQ attribute number
*/
@@ -2147,6 +2148,7 @@ enum nl80211_txq_attr {
NL80211_TXQ_ATTR_CWMIN,
NL80211_TXQ_ATTR_CWMAX,
NL80211_TXQ_ATTR_AIFS,
+ NL80211_TXQ_ATTR_NOACK,

/* keep last */
__NL80211_TXQ_ATTR_AFTER_LAST,
diff --git a/include/net/cfg80211.h b/include/net/cfg80211.h
index 8d7ba09..10884ef 100644
--- a/include/net/cfg80211.h
+++ b/include/net/cfg80211.h
@@ -825,6 +825,7 @@ struct mesh_setup {
* @cwmax: Maximum contention window [a value of the form 2^n-1 in the range
* 1..32767]
* @aifs: Arbitration interframe space [0..255]
+ * @noack: NoAck Policy, false meaning normal Ack operation
*/
struct ieee80211_txq_params {
enum nl80211_txq_q queue;
@@ -832,6 +833,7 @@ struct ieee80211_txq_params {
u16 cwmin;
u16 cwmax;
u8 aifs;
+ bool noack;
};

/* from net/wireless.h */
@@ -1341,6 +1343,8 @@ struct cfg80211_gtk_rekey_data {
*
* @set_txq_params: Set TX queue parameters
*
+ * @set_txq_noack: Set TX queue NoAck Parameter
+ *
* @set_channel: Set channel for a given wireless interface. Some devices
* may support multi-channel operation (by channel hopping) so cfg80211
* doesn't verify much. Note, however, that the passed netdev may be
@@ -1521,6 +1525,9 @@ struct cfg80211_ops {
int (*set_txq_params)(struct wiphy *wiphy, struct net_device *dev,
struct ieee80211_txq_params *params);

+ int (*set_txq_noack)(struct wiphy *wiphy, struct net_device *dev,
+ struct ieee80211_txq_params *params);
+
int (*set_channel)(struct wiphy *wiphy, struct net_device *dev,
struct ieee80211_channel *chan,
enum nl80211_channel_type channel_type);
diff --git a/include/net/mac80211.h b/include/net/mac80211.h
index 0756049..1bfcc21 100644
--- a/include/net/mac80211.h
+++ b/include/net/mac80211.h
@@ -122,6 +122,7 @@ enum ieee80211_ac_numbers {
* 2^n-1 in the range 1..32767]
* @cw_max: maximum contention window [like @cw_min]
* @txop: maximum burst time in units of 32 usecs, 0 meaning disabled
+ * @noack: NoAck Policy, false meaning normal Ack operation
* @uapsd: is U-APSD mode enabled for the queue
*/
struct ieee80211_tx_queue_params {
@@ -129,6 +130,7 @@ struct ieee80211_tx_queue_params {
u16 cw_min;
u16 cw_max;
u8 aifs;
+ bool noack;
bool uapsd;
};

diff --git a/net/mac80211/cfg.c b/net/mac80211/cfg.c
index 1063a7e..8298c51 100644
--- a/net/mac80211/cfg.c
+++ b/net/mac80211/cfg.c
@@ -1393,6 +1393,7 @@ static int ieee80211_set_txq_params(struct wiphy *wiphy,
p.cw_max = params->cwmax;
p.cw_min = params->cwmin;
p.txop = params->txop;
+ p.noack = params->noack;

/*
* Setting tx queue params disables u-apsd because it's only
@@ -1414,6 +1415,25 @@ static int ieee80211_set_txq_params(struct wiphy *wiphy,
return 0;
}

+static int ieee80211_set_txq_noack(struct wiphy *wiphy,
+ struct net_device *dev,
+ struct ieee80211_txq_params *params)
+{
+ struct ieee80211_local *local = wiphy_priv(wiphy);
+ struct ieee80211_sub_if_data *sdata = IEEE80211_DEV_TO_SUB_IF(dev);
+
+ if (!local->ops->conf_tx)
+ return -EOPNOTSUPP;
+
+ if (params->queue >= local->hw.queues)
+ return -EINVAL;
+
+ sdata->tx_conf[params->queue].noack = params->noack;
+
+ return 0;
+
+}
+
static int ieee80211_set_channel(struct wiphy *wiphy,
struct net_device *netdev,
struct ieee80211_channel *chan,
@@ -2662,6 +2682,7 @@ struct cfg80211_ops mac80211_config_ops = {
#endif
.change_bss = ieee80211_change_bss,
.set_txq_params = ieee80211_set_txq_params,
+ .set_txq_noack = ieee80211_set_txq_noack,
.set_channel = ieee80211_set_channel,
.suspend = ieee80211_suspend,
.resume = ieee80211_resume,
diff --git a/net/mac80211/tx.c b/net/mac80211/tx.c
index f044963..cb13ab3 100644
--- a/net/mac80211/tx.c
+++ b/net/mac80211/tx.c
@@ -1161,6 +1161,11 @@ ieee80211_tx_prepare(struct ieee80211_sub_if_data *sdata,
* explicitly unset IEEE80211_TX_CTL_NO_ACK since
* it might already be set for injected frames.
*/
+
+ qc = ieee80211_get_qos_ctl(hdr);
+ if (*qc & IEEE80211_QOS_CTL_ACK_POLICY_NOACK)
+ info->flags |= IEEE80211_TX_CTL_NO_ACK;
+
}

if (!(info->flags & IEEE80211_TX_CTL_DONTFRAG)) {
diff --git a/net/mac80211/util.c b/net/mac80211/util.c
index 3a00814..d298baf 100644
--- a/net/mac80211/util.c
+++ b/net/mac80211/util.c
@@ -629,6 +629,7 @@ void ieee80211_set_wmm_default(struct ieee80211_sub_if_data *sdata)
}

qparam.uapsd = false;
+ qparam.noack = false;

sdata->tx_conf[queue] = qparam;
drv_conf_tx(local, sdata, queue, &qparam);
diff --git a/net/mac80211/wme.c b/net/mac80211/wme.c
index 4332711..ba22a08 100644
--- a/net/mac80211/wme.c
+++ b/net/mac80211/wme.c
@@ -143,13 +143,16 @@ void ieee80211_set_qos_hdr(struct ieee80211_sub_if_data *sdata,
/* Fill in the QoS header if there is one. */
if (ieee80211_is_data_qos(hdr->frame_control)) {
u8 *p = ieee80211_get_qos_ctl(hdr);
- u8 ack_policy, tid;
+ u8 ack_policy = 0, tid, q;

tid = skb->priority & IEEE80211_QOS_CTL_TAG1D_MASK;
+ q = ieee802_1d_to_ac[tid];

/* preserve EOSP bit */
ack_policy = *p & IEEE80211_QOS_CTL_EOSP;

+ if (sdata->tx_conf[q].noack)
+ ack_policy |= IEEE80211_QOS_CTL_ACK_POLICY_NOACK;
if (unlikely(sdata->local->wifi_wme_noack_test) ||
is_multicast_ether_addr(hdr->addr1))
ack_policy |= IEEE80211_QOS_CTL_ACK_POLICY_NOACK;
diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c
index 6bc7c4b..1e8e2c0 100644
--- a/net/wireless/nl80211.c
+++ b/net/wireless/nl80211.c
@@ -1081,6 +1081,7 @@ static const struct nla_policy txq_params_policy[NL80211_TXQ_ATTR_MAX + 1] = {
[NL80211_TXQ_ATTR_CWMIN] = { .type = NLA_U16 },
[NL80211_TXQ_ATTR_CWMAX] = { .type = NLA_U16 },
[NL80211_TXQ_ATTR_AIFS] = { .type = NLA_U8 },
+ [NL80211_TXQ_ATTR_NOACK] = { .type = NLA_U8 },
};

static int parse_txq_params(struct nlattr *tb[],
@@ -1096,10 +1097,24 @@ static int parse_txq_params(struct nlattr *tb[],
txq_params->cwmin = nla_get_u16(tb[NL80211_TXQ_ATTR_CWMIN]);
txq_params->cwmax = nla_get_u16(tb[NL80211_TXQ_ATTR_CWMAX]);
txq_params->aifs = nla_get_u8(tb[NL80211_TXQ_ATTR_AIFS]);
+ txq_params->noack = false;

return 0;
}

+static int parse_txq_params_noack(struct nlattr *tb[],
+ struct ieee80211_txq_params *txq_params)
+{
+ if (!tb[NL80211_TXQ_ATTR_NOACK] || !tb[NL80211_TXQ_ATTR_QUEUE])
+ return -EINVAL;
+
+ txq_params->queue = nla_get_u8(tb[NL80211_TXQ_ATTR_QUEUE]);
+ txq_params->noack = !!nla_get_u8(tb[NL80211_TXQ_ATTR_NOACK]);
+
+ return 0;
+}
+
+
static bool nl80211_can_set_dev_channel(struct wireless_dev *wdev)
{
/*
@@ -1279,17 +1294,26 @@ static int nl80211_set_wiphy(struct sk_buff *skb, struct genl_info *info)
nla_for_each_nested(nl_txq_params,
info->attrs[NL80211_ATTR_WIPHY_TXQ_PARAMS],
rem_txq_params) {
+ int result_noack, result_param;
nla_parse(tb, NL80211_TXQ_ATTR_MAX,
nla_data(nl_txq_params),
nla_len(nl_txq_params),
txq_params_policy);
- result = parse_txq_params(tb, &txq_params);
- if (result)
+
+ result_param = parse_txq_params(tb, &txq_params);
+ result_noack = parse_txq_params_noack(tb, &txq_params);
+
+ if (!result_param)
+ result = rdev->ops->set_txq_params(&rdev->wiphy,
+ netdev,
+ &txq_params);
+ else if (!result_noack)
+ result = rdev->ops->set_txq_noack(&rdev->wiphy,
+ netdev,
+ &txq_params);
+ else
goto bad_res;

- result = rdev->ops->set_txq_params(&rdev->wiphy,
- netdev,
- &txq_params);
if (result)
goto bad_res;
}
--
1.7.7.2



2011-11-16 12:29:57

by Simon Wunderlich

[permalink] [raw]
Subject: Re: [PATCH] mac80211: Add NoAck per WMM Queue Support

Hi Johannes,

On Wed, Nov 16, 2011 at 11:57:47AM +0100, Johannes Berg wrote:
> >
> > I'm not sure what you mean by "per connection" - do you mean per AP/Sta
> > connection? We need definitely need it per queue as explained above.
>
> No, I mean ... it could be a parameter to connect() or associate(),
> which would get a bitmap of ACs to use no-ack policy for, instead of an
> explicit new configuration.
>

One requirement we had was to change this feature at runtime with iw - this
would be difficult to do when we put it as parameter into connect() or
associate() ...

I'll resend v2 in a few minutes.

best regards,
Simon


Attachments:
(No filename) (631.00 B)
signature.asc (198.00 B)
Digital signature
Download all attachments

2011-11-16 10:57:52

by Johannes Berg

[permalink] [raw]
Subject: Re: [PATCH] mac80211: Add NoAck per WMM Queue Support

Hi,

> > Can you explain what for? :-)
>
> We want to set NoAck for special traffic classes like video or voice where
> "old" packets are not useful, but keep it for background and best effort traffic.
> This can be applied on devices supporting both traffic classes (e.g.
> laptops, smartphones with VoIP software) as well as for backhaul links.

Ok.

> > I'm not sure this API is really the best way to handle it either, like I
> > just said it might be nicer per connection or so to get reset properly.
> >
>
> I'm not sure what you mean by "per connection" - do you mean per AP/Sta
> connection? We need definitely need it per queue as explained above.

No, I mean ... it could be a parameter to connect() or associate(),
which would get a bitmap of ACs to use no-ack policy for, instead of an
explicit new configuration.

> > > I have tested this feature on ath9k as well as ath5k devices. There is
> > > an iw patch as well to make use of this feature.
> >
> > Since you're adding 'real API' (unlike the debugfs file which you should
> > probably remove now!) you also should think about drivers like mwifiex
> > that don't support this and don't use mac80211.
>
> mwifiex neither supports set_txq_params nor the new function, nl80211 should
> just return "-EOPNOTSUPP". A short grep revealed that no driver in wireless-testing
> implements set_txq_params so far, so at least for the drivers within the kernel
> this should be safe. I forgot to explicitly check for rdev->ops->set_txq_noack
> and will add this check in v2 ...

Good point -- but if you'd change to make it a parameter for connect and
associate (I think that is better since it will clean up by itself
properly when disconnecting) then you would need some flag indicating
that the driver/device supports this (with mac80211 obviously always
supporting it).

johannes



2011-11-16 13:58:19

by Helmut Schaa

[permalink] [raw]
Subject: Re: [PATCH] mac80211: Add NoAck per WMM Queue Support

On Wed, Nov 16, 2011 at 12:21 AM, Simon Wunderlich
<[email protected]> wrote:
> This patch adds support for NoAck per WMM Queue. The Unicast QoS
> Header is adapted accordingly for each outgoing frame.
> The support is turned on and off through nl80211 by extending
> the WMM TX Queue Parameters, but can be triggered separately.

Just a thought: Assuming you've configured AC_VO to use NoAck and
you've only got VO traffic going on, is minstrel still able to collect enough
statistics to do proper rate selection?

Thanks,
Helmut

2011-11-16 15:21:21

by Simon Wunderlich

[permalink] [raw]
Subject: Re: [PATCH] mac80211: Add NoAck per WMM Queue Support

Hello Helmut,

thanks for pointing that out!

As far as I understand minstrel, it is using sent packets and ack statistics
to find out the percentage of successfulness. Therefore, minstrel should
probably not consider NOACK frames for its statistics.

If there are only AC_VO frames, there would not really be any data for
minstrel to build its statistics, so one could either rely on the
control traffic which may go over another queue or use fixed rates.
For example, SIP uses RTP for the voice data and SIP packets for
session control ...

best regards,
Simon

[Helmut, sorry for the resend, I accidently replied personally to only you first]

On Wed, Nov 16, 2011 at 02:58:18PM +0100, Helmut Schaa wrote:
> On Wed, Nov 16, 2011 at 12:21 AM, Simon Wunderlich
> <[email protected]> wrote:
> > This patch adds support for NoAck per WMM Queue. The Unicast QoS
> > Header is adapted accordingly for each outgoing frame.
> > The support is turned on and off through nl80211 by extending
> > the WMM TX Queue Parameters, but can be triggered separately.
>
> Just a thought: Assuming you've configured AC_VO to use NoAck and
> you've only got VO traffic going on, is minstrel still able to collect enough
> statistics to do proper rate selection?
>
> Thanks,
> Helmut
>


Attachments:
(No filename) (1.27 kB)
signature.asc (198.00 B)
Digital signature
Download all attachments

2011-11-15 23:51:41

by Simon Wunderlich

[permalink] [raw]
Subject: [PATCH] iw: add support for wmm NoAck per WMM Queue

This adds support for the new NoAck feature in nl80211/mac80211

Signed-off-by: Simon Wunderlich <[email protected]>
Signed-off-by: Mathias Kretschmer <[email protected]>
---
nl80211.h | 2 ++
phy.c | 51 +++++++++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 53 insertions(+), 0 deletions(-)

diff --git a/nl80211.h b/nl80211.h
index 8049bf7..3d199c6 100644
--- a/nl80211.h
+++ b/nl80211.h
@@ -2065,6 +2065,7 @@ enum nl80211_mesh_setup_params {
* @NL80211_TXQ_ATTR_CWMAX: Maximum contention window [a value of the form
* 2^n-1 in the range 1..32767]
* @NL80211_TXQ_ATTR_AIFS: Arbitration interframe space [0..255]
+ * @NL80211_TXQ_ATTR_NOACK: NoAck Mode, 0 meaning normal Ack operation
* @__NL80211_TXQ_ATTR_AFTER_LAST: Internal
* @NL80211_TXQ_ATTR_MAX: Maximum TXQ attribute number
*/
@@ -2075,6 +2076,7 @@ enum nl80211_txq_attr {
NL80211_TXQ_ATTR_CWMIN,
NL80211_TXQ_ATTR_CWMAX,
NL80211_TXQ_ATTR_AIFS,
+ NL80211_TXQ_ATTR_NOACK,

/* keep last */
__NL80211_TXQ_ATTR_AFTER_LAST,
diff --git a/phy.c b/phy.c
index d9090fd..e5f36be 100644
--- a/phy.c
+++ b/phy.c
@@ -131,6 +131,57 @@ COMMAND(set, frag, "<fragmentation threshold|off>",
NL80211_CMD_SET_WIPHY, 0, CIB_PHY, handle_fragmentation,
"Set fragmentation threshold.");

+static int handle_wmm_noack(struct nl80211_state *state,
+ struct nl_cb *cb, struct nl_msg *msg,
+ int argc, char **argv)
+{
+ enum nl80211_txq_q queue;
+ unsigned int noack;
+ struct nlattr *txq, *params;
+
+ if (argc != 2)
+ return 1;
+
+ if (strcmp("VO", argv[0]) == 0)
+ queue = NL80211_TXQ_Q_VO;
+ else if (strcmp("VI", argv[0]) == 0)
+ queue = NL80211_TXQ_Q_VI;
+ else if (strcmp("BE", argv[0]) == 0)
+ queue = NL80211_TXQ_Q_BE;
+ else if (strcmp("BK", argv[0]) == 0)
+ queue = NL80211_TXQ_Q_BK;
+ else
+ return 1;
+
+ if (strcmp("off", argv[1]) == 0)
+ noack = 0;
+ else if (strcmp("on", argv[1]) == 0)
+ noack = 1;
+ else
+ return 1;
+
+ txq = nla_nest_start(msg, NL80211_ATTR_WIPHY_TXQ_PARAMS);
+ if (!txq)
+ goto nla_put_failure;
+
+ params = nla_nest_start(msg, 1);
+ if (!params)
+ goto nla_put_failure;
+
+ NLA_PUT_U8(msg, NL80211_TXQ_ATTR_QUEUE, queue);
+ NLA_PUT_U8(msg, NL80211_TXQ_ATTR_NOACK, noack);
+ nla_nest_end(msg, params);
+ nla_nest_end(msg, txq);
+
+ return 0;
+ nla_put_failure:
+ return -ENOBUFS;
+}
+COMMAND(set, wmm_noack, "<VO|VI|BE|BK> <on|off>",
+ NL80211_CMD_SET_WIPHY, 0, CIB_NETDEV, handle_wmm_noack,
+ "Set WMM noack policy");
+
+
static int handle_rts(struct nl80211_state *state,
struct nl_cb *cb, struct nl_msg *msg,
int argc, char **argv)
--
1.7.7.2


2011-11-16 07:52:18

by Johannes Berg

[permalink] [raw]
Subject: Re: [PATCH] mac80211: Add NoAck per WMM Queue Support

On Wed, 2011-11-16 at 00:21 +0100, Simon Wunderlich wrote:
> This patch adds support for NoAck per WMM Queue. The Unicast QoS
> Header is adapted accordingly for each outgoing frame.
> The support is turned on and off through nl80211 by extending
> the WMM TX Queue Parameters, but can be triggered separately.

Can you explain what for? :-)

I'm not sure this API is really the best way to handle it either, like I
just said it might be nicer per connection or so to get reset properly.

> I have tested this feature on ath9k as well as ath5k devices. There is
> an iw patch as well to make use of this feature.

Since you're adding 'real API' (unlike the debugfs file which you should
probably remove now!) you also should think about drivers like mwifiex
that don't support this and don't use mac80211.

Also, given real API, this impacts the duration calculation etc.

johannes


2011-11-16 07:50:09

by Johannes Berg

[permalink] [raw]
Subject: Re: [PATCH] mac80211: Add NoAck per WMM Queue Support

On Wed, 2011-11-16 at 08:46 +0100, Felix Fietkau wrote:
> On 2011-11-16 12:21 AM, Simon Wunderlich wrote:
> > This patch adds support for NoAck per WMM Queue. The Unicast QoS
> > Header is adapted accordingly for each outgoing frame.
> > The support is turned on and off through nl80211 by extending
> > the WMM TX Queue Parameters, but can be triggered separately.
> >
> > I have tested this feature on ath9k as well as ath5k devices. There is
> > an iw patch as well to make use of this feature.
> >
> > It should apply well on the latest wireless-testing kernel.

> > + int (*set_txq_noack)(struct wiphy *wiphy, struct net_device *dev,
> > + struct ieee80211_txq_params *params);
> > +
> > int (*set_channel)(struct wiphy *wiphy, struct net_device *dev,
> > struct ieee80211_channel *chan,
> > enum nl80211_channel_type channel_type);
> Why add a separate cfg80211 op when you can just make use of the extra
> parameter in the existing one?

Can't actually -- that one's only allowed in AP mode :)

FWIW I think this API would also better be per connection or so.

johannes


2011-11-16 07:46:37

by Felix Fietkau

[permalink] [raw]
Subject: Re: [PATCH] mac80211: Add NoAck per WMM Queue Support

On 2011-11-16 12:21 AM, Simon Wunderlich wrote:
> This patch adds support for NoAck per WMM Queue. The Unicast QoS
> Header is adapted accordingly for each outgoing frame.
> The support is turned on and off through nl80211 by extending
> the WMM TX Queue Parameters, but can be triggered separately.
>
> I have tested this feature on ath9k as well as ath5k devices. There is
> an iw patch as well to make use of this feature.
>
> It should apply well on the latest wireless-testing kernel.
>
> Signed-off-by: Simon Wunderlich <[email protected]>
> Signed-off-by: Mathias Kretschmer <[email protected]>
> ---
> include/linux/nl80211.h | 2 ++
> include/net/cfg80211.h | 7 +++++++
> include/net/mac80211.h | 2 ++
> net/mac80211/cfg.c | 21 +++++++++++++++++++++
> net/mac80211/tx.c | 5 +++++
> net/mac80211/util.c | 1 +
> net/mac80211/wme.c | 5 ++++-
> net/wireless/nl80211.c | 34 +++++++++++++++++++++++++++++-----
> 8 files changed, 71 insertions(+), 6 deletions(-)
>
> @@ -832,6 +833,7 @@ struct ieee80211_txq_params {
> u16 cwmin;
> u16 cwmax;
> u8 aifs;
> + bool noack;
> };
>
> /* from net/wireless.h */
> @@ -1341,6 +1343,8 @@ struct cfg80211_gtk_rekey_data {
> *
> * @set_txq_params: Set TX queue parameters
> *
> + * @set_txq_noack: Set TX queue NoAck Parameter
> + *
> * @set_channel: Set channel for a given wireless interface. Some devices
> * may support multi-channel operation (by channel hopping) so cfg80211
> * doesn't verify much. Note, however, that the passed netdev may be
> @@ -1521,6 +1525,9 @@ struct cfg80211_ops {
> int (*set_txq_params)(struct wiphy *wiphy, struct net_device *dev,
> struct ieee80211_txq_params *params);
>
> + int (*set_txq_noack)(struct wiphy *wiphy, struct net_device *dev,
> + struct ieee80211_txq_params *params);
> +
> int (*set_channel)(struct wiphy *wiphy, struct net_device *dev,
> struct ieee80211_channel *chan,
> enum nl80211_channel_type channel_type);
Why add a separate cfg80211 op when you can just make use of the extra
parameter in the existing one?

- Felix

2011-11-16 10:51:27

by Simon Wunderlich

[permalink] [raw]
Subject: Re: [PATCH] mac80211: Add NoAck per WMM Queue Support

Hi Johannes,

thanks for the review!

On Wed, Nov 16, 2011 at 08:52:15AM +0100, Johannes Berg wrote:
> On Wed, 2011-11-16 at 00:21 +0100, Simon Wunderlich wrote:
> > This patch adds support for NoAck per WMM Queue. The Unicast QoS
> > Header is adapted accordingly for each outgoing frame.
> > The support is turned on and off through nl80211 by extending
> > the WMM TX Queue Parameters, but can be triggered separately.
>
> Can you explain what for? :-)

We want to set NoAck for special traffic classes like video or voice where
"old" packets are not useful, but keep it for background and best effort traffic.
This can be applied on devices supporting both traffic classes (e.g.
laptops, smartphones with VoIP software) as well as for backhaul links.

Something similar has been done previously on madwifi, what we used:

http://madwifi-project.org/wiki/UserDocs/UsersGuide#noackpolicy-WMMNoAckPolicyBitValue

>
> I'm not sure this API is really the best way to handle it either, like I
> just said it might be nicer per connection or so to get reset properly.
>

I'm not sure what you mean by "per connection" - do you mean per AP/Sta
connection? We need definitely need it per queue as explained above.

> > I have tested this feature on ath9k as well as ath5k devices. There is
> > an iw patch as well to make use of this feature.
>
> Since you're adding 'real API' (unlike the debugfs file which you should
> probably remove now!) you also should think about drivers like mwifiex
> that don't support this and don't use mac80211.

mwifiex neither supports set_txq_params nor the new function, nl80211 should
just return "-EOPNOTSUPP". A short grep revealed that no driver in wireless-testing
implements set_txq_params so far, so at least for the drivers within the kernel
this should be safe. I forgot to explicitly check for rdev->ops->set_txq_noack
and will add this check in v2 ...

I will also remove the debugfs entry.

>
> Also, given real API, this impacts the duration calculation etc.

I'm no expert in duration calculation, but will give it a try.

best regards,
Simon

>
> johannes
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>


Attachments:
(No filename) (2.27 kB)
signature.asc (198.00 B)
Digital signature
Download all attachments