> 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

Reply via email to