[PATCH 0/2] ath10k: tx stats for 10.2.4

[PATCH 0/2] ath10k: tx stats for 10.2.4

[akolli]

From: Anilkumar Kolli <akolli@qti.qualcomm.com>

Add tx stats supoort for QCA988X.
Parse peer stats from pktlog packets and update the tx rate
information per STA.
This way user space can query about transmit rate with iw.

Anilkumar Kolli (2):
  ath10k: move pktlog_filter out of ath10k_debug
  ath10k: add per peer tx stats support for 10.2.4

 drivers/net/wireless/ath/ath10k/core.h   |    2 +-
 drivers/net/wireless/ath/ath10k/debug.c  |   16 ++++----
 drivers/net/wireless/ath/ath10k/debug.h  |   18 ++++++++-
 drivers/net/wireless/ath/ath10k/htt.h    |   17 +++++++++
 drivers/net/wireless/ath/ath10k/htt_rx.c |   60 ++++++++++++++++++++++++++++++
 drivers/net/wireless/ath/ath10k/mac.c    |   10 +++++
 6 files changed, 113 insertions(+), 10 deletions(-)

-- 
1.7.9.5

[PATCH 1/2] ath10k: move pktlog_filter out of ath10k_debug

[akolli]

From: Anilkumar Kolli <akolli@qti.qualcomm.com>

Move pktlog_filter from struct ath10k_debug to struct ath10k
so that pktlog can be enabled even when debugfs is not
enabled, needed to enable peer tx stats for 10.2.4.

No changes in functionality.

Signed-off-by: Anilkumar Kolli <akolli@qti.qualcomm.com>
---
 drivers/net/wireless/ath/ath10k/core.h  |    2 +-
 drivers/net/wireless/ath/ath10k/debug.c |   16 ++++++++--------
 2 files changed, 9 insertions(+), 9 deletions(-)

diff --git a/drivers/net/wireless/ath/ath10k/core.h b/drivers/net/wireless/ath/ath10k/core.h
index 643041ef3271..75dbfffe677a 100644
--- a/drivers/net/wireless/ath/ath10k/core.h
+++ b/drivers/net/wireless/ath/ath10k/core.h
@@ -488,7 +488,6 @@ struct ath10k_debug {
 	/* protected by conf_mutex */
 	u64 fw_dbglog_mask;
 	u32 fw_dbglog_level;
-	u32 pktlog_filter;
 	u32 reg_addr;
 	u32 nf_cal_period;
 	void *cal_data;
@@ -963,6 +962,7 @@ struct ath10k {
 	} spectral;
 #endif
 
+	u32 pktlog_filter;
 	struct {
 		/* protected by conf_mutex */
 		struct ath10k_fw_components utf_mode_fw;
diff --git a/drivers/net/wireless/ath/ath10k/debug.c b/drivers/net/wireless/ath/ath10k/debug.c
index df514507d3f1..e278300b4f14 100644
--- a/drivers/net/wireless/ath/ath10k/debug.c
+++ b/drivers/net/wireless/ath/ath10k/debug.c
@@ -1950,14 +1950,14 @@ int ath10k_debug_start(struct ath10k *ar)
 				    ret);
 	}
 
