[PATCH v2 1/2] mac80211: minstrel_ht: pick only supported rates for sta and group max*rates

[PATCH v2 1/2] mac80211: minstrel_ht: pick only supported rates for sta and group max*rates

[Karl Beldan]

From: Karl Beldan <karl.beldan@rivierawaves.com>

minstrel_ht initializes max_tp_rate max_tp_rate2 and max_prob_rate to
zero both for minstrel_ht_sta and minstrel_mcs_group_data.
This is wrong since there is no guarantee that the 1st rate of any
group is supported.

Signed-off-by: Karl Beldan <karl.beldan@rivierawaves.com>
---
 net/mac80211/rc80211_minstrel_ht.c | 21 +++++++++++++++------
 1 file changed, 15 insertions(+), 6 deletions(-)

diff --git a/net/mac80211/rc80211_minstrel_ht.c b/net/mac80211/rc80211_minstrel_ht.c
index d2b264d..eb6dc3f 100644
--- a/net/mac80211/rc80211_minstrel_ht.c
+++ b/net/mac80211/rc80211_minstrel_ht.c
@@ -244,6 +244,7 @@ minstrel_ht_update_stats(struct minstrel_priv *mp, struct minstrel_ht_sta *mi)
 	struct minstrel_rate_stats *mr;
 	int cur_prob, cur_prob_tp, cur_tp, cur_tp2;
 	int group, i, index;
