Bug 227

Summary: tcp-large-transfer.cc does not work with smaller tcp sndbuf
Product: ns-3 Reporter: Florian Westphal <fw-ns3>
Component: samplesAssignee: ns-bugs <ns-bugs>
Status: RESOLVED FIXED    
Severity: normal CC: craigdo
Priority: P1    
Version: pre-release   
Hardware: All   
OS: All   
Bug Depends on: 226    
Bug Blocks:    
Attachments: Patch that allows tcp-large-transfer to send all data with smaller tcp sndbuf.
Florian's fix + cleanup + example documentation

Description Florian Westphal 2008-06-18 15:43:16 UTC
When adding something like
localSocket->SetAttribute("SndBufSize", UintegerValue(8192));
to tcp-large-transfer.cc, it sends out a lot less data than its supposed to
(~8k instead of ~2M).
This is caused by several factors:
1. tcp-impl 'forgetting' to call NotifySend (see bugzilla bug #226)
2. tcp-large-transfer calling Close() from the OnConnect Callback
3. tcp-large-transfer assuming that the argument to its
WriteUntilBufferFull function is the number of bytes to xmit (its
the amout of new space is the tcp sndbuf when invoked as the NotifySend callback)

To be honest i'm not entirely sure how the proper fix for this would look like.
I'm attaching a diff that makes the example work correctly even with a small tcpsndbuf for consideration.
Comment 1 Florian Westphal 2008-06-18 15:44:44 UTC
Created attachment 173 [details]
Patch that allows tcp-large-transfer to send all data with smaller tcp sndbuf.
Comment 2 Rajib Bhattacharjea 2008-06-19 17:42:49 UTC
Created attachment 178 [details]
Florian's fix + cleanup + example documentation

Okay, so after carefully reviewing the patch and the issue, I notice that since bug 226 was closed out, the only code touched to support this case is the example itself, tcp-large-transfer.  The provided patch causes no regression, while supporting the case of finite send buffers with this example program, if you uncomment the relevant line.  In documenting this usage, I ended up cleaning up the entire file and adding lots of comments explaining what's going on.  I also did some renaming of variables, and got rid of some dead code.

I think its ready to merge.
Comment 3 Craig Dowell 2008-06-19 17:58:44 UTC
Applied patch 2008-06-19 17:42