Re: [PATCH net] netfilter: flowtable: Fix expired flow not being deleted from software

[Pablo Neira Ayuso <pablo@netfilter.org>]

[-- Attachment #1: Type: text/plain, Size: 716 bytes --]

On Wed, May 06, 2020 at 02:27:29PM +0300, Paul Blakey wrote:
> Once a flow is considered expired, it is marked as DYING, and
> scheduled a delete from hardware. The flow will be deleted from
> software, in the next gc_step after hardware deletes the flow
> (and flow is marked DEAD). Till that happens, the flow's timeout
> might be updated from a previous scheduled stats, or software packets
> (refresh). This will cause the gc_step to no longer consider the flow
> expired, and it will not be deleted from software.
> 
> Fix that by looking at the DYING flag as in deciding
> a flow should be deleted from software.

Would this work for you?

The idea is to skip the refresh if this has already expired.

Thanks.

[-- Attachment #2: expired.patch --]
[-- Type: text/x-diff, Size: 1235 bytes --]

diff --git a/net/netfilter/nf_flow_table_core.c b/net/netfilter/nf_flow_table_core.c
index 4344e572b7f9..862efa7c606d 100644
--- a/net/netfilter/nf_flow_table_core.c
+++ b/net/netfilter/nf_flow_table_core.c
@@ -252,10 +252,18 @@ int flow_offload_add(struct nf_flowtable *flow_table, struct flow_offload *flow)
 }
 EXPORT_SYMBOL_GPL(flow_offload_add);
 
+static inline bool nf_flow_has_expired(const struct flow_offload *flow)
+{
+	return nf_flow_timeout_delta(flow->timeout) <= 0;
+}
+
 void flow_offload_refresh(struct nf_flowtable *flow_table,
 			  struct flow_offload *flow)
 {
-	flow->timeout = nf_flowtable_time_stamp + NF_FLOW_TIMEOUT;
+	if (!nf_flow_has_expired(flow)) {
+		flow->timeout = nf_flowtable_time_stamp + NF_FLOW_TIMEOUT;
+		return;
+	}
 
 	if (likely(!nf_flowtable_hw_offload(flow_table) ||
 		   !test_and_clear_bit(NF_FLOW_HW_REFRESH, &flow->flags)))
@@ -265,11 +273,6 @@ void flow_offload_refresh(struct nf_flowtable *flow_table,
 }
 EXPORT_SYMBOL_GPL(flow_offload_refresh);
 
-static inline bool nf_flow_has_expired(const struct flow_offload *flow)
-{
-	return nf_flow_timeout_delta(flow->timeout) <= 0;
-}
-
 static void flow_offload_del(struct nf_flowtable *flow_table,
 			     struct flow_offload *flow)
 {