Re: [Make-wifi-fast] Support for airtime scheduling using ath10k

Re: [Make-wifi-fast] Support for airtime scheduling using ath10k

[Toke Høiland-Jørgensen]

Miguel Catalan Cid <miguel.catalan@i2cat.net> writes:

> Hi,
>
> we are trying to apply different airtime weights to different stations in
> order to have some prioritization among connected stations. While this is
> working pretty well with ath9k, with ath10k we always obtain a fair
> distribution of the airtime (i.e. 50%-50% in the case of two stations),
> regardless of the airtime weight specified.
>
> E.g. STA1:
> RX: 0 us
> TX: 2295610622 us
>
> *Weight: 200*Deficit: VO: 256 us VI: 256 us BE: 34 us BK: 256 us
>
> E.g. STA2:
> RX: 0 us
> TX: 162597077 us

2295610622/162597077 ~= 14

which is not *too* far from the 20/1 ratio you've configured? Does the
ratio change at all when you change the weights (i.e., if they are
equal, do you get closer to a 50/50 split?).

Do the two stations have roughly the same signal strength / rate?

> *Weight: 10*Deficit: VO: 256 us VI: 256 us BE: 9 us BK: 256 us
>
> We are using Compex WLE650V5-18A cards.
>
> So, does ath10k support airtime scheduling? In such a case, do we need
> specific Wi-Fi cards?

It should. My guess would be that maybe you're not getting enough
backpressure for the scheduler to actually enforce things correctly. You
could try to look at the TXQ output and see if you actually have any
drops ('iw dev wlan0 station dump -v' and look at the drops/marks
columns).

What kernel version are you running? If it's not new enough to have AQL,
that might help moving the backlog to where the scheduler can do more
with it.

-Toke

Re: [Make-wifi-fast] Support for airtime scheduling using ath10k

[Miguel Catalan Cid]

El mar., 23 jun. 2020 a las 11:35, Toke Høiland-Jørgensen
(<toke@redhat.com>) escribió:
>
> Miguel Catalan Cid <miguel.catalan@i2cat.net> writes:
>
> > Hi,
> >
> > we are trying to apply different airtime weights to different stations in
> > order to have some prioritization among connected stations. While this is
> > working pretty well with ath9k, with ath10k we always obtain a fair
> > distribution of the airtime (i.e. 50%-50% in the case of two stations),
> > regardless of the airtime weight specified.
> >
> > E.g. STA1:
> > RX: 0 us
> > TX: 2295610622 us
> >
> > *Weight: 200*Deficit: VO: 256 us VI: 256 us BE: 34 us BK: 256 us
> >
> > E.g. STA2:
> > RX: 0 us
> > TX: 162597077 us
>
> 2295610622/162597077 ~= 14
>
> which is not *too* far from the 20/1 ratio you've configured? Does the
> ratio change at all when you change the weights (i.e., if they are
> equal, do you get closer to a 50/50 split?).
>
> Do the two stations have roughly the same signal strength / rate?

In this case I started the STA1 a bit earlier, so it had a higher
airtime aggregate. Indeed, to compare the airtime share, I was
continuously monitoring the "airtime rate" (i.e. the difference
between Airtime(now) and Airtime (now-4s)) and the results of both
STAs were the same (i.e. 50/50 split) independently of the weight
being used. But when using ath9k the same test runs perfectly
according to the weights.

>
> > *Weight: 10*Deficit: VO: 256 us VI: 256 us BE: 9 us BK: 256 us
> >
> > We are using Compex WLE650V5-18A cards.
> >
> > So, does ath10k support airtime scheduling? In such a case, do we need
> > specific Wi-Fi cards?
>
> It should. My guess would be that maybe you're not getting enough
> backpressure for the scheduler to actually enforce things correctly. You
> could try to look at the TXQ output and see if you actually have any
> drops ('iw dev wlan0 station dump -v' and look at the drops/marks
> columns).

ok, i will check!

>
> What kernel version are you running? If it's not new enough to have AQL,
> that might help moving the backlog to where the scheduler can do more
> with it.

Kernel 5.5.5.

>
> -Toke
>

Thanks for your reply.
Miguel.
--
Miguel Catalán Cid, PhD

