From owner-p4-projects@FreeBSD.ORG Thu Jul 23 15:07:06 2009 Return-Path: Delivered-To: p4-projects@freebsd.org Received: by hub.freebsd.org (Postfix, from userid 32767) id C0E5F1065670; Thu, 23 Jul 2009 15:07:05 +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 734F21065678 for ; Thu, 23 Jul 2009 15:07:05 +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 5FBB38FC1D for ; Thu, 23 Jul 2009 15:07:05 +0000 (UTC) (envelope-from andre@freebsd.org) Received: from repoman.freebsd.org (localhost [127.0.0.1]) by repoman.freebsd.org (8.14.3/8.14.3) with ESMTP id n6NF75H2035513 for ; Thu, 23 Jul 2009 15:07:05 GMT (envelope-from andre@freebsd.org) Received: (from perforce@localhost) by repoman.freebsd.org (8.14.3/8.14.3/Submit) id n6NF74vk035511 for perforce@freebsd.org; Thu, 23 Jul 2009 15:07:04 GMT (envelope-from andre@freebsd.org) Date: Thu, 23 Jul 2009 15:07:04 GMT Message-Id: <200907231507.n6NF74vk035511@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 166452 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, 23 Jul 2009 15:07:06 -0000 http://perforce.freebsd.org/chv.cgi?CH=166452 Change 166452 by andre@andre_t61 on 2009/07/23 15:06:21 Simplify m_trimhead(). In tcp_reass_verity() differentiate rcv_nxt check. Ignore rcv_reass_size, it's not tracked at the moment. Simplify and fix SACK tracking. Swap some KASSERT arguments for clarity. Simplify and fix the block pre-merge logic. Correctly set reassembly queue timeout. Separate block trimming into its own function tcp_reass_trim. Fix tcp_reass_merge to deal with all situations and edge-cases. Affected files ... .. //depot/projects/tcp_reass/netinet/tcp_reass.c#39 edit Differences ... ==== //depot/projects/tcp_reass/netinet/tcp_reass.c#39 (text+ko) ==== @@ -139,13 +139,10 @@ static struct mbuf * m_trimhead(struct mbuf *m) { - struct mbuf *n; + + while (m != NULL && m->m_len == 0) + m = m_free(m); - while (m->m_len == 0) { - n = m; - m = m->m_next; - m_free(n); - } return (m); } @@ -197,7 +194,7 @@ #ifdef INVARIANTS static int -tcp_reass_verify(struct tcpcb *tp) +tcp_reass_verify(struct tcpcb *tp, int prepost) { int i = 0, size = 0, total = 0; struct mbuf *m; @@ -206,8 +203,8 @@ RB_FOREACH_SAFE(trb, tcp_ra, &tp->rcv_reass, trbn) { KASSERT(SEQ_LT(trb->trb_seqs, trb->trb_seqe), ("%s: trb_seqs >= trb_seqe", __func__)); - KASSERT(SEQ_GT(trb->trb_seqs, tp->rcv_nxt), - ("%s: rcv_nxt >= trb_seqs", __func__)); + KASSERT(SEQ_GT(trb->trb_seqs, tp->rcv_nxt) || + prepost, ("%s: rcv_nxt >= trb_seqs", __func__)); KASSERT(trb->trb_m != NULL, ("%s: trb_m == NULL", __func__)); KASSERT(trb->trb_mt != NULL, @@ -222,8 +219,8 @@ total += size; i++; } - KASSERT(tp->rcv_reass_size == total, - ("%s: total not correct", __func__)); + //KASSERT(tp->rcv_reass_size == total, + // ("%s: total not correct", __func__)); LIST_FOREACH(trb, &tp->rcv_reass_sack, trb_sack) { i--; @@ -244,7 +241,6 @@ LIST_REMOVE(trb, trb_sack); if (trb->trb_m != NULL) m_freem(trb->trb_m); - tp->rcv_reass_size -= SEQ_DELTA(trb->trb_seqs, trb->trb_seqe); tp->rcv_reass_blocks--; uma_zfree(tcp_reass_zone, trb); } @@ -255,13 +251,13 @@ struct tcp_reass_block *trb, *trbn; INP_WLOCK_ASSERT(tp->t_inpcb); - KASSERT(tcp_reass_verify(tp), + KASSERT(tcp_reass_verify(tp, 0), ("%s: reassembly queue inconsistent", __func__)); RB_FOREACH_SAFE(trb, tcp_ra, &tp->rcv_reass, trbn) { tcp_reass_free(tp, trb); } - KASSERT(tp->rcv_reass_size == 0, ("%s: snd_sacked not zero", __func__)); + //KASSERT(tp->rcv_reass_size == 0, ("%s: snd_sacked not zero", __func__)); } static __inline void @@ -269,8 +265,7 @@ { if (LIST_FIRST(&tp->rcv_reass_sack) != trb) { - if (!LIST_EMPTY(&tp->rcv_reass_sack)) - LIST_REMOVE(trb, trb_sack); + LIST_REMOVE(trb, trb_sack); LIST_INSERT_HEAD(&tp->rcv_reass_sack, trb, trb_sack); } } @@ -325,11 +320,11 @@ if (!tcp_reass_enable && RB_EMPTY(&tp->rcv_reass)) goto done; - KASSERT(SEQ_LT(tp->rcv_nxt, th_seq), + KASSERT(SEQ_GEQ(th_seq, tp->rcv_nxt), ("%s: sequence number below rcv_nxt", __func__)); KASSERT(!(tp->rcv_nxt == th_seq) || !(RB_EMPTY(&tp->rcv_reass)), ("%s: got missing segment but queue is empty", __func__)); - KASSERT(tcp_reass_verify(tp), + KASSERT(tcp_reass_verify(tp, 0), ("%s: reassembly queue already inconsistent", __func__)); /* @@ -392,7 +387,7 @@ */ m_demote(m, 1); m = m_trimhead(m); - if (tcp_reass_spacetime) + if (tcp_reass_spacetime && m->m_next != NULL) m = m_collapse(m, M_DONTWAIT, 1024); KASSERT(m != NULL, ("%s: m is NULL after collapse", __func__)); @@ -408,7 +403,8 @@ * insert a new reassembly block. */ if ((trb = RB_FIND(tcp_ra, &tp->rcv_reass, &trbs)) != NULL) { - /* Within an already known block. */ + + /* Within an already received block. */ if (SEQ_GEQ(trbs.trb_seqs, trb->trb_seqs) && SEQ_LEQ(trbs.trb_seqe, trb->trb_seqe)) { tcp_reass_sacktrack(tp, trb); @@ -416,33 +412,25 @@ tp->rcv_reass_dsack.end = trbs.trb_seqe; goto done; } - tp->rcv_reass_size += SEQ_DELTA(trb->trb_seqs, trb->trb_seqe); - /* Extends the end, common case. */ - if (SEQ_GT(trbs.trb_seqe, trb->trb_seqe)) { - (void)tcp_reass_merge(trb, &trbs); - tcp_reass_sacktrack(tp, trb); + /* Merge in the new segment. */ + (void)tcp_reass_merge(trb, &trbs); + tcp_reass_sacktrack(tp, trb); - /* Merge in next blocks if there is overlap. */ - while ((trbn = RB_NEXT(tcp_ra, &tp->rcv_reass, trb)) != NULL && - SEQ_GEQ(trb->trb_seqe, trbn->trb_seqs)) { - trbn = tcp_reass_merge(trb, trbn); - tcp_reass_free(tp, trbn); - } + /* Merge in previous block(s) if there is overlap. */ + while ((trbn = RB_PREV(tcp_ra, &tp->rcv_reass, trb)) != NULL && + SEQ_LEQ(trb->trb_seqs, trbn->trb_seqe)) { + trbn = tcp_reass_merge(trb, trbn); + tcp_reass_free(tp, trbn); } - /* Extends the start. */ - if (SEQ_LT(trbs.trb_seqs, trb->trb_seqs)) { - (void)tcp_reass_merge(trb, &trbs); - tcp_reass_sacktrack(tp, trb); + /* Merge in next block(s) if there is overlap. */ + while ((trbn = RB_NEXT(tcp_ra, &tp->rcv_reass, trb)) != NULL && + SEQ_GEQ(trb->trb_seqe, trbn->trb_seqs)) { + trbn = tcp_reass_merge(trb, trbn); + tcp_reass_free(tp, trbn); + } - /* Merge in previous blocks if there is overlap. */ - while ((trbn = RB_PREV(tcp_ra, &tp->rcv_reass, trb)) != NULL && - SEQ_LEQ(trb->trb_seqs, trbn->trb_seqe)) { - trbn = tcp_reass_merge(trb, trbn); - tcp_reass_free(tp, trbn); - } - } } else if (tp->rcv_nxt == th_seq) { trb = &trbs; } else if ((trb = (struct tcp_reass_block *)uma_zalloc(tcp_reass_zone, (M_NOWAIT|M_ZERO))) != NULL) { @@ -452,14 +440,13 @@ trb->trb_mt = trbs.trb_mt; trbn = RB_INSERT(tcp_ra, &tp->rcv_reass, trb); KASSERT(trbn == NULL, ("%s: RB_INSERT failed", __func__)); - tcp_reass_sacktrack(tp, trb); - tp->rcv_reass_size += SEQ_DELTA(trb->trb_seqs, trb->trb_seqe); + LIST_INSERT_HEAD(&tp->rcv_reass_sack, trb, trb_sack); tp->rcv_reass_blocks++; } else { /* Memory allocation failure. */ goto done; } - KASSERT(tcp_reass_verify(tp), + KASSERT(tcp_reass_verify(tp, 1), ("%s: reassembly queue went inconsistent", __func__)); if (trb->trb_seqs == tp->rcv_nxt) @@ -500,8 +487,7 @@ * deactivate it. */ if (tcp_reass_timeout && !RB_EMPTY(&tp->rcv_reass)) - tcp_timer_activate(tp, TT_REASS, - tp->t_rxtcur * tcp_reass_timeout); + tcp_timer_activate(tp, TT_REASS, tcp_reass_timeout); else tcp_timer_activate(tp, TT_REASS, 0); @@ -515,8 +501,27 @@ } /* + * Trim a block. + * Positive value is from head, negative from tail. + */ +static void +tcp_reass_trim(struct tcp_reass_block *trb, int i) +{ + + m_adj(trb->trb_m, i); + + if (i < 0) { + trb->trb_mt = m_last(trb->trb_m); + trb->trb_seqe -= i; + } else { + trb->trb_m = m_trimhead(trb->trb_m); + trb->trb_seqs += i; + } +} + +/* * Merge one or more consecutive blocks together. - * Always merge trbn into trb! + * NB: trbn is always merged into trb! */ static struct tcp_reass_block * tcp_reass_merge(struct tcp_reass_block *trb, struct tcp_reass_block *trbn) @@ -526,40 +531,53 @@ KASSERT(trb != NULL && trbn != NULL, ("%s: incomplete input", __func__)); - /* Append and prepend. */ - if (SEQ_GEQ(trb->trb_seqe, trbn->trb_seqs)) { - if (SEQ_GEQ(trb->trb_seqe, trbn->trb_seqe)) - return (trbn); - if ((i = SEQ_DELTA(trb->trb_seqe, trbn->trb_seqs)) > 0) { - m_adj(trbn->trb_m, i); - trbn->trb_m = m_trimhead(trbn->trb_m); + KASSERT(SEQ_LT(trbn->trb_seqs, trb->trb_seqs) || + SEQ_GT(trbn->trb_seqe, trb->trb_seqe), + ("%s: ", __func__)); + + /* Replace, prepend and append. */ + if (SEQ_LEQ(trbn->trb_seqs, trb->trb_seqs) && + SEQ_GEQ(trbn->trb_seqe, trb->trb_seqe)) { + + m_freem(trb->trb_m); + trb->trb_seqs = trbn->trb_seqs; + trb->trb_seqe = trbn->trb_seqe; + trb->trb_m = trbn->trb_m; + trb->trb_mt = trbn->trb_mt; + + } else if (SEQ_LT(trbn->trb_seqs, trb->trb_seqs)) { + + if ((i = SEQ_DELTA(trb->trb_seqs, trbn->trb_seqe)) > 0) + tcp_reass_trim(trbn, -i); + + trb->trb_seqs = trbn->trb_seqs; + trbn->trb_mt->m_next = trb->trb_m; + trb->trb_m = trbn->trb_m; + + if (tcp_reass_spacetime) { + trbn->trb_mt = m_collapse(trbn->trb_mt, M_DONTWAIT, 1024); + trb->trb_mt = m_last(trbn->trb_mt); } + + } else if (SEQ_GT(trbn->trb_seqe, trb->trb_seqe)) { + + if ((i = SEQ_DELTA(trb->trb_seqe, trbn->trb_seqs)) > 0) + tcp_reass_trim(trbn, i); + trb->trb_seqe = trbn->trb_seqe; trb->trb_mt->m_next = trbn->trb_m; + if (tcp_reass_spacetime) { trb->trb_mt = m_collapse(trb->trb_mt, M_DONTWAIT, 1024); trb->trb_mt = m_last(trb->trb_mt); } else trb->trb_mt = trbn->trb_mt; - } else if (SEQ_LEQ(trb->trb_seqs, trbn->trb_seqe)) { - if (SEQ_LEQ(trb->trb_seqs, trbn->trb_seqs)) - return (trbn); - if ((i = SEQ_DELTA(trb->trb_seqs, trbn->trb_seqe)) > 0) { - m_adj(trb->trb_m, i); - trb->trb_m = m_trimhead(trb->trb_m); - } - trb->trb_seqs = trbn->trb_seqs; - trb->trb_m = trbn->trb_m; - trbn->trb_mt->m_next = trb->trb_m; - if (tcp_reass_spacetime) { - trbn->trb_mt = m_collapse(trbn->trb_mt, M_DONTWAIT, 1024); - trb->trb_mt = m_last(trbn->trb_mt); - } + } else return (NULL); trbn->trb_seqs = 0; - trbn->trb_seqe = i; + trbn->trb_seqe = 0; trbn->trb_m = NULL; trbn->trb_mt = NULL; return (trbn);