-	if (ar->debug.pktlog_filter) {
+	if (ar->pktlog_filter) {
 		ret = ath10k_wmi_pdev_pktlog_enable(ar,
-						    ar->debug.pktlog_filter);
+						    ar->pktlog_filter);
 		if (ret)
 			/* not serious */
 			ath10k_warn(ar,
 				    "failed to enable pktlog filter %x: %d\n",
-				    ar->debug.pktlog_filter, ret);
+				    ar->pktlog_filter, ret);
 	} else {
 		ret = ath10k_wmi_pdev_pktlog_disable(ar);
 		if (ret)
@@ -2097,12 +2097,12 @@ static ssize_t ath10k_write_pktlog_filter(struct file *file,
 	mutex_lock(&ar->conf_mutex);
 
 	if (ar->state != ATH10K_STATE_ON) {
-		ar->debug.pktlog_filter = filter;
+		ar->pktlog_filter = filter;
 		ret = count;
 		goto out;
 	}
 
-	if (filter == ar->debug.pktlog_filter) {
+	if (filter == ar->pktlog_filter) {
 		ret = count;
 		goto out;
 	}
@@ -2111,7 +2111,7 @@ static ssize_t ath10k_write_pktlog_filter(struct file *file,
 		ret = ath10k_wmi_pdev_pktlog_enable(ar, filter);
 		if (ret) {
 			ath10k_warn(ar, "failed to enable pktlog filter %x: %d\n",
-				    ar->debug.pktlog_filter, ret);
+				    ar->pktlog_filter, ret);
 			goto out;
 		}
 	} else {
@@ -2122,7 +2122,7 @@ static ssize_t ath10k_write_pktlog_filter(struct file *file,
 		}
 	}
 
-	ar->debug.pktlog_filter = filter;
+	ar->pktlog_filter = filter;
 	ret = count;
 
 out:
@@ -2139,7 +2139,7 @@ static ssize_t ath10k_read_pktlog_filter(struct file *file, char __user *ubuf,
 
 	mutex_lock(&ar->conf_mutex);
 	len = scnprintf(buf, sizeof(buf) - len, "%08x\n",
-			ar->debug.pktlog_filter);
+			ar->pktlog_filter);
 	mutex_unlock(&ar->conf_mutex);
 
 	return simple_read_from_buffer(ubuf, count, ppos, buf, len);
-- 
1.7.9.5

[PATCH 2/2] ath10k: add per peer tx stats support for 10.2.4

[akolli]

From: Anilkumar Kolli <akolli@qti.qualcomm.com>

10.2.4 firmware branch (used in QCA988X) does not support
HTT_10_4_T2H_MSG_TYPE_PEER_STATS and that's why ath10k does not provide
tranmission rate statistics to user space, instead it just shows
hardcoded 6 Mbit/s. But pktlog firmware facility provides per peer tx
statistics. The firmware sends one pktlog event for every four
PPDUs per peer, which include:

* successful number of packets and bytes transmitted
* number of packets and bytes dropped
* retried number of packets and bytes
* rate info per ppdu

Firmware supports WMI_SERVICE_PEER_STATS, pktlog is enabled through
ATH10K_FLAG_PEER_STATS, which is nowadays enabled by default in ath10k.

This patch does not impact throughput.

Tested on QCA9880 with firmware version 10.2.4.70.48. This should also
work with firmware branch 10.2.4-1.0-00029

Parse peer stats from pktlog packets and update the tx rate information
per STA. This way user space can query about transmit rate with iw:

$iw wlan0 station dump
Station 3c:a9:f4:72:bb:a4 (on wlan1)
        inactive time:  8210 ms
        rx bytes:       9166
        rx packets:     44
        tx bytes:       1105
        tx packets:     9
        tx retries:     0
        tx failed:      1
        rx drop misc:   3
        signal:         -75 [-75, -87, -88] dBm
        signal avg:     -75 [-75, -85, -88] dBm
        tx bitrate:     39.0 MBit/s MCS 10
        rx bitrate:     26.0 MBit/s MCS 3
        rx duration:    23250 us
        authorized:     yes
        authenticated:  yes
        associated:     yes
        preamble:       short
        WMM/WME:        yes
        MFP:            no
        TDLS peer:      no
        DTIM period:    2
        beacon interval:100
        short preamble: yes
        short slot time:yes
        connected time: 22 seconds

Signed-off-by: Anilkumar Kolli <akolli@qti.qualcomm.com>
---
 drivers/net/wireless/ath/ath10k/debug.h  |   18 ++++++++-
 drivers/net/wireless/ath/ath10k/htt.h    |   17 +++++++++
 drivers/net/wireless/ath/ath10k/htt_rx.c |   60 ++++++++++++++++++++++++++++++
 drivers/net/wireless/ath/ath10k/mac.c    |   10 +++++
 4 files changed, 104 insertions(+), 1 deletion(-)

diff --git a/drivers/net/wireless/ath/ath10k/debug.h b/drivers/net/wireless/ath/ath10k/debug.h
index 548ad5483a4a..290979ef4915 100644
--- a/drivers/net/wireless/ath/ath10k/debug.h
+++ b/drivers/net/wireless/ath/ath10k/debug.h
@@ -51,7 +51,8 @@ enum ath10k_pktlog_filter {
 	ATH10K_PKTLOG_RCFIND     = 0x000000004,
 	ATH10K_PKTLOG_RCUPDATE   = 0x000000008,
 	ATH10K_PKTLOG_DBG_PRINT  = 0x000000010,
-	ATH10K_PKTLOG_ANY        = 0x00000001f,
+	ATH10K_PKTLOG_PEER_STATS = 0x000000040,
+	ATH10K_PKTLOG_ANY        = 0x00000005f,
 };
 
 enum ath10k_dbg_aggr_mode {
@@ -60,6 +61,21 @@ enum ath10k_dbg_aggr_mode {
 	ATH10K_DBG_AGGR_MODE_MAX,
 };
 
+/* Types of packet log events */
+enum ath_pktlog_type {
+	ATH_PKTLOG_TYPE_TX_CTRL = 1,
+	ATH_PKTLOG_TYPE_TX_STAT,
+};
+
+struct ath10k_pktlog_hdr {
+	__le16 flags;
+	__le16 missed_cnt;
+	__le16 log_type; /* Type of log information foll this header */
+	__le16 size; /* Size of variable length log information in bytes */
+	__le32 timestamp;
+	u8 payload[0];
+} __packed;
+
 /* FIXME: How to calculate the buffer size sanely? */
 #define ATH10K_FW_STATS_BUF_SIZE (1024 * 1024)
 
diff --git a/drivers/net/wireless/ath/ath10k/htt.h b/drivers/net/wireless/ath/ath10k/htt.h
index 6305308422c4..89db96afea17 100644
--- a/drivers/net/wireless/ath/ath10k/htt.h
+++ b/drivers/net/wireless/ath/ath10k/htt.h
@@ -1497,6 +1497,23 @@ struct htt_peer_tx_stats {
 	u8 payload[0];
 } __packed;
 
+#define ATH10K_10_2_TX_STATS_OFFSET	136
+#define PEER_STATS_FOR_NO_OF_PPDUS	4
+
+struct ath10k_10_2_peer_tx_stats {
+	u8 ratecode[PEER_STATS_FOR_NO_OF_PPDUS];
+	u8 success_pkts[PEER_STATS_FOR_NO_OF_PPDUS];
+	__le16 success_bytes[PEER_STATS_FOR_NO_OF_PPDUS];
+	u8 retry_pkts[PEER_STATS_FOR_NO_OF_PPDUS];
+	__le16 retry_bytes[PEER_STATS_FOR_NO_OF_PPDUS];
+	u8 failed_pkts[PEER_STATS_FOR_NO_OF_PPDUS];
+	__le16 failed_bytes[PEER_STATS_FOR_NO_OF_PPDUS];
+	u8 flags[PEER_STATS_FOR_NO_OF_PPDUS];
+	__le32 tx_duration;
+	u8 tx_ppdu_cnt;
+	u8 peer_id;
+} __packed;
+
 union htt_rx_pn_t {
 	/* WEP: 24-bit PN */
 	u32 pn24;
diff --git a/drivers/net/wireless/ath/ath10k/htt_rx.c b/drivers/net/wireless/ath/ath10k/htt_rx.c
index e31438541ee1..3361807b87b3 100644
--- a/drivers/net/wireless/ath/ath10k/htt_rx.c
+++ b/drivers/net/wireless/ath/ath10k/htt_rx.c
@@ -2424,6 +2424,62 @@ static void ath10k_htt_fetch_peer_stats(struct ath10k *ar,
 	rcu_read_unlock();
 }
 
+static void ath10k_fetch_10_2_tx_stats(struct ath10k *ar, u8 *data)
+{
+	struct ath10k_pktlog_hdr *hdr = (struct ath10k_pktlog_hdr *)data;
+	struct ath10k_per_peer_tx_stats *p_tx_stats = &ar->peer_tx_stats;
+	struct ath10k_10_2_peer_tx_stats *tx_stats;
+	struct ieee80211_sta *sta;
+	struct ath10k_peer *peer;
+	u16 log_type = __le16_to_cpu(hdr->log_type);
+	u32 peer_id = 0, i;
+
+	if (log_type != ATH_PKTLOG_TYPE_TX_STAT)
+		return;
+
+	tx_stats = (struct ath10k_10_2_peer_tx_stats *)((hdr->payload) +
+		    ATH10K_10_2_TX_STATS_OFFSET);
+
+	if (!tx_stats->tx_ppdu_cnt)
+		return;
+
+	peer_id = tx_stats->peer_id;
+
+	rcu_read_lock();
+	spin_lock_bh(&ar->data_lock);
+	peer = ath10k_peer_find_by_id(ar, peer_id);
+	if (!peer) {
+		ath10k_warn(ar, "Invalid peer id %d in peer stats buffer\n",
+			    peer_id);
+		goto out;
+	}
+
+	sta = peer->sta;
+	for (i = 0; i < tx_stats->tx_ppdu_cnt; i++) {
+		p_tx_stats->succ_bytes =
+			__le16_to_cpu(tx_stats->success_bytes[i]);
+		p_tx_stats->retry_bytes =
+			__le16_to_cpu(tx_stats->retry_bytes[i]);
+		p_tx_stats->failed_bytes =
+			__le16_to_cpu(tx_stats->failed_bytes[i]);
+		p_tx_stats->ratecode = tx_stats->ratecode[i];
+		p_tx_stats->flags = tx_stats->flags[i];
+		p_tx_stats->succ_pkts = tx_stats->success_pkts[i];
+		p_tx_stats->retry_pkts = tx_stats->retry_pkts[i];
+		p_tx_stats->failed_pkts = tx_stats->failed_pkts[i];
+
+		ath10k_update_per_peer_tx_stats(ar, sta, p_tx_stats);
+	}
+	spin_unlock_bh(&ar->data_lock);
+	rcu_read_unlock();
+
+	return;
+
+out:
+	spin_unlock_bh(&ar->data_lock);
+	rcu_read_unlock();
+}
+
 bool ath10k_htt_t2h_msg_handler(struct ath10k *ar, struct sk_buff *skb)
 {
 	struct ath10k_htt *htt = &ar->htt;
@@ -2541,6 +2597,10 @@ bool ath10k_htt_t2h_msg_handler(struct ath10k *ar, struct sk_buff *skb)
 					skb->len -
 					offsetof(struct htt_resp,
 						 pktlog_msg.payload));
+
+		if (ath10k_peer_stats_enabled(ar))
+			ath10k_fetch_10_2_tx_stats(ar,
+						   resp->pktlog_msg.payload);
 		break;
 	}
 	case HTT_T2H_MSG_TYPE_RX_FLUSH: {
diff --git a/drivers/net/wireless/ath/ath10k/mac.c b/drivers/net/wireless/ath/ath10k/mac.c
index 0a947eef348d..39a5408a1ce2 100644
--- a/drivers/net/wireless/ath/ath10k/mac.c
+++ b/drivers/net/wireless/ath/ath10k/mac.c
@@ -7536,6 +7536,16 @@ struct ath10k_mac_change_chanctx_arg {
 				    arvif->vdev_id, ret);
 	}
 
+	if (ath10k_peer_stats_enabled(ar)) {
+		ar->pktlog_filter |= ATH10K_PKTLOG_PEER_STATS;
+		ret = ath10k_wmi_pdev_pktlog_enable(ar,
+						    ar->pktlog_filter);
+		if (ret) {
+			ath10k_warn(ar, "failed to enable pktlog %d\n", ret);
+			goto err_stop;
+		}
+	}
+
 	mutex_unlock(&ar->conf_mutex);
 	return 0;
 
-- 
1.7.9.5

Re: [PATCH 2/2] ath10k: add per peer tx stats support for 10.2.4

[Kalle Valo]

<akolli@qti.qualcomm.com> writes:

> From: Anilkumar Kolli <akolli@qti.qualcomm.com>
>
> 10.2.4 firmware branch (used in QCA988X) does not support
> HTT_10_4_T2H_MSG_TYPE_PEER_STATS and that's why ath10k does not provide
> tranmission rate statistics to user space, instead it just shows
> hardcoded 6 Mbit/s. But pktlog firmware facility provides per peer tx
> statistics. The firmware sends one pktlog event for every four
> PPDUs per peer, which include:
>
> * successful number of packets and bytes transmitted
> * number of packets and bytes dropped
> * retried number of packets and bytes
> * rate info per ppdu
>
> Firmware supports WMI_SERVICE_PEER_STATS, pktlog is enabled through
> ATH10K_FLAG_PEER_STATS, which is nowadays enabled by default in ath10k.
>
> This patch does not impact throughput.
>
> Tested on QCA9880 with firmware version 10.2.4.70.48. This should also
> work with firmware branch 10.2.4-1.0-00029
>
> Parse peer stats from pktlog packets and update the tx rate information
> per STA. This way user space can query about transmit rate with iw:
>
> $iw wlan0 station dump
> Station 3c:a9:f4:72:bb:a4 (on wlan1)
>         inactive time:  8210 ms
>         rx bytes:       9166
>         rx packets:     44
>         tx bytes:       1105
>         tx packets:     9
>         tx retries:     0
>         tx failed:      1
>         rx drop misc:   3
>         signal:         -75 [-75, -87, -88] dBm
>         signal avg:     -75 [-75, -85, -88] dBm
>         tx bitrate:     39.0 MBit/s MCS 10
>         rx bitrate:     26.0 MBit/s MCS 3
>         rx duration:    23250 us
>         authorized:     yes
>         authenticated:  yes
>         associated:     yes
>         preamble:       short
>         WMM/WME:        yes
>         MFP:            no
>         TDLS peer:      no
>         DTIM period:    2
>         beacon interval:100
>         short preamble: yes
>         short slot time:yes
>         connected time: 22 seconds
>
> Signed-off-by: Anilkumar Kolli <akolli@qti.qualcomm.com>

[...]

> +struct ath10k_10_2_peer_tx_stats {
> +	u8 ratecode[PEER_STATS_FOR_NO_OF_PPDUS];
> +	u8 success_pkts[PEER_STATS_FOR_NO_OF_PPDUS];
> +	__le16 success_bytes[PEER_STATS_FOR_NO_OF_PPDUS];
> +	u8 retry_pkts[PEER_STATS_FOR_NO_OF_PPDUS];
> +	__le16 retry_bytes[PEER_STATS_FOR_NO_OF_PPDUS];
> +	u8 failed_pkts[PEER_STATS_FOR_NO_OF_PPDUS];
> +	__le16 failed_bytes[PEER_STATS_FOR_NO_OF_PPDUS];
> +	u8 flags[PEER_STATS_FOR_NO_OF_PPDUS];
> +	__le32 tx_duration;
> +	u8 tx_ppdu_cnt;
> +	u8 peer_id;
> +} __packed;

Toke, hopefully the tx_duration value here helps with ATF implementation
using QCA988X.

--=20
Kalle Valo=

Re: [PATCH 2/2] ath10k: add per peer tx stats support for 10.2.4

[Toke Høiland-Jørgensen]

>> +struct ath10k_10_2_peer_tx_stats {
>> +	u8 ratecode[PEER_STATS_FOR_NO_OF_PPDUS];
>> +	u8 success_pkts[PEER_STATS_FOR_NO_OF_PPDUS];
>> +	__le16 success_bytes[PEER_STATS_FOR_NO_OF_PPDUS];
>> +	u8 retry_pkts[PEER_STATS_FOR_NO_OF_PPDUS];
>> +	__le16 retry_bytes[PEER_STATS_FOR_NO_OF_PPDUS];
>> +	u8 failed_pkts[PEER_STATS_FOR_NO_OF_PPDUS];
>> +	__le16 failed_bytes[PEER_STATS_FOR_NO_OF_PPDUS];
>> +	u8 flags[PEER_STATS_FOR_NO_OF_PPDUS];
>> +	__le32 tx_duration;
>> +	u8 tx_ppdu_cnt;
>> +	u8 peer_id;
>> +} __packed;
>
>Toke, hopefully the tx_duration value here helps with ATF
>implementation
>using QCA988X=2E

Awesome! What's the semantics of this field? Just total=20
duration spent serving that station in the reporting interval?
Does it include retry attempts?

-Toke

Re: [PATCH 2/2] ath10k: add per peer tx stats support for 10.2.4

[Kalle Valo]

VG9rZSBIw7hpbGFuZC1Kw7hyZ2Vuc2VuIDx0b2tlQHRva2UuZGs+IHdyaXRlczoNCg0KPj4+ICtz
dHJ1Y3QgYXRoMTBrXzEwXzJfcGVlcl90eF9zdGF0cyB7DQo+Pj4gKwl1OCByYXRlY29kZVtQRUVS
X1NUQVRTX0ZPUl9OT19PRl9QUERVU107DQo+Pj4gKwl1OCBzdWNjZXNzX3BrdHNbUEVFUl9TVEFU
U19GT1JfTk9fT0ZfUFBEVVNdOw0KPj4+ICsJX19sZTE2IHN1Y2Nlc3NfYnl0ZXNbUEVFUl9TVEFU
U19GT1JfTk9fT0ZfUFBEVVNdOw0KPj4+ICsJdTggcmV0cnlfcGt0c1tQRUVSX1NUQVRTX0ZPUl9O
T19PRl9QUERVU107DQo+Pj4gKwlfX2xlMTYgcmV0cnlfYnl0ZXNbUEVFUl9TVEFUU19GT1JfTk9f
T0ZfUFBEVVNdOw0KPj4+ICsJdTggZmFpbGVkX3BrdHNbUEVFUl9TVEFUU19GT1JfTk9fT0ZfUFBE
VVNdOw0KPj4+ICsJX19sZTE2IGZhaWxlZF9ieXRlc1tQRUVSX1NUQVRTX0ZPUl9OT19PRl9QUERV
U107DQo+Pj4gKwl1OCBmbGFnc1tQRUVSX1NUQVRTX0ZPUl9OT19PRl9QUERVU107DQo+Pj4gKwlf
X2xlMzIgdHhfZHVyYXRpb247DQo+Pj4gKwl1OCB0eF9wcGR1X2NudDsNCj4+PiArCXU4IHBlZXJf
aWQ7DQo+Pj4gK30gX19wYWNrZWQ7DQo+Pg0KPj5Ub2tlLCBob3BlZnVsbHkgdGhlIHR4X2R1cmF0
aW9uIHZhbHVlIGhlcmUgaGVscHMgd2l0aCBBVEYNCj4+aW1wbGVtZW50YXRpb24NCj4+dXNpbmcg
UUNBOTg4WC4NCj4NCj4gQXdlc29tZSEgV2hhdCdzIHRoZSBzZW1hbnRpY3Mgb2YgdGhpcyBmaWVs
ZD8gSnVzdCB0b3RhbCANCj4gZHVyYXRpb24gc3BlbnQgc2VydmluZyB0aGF0IHN0YXRpb24gaW4g
dGhlIHJlcG9ydGluZyBpbnRlcnZhbD8NCj4gRG9lcyBpdCBpbmNsdWRlIHJldHJ5IGF0dGVtcHRz
Pw0KDQpJIGhhdmUgbm8gY2x1ZSA6KSBJIGp1c3Qgbm90aWNlZCB0aGlzIHdoaWxlIEkgd2FzIHJl
dmlld2luZyB0aGUgcGF0Y2gNCmludGVybmFsbHkgYW5kIGltbWVkaWF0ZWx5IHJlY2FsbGVkIG91
ciBkaXNjdXNzaW9ucyBhdCBTZW91bC4gSSBjYW4gdHJ5DQp0byBmaW5kIG91dCwgYnV0IHRoYXQg
d2lsbCB0YWtlIGEgbG9uZyB0aW1lIGFzIEkgaGF2ZSB3YXkgdG9vIG11Y2ggc3R1ZmYNCnBlbmRp
bmcgYXQgdGhlIG1vbWVudC4gSG9wZWZ1bGx5IHNvbWVvbmUgbW9yZSBrbm93bGVkZ2VhYmxlIChB
bmlsa3VtYXI/KQ0KY2FuIGNoaW1lIGluIGFuZCBoZWxwLg0KDQotLSANCkthbGxlIFZhbG8=

Re: [PATCH 2/2] ath10k: add per peer tx stats support for 10.2.4

[akolli]

On 2017-11-30 22:08, Kalle Valo wrote:
> Toke Høiland-Jørgensen <toke@toke.dk> writes:
> 
>>>> +struct ath10k_10_2_peer_tx_stats {
>>>> +	u8 ratecode[PEER_STATS_FOR_NO_OF_PPDUS];
>>>> +	u8 success_pkts[PEER_STATS_FOR_NO_OF_PPDUS];
>>>> +	__le16 success_bytes[PEER_STATS_FOR_NO_OF_PPDUS];
>>>> +	u8 retry_pkts[PEER_STATS_FOR_NO_OF_PPDUS];
>>>> +	__le16 retry_bytes[PEER_STATS_FOR_NO_OF_PPDUS];
>>>> +	u8 failed_pkts[PEER_STATS_FOR_NO_OF_PPDUS];
>>>> +	__le16 failed_bytes[PEER_STATS_FOR_NO_OF_PPDUS];
>>>> +	u8 flags[PEER_STATS_FOR_NO_OF_PPDUS];
>>>> +	__le32 tx_duration;
>>>> +	u8 tx_ppdu_cnt;
>>>> +	u8 peer_id;
>>>> +} __packed;
>>> 
>>> Toke, hopefully the tx_duration value here helps with ATF
>>> implementation
>>> using QCA988X.
>> 
>> Awesome! What's the semantics of this field? Just total
>> duration spent serving that station in the reporting interval?
>> Does it include retry attempts?
> 
> I have no clue :) I just noticed this while I was reviewing the patch
> internally and immediately recalled our discussions at Seoul. I can try
> to find out, but that will take a long time as I have way too much 
> stuff
> pending at the moment. Hopefully someone more knowledgeable 
> (Anilkumar?)
> can chime in and help.

tx_duration is aggregate time duration of 4 PPDU sent to STA.
FW sends these values for retry packets also.

Thanks,
Anil.

Re: [PATCH 2/2] ath10k: add per peer tx stats support for 10.2.4

[Toke Høiland-Jørgensen]

akolli@codeaurora.org writes:

> On 2017-11-30 22:08, Kalle Valo wrote:
>> Toke H=C3=B8iland-J=C3=B8rgensen <toke@toke.dk> writes:
>>=20
>>>>> +struct ath10k_10_2_peer_tx_stats {
>>>>> +	u8 ratecode[PEER_STATS_FOR_NO_OF_PPDUS];
>>>>> +	u8 success_pkts[PEER_STATS_FOR_NO_OF_PPDUS];
>>>>> +	__le16 success_bytes[PEER_STATS_FOR_NO_OF_PPDUS];
>>>>> +	u8 retry_pkts[PEER_STATS_FOR_NO_OF_PPDUS];
>>>>> +	__le16 retry_bytes[PEER_STATS_FOR_NO_OF_PPDUS];
>>>>> +	u8 failed_pkts[PEER_STATS_FOR_NO_OF_PPDUS];
>>>>> +	__le16 failed_bytes[PEER_STATS_FOR_NO_OF_PPDUS];
>>>>> +	u8 flags[PEER_STATS_FOR_NO_OF_PPDUS];
>>>>> +	__le32 tx_duration;
>>>>> +	u8 tx_ppdu_cnt;
>>>>> +	u8 peer_id;
>>>>> +} __packed;
>>>>=20
>>>> Toke, hopefully the tx_duration value here helps with ATF
>>>> implementation
>>>> using QCA988X.
>>>=20
>>> Awesome! What's the semantics of this field? Just total
>>> duration spent serving that station in the reporting interval?
>>> Does it include retry attempts?
>>=20
>> I have no clue :) I just noticed this while I was reviewing the patch
>> internally and immediately recalled our discussions at Seoul. I can try
>> to find out, but that will take a long time as I have way too much=20
>> stuff
>> pending at the moment. Hopefully someone more knowledgeable=20
>> (Anilkumar?)
>> can chime in and help.
>
> tx_duration is aggregate time duration of 4 PPDU sent to STA.
> FW sends these values for retry packets also.

Great, that sounds like just what we need. Thanks for the pointer :)

