Bugzilla – Bug 132
TCP server sockets do not fork on new connections
Last modified: 2008-03-27 10:45:44 UTC
The TcpSocket class never calls NotifyNewConnectionCreated, and so server-side sockets accept connections allright but don't notify the user code that a new socket has been created. Without this it's impossible to receive data inside a server application, as far as i can tell...
This is part of the larger issue that listening server sockets don't actually spawn new sockets/endpoints for accepted connections, but rather simply communicate over the formerly listening socket. On today's call, we committed to having the correct behavior by the end of the week.
(In reply to comment #1) > This is part of the larger issue that listening server sockets don't actually > spawn new sockets/endpoints for accepted connections, but rather simply > communicate over the formerly listening socket. On today's call, we committed > to having the correct behavior by the end of the week. OK... it would be nice to have a complete TCP for the next ns-3 release. Otherwise the release notes should have a warning about this issue, to prevent users from spending time trying to look for bugs in their own code when it's simply a feature not implemented in ns-3 :-/
Implemented in tree: http://code.nsnam.org/raj/ns-3-dev/ Quick link to the file: http://code.nsnam.org/raj/ns-3-dev/file/d66ffafd78c9/src/internet-node/tcp-socket.cc The basic logic is that when a SYN comes in, the listening socket performs a stateful clone of itself using the copy constructor (eventually the "virtual constructor"/copy/clone pattern, but not yet), and then calls into the newly cloned socket to send back the reply and notify the application that a connection was requested. When it completes the handshake, the cloned socket fires a notify up to the application that a new socket has been created and passes up itself and the remote end's IP address. There are some memory management/smart pointer issues to figure out yet (very likely that the refcount of the cloned socket is not correct), but the code appears to work albeit with leaks. See examples/tcp-full-duplex.cc in my tree from above as it is the only TCP example scripts that uses the newly implemented semantics. All the rest need slight modification to support the change. I'll try to get this done over the weekend.
I think there are two issues; a simple one and a harder one. The simple one is to simply fix the behavior for a non-multitasking server. This should be fixable with minimal effort (Gustavo's initial comment). The second one is the multitasking server. Here, there may need to be more work done because tcp-full-duplex.cc (or any of our applications) do not even handle the forking sockets because they themselves do not fork. So I'd suggest to fix this one immediately without waiting to finalize the forking server case.
(In reply to comment #4) > I think there are two issues; a simple one and a harder one. The simple one is > to simply fix the behavior for a non-multitasking server. This should be > fixable with minimal effort (Gustavo's initial comment). > > The second one is the multitasking server. Here, there may need to be more > work done because tcp-full-duplex.cc (or any of our applications) do not even > handle the forking sockets because they themselves do not fork. > Our TCP sockets DO fork now whenever a SYN packet arrives. I'm not sure if I made this clear or not in the original post, but what is in http://code.nsnam.org/raj/ns-3-dev forks the socket and returns it to the application via a call to NotifyNewConnectionCreated. The original server socket keeps on listening for incoming SYNs. I'm not sure I understand what the two issues are?
Surprisingly, our current test cases with one sender and one packet-sink works with the change to forked sockets. The reason is that the packet sink registers a receive callback with its listening socket. When the listening socket is forked, the copy constructor copies the callback as well, and then invokes it upon data receipt, passing up to the application correctly. More testing remains to be done to see if the behavior is correct for multiple connections to one server.
(In reply to comment #5) > (In reply to comment #4) > > I think there are two issues; a simple one and a harder one. The simple one is > > to simply fix the behavior for a non-multitasking server. This should be > > fixable with minimal effort (Gustavo's initial comment). > > > > The second one is the multitasking server. Here, there may need to be more > > work done because tcp-full-duplex.cc (or any of our applications) do not even > > handle the forking sockets because they themselves do not fork. > > > Our TCP sockets DO fork now whenever a SYN packet arrives. I'm not sure if I > made this clear or not in the original post, but what is in > http://code.nsnam.org/raj/ns-3-dev forks the socket and returns it to the > application via a call to NotifyNewConnectionCreated. The original server > socket keeps on listening for incoming SYNs. I'm not sure I understand what > the two issues are? > The two issues are i) Gustavo's post that the current non-multitasking code needs fixed ii) Your suggestion that this is part of a broader issue with implementing server forks If ii) is not ready for this release, I suggest to address i) for this release
Created attachment 107 [details] Patch to notify app of the new connection This does the correct notification up to the application of the new connection. The server fork isn't quite ready yet for merging, so I'll merge this patch in before the release instead to address Gustavo's original issue.
Patch pushed. (http://code.nsnam.org/ns-3-dev/rev/6ae8117d4ffe) The issue of server forks is still open. It has been implemented (but not thorougly tested) in http://code.nsnam.org/raj/ns-3-dev/
Changed the name of the bug to address the broader issue.
Fixed in ns-3-dev, changesets 2362-2368.