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>
Cc:     Johannes Berg <johannes@sipsolutions.net>,
        linux-wireless <linux-wireless@vger.kernel.org>,
        Make-Wifi-fast <make-wifi-fast@lists.bufferbloat.net>,
        Yibo Zhao <yiboz@codeaurora.org>,
        John Crispin <john@phrozen.org>,
        Lorenzo Bianconi <lorenzo@kernel.org>,
        Rajkumar Manoharan <rmanohar@codeaurora.org>,
        Kevin Hayes <kevinhayes@google.com>
Subject: Re: [PATCH v11 4/4] mac80211: Use Airtime-based Queue Limits (AQL) on packet dequeue
In-Reply-To: <cedd9fa8-e6ff-be64-d013-4243cb2fd85f@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> <CA+iem5vcz4PqJR+hj9CyS4pJMD4RpNz2RHxQyUy+-qT9y2WhyQ@mail.gmail.com> <cedd9fa8-e6ff-be64-d013-4243cb2fd85f@nbd.name>
Date:   Thu, 27 Feb 2020 12:07:53 +0100
Message-ID: <87o8tkwanq.fsf@toke.dk>
MIME-Version: 1.0
Content-Type: text/plain
Sender: linux-wireless-owner@vger.kernel.org
Precedence: bulk

Felix Fietkau <nbd@nbd.name> writes:

>> Not sure I fully understand your concerns here. The AQL budget is per
>> STA, per TID, so frames queued in the driver's special queue for one
>> station won't impact another station's budget. Those frames are
>> counted towards the per interface pending airtime, which could trigger
>> AQL to switch to use the lower queue limit. In this case, that could
>> be the desirable behavior when there is heavy traffic.
> Yes, the per interface limit is what I'm concerned about. I'm not sure
> if it will be an issue in practice, it's just something that I noticed
> while reviewing the code.

Ah, right. Note that it's not a hard limit, though; it's just a
threshold for when the per-station limit switches to a smaller value.
The idea is that when there are many stations with outstanding data,
each station's latency will be higher since they have to also wait for
their turn to transmit. And so we compensate for this by lowering each
station's queue limit, since the hardware is unlikely to become idle
when there are many stations to send to.

The lower limit is still 5ms of data, though, so there should still be
enough for a full aggregate queued for each station. Arguably the limit
could actually be made *smaller*: On a very busy network with lots of
stations transmitting at once, IMO we *don't* want to send full-sized
aggregates as that will add up to way too much latency. Better to
sacrifice a bit of network efficiency to raise responsiveness :)

-Toke