From edcd9a6909bdf12ea837913bdcb6b4c5936c6a67 Mon Sep 17 00:00:00 2001 From: Stefano Avallone Date: Thu, 16 Sep 2021 17:54:18 +0200 Subject: [PATCH] wifi: Cleanup WifiMacQueue 1. MPDUs with expired lifetime are already discarded: - when a QosTxop releases/requests the channel - when we check if there are pending BlockAckReq frames hence, it is useless (and sometimes troublesome) to check for MPDUs with expired lifetime at every packet dequeue 2. Dequeue{ByTid, ByAddress, ...} were not used and hence removed (one can use a Peek* method and then Dequeue()) 3. Given that a WifiMacQueueItem stores an iterator, add a Dequeue method (the only one in addition to the default one) that receives a WifiMacQueueItem and dequeues it in constant time --- src/wifi/model/frame-exchange-manager.cc | 5 +- src/wifi/model/msdu-aggregator.cc | 18 ++- src/wifi/model/wifi-mac-queue.cc | 148 +++-------------------- src/wifi/model/wifi-mac-queue.h | 67 +--------- 4 files changed, 36 insertions(+), 202 deletions(-) diff --git a/src/wifi/model/frame-exchange-manager.cc b/src/wifi/model/frame-exchange-manager.cc index 9dec2030d..620719063 100644 --- a/src/wifi/model/frame-exchange-manager.cc +++ b/src/wifi/model/frame-exchange-manager.cc @@ -329,7 +329,7 @@ FrameExchangeManager::GetFirstFragmentIfNeeded (Ptr mpdu) m_fragmentedPacket = mpdu->GetPacket ()->Copy (); // dequeue the MSDU WifiMacQueueItem::QueueIteratorPair queueIt = mpdu->GetQueueIteratorPair (); - queueIt.queue->Dequeue (queueIt.it); + queueIt.queue->DequeueIfQueued (*queueIt.it); // create the first fragment mpdu->GetHeader ().SetMoreFragments (); Ptr fragment = m_fragmentedPacket->CreateFragment (0, m_mac->GetWifiRemoteStationManager ()->GetFragmentSize (mpdu, 0)); @@ -450,8 +450,7 @@ FrameExchangeManager::DequeueMpdu (Ptr mpdu) if (mpdu->IsQueued ()) { const WifiMacQueueItem::QueueIteratorPair& queueIt = mpdu->GetQueueIteratorPair (); - NS_ASSERT (*queueIt.it == mpdu); - queueIt.queue->Dequeue (queueIt.it); + queueIt.queue->DequeueIfQueued (*queueIt.it); } } diff --git a/src/wifi/model/msdu-aggregator.cc b/src/wifi/model/msdu-aggregator.cc index 42cc52b97..48393ed97 100644 --- a/src/wifi/model/msdu-aggregator.cc +++ b/src/wifi/model/msdu-aggregator.cc @@ -127,10 +127,20 @@ MsduAggregator::GetNextAmsdu (Ptr peekedItem, WifiTxPara while ((it = queue->PeekByTidAndAddress (tid, recipient, it)) != queue->end () && m_htFem->TryAggregateMsdu (*it, txParams, availableTime)) { - // Aggregate() dequeues the MSDU being aggregated, so we have to save an iterator - // pointing to the next item - auto msduIt = it++; - queue->Aggregate (amsdu->GetQueueIteratorPair ().it, msduIt); + // dequeue the MSDU being aggregated and advance the current iterator + // before it is invalidated + Ptr msdu = *it++; + queue->DequeueIfQueued (msdu); + + auto pos = std::next (amsdu->GetQueueIteratorPair ().it); + queue->DequeueIfQueued (amsdu); + + amsdu->Aggregate (msdu); + bool ret = queue->Insert (pos, amsdu); + // The size of a WifiMacQueue is measured as number of packets. We dequeued + // two packets, so there is certainly room for inserting one packet + NS_ABORT_IF (!ret); + nMsdu++; } diff --git a/src/wifi/model/wifi-mac-queue.cc b/src/wifi/model/wifi-mac-queue.cc index e616c7514..eb0a29ac3 100644 --- a/src/wifi/model/wifi-mac-queue.cc +++ b/src/wifi/model/wifi-mac-queue.cc @@ -63,8 +63,7 @@ WifiMacQueue::GetTypeId (void) } WifiMacQueue::WifiMacQueue () - : m_expiredPacketsPresent (false), - NS_LOG_TEMPLATE_DEFINE ("WifiMacQueue") + : NS_LOG_TEMPLATE_DEFINE ("WifiMacQueue") { } @@ -187,96 +186,18 @@ WifiMacQueue::Dequeue (void) return 0; } -Ptr -WifiMacQueue::DequeueByAddress (Mac48Address dest) +void +WifiMacQueue::DequeueIfQueued (Ptr mpdu) { - NS_LOG_FUNCTION (this << dest); - ConstIterator it = PeekByAddress (dest); + NS_LOG_FUNCTION (this << *mpdu); - if (it == end ()) + if (mpdu->IsQueued ()) { - return 0; + NS_ASSERT (mpdu->m_queueIt.queue == this); + NS_ASSERT (*mpdu->m_queueIt.it == mpdu); + + DoDequeue (mpdu->m_queueIt.it); } - return Dequeue (it); -} - -Ptr -WifiMacQueue::DequeueByTid (uint8_t tid) -{ - NS_LOG_FUNCTION (this << +tid); - ConstIterator it = PeekByTid (tid); - - if (it == end ()) - { - return 0; - } - return Dequeue (it); -} - -Ptr -WifiMacQueue::DequeueByTidAndAddress (uint8_t tid, Mac48Address dest) -{ - NS_LOG_FUNCTION (this << +tid << dest); - ConstIterator it = PeekByTidAndAddress (tid, dest); - - if (it == end ()) - { - return 0; - } - return Dequeue (it); -} - -Ptr -WifiMacQueue::DequeueFirstAvailable (const Ptr blockedPackets) -{ - NS_LOG_FUNCTION (this); - ConstIterator it = PeekFirstAvailable (blockedPackets); - - if (it == end ()) - { - return 0; - } - return Dequeue (it); -} - -Ptr -WifiMacQueue::Dequeue (ConstIterator pos) -{ - NS_LOG_FUNCTION (this); - - const Time now = Simulator::Now (); - if (!m_expiredPacketsPresent) - { - if (TtlExceeded (pos, now)) - { - NS_LOG_DEBUG ("Packet lifetime expired"); - return 0; - } - return DoDequeue (pos); - } - - // remove stale items queued before the given position - ConstIterator it = begin (); - while (it != end ()) - { - if (it == pos) - { - // reset the flag signaling the presence of expired packets before returning - m_expiredPacketsPresent = false; - - if (TtlExceeded (it, now)) - { - return 0; - } - return DoDequeue (it); - } - else if (!TtlExceeded (it, now)) - { - it++; - } - } - NS_LOG_DEBUG ("Invalid iterator"); - return 0; } Ptr @@ -292,8 +213,6 @@ WifiMacQueue::Peek (void) const { return DoPeek (it); } - // signal the presence of expired packets - m_expiredPacketsPresent = true; } NS_LOG_DEBUG ("The queue is empty"); return 0; @@ -317,11 +236,6 @@ WifiMacQueue::PeekByAddress (Mac48Address dest, ConstIterator pos) const return it; } } - else - { - // signal the presence of expired packets - m_expiredPacketsPresent = true; - } it++; } NS_LOG_DEBUG ("The queue is empty"); @@ -345,11 +259,6 @@ WifiMacQueue::PeekByTid (uint8_t tid, ConstIterator pos) const return it; } } - else - { - // signal the presence of expired packets - m_expiredPacketsPresent = true; - } it++; } NS_LOG_DEBUG ("The queue is empty"); @@ -374,11 +283,6 @@ WifiMacQueue::PeekByTidAndAddress (uint8_t tid, Mac48Address dest, ConstIterator return it; } } - else - { - // signal the presence of expired packets - m_expiredPacketsPresent = true; - } it++; } NS_LOG_DEBUG ("The queue is empty"); @@ -403,11 +307,6 @@ WifiMacQueue::PeekFirstAvailable (const Ptr blockedPacke return it; } } - else - { - // signal the presence of expired packets - m_expiredPacketsPresent = true; - } it++; } NS_LOG_DEBUG ("The queue is empty"); @@ -474,9 +373,6 @@ WifiMacQueue::Remove (ConstIterator pos, bool removeExpired) { if (it == pos) { - // reset the flag signaling the presence of expired packets before returning - m_expiredPacketsPresent = false; - ConstIterator curr = pos++; DoRemove (curr); return pos; @@ -643,6 +539,14 @@ WifiMacQueue::DoEnqueue (ConstIterator pos, Ptr item) Ptr WifiMacQueue::DoDequeue (ConstIterator pos) { + NS_LOG_FUNCTION (this); + + if (TtlExceeded (pos, Simulator::Now ())) + { + NS_LOG_DEBUG ("Packet lifetime expired"); + return nullptr; + } + Ptr item = Queue::DoDequeue (pos); if (item != 0 && item->GetHeader ().IsQosData ()) @@ -690,22 +594,4 @@ WifiMacQueue::DoRemove (ConstIterator pos) return item; } -void -WifiMacQueue::Aggregate (ConstIterator amsduIt, ConstIterator msduIt) -{ - NS_LOG_FUNCTION (this << **amsduIt << **msduIt); - NS_ASSERT (amsduIt != msduIt); - - Ptr msdu = DoDequeue (msduIt); - - ConstIterator pos = std::next (amsduIt); - Ptr amsdu = DoDequeue (amsduIt); - amsdu->Aggregate (msdu); - - bool ret = DoEnqueue (pos, amsdu); - // The size of a WifiMacQueue is measured as number of packets. We dequeued - // two packets, so there is certainly room for inserting one packet - NS_ABORT_IF (!ret); -} - } //namespace ns3 diff --git a/src/wifi/model/wifi-mac-queue.h b/src/wifi/model/wifi-mac-queue.h index 29381eca3..52e12bbb0 100644 --- a/src/wifi/model/wifi-mac-queue.h +++ b/src/wifi/model/wifi-mac-queue.h @@ -122,61 +122,11 @@ public: */ Ptr Dequeue (void) override; /** - * Search and return, if present in the queue, the first packet (either Data - * frame or QoS Data frame) having the receiver address equal to addr. - * This method removes the packet from the queue. - * It is typically used by ns3::Txop during the CF period. + * Dequeue the given MPDU if it is stored in this queue. * - * \param dest the given destination - * - * \return the packet + * \param mpdu the given MPDU */ - Ptr DequeueByAddress (Mac48Address dest); - /** - * Search and return, if present in the queue, the first packet having the - * TID equal to tid. - * This method removes the packet from the queue. - * - * \param tid the given TID - * - * \return the packet - */ - Ptr DequeueByTid (uint8_t tid); - /** - * Search and return, if present in the queue, the first packet having the - * address indicated by type equal to addr, and TID - * equal to tid. This method removes the packet from the queue. - * It is typically used by ns3::QosTxop in order to perform correct MSDU - * aggregation (A-MSDU). - * - * \param tid the given TID - * \param dest the given destination - * - * \return the packet - */ - Ptr DequeueByTidAndAddress (uint8_t tid, - Mac48Address dest); - /** - * Return first available packet for transmission. A packet could be no available - * if it is a QoS packet with a TID and an address1 fields equal to tid and addr - * respectively that index a pending agreement in the BlockAckManager object. - * So that packet must not be transmitted until reception of an ADDBA response frame from station - * addressed by addr. This method removes the packet from queue. - * - * \param blockedPackets the destination address & TID pairs that are waiting for a BlockAck response - * - * \return the packet - */ - Ptr DequeueFirstAvailable (const Ptr blockedPackets = nullptr); - /** - * Dequeue the item at position pos in the queue. Return a null - * pointer if the given iterator is invalid, the queue is empty or the - * lifetime of the item pointed to by the given iterator is expired. - * - * \param pos the position of the item to be dequeued - * \return the dequeued item, if any - */ - Ptr Dequeue (WifiMacQueue::ConstIterator pos); + void DequeueIfQueued (Ptr mpdu); /** * Peek the packet in the front of the queue. The packet is not removed. * @@ -260,16 +210,6 @@ public: * \return an iterator pointing to the item following the removed one */ ConstIterator Remove (ConstIterator pos, bool removeExpired = false); - /** - * Aggregate the MSDU pointed to by the msduIt iterator (source MPDU) - * to the (A-)MSDU pointed to by the amsduIt iterator (destination MPDU). - * Both MPDUs are dequeued and an MPDU containing the resulting A-MSDU is - * enqueued (replacing the destination MPDU). - * - * \param amsduIt the destination MPDU - * \param msduIt the source MPDU - */ - void Aggregate (ConstIterator amsduIt, ConstIterator msduIt); /** * Return the number of packets having destination address specified by * dest. The complexity is linear in the size of the queue. @@ -382,7 +322,6 @@ private: Time m_maxDelay; //!< Time to live for packets in the queue DropPolicy m_dropPolicy; //!< Drop behavior of queue - mutable bool m_expiredPacketsPresent; //!< True if expired packets are in the queue /// Per (MAC address, TID) pair queued packets std::unordered_map m_nQueuedPackets;