Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(283)

Issue 1702042: New TCP architecture for NS-3

Can't Edit
Can't Publish+Mail
Start Review
Created:
9 years, 11 months ago by Adrian
Modified:
9 years, 5 months ago
CC:
ns-3-reviews_googlegroups.com
Visibility:
Public.

Description

TcpSocketBase, a replacement for TcpSocketImpl, is proposed. This is doing the basic function of a TCP socket, such as the mechanics of its state machine and the sliding window. The new TCP architecture is modular and extensible. Two extensions are provided as examples: TcpNewReno and TcpRFC793. For more detail, please read http://www.nsnam.org/wiki/index.php/New_TCP_Socket_Architecture

Patch Set 1 #

Total comments: 32

Patch Set 2 : Coding style fix; Added checks against out-of-seq FIN and SYN+ACK #

Total comments: 40

Patch Set 3 : TCP variants: RFC793 (DoD), RFC2001 (Tahoe), RFC2581 (Reno), RFC2582 (NewReno) #

Total comments: 11

Patch Set 4 : Fix typo: Retransmit() -> DoRetransmit() #

Patch Set 5 : Various minor fixes #

Patch Set 6 : Recommanded Commit 1 #

Patch Set 7 : Recommended Commit 2 (fix bug in TcpSocketBase::DoRetransmit) #

Patch Set 8 : s/SequenceNumber/SequenceNumber32/g, diff against changeset 6444:9cdbf35532a3 #

Patch Set 9 : Added log messages to various TCP sockets #

Patch Set 10 : Guard against lost SYN #

Patch Set 11 : Update with TCP state machine verification program #

Patch Set 12 : Minor fixes and updated to changeset 6470:cd04d218ba62 #

Patch Set 13 : All variants inherited from TcpSocketBase. Updated to diff against 6595:3289a9d91620 #

Patch Set 14 : Fix tcp-loss-response and fast recovery exit condition in NewReno #

Patch Set 15 : Cancel RTO timers upon TCP closes #

Patch Set 16 : Separated RTT estimation code from TcpSocketBase::ForwardUp #

Patch Set 17 : Make some variables into TracedValue (testsuite will be in next patchset) #

