Bug 853

Summary: Rates for Wi-Fi control responses are incorrectly selected
Product: ns-3 Reporter: Dean Armstrong <deanarm>
Component: wifiAssignee: Dean Armstrong <deanarm>
Status: RESOLVED FIXED    
Severity: normal CC: deanarm, nicola, ns-bugs
Priority: P3 Keywords: bug
Version: ns-3-dev   
Hardware: All   
OS: All   
URL: http://codereview.appspot.com/855042

Description Dean Armstrong 2010-03-31 13:05:51 UTC
WifiRemoteStationManager::GetControlAnswerMode() contains the code responsible for selecting the PHY rate at which an automatic control response will be transmitted in response to receipt of a frame at a given rate.

Such selection should be performed in accordance with Section 9.6 of IEEE 802.11-2007 (or an earlier version - this text hasn't changed much).

This bit of the standard basically says (not quite verbatim) that the control response should be transmitted at the highest rate in the basic rate set that is less than or equal to the rate of the received frame and that is of the same modulation class and, if no suitable basic rate is found, that the response should be transmitted at the highest mandatory rate of the PHY that is less than or equal to the rate of the received frame and that is of the same modulation class.

There are two small (small enough that I thought a single bug was sufficient) problems with the current code in WifiRemoteStationManager::GetControlAnswerMode()...

Firstly, the current attempt to select a rate "of the same modulation class" is in fact testing the rate ModulationType. This is not correct - the modulation classes are defined in Section 9.6.1 and Table 9.2 of IEEE 802.11-2007, and (for example in contrast with the current approach) lump Clause 15 and Clause 18 rates together into one class, and Clause 17 rates together in another. The fix for this is probably to compare the WifiPhyStandard instead.

The second shortcoming is that the code deals with the search within the basic rate set, but doesn't fall back to consideration of mandatory rates. There is a comment there that says such a search is unnecessary because the basic rate set includes the mandatory rates, but this is incorrect.

A prime counter-example is the case of many "802.11g" APs currently available in their out-of-the-box configuration. These APs obviously support the PHYs described in IEEE 802.11 Clauses 15, 18, and 19, and thus the mandatory rates of 1, 2, 5.5, 6, 11, 12, and 24 Mbps, however their basic rate sets typically will comprise only the "802.11b" rates of 1, 2, 5.5, and 11 Mbps so as not to preclude an 802.11b-only STA from joining the BSS.
Comment 1 Dean Armstrong 2010-03-31 13:20:20 UTC
I've uploaded to codereview a proposed change to address this bug. It is here:

http://codereview.appspot.com/855042

This change causes some tests to fail - specifically, the wireless/wifi-simple-adhoc-grid example, and the routing-olsr-regression testsuite.

In this case it is actually the tests that are at fault (or, at least, need updating). In the wifi-simple-adhoc-grid test and the bug780-test part of the routing-olsr-regression testsuite, 802.11b rates were being requested with the Wi-Fi PHY standard left at the default of 802.11a. The fix here is to set the standard to 802.11b in the test.

In the tx-regression-test part of routing-olsr-regression, the problem was that the reference traces needed to be updated given the control response rate fix. In this case I have also modified the test to set the standard to 802.11a for clarity.

I have addressed these test issues in a separate changelist which is available for your perusal here:

http://codereview.appspot.com/861041
Comment 2 Nicola Baldo 2010-04-12 12:46:41 UTC
Thank you very much for the detailed bug report and for the patches!

I confirm this bug, the current code misinterprets the "modulation" (BPSK, QPSK, 16QAM) for the modulation class defined by section 9.6.1 in IEEE
Std. 802.11-2007. 

I agree that your proposed fix works, the only thing I don't like is the introduction of the method WifiRemoteStationManager::GetPhy
I would wait until we fix 871, to see if we can come with a modified patch that does not need this method.

I am also fine with the changes to the tests and the regression traces, of course to be applied after we have a version of the patch that we agree upon.
Comment 3 Nicola Baldo 2010-04-12 12:47:22 UTC
(In reply to comment #2)
> I would wait until we fix 871

I mean bug 871
Comment 4 Dean Armstrong 2010-06-23 04:20:22 UTC
Fixed in changeset 6372:0fafd9716f44 on ns-3-dev. Exposed test issues and reference trace updates addressed in changeset 6373:5467d6ae6ebe.

Hence marking resolved fixed.