netdev - Re: [PATCH v2 net-next 0/7] dpaa2-eth: add support for Rx traffic classes

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20200519143525.136d3c3c@kicinski-fedora-pc1c0hjn.dhcp.thefacebook.com>
Date:   Tue, 19 May 2020 14:35:25 -0700
From:   Jakub Kicinski <kuba@...nel.org>
To:     Ioana Ciornei <ioana.ciornei@....com>
Cc:     "davem@...emloft.net" <davem@...emloft.net>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>
Subject: Re: [PATCH v2 net-next 0/7] dpaa2-eth: add support for Rx traffic
 classes

On Tue, 19 May 2020 20:58:50 +0000 Ioana Ciornei wrote:
> > 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).

You're jumping to classification while the configuration of the queues
itself is still not defined. How does the user know how many queues
there are to classify into?

Does this driver has descriptor rings for RX / completion? How does it
decide which queue to pool at NAPI time?