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: <CAM0EoMm046Ur8o6g3FwMCKB-_p246rpqfDYgWnRsuXHBhruDpg@mail.gmail.com>
Date:   Thu, 12 Jan 2023 07:24:45 -0500
From:   Jamal Hadi Salim <jhs@...atatu.com>
To:     Paul Blakey <paulb@...dia.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 Thu, Jan 12, 2023 at 5:59 AM Paul Blakey <paulb@...dia.com> wrote:
>
> Hi,
>
> This series adds support for hardware miss to a specific tc action
> instance on a filter's action list. The mlx5 driver patch (besides
> the refactors) shows its usage instead of using just chain restore.
>
> This miss to action supports partial offload of a filter's action list,
> and and let software continue processing where hardware left off.
>
> Example is the CT action, where new connections need to be handled in
> software. And if there is a packet modifying action before the CT action,
> then restoring only the chain on misses might cause the rule to not
> re-execute the relevant filter in software.
>
> Consider the following scenario:
>
> $ tc filter add dev dev1 ingress chain 0 proto ip flower \
>   ct_state -trk dst_mac fe:50:56:26:13:7d \
>   action pedit ex munge eth dst aa:bb:cc:dd:ee:01 \
>   action ct \
>   action goto chain 1
> $ tc filter add dev dev1 ingress chain 1 proto ip flower \
>   ct_state +trk+est \
>   action mirred egress redirect dev ...
> $ tc filter add dev dev1 ingress chain 1 proto ip flower \
>   ct_state +trk+new \
>   action ct commit \
>   action mirred egress redirect dev dev2
>
> $ tc filter add dev dev2 ingress chain 0 proto ip flower \
>   action ct \
>   action mirred egress redirect dev dev1
>
> A packet doing the pedit in hardware (setting dst_mac to aa:bb:cc:dd:ee:01),
> missing in the ct action, and restarting in chain 0 in software will fail
> matching the original dst_mac in the flower filter on chain 0.


I had to read that a couple of times and didnt get it until i went and
started looking at the patches and i am not 100% sure still.. IIUC, you have
something like:
match X action A action B action C
You do action A in HW and then you want to continue with B and C in SW.
If that is correct then the cover letter would be an easier read if it
is laid out as
follows:

-"consider the following scenario" (with examples you had above)....
Actually those example were not clear, i think the reader is meant to
assume lack of skip_sw means they are available both in hw and sw.

-Explain your goals and why it wont work (I think you did that well above)

-"and now with the changes in this patchset, the issue can be resolved
with rearranging the policy setup as follows"...

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 above scenario is supported in mlx5 driver by reordering the actions
> so ct will be done in hardware before the pedit action, but some packet
> modifications can't be reordered in regards to the ct action. An example
> of that is a modification to the tuple fields (e.g action pedit ex munge ip
> dst 1.1.1.1) since it affects the ct action's result (as it does lookup based
> on ip).

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?

cheers,
jamal