converting mac80211 to TXQs entirely

converting mac80211 to TXQs entirely

[Johannes Berg]

Hi,

Part 1 is just a dump of my notes analysing the current TX scheme.


driver setup
============

non-QUEUE_CONTROL drivers
 * have >= 4 queues
   - per-AC queues [0-3]
 * have < 4 queues
   - all goes to queue 0

QUEUE_CONTROL drivers
 * hwsim (doesn't handle CAB correctly)
   - each vif: 0...3
   - each cab: 0
   - offchannel: 4
 * TI (doesn't set HOST_BROADCAST_PS_BUFFERING)
   - each vif: 4 queues (separate)
   - each cab: separate queue
   - offchannel: separate from all others
 * iwldvm/iwlmvm (doesn't set HOST_BROADCAST_PS_BUFFERING)
   - each vif: 4 queues (separate)
   - each cab: separate queue
   - offchannel: separate from all others (AUX)
 * ath9k (uses TXQ, sets HOST_BROADCAST_PS_BUFFERING)
   - each vif: 4 queues (shared based on chanctx)
   - each cab: all the same (# queues - 2)
   - offchannel: separate from all others
 * ath10k (may use TXQ, doesn't set HOST_BROADCAST_PS_BUFFERING)
   - each vif: 1 queue for all ACs
   - each cab: same queue as for ACs
   - offchannel: separate from all others


current TX scheme
=================
HOST_BROADCAST_PS_BUFFERING && IEEE80211_TX_CTL_SEND_AFTER_DTIM
 --> queue for ieee80211_get_buffered_bc()

!AP_LINK_PS && sta sleeping
 --> queue on sta->ps_tx_buf[ac] for wakeup/poll
 --> send on poll with IEEE80211_TX_CTRL_PS_RESPONSE
 --> send on wakeup as normal frame (with or without TXQ)
     [NB: with TXQs, this is buggy due to waking old TXQ before tx_filtered]

finally
 --> send directly (with or without TXQ)

if filtered TX status
 --> append to tx_filtered[ac] and use that before ps_tx_buf[ac]

TXQ scheme (where used)
=======================

MONITOR || IEEE80211_TX_CTL_SEND_AFTER_DTIM || IEEE80211_TX_CTRL_PS_RESPONSE || non-data
 --> send directly (to TX queue number as given above)
have STA
 --> per-STA/TID TXQ
otherwise
 --> per-VIF TXQ (for VLAN use AP instead)

Re: converting mac80211 to TXQs entirely

[Johannes Berg]

Part 2 - where can we go from here


Of course - as mentioned in the subject - my goal is to just convert
over to TXQs entirely in mac80211. That would get rid of a LOT of
special case code, like queueing for aggregation setup, powersave
buffering (both unicast and multicast), etc.


I think the following would be appropriate steps to take

1) convert multicast PS buffering

This is a bit strange today - when multicast PS buffering *isn't*
needed, frames go to TXQ drivers via the vif->txq, but when it *is*
done, then frames are tagged with IEEE80211_TX_CTL_SEND_AFTER_DTIM and
are buffered on ps->bc_buf (if HOST_BROADCAST_PS_BUFFERING is set) and
then handed to the driver through the legacy ->tx() path.

This should be reasonably simple to change - get rid of ps->bc_buf, and
keep the frames on the TXQ, making ieee80211_get_buffered_bc() retrieve
them from there instead. We'd have to tell the driver (it needs to know
in the wake callback) that it has sleeping clients and needs to buffer,
so it can decide whether or not to retrieve immediately (basically, for
TXQ drivers, implementing HOST_BROADCAST_PS_BUFFERING in driver logic:
retrieve immediately if it wants to buffer itself, or keep them there
for a later ieee80211_get_buffered_bc() call if it doesn't).

2) use TXQ for offchannel frames

I'm a bit unsure about this - we never really want to queue offchannel
packets, and if they don't go out immediately then we can basically
drop them. Nevertheless, having everything deal with the TXQ API will
be simpler, and so I think this also should. Perhaps for this we should
have a TXQ but only ever use txqi->frags, so that we never really have
to deal with the FQ stuff for these frames. Then the wake would
basically just pull down the packets and send them immediately.

3) handle monitor mode

I thought this was more complicated, but I think the easiest way to
solve this is to actually just use the local->monitor_sdata->vif.txq.
This requires that mac80211 be changed to always allocate local-
>monitor_sdata, even if WANT_MONITOR_VIF isn't set, and ath9k/ath10k do
something special for this TXQ (and perhaps ath9k should set
WANT_MONITOR_VIF), but that seems reasonable.

4) handle non-data frames for stations

This is probably the trickiest part. I have a patch to add a non-data
TID to the STA TXQs, and that's perhaps the right thing to do - though
I guess those frames should again always go onto txqi->frags so they
don't compete with data frames for resources.

For powersave reasons we'll discuss later, this should probably only
apply to bufferable MMPDUs, with others going to the vif->nondata_txq
(next item).

5) handle non-data frames for vifs

