Anilkumar Kolli [off-list ref] writes:

On 2018-08-31 19:52, Toke H=C3=B8iland-J=C3=B8rgensen wrote:

quoted

Kalle Valo [off-list ref] writes:
=20

quoted

Anilkumar Kolli [off-list ref] writes:
=20

quoted

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 <redacted>

=20
This is a patch from last March, full patch here:
=20
https://patchwork.kernel.org/patch/10267693/
=20

quoted

@@ -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;

quoted

+		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