Mobile Wireless Internet Group (MWI)
i2CAT Foundation, Barcelona, Spain
http://www.i2cat.net/

Re: [Make-wifi-fast] Support for airtime scheduling using ath10k

[Toke Høiland-Jørgensen]

Miguel Catalan Cid <miguel.catalan@i2cat.net> writes:

> El mar., 23 jun. 2020 a las 11:35, Toke Høiland-Jørgensen
> (<toke@redhat.com>) escribió:
>>
>> Miguel Catalan Cid <miguel.catalan@i2cat.net> writes:
>>
>> > Hi,
>> >
>> > we are trying to apply different airtime weights to different stations in
>> > order to have some prioritization among connected stations. While this is
>> > working pretty well with ath9k, with ath10k we always obtain a fair
>> > distribution of the airtime (i.e. 50%-50% in the case of two stations),
>> > regardless of the airtime weight specified.
>> >
>> > E.g. STA1:
>> > RX: 0 us
>> > TX: 2295610622 us
>> >
>> > *Weight: 200*Deficit: VO: 256 us VI: 256 us BE: 34 us BK: 256 us
>> >
>> > E.g. STA2:
>> > RX: 0 us
>> > TX: 162597077 us
>>
>> 2295610622/162597077 ~= 14
>>
>> which is not *too* far from the 20/1 ratio you've configured? Does the
>> ratio change at all when you change the weights (i.e., if they are
>> equal, do you get closer to a 50/50 split?).
>>
>> Do the two stations have roughly the same signal strength / rate?
>
> In this case I started the STA1 a bit earlier, so it had a higher
> airtime aggregate. Indeed, to compare the airtime share, I was
> continuously monitoring the "airtime rate" (i.e. the difference
> between Airtime(now) and Airtime (now-4s)) and the results of both
> STAs were the same (i.e. 50/50 split) independently of the weight
> being used. But when using ath9k the same test runs perfectly
> according to the weights.
>
>>
>> > *Weight: 10*Deficit: VO: 256 us VI: 256 us BE: 9 us BK: 256 us
>> >
>> > We are using Compex WLE650V5-18A cards.
>> >
>> > So, does ath10k support airtime scheduling? In such a case, do we need
>> > specific Wi-Fi cards?
>>
>> It should. My guess would be that maybe you're not getting enough
>> backpressure for the scheduler to actually enforce things correctly. You
>> could try to look at the TXQ output and see if you actually have any
>> drops ('iw dev wlan0 station dump -v' and look at the drops/marks
>> columns).
>
> ok, i will check!

Another thing to check is the value of 'new_flows' in the TXQ output; if
that is high, it indicates that the queues run empty a lot, which can
prevent the airtime fairness scheduler from working properly.

>> What kernel version are you running? If it's not new enough to have AQL,
>> that might help moving the backlog to where the scheduler can do more
>> with it.
>
> Kernel 5.5.5.

Hmm, that should have AQL, actually.

-Toke

Re: [Make-wifi-fast] Support for airtime scheduling using ath10k

[Toke Høiland-Jørgensen]

Miguel Catalan Cid <miguel.catalan@i2cat.net> writes:

> Hi,
>
> Do we need to use a specific firmware? It's ok to use the last one from
> kvalo, or should we use the one from candelatech?

Shouldn't matter, I think; the airtime scheduler tries to do its thing
in software. It does have to work around the firmware, to a certain
extent, though, which I suspect is why it doesn't work quite so well as
on ath9k.

Incidentally, this "impedance mismatch" between scheduler and firmware
is what this patch was supposed to fix:

https://lore.kernel.org/linux-wireless/20191222172423.131033-1-toke@redhat.com/

Never did get around to respinning it, so not sure if it still applies.
If it does, you could try taking it for a spin; otherwise I can try
updating it so you can apply it and test :)

-Toke

Re: [Make-wifi-fast] Support for airtime scheduling using ath10k

[Toke Høiland-Jørgensen]

Miguel Catalan Cid <miguel.catalan@i2cat.net> writes:

> Hi,
>
> after doing some tests with different ath10k Wi-Fi cards and clients, I
> found the following behaviour when combining AQL and the airtime
> scheduler:

Thank you for taking a closer look at this!

