Re: [PATCH net-next v2 0/8] Introduce conntrack offloading to the nfp driver

[Louis Peens <louis.peens@corigine.com>]

On 2021/05/31 20:20, Marcelo Ricardo Leitner wrote:
> On Mon, May 31, 2021 at 02:45:59PM +0200, Simon Horman wrote:
>> Louis Peens says:
>>
>> This is the first in a series of patches to offload conntrack
>> to the nfp. The approach followed is to flatten out three
>> different flow rules into a single offloaded flow. The three
>> different flows are:
>>
>> 1) The rule sending the packet to conntrack (pre_ct)
>> 2) The rule matching on +trk+est after a packet has been through
>>    conntrack. (post_ct)
> 
> I think this part (matching on +trk+est) was left to another series,
> but anyway, supporting only +trk+est is not very effective, btw.
> +rpl/-rpl is also welcomed.
The plan is to expand to other flags in the future as well, thanks
for highlighting these specific ones, they will likely be investigated
next after all the patches of the current version has been released.
> 
>> 3) The rule received via callback from the netfilter (nft)
>>
>> In order to offload a flow we need a combination of all three flows, but
>> they could be added/deleted at different times and in different order.
>>
>> To solve this we save potential offloadable CT flows in the driver,
>> and every time we receive a callback we check against these saved flows
>> for valid merges. Once we have a valid combination of all three flows
>> this will be offloaded to the NFP. This is demonstrated in the diagram
>> below.
>>
>> 	+-------------+                      +----------+
>> 	| pre_ct flow +--------+             | nft flow |
>> 	+-------------+        v             +------+---+
>> 	                  +----------+              |
>> 	                  | tc_merge +--------+     |
>> 	                  +----------+        v     v
>> 	+--------------+       ^           +-------------+
>> 	| post_ct flow +-------+       +---+nft_tc merge |
>> 	+--------------+               |   +-------------+
>> 	                               |
>> 	                               |
>> 	                               |
>> 	                               v
>> 	                        Offload to nfp
> 
> Sounds like the offloading of new conntrack entries is quite heavy
> this way. Hopefully not.
This is can indeed tend towards the heavy side, there is likely room for some
performance enhancements in the future, but it does seem to work well enough
in the scenarios we've encountered so far.

Thanks for the input
> 
>>
>> This series is only up to the point of the pre_ct and post_ct
>> merges into the tc_merge. Follow up series will continue
>> to add the nft flows and merging of these flows with the result
>> of the pre_ct and post_ct merged flows.
>>
>> Changes since v1:
>> - nfp: flower-ct: add ct zone table
>>     Fixed unused variable compile warning
>>     Fixed missing colon in struct description
>>
>> Louis Peens (8):
>>   nfp: flower: move non-zero chain check
>>   nfp: flower-ct: add pre and post ct checks
>>   nfp: flower-ct: add ct zone table
>>   nfp: flower-ct: add zone table entry when handling pre/post_ct flows
>>   nfp: flower-ct: add nfp_fl_ct_flow_entries
>>   nfp: flower-ct: add a table to map flow cookies to ct flows
>>   nfp: flower-ct: add tc_merge_tb
>>   nfp: flower-ct: add tc merge functionality
>>
>>  drivers/net/ethernet/netronome/nfp/Makefile   |   3 +-
>>  .../ethernet/netronome/nfp/flower/conntrack.c | 486 ++++++++++++++++++
>>  .../ethernet/netronome/nfp/flower/conntrack.h | 155 ++++++
>>  .../net/ethernet/netronome/nfp/flower/main.h  |   6 +
>>  .../ethernet/netronome/nfp/flower/metadata.c  | 101 +++-
>>  .../ethernet/netronome/nfp/flower/offload.c   |  31 +-
>>  6 files changed, 775 insertions(+), 7 deletions(-)
>>  create mode 100644 drivers/net/ethernet/netronome/nfp/flower/conntrack.c
>>  create mode 100644 drivers/net/ethernet/netronome/nfp/flower/conntrack.h
>>
>> --
>> 2.20.1
>>
>