[PATCH v5 0/4] add VHT support to minstrel_ht

[PATCH v5 0/4] add VHT support to minstrel_ht

[Karl Beldan]

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

Hi,

Varka Bhadram reported checkpatch is noisy on this series.
I made another pass which affects [2/4](trivially) and [4/4].
This could have been a patch on top of what Felix acked, I hope you
won't bother too much.
Thanks for reviewing,

Karl Beldan (4):
  mac80211: minstrel_ht: Increase the range of handled rate indexes
  mac80211: minstrel_ht: macros adjustments for future VHT_GROUPs
  mac80211: minstrel_ht: include type (cck/ht) in rates flag
  mac80211: minstrel_ht: add basic support for VHT rates <= 3SS@80MHz
v2:
- typo in Kconfig
- adjust room in debugfs rc_stats buffer
- add a module param to mix ht rates with vht ones
- default to 3SS instead of 2SS (with "mac80211: minstrel_ht: macros
  adjustments for future VHT_GROUPs")
- use common MINSTREL_MAX_STREAMS both for VHT and HT so that we can get
  rid of the 'MINSTREL_MAX_STREAMS >= 3' checks for minstrel_mcs_groups
v3:
- put module param *vht_only* under CONFIG_MAC80211_RC_MINSTREL_VHT
v4:
- rebase on v2 of "mac80211: minstrels: fix buffer overflow in HT debugfs rc_stats"
v5:
- fix some checkpatch complaints this, leaves 2 false positives 
  "ERROR: Macros with complex values should be enclosed in parenthesis"
  in array initialization

 net/mac80211/Kconfig                       |   7 +
 net/mac80211/rc80211_minstrel_ht.c         | 311 +++++++++++++++++++++++------
 net/mac80211/rc80211_minstrel_ht.h         |  40 +++-
 net/mac80211/rc80211_minstrel_ht_debugfs.c |  33 +--
 4 files changed, 309 insertions(+), 82 deletions(-)

-- 
2.0.1

[PATCH v5 1/4] mac80211: minstrel_ht: Increase the range of handled rate indexes

[Karl Beldan]

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

Since 5935839ad735 ("mac80211: improve minstrel_ht rate sorting by
throughput & probability"), the rate indexes are manipulated via u8's
and hence allow for a maximum of 256 mcs_group entries in
minstrel_mcs_groups.

ATM, minstrel_ht advertizes support up to 3HTSS@40MHz, consuming:
8(MCS_GROUP_RATES) * (3(SS)*2(GI)*2(BW)+1(CCK)), i.e. 104 entries.

Support for 3VHTSS@80MHz will require:
10(MCS_GROUP_RATES) * (3(SS)*2(GI)*2(BW)+1(CCK)) +
10(MCS_GROUP_RATES) * (3(SS)*2(GI)*3(BW)), i.e. 130 + 180 entries.

This change moves from u8s to u16s where necessary.

Signed-off-by: Karl Beldan <karl.beldan@rivierawaves.com>
Cc: Felix Fietkau <nbd@openwrt.org>
---
 net/mac80211/rc80211_minstrel_ht.c | 16 ++++++++--------
 net/mac80211/rc80211_minstrel_ht.h |  8 ++++----
 2 files changed, 12 insertions(+), 12 deletions(-)

diff --git a/net/mac80211/rc80211_minstrel_ht.c b/net/mac80211/rc80211_minstrel_ht.c
index 17ef54a..ccec718 100644
--- a/net/mac80211/rc80211_minstrel_ht.c
+++ b/net/mac80211/rc80211_minstrel_ht.c
@@ -240,8 +240,8 @@ minstrel_ht_calc_tp(struct minstrel_ht_sta *mi, int group, int rate)
  * MCS groups, CCK rates do not provide aggregation and are therefore at last.
  */
 static void
-minstrel_ht_sort_best_tp_rates(struct minstrel_ht_sta *mi, u8 index,
-			       u8 *tp_list)
+minstrel_ht_sort_best_tp_rates(struct minstrel_ht_sta *mi, u16 index,
+			       u16 *tp_list)
 {
 	int cur_group, cur_idx, cur_thr, cur_prob;
 	int tmp_group, tmp_idx, tmp_thr, tmp_prob;
@@ -278,7 +278,7 @@ minstrel_ht_sort_best_tp_rates(struct minstrel_ht_sta *mi, u8 index,
  * Find and set the topmost probability rate per sta and per group
  */
 static void
-minstrel_ht_set_best_prob_rate(struct minstrel_ht_sta *mi, u8 index)
+minstrel_ht_set_best_prob_rate(struct minstrel_ht_sta *mi, u16 index)
 {
 	struct minstrel_mcs_group_data *mg;
 	struct minstrel_rate_stats *mr;
@@ -321,8 +321,8 @@ minstrel_ht_set_best_prob_rate(struct minstrel_ht_sta *mi, u8 index)
  */
 static void
 minstrel_ht_assign_best_tp_rates(struct minstrel_ht_sta *mi,
-				 u8 tmp_mcs_tp_rate[MAX_THR_RATES],
-				 u8 tmp_cck_tp_rate[MAX_THR_RATES])
+				 u16 tmp_mcs_tp_rate[MAX_THR_RATES],
+				 u16 tmp_cck_tp_rate[MAX_THR_RATES])
 {
 	unsigned int tmp_group, tmp_idx, tmp_cck_tp, tmp_mcs_tp;
 	int i;
@@ -386,8 +386,8 @@ minstrel_ht_update_stats(struct minstrel_priv *mp, struct minstrel_ht_sta *mi)
 	struct minstrel_mcs_group_data *mg;
 	struct minstrel_rate_stats *mr;
 	int group, i, j;
-	u8 tmp_mcs_tp_rate[MAX_THR_RATES], tmp_group_tp_rate[MAX_THR_RATES];
-	u8 tmp_cck_tp_rate[MAX_THR_RATES], index;
+	u16 tmp_mcs_tp_rate[MAX_THR_RATES], tmp_group_tp_rate[MAX_THR_RATES];
+	u16 tmp_cck_tp_rate[MAX_THR_RATES], index;
 
 	if (mi->ampdu_packets > 0) {
 		mi->avg_ampdu_len = minstrel_ewma(mi->avg_ampdu_len,
@@ -517,7 +517,7 @@ minstrel_next_sample_idx(struct minstrel_ht_sta *mi)
 }
 
 static void
-minstrel_downgrade_rate(struct minstrel_ht_sta *mi, u8 *idx, bool primary)
+minstrel_downgrade_rate(struct minstrel_ht_sta *mi, u16 *idx, bool primary)
 {
 	int group, orig_group;
 
diff --git a/net/mac80211/rc80211_minstrel_ht.h b/net/mac80211/rc80211_minstrel_ht.h
index 01570e0..8b54e89 100644
--- a/net/mac80211/rc80211_minstrel_ht.h
+++ b/net/mac80211/rc80211_minstrel_ht.h
@@ -34,8 +34,8 @@ struct minstrel_mcs_group_data {
 	u8 supported;
 
 	/* sorted rate set within a MCS group*/
-	u8 max_group_tp_rate[MAX_THR_RATES];
-	u8 max_group_prob_rate;
+	u16 max_group_tp_rate[MAX_THR_RATES];
+	u16 max_group_prob_rate;
 
 	/* MCS rate statistics */
 	struct minstrel_rate_stats rates[MCS_GROUP_RATES];
@@ -52,8 +52,8 @@ struct minstrel_ht_sta {
 	unsigned int avg_ampdu_len;
 
 	/* overall sorted rate set */
-	u8 max_tp_rate[MAX_THR_RATES];
-	u8 max_prob_rate;
+	u16 max_tp_rate[MAX_THR_RATES];
+	u16 max_prob_rate;
 
 	/* time of last status update */
 	unsigned long stats_update;
-- 
2.0.1

[PATCH v5 2/4] mac80211: minstrel_ht: macros adjustments for future VHT_GROUPs

[Karl Beldan]

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

No functional change.

Signed-off-by: Karl Beldan <karl.beldan@rivierawaves.com>
Cc: Felix Fietkau <nbd@openwrt.org>
---
 net/mac80211/rc80211_minstrel_ht.c         | 51 +++++++++++++++---------------
 net/mac80211/rc80211_minstrel_ht.h         | 15 +++++++--
 net/mac80211/rc80211_minstrel_ht_debugfs.c | 10 +++---
 3 files changed, 42 insertions(+), 34 deletions(-)

diff --git a/net/mac80211/rc80211_minstrel_ht.c b/net/mac80211/rc80211_minstrel_ht.c
index ccec718..a48eb76 100644
--- a/net/mac80211/rc80211_minstrel_ht.c
+++ b/net/mac80211/rc80211_minstrel_ht.c
@@ -34,12 +34,16 @@
 /* Transmit duration for the raw data part of an average sized packet */
 #define MCS_DURATION(streams, sgi, bps) MCS_SYMBOL_TIME(sgi, MCS_NSYMS((streams) * (bps)))
 
+#define BW_20			0
+#define BW_40			1
+
 /*
  * Define group sort order: HT40 -> SGI -> #streams
  */
 #define GROUP_IDX(_streams, _sgi, _ht40)	\
+	MINSTREL_HT_GROUP_0 +			\
 	MINSTREL_MAX_STREAMS * 2 * _ht40 +	\
-	MINSTREL_MAX_STREAMS * _sgi +		\
+	MINSTREL_MAX_STREAMS * _sgi +	\
 	_streams - 1
 
 /* MCS rate information for an MCS group */
@@ -76,13 +80,13 @@
 	CCK_ACK_DURATION(55, _short),			\
 	CCK_ACK_DURATION(110, _short)
 
-#define CCK_GROUP						\
-	[MINSTREL_MAX_STREAMS * MINSTREL_STREAM_GROUPS] = {	\
-		.streams = 0,					\
-		.duration = {					\
-			CCK_DURATION_LIST(false),		\
-			CCK_DURATION_LIST(true)			\
-		}						\
+#define CCK_GROUP					\
+	[MINSTREL_CCK_GROUP] = {			\
+		.streams = 0,				\
+		.duration = {				\
+			CCK_DURATION_LIST(false),	\
+			CCK_DURATION_LIST(true)		\
+		}					\
 	}
 
 /*
@@ -91,38 +95,36 @@
  * use.
  *
  * Sortorder has to be fixed for GROUP_IDX macro to be applicable:
- * HT40 -> SGI -> #streams
+ * BW -> SGI -> #streams
  */
 const struct mcs_group minstrel_mcs_groups[] = {
-	MCS_GROUP(1, 0, 0),
-	MCS_GROUP(2, 0, 0),
+	MCS_GROUP(1, 0, BW_20),
+	MCS_GROUP(2, 0, BW_20),
 #if MINSTREL_MAX_STREAMS >= 3
-	MCS_GROUP(3, 0, 0),
+	MCS_GROUP(3, 0, BW_20),
 #endif
 
-	MCS_GROUP(1, 1, 0),
-	MCS_GROUP(2, 1, 0),
+	MCS_GROUP(1, 1, BW_20),
+	MCS_GROUP(2, 1, BW_20),
 #if MINSTREL_MAX_STREAMS >= 3
-	MCS_GROUP(3, 1, 0),
+	MCS_GROUP(3, 1, BW_20),
 #endif
 
-	MCS_GROUP(1, 0, 1),
-	MCS_GROUP(2, 0, 1),
+	MCS_GROUP(1, 0, BW_40),
+	MCS_GROUP(2, 0, BW_40),
 #if MINSTREL_MAX_STREAMS >= 3
-	MCS_GROUP(3, 0, 1),
+	MCS_GROUP(3, 0, BW_40),
 #endif
 
-	MCS_GROUP(1, 1, 1),
-	MCS_GROUP(2, 1, 1),
+	MCS_GROUP(1, 1, BW_40),
+	MCS_GROUP(2, 1, BW_40),
 #if MINSTREL_MAX_STREAMS >= 3
-	MCS_GROUP(3, 1, 1),
+	MCS_GROUP(3, 1, BW_40),
 #endif
 
-	/* must be last */
 	CCK_GROUP
 };
 
-#define MINSTREL_CCK_GROUP	(ARRAY_SIZE(minstrel_mcs_groups) - 1)
 
 static u8 sample_table[SAMPLE_COLUMNS][MCS_GROUP_RATES] __read_mostly;
 
@@ -971,8 +973,7 @@ minstrel_ht_update_caps(void *priv, struct ieee80211_supported_band *sband,
 	if (!sta->ht_cap.ht_supported)
 		goto use_legacy;
 
-	BUILD_BUG_ON(ARRAY_SIZE(minstrel_mcs_groups) !=
-		MINSTREL_MAX_STREAMS * MINSTREL_STREAM_GROUPS + 1);
+	BUILD_BUG_ON(ARRAY_SIZE(minstrel_mcs_groups) != MINSTREL_GROUPS_NB);
 
 	msp->is_ht = true;
 	memset(mi, 0, sizeof(*mi));
diff --git a/net/mac80211/rc80211_minstrel_ht.h b/net/mac80211/rc80211_minstrel_ht.h
index 8b54e89..e747ac6 100644
--- a/net/mac80211/rc80211_minstrel_ht.h
+++ b/net/mac80211/rc80211_minstrel_ht.h
@@ -13,8 +13,17 @@
  * The number of streams can be changed to 2 to reduce code
  * size and memory footprint.
  */
-#define MINSTREL_MAX_STREAMS	3
-#define MINSTREL_STREAM_GROUPS	4
+#define MINSTREL_MAX_STREAMS		3
+#define MINSTREL_HT_STREAM_GROUPS	4 /* BW(=2) * SGI(=2) */
+
+#define MINSTREL_HT_GROUPS_NB	(MINSTREL_MAX_STREAMS *		\
+				 MINSTREL_HT_STREAM_GROUPS)
+#define MINSTREL_CCK_GROUPS_NB	1
+#define MINSTREL_GROUPS_NB	(MINSTREL_HT_GROUPS_NB +	\
+				 MINSTREL_CCK_GROUPS_NB)
+
+#define MINSTREL_HT_GROUP_0	0
+#define MINSTREL_CCK_GROUP	(MINSTREL_HT_GROUP_0 + MINSTREL_HT_GROUPS_NB)
 
 #define MCS_GROUP_RATES	8
 
@@ -80,7 +89,7 @@ struct minstrel_ht_sta {
 	u8 cck_supported_short;
 
 	/* MCS rate group info and statistics */
-	struct minstrel_mcs_group_data groups[MINSTREL_MAX_STREAMS * MINSTREL_STREAM_GROUPS + 1];
+	struct minstrel_mcs_group_data groups[MINSTREL_GROUPS_NB];
 };
 
 struct minstrel_ht_sta_priv {
diff --git a/net/mac80211/rc80211_minstrel_ht_debugfs.c b/net/mac80211/rc80211_minstrel_ht_debugfs.c
index d537bec..d2f53b8 100644
--- a/net/mac80211/rc80211_minstrel_ht_debugfs.c
+++ b/net/mac80211/rc80211_minstrel_ht_debugfs.c
@@ -18,7 +18,6 @@
 static char *
 minstrel_ht_stats_dump(struct minstrel_ht_sta *mi, int i, char *p)
 {
-	unsigned int max_mcs = MINSTREL_MAX_STREAMS * MINSTREL_STREAM_GROUPS;
 	const struct mcs_group *mg;
 	unsigned int j, tp, prob, eprob;
 	char htmode = '2';
@@ -41,7 +40,7 @@ minstrel_ht_stats_dump(struct minstrel_ht_sta *mi, int i, char *p)
 		if (!(mi->groups[i].supported & BIT(j)))
 			continue;
 
-		if (i == max_mcs)
+		if (i == MINSTREL_CCK_GROUP)
 			p += sprintf(p, "CCK/%cP   ", j < 4 ? 'L' : 'S');
 		else
 			p += sprintf(p, "HT%c0/%cGI ", htmode, gimode);
@@ -52,7 +51,7 @@ minstrel_ht_stats_dump(struct minstrel_ht_sta *mi, int i, char *p)
 		*(p++) = (idx == mi->max_tp_rate[3]) ? 'D' : ' ';
 		*(p++) = (idx == mi->max_prob_rate) ? 'P' : ' ';
 
-		if (i == max_mcs) {
+		if (i == MINSTREL_CCK_GROUP) {
 			int r = bitrates[j % 4];
 			p += sprintf(p, " %2u.%1uM", r / 10, r % 10);
 		} else {
@@ -85,7 +84,6 @@ minstrel_ht_stats_open(struct inode *inode, struct file *file)
 	struct minstrel_ht_sta *mi = &msp->ht;
 	struct minstrel_debugfs_info *ms;
 	unsigned int i;
-	unsigned int max_mcs = MINSTREL_MAX_STREAMS * MINSTREL_STREAM_GROUPS;
 	char *p;
 	int ret;
 
@@ -106,8 +104,8 @@ minstrel_ht_stats_open(struct inode *inode, struct file *file)
 			"ret  *ok(*cum)        ok(      cum)\n");
 
 
-	p = minstrel_ht_stats_dump(mi, max_mcs, p);
-	for (i = 0; i < max_mcs; i++)
+	p = minstrel_ht_stats_dump(mi, MINSTREL_CCK_GROUP, p);
+	for (i = 0; i < MINSTREL_CCK_GROUP; i++)
 		p = minstrel_ht_stats_dump(mi, i, p);
 
 	p += sprintf(p, "\nTotal packet count::    ideal %d      "
-- 
2.0.1

[PATCH v5 3/4] mac80211: minstrel_ht: include type (cck/ht) in rates flag

[Karl Beldan]

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

ATM, we grep cck rates idx with idx / MCS_GROUP_RATES ==
MINSTREL_CCK_GROUP.
Matching neither-cck-non-ht rates could be done by replacing '==' with
'>', however it would be less versatile or explicit.
This will allow to match VHT rates with IEEE80211_TX_RC_VHT_MCS.

Signed-off-by: Karl Beldan <karl.beldan@rivierawaves.com>
Cc: Felix Fietkau <nbd@openwrt.org>
---
 net/mac80211/rc80211_minstrel_ht.c | 28 ++++++++++++++--------------
 1 file changed, 14 insertions(+), 14 deletions(-)

diff --git a/net/mac80211/rc80211_minstrel_ht.c b/net/mac80211/rc80211_minstrel_ht.c
index a48eb76..e760d3d 100644
--- a/net/mac80211/rc80211_minstrel_ht.c
+++ b/net/mac80211/rc80211_minstrel_ht.c
@@ -51,6 +51,7 @@
 	[GROUP_IDX(_streams, _sgi, _ht40)] = {				\
 	.streams = _streams,						\
 	.flags =							\
+		IEEE80211_TX_RC_MCS |					\
 		(_sgi ? IEEE80211_TX_RC_SHORT_GI : 0) |			\
 		(_ht40 ? IEEE80211_TX_RC_40_MHZ_WIDTH : 0),		\
 	.duration = {							\
@@ -83,6 +84,7 @@
 #define CCK_GROUP					\
 	[MINSTREL_CCK_GROUP] = {			\
 		.streams = 0,				\
+		.flags = 0,				\
 		.duration = {				\
 			CCK_DURATION_LIST(false),	\
 			CCK_DURATION_LIST(true)		\
@@ -716,7 +718,7 @@ minstrel_ht_set_rate(struct minstrel_priv *mp, struct minstrel_ht_sta *mi,
 	const struct mcs_group *group = &minstrel_mcs_groups[index / MCS_GROUP_RATES];
 	struct minstrel_rate_stats *mr;
 	u8 idx;
-	u16 flags;
+	u16 flags = group->flags;
 
 	mr = minstrel_get_ratestats(mi, index);
 	if (!mr->retry_updated)
@@ -732,13 +734,10 @@ minstrel_ht_set_rate(struct minstrel_priv *mp, struct minstrel_ht_sta *mi,
 		ratetbl->rate[offset].count_rts = mr->retry_count_rtscts;
 	}
 
-	if (index / MCS_GROUP_RATES == MINSTREL_CCK_GROUP) {
+	if (index / MCS_GROUP_RATES == MINSTREL_CCK_GROUP)
 		idx = mp->cck_rates[index % ARRAY_SIZE(mp->cck_rates)];
-		flags = 0;
-	} else {
+	else
 		idx = index % MCS_GROUP_RATES + (group->streams - 1) * 8;
-		flags = IEEE80211_TX_RC_MCS | group->flags;
-	}
 
 	if (offset > 0) {
 		ratetbl->rate[offset].count = ratetbl->rate[offset].count_rts;
@@ -918,13 +917,12 @@ minstrel_ht_get_rate(void *priv, struct ieee80211_sta *sta, void *priv_sta,
 	if (sample_idx / MCS_GROUP_RATES == MINSTREL_CCK_GROUP) {
 		int idx = sample_idx % ARRAY_SIZE(mp->cck_rates);
 		rate->idx = mp->cck_rates[idx];
-		rate->flags = 0;
-		return;
+	} else {
+		rate->idx = sample_idx % MCS_GROUP_RATES +
+			    (sample_group->streams - 1) * 8;
 	}
 
-	rate->idx = sample_idx % MCS_GROUP_RATES +
-		    (sample_group->streams - 1) * 8;
-	rate->flags = IEEE80211_TX_RC_MCS | sample_group->flags;
+	rate->flags = sample_group->flags;
 }
 
 static void
@@ -1006,14 +1004,16 @@ minstrel_ht_update_caps(void *priv, struct ieee80211_supported_band *sband,
 		mi->tx_flags |= IEEE80211_TX_CTL_LDPC;
 
 	for (i = 0; i < ARRAY_SIZE(mi->groups); i++) {
+		u32 gflags = minstrel_mcs_groups[i].flags;
+
 		mi->groups[i].supported = 0;
 		if (i == MINSTREL_CCK_GROUP) {
 			minstrel_ht_update_cck(mp, mi, sband, sta);
 			continue;
 		}
 
-		if (minstrel_mcs_groups[i].flags & IEEE80211_TX_RC_SHORT_GI) {
-			if (minstrel_mcs_groups[i].flags & IEEE80211_TX_RC_40_MHZ_WIDTH) {
+		if (gflags & IEEE80211_TX_RC_SHORT_GI) {
+			if (gflags & IEEE80211_TX_RC_40_MHZ_WIDTH) {
 				if (!(sta_cap & IEEE80211_HT_CAP_SGI_40))
 					continue;
 			} else {
@@ -1022,7 +1022,7 @@ minstrel_ht_update_caps(void *priv, struct ieee80211_supported_band *sband,
 			}
 		}
 
-		if (minstrel_mcs_groups[i].flags & IEEE80211_TX_RC_40_MHZ_WIDTH &&
+		if (gflags & IEEE80211_TX_RC_40_MHZ_WIDTH &&
 		    sta->bandwidth < IEEE80211_STA_RX_BW_40)
 			continue;
 
-- 
2.0.1

[PATCH v5 4/4] mac80211: minstrel_ht: add basic support for VHT rates <= 3SS@80MHz

[Karl Beldan]

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

When the new CONFIG_MAC80211_RC_MINSTREL_VHT is not set (default 'N'),
there is no behavioral change including in sampling and MCS_GROUP_RATES
remains 8.
Otherwise MCS_GROUP_RATES is 10, and a module parameter *vht_only*
(default 'true'), restricts the rates selection to VHT when vht is
supported.

Regarding the debugfs stats buffer:
It is explicitly increased from 8k to 32k to fit every rates incl. when
both ht and vht rates are enabled, as for the format, before:
type           rate     tpt eprob *prob ret  *ok(*cum)        ok(      cum)
HT20/LGI ABCDP MCS0     0.0   0.0   0.0   1    0(   0)         0(        0)
after:
 type           rate      tpt eprob *prob ret  *ok(*cum)        ok(      cum)
 HT20/LGI ABCDP MCS0      0.0   0.0   0.0   1    0(   0)         0(        0)
VHT40/LGI       MCS5/2    0.0   0.0   0.0   0    0(   0)         0(        0)

Signed-off-by: Karl Beldan <karl.beldan@rivierawaves.com>
Cc: Felix Fietkau <nbd@openwrt.org>
---
 net/mac80211/Kconfig                       |   7 +
 net/mac80211/rc80211_minstrel_ht.c         | 216 +++++++++++++++++++++++++++--
 net/mac80211/rc80211_minstrel_ht.h         |  17 ++-
 net/mac80211/rc80211_minstrel_ht_debugfs.c |  23 +--
 4 files changed, 241 insertions(+), 22 deletions(-)

diff --git a/net/mac80211/Kconfig b/net/mac80211/Kconfig
index aeb6a48..e8049ed 100644
--- a/net/mac80211/Kconfig
+++ b/net/mac80211/Kconfig
@@ -33,6 +33,13 @@ config MAC80211_RC_MINSTREL_HT
 	---help---
 	  This option enables the 'minstrel_ht' TX rate control algorithm
 
+config MAC80211_RC_MINSTREL_VHT
+	bool "Minstrel 802.11ac support" if EXPERT
+	depends on MAC80211_RC_MINSTREL_HT
+	default n
+	---help---
+	  This option enables vht in the 'minstrel_ht' TX rate control algorithm
+
 choice
 	prompt "Default rate control algorithm"
 	depends on MAC80211_HAS_RC
diff --git a/net/mac80211/rc80211_minstrel_ht.c b/net/mac80211/rc80211_minstrel_ht.c
index e760d3d..bd5acc0 100644
--- a/net/mac80211/rc80211_minstrel_ht.c
+++ b/net/mac80211/rc80211_minstrel_ht.c
@@ -10,6 +10,7 @@
 #include <linux/skbuff.h>
 #include <linux/debugfs.h>
 #include <linux/random.h>
+#include <linux/moduleparam.h>
 #include <linux/ieee80211.h>
 #include <net/mac80211.h>
 #include "rate.h"
@@ -36,6 +37,7 @@
 
 #define BW_20			0
 #define BW_40			1
+#define BW_80			2
 
 /*
  * Define group sort order: HT40 -> SGI -> #streams
@@ -66,6 +68,47 @@
 	}								\
 }
 
+#define VHT_GROUP_IDX(_streams, _sgi, _bw)				\
+	(MINSTREL_VHT_GROUP_0 +						\
+	 MINSTREL_MAX_STREAMS * 2 * (_bw) +				\
+	 MINSTREL_MAX_STREAMS * (_sgi) +				\
+	 (_streams) - 1)
+
+#define BW2VBPS(_bw, r3, r2, r1)					\
+	(_bw == BW_80 ? r3 : _bw == BW_40 ? r2 : r1)
+
+#define VHT_GROUP(_streams, _sgi, _bw)					\
+	[VHT_GROUP_IDX(_streams, _sgi, _bw)] = {			\
+	.streams = _streams,						\
+	.flags =							\
+		IEEE80211_TX_RC_VHT_MCS |				\
+		(_sgi ? IEEE80211_TX_RC_SHORT_GI : 0) |			\
+		(_bw == BW_80 ? IEEE80211_TX_RC_80_MHZ_WIDTH :		\
+		 _bw == BW_40 ? IEEE80211_TX_RC_40_MHZ_WIDTH : 0),	\
+	.duration = {							\
+		MCS_DURATION(_streams, _sgi,				\
+			     BW2VBPS(_bw,  117,  54,  26)),		\
+		MCS_DURATION(_streams, _sgi,				\
+			     BW2VBPS(_bw,  234, 108,  52)),		\
+		MCS_DURATION(_streams, _sgi,				\
+			     BW2VBPS(_bw,  351, 162,  78)),		\
+		MCS_DURATION(_streams, _sgi,				\
+			     BW2VBPS(_bw,  468, 216, 104)),		\
+		MCS_DURATION(_streams, _sgi,				\
+			     BW2VBPS(_bw,  702, 324, 156)),		\
+		MCS_DURATION(_streams, _sgi,				\
+			     BW2VBPS(_bw,  936, 432, 208)),		\
+		MCS_DURATION(_streams, _sgi,				\
+			     BW2VBPS(_bw, 1053, 486, 234)),		\
+		MCS_DURATION(_streams, _sgi,				\
+			     BW2VBPS(_bw, 1170, 540, 260)),		\
+		MCS_DURATION(_streams, _sgi,				\
+			     BW2VBPS(_bw, 1404, 648, 312)),		\
+		MCS_DURATION(_streams, _sgi,				\
+			     BW2VBPS(_bw, 1560, 720, 346))		\
+	}								\
+}
+
 #define CCK_DURATION(_bitrate, _short, _len)		\
 	(1000 * (10 /* SIFS */ +			\
 	 (_short ? 72 + 24 : 144 + 48) +		\
@@ -91,6 +134,13 @@
 		}					\
 	}
 
+#ifdef CONFIG_MAC80211_RC_MINSTREL_VHT
+static bool minstrel_vht_only = true;
+module_param(minstrel_vht_only, bool, 0644);
+MODULE_PARM_DESC(minstrel_vht_only,
+		 "Use only VHT rates when VHT is supported by sta.");
+#endif
+
 /*
  * To enable sufficiently targeted rate sampling, MCS rates are divided into
  * groups, based on the number of streams and flags (HT40, SGI) that they
@@ -124,9 +174,46 @@ const struct mcs_group minstrel_mcs_groups[] = {
 	MCS_GROUP(3, 1, BW_40),
 #endif
 
-	CCK_GROUP
-};
+	CCK_GROUP,
+
+#ifdef CONFIG_MAC80211_RC_MINSTREL_VHT
+	VHT_GROUP(1, 0, BW_20),
+	VHT_GROUP(2, 0, BW_20),
+#if MINSTREL_MAX_STREAMS >= 3
+	VHT_GROUP(3, 0, BW_20),
+#endif
+
+	VHT_GROUP(1, 1, BW_20),
+	VHT_GROUP(2, 1, BW_20),
+#if MINSTREL_MAX_STREAMS >= 3
+	VHT_GROUP(3, 1, BW_20),
+#endif
 
+	VHT_GROUP(1, 0, BW_40),
+	VHT_GROUP(2, 0, BW_40),
+#if MINSTREL_MAX_STREAMS >= 3
+	VHT_GROUP(3, 0, BW_40),
+#endif
+
+	VHT_GROUP(1, 1, BW_40),
+	VHT_GROUP(2, 1, BW_40),
+#if MINSTREL_MAX_STREAMS >= 3
+	VHT_GROUP(3, 1, BW_40),
+#endif
+
+	VHT_GROUP(1, 0, BW_80),
+	VHT_GROUP(2, 0, BW_80),
+#if MINSTREL_MAX_STREAMS >= 3
+	VHT_GROUP(3, 0, BW_80),
+#endif
+
+	VHT_GROUP(1, 1, BW_80),
+	VHT_GROUP(2, 1, BW_80),
+#if MINSTREL_MAX_STREAMS >= 3
+	VHT_GROUP(3, 1, BW_80),
+#endif
+#endif
+};
 
 static u8 sample_table[SAMPLE_COLUMNS][MCS_GROUP_RATES] __read_mostly;
 
@@ -134,6 +221,45 @@ static void
 minstrel_ht_update_rates(struct minstrel_priv *mp, struct minstrel_ht_sta *mi);
 
 /*
+ * Some VHT MCSes are invalid (when Ndbps / Nes is not an integer)
+ * e.g for MCS9@20MHzx1Nss: Ndbps=8x52*(5/6) Nes=1
+ *
+ * Returns the valid mcs map for struct minstrel_mcs_group_data.supported
+ */
+static u16
+minstrel_get_valid_vht_rates(int bw, int nss, u16 mcs_map)
+{
+	u16 mask = 0;
+
+	if (bw == BW_20) {
+		if (nss != 3 && nss != 6)
+			mask = BIT(9);
+	} else if (bw == BW_80) {
+		if (nss == 3 || nss == 7)
+			mask = BIT(6);
+		else if (nss == 6)
+			mask = BIT(9);
+	} else {
+		WARN_ON(bw != BW_40);
+	}
+
+	switch ((le16_to_cpu(mcs_map) >> (2 * (nss - 1))) & 3) {
+	case IEEE80211_VHT_MCS_SUPPORT_0_7:
+		mask |= 0x300;
+		break;
+	case IEEE80211_VHT_MCS_SUPPORT_0_8:
+		mask |= 0x200;
+		break;
+	case IEEE80211_VHT_MCS_SUPPORT_0_9:
+		break;
+	default:
+		mask = 0x3ff;
+	}
+
+	return 0x3ff & ~mask;
+}
+
+/*
  * Look up an MCS group index based on mac80211 rate information
  */
 static int
@@ -144,6 +270,15 @@ minstrel_ht_get_group_idx(struct ieee80211_tx_rate *rate)
 			 !!(rate->flags & IEEE80211_TX_RC_40_MHZ_WIDTH));
 }
 
+static int
+minstrel_vht_get_group_idx(struct ieee80211_tx_rate *rate)
+{
+	return VHT_GROUP_IDX(ieee80211_rate_get_vht_nss(rate),
+			     !!(rate->flags & IEEE80211_TX_RC_SHORT_GI),
+			     !!(rate->flags & IEEE80211_TX_RC_40_MHZ_WIDTH) +
+			     2*!!(rate->flags & IEEE80211_TX_RC_80_MHZ_WIDTH));
+}
+
 static struct minstrel_rate_stats *
 minstrel_ht_get_stats(struct minstrel_priv *mp, struct minstrel_ht_sta *mi,
 		      struct ieee80211_tx_rate *rate)
@@ -153,6 +288,9 @@ minstrel_ht_get_stats(struct minstrel_priv *mp, struct minstrel_ht_sta *mi,
 	if (rate->flags & IEEE80211_TX_RC_MCS) {
 		group = minstrel_ht_get_group_idx(rate);
 		idx = rate->idx % 8;
+	} else if (rate->flags & IEEE80211_TX_RC_VHT_MCS) {
+		group = minstrel_vht_get_group_idx(rate);
+		idx = ieee80211_rate_get_vht_mcs(rate);
 	} else {
 		group = MINSTREL_CCK_GROUP;
 
@@ -489,7 +627,8 @@ minstrel_ht_txstat_valid(struct minstrel_priv *mp, struct ieee80211_tx_rate *rat
 	if (!rate->count)
 		return false;
 
-	if (rate->flags & IEEE80211_TX_RC_MCS)
+	if (rate->flags & IEEE80211_TX_RC_MCS ||
+	    rate->flags & IEEE80211_TX_RC_VHT_MCS)
 		return true;
 
 	return rate->idx == mp->cck_rates[0] ||
@@ -736,6 +875,9 @@ minstrel_ht_set_rate(struct minstrel_priv *mp, struct minstrel_ht_sta *mi,
 
 	if (index / MCS_GROUP_RATES == MINSTREL_CCK_GROUP)
 		idx = mp->cck_rates[index % ARRAY_SIZE(mp->cck_rates)];
+	else if (flags & IEEE80211_TX_RC_VHT_MCS)
+		idx = ((group->streams - 1) << 4) |
+		      ((index % MCS_GROUP_RATES) & 0xF);
 	else
 		idx = index % MCS_GROUP_RATES + (group->streams - 1) * 8;
 
@@ -917,6 +1059,9 @@ minstrel_ht_get_rate(void *priv, struct ieee80211_sta *sta, void *priv_sta,
 	if (sample_idx / MCS_GROUP_RATES == MINSTREL_CCK_GROUP) {
 		int idx = sample_idx % ARRAY_SIZE(mp->cck_rates);
 		rate->idx = mp->cck_rates[idx];
+	} else if (sample_group->flags & IEEE80211_TX_RC_VHT_MCS) {
+		ieee80211_rate_set_vht(rate, sample_idx % MCS_GROUP_RATES,
+				       sample_group->streams);
 	} else {
 		rate->idx = sample_idx % MCS_GROUP_RATES +
 			    (sample_group->streams - 1) * 8;
@@ -962,6 +1107,8 @@ minstrel_ht_update_caps(void *priv, struct ieee80211_supported_band *sband,
 	struct minstrel_ht_sta *mi = &msp->ht;
 	struct ieee80211_mcs_info *mcs = &sta->ht_cap.mcs;
 	u16 sta_cap = sta->ht_cap.cap;
+	struct ieee80211_sta_vht_cap *vht_cap = &sta->vht_cap;
+	int use_vht;
 	int n_supported = 0;
 	int ack_dur;
 	int stbc;
@@ -973,6 +1120,13 @@ minstrel_ht_update_caps(void *priv, struct ieee80211_supported_band *sband,
 
 	BUILD_BUG_ON(ARRAY_SIZE(minstrel_mcs_groups) != MINSTREL_GROUPS_NB);
 
+#ifdef CONFIG_MAC80211_RC_MINSTREL_VHT
+	if (vht_cap->vht_supported)
+		use_vht = vht_cap->vht_mcs.tx_mcs_map != cpu_to_le16(~0);
+	else
+#endif
+	use_vht = 0;
+
 	msp->is_ht = true;
 	memset(mi, 0, sizeof(*mi));
 
@@ -996,15 +1150,19 @@ minstrel_ht_update_caps(void *priv, struct ieee80211_supported_band *sband,
 	}
 	mi->sample_tries = 4;
 
-	stbc = (sta_cap & IEEE80211_HT_CAP_RX_STBC) >>
-		IEEE80211_HT_CAP_RX_STBC_SHIFT;
-	mi->tx_flags |= stbc << IEEE80211_TX_CTL_STBC_SHIFT;
+	/* TODO tx_flags for vht - ATM the RC API is not fine-grained enough */
+	if (!use_vht) {
+		stbc = (sta_cap & IEEE80211_HT_CAP_RX_STBC) >>
+			IEEE80211_HT_CAP_RX_STBC_SHIFT;
+		mi->tx_flags |= stbc << IEEE80211_TX_CTL_STBC_SHIFT;
 
-	if (sta_cap & IEEE80211_HT_CAP_LDPC_CODING)
-		mi->tx_flags |= IEEE80211_TX_CTL_LDPC;
+		if (sta_cap & IEEE80211_HT_CAP_LDPC_CODING)
+			mi->tx_flags |= IEEE80211_TX_CTL_LDPC;
+	}
 
 	for (i = 0; i < ARRAY_SIZE(mi->groups); i++) {
 		u32 gflags = minstrel_mcs_groups[i].flags;
+		int bw, nss;
 
 		mi->groups[i].supported = 0;
 		if (i == MINSTREL_CCK_GROUP) {
@@ -1026,13 +1184,47 @@ minstrel_ht_update_caps(void *priv, struct ieee80211_supported_band *sband,
 		    sta->bandwidth < IEEE80211_STA_RX_BW_40)
 			continue;
 
+		nss = minstrel_mcs_groups[i].streams;
+
 		/* Mark MCS > 7 as unsupported if STA is in static SMPS mode */
-		if (sta->smps_mode == IEEE80211_SMPS_STATIC &&
-		    minstrel_mcs_groups[i].streams > 1)
+		if (sta->smps_mode == IEEE80211_SMPS_STATIC && nss > 1)
+			continue;
+
+		/* HT rate */
+		if (gflags & IEEE80211_TX_RC_MCS) {
+#ifdef CONFIG_MAC80211_RC_MINSTREL_VHT
+			if (minstrel_vht_only)
+				continue;
+#endif
+			mi->groups[i].supported = mcs->rx_mask[nss - 1];
+			if (mi->groups[i].supported)
+				n_supported++;
+			continue;
+		}
+
+		/* VHT rate */
+		if (!vht_cap->vht_supported ||
+		    WARN_ON(!(gflags & IEEE80211_TX_RC_VHT_MCS)) ||
+		    WARN_ON(gflags & IEEE80211_TX_RC_160_MHZ_WIDTH))
 			continue;
 
-		mi->groups[i].supported =
-			mcs->rx_mask[minstrel_mcs_groups[i].streams - 1];
+		if (gflags & IEEE80211_TX_RC_80_MHZ_WIDTH) {
+			if (sta->bandwidth < IEEE80211_STA_RX_BW_80 ||
+			    ((gflags & IEEE80211_TX_RC_SHORT_GI) &&
+			     !(vht_cap->cap & IEEE80211_VHT_CAP_SHORT_GI_80))) {
+				continue;
+			}
+		}
+
+		if (gflags & IEEE80211_TX_RC_40_MHZ_WIDTH)
+			bw = BW_40;
+		else if (gflags & IEEE80211_TX_RC_80_MHZ_WIDTH)
+			bw = BW_80;
+		else
+			bw = BW_20;
+
+		mi->groups[i].supported = minstrel_get_valid_vht_rates(bw, nss,
+				vht_cap->vht_mcs.tx_mcs_map);
 
 		if (mi->groups[i].supported)
 			n_supported++;
diff --git a/net/mac80211/rc80211_minstrel_ht.h b/net/mac80211/rc80211_minstrel_ht.h
index e747ac6..f2217d6 100644
--- a/net/mac80211/rc80211_minstrel_ht.h
+++ b/net/mac80211/rc80211_minstrel_ht.h
@@ -15,17 +15,30 @@
  */
 #define MINSTREL_MAX_STREAMS		3
 #define MINSTREL_HT_STREAM_GROUPS	4 /* BW(=2) * SGI(=2) */
+#ifdef CONFIG_MAC80211_RC_MINSTREL_VHT
+#define MINSTREL_VHT_STREAM_GROUPS	6 /* BW(=3) * SGI(=2) */
+#else
+#define MINSTREL_VHT_STREAM_GROUPS	0
+#endif
 
 #define MINSTREL_HT_GROUPS_NB	(MINSTREL_MAX_STREAMS *		\
 				 MINSTREL_HT_STREAM_GROUPS)
+#define MINSTREL_VHT_GROUPS_NB	(MINSTREL_MAX_STREAMS *		\
+				 MINSTREL_VHT_STREAM_GROUPS)
 #define MINSTREL_CCK_GROUPS_NB	1
 #define MINSTREL_GROUPS_NB	(MINSTREL_HT_GROUPS_NB +	\
+				 MINSTREL_VHT_GROUPS_NB +	\
 				 MINSTREL_CCK_GROUPS_NB)
 
 #define MINSTREL_HT_GROUP_0	0
 #define MINSTREL_CCK_GROUP	(MINSTREL_HT_GROUP_0 + MINSTREL_HT_GROUPS_NB)
+#define MINSTREL_VHT_GROUP_0	(MINSTREL_CCK_GROUP + 1)
 
-#define MCS_GROUP_RATES	8
+#ifdef CONFIG_MAC80211_RC_MINSTREL_VHT
+#define MCS_GROUP_RATES		10
+#else
+#define MCS_GROUP_RATES		8
+#endif
 
 struct mcs_group {
 	u32 flags;
@@ -40,7 +53,7 @@ struct minstrel_mcs_group_data {
 	u8 column;
 
 	/* bitfield of supported MCS rates of this group */
-	u8 supported;
+	u16 supported;
 
 	/* sorted rate set within a MCS group*/
 	u16 max_group_tp_rate[MAX_THR_RATES];
diff --git a/net/mac80211/rc80211_minstrel_ht_debugfs.c b/net/mac80211/rc80211_minstrel_ht_debugfs.c
index d2f53b8..52bb6ef 100644
--- a/net/mac80211/rc80211_minstrel_ht_debugfs.c
+++ b/net/mac80211/rc80211_minstrel_ht_debugfs.c
@@ -29,6 +29,8 @@ minstrel_ht_stats_dump(struct minstrel_ht_sta *mi, int i, char *p)
 	mg = &minstrel_mcs_groups[i];
 	if (mg->flags & IEEE80211_TX_RC_40_MHZ_WIDTH)
 		htmode = '4';
+	else if (mg->flags & IEEE80211_TX_RC_80_MHZ_WIDTH)
+		htmode = '8';
 	if (mg->flags & IEEE80211_TX_RC_SHORT_GI)
 		gimode = 'S';
 
@@ -41,9 +43,11 @@ minstrel_ht_stats_dump(struct minstrel_ht_sta *mi, int i, char *p)
 			continue;
 
 		if (i == MINSTREL_CCK_GROUP)
-			p += sprintf(p, "CCK/%cP   ", j < 4 ? 'L' : 'S');
+			p += sprintf(p, " CCK/%cP   ", j < 4 ? 'L' : 'S');
+		else if (i >= MINSTREL_VHT_GROUP_0)
+			p += sprintf(p, "VHT%c0/%cGI ", htmode, gimode);
 		else
-			p += sprintf(p, "HT%c0/%cGI ", htmode, gimode);
+			p += sprintf(p, " HT%c0/%cGI ", htmode, gimode);
 
 		*(p++) = (idx == mi->max_tp_rate[0]) ? 'A' : ' ';
 		*(p++) = (idx == mi->max_tp_rate[1]) ? 'B' : ' ';
@@ -53,9 +57,11 @@ minstrel_ht_stats_dump(struct minstrel_ht_sta *mi, int i, char *p)
 
 		if (i == MINSTREL_CCK_GROUP) {
 			int r = bitrates[j % 4];
-			p += sprintf(p, " %2u.%1uM", r / 10, r % 10);
+			p += sprintf(p, " %2u.%1uM ", r / 10, r % 10);
+		} else if (i >= MINSTREL_VHT_GROUP_0) {
+			p += sprintf(p, " MCS%-1u/%1u", j, mg->streams);
 		} else {
-			p += sprintf(p, " MCS%-2u", (mg->streams - 1) * 8 + j);
+			p += sprintf(p, " MCS%-2u ", (mg->streams - 1) * 8 + j);
 		}
 
 		tp = mr->cur_tp / 10;
@@ -94,19 +100,20 @@ minstrel_ht_stats_open(struct inode *inode, struct file *file)
 		return ret;
 	}
 
-	ms = kmalloc(8192, GFP_KERNEL);
+	ms = kmalloc(32768, GFP_KERNEL);
 	if (!ms)
 		return -ENOMEM;
 
 	file->private_data = ms;
 	p = ms->buf;
-	p += sprintf(p, "type           rate     tpt eprob *prob "
+	p += sprintf(p, " type           rate      tpt eprob *prob "
 			"ret  *ok(*cum)        ok(      cum)\n");
 
-
 	p = minstrel_ht_stats_dump(mi, MINSTREL_CCK_GROUP, p);
 	for (i = 0; i < MINSTREL_CCK_GROUP; i++)
 		p = minstrel_ht_stats_dump(mi, i, p);
+	for (i++; i < ARRAY_SIZE(mi->groups); i++)
+		p = minstrel_ht_stats_dump(mi, i, p);
 
 	p += sprintf(p, "\nTotal packet count::    ideal %d      "
 			"lookaround %d\n",
@@ -117,7 +124,7 @@ minstrel_ht_stats_open(struct inode *inode, struct file *file)
 		MINSTREL_TRUNC(mi->avg_ampdu_len * 10) % 10);
 	ms->len = p - ms->buf;
 
-	WARN_ON(ms->len + sizeof(*ms) > 8192);
+	WARN_ON(ms->len + sizeof(*ms) > 32768);
 
 	return nonseekable_open(inode, file);
 }
-- 
2.0.1

[PATCH v6] mac80211: minstrel_ht: add basic support for VHT rates <= 3SS@80MHz

[Karl Beldan]

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

When the new CONFIG_MAC80211_RC_MINSTREL_VHT is not set (default 'N'),
there is no behavioral change including in sampling and MCS_GROUP_RATES
remains 8.
Otherwise MCS_GROUP_RATES is 10, and a module parameter *vht_only*
(default 'true'), restricts the rates selection to VHT when vht is
supported.

Regarding the debugfs stats buffer:
It is explicitly increased from 8k to 32k to fit every rates incl. when
both ht and vht rates are enabled, as for the format, before:
type           rate     tpt eprob *prob ret  *ok(*cum)        ok(      cum)
HT20/LGI ABCDP MCS0     0.0   0.0   0.0   1    0(   0)         0(        0)
after:
 type           rate      tpt eprob *prob ret  *ok(*cum)        ok(      cum)
 HT20/LGI ABCDP MCS0      0.0   0.0   0.0   1    0(   0)         0(        0)
VHT40/LGI       MCS5/2    0.0   0.0   0.0   0    0(   0)         0(        0)

Signed-off-by: Karl Beldan <karl.beldan@rivierawaves.com>
Cc: Felix Fietkau <nbd@openwrt.org>
---

v2:
- typo in Kconfig
- adjust room in debugfs rc_stats buffer
- add a module param to mix ht rates with vht ones
- default to 3SS instead of 2SS (with "mac80211: minstrel_ht: macros
  adjustments for future VHT_GROUPs")
- use common MINSTREL_MAX_STREAMS both for VHT and HT so that we can get
  rid of the 'MINSTREL_MAX_STREAMS >= 3' checks for minstrel_mcs_groups
v3:
- put module param *vht_only* under CONFIG_MAC80211_RC_MINSTREL_VHT
v4:
- rebase on v2 of "mac80211: minstrels: fix buffer overflow in HT debugfs rc_stats"
v5:
- fix some checkpatch complaints this, leaves 2 false positives 
  "ERROR: Macros with complex values should be enclosed in parenthesis"
  in array initialization
v6:
- s/u16/__le16/ addressing Johannes' sparse pass

 net/mac80211/Kconfig                       |   7 +
 net/mac80211/rc80211_minstrel_ht.c         | 216 +++++++++++++++++++++++++++--
 net/mac80211/rc80211_minstrel_ht.h         |  17 ++-
 net/mac80211/rc80211_minstrel_ht_debugfs.c |  23 +--
 4 files changed, 241 insertions(+), 22 deletions(-)

diff --git a/net/mac80211/Kconfig b/net/mac80211/Kconfig
index aeb6a48..e8049ed 100644
--- a/net/mac80211/Kconfig
+++ b/net/mac80211/Kconfig
@@ -33,6 +33,13 @@ config MAC80211_RC_MINSTREL_HT
 	---help---
 	  This option enables the 'minstrel_ht' TX rate control algorithm
 
+config MAC80211_RC_MINSTREL_VHT
+	bool "Minstrel 802.11ac support" if EXPERT
+	depends on MAC80211_RC_MINSTREL_HT
+	default n
+	---help---
+	  This option enables vht in the 'minstrel_ht' TX rate control algorithm
+
 choice
 	prompt "Default rate control algorithm"
 	depends on MAC80211_HAS_RC
diff --git a/net/mac80211/rc80211_minstrel_ht.c b/net/mac80211/rc80211_minstrel_ht.c
index e760d3d..4666681 100644
--- a/net/mac80211/rc80211_minstrel_ht.c
+++ b/net/mac80211/rc80211_minstrel_ht.c
@@ -10,6 +10,7 @@
 #include <linux/skbuff.h>
 #include <linux/debugfs.h>
 #include <linux/random.h>
+#include <linux/moduleparam.h>
 #include <linux/ieee80211.h>
 #include <net/mac80211.h>
 #include "rate.h"
@@ -36,6 +37,7 @@
 
 #define BW_20			0
 #define BW_40			1
+#define BW_80			2
 
 /*
  * Define group sort order: HT40 -> SGI -> #streams
@@ -66,6 +68,47 @@
 	}								\
 }
 
+#define VHT_GROUP_IDX(_streams, _sgi, _bw)				\
+	(MINSTREL_VHT_GROUP_0 +						\
+	 MINSTREL_MAX_STREAMS * 2 * (_bw) +				\
+	 MINSTREL_MAX_STREAMS * (_sgi) +				\
+	 (_streams) - 1)
+
+#define BW2VBPS(_bw, r3, r2, r1)					\
+	(_bw == BW_80 ? r3 : _bw == BW_40 ? r2 : r1)
+
+#define VHT_GROUP(_streams, _sgi, _bw)					\
+	[VHT_GROUP_IDX(_streams, _sgi, _bw)] = {			\
+	.streams = _streams,						\
+	.flags =							\
+		IEEE80211_TX_RC_VHT_MCS |				\
+		(_sgi ? IEEE80211_TX_RC_SHORT_GI : 0) |			\
+		(_bw == BW_80 ? IEEE80211_TX_RC_80_MHZ_WIDTH :		\
+		 _bw == BW_40 ? IEEE80211_TX_RC_40_MHZ_WIDTH : 0),	\
+	.duration = {							\
+		MCS_DURATION(_streams, _sgi,				\
+			     BW2VBPS(_bw,  117,  54,  26)),		\
+		MCS_DURATION(_streams, _sgi,				\
+			     BW2VBPS(_bw,  234, 108,  52)),		\
+		MCS_DURATION(_streams, _sgi,				\
+			     BW2VBPS(_bw,  351, 162,  78)),		\
+		MCS_DURATION(_streams, _sgi,				\
+			     BW2VBPS(_bw,  468, 216, 104)),		\
+		MCS_DURATION(_streams, _sgi,				\
+			     BW2VBPS(_bw,  702, 324, 156)),		\
+		MCS_DURATION(_streams, _sgi,				\
+			     BW2VBPS(_bw,  936, 432, 208)),		\
+		MCS_DURATION(_streams, _sgi,				\
+			     BW2VBPS(_bw, 1053, 486, 234)),		\
+		MCS_DURATION(_streams, _sgi,				\
+			     BW2VBPS(_bw, 1170, 540, 260)),		\
+		MCS_DURATION(_streams, _sgi,				\
+			     BW2VBPS(_bw, 1404, 648, 312)),		\
+		MCS_DURATION(_streams, _sgi,				\
+			     BW2VBPS(_bw, 1560, 720, 346))		\
+	}								\
+}
+
 #define CCK_DURATION(_bitrate, _short, _len)		\
 	(1000 * (10 /* SIFS */ +			\
 	 (_short ? 72 + 24 : 144 + 48) +		\
@@ -91,6 +134,13 @@
 		}					\
 	}
 
+#ifdef CONFIG_MAC80211_RC_MINSTREL_VHT
+static bool minstrel_vht_only = true;
+module_param(minstrel_vht_only, bool, 0644);
+MODULE_PARM_DESC(minstrel_vht_only,
+		 "Use only VHT rates when VHT is supported by sta.");
+#endif
+
 /*
  * To enable sufficiently targeted rate sampling, MCS rates are divided into
  * groups, based on the number of streams and flags (HT40, SGI) that they
@@ -124,9 +174,46 @@ const struct mcs_group minstrel_mcs_groups[] = {
 	MCS_GROUP(3, 1, BW_40),
 #endif
 
-	CCK_GROUP
-};
+	CCK_GROUP,
+
+#ifdef CONFIG_MAC80211_RC_MINSTREL_VHT
+	VHT_GROUP(1, 0, BW_20),
+	VHT_GROUP(2, 0, BW_20),
+#if MINSTREL_MAX_STREAMS >= 3
+	VHT_GROUP(3, 0, BW_20),
+#endif
+
+	VHT_GROUP(1, 1, BW_20),
+	VHT_GROUP(2, 1, BW_20),
+#if MINSTREL_MAX_STREAMS >= 3
+	VHT_GROUP(3, 1, BW_20),
+#endif
 
+	VHT_GROUP(1, 0, BW_40),
+	VHT_GROUP(2, 0, BW_40),
+#if MINSTREL_MAX_STREAMS >= 3
+	VHT_GROUP(3, 0, BW_40),
+#endif
+
+	VHT_GROUP(1, 1, BW_40),
+	VHT_GROUP(2, 1, BW_40),
+#if MINSTREL_MAX_STREAMS >= 3
+	VHT_GROUP(3, 1, BW_40),
+#endif
+
+	VHT_GROUP(1, 0, BW_80),
+	VHT_GROUP(2, 0, BW_80),
+#if MINSTREL_MAX_STREAMS >= 3
+	VHT_GROUP(3, 0, BW_80),
+#endif
+
+	VHT_GROUP(1, 1, BW_80),
+	VHT_GROUP(2, 1, BW_80),
+#if MINSTREL_MAX_STREAMS >= 3
+	VHT_GROUP(3, 1, BW_80),
+#endif
+#endif
+};
 
 static u8 sample_table[SAMPLE_COLUMNS][MCS_GROUP_RATES] __read_mostly;
 
@@ -134,6 +221,45 @@ static void
 minstrel_ht_update_rates(struct minstrel_priv *mp, struct minstrel_ht_sta *mi);
 
 /*
+ * Some VHT MCSes are invalid (when Ndbps / Nes is not an integer)
+ * e.g for MCS9@20MHzx1Nss: Ndbps=8x52*(5/6) Nes=1
+ *
+ * Returns the valid mcs map for struct minstrel_mcs_group_data.supported
+ */
+static u16
+minstrel_get_valid_vht_rates(int bw, int nss, __le16 mcs_map)
+{
+	u16 mask = 0;
+
+	if (bw == BW_20) {
+		if (nss != 3 && nss != 6)
+			mask = BIT(9);
+	} else if (bw == BW_80) {
+		if (nss == 3 || nss == 7)
+			mask = BIT(6);
+		else if (nss == 6)
+			mask = BIT(9);
+	} else {
+		WARN_ON(bw != BW_40);
+	}
+
+	switch ((le16_to_cpu(mcs_map) >> (2 * (nss - 1))) & 3) {
+	case IEEE80211_VHT_MCS_SUPPORT_0_7:
+		mask |= 0x300;
+		break;
+	case IEEE80211_VHT_MCS_SUPPORT_0_8:
+		mask |= 0x200;
+		break;
+	case IEEE80211_VHT_MCS_SUPPORT_0_9:
+		break;
+	default:
+		mask = 0x3ff;
+	}
+
+	return 0x3ff & ~mask;
+}
+
+/*
  * Look up an MCS group index based on mac80211 rate information
  */
 static int
@@ -144,6 +270,15 @@ minstrel_ht_get_group_idx(struct ieee80211_tx_rate *rate)
 			 !!(rate->flags & IEEE80211_TX_RC_40_MHZ_WIDTH));
 }
 
+static int
+minstrel_vht_get_group_idx(struct ieee80211_tx_rate *rate)
+{
+	return VHT_GROUP_IDX(ieee80211_rate_get_vht_nss(rate),
+			     !!(rate->flags & IEEE80211_TX_RC_SHORT_GI),
+			     !!(rate->flags & IEEE80211_TX_RC_40_MHZ_WIDTH) +
+			     2*!!(rate->flags & IEEE80211_TX_RC_80_MHZ_WIDTH));
+}
+
 static struct minstrel_rate_stats *
 minstrel_ht_get_stats(struct minstrel_priv *mp, struct minstrel_ht_sta *mi,
 		      struct ieee80211_tx_rate *rate)
@@ -153,6 +288,9 @@ minstrel_ht_get_stats(struct minstrel_priv *mp, struct minstrel_ht_sta *mi,
 	if (rate->flags & IEEE80211_TX_RC_MCS) {
 		group = minstrel_ht_get_group_idx(rate);
 		idx = rate->idx % 8;
+	} else if (rate->flags & IEEE80211_TX_RC_VHT_MCS) {
+		group = minstrel_vht_get_group_idx(rate);
+		idx = ieee80211_rate_get_vht_mcs(rate);
 	} else {
 		group = MINSTREL_CCK_GROUP;
 
@@ -489,7 +627,8 @@ minstrel_ht_txstat_valid(struct minstrel_priv *mp, struct ieee80211_tx_rate *rat
 	if (!rate->count)
 		return false;
 
-	if (rate->flags & IEEE80211_TX_RC_MCS)
+	if (rate->flags & IEEE80211_TX_RC_MCS ||
+	    rate->flags & IEEE80211_TX_RC_VHT_MCS)
 		return true;
 
 	return rate->idx == mp->cck_rates[0] ||
@@ -736,6 +875,9 @@ minstrel_ht_set_rate(struct minstrel_priv *mp, struct minstrel_ht_sta *mi,
 
 	if (index / MCS_GROUP_RATES == MINSTREL_CCK_GROUP)
 		idx = mp->cck_rates[index % ARRAY_SIZE(mp->cck_rates)];
+	else if (flags & IEEE80211_TX_RC_VHT_MCS)
+		idx = ((group->streams - 1) << 4) |
+		      ((index % MCS_GROUP_RATES) & 0xF);
 	else
 		idx = index % MCS_GROUP_RATES + (group->streams - 1) * 8;
 
@@ -917,6 +1059,9 @@ minstrel_ht_get_rate(void *priv, struct ieee80211_sta *sta, void *priv_sta,
 	if (sample_idx / MCS_GROUP_RATES == MINSTREL_CCK_GROUP) {
 		int idx = sample_idx % ARRAY_SIZE(mp->cck_rates);
 		rate->idx = mp->cck_rates[idx];
+	} else if (sample_group->flags & IEEE80211_TX_RC_VHT_MCS) {
+		ieee80211_rate_set_vht(rate, sample_idx % MCS_GROUP_RATES,
+				       sample_group->streams);
 	} else {
 		rate->idx = sample_idx % MCS_GROUP_RATES +
 			    (sample_group->streams - 1) * 8;
@@ -962,6 +1107,8 @@ minstrel_ht_update_caps(void *priv, struct ieee80211_supported_band *sband,
 	struct minstrel_ht_sta *mi = &msp->ht;
 	struct ieee80211_mcs_info *mcs = &sta->ht_cap.mcs;
 	u16 sta_cap = sta->ht_cap.cap;
+	struct ieee80211_sta_vht_cap *vht_cap = &sta->vht_cap;
+	int use_vht;
 	int n_supported = 0;
 	int ack_dur;
 	int stbc;
@@ -973,6 +1120,13 @@ minstrel_ht_update_caps(void *priv, struct ieee80211_supported_band *sband,
 
 	BUILD_BUG_ON(ARRAY_SIZE(minstrel_mcs_groups) != MINSTREL_GROUPS_NB);
 
+#ifdef CONFIG_MAC80211_RC_MINSTREL_VHT
+	if (vht_cap->vht_supported)
+		use_vht = vht_cap->vht_mcs.tx_mcs_map != cpu_to_le16(~0);
+	else
+#endif
+	use_vht = 0;
+
 	msp->is_ht = true;
 	memset(mi, 0, sizeof(*mi));
 
@@ -996,15 +1150,19 @@ minstrel_ht_update_caps(void *priv, struct ieee80211_supported_band *sband,
 	}
 	mi->sample_tries = 4;
 
-	stbc = (sta_cap & IEEE80211_HT_CAP_RX_STBC) >>
-		IEEE80211_HT_CAP_RX_STBC_SHIFT;
-	mi->tx_flags |= stbc << IEEE80211_TX_CTL_STBC_SHIFT;
+	/* TODO tx_flags for vht - ATM the RC API is not fine-grained enough */
+	if (!use_vht) {
+		stbc = (sta_cap & IEEE80211_HT_CAP_RX_STBC) >>
+			IEEE80211_HT_CAP_RX_STBC_SHIFT;
+		mi->tx_flags |= stbc << IEEE80211_TX_CTL_STBC_SHIFT;
 
-	if (sta_cap & IEEE80211_HT_CAP_LDPC_CODING)
-		mi->tx_flags |= IEEE80211_TX_CTL_LDPC;
+		if (sta_cap & IEEE80211_HT_CAP_LDPC_CODING)
+			mi->tx_flags |= IEEE80211_TX_CTL_LDPC;
+	}
 
 	for (i = 0; i < ARRAY_SIZE(mi->groups); i++) {
 		u32 gflags = minstrel_mcs_groups[i].flags;
+		int bw, nss;
 
 		mi->groups[i].supported = 0;
 		if (i == MINSTREL_CCK_GROUP) {
@@ -1026,13 +1184,47 @@ minstrel_ht_update_caps(void *priv, struct ieee80211_supported_band *sband,
 		    sta->bandwidth < IEEE80211_STA_RX_BW_40)
 			continue;
 
+		nss = minstrel_mcs_groups[i].streams;
+
 		/* Mark MCS > 7 as unsupported if STA is in static SMPS mode */
-		if (sta->smps_mode == IEEE80211_SMPS_STATIC &&
-		    minstrel_mcs_groups[i].streams > 1)
+		if (sta->smps_mode == IEEE80211_SMPS_STATIC && nss > 1)
+			continue;
+
+		/* HT rate */
+		if (gflags & IEEE80211_TX_RC_MCS) {
+#ifdef CONFIG_MAC80211_RC_MINSTREL_VHT
+			if (minstrel_vht_only)
+				continue;
+#endif
+			mi->groups[i].supported = mcs->rx_mask[nss - 1];
+			if (mi->groups[i].supported)
+				n_supported++;
+			continue;
+		}
+
+		/* VHT rate */
+		if (!vht_cap->vht_supported ||
+		    WARN_ON(!(gflags & IEEE80211_TX_RC_VHT_MCS)) ||
+		    WARN_ON(gflags & IEEE80211_TX_RC_160_MHZ_WIDTH))
 			continue;
 
-		mi->groups[i].supported =
-			mcs->rx_mask[minstrel_mcs_groups[i].streams - 1];
+		if (gflags & IEEE80211_TX_RC_80_MHZ_WIDTH) {
+			if (sta->bandwidth < IEEE80211_STA_RX_BW_80 ||
+			    ((gflags & IEEE80211_TX_RC_SHORT_GI) &&
+			     !(vht_cap->cap & IEEE80211_VHT_CAP_SHORT_GI_80))) {
+				continue;
+			}
+		}
+
+		if (gflags & IEEE80211_TX_RC_40_MHZ_WIDTH)
+			bw = BW_40;
+		else if (gflags & IEEE80211_TX_RC_80_MHZ_WIDTH)
+			bw = BW_80;
+		else
+			bw = BW_20;
+
+		mi->groups[i].supported = minstrel_get_valid_vht_rates(bw, nss,
+				vht_cap->vht_mcs.tx_mcs_map);
 
 		if (mi->groups[i].supported)
 			n_supported++;
diff --git a/net/mac80211/rc80211_minstrel_ht.h b/net/mac80211/rc80211_minstrel_ht.h
index e747ac6..f2217d6 100644
--- a/net/mac80211/rc80211_minstrel_ht.h
+++ b/net/mac80211/rc80211_minstrel_ht.h
@@ -15,17 +15,30 @@
  */
 #define MINSTREL_MAX_STREAMS		3
 #define MINSTREL_HT_STREAM_GROUPS	4 /* BW(=2) * SGI(=2) */
+#ifdef CONFIG_MAC80211_RC_MINSTREL_VHT
+#define MINSTREL_VHT_STREAM_GROUPS	6 /* BW(=3) * SGI(=2) */
+#else
+#define MINSTREL_VHT_STREAM_GROUPS	0
+#endif
 
 #define MINSTREL_HT_GROUPS_NB	(MINSTREL_MAX_STREAMS *		\
 				 MINSTREL_HT_STREAM_GROUPS)
+#define MINSTREL_VHT_GROUPS_NB	(MINSTREL_MAX_STREAMS *		\
+				 MINSTREL_VHT_STREAM_GROUPS)
 #define MINSTREL_CCK_GROUPS_NB	1
 #define MINSTREL_GROUPS_NB	(MINSTREL_HT_GROUPS_NB +	\
+				 MINSTREL_VHT_GROUPS_NB +	\
 				 MINSTREL_CCK_GROUPS_NB)
 
 #define MINSTREL_HT_GROUP_0	0
 #define MINSTREL_CCK_GROUP	(MINSTREL_HT_GROUP_0 + MINSTREL_HT_GROUPS_NB)
+#define MINSTREL_VHT_GROUP_0	(MINSTREL_CCK_GROUP + 1)
 
-#define MCS_GROUP_RATES	8
+#ifdef CONFIG_MAC80211_RC_MINSTREL_VHT
+#define MCS_GROUP_RATES		10
+#else
+#define MCS_GROUP_RATES		8
+#endif
 
 struct mcs_group {
 	u32 flags;
@@ -40,7 +53,7 @@ struct minstrel_mcs_group_data {
 	u8 column;
 
 	/* bitfield of supported MCS rates of this group */
-	u8 supported;
+	u16 supported;
 
 	/* sorted rate set within a MCS group*/
 	u16 max_group_tp_rate[MAX_THR_RATES];
diff --git a/net/mac80211/rc80211_minstrel_ht_debugfs.c b/net/mac80211/rc80211_minstrel_ht_debugfs.c
index d2f53b8..52bb6ef 100644
--- a/net/mac80211/rc80211_minstrel_ht_debugfs.c
+++ b/net/mac80211/rc80211_minstrel_ht_debugfs.c
@@ -29,6 +29,8 @@ minstrel_ht_stats_dump(struct minstrel_ht_sta *mi, int i, char *p)
 	mg = &minstrel_mcs_groups[i];
 	if (mg->flags & IEEE80211_TX_RC_40_MHZ_WIDTH)
 		htmode = '4';
+	else if (mg->flags & IEEE80211_TX_RC_80_MHZ_WIDTH)
+		htmode = '8';
 	if (mg->flags & IEEE80211_TX_RC_SHORT_GI)
 		gimode = 'S';
 
@@ -41,9 +43,11 @@ minstrel_ht_stats_dump(struct minstrel_ht_sta *mi, int i, char *p)
 			continue;
 
 		if (i == MINSTREL_CCK_GROUP)
-			p += sprintf(p, "CCK/%cP   ", j < 4 ? 'L' : 'S');
+			p += sprintf(p, " CCK/%cP   ", j < 4 ? 'L' : 'S');
+		else if (i >= MINSTREL_VHT_GROUP_0)
+			p += sprintf(p, "VHT%c0/%cGI ", htmode, gimode);
 		else
-			p += sprintf(p, "HT%c0/%cGI ", htmode, gimode);
+			p += sprintf(p, " HT%c0/%cGI ", htmode, gimode);
 
 		*(p++) = (idx == mi->max_tp_rate[0]) ? 'A' : ' ';
 		*(p++) = (idx == mi->max_tp_rate[1]) ? 'B' : ' ';
@@ -53,9 +57,11 @@ minstrel_ht_stats_dump(struct minstrel_ht_sta *mi, int i, char *p)
 
 		if (i == MINSTREL_CCK_GROUP) {
 			int r = bitrates[j % 4];
-			p += sprintf(p, " %2u.%1uM", r / 10, r % 10);
+			p += sprintf(p, " %2u.%1uM ", r / 10, r % 10);
+		} else if (i >= MINSTREL_VHT_GROUP_0) {
+			p += sprintf(p, " MCS%-1u/%1u", j, mg->streams);
 		} else {
-			p += sprintf(p, " MCS%-2u", (mg->streams - 1) * 8 + j);
+			p += sprintf(p, " MCS%-2u ", (mg->streams - 1) * 8 + j);
 		}
 
 		tp = mr->cur_tp / 10;
@@ -94,19 +100,20 @@ minstrel_ht_stats_open(struct inode *inode, struct file *file)
 		return ret;
 	}
 
-	ms = kmalloc(8192, GFP_KERNEL);
+	ms = kmalloc(32768, GFP_KERNEL);
 	if (!ms)
 		return -ENOMEM;
 
 	file->private_data = ms;
 	p = ms->buf;
-	p += sprintf(p, "type           rate     tpt eprob *prob "
+	p += sprintf(p, " type           rate      tpt eprob *prob "
 			"ret  *ok(*cum)        ok(      cum)\n");
 
-
 	p = minstrel_ht_stats_dump(mi, MINSTREL_CCK_GROUP, p);
 	for (i = 0; i < MINSTREL_CCK_GROUP; i++)
 		p = minstrel_ht_stats_dump(mi, i, p);
+	for (i++; i < ARRAY_SIZE(mi->groups); i++)
+		p = minstrel_ht_stats_dump(mi, i, p);
 
 	p += sprintf(p, "\nTotal packet count::    ideal %d      "
 			"lookaround %d\n",
@@ -117,7 +124,7 @@ minstrel_ht_stats_open(struct inode *inode, struct file *file)
 		MINSTREL_TRUNC(mi->avg_ampdu_len * 10) % 10);
 	ms->len = p - ms->buf;
 
-	WARN_ON(ms->len + sizeof(*ms) > 8192);
+	WARN_ON(ms->len + sizeof(*ms) > 32768);
 
 	return nonseekable_open(inode, file);
 }
-- 
2.0.1

Re: [PATCH v6] mac80211: minstrel_ht: add basic support for VHT rates <= 3SS@80MHz

[Karl Beldan]

Hi,

On Tue, Oct 21, 2014 at 10:38:38AM +0200, Karl Beldan wrote:
> From: Karl Beldan <karl.beldan@rivierawaves.com>
> 
> @@ -1026,13 +1184,47 @@ minstrel_ht_update_caps(void *priv, struct ieee80211_supported_band *sband,
>  		    sta->bandwidth < IEEE80211_STA_RX_BW_40)
>  			continue;
>  
> +		nss = minstrel_mcs_groups[i].streams;
> +
>  		/* Mark MCS > 7 as unsupported if STA is in static SMPS mode */
> -		if (sta->smps_mode == IEEE80211_SMPS_STATIC &&
> -		    minstrel_mcs_groups[i].streams > 1)
> +		if (sta->smps_mode == IEEE80211_SMPS_STATIC && nss > 1)
> +			continue;
> +
> +		/* HT rate */
> +		if (gflags & IEEE80211_TX_RC_MCS) {
> +#ifdef CONFIG_MAC80211_RC_MINSTREL_VHT
> +			if (minstrel_vht_only)
> +				continue;
> +#endif

When reformatting for 80chars I introduced a pb, the test should be 
'if (use_vht && minstrel_vht_only)' instead.
The consequence is VHT-unable devices with
CONFIG_MAC80211_RC_MINSTREL_VHT set won't have HT rates enabled.
Johannes can you squash it in or should I send you a patch ?

 
Karl

Re: [PATCH v6] mac80211: minstrel_ht: add basic support for VHT rates <= 3SS@80MHz

[Johannes Berg]

On Fri, 2014-10-24 at 13:48 +0200, Karl Beldan wrote:
> Hi,
> 
> On Tue, Oct 21, 2014 at 10:38:38AM +0200, Karl Beldan wrote:
> > From: Karl Beldan <karl.beldan@rivierawaves.com>
> > 
> > @@ -1026,13 +1184,47 @@ minstrel_ht_update_caps(void *priv, struct ieee80211_supported_band *sband,
> >  		    sta->bandwidth < IEEE80211_STA_RX_BW_40)
> >  			continue;
> >  
> > +		nss = minstrel_mcs_groups[i].streams;
> > +
> >  		/* Mark MCS > 7 as unsupported if STA is in static SMPS mode */
> > -		if (sta->smps_mode == IEEE80211_SMPS_STATIC &&
> > -		    minstrel_mcs_groups[i].streams > 1)
> > +		if (sta->smps_mode == IEEE80211_SMPS_STATIC && nss > 1)
> > +			continue;
> > +
> > +		/* HT rate */
> > +		if (gflags & IEEE80211_TX_RC_MCS) {
> > +#ifdef CONFIG_MAC80211_RC_MINSTREL_VHT
> > +			if (minstrel_vht_only)
> > +				continue;
> > +#endif
> 
> When reformatting for 80chars I introduced a pb, the test should be 
> 'if (use_vht && minstrel_vht_only)' instead.
> The consequence is VHT-unable devices with
> CONFIG_MAC80211_RC_MINSTREL_VHT set won't have HT rates enabled.
> Johannes can you squash it in or should I send you a patch ?

I'm not sure I'm following, please send a patch, and I may decide to
fold that.

johannes

[PATCH] mac80211: minstrel_ht: do not always skip ht rates vht_only is true

[Karl Beldan]

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

When CONFIG_MAC80211_RC_MINSTREL_VHT is set, the module param
minstrel_vht_only tells minstrel_ht whether to allow the mix of ht rates
with vht rates.
ATM, minstrel_ht skips ht rates when minstrel_vht_only is true, but it does
that even if vht is not supported, which makes the sta rates fallback to
legacy as no ht rate gets enabled.

Fixes: 9208247d74bc ("mac80211: minstrel_ht: add basic support for VHT rates <= 3SS@80MHz")
Signed-off-by: Karl Beldan <karl.beldan@rivierawaves.com>
---
 net/mac80211/rc80211_minstrel_ht.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/mac80211/rc80211_minstrel_ht.c b/net/mac80211/rc80211_minstrel_ht.c
index 4666681..c50fd94 100644
--- a/net/mac80211/rc80211_minstrel_ht.c
+++ b/net/mac80211/rc80211_minstrel_ht.c
@@ -1193,7 +1193,7 @@ minstrel_ht_update_caps(void *priv, struct ieee80211_supported_band *sband,
 		/* HT rate */
 		if (gflags & IEEE80211_TX_RC_MCS) {
 #ifdef CONFIG_MAC80211_RC_MINSTREL_VHT
-			if (minstrel_vht_only)
+			if (use_vht && minstrel_vht_only)
 				continue;
 #endif
 			mi->groups[i].supported = mcs->rx_mask[nss - 1];
-- 
2.0.1

Re: [PATCH] mac80211: minstrel_ht: do not always skip ht rates vht_only is true

[Karl Beldan]

On Fri, Oct 24, 2014 at 02:34:49PM +0200, Karl Beldan wrote:
> From: Karl Beldan <karl.beldan@rivierawaves.com>
> 
> When CONFIG_MAC80211_RC_MINSTREL_VHT is set, the module param
> minstrel_vht_only tells minstrel_ht whether to allow the mix of ht rates
> with vht rates.
> ATM, minstrel_ht skips ht rates when minstrel_vht_only is true, but it does
> that even if vht is not supported, which makes the sta rates fallback to
> legacy as no ht rate gets enabled.
> 
> Fixes: 9208247d74bc ("mac80211: minstrel_ht: add basic support for VHT rates <= 3SS@80MHz")
> Signed-off-by: Karl Beldan <karl.beldan@rivierawaves.com>
> ---

This sneaked in between v4 and v5 when reformatting the code for 80ch.
v5:
- fix some checkpatch complaints this, leaves 2 false positives
  "ERROR: Macros with complex values should be enclosed in parenthesis"
      in array initialization
 
Karl

Re: [PATCH] mac80211: minstrel_ht: do not always skip ht rates vht_only is true

[Johannes Berg]

On Fri, 2014-10-24 at 14:34 +0200, Karl Beldan wrote:
> From: Karl Beldan <karl.beldan@rivierawaves.com>
> 
> When CONFIG_MAC80211_RC_MINSTREL_VHT is set, the module param
> minstrel_vht_only tells minstrel_ht whether to allow the mix of ht rates
> with vht rates.
> ATM, minstrel_ht skips ht rates when minstrel_vht_only is true, but it does
> that even if vht is not supported, which makes the sta rates fallback to
> legacy as no ht rate gets enabled.

Applied (as is)

johannes

Re: [PATCH] mac80211: minstrel_ht: do not always skip ht rates vht_only is true

[Karl Beldan]

On Fri, Oct 24, 2014 at 04:30:35PM +0200, Johannes Berg wrote:
> On Fri, 2014-10-24 at 14:34 +0200, Karl Beldan wrote:
> > From: Karl Beldan <karl.beldan@rivierawaves.com>
> > 
> > When CONFIG_MAC80211_RC_MINSTREL_VHT is set, the module param
> > minstrel_vht_only tells minstrel_ht whether to allow the mix of ht rates
> > with vht rates.
> > ATM, minstrel_ht skips ht rates when minstrel_vht_only is true, but it does
> > that even if vht is not supported, which makes the sta rates fallback to
> > legacy as no ht rate gets enabled.
> 
> Applied (as is)
> 
Ok, thanks.
 
Karl

Re: [PATCH v6] mac80211: minstrel_ht: add basic support for VHT rates <= 3SS@80MHz

[Jouni Malinen]

On Tue, Oct 21, 2014 at 10:38:38AM +0200, Karl Beldan wrote:
> When the new CONFIG_MAC80211_RC_MINSTREL_VHT is not set (default 'N'),
> there is no behavioral change including in sampling and MCS_GROUP_RATES
> remains 8.
> Otherwise MCS_GROUP_RATES is 10, and a module parameter *vht_only*
> (default 'true'), restricts the rates selection to VHT when vht is
> supported.

Have you tested this with mac80211_hwsim? I'm seeing number of issues in
hwsim test cases with CONFIG_MAC80211_RC_MINSTREL_VHT=y with
wireless-testing.git. These disappear if I disable this build option.

For example, with nfc_p2p_go_neg test case:

[    7.023869] wlan1: authenticate with 02:00:00:00:00:00
[    7.024421] wlan1: Allocated STA 02:00:00:00:00:00
[    7.024426] wlan1: capabilities/regulatory prevented using AP HT/VHT configuration, downgraded

(note: this part happens also with minstrel_ht VHT supported disabled,
but the issues below don't)

...

[    7.255983] wlan0: Allocated STA 02:00:00:00:01:00
[    7.255988] wlan0: moving STA 02:00:00:00:01:00 to state 2
[    7.255991] wlan0: moving STA 02:00:00:00:01:00 to state 3
[    7.256232] wlan0: Inserted STA 02:00:00:00:01:00
[    7.270176] wlan1: dropped frame to 02:00:00:00:00:00 (unauthorized port)
[    7.494771] ------------[ cut here ]------------
[    7.495194] WARNING: CPU: 1 PID: 584 at net/mac80211/rate.c:501 ieee80211_get_tx_rates+0x2c2/0x810()
[    7.495972] CPU: 1 PID: 584 Comm: wpa_supplicant Not tainted 3.18.0-rc4-wl #347
[    7.496607] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Bochs 01/01/2011
[    7.497322]  0000000000000009 ffff88001e327798 ffffffff814d7f34 ffffffff81089c41
[    7.498010]  0000000000000000 ffff88001e3277d8 ffffffff8103ab6c 0000000000000246
[    7.498703]  ffff88001dcb7600 ffff88001dcb7630 0000000000000000 ffff88001dcb763c
[    7.499399] Call Trace:
[    7.499628]  [<ffffffff814d7f34>] dump_stack+0x4e/0x71
[    7.500108]  [<ffffffff81089c41>] ? console_unlock+0x1f1/0x4d0
[    7.500628]  [<ffffffff8103ab6c>] warn_slowpath_common+0x7c/0xa0
[    7.501184]  [<ffffffff8103ac7a>] warn_slowpath_null+0x1a/0x20
[    7.501702]  [<ffffffff814598e2>] ieee80211_get_tx_rates+0x2c2/0x810
[    7.502256]  [<ffffffff81459f0a>] rate_control_get_rate+0xda/0xf0
[    7.502790]  [<ffffffff8146f8f2>] ieee80211_tx_h_rate_ctrl+0x1a2/0x430
[    7.503368]  [<ffffffff8146fda8>] invoke_tx_handlers+0x108/0x180
[    7.503898]  [<ffffffff81470d15>] ieee80211_tx+0x75/0xf0
[    7.504370]  [<ffffffff81470ef1>] ieee80211_xmit+0xa1/0x100
[    7.504862]  [<ffffffff8147239a>] ieee80211_subif_start_xmit+0xf9a/0x1150
[    7.505494]  [<ffffffff81471442>] ?  ieee80211_subif_start_xmit+0x42/0x1150
[    7.506039]  [<ffffffff812e9ee1>] dev_hard_start_xmit+0x271/0x830
[    7.506394]  [<ffffffff812e9cc8>] ? dev_hard_start_xmit+0x58/0x830
[    7.506754]  [<ffffffff8130d822>] sch_direct_xmit+0xf2/0x1f0
[    7.507086]  [<ffffffff812ea704>] __dev_queue_xmit+0x264/0x8e0
[    7.507426]  [<ffffffff812ea4f0>] ? __dev_queue_xmit+0x50/0x8e0
[    7.507774]  [<ffffffff812eae00>] dev_queue_xmit+0x10/0x20
[    7.508110]  [<ffffffff813cdcba>] packet_sendmsg+0xdfa/0x10a0
[    7.508450]  [<ffffffff812cec59>] sock_sendmsg+0x69/0x90
[    7.508762]  [<ffffffff812ce8f5>] ? move_addr_to_kernel+0x45/0x70
[    7.509130]  [<ffffffff812d02e2>] SyS_sendto+0x112/0x150
[    7.509469]  [<ffffffff814e2995>] ? sysret_check+0x22/0x5d
[    7.509793]  [<ffffffff81080de5>] ?  trace_hardirqs_on_caller+0x105/0x1d0
[    7.510208]  [<ffffffff811e7eab>] ? trace_hardirqs_on_thunk+0x3a/0x3f
[    7.510587]  [<ffffffff814e2969>] system_call_fastpath+0x12/0x17
[    7.510941] ---[ end trace a7659d7a05f5ad35 ]---
[    7.511249] ------------[ cut here ]------------
[    7.511525] WARNING: CPU: 1 PID: 584 at net/mac80211/rx.c:3452 ieee80211_rx+0xb54/0xb60()
[    7.512001] Rate marked as a VHT rate but data is invalid: MCS: 11, NSS: 1
[    7.512542] CPU: 1 PID: 584 Comm: wpa_supplicant Tainted: G        W 3.18.0-rc4-wl #347
[    7.513210] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Bochs 01/01/2011
[    7.513839]  0000000000000009 ffff88001fc83d98 ffffffff814d7f34 ffffffff81089c41
[    7.514463]  ffff88001fc83de8 ffff88001fc83dd8 ffffffff8103ab6c 0000000000000001
[    7.514926]  ffff88001dcb7d00 ffff88001e1f07c0 ffff88001e1f07c0 ffff88001e1f0ce0
[    7.515388] Call Trace:
[    7.515536]  <IRQ>  [<ffffffff814d7f34>] dump_stack+0x4e/0x71
[    7.515895]  [<ffffffff81089c41>] ? console_unlock+0x1f1/0x4d0
[    7.516242]  [<ffffffff8103ab6c>] warn_slowpath_common+0x7c/0xa0
[    7.516597]  [<ffffffff8103ac06>] warn_slowpath_fmt+0x46/0x50
[    7.516949]  [<ffffffff8146e404>] ieee80211_rx+0xb54/0xb60
[    7.517316]  [<ffffffff81080d92>] ?  trace_hardirqs_on_caller+0xb2/0x1d0
[    7.517710]  [<ffffffff81080ebd>] ? trace_hardirqs_on+0xd/0x10
[    7.518056]  [<ffffffff81430d94>] ieee80211_tasklet_handler+0xd4/0xe0
[    7.518435]  [<ffffffff8103f3d7>] tasklet_action+0xe7/0xf0
[    7.518761]  [<ffffffff8103e780>] __do_softirq+0x150/0x610
[    7.519083]  [<ffffffff812ea74a>] ? __dev_queue_xmit+0x2aa/0x8e0
[    7.519453]  [<ffffffff814e43cc>] do_softirq_own_stack+0x1c/0x30
[    7.519804]  <EOI>  [<ffffffff8103ed2d>] do_softirq+0x7d/0x90
[    7.520193]  [<ffffffff8103ee27>] __local_bh_enable_ip+0xe7/0xf0
[    7.520707]  [<ffffffff812ea773>] __dev_queue_xmit+0x2d3/0x8e0
[    7.521047]  [<ffffffff812ea4f0>] ? __dev_queue_xmit+0x50/0x8e0
[    7.521420]  [<ffffffff812eae00>] dev_queue_xmit+0x10/0x20
[    7.521743]  [<ffffffff813cdcba>] packet_sendmsg+0xdfa/0x10a0
[    7.522082]  [<ffffffff812cec59>] sock_sendmsg+0x69/0x90
[    7.522396]  [<ffffffff812ce8f5>] ? move_addr_to_kernel+0x45/0x70
[    7.522759]  [<ffffffff812d02e2>] SyS_sendto+0x112/0x150
[    7.523078]  [<ffffffff814e2995>] ? sysret_check+0x22/0x5d
[    7.523403]  [<ffffffff81080de5>] ?  trace_hardirqs_on_caller+0x105/0x1d0
[    7.523797]  [<ffffffff811e7eab>] ? trace_hardirqs_on_thunk+0x3a/0x3f
[    7.524176]  [<ffffffff814e2969>] system_call_fastpath+0x12/0x17
[    7.524531] ---[ end trace a7659d7a05f5ad36 ]---
[    7.910139] wlan1: dropped frame to 02:00:00:00:00:00 (unauthorized port)
[    8.050101] wlan1: dropped frame to 02:00:00:00:00:00 (unauthorized port)
[    8.910107] wlan1: dropped frame to 02:00:00:00:00:00 (unauthorized port)
[    8.910754] wlan1: dropped frame to 02:00:00:00:00:00 (unauthorized port)
[    9.210099] wlan1: dropped frame to 02:00:00:00:00:00 (unauthorized port)
[   12.512254] ------------[ cut here ]------------
[   12.512542] WARNING: CPU: 1 PID: 585 at net/mac80211/rate.c:501 ieee80211_get_tx_rates+0x2c2/0x810()
...

-- 
Jouni Malinen                                            PGP id EFC895FA

Re: [PATCH v6] mac80211: minstrel_ht: add basic support for VHT rates <= 3SS@80MHz

[Karl Beldan]

Hi,

On Fri, Nov 14, 2014 at 06:43:28PM +0200, Jouni Malinen wrote:
> On Tue, Oct 21, 2014 at 10:38:38AM +0200, Karl Beldan wrote:
> > When the new CONFIG_MAC80211_RC_MINSTREL_VHT is not set (default 'N'),
> > there is no behavioral change including in sampling and MCS_GROUP_RATES
> > remains 8.
> > Otherwise MCS_GROUP_RATES is 10, and a module parameter *vht_only*
> > (default 'true'), restricts the rates selection to VHT when vht is
> > supported.
> 
> Have you tested this with mac80211_hwsim? I'm seeing number of issues in

Yes, only with iw and nothing fancy, I also have had it running on some 
boards for some weeks as is and with rfc version for more than a year at
work (minstrel not the whole tree).
I guess I can take the tip of wireless-testing but can you give your
HEAD though ?

> hwsim test cases with CONFIG_MAC80211_RC_MINSTREL_VHT=y with
> wireless-testing.git. These disappear if I disable this build option.
> 
> For example, with nfc_p2p_go_neg test case:
> 

Have you reproduced it with other testcases (maybe more regular so that
I can get my hands on it faster) ?
Is it also happening when VHT support is not advertized by upper layers
?


Rgds,
 
Karl

Re: [PATCH v6] mac80211: minstrel_ht: add basic support for VHT rates <= 3SS@80MHz

[Jouni Malinen]

On Fri, Nov 14, 2014 at 06:18:32PM +0100, Karl Beldan wrote:
> Yes, only with iw and nothing fancy, I also have had it running on some 
> boards for some weeks as is and with rfc version for more than a year at
> work (minstrel not the whole tree).
> I guess I can take the tip of wireless-testing but can you give your
> HEAD though ?

It looks like pretty much any snapshot between master-2014-11-04 and now
is affected.. Anyway, the current master snapshot in my tests was
0e204e2422876c18034ca960c4fccf727a02a5c1.

> > For example, with nfc_p2p_go_neg test case:
> 
> Have you reproduced it with other testcases (maybe more regular so that
> I can get my hands on it faster) ?
> Is it also happening when VHT support is not advertized by upper layers
> ?

This seems to be specific to exact timing of frames since the same issue
does not show up in non-NFC P2P test cases. So no, this does not show up
with anything else than the nfc_p2p_* test cases (well, at least not in
my desktop+kvm setup; YMMV with other CPU speeds that could potentially
affect timing):
nfc_p2p_go_neg
nfc_p2p_go_neg_reverse
nfc_p2p_ip_addr_assignment
nfc_p2p_static_handover_tagdev_client
nfc_p2p_static_handover_tagdev_client_group_iface
nfc_p2p_static_handover_tagdev_go

I'd assume the key here is in P2P with NFC trigger having the fastest
possible connection process due to all the optimizations in channel
selection on WPS (which is those EAPOL Data frames that hit these rate
selection issues immediately after association).

That said, if you use the tests/hwsim scripts from
git://w1.fi/hostap.git there should be no difference on running some of
these cases vs. something more regular..

If you have not used these test scripts previously, you can find more
information about the setup here:
http://w1.fi/cgit/hostap/plain/tests/hwsim/README
http://w1.fi/cgit/hostap/plain/tests/hwsim/vm/README

-- 
Jouni Malinen                                            PGP id EFC895FA

Re: [PATCH v6] mac80211: minstrel_ht: add basic support for VHT rates <= 3SS@80MHz

[Karl Beldan]

On Fri, Nov 14, 2014 at 07:36:48PM +0200, Jouni Malinen wrote:
> On Fri, Nov 14, 2014 at 06:18:32PM +0100, Karl Beldan wrote:
> > Yes, only with iw and nothing fancy, I also have had it running on some 
> > boards for some weeks as is and with rfc version for more than a year at
> > work (minstrel not the whole tree).
> > I guess I can take the tip of wireless-testing but can you give your
> > HEAD though ?
> 
> It looks like pretty much any snapshot between master-2014-11-04 and now
> is affected.. Anyway, the current master snapshot in my tests was
> 0e204e2422876c18034ca960c4fccf727a02a5c1.
> 
> > > For example, with nfc_p2p_go_neg test case:
> > 
> > Have you reproduced it with other testcases (maybe more regular so that
> > I can get my hands on it faster) ?
> > Is it also happening when VHT support is not advertized by upper layers
> > ?
> 
> This seems to be specific to exact timing of frames since the same issue
> does not show up in non-NFC P2P test cases. So no, this does not show up
> with anything else than the nfc_p2p_* test cases (well, at least not in
> my desktop+kvm setup; YMMV with other CPU speeds that could potentially
> affect timing):
> nfc_p2p_go_neg
> nfc_p2p_go_neg_reverse
> nfc_p2p_ip_addr_assignment
> nfc_p2p_static_handover_tagdev_client
> nfc_p2p_static_handover_tagdev_client_group_iface
> nfc_p2p_static_handover_tagdev_go
> 
> I'd assume the key here is in P2P with NFC trigger having the fastest
> possible connection process due to all the optimizations in channel
> selection on WPS (which is those EAPOL Data frames that hit these rate
> selection issues immediately after association).
> 
> That said, if you use the tests/hwsim scripts from
> git://w1.fi/hostap.git there should be no difference on running some of
> these cases vs. something more regular..
> 
> If you have not used these test scripts previously, you can find more
> information about the setup here:
> http://w1.fi/cgit/hostap/plain/tests/hwsim/README
> http://w1.fi/cgit/hostap/plain/tests/hwsim/vm/README
> 

Ok, thanks for your pointers Jouni, I should get back to you soon.
 
Karl

Re: [PATCH v6] mac80211: minstrel_ht: add basic support for VHT rates <= 3SS@80MHz

[Karl Beldan]

On Fri, Nov 14, 2014 at 07:36:48PM +0200, Jouni Malinen wrote:
> On Fri, Nov 14, 2014 at 06:18:32PM +0100, Karl Beldan wrote:
> > Yes, only with iw and nothing fancy, I also have had it running on some 
> > boards for some weeks as is and with rfc version for more than a year at
> > work (minstrel not the whole tree).
> > I guess I can take the tip of wireless-testing but can you give your
> > HEAD though ?
> 
> It looks like pretty much any snapshot between master-2014-11-04 and now
> is affected.. Anyway, the current master snapshot in my tests was
> 0e204e2422876c18034ca960c4fccf727a02a5c1.
> 

I tried to trigger the warning with it and nfc_p2p_* (even looping with
stop.sh; start.sh; run-tests.py nfc_p2p_*), to no avail, the tests are
successful and 0 warning.
I'll see if anything stands out in the code tomorrow.
 
Karl

> > > For example, with nfc_p2p_go_neg test case:
> > 
> > Have you reproduced it with other testcases (maybe more regular so that
> > I can get my hands on it faster) ?
> > Is it also happening when VHT support is not advertized by upper layers
> > ?
> 
> This seems to be specific to exact timing of frames since the same issue
> does not show up in non-NFC P2P test cases. So no, this does not show up
> with anything else than the nfc_p2p_* test cases (well, at least not in
> my desktop+kvm setup; YMMV with other CPU speeds that could potentially
> affect timing):
> nfc_p2p_go_neg
> nfc_p2p_go_neg_reverse
> nfc_p2p_ip_addr_assignment
> nfc_p2p_static_handover_tagdev_client
> nfc_p2p_static_handover_tagdev_client_group_iface
> nfc_p2p_static_handover_tagdev_go
> 
> I'd assume the key here is in P2P with NFC trigger having the fastest
> possible connection process due to all the optimizations in channel
> selection on WPS (which is those EAPOL Data frames that hit these rate
> selection issues immediately after association).
> 
> That said, if you use the tests/hwsim scripts from
> git://w1.fi/hostap.git there should be no difference on running some of
> these cases vs. something more regular..
> 
> If you have not used these test scripts previously, you can find more
> information about the setup here:
> http://w1.fi/cgit/hostap/plain/tests/hwsim/README
> http://w1.fi/cgit/hostap/plain/tests/hwsim/vm/README
> 
> -- 
> Jouni Malinen                                            PGP id EFC895FA

Re: [PATCH v5 0/4] add VHT support to minstrel_ht

[Johannes Berg]

On Mon, 2014-10-20 at 15:45 +0200, Karl Beldan wrote:
> From: Karl Beldan <karl.beldan@rivierawaves.com>
> 
> Hi,
> 
> Varka Bhadram reported checkpatch is noisy on this series.
> I made another pass which affects [2/4](trivially) and [4/4].
> This could have been a patch on top of what Felix acked, I hope you
> won't bother too much.

This series introduces the following new sparse/smatch complaints:

  CHECK   /home/johannes/sys/wireless/net/mac80211/rc80211_minstrel_ht.c
/home/johannes/sys/wireless/net/mac80211/rc80211_minstrel_ht.c:246:18: warning: cast to restricted __le16
/home/johannes/sys/wireless/net/mac80211/rc80211_minstrel_ht.c:1227:49: warning: incorrect type in argument 3 (different base types)
/home/johannes/sys/wireless/net/mac80211/rc80211_minstrel_ht.c:1227:49:    expected unsigned short [unsigned] [usertype] mcs_map
/home/johannes/sys/wireless/net/mac80211/rc80211_minstrel_ht.c:1227:49:    got restricted __le16 [usertype] tx_mcs_map

Here it looks like the argument to minstrel_get_valid_vht_rates() should
be declared __le16.

  CHECK   /home/johannes/sys/wireless/net/mac80211/rc80211_minstrel_ht_debugfs.c
/home/johannes/sys/wireless/net/mac80211/rc80211_minstrel_ht_debugfs.c:47 minstrel_ht_stats_dump() error: testing array offset 'i' after use.
/home/johannes/sys/wireless/net/mac80211/rc80211_minstrel_ht_debugfs.c:61 minstrel_ht_stats_dump() error: testing array offset 'i' after use.

Here I'm not sure, seems like it's a false positive maybe? The array
seems large enough for everything that's going on here.

johannes

Re: [PATCH v5 0/4] add VHT support to minstrel_ht

[Karl Beldan]

On Mon, Oct 20, 2014 at 04:49:06PM +0200, Johannes Berg wrote:
> On Mon, 2014-10-20 at 15:45 +0200, Karl Beldan wrote:
> > From: Karl Beldan <karl.beldan@rivierawaves.com>
> > 
> > Hi,
> > 
> > Varka Bhadram reported checkpatch is noisy on this series.
> > I made another pass which affects [2/4](trivially) and [4/4].
> > This could have been a patch on top of what Felix acked, I hope you
> > won't bother too much.
> 
> This series introduces the following new sparse/smatch complaints:
> 
>   CHECK   /home/johannes/sys/wireless/net/mac80211/rc80211_minstrel_ht.c
> /home/johannes/sys/wireless/net/mac80211/rc80211_minstrel_ht.c:246:18: warning: cast to restricted __le16
> /home/johannes/sys/wireless/net/mac80211/rc80211_minstrel_ht.c:1227:49: warning: incorrect type in argument 3 (different base types)
> /home/johannes/sys/wireless/net/mac80211/rc80211_minstrel_ht.c:1227:49:    expected unsigned short [unsigned] [usertype] mcs_map
> /home/johannes/sys/wireless/net/mac80211/rc80211_minstrel_ht.c:1227:49:    got restricted __le16 [usertype] tx_mcs_map
> 
> Here it looks like the argument to minstrel_get_valid_vht_rates() should
> be declared __le16.
> 
Indeed, will resending only 4/4 do ?

>   CHECK   /home/johannes/sys/wireless/net/mac80211/rc80211_minstrel_ht_debugfs.c
> /home/johannes/sys/wireless/net/mac80211/rc80211_minstrel_ht_debugfs.c:47 minstrel_ht_stats_dump() error: testing array offset 'i' after use.
> /home/johannes/sys/wireless/net/mac80211/rc80211_minstrel_ht_debugfs.c:61 minstrel_ht_stats_dump() error: testing array offset 'i' after use.
> 
> Here I'm not sure, seems like it's a false positive maybe? The array
> seems large enough for everything that's going on here.
> 
False positive.
I am using CF="-Wsparse-all" and I don't get those warnings with sparse
0.4.5, I'll update.

 
Karl

Re: [PATCH v5 0/4] add VHT support to minstrel_ht

[Karl Beldan]

On Mon, Oct 20, 2014 at 05:13:02PM +0200, Karl Beldan wrote:
> On Mon, Oct 20, 2014 at 04:49:06PM +0200, Johannes Berg wrote:
> > On Mon, 2014-10-20 at 15:45 +0200, Karl Beldan wrote:
> > > From: Karl Beldan <karl.beldan@rivierawaves.com>
> > > 
> > > Hi,
> > > 
> > > Varka Bhadram reported checkpatch is noisy on this series.
> > > I made another pass which affects [2/4](trivially) and [4/4].
> > > This could have been a patch on top of what Felix acked, I hope you
> > > won't bother too much.
> > 
> > This series introduces the following new sparse/smatch complaints:
> > 
> >   CHECK   /home/johannes/sys/wireless/net/mac80211/rc80211_minstrel_ht.c
> > /home/johannes/sys/wireless/net/mac80211/rc80211_minstrel_ht.c:246:18: warning: cast to restricted __le16
> > /home/johannes/sys/wireless/net/mac80211/rc80211_minstrel_ht.c:1227:49: warning: incorrect type in argument 3 (different base types)
> > /home/johannes/sys/wireless/net/mac80211/rc80211_minstrel_ht.c:1227:49:    expected unsigned short [unsigned] [usertype] mcs_map
> > /home/johannes/sys/wireless/net/mac80211/rc80211_minstrel_ht.c:1227:49:    got restricted __le16 [usertype] tx_mcs_map
> > 
> > Here it looks like the argument to minstrel_get_valid_vht_rates() should
> > be declared __le16.
> > 
> Indeed, will resending only 4/4 do ?
> 
> >   CHECK   /home/johannes/sys/wireless/net/mac80211/rc80211_minstrel_ht_debugfs.c
> > /home/johannes/sys/wireless/net/mac80211/rc80211_minstrel_ht_debugfs.c:47 minstrel_ht_stats_dump() error: testing array offset 'i' after use.
> > /home/johannes/sys/wireless/net/mac80211/rc80211_minstrel_ht_debugfs.c:61 minstrel_ht_stats_dump() error: testing array offset 'i' after use.
> > 
> > Here I'm not sure, seems like it's a false positive maybe? The array
> > seems large enough for everything that's going on here.
> > 
> False positive.
> I am using CF="-Wsparse-all" and I don't get those warnings with sparse
> 0.4.5, I'll update.
> 
I don't get those with smatch either .. what options are you using ?
Thanks for the pass.
 
Karl

Re: [PATCH v5 0/4] add VHT support to minstrel_ht

[Johannes Berg]

On Mon, 2014-10-20 at 17:34 +0200, Karl Beldan wrote:
> > Indeed, will resending only 4/4 do ?

Sure.

> > >   CHECK   /home/johannes/sys/wireless/net/mac80211/rc80211_minstrel_ht_debugfs.c
> > > /home/johannes/sys/wireless/net/mac80211/rc80211_minstrel_ht_debugfs.c:47 minstrel_ht_stats_dump() error: testing array offset 'i' after use.
> > > /home/johannes/sys/wireless/net/mac80211/rc80211_minstrel_ht_debugfs.c:61 minstrel_ht_stats_dump() error: testing array offset 'i' after use.
> > > 
> > > Here I'm not sure, seems like it's a false positive maybe? The array
> > > seems large enough for everything that's going on here.
> > > 
> > False positive.
> > I am using CF="-Wsparse-all" and I don't get those warnings with sparse
> > 0.4.5, I'll update.
> > 
> I don't get those with smatch either .. what options are you using ?
> Thanks for the pass.

I'm using smatch git as of today (5167af76ad64) with no special
arguments. I have a script called "sparse" that does just this:

sparse-real -Wshadow "$@" || exit 100
smatch -p=kernel "$@" || exit 200

johannes

Re: [PATCH v5 0/4] add VHT support to minstrel_ht

[Johannes Berg]

On Mon, 2014-10-20 at 21:33 +0200, Johannes Berg wrote:
> On Mon, 2014-10-20 at 17:34 +0200, Karl Beldan wrote:
> > > Indeed, will resending only 4/4 do ?
> 
> Sure.

I merged patches 1-3 now.

johannes

Re: [PATCH v5 0/4] add VHT support to minstrel_ht

[Karl Beldan]

On Mon, Oct 20, 2014 at 09:40:35PM +0200, Johannes Berg wrote:
> On Mon, 2014-10-20 at 21:33 +0200, Johannes Berg wrote:
> > On Mon, 2014-10-20 at 17:34 +0200, Karl Beldan wrote:
> > > > Indeed, will resending only 4/4 do ?
> > 
> > Sure.
> 
> I merged patches 1-3 now.
> 
Thanks.
 
karl

Re: [PATCH v5 0/4] add VHT support to minstrel_ht

[Karl Beldan]

On Mon, Oct 20, 2014 at 09:33:17PM +0200, Johannes Berg wrote:
> On Mon, 2014-10-20 at 17:34 +0200, Karl Beldan wrote:
> > > Indeed, will resending only 4/4 do ?
> 
> Sure.
> 
> > > >   CHECK   /home/johannes/sys/wireless/net/mac80211/rc80211_minstrel_ht_debugfs.c
> > > > /home/johannes/sys/wireless/net/mac80211/rc80211_minstrel_ht_debugfs.c:47 minstrel_ht_stats_dump() error: testing array offset 'i' after use.
> > > > /home/johannes/sys/wireless/net/mac80211/rc80211_minstrel_ht_debugfs.c:61 minstrel_ht_stats_dump() error: testing array offset 'i' after use.
> > > > 
> > > > Here I'm not sure, seems like it's a false positive maybe? The array
> > > > seems large enough for everything that's going on here.
> > > > 
> > > False positive.
> > > I am using CF="-Wsparse-all" and I don't get those warnings with sparse
> > > 0.4.5, I'll update.
> > > 
> > I don't get those with smatch either .. what options are you using ?
> > Thanks for the pass.
> 
> I'm using smatch git as of today (5167af76ad64) with no special
> arguments. I have a script called "sparse" that does just this:
> 
> sparse-real -Wshadow "$@" || exit 100
> smatch -p=kernel "$@" || exit 200
> 
I still don't know why I don't see the smatch array warnings here, I
guess I'll live with that for now.
 
Karl

Re: [PATCH v5 0/4] add VHT support to minstrel_ht

[Johannes Berg]

On Tue, 2014-10-21 at 17:26 +0200, Karl Beldan wrote:

> I still don't know why I don't see the smatch array warnings here, I
> guess I'll live with that for now.

I'm hoping Dan will eventually pick up on it ;-)

johannes

Re: [PATCH v5 0/4] add VHT support to minstrel_ht

[Karl Beldan]

On Tue, Oct 21, 2014 at 08:47:26PM +0200, Johannes Berg wrote:
> On Tue, 2014-10-21 at 17:26 +0200, Karl Beldan wrote:
> 
> > I still don't know why I don't see the smatch array warnings here, I
> > guess I'll live with that for now.
> 
> I'm hoping Dan will eventually pick up on it ;-)
> 
smatch is good, I get your warnings now when unsetting
MAC80211_RC_MINSTREL_VHT. I did not click until after checking
checking smatch's check_overflow.c, when there's a use after check, it
only barks when the indexes checked are out of bounds.

As for the false positive, now we can check the flags instead of doing
comparisons on the indexes, it'd be cleaner and spare this warning,
maybe I should send something.
 
Karl

Re: [PATCH v5 0/4] add VHT support to minstrel_ht

[Johannes Berg]

On Wed, 2014-10-22 at 15:54 +0200, Karl Beldan wrote:
> On Tue, Oct 21, 2014 at 08:47:26PM +0200, Johannes Berg wrote:
> > On Tue, 2014-10-21 at 17:26 +0200, Karl Beldan wrote:
> > 
> > > I still don't know why I don't see the smatch array warnings here, I
> > > guess I'll live with that for now.
> > 
> > I'm hoping Dan will eventually pick up on it ;-)
> > 
> smatch is good, I get your warnings now when unsetting
> MAC80211_RC_MINSTREL_VHT. I did not click until after checking
> checking smatch's check_overflow.c, when there's a use after check, it
> only barks when the indexes checked are out of bounds.

Wait, are you saying it *is* out of bounds when VHT isn't turned on?

johannes

Re: [PATCH v5 0/4] add VHT support to minstrel_ht

[Karl Beldan]

On Wed, Oct 22, 2014 at 04:07:31PM +0200, Johannes Berg wrote:
> On Wed, 2014-10-22 at 15:54 +0200, Karl Beldan wrote:
> > On Tue, Oct 21, 2014 at 08:47:26PM +0200, Johannes Berg wrote:
> > > On Tue, 2014-10-21 at 17:26 +0200, Karl Beldan wrote:
> > > 
> > > > I still don't know why I don't see the smatch array warnings here, I
> > > > guess I'll live with that for now.
> > > 
> > > I'm hoping Dan will eventually pick up on it ;-)
> > > 
> > smatch is good, I get your warnings now when unsetting
> > MAC80211_RC_MINSTREL_VHT. I did not click until after checking
> > checking smatch's check_overflow.c, when there's a use after check, it
> > only barks when the indexes checked are out of bounds.
> 
> Wait, are you saying it *is* out of bounds when VHT isn't turned on?
> 

The smatch warning pops on
{
if (!(mi->groups[i].supported & BIT(j)))
	continue;
	...
else if (i >= MINSTREL_VHT_GROUP_0)
}

MINSTREL_VHT_GROUP_0 is out of bonds (not i which remains <
ARRAY_SIZE(mi->groups)).

 
Karl

Re: [PATCH v5 0/4] add VHT support to minstrel_ht

[Johannes Berg]

On Wed, 2014-10-22 at 16:15 +0200, Karl Beldan wrote:

> > Wait, are you saying it *is* out of bounds when VHT isn't turned on?
> > 
> 
> The smatch warning pops on
> {
> if (!(mi->groups[i].supported & BIT(j)))
> 	continue;
> 	...
> else if (i >= MINSTREL_VHT_GROUP_0)
> }
> 
> MINSTREL_VHT_GROUP_0 is out of bonds (not i which remains <
> ARRAY_SIZE(mi->groups)).

Ah, ok, right.

johannes