From owner-cvs-src@FreeBSD.ORG Wed Jan 23 21:54:06 2008 Return-Path: Delivered-To: cvs-src@FreeBSD.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 323EF16A481 for ; Wed, 23 Jan 2008 21:54:06 +0000 (UTC) (envelope-from andre@freebsd.org) Received: from c00l3r.networx.ch (c00l3r.networx.ch [62.48.2.2]) by mx1.freebsd.org (Postfix) with ESMTP id B31FF13C45B for ; Wed, 23 Jan 2008 21:54:05 +0000 (UTC) (envelope-from andre@freebsd.org) Received: (qmail 85219 invoked from network); 23 Jan 2008 21:15:26 -0000 Received: from localhost (HELO [127.0.0.1]) ([127.0.0.1]) (envelope-sender ) by c00l3r.networx.ch (qmail-ldap-1.03) with SMTP for ; 23 Jan 2008 21:15:26 -0000 Message-ID: <4797B77E.2090605@freebsd.org> Date: Wed, 23 Jan 2008 22:54:06 +0100 From: Andre Oppermann User-Agent: Thunderbird 1.5.0.14 (Windows/20071210) MIME-Version: 1.0 To: Mike Silbersack , kmacy@FreeBSD.org References: <200711200656.lAK6u4bc021279@repoman.freebsd.org> In-Reply-To: <200711200656.lAK6u4bc021279@repoman.freebsd.org> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Cc: cvs-src@FreeBSD.org, src-committers@FreeBSD.org, cvs-all@FreeBSD.org, freebsd-net@freebsd.org Subject: Re: cvs commit: src/sys/netinet tcp_syncache.c X-BeenThere: cvs-src@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: CVS commit messages for the src tree List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 23 Jan 2008 21:54:06 -0000 Mike Silbersack wrote: > silby 2007-11-20 06:56:04 UTC > > FreeBSD src repository > > Modified files: > sys/netinet tcp_syncache.c > Log: > Comment out the syncache's test which ensures that hosts which negotiate TCP > timestamps in the initial SYN packet actually use them in the rest of the > connection. Unfortunately, during the 7.0 testing cycle users have already > found network devices that violate this constraint. > > RFC 1323 states 'and may send a TSopt in other segments' rather than > 'and MUST send', so we must allow it. > > Discovered by: Rob Zietlow > Tracked down by: Kip Macy > PR: bin/118005 > > Revision Changes Path > 1.134 +6 -0 src/sys/netinet/tcp_syncache.c > > kmacy 2007-12-12 06:11:50 UTC > > FreeBSD src repository > > Modified files: > sys/netinet tcp_syncache.c > Log: > Remove spurious timestamp check. RFC 1323 explicitly states that timestamps MAY > be transmitted if negotiated. > > Revision Changes Path > 1.138 +1 -17 src/sys/netinet/tcp_syncache.c These changes and their rationale are not really correct. The citation from RFC1323 is grossly misrepresented. The partly cited sentence says that timestamps may only be transmitted if they were negotiated. It doesn't say that timestamps may or may not be sent after they were negotiated. In fact RFC1323 Section 3.2 explicitly states: "For simplicity and symmetry, we specify that timestamps always be sent and echoed in both directions." It is also the unchallenged consensus view of the IETF TCPM is that timestamps must be sent when negotiated [1]. It is not allowed to not include them when agreed so at SYN time. Any deviation of this is a violation of the RFC. Thus the test was correct and valid. OTOH the enforcement of this rule wasn't really there before and it may be argued that we've got a POLA violation here. A careful reading of the referenced PR bin/118005 and its audit trail do not support the conclusion that the enforcement is the root cause of the symptoms described. The submitter makes a clear distinction between inside and outside of his LAN. Inside everything continues to work just fine. It's outside of his LAN that certain machines can't establish connections to the upgraded machine. The cited outside machines are FreeBSD 6.2, OpenBSD 4.1 and Linux RHEL 4U4. All these operating systems correctly implement RFC1323 timestamps and do not omit them once negotiated. Hence it is very likely that the root cause is in some device along the path munging the packets in a non-RFC conform way. Perhaps a reverse-NAT gateway on his Internet gateway. Or some other packet rewriting system. The PR needs to reopened and the root cause properly inspected. Based on the rationale above I'd like to re-instate the test in HEAD because a) it is correct and b) even if disabled to serve as education for future readers of the SYN/SYN-ACK/ACK path. If the test has to remain disabled we should include a clear description including which systems and their revisions fail the test. Alternatively the test should be modified to disable sending of timestamps if this situation occurs as they are a pointless waste of bits anyway as they won't ever be returned back to us to measure anything. [1] http://www1.ietf.org/mail-archive/web/tcpm/current/msg02507.html and (many) following messages in that thread -- Andre Index: tcp_syncache.c =================================================================== RCS file: /home/ncvs/src/sys/netinet/tcp_syncache.c,v retrieving revision 1.141 diff -u -p -r1.141 tcp_syncache.c --- tcp_syncache.c 19 Dec 2007 16:56:28 -0000 1.141 +++ tcp_syncache.c 23 Jan 2008 21:33:49 -0000 @@ -914,12 +914,41 @@ syncache_expand(struct in_conninfo *inc, goto failed; } + /* + * If timestamps were present in the SYN and we accepted + * them in our SYN|ACK RFC1323 Section 3.2 requires them + * to be present from now on. And vice versa. + */ + if ((sc->sc_flags & SCF_TIMESTAMP) && !(to->to_flags & TOF_TS)) { + if ((s = tcp_log_addrs(inc, th, NULL, NULL))) + log(LOG_DEBUG, "%s; %s: Timestamp missing, " + "segment rejected\n", s, __func__); +#ifdef TCP_BROKEN_TS + /* + * There appear to be non-conformant devices that + * negotiate timestamps but then fail to include + * them in every segment from then on. + * + * Evidence of this really happening is inconclusive + * and specific operating systems or firmware and their + * revisions are not known. + * + * For a work-around disable sending of timestamps + * as they become a pointless waste of bandwidth + * if we won't get them reflected back anyway. + */ + sc->sc_flags &= ~SCF_TIMESTAMP; +#else + goto failed; +#endif + } if (!(sc->sc_flags & SCF_TIMESTAMP) && (to->to_flags & TOF_TS)) { if ((s = tcp_log_addrs(inc, th, NULL, NULL))) log(LOG_DEBUG, "%s; %s: Timestamp not expected, " "segment rejected\n", s, __func__); goto failed; } + /* * If timestamps were negotiated the reflected timestamp * must be equal to what we actually sent in the SYN|ACK.