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: <c149d5e1-e2a4-71c8-68e8-5a08b02a57f7@solarflare.com>
Date:   Thu, 27 Feb 2020 15:57:30 +0000
From:   Edward Cree <ecree@...arflare.com>
To:     Jiri Pirko <jiri@...nulli.us>, Jamal Hadi Salim <jhs@...atatu.com>
CC:     Jakub Kicinski <kuba@...nel.org>, <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>
Subject: Re: [patch net-next 00/10] net: allow user specify TC filter HW stats
 type

On 24/02/2020 16:25, Jiri Pirko wrote:
> In hardware, we have a separate "counter" action with counter index.
> You can reuse this index in multiple counter action instances.
> However, in tc there is implicit separate counter for every action.
Importantly, not all actions have counters.  Only those with a
 .stats_update() method in their struct tc_action_ops have an implicit
 counter (which requires hardware offload), and to a first approximation
 those are the 'deliverish' actions (pass, drop, mirred).
This informs the sfc design below.

> The counter is limited resource. So we overcome this mismatch in mlxsw
> by having action "counter" always first for every rule inserted:
> rule->action_counter,the_actual_action,the_actual_action2,...the_actual_actionN
>
> and we report stats from action_counter for all the_actual_actionX.
For comparison, here's how we're doing it in the upcoming sfc hardware:
Rather than a sequence of actions in an arbitrary order (applied
 cumulatively to the original packet), we have a concept of an 'action
 set', which is some subset of a fixed sequence of actions, the last of
 which is a delivery.  (One of these actions is 'count', which takes one
 or more counter IDs as its metadata.)  Then the result of a rule match
 is an _action set list_, one or more action sets each of which is
 applied to a separate copy of the original packet.
This works because the delivery (hw) action corresponds to the only (tc)
 action that wants stats, combined with some cleverness in the defined
 order of our fixed action sequence.  And it means that we can properly
 support per-action counters, including making stats type be a per-action
 property (if one of the mirreds doesn't want stats, just don't put a
 'count' in that action-set).
For shared counters we just use the same counter index.  Mapping from
 tc action index to hw counter index is handled in the driver (transparent
 to userspace).

So from our perspective, making stats-type a property of the TC action is
 the Right Thing and fits in with the existing design of TC.  See also my
 old RFC series restoring per-action stats to flow_offload [1] which (in
 patch 4/4) added a 'want_stats' field to struct flow_action_entry; this
 would presumably become an enum flow_cls_hw_stats_typeto support these
 new stats-type semantics, and would be set to
 FLOW_CLS_HW_STATS_TYPE_DISABLEDif act->ops->stats_update == NULL
 regardless of the stats-type specified by the user.

-ed

[1] http://patchwork.ozlabs.org/cover/1110071/