Bug 2944 - TcpSocketBase over count duplicated ACK
TcpSocketBase over count duplicated ACK
Status: ASSIGNED
Product: ns-3
Classification: Unclassified
Component: tcp
ns-3.27
All All
: P3 critical
Assigned To: natale.patriciello
:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2018-07-06 20:23 EDT by Chih-Yuan Chang
Modified: 2018-09-02 01:25 EDT (History)
1 user (show)

See Also:


Attachments
Patch (2.68 KB, patch)
2018-07-11 06:56 EDT, natale.patriciello
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Chih-Yuan Chang 2018-07-06 20:23:33 EDT
Inside TcpSocketBase::ProcessAck(), DupAck() will be called if the following condition is satisfied (without checking if there is non-zero payload).  

  if (scoreboardUpdated ||
      (!m_sackEnabled && ackNumber == m_txBuffer->HeadSequence ()
       && ackNumber < m_tcb->m_nextTxSequence))
    {
      ...
      // loss recovery check is done inside this function thanks to
      // the congestion state machine
      DupAck ();
    }

But if there is non-zero payload, even the ack number is old, Tcp shall not increment m_dupAckCount.

As a result, the following seemingly harmless Tcp packets exchanged caused NS_ASSERT (oldDupAckCount - segsAcked == 1) abort inside the CA_DISORDER block.

