Re: [PATCH v2] ath10k: Implement get_expected_throughput callback

[akolli@codeaurora.org]

On 2018-03-26 12:52, Sven Eckelmann wrote:
> On Freitag, 23. März 2018 19:37:14 CEST Anilkumar Kolli wrote:
>> +static u32 ath10k_get_expected_throughput(struct ieee80211_hw *hw,
>> +                                         struct ieee80211_sta *sta)
>> +{
>> +       struct ath10k_sta *arsta = (struct ath10k_sta *)sta->drv_priv;
>> +
>> +       return ewma_sta_txrate_read(&arsta->ave_sta_txrate);
>> +}
> 
> On Freitag, 23. März 2018 19:11:48 CEST akolli@codeaurora.org wrote:
>> > Antonio and Felix, please correct me when this statement is incorrect.
>> >
>> > The expected_throughput as initially implemented for minstrel(_ht) is
>> > not
>> > about the raw physical bitrate but about the throughput which is
>> > expected for
>> > things running on top of the wifi link. See
>> > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=cca674d47e59665630f3005291b61bb883015fc5
>> > for more details
>> >
>> > when I interpret your change correctly then your it doesn't get the
>> > information about packet loss or aggregation and doesn't do anything
>> > convert
>> > from raw physical rate to something the user could get see. It will
>> > just
>> > overestimate the throughput for ath10k links and thus give wrong
>> > information
>> > to routing algorithms. This could for example cause them to prefer
>> > links over
>> > ath10k based hw when mt76 would actually provide a significant better
>> > throughput.
>> >
>> > Beside that - why is the ave_sta_txrate only filled when with new
>> > information
>> > when someone requests the current expected_throughput via
>> > get_expected_throughput. I would have expected that it is filled
>> > everytime you
>> > get new information about the current rate from the firmware
>> > (ath10k_sta_statistics).
>> >
>> Yes. ideally it should be doing the rate avg. of all the sent packets.
> 
> No, not the PHY rate average - but the "throughput avg". And the 
> "ideally"
> here sounds a little bit like in "Our medical doctor would ideally not
> decapitate each patient but we have at least an MD".
> 

The rate average and throughput are relative. no?

- Anil.