[PATCH nf-next v2] netfilter: nf_conncount: fix tracking of connections from localhost

[PATCH nf-next v2] netfilter: nf_conncount: fix tracking of connections from localhost

[Fernando Fernandez Mancera]

Since commit be102eb6a0e7 ("netfilter: nf_conncount: rework API to use
sk_buff directly"), we skip the adding and trigger a GC when the ct is
confirmed. For connections originated from local to local it doesn't
work because the connection is confirmed on POSTROUTING, therefore
tracking on the INPUT hook is always skipped.

In order to fix this, we check whether skb input ifindex is set to
loopback ifindex. If it is then we fallback on a GC plus track operation
skipping the optimization. This fallback is necessary to avoid
duplicated tracking of a packet train e.g 10 UDP datagrams sent on a
burst when initiating the connection.

Tested with xt_connlimit/nft_connlimit and OVS limit and with a HTTP
server and iperf3 on UDP mode.

Fixes: be102eb6a0e7 ("netfilter: nf_conncount: rework API to use sk_buff directly")
Reported-by: Michal Slabihoudek <michal.slabihoudek@gooddata.com>
Closes: https://lore.kernel.org/netfilter/6989BD9F-8C24-4397-9AD7-4613B28BF0DB@gooddata.com/
Signed-off-by: Fernando Fernandez Mancera <fmancera@suse.de>
---
v2: use skb_iif to understand if this is a local connection
---
 net/netfilter/nf_conncount.c | 15 +++++++++++++--
 1 file changed, 13 insertions(+), 2 deletions(-)

diff --git a/net/netfilter/nf_conncount.c b/net/netfilter/nf_conncount.c
index 288936f5c1bf..14e62b3263cd 100644
--- a/net/netfilter/nf_conncount.c
+++ b/net/netfilter/nf_conncount.c
@@ -179,14 +179,25 @@ static int __nf_conncount_add(struct net *net,
 		return -ENOENT;
 
 	if (ct && nf_ct_is_confirmed(ct)) {
-		err = -EEXIST;
-		goto out_put;
+		/* local connections are confirmed in postrouting so confirmation
+		 * might have happened before hitting connlimit
+		 */
+		if (skb->skb_iif != LOOPBACK_IFINDEX) {
+			err = -EEXIST;
+			goto out_put;
+		}
+
+		/* this is likely a local connection, skip optimization to avoid
+		 * adding duplicates from a 'packet train'
+		 */
+		goto check_connections;
 	}
 
 	if ((u32)jiffies == list->last_gc &&
 	    (list->count - list->last_gc_count) < CONNCOUNT_GC_MAX_COLLECT)
 		goto add_new_node;
 
+check_connections:
 	/* check the saved connections */
 	list_for_each_entry_safe(conn, conn_n, &list->head, node) {
 		if (collect > CONNCOUNT_GC_MAX_COLLECT)
-- 
2.52.0

Re: [PATCH nf-next v2] netfilter: nf_conncount: fix tracking of connections from localhost

[Florian Westphal]

Fernando Fernandez Mancera <fmancera@suse.de> wrote:
> Since commit be102eb6a0e7 ("netfilter: nf_conncount: rework API to use
> sk_buff directly"), we skip the adding and trigger a GC when the ct is
> confirmed. For connections originated from local to local it doesn't
> work because the connection is confirmed on POSTROUTING, therefore
> tracking on the INPUT hook is always skipped.
> 
> In order to fix this, we check whether skb input ifindex is set to
> loopback ifindex. If it is then we fallback on a GC plus track operation
> skipping the optimization. This fallback is necessary to avoid
> duplicated tracking of a packet train e.g 10 UDP datagrams sent on a
> burst when initiating the connection.
> 
> Tested with xt_connlimit/nft_connlimit and OVS limit and with a HTTP
> server and iperf3 on UDP mode.

LGTM, thanks Fernando.  But shouldn't this go via nf tree?

Re: [PATCH nf-next v2] netfilter: nf_conncount: fix tracking of connections from localhost

[Fernando Fernandez Mancera]

On 1/19/26 9:45 PM, Florian Westphal wrote:
> Fernando Fernandez Mancera <fmancera@suse.de> wrote:
>> Since commit be102eb6a0e7 ("netfilter: nf_conncount: rework API to use
>> sk_buff directly"), we skip the adding and trigger a GC when the ct is
>> confirmed. For connections originated from local to local it doesn't
>> work because the connection is confirmed on POSTROUTING, therefore
>> tracking on the INPUT hook is always skipped.
>>
>> In order to fix this, we check whether skb input ifindex is set to
>> loopback ifindex. If it is then we fallback on a GC plus track operation
>> skipping the optimization. This fallback is necessary to avoid
>> duplicated tracking of a packet train e.g 10 UDP datagrams sent on a
>> burst when initiating the connection.
>>
>> Tested with xt_connlimit/nft_connlimit and OVS limit and with a HTTP
>> server and iperf3 on UDP mode.
> 
> LGTM, thanks Fernando.  But shouldn't this go via nf tree?
> 

Yes but it will conflict with 76c79f31706a ("netfilter: nf_conncount: 
increase the connection clean up limit to 64") when merging both trees. 
Also it is rc7 cycle now.. so not sure.

Anyway, if you think nf tree is better let me know if I should send a v3 
rebased in top of it. I am fine with both trees as long as the conflict 
isn't a problem.

Thanks,
Fernando.

Re: [PATCH nf-next v2] netfilter: nf_conncount: fix tracking of connections from localhost

[Florian Westphal]

Fernando Fernandez Mancera <fmancera@suse.de> wrote:
> > LGTM, thanks Fernando.  But shouldn't this go via nf tree?
> > 
> 
> Yes but it will conflict with 76c79f31706a ("netfilter: nf_conncount:
> increase the connection clean up limit to 64") when merging both trees. Also
> it is rc7 cycle now.. so not sure.

In your opinion, is this bug more severe than what 76c79f31706a fixes?

If yes, please resend vs. nf; I can hold 76c79f31706a back until
after release and then handle it via nf too.

If no, let me know and I will add handle it via -next.

Re: [PATCH nf-next v2] netfilter: nf_conncount: fix tracking of connections from localhost

[Fernando Fernandez Mancera]

On 1/20/26 12:35 AM, Florian Westphal wrote:
> Fernando Fernandez Mancera <fmancera@suse.de> wrote:
>>> LGTM, thanks Fernando.  But shouldn't this go via nf tree?
>>>
>>
>> Yes but it will conflict with 76c79f31706a ("netfilter: nf_conncount:
>> increase the connection clean up limit to 64") when merging both trees. Also
>> it is rc7 cycle now.. so not sure.
> 
> In your opinion, is this bug more severe than what 76c79f31706a fixes?
> 

Not at all. This is a corner case IMHO. While 76c79f31706a is affecting 
the main functionality when the packet rate is high enough. Therefore I 
think via nf-next is fine. Thank you Florian!

> If yes, please resend vs. nf; I can hold 76c79f31706a back until
> after release and then handle it via nf too.
> 
> If no, let me know and I will add handle it via -next.