On 19/08/2022 02:04, Pablo Neira Ayuso wrote:
Hi Paul,
On Thu, Aug 18, 2022 at 10:27:54AM +0300, Paul Blakey wrote:
[...]
diff --git a/net/netfilter/nf_flow_table_core.c b/net/netfilter/nf_flow_table_core.c
index f2def06d1070..19fd3b5f8a1b 100644
--- a/net/netfilter/nf_flow_table_core.c
+++ b/net/netfilter/nf_flow_table_core.c
@@ -605,6 +605,7 @@ void nf_flow_table_free(struct nf_flowtable *flow_table)
mutex_unlock(&flowtable_lock);
cancel_delayed_work_sync(&flow_table->gc_work);
+ nf_flow_table_offload_flush(flow_table);
nf_flow_table_iterate(flow_table, nf_flow_table_do_cleanup, NULL);
nf_flow_table_iterate(flow_table, nf_flow_offload_gc_step, NULL);
nf_flow_table_offload_flush(flow_table);
patch looks very similar to:
https://patchwork.ozlabs.org/project/netfilter-devel/patch/1633854320-12326-1-git-send-email-volodymyr.mytnyk@xxxxxxxxxxx/
I proposed these two instead to avoid reiterative calls to flush from
the cleanup path (see attached).
It should be possible to either take your patch to nf.git (easier for
-stable backport), then look into my patches for nf-next.git, would
you pick up on these follow up?
Hi!
The only functional difference here (for HW table) is your patches call
flush just for the del workqueue instead of del/stats/add, right?
Because in the end you do:
cancel_delayed_work_sync(&flow_table->gc_work);
nf_flow_table_offload_flush(flow_table);
nf_flow_table_iterate(flow_table, nf_flow_table_do_cleanup, NULL);
nf_flow_table_gc_run(flow_table);
nf_flow_table_offload_flush_cleanup(flow_table);
resulting in the following sequence (after expending flush_cleanup()):
cancel_delayed_work_sync(&flow_table->gc_work);
nf_flow_table_offload_flush(flow_table);
nf_flow_table_iterate(flow_table, nf_flow_table_do_cleanup, NULL);
nf_flow_table_gc_run(flow_table);
flush_workqueue(nf_flow_offload_del_wq);
nf_flow_table_gc_run(flowtable);
Where as my (and Volodymyr's) patch does:
cancel_delayed_work_sync(&flow_table->gc_work);
nf_flow_table_offload_flush(flow_table);
nf_flow_table_iterate(flow_table, nf_flow_table_do_cleanup, NULL);
nf_flow_table_iterate(flow_table, nf_flow_offload_gc_step, NULL);
nf_flow_table_offload_flush(flow_table);
nf_flow_table_iterate(flow_table, nf_flow_offload_gc_step, NULL);
so almost identical, I don't see "extra reiterative calls to flush"
here, but I'm fine with just your patch as it's more efficient, can we
take yours to both gits?
Thanks,
Paul.
