netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Petr Machata <petrm@nvidia.com>
To: <Daniel.Machon@microchip.com>
Cc: <petrm@nvidia.com>, <netdev@vger.kernel.org>, <kuba@kernel.org>,
	<vinicius.gomes@intel.com>, <vladimir.oltean@nxp.com>,
	<thomas.petazzoni@bootlin.com>, <Allan.Nielsen@microchip.com>,
	<maxime.chevallier@bootlin.com>, <nikolay@nvidia.com>,
	<roopa@nvidia.com>
Subject: Re: Basic PCP/DEI-based queue classification
Date: Mon, 22 Aug 2022 12:34:25 +0200	[thread overview]
Message-ID: <87v8qklbly.fsf@nvidia.com> (raw)
In-Reply-To: <YwKeVQWtVM9WC9Za@DEN-LT-70577>


<Daniel.Machon@microchip.com> writes:

>> > In the old thread, Maxime has compiled a list of ways we can possibly offload
>> > the queue classification. However none of them is a good match for our purpose,
>> > for the following reasons:
>> >
>> >  - tc-flower / tc-skbedit: The filter and action scheme maps poorly to hardware
>> >    and would require one hardware-table entry per rule. Even less of a match
>> >    when DEI is also considered. These tools are well suited for advanced
>> >    classification, and not so much for basic per-port classification.
>> 
>> Yeah.
>> 
>> Offloading this is a pain. You need to parse out the particular shape of
>> rules (which is not a big deal honestly), and make sure the ordering of
>> the rules is correct and matches what the HW is doing. And tolerate any
>> ACL-/TCAM- like rules as well. And there's mismatch between how a
>> missing rule behaves in SW (fall-through) and HW (likely priority 0 gets
>> assigned).
>> 
>> And configuration is pain as well, because a) it's a whole bunch of
>> rules to configure, and b) you need to be aware of all the limitations
>> from the previous paragraph and manage the coexistence with ACL/TCAM
>> rules.
>> 
>> It's just not a great story for this functionality.
>> 
>> I wonder if a specialized filter or action would make things easier to
>> work with. Something like "matchall action dcb dscp foo bar priority 7".
>> 
>
> I really think that pcp mapping should not go into tc. It is just not 
> user-friendly at all, and I believe better alternatives exists.

"better" along the "more specialized, hence easier to configure" axis.
But DCB in particular doesn't address the SW datapath at all, and if you
want this sort of prioritization in SW, you have to fall back on flower
et.al. anyway.

> So a pcp mapping functionality could very well go into dcb as an extension,
> for the following reasons:
>
>  - dcb already contains non-standard extension (dcb-maxrate)

Yeah, and if the standard DCB ever gets maxrate knobs that are not 1:1
compatible with what we now have, we're in trouble.

Whatever new things is getting added, needs to be added in a way that
makes standard collisions very unlikely.

>  - Adding an extension (dcb-pcp?) for configuring the pcp tables of ieee-802.1q
>    seems to be in line with what dcb-app is doing with the app table. Now, the
>    app table and the pcp tables are different, but they are both inteded to map
>    priority to queue (dscp or pcp/dei).

(Not priority to queue, but header field values to priority. Queue
assignment is "dcb buffer prio-buffer" on ingress and "dcb ets prio-tc"
on egress.)

>  - default prio for non-tagged frames is already possible in dcb-app
>
>  - dscp priority mapping is also possible in dcb-app
>
>  - dcb already has the necessary data structures for mapping priority to queue 
>    (array parameter)
>
>  - Seems conventient to place the priority mapping in one place (dscp and pcp/dei).
>
> Any thoughts?

How do the pcp-prio rules work with the APP rules? There's the dscp-prio
sparse table, then there will be the pcp-prio (sparse?) table, what
happens if a packet arrives that has both headers? In Spectrum switches,
DSCP takes precedence, but that may not be universal.

It looks like adding "PCP" to APP would make the integration easiest.
Maybe we could use an out-of-band sel value for the selector, say 256,
to likely avoid incompatible standardization?

Then the trust level can be an array of selectors that shows how the
rules should be applied. E.g. [TCPUDP, DSCP, PCP]. Some of these
configurations are not supported by the HW and will be bounced by the
driver.

(Am I missing something in the standard? It doesn't seem to deal with
how the APP rules are actually applied at all.)


Another issue: DCB APP is a sparse table. There's a question of what
should happen for the e.g. DSCP values that don't have an APP entry.
Logically I think they should "fall through" to other APP rules as per
the selector array.

Thing is, ASICs probably don't support this "fall-through" feature. So I
don't know what to do with this. Kinda brings back some of that TC
complexity, where you need to add all the rules, otherwise the HW can't
be compatibly configured.

  reply	other threads:[~2022-08-22 12:14 UTC|newest]

Thread overview: 22+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-08-19  9:09 Basic PCP/DEI-based queue classification Daniel.Machon
2022-08-19 10:50 ` Petr Machata
2022-08-21 20:58   ` Daniel.Machon
2022-08-22 10:34     ` Petr Machata [this message]
2022-08-24  7:39       ` Daniel.Machon
2022-08-24  9:45         ` Petr Machata
2022-08-24 17:55           ` Daniel.Machon
2022-08-24 19:36             ` Petr Machata
2022-08-25  0:54               ` Jakub Kicinski
2022-08-26 18:11                 ` Petr Machata
2022-08-29  7:53                 ` Allan W. Nielsen
2022-09-02 13:32                   ` Vladimir Oltean
2022-09-07 10:41                     ` Daniel.Machon
2022-09-07 17:26                       ` Vladimir Oltean
2022-09-07 19:57                         ` Daniel.Machon
2022-09-08  8:03                           ` Allan Nielsen - M31684
2022-09-08 11:18                           ` Petr Machata
2022-09-08 12:01                             ` Daniel.Machon
2022-09-09 12:11                           ` Vladimir Oltean
2022-09-08  8:27                         ` Petr Machata
2022-08-25 11:31               ` Daniel.Machon
2022-08-25 13:30                 ` Petr Machata

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=87v8qklbly.fsf@nvidia.com \
    --to=petrm@nvidia.com \
    --cc=Allan.Nielsen@microchip.com \
    --cc=Daniel.Machon@microchip.com \
    --cc=kuba@kernel.org \
    --cc=maxime.chevallier@bootlin.com \
    --cc=netdev@vger.kernel.org \
    --cc=nikolay@nvidia.com \
    --cc=roopa@nvidia.com \
    --cc=thomas.petazzoni@bootlin.com \
    --cc=vinicius.gomes@intel.com \
    --cc=vladimir.oltean@nxp.com \
    /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).