TX duration output of tx_stats in debugfs and station dump had big
difference because they got tx duration value from different statistic
data. We should use the same statistic data.
Tested: QCA988X with firmware ver 10.2.4-1.0-00043
Signed-off-by: Lei Wang <[email protected]>
---
drivers/net/wireless/ath/ath10k/mac.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/drivers/net/wireless/ath/ath10k/mac.c b/drivers/net/wireless/ath/ath10k/mac.c
index b73c23d..5414169 100644
--- a/drivers/net/wireless/ath/ath10k/mac.c
+++ b/drivers/net/wireless/ath/ath10k/mac.c
@@ -7977,6 +7977,11 @@ static void ath10k_sta_statistics(struct ieee80211_hw *hw,
sinfo->rx_duration = arsta->rx_duration;
sinfo->filled |= BIT_ULL(NL80211_STA_INFO_RX_DURATION);
+ if (arsta->tx_stats && ath10k_debug_is_extd_tx_stats_enabled(ar)) {
+ sinfo->tx_duration = arsta->tx_stats->tx_duration;
+ sinfo->filled |= 1ULL << NL80211_STA_INFO_TX_DURATION;
+ }
+
if (!arsta->txrate.legacy && !arsta->txrate.nss)
return;
--
1.9.1
Lei Wang <[email protected]> writes:
> TX duration output of tx_stats in debugfs and station dump had big
> difference because they got tx duration value from different statistic
> data. We should use the same statistic data.
So are you sure you picked the most accurate one of the two? :)
-Toke
On 2019-04-17 17:26, Toke Høiland-Jørgensen wrote:
> Lei Wang <[email protected]> writes:
>
>> TX duration output of tx_stats in debugfs and station dump had big
>> difference because they got tx duration value from different statistic
>> data. We should use the same statistic data.
>
> So are you sure you picked the most accurate one of the two? :)
>
> -Toke
Hi Toke,
Yes.
Now for ath10k, there are two ways to get tx duration output.
One is got from tx_stats in debugfs reported by firmware. It is a total
value including all the frames which created by host and firmware sent
to the peer.
And the second is calculated from
ath10k_htt_rx_tx_compl_ind()-->ieee80211_sta_register_airtime(), here
the tx duration just includes the data frames sent from host to the
peer.
So the first value is preferable for station dump.
Thanks.
Lei
[email protected] writes:
> On 2019-04-17 17:26, Toke Høiland-Jørgensen wrote:
>> Lei Wang <[email protected]> writes:
>>
>>> TX duration output of tx_stats in debugfs and station dump had big
>>> difference because they got tx duration value from different statistic
>>> data. We should use the same statistic data.
>>
>> So are you sure you picked the most accurate one of the two? :)
>>
>> -Toke
>
> Hi Toke,
>
> Yes.
> Now for ath10k, there are two ways to get tx duration output.
> One is got from tx_stats in debugfs reported by firmware. It is a total
> value including all the frames which created by host and firmware sent
> to the peer.
> And the second is calculated from
> ath10k_htt_rx_tx_compl_ind()-->ieee80211_sta_register_airtime(), here
> the tx duration just includes the data frames sent from host to the
> peer.
So the difference is that the former includes control frames as well? Is
that the only difference? And what exactly is a "big difference" (from
the commit message)?
> So the first value is preferable for station dump.
Hmm, I'm not sure if I agree with this. I specifically added the
tx_duration to the station dump to be able to get the values used by the
airtime scheduler. This breaks with this patch.
-Toke
On 2019-04-18 16:07, Toke Høiland-Jørgensen wrote:
> [email protected] writes:
>
>> On 2019-04-17 17:26, Toke Høiland-Jørgensen wrote:
>>> Lei Wang <[email protected]> writes:
>>>
>>>> TX duration output of tx_stats in debugfs and station dump had big
>>>> difference because they got tx duration value from different
>>>> statistic
>>>> data. We should use the same statistic data.
>>>
>>> So are you sure you picked the most accurate one of the two? :)
>>>
>>> -Toke
>>
>> Hi Toke,
>>
>> Yes.
>> Now for ath10k, there are two ways to get tx duration output.
>> One is got from tx_stats in debugfs reported by firmware. It is a
>> total
>> value including all the frames which created by host and firmware sent
>> to the peer.
>> And the second is calculated from
>> ath10k_htt_rx_tx_compl_ind()-->ieee80211_sta_register_airtime(), here
>> the tx duration just includes the data frames sent from host to the
>> peer.
>
> So the difference is that the former includes control frames as well?
> Is
> that the only difference? And what exactly is a "big difference" (from
> the commit message)?
>
Yes,it adds the duration time of receiving ACK frames.
From my test,TX from AP to station with iperf UDP test in
10s,tx_stats->tx_duration:5496623us,
and another value is 3934327us.
>> So the first value is preferable for station dump.
>
> Hmm, I'm not sure if I agree with this. I specifically added the
> tx_duration to the station dump to be able to get the values used by
> the
> airtime scheduler. This breaks with this patch.
>
> -Toke
From our internal discussing, we will revert this change.
Thanks.
Lei
[email protected] writes:
> On 2019-04-18 16:07, Toke Høiland-Jørgensen wrote:
>> [email protected] writes:
>>
>>> On 2019-04-17 17:26, Toke Høiland-Jørgensen wrote:
>>>> Lei Wang <[email protected]> writes:
>>>>
>>>>> TX duration output of tx_stats in debugfs and station dump had big
>>>>> difference because they got tx duration value from different
>>>>> statistic
>>>>> data. We should use the same statistic data.
>>>>
>>>> So are you sure you picked the most accurate one of the two? :)
>>>>
>>>> -Toke
>>>
>>> Hi Toke,
>>>
>>> Yes.
>>> Now for ath10k, there are two ways to get tx duration output.
>>> One is got from tx_stats in debugfs reported by firmware. It is a
>>> total
>>> value including all the frames which created by host and firmware sent
>>> to the peer.
>>> And the second is calculated from
>>> ath10k_htt_rx_tx_compl_ind()-->ieee80211_sta_register_airtime(), here
>>> the tx duration just includes the data frames sent from host to the
>>> peer.
>>
>> So the difference is that the former includes control frames as well?
>> Is
>> that the only difference? And what exactly is a "big difference" (from
>> the commit message)?
>>
> Yes,it adds the duration time of receiving ACK frames.
> From my test,TX from AP to station with iperf UDP test in
> 10s,tx_stats->tx_duration:5496623us,
> and another value is 3934327us.
Hmm, that's quite a big difference. Is this really only ACKs, or is it
also a question of whether retries are accounted? If so, it may actually
be that what we should do is change which value is passed to
ieee80211_sta_register_airtime()?
>>> So the first value is preferable for station dump.
>>
>> Hmm, I'm not sure if I agree with this. I specifically added the
>> tx_duration to the station dump to be able to get the values used by
>> the
>> airtime scheduler. This breaks with this patch.
>>
>> -Toke
> From our internal discussing, we will revert this change.
Cool, but see above :)
-Toke
Lei Wang <[email protected]> wrote:
> TX duration output of tx_stats in debugfs and station dump had big
> difference because they got tx duration value from different statistic
> data. We should use the same statistic data.
>
> Tested: QCA988X with firmware ver 10.2.4-1.0-00043
>
> Signed-off-by: Lei Wang <[email protected]>
> Signed-off-by: Kalle Valo <[email protected]>
Dropping per discussion, please resend once everything is clarified.
Patch set to Changes Requested.
--
https://patchwork.kernel.org/patch/10904909/
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches