Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(1520)

Issue 283960043: Correct most comments and bugs in MinstrelHT patchset from issue 254900043 (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
8 years, 3 months ago by Matías Richart
Modified:
7 years, 11 months ago
Reviewers:
S. Deronne
CC:
ns-3-reviews_googlegroups.com
Visibility:
Public.

Description

In this new patch set I try to deal with comments from issue 254900043 and also correct some bugs of previous patch. It follows minstrel_ht Linux implementation, as accurate as possible. The most important decisions are: - Use the idea of groups. Rates are arranged in groups, considering the streams used, if it uses short guard interval and the channel width. - There is a vector of all groups supported by the phy layer of the transmitter, for each group it is maintained the capabilities and the estimated duration of its rates. - Each station maintains a table of groups statistics. For each group, a flag indicates if the group is supported by the station. Different stations communicating with an AP can have different capabilities. - Adaptation of mcs, sgi, streams and channel width is made. - Stats and rates are updated per A-MPDU when receiving AmpduTxStatus - When HT and VHT are not supported, it uses legacy minstrel (minstrel-wifi-manager) for rate control. - Differently from legacy minstrel, sampling is not done based on "lookaround ratio", but assuring all rates are sampled at least once each interval. Some considerations: - Assume patches for bugs 2316, 2317, 2319, 2320 and 2321 - RTS is sent in non-HT PPDU with a NonHtReferenceRate following the standard. - When HT or VHT is supported, only uses HT or VHT rates. - Currently, NS3 does not retransmit an entire A-MPDU when BACK is missing but retransmits each MPDU until MPDUs lifetime expires (or a BACK is received). Then, we emulate MRR-chain only until the retry limit maintained at minstrel-ht is reached. Then a new rate will be selected. - Currently, NS3 does not support disabling aggregation for particular frames, then sample rates are used on an entire A-MPDU (differently from minstrel_ht design)

Patch Set 1 #

Total comments: 27

Patch Set 2 : #

Total comments: 11

Patch Set 3 : #

Patch Set 4 : #

Patch Set 5 : Improve sampling, mostly in legacy minstrel #

Patch Set 6 : Correct logical mistake in time comparison #

Patch Set 7 : Improve max probability sorting in legacy Minstrel. #

Unified diffs Side-by-side diffs Delta from patch set Stats (+16 lines, -8 lines) Patch
M src/wifi/model/minstrel-wifi-manager.cc View 1 2 3 4 5 6 3 chunks +16 lines, -8 lines 0 comments Download

Messages

Total messages: 10
S. Deronne
Some first comments and interrogations already. I still need to test it and have a ...
8 years, 2 months ago (2016-01-08 21:12:57 UTC) #1
Matías Richart
Hi Sebastien. I try to answer all the questions and I corrected the code where ...
8 years, 2 months ago (2016-01-11 11:58:14 UTC) #2
S. Deronne
Matias, It looks already better, thanks for having handle this so quickly. I have mainly ...
8 years, 2 months ago (2016-01-17 09:55:34 UTC) #3
Matías Richart
On 2016/01/17 09:55:34, S. Deronne wrote: > Matias, > It looks already better, thanks for ...
8 years, 2 months ago (2016-01-21 12:14:48 UTC) #4
Matías Richart
https://codereview.appspot.com/283960043/diff/20001/src/wifi/model/minstrel-ht-wifi-manager.cc File src/wifi/model/minstrel-ht-wifi-manager.cc (right): https://codereview.appspot.com/283960043/diff/20001/src/wifi/model/minstrel-ht-wifi-manager.cc#newcode20 src/wifi/model/minstrel-ht-wifi-manager.cc:20: * Ghada Badawy <gbadawy@gmail.com> On 2016/01/17 09:55:34, S. Deronne ...
8 years, 2 months ago (2016-01-21 12:14:57 UTC) #5
S. Deronne
https://codereview.appspot.com/283960043/diff/20001/src/wifi/model/minstrel-ht-wifi-manager.h File src/wifi/model/minstrel-ht-wifi-manager.h (right): https://codereview.appspot.com/283960043/diff/20001/src/wifi/model/minstrel-ht-wifi-manager.h#newcode143 src/wifi/model/minstrel-ht-wifi-manager.h:143: uint8_t N_GROUPS = MAX_SUPPORTED_STREAMS * MAX_STREAM_GROUPS; //!< Number of ...
8 years, 2 months ago (2016-01-22 20:21:46 UTC) #6
Matías Richart
Patch Set 3 includes the corrections to the second review done by Sebastien but also ...
8 years, 1 month ago (2016-02-10 15:45:11 UTC) #7
Matías Richart
In this patch set there is still missing two important things: - no fallback to ...
8 years, 1 month ago (2016-02-10 15:46:52 UTC) #8
S. Deronne
On 2016/02/10 15:45:11, Matías Richart wrote: > Patch Set 3 includes the corrections to the ...
8 years, 1 month ago (2016-02-14 20:43:33 UTC) #9
Matías Richart
8 years, 1 month ago (2016-02-15 11:13:22 UTC) #10
On 2016/02/14 20:43:33, S. Deronne wrote:
> On 2016/02/10 15:45:11, Matías Richart wrote:
> > Patch Set 3 includes the corrections to the second review done by Sebastien
> but
> > also some other improvements:
> > - Enable SGI and chWidth adaptation
> > - Improve RTS rate election. Add GetNonHtReferenceRate function to wifi-mode
> > (needed for RTS rate election)
> > - Change throughput calculation when probability is less than 10% or more
than
> > 90%. Copy what is done in linux
> > - Modify how sampling is made (based on Linux implementation). This make the
> > algorithm to react better to changes.
> > - In ad-hoc mode when the station supports HT install all MCSs supported.
This
> > was a bug that make HT rate adaptation not to work when using ad-hoc mode
> 
> Matias, was it not agreed to go for the extension of the legacy Minstrel iso
> adding a new HT Minstrel class, in order to support "fallback" to legacy
> Minstrel?

Yes, your are right. I'm working on that.
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld f62528b