Similarly, we need a vif->nondata_txq where we can put probe responses
and the (few) other frames that we send before we have a station entry.



According to my earlier analysis (previous email) after these steps we
have a TXQ for all frames. All of these steps will require certain
adjustments in the drivers currently using TXQs (ath9k & ath10k) since
they'll be relying on some amount of buffering and queue stop/wake in
mac80211 for these frames. We might just have to add some API to ask
"is queue stopped" to make the transition here easy.


After this, we can start thinking about doing internal cleanups in
mac80211.


6) First thing to do is probably to introduce a compat layer, so that
all drivers go through the TXQs, regardless of whether they handle
that. I have the beginnings of a patch that does that, it basically
requires drivers to set the wake_tx_queue method to a new mac80211
function ieee80211_wake_tx_queue() [so the ops can remain const] when
they don't actually want to deal with TXQs themselves.

This method can then check the queue stop reasons etc. and only service
TXQs that don't have their corresponding queue stopped. We'd also hook
into when the queues get re-enabled, and call the servicing function
from there for such drivers.

My current prototype just calls the existing __ieee80211_tx() but I no
longer think that's a good idea - the idea is that this would
eventually allow us to get rid of tx_pending.

So it'd be better to have ieee80211_wake_tx_queue() just check all the
required things, and once a frame is pulled from a TXQ it's committed
to be given to the driver.

For off-channel, a special case will be needed - dropping the frame
when there's no way to transmit it at the time.

7) Remove all the now-dead code

A lot of code is now dead - we'll never have multiple netdev queues,
all IFF_NOQUEUE etc - remove all the code associated with that


8) convert more infrastructure to TXQs

It gets more vague (starting from 6), but eventually I want to
 * get rid of tx_pending (why do we even use both TXQ and tx_pending
   for aggregation?)
 * do all powersave buffering on TXQs
   [this may be tricky for filtered frames, perhaps disallow filtered
   for TXQ drivers like ath9k/ath10k, and have a separate per-TXQI list
   in the private txq data for ieee80211_wake_tx_queue]


Thoughts?

johannes

Re: converting mac80211 to TXQs entirely

[Toke Høiland-Jørgensen]

Johannes Berg <johannes@sipsolutions.net> writes:

> Part 2 - where can we go from here
>
>
> Of course - as mentioned in the subject - my goal is to just convert
> over to TXQs entirely in mac80211. That would get rid of a LOT of
> special case code, like queueing for aggregation setup, powersave
> buffering (both unicast and multicast), etc.


At first glance this looks like a decent way forward. I'll think about
it some more and comment again later, but for now I just wanted to add
this:

I'm been thinking about how to move the airtime fairness scheduler out
of ath9k and into mac80211 (so more drivers can take advantage of it).
This will require some changes to the TXQ API that drivers speak to, so
wanted to add my thoughts here to make sure it's compatible with your
thinking.

I think the easiest way to have mac80211 handle airtime fairness is to
add a way for ieee80211_tx_dequeue() to return some sort of DEFER signal
which semantically means "there is no packet for this queue right now,
but please keep scheduling it" (which would be the result of a TXQ
belonging to a station that has used its airtime quota but still has
packets pending). This is different from the current meaning of NULL,
which will make the driver stop scheduling that TXQ until it gets a new
wake signal.

The alternative would be to change the API so the driver asks mac80211
which TXQ it should pull from next, instead of doing its own scheduling
as it does now. But I'm not sure that's doable with a sane API.

Now, I believe this is related to this point of yours:

> 5) handle non-data frames for vifs
>
> Similarly, we need a vif->nondata_txq where we can put probe responses
> and the (few) other frames that we send before we have a station entry.
>
> According to my earlier analysis (previous email) after these steps we
> have a TXQ for all frames. All of these steps will require certain
> adjustments in the drivers currently using TXQs (ath9k & ath10k) since
> they'll be relying on some amount of buffering and queue stop/wake in
> mac80211 for these frames. We might just have to add some API to ask
> "is queue stopped" to make the transition here easy.

And I'm wondering if this "is queue stopped" API could be the same one
used for the airtime DEFER case?


