[PATCH 2.4] netfilter: fix expectation evict order

[PATCH 2.4] netfilter: fix expectation evict order

[Harald Welte]

[-- Attachment #1: Type: text/plain, Size: 3428 bytes --]

Hi Dave!

This mail contains the fourth netfilter patch, please apply.

Author: Patrick McHardy <kaber@trash.net>

This patch fixes the order in whcih expectations are evicted when a helper
specifies IP_CT_HELPER_F_REUSE_EXPECT.  Prior to that patch, each new
expectation is added at the beginning of the masters sibling list.  When
max_expected is reached, the first non-established one on the list is reused
(which gave LIFO behaviour).

The patch
- adds new expectations at the end of the masters sibling list
- uses list_for_each_entry for finding the oldes one
- changes nat core to walk backwards throuth the sibling list when calling
  nat helpers

diff -Nru a/net/ipv4/netfilter/ip_conntrack_core.c b/net/ipv4/netfilter/ip_conntrack_core.c
--- a/net/ipv4/netfilter/ip_conntrack_core.c	Sun Sep 28 20:01:46 2003
+++ b/net/ipv4/netfilter/ip_conntrack_core.c	Sun Sep 28 20:01:46 2003
@@ -972,7 +972,6 @@
 		}
 	} else if (related_to->helper->max_expected && 
 		   related_to->expecting >= related_to->helper->max_expected) {
-		struct list_head *cur_item;
 		/* old == NULL */
 		if (!(related_to->helper->flags & 
 		      IP_CT_HELPER_F_REUSE_EXPECT)) {
@@ -997,21 +996,14 @@
 		       NIPQUAD(related_to->tuplehash[IP_CT_DIR_ORIGINAL].tuple.dst.ip));
  
 		/* choose the the oldest expectation to evict */
-		list_for_each(cur_item, &related_to->sibling_list) { 
-			struct ip_conntrack_expect *cur;
-
-			cur = list_entry(cur_item, 
-					 struct ip_conntrack_expect,
-					 expected_list);
-			if (cur->sibling == NULL) {
-				old = cur;
+		list_for_each_entry(old, &related_to->sibling_list, 
+		                                      expected_list)
+			if (old->sibling == NULL)
 				break;
-			}
-		}
 
-		/* (!old) cannot happen, since related_to->expecting is the
-		 * number of unconfirmed expects */
-		IP_NF_ASSERT(old);
+		/* We cannot fail since related_to->expecting is the number
+		 * of unconfirmed expectations */
+		IP_NF_ASSERT(old && old->sibling == NULL);
 
 		/* newnat14 does not reuse the real allocated memory
 		 * structures but rather unexpects the old and
@@ -1043,7 +1035,7 @@
 	atomic_set(&new->use, 1);
 	
 	/* add to expected list for this connection */	
-	list_add(&new->expected_list, &related_to->sibling_list);
+	list_add_tail(&new->expected_list, &related_to->sibling_list);
 	/* add to global list of expectations */
 	list_prepend(&ip_conntrack_expect_list, &new->list);
 	/* add and start timer if required */
diff -Nru a/net/ipv4/netfilter/ip_nat_core.c b/net/ipv4/netfilter/ip_nat_core.c
--- a/net/ipv4/netfilter/ip_nat_core.c	Sun Sep 28 20:01:46 2003
+++ b/net/ipv4/netfilter/ip_nat_core.c	Sun Sep 28 20:01:46 2003
@@ -810,7 +810,7 @@
 
 		/* Have to grab read lock before sibling_list traversal */
 		READ_LOCK(&ip_conntrack_lock);
-		list_for_each(cur_item, &ct->sibling_list) { 
+		list_for_each_prev(cur_item, &ct->sibling_list) { 
 			exp = list_entry(cur_item, struct ip_conntrack_expect, 
 					 expected_list);
 					 
-- 
- Harald Welte <laforge@netfilter.org>             http://www.netfilter.org/
============================================================================
  "Fragmentation is like classful addressing -- an interesting early
   architectural error that shows how much experimentation was going
   on while IP was being designed."                    -- Paul Vixie

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]

Re: [PATCH 2.4] netfilter: fix expectation evict order

[David S. Miller]

On Sun, 30 Nov 2003 21:01:52 +0530
Harald Welte <laforge@netfilter.org> wrote:

> Author: Patrick McHardy <kaber@trash.net>
> 
> This patch fixes the order in whcih expectations are evicted when a helper
> specifies IP_CT_HELPER_F_REUSE_EXPECT.  Prior to that patch, each new
> expectation is added at the beginning of the masters sibling list.  When
> max_expected is reached, the first non-established one on the list is reused
> (which gave LIFO behaviour).

Applied, thanks.