All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] ath10k: Fix reported HT MCS rates with NSS > 1
@ 2017-05-11  9:09 ` Sven Eckelmann
  0 siblings, 0 replies; 24+ messages in thread
From: Sven Eckelmann @ 2017-05-11  9:09 UTC (permalink / raw)
  To: ath10k; +Cc: linux-wireless, akolli, Sven Eckelmann

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 <sven.eckelmann@openmesh.com>
---
 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

^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [PATCH] ath10k: Fix reported HT MCS rates with NSS > 1
@ 2017-05-11  9:09 ` Sven Eckelmann
  0 siblings, 0 replies; 24+ messages in thread
From: Sven Eckelmann @ 2017-05-11  9:09 UTC (permalink / raw)
  To: ath10k; +Cc: akolli, linux-wireless, Sven Eckelmann

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 <sven.eckelmann@openmesh.com>
---
 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


_______________________________________________
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k

^ permalink raw reply related	[flat|nested] 24+ messages in thread

* Re: [PATCH] ath10k: Fix reported HT MCS rates with NSS > 1
  2017-05-11  9:09 ` Sven Eckelmann
@ 2017-05-11  9:39   ` Arend Van Spriel
  -1 siblings, 0 replies; 24+ messages in thread
From: Arend Van Spriel @ 2017-05-11  9:39 UTC (permalink / raw)
  To: Sven Eckelmann, ath10k; +Cc: linux-wireless, akolli

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 <sven.eckelmann@openmesh.com>
> ---
>  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

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH] ath10k: Fix reported HT MCS rates with NSS > 1
@ 2017-05-11  9:39   ` Arend Van Spriel
  0 siblings, 0 replies; 24+ messages in thread
From: Arend Van Spriel @ 2017-05-11  9:39 UTC (permalink / raw)
  To: Sven Eckelmann, ath10k; +Cc: akolli, linux-wireless

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 <sven.eckelmann@openmesh.com>
> ---
>  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

_______________________________________________
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH] ath10k: Fix reported HT MCS rates with NSS > 1
  2017-05-11  9:39   ` Arend Van Spriel
@ 2017-05-11 10:08     ` Sven Eckelmann
  -1 siblings, 0 replies; 24+ messages in thread
From: Sven Eckelmann @ 2017-05-11 10:08 UTC (permalink / raw)
  To: Arend Van Spriel; +Cc: ath10k, linux-wireless, akolli

[-- Attachment #1: Type: text/plain, Size: 1541 bytes --]

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/

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH] ath10k: Fix reported HT MCS rates with NSS > 1
@ 2017-05-11 10:08     ` Sven Eckelmann
  0 siblings, 0 replies; 24+ messages in thread
From: Sven Eckelmann @ 2017-05-11 10:08 UTC (permalink / raw)
  To: Arend Van Spriel; +Cc: akolli, linux-wireless, ath10k


