netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Vinicius Costa Gomes <vinicius.gomes@intel.com>
To: Johannes Berg <johannes@sipsolutions.net>, netdev@vger.kernel.org
Cc: "Toke Høiland-Jørgensen" <toke@toke.dk>,
	"Avi Stern" <avraham.stern@intel.com>,
	linux-wireless@vger.kernel.org
Subject: Re: taprio vs. wireless/mac80211
Date: Wed, 24 Aug 2022 16:33:43 -0700	[thread overview]
Message-ID: <87lerdmd2g.fsf@intel.com> (raw)
In-Reply-To: <117aa7ded81af97c7440a9bfdcdf287de095c44f.camel@sipsolutions.net>

Hi Johannes,

Johannes Berg <johannes@sipsolutions.net> writes:

> Hi,
>
> We're exploring the use of taprio with wireless/mac80211, and in
> mac80211 today (**) we don't have multiple queues (any more) since all
> the queuing actually happens in FQ/Codel inside mac80211. We also set
> IFF_NO_QUEUE, but that of course only really affects the default qdisc,
> not the fact that you could use it or not.
>
> In mac80211 thus we never back-pressure against the qdiscs, which is why
> we basically selected a single queue. Also, there's nothing else we do
> about the queue other than immediately pull a packet from it if
> available, so it'd basically pure overhead to have real queues there.
>
> In a sense, given that we cannot back-pressure against the queues, it
> feels a bit wrong to even have multiple queues. We also don't benefit in
> any way from splitting data structures onto multiple CPUs or something
> since we put it all into the same FQ/Codel anyway.
>
>
> Now, in order to use taprio, you're more or less assuming that you have
> multiple (equivalent) queues, as far as I can tell.
>
>
> Obviously we can trivially expose multiple equivalent queues from
> mac80211, but it feels somewhat wrong if that's just to make taprio be
> able to do something?
>
> Also how many should we have, there's more code to run so in many cases
> you probably don't want more than one, but if you want to use taprio you
> need at least two, but who says that's good enough, you might want more
> classes:
>
>         /* taprio imposes that traffic classes map 1:n to tx queues */
>         if (qopt->num_tc > dev->num_tx_queues) {
>                 NL_SET_ERR_MSG(extack, "Number of traffic classes is
> greater than number of HW queues");
>                 return -EINVAL;
>         }
>
>
> The way taprio is done almost feels like maybe it shouldn't even care
> about the number of queues since taprio_dequeue_soft() anyway only
> queries the sub-qdiscs? I mean, it's scheduling traffic, if you over-
> subscribe and send more than the link can handle, you've already lost
> anyway, no?
>
> (But then Avi pointed out that the sub qdiscs are initialized per HW
> queue, so this doesn't really hold either ...)
>
>
> Anyone have recommendations what we should do?

I will need to sleep on this, but at first glance, it seems you are
showing a limitation of taprio.

Removing that limitation seems possible, but it would add a bit of
complexity (but not much it seems) to the code, let me write down what I
am thinking:

 0. right now I can trust that there are more queues than traffic
 classes, and using the netdev prio->tc->queue map, I can do the
 scheduling almost entirely on queues. I have to remove this assumption.

 1. with that assumption removed, it means that I can have more traffic
 classes than queues, and so I have to be able to handle multiple
 traffic classes mapped to a single queue, i.e. one child qdisc per TC
 vs. one child per queue that we have today. Enqueueing each packet to
 the right child qdisc is easy. Dequeueing also is also very similar to
 what taprio does right now.

 2. it would be great if I knew the context in which each ->dequeue() is
 called, specifically which queue the ->dequeue() is for, it would
 reduce the number of children that I would have to check.

After writing this, I got the impression that it's feasible. Anyway,
will think a bit more about it.

(2) I don't think is possible right now, but I think we can go on
without it, and leave it as a future optimization.

Does it make sense? Did I understand the problem you are having right?


Cheers,
-- 
Vinicius

  reply	other threads:[~2022-08-24 23:34 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-08-24 21:50 taprio vs. wireless/mac80211 Johannes Berg
2022-08-24 23:33 ` Vinicius Costa Gomes [this message]
2022-08-24 23:55   ` Dave Taht
2022-08-25  2:15 ` Jakub Kicinski
2022-08-26 22:10   ` Vinicius Costa Gomes
2022-08-27  0:00     ` Jakub Kicinski
2022-08-27  0:13       ` Dave Taht
2022-08-28 19:50 ` Toke Høiland-Jørgensen

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=87lerdmd2g.fsf@intel.com \
    --to=vinicius.gomes@intel.com \
    --cc=avraham.stern@intel.com \
    --cc=johannes@sipsolutions.net \
    --cc=linux-wireless@vger.kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=toke@toke.dk \
    /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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).