> - When using the default AQL limits (threshold 24000, limits per AC
> 5000/12000), the airtime scheduler is not working at all, regardless of the
> airtime weights of the STAs. Indeed, in some cases, slower stations were
> able to use a higher amount of airtime, leading to unfairness. I was
> thinking that maybe the default AQL limits are too high to these slow
> stations, allowing them to obtain too much pending airtime. I already used
> the last patches from Felix Fietkau with the same results.

Hmm, I think I can see what's going on here: The AQL patches changed the
behaviour of ieee80211_next_txq() so it will keep iterating as long as
there's at least one station that's eligible for transmission from the
AQL PoV. Which means all station deficits will keep increasing, erasing
the fairness throttling...

> - Indeed, I was able to activate the airtime scheduler by fixing lower AQL
> limits (e.g. threshold of 5000, limits per AC 0/5000). This way, it seems
> that the STAs start competing again for the airtime, and their behaviour
> follows the airtime weights. However,  slower STAs lose a bit of
> performance due to these lower limits.

... and the effect of lowering the AQL parameters is that this situation
will occur less often, making the fairness scheduler work better.

> - The airtime weights have to be higher (e.g. 10000 vs 20000 to obtain a
> 33% vs 66% relation); I found the same behaviour using ath9k and 11n cards,
> so I guess this is due to the aggregation of packets.

This is a bit more puzzling, but I guess it's related to the above.
Also, for ath9k, the airtime check *always* returns true, which just
exacerbates the behaviour.

> Looking into the code, it seems that the key airtime check is the one
> in ieee80211_tx_dequeue. To enable the airtime scheduling, the
> "ieee80211_txq_airtime_check" function has to return false more usually;
> maybe it is just a matter of adjusting the AQL limits according to the
> airtime weights or to modify a bit the "ieee80211_txq_airtime_check"
> function to consider the airtime weight or the deficit of the
> stations.

Yup, you're right on point here, it's a bad interaction between those
two throttling mechanisms. The fairness mechanism relies on not
reshuffling the RR rotation.

I'll think a bit more about what we can do about this. May need to set
up a box for testing a few ideas, but I have been meaning to do that
anyway.

In the meantime, I wonder what happens if we just don't do that? I.e.,
something like the below (completely untested)?

-Toke

diff --git a/net/mac80211/tx.c b/net/mac80211/tx.c
index d2136007e2eb..3a2898cbb111 100644
--- a/net/mac80211/tx.c
+++ b/net/mac80211/tx.c
@@ -3743,8 +3743,7 @@ struct ieee80211_txq *ieee80211_next_txq(struct ieee80211_hw *hw, u8 ac)
 {
 	struct ieee80211_local *local = hw_to_local(hw);
 	struct ieee80211_txq *ret = NULL;
-	struct txq_info *txqi = NULL, *head = NULL;
-	bool found_eligible_txq = false;
+	struct txq_info *txqi = NULL;
 
 	spin_lock_bh(&local->active_txq_lock[ac]);
 
@@ -3755,34 +3754,22 @@ struct ieee80211_txq *ieee80211_next_txq(struct ieee80211_hw *hw, u8 ac)
 	if (!txqi)
 		goto out;
 
-	if (txqi == head) {
-		if (!found_eligible_txq)
-			goto out;
-		else
-			found_eligible_txq = false;
-	}
-
-	if (!head)
-		head = txqi;
-
 	if (txqi->txq.sta) {
 		struct sta_info *sta = container_of(txqi->txq.sta,
 						    struct sta_info, sta);
-		bool aql_check = ieee80211_txq_airtime_check(hw, &txqi->txq);
 		s64 deficit = sta->airtime[txqi->txq.ac].deficit;
 
-		if (aql_check)
-			found_eligible_txq = true;
-
-		if (deficit < 0)
+		if (deficit < 0) {
 			sta->airtime[txqi->txq.ac].deficit +=
 				sta->airtime_weight;
 
-		if (deficit < 0 || !aql_check) {
 			list_move_tail(&txqi->schedule_order,
 				       &local->active_txqs[txqi->txq.ac]);
 			goto begin;
 		}
+
+		if (ieee80211_txq_airtime_check(hw, &txqi->txq))
+			goto out;
 	}