[RFC] mac80211: Use libnl-configurable values for retry counts

[RFC] mac80211: Use libnl-configurable values for retry counts

[Jean-Pierre Tosoni]

From: J.P. Tosoni <jp.tosoni@acksys.fr>

In the rate control algorithms, the maximum retry count is limited by
a) a constant value obtained from the hardware driver
b) a constant limit (6ms) on the time allowed for all
   retries of each frame.

Replace the retry count by existing configurable values from nl80211.
Use wiphy->retry_short for management frames.
Use wiphy->retry_long for other frames.
Check that the configured value does not exceed driver capabilities.

Caveat: in minstrel rate control, the retry count is reused for each
rate of the rate table, potentially resulting in 4*max_retry retries.
---
Please comment on the patch and the caveat above, then I'll proceed
to update iw and to change the hard limit of 30 retries in ath9k.

 net/mac80211/cfg.c              |    4 ++++
 net/mac80211/rate.c             |    2 +-
 net/mac80211/rc80211_minstrel.c |    6 +-----
 3 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/net/mac80211/cfg.c b/net/mac80211/cfg.c
index 082f270..4d3eb56 100644
--- a/net/mac80211/cfg.c
+++ b/net/mac80211/cfg.c
@@ -2203,11 +2203,15 @@ static int ieee80211_set_wiphy_params(struct wiphy *wiphy, u32 changed)
 	if (changed & WIPHY_PARAM_RETRY_SHORT) {
 		if (wiphy->retry_short > IEEE80211_MAX_TX_RETRY)
 			return -EINVAL;
+		if (wiphy->retry_short > local->hw.max_rate_tries)
+			return -EINVAL;
 		local->hw.conf.short_frame_max_tx_count = wiphy->retry_short;
 	}
 	if (changed & WIPHY_PARAM_RETRY_LONG) {
 		if (wiphy->retry_long > IEEE80211_MAX_TX_RETRY)
 			return -EINVAL;
+		if (wiphy->retry_long > local->hw.max_rate_tries)
+			return -EINVAL;
 		local->hw.conf.long_frame_max_tx_count = wiphy->retry_long;
 	}
 	if (changed &
diff --git a/net/mac80211/rate.c b/net/mac80211/rate.c
index a02bef3..9be3006 100644
--- a/net/mac80211/rate.c
+++ b/net/mac80211/rate.c
@@ -266,7 +266,7 @@ static void __rate_control_send_low(struct ieee80211_hw *hw,
 
 	info->control.rates[0].count =
 		(info->flags & IEEE80211_TX_CTL_NO_ACK) ?
-		1 : hw->max_rate_tries;
+		1 : hw->conf.short_frame_max_tx_count;
 
 	info->control.skip_table = 1;
 }
diff --git a/net/mac80211/rc80211_minstrel.c b/net/mac80211/rc80211_minstrel.c
index ac7ef54..502d0c9 100644
--- a/net/mac80211/rc80211_minstrel.c
+++ b/net/mac80211/rc80211_minstrel.c
@@ -592,11 +592,7 @@ minstrel_alloc(struct ieee80211_hw *hw, struct dentry *debugfsdir)
 	/* maximum time that the hw is allowed to stay in one MRR segment */
 	mp->segment_size = 6000;
 
-	if (hw->max_rate_tries > 0)
-		mp->max_retry = hw->max_rate_tries;
-	else
-		/* safe default, does not necessarily have to match hw properties */
-		mp->max_retry = 7;
+	mp->max_retry = hw->conf.long_frame_max_tx_count;
 
 	if (hw->max_rates >= 4)
 		mp->has_mrr = true;
-- 
1.7.2.5

Re: [RFC] mac80211: Use libnl-configurable values for retry counts

[Felix Fietkau]

On 2013-06-26 7:28 PM, Jean-Pierre Tosoni wrote:
> From: J.P. Tosoni <jp.tosoni@acksys.fr>
> 
> In the rate control algorithms, the maximum retry count is limited by
> a) a constant value obtained from the hardware driver
> b) a constant limit (6ms) on the time allowed for all
>    retries of each frame.
> 
> Replace the retry count by existing configurable values from nl80211.
> Use wiphy->retry_short for management frames.
> Use wiphy->retry_long for other frames.
That seems a bit arbitrary.

> diff --git a/net/mac80211/rc80211_minstrel.c b/net/mac80211/rc80211_minstrel.c
> index ac7ef54..502d0c9 100644
> --- a/net/mac80211/rc80211_minstrel.c
> +++ b/net/mac80211/rc80211_minstrel.c
> @@ -592,11 +592,7 @@ minstrel_alloc(struct ieee80211_hw *hw, struct dentry *debugfsdir)
>  	/* maximum time that the hw is allowed to stay in one MRR segment */
>  	mp->segment_size = 6000;
>  
> -	if (hw->max_rate_tries > 0)
> -		mp->max_retry = hw->max_rate_tries;
> -	else
> -		/* safe default, does not necessarily have to match hw properties */
> -		mp->max_retry = 7;
> +	mp->max_retry = hw->conf.long_frame_max_tx_count;
minstrel_alloc is called at rate control init time. You're not making
max_retry configurable here, it'll pick the default value of
hw->conf.long_frame_max_tx_count and never update it again.

- Felix

RE: [RFC] mac80211: Use libnl-configurable values for retry counts

[Jean-Pierre Tosoni]

First, thanks for your prompt reply and for pointing out my mistakes.

> > Use wiphy->retry_short for management frames.
> > Use wiphy->retry_long for other frames.
> That seems a bit arbitrary.

Indeed.
Should I use the following?
  retry_long for data frames subject to RTS
  retry_short for all other frames.
That looks closer to the descriptions in 802.11-2012.
 
> > +	mp->max_retry = hw->conf.long_frame_max_tx_count;
> minstrel_alloc is called at rate control init time. You're not making
> max_retry configurable here, it'll pick the default value of
> hw->conf.long_frame_max_tx_count and never update it again.

Is it OK if I replace max_retry with the config values in minstrel_rate_init
and minstrel_update_rates instead?
Then we can get rid of max_retry completely?

(Sorry, I got mixed in the never-ending sequence of callbacks :)

> > diff --git a/net/mac80211/rc80211_minstrel.c
> b/net/mac80211/rc80211_minstrel.c
> > index ac7ef54..502d0c9 100644
> > --- a/net/mac80211/rc80211_minstrel.c
> > +++ b/net/mac80211/rc80211_minstrel.c
> > @@ -592,11 +592,7 @@ minstrel_alloc(
> >	/* maximum time that the hw is allowed to stay in one MRR  segment
*/
> >  	mp->segment_size = 6000;
> >
> > -	if (hw->max_rate_tries > 0)
> > -		mp->max_retry = hw->max_rate_tries;
> > -	else
> > -		/* safe default, does not necessarily have to match hw
properties */
> > -		mp->max_retry = 7;
> > +	mp->max_retry = hw->conf.long_frame_max_tx_count;> 
> - Felix

Jean-Pierre