[RFC v2] ath10k: report tx rate using ieee80211_tx_status()

[RFC v2] ath10k: report tx rate using ieee80211_tx_status()

[Anilkumar Kolli]

Mesh path metric needs txrate information from ieee80211_tx_status()
call but in ath10k there is no mechanism to report tx rate information
via ieee80211_tx_status(), the rate is only accessible via
sta_statiscs() op.

Per peer stats has tx rate info available, this patch stores per peer
last tx rate and updates the tx rate info structures in tx completition.
The rate updated in ieee80211_tx_status() is not exactly for the last
transmitted frame instead the rate is from one of the previous frames.

Per peer txrate information is updated through per peer statistics
and is available for QCA9888/QCA9984/QCA4019/QCA998X only

Tested on QCA9984 with firmware-5.bin_10.4-3.5.3-00053
Tested on QCA998X with firmware-5.bin_10.2.4-1.0-00036

Signed-off-by: Anilkumar Kolli <akolli@codeaurora.org>
---
V2:
  - Removed peer lookup to get the STA context.

 drivers/net/wireless/ath/ath10k/core.h   |    1 +
 drivers/net/wireless/ath/ath10k/htt_rx.c |  117 ++++++++++++++++++++----------
 drivers/net/wireless/ath/ath10k/mac.c    |   13 ----
 drivers/net/wireless/ath/ath10k/txrx.c   |   15 ++++
 drivers/net/wireless/ath/ath10k/wmi.h    |    1 +
 5 files changed, 95 insertions(+), 52 deletions(-)

diff --git a/drivers/net/wireless/ath/ath10k/core.h b/drivers/net/wireless/ath/ath10k/core.h
index fe6b30356d3b..d0ee02eaa04f 100644
--- a/drivers/net/wireless/ath/ath10k/core.h
+++ b/drivers/net/wireless/ath/ath10k/core.h
@@ -364,6 +364,7 @@ struct ath10k_sta {
 	u32 smps;
 	u16 peer_id;
 	struct rate_info txrate;
+	struct ieee80211_tx_info tx_info;
 
 	struct work_struct update_wk;
 	u64 rx_duration;
diff --git a/drivers/net/wireless/ath/ath10k/htt_rx.c b/drivers/net/wireless/ath/ath10k/htt_rx.c
index 6d96f9560950..bbe39c4ffbb6 100644
--- a/drivers/net/wireless/ath/ath10k/htt_rx.c
+++ b/drivers/net/wireless/ath/ath10k/htt_rx.c
@@ -2427,7 +2427,7 @@ void ath10k_htt_htc_t2h_msg_handler(struct ath10k *ar, struct sk_buff *skb)
 		dev_kfree_skb_any(skb);
 }
 
-static inline bool is_valid_legacy_rate(u8 rate)
+static inline int get_ath10k_rate_idx(u8 rate)
 {
 	static const u8 legacy_rates[] = {1, 2, 5, 11, 6, 9, 12,
 					  18, 24, 36, 48, 54};
@@ -2435,10 +2435,10 @@ static inline bool is_valid_legacy_rate(u8 rate)
 
 	for (i = 0; i < ARRAY_SIZE(legacy_rates); i++) {
 		if (rate == legacy_rates[i])
-			return true;
+			return i;
 	}
 
-	return false;
+	return -EINVAL;
 }
 
 static void
@@ -2447,59 +2447,98 @@ static inline bool is_valid_legacy_rate(u8 rate)
 				struct ath10k_per_peer_tx_stats *peer_stats)
 {
 	struct ath10k_sta *arsta = (struct ath10k_sta *)sta->drv_priv;
+	struct ieee80211_chanctx_conf *conf = NULL;
 	u8 rate = 0, sgi;
-	struct rate_info txrate;
+	struct rate_info *txrate = &arsta->txrate;
+	bool skip_auto_rate;
 
 	lockdep_assert_held(&ar->data_lock);
 
-	txrate.flags = ATH10K_HW_PREAMBLE(peer_stats->ratecode);
-	txrate.bw = ATH10K_HW_BW(peer_stats->flags);
-	txrate.nss = ATH10K_HW_NSS(peer_stats->ratecode);
-	txrate.mcs = ATH10K_HW_MCS_RATE(peer_stats->ratecode);
+	txrate->flags = ATH10K_HW_PREAMBLE(peer_stats->ratecode);
+	txrate->bw = ATH10K_HW_BW(peer_stats->flags);
+	txrate->nss = ATH10K_HW_NSS(peer_stats->ratecode);
+	txrate->mcs = ATH10K_HW_MCS_RATE(peer_stats->ratecode);
 	sgi = ATH10K_HW_GI(peer_stats->flags);
+	skip_auto_rate = ATH10K_FW_SKIPPED_RATE_CTRL(peer_stats->flags);
 
-	if (txrate.flags == WMI_RATE_PREAMBLE_VHT && txrate.mcs > 9) {
-		ath10k_warn(ar, "Invalid VHT mcs %hhd peer stats",  txrate.mcs);
+	/* Firmware's rate control skips broadcast/managment frames, if host has
+	 * configure fixed rates and in some other special cases.
+	 */
+	if (skip_auto_rate)
+		return;
+
+	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)) {
+	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);
+			    txrate->mcs, txrate->nss);
 		return;
 	}
 
-	memset(&arsta->txrate, 0, sizeof(arsta->txrate));
+	memset(&arsta->tx_info.status, 0, sizeof(arsta->tx_info.status));
 
