The QCA4019 firmware 10.4-3.2.1-00050 reports only HT MCS rates between
0-9. But 802.11n MCS rates can be larger than that. For example a 2x2
device can send with up to MCS 15.
The firmware encodes the higher MCS rates using the NSS field. The actual
calculation is not documented by QCA but it seems like the NSS field can be
mapped for HT rates to following MCS offsets:
* NSS 1: 0
* NSS 2: 8
* NSS 3: 16
* NSS 4: 24
This offset therefore has to be added for HT rates before they are stored
in the rate_info struct.
Fixes: cec17c382140 ("ath10k: add per peer htt tx stats support for 10.4")
Signed-off-by: Sven Eckelmann <[email protected]>
---
drivers/net/wireless/ath/ath10k/htt_rx.c | 14 ++++++++++----
1 file changed, 10 insertions(+), 4 deletions(-)
diff --git a/drivers/net/wireless/ath/ath10k/htt_rx.c b/drivers/net/wireless/ath/ath10k/htt_rx.c
index 84b6067ff6e7..6c0a821fe79d 100644
--- a/drivers/net/wireless/ath/ath10k/htt_rx.c
+++ b/drivers/net/wireless/ath/ath10k/htt_rx.c
@@ -2229,9 +2229,15 @@ ath10k_update_per_peer_tx_stats(struct ath10k *ar,
txrate.mcs = ATH10K_HW_MCS_RATE(peer_stats->ratecode);
sgi = ATH10K_HW_GI(peer_stats->flags);
- if (((txrate.flags == WMI_RATE_PREAMBLE_HT) ||
- (txrate.flags == WMI_RATE_PREAMBLE_VHT)) && txrate.mcs > 9) {
- ath10k_warn(ar, "Invalid mcs %hhd peer stats", txrate.mcs);
+ if (txrate.flags == WMI_RATE_PREAMBLE_VHT && txrate.mcs > 9) {
+ ath10k_warn(ar, "Invalid VHT mcs %hhd peer stats", txrate.mcs);
+ return;
+ }
+
+ if (txrate.flags == WMI_RATE_PREAMBLE_HT &&
+ (txrate.mcs > 7 || txrate.nss < 1)) {
+ ath10k_warn(ar, "Invalid HT mcs %hhd nss %hhd peer stats",
+ txrate.mcs, txrate.nss);
return;
}
@@ -2254,7 +2260,7 @@ ath10k_update_per_peer_tx_stats(struct ath10k *ar,
arsta->txrate.legacy = rate;
} else if (txrate.flags == WMI_RATE_PREAMBLE_HT) {
arsta->txrate.flags = RATE_INFO_FLAGS_MCS;
- arsta->txrate.mcs = txrate.mcs;
+ arsta->txrate.mcs = txrate.mcs + 8 * (txrate.nss - 1);
} else {
arsta->txrate.flags = RATE_INFO_FLAGS_VHT_MCS;
arsta->txrate.mcs = txrate.mcs;
--
2.11.0
On Donnerstag, 11. Mai 2017 11:39:46 CEST Arend Van Spriel wrote:
[...]
> So you leave VHT as is. Did you check with 11ac device? I am wondering
> if it needs the same change.
VHT MCS rates are reported by drivers with NSS + MCS rates 0-9 [1] as
separated values. So I would say that the code is correct to report them as
separate values. But no, I haven't tested with an 802.11ac capable device
which I can fully trust because I didn't have one in arms reach (and I would
have to compile a new firmware for it). But the output for a second ath10k
based device looks ok'ish:
Station ac:86:74:61:6b:30 (on wlan1)
inactive time: 700 ms
rx bytes: 10741
rx packets: 95
tx bytes: 9886
tx packets: 86
tx retries: 0
tx failed: 0
rx drop misc: 1
signal: -22 dBm
signal avg: -21 dBm
tx bitrate: 780.0 MBit/s VHT-MCS 8 80MHz short GI VHT-NSS 2
rx bitrate: 780.0 MBit/s VHT-MCS 8 80MHz short GI VHT-NSS 2
rx duration: 8172 us
authorized: yes
authenticated: yes
associated: yes
preamble: long
WMM/WME: yes
MFP: no
TDLS peer: no
DTIM period: 2
beacon interval:100
short slot time:yes
connected time: 106 seconds
Kind regards,
Sven
[1] http://mcsindex.com/
Sven Eckelmann <[email protected]> wrote:
> The QCA4019 firmware 10.4-3.2.1-00050 reports only HT MCS rates between
> 0-9. But 802.11n MCS rates can be larger than that. For example a 2x2
> device can send with up to MCS 15.
>
> The firmware encodes the higher MCS rates using the NSS field. The actual
> calculation is not documented by QCA but it seems like the NSS field can be
> mapped for HT rates to following MCS offsets:
>
> * NSS 1: 0
> * NSS 2: 8
> * NSS 3: 16
> * NSS 4: 24
>
> This offset therefore has to be added for HT rates before they are stored
> in the rate_info struct.
>
> Fixes: cec17c382140 ("ath10k: add per peer htt tx stats support for 10.4")
> Signed-off-by: Sven Eckelmann <[email protected]>
Patch applied to ath-next branch of ath.git, thanks.
c1dd8016ae02 ath10k: fix reported HT MCS rates with NSS > 1
--
https://patchwork.kernel.org/patch/9721111/
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
On 11-5-2017 11:09, Sven Eckelmann wrote:
> The QCA4019 firmware 10.4-3.2.1-00050 reports only HT MCS rates between
> 0-9. But 802.11n MCS rates can be larger than that. For example a 2x2
> device can send with up to MCS 15.
>
> The firmware encodes the higher MCS rates using the NSS field. The actual
> calculation is not documented by QCA but it seems like the NSS field can be
> mapped for HT rates to following MCS offsets:
>
> * NSS 1: 0
> * NSS 2: 8
> * NSS 3: 16
> * NSS 4: 24
>
> This offset therefore has to be added for HT rates before they are stored
> in the rate_info struct.
>
> Fixes: cec17c382140 ("ath10k: add per peer htt tx stats support for 10.4")
> Signed-off-by: Sven Eckelmann <[email protected]>
> ---
> drivers/net/wireless/ath/ath10k/htt_rx.c | 14 ++++++++++----
> 1 file changed, 10 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/net/wireless/ath/ath10k/htt_rx.c b/drivers/net/wireless/ath/ath10k/htt_rx.c
> index 84b6067ff6e7..6c0a821fe79d 100644
> --- a/drivers/net/wireless/ath/ath10k/htt_rx.c
> +++ b/drivers/net/wireless/ath/ath10k/htt_rx.c
> @@ -2229,9 +2229,15 @@ ath10k_update_per_peer_tx_stats(struct ath10k *ar,
> txrate.mcs = ATH10K_HW_MCS_RATE(peer_stats->ratecode);
> sgi = ATH10K_HW_GI(peer_stats->flags);
>
> - if (((txrate.flags == WMI_RATE_PREAMBLE_HT) ||
> - (txrate.flags == WMI_RATE_PREAMBLE_VHT)) && txrate.mcs > 9) {
> - ath10k_warn(ar, "Invalid mcs %hhd peer stats", txrate.mcs);
> + if (txrate.flags == WMI_RATE_PREAMBLE_VHT && txrate.mcs > 9) {
> + ath10k_warn(ar, "Invalid VHT mcs %hhd peer stats", txrate.mcs);
> + return;
> + }
So you leave VHT as is. Did you check with 11ac device? I am wondering
if it needs the same change.
Regards,
Arend
the assumption made in this patch is obviously wrong (at least for more
recent firmwares and 9984)
my log is flooded with messages like
[208802.803537] ath10k_pci 0001:03:00.0: Invalid VHT mcs 15 peer stats
[208805.108515] ath10k_pci 0001:03:00.0: Invalid VHT mcs 15 peer stats
[208821.747621] ath10k_pci 0001:03:00.0: Invalid VHT mcs 15 peer stats
[208822.516599] ath10k_pci 0001:03:00.0: Invalid VHT mcs 15 peer stats
[208841.257780] ath10k_pci 0001:03:00.0: Invalid VHT mcs 15 peer stats
i tested this with the 10.4.3.5-0038 firmware which isnt official
published but made from athwlan.bin i got from qca chipcode
Am 23.05.2017 um 17:28 schrieb Kalle Valo:
> Sven Eckelmann <[email protected]> wrote:
>> The QCA4019 firmware 10.4-3.2.1-00050 reports only HT MCS rates between
>> 0-9. But 802.11n MCS rates can be larger than that. For example a 2x2
>> device can send with up to MCS 15.
>>
>> The firmware encodes the higher MCS rates using the NSS field. The actual
>> calculation is not documented by QCA but it seems like the NSS field can be
>> mapped for HT rates to following MCS offsets:
>>
>> * NSS 1: 0
>> * NSS 2: 8
>> * NSS 3: 16
>> * NSS 4: 24
>>
>> This offset therefore has to be added for HT rates before they are stored
>> in the rate_info struct.
>>
>> Fixes: cec17c382140 ("ath10k: add per peer htt tx stats support for 10.4")
>> Signed-off-by: Sven Eckelmann <[email protected]>
> Patch applied to ath-next branch of ath.git, thanks.
>
> c1dd8016ae02 ath10k: fix reported HT MCS rates with NSS > 1
>
--
Mit freundlichen Grüssen / Regards
Sebastian Gottschall / CTO
NewMedia-NET GmbH - DD-WRT
Firmensitz: Stubenwaldallee 21a, 64625 Bensheim
Registergericht: Amtsgericht Darmstadt, HRB 25473
Geschäftsführer: Peter Steinhäuser, Christian Scheele
http://www.dd-wrt.com
email: [email protected]
Tel.: +496251-582650 / Fax: +496251-5826565
Am 06.11.2017 um 09:23 schrieb Sven Eckelmann:
> On Sonntag, 5. November 2017 10:22:22 CET Sebastian Gottschall wrote:
>> the assumption made in this patch is obviously wrong (at least for more
>> recent firmwares and 9984)
>> my log is flooded with messages like
>> [208802.803537] ath10k_pci 0001:03:00.0: Invalid VHT mcs 15 peer stats
>> [208805.108515] ath10k_pci 0001:03:00.0: Invalid VHT mcs 15 peer stats
>> [208821.747621] ath10k_pci 0001:03:00.0: Invalid VHT mcs 15 peer stats
>> [208822.516599] ath10k_pci 0001:03:00.0: Invalid VHT mcs 15 peer stats
>> [208841.257780] ath10k_pci 0001:03:00.0: Invalid VHT mcs 15 peer stats
>>
>>
>> i tested this with the 10.4.3.5-0038 firmware which isnt official
>> published but made from athwlan.bin i got from qca chipcode
> Hm, yes there is most likely something wrong. But not sure whether this patch
> is actually the culprit. It looks to me like this would also be reported the
> same way without the patch. cec17c382140 ("ath10k: add per peer htt tx stats
> support for 10.4") seems to be your problem, right?
>
> This patch only splits WMI_RATE_PREAMBLE_HT & WMI_RATE_PREAMBLE_VHT. And for
> WMI_RATE_PREAMBLE_HT (*not VHT*), it uses a slightly different approach. But
> the WMI_RATE_PREAMBLE_VHT part (which you see in your logs) is basically
> untouched.
then a question follows up. is this check really neccessary?
Sebastian
>
> Kind regards,
> Sven
--
Mit freundlichen Gr?ssen / Regards
Sebastian Gottschall / CTO
NewMedia-NET GmbH - DD-WRT
Firmensitz: Stubenwaldallee 21a, 64625 Bensheim
Registergericht: Amtsgericht Darmstadt, HRB 25473
Gesch?ftsf?hrer: Peter Steinh?user, Christian Scheele
http://www.dd-wrt.com
email: [email protected]
Tel.: +496251-582650 / Fax: +496251-5826565
On Sonntag, 5. November 2017 10:22:22 CET Sebastian Gottschall wrote:
> the assumption made in this patch is obviously wrong (at least for more
> recent firmwares and 9984)
> my log is flooded with messages like
> [208802.803537] ath10k_pci 0001:03:00.0: Invalid VHT mcs 15 peer stats
> [208805.108515] ath10k_pci 0001:03:00.0: Invalid VHT mcs 15 peer stats
> [208821.747621] ath10k_pci 0001:03:00.0: Invalid VHT mcs 15 peer stats
> [208822.516599] ath10k_pci 0001:03:00.0: Invalid VHT mcs 15 peer stats
> [208841.257780] ath10k_pci 0001:03:00.0: Invalid VHT mcs 15 peer stats
>
>
> i tested this with the 10.4.3.5-0038 firmware which isnt official
> published but made from athwlan.bin i got from qca chipcode
Hm, yes there is most likely something wrong. But not sure whether this patch
is actually the culprit. It looks to me like this would also be reported the
same way without the patch. cec17c382140 ("ath10k: add per peer htt tx stats
support for 10.4") seems to be your problem, right?
This patch only splits WMI_RATE_PREAMBLE_HT & WMI_RATE_PREAMBLE_VHT. And for
WMI_RATE_PREAMBLE_HT (*not VHT*), it uses a slightly different approach. But
the WMI_RATE_PREAMBLE_VHT part (which you see in your logs) is basically
untouched.
Kind regards,
Sven
On Montag, 6. November 2017 09:28:42 CET Sebastian Gottschall wrote:
> Am 06.11.2017 um 09:23 schrieb Sven Eckelmann:
> > On Sonntag, 5. November 2017 10:22:22 CET Sebastian Gottschall wrote:
> >> the assumption made in this patch is obviously wrong (at least for more
> >> recent firmwares and 9984)
> >> my log is flooded with messages like
> >> [208802.803537] ath10k_pci 0001:03:00.0: Invalid VHT mcs 15 peer stats
> >> [208805.108515] ath10k_pci 0001:03:00.0: Invalid VHT mcs 15 peer stats
> >> [208821.747621] ath10k_pci 0001:03:00.0: Invalid VHT mcs 15 peer stats
> >> [208822.516599] ath10k_pci 0001:03:00.0: Invalid VHT mcs 15 peer stats
> >> [208841.257780] ath10k_pci 0001:03:00.0: Invalid VHT mcs 15 peer stats
[...]
> > This patch only splits WMI_RATE_PREAMBLE_HT & WMI_RATE_PREAMBLE_VHT. And for
> > WMI_RATE_PREAMBLE_HT (*not VHT*), it uses a slightly different approach. But
> > the WMI_RATE_PREAMBLE_VHT part (which you see in your logs) is basically
> > untouched.
>
> then a question follows up. is this check really neccessary?
Until we find out what the heck VHT MCS 15 should mean in this context - maybe.
But to the message - no, the message is most likely not necessary for each
received "invalid" peer tx stat.
Kind regards.
Sven
On 2017-11-20 17:40, Kalle Valo wrote:
> Peter Oh <[email protected]> writes:
>
>> On 11/06/2017 01:02 AM, Sven Eckelmann wrote:
>>> On Montag, 6. November 2017 09:28:42 CET Sebastian Gottschall wrote:
>>>> Am 06.11.2017 um 09:23 schrieb Sven Eckelmann:
>>>>> On Sonntag, 5. November 2017 10:22:22 CET Sebastian Gottschall
>>>>> wrote:
>>>>>> the assumption made in this patch is obviously wrong (at least for
>>>>>> more
>>>>>> recent firmwares and 9984)
>>>>>> my log is flooded with messages like
>>>>>> [208802.803537] ath10k_pci 0001:03:00.0: Invalid VHT mcs 15 peer
>>>>>> stats
>>>>>> [208805.108515] ath10k_pci 0001:03:00.0: Invalid VHT mcs 15 peer
>>>>>> stats
>>>>>> [208821.747621] ath10k_pci 0001:03:00.0: Invalid VHT mcs 15 peer
>>>>>> stats
>>>>>> [208822.516599] ath10k_pci 0001:03:00.0: Invalid VHT mcs 15 peer
>>>>>> stats
>>>>>> [208841.257780] ath10k_pci 0001:03:00.0: Invalid VHT mcs 15 peer
>>>>>> stats
>>> [...]
>>>>> This patch only splits WMI_RATE_PREAMBLE_HT &
>>>>> WMI_RATE_PREAMBLE_VHT. And for
>>>>> WMI_RATE_PREAMBLE_HT (*not VHT*), it uses a slightly different
>>>>> approach. But
>>>>> the WMI_RATE_PREAMBLE_VHT part (which you see in your logs) is
>>>>> basically
>>>>> untouched.
>>>> then a question follows up. is this check really neccessary?
>>> Until we find out what the heck VHT MCS 15 should mean in this
>>> context - maybe.
>>> But to the message - no, the message is most likely not necessary for
>>> each
>>> received "invalid" peer tx stat.
>>
>> This validation check expects peer tx stat packets from FW contain
>> reasonable values and gives warning if values are different from
>> expectation. The problem comes from the assumption that "it always
>> contains reasonable stat value" which is wrong at least based on my
>> results. For instance, the reason MCS == 15 is because all 4 bits of
>> mcs potion set to 1, not because FW really sets it to 15
>> intentionally. when the mcs potion bits are set to all 1s, the other
>> bits such as nss are also set to all 1s.
>> Hence it looks FW passed totally invalid stat info to me.
>>
>> I don't have preference on whether it's better to split VHT and HT
>> check or not, but it is more appropriate to change that warning level
>> to debug level.
>
> I think we should add a special case to not print the warning if mcs ==
> 15 until we figure out what it means.
Fix identified in Firmware and will push ASAP.
--
Anil.
Peter Oh <[email protected]> writes:
> On 11/06/2017 01:02 AM, Sven Eckelmann wrote:
>> On Montag, 6. November 2017 09:28:42 CET Sebastian Gottschall wrote:
>>> Am 06.11.2017 um 09:23 schrieb Sven Eckelmann:
>>>> On Sonntag, 5. November 2017 10:22:22 CET Sebastian Gottschall wrote:
>>>>> the assumption made in this patch is obviously wrong (at least for mo=
re
>>>>> recent firmwares and 9984)
>>>>> my log is flooded with messages like
>>>>> [208802.803537] ath10k_pci 0001:03:00.0: Invalid VHT mcs 15 peer stat=
s
>>>>> [208805.108515] ath10k_pci 0001:03:00.0: Invalid VHT mcs 15 peer stat=
s
>>>>> [208821.747621] ath10k_pci 0001:03:00.0: Invalid VHT mcs 15 peer stat=
s
>>>>> [208822.516599] ath10k_pci 0001:03:00.0: Invalid VHT mcs 15 peer stat=
s
>>>>> [208841.257780] ath10k_pci 0001:03:00.0: Invalid VHT mcs 15 peer stat=
s
>> [...]
>>>> This patch only splits WMI_RATE_PREAMBLE_HT & WMI_RATE_PREAMBLE_VHT. A=
nd for
>>>> WMI_RATE_PREAMBLE_HT (*not VHT*), it uses a slightly different approac=
h. But
>>>> the WMI_RATE_PREAMBLE_VHT part (which you see in your logs) is basical=
ly
>>>> untouched.
>>> then a question follows up. is this check really neccessary?
>> Until we find out what the heck VHT MCS 15 should mean in this context -=
maybe.
>> But to the message - no, the message is most likely not necessary for ea=
ch
>> received "invalid" peer tx stat.
>
> This validation check expects peer tx stat packets from FW contain
> reasonable values and gives warning if values are different from
> expectation. The problem comes from the assumption that "it always
> contains reasonable stat value" which is wrong at least based on my
> results. For instance, the reason MCS =3D=3D 15 is because all 4 bits of
> mcs potion set to 1, not because FW really sets it to 15
> intentionally. when the mcs potion bits are set to all 1s, the other
> bits such as nss are also set to all 1s.
> Hence it looks FW passed totally invalid stat info to me.
>
> I don't have preference on whether it's better to split VHT and HT
> check or not, but it is more appropriate to change that warning level
> to debug level.
I think we should add a special case to not print the warning if mcs =3D=3D
15 until we figure out what it means.
--=20
Kalle Valo=
On 11/06/2017 01:02 AM, Sven Eckelmann wrote:
> On Montag, 6. November 2017 09:28:42 CET Sebastian Gottschall wrote:
>> Am 06.11.2017 um 09:23 schrieb Sven Eckelmann:
>>> On Sonntag, 5. November 2017 10:22:22 CET Sebastian Gottschall wrote:
>>>> the assumption made in this patch is obviously wrong (at least for more
>>>> recent firmwares and 9984)
>>>> my log is flooded with messages like
>>>> [208802.803537] ath10k_pci 0001:03:00.0: Invalid VHT mcs 15 peer stats
>>>> [208805.108515] ath10k_pci 0001:03:00.0: Invalid VHT mcs 15 peer stats
>>>> [208821.747621] ath10k_pci 0001:03:00.0: Invalid VHT mcs 15 peer stats
>>>> [208822.516599] ath10k_pci 0001:03:00.0: Invalid VHT mcs 15 peer stats
>>>> [208841.257780] ath10k_pci 0001:03:00.0: Invalid VHT mcs 15 peer stats
> [...]
>>> This patch only splits WMI_RATE_PREAMBLE_HT & WMI_RATE_PREAMBLE_VHT. And for
>>> WMI_RATE_PREAMBLE_HT (*not VHT*), it uses a slightly different approach. But
>>> the WMI_RATE_PREAMBLE_VHT part (which you see in your logs) is basically
>>> untouched.
>> then a question follows up. is this check really neccessary?
> Until we find out what the heck VHT MCS 15 should mean in this context - maybe.
> But to the message - no, the message is most likely not necessary for each
> received "invalid" peer tx stat.
This validation check expects peer tx stat packets from FW contain
reasonable values and gives warning if values are different from
expectation. The problem comes from the assumption that "it always
contains reasonable stat value" which is wrong at least based on my
results. For instance, the reason MCS == 15 is because all 4 bits of mcs
potion set to 1, not because FW really sets it to 15 intentionally. when
the mcs potion bits are set to all 1s, the other bits such as nss are
also set to all 1s.
Hence it looks FW passed totally invalid stat info to me.
I don't have preference on whether it's better to split VHT and HT check
or not, but it is more appropriate to change that warning level to debug
level.
On Dienstag, 21. November 2017 11:43:36 CET [email protected] wrote:
[...]
> > I think we should add a special case to not print the warning if mcs ==
> > 15 until we figure out what it means.
>
> Fix identified in Firmware and will push ASAP.
Is it known when this will be released and for which firmware/HW versions? And
will this also fix the legacy rate reports? At least in the moment,
ath10k_htt_fetch_peer_stats seems to generate following on QCA4019 for some
clients:
[ 1627.720177] ath10k_ahb a800000.wifi: Invalid legacy rate 26 peer stats
[ 1632.010290] ath10k_ahb a800000.wifi: Invalid legacy rate 26 peer stats
Kind regards,
Sven