All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH nf v2] netfilter: nf_conncount: prevent connlimit drops for early confirmed ct
@ 2026-05-14 14:16 Fernando Fernandez Mancera
  2026-05-19 20:23 ` Florian Westphal
  0 siblings, 1 reply; 4+ messages in thread
From: Fernando Fernandez Mancera @ 2026-05-14 14:16 UTC (permalink / raw)
  To: netfilter-devel
  Cc: coreteam, phil, fw, pablo, Fernando Fernandez Mancera,
	Alejandro Olivan Alvarez

Commit 69894e5b4c5e ("netfilter: nft_connlimit: update the count if add
was skipped") introduced a regression where packets for valid
connections are dropped when using connlimit for soft-limiting
scenarios.

The issue occurs when a new connection reuses a socket currently in
the TIME_WAIT state. In this scenario, the connection tracking entry
is evaluated as already confirmed. Previously, __nf_conncount_add()
assumed that if a connection was confirmed and did not originate from
the loopback interface, it should skip the addition and return -EEXIST.

Skipping the addition triggers a garbage collection run that cleans up
the TIME_WAIT connection. Consequently, the active connection count
drops to 0, which xt_connlimit mishandles, leading to the false rejection
of the perfectly valid new connection.

Fix this by replacing the interface check with protocol-agnostic state
checks. We now skip the tree insertion and preserve the lockless garbage
collection optimization only if the connection is IPS_ASSURED. This
allows early-confirmed setup packets (such as reused TIME_WAIT sockets
or locally generated SYN-ACKs) to be properly evaluated and counted
without falsely dropping. The goto check_connections path is maintained
to ensure these setup packets are deduplicated correctly.

This has been tested with slowhttptest and HTTP server configured
locally to ensure we are not breaking soft-limiting scenarios for local
or external connections. In addition, it was tested with a OVS zone
limit too.

Fixes: 69894e5b4c5e ("netfilter: nft_connlimit: update the count if add was skipped")
Reported-by: Alejandro Olivan Alvarez <alejandro.olivan.alvarez@gmail.com>
Closes: https://lore.kernel.org/netfilter-devel/177349610461.3071718.4083978280323144323@eldamar.lan/
Signed-off-by: Fernando Fernandez Mancera <fmancera@suse.de>
---
v2: remove the ESTABLISHED check as it is redundant for the relevant
protocols (UDP, TCP, SCTP).
---
 net/netfilter/nf_conncount.c | 11 +++++------
 1 file changed, 5 insertions(+), 6 deletions(-)

diff --git a/net/netfilter/nf_conncount.c b/net/netfilter/nf_conncount.c
index 00eed5b4d1b1..facd7ffdf5c2 100644
--- a/net/netfilter/nf_conncount.c
+++ b/net/netfilter/nf_conncount.c
@@ -179,17 +179,16 @@ static int __nf_conncount_add(struct net *net,
 		return -ENOENT;
 
 	if (ct && nf_ct_is_confirmed(ct)) {
-		/* local connections are confirmed in postrouting so confirmation
-		 * might have happened before hitting connlimit
+		/* Connection is confirmed but might still be in the setup phase.
+		 * Only skip the tracking if it is fully assured. This guarantees
+		 * that setup packets or retransmissions are properly counted and
+		 * deduplicated.
 		 */
-		if (skb->skb_iif != LOOPBACK_IFINDEX) {
+		if (test_bit(IPS_ASSURED_BIT, &ct->status)) {
 			err = -EEXIST;
 			goto out_put;
 		}
 
-		/* this is likely a local connection, skip optimization to avoid
-		 * adding duplicates from a 'packet train'
-		 */
 		goto check_connections;
 	}
 
-- 
2.53.0


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH nf v2] netfilter: nf_conncount: prevent connlimit drops for early confirmed ct
  2026-05-14 14:16 [PATCH nf v2] netfilter: nf_conncount: prevent connlimit drops for early confirmed ct Fernando Fernandez Mancera
@ 2026-05-19 20:23 ` Florian Westphal
  2026-05-19 21:31   ` Fernando Fernandez Mancera
  0 siblings, 1 reply; 4+ messages in thread
From: Florian Westphal @ 2026-05-19 20:23 UTC (permalink / raw)
  To: Fernando Fernandez Mancera
  Cc: netfilter-devel, coreteam, phil, pablo, Alejandro Olivan Alvarez

Fernando Fernandez Mancera <fmancera@suse.de> wrote:
> Commit 69894e5b4c5e ("netfilter: nft_connlimit: update the count if add
> was skipped") introduced a regression where packets for valid
> connections are dropped when using connlimit for soft-limiting
> scenarios.
> 
> The issue occurs when a new connection reuses a socket currently in
> the TIME_WAIT state. In this scenario, the connection tracking entry
> is evaluated as already confirmed. Previously, __nf_conncount_add()
> assumed that if a connection was confirmed and did not originate from
> the loopback interface, it should skip the addition and return -EEXIST.
> 
> Skipping the addition triggers a garbage collection run that cleans up
> the TIME_WAIT connection. Consequently, the active connection count
> drops to 0, which xt_connlimit mishandles, leading to the false rejection
> of the perfectly valid new connection.

What do you make of https://sashiko.dev/#/patchset/20260514141628.4636-1-fmancera%40suse.de

Is there a way to handle this with a different solution?
I don't see a good solution.   What about making
__nf_conncount_gc_list() return the number of removed elements and allow
a single re-add attempt if we released some entries?

(Note that I don't think that conncount with unidirectional traffic
 is a sensible thing to configure, but I can't say "not supported"
 either...)

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH nf v2] netfilter: nf_conncount: prevent connlimit drops for early confirmed ct
  2026-05-19 20:23 ` Florian Westphal
