[PATCH] aggressive early_drop and reserved conntrack entries

Martin Josefsson gandalf at wlug.westbo.se
Sat Dec 11 14:34:14 CET 2004


On Fri, 2004-12-10 at 23:27, Jozsef Kadlecsik wrote:
> On Thu, 9 Dec 2004, Jozsef Kadlecsik wrote:
> 
> > On Thu, 9 Dec 2004, Patrick Schaaf wrote:
> >
> > > This suggest, to me, that we keep unreplied connections on a new,
> > > additional list. They are put there at the HEAD upon creation,
> > > they are removed form the list when they make their transition
> > > to assured. And early_drop becomes a simple, O(1) operation:
> > > reap the connection which is at the TAIL of this new list.
> >
> > But I like it! Hmm, expect some new code soon...
> 
> Attached (;-) is the new patch, which implements the list of unassured
> connections. (Reserving conntracks is dropped completely as unnecessary.)
> I tested it slighgtly and seems to work fine. What do you think about it?

I've been thinking about this as well, but mostly I've been thinking
about how to get it to scale when we go for more finegrained locking.
The locking is going to be nasty.

The patch looks pretty good, just a few things...

@@ -461,34 +467,36 @@
 	return h != NULL;
 }
 
-/* There's a small race here where we may free a just-assured
-   connection.  Too bad: we're in trouble anyway. */
-static inline int unreplied(const struct ip_conntrack_tuple_hash *i)
+static int early_drop(void)
 {
-	return !(test_bit(IPS_ASSURED_BIT, &i->ctrack->status));
-}
-
-static int early_drop(struct list_head *chain)
-{
-	/* Traverse backwards: gives us oldest, which is roughly LRU */
-	struct ip_conntrack_tuple_hash *h;
+	struct ip_conntrack *ct = NULL;
+	struct list_head *entry;
 	int dropped = 0;
 
+    	/* There's a small race here where we may free a just-assured
+	   connection.  Too bad: we're in trouble anyway. */
 	READ_LOCK(&ip_conntrack_lock);
-	h = LIST_FIND_B(chain, unreplied, struct ip_conntrack_tuple_hash *);
-	if (h)
-		atomic_inc(&h->ctrack->ct_general.use);
+	__list_for_each(entry, &unassured_list) {
+		ct = list_entry(entry,
+			        struct ip_conntrack, unassured);
+		atomic_inc(&ct->ct_general.use);
+		break;
+	}
 	READ_UNLOCK(&ip_conntrack_lock);
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
Why not loop and kill multiple entries each time? Saves some locking and
cache. But in order to do that in a good way wee need a counter of how
many entries we have in the unassured list. But we don't want to kill
too many each time, then almost no real connections will get through.

-	if (!h)
+	if (!ct)
 		return dropped;
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
I think we are pretty much guranteed to have a valid ct here, otherwise
we would oops above :)
 
-	if (del_timer(&h->ctrack->timeout)) {
-		death_by_timeout((unsigned long)h->ctrack);
+	if (del_timer(&ct->timeout)) {
+		death_by_timeout((unsigned long)ct);
 		dropped = 1;
+		if (net_ratelimit())
+			printk(KERN_WARNING
+			       "ip_conntrack: table full, dropping"
+			       " unassured connection.\n");
 		CONNTRACK_STAT_INC(early_drop);
 	}
-	ip_conntrack_put(h->ctrack);
+	ip_conntrack_put(ct);
 	return dropped;
 }
 
@@ -1091,7 +1100,8 @@
 void ip_ct_refresh_acct(struct ip_conntrack *ct, 
 		        enum ip_conntrack_info ctinfo,
 			const struct sk_buff *skb,
-			unsigned long extra_jiffies)
+			unsigned long extra_jiffies,
+			int set_assured)
 {
 	IP_NF_ASSERT(ct->timeout.data == (unsigned long)ct);
 
@@ -1107,6 +1117,10 @@
 			add_timer(&ct->timeout);
 		}
 		ct_add_counters(ct, ctinfo, skb);
+		if (set_assured) {
+			set_bit(IPS_ASSURED_BIT, &ct->status);
+			list_del(&ct->unassured);
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
Huh? No check to see if we already are assured or not?
Not needed for icmp, tcp or sctp but udp and the generic handler does.

-- 
/Martin
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: This is a digitally signed message part
Url : /pipermail/netfilter-devel/attachments/20041211/8829b17b/attachment-0001.bin


More information about the netfilter-devel mailing list