From owner-freebsd-bugs@FreeBSD.ORG Tue Dec 21 06:55:34 2004 Return-Path: Delivered-To: freebsd-bugs@freebsd.org Received: from mx1.FreeBSD.org (mx1.freebsd.org [216.136.204.125]) by hub.freebsd.org (Postfix) with ESMTP id AE25416A4CE; Tue, 21 Dec 2004 06:55:34 +0000 (GMT) Received: from dan.emsphone.com (dan.emsphone.com [199.67.51.101]) by mx1.FreeBSD.org (Postfix) with ESMTP id C7ADD43D1F; Tue, 21 Dec 2004 06:55:33 +0000 (GMT) (envelope-from dan@dan.emsphone.com) Received: (from dan@localhost) by dan.emsphone.com (8.13.1/8.13.1) id iBL6tVjY047820; Tue, 21 Dec 2004 00:55:31 -0600 (CST) (envelope-from dan) Date: Tue, 21 Dec 2004 00:55:31 -0600 From: Dan Nelson To: Uwe Doering Message-ID: <20041221065531.GA78451@dan.emsphone.com> References: <200412151827.iBFIRqDB019997@dan.emsphone.com> <200412151830.iBFIUVwE052799@freefall.freebsd.org> <20041216190608.GA21382@dan.emsphone.com> <41C41DEA.9000504@geminix.org> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <41C41DEA.9000504@geminix.org> X-OS: FreeBSD 5.3-STABLE X-message-flag: Outlook Error User-Agent: Mutt/1.5.6i cc: freebsd-bugs@FreeBSD.org cc: FreeBSD-gnats-submit@FreeBSD.org Subject: Re: kern/75122: [PATCH] Incorrect inflight bandwidth calculation on first packet X-BeenThere: freebsd-bugs@freebsd.org X-Mailman-Version: 2.1.1 Precedence: list List-Id: Bug reports List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 21 Dec 2004 06:55:34 -0000 In the last episode (Dec 18), Uwe Doering said: > Dan Nelson wrote: > >Updated patch including Matt's recommended fix: > > > >+ /* > >+ * Sanity check, plus ignore pure window update acks. > >+ */ > >+ if ((int)(ack_seq - tp->t_bw_rtseq) <= 0) > >+ return; > > I wonder, isn't there a flaw in the logic with regard to the sequence > number handling? If the sequence number wraps around 't_bw_rtseq' no > longer gets set and therefore the bandwidth calculation stops until > 'ack_seq' either catches up with 't_bw_rtseq' again (which would take > quite a while), or 'ticks' wraps around as well, or there is > inactivity for more than 10 seconds. This is probably not the > intended behavior. I think the code works as-is. ack_seq and tp->t_bw_rtseq are both of type "tcp_seq" which is a u_int32_t. Wrap-around is handled transparently when your variables are unsigned and your sequence space covers all possible values. It's the magic of mod(2^32) arithmetic :) The (int) cast just makes the if simpler. Without the cast it would read if (ack_seq - tp->t_bw_rtseq > 2147483648U || ack_seq == tp->t_bw_rtseq) The sanity check is probably not even necessary, as any really invalid sequence numbers would have caused the packet to be dropped before it got this far. -- Dan Nelson dnelson@allantgroup.com