All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Toke Høiland-Jørgensen" <toke@toke.dk>
To: Johannes Berg <johannes@sipsolutions.net>,
	make-wifi-fast@lists.bufferbloat.net,
	linux-wireless@vger.kernel.org
Subject: Re: [RFC] mac80211: Add airtime fairness accounting
Date: Mon, 09 Oct 2017 11:42:50 +0200	[thread overview]
Message-ID: <878tgkd5d1.fsf@toke.dk> (raw)
In-Reply-To: <1507533328.26041.12.camel@sipsolutions.net>

Johannes Berg <johannes@sipsolutions.net> writes:

> On Sat, 2017-10-07 at 13:22 +0200, Toke H=C3=B8iland-J=C3=B8rgensen wrote:
>
>> Guess you are right that it will be difficult to get a completely
>> accurate number. But as David Lang notes, as long as we are off by
>> the same amount for all stations, that is fine - we're just
>> interested in relative numbers.
>
> That's not quite true though, you'd overestimate most on stations that
> are using aggregation, assuming you take into account the whole frame
> exchange sequence time. But maybe giving less than their fair share to
> fast stations isn't really that much of a problem.

Well, the padding and spacing between frames is at most 11 bytes (4-byte
delimiter, 4-byte FCS and 3-byte padding), which is ~0.7% of a
full-sized frame. I'm not too worried about errors on that scale, TBH.
Sure, it would be better to have it be accurate, but there are other
imperfections, especially on the RX side (we can't count
retransmissions, for instance, since the receiver obviously doesn't see
those).

>> > So ... no, I don't understand. You have a TXQ per _TID_, so
>> > splitting up this per _AC_ still doesn't make sense since you have
>> > two TXQs for each AC?
>>=20
>> Yeah, but ath9k schedules all TIDs on the same AC together. So if
>> station A has two TIDs active and station B one, the scheduling order
>> will be A1, A2, B1, basically. This is fine as long as they are all
>> on the same AC and scheduled together (in which case A1 and A2 will
>> just share the same deficit).
>
> Huh, I'm confused. Can you elaborate on this? How does it schedule two
> TIDs for _different_ ACs then?

There's a separate scheduling loop for each hardware queue (one per AC),
which only schedules all TXQs with that AC. The hardware will prioritise
higher ACs by dequeueing from the high-priority hardware queue first.

> It seems to me that to actually get the right QoS behaviour you have
> to schedule *stations*, and then while you're looking at a station,
> you need to select the highest-priority TXQ that has data? Otherwise,
> don't you end up doing fairness on a STA/AC rather than just on a STA,
> so that a station that uses two ACs gets twice as much airtime as one
> using just a single AC?

Yeah, that's what we have currently in ath9k. However, it's rare in
practice that a station transmits the same amount of data on all ACs
(for one, since the max aggregation size is smaller at the higher ACs
that becomes difficult). But you are quite right that this is something
that should be fixed :)

>> Ideally, I would prefer the scheduling to be "two-pass": First,
>> decide which physical station to send to, then decide which TID on
>> that station to service.=20
>
> Yeah, that would make more sense.
>
>> But because everything is done at the TID/TXQ level, that is not
>> quite trivial to achieve I think...
>
> Well you can group the TXQs, I guess. They all have a STA pointer, so
> you could put a one- or two-bit "schedule color" field into each
> station and if you find a TXQ with the same station color you just
> skip it or something like that?

Couldn't we add something like a get_next_txq(phy) function to mac80211
that the drivers can call to get the queue to pull packets from? That
way, responsibility for scheduling both stations and QoS levels falls to
mac80211, which makes it possible to do clever scheduling stuff without
having to re-implement it in every driver. Also, this function could
handle all the special TXQs for PS and non-data frames that you were
talking about in your other email?

Unless there's some reason I'm missing that the driver really needs to
schedule the TXQs, I think this would make a lot of sense?

-Toke

  parent reply	other threads:[~2017-10-09  9:42 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-10-06 11:52 [RFC] mac80211: Add airtime fairness accounting Toke Høiland-Jørgensen
2017-10-06 14:07 ` Johannes Berg
2017-10-06 14:29   ` Toke Høiland-Jørgensen
2017-10-06 17:18     ` Johannes Berg
2017-10-06 22:40       ` [Make-wifi-fast] " David Lang
2017-10-07 11:22       ` Toke Høiland-Jørgensen
2017-10-09  7:15         ` Johannes Berg
2017-10-09  7:50           ` [Make-wifi-fast] " David Lang
2017-10-09  9:42           ` Toke Høiland-Jørgensen [this message]
2017-10-09 11:40             ` Johannes Berg
2017-10-09 12:38               ` Toke Høiland-Jørgensen
2017-10-09 18:50                 ` Johannes Berg
2017-10-09 20:25                   ` Toke Høiland-Jørgensen
2017-10-11  8:55                     ` Johannes Berg
2017-10-11 13:50                       ` Toke Høiland-Jørgensen
     [not found]   ` <CAJq5cE0YewMkTcuWM_tRjJnP2vLa_cvoQEsgz8JhGLHxOOSRsw@mail.gmail.com>
2017-10-06 17:12     ` [Make-wifi-fast] " Johannes Berg

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=878tgkd5d1.fsf@toke.dk \
    --to=toke@toke.dk \
    --cc=johannes@sipsolutions.net \
    --cc=linux-wireless@vger.kernel.org \
    --cc=make-wifi-fast@lists.bufferbloat.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.