Bug 2587 - Function named UpdateBeta is wrong in tcp-htcp.cc file
Function named UpdateBeta is wrong in tcp-htcp.cc file
Status: RESOLVED FIXED
Product: ns-3
Classification: Unclassified
Component: tcp
ns-3.26
All All
: P3 major
Assigned To: natale.patriciello
:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2016-12-15 08:00 UTC by Mingyu Park
Modified: 2017-02-06 07:23 UTC (History)
3 users (show)

See Also:


Attachments
Corrected patch (915 bytes, patch)
2017-02-03 08:26 UTC, natale.patriciello
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
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