Right now, SACK is disabled by default; the internal of the
TcpSocketBase has been (partially) rewritten to respect RFC 6675,
the code has comments which reflects text from the RFC, to make it clear
all the passages. I don't know if, in the future, the flag will be
turned on by default, because it changes the outcome of many simulations
(it will improve the obtained results, I hope) but for sure not before
another release, to let bugs and/or time-performance improvements to be
discovered and proposed.
With the default configuration, all the tests run fine.
If you generally enable SACK, this is the tests status:
List of FAILed tests:
List of CRASHed tests:
src/traffic-control/examples/pfifo-vs-red --queueDiscType=PfifoFast --modeBytes=1
while for wifi-tcp, ns3-tcp-loss, ns3-tcp-state,
tcp-bytes-in-flight-test it's perfectly normal (these tests are tailored
on a specific byte sequence, which with SACK can change) and the normal
followup will be to disable (through Config system) SACK for these tests, I
don't know what to do with the others; they relies on the number of
bytes exchanged (which, with SACK, increase a lot).
Anyway, I ask for a general review; if no problems are discovered and no
concerns are raised, I plan to merge this the next week.
> On 10/17/2016 10:59 AM, Natale Patriciello wrote:
> > Hi all,
> > I'm about to merge the TCP sack feature developed this summer. You can
> > find the code here:
> Hi Natale,
> I left some review comments in the tracker issue devoted to this:
> https://www.nsnam.org/bugzilla/show_bug.cgi?id=2248 >
> Thanks for your efforts to finally finish this. I think it would be very
> nice to resolve the comments and close this out early in our release cycle.
Just to remember the SACK patch, which after some iterations now is in
the right shape. There are only some small problems in the AQM tests,
but all the rest seems good. Give it a try !