From owner-freebsd-net@FreeBSD.ORG Sat Jun 9 21:14:03 2007 Return-Path: X-Original-To: net@freebsd.org Delivered-To: freebsd-net@FreeBSD.ORG Received: from mx1.freebsd.org (mx1.freebsd.org [69.147.83.52]) by hub.freebsd.org (Postfix) with ESMTP id 0741B16A469 for ; Sat, 9 Jun 2007 21:14:03 +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 7466113C480 for ; Sat, 9 Jun 2007 21:14:02 +0000 (UTC) (envelope-from andre@freebsd.org) Received: (qmail 58356 invoked from network); 9 Jun 2007 20:28:05 -0000 Received: from c00l3r.networx.ch (HELO [127.0.0.1]) ([62.48.2.2]) (envelope-sender ) by c00l3r.networx.ch (qmail-ldap-1.03) with SMTP for ; 9 Jun 2007 20:28:05 -0000 Message-ID: <466B1817.4090900@freebsd.org> Date: Sat, 09 Jun 2007 23:13:59 +0200 From: Andre Oppermann User-Agent: Thunderbird 1.5.0.12 (Windows/20070509) MIME-Version: 1.0 To: Yar Tikhiy References: <20070608142641.GA25127@comp.chem.msu.su> <20070608150512.GC25127@comp.chem.msu.su> In-Reply-To: <20070608150512.GC25127@comp.chem.msu.su> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Cc: net@freebsd.org Subject: Re: A small window-related bug in tcp_input.c? X-BeenThere: freebsd-net@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Networking and TCP/IP with FreeBSD List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sat, 09 Jun 2007 21:14:03 -0000 Yar Tikhiy wrote: > On Fri, Jun 08, 2007 at 06:26:41PM +0400, Yar Tikhiy wrote: >> There is the following code in tcp_input.c (I "underlined" two >> questionable lines): >> >> /* >> * Process options only when we get SYN/ACK back. The SYN case >> * for incoming connections is handled in tcp_syncache. >> * XXX this is traditional behavior, may need to be cleaned up. >> */ >> if (tp->t_state == TCPS_SYN_SENT && (thflags & TH_SYN)) { >> if ((to.to_flags & TOF_SCALE) && >> (tp->t_flags & TF_REQ_SCALE)) { >> tp->t_flags |= TF_RCVD_SCALE; >> tp->snd_scale = to.to_wscale; >> tp->snd_wnd = th->th_win << tp->snd_scale; >> ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ >> tiwin = tp->snd_wnd; >> } >> if (to.to_flags & TOF_TS) { >> tp->t_flags |= TF_RCVD_TSTMP; >> tp->ts_recent = to.to_tsval; >> tp->ts_recent_age = ticks; >> } >> /* Initial send window, already scaled. */ >> tp->snd_wnd = th->th_win; >> ^^^^^^^^^^^^^^^^^^^^^^^^^ >> if (to.to_flags & TOF_MSS) >> tcp_mss(tp, to.to_mss); >> if ((tp->t_flags & TF_SACK_PERMIT) && >> (to.to_flags & TOF_SACKPERM) == 0) >> tp->t_flags &= ~TF_SACK_PERMIT; >> } >> >> Is it correct that the scaled value in tp->snd_wnd is later overwritten >> with the unscaled value from th->th_win? > > In addition, the first underlined line and the comment above the > second underlined line seem to contradict RFC 1323: > > The Window field in a SYN (i.e., a or ) > segment itself is never scaled. > > Perhaps tp->snd_scale should be set but no scaling done for a ? Thanks for noticing. Fixed in rev. 1.356 of tcp_input.c. The whole tcp input processing has quite some bitrot and duplication due to not really being cleaned up after the addition of syncache and compressed timewait. All this stuff is fixed in the upcoming vastly cleaned up, simplified and rewritten tcp_do_segment() I have in the works. Until then I'm backporting a number of incremental fixes to reduce the functional diff. -- Andre