Bug 1256

Summary: Unnecessary SND.NXT advance, missing ACK for Out of Order segments
Product: ns-3 Reporter: John Abraham <john.abraham.in>
Component: tcpAssignee: Brian Swenson <bswenson3>
Status: RESOLVED FIXED    
Severity: major CC: bswenson3, ns-bugs, tomh
Priority: P5    
Version: pre-release   
Hardware: All   
OS: All   
Attachments: Testcase that works with 3.14.1
Proposed change to address issue

Description John Abraham 2011-09-05 23:17:31 UTC 
The issue can be reproduced per the google thread
http://groups.google.com/group/ns-3-users/browse_thread/thread/a4fb8f5cffe784be

I speculate the following issues:
1.  Out-of-order segments should be acked unless it is a RST segment
2.  The SND.NXT (m_TxNextSequence) was bumped up for CLOSING/FIN_WAIT1/LAST_ACK states.However the very nature of these states imply that the peer has completed sending its highest sequence number which is the FIN sequence number
3. When in CLOSE_WAIT/CLOSING/LAST_ACK states, SEG.SEQ must be equal to RCV.NXT
4. Even if the acceptable Segment test fails, we must continue to process ACKs
using the "acceptable ack" RFC 793 test

Perhaps the following diff can be used as a starting point for discussion (as the fix may be non-trivial) 


diff -r 5b3af869c7f8 src/internet/model/tcp-socket-base.cc
--- a/src/internet/model/tcp-socket-base.cc	Sat Aug 20 14:41:19 2011 -0400
+++ b/src/internet/model/tcp-socket-base.cc	Mon Sep 05 23:03:59 2011 -0400
@@ -603,7 +603,7 @@
     { // Rx buffer in these states are not initialized.
       return false;
     }
-  if (m_state == LAST_ACK || m_state == CLOSING)
+  if (m_state == LAST_ACK || m_state == CLOSING || m_state == CLOSE_WAIT) //JOHN
     { // In LAST_ACK and CLOSING states, it only wait for an ACK and the
       // sequence number must equals to m_rxBuffer.NextRxSequence ()
       return (m_rxBuffer.NextRxSequence () != s);
@@ -651,6 +651,11 @@
                     " received packet of seq " << tcpHeader.GetSequenceNumber () <<
                     " out of range [" << m_rxBuffer.NextRxSequence () << ":" <<
                     m_rxBuffer.MaxRxSequence () << "]");
+	  if (!(tcpHeader.GetFlags () & TcpHeader::RST)) //JOHN
+		{
+			ReceivedAck (packet,tcpHeader);
+			SendEmptyPacket (TcpHeader::ACK);
+	    }
       return;
     }
 
@@ -764,7 +769,7 @@
           DupAck (tcpHeader, ++m_dupAckCount);
         }
       // otherwise, the ACK is precisely equal to the nextTxSequence