Total comments: 5
Unified diffs Side-by-side diffs Delta from patch set Stats (+4387 lines, -299 lines) Patch
A examples/tcp/tcp-loss-response.cc View 9 10 11 12 13 1 chunk +291 lines, -0 lines 0 comments Download
A examples/tcp/tcp-testcases.cc View 11 1 chunk +282 lines, -0 lines 0 comments Download
M examples/tcp/wscript View 9 10 1 chunk +8 lines, -0 lines 0 comments Download
M src/common/output-stream-wrapper.h View 2 chunks +3 lines, -1 line 0 comments Download
M src/common/output-stream-wrapper.cc View 1 chunk +14 lines, -5 lines 0 comments Download
M src/helper/point-to-point-helper.cc View 2 chunks +7 lines, -0 lines 0 comments Download
M src/internet-stack/nsc-tcp-socket-impl.h View 6 7 2 chunks +3 lines, -0 lines 0 comments Download
M src/internet-stack/nsc-tcp-socket-impl.cc View 6 7 8 9 10 11 12 2 chunks +13 lines, -1 line 0 comments Download
M src/internet-stack/tcp-header.cc View 1 chunk +0 lines, -1 line 0 comments Download
M src/internet-stack/tcp-l4-protocol.h View 1 2 3 4 5 6 7 8 3 chunks +6 lines, -5 lines 0 comments Download
M src/internet-stack/tcp-l4-protocol.cc View 1 2 3 4 5 6 7 8 6 chunks +26 lines, -283 lines 0 comments Download
A src/internet-stack/tcp-newreno.h View 1 2 3 4 5 6 7 8 9 10 11 12 1 chunk +74 lines, -0 lines 0 comments Download
A src/internet-stack/tcp-newreno.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 1 chunk +221 lines, -0 lines 0 comments Download
A src/internet-stack/tcp-reno.h View 3 4 5 6 7 8 9 10 11 12 1 chunk +75 lines, -0 lines 0 comments Download
A src/internet-stack/tcp-reno.cc View 3 4 5 6 7 8 9 10 11 12 13 14 15 16 1 chunk +212 lines, -0 lines 0 comments Download
A src/internet-stack/tcp-rfc793.h View 1 2 1 chunk +61 lines, -0 lines 0 comments Download
A src/internet-stack/tcp-rfc793.cc View 1 2 3 4 1 chunk +91 lines, -0 lines 0 comments Download
A src/internet-stack/tcp-rx-buffer.h View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 1 chunk +89 lines, -0 lines 0 comments Download
A src/internet-stack/tcp-rx-buffer.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 1 chunk +264 lines, -0 lines 0 comments Download
A src/internet-stack/tcp-socket-base.h View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 1 chunk +221 lines, -0 lines 0 comments Download
A src/internet-stack/tcp-socket-base.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 1 chunk +1749 lines, -0 lines 5 comments Download
A src/internet-stack/tcp-tahoe.h View 3 4 5 6 7 1 chunk +78 lines, -0 lines 0 comments Download
A src/internet-stack/tcp-tahoe.cc View 3 4 5 6 7 8 9 10 11 12 13 14 15 16 1 chunk +190 lines, -0 lines 0 comments Download
A src/internet-stack/tcp-tx-buffer.h View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 1 chunk +121 lines, -0 lines 0 comments Download
A src/internet-stack/tcp-tx-buffer.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 1 chunk +250 lines, -0 lines 0 comments Download
M src/internet-stack/wscript View 1 2 3 4 5 6 7 8 9 10 11 12 2 chunks +8 lines, -1 line 0 comments Download
M src/node/simple-net-device.cc View 1 chunk +1 line, -1 line 0 comments Download
M src/node/tcp-socket.h View 6 7 8 9 10 11 12 13 14 15 16 3 chunks +20 lines, -0 lines 0 comments Download
M src/node/tcp-socket.cc View 6 7 8 9 10 11 12 13 14 15 16 3 chunks +9 lines, -1 line 0 comments Download
M src/routing/aodv/test/tcp-chain-test-0-0.pcap View Binary file 0 comments Download
M src/routing/aodv/test/tcp-chain-test-9-0.pcap View Binary file 0 comments Download

Messages

