Subject: Re: [PATCH net-next v5 6/7] net/sched: act_ct: offload UDP NEW connections

From: Vlad Buslov <[email-masked]>
Date: Sat, 28 Jan 2023 17:31:40 +0200

On Sat 28 Jan 2023 at 16:26, Pablo Neira Ayuso <[email-masked]> 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?