| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <CAHzwrcfpsBpiEsf4b2Y6xEAhno3rNKz6tWuxqRAUb0HyBT6c7Q@mail.gmail.com>
Date: Mon, 20 Feb 2023 17:01:57 +0100
From: Péter Antal <peti.antal99@...il.com>
To: Vladimir Oltean <vladimir.oltean@....com>
Cc: netdev@...r.kernel.org, John Fastabend <john.fastabend@...il.com>,
Stephen Hemminger <stephen@...workplumber.org>,
Ferenc Fejes <fejes@....elte.hu>,
Ferenc Fejes <ferenc.fejes@...csson.com>,
Vinicius Costa Gomes <vinicius.gomes@...el.com>,
Péter Antal <antal.peti99@...il.com>
Subject: Re: [PATCH iproute2] man: tc-mqprio: extend prio-tc-queue mapping
with examples
Hi Vladimir,
Vladimir Oltean <vladimir.oltean@....com> ezt írta (időpont: 2023.
febr. 20., H, 16:29):
>
> Hi Péter,
>
> On Mon, Feb 20, 2023 at 04:05:48PM +0100, Péter Antal wrote:
> > The current mqprio manual is not detailed about queue mapping
> > and priorities, this patch adds some examples to it.
> >
> > Suggested-by: Ferenc Fejes <fejes@....elte.hu>
> > Signed-off-by: Péter Antal <peti.antal99@...il.com>
> > ---
>
> I think it's great that you are doing this. However, with all due respect,
> this conflicts with the man page restructuring I am already doing for the
> frame preemption work. Do you mind if I fix up some things and I pick your
> patch up, and submit it as part of my series? I have some comments below.
That's all right, thank you for doing this, just please carry my
signoff as co-developer if possible.
I agree with most of your suggestions.
>
> > man/man8/tc-mqprio.8 | 96 ++++++++++++++++++++++++++++++++++++++++++++
> > 1 file changed, 96 insertions(+)
> >
> > diff --git a/man/man8/tc-mqprio.8 b/man/man8/tc-mqprio.8
> > index 4b9e942e..16ecb9a1 100644
> > --- a/man/man8/tc-mqprio.8
> > +++ b/man/man8/tc-mqprio.8
> > @@ -98,6 +98,7 @@ belong to an application. See kernel and cgroup documentation for details.
> > .TP
> > num_tc
> > Number of traffic classes to use. Up to 16 classes supported.
> > +You cannot have more classes than queues
>
> More impersonal: "There cannot be more traffic classes than TX queues".
>
> >
> > .TP
> > map
> > @@ -119,6 +120,8 @@ Set to
> > to support hardware offload. Set to
> > .B 0
> > to configure user specified values in software only.
> > +The default value of this parameter is
> > +.B 1
> >
> > .TP
> > mode
> > @@ -146,5 +149,98 @@ max_rate
> > Maximum value of bandwidth rate limit for a traffic class.
> >
> >
> > +.SH EXAMPLE
> > +
> > +The following example shows how to attach priorities to 4 traffic classes ("num_tc 4"),
> > +and then how to pair these traffic classes with 4 hardware queues with mqprio,
>
> "with mqprio" is understated I think, this is the mqprio man page
>
> > +with hardware coordination ("hw 1", or does not specified, because 1 is the default value).
>
> Just leave it at that, "hw 1".
>
> > +Traffic class 0 (tc0) is mapped to hardware queue 0 (q0), tc1 is mapped to q1,
> > +tc2 is mapped to q2, and tc3 is mapped q3.
>
> Would prefer saying TC0, TXQ0 etc if you don't mind.
>
> > +
> > +.EX
> > +# tc qdisc add dev eth0 root mqprio \
> > + num_tc 4 \
> > + map 0 0 0 0 1 1 1 1 2 2 2 2 3 3 3 3 \
> > + queues 1@0 1@1 1@2 1@3 \
> > + hw 1
> > +.EE
> > +
> > +The next example shows how to attach priorities to 3 traffic classes ("num_tc 3"),
> > +and how to pair these traffic classes with 4 queues,
> > +without hardware coordination ("hw 0").
> > +Traffic class 0 (tc0) is mapped to hardware queue 0 (q0), tc1 is mapped to q1,
> > +tc2 and is mapped to q2 and q3, where the queue selection between these
> > +two queues is somewhat randomly decided.
>
> Would rather say that packets are hashed arbitrarily between TXQ3 and
> TXQ4, which have the same scheduling priority.
If I understand it correctly, this depends on the "hw" parameter.
So if the driver handles this (hw 1), then it may prioritize a queue
over another,
even if they have the same priority.
>
> We should probably clarify what "hardware coordination" means, exactly.
> Without hardware coordination, the device driver is not notified of the
> number of traffic classes and their mapping to TXQs. The device is not
> expected to prioritize between traffic classes without hardware
> coordination.
>
> We should also probably clarify that with hardware coordination, the
> device driver can install a different TXQ configuration than requested,
> and that the default TC to TXQ mapping is:
>
> TC 0: 0 queues @ offset 0
> TC 1: 0 queues @ offset 0
> ...
> TC N: 0 queues @ offset 0
>
First time I see hardware coordination in this manual is the hw parameter,
so I think we should clarify this there.
> Should also probably clarify that there is a default prio:tc map of:
>
> .prio_tc_map = { 0, 1, 2, 3, 4, 5, 6, 7, 0, 1, 1, 1, 3, 3, 3, 3 },
>
> hmm... you gave me more work than I was intending to do :)
Sorry about that :)
>
> > +
> > +.EX
> > +# tc qdisc add dev eth0 root mqprio \
> > + num_tc 3 \
> > + map 0 0 0 0 1 1 1 1 2 2 2 2 2 2 2 2 \
> > + queues 1@0 1@1 2@2 \
> > + hw 0
> > +.EE
> > +
> > +
> > +In both cases from above the priority values from 0 to 3 (prio0-3) are
> > +mapped to tc0, prio4-7 are mapped to tc1, and the
> > +prio8-11 are mapped to tc2 ("map" attribute). The last four priority values
> > +(prio12-15) are mapped in different ways in the two examples.
> > +They are mapped to tc3 in the first example and mapped to tc2 in the second example.
> > +The values of these two examples are the following:
> > +
> > + ┌────┬────┬───────┐ ┌────┬────┬────────┐
> > + │Prio│ tc │ queue │ │Prio│ tc │ queue │
> > + ├────┼────┼───────┤ ├────┼────┼────────┤
> > + │ 0 │ 0 │ 0 │ │ 0 │ 0 │ 0 │
> > + │ 1 │ 0 │ 0 │ │ 1 │ 0 │ 0 │
> > + │ 2 │ 0 │ 0 │ │ 2 │ 0 │ 0 │
> > + │ 3 │ 0 │ 0 │ │ 3 │ 0 │ 0 │
> > + │ 4 │ 1 │ 1 │ │ 4 │ 1 │ 1 │
> > + │ 5 │ 1 │ 1 │ │ 5 │ 1 │ 1 │
> > + │ 6 │ 1 │ 1 │ │ 6 │ 1 │ 1 │
> > + │ 7 │ 1 │ 1 │ │ 7 │ 1 │ 1 │
> > + │ 8 │ 2 │ 2 │ │ 8 │ 2 │ 2 or 3 │
> > + │ 9 │ 2 │ 2 │ │ 9 │ 2 │ 2 or 3 │
> > + │ 10 │ 2 │ 2 │ │ 10 │ 2 │ 2 or 3 │
> > + │ 11 │ 2 │ 2 │ │ 11 │ 2 │ 2 or 3 │
> > + │ 12 │ 3 │ 3 │ │ 12 │ 2 │ 2 or 3 │
> > + │ 13 │ 3 │ 3 │ │ 13 │ 2 │ 2 or 3 │
> > + │ 14 │ 3 │ 3 │ │ 14 │ 2 │ 2 or 3 │
> > + │ 15 │ 3 │ 3 │ │ 15 │ 2 │ 2 or 3 │
> > + └────┴────┴───────┘ └────┴────┴────────┘
> > + example1 example2
>
> What would you say if we didn't put the 2 examples side by side, but
> kept them separately? Also, I get the feeling that the verbiage above
> the tables is a bit too much. Having the tables is already good enough.
>
> > +
> > +Another example of queue mapping is the following.
> > +There are 5 traffic classes, and there are 8 hardware queues.
>
> I think for maintainability, the examples should be fairly independent,
> because they might be moved around in the future. This story-like
> "the following example" -> "the next example" -> "another example"
> doesn't really work.
>
> > +.EX
> > +# tc qdisc add dev eth0 root mqprio \
> > + num_tc 5 \
> > + map 0 0 0 1 1 1 1 2 2 3 3 4 4 4 4 4 \
> > + queues 1@0 2@1 1@3 1@4 3@5
>
> The formatting here and everywhere doesn't look very well when viewed
> with "man -l man/man8/tc-mqprio.8". If you look at tc-taprio.8, it uses
> "\\" to render properly. If you don't mind, I'll do that here too.
>
> > +.EE
> > +
> > +The value mapping is the following for this example:
> > +
> > + ┌───────┐
> > + tc0────┤Queue 0│◄────1@0
> > + ├───────┤
> > + ┌─┤Queue 1│◄────2@1
> > + tc1──┤ ├───────┤
> > + └─┤Queue 2│
> > + ├───────┤
> > + tc2────┤Queue 3│◄────1@3
> > + ├───────┤
> > + tc3────┤Queue 4│◄────1@4
> > + ├───────┤
> > + ┌─┤Queue 5│◄────3@5
> > + │ ├───────┤
> > + tc4──┼─┤Queue 6│
> > + │ ├───────┤
> > + └─┤Queue 7│
> > + └───────┘
> > +
> > +
> > .SH AUTHORS
> > John Fastabend, <john.r.fastabend@...el.com>
> > --
> > 2.34.1
> >
Best,
Peter
Powered by blists - more mailing lists