Bug 2877

Summary: Wrong data types for CWmin and CWmax
Product: ns-3 Reporter: Won Hyoung Lee <circlelee92>
Component: wifiAssignee: sebastien.deronne
Status: RESOLVED FIXED    
Severity: normal CC: ns-bugs, tomh
Priority: P3    
Version: ns-3.27   
Hardware: All   
OS: All   
Attachments: Proposed patch for EdcaParameterSet
Updated patch for EdcaParameterSet

Description Won Hyoung Lee 2018-02-17 04:52:02 UTC
Created attachment 3046 [details]
Proposed patch for EdcaParameterSet

Before describe the detail, I'm sorry for my bad English.

The data types for CWmin and CWmax are uint8_t in NS-3.
However, default values of CWmax for the background traffic and the best effort traffic are 1023 (from IEEE 802.11-2016 standard)

Because 1023 is 2^10 - 1, although the default values of CWmax for the background traffic and the best effort traffic are set as 1023 (0001 1111 1111 in bits), it is set as 255 (1111 1111 in bits).

In addition, at 3111 and 3113 pages in IEEE 802.11-2016 standard (revision of IEEE 802.11-2012 standard), the sizes for CWmin and CWmax are unsigned32.

Thus, I think that the data types for CWmin and CWmax should be changed from uint8_t to uint32_t.

Please check the IEEE 802.11 standard and the codes.
Thanks.

Won Hyoung Lee
Comment 1 sebastien.deronne 2018-02-17 05:08:04 UTC
It makes sense that uint8_t is not ok here. But why uint32_t then? uint16_t is enough to fit accepted values.
Comment 2 Tom Henderson 2018-02-17 10:46:48 UTC
I think he is suggesting to align with the data type defined in the MIB, which is an Unsigned32.  However, there is no 16-bit type in the MIB, and I don't think it much matters whether it is 16- or 32-bit in ns-3.
Comment 3 Won Hyoung Lee 2018-02-17 11:26:35 UTC
Well, I misunderstand IEEE 802.11 standard which is revision in 2016 (IEEE Std 802.11-2016).
I found that the maximum values for CWmin and CWmax are 32,767 (2^15 - 1) from page 898 in IEEE Std 802.11-2016. Thus, uint16_t is enough to CWmin and CWmax.

However, at page 3111 and 3113 in IEEE Std 802.11-2016, the types for CWmin and CWmax represent as Unsigned32.
And the data types for CWmin and CWmax of related classes or structs in NS-3 are also uint32_t. So, I think that uint32_t is ok for CWmin and CWmax.

Please check the IEEE 802.11 standard. Due to my English, I may misunderstand something else.
Thanks.

Won Hyoung Lee
Comment 4 sebastien.deronne 2018-02-22 15:41:23 UTC
OK, I will keep uint32_t, I also suggest to adapt SetEdcaParameters. I will provide an updated patch.
Comment 5 sebastien.deronne 2018-02-22 15:43:01 UTC
Created attachment 3063 [details]
Updated patch for EdcaParameterSet
Comment 6 Tom Henderson 2018-02-22 15:56:07 UTC
(In reply to sebastien.deronne from comment #5)
> Created attachment 3063 [details]
> Updated patch for EdcaParameterSet

I agree with the patch.
Comment 7 sebastien.deronne 2018-02-23 09:52:12 UTC
I do see higher throughput for simple-ht-hidden-stations.
Before patch: 16.9221 Mbit/s
After patch: 22.4686 Mbit/s

I assume this is good and I will update regression, I guess CW was not correct before the patch, resulting in a smaller contention window and thus more collisions.
Comment 8 sebastien.deronne 2018-02-23 13:35:53 UTC
Pushed in changeset 13309:92277625f68e