[Ns-bugs] [Bug 1256] New: Unnecessary SND.NXT advance, missing ACK for Out of Order segments

[code@nsnam.ece.gatech.edu]

https://www.nsnam.org/bugzilla/show_bug.cgi?id=1256

           Summary: Unnecessary SND.NXT advance, missing ACK for Out of
                    Order segments
           Product: ns-3
           Version: pre-release
          Platform: All
        OS/Version: All
            Status: NEW
          Severity: major
          Priority: P5
         Component: tcp
        AssignedTo: adrian.sw.tam at gmail.com
        ReportedBy: jabraham3 at mail.gatech.edu
                CC: ns-bugs at isi.edu
   Estimated Hours: 0.0


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
     }

-- 
Configure bugmail: https://www.nsnam.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug.