[PATCH 0/4] Fix inconsistencies observed in population of tx_stats in debugfs

[PATCH 0/4] Fix inconsistencies observed in population of tx_stats in debugfs

[Surabhi Vishnoi]

There are some inconsistencies observed while filling various peer tx stats in tx_stats
debugfs entry per sta.

This patchset fixes the wrong updation of NSS, SGI, Bandwidth and rate_table in tx_stats
debugfs entry per sta.

Surabhi Vishnoi (4):
  ath10k: Fix the incorrect updation of NSS data in tx stats
  ath10k: Fix the wrong updation of BW in tx_stats debugfs entry
  ath10k: Fix the wrong updation of SGI in tx_stats debugfs
  ath10k: Fix the wrong calculation ht_idx and idx of rate table for
    tx_stats

 drivers/net/wireless/ath/ath10k/debugfs_sta.c |  8 +++++---
 drivers/net/wireless/ath/ath10k/htt_rx.c      | 25 +++++++++++++------------
 drivers/net/wireless/ath/ath10k/wmi.h         |  3 ++-
 3 files changed, 20 insertions(+), 16 deletions(-)

-- 
1.9.1

[PATCH 1/4] ath10k: Fix the incorrect updation of NSS data in tx stats

[Surabhi Vishnoi]

The NSS data is updated incorrectly in the tx stats as the array
indexing starts from zero.

Fix the incorrect updation of NSS data in tx_stats by taking into
consideration the array index starting from zero.

Tested HW: WCN3990
Tested FW: WLAN.HL.3.1-00784-QCAHLSWMTPLZ-1

Fixes: a904417fc876 ("ath10k: add extended per sta tx statistics support")
Signed-off-by: Surabhi Vishnoi <svishnoi@codeaurora.org>
---
 drivers/net/wireless/ath/ath10k/htt_rx.c | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/drivers/net/wireless/ath/ath10k/htt_rx.c b/drivers/net/wireless/ath/ath10k/htt_rx.c
index d5c666c..3a02a76 100644
--- a/drivers/net/wireless/ath/ath10k/htt_rx.c
+++ b/drivers/net/wireless/ath/ath10k/htt_rx.c
@@ -2980,7 +2980,7 @@ static inline s8 ath10k_get_legacy_rate_idx(struct ath10k *ar, u8 rate)
 		}
 		STATS_OP_FMT(AMPDU).bw[0][bw] +=
 			pstats->succ_bytes + pstats->retry_bytes;
-		STATS_OP_FMT(AMPDU).nss[0][nss] +=
+		STATS_OP_FMT(AMPDU).nss[0][nss - 1] +=
 			pstats->succ_bytes + pstats->retry_bytes;
 		STATS_OP_FMT(AMPDU).gi[0][gi] +=
 			pstats->succ_bytes + pstats->retry_bytes;
@@ -2988,7 +2988,7 @@ static inline s8 ath10k_get_legacy_rate_idx(struct ath10k *ar, u8 rate)
 			pstats->succ_bytes + pstats->retry_bytes;
 		STATS_OP_FMT(AMPDU).bw[1][bw] +=
 			pstats->succ_pkts + pstats->retry_pkts;
-		STATS_OP_FMT(AMPDU).nss[1][nss] +=
+		STATS_OP_FMT(AMPDU).nss[1][nss - 1] +=
 			pstats->succ_pkts + pstats->retry_pkts;
 		STATS_OP_FMT(AMPDU).gi[1][gi] +=
 			pstats->succ_pkts + pstats->retry_pkts;
@@ -3000,27 +3000,27 @@ static inline s8 ath10k_get_legacy_rate_idx(struct ath10k *ar, u8 rate)
 	}
 
 	STATS_OP_FMT(SUCC).bw[0][bw] += pstats->succ_bytes;
-	STATS_OP_FMT(SUCC).nss[0][nss] += pstats->succ_bytes;
+	STATS_OP_FMT(SUCC).nss[0][nss - 1] += pstats->succ_bytes;
 	STATS_OP_FMT(SUCC).gi[0][gi] += pstats->succ_bytes;
 
 	STATS_OP_FMT(SUCC).bw[1][bw] += pstats->succ_pkts;
-	STATS_OP_FMT(SUCC).nss[1][nss] += pstats->succ_pkts;
+	STATS_OP_FMT(SUCC).nss[1][nss - 1] += pstats->succ_pkts;
 	STATS_OP_FMT(SUCC).gi[1][gi] += pstats->succ_pkts;
 
 	STATS_OP_FMT(FAIL).bw[0][bw] += pstats->failed_bytes;