-Toke

Re: [PATCH 2/2] ath10k: add per peer tx stats support for 10.2.4

[Kalle Valo]

VG9rZSBIw7hpbGFuZC1Kw7hyZ2Vuc2VuIDx0b2tlQHRva2UuZGs+IHdyaXRlczoNCg0KPiBha29s
bGlAY29kZWF1cm9yYS5vcmcgd3JpdGVzOg0KPg0KPj4gT24gMjAxNy0xMS0zMCAyMjowOCwgS2Fs
bGUgVmFsbyB3cm90ZToNCj4+PiBUb2tlIEjDuGlsYW5kLUrDuHJnZW5zZW4gPHRva2VAdG9rZS5k
az4gd3JpdGVzOg0KPj4+IA0KPj4+Pj4+ICtzdHJ1Y3QgYXRoMTBrXzEwXzJfcGVlcl90eF9zdGF0
cyB7DQo+Pj4+Pj4gKwl1OCByYXRlY29kZVtQRUVSX1NUQVRTX0ZPUl9OT19PRl9QUERVU107DQo+
Pj4+Pj4gKwl1OCBzdWNjZXNzX3BrdHNbUEVFUl9TVEFUU19GT1JfTk9fT0ZfUFBEVVNdOw0KPj4+
Pj4+ICsJX19sZTE2IHN1Y2Nlc3NfYnl0ZXNbUEVFUl9TVEFUU19GT1JfTk9fT0ZfUFBEVVNdOw0K
Pj4+Pj4+ICsJdTggcmV0cnlfcGt0c1tQRUVSX1NUQVRTX0ZPUl9OT19PRl9QUERVU107DQo+Pj4+
Pj4gKwlfX2xlMTYgcmV0cnlfYnl0ZXNbUEVFUl9TVEFUU19GT1JfTk9fT0ZfUFBEVVNdOw0KPj4+
Pj4+ICsJdTggZmFpbGVkX3BrdHNbUEVFUl9TVEFUU19GT1JfTk9fT0ZfUFBEVVNdOw0KPj4+Pj4+
ICsJX19sZTE2IGZhaWxlZF9ieXRlc1tQRUVSX1NUQVRTX0ZPUl9OT19PRl9QUERVU107DQo+Pj4+
Pj4gKwl1OCBmbGFnc1tQRUVSX1NUQVRTX0ZPUl9OT19PRl9QUERVU107DQo+Pj4+Pj4gKwlfX2xl
MzIgdHhfZHVyYXRpb247DQo+Pj4+Pj4gKwl1OCB0eF9wcGR1X2NudDsNCj4+Pj4+PiArCXU4IHBl
ZXJfaWQ7DQo+Pj4+Pj4gK30gX19wYWNrZWQ7DQo+Pj4+PiANCj4+Pj4+IFRva2UsIGhvcGVmdWxs
eSB0aGUgdHhfZHVyYXRpb24gdmFsdWUgaGVyZSBoZWxwcyB3aXRoIEFURg0KPj4+Pj4gaW1wbGVt
ZW50YXRpb24NCj4+Pj4+IHVzaW5nIFFDQTk4OFguDQo+Pj4+IA0KPj4+PiBBd2Vzb21lISBXaGF0
J3MgdGhlIHNlbWFudGljcyBvZiB0aGlzIGZpZWxkPyBKdXN0IHRvdGFsDQo+Pj4+IGR1cmF0aW9u
IHNwZW50IHNlcnZpbmcgdGhhdCBzdGF0aW9uIGluIHRoZSByZXBvcnRpbmcgaW50ZXJ2YWw/DQo+
Pj4+IERvZXMgaXQgaW5jbHVkZSByZXRyeSBhdHRlbXB0cz8NCj4+PiANCj4+PiBJIGhhdmUgbm8g
Y2x1ZSA6KSBJIGp1c3Qgbm90aWNlZCB0aGlzIHdoaWxlIEkgd2FzIHJldmlld2luZyB0aGUgcGF0
Y2gNCj4+PiBpbnRlcm5hbGx5IGFuZCBpbW1lZGlhdGVseSByZWNhbGxlZCBvdXIgZGlzY3Vzc2lv
bnMgYXQgU2VvdWwuIEkgY2FuIHRyeQ0KPj4+IHRvIGZpbmQgb3V0LCBidXQgdGhhdCB3aWxsIHRh
a2UgYSBsb25nIHRpbWUgYXMgSSBoYXZlIHdheSB0b28gbXVjaCANCj4+PiBzdHVmZg0KPj4+IHBl
bmRpbmcgYXQgdGhlIG1vbWVudC4gSG9wZWZ1bGx5IHNvbWVvbmUgbW9yZSBrbm93bGVkZ2VhYmxl
IA0KPj4+IChBbmlsa3VtYXI/KQ0KPj4+IGNhbiBjaGltZSBpbiBhbmQgaGVscC4NCj4+DQo+PiB0
eF9kdXJhdGlvbiBpcyBhZ2dyZWdhdGUgdGltZSBkdXJhdGlvbiBvZiA0IFBQRFUgc2VudCB0byBT
VEEuDQo+PiBGVyBzZW5kcyB0aGVzZSB2YWx1ZXMgZm9yIHJldHJ5IHBhY2tldHMgYWxzby4NCj4N
Cj4gR3JlYXQsIHRoYXQgc291bmRzIGxpa2UganVzdCB3aGF0IHdlIG5lZWQuDQoNCkV4Y2VwdCBt
YXBwaW5nIHRvIHRoZSBpZWU4MDIxMV90eF9zdGF0dXMoKSBtaWdodCBiZSBkaWZmaWN1bHQuIEkn
bSBub3QNCnN1cmUgaG93IEhUVF9UMkhfTVNHX1RZUEVfUEtUTE9HIGV2ZW50cyBhcmUgc2VudCBp
biByZWxhdGlvbiB0bw0KSFRUX1QySF9NU0dfVFlQRV9UWF9DT01QTF9JTkQuDQoNCi0tIA0KS2Fs
bGUgVmFsbw==

