[PATCHv4 0/2] configure dynack through mac80211/cfg80211 stack

[PATCHv4 0/2] configure dynack through mac80211/cfg80211 stack

[Lorenzo Bianconi]

Extend set_coverage_class API in order to enable ACK timeout estimation
algorithm (dynack) passing coverage_class equals to -1 to lower drivers.
Currently dynack is supported just by ath9k.
Fix set_coverage_class signature for p54, ath9k, ath9k_htc and ath5k drivers.

Changes since v3:
- squash driver patches and mac80211 one

Changes since v2:
- add set_coverage_class signature fix to patchset
- define NL80211_FEATURE_ACKTO_ESTIMATION to report driver dynack capability
- reject configuration if both NL80211_ATTR_WIPHY_COVERAGE_CLASS and
  NL80211_ATTR_WIPHY_DYN_ACK are given

Changes since v1:
- extend set_coverage_class signature insted of define enable_dynack API

Lorenzo Bianconi (2):
  cfg80211: enable dynack through nl80211
  mac80211: extend set_coverage_class signature

 drivers/net/wireless/ath/ath5k/mac80211-ops.c |  2 +-
 drivers/net/wireless/ath/ath9k/htc_drv_main.c |  2 +-
 drivers/net/wireless/ath/ath9k/main.c         |  3 ++-
 drivers/net/wireless/p54/main.c               |  3 ++-
 include/net/cfg80211.h                        |  2 ++
 include/net/mac80211.h                        |  5 +++--
 include/uapi/linux/nl80211.h                  |  8 ++++++++
 net/mac80211/cfg.c                            |  9 +++++++--
 net/mac80211/driver-ops.h                     |  2 +-
 net/mac80211/trace.h                          |  4 ++--
 net/wireless/nl80211.c                        | 11 +++++++++++
 11 files changed, 40 insertions(+), 11 deletions(-)

-- 
1.9.1

[PATCHv4 1/2] cfg80211: enable dynack through nl80211

[Lorenzo Bianconi]

Define NL80211_ATTR_WIPHY_DYN_ACK attribute in order to enable ack timeout
estimation algorithm (dynack) using set_coverage_class codepath.
Define NL80211_FEATURE_ACKTO_ESTIMATION flag, which is set by lower drivers to
show dynack capability

Signed-off-by: Lorenzo Bianconi <lorenzo.bianconi83@gmail.com>
---
 include/net/cfg80211.h       |  2 ++
 include/uapi/linux/nl80211.h |  8 ++++++++
 net/wireless/nl80211.c       | 11 +++++++++++
 3 files changed, 21 insertions(+)