[-- Attachment #1.1: Type: text/plain, Size: 1541 bytes --]

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/

[-- Attachment #1.2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[-- Attachment #2: Type: text/plain, Size: 146 bytes --]

_______________________________________________
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: ath10k: Fix reported HT MCS rates with NSS > 1
  2017-05-11  9:09 ` Sven Eckelmann
@ 2017-05-23 15:28   ` Kalle Valo
  -1 siblings, 0 replies; 24+ messages in thread
From: Kalle Valo @ 2017-05-23 15:28 UTC (permalink / raw)
  To: Sven Eckelmann; +Cc: ath10k, akolli, linux-wireless, Sven Eckelmann

Sven Eckelmann <sven.eckelmann@openmesh.com> 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 <sven.eckelmann@openmesh.com>

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

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: ath10k: Fix reported HT MCS rates with NSS > 1
@ 2017-05-23 15:28   ` Kalle Valo
  0 siblings, 0 replies; 24+ messages in thread
From: Kalle Valo @ 2017-05-23 15:28 UTC (permalink / raw)
  To: Sven Eckelmann; +Cc: akolli, linux-wireless, ath10k

Sven Eckelmann <sven.eckelmann@openmesh.com> 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 <sven.eckelmann@openmesh.com>

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


_______________________________________________
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: ath10k: Fix reported HT MCS rates with NSS > 1
  2017-05-23 15:28   ` Kalle Valo
@ 2017-11-05  9:22     ` Sebastian Gottschall
  -1 siblings, 0 replies; 24+ messages in thread
From: Sebastian Gottschall @ 2017-11-05  9:22 UTC (permalink / raw)
  To: Kalle Valo, Sven Eckelmann; +Cc: ath10k, akolli, linux-wireless

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 <sven.eckelmann@openmesh.com> 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 <sven.eckelmann@openmesh.com>
> 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: s.gottschall@dd-wrt.com
Tel.: +496251-582650 / Fax: +496251-5826565

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: ath10k: Fix reported HT MCS rates with NSS > 1
@ 2017-11-05  9:22     ` Sebastian Gottschall
  0 siblings, 0 replies; 24+ messages in thread
From: Sebastian Gottschall @ 2017-11-05  9:22 UTC (permalink / raw)
  To: Kalle Valo, Sven Eckelmann; +Cc: akolli, linux-wireless, ath10k

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 <sven.eckelmann@openmesh.com> 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 <sven.eckelmann@openmesh.com>
> 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: s.gottschall@dd-wrt.com
Tel.: +496251-582650 / Fax: +496251-5826565


_______________________________________________
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: ath10k: Fix reported HT MCS rates with NSS > 1
  2017-11-05  9:22     ` Sebastian Gottschall
@ 2017-11-06  8:23       ` Sven Eckelmann
  -1 siblings, 0 replies; 24+ messages in thread
From: Sven Eckelmann @ 2017-11-06  8:23 UTC (permalink / raw)
  To: Sebastian Gottschall; +Cc: Kalle Valo, ath10k, akolli, linux-wireless

[-- Attachment #1: Type: text/plain, Size: 1266 bytes --]

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

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: ath10k: Fix reported HT MCS rates with NSS > 1
@ 2017-11-06  8:23       ` Sven Eckelmann
  0 siblings, 0 replies; 24+ messages in thread
From: Sven Eckelmann @ 2017-11-06  8:23 UTC (permalink / raw)
  To: Sebastian Gottschall; +Cc: akolli, Kalle Valo, linux-wireless, ath10k


[-- Attachment #1.1: Type: text/plain, Size: 1266 bytes --]

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

[-- Attachment #1.2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[-- Attachment #2: Type: text/plain, Size: 146 bytes --]

_______________________________________________
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: ath10k: Fix reported HT MCS rates with NSS > 1
  2017-11-06  8:23       ` Sven Eckelmann
@ 2017-11-06  8:28         ` Sebastian Gottschall
  -1 siblings, 0 replies; 24+ messages in thread
From: Sebastian Gottschall @ 2017-11-06  8:28 UTC (permalink / raw)
  To: Sven Eckelmann; +Cc: Kalle Valo, ath10k, akolli, linux-wireless

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: s.gottschall@dd-wrt.com
Tel.: +496251-582650 / Fax: +496251-5826565

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: ath10k: Fix reported HT MCS rates with NSS > 1
@ 2017-11-06  8:28         ` Sebastian Gottschall
  0 siblings, 0 replies; 24+ messages in thread
From: Sebastian Gottschall @ 2017-11-06  8:28 UTC (permalink / raw)
  To: Sven Eckelmann; +Cc: akolli, Kalle Valo, linux-wireless, ath10k

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: s.gottschall@dd-wrt.com
Tel.: +496251-582650 / Fax: +496251-5826565


_______________________________________________
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: ath10k: Fix reported HT MCS rates with NSS > 1
  2017-11-06  8:28         ` Sebastian Gottschall
@ 2017-11-06  9:02           ` Sven Eckelmann
  -1 siblings, 0 replies; 24+ messages in thread
From: Sven Eckelmann @ 2017-11-06  9:02 UTC (permalink / raw)
  To: Sebastian Gottschall; +Cc: Kalle Valo, ath10k, akolli, linux-wireless

[-- Attachment #1: Type: text/plain, Size: 1259 bytes --]

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

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: ath10k: Fix reported HT MCS rates with NSS > 1
@ 2017-11-06  9:02           ` Sven Eckelmann
  0 siblings, 0 replies; 24+ messages in thread
From: Sven Eckelmann @ 2017-11-06  9:02 UTC (permalink / raw)
  To: Sebastian Gottschall; +Cc: akolli, Kalle Valo, linux-wireless, ath10k


[-- Attachment #1.1: Type: text/plain, Size: 1259 bytes --]

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

[-- Attachment #1.2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[-- Attachment #2: Type: text/plain, Size: 146 bytes --]

_______________________________________________
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: ath10k: Fix reported HT MCS rates with NSS > 1
  2017-11-06  9:02           ` Sven Eckelmann
@ 2017-11-06 21:25             ` Peter Oh
  -1 siblings, 0 replies; 24+ messages in thread
From: Peter Oh @ 2017-11-06 21:25 UTC (permalink / raw)
  To: Sven Eckelmann, Sebastian Gottschall
  Cc: Kalle Valo, ath10k, akolli, linux-wireless



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.

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: ath10k: Fix reported HT MCS rates with NSS > 1
@ 2017-11-06 21:25             ` Peter Oh
  0 siblings, 0 replies; 24+ messages in thread
From: Peter Oh @ 2017-11-06 21:25 UTC (permalink / raw)
  To: Sven Eckelmann, Sebastian Gottschall
  Cc: akolli, Kalle Valo, linux-wireless, ath10k



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.


_______________________________________________
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: ath10k: Fix reported HT MCS rates with NSS > 1
  2017-11-06 21:25             ` Peter Oh
@ 2017-11-20 12:10               ` Kalle Valo
  -1 siblings, 0 replies; 24+ messages in thread
From: Kalle Valo @ 2017-11-20 12:10 UTC (permalink / raw)
  To: Peter Oh
  Cc: Sven Eckelmann, Sebastian Gottschall, ath10k, Anilkumar Kolli,
	linux-wireless

Peter Oh <peter.oh@bowerswilkins.com> 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=

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: ath10k: Fix reported HT MCS rates with NSS > 1
@ 2017-11-20 12:10               ` Kalle Valo
  0 siblings, 0 replies; 24+ messages in thread
From: Kalle Valo @ 2017-11-20 12:10 UTC (permalink / raw)
  To: Peter Oh
  Cc: Anilkumar Kolli, linux-wireless, ath10k, Sebastian Gottschall,
	Sven Eckelmann

Peter Oh <peter.oh@bowerswilkins.com> 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.

-- 
Kalle Valo
_______________________________________________
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: ath10k: Fix reported HT MCS rates with NSS > 1
  2017-11-20 12:10               ` Kalle Valo
@ 2017-11-21  6:13                 ` akolli
  -1 siblings, 0 replies; 24+ messages in thread
From: akolli @ 2017-11-21  6:13 UTC (permalink / raw)
  To: Kalle Valo
  Cc: Peter Oh, Sven Eckelmann, Sebastian Gottschall, ath10k,
	Anilkumar Kolli, linux-wireless, linux-wireless-owner

On 2017-11-20 17:40, Kalle Valo wrote:
> Peter Oh <peter.oh@bowerswilkins.com> 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.

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: ath10k: Fix reported HT MCS rates with NSS > 1
@ 2017-11-21  6:13                 ` akolli
  0 siblings, 0 replies; 24+ messages in thread
From: akolli @ 2017-11-21  6:13 UTC (permalink / raw)
  To: Kalle Valo
  Cc: linux-wireless-owner, Anilkumar Kolli, linux-wireless, ath10k,
	Sebastian Gottschall, Sven Eckelmann, Peter Oh

On 2017-11-20 17:40, Kalle Valo wrote:
> Peter Oh <peter.oh@bowerswilkins.com> 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.

_______________________________________________
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: ath10k: Fix reported HT MCS rates with NSS > 1
  2017-11-21  6:13                 ` akolli
@ 2017-12-05  9:16                   ` Sven Eckelmann
  -1 siblings, 0 replies; 24+ messages in thread
From: Sven Eckelmann @ 2017-12-05  9:16 UTC (permalink / raw)
  To: akolli
  Cc: Kalle Valo, Peter Oh, Sebastian Gottschall, ath10k,
	Anilkumar Kolli, linux-wireless

[-- Attachment #1: Type: text/plain, Size: 662 bytes --]

On Dienstag, 21. November 2017 11:43:36 CET akolli@codeaurora.org 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

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: ath10k: Fix reported HT MCS rates with NSS > 1
@ 2017-12-05  9:16                   ` Sven Eckelmann
  0 siblings, 0 replies; 24+ messages in thread
From: Sven Eckelmann @ 2017-12-05  9:16 UTC (permalink / raw)
  To: akolli
  Cc: Anilkumar Kolli, linux-wireless, ath10k, Kalle Valo,
	Sebastian Gottschall, Peter Oh


[-- Attachment #1.1: Type: text/plain, Size: 662 bytes --]

On Dienstag, 21. November 2017 11:43:36 CET akolli@codeaurora.org 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

[-- Attachment #1.2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[-- Attachment #2: Type: text/plain, Size: 146 bytes --]

_______________________________________________
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k

^ permalink raw reply	[flat|nested] 24+ messages in thread

end of thread, other threads:[~2017-12-05  9:16 UTC | newest]

Thread overview: 24+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-05-11  9:09 [PATCH] ath10k: Fix reported HT MCS rates with NSS > 1 Sven Eckelmann
2017-05-11  9:09 ` Sven Eckelmann
2017-05-11  9:39 ` Arend Van Spriel
2017-05-11  9:39   ` Arend Van Spriel
2017-05-11 10:08   ` Sven Eckelmann
2017-05-11 10:08     ` Sven Eckelmann
2017-05-23 15:28 ` Kalle Valo
2017-05-23 15:28   ` Kalle Valo
2017-11-05  9:22   ` Sebastian Gottschall
2017-11-05  9:22     ` Sebastian Gottschall
2017-11-06  8:23     ` Sven Eckelmann
2017-11-06  8:23       ` Sven Eckelmann
2017-11-06  8:28       ` Sebastian Gottschall
2017-11-06  8:28         ` Sebastian Gottschall
2017-11-06  9:02         ` Sven Eckelmann
2017-11-06  9:02           ` Sven Eckelmann
2017-11-06 21:25           ` Peter Oh
2017-11-06 21:25             ` Peter Oh
2017-11-20 12:10             ` Kalle Valo
2017-11-20 12:10               ` Kalle Valo
2017-11-21  6:13               ` akolli
2017-11-21  6:13                 ` akolli
2017-12-05  9:16                 ` Sven Eckelmann
2017-12-05  9:16                   ` Sven Eckelmann

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.