Re: [PATCH 2/2] ath10k: add per peer tx stats support for 10.2.4

[Toke Høiland-Jørgensen]

Kalle Valo <kvalo@qca.qualcomm.com> writes:

>>> tx_duration is aggregate time duration of 4 PPDU sent to STA.
>>> FW sends these values for retry packets also.
>>
>> Great, that sounds like just what we need.
>
> Except mapping to the iee80211_tx_status() might be difficult. I'm not
> sure how HTT_T2H_MSG_TYPE_PKTLOG events are sent in relation to
> HTT_T2H_MSG_TYPE_TX_COMPL_IND.

Yeah, figures. Well, maybe we'll just have to support an asynchronous
callback into mac80211 to register airtime usage. Will probably have to
add some asynchronicity anyway to avoid doing too much work in the fast
path.

Will keep that in mind when I get around to revisiting those patches :)

-Toke

Re: [PATCH 2/2] ath10k: add per peer tx stats support for 10.2.4

[Johannes Berg]

On Fri, 2017-12-01 at 16:29 +0100, Toke Høiland-Jørgensen wrote:
> 
> Yeah, figures. Well, maybe we'll just have to support an asynchronous
> callback into mac80211 to register airtime usage. Will probably have to
> add some asynchronicity anyway to avoid doing too much work in the fast
> path.

