Date: Sat, 17 Aug 2002 11:36:20 -0700 (PDT) From: Matthew Dillon <dillon@apollo.backplane.com> To: Nate Lawson <nate@root.org> Cc: freebsd-hackers@FreeBSD.ORG, freebsd-net@FreeBSD.ORG Subject: Re: Commit schedule for bandwidth delay product pipeline limiting for TCP Message-ID: <200208171836.g7HIaKZ3067940@apollo.backplane.com> References: <Pine.BSF.4.21.0208171027280.47412-100000@root.org>
next in thread | previous in thread | raw e-mail | index | archive | help
:Comments below. Consider them only semi-informed. :)
:
:> + save_ticks = ticks;
:> + if ((u_int)(save_ticks - tp->t_bw_rtttime) < 1)
:> + return;
:
:If tp->t_bw_rtttime is greater than save_ticks, the unsigned compare will
:fail. Can this ever happen (say with the "tick count goes
:backwards" issue)? Why not do this as a signed compare and check for
:<= 0?
Yes, when ticks rolls over. This is on purpose and is designed to
handle both the rollover condition and someone setting the time of day
on the machine (though perhaps 'ticks' doesn't reverse index in that
case, I wasn't sure). e.g. if ticks rolls over we fall through the
conditional and proceed to the calculation, which resets t_bw_rtttime.
:> + tp->t_bw_rtseq = ack_seq;
:> + if (tp->t_bw_rtttime == 0)
:> + return;
:> + bw = ((int64_t)tp->snd_bandwidth * 15 + bw) >> 4;
:
:I'm not familiar with the paper -- where does 15 come from? I'm guessing
:this is a cheap way to perturb the lower bits.
It's a cheap way to do an exponential average. The >> 4 is a divide by
16. So the equivalent is:
bw = (average_bw * 15 + bw) / 16
average_bw = bw;
That should be more obvious to you... a long term exponential average of
the bandwidth.
:> + * bandwidth balancing between connections.
:> + */
:> +#define USERTT ((tp->t_srtt + tp->t_rttbest) / 2)
:> + bwnd = (int64_t)bw * USERTT / (hz << TCP_RTT_SHIFT) + 2 * tp->t_maxseg;
:
:Would prefer an #undef USERTT after you're done with it.
Too late, I just did the commit. I'll make the addition in my local tree.
I expect to make more commits in the future to this segment of code and
it will show up in one of those.
:> + if (tcp_inflight_debug > 0) {
:> + static int ltime;
:
:What happens when multiple packets arrive at once. Can they make the
:calculation below fail? Do you want splimp or a lock on ltime?
No, this is just debugging code designed to limit the rate of
kernel printfs. A tcp_inflight_debug of 1 limits the rate to 1/sec.
2 will be 2/sec. 3 will be 3/sec. It's just debugging code, it
isn't meant to be 100% robust.
:> + if ((u_int)(ticks - ltime) >= hz / tcp_inflight_debug) {
:
:Why the division by a debug enable/disable int? Is it possible for this
:to ever be 0 at this point?
No, because of the check above. Though I suppose in -current there
could be a race.
:> tcp_seq t_rtseq; /* sequence number being timed */
:>
:> + int t_bw_rtttime; /* used for bandwidth calculation */
:
:Does this need to be signed?
I think you meant unsigned. No, it doesn't need to be unsigned. Hmm.
though it's possible rollover will skew the 'bw' calculation for a
few milliseconds. I'll look into that.
-Matt
Matthew Dillon
<dillon@backplane.com>
To Unsubscribe: send mail to majordomo@FreeBSD.org
with "unsubscribe freebsd-net" in the body of the message
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?200208171836.g7HIaKZ3067940>