At time 2.00001 Ipv4Header: tos 0x0 DSCP Default ECN Not-ECT ttl 63 id 0 protocol 6 offset (bytes) 0 flags [none] length: 44 10.0.8.15 > 10.0.8.13 TcpHeader: 49154 > 50050 [SYN] Seq=0 Ack=0 Win=65535 ns3::TcpOptionWinScale(3) ns3::TcpOptionEnd(EOL) Payload:0
At time 2.00003 Ipv4Header: tos 0x0 DSCP Default ECN Not-ECT ttl 63 id 0 protocol 6 offset (bytes) 0 flags [none] length: 44 10.0.8.13 > 10.0.8.15 TcpHeader: 50050 > 49154 [SYN|ACK] Seq=0 Ack=1 Win=65535 ns3::TcpOptionWinScale(7) ns3::TcpOptionEnd(EOL) Payload:0
At time 2.00005 Ipv4Header: tos 0x0 DSCP Default ECN Not-ECT ttl 63 id 1 protocol 6 offset (bytes) 0 flags [none] length: 40 10.0.8.15 > 10.0.8.13 TcpHeader: 49154 > 50050 [ACK] Seq=1 Ack=1 Win=51200 Payload:0
At time 2.00005 Ipv4Header: tos 0x0 DSCP Default ECN Not-ECT ttl 63 id 2 protocol 6 offset (bytes) 0 flags [none] length: 176 10.0.8.15 > 10.0.8.13 TcpHeader: 49154 > 50050 [ACK] Seq=1 Ack=1 Win=51200 Payload:136
At time 2.00006 Ipv4Header: tos 0x0 DSCP Default ECN Not-ECT ttl 63 id 3 protocol 6 offset (bytes) 0 flags [none] length: 108 10.0.8.15 > 10.0.8.13 TcpHeader: 49154 > 50050 [ACK] Seq=137 Ack=1 Win=51200 Payload:68
At time 2.00006 Ipv4Header: tos 0x0 DSCP Default ECN Not-ECT ttl 63 id 4 protocol 6 offset (bytes) 0 flags [none] length: 108 10.0.8.15 > 10.0.8.13 TcpHeader: 49154 > 50050 [ACK] Seq=205 Ack=1 Win=51200 Payload:68
At time 2.00007 Ipv4Header: tos 0x0 DSCP Default ECN Not-ECT ttl 63 id 1 protocol 6 offset (bytes) 0 flags [none] length: 40 10.0.8.13 > 10.0.8.15 TcpHeader: 50050 > 49154 [ACK] Seq=1 Ack=137 Win=3200 Payload:0
At time 2.00007 Ipv4Header: tos 0x0 DSCP Default ECN Not-ECT ttl 63 id 2 protocol 6 offset (bytes) 0 flags [none] length: 176 10.0.8.13 > 10.0.8.15 TcpHeader: 50050 > 49154 [ACK] Seq=1 Ack=137 Win=3200 Payload:136
At time 2.00008 Ipv4Header: tos 0x0 DSCP Default ECN Not-ECT ttl 63 id 3 protocol 6 offset (bytes) 0 flags [none] length: 40 10.0.8.13 > 10.0.8.15 TcpHeader: 50050 > 49154 [ACK] Seq=137 Ack=137 Win=3200 Payload:0
At time 2.00008 Ipv4Header: tos 0x0 DSCP Default ECN Not-ECT ttl 63 id 4 protocol 6 offset (bytes) 0 flags [none] length: 108 10.0.8.13 > 10.0.8.15 TcpHeader: 50050 > 49154 [ACK] Seq=137 Ack=205 Win=3200 Payload:68
At time 2.00008 Ipv4Header: tos 0x0 DSCP Default ECN Not-ECT ttl 63 id 5 protocol 6 offset (bytes) 0 flags [none] length: 40 10.0.8.13 > 10.0.8.15 TcpHeader: 50050 > 49154 [ACK] Seq=205 Ack=205 Win=3200 Payload:0
At time 2.00008 Ipv4Header: tos 0x0 DSCP Default ECN Not-ECT ttl 63 id 6 protocol 6 offset (bytes) 0 flags [none] length: 108 10.0.8.13 > 10.0.8.15 TcpHeader: 50050 > 49154 [ACK] Seq=205 Ack=273 Win=3200 Payload:68
Comment 1 natale.patriciello 2018-07-10 10:58:02 EDT
(In reply to Chih-Yuan Chang from comment #0)
> Inside TcpSocketBase::ProcessAck(), DupAck() will be called if the following
> condition is satisfied (without checking if there is non-zero payload).  
> 
>   if (scoreboardUpdated ||
>       (!m_sackEnabled && ackNumber == m_txBuffer->HeadSequence ()
>        && ackNumber < m_tcb->m_nextTxSequence))
>     {
>       ...
>       // loss recovery check is done inside this function thanks to
>       // the congestion state machine
>       DupAck ();
>     }
> 
> But if there is non-zero payload, even the ack number is old, Tcp shall not
> increment m_dupAckCount.

Do you have SACK enabled? Because with SACK, if the scoreboard is updated, then that is a dupack.
Comment 2 Chih-Yuan Chang 2018-07-10 13:07:21 EDT
(In reply to natale.patriciello from comment #1)
> (In reply to Chih-Yuan Chang from comment #0)
> > Inside TcpSocketBase::ProcessAck(), DupAck() will be called if the following
> > condition is satisfied (without checking if there is non-zero payload).  
> > 
> >   if (scoreboardUpdated ||
> >       (!m_sackEnabled && ackNumber == m_txBuffer->HeadSequence ()
> >        && ackNumber < m_tcb->m_nextTxSequence))
> >     {
> >       ...
> >       // loss recovery check is done inside this function thanks to
> >       // the congestion state machine
> >       DupAck ();
> >     }
> > 
> > But if there is non-zero payload, even the ack number is old, Tcp shall not
> > increment m_dupAckCount.
> 
> Do you have SACK enabled? Because with SACK, if the scoreboard is updated,
> then that is a dupack.


SACK is DISABLED.
Comment 3 natale.patriciello 2018-07-11 06:56:56 EDT
Created attachment 3132 [details]
Patch

Could you please try this patch? I would like to see a test case, though. So if you can build one, that'd be awesome.
Comment 4 Chih-Yuan Chang 2018-07-11 15:00:11 EDT
(In reply to natale.patriciello from comment #3)
> Created attachment 3132 [details]
> Patch
> 
> Could you please try this patch? I would like to see a test case, though. So
> if you can build one, that'd be awesome.

Hi, 

I am working on ns3.27, which is the last matching version for a TCP Cubic implementation from WPI.

Would you supply the diff for ns3.27?
Comment 5 Chih-Yuan Chang 2018-07-11 15:02:41 EDT
In addition, I need SACK from time to time. And I noticed the same assertion NS_ASSERT (oldDupAckCount - segsAcked == 1) fired up when there are packets lost and the packets are small. For example, the following TCP exchanges have the assertion fired.

At time 2.00423 Ipv4Header: tos 0x0 DSCP Default ECN Not-ECT ttl 62 id 34 protocol 6 offset (bytes) 0 flags [none] length: 40 10.0.15.10 > 10.0.4.2 TcpHeader: 50020 > 49153 [ACK] Seq=2 Ack=1497 Win=3200 Payload:0
At time 2.00425 Ipv4Header: tos 0x0 DSCP Default ECN Not-ECT ttl 62 id 38 protocol 6 offset (bytes) 0 flags [none] length: 108 10.0.4.2 > 10.0.15.10 TcpHeader: 49153 > 50020 [ACK] Seq=1633 Ack=2 Win=51200 Payload:68
At time 2.00425 Ipv4Header: tos 0x0 DSCP Default ECN Not-ECT ttl 62 id 39 protocol 6 offset (bytes) 0 flags [none] length: 108 10.0.4.2 > 10.0.15.10 TcpHeader: 49153 > 50020 [ACK] Seq=1701 Ack=2 Win=51200 Payload:68
At time 2.00442 Ipv4Header: tos 0x0 DSCP Default ECN Not-ECT ttl 62 id 40 protocol 6 offset (bytes) 0 flags [none] length: 108 10.0.4.2 > 10.0.15.10 TcpHeader: 49153 > 50020 [ACK] Seq=1769 Ack=2 Win=51200 Payload:68
At time 2.00472 Ipv4Header: tos 0x0 DSCP Default ECN Not-ECT ttl 62 id 41 protocol 6 offset (bytes) 0 flags [none] length: 108 10.0.4.2 > 10.0.15.10 TcpHeader: 49153 > 50020 [ACK] Seq=1837 Ack=2 Win=51200 Payload:68
At time 2.00485 Ipv4Header: tos 0x0 DSCP Default ECN Not-ECT ttl 62 id 42 protocol 6 offset (bytes) 0 flags [none] length: 108 10.0.4.2 > 10.0.15.10 TcpHeader: 49153 > 50020 [ACK] Seq=1905 Ack=2 Win=51200 Payload:68
At time 2.00487 Ipv4Header: tos 0x0 DSCP Default ECN Not-ECT ttl 62 id 43 protocol 6 offset (bytes) 0 flags [none] length: 108 10.0.4.2 > 10.0.15.10 TcpHeader: 49153 > 50020 [ACK] Seq=1973 Ack=2 Win=51200 Payload:68
At time 2.00503 Ipv4Header: tos 0x0 DSCP Default ECN Not-ECT ttl 62 id 37 protocol 6 offset (bytes) 0 flags [none] length: 52 10.0.15.10 > 10.0.4.2 TcpHeader: 50020 > 49153 [ACK] Seq=2 Ack=1565 Win=3200 ns3::TcpOptionSack(blocks: 1,[1633,1701]) ns3::TcpOptionEnd(EOL) Payload:0
At time 2.00503 Ipv4Header: tos 0x0 DSCP Default ECN Not-ECT ttl 62 id 38 protocol 6 offset (bytes) 0 flags [none] length: 52 10.0.15.10 > 10.0.4.2 TcpHeader: 50020 > 49153 [ACK] Seq=2 Ack=1565 Win=3200 ns3::TcpOptionSack(blocks: 1,[1633,1769]) ns3::TcpOptionEnd(EOL) Payload:0
At time 2.00524 Ipv4Header: tos 0x0 DSCP Default ECN Not-ECT ttl 62 id 39 protocol 6 offset (bytes) 0 flags [none] length: 52 10.0.15.10 > 10.0.4.2 TcpHeader: 50020 > 49153 [ACK] Seq=2 Ack=1565 Win=3200 ns3::TcpOptionSack(blocks: 1,[1633,1837]) ns3::TcpOptionEnd(EOL) Payload:0
At time 2.00527 Ipv4Header: tos 0x0 DSCP Default ECN Not-ECT ttl 62 id 45 protocol 6 offset (bytes) 0 flags [none] length: 516 10.0.4.2 > 10.0.15.10 TcpHeader: 49153 > 50020 [ACK] Seq=1565 Ack=2 Win=51200 Payload:476
At time 2.0055 Ipv4Header: tos 0x0 DSCP Default ECN Not-ECT ttl 62 id 40 protocol 6 offset (bytes) 0 flags [none] length: 52 10.0.15.10 > 10.0.4.2 TcpHeader: 50020 > 49153 [ACK] Seq=2 Ack=1565 Win=3200 ns3::TcpOptionSack(blocks: 1,[1633,1905]) ns3::TcpOptionEnd(EOL) Payload:0
At time 2.00558 Ipv4Header: tos 0x0 DSCP Default ECN Not-ECT ttl 62 id 41 protocol 6 offset (bytes) 0 flags [none] length: 52 10.0.15.10 > 10.0.4.2 TcpHeader: 50020 > 49153 [ACK] Seq=2 Ack=1565 Win=3200 ns3::TcpOptionSack(blocks: 1,[1633,1973]) ns3::TcpOptionEnd(EOL) Payload:0
At time 2.00587 Ipv4Header: tos 0x0 DSCP Default ECN Not-ECT ttl 62 id 46 protocol 6 offset (bytes) 0 flags [none] length: 108 10.0.4.2 > 10.0.15.10 TcpHeader: 49153 > 50020 [ACK] Seq=2041 Ack=2 Win=51200 Payload:68
At time 2.00595 Ipv4Header: tos 0x0 DSCP Default ECN Not-ECT ttl 62 id 48 protocol 6 offset (bytes) 0 flags [none] length: 108 10.0.4.2 > 10.0.15.10 TcpHeader: 49153 > 50020 [ACK] Seq=2109 Ack=2 Win=51200 Payload:68
At time 2.00601 Ipv4Header: tos 0x0 DSCP Default ECN Not-ECT ttl 62 id 49 protocol 6 offset (bytes) 0 flags [none] length: 108 10.0.4.2 > 10.0.15.10 TcpHeader: 49153 > 50020 [ACK] Seq=2177 Ack=2 Win=51200 Payload:68
At time 2.00601 Ipv4Header: tos 0x0 DSCP Default ECN Not-ECT ttl 62 id 50 protocol 6 offset (bytes) 0 flags [none] length: 108 10.0.4.2 > 10.0.15.10 TcpHeader: 49153 > 50020 [ACK] Seq=2245 Ack=2 Win=51200 Payload:68
At time 2.00614 Ipv4Header: tos 0x0 DSCP Default ECN Not-ECT ttl 62 id 51 protocol 6 offset (bytes) 0 flags [none] length: 108 10.0.4.2 > 10.0.15.10 TcpHeader: 49153 > 50020 [ACK] Seq=2313 Ack=2 Win=51200 Payload:68
At time 2.00614 Ipv4Header: tos 0x0 DSCP Default ECN Not-ECT ttl 62 id 52 protocol 6 offset (bytes) 0 flags [none] length: 108 10.0.4.2 > 10.0.15.10 TcpHeader: 49153 > 50020 [ACK] Seq=2381 Ack=2 Win=51200 Payload:68
At time 2.00623 Ipv4Header: tos 0x0 DSCP Default ECN Not-ECT ttl 62 id 53 protocol 6 offset (bytes) 0 flags [none] length: 108 10.0.4.2 > 10.0.15.10 TcpHeader: 49153 > 50020 [ACK] Seq=2449 Ack=2 Win=51200 Payload:68
At time 2.00634 Ipv4Header: tos 0x0 DSCP Default ECN Not-ECT ttl 62 id 55 protocol 6 offset (bytes) 0 flags [none] length: 108 10.0.4.2 > 10.0.15.10 TcpHeader: 49153 > 50020 [ACK] Seq=2517 Ack=2 Win=51200 Payload:68
At time 2.00641 Ipv4Header: tos 0x0 DSCP Default ECN Not-ECT ttl 62 id 43 protocol 6 offset (bytes) 0 flags [none] length: 40 10.0.15.10 > 10.0.4.2 TcpHeader: 50020 > 49153 [ACK] Seq=2 Ack=2109 Win=3200 Payload:0
Comment 6 natale.patriciello 2018-07-11 17:03:07 EDT
(In reply to Chih-Yuan Chang from comment #4)
> (In reply to natale.patriciello from comment #3)
> > Created attachment 3132 [details]
> > Patch
> > 
> > Could you please try this patch? I would like to see a test case, though. So
> > if you can build one, that'd be awesome.
> 
> Hi, 
> 
> I am working on ns3.27, which is the last matching version for a TCP Cubic
> implementation from WPI.
> 
> Would you supply the diff for ns3.27?

So... we have fixed many things in the meanwhile, I suggest to port your TCP Cubic patch to ns-3-dev (and maybe share it with others?!) and then try my patch (if the problem persists).

I don't know why people are so scared about trying to upstream their patches, feel like very sad to me.
Comment 7 Chih-Yuan Chang 2018-07-11 17:29:00 EDT
(In reply to natale.patriciello from comment #6)
> (In reply to Chih-Yuan Chang from comment #4)
> > (In reply to natale.patriciello from comment #3)
> > > Created attachment 3132 [details]
> > > Patch
> > > 
> > > Could you please try this patch? I would like to see a test case, though. So
> > > if you can build one, that'd be awesome.
> > 
> > Hi, 
> > 
> > I am working on ns3.27, which is the last matching version for a TCP Cubic
> > implementation from WPI.
> > 
> > Would you supply the diff for ns3.27?
> 
> So... we have fixed many things in the meanwhile, I suggest to port your TCP
> Cubic patch to ns-3-dev (and maybe share it with others?!) and then try my
> patch (if the problem persists).
> 
> I don't know why people are so scared about trying to upstream their
> patches, feel like very sad to me.


I don't own the TCP Cubic patch. 
It is from http://perform.wpi.edu/downloads/#cubic
But it only work upto ns3.27.
Comment 8 Chih-Yuan Chang 2018-07-13 19:31:34 EDT
I applied the patch & re-run the simulation. 
This time, a different assertion fired

assert failed. cond="AreEquals (t1->m_lost, t2->m_lost)", msg="Merging one lost and another not lost. Impossible", file=../src/internet/model/tcp-tx-buffer.cc, line=598
Comment 9 Chih-Yuan Chang 2018-07-15 00:28:47 EDT
In last comment (comment#8), SACK is enabled.

I disabled SACK and re-ran, different NS_ASSERT fired.
assert failed. cond="m_sentList.size () > 1", file=../src/internet/model/tcp-tx-buffer.cc, line=1343
terminate called without an active exception

In my simulation, I have the following scenarios
1. there are lots of small packets
2. there are lots of packets dropped

Hope these clues help.
Comment 10 natale.patriciello 2018-07-15 10:34:38 EDT
Hi,

not really. It would be awesome to have a simple example based on ns-3-dev with a congestion control currently supported.

I don't have time to create from scratch an example in which your problem appears.
Comment 11 Chih-Yuan Chang 2018-07-17 13:27:53 EDT
NS3 Version: tip of dev (on July 16, 2018)
Assertion fired: assert failed. cond="m_sentList.size () > 1", file=../src/internet/model/tcp-tx-buffer.cc, line=1343

Condition: SACK disabled. TCP Full-duplex data transfer. estimated 10% packets dropped.
Comment 12 Chih-Yuan Chang 2018-09-02 01:25:27 EDT
When SACK is enabled, TCP works properly with the patch. 
I would suggest include the patch into the next release.

When SACK is disabled, assertion fired as described in comment#11.