Bug 2097 - ACKs should be sent using legacy rates and preambles in 802.11n
ACKs should be sent using legacy rates and preambles in 802.11n
Status: RESOLVED FIXED
Product: ns-3
Classification: Unclassified
Component: wifi
ns-3-dev
All All
: P5 normal
Assigned To: sebastien.deronne
:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2015-04-15 15:27 EDT by sebastien.deronne
Modified: 2015-05-27 16:41 EDT (History)
3 users (show)

See Also:


Attachments
patch to follow the standard rules for rate selection for control frames (76.86 KB, patch)
2015-05-01 16:23 EDT, sebastien.deronne
Details | Diff
new patch based on Ghada's comments (44.94 KB, patch)
2015-05-05 16:47 EDT, sebastien.deronne
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description sebastien.deronne 2015-04-15 15:27:14 EDT
CC: discussions around ACK and CTS durations in 802.11 at https://www.nsnam.org/bugzilla/show_bug.cgi?id=2004

The standard says that ack should be sent using legacy rates and preambles.
There is however still clarifications to be brought around RTS/CTS:

Ghada: "The same should be done for CTS but CTS can be sent using HT PPDU if it is in response to an RTS that is sent using HT PPDU. I didn't read yet under what conditions can an RTS be sent using HT PPDU to decide if we should keep it as is or change it as Ack"

Ghada: "As for CTS I think the best is to check the standard and see what it says not just send it as Ack. If ns3 supports features that RTS/CTS can be sent using HT PPDU then we should add some logic (probably in mac-low.cc) to do this selection, if not then it can be sent like Ack in a non HT PPDU.
Note that if CTS can be sent using HT PPDU then we need to update CTSTimeout in configure11n since MF preamble is longer than legacy (11a and 11g)"
Comment 1 gbadawy 2015-04-15 15:32:48 EDT
I just checked and again ns3 doesn't support any of the conditions where RTS can be sent as an HT PPDU yet. So I think it is safe to update mac-low.cc to send RTS/CTS and Ack using preamble LONG
Comment 2 sebastien.deronne 2015-05-01 16:23:03 EDT
Created attachment 2033 [details]
patch to follow the standard rules for rate selection for control frames

I went through this issue and I attached a patch to fix it.

Some other related issues popped up and have been corrected as well in this patch:
-> add basic rates which will be used to send control frames
-> make a distinction between BSS basic rate set and mandatory rates
-> make clear ControlMode attribute in ConstantRateWifiManager is for RTS transmissions
-> use TXVECTOR instead of TX MODE (and avoid double use)

Any comment is welcome!
Comment 3 gbadawy 2015-05-04 16:04:31 EDT
Hi Sebastien,
BSSBasicRateSet is defined for each AP. It is not part of the WifiMode. Mandatory rates are by definition part of the BSSBasicRateSet but other modes can be added. I don't see a need to change this part of NS3. NS3 adds mandatory modes by default to the BSSBasicRateSet which is the correct behavior and if a user wants to add more rate then they can do that in their script using 
wifiRemoteStation->AddBasicMode(mode).
Comment 4 sebastien.deronne 2015-05-04 16:11:52 EDT
This came because it seems BSSBasicRateSet for 802.11b only includes 1 Mbit/s, while 1, 2, 5.5 and 11 Mbit/s are all mandatory rates. Is this wrong?
Comment 5 gbadawy 2015-05-05 10:28:32 EDT
The rates inside a BSSBasicRateSet are not defined in the standard they are implementation dependent. This set is used by the AP to make sure all associated station can send/receive packets using these rates. it is  also used for rate selection but it can also be empty. Here is a quote from the standard "the transmitting STA shall transmit using a rate in the BSSBasicRateSet parameter, or an MCS in the BSSBasicMCSSet parametera rate from the mandatory rate set of the attached PHY if both the BSSBasicRateSet and the BSSBasicMCSSet are empty"

So there is no right and wrong common practice is that the BSSBasicRateSet contains the mandatory phy rates but it can contain more or less

Your patch might solve the issue we have on hand now but it is a quick solution. I propose we update rate selection to match what is explained in the standard (i.e. put if conditions in the function to check for conditions as defined in the standard right now all will be false so the rate selection for ack will fall to non-HT but later when we add more options the conditions will be there and ack rate selection will be correct) in that way when we extend the wifi module to include more options (like MIMO) building block will already be there and we don't have to re-implement. That's just my 2 cents :)
Comment 6 sebastien.deronne 2015-05-05 10:36:43 EDT
Ghada, thanks for your comment.

I think you are right about the BSSBasicRateSet.
But in ns-3, which rate should be added if it's implementation dependent? Do you have a reference on what is the common practice?

The patch is indeed a quick solution in the sense it will always use the non-HT format, which is not fully standard compliant.
But since HT format cases are not (yet) supported, it should be enough and at least it solves this issue :-)

You are mentioning a if condition somewhere, which kind of checks are missing and where? I do not really get your explanation on this
Comment 7 gbadawy 2015-05-05 11:06:01 EDT
I think in NS3 we should add all or some mandatory phy rates. In reality I have some 11n AP sniffs that only has 9 (not mandatory),12 and 24Mb/s as basic rates. It affects association so we just need to make sure that if the user didn't change anything in a script default AP and STA could associate and they perform as expected (i.e. control frames pick from basic rates so if we only include 1Mb/s throughput will be affected a lot)

