[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAA93jw4VphJ17yoV1S6aDRg2=W7hg=02Yr3XcX_aEBTzAt0ezw@mail.gmail.com>
Date: Sun, 31 Oct 2021 05:03:19 -0700
From: Dave Taht <dave.taht@...il.com>
To: Oz Shlomo <ozsh@...dia.com>
Cc: Simon Horman <simon.horman@...igine.com>,
Linux Kernel Network Developers <netdev@...r.kernel.org>,
Vlad Buslov <vladbu@...dia.com>,
Jamal Hadi Salim <jhs@...atatu.com>,
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, Oct 31, 2021 at 2:51 AM Oz Shlomo <ozsh@...dia.com> 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
>
>
> > Changes compared to v2 patches:
> >
> > * Made changes according to the review comments.
> > * Delete in_hw and not_in_hw flag and user can judge if the action is
> > offloaded to any hardware by in_hw_count.
> > * Split the main patch of the action offload to three single patch to
> > facilitate code review.
> >
> > Posting this revision of the patchset as an RFC as while we feel it is
> > ready for review we would like an opportunity to conduct further testing
> > before acceptance into upstream.
> >
> > Baowen Zheng (8):
> > flow_offload: fill flags to action structure
> > flow_offload: reject to offload tc actions in offload drivers
> > flow_offload: allow user to offload tc action to net device
> > flow_offload: add skip_hw and skip_sw to control if offload the action
> > flow_offload: add process to update action stats from hardware
> > net: sched: save full flags for tc action
> > flow_offload: add reoffload process to update hw_count
> > flow_offload: validate flags of filter and actions
> >
> > drivers/net/ethernet/broadcom/bnxt/bnxt_tc.c | 2 +-
> > .../ethernet/mellanox/mlx5/core/en/rep/tc.c | 3 +
> > .../ethernet/netronome/nfp/flower/offload.c | 3 +
> > include/linux/netdevice.h | 1 +
> > include/net/act_api.h | 34 +-
> > include/net/flow_offload.h | 17 +
> > include/net/pkt_cls.h | 61 ++-
> > include/uapi/linux/pkt_cls.h | 9 +-
> > net/core/flow_offload.c | 48 +-
> > net/sched/act_api.c | 440 +++++++++++++++++-
> > net/sched/act_bpf.c | 2 +-
> > net/sched/act_connmark.c | 2 +-
> > net/sched/act_ctinfo.c | 2 +-
> > net/sched/act_gate.c | 2 +-
> > net/sched/act_ife.c | 2 +-
> > net/sched/act_ipt.c | 2 +-
> > net/sched/act_mpls.c | 2 +-
> > net/sched/act_nat.c | 2 +-
> > net/sched/act_pedit.c | 2 +-
> > net/sched/act_police.c | 2 +-
> > net/sched/act_sample.c | 2 +-
> > net/sched/act_simple.c | 2 +-
> > net/sched/act_skbedit.c | 2 +-
> > net/sched/act_skbmod.c | 2 +-
> > net/sched/cls_api.c | 55 ++-
> > net/sched/cls_flower.c | 3 +-
> > net/sched/cls_matchall.c | 4 +-
> > net/sched/cls_u32.c | 7 +-
> > 28 files changed, 661 insertions(+), 54 deletions(-)
> >
Just as an on-going grump: It has been my hope that policing as a
technique would have died a horrible death by now. Seeing it come back
as an "easy to offload" operation here - fresh from the 1990s! does
not mean it's a good idea, and I'd rather like it if we were finding
ways to
offload newer things that work better, such as modern aqm, fair
queuing, and shaping technologies that are in pie, fq_codel, and cake.
policing leads to bursty loss, especially at higher rates, BBR has a
specific mode designed to defeat it, and I ripped it out of
wondershaper
long ago for very good reasons:
https://www.bufferbloat.net/projects/bloat/wiki/Wondershaper_Must_Die/
I did a long time ago start working on a better policing idea based on
some good aqm ideas like AFD, but dropped it figuring that policing
was going to vanish
from the planet. It's baaaaaack.
--
I tried to build a better future, a few times:
https://wayforward.archive.org/?site=https%3A%2F%2Fwww.icei.org
Dave Täht CEO, TekLibre, LLC
Powered by blists - more mailing lists