Bugzilla – Bug 1405
Last modified: 2015-01-30 17:09:44 EST
I'm not confident in the results of this class; a unit test is needed.
Created attachment 1408 [details]
Daniel's patch from bug 1399 for RFC 6298 behavior
I moved Daniel's patch from bug 1399 here. It implements RFC 6298 behavior (slightly different than the MeanDeviation algorithm currently implemented).
Another aspect that must be considered is whether the TCP or the RttEstimator class maintains the state for the backoff. In the current design, RttEstimator is managing all of this state, but in Daniel's patch, he moves the state for managing backoff to the TcpSocketBase class.
I'm in favor of Daniel's proposal to make TCP manage the backoff state and just have RttEstimator maintain actual estimates, but this would require removing some state variables and the "multiplier" API from RttEstimator and should be considered again once we are not close to a release deadline. The reason that I am in favor of this is the separate proposal (bug 1412) that wants to reuse RttEstimator for other protocols.
I'm going to put this in the list of issues to resolve for ns-3.23 release.
This patch isn't working as intended.
The rto value is not multiplied as it should in case of "normal" retransmissions.
I'm working on a new patch.
Created attachment 1955 [details]
I'll attach the patch here, but this is a cumulative patch, fixing this bug, bug #2041 and another undiscovered bug.
For the records, the undiscovered bug is: RTO should increase exponentially, while it was increasing linearly (in case of consecutive lost packets).
Sorry, but the patch is huge and can't really be split in sub-components.
What's been done:
1) update the RTT estimation to RFC 6298 values, including *two* smoothing factors for mean and variance
2) remove RTO calculation from RttEstimator and place it into TcpSocketBase
3) move to TcpSocketBase the RTT sample calc (and move RttHistory there as well)
4) remove RTO multiplier and use a different approach
5) add limits to the RTO, as suggested by RFC 6298
6) something more I can't remember.
Please test is as much as you can. The Tcp performances are expected to vary slightly, and we'll need to re-generate some tracefiles.
Created attachment 1957 [details]
modifications to Tommaso's Huge patch
I made a number of changes to Tommaso's huge patch, driven by a rewrite of the existing unit test, which now is validating the arithmetic for selected (rather than random) values.
The main change in the computation is to use integer arithmetic when the values of alpha and beta support it (since time is maintained as an integer) and fall back to floating point arithmetic when integer can't be supported.
Other small changes:
* add a GetNSamples() method
* more Doxygen
* check bounds on Alpha and Beta attributes
* cleanup includes and forward declarations
This does not yet address bug 1412 (move rtt-estimator to network module) but such a change should be trivial.
This does not yet fully address bug 2041 (does not test RTO computations in any way).
Created attachment 1959 [details]
Mods to the mods to my patch
Just some small improvements to Tom's modifications.
1) Use Abs, Min and Max (the ones defined in Time class) where appropriate
2) fix in the test (wrong function name)
3) use an optimized version of the floating-point update formulas.
The last point is because
X = (1-a) X' + a m
is equivalent to
X += a (m - X')
For the records, not only this is important in hardware implementation (1 mul less), but it is also more precise (when a is very low).
Created attachment 1960 [details]
Mods to the mods to my patch (fix)
Forgot to refresh the diff.
(In reply to Tommaso Pecorella from comment #7)
> Created attachment 1959 [details]
> Mods to the mods to my patch
> Just some small improvements to Tom's modifications.
> 1) Use Abs, Min and Max (the ones defined in Time class) where appropriate
> 2) fix in the test (wrong function name)
> 3) use an optimized version of the floating-point update formulas.
> The last point is because
> X = (1-a) X' + a m
> is equivalent to
> X += a (m - X')
> For the records, not only this is important in hardware implementation (1
> mul less), but it is also more precise (when a is very low).
Thanks, I'm fine with your improvements to this patch.