Subject: Re: NMBCLUSTERS again
To: None <6bone@6bone.informatik.uni-leipzig.de>
From: Darren Reed <darrenr@netbsd.org>
List: tech-net
Date: 11/04/2007 10:53:03
This is a multi-part message in MIME format.
--------------000902010204040509000801
Content-Type: text/plain; charset=ISO-8859-1; format=flowed
Content-Transfer-Encoding: 7bit

6bone@6bone.informatik.uni-leipzig.de wrote:
> Hello,
>
> I think, I cannot help to include the debug code into the kernel but I 
> played something with my system. I think the first idea of Darren was 
> correct. If I disable ipfilter, the used mbufs are not longer increasing.

I've looked at the code again and found a few more potential issues
with mbuf's in ipfilter when it comes to pulldown and pullup.  This
patch should be applied on top of the other patch I sent out recently.

Darren



--------------000902010204040509000801
Content-Type: text/plain;
 name="ipf.patch"
Content-Transfer-Encoding: 7bit
Content-Disposition: inline;
 filename="ipf.patch"

Index: ip_fil_netbsd.c
===================================================================
RCS file: /devel/CVS/IP-Filter/ip_fil_netbsd.c,v
retrieving revision 2.55.2.56
diff -u -r2.55.2.56 ip_fil_netbsd.c
--- ip_fil_netbsd.c	27 Oct 2007 23:17:26 -0000	2.55.2.56
+++ ip_fil_netbsd.c	4 Nov 2007 18:45:54 -0000
@@ -939,8 +939,7 @@
 	ohlen = 0;
 	ifp = fin->fin_ifp;
 	if (fin->fin_v == 4) {
-		if ((fin->fin_p == IPPROTO_ICMP) &&
-		    !(fin->fin_flx & FI_SHORT))
+		if ((fin->fin_p == IPPROTO_ICMP) && !(fin->fin_flx & FI_SHORT))
 			switch (ntohs(fin->fin_data[0]) >> 8)
 			{
 			case ICMP_ECHO :
@@ -1871,7 +1870,7 @@
 int len;
 {
 	int out = fin->fin_out, dpoff, ipoff;
-	mb_t *m = xmin;
+	mb_t *m = xmin, *n;
 	char *ip;
 
 	if (m == NULL)
@@ -1894,6 +1893,19 @@
 		 * rather than check..
 		 * Should not make any real difference, anyway.
 		 */
+		n = *fin->fin_mp;
+		if (m != n) {
+			/*
+			 * Record the mbuf that points to the mbuf that we're
+			 * about to go to work on so that we can update the
+			 * m_next appropriately later.
+			 */
+			for (; n->m_next != m; n = n->m_next)
+				;
+		} else {
+			n = NULL;
+		}
+
 		if (len > MHLEN)
 #else
 		if (len > MLEN)
@@ -1905,13 +1917,25 @@
 #else
 			FREE_MB_T(*fin->fin_mp);
 			m = NULL;
+			n = NULL;
 #endif
 		} else
 		{
 			m = m_pullup(m, len);
 		}
-		*fin->fin_mp = m;
+		if (n != NULL)
+			n->m_next = m;
 		if (m == NULL) {
+			/*
+			 * When n is non-NULL, it indicates that m pointed to
+			 * a sub-chain (tail) of the mbuf and that the head
+			 * of this chain has not yet been free'd.
+			 */
+			if (n != NULL) {
+				FREE_MB_T(*fin->fin_mp);
+			}
+
+			*fin->fin_mp = NULL;
 			fin->fin_m = NULL;
 			ATOMIC_INCL(frstats[out].fr_pull[1]);
 			return NULL;

--------------000902010204040509000801--