TCP Veno implementation in |ns3|

TCP Veno implementation in |ns3|
------------------------------------------------

This chapter describes TCP Veno implementation in |ns3|. This implementation is contained in the following files:

.. sourcecode:: text

    src/internet/model/tcp-veno.{cc,h}

Model Description
*****************
TCP Veno enhances Reno algorithm to handle random packet losses in wireless access networks more effectively 
by employing Vegas's method in estimating the backlog at the bottleneck queue to distinguish between congestive and
non-congestive states.

The backlog (the number of packets accumulated at the bottleneck queue) is calculated using Equation (1):
        N = Actual * (RTT - BaseRTT) = Diff * BaseRTT        (1)
where
        Diff = Expected - Actual = cwnd/BaseRTT - cwnd/RTT

Veno makes decision on cwnd modification based on the calculated N and its predefined threshold beta. Following
Veno implementation in the Linux kernel, beta has a default value of 6. 

Specifically, Veno refines the additive increase algorithm of Reno so that the connection can stay longer in the stable 
state by incrementing cwnd by 1/cwnd for every other new ACK received after the available bandwidth has been fully utilized,
i.e. when N exceeds beta.  Otherwise, Veno increases its cwnd by 1/cwnd upon every new ACK receipt as in Reno.

In the multiplicative decrease algorithm, when Veno is in the non-congestive state, i.e. when N is less than beta, 
Veno decrements its cwnd by only 1/5 because the loss encountered is more likely a corruption-based loss than a congestion-based.  
Only when N is greater than beta, Veno halves its sending rate as in Reno.

References
==========

.. [Fu2003] Cheng Peng Fu and S. C. Liew, "TCP Veno: TCP enhancement for transmission over wireless access networks," in IEEE Journal on Selected Areas in Communications, vol. 21, no. 2, pp. 216-228, Feb 2003. DOI=http://dx.doi.org/10.1109/JSAC.2002.807336 

Validation
==========
The TCP Veno model is tested using :cpp:class:`TcpVenoTestSuite` class defined in `src/internet/test/tcp-veno-test.{cc,h}`.  This test suit can be run using the following commands:

::

  $ ./waf configure --enable-examples --enable-tests
  $ ./waf build
  $ ./test.py -s tcp-veno-test
  
Example
=======
TCP Veno can be simulated using the example `tcp-variants-comparison.cc` located in ``examples/tcp``.

[n.p, 2016-04-06 12:31:51]

Minimum comments here. As always, I'd like to put some informations of Veno in
the .rst (model documentation).

https://codereview.appspot.com/288520043/diff/1/src/internet/model/tcp-conges...
File src/internet/model/tcp-congestion-ops.h (left):

https://codereview.appspot.com/288520043/diff/1/src/internet/model/tcp-conges...
src/internet/model/tcp-congestion-ops.h:28: class TcpSocketBase;
why you removed these forward declarations? Where the compiler can get
informations for Ptr<> ?

https://codereview.appspot.com/288520043/diff/1/src/internet/model/tcp-l4-pro...
File src/internet/model/tcp-l4-protocol.cc (right):

https://codereview.appspot.com/288520043/diff/1/src/internet/model/tcp-l4-pro...
src/internet/model/tcp-l4-protocol.cc:94: {
mmm, this part adds only some formatting ... I think it's not mergeable inside
the Veno implementation

https://codereview.appspot.com/288520043/diff/1/src/internet/model/tcp-socket...
File src/internet/model/tcp-socket-base.cc (right):

https://codereview.appspot.com/288520043/diff/1/src/internet/model/tcp-socket...
src/internet/model/tcp-socket-base.cc:278: // Change this for non-zero initial
sequence number
This should be avoided (see below)

https://codereview.appspot.com/288520043/diff/1/src/internet/model/tcp-socket...
src/internet/model/tcp-socket-base.cc:302: // Set to the initial sequence number
Please, this should be avoided since it is not clear to what the comment refers
to

https://codereview.appspot.com/288520043/diff/1/src/internet/model/tcp-veno.cc
File src/internet/model/tcp-veno.cc (right):

https://codereview.appspot.com/288520043/diff/1/src/internet/model/tcp-veno.c...
src/internet/model/tcp-veno.cc:46: MakeUintegerChecker<uint32_t> ())
There are some interesting trace sources ? E.g. minRtt

https://codereview.appspot.com/288520043/diff/1/src/internet/model/tcp-veno.c...
src/internet/model/tcp-veno.cc:95: 
Check if rtt is 0.. sometime happens (I don't know why), but if it is 0 the
value should be ignored

https://codereview.appspot.com/288520043/diff/1/src/internet/model/tcp-veno.c...
src/internet/model/tcp-veno.cc:108: m_baseRtt = rtt;
check rtt != 0

https://codereview.appspot.com/288520043/diff/1/src/internet/model/tcp-veno.c...
src/internet/model/tcp-veno.cc:223: m_minRtt = Time (0);
Why the min RTT is reset every time the function is called?

https://codereview.appspot.com/288520043/diff/1/src/internet/test/tcp-general...
File src/internet/test/tcp-general-test.h (right):

https://codereview.appspot.com/288520043/diff/1/src/internet/test/tcp-general...
src/internet/test/tcp-general-test.h:27: 
I have not clear in mind what is the purpose of removing the forward declaration
from tcp-congestion-ops and the include from tcp-socket-base ... that leads to
the necessity to rethink the header inclusion, as you are doing with this patch

https://codereview.appspot.com/288520043/diff/1/src/internet/test/tcp-veno-te...
File src/internet/test/tcp-veno-test.cc (right):

https://codereview.appspot.com/288520043/diff/1/src/internet/test/tcp-veno-te...
src/internet/test/tcp-veno-test.cc:97: 
Use MilliSeconds (100), it's more readable

https://codereview.appspot.com/288520043/diff/1/src/internet/test/tcp-veno-te...
src/internet/test/tcp-veno-test.cc:105: Time baseRtt = Time (100000000);
Not readable

https://codereview.appspot.com/288520043/diff/1/src/internet/test/tcp-veno-te...
src/internet/test/tcp-veno-test.cc:211: AddTestCase (new TcpVenoTest (30 * 536,
536, 20 * 536, Time (105759978), 1, 1,
Are these value important for some thing? or they are random?

https://codereview.appspot.com/288520043/diff/1/src/internet/test/tcp-veno-te...
File src/internet/test/tcp-veno-test.h (right):

https://codereview.appspot.com/288520043/diff/1/src/internet/test/tcp-veno-te...
src/internet/test/tcp-veno-test.h:6: {
Useless, please remove