2013-04-15 15:14:10

by Karl Beldan

[permalink] [raw]
Subject: [PATCH 1/2] mac80211: VHT off-by-one NSS

From: Karl Beldan <[email protected]>

The number of VHT spatial streams (NSS) is found in:
- s8 ieee80211_tx_rate.rate.idx[6:4] (tx - filled by rate control)
- u8 ieee80211_rx_status.vht_nss (rx - filled by driver)
Tx discriminates valid rates indexes with the sign bit and encodes NSS
starting from 0 to 7 (note this matches some hw encodings e.g IWLMVM).
Rx does not have the same constraints, and encodes NSS starting from 1
to 8 (note this matches what wireshark expects in the radiotap header).

To handle ieee80211_tx_rate.rate.idx[6:4] ieee80211_rate_set_vht() and
ieee80211_rate_get_vht_nss() assume their nss parameter and return value
respectively runs from 0 to 7.
ATM, there are only 2 users of these: cfg.c:sta_set_rate_info_t() and
iwlwifi/mvm/tx.c:iwl_mvm_hwrate_to_tx_control(), but both assume nss
runs from 1 to 8.
This patch fixes this inconsistency by making ieee80211_rate_set_vht()
and ieee80211_rate_get_vht_nss() handle an nss running from 1 to 8.

Signed-off-by: Karl Beldan <[email protected]>
---
This might feel very verbose for such a trivial thing.

include/net/mac80211.h | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/include/net/mac80211.h b/include/net/mac80211.h
index 0dde213..2317ca9 100644
--- a/include/net/mac80211.h
+++ b/include/net/mac80211.h
@@ -601,8 +601,8 @@ static inline void ieee80211_rate_set_vht(struct ieee80211_tx_rate *rate,
u8 mcs, u8 nss)
{
WARN_ON(mcs & ~0xF);
- WARN_ON(nss & ~0x7);
- rate->idx = (nss << 4) | mcs;
+ WARN_ON((nss - 1) & ~0x7);
+ rate->idx = ((nss - 1) << 4) | mcs;
}

static inline u8
@@ -614,7 +614,7 @@ ieee80211_rate_get_vht_mcs(const struct ieee80211_tx_rate *rate)
static inline u8
ieee80211_rate_get_vht_nss(const struct ieee80211_tx_rate *rate)
{
- return rate->idx >> 4;
+ return (rate->idx >> 4) + 1;
}

/**
--
1.8.2



2013-04-15 15:14:11

by Karl Beldan

[permalink] [raw]
Subject: [PATCH 2/2] mac80211_hwsim: handle VHT rates in rx_status

From: Karl Beldan <[email protected]>

Signed-off-by: Karl Beldan <[email protected]>
---
drivers/net/wireless/mac80211_hwsim.c | 14 +++++++++++---
1 file changed, 11 insertions(+), 3 deletions(-)

diff --git a/drivers/net/wireless/mac80211_hwsim.c b/drivers/net/wireless/mac80211_hwsim.c
index b5117f5..7ede240 100644
--- a/drivers/net/wireless/mac80211_hwsim.c
+++ b/drivers/net/wireless/mac80211_hwsim.c
@@ -718,9 +718,17 @@ static bool mac80211_hwsim_tx_frame_no_nl(struct ieee80211_hw *hw,
rx_status.flag |= RX_FLAG_MACTIME_START;
rx_status.freq = chan->center_freq;
rx_status.band = chan->band;
- rx_status.rate_idx = info->control.rates[0].idx;
- if (info->control.rates[0].flags & IEEE80211_TX_RC_MCS)
- rx_status.flag |= RX_FLAG_HT;
+ if (info->control.rates[0].flags & IEEE80211_TX_RC_VHT_MCS) {
+ rx_status.rate_idx =
+ ieee80211_rate_get_vht_mcs(&info->control.rates[0]);
+ rx_status.vht_nss =
+ ieee80211_rate_get_vht_nss(&info->control.rates[0]);
+ rx_status.flag |= RX_FLAG_VHT;
+ } else {
+ rx_status.rate_idx = info->control.rates[0].idx;
+ if (info->control.rates[0].flags & IEEE80211_TX_RC_MCS)
+ rx_status.flag |= RX_FLAG_HT;
+ }
if (info->control.rates[0].flags & IEEE80211_TX_RC_40_MHZ_WIDTH)
rx_status.flag |= RX_FLAG_40MHZ;
if (info->control.rates[0].flags & IEEE80211_TX_RC_SHORT_GI)
--
1.8.2


2013-04-16 14:06:23

by Johannes Berg

[permalink] [raw]
Subject: Re: [PATCH 1/2] mac80211: VHT off-by-one NSS

On Mon, 2013-04-15 at 17:09 +0200, Karl Beldan wrote:
> From: Karl Beldan <[email protected]>
>
> The number of VHT spatial streams (NSS) is found in:
> - s8 ieee80211_tx_rate.rate.idx[6:4] (tx - filled by rate control)
> - u8 ieee80211_rx_status.vht_nss (rx - filled by driver)
> Tx discriminates valid rates indexes with the sign bit and encodes NSS
> starting from 0 to 7 (note this matches some hw encodings e.g IWLMVM).
> Rx does not have the same constraints, and encodes NSS starting from 1
> to 8 (note this matches what wireshark expects in the radiotap header).
>
> To handle ieee80211_tx_rate.rate.idx[6:4] ieee80211_rate_set_vht() and
> ieee80211_rate_get_vht_nss() assume their nss parameter and return value
> respectively runs from 0 to 7.
> ATM, there are only 2 users of these: cfg.c:sta_set_rate_info_t() and
> iwlwifi/mvm/tx.c:iwl_mvm_hwrate_to_tx_control(), but both assume nss
> runs from 1 to 8.
> This patch fixes this inconsistency by making ieee80211_rate_set_vht()
> and ieee80211_rate_get_vht_nss() handle an nss running from 1 to 8.
>
> Signed-off-by: Karl Beldan <[email protected]>
> ---
> This might feel very verbose for such a trivial thing.

I think it's good, it's clearly a tricky area :-)

Applied both.

Btw, it seems we're missing VHT rates in net/mac80211/status.c, I was
wondering why that wasn't wrong but it's simply not there, heh. In fact
we probably mess it up when using VHT ...

johannes