Re: [PATCH 7/7] netfilter: connlimit: use rbtree for per-host conntrack obj storage

[Florian Westphal <fw@strlen.de>]

Florian Westphal <fw@strlen.de> wrote:
> Eric Dumazet <eric.dumazet@gmail.com> wrote:
> > > Hmm, that would be nice.  I need to think about it again,
> > > problem is that moving it at this time could result in
> > > freeing the would-be parent of the new node.
> > 
> > Yeah, thats why fq_gc() is followed by a full lookup.
> > 
> > In practice, the lookup done in fq_gc() brings in cpu cache all the
> > cache lines, and second lookup is very fast.
> 
> I had wondered about this.  Ok, that makes sense.
> I'll change it to be more like fq.
> 
> Thanks for explaining this.

Not exactly pretty, but in most cases the restart won't be needed
(either because we found node-to-add-to or no stale node to remove).

I'll fold the following into patch #7, but will wait until Tuesday
before resend in order to give others a chance to comment.

diff --git a/net/netfilter/xt_connlimit.c b/net/netfilter/xt_connlimit.c
--- a/net/netfilter/xt_connlimit.c
+++ b/net/netfilter/xt_connlimit.c
@@ -202,7 +202,9 @@ count_tree(struct net *net, struct rb_root *root,
 	struct xt_connlimit_conn *conn;
 	unsigned int count = 0;
 	unsigned int gc_count = 0;
+	bool no_gc = false;
 
+ restart:
 	rbnode = &(root->rb_node);
 	while (*rbnode) {
 		int diff;
@@ -230,7 +232,7 @@ count_tree(struct net *net, struct rb_root *root,
 			return count + 1;
 		}
 
-		if (gc_count >= ARRAY_SIZE(gc_nodes))
+		if (no_gc || gc_count >= ARRAY_SIZE(gc_nodes))
 			continue;
 
 		/* only used for GC on hhead, retval and 'addit' ignored */
@@ -239,15 +241,22 @@ count_tree(struct net *net, struct rb_root *root,
 			gc_nodes[gc_count++] = rbconn;
 	}
 
+	if (gc_count) {
+		no_gc = true;
+		tree_nodes_free(root, gc_nodes, gc_count);
+		gc_count = 0;
+		goto restart;
+	}
+
 	/* no match, need to insert new node */
 	rbconn = kmem_cache_alloc(connlimit_rb_cachep, GFP_ATOMIC);
 	if (rbconn == NULL)
-		goto out;
+		return 0;
 
 	conn = kmem_cache_alloc(connlimit_conn_cachep, GFP_ATOMIC);
 	if (conn == NULL) {
 		kmem_cache_free(connlimit_rb_cachep, rbconn);
-		goto out;
+		return 0;
 	}
 
 	conn->tuple = *tuple;
@@ -259,10 +268,7 @@ count_tree(struct net *net, struct rb_root *root,
 
 	rb_link_node(&rbconn->node, parent, rbnode);
 	rb_insert_color(&rbconn->node, root);
-	count = 1;
- out:
-	tree_nodes_free(root, gc_nodes, gc_count);
-	return count;
+	return 1;
 }
 
 static int count_them(struct net *net,