Bug 138 - Point-to-point netdevice queues aren't configurable from higher layers
Point-to-point netdevice queues aren't configurable from higher layers
Status: RESOLVED FIXED
Product: ns-3
Classification: Unclassified
Component: devices
pre-release
All All
: P3 normal
Assigned To: ns-bugs
:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2008-02-12 14:05 UTC by Rajib Bhattacharjea
Modified: 2008-07-01 13:32 UTC (History)
1 user (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Rajib Bhattacharjea 2008-02-12 14:05:50 UTC
Summary from off-list/off-tracker emails:
Changing the queue sizes on a per queue basis should be like:
PointToPointTopology::GetNetDevice (n1,channel1)->GetQueue()->SetMaxPackets(25);

But there are two issues that prevent this from working:

   1. The GetQueue method of the NetDevice is private.
   2. SetMaxPackets (or indeed any Queue API related to resizing/setting limits) isn't a Queue base class API, but is in DropTailQueue.

-----------------------------
Tom's Reply:
I can tell you the historical reason for #2, which is that Mathieu argued that SetMaxPackets is semantically vague when you start talking about certain types of 802.11 queues (such as 802.11e) and therefore he opposed putting this method into the Queue base class.

However, Mathieu ended up not even using the queue base class (he implemented 802.11-specific queues for his wifi net device) so it may be a moot point now.

I do not know the rationale for #1.  I think that adding a public GetQueue() to PointToPointNetDevice would be fine.

Another possibility would be to aggregate a Queue to a PointToPointNetDevice and then GetObject<Queue> would be publicly available.  This would allow you to do "safe downcasts" in the future (such as GetObject<REDQueue> to see if it has a RED interface).
Comment 1 Gustavo J. A. M. Carneiro 2008-02-13 09:02:09 UTC
(In reply to comment #0)
> Summary from off-list/off-tracker emails:
> Changing the queue sizes on a per queue basis should be like:
> PointToPointTopology::GetNetDevice
> (n1,channel1)->GetQueue()->SetMaxPackets(25);
> 
> But there are two issues that prevent this from working:
> 
>    1. The GetQueue method of the NetDevice is private.
>    2. SetMaxPackets (or indeed any Queue API related to resizing/setting
> limits) isn't a Queue base class API, but is in DropTailQueue.
> 
> -----------------------------
> Tom's Reply:
> I can tell you the historical reason for #2, which is that Mathieu argued that
> SetMaxPackets is semantically vague when you start talking about certain types
> of 802.11 queues (such as 802.11e) and therefore he opposed putting this method
> into the Queue base class.

SetMaxPackets should work for any kind of Queue, even 802.11e.  In some queue types, like 802.11e, it may happen that a "queue" will be actually composed of several "sub-queues", one for each class of service.  In this case we could just define SetMaxPackets as calling SetMaxPackets for each of the sub-queues, with the same value for all (so SetMaxPackets(N) in the queue will end up potentially accepting between N and N*M packets, depending on the distribution of packets per class of service, K being the number of subqueues).

> 
> However, Mathieu ended up not even using the queue base class (he implemented
> 802.11-specific queues for his wifi net device) so it may be a moot point now.
> 
> I do not know the rationale for #1.  I think that adding a public GetQueue() to
> PointToPointNetDevice would be fine.
> 
> Another possibility would be to aggregate a Queue to a PointToPointNetDevice
> and then GetObject<Queue> would be publicly available.  This would allow you to
> do "safe downcasts" in the future (such as GetObject<REDQueue> to see if it has
> a RED interface).
 
I'm not sure GetObject is appropriate; GetObject (formerly QueryInterface) is for obtaining functionality from an object that is optionally implemented.  A GetQueue () method is better IMHO because a Queue is supposed to be always available in a NetDevice, so an explicit geter method is clearer and more compile-time safe.  For downcast it is always easy to do:

 Ptr<RedQueue> = dynamic_cast<RedQueue*> (PeekPointer (p2p.GetQueue ()));

Well, I know, the downcast could be easier, perhaps.  Maybe we could add a downcast helper function to the ptr.h header?

template <typename BASE_TYPE, typename SUBTYPE>
Ptr<SUBTYPE> DynamicCast<SUBTYPE> (Ptr<BASE_TYPE> obj)
{
  return Ptr<SUBTYPE> (dynamic_cast<SUBTYPE*> (PeekPointer (obj)));
}
Comment 2 Tom Henderson 2008-03-06 17:58:28 UTC
Gustavo,
I think you raise an interesting question-- whether GetObject () should be used for cases where dynamic_cast<> can be used.  When we were considering more fully COM-like approaches, the answer seemed to be that we would use QueryInterface for such downcasts.  We could choose between two policies in our code base:
- avoid dynamic_cast, use GetObject
- use dynamic_cast where you want a subclass pointer, and GetObject for when an entirely different object needs to be fetched

Other opinions?  
Comment 3 Mathieu Lacage 2008-03-06 20:25:52 UTC
(In reply to comment #2)

> - avoid dynamic_cast, use GetObject
> - use dynamic_cast where you want a subclass pointer, and GetObject for when an
> entirely different object needs to be fetched


The former is definitely the easiest to use. dynamic_cast would require the user to call PeekPointer which is definitely something we would like to avoid.
Comment 4 Craig Dowell 2008-03-06 21:56:40 UTC
I prefer using GetObject.
Comment 5 Gustavo J. A. M. Carneiro 2008-03-07 06:01:49 UTC
(In reply to comment #4)
> I prefer using GetObject.
> 

But keep in mind that:
 1- GetObject does some completely redundant work that dynamic_cast doesn't have to do.  In particular now that types have parent types and all, I have seen GetObject and TypeId::GetParent starting to appear in performance profiling.

 2- Having a GetQueue () explicit method makes the code easier to use than GetObject <something> ().  GetObject makes some things rather hidden from the programmer; you can't tell there is a certain object aggregated by looking at the header file alone.

 3- The main point of GetObject is to gain modularity and decoupling of orthogonal aspects.  If netdevices always need to have queues, then we are not getting anything.

So we potentially lose performance (1), readability (2), and do not gain anything in return (3).  There's only one argument in favor of GetObject, here, which is that it is easier to use.  But if we want easy to use, we might as well redesign the GetQueue method like this:

template<typename QUEUE>
Ptr<QUEUE> GetQueue (void) const
{
  return dynamic_cast<QUEUE*> (PeekPointer (DoGetQueue ()));
}

Problem solved.  Or add my DynamicCast function.
Comment 6 Tom Henderson 2008-03-07 17:46:08 UTC
an additional technique may be to use an attribute.  I'd like to suggest that we try to support this particular case with by setting an attribute, once that system is finalized, before deciding on trying to support it otherwise.
Comment 7 Mathieu Lacage 2008-04-15 11:50:29 UTC
marking as FIXED since you can now control this with Attributes.