Oh, and BTW: I see this is on the list of topics for the wireless summit
in Seoul. How do I go about signing up for that? I'll be at netdev
talking about some of this stuff anyway[1] :)

-Toke

[1] https://www.netdevconf.org/2.2/session.html?jorgensen-wifistack-talk

Re: converting mac80211 to TXQs entirely

[Johannes Berg]

On Thu, 2017-10-05 at 18:28 +0200, Toke Høiland-Jørgensen wrote:

> I'm been thinking about how to move the airtime fairness scheduler
> out of ath9k and into mac80211 (so more drivers can take advantage of
> it). This will require some changes to the TXQ API that drivers speak
> to, so wanted to add my thoughts here to make sure it's compatible
> with your thinking.
> 
> I think the easiest way to have mac80211 handle airtime fairness is
> to add a way for ieee80211_tx_dequeue() to return some sort of DEFER
> signal which semantically means "there is no packet for this queue
> right now, but please keep scheduling it" (which would be the result
> of a TXQ belonging to a station that has used its airtime quota but
> still has packets pending). This is different from the current
> meaning of NULL, which will make the driver stop scheduling that TXQ
> until it gets a new wake signal.

I think that's reasonable. I'm not really sure it's *necessary* though?
Couldn't mac80211 return NULL, and then simply call wake_tx_queue again
when the TXQ becomes eligible for scheduling again? Otherwise the
driver might end up doing a lot of polling for it to become eligible
again?

I've mostly glossed over a mac80211 scheduler, which is obviously
needed as part of a complete conversion, and it'd just have to
integrate with this new return value.

> The alternative would be to change the API so the driver asks
> mac80211 which TXQ it should pull from next, instead of doing its own
> scheduling as it does now. But I'm not sure that's doable with a sane
> API.
> 
> Now, I believe this is related to this point of yours:
> 
> > 5) handle non-data frames for vifs
> > 
> > Similarly, we need a vif->nondata_txq where we can put probe
> > responses and the (few) other frames that we send before we have a
> > station entry.
> > 
> > According to my earlier analysis (previous email) after these steps
> > we have a TXQ for all frames. All of these steps will require
> > certain adjustments in the drivers currently using TXQs (ath9k &
> > ath10k) since they'll be relying on some amount of buffering and
> > queue stop/wake in mac80211 for these frames. We might just have to
> > add some API to ask "is queue stopped" to make the transition here
> > easy.
> 
> And I'm wondering if this "is queue stopped" API could be the same
> one used for the airtime DEFER case?

I think these are two completely different cases.

The "is queue stopped" I was thinking of would be basically checking
the variable local->queue_stop_reasons, so that the driver could still
use the stop_queue API(s). Yeah, this would be very roundabout, but as
a conversion step I think that'd not be a bad thing.

A more complete conversion likely wouldn't need this, but would instead
have the driver record its own stop reasons and just stop scheduling
those TXQs that belong to a stopped "class" (it's no longer really a
queue).

(and for mac80211 stop reasons, it would just return NULL and re-
schedule the TXQ when it becomes eligible again)


> Oh, and BTW: I see this is on the list of topics for the wireless
> summit in Seoul. How do I go about signing up for that? I'll be at
> netdev talking about some of this stuff anyway[1] :)

Just show up there, or you can add yourself to the list on the wiki
page :)

johannes

Re: converting mac80211 to TXQs entirely

[Toke Høiland-Jørgensen]

Johannes Berg <johannes@sipsolutions.net> writes:

> On Thu, 2017-10-05 at 18:28 +0200, Toke H=C3=B8iland-J=C3=B8rgensen wrote:
>
>> I'm been thinking about how to move the airtime fairness scheduler
>> out of ath9k and into mac80211 (so more drivers can take advantage of
>> it). This will require some changes to the TXQ API that drivers speak
>> to, so wanted to add my thoughts here to make sure it's compatible
>> with your thinking.
>>=20
>> I think the easiest way to have mac80211 handle airtime fairness is
>> to add a way for ieee80211_tx_dequeue() to return some sort of DEFER
>> signal which semantically means "there is no packet for this queue
>> right now, but please keep scheduling it" (which would be the result
>> of a TXQ belonging to a station that has used its airtime quota but
>> still has packets pending). This is different from the current
>> meaning of NULL, which will make the driver stop scheduling that TXQ
>> until it gets a new wake signal.
>
> I think that's reasonable. I'm not really sure it's *necessary*
> though? Couldn't mac80211 return NULL, and then simply call
> wake_tx_queue again when the TXQ becomes eligible for scheduling
> again? Otherwise the driver might end up doing a lot of polling for it
> to become eligible again?

