Date: Sat, 09 Jun 2007 23:13:59 +0200 From: Andre Oppermann <andre@freebsd.org> To: Yar Tikhiy <yar@comp.chem.msu.su> Cc: net@freebsd.org Subject: Re: A small window-related bug in tcp_input.c? Message-ID: <466B1817.4090900@freebsd.org> In-Reply-To: <20070608150512.GC25127@comp.chem.msu.su> References: <20070608142641.GA25127@comp.chem.msu.su> <20070608150512.GC25127@comp.chem.msu.su>
next in thread | previous in thread | raw e-mail | index | archive | help
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 <SYN> or <SYN,ACK>)
> segment itself is never scaled.
>
> Perhaps tp->snd_scale should be set but no scaling done for a <SYN,ACK>?
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
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?466B1817.4090900>