-	if (txrate.flags == WMI_RATE_PREAMBLE_CCK ||
-	    txrate.flags == WMI_RATE_PREAMBLE_OFDM) {
+	if (txrate->flags == WMI_RATE_PREAMBLE_CCK ||
+	    txrate->flags == WMI_RATE_PREAMBLE_OFDM) {
 		rate = ATH10K_HW_LEGACY_RATE(peer_stats->ratecode);
-
-		if (!is_valid_legacy_rate(rate)) {
-			ath10k_warn(ar, "Invalid legacy rate %hhd peer stats",
-				    rate);
-			return;
-		}
-
 		/* This is hacky, FW sends CCK rate 5.5Mbps as 6 */
-		rate *= 10;
-		if (rate == 60 && txrate.flags == WMI_RATE_PREAMBLE_CCK)
-			rate = rate - 5;
-		arsta->txrate.legacy = rate;
-	} else if (txrate.flags == WMI_RATE_PREAMBLE_HT) {
-		arsta->txrate.flags = RATE_INFO_FLAGS_MCS;
-		arsta->txrate.mcs = txrate.mcs + 8 * (txrate.nss - 1);
-	} else {
-		arsta->txrate.flags = RATE_INFO_FLAGS_VHT_MCS;
-		arsta->txrate.mcs = txrate.mcs;
+		if (rate == 6 && txrate.flags == WMI_RATE_PREAMBLE_CCK)
+			rate = 5;
+		txrate->legacy = get_ath10k_rate_idx(rate);
+	}
+	txrate->bw = txrate->bw + RATE_INFO_BW_20;
+	arsta->tx_info.status.rates[0].count = 1;
+
+	switch (txrate->flags) {
+	case WMI_RATE_PREAMBLE_OFDM:
+		if (arsta->arvif && arsta->arvif->vif)
+			conf = rcu_dereference(arsta->arvif->vif->chanctx_conf);
+		if (conf && conf->def.chan->band == NL80211_BAND_5GHZ)
+			arsta->tx_info.status.rates[0].idx = txrate->legacy - 4;
+		break;
+	case WMI_RATE_PREAMBLE_CCK:
+		arsta->tx_info.status.rates[0].idx = txrate->legacy;
+		if (sgi)
+			arsta->tx_info.status.rates[0].flags |=
+				(IEEE80211_TX_RC_USE_SHORT_PREAMBLE |
+				 IEEE80211_TX_RC_SHORT_GI);
+		break;
+	case WMI_RATE_PREAMBLE_HT:
+		arsta->tx_info.status.rates[0].idx =
+				txrate->mcs + ((txrate->nss - 1) * 8);
+		if (sgi)
+			arsta->tx_info.status.rates[0].flags |=
+					IEEE80211_TX_RC_SHORT_GI;
+		arsta->tx_info.status.rates[0].flags |= IEEE80211_TX_RC_MCS;
+		break;
+	case WMI_RATE_PREAMBLE_VHT:
+		ieee80211_rate_set_vht(&arsta->tx_info.status.rates[0],
+				       txrate->mcs, txrate->nss);
+		if (sgi)
+			arsta->tx_info.status.rates[0].flags |=
+						IEEE80211_TX_RC_SHORT_GI;
+		arsta->tx_info.status.rates[0].flags |= IEEE80211_TX_RC_VHT_MCS;
+		break;
+ 	}
+ 
+	switch (txrate->bw) {
+	case RATE_INFO_BW_40:
+		arsta->tx_info.status.rates[0].flags |=
+				IEEE80211_TX_RC_40_MHZ_WIDTH;
+		break;
+	case RATE_INFO_BW_80:
+		arsta->tx_info.status.rates[0].flags |=
+				IEEE80211_TX_RC_80_MHZ_WIDTH;
+		break;
+	default:
+		break;
 	}
-
-	if (sgi)
-		arsta->txrate.flags |= RATE_INFO_FLAGS_SHORT_GI;
-
-	arsta->txrate.nss = txrate.nss;
-	arsta->txrate.bw = txrate.bw + RATE_INFO_BW_20;
+ 
+	if (peer_stats->succ_pkts)
+		arsta->tx_info.flags = IEEE80211_TX_STAT_ACK;
 }
 
 static void ath10k_htt_fetch_peer_stats(struct ath10k *ar,
diff --git a/drivers/net/wireless/ath/ath10k/mac.c b/drivers/net/wireless/ath/ath10k/mac.c
index 7e02ca02b28e..a8f7df0ae97d 100644
--- a/drivers/net/wireless/ath/ath10k/mac.c
+++ b/drivers/net/wireless/ath/ath10k/mac.c
@@ -7673,19 +7673,6 @@ static void ath10k_sta_statistics(struct ieee80211_hw *hw,
 
 	sinfo->rx_duration = arsta->rx_duration;
 	sinfo->filled |= 1ULL << NL80211_STA_INFO_RX_DURATION;
-
-	if (!arsta->txrate.legacy && !arsta->txrate.nss)
-		return;
-
-	if (arsta->txrate.legacy) {
-		sinfo->txrate.legacy = arsta->txrate.legacy;
-	} else {
-		sinfo->txrate.mcs = arsta->txrate.mcs;
-		sinfo->txrate.nss = arsta->txrate.nss;
-		sinfo->txrate.bw = arsta->txrate.bw;
-	}
-	sinfo->txrate.flags = arsta->txrate.flags;
-	sinfo->filled |= 1ULL << NL80211_STA_INFO_TX_BITRATE;
 }
 
 static const struct ieee80211_ops ath10k_ops = {
diff --git a/drivers/net/wireless/ath/ath10k/txrx.c b/drivers/net/wireless/ath/ath10k/txrx.c
index 70e23bbf7171..04ed7987d5bc 100644
--- a/drivers/net/wireless/ath/ath10k/txrx.c
+++ b/drivers/net/wireless/ath/ath10k/txrx.c
@@ -60,6 +60,8 @@ int ath10k_txrx_tx_unref(struct ath10k_htt *htt,
 	struct ath10k_skb_cb *skb_cb;
 	struct ath10k_txq *artxq;
 	struct sk_buff *msdu;
+	struct ath10k_sta *arsta;
+	struct ieee80211_sta *sta = NULL;
 
 	ath10k_dbg(ar, ATH10K_DBG_HTT,
 		   "htt tx completion msdu_id %u status %d\n",
@@ -86,6 +88,7 @@ int ath10k_txrx_tx_unref(struct ath10k_htt *htt,
 	if (txq) {
 		artxq = (void *)txq->drv_priv;
 		artxq->num_fw_queued--;
+		sta = txq->sta;
 	}
 
 	ath10k_htt_tx_free_msdu_id(htt, tx_done->msdu_id);
@@ -119,6 +122,18 @@ int ath10k_txrx_tx_unref(struct ath10k_htt *htt,
 			info->flags &= ~IEEE80211_TX_STAT_ACK;
 	}
 
+	if (sta) {
+		spin_lock_bh(&ar->data_lock);
+		arsta = (struct ath10k_sta *)sta->drv_priv;
+		info->status.rates[0].idx =
+			arsta->tx_info.status.rates[0].idx;
+		info->status.rates[0].count =
+			arsta->tx_info.status.rates[0].count;
+		info->status.rates[0].flags =
+			arsta->tx_info.status.rates[0].flags;
+		spin_unlock_bh(&ar->data_lock);
+	}
+
 	ieee80211_tx_status(htt->ar->hw, msdu);
 	/* we do not own the msdu anymore */
 
diff --git a/drivers/net/wireless/ath/ath10k/wmi.h b/drivers/net/wireless/ath/ath10k/wmi.h
index c7b30ed9015d..1bea1a2c3157 100644
--- a/drivers/net/wireless/ath/ath10k/wmi.h
+++ b/drivers/net/wireless/ath/ath10k/wmi.h
@@ -4819,6 +4819,7 @@ enum wmi_rate_preamble {
 #define ATH10K_HW_GI(flags)		(((flags) >> 5) & 0x1)
 #define ATH10K_HW_RATECODE(rate, nss, preamble) \
 	(((preamble) << 6) | ((nss) << 4) | (rate))
+#define ATH10K_FW_SKIPPED_RATE_CTRL(flags)	(((flags) >> 6) & 0x1)
 
 #define VHT_MCS_NUM     10
 #define VHT_BW_NUM      4
-- 
1.7.9.5

[RFC v2] ath10k: report tx rate using ieee80211_tx_status()

[Anilkumar Kolli]

Mesh path metric needs txrate information from ieee80211_tx_status()
call but in ath10k there is no mechanism to report tx rate information
via ieee80211_tx_status(), the rate is only accessible via
sta_statiscs() op.

Per peer stats has tx rate info available, this patch stores per peer
last tx rate and updates the tx rate info structures in tx completition.
The rate updated in ieee80211_tx_status() is not exactly for the last
transmitted frame instead the rate is from one of the previous frames.

Per peer txrate information is updated through per peer statistics
and is available for QCA9888/QCA9984/QCA4019/QCA998X only

Tested on QCA9984 with firmware-5.bin_10.4-3.5.3-00053
Tested on QCA998X with firmware-5.bin_10.2.4-1.0-00036

Signed-off-by: Anilkumar Kolli <akolli@codeaurora.org>
---
V2:
  - Removed peer lookup to get the STA context.

 drivers/net/wireless/ath/ath10k/core.h   |    1 +
 drivers/net/wireless/ath/ath10k/htt_rx.c |  117 ++++++++++++++++++++----------
 drivers/net/wireless/ath/ath10k/mac.c    |   13 ----
 drivers/net/wireless/ath/ath10k/txrx.c   |   15 ++++
 drivers/net/wireless/ath/ath10k/wmi.h    |    1 +
 5 files changed, 95 insertions(+), 52 deletions(-)

diff --git a/drivers/net/wireless/ath/ath10k/core.h b/drivers/net/wireless/ath/ath10k/core.h
index fe6b30356d3b..d0ee02eaa04f 100644
--- a/drivers/net/wireless/ath/ath10k/core.h
+++ b/drivers/net/wireless/ath/ath10k/core.h
@@ -364,6 +364,7 @@ struct ath10k_sta {
 	u32 smps;
 	u16 peer_id;
 	struct rate_info txrate;
+	struct ieee80211_tx_info tx_info;
 
 	struct work_struct update_wk;
 	u64 rx_duration;
diff --git a/drivers/net/wireless/ath/ath10k/htt_rx.c b/drivers/net/wireless/ath/ath10k/htt_rx.c
index 6d96f9560950..bbe39c4ffbb6 100644
--- a/drivers/net/wireless/ath/ath10k/htt_rx.c
+++ b/drivers/net/wireless/ath/ath10k/htt_rx.c
@@ -2427,7 +2427,7 @@ void ath10k_htt_htc_t2h_msg_handler(struct ath10k *ar, struct sk_buff *skb)
 		dev_kfree_skb_any(skb);
 }
 
-static inline bool is_valid_legacy_rate(u8 rate)
+static inline int get_ath10k_rate_idx(u8 rate)
 {
 	static const u8 legacy_rates[] = {1, 2, 5, 11, 6, 9, 12,
 					  18, 24, 36, 48, 54};
@@ -2435,10 +2435,10 @@ static inline bool is_valid_legacy_rate(u8 rate)
 
 	for (i = 0; i < ARRAY_SIZE(legacy_rates); i++) {
 		if (rate == legacy_rates[i])
-			return true;
+			return i;
 	}
 
-	return false;
+	return -EINVAL;
 }
 
 static void
@@ -2447,59 +2447,98 @@ static inline bool is_valid_legacy_rate(u8 rate)
 				struct ath10k_per_peer_tx_stats *peer_stats)
 {
 	struct ath10k_sta *arsta = (struct ath10k_sta *)sta->drv_priv;
+	struct ieee80211_chanctx_conf *conf = NULL;
 	u8 rate = 0, sgi;
-	struct rate_info txrate;
+	struct rate_info *txrate = &arsta->txrate;
+	bool skip_auto_rate;
 
 	lockdep_assert_held(&ar->data_lock);
 
-	txrate.flags = ATH10K_HW_PREAMBLE(peer_stats->ratecode);
-	txrate.bw = ATH10K_HW_BW(peer_stats->flags);
-	txrate.nss = ATH10K_HW_NSS(peer_stats->ratecode);
-	txrate.mcs = ATH10K_HW_MCS_RATE(peer_stats->ratecode);
+	txrate->flags = ATH10K_HW_PREAMBLE(peer_stats->ratecode);
+	txrate->bw = ATH10K_HW_BW(peer_stats->flags);
+	txrate->nss = ATH10K_HW_NSS(peer_stats->ratecode);
+	txrate->mcs = ATH10K_HW_MCS_RATE(peer_stats->ratecode);
 	sgi = ATH10K_HW_GI(peer_stats->flags);
+	skip_auto_rate = ATH10K_FW_SKIPPED_RATE_CTRL(peer_stats->flags);
 
-	if (txrate.flags == WMI_RATE_PREAMBLE_VHT && txrate.mcs > 9) {
-		ath10k_warn(ar, "Invalid VHT mcs %hhd peer stats",  txrate.mcs);
+	/* Firmware's rate control skips broadcast/managment frames, if host has
+	 * configure fixed rates and in some other special cases.
+	 */
+	if (skip_auto_rate)
+		return;
+
+	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)) {
+	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);
+			    txrate->mcs, txrate->nss);
 		return;
 	}
 
-	memset(&arsta->txrate, 0, sizeof(arsta->txrate));
+	memset(&arsta->tx_info.status, 0, sizeof(arsta->tx_info.status));
 
-	if (txrate.flags == WMI_RATE_PREAMBLE_CCK ||
-	    txrate.flags == WMI_RATE_PREAMBLE_OFDM) {
+	if (txrate->flags == WMI_RATE_PREAMBLE_CCK ||
+	    txrate->flags == WMI_RATE_PREAMBLE_OFDM) {
 		rate = ATH10K_HW_LEGACY_RATE(peer_stats->ratecode);
-
-		if (!is_valid_legacy_rate(rate)) {
-			ath10k_warn(ar, "Invalid legacy rate %hhd peer stats",
-				    rate);
-			return;
-		}
-
 		/* This is hacky, FW sends CCK rate 5.5Mbps as 6 */
-		rate *= 10;
-		if (rate == 60 && txrate.flags == WMI_RATE_PREAMBLE_CCK)
-			rate = rate - 5;
-		arsta->txrate.legacy = rate;
-	} else if (txrate.flags == WMI_RATE_PREAMBLE_HT) {
-		arsta->txrate.flags = RATE_INFO_FLAGS_MCS;
-		arsta->txrate.mcs = txrate.mcs + 8 * (txrate.nss - 1);
-	} else {
-		arsta->txrate.flags = RATE_INFO_FLAGS_VHT_MCS;
-		arsta->txrate.mcs = txrate.mcs;
+		if (rate == 6 && txrate.flags == WMI_RATE_PREAMBLE_CCK)
+			rate = 5;
+		txrate->legacy = get_ath10k_rate_idx(rate);
+	}
+	txrate->bw = txrate->bw + RATE_INFO_BW_20;
+	arsta->tx_info.status.rates[0].count = 1;
+
+	switch (txrate->flags) {
+	case WMI_RATE_PREAMBLE_OFDM:
+		if (arsta->arvif && arsta->arvif->vif)
+			conf = rcu_dereference(arsta->arvif->vif->chanctx_conf);
+		if (conf && conf->def.chan->band == NL80211_BAND_5GHZ)
+			arsta->tx_info.status.rates[0].idx = txrate->legacy - 4;
+		break;
+	case WMI_RATE_PREAMBLE_CCK:
+		arsta->tx_info.status.rates[0].idx = txrate->legacy;
+		if (sgi)
+			arsta->tx_info.status.rates[0].flags |=
+				(IEEE80211_TX_RC_USE_SHORT_PREAMBLE |
+				 IEEE80211_TX_RC_SHORT_GI);
+		break;
+	case WMI_RATE_PREAMBLE_HT:
+		arsta->tx_info.status.rates[0].idx =
+				txrate->mcs + ((txrate->nss - 1) * 8);
+		if (sgi)
+			arsta->tx_info.status.rates[0].flags |=
+					IEEE80211_TX_RC_SHORT_GI;
+		arsta->tx_info.status.rates[0].flags |= IEEE80211_TX_RC_MCS;
+		break;
+	case WMI_RATE_PREAMBLE_VHT:
+		ieee80211_rate_set_vht(&arsta->tx_info.status.rates[0],
+				       txrate->mcs, txrate->nss);
+		if (sgi)
+			arsta->tx_info.status.rates[0].flags |=
+						IEEE80211_TX_RC_SHORT_GI;
+		arsta->tx_info.status.rates[0].flags |= IEEE80211_TX_RC_VHT_MCS;
+		break;
+ 	}
+ 
+	switch (txrate->bw) {
+	case RATE_INFO_BW_40:
+		arsta->tx_info.status.rates[0].flags |=
+				IEEE80211_TX_RC_40_MHZ_WIDTH;
+		break;
+	case RATE_INFO_BW_80:
+		arsta->tx_info.status.rates[0].flags |=
+				IEEE80211_TX_RC_80_MHZ_WIDTH;
+		break;
+	default:
+		break;
 	}
-
-	if (sgi)
-		arsta->txrate.flags |= RATE_INFO_FLAGS_SHORT_GI;
-
-	arsta->txrate.nss = txrate.nss;
-	arsta->txrate.bw = txrate.bw + RATE_INFO_BW_20;
+ 
+	if (peer_stats->succ_pkts)
+		arsta->tx_info.flags = IEEE80211_TX_STAT_ACK;
 }
 
 static void ath10k_htt_fetch_peer_stats(struct ath10k *ar,
diff --git a/drivers/net/wireless/ath/ath10k/mac.c b/drivers/net/wireless/ath/ath10k/mac.c
index 7e02ca02b28e..a8f7df0ae97d 100644
--- a/drivers/net/wireless/ath/ath10k/mac.c
+++ b/drivers/net/wireless/ath/ath10k/mac.c
@@ -7673,19 +7673,6 @@ static void ath10k_sta_statistics(struct ieee80211_hw *hw,
 
 	sinfo->rx_duration = arsta->rx_duration;
 	sinfo->filled |= 1ULL << NL80211_STA_INFO_RX_DURATION;
-
-	if (!arsta->txrate.legacy && !arsta->txrate.nss)
-		return;
-
-	if (arsta->txrate.legacy) {
-		sinfo->txrate.legacy = arsta->txrate.legacy;
-	} else {
-		sinfo->txrate.mcs = arsta->txrate.mcs;
-		sinfo->txrate.nss = arsta->txrate.nss;
-		sinfo->txrate.bw = arsta->txrate.bw;
-	}
-	sinfo->txrate.flags = arsta->txrate.flags;
-	sinfo->filled |= 1ULL << NL80211_STA_INFO_TX_BITRATE;
 }
 
 static const struct ieee80211_ops ath10k_ops = {
diff --git a/drivers/net/wireless/ath/ath10k/txrx.c b/drivers/net/wireless/ath/ath10k/txrx.c
index 70e23bbf7171..04ed7987d5bc 100644
--- a/drivers/net/wireless/ath/ath10k/txrx.c
+++ b/drivers/net/wireless/ath/ath10k/txrx.c
@@ -60,6 +60,8 @@ int ath10k_txrx_tx_unref(struct ath10k_htt *htt,
 	struct ath10k_skb_cb *skb_cb;
 	struct ath10k_txq *artxq;
 	struct sk_buff *msdu;
+	struct ath10k_sta *arsta;
+	struct ieee80211_sta *sta = NULL;
 
 	ath10k_dbg(ar, ATH10K_DBG_HTT,
 		   "htt tx completion msdu_id %u status %d\n",
@@ -86,6 +88,7 @@ int ath10k_txrx_tx_unref(struct ath10k_htt *htt,
 	if (txq) {
 		artxq = (void *)txq->drv_priv;
 		artxq->num_fw_queued--;
+		sta = txq->sta;
 	}
 
 	ath10k_htt_tx_free_msdu_id(htt, tx_done->msdu_id);
@@ -119,6 +122,18 @@ int ath10k_txrx_tx_unref(struct ath10k_htt *htt,
 			info->flags &= ~IEEE80211_TX_STAT_ACK;
 	}
 
+	if (sta) {
+		spin_lock_bh(&ar->data_lock);
+		arsta = (struct ath10k_sta *)sta->drv_priv;
+		info->status.rates[0].idx =
+			arsta->tx_info.status.rates[0].idx;
+		info->status.rates[0].count =
+			arsta->tx_info.status.rates[0].count;
+		info->status.rates[0].flags =
+			arsta->tx_info.status.rates[0].flags;
+		spin_unlock_bh(&ar->data_lock);
+	}
+
 	ieee80211_tx_status(htt->ar->hw, msdu);
 	/* we do not own the msdu anymore */
 
diff --git a/drivers/net/wireless/ath/ath10k/wmi.h b/drivers/net/wireless/ath/ath10k/wmi.h
index c7b30ed9015d..1bea1a2c3157 100644
--- a/drivers/net/wireless/ath/ath10k/wmi.h
+++ b/drivers/net/wireless/ath/ath10k/wmi.h
@@ -4819,6 +4819,7 @@ enum wmi_rate_preamble {
 #define ATH10K_HW_GI(flags)		(((flags) >> 5) & 0x1)
 #define ATH10K_HW_RATECODE(rate, nss, preamble) \
 	(((preamble) << 6) | ((nss) << 4) | (rate))
+#define ATH10K_FW_SKIPPED_RATE_CTRL(flags)	(((flags) >> 6) & 0x1)
 
 #define VHT_MCS_NUM     10
 #define VHT_BW_NUM      4
-- 
1.7.9.5


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

Re: [RFC v2] ath10k: report tx rate using ieee80211_tx_status()

[Kalle Valo]

Anilkumar Kolli <akolli@codeaurora.org> writes:

> Mesh path metric needs txrate information from ieee80211_tx_status()
> call but in ath10k there is no mechanism to report tx rate information
> via ieee80211_tx_status(), the rate is only accessible via
> sta_statiscs() op.
>
> Per peer stats has tx rate info available, this patch stores per peer
> last tx rate and updates the tx rate info structures in tx completition.
> The rate updated in ieee80211_tx_status() is not exactly for the last
> transmitted frame instead the rate is from one of the previous frames.
>
> Per peer txrate information is updated through per peer statistics
> and is available for QCA9888/QCA9984/QCA4019/QCA998X only
>
> Tested on QCA9984 with firmware-5.bin_10.4-3.5.3-00053
> Tested on QCA998X with firmware-5.bin_10.2.4-1.0-00036
>
> Signed-off-by: Anilkumar Kolli <akolli@codeaurora.org>

BTW, your name in patchwork is "akolli@codeaurora.org" but it should be
"Anilkumar Kolli" and that's why my script uses the wrong name. Please
fix it by registering to patchwork as it's possible to change your name
during registration, but only one time. If that doesn't work then send a
request to helpdesk@kernel.org and the admins will fix it.

-- 
Kalle Valo

Re: [RFC v2] ath10k: report tx rate using ieee80211_tx_status()

[Kalle Valo]

Anilkumar Kolli <akolli@codeaurora.org> writes:

> Mesh path metric needs txrate information from ieee80211_tx_status()
> call but in ath10k there is no mechanism to report tx rate information
> via ieee80211_tx_status(), the rate is only accessible via
> sta_statiscs() op.
>
> Per peer stats has tx rate info available, this patch stores per peer
> last tx rate and updates the tx rate info structures in tx completition.
> The rate updated in ieee80211_tx_status() is not exactly for the last
> transmitted frame instead the rate is from one of the previous frames.
>
> Per peer txrate information is updated through per peer statistics
> and is available for QCA9888/QCA9984/QCA4019/QCA998X only
>
> Tested on QCA9984 with firmware-5.bin_10.4-3.5.3-00053
> Tested on QCA998X with firmware-5.bin_10.2.4-1.0-00036
>
> Signed-off-by: Anilkumar Kolli <akolli@codeaurora.org>

BTW, your name in patchwork is "akolli@codeaurora.org" but it should be
"Anilkumar Kolli" and that's why my script uses the wrong name. Please
fix it by registering to patchwork as it's possible to change your name
during registration, but only one time. If that doesn't work then send a
request to helpdesk@kernel.org and the admins will fix it.

-- 
Kalle Valo

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

Re: [RFC v2] ath10k: report tx rate using ieee80211_tx_status()

[Anilkumar Kolli]

On 2018-04-24 13:32, Kalle Valo wrote:
> Anilkumar Kolli <akolli@codeaurora.org> writes:
> 
>> Mesh path metric needs txrate information from ieee80211_tx_status()
>> call but in ath10k there is no mechanism to report tx rate information
>> via ieee80211_tx_status(), the rate is only accessible via
>> sta_statiscs() op.
>> 
>> Per peer stats has tx rate info available, this patch stores per peer
>> last tx rate and updates the tx rate info structures in tx 
>> completition.
>> The rate updated in ieee80211_tx_status() is not exactly for the last
>> transmitted frame instead the rate is from one of the previous frames.
>> 
>> Per peer txrate information is updated through per peer statistics
>> and is available for QCA9888/QCA9984/QCA4019/QCA998X only
>> 
>> Tested on QCA9984 with firmware-5.bin_10.4-3.5.3-00053
>> Tested on QCA998X with firmware-5.bin_10.2.4-1.0-00036
>> 
>> Signed-off-by: Anilkumar Kolli <akolli@codeaurora.org>
> 
> BTW, your name in patchwork is "akolli@codeaurora.org" but it should be
> "Anilkumar Kolli" and that's why my script uses the wrong name. Please
> fix it by registering to patchwork as it's possible to change your name
> during registration, but only one time. If that doesn't work then send 
> a
> request to helpdesk@kernel.org and the admins will fix it.

I registered and updated name in the patchwork website.
Do I need to resend the patch ?


Thanks
Anil.

Re: [RFC v2] ath10k: report tx rate using ieee80211_tx_status()

[Anilkumar Kolli]

On 2018-04-24 13:32, Kalle Valo wrote:
> Anilkumar Kolli <akolli@codeaurora.org> writes:
> 
>> Mesh path metric needs txrate information from ieee80211_tx_status()
>> call but in ath10k there is no mechanism to report tx rate information
>> via ieee80211_tx_status(), the rate is only accessible via
>> sta_statiscs() op.
>> 
>> Per peer stats has tx rate info available, this patch stores per peer
>> last tx rate and updates the tx rate info structures in tx 
>> completition.
>> The rate updated in ieee80211_tx_status() is not exactly for the last
>> transmitted frame instead the rate is from one of the previous frames.
>> 
>> Per peer txrate information is updated through per peer statistics
>> and is available for QCA9888/QCA9984/QCA4019/QCA998X only
>> 
>> Tested on QCA9984 with firmware-5.bin_10.4-3.5.3-00053
>> Tested on QCA998X with firmware-5.bin_10.2.4-1.0-00036
>> 
>> Signed-off-by: Anilkumar Kolli <akolli@codeaurora.org>
> 
> BTW, your name in patchwork is "akolli@codeaurora.org" but it should be
> "Anilkumar Kolli" and that's why my script uses the wrong name. Please
> fix it by registering to patchwork as it's possible to change your name
> during registration, but only one time. If that doesn't work then send 
> a
> request to helpdesk@kernel.org and the admins will fix it.

I registered and updated name in the patchwork website.
Do I need to resend the patch ?


Thanks
Anil.

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

Re: [RFC v2] ath10k: report tx rate using ieee80211_tx_status()

[Kalle Valo]

Anilkumar Kolli <akolli@codeaurora.org> writes:

> On 2018-04-24 13:32, Kalle Valo wrote:
>> Anilkumar Kolli <akolli@codeaurora.org> writes:
>>
>>> Mesh path metric needs txrate information from ieee80211_tx_status()
>>> call but in ath10k there is no mechanism to report tx rate information
>>> via ieee80211_tx_status(), the rate is only accessible via
>>> sta_statiscs() op.
>>>
>>> Per peer stats has tx rate info available, this patch stores per peer
>>> last tx rate and updates the tx rate info structures in tx
>>> completition.
>>> The rate updated in ieee80211_tx_status() is not exactly for the last
>>> transmitted frame instead the rate is from one of the previous frames.
>>>
>>> Per peer txrate information is updated through per peer statistics
>>> and is available for QCA9888/QCA9984/QCA4019/QCA998X only
>>>
>>> Tested on QCA9984 with firmware-5.bin_10.4-3.5.3-00053
>>> Tested on QCA998X with firmware-5.bin_10.2.4-1.0-00036
>>>
>>> Signed-off-by: Anilkumar Kolli <akolli@codeaurora.org>
>>
>> BTW, your name in patchwork is "akolli@codeaurora.org" but it should be
>> "Anilkumar Kolli" and that's why my script uses the wrong name. Please
>> fix it by registering to patchwork as it's possible to change your name
>> during registration, but only one time. If that doesn't work then
>> send a
>> request to helpdesk@kernel.org and the admins will fix it.
>
> I registered and updated name in the patchwork website.

It seems that your name still is "akolli@codeaurora.org" in the
Submitter field:

https://patchwork.kernel.org/patch/10353959/

An example how it should look like:

https://patchwork.kernel.org/patch/10359823/

I think you need to contact helpdesk now and ask them to fix it.

> Do I need to resend the patch ?

No need to resend because of this.

-- 
Kalle Valo

Re: [RFC v2] ath10k: report tx rate using ieee80211_tx_status()

[Kalle Valo]

Anilkumar Kolli <akolli@codeaurora.org> writes:

> On 2018-04-24 13:32, Kalle Valo wrote:
>> Anilkumar Kolli <akolli@codeaurora.org> writes:
>>
>>> Mesh path metric needs txrate information from ieee80211_tx_status()
>>> call but in ath10k there is no mechanism to report tx rate information
>>> via ieee80211_tx_status(), the rate is only accessible via
>>> sta_statiscs() op.
>>>
>>> Per peer stats has tx rate info available, this patch stores per peer
>>> last tx rate and updates the tx rate info structures in tx
>>> completition.
>>> The rate updated in ieee80211_tx_status() is not exactly for the last
>>> transmitted frame instead the rate is from one of the previous frames.
>>>
>>> Per peer txrate information is updated through per peer statistics
>>> and is available for QCA9888/QCA9984/QCA4019/QCA998X only
>>>
>>> Tested on QCA9984 with firmware-5.bin_10.4-3.5.3-00053
>>> Tested on QCA998X with firmware-5.bin_10.2.4-1.0-00036
>>>
>>> Signed-off-by: Anilkumar Kolli <akolli@codeaurora.org>
>>
>> BTW, your name in patchwork is "akolli@codeaurora.org" but it should be
>> "Anilkumar Kolli" and that's why my script uses the wrong name. Please
>> fix it by registering to patchwork as it's possible to change your name
>> during registration, but only one time. If that doesn't work then
>> send a
>> request to helpdesk@kernel.org and the admins will fix it.
>
> I registered and updated name in the patchwork website.

It seems that your name still is "akolli@codeaurora.org" in the
Submitter field:

https://patchwork.kernel.org/patch/10353959/

An example how it should look like:

https://patchwork.kernel.org/patch/10359823/

I think you need to contact helpdesk now and ask them to fix it.

> Do I need to resend the patch ?

No need to resend because of this.

-- 
Kalle Valo

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

Re: [RFC v2] ath10k: report tx rate using ieee80211_tx_status()

[Anilkumar Kolli]

On 2018-04-25 19:33, Kalle Valo wrote:
> Anilkumar Kolli <akolli@codeaurora.org> writes:
> 
>> On 2018-04-24 13:32, Kalle Valo wrote:
>>> Anilkumar Kolli <akolli@codeaurora.org> writes:
>>> 
>>>> Mesh path metric needs txrate information from ieee80211_tx_status()
>>>> call but in ath10k there is no mechanism to report tx rate 
>>>> information
>>>> via ieee80211_tx_status(), the rate is only accessible via
>>>> sta_statiscs() op.
>>>> 
>>>> Per peer stats has tx rate info available, this patch stores per 
>>>> peer
>>>> last tx rate and updates the tx rate info structures in tx
>>>> completition.
>>>> The rate updated in ieee80211_tx_status() is not exactly for the 
>>>> last
>>>> transmitted frame instead the rate is from one of the previous 
>>>> frames.
>>>> 
>>>> Per peer txrate information is updated through per peer statistics
>>>> and is available for QCA9888/QCA9984/QCA4019/QCA998X only
>>>> 
>>>> Tested on QCA9984 with firmware-5.bin_10.4-3.5.3-00053
>>>> Tested on QCA998X with firmware-5.bin_10.2.4-1.0-00036
>>>> 
>>>> Signed-off-by: Anilkumar Kolli <akolli@codeaurora.org>
>>> 
>>> BTW, your name in patchwork is "akolli@codeaurora.org" but it should 
>>> be
>>> "Anilkumar Kolli" and that's why my script uses the wrong name. 
>>> Please
>>> fix it by registering to patchwork as it's possible to change your 
>>> name
>>> during registration, but only one time. If that doesn't work then
>>> send a
>>> request to helpdesk@kernel.org and the admins will fix it.
>> 
>> I registered and updated name in the patchwork website.
> 
> It seems that your name still is "akolli@codeaurora.org" in the
> Submitter field:
> 
> https://patchwork.kernel.org/patch/10353959/
> 
> An example how it should look like:
> 
> https://patchwork.kernel.org/patch/10359823/
> 
> I think you need to contact helpdesk now and ask them to fix it.
> 
>> Do I need to resend the patch ?
> 
> No need to resend because of this.

Hi Kalle,

Do you have any comments on this RFC. Shall I submit as a patch ?

Thanks
Anil.

Re: [RFC v2] ath10k: report tx rate using ieee80211_tx_status()

[Anilkumar Kolli]

On 2018-04-25 19:33, Kalle Valo wrote:
> Anilkumar Kolli <akolli@codeaurora.org> writes:
> 
>> On 2018-04-24 13:32, Kalle Valo wrote:
>>> Anilkumar Kolli <akolli@codeaurora.org> writes:
>>> 
>>>> Mesh path metric needs txrate information from ieee80211_tx_status()
>>>> call but in ath10k there is no mechanism to report tx rate 
>>>> information
>>>> via ieee80211_tx_status(), the rate is only accessible via
>>>> sta_statiscs() op.
>>>> 
>>>> Per peer stats has tx rate info available, this patch stores per 
>>>> peer
>>>> last tx rate and updates the tx rate info structures in tx
>>>> completition.
>>>> The rate updated in ieee80211_tx_status() is not exactly for the 
>>>> last
>>>> transmitted frame instead the rate is from one of the previous 
>>>> frames.
>>>> 
>>>> Per peer txrate information is updated through per peer statistics
>>>> and is available for QCA9888/QCA9984/QCA4019/QCA998X only
>>>> 
>>>> Tested on QCA9984 with firmware-5.bin_10.4-3.5.3-00053
>>>> Tested on QCA998X with firmware-5.bin_10.2.4-1.0-00036
>>>> 
>>>> Signed-off-by: Anilkumar Kolli <akolli@codeaurora.org>
>>> 
>>> BTW, your name in patchwork is "akolli@codeaurora.org" but it should 
>>> be
>>> "Anilkumar Kolli" and that's why my script uses the wrong name. 
>>> Please
>>> fix it by registering to patchwork as it's possible to change your 
>>> name
>>> during registration, but only one time. If that doesn't work then
>>> send a
>>> request to helpdesk@kernel.org and the admins will fix it.
>> 
>> I registered and updated name in the patchwork website.
> 
> It seems that your name still is "akolli@codeaurora.org" in the
> Submitter field:
> 
> https://patchwork.kernel.org/patch/10353959/
> 
> An example how it should look like:
> 
> https://patchwork.kernel.org/patch/10359823/
> 
> I think you need to contact helpdesk now and ask them to fix it.
> 
>> Do I need to resend the patch ?
> 
> No need to resend because of this.

Hi Kalle,

Do you have any comments on this RFC. Shall I submit as a patch ?

Thanks
Anil.

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

Re: [RFC v2] ath10k: report tx rate using ieee80211_tx_status()

[Kalle Valo]

Anilkumar Kolli <akolli@codeaurora.org> writes:

> On 2018-04-25 19:33, Kalle Valo wrote:
>> Anilkumar Kolli <akolli@codeaurora.org> writes:
>>
>>> On 2018-04-24 13:32, Kalle Valo wrote:
>>>> Anilkumar Kolli <akolli@codeaurora.org> writes:
>>>>
>>>>> Mesh path metric needs txrate information from ieee80211_tx_status()
>>>>> call but in ath10k there is no mechanism to report tx rate
>>>>> information
>>>>> via ieee80211_tx_status(), the rate is only accessible via
>>>>> sta_statiscs() op.
>>>>>
>>>>> Per peer stats has tx rate info available, this patch stores per
>>>>> peer
>>>>> last tx rate and updates the tx rate info structures in tx
>>>>> completition.
>>>>> The rate updated in ieee80211_tx_status() is not exactly for the
>>>>> last
>>>>> transmitted frame instead the rate is from one of the previous
>>>>> frames.
>>>>>
>>>>> Per peer txrate information is updated through per peer statistics
>>>>> and is available for QCA9888/QCA9984/QCA4019/QCA998X only
>>>>>
>>>>> Tested on QCA9984 with firmware-5.bin_10.4-3.5.3-00053
>>>>> Tested on QCA998X with firmware-5.bin_10.2.4-1.0-00036
>>>>>
>>>>> Signed-off-by: Anilkumar Kolli <akolli@codeaurora.org>
>>>>
>>>> BTW, your name in patchwork is "akolli@codeaurora.org" but it
>>>> should be
>>>> "Anilkumar Kolli" and that's why my script uses the wrong name.
>>>> Please
>>>> fix it by registering to patchwork as it's possible to change your
>>>> name
>>>> during registration, but only one time. If that doesn't work then
>>>> send a
>>>> request to helpdesk@kernel.org and the admins will fix it.
>>>
>>> I registered and updated name in the patchwork website.
>>
>> It seems that your name still is "akolli@codeaurora.org" in the
>> Submitter field:
>>
>> https://patchwork.kernel.org/patch/10353959/
>>
>> An example how it should look like:
>>
>> https://patchwork.kernel.org/patch/10359823/
>>
>> I think you need to contact helpdesk now and ask them to fix it.
>>
>>> Do I need to resend the patch ?
>>
>> No need to resend because of this.
>
> Hi Kalle,
>
> Do you have any comments on this RFC. Shall I submit as a patch ?

I have some questions about it but I haven't had a chance to look at in
detail yet, hopefully later this week.

-- 
Kalle Valo

Re: [RFC v2] ath10k: report tx rate using ieee80211_tx_status()

[Kalle Valo]

Anilkumar Kolli <akolli@codeaurora.org> writes:

> On 2018-04-25 19:33, Kalle Valo wrote:
>> Anilkumar Kolli <akolli@codeaurora.org> writes:
>>
>>> On 2018-04-24 13:32, Kalle Valo wrote:
>>>> Anilkumar Kolli <akolli@codeaurora.org> writes:
>>>>
>>>>> Mesh path metric needs txrate information from ieee80211_tx_status()
>>>>> call but in ath10k there is no mechanism to report tx rate
>>>>> information
>>>>> via ieee80211_tx_status(), the rate is only accessible via
>>>>> sta_statiscs() op.
>>>>>
>>>>> Per peer stats has tx rate info available, this patch stores per
>>>>> peer
>>>>> last tx rate and updates the tx rate info structures in tx
>>>>> completition.
>>>>> The rate updated in ieee80211_tx_status() is not exactly for the
>>>>> last
>>>>> transmitted frame instead the rate is from one of the previous
>>>>> frames.
>>>>>
>>>>> Per peer txrate information is updated through per peer statistics
>>>>> and is available for QCA9888/QCA9984/QCA4019/QCA998X only
>>>>>
>>>>> Tested on QCA9984 with firmware-5.bin_10.4-3.5.3-00053
>>>>> Tested on QCA998X with firmware-5.bin_10.2.4-1.0-00036
>>>>>
>>>>> Signed-off-by: Anilkumar Kolli <akolli@codeaurora.org>
>>>>
>>>> BTW, your name in patchwork is "akolli@codeaurora.org" but it
>>>> should be
>>>> "Anilkumar Kolli" and that's why my script uses the wrong name.
>>>> Please
>>>> fix it by registering to patchwork as it's possible to change your
>>>> name
>>>> during registration, but only one time. If that doesn't work then
>>>> send a
>>>> request to helpdesk@kernel.org and the admins will fix it.
>>>
>>> I registered and updated name in the patchwork website.
>>
>> It seems that your name still is "akolli@codeaurora.org" in the
>> Submitter field:
>>
>> https://patchwork.kernel.org/patch/10353959/
>>
>> An example how it should look like:
>>
>> https://patchwork.kernel.org/patch/10359823/
>>
>> I think you need to contact helpdesk now and ask them to fix it.
>>
>>> Do I need to resend the patch ?
>>
>> No need to resend because of this.
>
> Hi Kalle,
>
> Do you have any comments on this RFC. Shall I submit as a patch ?

I have some questions about it but I haven't had a chance to look at in
detail yet, hopefully later this week.

-- 
Kalle Valo

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

Re: [RFC v2] ath10k: report tx rate using ieee80211_tx_status()

[Kalle Valo]

Anilkumar Kolli <akolli@codeaurora.org> writes:

> Mesh path metric needs txrate information from ieee80211_tx_status()
> call but in ath10k there is no mechanism to report tx rate information
> via ieee80211_tx_status(), the rate is only accessible via
> sta_statiscs() op.
>
> Per peer stats has tx rate info available, this patch stores per peer
> last tx rate and updates the tx rate info structures in tx completition.
> The rate updated in ieee80211_tx_status() is not exactly for the last
> transmitted frame instead the rate is from one of the previous frames.
>
> Per peer txrate information is updated through per peer statistics
> and is available for QCA9888/QCA9984/QCA4019/QCA998X only
>
> Tested on QCA9984 with firmware-5.bin_10.4-3.5.3-00053
> Tested on QCA998X with firmware-5.bin_10.2.4-1.0-00036
>
> Signed-off-by: Anilkumar Kolli <akolli@codeaurora.org>

This is a patch from last March, full patch here:

https://patchwork.kernel.org/patch/10267693/

> @@ -119,6 +122,18 @@ int ath10k_txrx_tx_unref(struct ath10k_htt *htt,
>  			info->flags &= ~IEEE80211_TX_STAT_ACK;
>  	}
>  
> +	if (sta) {
> +		spin_lock_bh(&ar->data_lock);
> +		arsta = (struct ath10k_sta *)sta->drv_priv;
> +		info->status.rates[0].idx =
> +			arsta->tx_info.status.rates[0].idx;
> +		info->status.rates[0].count =
> +			arsta->tx_info.status.rates[0].count;
> +		info->status.rates[0].flags =
<> +			arsta->tx_info.status.rates[0].flags;
> +		spin_unlock_bh(&ar->data_lock);
> +	}
> +
>  	ieee80211_tx_status(htt->ar->hw, msdu);
>  	/* we do not own the msdu anymore */

"is not exactly" is IMHO an understatement. What it means that with this
patch ath10k reports the rate information from another frame instead of
the current skb, because the firmware provides the rate information "out
of band". A simple example to clarify:

   Let's say ath10k transmits frames A, B and C. Then ath10k calls
   ieee80211_tx_status() for frame C the rate information could be for
   frame A, or it could be the other around for frame A status the rate
   information is from frame C.

In other words, there's no guarantee from what frame the rate
information is from.

Too me this feels like a bad idea but I'm not familiar enough with
mac80211 to really comment on this. What kind of implications does this
have for Mesh or ATF, for example? Adding Johannes and Toke to hear
about their opinion about this.

-- 
Kalle Valo

Re: [RFC v2] ath10k: report tx rate using ieee80211_tx_status()

[Kalle Valo]

Anilkumar Kolli <akolli@codeaurora.org> writes:

> Mesh path metric needs txrate information from ieee80211_tx_status()
> call but in ath10k there is no mechanism to report tx rate information
> via ieee80211_tx_status(), the rate is only accessible via
> sta_statiscs() op.
>
> Per peer stats has tx rate info available, this patch stores per peer
> last tx rate and updates the tx rate info structures in tx completition.
> The rate updated in ieee80211_tx_status() is not exactly for the last
> transmitted frame instead the rate is from one of the previous frames.
>
> Per peer txrate information is updated through per peer statistics
> and is available for QCA9888/QCA9984/QCA4019/QCA998X only
>
> Tested on QCA9984 with firmware-5.bin_10.4-3.5.3-00053
> Tested on QCA998X with firmware-5.bin_10.2.4-1.0-00036
>
> Signed-off-by: Anilkumar Kolli <akolli@codeaurora.org>

This is a patch from last March, full patch here:

https://patchwork.kernel.org/patch/10267693/

> @@ -119,6 +122,18 @@ int ath10k_txrx_tx_unref(struct ath10k_htt *htt,
>  			info->flags &= ~IEEE80211_TX_STAT_ACK;
>  	}
>  
> +	if (sta) {
> +		spin_lock_bh(&ar->data_lock);
> +		arsta = (struct ath10k_sta *)sta->drv_priv;
> +		info->status.rates[0].idx =
> +			arsta->tx_info.status.rates[0].idx;
> +		info->status.rates[0].count =
> +			arsta->tx_info.status.rates[0].count;
> +		info->status.rates[0].flags =
<> +			arsta->tx_info.status.rates[0].flags;
> +		spin_unlock_bh(&ar->data_lock);
> +	}
> +
>  	ieee80211_tx_status(htt->ar->hw, msdu);
>  	/* we do not own the msdu anymore */

"is not exactly" is IMHO an understatement. What it means that with this
patch ath10k reports the rate information from another frame instead of
the current skb, because the firmware provides the rate information "out
of band". A simple example to clarify:

   Let's say ath10k transmits frames A, B and C. Then ath10k calls
   ieee80211_tx_status() for frame C the rate information could be for
   frame A, or it could be the other around for frame A status the rate
   information is from frame C.

In other words, there's no guarantee from what frame the rate
information is from.

Too me this feels like a bad idea but I'm not familiar enough with
mac80211 to really comment on this. What kind of implications does this
have for Mesh or ATF, for example? Adding Johannes and Toke to hear
about their opinion about this.

-- 
Kalle Valo

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

Re: [RFC v2] ath10k: report tx rate using ieee80211_tx_status()

[Toke Høiland-Jørgensen]

Kalle Valo <kvalo@codeaurora.org> writes:

> Anilkumar Kolli <akolli@codeaurora.org> writes:
>
>> Mesh path metric needs txrate information from ieee80211_tx_status()
>> call but in ath10k there is no mechanism to report tx rate information
>> via ieee80211_tx_status(), the rate is only accessible via
>> sta_statiscs() op.
>>
>> Per peer stats has tx rate info available, this patch stores per peer
>> last tx rate and updates the tx rate info structures in tx completition.
>> The rate updated in ieee80211_tx_status() is not exactly for the last
>> transmitted frame instead the rate is from one of the previous frames.
>>
>> Per peer txrate information is updated through per peer statistics
>> and is available for QCA9888/QCA9984/QCA4019/QCA998X only
>>
>> Tested on QCA9984 with firmware-5.bin_10.4-3.5.3-00053
>> Tested on QCA998X with firmware-5.bin_10.2.4-1.0-00036
>>
>> Signed-off-by: Anilkumar Kolli <akolli@codeaurora.org>
>
> This is a patch from last March, full patch here:
>
> https://patchwork.kernel.org/patch/10267693/
>
>> @@ -119,6 +122,18 @@ int ath10k_txrx_tx_unref(struct ath10k_htt *htt,
>>  			info->flags &= ~IEEE80211_TX_STAT_ACK;
>>  	}
>>  
>> +	if (sta) {
>> +		spin_lock_bh(&ar->data_lock);
>> +		arsta = (struct ath10k_sta *)sta->drv_priv;
>> +		info->status.rates[0].idx =
>> +			arsta->tx_info.status.rates[0].idx;
>> +		info->status.rates[0].count =
>> +			arsta->tx_info.status.rates[0].count;
>> +		info->status.rates[0].flags =
> <> +			arsta->tx_info.status.rates[0].flags;
>> +		spin_unlock_bh(&ar->data_lock);
>> +	}
>> +
>>  	ieee80211_tx_status(htt->ar->hw, msdu);
>>  	/* we do not own the msdu anymore */
>
> "is not exactly" is IMHO an understatement. What it means that with this
> patch ath10k reports the rate information from another frame instead of
> the current skb, because the firmware provides the rate information "out
> of band". A simple example to clarify:
>
>    Let's say ath10k transmits frames A, B and C. Then ath10k calls
>    ieee80211_tx_status() for frame C the rate information could be for
>    frame A, or it could be the other around for frame A status the rate
>    information is from frame C.
>
> In other words, there's no guarantee from what frame the rate
> information is from.
>
> Too me this feels like a bad idea but I'm not familiar enough with
> mac80211 to really comment on this. What kind of implications does this
> have for Mesh or ATF, for example? Adding Johannes and Toke to hear
> about their opinion about this.

I was under the impression that the values gathered (at least for
airtime) would be cumulative values? If it's just the airtime of a
single random frame, which is what I understand from your example, it's
not going to be terribly useful to provide ATF at least...

-Toke

Re: [RFC v2] ath10k: report tx rate using ieee80211_tx_status()

[Toke Høiland-Jørgensen]

Kalle Valo <kvalo@codeaurora.org> writes:

> Anilkumar Kolli <akolli@codeaurora.org> writes:
>
>> Mesh path metric needs txrate information from ieee80211_tx_status()
>> call but in ath10k there is no mechanism to report tx rate information
>> via ieee80211_tx_status(), the rate is only accessible via
>> sta_statiscs() op.
>>
>> Per peer stats has tx rate info available, this patch stores per peer
>> last tx rate and updates the tx rate info structures in tx completition.
>> The rate updated in ieee80211_tx_status() is not exactly for the last
>> transmitted frame instead the rate is from one of the previous frames.
>>
>> Per peer txrate information is updated through per peer statistics
>> and is available for QCA9888/QCA9984/QCA4019/QCA998X only
>>
>> Tested on QCA9984 with firmware-5.bin_10.4-3.5.3-00053
>> Tested on QCA998X with firmware-5.bin_10.2.4-1.0-00036
>>
>> Signed-off-by: Anilkumar Kolli <akolli@codeaurora.org>
>
> This is a patch from last March, full patch here:
>
> https://patchwork.kernel.org/patch/10267693/
>
>> @@ -119,6 +122,18 @@ int ath10k_txrx_tx_unref(struct ath10k_htt *htt,
>>  			info->flags &= ~IEEE80211_TX_STAT_ACK;
>>  	}
>>  
>> +	if (sta) {
>> +		spin_lock_bh(&ar->data_lock);
>> +		arsta = (struct ath10k_sta *)sta->drv_priv;
>> +		info->status.rates[0].idx =
>> +			arsta->tx_info.status.rates[0].idx;
>> +		info->status.rates[0].count =
>> +			arsta->tx_info.status.rates[0].count;
>> +		info->status.rates[0].flags =
> <> +			arsta->tx_info.status.rates[0].flags;
>> +		spin_unlock_bh(&ar->data_lock);
>> +	}
>> +
>>  	ieee80211_tx_status(htt->ar->hw, msdu);
>>  	/* we do not own the msdu anymore */
>
> "is not exactly" is IMHO an understatement. What it means that with this
> patch ath10k reports the rate information from another frame instead of
> the current skb, because the firmware provides the rate information "out
> of band". A simple example to clarify:
>
>    Let's say ath10k transmits frames A, B and C. Then ath10k calls
>    ieee80211_tx_status() for frame C the rate information could be for
>    frame A, or it could be the other around for frame A status the rate
>    information is from frame C.
>
> In other words, there's no guarantee from what frame the rate
> information is from.
>
> Too me this feels like a bad idea but I'm not familiar enough with
> mac80211 to really comment on this. What kind of implications does this
> have for Mesh or ATF, for example? Adding Johannes and Toke to hear
> about their opinion about this.

I was under the impression that the values gathered (at least for
airtime) would be cumulative values? If it's just the airtime of a
single random frame, which is what I understand from your example, it's
not going to be terribly useful to provide ATF at least...

-Toke

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

Re: [RFC v2] ath10k: report tx rate using ieee80211_tx_status()

[Anilkumar Kolli]

On 2018-08-31 19:52, Toke Høiland-Jørgensen wrote:
> Kalle Valo <kvalo@codeaurora.org> writes:
> 
>> Anilkumar Kolli <akolli@codeaurora.org> writes:
>> 
>>> Mesh path metric needs txrate information from ieee80211_tx_status()
>>> call but in ath10k there is no mechanism to report tx rate 
>>> information
>>> via ieee80211_tx_status(), the rate is only accessible via
>>> sta_statiscs() op.
>>> 
>>> Per peer stats has tx rate info available, this patch stores per peer
>>> last tx rate and updates the tx rate info structures in tx 
>>> completition.
>>> The rate updated in ieee80211_tx_status() is not exactly for the last
>>> transmitted frame instead the rate is from one of the previous 
>>> frames.
>>> 
>>> Per peer txrate information is updated through per peer statistics
>>> and is available for QCA9888/QCA9984/QCA4019/QCA998X only
>>> 
>>> Tested on QCA9984 with firmware-5.bin_10.4-3.5.3-00053
>>> Tested on QCA998X with firmware-5.bin_10.2.4-1.0-00036
>>> 
>>> Signed-off-by: Anilkumar Kolli <akolli@codeaurora.org>
>> 
>> This is a patch from last March, full patch here:
>> 
>> https://patchwork.kernel.org/patch/10267693/
>> 
>>> @@ -119,6 +122,18 @@ int ath10k_txrx_tx_unref(struct ath10k_htt *htt,
>>>  			info->flags &= ~IEEE80211_TX_STAT_ACK;
>>>  	}
>>> 
>>> +	if (sta) {
>>> +		spin_lock_bh(&ar->data_lock);
>>> +		arsta = (struct ath10k_sta *)sta->drv_priv;
>>> +		info->status.rates[0].idx =
>>> +			arsta->tx_info.status.rates[0].idx;
>>> +		info->status.rates[0].count =
>>> +			arsta->tx_info.status.rates[0].count;
>>> +		info->status.rates[0].flags =
>> <> +			arsta->tx_info.status.rates[0].flags;
>>> +		spin_unlock_bh(&ar->data_lock);
>>> +	}
>>> +
>>>  	ieee80211_tx_status(htt->ar->hw, msdu);
>>>  	/* we do not own the msdu anymore */
>> 
>> "is not exactly" is IMHO an understatement. What it means that with 
>> this
>> patch ath10k reports the rate information from another frame instead 
>> of
>> the current skb, because the firmware provides the rate information 
>> "out
>> of band". A simple example to clarify:
>> 
>>    Let's say ath10k transmits frames A, B and C. Then ath10k calls
>>    ieee80211_tx_status() for frame C the rate information could be for
>>    frame A, or it could be the other around for frame A status the 
>> rate
>>    information is from frame C.
>> 
>> In other words, there's no guarantee from what frame the rate
>> information is from.
>> 
>> Too me this feels like a bad idea but I'm not familiar enough with
>> mac80211 to really comment on this. What kind of implications does 
>> this
>> have for Mesh or ATF, for example? Adding Johannes and Toke to hear
>> about their opinion about this.
> 
> I was under the impression that the values gathered (at least for
> airtime) would be cumulative values? If it's just the airtime of a
> single random frame, which is what I understand from your example, it's
> not going to be terribly useful to provide ATF at least...
> 
> -Toke

The design:
Whenever radio transmits packet, firmware will record numbers of bytes 
sent, MSDU’s sent, TX duration, AMPDU information, ACK fail, BA fail, 
Rate at which packet is sent. This is recorded for 4 frames sent on that 
peer. Once 4 frames are sent for that peer, TX packet event is sent to 
ath10k driver with the recorded values. These rate values are updated to 
mac80211 using ieee80211_tx_status().

Agreed mac80211 is not getting the tx rate value for the last packet 
sent on the air, but this will be delivered to mac80211 at a later time 
for each transmitted packet.

Example:
Firmware TX for peer 1 -> pkt1 -> pkt2 -> pkt3 -> pkt4 -> pkt5 -> pkt6 
-> pkt7 -> pkt8 -> pkt9 -> pkt10
Driver updates                                         -> 
ieee80211_tx_status()        ->ieee80211_tx_status()

On each 4th packet sent in air, driver will update the tx rate.


Thanks
Anil.

Re: [RFC v2] ath10k: report tx rate using ieee80211_tx_status()

[Anilkumar Kolli]

On 2018-08-31 19:52, Toke Høiland-Jørgensen wrote:
> Kalle Valo <kvalo@codeaurora.org> writes:
> 
>> Anilkumar Kolli <akolli@codeaurora.org> writes:
>> 
>>> Mesh path metric needs txrate information from ieee80211_tx_status()
>>> call but in ath10k there is no mechanism to report tx rate 
>>> information
>>> via ieee80211_tx_status(), the rate is only accessible via
>>> sta_statiscs() op.
>>> 
>>> Per peer stats has tx rate info available, this patch stores per peer
>>> last tx rate and updates the tx rate info structures in tx 
>>> completition.
>>> The rate updated in ieee80211_tx_status() is not exactly for the last
>>> transmitted frame instead the rate is from one of the previous 
>>> frames.
>>> 
>>> Per peer txrate information is updated through per peer statistics
>>> and is available for QCA9888/QCA9984/QCA4019/QCA998X only
>>> 
>>> Tested on QCA9984 with firmware-5.bin_10.4-3.5.3-00053
>>> Tested on QCA998X with firmware-5.bin_10.2.4-1.0-00036
>>> 
>>> Signed-off-by: Anilkumar Kolli <akolli@codeaurora.org>
>> 
>> This is a patch from last March, full patch here:
>> 
>> https://patchwork.kernel.org/patch/10267693/
>> 
>>> @@ -119,6 +122,18 @@ int ath10k_txrx_tx_unref(struct ath10k_htt *htt,
>>>  			info->flags &= ~IEEE80211_TX_STAT_ACK;
>>>  	}
>>> 
>>> +	if (sta) {
>>> +		spin_lock_bh(&ar->data_lock);
>>> +		arsta = (struct ath10k_sta *)sta->drv_priv;
>>> +		info->status.rates[0].idx =
>>> +			arsta->tx_info.status.rates[0].idx;
>>> +		info->status.rates[0].count =
>>> +			arsta->tx_info.status.rates[0].count;
>>> +		info->status.rates[0].flags =
>> <> +			arsta->tx_info.status.rates[0].flags;
>>> +		spin_unlock_bh(&ar->data_lock);
>>> +	}
>>> +
>>>  	ieee80211_tx_status(htt->ar->hw, msdu);
>>>  	/* we do not own the msdu anymore */
>> 
>> "is not exactly" is IMHO an understatement. What it means that with 
>> this
>> patch ath10k reports the rate information from another frame instead 
>> of
>> the current skb, because the firmware provides the rate information 
>> "out
>> of band". A simple example to clarify:
>> 
>>    Let's say ath10k transmits frames A, B and C. Then ath10k calls
>>    ieee80211_tx_status() for frame C the rate information could be for
>>    frame A, or it could be the other around for frame A status the 
>> rate
>>    information is from frame C.
>> 
>> In other words, there's no guarantee from what frame the rate
>> information is from.
>> 
>> Too me this feels like a bad idea but I'm not familiar enough with
>> mac80211 to really comment on this. What kind of implications does 
>> this
>> have for Mesh or ATF, for example? Adding Johannes and Toke to hear
>> about their opinion about this.
> 
> I was under the impression that the values gathered (at least for
> airtime) would be cumulative values? If it's just the airtime of a
> single random frame, which is what I understand from your example, it's
> not going to be terribly useful to provide ATF at least...
> 
> -Toke

The design:
Whenever radio transmits packet, firmware will record numbers of bytes 
sent, MSDU’s sent, TX duration, AMPDU information, ACK fail, BA fail, 
Rate at which packet is sent. This is recorded for 4 frames sent on that 
peer. Once 4 frames are sent for that peer, TX packet event is sent to 
ath10k driver with the recorded values. These rate values are updated to 
mac80211 using ieee80211_tx_status().

Agreed mac80211 is not getting the tx rate value for the last packet 
sent on the air, but this will be delivered to mac80211 at a later time 
for each transmitted packet.

Example:
Firmware TX for peer 1 -> pkt1 -> pkt2 -> pkt3 -> pkt4 -> pkt5 -> pkt6 
-> pkt7 -> pkt8 -> pkt9 -> pkt10
Driver updates                                         -> 
ieee80211_tx_status()        ->ieee80211_tx_status()

On each 4th packet sent in air, driver will update the tx rate.


Thanks
Anil.

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

Re: [RFC v2] ath10k: report tx rate using ieee80211_tx_status()

[Toke Høiland-Jørgensen]

Anilkumar Kolli <akolli@codeaurora.org> writes:

> On 2018-08-31 19:52, Toke H=C3=B8iland-J=C3=B8rgensen wrote:
>> Kalle Valo <kvalo@codeaurora.org> writes:
>>=20
>>> Anilkumar Kolli <akolli@codeaurora.org> writes:
>>>=20
>>>> Mesh path metric needs txrate information from ieee80211_tx_status()
>>>> call but in ath10k there is no mechanism to report tx rate=20
>>>> information
>>>> via ieee80211_tx_status(), the rate is only accessible via
>>>> sta_statiscs() op.
>>>>=20
>>>> Per peer stats has tx rate info available, this patch stores per peer
>>>> last tx rate and updates the tx rate info structures in tx=20
>>>> completition.
>>>> The rate updated in ieee80211_tx_status() is not exactly for the last
>>>> transmitted frame instead the rate is from one of the previous=20
>>>> frames.
>>>>=20
>>>> Per peer txrate information is updated through per peer statistics
>>>> and is available for QCA9888/QCA9984/QCA4019/QCA998X only
>>>>=20
>>>> Tested on QCA9984 with firmware-5.bin_10.4-3.5.3-00053
>>>> Tested on QCA998X with firmware-5.bin_10.2.4-1.0-00036
>>>>=20
>>>> Signed-off-by: Anilkumar Kolli <akolli@codeaurora.org>
>>>=20
>>> This is a patch from last March, full patch here:
>>>=20
>>> https://patchwork.kernel.org/patch/10267693/
>>>=20
>>>> @@ -119,6 +122,18 @@ int ath10k_txrx_tx_unref(struct ath10k_htt *htt,
>>>>  			info->flags &=3D ~IEEE80211_TX_STAT_ACK;
>>>>  	}
>>>>=20
>>>> +	if (sta) {
>>>> +		spin_lock_bh(&ar->data_lock);
>>>> +		arsta =3D (struct ath10k_sta *)sta->drv_priv;
>>>> +		info->status.rates[0].idx =3D
>>>> +			arsta->tx_info.status.rates[0].idx;
>>>> +		info->status.rates[0].count =3D
>>>> +			arsta->tx_info.status.rates[0].count;
>>>> +		info->status.rates[0].flags =3D
>>> <> +			arsta->tx_info.status.rates[0].flags;
>>>> +		spin_unlock_bh(&ar->data_lock);
>>>> +	}
>>>> +
>>>>  	ieee80211_tx_status(htt->ar->hw, msdu);
>>>>  	/* we do not own the msdu anymore */
>>>=20
>>> "is not exactly" is IMHO an understatement. What it means that with=20
>>> this
>>> patch ath10k reports the rate information from another frame instead=20
>>> of
>>> the current skb, because the firmware provides the rate information=20
>>> "out
>>> of band". A simple example to clarify:
>>>=20
>>>    Let's say ath10k transmits frames A, B and C. Then ath10k calls
>>>    ieee80211_tx_status() for frame C the rate information could be for
>>>    frame A, or it could be the other around for frame A status the=20
>>> rate
>>>    information is from frame C.
>>>=20
>>> In other words, there's no guarantee from what frame the rate
>>> information is from.
>>>=20
>>> Too me this feels like a bad idea but I'm not familiar enough with
>>> mac80211 to really comment on this. What kind of implications does=20
>>> this
>>> have for Mesh or ATF, for example? Adding Johannes and Toke to hear
>>> about their opinion about this.
>>=20
>> I was under the impression that the values gathered (at least for
>> airtime) would be cumulative values? If it's just the airtime of a
>> single random frame, which is what I understand from your example, it's
>> not going to be terribly useful to provide ATF at least...
>>=20
>> -Toke
>
> The design:
> Whenever radio transmits packet, firmware will record numbers of bytes=20
> sent, MSDU=E2=80=99s sent, TX duration, AMPDU information, ACK fail, BA f=
ail,=20
> Rate at which packet is sent. This is recorded for 4 frames sent on that=
=20
> peer. Once 4 frames are sent for that peer, TX packet event is sent to=20
> ath10k driver with the recorded values. These rate values are updated to=
=20
> mac80211 using ieee80211_tx_status().

So the values reported are the sums for all four packets? But the latest
value for rate information?

-Toke

Re: [RFC v2] ath10k: report tx rate using ieee80211_tx_status()

[Toke Høiland-Jørgensen]

Anilkumar Kolli <akolli@codeaurora.org> writes:

> On 2018-08-31 19:52, Toke Høiland-Jørgensen wrote:
>> Kalle Valo <kvalo@codeaurora.org> writes:
>> 
>>> Anilkumar Kolli <akolli@codeaurora.org> writes:
>>> 
>>>> Mesh path metric needs txrate information from ieee80211_tx_status()
>>>> call but in ath10k there is no mechanism to report tx rate 
>>>> information
>>>> via ieee80211_tx_status(), the rate is only accessible via
>>>> sta_statiscs() op.
>>>> 
>>>> Per peer stats has tx rate info available, this patch stores per peer
>>>> last tx rate and updates the tx rate info structures in tx 
>>>> completition.
>>>> The rate updated in ieee80211_tx_status() is not exactly for the last
>>>> transmitted frame instead the rate is from one of the previous 
>>>> frames.
>>>> 
>>>> Per peer txrate information is updated through per peer statistics
>>>> and is available for QCA9888/QCA9984/QCA4019/QCA998X only
>>>> 
>>>> Tested on QCA9984 with firmware-5.bin_10.4-3.5.3-00053
>>>> Tested on QCA998X with firmware-5.bin_10.2.4-1.0-00036
>>>> 
>>>> Signed-off-by: Anilkumar Kolli <akolli@codeaurora.org>
>>> 
>>> This is a patch from last March, full patch here:
>>> 
>>> https://patchwork.kernel.org/patch/10267693/
>>> 
>>>> @@ -119,6 +122,18 @@ int ath10k_txrx_tx_unref(struct ath10k_htt *htt,
>>>>  			info->flags &= ~IEEE80211_TX_STAT_ACK;
>>>>  	}
>>>> 
>>>> +	if (sta) {
>>>> +		spin_lock_bh(&ar->data_lock);
>>>> +		arsta = (struct ath10k_sta *)sta->drv_priv;
>>>> +		info->status.rates[0].idx =
>>>> +			arsta->tx_info.status.rates[0].idx;
>>>> +		info->status.rates[0].count =
>>>> +			arsta->tx_info.status.rates[0].count;
>>>> +		info->status.rates[0].flags =
>>> <> +			arsta->tx_info.status.rates[0].flags;
>>>> +		spin_unlock_bh(&ar->data_lock);
>>>> +	}
>>>> +
>>>>  	ieee80211_tx_status(htt->ar->hw, msdu);
>>>>  	/* we do not own the msdu anymore */
>>> 
>>> "is not exactly" is IMHO an understatement. What it means that with 
>>> this
>>> patch ath10k reports the rate information from another frame instead 
>>> of
>>> the current skb, because the firmware provides the rate information 
>>> "out
>>> of band". A simple example to clarify:
>>> 
>>>    Let's say ath10k transmits frames A, B and C. Then ath10k calls
>>>    ieee80211_tx_status() for frame C the rate information could be for
>>>    frame A, or it could be the other around for frame A status the 
>>> rate
>>>    information is from frame C.
>>> 
>>> In other words, there's no guarantee from what frame the rate
>>> information is from.
>>> 
>>> Too me this feels like a bad idea but I'm not familiar enough with
>>> mac80211 to really comment on this. What kind of implications does 
>>> this
>>> have for Mesh or ATF, for example? Adding Johannes and Toke to hear
>>> about their opinion about this.
>> 
>> I was under the impression that the values gathered (at least for
>> airtime) would be cumulative values? If it's just the airtime of a
>> single random frame, which is what I understand from your example, it's
>> not going to be terribly useful to provide ATF at least...
>> 
>> -Toke
>
> The design:
> Whenever radio transmits packet, firmware will record numbers of bytes 
> sent, MSDU’s sent, TX duration, AMPDU information, ACK fail, BA fail, 
> Rate at which packet is sent. This is recorded for 4 frames sent on that 
> peer. Once 4 frames are sent for that peer, TX packet event is sent to 
> ath10k driver with the recorded values. These rate values are updated to 
> mac80211 using ieee80211_tx_status().

So the values reported are the sums for all four packets? But the latest
value for rate information?

-Toke

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

Re: [RFC v2] ath10k: report tx rate using ieee80211_tx_status()

[Johannes Berg]

On Fri, 2018-08-31 at 15:29 +0300, Kalle Valo wrote:

> Too me this feels like a bad idea but I'm not familiar enough with
> mac80211 to really comment on this. What kind of implications does this
> have for Mesh or ATF, for example? Adding Johannes and Toke to hear
> about their opinion about this.

The biggest implication is probably radiotap. Beyond that, it's using
this to report the "last rate" to nl80211, but that's not all super
useful anyway.

The retry count is also affected, and since you report "somewhat
liberally" that would lead to erroneous statistics.

I'd recommend against doing this and disentangling the necessary code in
mac80211, e.g. with ieee80211_tx_status_ext() or adding similar APIs.

johannes

Re: [RFC v2] ath10k: report tx rate using ieee80211_tx_status()

[Johannes Berg]

On Fri, 2018-08-31 at 15:29 +0300, Kalle Valo wrote:

> Too me this feels like a bad idea but I'm not familiar enough with
> mac80211 to really comment on this. What kind of implications does this
> have for Mesh or ATF, for example? Adding Johannes and Toke to hear
> about their opinion about this.

The biggest implication is probably radiotap. Beyond that, it's using
this to report the "last rate" to nl80211, but that's not all super
useful anyway.

The retry count is also affected, and since you report "somewhat
liberally" that would lead to erroneous statistics.

I'd recommend against doing this and disentangling the necessary code in
mac80211, e.g. with ieee80211_tx_status_ext() or adding similar APIs.

johannes

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

Re: [RFC v2] ath10k: report tx rate using ieee80211_tx_status()

[Anilkumar Kolli]

On 2018-09-03 15:43, Toke Høiland-Jørgensen wrote:
> Anilkumar Kolli <akolli@codeaurora.org> writes:
> 
>> On 2018-08-31 19:52, Toke Høiland-Jørgensen wrote:
>>> Kalle Valo <kvalo@codeaurora.org> writes:
>>> 
>>>> Anilkumar Kolli <akolli@codeaurora.org> writes:
>>>> 
>>>>> Mesh path metric needs txrate information from 
>>>>> ieee80211_tx_status()
>>>>> call but in ath10k there is no mechanism to report tx rate
>>>>> information
>>>>> via ieee80211_tx_status(), the rate is only accessible via
>>>>> sta_statiscs() op.
>>>>> 
>>>>> Per peer stats has tx rate info available, this patch stores per 
>>>>> peer
>>>>> last tx rate and updates the tx rate info structures in tx
>>>>> completition.
>>>>> The rate updated in ieee80211_tx_status() is not exactly for the 
>>>>> last
>>>>> transmitted frame instead the rate is from one of the previous
>>>>> frames.
>>>>> 
>>>>> Per peer txrate information is updated through per peer statistics
>>>>> and is available for QCA9888/QCA9984/QCA4019/QCA998X only
>>>>> 
>>>>> Tested on QCA9984 with firmware-5.bin_10.4-3.5.3-00053
>>>>> Tested on QCA998X with firmware-5.bin_10.2.4-1.0-00036
>>>>> 
>>>>> Signed-off-by: Anilkumar Kolli <akolli@codeaurora.org>
>>>> 
>>>> This is a patch from last March, full patch here:
>>>> 
>>>> https://patchwork.kernel.org/patch/10267693/
>>>> 
>>>>> @@ -119,6 +122,18 @@ int ath10k_txrx_tx_unref(struct ath10k_htt 
>>>>> *htt,
>>>>>  			info->flags &= ~IEEE80211_TX_STAT_ACK;
>>>>>  	}
>>>>> 
>>>>> +	if (sta) {
>>>>> +		spin_lock_bh(&ar->data_lock);
>>>>> +		arsta = (struct ath10k_sta *)sta->drv_priv;
>>>>> +		info->status.rates[0].idx =
>>>>> +			arsta->tx_info.status.rates[0].idx;
>>>>> +		info->status.rates[0].count =
>>>>> +			arsta->tx_info.status.rates[0].count;
>>>>> +		info->status.rates[0].flags =
>>>> <> +			arsta->tx_info.status.rates[0].flags;
>>>>> +		spin_unlock_bh(&ar->data_lock);
>>>>> +	}
>>>>> +
>>>>>  	ieee80211_tx_status(htt->ar->hw, msdu);
>>>>>  	/* we do not own the msdu anymore */
>>>> 
>>>> "is not exactly" is IMHO an understatement. What it means that with
>>>> this
>>>> patch ath10k reports the rate information from another frame instead
>>>> of
>>>> the current skb, because the firmware provides the rate information
>>>> "out
>>>> of band". A simple example to clarify:
>>>> 
>>>>    Let's say ath10k transmits frames A, B and C. Then ath10k calls
>>>>    ieee80211_tx_status() for frame C the rate information could be 
>>>> for
>>>>    frame A, or it could be the other around for frame A status the
>>>> rate
>>>>    information is from frame C.
>>>> 
>>>> In other words, there's no guarantee from what frame the rate
>>>> information is from.
>>>> 
>>>> Too me this feels like a bad idea but I'm not familiar enough with
>>>> mac80211 to really comment on this. What kind of implications does
>>>> this
>>>> have for Mesh or ATF, for example? Adding Johannes and Toke to hear
>>>> about their opinion about this.
>>> 
>>> I was under the impression that the values gathered (at least for
>>> airtime) would be cumulative values? If it's just the airtime of a
>>> single random frame, which is what I understand from your example, 
>>> it's
>>> not going to be terribly useful to provide ATF at least...
>>> 
>>> -Toke
>> 
>> The design:
>> Whenever radio transmits packet, firmware will record numbers of bytes
>> sent, MSDU’s sent, TX duration, AMPDU information, ACK fail, BA fail,
>> Rate at which packet is sent. This is recorded for 4 frames sent on 
>> that
>> peer. Once 4 frames are sent for that peer, TX packet event is sent to
>> ath10k driver with the recorded values. These rate values are updated 
>> to
>> mac80211 using ieee80211_tx_status().
> 
> So the values reported are the sums for all four packets? But the 
> latest
> value for rate information?
> 
> -Toke

Tx rate is updated for the 4th packet.

Thanks
Anil.

Re: [RFC v2] ath10k: report tx rate using ieee80211_tx_status()

[Anilkumar Kolli]

On 2018-09-03 15:43, Toke Høiland-Jørgensen wrote:
> Anilkumar Kolli <akolli@codeaurora.org> writes:
> 
>> On 2018-08-31 19:52, Toke Høiland-Jørgensen wrote:
>>> Kalle Valo <kvalo@codeaurora.org> writes:
>>> 
>>>> Anilkumar Kolli <akolli@codeaurora.org> writes:
>>>> 
>>>>> Mesh path metric needs txrate information from 
>>>>> ieee80211_tx_status()
>>>>> call but in ath10k there is no mechanism to report tx rate
>>>>> information
>>>>> via ieee80211_tx_status(), the rate is only accessible via
>>>>> sta_statiscs() op.
>>>>> 
>>>>> Per peer stats has tx rate info available, this patch stores per 
>>>>> peer
>>>>> last tx rate and updates the tx rate info structures in tx
>>>>> completition.
>>>>> The rate updated in ieee80211_tx_status() is not exactly for the 
>>>>> last
>>>>> transmitted frame instead the rate is from one of the previous
>>>>> frames.
>>>>> 
>>>>> Per peer txrate information is updated through per peer statistics
>>>>> and is available for QCA9888/QCA9984/QCA4019/QCA998X only
>>>>> 
>>>>> Tested on QCA9984 with firmware-5.bin_10.4-3.5.3-00053
>>>>> Tested on QCA998X with firmware-5.bin_10.2.4-1.0-00036
>>>>> 
>>>>> Signed-off-by: Anilkumar Kolli <akolli@codeaurora.org>
>>>> 
>>>> This is a patch from last March, full patch here:
>>>> 
>>>> https://patchwork.kernel.org/patch/10267693/
>>>> 
>>>>> @@ -119,6 +122,18 @@ int ath10k_txrx_tx_unref(struct ath10k_htt 
>>>>> *htt,
>>>>>  			info->flags &= ~IEEE80211_TX_STAT_ACK;
>>>>>  	}
>>>>> 
>>>>> +	if (sta) {
>>>>> +		spin_lock_bh(&ar->data_lock);
>>>>> +		arsta = (struct ath10k_sta *)sta->drv_priv;
>>>>> +		info->status.rates[0].idx =
>>>>> +			arsta->tx_info.status.rates[0].idx;
>>>>> +		info->status.rates[0].count =
>>>>> +			arsta->tx_info.status.rates[0].count;
>>>>> +		info->status.rates[0].flags =
>>>> <> +			arsta->tx_info.status.rates[0].flags;
>>>>> +		spin_unlock_bh(&ar->data_lock);
>>>>> +	}
>>>>> +
>>>>>  	ieee80211_tx_status(htt->ar->hw, msdu);
>>>>>  	/* we do not own the msdu anymore */
>>>> 
>>>> "is not exactly" is IMHO an understatement. What it means that with
>>>> this
>>>> patch ath10k reports the rate information from another frame instead
>>>> of
>>>> the current skb, because the firmware provides the rate information
>>>> "out
>>>> of band". A simple example to clarify:
>>>> 
>>>>    Let's say ath10k transmits frames A, B and C. Then ath10k calls
>>>>    ieee80211_tx_status() for frame C the rate information could be 
>>>> for
>>>>    frame A, or it could be the other around for frame A status the
>>>> rate
>>>>    information is from frame C.
>>>> 
>>>> In other words, there's no guarantee from what frame the rate
>>>> information is from.
>>>> 
>>>> Too me this feels like a bad idea but I'm not familiar enough with
>>>> mac80211 to really comment on this. What kind of implications does
>>>> this
>>>> have for Mesh or ATF, for example? Adding Johannes and Toke to hear
>>>> about their opinion about this.
>>> 
>>> I was under the impression that the values gathered (at least for
>>> airtime) would be cumulative values? If it's just the airtime of a
>>> single random frame, which is what I understand from your example, 
>>> it's
>>> not going to be terribly useful to provide ATF at least...
>>> 
>>> -Toke
>> 
>> The design:
>> Whenever radio transmits packet, firmware will record numbers of bytes
>> sent, MSDU’s sent, TX duration, AMPDU information, ACK fail, BA fail,
>> Rate at which packet is sent. This is recorded for 4 frames sent on 
>> that
>> peer. Once 4 frames are sent for that peer, TX packet event is sent to
>> ath10k driver with the recorded values. These rate values are updated 
>> to
>> mac80211 using ieee80211_tx_status().
> 
> So the values reported are the sums for all four packets? But the 
> latest
> value for rate information?
> 
> -Toke

Tx rate is updated for the 4th packet.

Thanks
Anil.

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

Re: [RFC v2] ath10k: report tx rate using ieee80211_tx_status()

[Anilkumar Kolli]

On 2018-09-03 15:48, Johannes Berg wrote:
> On Fri, 2018-08-31 at 15:29 +0300, Kalle Valo wrote:
> 
>> Too me this feels like a bad idea but I'm not familiar enough with
>> mac80211 to really comment on this. What kind of implications does 
>> this
>> have for Mesh or ATF, for example? Adding Johannes and Toke to hear
>> about their opinion about this.
> 
> The biggest implication is probably radiotap. Beyond that, it's using
> this to report the "last rate" to nl80211, but that's not all super
> useful anyway.
> 
> The retry count is also affected, and since you report "somewhat
> liberally" that would lead to erroneous statistics.
> 
> I'd recommend against doing this and disentangling the necessary code 
> in
> mac80211, e.g. with ieee80211_tx_status_ext() or adding similar APIs.
> 
> johannes

Thanks for the review.

Is this okay to implement a new API to update the tx rate alone?

Code snippet:
  /**
  * ieee80211_tx_status_rate_upd - transmit rate update callback
  *
  * This function can be used in drivers that does not have provision
  * in updating the tx rate in data path.
  *
  * @hw: the hardware the frame was transmitted by
  * @status: tx status information
  * @pubsta: the station to update the tx rate for.
  */
void ieee80211_tx_status_rate_upd(struct ieee80211_hw *hw,
				  struct ieee80211_sta *pubsta,
				  struct ieee80211_tx_info *info);


Source:
void ieee80211_tx_status_rate_upd(struct ieee80211_hw *hw,
				  struct ieee80211_sta *pubsta,
				  struct ieee80211_tx_info *info)
{
	struct ieee80211_local *local = hw_to_local(hw);
	struct ieee80211_supported_band *sband;
	int retry_count;
	int rates_idx;
	struct ieee80211_tx_status status = {
		.skb = NULL,
		.info = info,
	};

	ieee80211_tx_get_rates(hw, info, &retry_count);
	sband = hw->wiphy->bands[info->band];

	if (pubsta) {
		struct sta_info *sta;

		sta = container_of(pubsta, struct sta_info, sta);
		status.sta = &sta->sta;
		rate_control_tx_status(local, sband, &status);
		if (ieee80211_hw_check(&local->hw, HAS_RATE_CONTROL) &&
		    (rates_idx != -1))
			sta->tx_stats.last_rate = info->status.rates[rates_idx];
	}

}
EXPORT_SYMBOL(ieee80211_tx_status_rate_upd);


Thanks
Anil.

Re: [RFC v2] ath10k: report tx rate using ieee80211_tx_status()

[Anilkumar Kolli]

On 2018-09-03 15:48, Johannes Berg wrote:
> On Fri, 2018-08-31 at 15:29 +0300, Kalle Valo wrote:
> 
>> Too me this feels like a bad idea but I'm not familiar enough with
>> mac80211 to really comment on this. What kind of implications does 
>> this
>> have for Mesh or ATF, for example? Adding Johannes and Toke to hear
>> about their opinion about this.
> 
> The biggest implication is probably radiotap. Beyond that, it's using
> this to report the "last rate" to nl80211, but that's not all super
> useful anyway.
> 
> The retry count is also affected, and since you report "somewhat
> liberally" that would lead to erroneous statistics.
> 
> I'd recommend against doing this and disentangling the necessary code 
> in
> mac80211, e.g. with ieee80211_tx_status_ext() or adding similar APIs.
> 
> johannes

Thanks for the review.

Is this okay to implement a new API to update the tx rate alone?

Code snippet:
  /**
  * ieee80211_tx_status_rate_upd - transmit rate update callback
  *
  * This function can be used in drivers that does not have provision
  * in updating the tx rate in data path.
  *
  * @hw: the hardware the frame was transmitted by
  * @status: tx status information
  * @pubsta: the station to update the tx rate for.
  */
void ieee80211_tx_status_rate_upd(struct ieee80211_hw *hw,
				  struct ieee80211_sta *pubsta,
				  struct ieee80211_tx_info *info);


Source:
void ieee80211_tx_status_rate_upd(struct ieee80211_hw *hw,
				  struct ieee80211_sta *pubsta,
				  struct ieee80211_tx_info *info)
{
	struct ieee80211_local *local = hw_to_local(hw);
	struct ieee80211_supported_band *sband;
	int retry_count;
	int rates_idx;
	struct ieee80211_tx_status status = {
		.skb = NULL,
		.info = info,
	};

	ieee80211_tx_get_rates(hw, info, &retry_count);
	sband = hw->wiphy->bands[info->band];

	if (pubsta) {
		struct sta_info *sta;

		sta = container_of(pubsta, struct sta_info, sta);
		status.sta = &sta->sta;
		rate_control_tx_status(local, sband, &status);
		if (ieee80211_hw_check(&local->hw, HAS_RATE_CONTROL) &&
		    (rates_idx != -1))
			sta->tx_stats.last_rate = info->status.rates[rates_idx];
	}

}
EXPORT_SYMBOL(ieee80211_tx_status_rate_upd);


Thanks
Anil.

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