Skip to content

Wifi MSDU and MPDU aggregation rework

Stefano Avallone requested to merge stavallo/ns-3-dev:wifi-fixes into master

This series of commits aim to make the wifi code related to MSDU and MPDU aggregation more readable and suitable for future extensions. Notable changes are the introduction of two methods (MsduAggregator::GetNextAmsdu and MpduAggregator::GetNextAmpdu) that are in charge of performing aggregation and the removal of the very complicated MacLow::AggregateToMpdu. Most of the commits are just preparatory work; wifi: Rework MSDU and MPDU aggregation introduces the two aggregation methods (though they are not used yet) while wifi: Make use of the new MSDU and MPDU aggregators introduces the needed changes to use the new aggregation methods. The first commit is a proposed fix for issue #23 (closed) . Some comments:

  • WifiPhy::GetPpduMaxTime always returns 10ms if the modulation class is HT (according to table 19-25 of 802.11-2016). This is different than before (depending on the frame format, it could be 5.484ms), hence the throughput achieved in the HT case is now a bit higher. This causes an example to fail because the maximum expected throughput is exceeded.

  • All the other tests and examples pass. Examples with HE stations return the exact same throughput as before

  • The meaning of the {Be,Bk,Vi,Vo}MaxAmsduSize attributes shall be discussed. The {Be,Bk,Vi,Vo}MaxAmpduSize attributes may be replaced by a single attribute because, if in the future we will support multi-TID A-MPDU, it will make little sense to define a per-TID maximum A-MPDU size

  • MacLow::GetMaxAmpduSize could be removed but it is used by some QosTxop methods. A discussion is needed to agree on the best way to provide such methods with the modulation class of the frame being transmitted, so that the correct maximum A-MPDU size can be determined

  • From very quick tests, this cleanup resulted in a 5% decrease of the running time

Version 2

  • WifiPhy::GetPpduMaxTime receives the preamble type, so it can distinguish between HT_MF and HT_GF and return the correct max PPDU duration (thanks @rediet for the suggestion). Now, all the HT examples, too, return the same throughput as before and all the examples and tests pass.

  • rebased on the current master, thus the commits to fix issue !23 (closed) are no longer part of this series

Version 3

  • Addressed some comments by Sebastien and Rediet

Version 4

  • Rebased on current master

Version 6 (disregard version 5)

  • Rebased on current master
  • Make MpduAggregator::GetMaxAmpduSize dependent on TID (as per Sebastien's comment)

Version 7

  • No need for MacLow::StartTransmission to add the A-MPDU tag (it is added by MacLow::ForwardDown)

Version 8

  • Adding header and trailer to the payload is performed by MpduAggregator::Aggregate
  • Replace M{s,p}du::CanBeAggregated with M{s,p}du::GetSizeIfAggregated, which simply returns the size of the A-M{S,P}DU resulting from the aggregation (checking if the maximum size is exceeded can be done by the caller)

Version 9

  • Fix logic when A-MPDU aggregation is disabled
  • Rebased on current master

Version 10

  • BlockAckManager::GetNextPacket returns a WifiMacQueueItem

Version 11

  • Rebased on current master

Version 12

  • Add the std::ostream operator<< to print the content of a WifiMacQueueItem

Version 13

  • Try A-MPDU aggregation only for non-broadcast QoS Data frames
  • The size returned by WifiMacQueueItem::GetSize includes the size of the trailer, too

Version 14

  • Remove max A-MSDU and A-MPDU size attributes from Configuration objects
  • Address comments by Sebastien and Rediet

Version 15

  • Rebase on current master

Version 16

  • Remove the remaining calls to MacLow::GetMaxAmpduSize ()
  • Remove the unneeded variant of M{s,p}duAggregator::GetSizeIfAggregated ()

Version 17

  • Check if fragmentation is needed before attempting A-MSDU aggregation

Version 18

  • Merge the change to check fragmentation before A-MSDU aggregation into the commit introducing the change
  • Fix Valgrind error "conditional jump or move depends on uninitialized value(s)"
Edited by Stefano Avallone

Merge request reports