Bug 2587

Summary: Function named UpdateBeta is wrong in tcp-htcp.cc file
Product: ns-3 Reporter: Mingyu Park <darkpmg>
Component: tcpAssignee: natale.patriciello
Status: RESOLVED FIXED    
Severity: major CC: amir.arc, ns-bugs, tomh
Priority: P3    
Version: ns-3.26   
Hardware: All   
OS: All   
Attachments: Corrected patch

Description Mingyu Park 2016-12-15 08:00:10 UTC
First of all, m_throughput's type is uint32. And, if m_lastThroughput is greater than m_throughput, the result of '((m_throughput - m_lastThroughput) / m_lastThroughput)' should be negative value like following algorithm. 

if (((m_throughput - m_lastThroughput) / m_lastThroughput) > m_throughputRatio)
{
   m_beta = m_defaultBackoff;
}
else
{
   m_beta = m_minRtt.GetDouble () / m_maxRtt.GetDouble ();
}

But! since m_throughput's type is uint32, overflow is occurred. So, m_beta's value will be mostly m_defaultBackoff. 

Finally we suggest change algorithm like that

if((m_throughput - m_lastThroughput) > 0) { 
    if (((m_throughput - m_lastThroughput) / m_lastThroughput) > m_throughputRatio)
    {
       m_beta = m_defaultBackoff;
    }
}
else
{
    m_beta = m_minRtt.GetDouble () / m_maxRtt.GetDouble ();
}
Comment 1 natale.patriciello 2017-02-03 08:26:10 UTC
Created attachment 2776 [details]
Corrected patch

Hi,

Your patch still present a problem of overflow because of the fact that uint32 - uint32 is always > 0 (as you said, there could be overflow).

The corrected behavior (IMHO) is reported in this patch. Please check.
Comment 2 Amir M 2017-02-05 11:52:38 UTC
The patch looks correct. Thank you
Comment 3 natale.patriciello 2017-02-06 07:23:44 UTC
Committed in 12670:0c9178f1709d, thanks