I really think we should have that anyway. In fact, perhaps we should
just enforce it that way? Then ath9k would have to do a bit more work,
but it's also the slower of the devices :-)

But this plays in with the whole multi-queue RX we discussed before.

johannes

Re: [PATCH 2/2] ath10k: add per peer tx stats support for 10.2.4

[Toke Høiland-Jørgensen]

Johannes Berg <johannes@sipsolutions.net> writes:

> On Fri, 2017-12-01 at 16:29 +0100, Toke H=C3=B8iland-J=C3=B8rgensen wrote:
>>=20
>> Yeah, figures. Well, maybe we'll just have to support an asynchronous
>> callback into mac80211 to register airtime usage. Will probably have to
>> add some asynchronicity anyway to avoid doing too much work in the fast
>> path.
>
> I really think we should have that anyway. In fact, perhaps we should
> just enforce it that way? Then ath9k would have to do a bit more work,
> but it's also the slower of the devices :-)
>
> But this plays in with the whole multi-queue RX we discussed before.

Yeah, that's what I meant by 'add some asynchronicity anyway' :)

I guess that once we have an asynchronous scheduler task of some kind,
supporting the callback is easy, but pulling the data out of each packet
needs more work. But since we'll have to do that for ath9k anyway it
becomes more a question of whether it's something that should be
supported in mac80211 or if it should be implemented in each driver.
But since that logic will have to be written from scratch anyway, is
there any reason to not just do it in mac80211 from the beginning?

