Module Name:    src
Committed By:   martin
Date:           Wed Oct 17 13:38:04 UTC 2018

Modified Files:
        src/sys/netinet [netbsd-8]: ip_reass.c

Log Message:
Pull up following revision(s) (requested by maxv in ticket #1045):

        sys/netinet/ip_reass.c: revision 1.19-1.21

Hold ip_off and ip_len in the fragment entry, instead of always reading
the associated mbuf (and converting to host order). This reduces the
cache/TLB misses when processing long lists.

 -

Kick fragments that would introduce several !MFFs in a reassembly chain.

The problem arises if we receive three fragments of the kind
        3.  A -> has MFF
        1.  B -> doesn't have MFF
        2.  C -> doesn't have MFF

Because of the received order B->C->A, we don't see that B is !MFF, and
therefore that there is a problem in this chain.

Now we do two checks, and drop us if:

 * there is a fragment preceding us, and this fragment is !MFF, or
 * there is a fragment following us, and we are !MFF

Spotted a long time ago.

 -

Force ip_off to zero when the reassembly is complete. This was lost in my
rev1.19 - before that the IP struct was clobbered for the reassembly, but
it actually implicitly guaranteed that the first fragment of the packet
would end up with ip_off = 0, and this was a desired behavior.


To generate a diff of this commit:
cvs rdiff -u -r1.11.8.6 -r1.11.8.7 src/sys/netinet/ip_reass.c

Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.

Modified files:

Index: src/sys/netinet/ip_reass.c
diff -u src/sys/netinet/ip_reass.c:1.11.8.6 src/sys/netinet/ip_reass.c:1.11.8.7
--- src/sys/netinet/ip_reass.c:1.11.8.6	Tue Oct  9 09:44:31 2018
+++ src/sys/netinet/ip_reass.c	Wed Oct 17 13:38:04 2018
@@ -1,4 +1,4 @@
-/*	$NetBSD: ip_reass.c,v 1.11.8.6 2018/10/09 09:44:31 martin Exp $	*/
+/*	$NetBSD: ip_reass.c,v 1.11.8.7 2018/10/17 13:38:04 martin Exp $	*/
 
 /*
  * Copyright (c) 1982, 1986, 1988, 1993
@@ -46,7 +46,7 @@
  */
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: ip_reass.c,v 1.11.8.6 2018/10/09 09:44:31 martin Exp $");
+__KERNEL_RCSID(0, "$NetBSD: ip_reass.c,v 1.11.8.7 2018/10/17 13:38:04 martin Exp $");
 
 #include <sys/param.h>
 #include <sys/types.h>
@@ -80,6 +80,8 @@ typedef struct ipfr_qent {
 	struct ip *		ipqe_ip;
 	struct mbuf *		ipqe_m;
 	bool			ipqe_mff;
+	uint16_t		ipqe_off;
+	uint16_t		ipqe_len;
 } ipfr_qent_t;
 
 TAILQ_HEAD(ipfr_qent_head, ipfr_qent);
@@ -215,7 +217,7 @@ ip_nmbclusters_changed(void)
 struct mbuf *
 ip_reass(ipfr_qent_t *ipqe, ipfr_queue_t *fp, const u_int hash)
 {
-	struct ip *ip = ipqe->ipqe_ip, *qip;
+	struct ip *ip = ipqe->ipqe_ip;
 	const int hlen = ip->ip_hl << 2;
 	struct mbuf *m = ipqe->ipqe_m, *t;
 	int ipsecflags = m->m_flags & (M_DECRYPTED|M_AUTHIPHDR);
@@ -230,16 +232,6 @@ ip_reass(ipfr_qent_t *ipqe, ipfr_queue_t
 	m->m_data += hlen;
 	m->m_len -= hlen;
 
-#ifdef	notyet
-	/* Make sure fragment limit is up-to-date. */
-	CHECK_NMBCLUSTER_PARAMS();
-
-	/* If we have too many fragments, drop the older half. */
-	if (ip_nfrags >= ip_maxfrags) {
-		ip_reass_drophalf(void);
-	}
-#endif
-
 	/*
 	 * We are about to add a fragment; increment frag count.
 	 */
@@ -255,9 +247,9 @@ ip_reass(ipfr_qent_t *ipqe, ipfr_queue_t
 		 * never accept fragments  b) if maxfrag is -1, accept
 		 * all fragments without limitation.
 		 */
-		if (ip_maxfragpackets < 0)
-			;
-		else if (ip_nfragpackets >= ip_maxfragpackets) {
+		if (ip_maxfragpackets < 0) {
+			/* no limit */
+		} else if (ip_nfragpackets >= ip_maxfragpackets) {
 			goto dropfrag;
 		}
 		fp = malloc(sizeof(ipfr_queue_t), M_FTABLE, M_NOWAIT);
@@ -285,7 +277,7 @@ ip_reass(ipfr_qent_t *ipqe, ipfr_queue_t
 	 * Find a segment which begins after this one does.
 	 */
 	TAILQ_FOREACH(q, &fp->ipq_fragq, ipqe_q) {
-		if (ntohs(q->ipqe_ip->ip_off) > ntohs(ip->ip_off))
+		if (q->ipqe_off > ipqe->ipqe_off)
 			break;
 	}
 	if (q != NULL) {
@@ -295,39 +287,45 @@ ip_reass(ipfr_qent_t *ipqe, ipfr_queue_t
 	}
 
 	/*
-	 * If there is a preceding segment, it may provide some of our
-	 * data already.  If so, drop the data from the incoming segment.
-	 * If it provides all of our data, drop us.
+	 * Look at the preceding segment.
+	 *
+	 * If it provides some of our data already, in part or entirely, trim
+	 * us or drop us.
+	 *
+	 * If a preceding segment exists, and was marked as the last segment,
+	 * drop us.
 	 */
 	if (p != NULL) {
-		i = ntohs(p->ipqe_ip->ip_off) + ntohs(p->ipqe_ip->ip_len) -
-		    ntohs(ip->ip_off);
+		i = p->ipqe_off + p->ipqe_len - ipqe->ipqe_off;
 		if (i > 0) {
-			if (i >= ntohs(ip->ip_len)) {
+			if (i >= ipqe->ipqe_len) {
 				goto dropfrag;
 			}
 			m_adj(ipqe->ipqe_m, i);
-			ip->ip_off = htons(ntohs(ip->ip_off) + i);
-			ip->ip_len = htons(ntohs(ip->ip_len) - i);
+			ipqe->ipqe_off = ipqe->ipqe_off + i;
+			ipqe->ipqe_len = ipqe->ipqe_len - i;
 		}
 	}
+	if (p != NULL && !p->ipqe_mff) {
+		goto dropfrag;
+	}
 
 	/*
-	 * While we overlap succeeding segments trim them or, if they are
-	 * completely covered, dequeue them.
+	 * Look at the segments that follow.
+	 *
+	 * If we cover them, in part or entirely, trim them or dequeue them.
+	 *
+	 * If a following segment exists, and we are marked as the last
+	 * segment, drop us.
 	 */
 	while (q != NULL) {
-		size_t end;
-
-		qip = q->ipqe_ip;
-		end = ntohs(ip->ip_off) + ntohs(ip->ip_len);
-		if (end <= ntohs(qip->ip_off)) {
+		i = ipqe->ipqe_off + ipqe->ipqe_len - q->ipqe_off;
+		if (i <= 0) {
 			break;
 		}
-		i = end - ntohs(qip->ip_off);
-		if (i < ntohs(qip->ip_len)) {
-			qip->ip_len = htons(ntohs(qip->ip_len) - i);
-			qip->ip_off = htons(ntohs(qip->ip_off) + i);
+		if (i < q->ipqe_len) {
+			q->ipqe_off = q->ipqe_off + i;
+			q->ipqe_len = q->ipqe_len - i;
 			m_adj(q->ipqe_m, i);
 			break;
 		}
@@ -339,6 +337,9 @@ ip_reass(ipfr_qent_t *ipqe, ipfr_queue_t
 		ip_nfrags--;
 		q = nq;
 	}
+	if (q != NULL && !ipqe->ipqe_mff) {
+		goto dropfrag;
+	}
 
 insert:
 	/*
@@ -351,12 +352,11 @@ insert:
 	}
 	next = 0;
 	TAILQ_FOREACH(q, &fp->ipq_fragq, ipqe_q) {
-		qip = q->ipqe_ip;
-		if (ntohs(qip->ip_off) != next) {
+		if (q->ipqe_off != next) {
 			mutex_exit(&ipfr_lock);
 			return NULL;
 		}
-		next += ntohs(qip->ip_len);
+		next += q->ipqe_len;
 	}
 	p = TAILQ_LAST(&fp->ipq_fragq, ipfr_qent_head);
 	if (p->ipqe_mff) {
@@ -402,6 +402,7 @@ insert:
 	 * header visible.
 	 */
 	ip->ip_len = htons((ip->ip_hl << 2) + next);
+	ip->ip_off = htons(0);
 	ip->ip_src = fp->ipq_src;
 	ip->ip_dst = fp->ipq_dst;
 	free(fp, M_FTABLE);
@@ -651,13 +652,6 @@ ip_reass_packet(struct mbuf **m0, struct
 		return EINVAL;
 	}
 
-	/*
-	 * Adjust total IP length to not reflect header and convert
-	 * offset of this to bytes.  XXX: clobbers struct ip.
-	 */
-	ip->ip_len = htons(flen);
-	ip->ip_off = htons(off);
-
 	/* Look for queue of fragments of this datagram. */
 	mutex_enter(&ipfr_lock);
 	hash = IPREASS_HASH(ip->ip_src.s_addr, ip->ip_id);
@@ -702,6 +696,8 @@ ip_reass_packet(struct mbuf **m0, struct
 	ipqe->ipqe_mff = mff;
 	ipqe->ipqe_m = m;
 	ipqe->ipqe_ip = ip;
+	ipqe->ipqe_off = off;
+	ipqe->ipqe_len = flen;
 
 	*m0 = ip_reass(ipqe, fp, hash);
 	if (*m0) {

Reply via email to