[PATCH 0/6 nf-next v3] netfilter: rework conncount API to receive sk_buff directly

[PATCH 0/6 nf-next v3] netfilter: rework conncount API to receive sk_buff directly

[Fernando Fernandez Mancera]

This series is fixing two different problems. The first issue is related
to duplicated entries when used for non-confirmed connections in
nft_connlimit and xt_connlimit. Now, nf_conncount_add() checks whether
the connection is confirmed or not. If the connection is confirmed,
skip the add.

In order to do that, nf_conncount_count_skb() and nf_conncount_add_skb()
API has been introduced. They allow the user to pass the sk_buff
directly. The old API has been removed.

The second issue this series is fixing is related to
nft_connlimit/xt_connlimit not updating the list of connection for
confirmed connections breaking softlimiting use-cases like limiting the
bandwidth when too many connections are open.

This has been tested with nftables and iptables both in filter and raw
priorities. I have stressed the system up to 2000 connections.

CC'ing openvswitch maintainers as this change on the API required me to
touch their code. I am not very familiar with the internals of
openvswitch but I believe this should be fine. If you could provide some
testing from openvswitch side it would be really helpful.

Fernando Fernandez Mancera (6):
  netfilter: nf_conncount: introduce new nf_conncount_count_skb() API
  netfilter: xt_connlimit: use nf_conncount_count_skb() directly
  openvswitch: use nf_conncount_count_skb() directly
  netfilter: nf_conncount: pass the sk_buff down to __nf_conncount_add()
  netfilter: nf_conncount: make nf_conncount_gc_list() to disable BH
  netfilter: nft_connlimit: update the count if add was skipped

 include/net/netfilter/nf_conntrack_count.h |  17 +-
 net/netfilter/nf_conncount.c               | 193 ++++++++++++++-------
 net/netfilter/nft_connlimit.c              |  41 ++---
 net/netfilter/xt_connlimit.c               |  14 +-
 net/openvswitch/conntrack.c                |  16 +-
 5 files changed, 169 insertions(+), 112 deletions(-)

-- 
2.51.0

[PATCH 1/6 nf-next v3] netfilter: nf_conncount: introduce new nf_conncount_count_skb() API

[Fernando Fernandez Mancera]

This API allows the caller to pass directly an sk_buff struct. It
fetches the tuple and zone and the corresponding ct from it and performs
a count_tree() operation.

Signed-off-by: Fernando Fernandez Mancera <fmancera@suse.de>
---
 include/net/netfilter/nf_conntrack_count.h |  6 +++
 net/netfilter/nf_conncount.c               | 48 ++++++++++++++++++++++
 2 files changed, 54 insertions(+)

diff --git a/include/net/netfilter/nf_conntrack_count.h b/include/net/netfilter/nf_conntrack_count.h
index 1b58b5b91ff6..706b5a82386e 100644
--- a/include/net/netfilter/nf_conntrack_count.h
+++ b/include/net/netfilter/nf_conntrack_count.h
@@ -24,6 +24,12 @@ unsigned int nf_conncount_count(struct net *net,
 				const struct nf_conntrack_tuple *tuple,
 				const struct nf_conntrack_zone *zone);
 
+unsigned int nf_conncount_count_skb(struct net *net,
+				    const struct sk_buff *skb,
+				    u16 l3num,
+				    struct nf_conncount_data *data,
+				    const u32 *key);
+
 int nf_conncount_add(struct net *net, struct nf_conncount_list *list,
 		     const struct nf_conntrack_tuple *tuple,
 		     const struct nf_conntrack_zone *zone);
diff --git a/net/netfilter/nf_conncount.c b/net/netfilter/nf_conncount.c
index 913ede2f57f9..cf5ed5c6bfba 100644
--- a/net/netfilter/nf_conncount.c
+++ b/net/netfilter/nf_conncount.c
@@ -524,6 +524,54 @@ unsigned int nf_conncount_count(struct net *net,
 }
 EXPORT_SYMBOL_GPL(nf_conncount_count);
 