-	STATS_OP_FMT(FAIL).nss[0][nss] += pstats->failed_bytes;
+	STATS_OP_FMT(FAIL).nss[0][nss - 1] += pstats->failed_bytes;
 	STATS_OP_FMT(FAIL).gi[0][gi] += pstats->failed_bytes;
 
 	STATS_OP_FMT(FAIL).bw[1][bw] += pstats->failed_pkts;
-	STATS_OP_FMT(FAIL).nss[1][nss] += pstats->failed_pkts;
+	STATS_OP_FMT(FAIL).nss[1][nss - 1] += pstats->failed_pkts;
 	STATS_OP_FMT(FAIL).gi[1][gi] += pstats->failed_pkts;
 
 	STATS_OP_FMT(RETRY).bw[0][bw] += pstats->retry_bytes;
-	STATS_OP_FMT(RETRY).nss[0][nss] += pstats->retry_bytes;
+	STATS_OP_FMT(RETRY).nss[0][nss - 1] += pstats->retry_bytes;
 	STATS_OP_FMT(RETRY).gi[0][gi] += pstats->retry_bytes;
 
 	STATS_OP_FMT(RETRY).bw[1][bw] += pstats->retry_pkts;
-	STATS_OP_FMT(RETRY).nss[1][nss] += pstats->retry_pkts;
+	STATS_OP_FMT(RETRY).nss[1][nss - 1] += pstats->retry_pkts;
 	STATS_OP_FMT(RETRY).gi[1][gi] += pstats->retry_pkts;
 
 	if (txrate->flags >= RATE_INFO_FLAGS_MCS) {
-- 
1.9.1

[PATCH 2/4] ath10k: Fix the wrong updation of BW in tx_stats debugfs entry

[Surabhi Vishnoi]

Currently, the bandwidth is updated wrongly in BW table in tx_stats
debugfs per sta as there is difference in number of bandwidth type
in mac80211 and driver stats table. This leads to bandwidth getting
updated at wrong index in bandwidth table in tx_stats.

Fix this index mismatch between mac80211 and driver stats table (BW table)
by making the number of bandwidth type in driver compatible with mac80211.

Tested HW: WCN3990
Tested FW: WLAN.HL.3.1-00784-QCAHLSWMTPLZ-1

Fixes: a904417fc876 ("ath10k: add extended per sta tx statistics support")
Signed-off-by: Surabhi Vishnoi <svishnoi@codeaurora.org>
---
 drivers/net/wireless/ath/ath10k/debugfs_sta.c | 8 +++++---
 drivers/net/wireless/ath/ath10k/wmi.h         | 2 +-
 2 files changed, 6 insertions(+), 4 deletions(-)

diff --git a/drivers/net/wireless/ath/ath10k/debugfs_sta.c b/drivers/net/wireless/ath/ath10k/debugfs_sta.c
index 4778a45..645ba9c 100644
--- a/drivers/net/wireless/ath/ath10k/debugfs_sta.c
+++ b/drivers/net/wireless/ath/ath10k/debugfs_sta.c
@@ -696,11 +696,13 @@ static ssize_t ath10k_dbg_sta_dump_tx_stats(struct file *file,
 						 "  %llu ", stats->ht[j][i]);
 			len += scnprintf(buf + len, size - len, "\n");
 			len += scnprintf(buf + len, size - len,
-					" BW %s (20,40,80,160 MHz)\n", str[j]);
+					" BW %s (20,5,10,40,80,160 MHz)\n",
+					str[j]);
 			len += scnprintf(buf + len, size - len,
-					 "  %llu %llu %llu %llu\n",
+					 "  %llu %llu %llu %llu %llu %llu\n",
 					 stats->bw[j][0], stats->bw[j][1],
-					 stats->bw[j][2], stats->bw[j][3]);
+					 stats->bw[j][2], stats->bw[j][3],
+					 stats->bw[j][4], stats->bw[j][5]);
 			len += scnprintf(buf + len, size - len,
 					 " NSS %s (1x1,2x2,3x3,4x4)\n", str[j]);
 			len += scnprintf(buf + len, size - len,
diff --git a/drivers/net/wireless/ath/ath10k/wmi.h b/drivers/net/wireless/ath/ath10k/wmi.h
index 0e27878..7053db4 100644
--- a/drivers/net/wireless/ath/ath10k/wmi.h
+++ b/drivers/net/wireless/ath/ath10k/wmi.h
@@ -5056,7 +5056,7 @@ enum wmi_rate_preamble {
 #define ATH10K_FW_SKIPPED_RATE_CTRL(flags)	(((flags) >> 6) & 0x1)
 
 #define ATH10K_VHT_MCS_NUM	10
-#define ATH10K_BW_NUM		4
+#define ATH10K_BW_NUM		6
 #define ATH10K_NSS_NUM		4
 #define ATH10K_LEGACY_NUM	12
 #define ATH10K_GI_NUM		2
-- 
1.9.1

[PATCH 3/4] ath10k: Fix the wrong updation of SGI in tx_stats debugfs

[Surabhi Vishnoi]

The SGI is updated wrongly in tx stats table in debugfs per sta
entry. To know whether the packets/bytes are sent with SHORT GI,
test whether the SGI bit(ATH10K_RATE_INFO_FLAGS_SGI_BIT) is set or
not in the txrate flags.

Tested HW: WCN3990
Tested FW: WLAN.HL.3.1-00784-QCAHLSWMTPLZ-1

Fixes: a904417fc876 ("ath10k: add extended per sta tx statistics support")
Signed-off-by: Surabhi Vishnoi <svishnoi@codeaurora.org>
---
 drivers/net/wireless/ath/ath10k/htt_rx.c | 3 ++-
 drivers/net/wireless/ath/ath10k/wmi.h    | 1 +
 2 files changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/net/wireless/ath/ath10k/htt_rx.c b/drivers/net/wireless/ath/ath10k/htt_rx.c
index 3a02a76..5b1dd58 100644
--- a/drivers/net/wireless/ath/ath10k/htt_rx.c
+++ b/drivers/net/wireless/ath/ath10k/htt_rx.c
@@ -2924,12 +2924,13 @@ static inline s8 ath10k_get_legacy_rate_idx(struct ath10k *ar, u8 rate)
 	struct rate_info *txrate = &arsta->txrate;
 	struct ath10k_htt_tx_stats *tx_stats;
 	int idx, ht_idx, gi, mcs, bw, nss;
+	unsigned long flags;
 
 	if (!arsta->tx_stats)
 		return;
 
 	tx_stats = arsta->tx_stats;
-	gi = (arsta->txrate.flags & RATE_INFO_FLAGS_SHORT_GI);
+	gi = test_bit(ATH10K_RATE_INFO_FLAGS_SGI_BIT, &flags);
 	ht_idx = txrate->mcs + txrate->nss * 8;
 	mcs = txrate->mcs;
 	bw = txrate->bw;
diff --git a/drivers/net/wireless/ath/ath10k/wmi.h b/drivers/net/wireless/ath/ath10k/wmi.h
index 7053db4..b727232 100644
--- a/drivers/net/wireless/ath/ath10k/wmi.h
+++ b/drivers/net/wireless/ath/ath10k/wmi.h
@@ -5062,6 +5062,7 @@ enum wmi_rate_preamble {
 #define ATH10K_GI_NUM		2
 #define ATH10K_HT_MCS_NUM	32
 #define ATH10K_RATE_TABLE_NUM	320
+#define ATH10K_RATE_INFO_FLAGS_SGI_BIT	2
 
 /* Value to disable fixed rate setting */
 #define WMI_FIXED_RATE_NONE    (0xff)
-- 
1.9.1

Re: [PATCH 3/4] ath10k: Fix the wrong updation of SGI in tx_stats debugfs

[kbuild test robot]

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

Hi Surabhi,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on ath6kl/ath-next]
[also build test WARNING on v5.0-rc4 next-20190222]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Surabhi-Vishnoi/Fix-inconsistencies-observed-in-population-of-tx_stats-in-debugfs/20190225-041038
base:   https://git.kernel.org/pub/scm/linux/kernel/git/kvalo/ath.git ath-next
config: alpha-allyesconfig (attached as .config)
compiler: alpha-linux-gnu-gcc (Debian 8.2.0-11) 8.2.0
reproduce:
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        GCC_VERSION=8.2.0 make.cross ARCH=alpha 

Note: it may well be a FALSE warning. FWIW you are at least aware of it now.
http://gcc.gnu.org/wiki/Better_Uninitialized_Warnings

All warnings (new ones prefixed by >>):

   In file included from include/linux/bitops.h:19,
                    from include/linux/kernel.h:11,
                    from include/linux/list.h:9,
                    from include/linux/wait.h:7,
                    from include/linux/completion.h:12,
                    from drivers/net//wireless/ath/ath10k/core.h:11,
                    from drivers/net//wireless/ath/ath10k/htt_rx.c:8:
   drivers/net//wireless/ath/ath10k/htt_rx.c: In function 'ath10k_update_per_peer_tx_stats':
>> arch/alpha/include/asm/bitops.h:289:37: warning: 'flags' may be used uninitialized in this function [-Wmaybe-uninitialized]
     return (1UL & (((const int *) addr)[nr >> 5] >> (nr & 31))) != 0UL;
                    ~~~~~~~~~~~~~~~~~~~~^~~~~~~~~
   drivers/net//wireless/ath/ath10k/htt_rx.c:2916:16: note: 'flags' was declared here
     unsigned long flags;
                   ^~~~~
--
   In file included from include/linux/bitops.h:19,
                    from include/linux/kernel.h:11,
                    from include/linux/list.h:9,
                    from include/linux/wait.h:7,
                    from include/linux/completion.h:12,
                    from drivers/net/wireless/ath/ath10k/core.h:11,
                    from drivers/net/wireless/ath/ath10k/htt_rx.c:8:
   drivers/net/wireless/ath/ath10k/htt_rx.c: In function 'ath10k_update_per_peer_tx_stats':
>> arch/alpha/include/asm/bitops.h:289:37: warning: 'flags' may be used uninitialized in this function [-Wmaybe-uninitialized]
     return (1UL & (((const int *) addr)[nr >> 5] >> (nr & 31))) != 0UL;
                    ~~~~~~~~~~~~~~~~~~~~^~~~~~~~~
   drivers/net/wireless/ath/ath10k/htt_rx.c:2916:16: note: 'flags' was declared here
     unsigned long flags;
                   ^~~~~

vim +/flags +289 arch/alpha/include/asm/bitops.h

^1da177e include/asm-alpha/bitops.h Linus Torvalds 2005-04-16  285  
^1da177e include/asm-alpha/bitops.h Linus Torvalds 2005-04-16  286  static inline int
^1da177e include/asm-alpha/bitops.h Linus Torvalds 2005-04-16  287  test_bit(int nr, const volatile void * addr)
^1da177e include/asm-alpha/bitops.h Linus Torvalds 2005-04-16  288  {
^1da177e include/asm-alpha/bitops.h Linus Torvalds 2005-04-16 @289  	return (1UL & (((const int *) addr)[nr >> 5] >> (nr & 31))) != 0UL;
^1da177e include/asm-alpha/bitops.h Linus Torvalds 2005-04-16  290  }
^1da177e include/asm-alpha/bitops.h Linus Torvalds 2005-04-16  291  

:::::: The code at line 289 was first introduced by commit
:::::: 1da177e4c3f41524e886b7f1b8a0c1fc7321cac2 Linux-2.6.12-rc2

:::::: TO: Linus Torvalds <torvalds@ppc970.osdl.org>
:::::: CC: Linus Torvalds <torvalds@ppc970.osdl.org>

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 55538 bytes --]

Re: [PATCH 3/4] ath10k: Fix the wrong updation of SGI in tx_stats debugfs

[kbuild test robot]

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

Hi Surabhi,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on ath6kl/ath-next]
[also build test WARNING on v5.0-rc4 next-20190222]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Surabhi-Vishnoi/Fix-inconsistencies-observed-in-population-of-tx_stats-in-debugfs/20190225-041038
base:   https://git.kernel.org/pub/scm/linux/kernel/git/kvalo/ath.git ath-next
config: x86_64-fedora-25 (attached as .config)
compiler: gcc-7 (Debian 7.3.0-1) 7.3.0
reproduce:
        # save the attached .config to linux build tree
        make ARCH=x86_64 