+	bool mi_rates_valid = false;
 
 	if (mi->ampdu_packets > 0) {
 		mi->avg_ampdu_len = minstrel_ewma(mi->avg_ampdu_len,
@@ -254,11 +255,10 @@ minstrel_ht_update_stats(struct minstrel_priv *mp, struct minstrel_ht_sta *mi)
 
 	mi->sample_slow = 0;
 	mi->sample_count = 0;
-	mi->max_tp_rate = 0;
-	mi->max_tp_rate2 = 0;
-	mi->max_prob_rate = 0;
 
 	for (group = 0; group < ARRAY_SIZE(minstrel_mcs_groups); group++) {
+		bool mg_rates_valid = false;
+
 		cur_prob = 0;
 		cur_prob_tp = 0;
 		cur_tp = 0;
@@ -268,15 +268,24 @@ minstrel_ht_update_stats(struct minstrel_priv *mp, struct minstrel_ht_sta *mi)
 		if (!mg->supported)
 			continue;
 
-		mg->max_tp_rate = 0;
-		mg->max_tp_rate2 = 0;
-		mg->max_prob_rate = 0;
 		mi->sample_count++;
 
 		for (i = 0; i < MCS_GROUP_RATES; i++) {
 			if (!(mg->supported & BIT(i)))
 				continue;
 
+			/* initialize rates selections starting indexes */
+			if (!mg_rates_valid) {
+				mg->max_tp_rate = mg->max_tp_rate2 =
+					mg->max_prob_rate = i;
+				if (!mi_rates_valid) {
+					mi->max_tp_rate = mi->max_tp_rate2 =
+						mi->max_prob_rate = i;
+					mi_rates_valid = true;
+				}
+				mg_rates_valid = true;
+			}
+
 			mr = &mg->rates[i];
 			mr->retry_updated = false;
 			index = MCS_GROUP_RATES * group + i;
-- 
1.8.2

[PATCH v2 2/2] mac80211: minstrel_ht: initialize rates selection

[Karl Beldan]

From: Karl Beldan <karl.beldan@rivierawaves.com>

Initialize {mp,mi}->{max_tp_rate,max_tp_rate2,max_prob_rate} in
minstrel_ht's rate_init and rate_update.

Signed-off-by: Karl Beldan <karl.beldan@rivierawaves.com>
---
 net/mac80211/rc80211_minstrel_ht.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/net/mac80211/rc80211_minstrel_ht.c b/net/mac80211/rc80211_minstrel_ht.c
index eb6dc3f..5768cc0 100644
--- a/net/mac80211/rc80211_minstrel_ht.c
+++ b/net/mac80211/rc80211_minstrel_ht.c
@@ -907,6 +907,9 @@ minstrel_ht_update_caps(void *priv, struct ieee80211_supported_band *sband,
 	if (!n_supported)
 		goto use_legacy;
 
+	/* init {mi,mi->groups[*]}->{max_tp_rate,max_tp_rate2,max_prob_rate} */
+	minstrel_ht_update_stats(mp, mi);
+
 	return;
 
 use_legacy:
-- 
1.8.2

Re: [PATCH v2 2/2] mac80211: minstrel_ht: initialize rates selection

[Karl Beldan]

Johannes, Felix,

This seems to me like an important enough issue to IMHO, make it into
3.9/-stable. But since we only have a few days before Linus tags the 3.9
I understand the boldness of the thought.
 
Karl

Re: [PATCH v2 2/2] mac80211: minstrel_ht: initialize rates selection

[Felix Fietkau]

On 2013-04-18 2:40 PM, Karl Beldan wrote:
> Johannes, Felix,
> 
> This seems to me like an important enough issue to IMHO, make it into
> 3.9/-stable. But since we only have a few days before Linus tags the 3.9
> I understand the boldness of the thought.
I'm not sure it's important enough for 3.9. Is disabling MCS0 really
that common?

- Felix

Re: [PATCH v2 2/2] mac80211: minstrel_ht: initialize rates selection

[Karl Beldan]

On Thu, Apr 18, 2013 at 03:04:03PM +0200, Felix Fietkau wrote:
> On 2013-04-18 2:40 PM, Karl Beldan wrote:
> > Johannes, Felix,
> > 
> > This seems to me like an important enough issue to IMHO, make it into
> > 3.9/-stable. But since we only have a few days before Linus tags the 3.9
> > I understand the boldness of the thought.
> I'm not sure it's important enough for 3.9. Is disabling MCS0 really
> that common?
> 
Right.
 
Karl

Re: [PATCH v2 2/2] mac80211: minstrel_ht: initialize rates selection

[Johannes Berg]

On Thu, 2013-04-18 at 14:40 +0200, Karl Beldan wrote:
> Johannes, Felix,
> 
> This seems to me like an important enough issue to IMHO, make it into
> 3.9/-stable. But since we only have a few days before Linus tags the 3.9
> I understand the boldness of the thought.

I'm not going to try for 3.9, it wouldn't happen anyway, but if you want
to tag them as Cc stable it's your call. I'm not familiar enough with
the code to really be able to judge the impact I guess. It seems
unlikely to me though that some device would support MCS 1 but not MCS
0, or MCS 9 but not 8, etc.?

johannes

Re: [PATCH v2 2/2] mac80211: minstrel_ht: initialize rates selection

[Karl Beldan]

On Thu, Apr 18, 2013 at 03:36:39PM +0200, Johannes Berg wrote:
> On Thu, 2013-04-18 at 14:40 +0200, Karl Beldan wrote:
> > Johannes, Felix,
> > 
> > This seems to me like an important enough issue to IMHO, make it into
> > 3.9/-stable. But since we only have a few days before Linus tags the 3.9
> > I understand the boldness of the thought.
> 
> I'm not going to try for 3.9, it wouldn't happen anyway, but if you want
> to tag them as Cc stable it's your call. I'm not familiar enough with
> the code to really be able to judge the impact I guess. It seems
> unlikely to me though that some device would support MCS 1 but not MCS
> 0, or MCS 9 but not 8, etc.?
> 
I imagined devices that would have their -supported MCSes configure their
hw- that for any reason would not "handle" other ones.
I don't know why I deemed it that important, and agree with you and
Felix ;)

 
Karl

Re: [PATCH v2 1/2] mac80211: minstrel_ht: pick only supported rates for sta and group max*rates

[Johannes Berg]

On Thu, 2013-04-18 at 14:26 +0200, Karl Beldan wrote:
> From: Karl Beldan <karl.beldan@rivierawaves.com>
> 
> minstrel_ht initializes max_tp_rate max_tp_rate2 and max_prob_rate to
> zero both for minstrel_ht_sta and minstrel_mcs_group_data.
> This is wrong since there is no guarantee that the 1st rate of any
> group is supported.

Applied both.

johannes