Re: [ns] sack1 tcp bug: pipe_ being set incorrectly

[Sally Floyd <floyd@aciri.org>]

Mark and Ethan -

>Ethan Blanton and I believe we have identified a bug in the one-way
>ns SACK code.  

Many thanks.  I have finally added this change to NS (from your
email on 7/3).  This also changed the output of several of the
validation tests.  The old code can be run by setting the new
variable "oldCode_" to true.  (This is also useful because it
identifies the functional change in the code itself.)

This is validated in "./test-all-tcp underutilized_100ms_control_Sack".


>Specifically, we are looking at this portion of
>Sack1TcpAgent::dupack_action():
>
>sack_action:
>	recover_ = maxseq_;
>	last_cwnd_action_ = CWND_ACTION_DUPACK;
>	pipe_ = int(cwnd_) - NUMDUPACKS;
>	slowdown(CLOSE_SSTHRESH_HALF|CLOSE_CWND_HALF);
>	reset_rtx_timer(1,0);
>	fastrecov_ = TRUE;
>	scb_.MarkRetran(highest_ack_+1);
>	output(last_ack_ + 1, TCP_REASON_DUPACK);	// from top
>	/*
>	 * If dynamically adjusting NUMDUPACKS, record information
>	 *  at this point.
>	 */
>	return;
>
>We believe the "pipe_ = ..." line above should be changed to:
>
>	pipe_ = maxseq_ - highest_ack_ - NUMDUPACKS;
>
>as it appears elsewhere in the same function.  Making the pipe_
>depend on cwnd_ does not account for the fact that cwnd_ can grow
>much larger than the maximum (advertised) TCP window.  In this case,
>pipe_ becomes much larger than the number of segments that are
>actually outstanding which can prevent the sender from transmitting
>new segments during recovery.

- Sally