Bug 2053

Summary: In tcp-socket-base.cc, NotifyDataSent incorrectly called with retransmits
Product: ns-3 Reporter: Steve Zabele <zabele>
Component: internetAssignee: George Riley <riley>
Status: RESOLVED FIXED    
Severity: normal CC: ns-bugs, tommaso.pecorella, zabele
Priority: P5    
Version: ns-3.21   
Hardware: PC   
OS: Linux   

Description Steve Zabele 2015-01-30 06:50:23 EST
I found this bug when using the TcpSocketBase::NotifyDataSent callback to track the sizes and times packets were being sent and found that Tcp was reporting that more data was being sent that I was supplying. To be clear, the packet sizes were correct, however the callback was reporting many more packets going out than there should have been.

Details:

In SendDataPacket near the bottom of the method there is a comment

// Notify the application of the data being sent unless this is a retransmit

The test for retransmission that immediately follows is:

if (seq == m_nextTxSequence)

This is incorrect since retransmission processing sets m_nextTxSequence. Hence the notification

Simulator::ScheduleNow(&TcpSocketBase::NotifyDataSent, this, sz);

can be (and is from my test code) called when a data packet is sent on retransmits, not just original transmits as the comment implies

FIX:

The simple fix is to change the test to

if (seq == m_highTxMark)

This seems pretty straightforward, but if need be I can supply a test program and/or a debug mod to tcp-socket-base.cc that prints out the sequence numbers for packets causing the notification -- this latter approach shows that multiple notifications are being generated for some sequence numbers.
Comment 1 Tommaso Pecorella 2015-01-31 07:23:52 EST
Confirmed the bug, confirmed the patch. Closed in changeset: 11193:f35b8f7e50f1

Thanks.