2014-07-20 12:32:38

by Lorenzo Bianconi

[permalink] [raw]
Subject: [PATCH 0/2] configure ack timeout estimation algorithm through

Define enable_dynack API to enable ack timeout estimation algorithm (dynack)
through nl80211. Currently dynack is supported just by ath9k

Lorenzo Bianconi (2):
cfg80211: add enable_dynack API
mac80211: add enable_dynack API

include/net/cfg80211.h | 4 ++++
include/net/mac80211.h | 4 ++++
include/uapi/linux/nl80211.h | 5 +++++
net/mac80211/cfg.c | 8 ++++++++
net/mac80211/driver-ops.h | 14 ++++++++++++++
net/mac80211/trace.h | 5 +++++
net/wireless/nl80211.c | 7 +++++++
net/wireless/rdev-ops.h | 11 +++++++++++
net/wireless/trace.h | 5 +++++
9 files changed, 63 insertions(+)

--
1.9.1



2014-07-21 10:13:42

by Johannes Berg

[permalink] [raw]
Subject: Re: [PATCH 1/2] cfg80211: add enable_dynack API

On Sun, 2014-07-20 at 14:32 +0200, Lorenzo Bianconi wrote:

> @@ -2192,6 +2193,12 @@ static int nl80211_set_wiphy(struct sk_buff *skb, struct genl_info *info)
> return result;
> }
>
> + if (info->attrs[NL80211_ATTR_WIPHY_DYNACK]) {
> + result = rdev_enable_dynack(rdev);
> + if (result)
> + return result;
> + }

I think it may be better to handle this in the call that's normally used
(coverage class setting), but allow the other attribute to make a sort
of "dynamic coverage class". And internally, calling drivers, it seems
fine to pass -1 or so since you can extend the datatype there, instead
of introducing a new internal callback. External/internal doesn't always
have to match perfectly.

Additionally, you'll need some capability flag I think.

johannes


2014-07-20 12:32:39

by Lorenzo Bianconi

[permalink] [raw]
Subject: [PATCH 1/2] cfg80211: add enable_dynack API

Define enable_dynack API to enable ack timeout estimation algorithm through
nl80211

Signed-off-by: Lorenzo Bianconi <[email protected]>
---
include/net/cfg80211.h | 4 ++++
include/uapi/linux/nl80211.h | 5 +++++
net/wireless/nl80211.c | 7 +++++++
net/wireless/rdev-ops.h | 11 +++++++++++
net/wireless/trace.h | 5 +++++
5 files changed, 32 insertions(+)

