traffic-control: Do not fire the dequeue trace when a packet is peeked

src/traffic-control/doc/queue-discs.rst

@@ -107,8 +107,9 @@ especially for queue discs for which it is not obvious what is the next
 packet that will be dequeued (e.g., queue discs having multiple internal
 queues or child queue discs or queue discs that drop packets after dequeue).
 Therefore, unless the subclass redefines the ``DoPeek`` method, calling ``Peek`` causes
-the next packet to be dequeued from the queue disc, though it is still retained
-within the queue disc.
+the next packet to be dequeued from the queue disc, though the packet is still
+considered to be part of the queue disc and the dequeue trace is fired when
+Dequeue is called and the packet is actually extracted from the queue disc.
 
 The C++ base class QueueDisc implements:
 

src/traffic-control/model/queue-disc.cc

@@ -328,6 +328,7 @@ QueueDisc::QueueDisc (QueueDiscSizePolicy policy)
      m_sojourn (0),
      m_maxSize (QueueSize ("1p")),         // to avoid that setting the mode at construction time is ignored
      m_running (false),
+     m_peeked (false),
      m_sizePolicy (policy),
      m_prohibitChangeMode (false)
 {
@@ -689,15 +690,23 @@ QueueDisc::PacketEnqueued (Ptr<const QueueDiscItem> item)
 void
 QueueDisc::PacketDequeued (Ptr<const QueueDiscItem> item)
 {
-  m_nPackets--;
-  m_nBytes -= item->GetSize ();
-  m_stats.nTotalDequeuedPackets++;
-  m_stats.nTotalDequeuedBytes += item->GetSize ();
+  // If the queue disc asked the internal queue or the child queue disc to
+  // dequeue a packet because a peek operation was requested, the packet is
+  // still held by the queue disc, hence we do not need to update statistics
+  // and fire the dequeue trace. This function will be explicitly called when
+  // the packet will be actually dequeued.
+  if (!m_peeked)
+    {
+      m_nPackets--;
+      m_nBytes -= item->GetSize ();
+      m_stats.nTotalDequeuedPackets++;
+      m_stats.nTotalDequeuedBytes += item->GetSize ();
 
-  m_sojourn = Simulator::Now () - item->GetTimeStamp ();
+      m_sojourn = Simulator::Now () - item->GetTimeStamp ();
 
-  NS_LOG_LOGIC ("m_traceDequeue (p)");
-  m_traceDequeue (item);
+      NS_LOG_LOGIC ("m_traceDequeue (p)");
+      m_traceDequeue (item);
+    }
 }
 
 void
@@ -770,6 +779,17 @@ QueueDisc::DropAfterDequeue (Ptr<const QueueDiscItem> item, const char* reason)
       m_stats.nDroppedBytesAfterDequeue[reason] = item->GetSize ();
     }
 
+  // if in the context of a peek request a dequeued packet is dropped, we need
+  // to update the statistics and fire the dequeue trace before firing the drop
+  // after dequeue trace
+  if (m_peeked)
+    {
+      // temporarily set m_peeked to false, otherwise PacketDequeued does nothing
+      m_peeked = false;
+      PacketDequeued (item);
+      m_peeked = true;
+    }
+
   NS_LOG_DEBUG ("Total packets/bytes dropped after dequeue: "
                 << m_stats.nTotalDroppedPacketsAfterDequeue << " / "
                 << m_stats.nTotalDroppedBytesAfterDequeue);
@@ -869,6 +889,15 @@ QueueDisc::Dequeue (void)
   if (item)
     {
       m_requeued = 0;
+      if (m_peeked)
+        {
+          // If the packet was requeued because a peek operation was requested
+          // (which is the case here because DequeuePacket calls Dequeue only
+          // when m_requeued is null), we need to explicitly call PacketDequeued
+          // to update statistics about dequeued packets and fire the dequeue trace.
+          m_peeked = false;
+          PacketDequeued (item);
+        }
     }
   else
     {
@@ -895,7 +924,13 @@ QueueDisc::DoPeek (void)
 
   if (!m_requeued)
     {
+      m_peeked = true;
       m_requeued = Dequeue ();
+      // if no packet is returned, reset the m_peeked flag
+      if (!m_requeued)
+        {
+          m_peeked = false;
+        }
     }
   return m_requeued;
 }
@@ -972,6 +1007,14 @@ QueueDisc::DequeuePacket ()
           {
             item = m_requeued;
             m_requeued = 0;
+            if (m_peeked)
+              {
+                // If the packet was requeued because a peek operation was requested
+                // we need to explicitly call PacketDequeued to update statistics
+                // about dequeued packets and fire the dequeue trace.
+                m_peeked = false;
+                PacketDequeued (item);
+              }
           }
     }
   else

src/traffic-control/model/queue-disc.h

@@ -574,14 +574,16 @@ private:
    * The implementation of this method is based on the qdisc_peek_dequeued
    * function of the Linux kernel, which dequeues a packet and retains it in the
    * queue disc as a requeued packet. The packet is not traced as requeued, nor
-   * is the total count of requeued packets increased. The dequeued packet is
-   * not counted in the backlog of the queue disc and is actually extracted from
-   * the queue disc by calling Dequeue. This approach is especially recommended
-   * for queue discs for which it is not obvious what is the next packet that
-   * will be dequeued (e.g., queue discs having multiple internal queues or
-   * child queue discs or queue discs that drop packets after dequeue).
-   * Subclasses can however provide their own implementation of this method that
-   * overrides the default one.
+   * is the total count of requeued packets increased. The packet is still
+   * considered to be part of the queue disc and the dequeue trace is fired
+   * when Dequeue is called and the packet is actually extracted from the
+   * queue disc.
+   *
+   * This approach is especially recommended for queue discs for which it is not
+   * obvious what is the next packet that will be dequeued (e.g., queue discs
+   * having multiple internal queues or child queue discs or queue discs that
+   * drop packets after dequeue). Subclasses can however provide their own
+   * implementation of this method that overrides the default one.
    *
    * \return 0 if the operation was not successful; the packet otherwise.
    */
@@ -672,6 +674,7 @@ private:
   Ptr<NetDeviceQueueInterface> m_devQueueIface;   //!< NetDevice queue interface
   bool m_running;                   //!< The queue disc is performing multiple dequeue operations
   Ptr<QueueDiscItem> m_requeued;    //!< The last packet that failed to be transmitted
+  bool m_peeked;                    //!< A packet was dequeued because Peek was called
   std::string m_childQueueDiscDropMsg;  //!< Reason why a packet was dropped by a child queue disc
   QueueDiscSizePolicy m_sizePolicy;     //!< The queue disc size policy
   bool m_prohibitChangeMode;            //!< True if changing mode is prohibited