[PATCH v2] ath10k: fix different tx duration output

[PATCH v2] ath10k: fix different tx duration output

[Lei Wang]

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 <leiwa@codeaurora.org>
---
 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

Re: [PATCH v2] ath10k: fix different tx duration output

[Toke Høiland-Jørgensen]

Lei Wang <leiwa@codeaurora.org> 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

Re: [PATCH v2] ath10k: fix different tx duration output

[leiwa]

On 2019-04-17 17:26, Toke Høiland-Jørgensen wrote:
> Lei Wang <leiwa@codeaurora.org> 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

Re: [PATCH v2] ath10k: fix different tx duration output

[Toke Høiland-Jørgensen]

leiwa@codeaurora.org writes:

> On 2019-04-17 17:26, Toke Høiland-Jørgensen wrote:
>> Lei Wang <leiwa@codeaurora.org> 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

Re: [PATCH v2] ath10k: fix different tx duration output

[leiwa]

On 2019-04-18 16:07, Toke Høiland-Jørgensen wrote:
> leiwa@codeaurora.org writes:
> 
>> On 2019-04-17 17:26, Toke Høiland-Jørgensen wrote:
>>> Lei Wang <leiwa@codeaurora.org> 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

Re: [PATCH v2] ath10k: fix different tx duration output

[Toke Høiland-Jørgensen]

leiwa@codeaurora.org writes:

> On 2019-04-18 16:07, Toke Høiland-Jørgensen wrote:
>> leiwa@codeaurora.org writes:
>> 
>>> On 2019-04-17 17:26, Toke Høiland-Jørgensen wrote:
>>>> Lei Wang <leiwa@codeaurora.org> 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

Re: [PATCH v2] ath10k: fix different tx duration output

[Kalle Valo]

Lei Wang <leiwa@codeaurora.org> 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 <leiwa@codeaurora.org>
> Signed-off-by: Kalle Valo <kvalo@codeaurora.org>

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