From 6bce3d9916475fe08e7d12c0db31e3d507ec87cd Mon Sep 17 00:00:00 2001 From: Stefano Avallone Date: Thu, 26 Aug 2021 17:36:56 +0200 Subject: [PATCH] wifi: Replace aggregated MSDUs with the A-MSDU upon aggregation So far, when A-MSDU aggregation is performed, MPDUs containing the constituent MSDUs are kept in the queue until the MPDU containing the A-MSDU (which is kept out of the queue) is transmitted. Now, as soon as A-MSDU aggregation is performed, MPDUs containing the constituent MSDUs are dequeued and the MPDU containing the A-MSDU is enqueued in their place. --- src/wave/model/wave-frame-exchange-manager.cc | 2 +- src/wifi/model/frame-exchange-manager.cc | 6 ++-- src/wifi/model/he/rr-multi-user-scheduler.cc | 4 +-- .../model/ht/ht-frame-exchange-manager.cc | 22 +-------------- src/wifi/model/ht/ht-frame-exchange-manager.h | 1 - src/wifi/model/msdu-aggregator.cc | 23 +++++++-------- src/wifi/model/msdu-aggregator.h | 6 ++-- src/wifi/model/qos-txop.cc | 4 +-- src/wifi/model/wifi-mac-queue-item.cc | 12 ++++---- src/wifi/model/wifi-mac-queue-item.h | 14 +++++----- src/wifi/model/wifi-mac-queue.cc | 28 +++++++++++++++---- src/wifi/model/wifi-mac-queue.h | 10 +++++++ src/wifi/test/wifi-aggregation-test.cc | 2 +- 13 files changed, 71 insertions(+), 63 deletions(-) diff --git a/src/wave/model/wave-frame-exchange-manager.cc b/src/wave/model/wave-frame-exchange-manager.cc index 723a15411..7f69c37a3 100644 --- a/src/wave/model/wave-frame-exchange-manager.cc +++ b/src/wave/model/wave-frame-exchange-manager.cc @@ -133,7 +133,7 @@ WaveFrameExchangeManager::StartTransmission (Ptr dcf) } m_dcf->NotifyChannelAccessed (); - Ptr mpdu = *queue->Peek ()->GetQueueIteratorPairs ().front ().it; + Ptr mpdu = *queue->Peek ()->GetQueueIteratorPair ().it; NS_ASSERT (mpdu != 0); // assign a sequence number if this is not a fragment nor a retransmission diff --git a/src/wifi/model/frame-exchange-manager.cc b/src/wifi/model/frame-exchange-manager.cc index 54ee7c016..9dec2030d 100644 --- a/src/wifi/model/frame-exchange-manager.cc +++ b/src/wifi/model/frame-exchange-manager.cc @@ -281,7 +281,7 @@ FrameExchangeManager::StartTransmission (Ptr dcf) } m_dcf->NotifyChannelAccessed (); - Ptr mpdu = *queue->Peek ()->GetQueueIteratorPairs ().front ().it; + Ptr mpdu = *queue->Peek ()->GetQueueIteratorPair ().it; NS_ASSERT (mpdu != 0); NS_ASSERT (mpdu->GetHeader ().IsData () || mpdu->GetHeader ().IsMgt ()); @@ -328,7 +328,7 @@ FrameExchangeManager::GetFirstFragmentIfNeeded (Ptr mpdu) NS_LOG_DEBUG ("Fragmenting the MSDU"); m_fragmentedPacket = mpdu->GetPacket ()->Copy (); // dequeue the MSDU - WifiMacQueueItem::QueueIteratorPair queueIt = mpdu->GetQueueIteratorPairs ().front (); + WifiMacQueueItem::QueueIteratorPair queueIt = mpdu->GetQueueIteratorPair (); queueIt.queue->Dequeue (queueIt.it); // create the first fragment mpdu->GetHeader ().SetMoreFragments (); @@ -449,7 +449,7 @@ FrameExchangeManager::DequeueMpdu (Ptr mpdu) if (mpdu->IsQueued ()) { - WifiMacQueueItem::QueueIteratorPair queueIt = mpdu->GetQueueIteratorPairs ().front (); + const WifiMacQueueItem::QueueIteratorPair& queueIt = mpdu->GetQueueIteratorPair (); NS_ASSERT (*queueIt.it == mpdu); queueIt.queue->Dequeue (queueIt.it); } diff --git a/src/wifi/model/he/rr-multi-user-scheduler.cc b/src/wifi/model/he/rr-multi-user-scheduler.cc index 8b2b47608..b64e0ebe2 100644 --- a/src/wifi/model/he/rr-multi-user-scheduler.cc +++ b/src/wifi/model/he/rr-multi-user-scheduler.cc @@ -728,7 +728,7 @@ RrMultiUserScheduler::ComputeDlMuInfo (void) NS_ASSERT (receiver == candidate.first->address); NS_ASSERT (mpdu->IsQueued ()); - WifiMacQueueItem::QueueIteratorPair queueIt = mpdu->GetQueueIteratorPairs ().front (); + WifiMacQueueItem::QueueIteratorPair queueIt = mpdu->GetQueueIteratorPair (); NS_ASSERT (queueIt.queue != nullptr); Ptr item = *queueIt.it; queueIt.it++; @@ -742,7 +742,7 @@ RrMultiUserScheduler::ComputeDlMuInfo (void) if (item == nullptr) { // A-MSDU aggregation failed or disabled - item = *mpdu->GetQueueIteratorPairs ().front ().it; + item = *mpdu->GetQueueIteratorPair ().it; } m_apMac->GetQosTxop (QosUtilsMapTidToAc (tid))->AssignSequenceNumber (item); } diff --git a/src/wifi/model/ht/ht-frame-exchange-manager.cc b/src/wifi/model/ht/ht-frame-exchange-manager.cc index 7da726f67..fba1ef094 100644 --- a/src/wifi/model/ht/ht-frame-exchange-manager.cc +++ b/src/wifi/model/ht/ht-frame-exchange-manager.cc @@ -897,12 +897,6 @@ HtFrameExchangeManager::NotifyTxToEdca (Ptr psdu) const } } -void -HtFrameExchangeManager::DequeueMpdu (Ptr mpdu) -{ - DequeuePsdu (Create (mpdu, true)); -} - void HtFrameExchangeManager::DequeuePsdu (Ptr psdu) { @@ -910,21 +904,7 @@ HtFrameExchangeManager::DequeuePsdu (Ptr psdu) for (const auto& mpdu : *PeekPointer (psdu)) { - if (mpdu->GetQueueIteratorPairs ().size () > 1) - { - // this MPDU contains an A-MSDU - for (const auto& queueIt : mpdu->GetQueueIteratorPairs ()) - { - NS_ASSERT (*queueIt.it != mpdu); - queueIt.queue->Dequeue (queueIt.it); - } - } - else if (mpdu->IsQueued ()) - { - WifiMacQueueItem::QueueIteratorPair queueIt = mpdu->GetQueueIteratorPairs ().front (); - NS_ASSERT (*queueIt.it == mpdu); - queueIt.queue->Dequeue (queueIt.it); - } + DequeueMpdu (mpdu); } } diff --git a/src/wifi/model/ht/ht-frame-exchange-manager.h b/src/wifi/model/ht/ht-frame-exchange-manager.h index 1f6ec2fc5..650637dde 100644 --- a/src/wifi/model/ht/ht-frame-exchange-manager.h +++ b/src/wifi/model/ht/ht-frame-exchange-manager.h @@ -212,7 +212,6 @@ protected: void ForwardMpduDown (Ptr mpdu, WifiTxVector& txVector) override; void CtsTimeout (Ptr rts, const WifiTxVector& txVector) override; void TransmissionSucceeded (void) override; - void DequeueMpdu (Ptr mpdu) override; /** * Get a PSDU containing the given MPDU diff --git a/src/wifi/model/msdu-aggregator.cc b/src/wifi/model/msdu-aggregator.cc index 4feb8a181..42cc52b97 100644 --- a/src/wifi/model/msdu-aggregator.cc +++ b/src/wifi/model/msdu-aggregator.cc @@ -88,10 +88,9 @@ MsduAggregator::GetNextAmsdu (Ptr peekedItem, WifiTxPara { NS_LOG_FUNCTION (this << *peekedItem << &txParams << availableTime); - NS_ASSERT (peekedItem->IsQueued ()); - NS_ASSERT (peekedItem->GetQueueIteratorPairs ().size () == 1); - WifiMacQueueItem::QueueIteratorPair peekedIt = peekedItem->GetQueueIteratorPairs ().front (); - NS_ASSERT ((*peekedIt.it)->GetPacket () == peekedItem->GetPacket ()); + WifiMacQueue* queue = peekedItem->GetQueueIteratorPair ().queue; + WifiMacQueue::ConstIterator it = peekedItem->GetQueueIteratorPair ().it; + NS_ASSERT ((*it)->GetPacket () == peekedItem->GetPacket ()); uint8_t tid = peekedItem->GetHeader ().GetQosTid (); Mac48Address recipient = peekedItem->GetHeader ().GetAddr1 (); @@ -120,16 +119,18 @@ MsduAggregator::GetNextAmsdu (Ptr peekedItem, WifiTxPara return nullptr; } - Ptr amsdu = Copy (peekedItem); + Ptr amsdu = *it; // amsdu points to the peeked MPDU, but it's non-const uint8_t nMsdu = 1; - peekedIt.it++; + it++; - while ((peekedIt.it = peekedIt.queue->PeekByTidAndAddress (tid, recipient, peekedIt.it)) != peekedIt.queue->end () - && m_htFem->TryAggregateMsdu (*peekedIt.it, txParams, availableTime)) + while ((it = queue->PeekByTidAndAddress (tid, recipient, it)) != queue->end () + && m_htFem->TryAggregateMsdu (*it, txParams, availableTime)) { - amsdu->Aggregate (*peekedIt.it); - peekedIt.it++; + // 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); nMsdu++; } @@ -140,7 +141,7 @@ MsduAggregator::GetNextAmsdu (Ptr peekedItem, WifiTxPara } // Aggregation succeeded - queueIt = peekedIt; + queueIt = {queue, it}; return amsdu; } diff --git a/src/wifi/model/msdu-aggregator.h b/src/wifi/model/msdu-aggregator.h index 38a167348..92419896f 100644 --- a/src/wifi/model/msdu-aggregator.h +++ b/src/wifi/model/msdu-aggregator.h @@ -88,8 +88,10 @@ public: * acknowledgment, as specified by the given TX parameters, does not exceed the * given available time (if distinct from Time::Min ()) * - * If it is not possible to aggregate at least two MSDUs, no MSDU is dequeued - * from the EDCA queue and a null pointer is returned. + * If aggregation succeeds (it was possible to aggregate at least an MSDU to the + * given MSDU), all the aggregated MSDUs are dequeued and an MPDU containing the + * A-MSDU is enqueued in the queue (replacing the given MPDU) and returned. + * Otherwise, no MSDU is dequeued from the EDCA queue and a null pointer is returned. * * \param peekedItem the MSDU which we attempt to aggregate other MSDUs to * \param txParams the TX parameters for the current frame diff --git a/src/wifi/model/qos-txop.cc b/src/wifi/model/qos-txop.cc index 2868230e7..5c4bf4aea 100644 --- a/src/wifi/model/qos-txop.cc +++ b/src/wifi/model/qos-txop.cc @@ -453,9 +453,7 @@ QosTxop::GetNextMpdu (Ptr peekedItem, WifiTxParameters& } NS_ASSERT (peekedItem->IsQueued ()); - NS_ASSERT_MSG (peekedItem->GetQueueIteratorPairs ().size () == 1, - "An item in the MAC queue cannot contain an A-MSDU"); - WifiMacQueueItem::QueueIteratorPair peekedIt = peekedItem->GetQueueIteratorPairs ().front (); + WifiMacQueueItem::QueueIteratorPair peekedIt = peekedItem->GetQueueIteratorPair (); NS_ASSERT ((*peekedIt.it)->GetPacket () == peekedItem->GetPacket ()); if (peekedIt.queue == PeekPointer (m_baManager->GetRetransmitQueue ())) diff --git a/src/wifi/model/wifi-mac-queue-item.cc b/src/wifi/model/wifi-mac-queue-item.cc index 95f2a8932..54a9e7505 100644 --- a/src/wifi/model/wifi-mac-queue-item.cc +++ b/src/wifi/model/wifi-mac-queue-item.cc @@ -47,6 +47,7 @@ WifiMacQueueItem::WifiMacQueueItem (Ptr p, const WifiMacHeader & h { m_msduList = MsduAggregator::Deaggregate (p->Copy ()); } + m_queueIt.queue = nullptr; } WifiMacQueueItem::~WifiMacQueueItem () @@ -123,7 +124,6 @@ WifiMacQueueItem::Aggregate (Ptr msdu) // An MSDU is going to be aggregated to this MPDU, hence this has to be an A-MSDU now Ptr firstMsdu = Create (*this); m_packet = Create (); - m_queueIts.clear (); DoAggregate (firstMsdu); m_header.SetQosAmsdu (); @@ -170,7 +170,6 @@ WifiMacQueueItem::DoAggregate (Ptr msdu) hdr.SetLength (static_cast (msdu->GetPacket ()->GetSize ())); m_msduList.push_back ({msdu->GetPacket (), hdr}); - m_queueIts.insert (m_queueIts.end (), msdu->m_queueIts.begin (), msdu->m_queueIts.end ()); // build the A-MSDU NS_ASSERT (m_packet); @@ -204,13 +203,14 @@ WifiMacQueueItem::DoAggregate (Ptr msdu) bool WifiMacQueueItem::IsQueued (void) const { - return !m_queueIts.empty (); + return m_queueIt.queue != nullptr; } -const std::list& -WifiMacQueueItem::GetQueueIteratorPairs (void) const +const WifiMacQueueItem::QueueIteratorPair& +WifiMacQueueItem::GetQueueIteratorPair (void) const { - return m_queueIts; + NS_ASSERT (IsQueued ()); + return m_queueIt; } WifiMacQueueItem::DeaggregatedMsdusCI diff --git a/src/wifi/model/wifi-mac-queue-item.h b/src/wifi/model/wifi-mac-queue-item.h index 221f4c078..13f001827 100644 --- a/src/wifi/model/wifi-mac-queue-item.h +++ b/src/wifi/model/wifi-mac-queue-item.h @@ -157,14 +157,14 @@ public: bool IsQueued (void) const; /** - * Get a const reference to the list of iterators pointing to the positions - * of the items in the queue. The list is empty if the item is not stored in - * a queue. The list contains multiple iterators in case of A-MSDU that is not - * stored in the Block Ack Manager retransmit queue. + * Get a const reference to the QueueIteratorPair struct variable containing + * a pointer to the queue where the MPDU is stored and an iterator pointing to + * the position of the MPDU in the queue. This method should not be called if + * the MPDU is not stored in a queue. * - * \return the list of iterators pointing to the positions of the items in the queue + * \return an iterator pointing to the position of the MPDU in the queue */ - const std::list& GetQueueIteratorPairs (void) const; + const QueueIteratorPair& GetQueueIteratorPair (void) const; /** * \brief Get the MAC protocol data unit (MPDU) corresponding to this item @@ -195,7 +195,7 @@ private: WifiMacHeader m_header; //!< Wifi MAC header associated with the packet Time m_tstamp; //!< timestamp when the packet arrived at the queue DeaggregatedMsdus m_msduList; //!< The list of aggregated MSDUs included in this MPDU - std::list m_queueIts; //!< Queue iterators pointing to this MSDU(s), if queued + QueueIteratorPair m_queueIt; //!< Queue iterator pointing to this MPDU, if queued }; /** diff --git a/src/wifi/model/wifi-mac-queue.cc b/src/wifi/model/wifi-mac-queue.cc index e116da06a..e616c7514 100644 --- a/src/wifi/model/wifi-mac-queue.cc +++ b/src/wifi/model/wifi-mac-queue.cc @@ -634,7 +634,7 @@ WifiMacQueue::DoEnqueue (ConstIterator pos, Ptr item) m_nQueuedBytes[addressTidPair] += item->GetSize (); } // set item's information about its position in the queue - item->m_queueIts = {{this, ret}}; + item->m_queueIt = {this, ret}; return true; } return false; @@ -658,8 +658,8 @@ WifiMacQueue::DoDequeue (ConstIterator pos) if (item != 0) { - NS_ASSERT (item->m_queueIts.size () == 1); - item->m_queueIts.clear (); + NS_ASSERT (item->IsQueued ()); + item->m_queueIt.queue = nullptr; } return item; @@ -683,11 +683,29 @@ WifiMacQueue::DoRemove (ConstIterator pos) if (item != 0) { - NS_ASSERT (item->m_queueIts.size () == 1); - item->m_queueIts.clear (); + NS_ASSERT (item->IsQueued ()); + item->m_queueIt.queue = nullptr; } 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 d5553004a..29381eca3 100644 --- a/src/wifi/model/wifi-mac-queue.h +++ b/src/wifi/model/wifi-mac-queue.h @@ -260,6 +260,16 @@ 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. diff --git a/src/wifi/test/wifi-aggregation-test.cc b/src/wifi/test/wifi-aggregation-test.cc index a86f396a2..b128c8427 100644 --- a/src/wifi/test/wifi-aggregation-test.cc +++ b/src/wifi/test/wifi-aggregation-test.cc @@ -451,7 +451,7 @@ TwoLevelAggregationTest::DoRun (void) NS_TEST_EXPECT_MSG_EQ ((item == 0), true, "A-MSDU aggregation did not fail"); - htFem->DequeueMpdu (*peeked->GetQueueIteratorPairs ().front ().it); + htFem->DequeueMpdu (*peeked->GetQueueIteratorPair ().it); NS_TEST_EXPECT_MSG_EQ (m_mac->GetBEQueue ()->GetWifiMacQueue ()->GetNPackets (), 0, "queue should be empty");