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

6bone@6bone.informatik.uni-leipzig.de wrote:
> Hello Darren,
>
> I have applied your patch and rebuilt the kernel. After booting with 
> the new one I got a kernel trace. You can download it from 
> http://6bone.informatik.uni-leipzig.de/netbsd/trace.jpg

Can you please reverse out the previous patch and try this one?

I've tested it as far as ifconfig (that produced those panics) and believe
the logic is much better now.

Thanks,
Darren


--------------050708060003000003080007
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 -c -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	5 Nov 2007 09:52:50 -0000
***************
*** 1871,1877 ****
  int len;
  {
  	int out = fin->fin_out, dpoff, ipoff;
! 	mb_t *m = xmin;
  	char *ip;
  
  	if (m == NULL)
--- 1875,1881 ----
  int len;
  {
  	int out = fin->fin_out, dpoff, ipoff;
! 	mb_t *m = xmin, *n;
  	char *ip;
  
  	if (m == NULL)
***************
*** 1888,1899 ****
  		dpoff = 0;
  
  	if (M_LEN(m) < len) {
! #ifdef MHLEN
  		/*
  		 * Assume that M_PKTHDR is set and just work with what is left
  		 * rather than check..
  		 * Should not make any real difference, anyway.
  		 */
  		if (len > MHLEN)
  #else
  		if (len > MLEN)
--- 1892,1918 ----
  		dpoff = 0;
  
  	if (M_LEN(m) < len) {
! 		mb_t *ms = m;
! 
! 		n = *fin->fin_mp;
  		/*
  		 * Assume that M_PKTHDR is set and just work with what is left
  		 * rather than check..
  		 * Should not make any real difference, anyway.
  		 */
+ 		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;
+ 		}
+ 
+ #ifdef MHLEN
  		if (len > MHLEN)
  #else
  		if (len > MLEN)
***************
*** 1905,1922 ****
  #else
  			FREE_MB_T(*fin->fin_mp);
  			m = NULL;
  #endif
  		} else
  		{
  			m = m_pullup(m, len);
  		}
! 		*fin->fin_mp = m;
  		if (m == NULL) {
  			fin->fin_m = NULL;
  			ATOMIC_INCL(frstats[out].fr_pull[1]);
  			return NULL;
  		}
  
  		while (M_LEN(m) == 0) {
  			m = m->m_next;
  		}
--- 1924,1956 ----
  #else
  			FREE_MB_T(*fin->fin_mp);
  			m = NULL;
+ 			n = NULL;
  #endif
  		} else
  		{
  			m = m_pullup(m, len);
  		}
! 		if ((ms != n) && (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 ((ms != n) && (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;
  		}
  
+ 		if (ms == n || n == NULL)
+ 			*fin->fin_mp = m;
+ 
  		while (M_LEN(m) == 0) {
  			m = m->m_next;
  		}

--------------050708060003000003080007--