From owner-p4-projects@FreeBSD.ORG Thu Jan 17 00:57:30 2008 Return-Path: Delivered-To: p4-projects@freebsd.org Received: by hub.freebsd.org (Postfix, from userid 32767) id 49C8116A46B; Thu, 17 Jan 2008 00:57:30 +0000 (UTC) Delivered-To: perforce@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 0F7F916A418 for ; Thu, 17 Jan 2008 00:57:30 +0000 (UTC) (envelope-from andre@freebsd.org) Received: from repoman.freebsd.org (repoman.freebsd.org [IPv6:2001:4f8:fff6::29]) by mx1.freebsd.org (Postfix) with ESMTP id 004D613C474 for ; Thu, 17 Jan 2008 00:57:29 +0000 (UTC) (envelope-from andre@freebsd.org) Received: from repoman.freebsd.org (localhost [127.0.0.1]) by repoman.freebsd.org (8.14.1/8.14.1) with ESMTP id m0H0vTsr077319 for ; Thu, 17 Jan 2008 00:57:29 GMT (envelope-from andre@freebsd.org) Received: (from perforce@localhost) by repoman.freebsd.org (8.14.1/8.14.1/Submit) id m0H0vTJx077316 for perforce@freebsd.org; Thu, 17 Jan 2008 00:57:29 GMT (envelope-from andre@freebsd.org) Date: Thu, 17 Jan 2008 00:57:29 GMT Message-Id: <200801170057.m0H0vTJx077316@repoman.freebsd.org> X-Authentication-Warning: repoman.freebsd.org: perforce set sender to andre@freebsd.org using -f From: Andre Oppermann To: Perforce Change Reviews Cc: Subject: PERFORCE change 133444 for review X-BeenThere: p4-projects@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: p4 projects tree changes List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 17 Jan 2008 00:57:30 -0000 http://perforce.freebsd.org/chv.cgi?CH=133444 Change 133444 by andre@andre_flirtbox on 2008/01/17 00:56:27 Fix KASSERTs to match inverse test logic. Remove bogus KASSERT. TAILQ_FOREACH_SAFE doesn't initialize the temp variable on the first loop, do it manually. The code depends on it being tracked correctly. Maybe this should be fixed in the macro itself. Tighten segment to block match tests. Affected files ... .. //depot/projects/tcp_reass/netinet/tcp_reass.c#8 edit Differences ... ==== //depot/projects/tcp_reass/netinet/tcp_reass.c#8 (text+ko) ==== @@ -163,11 +163,22 @@ KASSERT(*tlenp > 0, ("%s: segment doesn't contain any data", __func__)); - KASSERT(SEQ_GT(tp->rcv_nxt, th->th_seq), + KASSERT(SEQ_LEQ(tp->rcv_nxt, th->th_seq), ("%s: sequence number below rcv_nxt", __func__)); KASSERT(!(tp->rcv_nxt == th->th_seq) || !(TAILQ_EMPTY(&tp->t_trq)), ("%s: got missing segment but queue is empty", __func__)); +#ifdef INVARIANTS + TAILQ_FOREACH(tqe, &tp->t_trq, trq_q) { + tqen = TAILQ_NEXT(tqe, trq_q); + KASSERT(SEQ_GEQ(tqe->trq_seq, tp->rcv_nxt), + ("%s: trq_seq < rcv_nxt", __func__)); + KASSERT(tqen == NULL || + SEQ_LT(tqe->trq_seq + tqe->trq_len, tqen->trq_seq), + ("%s: overlapping blocks", __func__)); + } +#endif + /* * Limit the number of segments in the reassembly queue to prevent * holding on to too many segments (and thus running out of mbufs). @@ -236,7 +247,7 @@ /* Check if this is the missing segment. */ if (tp->rcv_nxt == th->th_seq) { tqe = TAILQ_FIRST(&tp->t_trq); - KASSERT(SEQ_LEQ(tqe->trq_seq, th->th_seq), + KASSERT(SEQ_GT(tqe->trq_seq, th->th_seq), ("%s: first block starts below missing segment", __func__)); /* Check if segment prepends first block. */ if (SEQ_LEQ(tqe->trq_seq, th->th_seq + *tlenp)) { @@ -269,19 +280,15 @@ tcpstat.tcps_rcvoobyte += *tlenp; /* See where it fits. */ - TAILQ_FOREACH_SAFE(tqe, &tp->t_trq, trq_q, tqen) { -#if 1 + TAILQ_FOREACH(tqe, &tp->t_trq, trq_q) { + tqen = TAILQ_NEXT(tqe, trq_q); /* Segment is after this blocks coverage. */ if (SEQ_LT(tqe->trq_seq + tqe->trq_len, th->th_seq)) continue; -#endif /* Segment is after the previous one but before this one. */ if (SEQ_GT(tqe->trq_seq, th->th_seq + *tlenp)) break; /* Insert as new block. */ - KASSERT(SEQ_GT(tqe->trq_seq, th->th_seq + *tlenp), - ("%s: iterated past insert point", __func__)); - /* Segment is already fully covered. */ if (SEQ_LEQ(tqe->trq_seq, th->th_seq) && SEQ_GEQ(tqe->trq_seq + tqe->trq_len, th->th_seq + *tlenp)) { @@ -293,7 +300,7 @@ } /* Segment covers and extends on both ends. */ - if (SEQ_GEQ(tqe->trq_seq, th->th_seq) && + if (SEQ_GT(tqe->trq_seq, th->th_seq) && SEQ_LT(tqe->trq_seq + tqe->trq_len, th->th_seq + *tlenp)) { /* Replace block content. */ tp->t_trqmcnt -= tqe->trq_mcnt; @@ -313,6 +320,7 @@ /* Segment prepends to this block. */ if (SEQ_GT(tqe->trq_seq, th->th_seq) && + SEQ_LEQ(tqe->trq_seq, th->th_seq + *tlenp) && SEQ_GEQ(tqe->trq_seq + tqe->trq_len, th->th_seq + *tlenp)) { /* Trim tail of segment. */ if ((i = SEQ_DELTA(tqe->trq_seq, th->th_seq + *tlenp))) { @@ -337,8 +345,9 @@ } /* Segment appends to this block. */ - if (SEQ_LEQ(tqe->trq_seq + tqe->trq_len, th->th_seq) && - SEQ_LT(tqe->trq_seq + tqe->trq_len, th->th_seq + *tlenp)) { + if (SEQ_LT(tqe->trq_seq + tqe->trq_len, th->th_seq + *tlenp) && + SEQ_LEQ(tqe->trq_seq, th->th_seq) && + SEQ_GEQ(tqe->trq_seq + tqe->trq_len, th->th_seq)) { /* Trim head of segment. */ if ((i = SEQ_DELTA(tqe->trq_seq + tqe->trq_len, th->th_seq))) { m_adj(m, i); @@ -379,7 +388,7 @@ /* Where to insert. */ if (tqe != NULL && SEQ_LT(tqe->trq_seq + tqe->trq_len, th->th_seq)) - TAILQ_INSERT_TAIL(&tp->t_trq, tqen, trq_q); + TAILQ_INSERT_AFTER(&tp->t_trq, tqe, tqen, trq_q); else if (tqe != NULL) TAILQ_INSERT_BEFORE(tqe, tqen, trq_q); else { @@ -396,10 +405,16 @@ * Present data to user, advancing rcv_nxt through * completed sequence space. */ + KASSERT(!TAILQ_EMPTY(&tp->t_trq), + ("%s: queue empty at present", __func__)); SOCKBUF_LOCK(&so->so_rcv); + tqen = TAILQ_NEXT(TAILQ_FIRST(&tp->t_trq), trq_q); TAILQ_FOREACH_SAFE(tqe, &tp->t_trq, trq_q, tqen) { KASSERT(SEQ_GEQ(tqe->trq_seq, tp->rcv_nxt), ("%s: trq_seq < rcv_nxt", __func__)); + KASSERT(tqen == NULL || + SEQ_LEQ(tqe->trq_seq + tqe->trq_len, tqen->trq_seq), + ("%s: block overlaps into next one", __func__)); if (tqe->trq_seq != tp->rcv_nxt) break; #if 1