Bug 2950 - Regression in Minstrel HT
Regression in Minstrel HT
Status: RESOLVED FIXED
Product: ns-3
Classification: Unclassified
Component: wifi
ns-3-dev
All All
: P3 major
Assigned To: sebastien.deronne
:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2018-07-12 11:04 UTC by Alexander Krotov
Modified: 2018-07-16 14:29 UTC (History)
1 user (show)

See Also:


Attachments
path to fix (31.74 KB, patch)
2018-07-15 05:08 UTC, sebastien.deronne
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Alexander Krotov 2018-07-12 11:04:24 UTC
I am running experiments with 802.11ac: wifi.SetStandard (WIFI_PHY_STANDARD_80211ac);

I have started merging with ns-3-dev, and apparently this commit breaks things: https://code.nsnam.org/index.cgi/ns-3-dev/rev/de0e4a4b3957

I have added some tracing, and it looks like station->m_maxTpRate contains values > 256. In my case I trace values like 329, 328, 328, ...

So changing its type to uint8_t was a bad idea, I think types for throughput should be changed to uint16_t at least.

By the way, I have skimmed through the commit and it looks like it changes more than just types. For example this line sets station->m_minstrelTable: https://code.nsnam.org/index.cgi/ns-3-dev/rev/de0e4a4b3957#l1.111 Is this change intended?
Comment 1 sebastien.deronne 2018-07-12 14:18:38 UTC
Alexander, could you provide a script to reproduce the problem?
Thanks!
Comment 2 Alexander Krotov 2018-07-13 04:56:11 UTC
Update to the correct revision (parent of de0e4a4b3957):

$ hg up -r e35caba656f3

Modify examples/wireless/rate-adaptation-distance.cc:

-  onoff.SetConstantRate (DataRate ("200Mb/s"), 1420);
+  onoff.SetConstantRate (DataRate ("400Mb/s"), 1420);

Run:
$ ./waf --run "rate-adaptation-distance --staManager=ns3::MinstrelHtWifiManager --apManager=ns3::MinstrelHtWifiManager --outputFileName=minstrelHt --channelWidth=80 --steps=2 --stepsTime=5 --standard=802.11ac"

Read throughput-minstrelHt.plt, throughput is 308.197

Update to latest, revision, rerun. Throughput is 156.118.
Comment 3 sebastien.deronne 2018-07-13 16:03:47 UTC
Thanks, I am working on a fix.
Comment 4 sebastien.deronne 2018-07-15 05:08:12 UTC
Created attachment 3137 [details]
path to fix

I used some uint16_t instead of uint8_t as you suggested.
Comment 5 Alexander Krotov 2018-07-16 04:19:56 UTC
I have tested the patch, it fixes regression.

Now throughput is 308.17 (not 308.197), but it is probably the result of rounding errors, other commits, etc.

By the way, maybe it makes sense to increase DataRate in the example permanently? 200 Mbit/s is not the upper bound anymore.
Comment 6 sebastien.deronne 2018-07-16 12:32:34 UTC
(In reply to Alexander Krotov from comment #5)
> I have tested the patch, it fixes regression.
> 
> Now throughput is 308.17 (not 308.197), but it is probably the result of
> rounding errors, other commits, etc.
> 
> By the way, maybe it makes sense to increase DataRate in the example
> permanently? 200 Mbit/s is not the upper bound anymore.

Yes, I do not have objections for this.
Comment 7 Alexander Krotov 2018-07-16 13:01:36 UTC
Ok, I have increased to 400 Mbit/s and rerun all tests.

Can you push the fix to ns-3-dev so I can merge with it?
Comment 8 sebastien.deronne 2018-07-16 14:29:03 UTC
Fixed in changeset 13694:5176532f55c7