diff --git a/include/net/cfg80211.h b/include/net/cfg80211.h
index 0a080c4..ef7f250 100644
--- a/include/net/cfg80211.h
+++ b/include/net/cfg80211.h
@@ -1800,6 +1800,7 @@ struct cfg80211_connect_params {
  * @WIPHY_PARAM_FRAG_THRESHOLD: wiphy->frag_threshold has changed
  * @WIPHY_PARAM_RTS_THRESHOLD: wiphy->rts_threshold has changed
  * @WIPHY_PARAM_COVERAGE_CLASS: coverage class changed
+ * @WIPHY_PARAM_DYN_ACK: dynack has been enabled
  */
 enum wiphy_params_flags {
 	WIPHY_PARAM_RETRY_SHORT		= 1 << 0,
@@ -1807,6 +1808,7 @@ enum wiphy_params_flags {
 	WIPHY_PARAM_FRAG_THRESHOLD	= 1 << 2,
 	WIPHY_PARAM_RTS_THRESHOLD	= 1 << 3,
 	WIPHY_PARAM_COVERAGE_CLASS	= 1 << 4,
+	WIPHY_PARAM_DYN_ACK		= 1 << 5,
 };
 
 /*
diff --git a/include/uapi/linux/nl80211.h b/include/uapi/linux/nl80211.h
index f1db15b..0b2d55d 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_DYN_ACK: 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_DYN_ACK,
+
 	/* add attributes here, update the policy in nl80211.c */
 
 	__NL80211_ATTR_AFTER_LAST,
@@ -3956,6 +3961,8 @@ enum nl80211_ap_sme_features {
  * @NL80211_FEATURE_AP_MODE_CHAN_WIDTH_CHANGE: This driver supports dynamic
  *	channel bandwidth change (e.g., HT 20 <-> 40 MHz channel) during the
  *	lifetime of a BSS.
+ * @NL80211_FEATURE_ACKTO_ESTIMATION: This driver supports dynamic ACK timeout
+ *	estimation.
  */
 enum nl80211_feature_flags {
 	NL80211_FEATURE_SK_TX_STATUS			= 1 << 0,
@@ -3977,6 +3984,7 @@ enum nl80211_feature_flags {
 	NL80211_FEATURE_USERSPACE_MPM			= 1 << 16,
 	NL80211_FEATURE_ACTIVE_MONITOR			= 1 << 17,
 	NL80211_FEATURE_AP_MODE_CHAN_WIDTH_CHANGE	= 1 << 18,
+	NL80211_FEATURE_ACKTO_ESTIMATION		= 1 << 19,
 };
 
 /**
diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c
index df7b133..b00cd11 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_DYN_ACK] = { .type = NLA_FLAG },
 
 	[NL80211_ATTR_IFTYPE] = { .type = NLA_U32 },
 	[NL80211_ATTR_IFINDEX] = { .type = NLA_U32 },
@@ -2237,11 +2238,21 @@ static int nl80211_set_wiphy(struct sk_buff *skb, struct genl_info *info)
 	}
 
 	if (info->attrs[NL80211_ATTR_WIPHY_COVERAGE_CLASS]) {
+		if (info->attrs[NL80211_ATTR_WIPHY_DYN_ACK])
+			return -EINVAL;
+
 		coverage_class = nla_get_u8(
 			info->attrs[NL80211_ATTR_WIPHY_COVERAGE_CLASS]);
 		changed |= WIPHY_PARAM_COVERAGE_CLASS;
 	}
 
+	if (info->attrs[NL80211_ATTR_WIPHY_DYN_ACK]) {
+		if (!(rdev->wiphy.features & NL80211_FEATURE_ACKTO_ESTIMATION))
+			return -EOPNOTSUPP;
+
+		changed |= WIPHY_PARAM_DYN_ACK;
+	}
+
 	if (changed) {
 		u8 old_retry_short, old_retry_long;
 		u32 old_frag_threshold, old_rts_threshold;
-- 
1.9.1

[PATCHv4 2/2] mac80211: extend set_coverage_class signature

[Lorenzo Bianconi]

Extend set_coverage_class API in order to enable ACK timeout estimation
algorithm (dynack) passing coverage_class equals to -1 to lower drivers.
Synchronize set_coverage_class routine signature with mac80211 function pointer
for p54, ath9k, ath9k_htc and ath5k drivers.

Signed-off-by: Lorenzo Bianconi <lorenzo.bianconi83@gmail.com>
---
 drivers/net/wireless/ath/ath5k/mac80211-ops.c | 2 +-
 drivers/net/wireless/ath/ath9k/htc_drv_main.c | 2 +-
 drivers/net/wireless/ath/ath9k/main.c         | 3 ++-
 drivers/net/wireless/p54/main.c               | 3 ++-
 include/net/mac80211.h                        | 5 +++--
 net/mac80211/cfg.c                            | 9 +++++++--
 net/mac80211/driver-ops.h                     | 2 +-
 net/mac80211/trace.h                          | 4 ++--
 8 files changed, 19 insertions(+), 11 deletions(-)

diff --git a/drivers/net/wireless/ath/ath5k/mac80211-ops.c b/drivers/net/wireless/ath/ath5k/mac80211-ops.c
index b65c38f..ab2709a 100644
--- a/drivers/net/wireless/ath/ath5k/mac80211-ops.c
+++ b/drivers/net/wireless/ath/ath5k/mac80211-ops.c
@@ -704,7 +704,7 @@ ath5k_get_survey(struct ieee80211_hw *hw, int idx, struct survey_info *survey)
  * reset.
  */
 static void
-ath5k_set_coverage_class(struct ieee80211_hw *hw, u8 coverage_class)
+ath5k_set_coverage_class(struct ieee80211_hw *hw, s16 coverage_class)
 {
 	struct ath5k_hw *ah = hw->priv;
 
diff --git a/drivers/net/wireless/ath/ath9k/htc_drv_main.c b/drivers/net/wireless/ath/ath9k/htc_drv_main.c
index 5627917..994fff1 100644
--- a/drivers/net/wireless/ath/ath9k/htc_drv_main.c
+++ b/drivers/net/wireless/ath/ath9k/htc_drv_main.c
@@ -1722,7 +1722,7 @@ static int ath9k_htc_set_rts_threshold(struct ieee80211_hw *hw, u32 value)
 }
 
 static void ath9k_htc_set_coverage_class(struct ieee80211_hw *hw,
-					 u8 coverage_class)
+					 s16 coverage_class)
 {
 	struct ath9k_htc_priv *priv = hw->priv;
 
diff --git a/drivers/net/wireless/ath/ath9k/main.c b/drivers/net/wireless/ath/ath9k/main.c
index e6ac8d2..97c028e 100644
--- a/drivers/net/wireless/ath/ath9k/main.c
+++ b/drivers/net/wireless/ath/ath9k/main.c
@@ -1959,7 +1959,8 @@ static int ath9k_get_survey(struct ieee80211_hw *hw, int idx,
 	return 0;
 }
 
-static void ath9k_set_coverage_class(struct ieee80211_hw *hw, u8 coverage_class)
+static void ath9k_set_coverage_class(struct ieee80211_hw *hw,
+				     s16 coverage_class)
 {
 	struct ath_softc *sc = hw->priv;
 	struct ath_hw *ah = sc->sc_ah;
diff --git a/drivers/net/wireless/p54/main.c b/drivers/net/wireless/p54/main.c
index 7be3a48..97aeff0 100644
--- a/drivers/net/wireless/p54/main.c
+++ b/drivers/net/wireless/p54/main.c
@@ -696,7 +696,8 @@ static void p54_flush(struct ieee80211_hw *dev, struct ieee80211_vif *vif,
 	WARN(total, "tx flush timeout, unresponsive firmware");
 }
 
-static void p54_set_coverage_class(struct ieee80211_hw *dev, u8 coverage_class)
+static void p54_set_coverage_class(struct ieee80211_hw *dev,
+				   s16 coverage_class)
 {
 	struct p54_common *priv = dev->priv;
 
diff --git a/include/net/mac80211.h b/include/net/mac80211.h
index dae2e24..3a43f66 100644
--- a/include/net/mac80211.h
+++ b/include/net/mac80211.h
@@ -2666,7 +2666,8 @@ enum ieee80211_roc_type {
  *
  * @set_coverage_class: Set slot time for given coverage class as specified
  *	in IEEE 802.11-2007 section 17.3.8.6 and modify ACK timeout
- *	accordingly. This callback is not required and may sleep.
+ *	accordingly; coverage_class equals to -1 to enable ACK timeout
+ *	estimation algorithm. This callback is not required and may sleep.
  *
  * @testmode_cmd: Implement a cfg80211 test mode command. The passed @vif may
  *	be %NULL. The callback can sleep.
@@ -2950,7 +2951,7 @@ struct ieee80211_ops {
 	int (*get_survey)(struct ieee80211_hw *hw, int idx,
 		struct survey_info *survey);
 	void (*rfkill_poll)(struct ieee80211_hw *hw);
-	void (*set_coverage_class)(struct ieee80211_hw *hw, u8 coverage_class);
+	void (*set_coverage_class)(struct ieee80211_hw *hw, s16 coverage_class);
 #ifdef CONFIG_NL80211_TESTMODE
 	int (*testmode_cmd)(struct ieee80211_hw *hw, struct ieee80211_vif *vif,
 			    void *data, int len);
diff --git a/net/mac80211/cfg.c b/net/mac80211/cfg.c
index 927b4ea..5927822 100644
--- a/net/mac80211/cfg.c
+++ b/net/mac80211/cfg.c
@@ -1984,8 +1984,13 @@ static int ieee80211_set_wiphy_params(struct wiphy *wiphy, u32 changed)
 			return err;
 	}
 
-	if (changed & WIPHY_PARAM_COVERAGE_CLASS) {
-		err = drv_set_coverage_class(local, wiphy->coverage_class);
+	if ((changed & WIPHY_PARAM_COVERAGE_CLASS) ||
+	    (changed & WIPHY_PARAM_DYN_ACK)) {
+		s16 coverage_class;
+
+		coverage_class = (changed & WIPHY_PARAM_COVERAGE_CLASS)
+				  ? wiphy->coverage_class : -1;
+		err = drv_set_coverage_class(local, coverage_class);
 
 		if (err)
 			return err;
diff --git a/net/mac80211/driver-ops.h b/net/mac80211/driver-ops.h
index 1142395..196d48c 100644
--- a/net/mac80211/driver-ops.h
+++ b/net/mac80211/driver-ops.h
@@ -450,7 +450,7 @@ static inline int drv_set_rts_threshold(struct ieee80211_local *local,
 }
 
 static inline int drv_set_coverage_class(struct ieee80211_local *local,
-					 u8 value)
+					 s16 value)
 {
 	int ret = 0;
 	might_sleep();
diff --git a/net/mac80211/trace.h b/net/mac80211/trace.h
index 02ac535..38fae7e 100644
--- a/net/mac80211/trace.h
+++ b/net/mac80211/trace.h
@@ -672,13 +672,13 @@ DEFINE_EVENT(local_u32_evt, drv_set_rts_threshold,
 );
 
 TRACE_EVENT(drv_set_coverage_class,
-	TP_PROTO(struct ieee80211_local *local, u8 value),
+	TP_PROTO(struct ieee80211_local *local, s16 value),
 
 	TP_ARGS(local, value),
 
 	TP_STRUCT__entry(
 		LOCAL_ENTRY
-		__field(u8, value)
+		__field(s16, value)
 	),
 
 	TP_fast_assign(
-- 
1.9.1

Re: [PATCHv4 1/2] cfg80211: enable dynack through nl80211

[Johannes Berg]

> diff --git a/include/uapi/linux/nl80211.h b/include/uapi/linux/nl80211.h
> index f1db15b..0b2d55d 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_DYN_ACK: whether dynamic ack timeout estimation
> + *	algorithm is enabled

You should probably describe that it's related to the
NL80211_FEATURE_ACKTO_ESTIMATION feature (and you should document that
it's a flag) since otherwise nobody will be able to correlate the two
things easily. :)


> +       if (info->attrs[NL80211_ATTR_WIPHY_DYN_ACK]) {
> +               if (!(rdev->wiphy.features &
> NL80211_FEATURE_ACKTO_ESTIMATION))
> +                       return -EOPNOTSUPP;
> +
> +               changed |= WIPHY_PARAM_DYN_ACK;
> +       }

that doesn't really seem right, how do you turn it off??

johannes

Re: [PATCHv4 1/2] cfg80211: enable dynack through nl80211

[Lorenzo Bianconi]

>> diff --git a/include/uapi/linux/nl80211.h b/include/uapi/linux/nl80211.h
>> index f1db15b..0b2d55d 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_DYN_ACK: whether dynamic ack timeout estimation
>> + *   algorithm is enabled
>
> You should probably describe that it's related to the
> NL80211_FEATURE_ACKTO_ESTIMATION feature (and you should document that
> it's a flag) since otherwise nobody will be able to correlate the two
> things easily. :)
>
>
>> +       if (info->attrs[NL80211_ATTR_WIPHY_DYN_ACK]) {
>> +               if (!(rdev->wiphy.features &
>> NL80211_FEATURE_ACKTO_ESTIMATION))
>> +                       return -EOPNOTSUPP;
>> +
>> +               changed |= WIPHY_PARAM_DYN_ACK;
>> +       }
>
> that doesn't really seem right, how do you turn it off??
>

dynack is disable by default at bootstrap and it is automatically
disabled as soon as you set coverage_class (coverage_class >= 0) after
dynack has been enabled (coverage_class = -1). E.g:

- iw phy phy0 set distance auto (to enable dynack)

- iw phy phy0 set distance x (to set coverage class and disable dynack)


This logic is implemented in dynack code. Is it fine for you?
Anyway documentation should be clearer on that stuff. :)

> johannes
>

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

Re: [PATCHv4 1/2] cfg80211: enable dynack through nl80211

[Johannes Berg]

Hi,

Sorry for the delay.

> dynack is disable by default at bootstrap and it is automatically
> disabled as soon as you set coverage_class (coverage_class >= 0) after
> dynack has been enabled (coverage_class = -1). E.g:
> 
> - iw phy phy0 set distance auto (to enable dynack)
> 
> - iw phy phy0 set distance x (to set coverage class and disable dynack)
> 
> 
> This logic is implemented in dynack code. Is it fine for you?

Ok, I guess that's reasonable. The default coverage class is 0 I guess?

> Anyway documentation should be clearer on that stuff. :)

Please extend it then? :)

johannes

Re: [PATCHv4 1/2] cfg80211: enable dynack through nl80211

[Lorenzo Bianconi]

> Hi,
>
> Sorry for the delay.
>
>> dynack is disable by default at bootstrap and it is automatically
>> disabled as soon as you set coverage_class (coverage_class >= 0) after
>> dynack has been enabled (coverage_class = -1). E.g:
>>
>> - iw phy phy0 set distance auto (to enable dynack)
>>
>> - iw phy phy0 set distance x (to set coverage class and disable dynack)
>>
>>
>> This logic is implemented in dynack code. Is it fine for you?
>
> Ok, I guess that's reasonable. The default coverage class is 0 I guess?

Yes, dynack is disabled at bootstrap and default coverage class is 0

>
>> Anyway documentation should be clearer on that stuff. :)
>
> Please extend it then? :)
>

I will send a new patchset with more comments on that stuff :)

> johannes
>

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

Re: [PATCHv4 1/2] cfg80211: enable dynack through nl80211

[Johannes Berg]

On Fri, 2014-08-29 at 22:46 +0200, Lorenzo Bianconi wrote:
> > Hi,
> >
> > Sorry for the delay.
> >
> >> dynack is disable by default at bootstrap and it is automatically
> >> disabled as soon as you set coverage_class (coverage_class >= 0) after
> >> dynack has been enabled (coverage_class = -1). E.g:
> >>
> >> - iw phy phy0 set distance auto (to enable dynack)
> >>
> >> - iw phy phy0 set distance x (to set coverage class and disable dynack)
> >>
> >>
> >> This logic is implemented in dynack code. Is it fine for you?
> >
> > Ok, I guess that's reasonable. The default coverage class is 0 I guess?
> 
> Yes, dynack is disabled at bootstrap and default coverage class is 0
> 
> >
> >> Anyway documentation should be clearer on that stuff. :)
> >
> > Please extend it then? :)
> >
> 
> I will send a new patchset with more comments on that stuff :)

Please also include a better commit message actually explaining the
feature :)

johannes