Discussion:
[PATCH v5 0/4] add VHT support to minstrel_ht
Karl Beldan
2014-10-20 13:45:58 UTC
Permalink
From: Karl Beldan <karl.beldan-***@public.gmane.org>

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 <= ***@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

--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo-***@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Karl Beldan
2014-10-20 13:45:59 UTC
Permalink
From: Karl Beldan <karl.beldan-***@public.gmane.org>

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 ***@40MHz, consuming:
8(MCS_GROUP_RATES) * (3(SS)*2(GI)*2(BW)+1(CCK)), i.e. 104 entries.

Support for ***@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-***@public.gmane.org>
Cc: Felix Fietkau <nbd-***@public.gmane.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

--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo-***@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Karl Beldan
2014-10-20 13:46:01 UTC
Permalink
From: Karl Beldan <karl.beldan-***@public.gmane.org>

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-***@public.gmane.org>
Cc: Felix Fietkau <nbd-***@public.gmane.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

--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo-***@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Karl Beldan
2014-10-20 13:46:00 UTC
Permalink
From: Karl Beldan <karl.beldan-***@public.gmane.org>

No functional change.

Signed-off-by: Karl Beldan <karl.beldan-***@public.gmane.org>
Cc: Felix Fietkau <nbd-***@public.gmane.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

--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo-***@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Karl Beldan
2014-10-20 13:46:02 UTC
Permalink
From: Karl Beldan <karl.beldan-***@public.gmane.org>

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-***@public.gmane.org>
Cc: Felix Fietkau <nbd-***@public.gmane.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 ***@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");
Karl Beldan
2014-10-21 08:38:38 UTC
Permalink
From: Karl Beldan <karl.beldan-***@public.gmane.org>

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-***@public.gmane.org>
Cc: Felix Fietkau <nbd-***@public.gmane.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 ***@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");
Johannes Berg
2014-10-20 14:49:06 UTC
Permalink
On Mon, 2014-10-20 at 15:45 +0200, Karl Beldan wrote:
> From: Karl Beldan <karl.beldan-***@public.gmane.org>
>
> 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

--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo-***@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Karl Beldan
2014-10-20 15:13:02 UTC
Permalink
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-***@public.gmane.org>
> >
> > 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
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo-***@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Karl Beldan
2014-10-20 15:34:21 UTC
Permalink
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-***@public.gmane.org>
> > >
> > > 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
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo-***@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Johannes Berg
2014-10-20 19:33:17 UTC
Permalink
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

--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo-***@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Johannes Berg
2014-10-20 19:40:35 UTC
Permalink
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

--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo-***@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Karl Beldan
2014-10-21 08:41:50 UTC
Permalink
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
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo-***@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Karl Beldan
2014-10-21 15:26:58 UTC
Permalink
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
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo-***@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Johannes Berg
2014-10-21 18:47:26 UTC
Permalink
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

--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo-***@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Karl Beldan
2014-10-22 13:54:56 UTC
Permalink
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
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo-***@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Johannes Berg
2014-10-22 14:07:31 UTC
Permalink
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

--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo-***@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Karl Beldan
2014-10-22 14:15:48 UTC
Permalink
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
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo-***@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Johannes Berg
2014-10-22 14:16:34 UTC
Permalink
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

--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo-***@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Loading...