[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <Y9Vy8Duq8nf/KeRW@salvia>
Date: Sat, 28 Jan 2023 20:09:36 +0100
From: Pablo Neira Ayuso <pablo@...filter.org>
To: Vlad Buslov <vladbu@...dia.com>
Cc: davem@...emloft.net, kuba@...nel.org, pabeni@...hat.com,
netdev@...r.kernel.org, netfilter-devel@...r.kernel.org,
jhs@...atatu.com, xiyou.wangcong@...il.com, jiri@...nulli.us,
ozsh@...dia.com, marcelo.leitner@...il.com,
simon.horman@...igine.com
Subject: Re: [PATCH net-next v5 6/7] net/sched: act_ct: offload UDP NEW
connections
On Sat, Jan 28, 2023 at 05:31:40PM +0200, Vlad Buslov wrote:
>
> On Sat 28 Jan 2023 at 16:26, Pablo Neira Ayuso <pablo@...filter.org> wrote:
> > Hi Vlad,
> >
> > On Fri, Jan 27, 2023 at 07:38:44PM +0100, Vlad Buslov wrote:
> >> Modify the offload algorithm of UDP connections to the following:
> >>
> >> - Offload NEW connection as unidirectional.
> >>
> >> - When connection state changes to ESTABLISHED also update the hardware
> >> flow. However, in order to prevent act_ct from spamming offload add wq for
> >> every packet coming in reply direction in this state verify whether
> >> connection has already been updated to ESTABLISHED in the drivers. If that
> >> it the case, then skip flow_table and let conntrack handle such packets
> >> which will also allow conntrack to potentially promote the connection to
> >> ASSURED.
> >>
> >> - When connection state changes to ASSURED set the flow_table flow
> >> NF_FLOW_HW_BIDIRECTIONAL flag which will cause refresh mechanism to offload
> >> the reply direction.
> >>
> >> All other protocols have their offload algorithm preserved and are always
> >> offloaded as bidirectional.
> >>
> >> Note that this change tries to minimize the load on flow_table add
> >> workqueue. First, it tracks the last ctinfo that was offloaded by using new
> >> flow 'ext_data' field and doesn't schedule the refresh for reply direction
> >> packets when the offloads have already been updated with current ctinfo.
> >> Second, when 'add' task executes on workqueue it always update the offload
> >> with current flow state (by checking 'bidirectional' flow flag and
> >> obtaining actual ctinfo/cookie through meta action instead of caching any
> >> of these from the moment of scheduling the 'add' work) preventing the need
> >> from scheduling more updates if state changed concurrently while the 'add'
> >> work was pending on workqueue.
> >
> > Could you use a flag to achieve what you need instead of this ext_data
> > field? Better this ext_data and the flag, I prefer the flags.
>
> Sure, np. Do you prefer the functionality to be offloaded to gc (as in
> earlier versions of this series) or leverage 'refresh' code as in
> versions 4-5?
No, I prefer generic gc/refresh mechanism is not used for this.
What I mean is: could you replace this new ->ext_data generic pointer
by a flag to annotate what you need? Between this generic pointer and
a flag, I prefer a flag.
Powered by blists - more mailing lists