-Toke

Re: [PATCH 2/2] ath10k: add per peer tx stats support for 10.2.4

[Johannes Berg]

On Fri, 2017-12-01 at 16:54 +0100, Toke Høiland-Jørgensen wrote:

> I guess that once we have an asynchronous scheduler task of some kind,
> supporting the callback is easy, but pulling the data out of each packet
> needs more work.

Right.


> But since we'll have to do that for ath9k anyway it
> becomes more a question of whether it's something that should be
> supported in mac80211 or if it should be implemented in each driver.
> But since that logic will have to be written from scratch anyway, is
> there any reason to not just do it in mac80211 from the beginning?

I was thinking in the driver so that there's no overhead for the other
drivers that don't want it that way.

Dunno. It probably doesn't really matter.

johannes

Re: [PATCH 2/2] ath10k: add per peer tx stats support for 10.2.4

[Toke Høiland-Jørgensen]

Johannes Berg <johannes@sipsolutions.net> writes:

> On Fri, 2017-12-01 at 16:54 +0100, Toke H=C3=B8iland-J=C3=B8rgensen wrote:
>
>> But since we'll have to do that for ath9k anyway it
>> becomes more a question of whether it's something that should be
>> supported in mac80211 or if it should be implemented in each driver.
>> But since that logic will have to be written from scratch anyway, is
>> there any reason to not just do it in mac80211 from the beginning?
>
> I was thinking in the driver so that there's no overhead for the other
> drivers that don't want it that way.
>
> Dunno. It probably doesn't really matter.

I'll prototype something, then we can revisit this based on actual code :)

-Toke

Re: [PATCH 2/2] ath10k: add per peer tx stats support for 10.2.4

[Maxime Bizon]

On Thu, 2017-11-30 at 18:28 +0530, akolli@qti.qualcomm.com wrote:

Hello,

> Tested on QCA9880 with firmware version 10.2.4.70.48. This should also
> work with firmware branch 10.2.4-1.0-00029

I tried using your patch on 4.14 with firmware 10.2.4-1.0-00029

> Parse peer stats from pktlog packets and update the tx rate information
> per STA. This way user space can query about transmit rate with iw:

everything works ok, ath10k_update_per_peer_tx_stats() is called and
ath10k_sta fields are updated correctly

but tx bitrate is still 6 MBit/s in station dump

 # iw dev wlan1 station dump
Station e4:42:a6:24:c8:95 (on wlan1)
	inactive time:	0 ms
	rx bytes:	222415
	rx packets:	1678
	tx bytes:	8511140
	tx packets:	5828
	tx retries:	0
	tx failed:	4
	rx drop misc:	2
	signal:  	-39 [-54, -56, -39] dBm
	signal avg:	-39 [-53, -55, -39] dBm
	tx bitrate:	6.0 MBit/s
	rx bitrate:	360.0 MBit/s VHT-MCS 8 40MHz short GI VHT-NSS 2
	authorized:	yes
	authenticated:	yes
	associated:	yes
	preamble:	long
	WMM/WME:	yes
	MFP:		no
	TDLS peer:	no
	DTIM period:	2
	beacon interval:96
	short slot time:yes
	connected time:	1136 seconds

I see that ath10k does not implement ->dump_station callback, so which
part of the code updates the generic "struct station_info" fields ?

Am I missing a patch ?

Thanks

-- 
Maxime

Re: [PATCH 2/2] ath10k: add per peer tx stats support for 10.2.4

[akolli]

Hello,

Thanks for verifying the patch.

On 2017-12-01 16:15, Maxime Bizon wrote:
> On Thu, 2017-11-30 at 18:28 +0530, akolli@qti.qualcomm.com wrote:
> 
> Hello,
> 
>> Tested on QCA9880 with firmware version 10.2.4.70.48. This should also
>> work with firmware branch 10.2.4-1.0-00029
> 
> I tried using your patch on 4.14 with firmware 10.2.4-1.0-00029
> 
I will test update.

>> Parse peer stats from pktlog packets and update the tx rate 
>> information
>> per STA. This way user space can query about transmit rate with iw:
> 
> everything works ok, ath10k_update_per_peer_tx_stats() is called and
> ath10k_sta fields are updated correctly
> 
> but tx bitrate is still 6 MBit/s in station dump
> 
Hope CONFIG_MAC80211_DEBUGFS is enabled in your build.
Please Run ping traffic and see and 'station dump'

>  # iw dev wlan1 station dump
> Station e4:42:a6:24:c8:95 (on wlan1)
> 	inactive time:	0 ms
> 	rx bytes:	222415
> 	rx packets:	1678
> 	tx bytes:	8511140
> 	tx packets:	5828
> 	tx retries:	0
> 	tx failed:	4
> 	rx drop misc:	2
> 	signal:  	-39 [-54, -56, -39] dBm
> 	signal avg:	-39 [-53, -55, -39] dBm
> 	tx bitrate:	6.0 MBit/s
> 	rx bitrate:	360.0 MBit/s VHT-MCS 8 40MHz short GI VHT-NSS 2
> 	authorized:	yes
> 	authenticated:	yes
> 	associated:	yes
> 	preamble:	long
> 	WMM/WME:	yes
> 	MFP:		no
> 	TDLS peer:	no
> 	DTIM period:	2
> 	beacon interval:96
> 	short slot time:yes
> 	connected time:	1136 seconds
> 
> I see that ath10k does not implement ->dump_station callback, so which
> part of the code updates the generic "struct station_info" fields ?
> 
> Am I missing a patch ?
> 
Rate parameters are updated from below call,
.sta_statistics                 = ath10k_sta_statistics,

Thanks,
Anil.

Re: [PATCH 2/2] ath10k: add per peer tx stats support for 10.2.4

[Maxime Bizon]

On Fri, 2017-12-01 at 19:18 +0530, akolli@codeaurora.org wrote:

> Hope CONFIG_MAC80211_DEBUGFS is enabled in your build.

it wasn't and IMHO it's confusing because tx rate is filled by the other
drivers without it.

I now have the following warning:

