Bug 1601 - RttEstimator doesn't set the m_currentEstimatedRtt to m_initialEstimatedRtt on creation
RttEstimator doesn't set the m_currentEstimatedRtt to m_initialEstimatedRtt o...
Status: RESOLVED FIXED
Product: ns-3
Classification: Unclassified
Component: internet
ns-3-dev
All All
: P5 critical
Assigned To: George Riley
:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2013-03-12 17:43 UTC by Tommaso Pecorella
Modified: 2013-04-19 12:24 UTC (History)
2 users (show)

See Also:


Attachments
Patch fixing the bug (1.24 KB, patch)
2013-03-13 06:28 UTC, Tommaso Pecorella
Details | Diff
new patch (1.91 KB, patch)
2013-03-17 18:01 UTC, Tommaso Pecorella
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Tommaso Pecorella 2013-03-12 17:43:12 UTC
This bug is quite puzzling me. Not the bug itself, the problem is on how to fix it.

Now, RttEstimator's constructor is something like:
{
  ObjectBase::ConstructSelf (AttributeConstructionList ());
  m_currentEstimatedRtt = m_initialEstimatedRtt;
}
whereas m_initialEstimatedRtt is set by an attribute.

The problem (huge one) is this: the attribute default value is 1.0 sec, but m_currentEstimatedRtt is set to ZERO.

The calls traces are:
RttEstimator:RttEstimator(0x7ff2c1c87de0)
Initialize m_currentEstimatedRtt to 0 sec.
Initialize m_initialEstimatedRtt to 0 sec.
RttEstimator:RttMeanDeviation(0x7ff2c1c87de0)
RttEstimator:SetInitialEstimatedRtt(0x7ff2c1c87de0, +1000000000.0ns)
RttEstimator:SetMinRto(0x7ff2c1c87de0, +200000000.0ns)

Mind that I added an explicit setter and getter for InitialEstimatedRtt.

How this is possible? Top ranked guess (I'm not an attribute wizard):
- the ObjectBase::ConstructSelf (AttributeConstructionList ()) call is not doing what's supposed to do. Maybe because the TypeId is the one of the derived class ? How can we force the base class attributes to be read ?

Another solution is to use the DoStart() method, however the RttEstimator is *not* aggregated to the node, it's just built on-the-fly when a socket is created.

The only (ugly) solution I found is to add another *public* function DoStart (DoStart is protected usually) and call it explicitly when the RttEstimator is created. Of course we can call it in a different way, but the idea is still ugly, no matter how we call the function.

Any better solution ?

T.
Comment 1 Tommaso Pecorella 2013-03-12 19:11:43 UTC
Forget about it. I have the solution. And it's damn simple. And stupid. And undocumented.

Tomorrow I'll post it.

T.
Comment 2 Tommaso Pecorella 2013-03-13 06:28:15 UTC
Created attachment 1532 [details]
Patch fixing the bug

Patch fixing the bug

Ok, the bug is this.

The ObjectBase::ConstructSelf (AttributeConstructionList ()) function will call
GetInstanceTypeId, which usually is a virtual function, inherited from Object.
The function should call GetTypeId, which is static to the class.

On the other hand, inside the constructor the object is still not fully
created, hence its virtual inherited stuff is kinda... strange.

To make a long story short. This will work *only* if the object is actually
implementing a GetInstanceTypeId function.

The patch does this and it clarifies the manual about this point.

T.
Comment 3 Tommaso Pecorella 2013-03-17 18:01:44 UTC
Created attachment 1534 [details]
new patch

Also the derived classes must implement GetInstanceTypeId
Comment 4 Brian Swenson 2013-04-19 12:24:51 UTC
changeset:   9698:9d91a3c643b2
tag:         tip
user:        Tommaso Pecorella <tommaso.pecorella@unifi.it>
date:        Fri Apr 19 12:18:00 2013 -0400
summary:     Bug 1601 RttEstimator doesn't set m_currentEstimatedRtt fix

Updated RELEASE_NOTES