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:     Felix Fietkau <nbd@nbd.name>, Kan Yan <kyan@google.com>,
        johannes@sipsolutions.net
Cc:     linux-wireless@vger.kernel.org,
        make-wifi-fast@lists.bufferbloat.net, yiboz@codeaurora.org,
        john@phrozen.org, lorenzo@kernel.org, rmanohar@codeaurora.org,
        kevinhayes@google.com
Subject: Re: [PATCH v11 4/4] mac80211: Use Airtime-based Queue Limits (AQL) on packet dequeue
In-Reply-To: <829b6b28-99cd-ea9d-fea3-603a10eae401@nbd.name>
References: <20191119060610.76681-1-kyan@google.com> <20191119060610.76681-5-kyan@google.com> <789d592c-5b1b-b785-6d9c-86b7cc7d57f4@nbd.name> <87k149xbb4.fsf@toke.dk> <829b6b28-99cd-ea9d-fea3-603a10eae401@nbd.name>
Date:   Thu, 27 Feb 2020 11:07:29 +0100
Message-ID: <875zfsxs0u.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

Felix Fietkau <nbd@nbd.name> writes:

> On 2020-02-26 22:56, Toke H=C3=B8iland-J=C3=B8rgensen wrote:
>> Felix Fietkau <nbd@nbd.name> writes:
>>> - We need an API that allows the driver to change the pending airtime
>>> values, e.g. subtract estimated tx time for a packet.
>>> mt76 an ath9k can queue packets inside the driver that are not currently
>>> in the hardware queues. Typically if the txqs have more data than what
>>> gets put into the hardware queue, both drivers will pull an extra frame
>>> and queue it in its private txq struct. This frame will get used on the
>>> next txq scheduling round for that particular station.
>>> If you have lots of stations doing traffic (or having driver buffered
>>> frames in powersave mode), this could use up a sizable chunk of the AQL
>>> budget.
>>=20
>> I'm a bit more skeptical about this. If the driver buffers a bunch of
>> packets that are not accounted that will hurt that station due to extra
>> latency when it wakes up. For ath9k, this is the retry_q you're talking
>> about, right? The number of packets queued on that is fairly limited,
>> isn't it? What kind of powersave buffering is the driver doing, and why
>> can't it leave the packets on the TXQ? That would allow them to be
>> scheduled along with any new ones that might have arrived in the
>> meantime, which would be a benefit for latency.
> For mt76 there should be max. 1 frame in the retry queue, it's just a
> frame that was pulled from the txq in a transmission attempt but that it
> couldn't put in the hw queue because it didn't fit in the current
> aggregate batch.

Wait, if it's only a single frame that is queued in the driver, how is
this causing problems? We deliberately set the limit so there was a bit
of slack above the size of an aggregate for things like this. Could you
please describe in a bit more detail what symptoms you are seeing of
this problem? :)

-Toke