netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Allan Nielsen - M31684 <Allan.Nielsen@microchip.com>
To: Daniel Machon - M70577 <Daniel.Machon@microchip.com>
Cc: Vladimir Oltean <vladimir.oltean@nxp.com>,
	"kuba@kernel.org" <kuba@kernel.org>,
	"petrm@nvidia.com" <petrm@nvidia.com>,
	"netdev@vger.kernel.org" <netdev@vger.kernel.org>,
	"vinicius.gomes@intel.com" <vinicius.gomes@intel.com>,
	"thomas.petazzoni@bootlin.com" <thomas.petazzoni@bootlin.com>,
	"maxime.chevallier@bootlin.com" <maxime.chevallier@bootlin.com>,
	"roopa@nvidia.com" <roopa@nvidia.com>
Subject: Re: Basic PCP/DEI-based queue classification
Date: Thu, 8 Sep 2022 10:03:58 +0200	[thread overview]
Message-ID: <20220908080358.islaqh4irglbyp4s@lx-anielsen> (raw)
In-Reply-To: <Yxj5smlnHEMn0sq2@DEN-LT-70577>

Hi Vladimir, Daniel,

On 07.09.2022 19:57, Daniel Machon - M70577 wrote:
>> On Wed, Sep 07, 2022 at 10:41:10AM +0000, Daniel.Machon@microchip.com wrote:
>> > > Regarding the topic at hand, and the apparent lack of PCP-based
>> > > prioritization in the software data path. VLAN devices have an
>> > > ingress-qos-map and an egress-qos-map. How would prioritization done via
>> > > dcbnl interact with those (who would take precedence)?
>> >
>> > Hi Vladimir,
>> >
>> > They shouldn't interact (at least this is my understanding).
>> >
>> > The ingress and egress maps are for vlan interfaces, and dcb operates
>> > on physical interfaces (dcbx too). You cannot use dcbnl to do
>> > prioritization for vlan interfaces.
>> >
>> > Anyway, I think much of the stuff in DCB is for hw offload only, incl. the
>> > topic at hand. Is the APP table even consulted by the sw stack at all - I
>> > dont think so (apart from drivers).
>>
>> Not directly, but at least ocelot (or in fact felix) does set
>> skb->priority based on the QoS class from the Extraction Frame Header.
>> So the stack does end up consulting and meaningfully using something
>> that was set based on the dcbnl APP table.
>>
>> In this sense, for ocelot, there is a real overlap between skb->priority
>> being initially set based on ocelot_xfh_get_qos_class(), and later being
>> overwritten based on the QoS maps of a VLAN interface.
I do not think this is an overlap.

The mappings Daniel is proposing will cause the QoS class from the
extraction header to be mapped (actually they are mapped today, the
mapping is just a 1:1). If the frame is extracted by the CPU, the
classified QoS class will (and shall) be used to set the skb->priority.

As long as the frame is considered to belong to the physical interface,
this is correct.

If this frame then at a later point is being processed by a VLAN
interface, then the mapping will be updated in the context if the VLAN
interface.

>Right, so VLAN QoS maps currently takes precedence, if somebody would choose
>to add a tagged vlan interface on-top of a physical interface with existing
>PCP prioritization - is this a real problem, or just a question of configuration?
It is not like VLAN QoS maps takes precedence. The mapping is just
updated if the frame goes through a (SW) VLAN interface.

>> The problem with the ingress-qos-map and egress-qos-map from 802.1Q that
>> I see is that they allow for per-VID prioritization, which is way more
>> fine grained than what we need. This, plus the fact that bridge VLANs
>> don't have this setting, only termination (8021q) VLANs do. How about an
>> ingress-qos-map and an egress-qos-map per port rather than per VID,
>> potentially even a bridge_slave netlink attribute, offloadable through
>> switchdev? We could make the bridge input fast path alter skb->priority
>> for the VLAN-tagged code paths, and this could give us superior
>> semantics compared to putting this non-standardized knob in the hardware
>> only dcbnl.
>
>This is a valid alternative solution to dcbnl, although this seems to be a
>much more complex solution, to something that is easily solved in dcbnl,
>and actually is in-line with what dcbnl already does. On-top of this, the
>notion of 'trust' has also been raised by this topic. It makes a lot of sense
>to me to add APP selector trust and trust order to dcbnl.
I agree that this mapping could be done by allowing to set the
ingress-qos-map/egress-qos-map maps on physical interfaces (and to
handle the trust aspect, we would have to introduce a anoter flag which
does not make sense on VLAN interfaces, but only on physical
interfaces).

Still I think it is better to do it in the dcb-app (and skip the
sw-fallback) for the following reasons:
- This mapping is important to supprot meaning use-cases with PFC
   (Priority Flow Control) Buffer handling and in furture maybe
   FramePreemption. These are all features which cannot be done in a
   meaningfull way in SW. Therefore the SW-path will not bennefit from
   this (but rahter will be be slowed down by supporting more features).
- This is closely related to others features found DCB, and as a user I
   just think it make sense to collect related features in the same
   tool and man-page.

>This is the solution that I have been implementing lately, and is ready
>for posting very soon.
Given that this is mostly implemented in accordance with the initial
discussion with Petr, I think we should see it and evaluate it. If we
think it is better to move the ingress-qos-map/egress-qos-map then we
need to try out that.

/Allan

  reply	other threads:[~2022-09-08  8:05 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
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 [this message]
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=20220908080358.islaqh4irglbyp4s@lx-anielsen \
    --to=allan.nielsen@microchip.com \
    --cc=Daniel.Machon@microchip.com \
    --cc=kuba@kernel.org \
    --cc=maxime.chevallier@bootlin.com \
    --cc=netdev@vger.kernel.org \
    --cc=petrm@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).