I meant we should follow sections "9.7.6.5.2 Selection of a rate or MCS and 9.7.6.5.3 Control response frame MCS computation" in the standard. And if the BssBasicMcsSet code piece is correct it shouldn't be deleted the code should be left there but an if condition can be added so that it is not used now and it gets used when the correct conditions are implemented (i.e. if modulation==HT then use BssBasicMcsSet part else use use BssBasicRateSet part)
Comment 8 sebastien.deronne 2015-05-05 11:42:03 EDT
Ghada, why are you mentioning 1 Mbit/s?
In 802.11b, it is the current rate used for control frames, which is correct behavior.

if modulation==HT seems not correct as a condition, since a HT data frame may be followed by a non-HT control frame, and it's indeed the purpose of the patch here, since we do not support yet cases where control frames are sent with a HT format. But I agree I should not delete the code you had added at the time (i.e. those starting with if htSupported). But it miss a condition to go into those cases (don't see which one for the moment), which should be fulfilled in future improvements (MIMO, ...).
Comment 9 gbadawy 2015-05-05 12:06:35 EDT
1 Mb/s was just an example. Didn't mean anything with it

Yes you are correct I meant by modulation == HT the ack modulation not the previous data modulation. But you get my point :)
Comment 10 sebastien.deronne 2015-05-05 12:08:37 EDT
Ok, I will propose a new patch in few hours.
Comment 11 sebastien.deronne 2015-05-05 14:41:45 EDT
At the association, the BSSBasicRateSet is empty, and it thus selects MCS0.
Is this really expected? 
Or should it use the lowest legacy rate, i.e. 6 Mbit/s (@ 5GHz)? 

I would say the second case.
If I am correct, I thus propose to change this condition:

     thismode.GetPhyRate () > mode.GetPhyRate () 
  => thismode.GetPhyRate () >= mode.GetPhyRate ()
Comment 12 sebastien.deronne 2015-05-05 15:45:44 EDT
Forget my last comment about the proposed changes, the issue was due to the fact that even though the rate was found, it was continuing in the function and searched for a higher rate and got MCS 0 (6.5 > 6).
So, since we do not yet support HT rates for control frames, if it has been found in the legacy rates, it should not search for a MCS.

I also notice that the BSSBasicRateSet is never set (even in 802.11a!), which means that it always use the lowest mandatory rate (6 Mbit/s in 802.11a).

In most implementations, 6-12-24 Mbit/s are added to the BSSBasicRateSet, and it selects the highest rate which is smaller or equal to the previous rate.

Those changes in ap-wifi-mac enable to set all mandatory rates in the BSSBasicRateSet:
if(mode.IsMandatory())
{
  m_stationManager->AddBasicMode (mode);
}

The only issue is for 802.11b, which have al its supported rate as mandatory, but generally only 1 Mbit/s is added to the BSSBasicRateSet.
Ghada, do you have a proposal here?
Comment 13 sebastien.deronne 2015-05-05 16:47:51 EDT
Created attachment 2037 [details]
new patch based on Ghada's comments

Here's the changes compared to previous patch:
-> I have not removed the selection of HT rate for control frame, but I changed the condition so that it cannot be used for now (since cases where ht format should be used are not yet supported)
-> AP adds the mandatory rates in BSSBasicRateSet (6, 12, 24), and in 802.11b we decide to select the two lowest rate as part of the BSSBasicRateSet (1, 2) as seen in most implementations.
Comment 14 gbadawy 2015-05-06 08:58:33 EDT
The new patch looks good to me.
Comment 15 sebastien.deronne 2015-05-06 09:02:10 EDT
Ghada, thanks.
It will be committed later today together with fix for bug 2004.
Comment 16 sebastien.deronne 2015-05-06 16:29:48 EDT
committed in changeset 11384:935dc1cc08a4
Comment 17 gbadawy 2015-05-21 15:55:55 EDT
The current implementation doesn't allow the user to add HT rates to the BSSBasicRateSet. When that happens the Ack is sent using the HT rate and that fires an ASSERT in mac-low.cc. 
Here is how the standard deals with this situation:
"If a CTS or ACK control response frame is carried in a non-HT PPDU, the primary rate is defined to be the highest rate in the BSSBasicRateSet parameter that is less than or equal to the rate (or non-HT reference rate; see 9.7.9) of the previous frame"

GetControlAnswerMode should be updated to use the non-HT reference rate when the BSSBasicRateSet has HT rates and the ack should be sent using non-HT PPDU.
Comment 18 sebastien.deronne 2015-05-21 16:20:10 EDT
Ghada, I don't get your comment, HT rates are only in the BSSBasicMCSSet, not in the BSSBasicRateSet.
Comment 19 gbadawy 2015-05-25 13:23:24 EDT
Sebastien, you are right so in this case should we generate an error when a user tries to add an HT rate to the BssBasicRateSet? Because right now the user is allowed to do that but it will result in an assert fail in mac-low because the ACK rate will be HT. 
I changed the status to resolved since the ack rate selection has nothing wrong with it
Comment 20 sebastien.deronne 2015-05-25 15:49:26 EDT
Yes, we should forbid that a user tries to add an HT rate to the BssBasicRateSet.
Comment 21 sebastien.deronne 2015-05-27 07:16:52 EDT
The patch triggering an error when a user add an HT rate in the BSSBasicRateSet is committed in changeset 11419:d118c95b966c.
Comment 22 sebastien.deronne 2015-05-27 16:41:09 EDT
Minor issues have been found in the patch, mainly related to the use of legacy long preambles even for HT data frame transmissions.

A postfix for this bug is committed in changeset 11420:ef4dc0386e71.