Extend set_coverage_class API in order to enable ACK timeout estimation
algorithm (dynack). Currently dynack is supported just by ath9k. In a different
patchset I will fix signature issue for p54, ath9k_htc and ath5k drivers.
Changes since v1:
- extend set_coverage_class signature insted of define enable_dynack API
Lorenzo Bianconi (2):
cfg80211: extend coverage_class datatype
mac80211: extend set_coverage_class signature
include/net/cfg80211.h | 5 +++--
include/net/mac80211.h | 5 +++--
include/uapi/linux/nl80211.h | 5 +++++
net/mac80211/driver-ops.h | 9 +++------
net/mac80211/trace.h | 4 ++--
net/wireless/nl80211.c | 14 ++++++++++----
6 files changed, 26 insertions(+), 16 deletions(-)
--
1.9.1
Use s16 as datatype for coverage_class in order to enable ack timeout
estimation algorithm through nl80211
Signed-off-by: Lorenzo Bianconi <[email protected]>
---
include/net/cfg80211.h | 5 +++--
include/uapi/linux/nl80211.h | 5 +++++
net/wireless/nl80211.c | 14 ++++++++++----
3 files changed, 18 insertions(+), 6 deletions(-)
diff --git a/include/net/cfg80211.h b/include/net/cfg80211.h
index 0a080c4..1c8e281 100644
--- a/include/net/cfg80211.h
+++ b/include/net/cfg80211.h
@@ -2876,7 +2876,8 @@ struct wiphy_vendor_command {
* include fixed IEs like supported rates
* @max_sched_scan_ie_len: same as max_scan_ie_len, but for scheduled
* scans
- * @coverage_class: current coverage class
+ * @coverage_class: current coverage class, -1 if dynamic ack timeout
+ * estimation has been enabled
* @fw_version: firmware version for ethtool reporting
* @hw_version: hardware version for ethtool reporting
* @max_num_pmkids: maximum number of PMKIDs supported by device
@@ -2990,7 +2991,7 @@ struct wiphy {
u8 retry_long;
u32 frag_threshold;
u32 rts_threshold;
- u8 coverage_class;
+ s16 coverage_class;
char fw_version[ETHTOOL_FWVERS_LEN];
u32 hw_version;
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 df7b133..c097827 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 },
@@ -1269,8 +1270,8 @@ static int nl80211_send_wiphy(struct cfg80211_registered_device *rdev,
rdev->wiphy.frag_threshold) ||
nla_put_u32(msg, NL80211_ATTR_WIPHY_RTS_THRESHOLD,
rdev->wiphy.rts_threshold) ||
- nla_put_u8(msg, NL80211_ATTR_WIPHY_COVERAGE_CLASS,
- rdev->wiphy.coverage_class) ||
+ nla_put_s16(msg, NL80211_ATTR_WIPHY_COVERAGE_CLASS,
+ rdev->wiphy.coverage_class) ||
nla_put_u8(msg, NL80211_ATTR_MAX_NUM_SCAN_SSIDS,
rdev->wiphy.max_scan_ssids) ||
nla_put_u8(msg, NL80211_ATTR_MAX_NUM_SCHED_SCAN_SSIDS,
@@ -2046,7 +2047,7 @@ static int nl80211_set_wiphy(struct sk_buff *skb, struct genl_info *info)
u32 changed;
u8 retry_short = 0, retry_long = 0;
u32 frag_threshold = 0, rts_threshold = 0;
- u8 coverage_class = 0;
+ s16 coverage_class = 0;
ASSERT_RTNL();
@@ -2242,10 +2243,15 @@ static int nl80211_set_wiphy(struct sk_buff *skb, struct genl_info *info)
changed |= WIPHY_PARAM_COVERAGE_CLASS;
}
+ if (info->attrs[NL80211_ATTR_WIPHY_DYNACK]) {
+ coverage_class = -1;
+ changed |= WIPHY_PARAM_COVERAGE_CLASS;
+ }
+
if (changed) {
u8 old_retry_short, old_retry_long;
u32 old_frag_threshold, old_rts_threshold;
- u8 old_coverage_class;
+ s16 old_coverage_class;
if (!rdev->ops->set_wiphy_params)
return -EOPNOTSUPP;
--
1.9.1
> On Thu, 2014-07-24 at 14:12 +0200, Lorenzo Bianconi wrote:
>> > On Thu, 2014-07-24 at 12:39 +0200, Lorenzo Bianconi wrote:
>> >
>> >> @@ -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);
>> >> + int (*set_coverage_class)(struct ieee80211_hw *hw, s16 coverage_class);
>> >
>> > This patch also needs to update all drivers ......
>>
>> Currently just ath9k, ath5k and p54 implement set_coverage_class
>> function pointer. I would send a different patch for these changes.
>> Should I include them in this patchset?
>
> Guess what will happen if I merge this patch?
>
> Hint: the kernel will not compile correctly any more.
>
> johannes
>
Ok, I will include changes to lower drivers in this patchset.
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
On Thu, 2014-07-24 at 14:21 +0200, Lorenzo Bianconi wrote:
> Currently iw does not allow to set both attribute at the same time,
> but I can use "if-else if" statement in order to exclude that
> condition.
I think you should explicitly reject the configuration if both
attributes are given, not silently use one of them.
> > You should also have a feature flag that drivers set that you test here
> > and refuse dynamic if it's not supported, I guess?
> >
>
> I would rely on lower drivers to fail (returning -EOPNOTSUPP to
> set_coverage_class caller) if dynack it is not supported and user is
> trying to enable it. Does it sound good for you?
I imagined you'd want to know in 'iw list' whether or not it's supported
too.
johannes
On Thu, 2014-07-24 at 12:39 +0200, Lorenzo Bianconi wrote:
> @@ -1269,8 +1270,8 @@ static int nl80211_send_wiphy(struct cfg80211_registered_device *rdev,
> rdev->wiphy.frag_threshold) ||
> nla_put_u32(msg, NL80211_ATTR_WIPHY_RTS_THRESHOLD,
> rdev->wiphy.rts_threshold) ||
> - nla_put_u8(msg, NL80211_ATTR_WIPHY_COVERAGE_CLASS,
> - rdev->wiphy.coverage_class) ||
> + nla_put_s16(msg, NL80211_ATTR_WIPHY_COVERAGE_CLASS,
> + rdev->wiphy.coverage_class) ||
> nla_put_u8(msg, NL80211_ATTR_MAX_NUM_SCAN_SSIDS,
> rdev->wiphy.max_scan_ssids) ||
> nla_put_u8(msg, NL80211_ATTR_MAX_NUM_SCHED_SCAN_SSIDS,
This will break existing userspace.
Also please supply a better commit message.
johannes
> On Thu, 2014-07-24 at 12:39 +0200, Lorenzo Bianconi wrote:
>
>> @@ -2242,10 +2243,15 @@ static int nl80211_set_wiphy(struct sk_buff *skb, struct genl_info *info)
>> changed |= WIPHY_PARAM_COVERAGE_CLASS;
>> }
>>
>> + if (info->attrs[NL80211_ATTR_WIPHY_DYNACK]) {
>> + coverage_class = -1;
>> + changed |= WIPHY_PARAM_COVERAGE_CLASS;
>> + }
>
> I think you should also reject having both dyn and fixed attributes.
>
Currently iw does not allow to set both attribute at the same time,
but I can use "if-else if" statement in order to exclude that
condition.
> You should also have a feature flag that drivers set that you test here
> and refuse dynamic if it's not supported, I guess?
>
I would rely on lower drivers to fail (returning -EOPNOTSUPP to
set_coverage_class caller) if dynack it is not supported and user is
trying to enable it. Does it sound good for you?
> 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
> On Thu, 2014-07-24 at 12:39 +0200, Lorenzo Bianconi wrote:
>
>> @@ -1269,8 +1270,8 @@ static int nl80211_send_wiphy(struct cfg80211_registered_device *rdev,
>> rdev->wiphy.frag_threshold) ||
>> nla_put_u32(msg, NL80211_ATTR_WIPHY_RTS_THRESHOLD,
>> rdev->wiphy.rts_threshold) ||
>> - nla_put_u8(msg, NL80211_ATTR_WIPHY_COVERAGE_CLASS,
>> - rdev->wiphy.coverage_class) ||
>> + nla_put_s16(msg, NL80211_ATTR_WIPHY_COVERAGE_CLASS,
>> + rdev->wiphy.coverage_class) ||
>> nla_put_u8(msg, NL80211_ATTR_MAX_NUM_SCAN_SSIDS,
>> rdev->wiphy.max_scan_ssids) ||
>> nla_put_u8(msg, NL80211_ATTR_MAX_NUM_SCHED_SCAN_SSIDS,
>
> This will break existing userspace.
>
> Also please supply a better commit message.
>
> johannes
>
Ack
--
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
Extend set_coverage_class API in order to enable ACK timeout estimation
algorithm (dynack).
Signed-off-by: Lorenzo Bianconi <[email protected]>
---
include/net/mac80211.h | 5 +++--
net/mac80211/driver-ops.h | 9 +++------
net/mac80211/trace.h | 4 ++--
3 files changed, 8 insertions(+), 10 deletions(-)
diff --git a/include/net/mac80211.h b/include/net/mac80211.h
index dae2e24..30fd354 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);
+ int (*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/driver-ops.h b/net/mac80211/driver-ops.h
index 1142395..68a840d 100644
--- a/net/mac80211/driver-ops.h
+++ b/net/mac80211/driver-ops.h
@@ -450,17 +450,14 @@ 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;
+ int ret = -EOPNOTSUPP;
might_sleep();
trace_drv_set_coverage_class(local, value);
if (local->ops->set_coverage_class)
- local->ops->set_coverage_class(&local->hw, value);
- else
- ret = -EOPNOTSUPP;
-
+ ret = local->ops->set_coverage_class(&local->hw, value);
trace_drv_return_int(local, ret);
return ret;
}
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
> On Thu, 2014-07-24 at 14:21 +0200, Lorenzo Bianconi wrote:
>
>> Currently iw does not allow to set both attribute at the same time,
>> but I can use "if-else if" statement in order to exclude that
>> condition.
>
> I think you should explicitly reject the configuration if both
> attributes are given, not silently use one of them.
>
Ack
>> > You should also have a feature flag that drivers set that you test here
>> > and refuse dynamic if it's not supported, I guess?
>> >
>>
>> I would rely on lower drivers to fail (returning -EOPNOTSUPP to
>> set_coverage_class caller) if dynack it is not supported and user is
>> trying to enable it. Does it sound good for you?
>
> I imagined you'd want to know in 'iw list' whether or not it's supported
> too.
>
Ok, I can add an entry in nl80211_feature_flags, something like
NL80211_FEATURE_ACKTO_ESTIMATION. This flags will be set by lower
driver and will be checked to very if driver supports this feature.
Moreover NL80211_FEATURE_ACKTO_ESTIMATION will be reported to
userspace.
> 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
On Thu, 2014-07-24 at 12:39 +0200, Lorenzo Bianconi wrote:
> @@ -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);
> + int (*set_coverage_class)(struct ieee80211_hw *hw, s16 coverage_class);
This patch also needs to update all drivers ......
johannes
> On Thu, 2014-07-24 at 12:39 +0200, Lorenzo Bianconi wrote:
>
>> @@ -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);
>> + int (*set_coverage_class)(struct ieee80211_hw *hw, s16 coverage_class);
>
> This patch also needs to update all drivers ......
Currently just ath9k, ath5k and p54 implement set_coverage_class
function pointer. I would send a different patch for these changes.
Should I include them in this patchset?
>
> 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
On Thu, 2014-07-24 at 12:39 +0200, Lorenzo Bianconi wrote:
> @@ -2242,10 +2243,15 @@ static int nl80211_set_wiphy(struct sk_buff *skb, struct genl_info *info)
> changed |= WIPHY_PARAM_COVERAGE_CLASS;
> }
>
> + if (info->attrs[NL80211_ATTR_WIPHY_DYNACK]) {
> + coverage_class = -1;
> + changed |= WIPHY_PARAM_COVERAGE_CLASS;
> + }
I think you should also reject having both dyn and fixed attributes.
You should also have a feature flag that drivers set that you test here
and refuse dynamic if it's not supported, I guess?
johannes
On Thu, 2014-07-24 at 14:12 +0200, Lorenzo Bianconi wrote:
> > On Thu, 2014-07-24 at 12:39 +0200, Lorenzo Bianconi wrote:
> >
> >> @@ -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);
> >> + int (*set_coverage_class)(struct ieee80211_hw *hw, s16 coverage_class);
> >
> > This patch also needs to update all drivers ......
>
> Currently just ath9k, ath5k and p54 implement set_coverage_class
> function pointer. I would send a different patch for these changes.
> Should I include them in this patchset?
Guess what will happen if I merge this patch?
Hint: the kernel will not compile correctly any more.
johannes