TCP options feature for ns-3

initial upload

[Peter Barnes, 2012-01-04 18:55:18]

Minor suggestions:

http://codereview.appspot.com/5452045/diff/1/src/internet/model/tcp-option-sa...
File src/internet/model/tcp-option-sack.cc (right):

http://codereview.appspot.com/5452045/diff/1/src/internet/model/tcp-option-sa...
src/internet/model/tcp-option-sack.cc:52: {
Sack printer would be really useful for illustration, debugging.

http://codereview.appspot.com/5452045/diff/1/src/internet/model/tcp-option-wi...
File src/internet/model/tcp-option-winscale.cc (right):

http://codereview.appspot.com/5452045/diff/1/src/internet/model/tcp-option-wi...
src/internet/model/tcp-option-winscale.cc:60: return 3;
Should this be an enum or const in the class?  It's used three places in this
file, and similarly in the other options classes.  Too many places to fix if the
size ever changes, IMO.

http://codereview.appspot.com/5452045/diff/1/src/internet/model/tcp-option-wi...
src/internet/model/tcp-option-winscale.cc:67: i.WriteU8 (2); // Kind
Should this 'kind' be 3?  See comment in tcp-option.cc

http://codereview.appspot.com/5452045/diff/1/src/internet/model/tcp-option-wi...
src/internet/model/tcp-option-winscale.cc:68: i.WriteU8 (3); // Length
Is this the 'size' var in Deserialize?  Can we use the same label for the var
and the comment?

http://codereview.appspot.com/5452045/diff/1/src/internet/model/tcp-option.cc
File src/internet/model/tcp-option.cc (right):

http://codereview.appspot.com/5452045/diff/1/src/internet/model/tcp-option.cc...
src/internet/model/tcp-option.cc:60: if (kind == 0)
Should these 'kind' constants be enums in this base class?  This would avoid
problems like tcp-option-winscale.cc where kind is serialized as 2 but GetKind
returns 3.

[Tom Henderson, 2014-04-18 17:05:46]

These need some additional work, since (if I recall correctly the last time I
tested this patch) they can crash when run against an implementation TCP that
generates additional options like SackPermitted).  Peter also raises some good
suggestions.

I've suggested to Anh to try to address Peter's comments, and then I'd like to
proceed with these steps:

1) get Adrian's base TCP option code in with support for option kinds 0, 1, 2,
and > 2.  The code should not crash when run against a TCP that generates
options (i.e. a real TCP that generates Sack permitted, or possibly others that
come in the SYN).  A TCP test that tests the deserialization logic for specially
constructed test TCP options would be useful.  For instance, create two test
options that, when included, require also the use of No-op option and padding.

Once we are OK with this design, then add Window Scale and Timestamp as separate
patches on top of this.

[Tom Henderson, 2014-04-18 17:09:29]

oops, I posted the most recent comments against the wrong issue.

I meant to post against 
https://codereview.appspot.com/24900043

it may be the case that once 24900043 is done, we can pull any additional
options from here (e.g. SackPermitted).