Total messages: 27
Adrian
http://codereview.appspot.com/1702042/diff/1/2 File src/internet-stack/tcp-header.cc (left): http://codereview.appspot.com/1702042/diff/1/2#oldcode23 src/internet-stack/tcp-header.cc:23: #include "tcp-socket-impl.h" This include is indeed redundant. Thus I ...
9 years, 11 months ago (2010-06-16 19:00:35 UTC) #1
Josh Pelkey
Here is my initial review. Overall, I really like it. A few small comments on ...
9 years, 11 months ago (2010-06-23 17:30:23 UTC) #2
Adrian
I am uploading Patch Set 2. This is the update of the previous one. In ...
9 years, 11 months ago (2010-06-24 06:55:31 UTC) #3
Adrian
I uploaded Patch sets 3 and 4. This includes four TCP variants, namely, DoD (RFC793), ...
9 years, 11 months ago (2010-06-25 08:46:10 UTC) #4
Tom Henderson
Please see comments against previous patchset although I think most or all still apply. I ...
9 years, 11 months ago (2010-06-26 00:54:33 UTC) #5
Adrian
Patch set 5 uploaded. With various minor fixes (also detailed below). I feel good about ...
9 years, 11 months ago (2010-06-28 21:14:51 UTC) #6
Adrian
I uploaded the latest update (patch set 6, recommended commit) on the TCP architecture. Personally, ...
9 years, 11 months ago (2010-07-02 18:06:44 UTC) #7
Adrian
Patch set 12 is uploaded. This is a patch against ns-3-dev changeset 6470:cd04d218ba62 (most recent ...
9 years, 10 months ago (2010-07-30 01:45:35 UTC) #8
Tom Henderson
I really like your wiki page and tests/examples. I had a question about your One ...
9 years, 9 months ago (2010-08-27 23:23:36 UTC) #9
Adrian
Hi Tom, > Now, in RFC 3782, ssthresh is set to max (FlightSize/2, 2*MSS) and ...
9 years, 9 months ago (2010-08-28 03:33:46 UTC) #10
Tom Henderson
On 2010/08/28 03:33:46, Adrian wrote: > Hi Tom, > > > Now, in RFC 3782, ...
9 years, 9 months ago (2010-08-28 16:23:12 UTC) #11
Josh Pelkey
Adrian, Thank you very much for your work on TCP. It is really great! After ...
9 years, 9 months ago (2010-08-28 19:29:57 UTC) #12
Adrian
Hi Tom, > I am not too concerned about the difference between RFC2582 and 3782; ...
9 years, 9 months ago (2010-08-30 04:13:34 UTC) #13
Adrian
Hi Josh, > I think it is a little confusing that NewReno inherits from Reno ...
9 years, 9 months ago (2010-08-30 04:50:26 UTC) #14
Adrian
Hi Josh, I uploaded patchset 13, which is using a flat hierarchy for all TCP ...
9 years, 9 months ago (2010-08-31 09:07:25 UTC) #15
Josh Pelkey
Adrian, Thank you very much for doing this! I think patchset 13 is a bit ...
9 years, 9 months ago (2010-08-31 14:25:43 UTC) #16
Adrian
Hi all, I uploaded Patchset 14. I wish I have everything fixed. Three changes in ...
9 years, 9 months ago (2010-09-02 21:42:07 UTC) #17
Adrian
I just uploaded patchset 15 --- now this can pass all test.py test cases Here ...
9 years, 9 months ago (2010-09-03 07:34:57 UTC) #18
Adrian
Patchset 16 is uploaded, a minor feature fix of patchset 15. The only change is ...
9 years, 8 months ago (2010-09-06 16:18:05 UTC) #19
Tom Henderson
On 2010/09/03 07:34:57, Adrian wrote: > I just uploaded patchset 15 --- now this can ...
9 years, 8 months ago (2010-09-06 22:18:13 UTC) #20
Tom Henderson
> > I think we should also get some test cases in there for 3.10 ...
9 years, 8 months ago (2010-09-06 22:19:33 UTC) #21
Adrian
Including tcp-testcases as a test suite is a good idea. Unlike tcp-loss-response, what tcp-testcases checks ...
9 years, 8 months ago (2010-09-06 22:53:09 UTC) #22
Tom Henderson
On 2010/09/06 22:53:09, Adrian wrote: > Including tcp-testcases as a test suite is a good ...
9 years, 8 months ago (2010-09-07 04:58:46 UTC) #23
Adrian
On Tue, Sep 7, 2010 at 12:58 AM, <tomh.org@gmail.com> wrote: >> For (1), packet trace ...
9 years, 8 months ago (2010-09-07 16:16:13 UTC) #24
Tom Henderson
> If that's the case, I think this is the list of variables that someone ...
9 years, 8 months ago (2010-09-12 05:37:02 UTC) #25
joao.taveira
Hi, Here are a few comments from having tried the TCP refactoring out. I only ...
9 years, 7 months ago (2010-10-24 18:11:49 UTC) #26
Josh Pelkey
9 years, 5 months ago (2010-12-15 18:07:39 UTC) #27
Adrian, this was pointed out to me by a student a Georgia Tech (see comment
below).

http://codereview.appspot.com/1702042/diff/168002/src/internet-stack/tcp-sock...
File src/internet-stack/tcp-socket-base.cc (right):

http://codereview.appspot.com/1702042/diff/168002/src/internet-stack/tcp-sock...
src/internet-stack/tcp-socket-base.cc:437: address = InetSocketAddress
(m_endPoint->GetLocalAddress (), m_endPoint->GetLocalPort ());
RecvFrom calls this method which is supposed to return the remote address;
however, here you are returning the local address.
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld f62528b