ip_reass() - possibly incorrect goto

[Maxim Konovalov]

On Tue, 22 Mar 2005, 12:08-0800, Vijay.Singh at nokia.com wrote:

> Hi hackers, I am looking at the ip_reass() routine. In case of the
> 1st fragment we create the reassembly queue. After the queue has
> been inserted in the hash bucket, the if () code does a " goto
> inserted". Should this be changed to "goto done" instead? Any code
> that is executed for the 1st fragment, like frag per packet limiting
> and complete reassembly are not valid. Am I mistaken?

Yep, it seems you are right.  The second micro optimization - drop the
fragment early if maxfragsperpacket == 0.

Andre, Mike, what do you think?

Index: ip_input.c
===================================================================
RCS file: /home/ncvs/src/sys/netinet/ip_input.c,v
retrieving revision 1.299
diff -u -r1.299 ip_input.c
--- ip_input.c	16 Mar 2005 05:27:19 -0000	1.299
+++ ip_input.c	23 Mar 2005 13:12:00 -0000
@@ -801,8 +801,8 @@
 	u_int8_t ecn, ecn0;
 	u_short hash;

-	/* If maxnipq is 0, never accept fragments. */
-	if (maxnipq == 0) {
+	/* If maxnipq or maxfragsperpacket are 0, never accept fragments. */
+	if (maxnipq == 0 || maxfragsperpacket == 0) {
 		ipstat.ips_fragments++;
 		ipstat.ips_fragdropped++;
 		m_freem(m);
@@ -918,7 +918,7 @@
 		fp->ipq_dst = ip->ip_dst;
 		fp->ipq_frags = m;
 		m->m_nextpkt = NULL;
-		goto inserted;
+		goto done;
 	} else {
 		fp->ipq_nfrags++;
 #ifdef MAC
@@ -998,8 +998,6 @@
 		m_freem(q);
 	}

-inserted:
-
 	/*
 	 * Check for complete reassembly and perform frag per packet
 	 * limiting.
%%%

-- 
Maxim Konovalov