[v2] mac80211: minstrel_ht: do not sample unsupported rates

Commit Message

Karl Beldan Nov. 11, 2013, 12:10 p.m. UTC

From: Karl Beldan <[e-mail]>

ATM minstrel_ht does not check whether a sampling rate is supported.
Unsupported rates attempts can trigger when there are holes in bitfields
of supported MCSes belonging to the same group (e.g many devices are
MCS32 capable without MCS33->39 capable, also we systematically have a
hole for CCK rates).
I originally replaced an unsupported sample index with the fls of the
bitfield of supported indexes of the sta current sample group, instead,
this change simply drops the sample attempt, as suggested by Felix.

This is not a problem in minstrel which fills a per STA sample table
with only supported rates (though only at init).

Signed-off-by: Karl Beldan <[e-mail]>
---
 net/mac80211/rc80211_minstrel_ht.c | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

Comments

Johannes Berg Nov. 11, 2013, 3:46 p.m. UTC | #1

On Mon, 2013-11-11 at 13:10 +0100, Karl Beldan wrote:
> From: Karl Beldan <[e-mail]>
> 
> ATM minstrel_ht does not check whether a sampling rate is supported.
> Unsupported rates attempts can trigger when there are holes in bitfields
> of supported MCSes belonging to the same group (e.g many devices are
> MCS32 capable without MCS33->39 capable, also we systematically have a
> hole for CCK rates).
> I originally replaced an unsupported sample index with the fls of the
> bitfield of supported indexes of the sta current sample group, instead,
> this change simply drops the sample attempt, as suggested by Felix.

That paragraph doesn't really belong here - you should describe what
this change is doing (now). There may be some value in describing how
you arrived at the solution, but this particular description seems
unnecessary?

johannes

--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Karl Beldan Nov. 11, 2013, 7:44 p.m. UTC | #2

On Mon, Nov 11, 2013 at 04:46:36PM +0100, Johannes Berg wrote:
> On Mon, 2013-11-11 at 13:10 +0100, Karl Beldan wrote:
> > From: Karl Beldan <[e-mail]>
> > 
> > ATM minstrel_ht does not check whether a sampling rate is supported.
> > Unsupported rates attempts can trigger when there are holes in bitfields
> > of supported MCSes belonging to the same group (e.g many devices are
> > MCS32 capable without MCS33->39 capable, also we systematically have a
> > hole for CCK rates).
> > I originally replaced an unsupported sample index with the fls of the
> > bitfield of supported indexes of the sta current sample group, instead,
> > this change simply drops the sample attempt, as suggested by Felix.
> 
> That paragraph doesn't really belong here - you should describe what
> this change is doing (now). There may be some value in describing how
> you arrived at the solution, but this particular description seems
> unnecessary?
> 
Feel free to get rid of the superfluous comments.
 
Karl
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch

diff --git a/net/mac80211/rc80211_minstrel_ht.c b/net/mac80211/rc80211_minstrel_ht.c
index aeec401..1076bca 100644
--- a/net/mac80211/rc80211_minstrel_ht.c
+++ b/net/mac80211/rc80211_minstrel_ht.c
@@ -701,12 +701,16 @@  minstrel_get_sample_rate(struct minstrel_priv *mp, struct minstrel_ht_sta *mi)
 	if (!mi->sample_tries)
 		return -1;
 
-	mg = &mi->groups[mi->sample_group];
+	sample_group = mi->sample_group;
+	mg = &mi->groups[sample_group];
 	sample_idx = sample_table[mg->column][mg->index];
+	minstrel_next_sample_idx(mi);
+
+	if (!(mg->supported & BIT(sample_idx)))
+		return -1;
+
 	mr = &mg->rates[sample_idx];
-	sample_group = mi->sample_group;
 	sample_idx += sample_group * MCS_GROUP_RATES;
-	minstrel_next_sample_idx(mi);
 
 	/*
 	 * Sampling might add some overhead (RTS, no aggregation)