diff mbox series

[02/38] netfilter: flowtables: use fixed renew timeout on teardown

Message ID 20180720130906.27687-3-pablo@netfilter.org
State Accepted
Delegated to: Pablo Neira
Headers show
Series [01/38] netfilter: nft_reject_bridge: remove unnecessary ttl set | expand

Commit Message

Pablo Neira Ayuso July 20, 2018, 1:08 p.m. UTC 
From: Florian Westphal <fw@strlen.de>

This is one of the very few external callers of ->get_timeouts(),

We can use a fixed timeout instead, conntrack core will refresh this in
case a new packet comes within this period.

Use of ESTABLISHED timeout seems way too huge anyway.

Signed-off-by: Florian Westphal <fw@strlen.de>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
 net/netfilter/nf_flow_table_core.c | 13 +++++--------
 1 file changed, 5 insertions(+), 8 deletions(-)

Comments

Felix Fietkau July 20, 2018, 1:20 p.m. UTC | #1 
On 2018-07-20 15:08, Pablo Neira Ayuso wrote:
> From: Florian Westphal <fw@strlen.de>
> 
> This is one of the very few external callers of ->get_timeouts(),
> 
> We can use a fixed timeout instead, conntrack core will refresh this in
> case a new packet comes within this period.
> 
> Use of ESTABLISHED timeout seems way too huge anyway.
It seems to me that this could easily break long-lived connections that
are idle most of the time.

- Felix
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Florian Westphal July 20, 2018, 1:32 p.m. UTC | #2 
Felix Fietkau <nbd@nbd.name> wrote:
> On 2018-07-20 15:08, Pablo Neira Ayuso wrote:
> > From: Florian Westphal <fw@strlen.de>
> > 
> > This is one of the very few external callers of ->get_timeouts(),
> > 
> > We can use a fixed timeout instead, conntrack core will refresh this in
> > case a new packet comes within this period.
> > 
> > Use of ESTABLISHED timeout seems way too huge anyway.
> It seems to me that this could easily break long-lived connections that
> are idle most of the time.

Problem is that we don't know state of connection, since it was
offloaded.

We don't know if connection 'died' with unacked data (short default
timeout) or not (long default timeout).

So I would prefer to err on the 'evict idle connection that had no
keepalives early' side rather than the 'add dead connection hanging
around forever'.
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox series

Patch

diff --git a/net/netfilter/nf_flow_table_core.c b/net/netfilter/nf_flow_table_core.c
index eb0d1658ac05..d8125616edc7 100644
--- a/net/netfilter/nf_flow_table_core.c
+++ b/net/netfilter/nf_flow_table_core.c
@@ -107,11 +107,12 @@  static void flow_offload_fixup_tcp(struct ip_ct_tcp *tcp)
 	tcp->seen[1].td_maxwin = 0;
 }
 
+#define NF_FLOWTABLE_TCP_PICKUP_TIMEOUT	(120 * HZ)
+#define NF_FLOWTABLE_UDP_PICKUP_TIMEOUT	(30 * HZ)
+
 static void flow_offload_fixup_ct_state(struct nf_conn *ct)
 {
 	const struct nf_conntrack_l4proto *l4proto;
-	struct net *net = nf_ct_net(ct);
-	unsigned int *timeouts;
 	unsigned int timeout;
 	int l4num;
 
@@ -123,14 +124,10 @@  static void flow_offload_fixup_ct_state(struct nf_conn *ct)
 	if (!l4proto)
 		return;
 
-	timeouts = l4proto->get_timeouts(net);
-	if (!timeouts)
-		return;
-
 	if (l4num == IPPROTO_TCP)
-		timeout = timeouts[TCP_CONNTRACK_ESTABLISHED];
+		timeout = NF_FLOWTABLE_TCP_PICKUP_TIMEOUT;
 	else if (l4num == IPPROTO_UDP)
-		timeout = timeouts[UDP_CT_REPLIED];
+		timeout = NF_FLOWTABLE_UDP_PICKUP_TIMEOUT;
 	else
 		return;