-      NS_ASSERT (tcpHeader.GetAckNumber () <= m_nextTxSequence);
+      // NS_ASSERT (tcpHeader.GetAckNumber () <= m_nextTxSequence);
     }
   else if (tcpHeader.GetAckNumber () > m_txBuffer.HeadSequence ())
     { // Case 3: New ACK, reset m_dupAckCount and update m_txBuffer
@@ -1170,10 +1175,6 @@
     {
       flags |= TcpHeader::ACK;
     }
-  else if (m_state == FIN_WAIT_1 || m_state == LAST_ACK || m_state == CLOSING)
-    {
-      ++s;
-    }
 
   header.SetFlags (flags);
   header.SetSequenceNumber (s);

@@ -1521,7 +1523,7 @@
     {
       NotifySend (GetTxAvailable ());
     }
-  if (ack > m_nextTxSequence)
+  if (ack > m_nextTxSequence && !(m_state == FIN_WAIT_1 || m_state == LAST_ACK || m_state == FIN_WAIT_2 || m_state == CLOSING)) //JOHN
     {
       m_nextTxSequence = ack; // If advanced
     }
Comment 1 Brian Swenson 2012-07-26 12:47:33 UTC 
Created attachment 1429 [details]
Testcase that works with 3.14.1

Updated test case to work with version 3.14.1 of NS3 (changed header files)
Comment 2 Brian Swenson 2012-07-26 13:01:34 UTC 
With the testcase the ACK sent by n0 in response to the FIN sent from n2 is lost.  n2 continues to send out FINs.

//FIN Sent
58 [node 2] TcpSocketBase:Close(0x11db2d0)
58 [node 2] TcpSocketBase:DoClose(0x11db2d0)
58 [node 2] TcpSocketBase:SendEmptyPacket(0x11db2d0, 1)
58 [node 2] Schedule retransmission timeout at time 58 to expire at time 58.2
58 [node 2] ESTABLISHED -> FIN_WAIT_1
58 [node 2] TcpSocketBase:Close(0x11d9c20)
58 [node 2] TcpSocketBase:DoClose(0x11d9c20)
58 [node 2] TcpSocketBase:CloseAndNotify(0x11d9c20)
58 [node 2] LISTEN -> CLOSED

//ACK dropped
58.0057 [node 0] Socket 0x11da2e0 forward up 192.168.1.2:8080 to 192.168.0.1:49153
58.0057 [node 0] TcpSocketBase:ProcessEstablished(0x11da2e0, 8080 > 49153 [ FIN  ACK ] Seq=1 Ack=653361 Win=65535)
58.0057 [node 0] TcpSocketBase:PeerClose(0x11da2e0, 8080 > 49153 [ FIN  ACK ] Seq=1 Ack=653361 Win=65535)
58.0057 [node 0] Accepted FIN at seq 1
58.0057 [node 0] ESTABLISHED -> CLOSE_WAIT
58.0057 [node 0] TCP 0x11da2e0 calling NotifyNormalClose
58.0057 [node 0] TcpSocketBase:SendEmptyPacket(0x11da2e0, 16)
RxDrop at 58.007

//FIN sent again
58.2 [node 2] TcpSocketBase:SendEmptyPacket(0x11db2d0, 17)
58.2 [node 2] Schedule retransmission timeout at time 58.2 to expire at time 58.4

//FIN sent again
58.4 [node 2] TcpSocketBase:SendEmptyPacket(0x11db2d0, 17)
58.4 [node 2] Schedule retransmission timeout at time 58.4 to expire at time 58.6

//FIN sent again
58.6 [node 2] TcpSocketBase:SendEmptyPacket(0x11db2d0, 17)
58.6 [node 2] Schedule retransmission timeout at time 58.6 to expire at time 58.8

//And so on..  However n0 is still sending data to n2.  And it contains the correct ACK = 2.

58.193 [node 2] Socket 0x11db2d0 forward up 192.168.0.1:49153 to 192.168.1.2:8080
58.193 [node 2] TcpSocketBase:ProcessWait(0x11db2d0, 49153 > 8080 [ ACK ] Seq=655505 Ack=2 Win=65535)
58.193 [node 2] TcpSocketBase:ReceivedData(0x11db2d0, 49153 > 8080 [ ACK ] Seq=655505 Ack=2 Win=65535)

However, n2 doesn't not correctly recognize the FIN packet as being ACK'd because it is expecting an empty packet with ACK=2.
Comment 3 Brian Swenson 2012-07-26 13:06:19 UTC 
Also n0 is receiving the duplicate FINs from n2, however it doesn't send an empty ACK because it is now in state CLOSE_WAIT and not ESTABLISHED
Comment 4 Brian Swenson 2012-07-26 13:34:57 UTC 
Created attachment 1430 [details]
Proposed change to address issue
Comment 5 Brian Swenson 2012-07-26 13:40:26 UTC 
Log with Patch:

//FIN sent
58 [node 2] TcpSocketBase:Close(0x14ff2d0)
58 [node 2] TcpSocketBase:DoClose(0x14ff2d0)
58 [node 2] TcpSocketBase:SendEmptyPacket(0x14ff2d0, 1)
58 [node 2] Schedule retransmission timeout at time 58 to expire at time 58.2
58 [node 2] ESTABLISHED -> FIN_WAIT_1
58 [node 2] TcpSocketBase:Close(0x14fdc20)
58 [node 2] TcpSocketBase:DoClose(0x14fdc20)
58 [node 2] TcpSocketBase:CloseAndNotify(0x14fdc20)
58 [node 2] LISTEN -> CLOSED

//ACK dropped
58.0057 [node 0] Socket 0x14fe2e0 forward up 192.168.1.2:8080 to 192.168.0.1:49153
58.0057 [node 0] TcpSocketBase:ProcessEstablished(0x14fe2e0, 8080 > 49153 [ FIN  ACK ] Seq=1 Ack=653361 Win=65535)
58.0057 [node 0] TcpSocketBase:PeerClose(0x14fe2e0, 8080 > 49153 [ FIN  ACK ] Seq=1 Ack=653361 Win=65535)
58.0057 [node 0] Accepted FIN at seq 1
58.0057 [node 0] ESTABLISHED -> CLOSE_WAIT
58.0057 [node 0] TCP 0x14fe2e0 calling NotifyNormalClose
58.0057 [node 0] TcpSocketBase:SendEmptyPacket(0x14fe2e0, 16)
RxDrop at 58.007

//Received data with ACK = 2
58.193 [node 2] Socket 0x14ff2d0 forward up 192.168.0.1:49153 to 192.168.1.2:8080
58.193 [node 2] TcpSocketBase:ProcessWait(0x14ff2d0, 49153 > 8080 [ ACK ] Seq=655505 Ack=2 Win=65535)
58.193 [node 2] TcpSocketBase:ReceivedAck(0x14ff2d0, 49153 > 8080 [ ACK ] Seq=655505 Ack=2 Win=65535)
58.193 [node 2] New ack of 2
...
//Doesn't resend FIN
58.193 [node 2] 0x14ff2d0 Cancelled ReTxTimeout event which was set to expire at 58.2
...
//Goes into FIN_WAIT_2
58.193 [node 2] FIN_WAIT_1 -> FIN_WAIT_2
Comment 6 Brian Swenson 2013-04-09 15:37:41 UTC 
9020:200c9dd61e1a