[   96.174967] ------------[ cut here ]------------
[   96.179640] WARNING: CPU: 0 PID: 609 at net/wireless/util.c:1254 cfg80211_calculate_bitrate+0x174/0x220
[   96.189538] invalid rate bw=1, mcs=9, nss=2
[   96.219736] CPU: 0 PID: 609 Comm: hostapd Not tainted 4.14.3-00381-gec9756b0f64d #28
[   96.227910] Hardware name: Marvell Kirkwood (Flattened Device Tree)
[   96.235450] [<80010124>] (unwind_backtrace) from [<8000d9ec>] (show_stack+0x10/0x14)
[   96.247180] [<8000d9ec>] (show_stack) from [<8002016c>] (__warn+0xdc/0xf8)
[   96.254243] [<8002016c>] (__warn) from [<800201bc>] (warn_slowpath_fmt+0x34/0x44)
[   96.262016] [<800201bc>] (warn_slowpath_fmt) from [<8064dfdc>] (cfg80211_calculate_bitrate+0x174/0x220)
[   96.272652] [<8064dfdc>] (cfg80211_calculate_bitrate) from [<806692d4>] (nl80211_put_sta_rate+0x44/0x1dc)
[   96.282509] [<806692d4>] (nl80211_put_sta_rate) from [<8066001c>] (nl80211_send_station+0x388/0xaf0)
[   96.292261] [<8066001c>] (nl80211_send_station) from [<8066082c>] (nl80211_get_station+0xa8/0xec)
[   96.304166] [<8066082c>] (nl80211_get_station) from [<80509c20>] (genl_rcv_msg+0x2dc/0x34c)
[   96.313324] [<80509c20>] (genl_rcv_msg) from [<8050890c>] (netlink_rcv_skb+0x84/0xdc)
[   96.321880] [<8050890c>] (netlink_rcv_skb) from [<805093c0>] (genl_rcv+0x20/0x34)
[   96.329668] [<805093c0>] (genl_rcv) from [<80508188>] (netlink_unicast+0x12c/0x1e0)
[   96.338408] [<80508188>] (netlink_unicast) from [<805085d8>] (netlink_sendmsg+0x2e0/0x304)
[   96.350736] [<805085d8>] (netlink_sendmsg) from [<804b5f9c>] (sock_sendmsg+0x14/0x24)
[   96.358656] [<804b5f9c>] (sock_sendmsg) from [<804b66e8>] (___sys_sendmsg+0x1c8/0x20c)
[   96.367093] [<804b66e8>] (___sys_sendmsg) from [<804b739c>] (__sys_sendmsg+0x40/0x64)
[   96.375276] [<804b739c>] (__sys_sendmsg) from [<8000a3e0>] (ret_fast_syscall+0x0/0x44)
[   96.383455] ---[ end trace da8257d6a850e91a ]---

# iw dev wlan1 station dump
Station e4:42:a6:24:c8:95 (on wlan1)
	inactive time:	550 ms
	rx bytes:	41217
	rx packets:	152
	tx bytes:	49397
	tx packets:	107
	tx retries:	0
	tx failed:	1
	rx drop misc:	0
	signal:  	-62 [-66, -65, -83] dBm
	signal avg:	-61 [-65, -65, -78] dBm
	tx bitrate:	 VHT-MCS 9 short GI VHT-NSS 2
	rx bitrate:	360.0 MBit/s VHT-MCS 8 40MHz short GI VHT-NSS 2
	rx duration:	0 us
	authorized:	yes
	authenticated:	yes
	associated:	yes
	preamble:	long
	WMM/WME:	yes
	MFP:		no
	TDLS peer:	no
	DTIM period:	2
	beacon interval:96
	short slot time:yes
	connected time:	5 seconds

Thanks

-- 
Maxime

Re: [PATCH 2/2] ath10k: add per peer tx stats support for 10.2.4

[Christian Lamparter]

On Friday, December 1, 2017 4:05:10 PM CET Maxime Bizon wrote:
> 
> On Fri, 2017-12-01 at 19:18 +0530, akolli@codeaurora.org wrote:
> 
> > Hope CONFIG_MAC80211_DEBUGFS is enabled in your build.
> 
> it wasn't and IMHO it's confusing because tx rate is filled by the other
> drivers without it.
> 
> I now have the following warning:
> 
> [   96.174967] ------------[ cut here ]------------
> [   96.179640] WARNING: CPU: 0 PID: 609 at net/wireless/util.c:1254 cfg80211_calculate_bitrate+0x174/0x220
> [   96.189538] invalid rate bw=1, mcs=9, nss=2
> [   96.219736] CPU: 0 PID: 609 Comm: hostapd Not tainted 4.14.3-00381-gec9756b0f64d #28
> [   96.227910] Hardware name: Marvell Kirkwood (Flattened Device Tree)
> [   96.235450] [<80010124>] (unwind_backtrace) from [<8000d9ec>] (show_stack+0x10/0x14)
> [   96.247180] [<8000d9ec>] (show_stack) from [<8002016c>] (__warn+0xdc/0xf8)
> [   96.254243] [<8002016c>] (__warn) from [<800201bc>] (warn_slowpath_fmt+0x34/0x44)
> [   96.262016] [<800201bc>] (warn_slowpath_fmt) from [<8064dfdc>] (cfg80211_calculate_bitrate+0x174/0x220)
> [   96.272652] [<8064dfdc>] (cfg80211_calculate_bitrate) from [<806692d4>] (nl80211_put_sta_rate+0x44/0x1dc)
> [   96.282509] [<806692d4>] (nl80211_put_sta_rate) from [<8066001c>] (nl80211_send_station+0x388/0xaf0)
> [   96.292261] [<8066001c>] (nl80211_send_station) from [<8066082c>] (nl80211_get_station+0xa8/0xec)
> [   96.304166] [<8066082c>] (nl80211_get_station) from [<80509c20>] (genl_rcv_msg+0x2dc/0x34c)
> [   96.313324] [<80509c20>] (genl_rcv_msg) from [<8050890c>] (netlink_rcv_skb+0x84/0xdc)
> [   96.321880] [<8050890c>] (netlink_rcv_skb) from [<805093c0>] (genl_rcv+0x20/0x34)
> [   96.329668] [<805093c0>] (genl_rcv) from [<80508188>] (netlink_unicast+0x12c/0x1e0)
> [   96.338408] [<80508188>] (netlink_unicast) from [<805085d8>] (netlink_sendmsg+0x2e0/0x304)
> [   96.350736] [<805085d8>] (netlink_sendmsg) from [<804b5f9c>] (sock_sendmsg+0x14/0x24)
> [   96.358656] [<804b5f9c>] (sock_sendmsg) from [<804b66e8>] (___sys_sendmsg+0x1c8/0x20c)
> [   96.367093] [<804b66e8>] (___sys_sendmsg) from [<804b739c>] (__sys_sendmsg+0x40/0x64)
> [   96.375276] [<804b739c>] (__sys_sendmsg) from [<8000a3e0>] (ret_fast_syscall+0x0/0x44)
> [   96.383455] ---[ end trace da8257d6a850e91a ]---
> 
Look for:

"[PATCH] ath10k: fix recent bandwidth conversion bug"

Re: [PATCH 2/2] ath10k: add per peer tx stats support for 10.2.4

[Maxime Bizon]

On Fri, 2017-12-01 at 16:53 +0100, Christian Lamparter wrote:

> "[PATCH] ath10k: fix recent bandwidth conversion bug"

indeed, much better

thanks

-- 
Maxime

Re: [PATCH 2/2] ath10k: add per peer tx stats support for 10.2.4

[akolli]

