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: <51066434-3973-9b19-85a6-e57fcd67af79@nvidia.com>
Date:   Wed, 3 Nov 2021 12:56:06 +0200
From:   Oz Shlomo <ozsh@...dia.com>
To:     Simon Horman <simon.horman@...igine.com>,
        Vlad Buslov <vladbu@...dia.com>
Cc:     Jamal Hadi Salim <jhs@...atatu.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 11/2/2021 6:15 PM, Simon Horman wrote:
> On Tue, Nov 02, 2021 at 05:33:14PM +0200, Vlad Buslov wrote:
>> 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?
> 
> Thanks,
> 
> I misread the use case described by Oz, but I believe I understand it now.
> 
> I agree that the case described is neither currently supported, nor
> supported by this patchset (to be honest I for one had not considered it).
> 
> So, I think the question is: does upporting this use-case make sense - from
> implementation, use-case, and consistency perspectives - in the context of
> this patchset?
> 
> Am I on the right track?
> 

Currently we don't have a specific application use case for sharing actions that were created by tc 
filters. However, we do have future use case in mind.
We could add such functionality on top of this series when a use case will materialize.
Perhaps, at that point, we can also introduce a control flag in order to avoid unnecessary insertion 
rate performance degradation.