Bug 2384 - Child queue discs need to notify their parent queue disc of packet drops
Child queue discs need to notify their parent queue disc of packet drops
Status: RESOLVED FIXED
Product: ns-3
Classification: Unclassified
Component: traffic-control
ns-3-dev
PC Linux
: P5 enhancement
Assigned To: Stefano Avallone
:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2016-04-22 14:02 UTC by Stefano Avallone
Modified: 2016-05-11 06:26 UTC (History)
2 users (show)

See Also:


Attachments
Proposed fix (3.99 KB, patch)
2016-04-22 14:02 UTC, Stefano Avallone
Details | Diff
Proposed fix v2 (4.19 KB, patch)
2016-04-26 11:47 UTC, Stefano Avallone
Details | Diff
Proposed fix v3 (4.20 KB, patch)
2016-04-27 09:13 UTC, Stefano Avallone
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Stefano Avallone 2016-04-22 14:02:33 UTC
Created attachment 2396 [details]
Proposed fix

Otherwise the packet/byte counters kept by the parent queue disc are incorrect, as they are updated when a packet is enqueued or dequeued but not when a packet is dropped by a child queue disc.

In order to avoid introducing a reference cycle, I could not add a Ptr<QueueDisc> m_parent method in the QueueDisc class and used a callback.

This issue has been discovered while implementing fq-codel (for which a code review is basically ready), which checks that the number of packets enqueued in the whole queue disc does not exceed a given limit.

Thanks,
Stefano
Comment 1 Stefano Avallone 2016-04-26 11:47:32 UTC
Created attachment 2401 [details]
Proposed fix v2

Second version of the proposed fix, which differs from the previous one in that the signature of

void QueueDisc::Drop (Ptr<QueueDiscItem> item);

is changed into

void QueueDisc::Drop (Ptr<QueueItem> item);

This is to allow queues to notify queue discs of packet drops (see bug #2389) through a callback set to QueueDisc::Drop. This is safe because QueueDiscItem inherits from QueueItem and QueueDisc::Drop only calls methods from the QueueItem base class (no methods specific to QueueDiscItem are called).
Comment 2 Stefano Avallone 2016-04-27 09:13:32 UTC
Created attachment 2405 [details]
Proposed fix v3

Fix a typo spotted by Pasquale
Comment 3 Tom Henderson 2016-05-09 13:08:30 UTC
Looks OK to me.
Comment 4 Stefano Avallone 2016-05-11 06:26:48 UTC
Fixed by changeset 12108:a1ba150121f3