netdev - Re: [RFC/PATCH net-next v3 0/8] allow user to offload tc action to net device

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]

Date:   Tue, 2 Nov 2021 17:33:14 +0200
From:   Vlad Buslov <vladbu@...dia.com>
To:     Simon Horman <simon.horman@...igine.com>
CC:     Jamal Hadi Salim <jhs@...atatu.com>, Oz Shlomo <ozsh@...dia.com>,
        <netdev@...r.kernel.org>, Roi Dayan <roid@...dia.com>,
        Ido Schimmel <idosch@...dia.com>,
        Cong Wang <xiyou.wangcong@...il.com>,
        Jiri Pirko <jiri@...nulli.us>,
        Baowen Zheng <notifications@...hub.com>,
        Louis Peens <louis.peens@...igine.com>,
        <oss-drivers@...igine.com>
Subject: Re: [RFC/PATCH net-next v3 0/8] allow user to offload tc action to
 net device

On Tue 02 Nov 2021 at 14:51, Simon Horman <simon.horman@...igine.com> wrote:
> On Mon, Nov 01, 2021 at 10:01:28AM +0200, Vlad Buslov wrote:
>> On Sun 31 Oct 2021 at 15:40, Jamal Hadi Salim <jhs@...atatu.com> wrote:
>> > On 2021-10-31 05:50, Oz Shlomo wrote:
>> >> 
>> >> On 10/28/2021 2:06 PM, Simon Horman wrote:
>
> ...
>
>> >> Actions are also (implicitly) instantiated when filters are created.
>> >> In the following example the mirred action instance (created by the first
>> >> filter) is shared by the second filter:
>> >> tc filter add dev $DEV1 proto ip parent ffff: flower \
>> >>      ip_proto tcp action mirred egress redirect dev $DEV3
>> >> tc filter add dev $DEV2 proto ip parent ffff: flower \
>> >>      ip_proto tcp action mirred index 1
>> >> 
>> >
>> > I sure hope this is supported. At least the discussions so far
>> > are a nod in that direction...
>> > I know there is hardware that is not capable of achieving this
>> > (little CPE type devices) but lets not make that the common case.
>> 
>> Looks like it isn't supported in this change since
>> tcf_action_offload_add() is only called by tcf_action_init() when BIND
>> flag is not set (the flag is always set when called from cls code).
>> Moreover, I don't think it is good idea to support such use-case because
>> that would require to increase number of calls to driver offload
>> infrastructure from 1 per filter to 1+number_of_actions, which would
>> significantly impact insertion rate.
>
> Hi,
>
> I feel that I am missing some very obvious point here.
>
> But from my perspective the use case described by Oz is supported
> by existing offload of the flower classifier (since ~4.13 IIRC).

Mlx5 driver can't support such case without infrastructure change in
kernel for following reasons:

- Action index is not provided by flow_action offload infrastructure for
  most of the actions, so there is no way for driver to determine
  whether the action is shared.

- If we extend the infrastructure to always provide tcfa_index (a
  trivial change), there would be not much use for it because there is
  no way to properly update shared action counters without
  infrastructure code similar to what you implemented as part of this
  series.

How do you support shared actions created through cls_api in your
driver, considering described limitations?