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]

Message-ID: <ygnha6io9uef.fsf@nvidia.com>
Date:   Mon, 1 Nov 2021 10:01:28 +0200
From:   Vlad Buslov <vladbu@...dia.com>
To:     Jamal Hadi Salim <jhs@...atatu.com>
CC:     Oz Shlomo <ozsh@...dia.com>,
        Simon Horman <simon.horman@...igine.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 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:
>>> Baowen Zheng says:
>>>
>>> Allow use of flow_indr_dev_register/flow_indr_dev_setup_offload to offload
>>> tc actions independent of flows.
>>>
>>> The motivation for this work is to prepare for using TC police action
>>> instances to provide hardware offload of OVS metering feature - which calls
>>> for policers that may be used by multiple flows and whose lifecycle is
>>> independent of any flows that use them.
>>>
>>> This patch includes basic changes to offload drivers to return EOPNOTSUPP
>>> if this feature is used - it is not yet supported by any driver.
>>>
>>> Tc cli command to offload and quote an action:
>>>
>>> tc qdisc del dev $DEV ingress && sleep 1 || true
>>> tc actions delete action police index 99 || true
>>>
>>> tc qdisc add dev $DEV ingress
>>> tc qdisc show dev $DEV ingress
>>>
>>> tc actions add action police index 99 rate 1mbit burst 100k skip_sw
>>> tc actions list action police
>>>
>>> tc filter add dev $DEV protocol ip parent ffff:
>>> flower ip_proto tcp action police index 99
>>> tc -s -d filter show dev $DEV protocol ip parent ffff:
>>> tc filter add dev $DEV protocol ipv6 parent ffff:
>>> flower skip_sw ip_proto tcp action police index 99
>>> tc -s -d filter show dev $DEV protocol ipv6 parent ffff:
>>> tc actions list action police
>>>
>>> tc qdisc del dev $DEV ingress && sleep 1
>>> tc actions delete action police index 99
>>> tc actions list action police
>>>
>> 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.