> Subject: Re: [PATCH v2 net-next 0/7] dpaa2-eth: add support for Rx traffic > classes > > On Tue, 19 May 2020 07:38:57 +0000 Ioana Ciornei wrote: > > > Subject: Re: [PATCH v2 net-next 0/7] dpaa2-eth: add support for Rx > > > traffic classes > > > > > > On Sat, 16 May 2020 08:16:47 +0000 Ioana Ciornei wrote: > > > > > With the Rx QoS features users won't even be able to tell via > > > > > standard Linux interfaces what the config was. > > > > > > > > Ok, that is true. So how should this information be exported to the > > > > user? > > > > > > I believe no such interface currently exists. > > > > I am having a bit of trouble understanding what should be the route > > for this feature to get accepted. > > What's the feature you're trying to get accepted? Driver's datapath to behave > correctly when some proprietary out-of-tree API is used to do the actual > configuration? Unexciting. >
I don't think this comment is very productive. The out-of-tree API has nothing to do with this. The NIC parameters like number of traffic classes and number of queues per traffic class are fixed as far as the Linux driver is concerned. What this patch set does is make use of the queues corresponding to traffic classes different than 0, if those exist. > > Is the problem having the classification to a TC based on the VLAN PCP > > or is there anything else? > > What you have is basically RX version of mqprio, right? Multiple rings per > "channel" each gets frames with specific priorities? You can think of it like that (maybe, understanding that mqprio cannot be attached to the ingress qdisc, indeed). DPAA2 has many peculiarities but I don't think this is one of them. There are some RX queues, some of which can be given a hardware priority at dequeue time and they form a traffic class together. The assumption being that you don't want low-priority traffic to cause congestion for high-priority traffic, to have lower latency for high-priority traffic, etc. Some 802.1Q standards like PFC actually depend on having multiple traffic classes too, based on VLAN PCP. > This needs to be well > integrated with the rest of the stack, but I don't think TC qdisc offload is > a fit. > Given we don't have qdiscs on ingress. As I said a new API for this would most > likely have to be created. For just assigning a traffic class based on packet headers a tc filter with the skbedit priority action on ingress is enough (maybe even too much since there are other drivers that have the same default prioritization based on VLAN PCP). But you are correct that this would not be enough to cover all possible use cases except for the most simple ones. There are per-traffic class ingress policers, and those become tricky to support since there's nothing that denotes the traffic class to match on, currently. I see 2 possible approaches, each with its own drawbacks: - Allow clsact to be classful, similar to mqprio, and attach filters to its classes (each class would correspond to an ingress traffic class). But this would make the skbedit action redundant, since QoS classification with a classful clsact should be done completely differently now. Also, the classful clsact would have to deny qdiscs attached to it that don't make sense, because all of those were written with egress in mind. - Try to linearize the ingress filter rules under the classless clsact, both the ones that have a skbedit action, and the ones that match on a skb priority in order to perform ingress policing. But this would be very brittle because the matching order would depend on the order in which the rules were introduced: rule 1: flower skb-priority 5 action police rate 34Mbps # note: matching on skb-priority doesn't exist (yet?) rule 2: flower vlan_prio 5 action skbedit priority 5 In this case, traffic with VLAN priority 5 would not get rate-limited to 34Mbps. So this is one of the reasons why I preferred to defer the hard questions and start with something simple (which for some reason still gets pushback). Ioana