Date: Tue, 21 Dec 2004 07:00:49 GMT From: Dan Nelson <dnelson@allantgroup.com> To: freebsd-bugs@FreeBSD.org Subject: Re: kern/75122: [PATCH] Incorrect inflight bandwidth calculation on first packet Message-ID: <200412210700.iBL70nqt029746@freefall.freebsd.org>
next in thread | raw e-mail | index | archive | help
The following reply was made to PR kern/75122; it has been noted by GNATS. From: Dan Nelson <dnelson@allantgroup.com> To: Uwe Doering <gemini@geminix.org> 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
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?200412210700.iBL70nqt029746>