| 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
| ||
|
Message-ID: <20230210015607.xjq2gorwpg4q3zxv@t14s.localdomain>
Date: Thu, 9 Feb 2023 17:56:07 -0800
From: Marcelo Ricardo Leitner <marcelo.leitner@...il.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>,
Jamal Hadi Salim <jhs@...atatu.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 v9 0/7] net/sched: cls_api: Support hardware
miss to tc action
Hi,
On Mon, Feb 06, 2023 at 07:43:56PM +0200, Paul Blakey wrote:
> Hi,
>
> This series adds support for hardware miss to instruct tc to continue execution
> in 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.
>
> Currently a filter's action list must be executed all together or
> not at all as driver are only able to tell tc to continue executing from a
> specific tc chain, and not a specific filter/action.
>
> This is troublesome with regards to action CT, where new connections should
> be sent to software (via tc chain restore), and established connections can
> be handled in hardware.
>
> Checking for new connections is done when executing the ct action in hardware
> (by checking the packet's tuple against known established tuples).
> But if there is a packet modification (pedit) action before action CT and the
> checked tuple is a new connection, hardware will need to revert the previous
> packet modifications before sending it back to software so it can
> re-match the same tc filter in software and re-execute its CT action.
>
> The following is an example configuration of stateless nat
> on mlx5 driver that isn't supported before this patchet:
>
> #Setup corrosponding mlx5 VFs in namespaces
> $ ip netns add ns0
> $ ip netns add ns1
> $ ip link set dev enp8s0f0v0 netns ns0
> $ ip netns exec ns0 ifconfig enp8s0f0v0 1.1.1.1/24 up
> $ ip link set dev enp8s0f0v1 netns ns1
> $ ip netns exec ns1 ifconfig enp8s0f0v1 1.1.1.2/24 up
>
> #Setup tc arp and ct rules on mxl5 VF representors
> $ tc qdisc add dev enp8s0f0_0 ingress
> $ tc qdisc add dev enp8s0f0_1 ingress
> $ ifconfig enp8s0f0_0 up
> $ ifconfig enp8s0f0_1 up
>
> #Original side
> $ tc filter add dev enp8s0f0_0 ingress chain 0 proto ip flower \
> ct_state -trk ip_proto tcp dst_port 8888 \
> action pedit ex munge tcp dport set 5001 pipe \
> action csum ip tcp pipe \
> action ct pipe \
> action goto chain 1
> $ tc filter add dev enp8s0f0_0 ingress chain 1 proto ip flower \
> ct_state +trk+est \
> action mirred egress redirect dev enp8s0f0_1
> $ tc filter add dev enp8s0f0_0 ingress chain 1 proto ip flower \
> ct_state +trk+new \
> action ct commit pipe \
> action mirred egress redirect dev enp8s0f0_1
> $ tc filter add dev enp8s0f0_0 ingress chain 0 proto arp flower \
> action mirred egress redirect dev enp8s0f0_1
>
> #Reply side
> $ tc filter add dev enp8s0f0_1 ingress chain 0 proto arp flower \
> action mirred egress redirect dev enp8s0f0_0
> $ tc filter add dev enp8s0f0_1 ingress chain 0 proto ip flower \
> ct_state -trk ip_proto tcp \
> action ct pipe \
> action pedit ex munge tcp sport set 8888 pipe \
> action csum ip tcp pipe \
> action mirred egress redirect dev enp8s0f0_0
>
> #Run traffic
> $ ip netns exec ns1 iperf -s -p 5001&
> $ sleep 2 #wait for iperf to fully open
> $ ip netns exec ns0 iperf -c 1.1.1.2 -p 8888
>
> #dump tc filter stats on enp8s0f0_0 chain 0 rule and see hardware packets:
> $ tc -s filter show dev enp8s0f0_0 ingress chain 0 proto ip | grep "hardware.*pkt"
> Sent hardware 9310116832 bytes 6149672 pkt
> Sent hardware 9310116832 bytes 6149672 pkt
> Sent hardware 9310116832 bytes 6149672 pkt
I see Jamal had asked about stats on the other version, but then no
dependency was set. I think we _must_ have a dependency of this
patchet on the per-action stats one. Otherwise the stats above will
get messy. Without the per-action stats, the last one is replicated
to the other actions. But then, will hw count the packet that it did
only the first action? I don't see how it would, and then for the all
but first one the packet will be accounted twice.
With this said, it would be nice to provide a sample of how the sw and
hw stats would look like _after_ this patchset as well.
Btw I'll add my Reviewed-by tag to the per-action stats one in a few.
>
> A new connection executing the first filter in hardware will first rewrite
> the dst port to the new port, and then the ct action is executed,
> because this is a new connection, hardware will need to be send this back
> to software, on chain 0, to execute the first filter again in software.
> The dst port needs to be reverted otherwise it won't re-match the old
> dst port in the first filter. Because of that, currently mlx5 driver will
> reject offloading the above action ct rule.
>
> This series adds supports partial offload of a filter's action list,
We should avoid this terminology as is, as it can create confusion. It
is not that it is offloading action 1 and not action 2. Instead, it is
adding support to a more fine grained miss to sw. Perhaps "support for
partially executing in hw".
> and letting tc software continue processing in the specific action instance
> where hardware left off (in the above case after the "action pedit ex munge tcp
> dport... of the first rule") allowing support for scenarios such as the above.
>
> Changelog:
> v1->v2:
> Fixed compilation without CONFIG_NET_CLS
> Cover letter re-write
>
> v2->v3:
> Unlock spin_lock on error in cls flower filter handle refactor
> Cover letter
>
> v3->v4:
> Silence warning by clang
>
> v4->v5:
> Cover letter example
> Removed ifdef as much as possible by using inline stubs
>
> v5->v6:
> Removed new inlines in cls_api.c (bot complained in patchwork)
> Added reviewed-by/ack - Thanks!
>
> v6->v7:
> Removed WARN_ON from pkt path (leon)
> Removed unnecessary return in void func
>
> v7->v8:
> Removed #if IS_ENABLED on skb ext adding Kconfig changes
> Complex variable init in seperate lines
> if,else if, else if ---> switch case
>
> v8->v9:
> Removed even more IS_ENABLED because of Kconfig
>
> Paul Blakey (7):
> net/sched: cls_api: Support hardware miss to tc action
> net/sched: flower: Move filter handle initialization earlier
> net/sched: flower: Support hardware miss to tc action
> net/mlx5: Kconfig: Make tc offload depend on tc skb extension
> net/mlx5: Refactor tc miss handling to a single function
> net/mlx5e: Rename CHAIN_TO_REG to MAPPED_OBJ_TO_REG
> net/mlx5e: TC, Set CT miss to the specific ct action instance
>
> .../net/ethernet/mellanox/mlx5/core/Kconfig | 4 +-
> .../ethernet/mellanox/mlx5/core/en/rep/tc.c | 225 ++------------
> .../mellanox/mlx5/core/en/tc/sample.c | 2 +-
> .../ethernet/mellanox/mlx5/core/en/tc_ct.c | 39 +--
> .../ethernet/mellanox/mlx5/core/en/tc_ct.h | 2 +
> .../net/ethernet/mellanox/mlx5/core/en_rx.c | 4 +-
> .../net/ethernet/mellanox/mlx5/core/en_tc.c | 280 ++++++++++++++++--
> .../net/ethernet/mellanox/mlx5/core/en_tc.h | 23 +-
> .../net/ethernet/mellanox/mlx5/core/eswitch.h | 2 +
> .../mellanox/mlx5/core/lib/fs_chains.c | 14 +-
> include/linux/skbuff.h | 6 +-
> include/net/flow_offload.h | 1 +
> include/net/pkt_cls.h | 34 ++-
> include/net/sch_generic.h | 2 +
> net/openvswitch/flow.c | 3 +-
> net/sched/act_api.c | 2 +-
> net/sched/cls_api.c | 213 ++++++++++++-
> net/sched/cls_flower.c | 73 +++--
> 18 files changed, 602 insertions(+), 327 deletions(-)
>
> --
> 2.30.1
>
Powered by blists - more mailing lists