netdev - Re: [patch net-next 00/10] net: allow user specify TC filter HW stats type

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: <20200225100625.01887735@kicinski-fedora-pc1c0hjn.dhcp.thefacebook.com>
Date:   Tue, 25 Feb 2020 10:06:25 -0800
From:   Jakub Kicinski <kuba@...nel.org>
To:     Jiri Pirko <jiri@...nulli.us>
Cc:     Jamal Hadi Salim <jhs@...atatu.com>,
        Edward Cree <ecree@...arflare.com>, netdev@...r.kernel.org,
        davem@...emloft.net, saeedm@...lanox.com, leon@...nel.org,
        michael.chan@...adcom.com, vishal@...lsio.com,
        jeffrey.t.kirsher@...el.com, idosch@...lanox.com,
        aelior@...vell.com, peppe.cavallaro@...com,
        alexandre.torgue@...com, xiyou.wangcong@...il.com,
        pablo@...filter.org, mlxsw@...lanox.com,
        Marian Pritsak <marianp@...lanox.com>
Subject: Re: [patch net-next 00/10] net: allow user specify TC filter HW
 stats type

On Tue, 25 Feb 2020 17:22:03 +0100 Jiri Pirko wrote:
> >> You can reuse this index in multiple counter action instances.  
> >
> >That is great because it maps to tc semantics. When you create
> >an action of the same type, you can specify the index and it
> >is re-used. Example:
> >
> >sudo tc filter add dev lo parent ffff: protocol ip prio 8 u32 \
> >match ip dst 127.0.0.8/32 flowid 1:8 \
> >action vlan push id 8 protocol 802.1q index 8\
> >action mirred egress mirror dev eth0 index 111
> >
> >sudo tc filter add dev lo parent ffff: protocol ip prio 8 u32 \
> >match ip dst 127.0.0.15/32 flowid 1:10 \
> >action vlan push id 15 protocol 802.1q index 15 \
> >action mirred egress mirror index 111 \
> >action drop index 111
> >
> >So for the shared mirror action the counter is shared
> >by virtue of specifying index 111.
> >
> >What tc _doesnt allow_ is to re-use the same
> >counter index across different types of actions (example
> >mirror index 111 is not the same instance as drop 111).
> >Thats why i was asking if you are exposing the hw index.  
> 
> User does not care about any "hw index". That should be abstracted out
> by the driver.

+1

> >> and we report stats from action_counter for all the_actual_actionX.  
> >
> >This may not be accurate if you are branching - for example
> >a policer or quota enforcer which either accepts or drops or sends next
> >to a marker action etc .
> >IMO, this was fine in the old days when you had one action per match.
> >Best is to leave it to whoever creates the policy to decide what to
> >count. IOW, I think modelling it as a pipe or ok or drop or continue
> >and be placed anywhere in the policy graph instead of the begining.  
> 
> Eh, that is not that simple. The existing users are used to the fact
> that the actions are providing counters by themselves. Having and
> explicit counter action like this would break that expectation.
> Also, I think it should be up to the driver implementation. Some HW
> might only support stats per rule, not the actions. Driver should fit
> into the existing abstraction, I think it is fine.

+1