From owner-freebsd-bugs@FreeBSD.ORG Tue Dec 21 07:00:50 2004 Return-Path: Delivered-To: freebsd-bugs@hub.freebsd.org Received: from mx1.FreeBSD.org (mx1.freebsd.org [216.136.204.125]) by hub.freebsd.org (Postfix) with ESMTP id 0353216A4CF for ; Tue, 21 Dec 2004 07:00:50 +0000 (GMT) Received: from freefall.freebsd.org (freefall.freebsd.org [216.136.204.21]) by mx1.FreeBSD.org (Postfix) with ESMTP id DF96743D5D for ; Tue, 21 Dec 2004 07:00:49 +0000 (GMT) (envelope-from gnats@FreeBSD.org) Received: from freefall.freebsd.org (gnats@localhost [127.0.0.1]) by freefall.freebsd.org (8.13.1/8.13.1) with ESMTP id iBL70n9p029747 for ; Tue, 21 Dec 2004 07:00:49 GMT (envelope-from gnats@freefall.freebsd.org) Received: (from gnats@localhost) by freefall.freebsd.org (8.13.1/8.13.1/Submit) id iBL70nqt029746; Tue, 21 Dec 2004 07:00:49 GMT (envelope-from gnats) Date: Tue, 21 Dec 2004 07:00:49 GMT Message-Id: <200412210700.iBL70nqt029746@freefall.freebsd.org> To: freebsd-bugs@FreeBSD.org From: Dan Nelson 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 Reply-To: Dan Nelson List-Id: Bug reports List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 21 Dec 2004 07:00:50 -0000 The following reply was made to PR kern/75122; it has been noted by GNATS. From: Dan Nelson To: Uwe Doering Cc: FreeBSD-gnats-submit@FreeBSD.org, freebsd-bugs@FreeBSD.org Subject: Re: kern/75122: [PATCH] Incorrect inflight bandwidth calculation on first packet Date: Tue, 21 Dec 2004 00:55:31 -0600 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