From owner-p4-projects@FreeBSD.ORG Sat Mar 22 14:59:16 2008 Return-Path: Delivered-To: p4-projects@freebsd.org Received: by hub.freebsd.org (Postfix, from userid 32767) id 7132E1065675; Sat, 22 Mar 2008 14:59:16 +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 14D971065671 for ; Sat, 22 Mar 2008 14:59:16 +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 0C84F8FC1D for ; Sat, 22 Mar 2008 14:59:16 +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 m2MExFfb022184 for ; Sat, 22 Mar 2008 14:59:15 GMT (envelope-from andre@freebsd.org) Received: (from perforce@localhost) by repoman.freebsd.org (8.14.1/8.14.1/Submit) id m2MExFxk022182 for perforce@freebsd.org; Sat, 22 Mar 2008 14:59:15 GMT (envelope-from andre@freebsd.org) Date: Sat, 22 Mar 2008 14:59:15 GMT Message-Id: <200803221459.m2MExFxk022182@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 138280 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: Sat, 22 Mar 2008 14:59:17 -0000 http://perforce.freebsd.org/chv.cgi?CH=138280 Change 138280 by andre@andre_flirtbox on 2008/03/22 14:58:56 Undo overly complex FIN handling in reassembly queue. It's a can of worms and based on a discussion on TCPM not necessary in this complexity. A simpler approach to follow. Affected files ... .. //depot/projects/tcp_reass/netinet/tcp_reass.c#18 edit .. //depot/projects/tcp_reass/netinet/tcp_var.h#8 edit Differences ... ==== //depot/projects/tcp_reass/netinet/tcp_reass.c#18 (text+ko) ==== @@ -198,7 +198,7 @@ * Store TCP header information in local variables as * we may lose access to it after mbuf compacting. */ - thflags = (th->th_flags & TH_FIN); + thflags = th->th_flags; th_seq = th->th_seq; th = NULL; /* Prevent further use. */ @@ -210,6 +210,7 @@ } /* XXX: should not happen, but does for some reason. */ + /* XXX: May get FIN on otherwise empty segment. */ if (*tlenp == 0) { m_freem(m); return (0); @@ -230,15 +231,13 @@ KASSERT(tqen == NULL || SEQ_LT(tqe->trq_seq + tqe->trq_len, tqen->trq_seq), ("%s: overlapping blocks", __func__)); - KASSERT(!(thflags & TH_FIN) || tqe == TAILQ_LAST(&tp->t_trq, trq_head), - ("%s: FIN on block before last one", __func__)); i++; } LIST_FOREACH(tqe, &tp->t_trq_sack, trq_s) { i--; } KASSERT(i == 0, ("%s: SEQ# ordered tailq and arrival ordered " - "list are not equally long", __func__)); + "SACK list are not equally long", __func__)); #endif /* @@ -287,30 +286,14 @@ mcnt += (n->m_flags & M_EXT) ? n->m_ext.ext_size + MSIZE : MSIZE; - /* Check if we've already received FIN; we can't accept data beyond it. */ tqe = TAILQ_LAST(&tp->t_trq, trq_head); - if (tqe && (tqe->trq_thflags & TH_FIN) && SEQ_LEQ(tqe->trq_seq, th_seq)) { - /* Properly count retransmitted perfect matching FIN. */ - if (tqe->trq_seq == th_seq && *tlenp > 0) - tcpstat.tcps_rcvoopack++; - else { - tcpstat.tcps_rcvpackafterfin++; - tcpstat.tcps_rcvbyteafterfin += *tlenp; - } - m_freem(m); - *tlenp = 0; - return (0); - } - /* Check if this segments FIN is before the end of the last block. */ - if (tqe && (thflags & TH_FIN) && - SEQ_GT(tqe->trq_seq + tqe->trq_len, th_seq + *tlenp)) { - /* TCP statistics. */ - tcpstat.tcps_rcvpackafterfin++; - tcpstat.tcps_rcvbyteafterfin += *tlenp; - m_freem(m); - *tlenp = 0; - return (0); + /* + * FIN handling is a bit tricky. + * We only accept a FIN if it matches the right side of the sequence + * space. + */ + if (thflags & TH_FIN) { } /* Check if this segment directly attaches to the end. */ @@ -321,7 +304,6 @@ tcp_reass_mcnt += mcnt; tqe->trq_ml->m_next = m; tqe->trq_ml = m_last(m); - tqe->trq_thflags |= thflags; if (LIST_FIRST(&tp->t_trq_sack) != tqe) { LIST_REMOVE(tqe, trq_s); LIST_INSERT_HEAD(&tp->t_trq_sack, tqe, trq_s); @@ -392,7 +374,6 @@ tcpstat.tcps_rcvduppack++; tcpstat.tcps_rcvdupbyte += *tlenp; tcpstat.tcps_reass_covered++; - tqe->trq_thflags |= thflags; /* XXXAO: What to SACK report when duplicate? */ if (LIST_FIRST(&tp->t_trq_sack) != tqe) { LIST_REMOVE(tqe, trq_s); @@ -416,7 +397,6 @@ tqe->trq_seq = th_seq; tqe->trq_m = m; tqe->trq_ml = m_last(m); - tqe->trq_thflags |= thflags; /* Check if segment bridges next block to merge. */ if (tqen != NULL && SEQ_GEQ(tqe->trq_seq + tqe->trq_len, tqen->trq_seq)) @@ -481,7 +461,6 @@ tcp_reass_mcnt += mcnt; tqe->trq_ml->m_next = m; tqe->trq_ml = m_last(m); - tqe->trq_thflags |= thflags; /* Check if segment bridges two blocks to merge. */ if (tqen != NULL && SEQ_GEQ(tqe->trq_seq + tqe->trq_len, tqen->trq_seq)) @@ -522,7 +501,6 @@ tcp_reass_mcnt += mcnt; tqen->trq_m = m; tqen->trq_ml = m_last(m); - tqen->trq_thflags |= thflags; /* Where to insert. */ if (tqe != NULL && SEQ_LT(tqe->trq_seq + tqe->trq_len, th_seq)) @@ -569,7 +547,6 @@ else sbappendstream_locked(&so->so_rcv, tqe->trq_m); tp->rcv_nxt += tqe->trq_len; - thflags = tqe->trq_thflags; KASSERT(!(thflags & TH_FIN) || tqe == TAILQ_LAST(&tp->t_trq, trq_head), ("%s: FIN not on last block", __func__)); tp->t_trqmcnt -= tqe->trq_mcnt; ==== //depot/projects/tcp_reass/netinet/tcp_var.h#8 (text+ko) ==== @@ -47,7 +47,6 @@ tcp_seq trq_seq; /* start of block */ int trq_len; /* length of block */ int trq_mcnt; /* gross mbuf size of block */ - int trq_thflags; /* thflags for segment chain */ struct mbuf *trq_m; /* mbuf chain of data */ struct mbuf *trq_ml; /* last mbuf in chain of data */ };