+/* Count and return number of conntrack entries in 'net' with particular 'key'.
+ * If 'skb' is not null, insert the corresponding tuple into the accounting
+ * data structure. Call with RCU read lock.
+ */
+unsigned int nf_conncount_count_skb(struct net *net,
+				    const struct sk_buff *skb,
+				    u16 l3num,
+				    struct nf_conncount_data *data,
+				    const u32 *key)
+{
+	const struct nf_conntrack_tuple_hash *h;
+	const struct nf_conntrack_zone *zone;
+	struct nf_conntrack_tuple tuple;
+	enum ip_conntrack_info ctinfo;
+	unsigned int connections;
+	struct nf_conn *ct;
+
+	if (!skb)
+		return count_tree(net, data, key, NULL, NULL);
+
+	ct = nf_ct_get(skb, &ctinfo);
+	if (ct && !nf_ct_is_template(ct))
+		return count_tree(net, data, key,
+				  &ct->tuplehash[IP_CT_DIR_ORIGINAL].tuple,
+				  nf_ct_zone(ct));
+
+	if (!nf_ct_get_tuplepr(skb, skb_network_offset(skb), l3num, net, &tuple))
+		return 0;
+
+	if (ct)
+		zone = nf_ct_zone(ct);
+	else
+		zone = &nf_ct_zone_dflt;
+
+	h = nf_conntrack_find_get(net, zone, &tuple);
+	if (!h)
+		return count_tree(net, data, key, &tuple, zone);
+
+	ct = nf_ct_tuplehash_to_ctrack(h);
+	connections = count_tree(net, data, key,
+				 &ct->tuplehash[IP_CT_DIR_ORIGINAL].tuple,
+				 zone);
+	nf_ct_put(ct);
+
+	return connections;
+}
+EXPORT_SYMBOL_GPL(nf_conncount_count_skb);
+
 struct nf_conncount_data *nf_conncount_init(struct net *net, unsigned int keylen)
 {
 	struct nf_conncount_data *data;
-- 
2.51.0

[PATCH 2/6 nf-next v3] netfilter: xt_connlimit: use nf_conncount_count_skb() directly

[Fernando Fernandez Mancera]

The implementation in xt_connlimit can be simplified as now
nf_conncount_count_skb() allow the conncount users to pass the sk_buff.

Signed-off-by: Fernando Fernandez Mancera <fmancera@suse.de>
---
 net/netfilter/xt_connlimit.c | 14 +++-----------
 1 file changed, 3 insertions(+), 11 deletions(-)

diff --git a/net/netfilter/xt_connlimit.c b/net/netfilter/xt_connlimit.c
index 0189f8b6b0bd..848287ab79cf 100644
--- a/net/netfilter/xt_connlimit.c
+++ b/net/netfilter/xt_connlimit.c
@@ -31,8 +31,6 @@ connlimit_mt(const struct sk_buff *skb, struct xt_action_param *par)
 {
 	struct net *net = xt_net(par);
 	const struct xt_connlimit_info *info = par->matchinfo;
-	struct nf_conntrack_tuple tuple;
-	const struct nf_conntrack_tuple *tuple_ptr = &tuple;
 	const struct nf_conntrack_zone *zone = &nf_ct_zone_dflt;
 	enum ip_conntrack_info ctinfo;
 	const struct nf_conn *ct;
@@ -40,13 +38,8 @@ connlimit_mt(const struct sk_buff *skb, struct xt_action_param *par)
 	u32 key[5];
 
 	ct = nf_ct_get(skb, &ctinfo);
-	if (ct != NULL) {
-		tuple_ptr = &ct->tuplehash[IP_CT_DIR_ORIGINAL].tuple;
+	if (ct)
 		zone = nf_ct_zone(ct);
-	} else if (!nf_ct_get_tuplepr(skb, skb_network_offset(skb),
-				      xt_family(par), net, &tuple)) {
-		goto hotdrop;
-	}
 
 	if (xt_family(par) == NFPROTO_IPV6) {
 		const struct ipv6hdr *iph = ipv6_hdr(skb);
@@ -69,10 +62,9 @@ connlimit_mt(const struct sk_buff *skb, struct xt_action_param *par)
 		key[1] = zone->id;
 	}
 
-	connections = nf_conncount_count(net, info->data, key, tuple_ptr,
-					 zone);
+	connections = nf_conncount_count_skb(net, skb, xt_family(par), info->data, key);
 	if (connections == 0)
-		/* kmalloc failed, drop it entirely */
+		/* kmalloc failed or tuple couldn't be found, drop it entirely */
 		goto hotdrop;
 
 	return (connections > info->limit) ^ !!(info->flags & XT_CONNLIMIT_INVERT);
-- 
2.51.0

[PATCH 3/6 nf-next v3] openvswitch: use nf_conncount_count_skb() directly

[Fernando Fernandez Mancera]

Use the new nf_conncount_count_skb() which allows the caller to pass the
sk_buff. As openvswitch is the last user of the old nf_conncount_count()
API, remove the old API too.

Signed-off-by: Fernando Fernandez Mancera <fmancera@suse.de>
---
 include/net/netfilter/nf_conntrack_count.h |  6 ------
 net/netfilter/nf_conncount.c               | 14 --------------
 net/openvswitch/conntrack.c                | 16 ++++++++--------
 3 files changed, 8 insertions(+), 28 deletions(-)

diff --git a/include/net/netfilter/nf_conntrack_count.h b/include/net/netfilter/nf_conntrack_count.h
index 706b5a82386e..8d980f7bff8e 100644
--- a/include/net/netfilter/nf_conntrack_count.h
+++ b/include/net/netfilter/nf_conntrack_count.h
@@ -18,12 +18,6 @@ struct nf_conncount_list {
 struct nf_conncount_data *nf_conncount_init(struct net *net, unsigned int keylen);
 void nf_conncount_destroy(struct net *net, struct nf_conncount_data *data);
 
-unsigned int nf_conncount_count(struct net *net,
-				struct nf_conncount_data *data,
-				const u32 *key,
-				const struct nf_conntrack_tuple *tuple,
-				const struct nf_conntrack_zone *zone);
-
 unsigned int nf_conncount_count_skb(struct net *net,
 				    const struct sk_buff *skb,
 				    u16 l3num,
diff --git a/net/netfilter/nf_conncount.c b/net/netfilter/nf_conncount.c
index cf5ed5c6bfba..fc5798db7727 100644
--- a/net/netfilter/nf_conncount.c
+++ b/net/netfilter/nf_conncount.c
@@ -510,20 +510,6 @@ static void tree_gc_worker(struct work_struct *work)
 	spin_unlock_bh(&nf_conncount_locks[tree]);
 }
 
-/* Count and return number of conntrack entries in 'net' with particular 'key'.
- * If 'tuple' is not null, insert it into the accounting data structure.
- * Call with RCU read lock.
- */
-unsigned int nf_conncount_count(struct net *net,
-				struct nf_conncount_data *data,
-				const u32 *key,
-				const struct nf_conntrack_tuple *tuple,
-				const struct nf_conntrack_zone *zone)
-{
-	return count_tree(net, data, key, tuple, zone);
-}
-EXPORT_SYMBOL_GPL(nf_conncount_count);
-
 /* Count and return number of conntrack entries in 'net' with particular 'key'.
  * If 'skb' is not null, insert the corresponding tuple into the accounting
  * data structure. Call with RCU read lock.
diff --git a/net/openvswitch/conntrack.c b/net/openvswitch/conntrack.c
index e573e9221302..a0811e1fba65 100644
--- a/net/openvswitch/conntrack.c
+++ b/net/openvswitch/conntrack.c
@@ -928,8 +928,8 @@ static u32 ct_limit_get(const struct ovs_ct_limit_info *info, u16 zone)
 }
 
 static int ovs_ct_check_limit(struct net *net,
-			      const struct ovs_conntrack_info *info,
-			      const struct nf_conntrack_tuple *tuple)
+			      const struct sk_buff *skb,
+			      const struct ovs_conntrack_info *info)
 {
 	struct ovs_net *ovs_net = net_generic(net, ovs_net_id);
 	const struct ovs_ct_limit_info *ct_limit_info = ovs_net->ct_limit_info;
@@ -942,8 +942,9 @@ static int ovs_ct_check_limit(struct net *net,
 	if (per_zone_limit == OVS_CT_LIMIT_UNLIMITED)
 		return 0;
 
-	connections = nf_conncount_count(net, ct_limit_info->data,
-					 &conncount_key, tuple, &info->zone);
+	connections = nf_conncount_count_skb(net, skb, info->family,
+					     ct_limit_info->data,
+					     &conncount_key);
 	if (connections > per_zone_limit)
 		return -ENOMEM;
 
@@ -972,8 +973,7 @@ static int ovs_ct_commit(struct net *net, struct sw_flow_key *key,
 #if	IS_ENABLED(CONFIG_NETFILTER_CONNCOUNT)
 	if (static_branch_unlikely(&ovs_ct_limit_enabled)) {
 		if (!nf_ct_is_confirmed(ct)) {
-			err = ovs_ct_check_limit(net, info,
-				&ct->tuplehash[IP_CT_DIR_ORIGINAL].tuple);
+			err = ovs_ct_check_limit(net, skb, info);
 			if (err) {
 				net_warn_ratelimited("openvswitch: zone: %u "
 					"exceeds conntrack limit\n",
@@ -1770,8 +1770,8 @@ static int __ovs_ct_limit_get_zone_limit(struct net *net,
 	zone_limit.limit = limit;
 	nf_ct_zone_init(&ct_zone, zone_id, NF_CT_DEFAULT_ZONE_DIR, 0);
 
-	zone_limit.count = nf_conncount_count(net, data, &conncount_key, NULL,
-					      &ct_zone);
+	zone_limit.count = nf_conncount_count_skb(net, NULL, 0, data,
+						  &conncount_key);
 	return nla_put_nohdr(reply, sizeof(zone_limit), &zone_limit);
 }
 
-- 
2.51.0

[PATCH 4/6 nf-next v3] netfilter: nf_conncount: pass the sk_buff down to __nf_conncount_add()

[Fernando Fernandez Mancera]

When using nf_conncount infrastructure for non-confirmed connections a
duplicated track is possible due to an optimization introduced since
commit d265929930e2 ("netfilter: nf_conncount: reduce unnecessary GC").

This is solved by passing the sk_buff down to __nf_conncount_add() and
there if the ct is present and already confirmed, skip the add
operation.

As __nf_conncount_add() is now handling the sk_buff directly, introduce
a new API nf_conncount_add_skb() similar to the already introduced
nf_conncount_count_skb(). In addition, add a helper function to fetch
tuple, zone and ct from the skb to avoid duplicated code.

Use the newly introduce API from nft_connlimit.

Fixes: d265929930e2 ("netfilter: nf_conncount: reduce unnecessary GC")
Signed-off-by: Fernando Fernandez Mancera <fmancera@suse.de>
---
 include/net/netfilter/nf_conntrack_count.h |   5 +-
 net/netfilter/nf_conncount.c               | 175 ++++++++++++---------
 net/netfilter/nft_connlimit.c              |  21 +--
 3 files changed, 107 insertions(+), 94 deletions(-)

diff --git a/include/net/netfilter/nf_conntrack_count.h b/include/net/netfilter/nf_conntrack_count.h
index 8d980f7bff8e..52a06de41aa0 100644
--- a/include/net/netfilter/nf_conntrack_count.h
+++ b/include/net/netfilter/nf_conntrack_count.h
@@ -24,9 +24,8 @@ unsigned int nf_conncount_count_skb(struct net *net,
 				    struct nf_conncount_data *data,
 				    const u32 *key);
 
-int nf_conncount_add(struct net *net, struct nf_conncount_list *list,
-		     const struct nf_conntrack_tuple *tuple,
-		     const struct nf_conntrack_zone *zone);
+int nf_conncount_add_skb(struct net *net, const struct sk_buff *skb,
+			 u16 l3num, struct nf_conncount_list *list);
 
 void nf_conncount_list_init(struct nf_conncount_list *list);
 
diff --git a/net/netfilter/nf_conncount.c b/net/netfilter/nf_conncount.c
index fc5798db7727..e6d30a54b1fc 100644
--- a/net/netfilter/nf_conncount.c
+++ b/net/netfilter/nf_conncount.c
@@ -122,15 +122,65 @@ find_or_evict(struct net *net, struct nf_conncount_list *list,
 	return ERR_PTR(-EAGAIN);
 }
 
+static bool get_ct_or_tuple_from_skb(struct net *net,
+				     const struct sk_buff *skb,
+				     u16 l3num,
+				     struct nf_conn **ct,
+				     struct nf_conntrack_tuple *tuple,
+				     const struct nf_conntrack_zone **zone,
+				     bool *refcounted)
+{
+	const struct nf_conntrack_tuple_hash *h;
+	enum ip_conntrack_info ctinfo;
+	struct nf_conn *found_ct;
+
+	found_ct = nf_ct_get(skb, &ctinfo);
+	if (found_ct && !nf_ct_is_template(found_ct)) {
+		*tuple = found_ct->tuplehash[IP_CT_DIR_ORIGINAL].tuple;
+		*zone = nf_ct_zone(found_ct);
+		*ct = found_ct;
+		return true;
+	}
+
+	if (!nf_ct_get_tuplepr(skb, skb_network_offset(skb), l3num, net, tuple))
+		return false;
+
+	if (found_ct)
+		*zone = nf_ct_zone(found_ct);
+
+	h = nf_conntrack_find_get(net, *zone, tuple);
+	if (!h)
+		return true;
+
+	found_ct = nf_ct_tuplehash_to_ctrack(h);
+	*refcounted = true;
+	*ct = found_ct;
+
+	return true;
+}
+
 static int __nf_conncount_add(struct net *net,
-			      struct nf_conncount_list *list,
-			      const struct nf_conntrack_tuple *tuple,
-			      const struct nf_conntrack_zone *zone)
+			      const struct sk_buff *skb,
+			      u16 l3num,
+			      struct nf_conncount_list *list)
 {
+	const struct nf_conntrack_zone *zone = &nf_ct_zone_dflt;
 	const struct nf_conntrack_tuple_hash *found;
 	struct nf_conncount_tuple *conn, *conn_n;
+	struct nf_conntrack_tuple tuple;
+	struct nf_conn *ct = NULL;
 	struct nf_conn *found_ct;
 	unsigned int collect = 0;
+	bool refcounted = false;
+
+	if (!get_ct_or_tuple_from_skb(net, skb, l3num, &ct, &tuple, &zone, &refcounted))
+		return -ENOENT;
+
+	if (ct && nf_ct_is_confirmed(ct)) {
+		if (refcounted)
+			nf_ct_put(ct);
+		return -EINVAL;
+	}
 
 	if ((u32)jiffies == list->last_gc)
 		goto add_new_node;
@@ -144,10 +194,10 @@ static int __nf_conncount_add(struct net *net,
 		if (IS_ERR(found)) {
 			/* Not found, but might be about to be confirmed */
 			if (PTR_ERR(found) == -EAGAIN) {
-				if (nf_ct_tuple_equal(&conn->tuple, tuple) &&
+				if (nf_ct_tuple_equal(&conn->tuple, &tuple) &&
 				    nf_ct_zone_id(&conn->zone, conn->zone.dir) ==
 				    nf_ct_zone_id(zone, zone->dir))
-					return 0; /* already exists */
+					goto out_put; /* already exists */
 			} else {
 				collect++;
 			}
@@ -156,7 +206,7 @@ static int __nf_conncount_add(struct net *net,
 
 		found_ct = nf_ct_tuplehash_to_ctrack(found);
 
-		if (nf_ct_tuple_equal(&conn->tuple, tuple) &&
+		if (nf_ct_tuple_equal(&conn->tuple, &tuple) &&
 		    nf_ct_zone_equal(found_ct, zone, zone->dir)) {
 			/*
 			 * We should not see tuples twice unless someone hooks
@@ -165,7 +215,7 @@ static int __nf_conncount_add(struct net *net,
 			 * Attempt to avoid a re-add in this case.
 			 */
 			nf_ct_put(found_ct);
-			return 0;
+			goto out_put;
 		} else if (already_closed(found_ct)) {
 			/*
 			 * we do not care about connections which are
@@ -188,31 +238,35 @@ static int __nf_conncount_add(struct net *net,
 	if (conn == NULL)
 		return -ENOMEM;
 
-	conn->tuple = *tuple;
+	conn->tuple = tuple;
 	conn->zone = *zone;
 	conn->cpu = raw_smp_processor_id();
 	conn->jiffies32 = (u32)jiffies;
 	list_add_tail(&conn->node, &list->head);
 	list->count++;
 	list->last_gc = (u32)jiffies;
+
+out_put:
+	if (refcounted)
+		nf_ct_put(ct);
 	return 0;
 }
 
-int nf_conncount_add(struct net *net,
-		     struct nf_conncount_list *list,
-		     const struct nf_conntrack_tuple *tuple,
-		     const struct nf_conntrack_zone *zone)
+int nf_conncount_add_skb(struct net *net,
+			 const struct sk_buff *skb,
+			 u16 l3num,
+			 struct nf_conncount_list *list)
 {
 	int ret;
 
 	/* check the saved connections */
 	spin_lock_bh(&list->list_lock);
-	ret = __nf_conncount_add(net, list, tuple, zone);
+	ret = __nf_conncount_add(net, skb, l3num, list);
 	spin_unlock_bh(&list->list_lock);
 
 	return ret;
 }
-EXPORT_SYMBOL_GPL(nf_conncount_add);
+EXPORT_SYMBOL_GPL(nf_conncount_add_skb);
 
 void nf_conncount_list_init(struct nf_conncount_list *list)
 {
@@ -309,19 +363,22 @@ static void schedule_gc_worker(struct nf_conncount_data *data, int tree)
 
 static unsigned int
 insert_tree(struct net *net,
+	    const struct sk_buff *skb,
+	    u16 l3num,
 	    struct nf_conncount_data *data,
 	    struct rb_root *root,
 	    unsigned int hash,
-	    const u32 *key,
-	    const struct nf_conntrack_tuple *tuple,
-	    const struct nf_conntrack_zone *zone)
+	    const u32 *key)
 {
 	struct nf_conncount_rb *gc_nodes[CONNCOUNT_GC_MAX_NODES];
+	const struct nf_conntrack_zone *zone = &nf_ct_zone_dflt;
+	bool do_gc = true, refcounted = false;
+	unsigned int count = 0, gc_count = 0;
 	struct rb_node **rbnode, *parent;
-	struct nf_conncount_rb *rbconn;
+	struct nf_conntrack_tuple tuple;
 	struct nf_conncount_tuple *conn;
-	unsigned int count = 0, gc_count = 0;
-	bool do_gc = true;
+	struct nf_conncount_rb *rbconn;
+	struct nf_conn *ct = NULL;
 
 	spin_lock_bh(&nf_conncount_locks[hash]);
 restart:
@@ -340,7 +397,7 @@ insert_tree(struct net *net,
 		} else {
 			int ret;
 
-			ret = nf_conncount_add(net, &rbconn->list, tuple, zone);
+			ret = nf_conncount_add_skb(net, skb, l3num, &rbconn->list);
 			if (ret)
 				count = 0; /* hotdrop */
 			else
@@ -375,19 +432,24 @@ insert_tree(struct net *net,
 		goto out_unlock;
 	}
 
-	conn->tuple = *tuple;
-	conn->zone = *zone;
-	conn->cpu = raw_smp_processor_id();
-	conn->jiffies32 = (u32)jiffies;
-	memcpy(rbconn->key, key, sizeof(u32) * data->keylen);
+	if (get_ct_or_tuple_from_skb(net, skb, l3num, &ct, &tuple, &zone, &refcounted)) {
+		conn->tuple = tuple;
+		conn->zone = *zone;
+		conn->cpu = raw_smp_processor_id();
+		conn->jiffies32 = (u32)jiffies;
+		memcpy(rbconn->key, key, sizeof(u32) * data->keylen);
 
-	nf_conncount_list_init(&rbconn->list);
-	list_add(&conn->node, &rbconn->list.head);
-	count = 1;
-	rbconn->list.count = count;
+		nf_conncount_list_init(&rbconn->list);
+		list_add(&conn->node, &rbconn->list.head);
+		count = 1;
+		rbconn->list.count = count;
 
-	rb_link_node_rcu(&rbconn->node, parent, rbnode);
-	rb_insert_color(&rbconn->node, root);
+		rb_link_node_rcu(&rbconn->node, parent, rbnode);
+		rb_insert_color(&rbconn->node, root);
+
+		if (refcounted)
+			nf_ct_put(ct);
+	}
 out_unlock:
 	spin_unlock_bh(&nf_conncount_locks[hash]);
 	return count;
@@ -395,10 +457,10 @@ insert_tree(struct net *net,
 
 static unsigned int
 count_tree(struct net *net,
+	   const struct sk_buff *skb,
+	   u16 l3num,
 	   struct nf_conncount_data *data,
-	   const u32 *key,
-	   const struct nf_conntrack_tuple *tuple,
-	   const struct nf_conntrack_zone *zone)
+	   const u32 *key)
 {
 	struct rb_root *root;
 	struct rb_node *parent;
@@ -422,7 +484,7 @@ count_tree(struct net *net,
 		} else {
 			int ret;
 
-			if (!tuple) {
+			if (!skb) {
 				nf_conncount_gc_list(net, &rbconn->list);
 				return rbconn->list.count;
 			}
@@ -437,7 +499,7 @@ count_tree(struct net *net,
 			}
 
 			/* same source network -> be counted! */
-			ret = __nf_conncount_add(net, &rbconn->list, tuple, zone);
+			ret = __nf_conncount_add(net, skb, l3num, &rbconn->list);
 			spin_unlock_bh(&rbconn->list.list_lock);
 			if (ret)
 				return 0; /* hotdrop */
@@ -446,10 +508,10 @@ count_tree(struct net *net,
 		}
 	}
 
-	if (!tuple)
+	if (!skb)
 		return 0;
 
-	return insert_tree(net, data, root, hash, key, tuple, zone);
+	return insert_tree(net, skb, l3num, data, root, hash, key);
 }
 
 static void tree_gc_worker(struct work_struct *work)
@@ -520,41 +582,8 @@ unsigned int nf_conncount_count_skb(struct net *net,
 				    struct nf_conncount_data *data,
 				    const u32 *key)
 {
-	const struct nf_conntrack_tuple_hash *h;
-	const struct nf_conntrack_zone *zone;
-	struct nf_conntrack_tuple tuple;
-	enum ip_conntrack_info ctinfo;
-	unsigned int connections;
-	struct nf_conn *ct;
-
-	if (!skb)
-		return count_tree(net, data, key, NULL, NULL);
-
-	ct = nf_ct_get(skb, &ctinfo);
-	if (ct && !nf_ct_is_template(ct))
-		return count_tree(net, data, key,
-				  &ct->tuplehash[IP_CT_DIR_ORIGINAL].tuple,
-				  nf_ct_zone(ct));
-
-	if (!nf_ct_get_tuplepr(skb, skb_network_offset(skb), l3num, net, &tuple))
-		return 0;
-
-	if (ct)
-		zone = nf_ct_zone(ct);
-	else
-		zone = &nf_ct_zone_dflt;
-
-	h = nf_conntrack_find_get(net, zone, &tuple);
-	if (!h)
-		return count_tree(net, data, key, &tuple, zone);
-
-	ct = nf_ct_tuplehash_to_ctrack(h);
-	connections = count_tree(net, data, key,
-				 &ct->tuplehash[IP_CT_DIR_ORIGINAL].tuple,
-				 zone);
-	nf_ct_put(ct);
+	return count_tree(net, skb, l3num, data, key);
 
-	return connections;
 }
 EXPORT_SYMBOL_GPL(nf_conncount_count_skb);
 
diff --git a/net/netfilter/nft_connlimit.c b/net/netfilter/nft_connlimit.c
index fc35a11cdca2..4bee5a8f9ef8 100644
--- a/net/netfilter/nft_connlimit.c
+++ b/net/netfilter/nft_connlimit.c
@@ -24,26 +24,11 @@ static inline void nft_connlimit_do_eval(struct nft_connlimit *priv,
 					 const struct nft_pktinfo *pkt,
 					 const struct nft_set_ext *ext)
 {
-	const struct nf_conntrack_zone *zone = &nf_ct_zone_dflt;
-	const struct nf_conntrack_tuple *tuple_ptr;
-	struct nf_conntrack_tuple tuple;
-	enum ip_conntrack_info ctinfo;
-	const struct nf_conn *ct;
 	unsigned int count;
+	int err;
 
-	tuple_ptr = &tuple;
-
-	ct = nf_ct_get(pkt->skb, &ctinfo);
-	if (ct != NULL) {
-		tuple_ptr = &ct->tuplehash[IP_CT_DIR_ORIGINAL].tuple;
-		zone = nf_ct_zone(ct);
-	} else if (!nf_ct_get_tuplepr(pkt->skb, skb_network_offset(pkt->skb),
-				      nft_pf(pkt), nft_net(pkt), &tuple)) {
-		regs->verdict.code = NF_DROP;
-		return;
-	}
-
-	if (nf_conncount_add(nft_net(pkt), priv->list, tuple_ptr, zone)) {
+	err = nf_conncount_add_skb(nft_net(pkt), pkt->skb, nft_pf(pkt), priv->list);
+	if (err != EINVAL) {
 		regs->verdict.code = NF_DROP;
 		return;
 	}
-- 
2.51.0

[PATCH 5/6 nf-next v3] netfilter: nf_conncount: make nf_conncount_gc_list() to disable BH

[Fernando Fernandez Mancera]

For convenience when performing GC over the connection list, make
nf_conncount_gc_list() to disable BH. This unifies the behavior with
nf_conncount_add() and nf_conncount_count().

Signed-off-by: Fernando Fernandez Mancera <fmancera@suse.de>
---
 net/netfilter/nf_conncount.c  | 24 +++++++++++++++++-------
 net/netfilter/nft_connlimit.c |  7 +------
 2 files changed, 18 insertions(+), 13 deletions(-)

diff --git a/net/netfilter/nf_conncount.c b/net/netfilter/nf_conncount.c
index e6d30a54b1fc..833ef7507af5 100644
--- a/net/netfilter/nf_conncount.c
+++ b/net/netfilter/nf_conncount.c
@@ -278,8 +278,8 @@ void nf_conncount_list_init(struct nf_conncount_list *list)
 EXPORT_SYMBOL_GPL(nf_conncount_list_init);
 
 /* Return true if the list is empty. Must be called with BH disabled. */
-bool nf_conncount_gc_list(struct net *net,
-			  struct nf_conncount_list *list)
+static bool __nf_conncount_gc_list(struct net *net,
+				   struct nf_conncount_list *list)
 {
 	const struct nf_conntrack_tuple_hash *found;
 	struct nf_conncount_tuple *conn, *conn_n;
@@ -291,10 +291,6 @@ bool nf_conncount_gc_list(struct net *net,
 	if ((u32)jiffies == READ_ONCE(list->last_gc))
 		return false;
 
-	/* don't bother if other cpu is already doing GC */
-	if (!spin_trylock(&list->list_lock))
-		return false;
-
 	list_for_each_entry_safe(conn, conn_n, &list->head, node) {
 		found = find_or_evict(net, list, conn);
 		if (IS_ERR(found)) {
@@ -323,7 +319,21 @@ bool nf_conncount_gc_list(struct net *net,
 	if (!list->count)
 		ret = true;
 	list->last_gc = (u32)jiffies;
-	spin_unlock(&list->list_lock);
+
+	return ret;
+}
+
+bool nf_conncount_gc_list(struct net *net,
+			  struct nf_conncount_list *list)
+{
+	bool ret;
+
+	/* don't bother if other cpu is already doing GC */
+	if (!spin_trylock_bh(&list->list_lock))
+		return false;
+
+	ret = __nf_conncount_gc_list(net, list);
+	spin_unlock_bh(&list->list_lock);
 
 	return ret;
 }
diff --git a/net/netfilter/nft_connlimit.c b/net/netfilter/nft_connlimit.c
index 4bee5a8f9ef8..00c247601173 100644
--- a/net/netfilter/nft_connlimit.c
+++ b/net/netfilter/nft_connlimit.c
@@ -223,13 +223,8 @@ static void nft_connlimit_destroy_clone(const struct nft_ctx *ctx,
 static bool nft_connlimit_gc(struct net *net, const struct nft_expr *expr)
 {
 	struct nft_connlimit *priv = nft_expr_priv(expr);
-	bool ret;
 
-	local_bh_disable();
-	ret = nf_conncount_gc_list(net, priv->list);
-	local_bh_enable();
-
-	return ret;
+	return nf_conncount_gc_list(net, priv->list);
 }
 
 static struct nft_expr_type nft_connlimit_type;
-- 
2.51.0

[PATCH 6/6 nf-next v3] netfilter: nft_connlimit: update the count if add was skipped

[Fernando Fernandez Mancera]

Connlimit expression can be used for all kind of packets and not only
for packets with connection state new. See this ruleset as example:

table ip filter {
        chain input {
                type filter hook input priority filter; policy accept;
                tcp dport 22 ct count over 4 counter
        }
}

Currently, if the connection count goes over the limit the counter will
count the packets. When a connection is closed, the connection count
won't decrement as it should because it is only updated for new
connections due to an optimization on __nf_conncount_add() that prevents
updating the list if the connection is duplicated.

To solve this problem, check whether the connection was skipped and if
so, update the list. Adjust count_tree() too so the same fix is applied
for xt_connlimit.

Fixes: 976afca1ceba ("netfilter: nf_conncount: Early exit in nf_conncount_lookup() and cleanup")
Closes: https://lore.kernel.org/netfilter/trinity-85c72a88-d762-46c3-be97-36f10e5d9796-1761173693813@3c-app-mailcom-bs12/
Signed-off-by: Fernando Fernandez Mancera <fmancera@suse.de>
---
 net/netfilter/nf_conncount.c  | 10 +++++++---
 net/netfilter/nft_connlimit.c | 15 ++++++++++++---
 2 files changed, 19 insertions(+), 6 deletions(-)

diff --git a/net/netfilter/nf_conncount.c b/net/netfilter/nf_conncount.c
index 833ef7507af5..cd26ba65eb09 100644
--- a/net/netfilter/nf_conncount.c
+++ b/net/netfilter/nf_conncount.c
@@ -408,7 +408,7 @@ insert_tree(struct net *net,
 			int ret;
 
 			ret = nf_conncount_add_skb(net, skb, l3num, &rbconn->list);
-			if (ret)
+			if (ret && ret != -EINVAL)
 				count = 0; /* hotdrop */
 			else
 				count = rbconn->list.count;
@@ -511,10 +511,14 @@ count_tree(struct net *net,
 			/* same source network -> be counted! */
 			ret = __nf_conncount_add(net, skb, l3num, &rbconn->list);
 			spin_unlock_bh(&rbconn->list.list_lock);
-			if (ret)
+			if (ret && ret != -EINVAL) {
 				return 0; /* hotdrop */
-			else
+			} else {
+				/* -EINVAL means add was skipped, update the list */
+				if (ret == -EINVAL)
+					nf_conncount_gc_list(net, &rbconn->list);
 				return rbconn->list.count;
+			}
 		}
 	}
 
diff --git a/net/netfilter/nft_connlimit.c b/net/netfilter/nft_connlimit.c
index 00c247601173..ba786085389b 100644
--- a/net/netfilter/nft_connlimit.c
+++ b/net/netfilter/nft_connlimit.c
@@ -28,9 +28,18 @@ static inline void nft_connlimit_do_eval(struct nft_connlimit *priv,
 	int err;
 
 	err = nf_conncount_add_skb(nft_net(pkt), pkt->skb, nft_pf(pkt), priv->list);
-	if (err != EINVAL) {
-		regs->verdict.code = NF_DROP;
-		return;
+	if (err) {
+		if (err == -EINVAL) {
+			/* Call gc to update the list count if any connection has
+			 * been closed already. This is useful for softlimit
+			 * connections like limiting bandwidth based on a number
+			 * of open connections.
+			 */
+			nf_conncount_gc_list(nft_net(pkt), priv->list);
+		} else {
+			regs->verdict.code = NF_DROP;
+			return;
+		}
 	}
 
 	count = READ_ONCE(priv->list->count);
-- 
2.51.0

Re: [PATCH 0/6 nf-next v3] netfilter: rework conncount API to receive sk_buff directly

[Pablo Neira Ayuso]

Hi Fernando,

On Wed, Nov 12, 2025 at 12:43:46PM +0100, Fernando Fernandez Mancera wrote:
> This series is fixing two different problems. The first issue is related
> to duplicated entries when used for non-confirmed connections in
> nft_connlimit and xt_connlimit. Now, nf_conncount_add() checks whether
> the connection is confirmed or not. If the connection is confirmed,
> skip the add.
> 
> In order to do that, nf_conncount_count_skb() and nf_conncount_add_skb()
> API has been introduced. They allow the user to pass the sk_buff
> directly. The old API has been removed.
> 
> The second issue this series is fixing is related to
> nft_connlimit/xt_connlimit not updating the list of connection for
> confirmed connections breaking softlimiting use-cases like limiting the
> bandwidth when too many connections are open.
> 
> This has been tested with nftables and iptables both in filter and raw
> priorities. I have stressed the system up to 2000 connections.
> 
> CC'ing openvswitch maintainers as this change on the API required me to
> touch their code. I am not very familiar with the internals of
> openvswitch but I believe this should be fine. If you could provide some
> testing from openvswitch side it would be really helpful.
> 
> Fernando Fernandez Mancera (6):
>   netfilter: nf_conncount: introduce new nf_conncount_count_skb() API
>   netfilter: xt_connlimit: use nf_conncount_count_skb() directly
>   openvswitch: use nf_conncount_count_skb() directly
>   netfilter: nf_conncount: pass the sk_buff down to __nf_conncount_add()

I have collapsed this four patches initial patches (1-4) to see how it
looks:

 include/net/netfilter/nf_conntrack_count.h |   17 ++++-----
 net/netfilter/nf_conncount.c               |  159 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++--------------------------
 net/netfilter/nft_connlimit.c              |   21 +----------
 net/netfilter/xt_connlimit.c               |   14 +------
 net/openvswitch/conntrack.c                |   16 ++++----
 5 files changed, 133 insertions(+), 94 deletions(-)

It is a bit large, but I find it easier to understand the goal,
because this patch is pushing down the skb into the conncount core and
adapting callers at the same time, which is what Florian suggested.

Then, another patch to add the special -EINVAL case for already
confirmed conntracks that is in patch 6/6 in this series. This is to
deal with the new use-case of using ct count really for counting, not
just for limiting.

Finally, the gc consolidation.

I pushed it to this branch in nf-next.git,

        https://git.kernel.org/pub/scm/linux/kernel/git/netfilter/nf-next.git/log/?h=tentative-conncount-series

NOTE: commit messages would need an adjustment.

Sidenote: Not related, but connlimit does not work for bridge and
netdev families because of nft_pf(). This relates to another topic
that is being discussing about how to handle vlan/pppoe packets.
**No need to address this series**, just mentioning it.

Re: [PATCH 0/6 nf-next v3] netfilter: rework conncount API to receive sk_buff directly

[Fernando Fernandez Mancera]

On 11/21/25 12:43 AM, Pablo Neira Ayuso wrote:
> Hi Fernando,
> 
> On Wed, Nov 12, 2025 at 12:43:46PM +0100, Fernando Fernandez Mancera wrote:
>> This series is fixing two different problems. The first issue is related
>> to duplicated entries when used for non-confirmed connections in
>> nft_connlimit and xt_connlimit. Now, nf_conncount_add() checks whether
>> the connection is confirmed or not. If the connection is confirmed,
>> skip the add.
>>
>> In order to do that, nf_conncount_count_skb() and nf_conncount_add_skb()
>> API has been introduced. They allow the user to pass the sk_buff
>> directly. The old API has been removed.
>>
>> The second issue this series is fixing is related to
>> nft_connlimit/xt_connlimit not updating the list of connection for
>> confirmed connections breaking softlimiting use-cases like limiting the
>> bandwidth when too many connections are open.
>>
>> This has been tested with nftables and iptables both in filter and raw
>> priorities. I have stressed the system up to 2000 connections.
>>
>> CC'ing openvswitch maintainers as this change on the API required me to
>> touch their code. I am not very familiar with the internals of
>> openvswitch but I believe this should be fine. If you could provide some
>> testing from openvswitch side it would be really helpful.
>>
>> Fernando Fernandez Mancera (6):
>>    netfilter: nf_conncount: introduce new nf_conncount_count_skb() API
>>    netfilter: xt_connlimit: use nf_conncount_count_skb() directly
>>    openvswitch: use nf_conncount_count_skb() directly
>>    netfilter: nf_conncount: pass the sk_buff down to __nf_conncount_add()
> 
> I have collapsed this four patches initial patches (1-4) to see how it
> looks:
> 
>   include/net/netfilter/nf_conntrack_count.h |   17 ++++-----
>   net/netfilter/nf_conncount.c               |  159 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++--------------------------
>   net/netfilter/nft_connlimit.c              |   21 +----------
>   net/netfilter/xt_connlimit.c               |   14 +------
>   net/openvswitch/conntrack.c                |   16 ++++----
>   5 files changed, 133 insertions(+), 94 deletions(-)
> 
> It is a bit large, but I find it easier to understand the goal,
> because this patch is pushing down the skb into the conncount core and
> adapting callers at the same time, which is what Florian suggested.
> 
> Then, another patch to add the special -EINVAL case for already
> confirmed conntracks that is in patch 6/6 in this series. This is to
> deal with the new use-case of using ct count really for counting, not
> just for limiting.
> 
> Finally, the gc consolidation.
> 

All looks good to me, except the gc consolidation must be before the 
nft_connlimit one.. otherwise nft_connlimit would be calling it without 
disabling bh. Anyway, I just sent a v4 with the collapsed patches, 
commit description adjusted and the correct ordering for the last two 
patches.

Thanks,
Fernando.

> I pushed it to this branch in nf-next.git,
> 
>          https://git.kernel.org/pub/scm/linux/kernel/git/netfilter/nf-next.git/log/?h=tentative-conncount-series
> 
> NOTE: commit messages would need an adjustment.
> 
> Sidenote: Not related, but connlimit does not work for bridge and
> netdev families because of nft_pf(). This relates to another topic
> that is being discussing about how to handle vlan/pppoe packets.
> **No need to address this series**, just mentioning it.
> 

Yes, I will need to look into this during the next release cycle I 
guess. Thanks for mentioning it.