@ 2026-05-19 21:31   ` Fernando Fernandez Mancera
  2026-05-22  9:29     ` Fernando Fernandez Mancera
  0 siblings, 1 reply; 4+ messages in thread
From: Fernando Fernandez Mancera @ 2026-05-19 21:31 UTC (permalink / raw)
  To: Florian Westphal
  Cc: netfilter-devel, coreteam, phil, pablo, Alejandro Olivan Alvarez

On 5/19/26 10:23 PM, Florian Westphal wrote:
> Fernando Fernandez Mancera <fmancera@suse.de> wrote:
>> Commit 69894e5b4c5e ("netfilter: nft_connlimit: update the count if add
>> was skipped") introduced a regression where packets for valid
>> connections are dropped when using connlimit for soft-limiting
>> scenarios.
>>
>> The issue occurs when a new connection reuses a socket currently in
>> the TIME_WAIT state. In this scenario, the connection tracking entry
>> is evaluated as already confirmed. Previously, __nf_conncount_add()
>> assumed that if a connection was confirmed and did not originate from
>> the loopback interface, it should skip the addition and return -EEXIST.
>>
>> Skipping the addition triggers a garbage collection run that cleans up
>> the TIME_WAIT connection. Consequently, the active connection count
>> drops to 0, which xt_connlimit mishandles, leading to the false rejection
>> of the perfectly valid new connection.
> 
> What do you make of https://sashiko.dev/#/patchset/20260514141628.4636-1-fmancera%40suse.de
> 
> Is there a way to handle this with a different solution?
> I don't see a good solution.   What about making
> __nf_conncount_gc_list() return the number of removed elements and allow
> a single re-add attempt if we released some entries?
> 
> (Note that I don't think that conncount with unidirectional traffic
>   is a sensible thing to configure, but I can't say "not supported"
>   either...)

Ugh I read it and to be honest, I am not sure this is valid feedback. 
The problematic thing here would be that GC is always called.. which is 
fine?

I can try to generate 50k connections and generate unidirectional 
traffic to understand the consequences but I expect that it will not be 
that bad.

Let me get back to you..

Thanks Florian!


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH nf v2] netfilter: nf_conncount: prevent connlimit drops for early confirmed ct
  2026-05-19 21:31   ` Fernando Fernandez Mancera
@ 2026-05-22  9:29     ` Fernando Fernandez Mancera
  0 siblings, 0 replies; 4+ messages in thread
From: Fernando Fernandez Mancera @ 2026-05-22  9:29 UTC (permalink / raw)
  To: Florian Westphal
  Cc: netfilter-devel, coreteam, phil, pablo, Alejandro Olivan Alvarez

On 5/19/26 11:31 PM, Fernando Fernandez Mancera wrote:
> On 5/19/26 10:23 PM, Florian Westphal wrote:
>> Fernando Fernandez Mancera <fmancera@suse.de> wrote:
>>> Commit 69894e5b4c5e ("netfilter: nft_connlimit: update the count if add
>>> was skipped") introduced a regression where packets for valid
>>> connections are dropped when using connlimit for soft-limiting
>>> scenarios.
>>>
>>> The issue occurs when a new connection reuses a socket currently in
>>> the TIME_WAIT state. In this scenario, the connection tracking entry
>>> is evaluated as already confirmed. Previously, __nf_conncount_add()
>>> assumed that if a connection was confirmed and did not originate from
>>> the loopback interface, it should skip the addition and return -EEXIST.
>>>
>>> Skipping the addition triggers a garbage collection run that cleans up
>>> the TIME_WAIT connection. Consequently, the active connection count
>>> drops to 0, which xt_connlimit mishandles, leading to the false 
>>> rejection
>>> of the perfectly valid new connection.
>>
>> What do you make of https://sashiko.dev/#/ 
>> patchset/20260514141628.4636-1-fmancera%40suse.de
>>
>> Is there a way to handle this with a different solution?
>> I don't see a good solution.   What about making
>> __nf_conncount_gc_list() return the number of removed elements and allow
>> a single re-add attempt if we released some entries?
>>
>> (Note that I don't think that conncount with unidirectional traffic
>>   is a sensible thing to configure, but I can't say "not supported"
>>   either...)
> 
> Ugh I read it and to be honest, I am not sure this is valid feedback. 
> The problematic thing here would be that GC is always called.. which is 
> fine?
> 
> I can try to generate 50k connections and generate unidirectional 
> traffic to understand the consequences but I expect that it will not be 
> that bad.
> 

All seems to be working fine on my side. I couldn't reproduce any 
soft-lockup or anything. IMHO, we could disregard that feedback unless 
someone finds a real issue..

Thanks,
Fernando.


^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2026-05-22  9:29 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-05-14 14:16 [PATCH nf v2] netfilter: nf_conncount: prevent connlimit drops for early confirmed ct Fernando Fernandez Mancera
2026-05-19 20:23 ` Florian Westphal
2026-05-19 21:31   ` Fernando Fernandez Mancera
2026-05-22  9:29     ` Fernando Fernandez Mancera

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.