Bug 2950

Summary:	Regression in Minstrel HT
Product:	ns-3	Reporter:	Alexander Krotov <krotov>
Component:	wifi	Assignee:	sebastien.deronne
Status:	RESOLVED FIXED
Severity:	major	CC:	ns-bugs
Priority:	P3
Version:	ns-3-dev
Hardware:	All
OS:	All
Attachments:	path to fix

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