Theoretically, but then there would have to be some kind of callback or
another mechanism to figure out when the queue is ready again. The neat
thing about DRR scheduling is that we just use the fact that there was
an attempt to schedule the queue as an opportunity to increase that
queue's deficit by one quantum. This does give a bit of "polling
overhead" as you say, but it has been hidden in the measurement noise in
all my tests.

The obvious alternative is to do a token-based airtime scheduler, where
the airtime used by one station is immediately divided out to all active
stations. But that requires walking over all active stations on every TX
completion, and there's a lot of housekeeping to make sure we don't
accidentally starve everyone. So I'd prefer to stick with the DRR
scheduler :)

>> And I'm wondering if this "is queue stopped" API could be the same
>> one used for the airtime DEFER case?
>
> I think these are two completely different cases.
>
> The "is queue stopped" I was thinking of would be basically checking
> the variable local->queue_stop_reasons, so that the driver could still
> use the stop_queue API(s). Yeah, this would be very roundabout, but as
> a conversion step I think that'd not be a bad thing.

Ah, I see. Yeah, these are different, and the existing packet/NULL
return is probably sufficient, then :)

>> Oh, and BTW: I see this is on the list of topics for the wireless
>> summit in Seoul. How do I go about signing up for that? I'll be at
>> netdev talking about some of this stuff anyway[1] :)
>
> Just show up there, or you can add yourself to the list on the wiki
> page :)

Cool, I did. See you in Seoul :)

-Toke

Re: converting mac80211 to TXQs entirely

[Johannes Berg]

On Thu, 2017-10-05 at 23:52 +0200, Toke Høiland-Jørgensen wrote:
> 
> > I think that's reasonable. I'm not really sure it's *necessary*
> > though? Couldn't mac80211 return NULL, and then simply call
> > wake_tx_queue again when the TXQ becomes eligible for scheduling
> > again? Otherwise the driver might end up doing a lot of polling for
> > it to become eligible again?
> 
> Theoretically, but then there would have to be some kind of callback
> or another mechanism to figure out when the queue is ready again. The
> neat thing about DRR scheduling is that we just use the fact that
> there was an attempt to schedule the queue as an opportunity to
> increase that queue's deficit by one quantum. This does give a bit of
> "polling overhead" as you say, but it has been hidden in the
> measurement noise in all my tests.

Ok.

> The obvious alternative is to do a token-based airtime scheduler,
> where the airtime used by one station is immediately divided out to
> all active stations. But that requires walking over all active
> stations on every TX completion, and there's a lot of housekeeping to
> make sure we don't accidentally starve everyone. So I'd prefer to
> stick with the DRR scheduler :)

Sure, works for me.

I have no objection to defining a special error code (we can always use
ERR_PTR(-EAGAIN) or something like that, after all) - we just need to
be careful with driver updates.

Given all these discussions, I'd actually like to put a temporary
restriction on merging new drivers with TXQ support - Felix, I think
you were planning to eventually use this for mt7601u? How far along is
that?

johannes

Re: converting mac80211 to TXQs entirely

[Toke Høiland-Jørgensen]

Johannes Berg <johannes@sipsolutions.net> writes:

>> The obvious alternative is to do a token-based airtime scheduler,
>> where the airtime used by one station is immediately divided out to
>> all active stations. But that requires walking over all active
>> stations on every TX completion, and there's a lot of housekeeping to
>> make sure we don't accidentally starve everyone. So I'd prefer to
>> stick with the DRR scheduler :)
>
> Sure, works for me.
>
> I have no objection to defining a special error code (we can always
> use ERR_PTR(-EAGAIN) or something like that, after all) - we just need
> to be careful with driver updates.

Right. I'll see if I can cook up an RFC.

Do you have any opinion on whether to use ERR_PTR or change the function
to an int return and pass the pointer as an argument? At least ath10k
seems to do the latter (returning -ENOENT when no packet is available).

-Toke

Re: converting mac80211 to TXQs entirely

[Johannes Berg]

On Fri, 2017-10-06 at 12:17 +0200, Toke Høiland-Jørgensen wrote:
> 
> Do you have any opinion on whether to use ERR_PTR or change the
> function to an int return and pass the pointer as an argument? At
> least ath10k seems to do the latter (returning -ENOENT when no packet
> is available).

I think using an ERR_PTR is nicer, but I have no strong opinions (I
just don't like out parameters all that much and avoid them when
possible)

johannes