Bug 2757

Summary:	802.11n/ac/ax maximum TXOP is not properly enforced
Product:	ns-3	Reporter:	Tom Henderson <tomh>
Component:	wifi	Assignee:	sebastien.deronne
Status:	RESOLVED FIXED
Severity:	minor	CC:	ns-bugs, selinis.g
Priority:	P3
Version:	pre-release
Hardware:	All
OS:	All
Attachments:	patch

Description Tom Henderson 2017-06-19 23:05:25 UTC

Many sources cite 5.424ms as the maximum TXOP duration for 11n/ac; e.g.:
http://chimera.labs.oreilly.com/books/1234000001739/ch03.html#section-mac-agg

This is about 36000 bytes at 54 Mb/s rate, and 4407 bytes at 6.5 Mb/s rate.

However, I am observing 8ms AMPDU durations when running 
'wifi-aggregation' example with this simple patch:

diff -r 98ce80ed995f src/wifi/model/mac-low.cc
--- a/src/wifi/model/mac-low.cc	Fri Jun 02 17:13:54 2017 -0700
+++ b/src/wifi/model/mac-low.cc	Sat Jun 03 11:16:27 2017 -0700
@@ -1524,6 +1524,7 @@
        ampdutag.SetAmpdu (true);
        Time delay = Seconds (0);
        Time remainingAmpduDuration = m_phy->CalculateTxDuration 
(packet->GetSize (), txVector, m_phy->GetFrequency ());
+      std::cout << "Remaining " << remainingAmpduDuration.GetSeconds () 
<< std::endl;
        if (queueSize > 1 || singleMpdu)
          {
            txVector.SetAggregation (true);


The enforcement is done in MacLow::StopMpduAggregation:

  if (m_currentTxVector.GetMode ().GetModulationClass () == WIFI_MOD_CLASS_VHT)
    {
      aPPDUMaxTime = MicroSeconds (5484);
    }

but the test condition appears to be wrong (limited to VHT).

Comment 1 sebastien.deronne 2017-06-21 13:24:45 UTC

Tom, you forgot to mention you also need to set the standard to 802.11ac:

-  wifi.SetStandard (WIFI_PHY_STANDARD_80211n_5GHZ);
+  wifi.SetStandard (WIFI_PHY_STANDARD_80211ac);

I checked and indeed the condition should become:

-  if (m_currentTxVector.GetMode ().GetModulationClass () == WIFI_MOD_CLASS_VHT)
+  if (m_stationManager->HasVhtSupported ())
     {
       aPPDUMaxTime = MicroSeconds (5484);
     }

With this change I do not go over 5.484ms.
If fine for you, I will push the patch.

Comment 2 sebastien.deronne 2017-06-21 13:29:22 UTC

I also see you mention 802.11n in the description, this is only for 802.11ac IMO.
Please confirm :-)

Comment 3 Ioannis 2017-06-22 06:53:01 UTC

I think that 11n should be also included. Based on IEEE-802.11ac (Table 8-13c)
maximum aPPDUMaxTime duration for 11n/ac is 5484 microseconds, except if Greenfield for 11n is supported. In that case aPPDUMaxTime is 10000 microseconds.

So, I guess it should be something like:

- Time aPPDUMaxTime = MilliSeconds (10);
+ Time aPPDUMaxTime = MicroSeconds (5484);

+ if(m_phy->GetGreenfield ())
+   aPPDUMaxTime = MilliSeconds(10); // 10 Milliseconds only for GF (IEEE-802.11ac (Table 8-13c))

Comment 4 Tom Henderson 2017-06-22 12:19:01 UTC

(In reply to Ioannis from comment #3)
> I think that 11n should be also included. Based on IEEE-802.11ac (Table
> 8-13c)
> maximum aPPDUMaxTime duration for 11n/ac is 5484 microseconds, except if
> Greenfield for 11n is supported. In that case aPPDUMaxTime is 10000
> microseconds.
> 
> So, I guess it should be something like:
> 
> - Time aPPDUMaxTime = MilliSeconds (10);
> + Time aPPDUMaxTime = MicroSeconds (5484);
> 
> + if(m_phy->GetGreenfield ())
> +   aPPDUMaxTime = MilliSeconds(10); // 10 Milliseconds only for GF
> (IEEE-802.11ac (Table 8-13c))

I agree with Ioannis that 11n should be included.  My only small suggestion is to change MilliSeconds (10) to MicroSeconds (10000), for future searchability of this value.

Comment 5 sebastien.deronne 2017-06-23 07:29:04 UTC

OK, I had missed the 802.11n case in the provided link, I agree with the changes and I will prepare a final patch. Any idea what it should be for 802.11ax?

Comment 6 Ioannis 2017-06-23 08:08:17 UTC

(In reply to sebastien.deronne from comment #5)
> OK, I had missed the 802.11n case in the provided link, I agree with the
> changes and I will prepare a final patch. Any idea what it should be for
> 802.11ax?

Based on the latest draft, 11ax will use the same value as 11ac (5484 Microseconds)

Comment 7 sebastien.deronne 2017-06-24 08:21:12 UTC

Created attachment 2872 [details]
patch

Here is the final patch, I will push it soon so that it can be part of the next release.

Comment 8 Tom Henderson 2017-06-26 10:29:01 UTC

(In reply to sebastien.deronne from comment #7)
> Created attachment 2872 [details]
> patch
> 
> Here is the final patch, I will push it soon so that it can be part of the
> next release.

Works for me.

Comment 9 sebastien.deronne 2017-06-26 16:54:39 UTC

fixed in changeset 12943:05b8f847754d