From 9f60774b59b8271631e5e558d215486eeec1f71f Mon Sep 17 00:00:00 2001 From: Stefano Avallone Date: Fri, 27 Jan 2017 18:02:38 +0100 Subject: [PATCH] traffic-control: (fixes #2537) Fix dead assignment on CoDel::DoDequeue --- RELEASE_NOTES | 1 + src/traffic-control/model/codel-queue-disc.cc | 69 ++++++++----------- src/traffic-control/model/codel-queue-disc.h | 4 +- 3 files changed, 32 insertions(+), 42 deletions(-) diff --git a/RELEASE_NOTES b/RELEASE_NOTES index 45a8d9c44..0e4f744b8 100644 --- a/RELEASE_NOTES +++ b/RELEASE_NOTES @@ -46,6 +46,7 @@ Bugs fixed - Bug 2533 - Provide a better 802.11n/ac PHY abstraction model for SIMO, MISO and MIMO - Bug 2535 - memory leak in bench-simulator.cc - Bug 2536 - fixed dead assignment and potential memory leak in wimax +- Bug 2537 - Dead assigment on CoDel::DoDequeue - Bug 2538 - fixed dead assignment on tap-bridge - Bug 2540 - fixed dead assignment on mesh/ie-dot11s-perr - Bug 2541 - preamble not assigned correctly diff --git a/src/traffic-control/model/codel-queue-disc.cc b/src/traffic-control/model/codel-queue-disc.cc index cc2860b72..d0d5f65cf 100644 --- a/src/traffic-control/model/codel-queue-disc.cc +++ b/src/traffic-control/model/codel-queue-disc.cc @@ -309,13 +309,19 @@ CoDelQueueDisc::DoEnqueue (Ptr item) } bool -CoDelQueueDisc::OkToDrop (Ptr p, uint32_t now) +CoDelQueueDisc::OkToDrop (Ptr item, uint32_t now) { NS_LOG_FUNCTION (this); CoDelTimestampTag tag; bool okToDrop; - bool found = p->RemovePacketTag (tag); + if (!item) + { + m_firstAboveTime = 0; + return false; + } + + bool found = item->GetPacket ()->RemovePacketTag (tag); NS_ASSERT_MSG (found, "found a packet without an input timestamp tag"); NS_UNUSED (found); //silence compiler warning Time delta = Simulator::Now () - tag.GetTxTime (); @@ -340,8 +346,7 @@ CoDelQueueDisc::OkToDrop (Ptr p, uint32_t now) NS_LOG_LOGIC ("Sojourn time has just gone above target from below, need to stay above for at least q->interval before packet can be dropped. "); m_firstAboveTime = now + Time2CoDel (m_interval); } - else - if (CoDelTimeAfter (now, m_firstAboveTime)) + else if (CoDelTimeAfter (now, m_firstAboveTime)) { NS_LOG_LOGIC ("Sojourn time has been above target for at least q->interval; it's OK to (possibly) drop packet."); okToDrop = true; @@ -355,24 +360,22 @@ CoDelQueueDisc::DoDequeue (void) { NS_LOG_FUNCTION (this); - if (GetInternalQueue (0)->IsEmpty ()) + Ptr item = StaticCast (GetInternalQueue (0)->Dequeue ()); + if (!item) { // Leave dropping state when queue is empty m_dropping = false; - m_firstAboveTime = 0; NS_LOG_LOGIC ("Queue empty"); return 0; } uint32_t now = CoDelGetTime (); - Ptr item = StaticCast (GetInternalQueue (0)->Dequeue ()); - Ptr p = item->GetPacket (); NS_LOG_LOGIC ("Popped " << item); NS_LOG_LOGIC ("Number packets remaining " << GetInternalQueue (0)->GetNPackets ()); NS_LOG_LOGIC ("Number bytes remaining " << GetInternalQueue (0)->GetNBytes ()); - // Determine if p should be dropped - bool okToDrop = OkToDrop (p, now); + // Determine if item should be dropped + bool okToDrop = OkToDrop (item, now); if (m_dropping) { // In the dropping state (sojourn time has gone above target and hasn't come down yet) @@ -384,8 +387,7 @@ CoDelQueueDisc::DoDequeue (void) NS_LOG_LOGIC ("Sojourn time goes below target, it's OK to leave dropping state."); m_dropping = false; } - else - if (CoDelTimeAfterEq (now, m_dropNext)) + else if (CoDelTimeAfterEq (now, m_dropNext)) { m_state2++; while (m_dropping && CoDelTimeAfterEq (now, m_dropNext)) @@ -396,27 +398,22 @@ CoDelQueueDisc::DoDequeue (void) // A large amount of packets in queue might result in drop // rates so high that the next drop should happen now, // hence the while loop. - NS_LOG_LOGIC ("Sojourn time is still above target and it's time for next drop; dropping " << p); + NS_LOG_LOGIC ("Sojourn time is still above target and it's time for next drop; dropping " << item); Drop (item); ++m_dropCount; ++m_count; NewtonStep (); - if (GetInternalQueue (0)->IsEmpty ()) - { - m_dropping = false; - NS_LOG_LOGIC ("Queue empty"); - ++m_states; - return 0; - } item = StaticCast (GetInternalQueue (0)->Dequeue ()); - p = item ->GetPacket (); - NS_LOG_LOGIC ("Popped " << item); - NS_LOG_LOGIC ("Number packets remaining " << GetInternalQueue (0)->GetNPackets ()); - NS_LOG_LOGIC ("Number bytes remaining " << GetInternalQueue (0)->GetNBytes ()); + if (item) + { + NS_LOG_LOGIC ("Popped " << item); + NS_LOG_LOGIC ("Number packets remaining " << GetInternalQueue (0)->GetNPackets ()); + NS_LOG_LOGIC ("Number bytes remaining " << GetInternalQueue (0)->GetNBytes ()); + } - if (!OkToDrop (p, now)) + if (!OkToDrop (item, now)) { /* leave dropping state */ NS_LOG_LOGIC ("Leaving dropping state"); @@ -440,29 +437,21 @@ CoDelQueueDisc::DoDequeue (void) if (okToDrop) { // Drop the first packet and enter dropping state unless the queue is empty - NS_LOG_LOGIC ("Sojourn time goes above target, dropping the first packet " << p << " and entering the dropping state"); + NS_LOG_LOGIC ("Sojourn time goes above target, dropping the first packet " << item << " and entering the dropping state"); ++m_dropCount; Drop (item); - if (GetInternalQueue (0)->IsEmpty ()) - { - m_dropping = false; - okToDrop = false; - NS_LOG_LOGIC ("Queue empty"); - ++m_states; - } - else - { - item = StaticCast (GetInternalQueue (0)->Dequeue ()); - p = item->GetPacket (); + item = StaticCast (GetInternalQueue (0)->Dequeue ()); + if (item) + { NS_LOG_LOGIC ("Popped " << item); NS_LOG_LOGIC ("Number packets remaining " << GetInternalQueue (0)->GetNPackets ()); NS_LOG_LOGIC ("Number bytes remaining " << GetInternalQueue (0)->GetNBytes ()); - - okToDrop = OkToDrop (p, now); - m_dropping = true; } + + OkToDrop (item, now); + m_dropping = true; ++m_state3; /* * if min went above target close to when we last went below it diff --git a/src/traffic-control/model/codel-queue-disc.h b/src/traffic-control/model/codel-queue-disc.h index 5bf171ea5..09a9e39d8 100644 --- a/src/traffic-control/model/codel-queue-disc.h +++ b/src/traffic-control/model/codel-queue-disc.h @@ -182,11 +182,11 @@ private: * \brief Determine whether a packet is OK to be dropped. The packet * may not be actually dropped (depending on the drop state) * - * \param p The packet that is considered + * \param item The packet that is considered * \param now The current time represented as 32-bit unsigned integer (us) * \returns True if it is OK to drop the packet (sojourn time above target for at least interval) */ - bool OkToDrop (Ptr p, uint32_t now); + bool OkToDrop (Ptr item, uint32_t now); /** * Check if CoDel time a is successive to b