Thread (13 messages) 13 messages, 4 authors, 2018-09-06

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

From: Toke Høiland-Jørgensen <toke@toke.dk>
Date: 2018-08-31 18:30:20

Kalle Valo [off-list ref] writes:
Anilkumar Kolli [off-list ref] writes:
quoted
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 <redacted>
This is a patch from last March, full patch here:

https://patchwork.kernel.org/patch/10267693/
quoted
@@ -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;
quoted
+		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
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help