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

@@ -133,7 +133,7 @@ WaveFrameExchangeManager::StartTransmission (Ptr<Txop> dcf)
     }
 
   m_dcf->NotifyChannelAccessed ();
-  Ptr<WifiMacQueueItem> mpdu = *queue->Peek ()->GetQueueIteratorPairs ().front ().it;
+  Ptr<WifiMacQueueItem> mpdu = *queue->Peek ()->GetQueueIteratorPair ().it;
   NS_ASSERT (mpdu != 0);
 
   // assign a sequence number if this is not a fragment nor a retransmission

src/wifi/model/frame-exchange-manager.cc

@@ -281,7 +281,7 @@ FrameExchangeManager::StartTransmission (Ptr<Txop> dcf)
     }
 
   m_dcf->NotifyChannelAccessed ();
-  Ptr<WifiMacQueueItem> mpdu = *queue->Peek ()->GetQueueIteratorPairs ().front ().it;
+  Ptr<WifiMacQueueItem> mpdu = *queue->Peek ()->GetQueueIteratorPair ().it;
   NS_ASSERT (mpdu != 0);
   NS_ASSERT (mpdu->GetHeader ().IsData () || mpdu->GetHeader ().IsMgt ());
 
@@ -328,7 +328,7 @@ FrameExchangeManager::GetFirstFragmentIfNeeded (Ptr<WifiMacQueueItem> 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<WifiMacQueueItem> 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);
     }

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<WifiMacQueueItem> 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);
         }

src/wifi/model/ht/ht-frame-exchange-manager.cc

@@ -897,12 +897,6 @@ HtFrameExchangeManager::NotifyTxToEdca (Ptr<const WifiPsdu> psdu) const
     }
 }
 
-void
-HtFrameExchangeManager::DequeueMpdu (Ptr<WifiMacQueueItem> mpdu)
-{
-  DequeuePsdu (Create<const WifiPsdu> (mpdu, true));
-}
-
 void
 HtFrameExchangeManager::DequeuePsdu (Ptr<const WifiPsdu> psdu)
 {
@@ -910,21 +904,7 @@ HtFrameExchangeManager::DequeuePsdu (Ptr<const WifiPsdu> 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);
     }
 }
 

src/wifi/model/ht/ht-frame-exchange-manager.h

@@ -212,7 +212,6 @@ protected:
   void ForwardMpduDown (Ptr<WifiMacQueueItem> mpdu, WifiTxVector& txVector) override;
   void CtsTimeout (Ptr<WifiMacQueueItem> rts, const WifiTxVector& txVector) override;
   void TransmissionSucceeded (void) override;
-  void DequeueMpdu (Ptr<WifiMacQueueItem> mpdu) override;
 
   /**
    * Get a PSDU containing the given MPDU

src/wifi/model/msdu-aggregator.cc

@@ -88,10 +88,9 @@ MsduAggregator::GetNextAmsdu (Ptr<const WifiMacQueueItem> 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<const WifiMacQueueItem> peekedItem, WifiTxPara
       return nullptr;
     }
 
-  Ptr<WifiMacQueueItem> amsdu = Copy (peekedItem);
+  Ptr<WifiMacQueueItem> 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<const WifiMacQueueItem> peekedItem, WifiTxPara
     }
 
   // Aggregation succeeded
-  queueIt = peekedIt;
+  queueIt = {queue, it};
 
   return amsdu;
 }

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

src/wifi/model/qos-txop.cc

@@ -453,9 +453,7 @@ QosTxop::GetNextMpdu (Ptr<const WifiMacQueueItem> 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 ()))

src/wifi/model/wifi-mac-queue-item.cc

@@ -47,6 +47,7 @@ WifiMacQueueItem::WifiMacQueueItem (Ptr<const Packet> p, const WifiMacHeader & h
     {
       m_msduList = MsduAggregator::Deaggregate (p->Copy ());
     }
+  m_queueIt.queue = nullptr;
 }
 
 WifiMacQueueItem::~WifiMacQueueItem ()
@@ -123,7 +124,6 @@ WifiMacQueueItem::Aggregate (Ptr<const WifiMacQueueItem> msdu)
       // An MSDU is going to be aggregated to this MPDU, hence this has to be an A-MSDU now
       Ptr<const WifiMacQueueItem> firstMsdu = Create<const WifiMacQueueItem> (*this);
       m_packet = Create<Packet> ();
-      m_queueIts.clear ();
       DoAggregate (firstMsdu);
 
       m_header.SetQosAmsdu ();
@@ -170,7 +170,6 @@ WifiMacQueueItem::DoAggregate (Ptr<const WifiMacQueueItem> msdu)
   hdr.SetLength (static_cast<uint16_t> (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<const WifiMacQueueItem> msdu)
 bool
 WifiMacQueueItem::IsQueued (void) const
 {
-  return !m_queueIts.empty ();
+  return m_queueIt.queue != nullptr;
 }
 
-const std::list<WifiMacQueueItem::QueueIteratorPair>&
-WifiMacQueueItem::GetQueueIteratorPairs (void) const
+const WifiMacQueueItem::QueueIteratorPair&
+WifiMacQueueItem::GetQueueIteratorPair (void) const
 {
-  return m_queueIts;
+  NS_ASSERT (IsQueued ());
+  return m_queueIt;
 }
 
 WifiMacQueueItem::DeaggregatedMsdusCI

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<QueueIteratorPair>& 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<QueueIteratorPair> m_queueIts;      //!< Queue iterators pointing to this MSDU(s), if queued
+  QueueIteratorPair m_queueIt;                  //!< Queue iterator pointing to this MPDU, if queued
 };
 
 /**

src/wifi/model/wifi-mac-queue.cc

@@ -634,7 +634,7 @@ WifiMacQueue::DoEnqueue (ConstIterator pos, Ptr<WifiMacQueueItem> 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<WifiMacQueueItem> msdu = DoDequeue (msduIt);
+
+  ConstIterator pos = std::next (amsduIt);
+  Ptr<WifiMacQueueItem> 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

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 <i>msduIt</i> iterator (source MPDU)
+   * to the (A-)MSDU pointed to by the <i>amsduIt</i> 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
    * <i>dest</i>. The complexity is linear in the size of the queue.

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");