Note: it may well be a FALSE warning. FWIW you are at least aware of it now.
http://gcc.gnu.org/wiki/Better_Uninitialized_Warnings

All warnings (new ones prefixed by >>):

   In file included from include/linux/bitops.h:19:0,
                    from include/linux/kernel.h:11,
                    from include/linux/list.h:9,
                    from include/linux/wait.h:7,
                    from include/linux/completion.h:12,
                    from drivers/net/wireless/ath/ath10k/core.h:11,
                    from drivers/net/wireless/ath/ath10k/htt_rx.c:8:
   drivers/net/wireless/ath/ath10k/htt_rx.c: In function 'ath10k_update_per_peer_tx_stats':
>> arch/x86/include/asm/bitops.h:325:37: warning: 'flags' may be used uninitialized in this function [-Wmaybe-uninitialized]
     return ((1UL << (nr & (BITS_PER_LONG-1))) &
            ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
      (addr[nr >> _BITOPS_LONG_SHIFT])) != 0;
      ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~
   drivers/net/wireless/ath/ath10k/htt_rx.c:2916:16: note: 'flags' was declared here
     unsigned long flags;
                   ^~~~~
--
   In file included from include/linux/bitops.h:19:0,
                    from include/linux/kernel.h:11,
                    from include/linux/list.h:9,
                    from include/linux/wait.h:7,
                    from include/linux/completion.h:12,
                    from drivers/net//wireless/ath/ath10k/core.h:11,
                    from drivers/net//wireless/ath/ath10k/htt_rx.c:8:
   drivers/net//wireless/ath/ath10k/htt_rx.c: In function 'ath10k_update_per_peer_tx_stats':
