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

2019-05-07 Thread Kalle Valo
Lei Wang  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 
> Signed-off-by: Kalle Valo 

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


___
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k


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

2019-05-07 Thread Toke Høiland-Jørgensen
le...@codeaurora.org writes:

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

___
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k


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

2019-05-07 Thread leiwa

On 2019-04-18 16:07, Toke Høiland-Jørgensen wrote:

le...@codeaurora.org writes:


On 2019-04-17 17:26, Toke Høiland-Jørgensen wrote:

Lei Wang  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

___
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k


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

2019-04-18 Thread Toke Høiland-Jørgensen
le...@codeaurora.org writes:

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

___
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k


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

2019-04-18 Thread leiwa

On 2019-04-17 17:26, Toke Høiland-Jørgensen wrote:

Lei Wang  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

___
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k


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

2019-04-17 Thread Toke Høiland-Jørgensen
Lei Wang  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

___
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k