2013-04-18 12:30:44

by Karl Beldan

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

From: Karl Beldan <[email protected]>

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 <[email protected]>
---
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



2013-04-22 13:47:50

by Johannes Berg

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

On Thu, 2013-04-18 at 14:26 +0200, Karl Beldan wrote:
> From: Karl Beldan <[email protected]>
>
> 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


2013-04-18 12:44:59

by Karl Beldan

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

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

2013-04-18 13:14:47

by Karl Beldan

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

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

2013-04-18 13:36:45

by Johannes Berg

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

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


2013-04-18 13:50:11

by Karl Beldan

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

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

2013-04-18 13:04:06

by Felix Fietkau

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

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


2013-04-18 12:30:46

by Karl Beldan

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

From: Karl Beldan <[email protected]>

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 <[email protected]>
---
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