>> arch/x86/include/asm/bitops.h:325:37: warning: 'flags' may be used uninitialized in this function [-Wmaybe-uninitialized]
     return ((1UL << (nr & (BITS_PER_LONG-1))) &
            ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
      (addr[nr >> _BITOPS_LONG_SHIFT])) != 0;
      ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~
   drivers/net//wireless/ath/ath10k/htt_rx.c:2916:16: note: 'flags' was declared here
     unsigned long flags;
                   ^~~~~

vim +/flags +325 arch/x86/include/asm/bitops.h

1c54d770 include/asm-x86/bitops.h      Jeremy Fitzhardinge 2008-01-30  321  
117780ee arch/x86/include/asm/bitops.h H. Peter Anvin      2016-06-08  322  static __always_inline bool constant_test_bit(long nr, const volatile unsigned long *addr)
1c54d770 include/asm-x86/bitops.h      Jeremy Fitzhardinge 2008-01-30  323  {
9b710506 arch/x86/include/asm/bitops.h H. Peter Anvin      2013-07-16  324  	return ((1UL << (nr & (BITS_PER_LONG-1))) &
9b710506 arch/x86/include/asm/bitops.h H. Peter Anvin      2013-07-16 @325  		(addr[nr >> _BITOPS_LONG_SHIFT])) != 0;
1c54d770 include/asm-x86/bitops.h      Jeremy Fitzhardinge 2008-01-30  326  }
1c54d770 include/asm-x86/bitops.h      Jeremy Fitzhardinge 2008-01-30  327  

