Re: [PATCH 3/6] mac80211: Add airtime accounting and scheduling to TXQs

["Toke Høiland-Jørgensen" <toke@toke.dk>]

Rajkumar Manoharan <rmanohar@codeaurora.org> writes:

> On 2018-10-26 07:16, Toke Høiland-Jørgensen wrote:
>> Rajkumar Manoharan <rmanohar@codeaurora.org> writes:
>> 
>>> From: Toke Høiland-Jørgensen <toke@toke.dk>
> [...]
>>>  	u8 max_nan_de_entries;
>>>  	u8 tx_sk_pacing_shift;
>>> +	u32 airtime_weight;
>>>  };
>> 
>> This doesn't make sense. Airtime weights can be set by userspace, so
>> even if a driver sets another default it is not guaranteed to be
>> honoured. So what's the point?
>> 
> The reason for driver specific default is to avoid performance impact
> in ath10k when the user is using vanilla ath10k with default airtime.
> As I mentioned earlier, mac80211 default (256us) is too low for 11ac
> devices especially with driver is bursting aggregation.
>
> Yes. I do understand the user can change airtime at anytime but It
> must be noted that different airtime weight will result in different
> throughput. IMHO the defaults should not impact current benchmark.
> Otherwise it will be alarmed as regression later. isn't it?

My point is that if the user has to know the implementation-specific
limitations of each driver before setting a weight, then it's not a
particularly friendly API. I think we should be able to do better than
that...

>> So since we're rotating the queue on every call to the function, I'm
>> wondering if this actually succeeds in throttling the slow station's
>> airtime usage enough to achieve fairness? So I'll ask again: Did you
>> test the fairness performance, and how?
>> 
> We are collecting data of mixed clients (11ac, 11n and legacy) keeping
> them at same distance and different distance. So that lower rate
> clients will interfere higher MCS rate station. Also configuring
> different airtime weight for each stations so that throttling low rate
> clients more should help higher performing(better MCS) clients.
>
> By allowing different airtime for each stations, the user can control
> guest network over primary network. Also It helped to improve
> performance of preferred station and algo. to control station is given
> to cloud or user application.
>
> As of now, We are testing with 4 11ac clients of same distance. And
> collecting the performance data in multiple iteration. Below are
> current data of station's performance (Mbps) against airtime weight.
>
> airtime   station1(%airtime)  station2     station3       station4 
> (Mbps)
>
> No ATF      182               168          166            169
>
> 4ms         170 (100%)        164 (100%)   185  (100%)    175 (100%)
>
> 4ms         277 (70%)         115 (10%)    103 (10%)      105 (10%)
>
> 4ms         223 (40%)         214 (40%)    109 (10%)       94 (10%)
>
> 4ms         337 (90%)         182 (8%)      23 (1%)        30 (1%)

So this looks like it's doing *something*, but not like it's succeeding
in achieving the set percentages. Did you check if the actual airtime
values (in debugfs) corresponds to the configured weights?

>
>              STA1(11ac)  STA2 (11n)  STA3(11a)
>              ==========  ==========  =========
>
> No ATF       225         166         3.5
>
> ATF (4ms)    234         151         3.5

This also shows like ATF has no effect?

>> Also, taking a step back:
>> 
>> With this, we're doing lots of work to make sure that the hardware's
>> round-robin scheduling queue lines up with mac80211; so if we do this
>> anyway, why can't we just control the order directly from mac80211
>> (which is what we do with the other next_txq() API)?
>> 
> The only way to enforce mac80211 ordering in ath10k is to disable pull
> mode in firmware and always operates in push mode similar to ath9k.

And I assume that is not too likely to happen, or? What is the benefit
of pull mode at the firmware level?

-Toke