(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.

(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.

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.

(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?

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

(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.

(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.

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

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.

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.

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.

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.