:::::: The code at line 325 was first introduced by commit
:::::: 9b710506a03b01a9fdd83962912bc9d8237b82e8 x86, bitops: Change bitops to be native operand size

:::::: TO: H. Peter Anvin <hpa@linux.intel.com>
:::::: CC: H. Peter Anvin <hpa@linux.intel.com>

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 49043 bytes --]

[PATCH 4/4] ath10k: Fix the wrong calculation ht_idx and idx of rate table for tx_stats

[Surabhi Vishnoi]

ht_idx (ht rate index) and idx (rate table index) are calculated based on
mcs index. This mcs index used in the above calculation should be 0-9 for
getting the correct ht_idx and idx.

Currently the mcs index used for the above calculations is mcs index which
can be 0-31 (in case of HT), leading to incorrect rate index and ht index
values.

Fix the issue by obtaining mcs value from the ratecode reported by firmware
and use it for calculating ht_idx and idx (rate-table index).

Tested HW: WCN3990
Tested FW: WLAN.HL.3.1-00784-QCAHLSWMTPLZ-1

Fixes: e88975ca37d1 ("ath10k: dump tx stats in rate table format")
Signed-off-by: Surabhi Vishnoi <svishnoi@codeaurora.org>
---
 drivers/net/wireless/ath/ath10k/htt_rx.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/net/wireless/ath/ath10k/htt_rx.c b/drivers/net/wireless/ath/ath10k/htt_rx.c
index 5b1dd58..b504c4f 100644
--- a/drivers/net/wireless/ath/ath10k/htt_rx.c
+++ b/drivers/net/wireless/ath/ath10k/htt_rx.c
@@ -2931,11 +2931,11 @@ static inline s8 ath10k_get_legacy_rate_idx(struct ath10k *ar, u8 rate)
 
 	tx_stats = arsta->tx_stats;
 	gi = test_bit(ATH10K_RATE_INFO_FLAGS_SGI_BIT, &flags);
-	ht_idx = txrate->mcs + txrate->nss * 8;
-	mcs = txrate->mcs;
+	mcs = ATH10K_HW_MCS_RATE(pstats->ratecode);
 	bw = txrate->bw;
 	nss = txrate->nss;
-	idx = mcs * 8 + 8 * 10 * nss;
+	ht_idx = mcs + (nss - 1) * 8;
+	idx = mcs * 8 + 8 * 10 * (nss - 1);
 	idx += bw * 2 + gi;
 
 #define STATS_OP_FMT(name) tx_stats->stats[ATH10K_STATS_TYPE_##name]
-- 
1.9.1