diff mbox series

[nf] netfilter: flowtable: clean up entries for FLOW_BLOCK_UNBIND

Message ID 20191219135620.350881-1-pablo@netfilter.org
State Changes Requested
Delegated to: Pablo Neira
Headers show
Series [nf] netfilter: flowtable: clean up entries for FLOW_BLOCK_UNBIND | expand

Commit Message

Pablo Neira Ayuso Dec. 19, 2019, 1:56 p.m. UTC
Call nf_flow_table_iterate_cleanup() to remove flowtable entries.
This patch is implicitly handling the NETDEV_UNREGISTER and the
flowtable removal cases (while there are still entries in place).

Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
This patch supersedes: https://patchwork.ozlabs.org/patch/1213189/

 include/net/netfilter/nf_flow_table.h | 2 ++
 net/netfilter/nf_flow_table_core.c    | 4 ++--
 net/netfilter/nf_flow_table_offload.c | 3 +++
 3 files changed, 7 insertions(+), 2 deletions(-)

Comments

wenxu Dec. 20, 2019, 7:03 a.m. UTC | #1
On 12/19/2019 9:56 PM, Pablo Neira Ayuso wrote:
> Call nf_flow_table_iterate_cleanup() to remove flowtable entries.
> This patch is implicitly handling the NETDEV_UNREGISTER and the
> flowtable removal cases (while there are still entries in place).

Hi Pablo,

I  test the flowtable meeting the same problem with flowtable delete.


For NETDEV_UNREGISTER case there is no necessary to do this.

The flow_offload_netdev_event in the nft_flow_offload with NETDEV_DOWN(

previous to UNREGISTER) will do nf_flow_table_cleanup(dev);


For flowtable delete case, it will  do cleanup things in nf_flow_table_free, but this is

later than UNBIND. First it make UNBIND call after the free operation.

But only UNBIND setup before flows cleanup can't guarantee the flows
delete in the hardware. The real delete in nf_flow_offload_work handler.


I fix it through adding a refcont for the flow_block to make sure the hardware
flows clean before UNBIND setup. I test my patch with mellaonx card.

This patch is http://patchwork.ozlabs.org/patch/1213936/

welcome some commends and other solution idea. Thx!


BR

wenxu
diff mbox series

Patch

diff --git a/include/net/netfilter/nf_flow_table.h b/include/net/netfilter/nf_flow_table.h
index f0897b3c97fb..09a7bcbd3cd7 100644
--- a/include/net/netfilter/nf_flow_table.h
+++ b/include/net/netfilter/nf_flow_table.h
@@ -122,6 +122,8 @@  int flow_offload_route_init(struct flow_offload *flow,
 int flow_offload_add(struct nf_flowtable *flow_table, struct flow_offload *flow);
 struct flow_offload_tuple_rhash *flow_offload_lookup(struct nf_flowtable *flow_table,
 						     struct flow_offload_tuple *tuple);
+void nf_flow_table_iterate_cleanup(struct nf_flowtable *flowtable,
+				   struct net_device *dev);
 void nf_flow_table_cleanup(struct net_device *dev);
 
 int nf_flow_table_init(struct nf_flowtable *flow_table);
diff --git a/net/netfilter/nf_flow_table_core.c b/net/netfilter/nf_flow_table_core.c
index 9889d52eda82..9a7421e2b039 100644
--- a/net/netfilter/nf_flow_table_core.c
+++ b/net/netfilter/nf_flow_table_core.c
@@ -532,8 +532,8 @@  static void nf_flow_table_do_cleanup(struct flow_offload *flow, void *data)
 		flow_offload_dead(flow);
 }
 
-static void nf_flow_table_iterate_cleanup(struct nf_flowtable *flowtable,
-					  struct net_device *dev)
+void nf_flow_table_iterate_cleanup(struct nf_flowtable *flowtable,
+				   struct net_device *dev)
 {
 	nf_flow_table_offload_flush(flowtable);
 	nf_flow_table_iterate(flowtable, nf_flow_table_do_cleanup, dev);
diff --git a/net/netfilter/nf_flow_table_offload.c b/net/netfilter/nf_flow_table_offload.c
index c54c9a6cc981..506aaaf8151d 100644
--- a/net/netfilter/nf_flow_table_offload.c
+++ b/net/netfilter/nf_flow_table_offload.c
@@ -809,6 +809,9 @@  int nf_flow_table_offload_setup(struct nf_flowtable *flowtable,
 	struct flow_block_offload bo = {};
 	int err;
 
+	if (cmd == FLOW_BLOCK_UNBIND)
+		nf_flow_table_iterate_cleanup(flowtable, dev);
+
 	if (!(flowtable->flags & NF_FLOWTABLE_HW_OFFLOAD))
 		return 0;