Bug 426 - TCP: close does not send RST
TCP: close does not send RST
Status: RESOLVED FIXED
Product: ns-3
Classification: Unclassified
Component: internet
pre-release
All All
: P1 blocker
Assigned To: George Riley
:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2008-11-28 06:03 UTC by Mathieu Lacage
Modified: 2009-10-07 23:13 UTC (History)
5 users (show)

See Also:


Attachments
refactor close and shutdownsend to be more "real world" (2.86 KB, patch)
2009-03-30 01:54 UTC, Rajib Bhattacharjea
Details | Diff
test case (8.13 KB, text/x-c++src)
2009-06-29 01:05 UTC, Tom Henderson
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Mathieu Lacage 2008-11-28 06:03:20 UTC
If I have a socket receiving data from a remote side, and if I close it before the other side is done sending, Close never sends RST. It should send RST unless the SO_LINGER option is set or unless the process which owns the socket exits (in which case the socket always lingers in the background).

To summarize, Close should send RST by default.

Note: Shutdown (SHUT_RDWR) should not send RST even though one might think that it would do the same.
Comment 1 Mathieu Lacage 2008-11-28 06:18:48 UTC
I wanted to point out that I think there is some confusion in the code between the 'CLOSE' action/event in the TCP RFC state machine descriptions and the socket close function. the 'CLOSE' action in the TCP RFC corresponds to the shutdown function. 

On linux, the close function seems to trigger a RST to be sent only when the rx queue is non-empty. See http://cs.baylor.edu/~donahoo/practical/CSockets/TCPRST.pdf for a practical experiment which outlines this behavior.
Comment 2 Mathieu Lacage 2008-11-28 06:30:50 UTC
So, to summarize, it looks like the tcp state machine should send a FIN unless rx data is queued in which case it should send a RST. So, my initial idea was at least partly wrong.

Note: I have verified that the above is indeed the linux behavior both in the code and with actual experiments.
Comment 3 Rajib Bhattacharjea 2009-03-17 16:51:23 UTC
Is this the behavior for both Shutdown AND Close?
Comment 4 Tom Henderson 2009-03-18 01:12:58 UTC
(In reply to comment #3)
> Is this the behavior for both Shutdown AND Close?
> 

It seems to me that this is related to bug 424 (need for a FIN-received callback).

To summarize, if close() is called on a socket that has unread data, TCP should send a RST.  If close() is called on a socket that has no unread data, TCP can send a FIN, but if it subsequently receives any more data (that is not a bare FIN) it should also send a RST.

A clean way to close an ns-3 socket that was used to send data is to call ShutdownSend(), and then wait for the NotifyClose() callback to be invoked, at which point the application can call Close().

This is worth a read:
http://blog.netherlabs.nl/articles/2009/01/18/the-ultimate-so_linger-page-or-why-is-my-tcp-not-reliable
Comment 5 Rajib Bhattacharjea 2009-03-30 01:54:52 UTC
Created attachment 407 [details]
refactor close and shutdownsend to be more "real world"
Comment 6 Tom Henderson 2009-03-30 10:06:43 UTC
Comment on attachment 407 [details]
refactor close and shutdownsend to be more "real world"

>diff -r 80601ddf444a examples/tcp-large-transfer.cc
>--- a/examples/tcp-large-transfer.cc	Mon Mar 23 00:06:28 2009 -0400
>+++ b/examples/tcp-large-transfer.cc	Mon Mar 30 01:49:51 2009 -0400
>@@ -212,5 +212,5 @@
>         return;
>       }
>   }
>-  localSocket->Close ();
>+  localSocket->ShutdownSend ();
> }

I would not necessarily change the above; Close() is still valid here.  The program otherwise doesn't call Close() or ShutdownRecv() so it seems insufficient to just make the above change.