diff --git a/include/net/cfg80211.h b/include/net/cfg80211.h
index 0a080c4..5a86922 100644
--- a/include/net/cfg80211.h
+++ b/include/net/cfg80211.h
@@ -2313,6 +2313,8 @@ struct cfg80211_qos_map {
* @set_ap_chanwidth: Set the AP (including P2P GO) mode channel width for the
* given interface This is used e.g. for dynamic HT 20/40 MHz channel width
* changes during the lifetime of the BSS.
+ *
+ * @enable_dynack: Enable ack timeout estimation algorithm
*/
struct cfg80211_ops {
int (*suspend)(struct wiphy *wiphy, struct cfg80211_wowlan *wow);
@@ -2553,6 +2555,8 @@ struct cfg80211_ops {

int (*set_ap_chanwidth)(struct wiphy *wiphy, struct net_device *dev,
struct cfg80211_chan_def *chandef);
+
+ int (*enable_dynack)(struct wiphy *wiphy);
};

/*
diff --git a/include/uapi/linux/nl80211.h b/include/uapi/linux/nl80211.h
index f1db15b..7cc5ecd 100644
--- a/include/uapi/linux/nl80211.h
+++ b/include/uapi/linux/nl80211.h
@@ -1594,6 +1594,9 @@ enum nl80211_commands {
* @NL80211_ATTR_TDLS_INITIATOR: flag attribute indicating the current end is
* the TDLS link initiator.
*
+ * @NL80211_ATTR_WIPHY_DYNACK: whether dynamic ack timeout estimation algorithm
+ * is enabled
+ *
* @NL80211_ATTR_MAX: highest attribute number currently defined
* @__NL80211_ATTR_AFTER_LAST: internal use
*/
@@ -1936,6 +1939,8 @@ enum nl80211_attrs {

NL80211_ATTR_TDLS_INITIATOR,

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

__NL80211_ATTR_AFTER_LAST,
diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c
index 082f5c6..c814b45 100644
--- a/net/wireless/nl80211.c
+++ b/net/wireless/nl80211.c
@@ -225,6 +225,7 @@ static const struct nla_policy nl80211_policy[NL80211_ATTR_MAX+1] = {
[NL80211_ATTR_WIPHY_FRAG_THRESHOLD] = { .type = NLA_U32 },
[NL80211_ATTR_WIPHY_RTS_THRESHOLD] = { .type = NLA_U32 },
[NL80211_ATTR_WIPHY_COVERAGE_CLASS] = { .type = NLA_U8 },
+ [NL80211_ATTR_WIPHY_DYNACK] = { .type = NLA_FLAG },

[NL80211_ATTR_IFTYPE] = { .type = NLA_U32 },
[NL80211_ATTR_IFINDEX] = { .type = NLA_U32 },
@@ -2192,6 +2193,12 @@ static int nl80211_set_wiphy(struct sk_buff *skb, struct genl_info *info)
return result;
}

+ if (info->attrs[NL80211_ATTR_WIPHY_DYNACK]) {
+ result = rdev_enable_dynack(rdev);
+ if (result)
+ return result;
+ }
+
changed = 0;

if (info->attrs[NL80211_ATTR_WIPHY_RETRY_SHORT]) {
diff --git a/net/wireless/rdev-ops.h b/net/wireless/rdev-ops.h
index 56c2240..48df1f5 100644
--- a/net/wireless/rdev-ops.h
+++ b/net/wireless/rdev-ops.h
@@ -915,4 +915,15 @@ rdev_set_ap_chanwidth(struct cfg80211_registered_device *rdev,
return ret;
}

+static inline int rdev_enable_dynack(struct cfg80211_registered_device *rdev)
+{
+ int ret;
+
+ trace_rdev_enable_dynack(&rdev->wiphy);
+ ret = rdev->ops->enable_dynack(&rdev->wiphy);
+ trace_rdev_return_int(&rdev->wiphy, ret);
+
+ return ret;
+}
+
#endif /* __CFG80211_RDEV_OPS */
diff --git a/net/wireless/trace.h b/net/wireless/trace.h
index 0c524cd..0c222b5 100644
--- a/net/wireless/trace.h
+++ b/net/wireless/trace.h
@@ -1896,6 +1896,11 @@ TRACE_EVENT(rdev_set_ap_chanwidth,
WIPHY_PR_ARG, NETDEV_PR_ARG, CHAN_DEF_PR_ARG)
);

+DEFINE_EVENT(wiphy_only_evt, rdev_enable_dynack,
+ TP_PROTO(struct wiphy *wiphy),
+ TP_ARGS(wiphy)
+);
+
/*************************************************************
* cfg80211 exported functions traces *
*************************************************************/
--
1.9.1


2014-07-20 12:32:40

by Lorenzo Bianconi

[permalink] [raw]
Subject: [PATCH 2/2] mac80211: add enable_dynack API

Add enable_dynack API to mac80211 in order to enable the estimation
of ack timeout (dynack). Currently dynack is supported just by ath9k

Signed-off-by: Lorenzo Bianconi <[email protected]>
---
include/net/mac80211.h | 4 ++++
net/mac80211/cfg.c | 8 ++++++++
net/mac80211/driver-ops.h | 14 ++++++++++++++
net/mac80211/trace.h | 5 +++++
4 files changed, 31 insertions(+)

diff --git a/include/net/mac80211.h b/include/net/mac80211.h
index 9ce5cb1..723db9b 100644
--- a/include/net/mac80211.h
+++ b/include/net/mac80211.h
@@ -2840,6 +2840,8 @@ enum ieee80211_roc_type {
* @get_expected_throughput: extract the expected throughput towards the
* specified station. The returned value is expressed in Kbps. It returns 0
* if the RC algorithm does not have proper data to provide.
+ *
+ * @enable_dynack: Enable ack timeout estimation algorithm
*/
struct ieee80211_ops {
void (*tx)(struct ieee80211_hw *hw,
@@ -3041,6 +3043,8 @@ struct ieee80211_ops {
int (*join_ibss)(struct ieee80211_hw *hw, struct ieee80211_vif *vif);
void (*leave_ibss)(struct ieee80211_hw *hw, struct ieee80211_vif *vif);
u32 (*get_expected_throughput)(struct ieee80211_sta *sta);
+
+ int (*enable_dynack)(struct ieee80211_hw *hw);
};

/**
diff --git a/net/mac80211/cfg.c b/net/mac80211/cfg.c
index 927b4ea..cb7c514 100644
--- a/net/mac80211/cfg.c
+++ b/net/mac80211/cfg.c
@@ -3481,6 +3481,13 @@ static int ieee80211_set_ap_chanwidth(struct wiphy *wiphy,
return ret;
}

+static int ieee80211_enable_dynack(struct wiphy *wiphy)
+{
+ struct ieee80211_local *local = wiphy_priv(wiphy);
+
+ return drv_enable_dynack(local);
+}
+
const struct cfg80211_ops mac80211_config_ops = {
.add_virtual_intf = ieee80211_add_iface,
.del_virtual_intf = ieee80211_del_iface,
@@ -3557,4 +3564,5 @@ const struct cfg80211_ops mac80211_config_ops = {
.channel_switch = ieee80211_channel_switch,
.set_qos_map = ieee80211_set_qos_map,
.set_ap_chanwidth = ieee80211_set_ap_chanwidth,
+ .enable_dynack = ieee80211_enable_dynack,
};
diff --git a/net/mac80211/driver-ops.h b/net/mac80211/driver-ops.h
index 1142395..5968c00 100644
--- a/net/mac80211/driver-ops.h
+++ b/net/mac80211/driver-ops.h
@@ -1238,4 +1238,18 @@ static inline u32 drv_get_expected_throughput(struct ieee80211_local *local,
return ret;
}

+static inline int drv_enable_dynack(struct ieee80211_local *local)
+{
+ int ret = -EOPNOTSUPP;
+
+ might_sleep();
+
+ trace_drv_enable_dynack(local);
+ if (local->ops->enable_dynack)
+ ret = local->ops->enable_dynack(&local->hw);
+ trace_drv_return_int(local, ret);
+
+ return ret;
+}
+
#endif /* __MAC80211_DRIVER_OPS */
diff --git a/net/mac80211/trace.h b/net/mac80211/trace.h
index 02ac535..7523eee 100644
--- a/net/mac80211/trace.h
+++ b/net/mac80211/trace.h
@@ -1623,6 +1623,11 @@ TRACE_EVENT(drv_get_expected_throughput,
)
);

+DEFINE_EVENT(local_only_evt, drv_enable_dynack,
+ TP_PROTO(struct ieee80211_local *local),
+ TP_ARGS(local)
+);
+
/*
* Tracing for API calls that drivers call.
*/
--
1.9.1


2014-07-22 22:56:08

by Lorenzo Bianconi

[permalink] [raw]
Subject: Re: [PATCH 1/2] cfg80211: add enable_dynack API

> On Sun, 2014-07-20 at 14:32 +0200, Lorenzo Bianconi wrote:
>
>> @@ -2192,6 +2193,12 @@ static int nl80211_set_wiphy(struct sk_buff *skb, struct genl_info *info)
>> return result;
>> }
>>
>> + if (info->attrs[NL80211_ATTR_WIPHY_DYNACK]) {
>> + result = rdev_enable_dynack(rdev);
>> + if (result)
>> + return result;
>> + }
>
> I think it may be better to handle this in the call that's normally used
> (coverage class setting), but allow the other attribute to make a sort
> of "dynamic coverage class". And internally, calling drivers, it seems
> fine to pass -1 or so since you can extend the datatype there, instead
> of introducing a new internal callback. External/internal doesn't always
> have to match perfectly.
>

So we can pass coverage class equal to -1 to lower drivers in order to
enable ack timeout estimation. In this case I have to modify driver
routine signature in p54, ath5k and ath9k drivers. Moreover in
cfg80211/mac80211 stack we can take into account dynamic coverage
class using s16 datatype instead of u8 for coverage_class in wiphy
data structure.

> Additionally, you'll need some capability flag I think.
>
> johannes
>

Best regards,
Lorenzo


--
UNIX is Sexy: who | grep -i blonde | talk; cd ~; wine; talk; touch;
unzip; touch; strip; gasp; finger; gasp; mount; fsck; more; yes; gasp;
umount; make clean; sleep

2014-07-23 14:49:27

by Lorenzo Bianconi

[permalink] [raw]
Subject: Re: [PATCH 1/2] cfg80211: add enable_dynack API

> On Wed, 2014-07-23 at 00:56 +0200, Lorenzo Bianconi wrote:
>
>> > I think it may be better to handle this in the call that's normally used
>> > (coverage class setting), but allow the other attribute to make a sort
>> > of "dynamic coverage class". And internally, calling drivers, it seems
>> > fine to pass -1 or so since you can extend the datatype there, instead
>> > of introducing a new internal callback. External/internal doesn't always
>> > have to match perfectly.
>> >
>>
>> So we can pass coverage class equal to -1 to lower drivers in order to
>> enable ack timeout estimation. In this case I have to modify driver
>> routine signature in p54, ath5k and ath9k drivers. Moreover in
>> cfg80211/mac80211 stack we can take into account dynamic coverage
>> class using s16 datatype instead of u8 for coverage_class in wiphy
>> data structure.
>
> Right. But that seems reasonable, no?

Yes, it sounds good for me. Moreover using s16 as datatype for
coverage_class in wiphy structure allows to detect if dynack is
enabled using iw (i.e. iw phy phy0 info)

>
> johannes
>

Lorenzo

--
UNIX is Sexy: who | grep -i blonde | talk; cd ~; wine; talk; touch;
unzip; touch; strip; gasp; finger; gasp; mount; fsck; more; yes; gasp;
umount; make clean; sleep

2014-07-23 14:29:55

by Johannes Berg

[permalink] [raw]
Subject: Re: [PATCH 1/2] cfg80211: add enable_dynack API

On Wed, 2014-07-23 at 00:56 +0200, Lorenzo Bianconi wrote:

> > I think it may be better to handle this in the call that's normally used
> > (coverage class setting), but allow the other attribute to make a sort
> > of "dynamic coverage class". And internally, calling drivers, it seems
> > fine to pass -1 or so since you can extend the datatype there, instead
> > of introducing a new internal callback. External/internal doesn't always
> > have to match perfectly.
> >
>
> So we can pass coverage class equal to -1 to lower drivers in order to
> enable ack timeout estimation. In this case I have to modify driver
> routine signature in p54, ath5k and ath9k drivers. Moreover in
> cfg80211/mac80211 stack we can take into account dynamic coverage
> class using s16 datatype instead of u8 for coverage_class in wiphy
> data structure.

Right. But that seems reasonable, no?

johannes