Re: gradual increase in ip_nfragpackets

[Jonathan Larmour <jifl at eCosCentric dot com>]

Motoya Kurotsu wrote:
> Hi, all,
>
> The attachment is the patch to fix the problems below;
>
>     1) ip_nfragpackets is increasing by dropping fragments due to mbuf 
>        starvation and eventually exceeds ip_maxfragpackets, which 
>        results in refusing all fragments later. 
>     2) maxfragpackets and maxnipq functions implemented approximately 
>        the same limits on fragment memory usage, but in different fashions.

I would like to apply this *but* I would ask you if you can let me know 
what kind of testing you have done on it first?

Although I am quite reassured that it did come from the FreeBSD master 
sources.

Thanks,

Jifl

> 6/13/2003
> Motoya Kurotsu
> Allied Telesis K.K.
>
> diff -Nur bsd_tcpip.orig/current/ChangeLog bsd_tcpip/current/ChangeLog
> --- bsd_tcpip.orig/current/ChangeLog	Mon May 12 20:03:05 2003
> +++ bsd_tcpip/current/ChangeLog	Fri Jun 13 14:44:40 2003
> @@ -1,3 +1,14 @@
> +2003-06-12  Motoya Kurotsu  <kurotsu@allied-telesis.co.jp>
> +
> +        * src/sys/netinet/ip_input.c(ip_reass):
> +	ip_nfragpackets is increasing by dropping fragments due to mbuf 
> +	starvation and eventually exceeds ip_maxfragpackets, which 
> +	results in refusing all fragments later. On the other hand, nipq 
> +	and maxnipq can be used as a substitute for ip_nfragpackets and 
> +	ip_maxfragpackets. This fix is the collection of the fixes done 
> +	at the following revisions of freebsd main branch; 1.221, 1.222, 
> +	1.223 and 1.229.
> +
>  2003-05-12  Motoya Kurotsu  <kurotsu@allied-telesis.co.jp>
>  
>  	* src/ecos/timeout.c (do_timeout): The head of the linked list of
> diff -Nur bsd_tcpip.orig/current/src/sys/netinet/ip_input.c bsd_tcpip/current/src/sys/netinet/ip_input.c
> --- bsd_tcpip.orig/current/src/sys/netinet/ip_input.c	Tue May 21 07:25:02 2002
> +++ bsd_tcpip/current/src/sys/netinet/ip_input.c	Fri Jun 13 14:38:13 2003
> @@ -102,8 +102,8 @@
>  static int	ip_dosourceroute = 0;
>  static int	ip_acceptsourceroute = 0;
>  static int	ip_keepfaith = 0;
> -static int	ip_nfragpackets = 0;
> -static int	ip_maxfragpackets;	/* initialized in ip_init() */
> +static int    nipq = 0;         /* total # of reass queues */
> +static int    maxnipq;
>  
>  /*
>   * XXX - Setting ip_checkinterface mostly implements the receive side of
> @@ -139,8 +139,6 @@
>  	(((((x) & 0xF) | ((((x) >> 8) & 0xF) << 4)) ^ (y)) & IPREASS_HMASK)
>  
>  static struct ipq ipq[IPREASS_NHASH];
> -static int    nipq = 0;         /* total # of reass queues */
> -static int    maxnipq;
>  const  int    ipintrq_present = 1;
>  
>  #ifdef IPSTEALTH
> @@ -226,7 +224,6 @@
>  	    ipq[i].next = ipq[i].prev = &ipq[i];
>  
>  	maxnipq = nmbclusters / 4;
> -	ip_maxfragpackets = nmbclusters / 4;
>  
>  #ifndef RANDOM_IP_ID
>  	ip_id = time_second & 0xffff;
> @@ -678,6 +675,14 @@
>  			ip = mtod(m, struct ip *);
>  		}
>  #endif
> +
> +		/* If maxnipq is 0, never accept fragments. */
> +		if (maxnipq == 0) {
> +                	ipstat.ips_fragments++;
> +			ipstat.ips_fragdropped++;
> +			goto bad;
> +		}
> +
>  		sum = IPREASS_HASH(ip->ip_src.s_addr, ip->ip_id);
>  		/*
>  		 * Look for queue of fragments
> @@ -692,8 +697,12 @@
>  
>  		fp = 0;
>  
> -		/* check if there's a place for the new queue */
> -		if (nipq > maxnipq) {
> +		/*
> +		 * Enforce upper bound on number of fragmented packets
> +		 * for which we attempt reassembly;
> +		 * If maxnipq is -1, accept all fragments without limitation.
> +		 */
> +		if ((nipq > maxnipq) && (maxnipq > 0)) {
>  		    /*
>  		     * drop something from the tail of the current queue
>  		     * before proceeding further
> @@ -702,11 +711,14 @@
>  			for (i = 0; i < IPREASS_NHASH; i++) {
>  			    if (ipq[i].prev != &ipq[i]) {
>  				ip_freef(ipq[i].prev);
> +				ipstat.ips_fragtimeout++;
>  				break;
>  			    }
>  			}
> -		    } else
> +		    } else {
>  			ip_freef(ipq[sum].prev);
> +			ipstat.ips_fragtimeout++;
> +		    }
>  		}
>  found:
>  		/*
> @@ -899,15 +911,6 @@
>  	 * If first fragment to arrive, create a reassembly queue.
>  	 */
>  	if (fp == 0) {
> -		/*
> -		 * Enforce upper bound on number of fragmented packets
> -		 * for which we attempt reassembly;
> -		 * If maxfrag is 0, never accept fragments.
> -		 * If maxfrag is -1, accept all fragments without limitation.
> -		 */
> -		if ((ip_maxfragpackets >= 0) && (ip_nfragpackets >= ip_maxfragpackets))
> -			goto dropfrag;
> -		ip_nfragpackets++;
>  		if ((t = m_get(M_DONTWAIT, MT_FTABLE)) == NULL)
>  			goto dropfrag;
>  		fp = mtod(t, struct ipq *);
> @@ -1056,7 +1059,6 @@
>  	remque(fp);
>  	nipq--;
>  	(void) m_free(dtom(fp));
> -	ip_nfragpackets--;
>  	m->m_len += (IP_VHL_HL(ip->ip_vhl) << 2);
>  	m->m_data -= (IP_VHL_HL(ip->ip_vhl) << 2);
>  	/* some debugging cruft by sklower, below, will go away soon */
> @@ -1097,7 +1099,6 @@
>  	}
>  	remque(fp);
>  	(void) m_free(dtom(fp));
> -	ip_nfragpackets--;
>  	nipq--;
>  }
>  
> @@ -1131,9 +1132,9 @@
>  	 * (due to the limit being lowered), drain off
>  	 * enough to get down to the new limit.
>  	 */
> -	for (i = 0; i < IPREASS_NHASH; i++) {
> -		if (ip_maxfragpackets >= 0) {
> -			while ((ip_nfragpackets > ip_maxfragpackets) &&
> +	if (maxnipq >= 0 && nipq > maxnipq) {
> +		for (i = 0; i < IPREASS_NHASH; i++) {
> +			while ((nipq > maxnipq) &&
>  				(ipq[i].next != &ipq[i])) {
>  				ipstat.ips_fragdropped++;
>  				ip_freef(ipq[i].next);


--
eCosCentric    http://www.eCosCentric.com/    The eCos and RedBoot experts
--[ "You can complain because roses have thorns, or you ]--
--[  can rejoice because thorns have roses." -Lincoln   ]-- Opinions==mine