>diff -r 80601ddf444a src/internet-stack/tcp-socket-impl.cc
>--- a/src/internet-stack/tcp-socket-impl.cc	Mon Mar 23 00:06:28 2009 -0400
>+++ b/src/internet-stack/tcp-socket-impl.cc	Mon Mar 30 01:49:51 2009 -0400
>@@ -73,6 +73,7 @@
>     m_closeRequestNotified (false),
>     m_closeOnEmpty (false),
>     m_pendingClose (false),
>+    m_closeCalled (false),
>     m_nextTxSequence (0),
>     m_highTxMark (0),
>     m_highestRxAck (0),
>@@ -112,6 +113,7 @@
>     m_closeRequestNotified (sock.m_closeRequestNotified),
>     m_closeOnEmpty (sock.m_closeOnEmpty),
>     m_pendingClose (sock.m_pendingClose),
>+    m_closeCalled (sock.m_closeCalled),
>     m_nextTxSequence (sock.m_nextTxSequence),
>     m_highTxMark (sock.m_highTxMark),
>     m_highestRxAck (sock.m_highestRxAck),
>@@ -294,6 +296,16 @@
> TcpSocketImpl::ShutdownSend (void)
> {
>   NS_LOG_FUNCTION_NOARGS ();
>+  if (m_pendingData && m_pendingData->Size() != 0)
>+  { // App close with pending data must wait until all data transmitted
>+    m_closeOnEmpty = true;
>+    NS_LOG_LOGIC("Socket " << this << 
>+                 " deferring close, state " << m_state);
>+    return 0;
>+  }
>+  
>+  Actions_t action  = ProcessEvent (APP_CLOSE);
>+  ProcessAction (action);
>   m_shutdownSend = true;
>   return 0;
> }
>@@ -308,18 +320,15 @@
> int
> TcpSocketImpl::Close (void)
> {
>-  NS_LOG_FUNCTION_NOARGS ();

do not delete the log statement

>-  if (m_pendingData && m_pendingData->Size() != 0)
>-    { // App close with pending data must wait until all data transmitted
>-      m_closeOnEmpty = true;
>-      NS_LOG_LOGIC("Socket " << this << 
>-                   " deferring close, state " << m_state);
>-      return 0;
>-    }
>-
>-  Actions_t action  = ProcessEvent (APP_CLOSE);
>-  ProcessAction (action);
>-  ShutdownSend ();
>+  if (GetRxAvailable () > 0)
>+  {
>+    ProcessAction (RST_TX);
>+  }
>+  else 
>+  {
>+    ShutdownSend();
>+  }
>+  m_closeCalled = true;

doesn't ShutdownRecv() need to be called also?  (and fix ShutdownRecv() as you previously mentioned)

I am not sure the code will honor shutdownRecv; it seems that RecvFrom ignores this.  After close(), you should not be able to read from the socket.

>   return 0;
> }
> 
>@@ -1125,6 +1134,11 @@
>                 " seq " << tcpHeader.GetSequenceNumber() <<
>                 " ack " << tcpHeader.GetAckNumber() <<
>                 " p.size is " << p->GetSize());
>+  if(m_closeCalled) //send RST accordingly
>+  {
>+    ProcessAction(RST_TX);
>+    return;
>+  }

Do you send RST for each NewRx?  I don't know if these are rate limited in any way.

>   States_t origState = m_state;
>   if (RxBufferFreeSpace() < p->GetSize()) 
>     { //if not enough room, fragment
>diff -r 80601ddf444a src/internet-stack/tcp-socket-impl.h
>--- a/src/internet-stack/tcp-socket-impl.h	Mon Mar 23 00:06:28 2009 -0400
>+++ b/src/internet-stack/tcp-socket-impl.h	Mon Mar 30 01:49:51 2009 -0400
>@@ -191,6 +191,7 @@
>   bool m_closeRequestNotified;
>   bool m_closeOnEmpty;
>   bool m_pendingClose;
>+  bool m_closeCalled;
> 
>   
>   //sequence info, sender side
Comment 7 Tom Henderson 2009-06-29 01:05:56 UTC
Created attachment 505 [details]
test case

If I close the socket while the sender is sending data, and I use an NSC TCP, here is the (expected) behavior:

0.160000 IP 10.1.2.2.50000 > 10.1.3.1.51089: F 1:1(0) ack 19865 win 1448 <nop,nop,timestamp 160 129>
0.189460 IP 10.1.3.1.51089 > 10.1.2.2.50000: P 19865:21313(1448) ack 1 win 183 <nop,nop,timestamp 168 147>
0.189460 IP 10.1.2.2.50000 > 10.1.3.1.51089: R 5177816:5177816(0) win 0
0.189926 IP 10.1.3.1.51089 > 10.1.2.2.50000: P 21313:21841(528) ack 1 win 183 <nop,nop,timestamp 168 147>
0.189926 IP 10.1.2.2.50000 > 10.1.3.1.51089: R 5177816:5177816(0) win 0

If I run this without NSC, I see:

0.160000 IP 10.1.2.2.50000 > 10.1.3.1.49153: F 1:1(0) ack 537 win 65535
0.200134 IP 10.1.3.1.49153 > 10.1.2.2.50000: . ack 2 win 65535
0.261092 IP 10.1.2.2.50000 > 10.1.3.1.49153: . ack 537 win 65535
0.302084 IP 10.1.3.1.49153 > 10.1.2.2.50000: . 537:1073(536) ack 2 win 65535
0.302084 IP 10.1.2.2.50000 > 10.1.3.1.49153: . ack 1073 win 65535
0.302547 IP 10.1.3.1.49153 > 10.1.2.2.50000: . 1073:1609(536) ack 2 win 65535
0.302547 IP 10.1.2.2.50000 > 10.1.3.1.49153: . ack 1609 win 65535
0.343076 IP 10.1.3.1.49153 > 10.1.2.2.50000: . 1609:2145(536) ack 2 win 65535

i.e., the sink sends a FIN, but has not closed the socket for reading, and never sends a reset.
Comment 8 Josh Pelkey 2009-10-07 23:13:02 UTC
changeset c54b36fb7317