NACK/Cmnt: [SRU][F:linux-bluefield][PATCH 0/4] Connection tracking fixes

[Stefan Bader]

On 05.05.21 18:34, Daniel Jurgens wrote:
> These patches aren't neccsarily related, but are order dependent, so sending
> as a series. SRU justification for each are provided below.
> 
> Alaa Hleihel (2):
>    net/sched: act_ct: Make tcf_ct_flow_table_restore_skb inline
>    netfilter: flowtable: Make nf_flow_table_offload_add/del_cb inline
> 
> SRU Justification:
> 
> The patches are for removing module dependency between software module and driver module.
> 
> * Explain the bug(s)
> 
> Without the patches there is a dependency between mlx5_core, act_ct and nf_flow_table.
> 
> * brief explanation of fixes
> 
> The fix is moving used function from c source file to header file.
> 
> * How to test
> 
> Check module dependency with modinfo, lsmod, etc.
> 
> * What it could break.
> 
> If a sofwtare module doesn't exists, i.e. disabling in .config. then the driver module, mlx5_core,
> in this case will fail to load.
> 
> 
> 
> Paul Blakey (1):
>    netfilter: flowtable: Use rw sem as flow block lock
> 
> SRU Justification:
> 
> Increase flow insertion rate by using rw lock instead of mutex on the flow block.
> 
> * Explain the bug(s)
> 
> Insertion rate is done in multiple threads but there is a mutex lock between looking for the flow block
> which slows things down. So use a rw lock and take a read lock which is sufficient.
> 
> * brief explanation of fixes
> 
> Use a rw lock on the flow block.
> 
> * How to test
> 
> CT offload and check insertion rate of 5 tuples rules.
> 
> * What it could break.
> 
> Flow insertion rate.
> 
> 
> 
> Roi Dayan (1):
>    netfilter: flowtable: Free block_cb when being deleted
> 
> SRU Justification:
> 
> Free block_cb memory when asked to be deleted
> 
> * Explain the bug(s)
> 
> Missing memory cleanup.
> 
> * brief explanation of fixes
> 
> Free the memory
> 
> * How to test
> 
> Can be tested with kmemleak. Do ct offload then stop and clean tc ct rules and scan kmemleak.
> 
> * What it could break.
> 
> Memleak.
> 
>   include/net/netfilter/nf_flow_table.h | 51 +++++++++++++++++++++++++++++++----
>   include/net/tc_act/tc_ct.h            | 11 +++++++-
>   net/netfilter/nf_flow_table_core.c    | 46 +------------------------------
>   net/netfilter/nf_flow_table_offload.c |  4 +--
>   net/sched/act_ct.c                    | 11 --------
>   5 files changed, 59 insertions(+), 64 deletions(-)
> 
This mixes multiple bug reports in one submission. If the patches are required 
to be applied together, this should be one bug report. If things are independent 
the multiple bug reports would be correct but then each of the patches which 
belong to the same bug report should be submitted individually. Then those can 
be looked at and applied without relying on each other.

-Stefan

-------------- next part --------------
A non-text attachment was scrubbed...
Name: OpenPGP_signature
Type: application/pgp-signature
Size: 833 bytes
Desc: OpenPGP digital signature
URL: <https://lists.ubuntu.com/archives/kernel-team/attachments/20210511/ceb5ec49/attachment.sig>