netdev - Re: [PATCH net-next 0/4] net/sched: cls_api: Support hardware miss to tc action

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: <8f9ca91e-1f3f-c3c1-cbab-4f9af3884b43@nvidia.com>
Date:   Tue, 17 Jan 2023 16:48:23 +0200
From:   Paul Blakey <paulb@...dia.com>
To:     Jamal Hadi Salim <jhs@...atatu.com>
Cc:     netdev@...r.kernel.org, Saeed Mahameed <saeedm@...dia.com>,
        Paolo Abeni <pabeni@...hat.com>,
        Jakub Kicinski <kuba@...nel.org>,
        Eric Dumazet <edumazet@...gle.com>,
        Cong Wang <xiyou.wangcong@...il.com>,
        "David S. Miller" <davem@...emloft.net>,
        Oz Shlomo <ozsh@...dia.com>, Jiri Pirko <jiri@...dia.com>,
        Roi Dayan <roid@...dia.com>, Vlad Buslov <vladbu@...dia.com>
Subject: Re: [PATCH net-next 0/4] net/sched: cls_api: Support hardware miss to
 tc action



On 17/01/2023 15:40, Jamal Hadi Salim wrote:
> On Sun, Jan 15, 2023 at 7:04 AM Paul Blakey <paulb@...dia.com> wrote:
>>
>>
>>
>> On 12/01/2023 14:24, Jamal Hadi Salim wrote:
>>> On Thu, Jan 12, 2023 at 5:59 AM Paul Blakey <paulb@...dia.com> wrote:
> 
> [..]
> 
>>>
>>> Having said that: Would this have been equally achieved with using skbmark?
>>> The advantage is removing the need to change the cls sw datapath.
>>> The HW doesnt have to support skb mark setting, just driver transform.
>>> i.e the driver can keep track of the action mapping and set the mark when
>>> it receives the packet from hw.
>>> You rearrange your rules to use cls fw and have action B and C  match
>>> an skb mark.
>>>
>>
>> The user would then need to be familiar with how it works for this
>> specific vendor, of which actions are supported, and do so for all the
>> rules that have such actions. He will also need to add a
>> skb mark action after each such successful execution that the driver
>> should ignore.
>>
> 
> I agree that with your patch it will be operationally simpler. I hope other
> vendors will be able to use this feature (and the only reason i am saying
> this is because you are making core tc changes).
> 
> Question: How does someone adding these rules tell whether some of
> the actions are offloaded and some are not? If i am debugging this because
> something was wrong I would like to know.

Currently by looking at the per action hw stats, and if they are 
advancing. This is the same now with filters and the CT action for
new connections (driver reports offload, but it means that only for 
established connections).

> 
>> Also with this patchset we actually support tc action continue, where
>> with cls_fw it wont' be possible.
> 
> It will be an action continue for a scenario where (on ingress) you have
> action A from A,B,C being offloaded and B,C is in s/w - the fw filter
> will have the
> B,C and flower can have A offloaded.
> Yes, someone/thing programming these will have to know that only A can
> be offloaded
> in that graph.

I meant using continue to go to next tc priority "as in "action A action 
continue" but I'm not sure about the actual details of fully supporting 
this as its not the purpose of this patch, but maybe will lead there.



> 
> [..]
> 
>>> Also above text wasnt clear. It sounded like the driver would magically
>>> know that it has to continue action B and C in s/w because it knows the
>>> hardware wont be able to exec them?
>>
>> It means that if MLX5 driver gets "action A, action CT, action B",
>> where action CT is only possible in hardware for offloaded established
>> connections (offloaded via flow table).
>>
>> We reorder the actions and parse it as if the action list was: "action
>> CT, action A, action B" and then if we miss in action CT we didn't do
>> any modifications (actions A/B, and counters) in hardware.
>>
>> We can only do this if the actions are independent, and so to support
>> dependent actions (pedit for src ip followed by action CT), we have this
>> patchset.
> 
> Ok, so would this work for the scenario I described above? i.e A,B, C where
> A is offloaded but not B, C?

You mean the reorder? we reorder the CT action first if all other 
actions are supported, so we do all or nothing.


> 
> I also think the above text needs to go in the cover letter.
> 
> cheers,
> jamal