Kalle Valo <kvalo@xxxxxxxxxxxxxx> writes: > Anilkumar Kolli <akolli@xxxxxxxxxxxxxx> 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@xxxxxxxxxxxxxx> > > 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
