Bug 1399 - TCP not backing off data retransmissions properly
TCP not backing off data retransmissions properly
Status: RESOLVED FIXED
Product: ns-3
Classification: Unclassified
Component: tcp
pre-release
All All
: P5 normal
Assigned To: Adrian S.-W. Tam
:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2012-03-27 16:40 UTC by Tom Henderson
Modified: 2012-05-30 13:40 UTC (History)
2 users (show)

See Also:


Attachments
Path that follows RFC 6298 recomendations (4.57 KB, patch)
2012-05-29 07:46 UTC, Daniel Camara
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Tom Henderson 2012-03-27 16:40:31 UTC
RFC 6298 (5.5) states:

   When the retransmission timer expires, do the following:

   (5.4) Retransmit the earliest segment that has not been acknowledged
         by the TCP receiver.

   (5.5) The host MUST set RTO <- RTO * 2 ("back off the timer").  The
         maximum value discussed in (2.5) above may be used to provide
         an upper bound to this doubling operation.


However, the code path within TcpSocketBase::SendDataPacket() does not backoff the m_rto (the only backoff is done to the SendEmptyPacket() codepath).

To reproduce, checkout ns-3-dev as of changeset:  dea445e1c3e2; enable logging on the routing-aodv-regression test:

NS_LOG="TcpSocketBase" ./waf --run "test-runner --suite=routing-aodv-regression"

One can observe (within the log generated):

0x158e0f0 SendDataPacket Schedule ReTxTimeout at time 2.11474 to expire at time
2.31474
0x158e0f0 SendDataPacket Schedule ReTxTimeout at time 2.31474 to expire at time
2.51474
0x158e0f0 SendDataPacket Schedule ReTxTimeout at time 2.51474 to expire at time
2.71474
0x158e0f0 SendDataPacket Schedule ReTxTimeout at time 2.71474 to expire at time
2.91474
0x158e0f0 SendDataPacket Schedule ReTxTimeout at time 2.91474 to expire at time
3.11474
Comment 1 Daniel Camara 2012-05-29 07:46:11 UTC
Created attachment 1406 [details]
Path that follows RFC 6298 recomendations
Comment 2 Daniel Camara 2012-05-29 07:50:17 UTC
The attached path follows the recommendations of RFC 6298. Changes where made on the calculation of the Retransmit Timeout estimation and on the process followed on the retransmission, in case of a timeout.  The AODV test, providing as example, does not pass though :).
Comment 3 Tom Henderson 2012-05-29 09:50:49 UTC
(In reply to comment #2)
> The attached path follows the recommendations of RFC 6298. Changes where made
> on the calculation of the Retransmit Timeout estimation and on the process
> followed on the retransmission, in case of a timeout.  The AODV test, providing
> as example, does not pass though :).


I'm wondering about splitting your patch for now, with the tcp-socket-base.cc changes made now, and rtt-estimator.cc for further study (perhaps attached to bug 1405).

The reason is that I'm wondering whether the rtt-estimator.cc changes are aligned with the code in Measurement() and whether the code still conforms to the algorithm advertised (mean deviation algorithm by Jacobson and Karels, implemented with integer arithmetic), or is now a mashup of both algorithms.  For example,
- should smoothing be performed in RetransmitTimeout() if it is already done in Measurement()?
- the new RetransmitTimeout() code doesn't use the variable "gain" but has hardcoded values
- are the variables m_currentEstimatedRtt and m_srtt redundant?

Any thoughts on this proposal?
Comment 4 Tommaso Pecorella 2012-05-29 17:14:51 UTC
I'd split the issues as well (if possible).

It is even possible to have a different RTT estimator (the one we have is one of the many possibilities), so a class name refactoring might be envisaged (e.g.: RttEstimator -> JacobsonKarelsRttEstimator and the new one RfcSomethingRttEstimator).

T.
Comment 5 Tom Henderson 2012-05-30 13:40:37 UTC
I made direct fix to the existing RttEstimator class for now (changeset e3e89a0ccb7d) and moved your proposal to migrate the management of backoff state to TcpSocketBase class (as well as create an RFC 6298 variant) to bug 1405, which can be worked post-release.