On 2017-12-01 20:35, Maxime Bizon wrote:
> On Fri, 2017-12-01 at 19:18 +0530, akolli@codeaurora.org wrote:
> 
>> Hope CONFIG_MAC80211_DEBUGFS is enabled in your build.
> 
> it wasn't and IMHO it's confusing because tx rate is filled by the 
> other
> drivers without it.
> 
> I now have the following warning:
> 
> [   96.174967] ------------[ cut here ]------------
> [   96.179640] WARNING: CPU: 0 PID: 609 at net/wireless/util.c:1254
> cfg80211_calculate_bitrate+0x174/0x220
> [   96.189538] invalid rate bw=1, mcs=9, nss=2
> [   96.219736] CPU: 0 PID: 609 Comm: hostapd Not tainted
> 4.14.3-00381-gec9756b0f64d #28
> [   96.227910] Hardware name: Marvell Kirkwood (Flattened Device Tree)
> [   96.235450] [<80010124>] (unwind_backtrace) from [<8000d9ec>]
> (show_stack+0x10/0x14)
> [   96.247180] [<8000d9ec>] (show_stack) from [<8002016c>] 
> (__warn+0xdc/0xf8)
> [   96.254243] [<8002016c>] (__warn) from [<800201bc>]
> (warn_slowpath_fmt+0x34/0x44)
> [   96.262016] [<800201bc>] (warn_slowpath_fmt) from [<8064dfdc>]
> (cfg80211_calculate_bitrate+0x174/0x220)
> [   96.272652] [<8064dfdc>] (cfg80211_calculate_bitrate) from
> [<806692d4>] (nl80211_put_sta_rate+0x44/0x1dc)
> [   96.282509] [<806692d4>] (nl80211_put_sta_rate) from [<8066001c>]
> (nl80211_send_station+0x388/0xaf0)
> [   96.292261] [<8066001c>] (nl80211_send_station) from [<8066082c>]
> (nl80211_get_station+0xa8/0xec)
> [   96.304166] [<8066082c>] (nl80211_get_station) from [<80509c20>]
> (genl_rcv_msg+0x2dc/0x34c)
> [   96.313324] [<80509c20>] (genl_rcv_msg) from [<8050890c>]
> (netlink_rcv_skb+0x84/0xdc)
> [   96.321880] [<8050890c>] (netlink_rcv_skb) from [<805093c0>]
> (genl_rcv+0x20/0x34)
> [   96.329668] [<805093c0>] (genl_rcv) from [<80508188>]
> (netlink_unicast+0x12c/0x1e0)
> [   96.338408] [<80508188>] (netlink_unicast) from [<805085d8>]
> (netlink_sendmsg+0x2e0/0x304)
> [   96.350736] [<805085d8>] (netlink_sendmsg) from [<804b5f9c>]
> (sock_sendmsg+0x14/0x24)
> [   96.358656] [<804b5f9c>] (sock_sendmsg) from [<804b66e8>]
> (___sys_sendmsg+0x1c8/0x20c)
> [   96.367093] [<804b66e8>] (___sys_sendmsg) from [<804b739c>]
> (__sys_sendmsg+0x40/0x64)
> [   96.375276] [<804b739c>] (__sys_sendmsg) from [<8000a3e0>]
> (ret_fast_syscall+0x0/0x44)
> [   96.383455] ---[ end trace da8257d6a850e91a ]---
> 
> # iw dev wlan1 station dump
> Station e4:42:a6:24:c8:95 (on wlan1)
> 	inactive time:	550 ms
> 	rx bytes:	41217
> 	rx packets:	152
> 	tx bytes:	49397
> 	tx packets:	107
> 	tx retries:	0
> 	tx failed:	1
> 	rx drop misc:	0
> 	signal:  	-62 [-66, -65, -83] dBm
> 	signal avg:	-61 [-65, -65, -78] dBm
> 	tx bitrate:	 VHT-MCS 9 short GI VHT-NSS 2
> 	rx bitrate:	360.0 MBit/s VHT-MCS 8 40MHz short GI VHT-NSS 2
> 	rx duration:	0 us
> 	authorized:	yes
> 	authenticated:	yes
> 	associated:	yes
> 	preamble:	long
> 	WMM/WME:	yes
> 	MFP:		no
> 	TDLS peer:	no
> 	DTIM period:	2
> 	beacon interval:96
> 	short slot time:yes
> 	connected time:	5 seconds
> 
> Thanks

Hope 10.2.4-1.0-00029 Firmware binary works for you.
I will check this warning.

Thanks,
Anil.

Re: [PATCH 2/2] ath10k: add per peer tx stats support for 10.2.4

[Maxime Bizon]

On Mon, 2017-12-04 at 18:54 +0530, akolli@codeaurora.org wrote:

> Hope 10.2.4-1.0-00029 Firmware binary works for you.

it does

> I will check this warning.

fixed by applying patch:
 "[PATCH] ath10k: fix recent bandwidth conversion bug"

as suggested by Christian Lamparter

only remaining oddity is the CONFIG_MAC80211_DEBUGFS dependency

-- 
Maxime

Re: [PATCH 2/2] ath10k: add per peer tx stats support for 10.2.4

[akolli]

On 2017-12-04 19:53, Maxime Bizon wrote:
> On Mon, 2017-12-04 at 18:54 +0530, akolli@codeaurora.org wrote:
> 
>> Hope 10.2.4-1.0-00029 Firmware binary works for you.
> 
> it does
> 
>> I will check this warning.
> 
> fixed by applying patch:
>  "[PATCH] ath10k: fix recent bandwidth conversion bug"
> 
> as suggested by Christian Lamparter
> 
> only remaining oddity is the CONFIG_MAC80211_DEBUGFS dependency

I will send a patch to remove this dependency.

Thanks,
Anil.

Re: [PATCH 2/2] ath10k: add per peer tx stats support for 10.2.4

[Kalle Valo]

akolli@codeaurora.org writes:

>>> Parse peer stats from pktlog packets and update the tx rate
>>> information
>>> per STA. This way user space can query about transmit rate with iw:
>>
>> everything works ok, ath10k_update_per_peer_tx_stats() is called and
>> ath10k_sta fields are updated correctly
>>
>> but tx bitrate is still 6 MBit/s in station dump
>>
> Hope CONFIG_MAC80211_DEBUGFS is enabled in your build.
> Please Run ping traffic and see and 'station dump'

This feature should not depend on debugfs. Why is it needed? In patch 1
you already moved pktlog_filter to struct ath10k, does it need something
else as well?

--=20
Kalle Valo=

Re: [PATCH 2/2] ath10k: add per peer tx stats support for 10.2.4

[akolli]

> 
>>>> Parse peer stats from pktlog packets and update the tx rate
>>>> information
>>>> per STA. This way user space can query about transmit rate with iw:
>>> 
>>> everything works ok, ath10k_update_per_peer_tx_stats() is called and
>>> ath10k_sta fields are updated correctly
>>> 
>>> but tx bitrate is still 6 MBit/s in station dump
>>> 
>> Hope CONFIG_MAC80211_DEBUGFS is enabled in your build.
>> Please Run ping traffic and see and 'station dump'
> 
> This feature should not depend on debugfs. Why is it needed? In patch 1
> you already moved pktlog_filter to struct ath10k, does it need 
> something
> else as well?

ATH10K updates tx rate from this callback,
.sta_statistics                 = ath10k_sta_statistics,

This callback is defined under CONFIG_MAC80211_DEBUGFS.

Thanks,
Anil.