Received-SPF: pass (google.com: best guess record for domain of linux-wireless-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
From:   Toke =?utf-8?Q?H=C3=B8iland-J=C3=B8rgensen?= <toke@redhat.com>
To:     Peter Oh <peter.oh@eero.com>, Ben Greear <greearb@candelatech.com>,
        Johannes Berg <johannes@sipsolutions.net>
Cc:     linux-wireless@vger.kernel.org,
        make-wifi-fast@lists.bufferbloat.net,
        John Crispin <john@phrozen.org>,
        Lorenzo Bianconi <lorenzo@kernel.org>,
        Felix Fietkau <nbd@nbd.name>
Subject: Re: [PATCH RFC/RFT 4/4] mac80211: Apply Airtime-based Queue Limit (AQL) on packet dequeue
In-Reply-To: <f9b224a0-305f-9715-f627-7a5fae0988d3@eero.com>
References: <156889576422.191202.5906619710809654631.stgit@alrua-x1> <156889576869.191202.510507546538322707.stgit@alrua-x1> <f21b10fd-02ae-af59-8e2f-88f654627ab3@eero.com> <879913e9-4254-1381-07f6-d860fb0b8de0@candelatech.com> <f9b224a0-305f-9715-f627-7a5fae0988d3@eero.com>
Date:   Thu, 19 Sep 2019 20:03:49 +0200
Message-ID: <87k1a4duyi.fsf@toke.dk>
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: quoted-printable
Sender: linux-wireless-owner@vger.kernel.org
Precedence: bulk

Peter Oh <peter.oh@eero.com> writes:

> On 9/19/19 10:46 AM, Ben Greear wrote:
>> On 9/19/19 10:44 AM, Peter Oh wrote:
>>> On 9/19/19 5:22 AM, Toke H=C3=B8iland-J=C3=B8rgensen wrote:
>>>> From: Toke H=C3=B8iland-J=C3=B8rgensen <toke@redhat.com>
>>>>
>>>> This patch ports that idea over to mac80211. The basic idea is simple
>>>> enough: Whenever we dequeue a packet from the TXQs and send it to the
>>>> driver, we estimate its airtime usage, based on the last recorded TX=
=20
>>>> rate
>>>> of the station that packet is destined for.
>>>
>>> The way to decide the last recorded TX rate could be vary among=20
>>> drivers. In terms of ath10k driver and FW, they use 4 PPDUs to update=
=20
>>> the Tx rate. Isn't it too small sampling number to be used for AQL?
>>
>> Probably it is not exactly the last 4 either, since the report comes=20
>> back indirectly and not
>> synchronized with the tx path?
>>
> The point of my question is "the last recorded Tx raith small nte is
> derived wumber of PPDUs and if it's ok to use it for AQL calculation
> or not".

We're leaving a bit of slack in the system by limiting the buffering to
two aggregates' worth of buffering instead of just one. This is to
prevent starvation in case our estimate is off. In the other direction,
(i.e., if the rate drops suddenly), that will translate to more bloat
until the queue drains. Not much we can do about that; we can only work
with the data we have...

Still, the Google guys reported pretty good results using this method
for ath10k with their out of tree patch. So I think that in many cases,
doing this will be an improvement; obviously, it won't be perfect. But
it beats the 1000 pkt+ queue limit currently in (some versions of)
ath10k firmware.

In an ideal world, the firmware would enforce this minimum queueing
and throttle itself, but, well, sadly we don't live an ideal world...

-Toke