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

Issue 14549044: Adding MPDU aggregation

Can't Edit
Can't Publish+Mail
Start Review
Created:
7 years ago by gbadawy
Modified:
5 years, 8 months ago
CC:
ns-3-reviews_googlegroups.com
Visibility:
Public.

Description

Adding MPDU aggregation

Patch Set 1 #

Total comments: 5

Patch Set 2 : Correcting some compilation errors #

Total comments: 8

Patch Set 3 : changed A-MPDU implementation to simplify PCAP generation #

Unified diffs Side-by-side diffs Delta from patch set Stats (+2541 lines, -231 lines) Patch
M examples/wireless/ht-wifi-network.cc View 1 2 4 chunks +6 lines, -24 lines 0 comments Download
A scratch/simple-wifi-mpdu-aggregation.cc View 1 2 1 chunk +221 lines, -0 lines 0 comments Download
M src/energy/model/wifi-radio-energy-model.h View 1 2 1 chunk +0 lines, -12 lines 0 comments Download
M src/mesh/model/dot11s/airtime-metric.cc View 1 2 1 chunk +1 line, -1 line 0 comments Download
M src/wifi/examples/wifi-phy-test.cc View 1 2 3 chunks +5 lines, -5 lines 0 comments Download
M src/wifi/helper/qos-wifi-mac-helper.h View 2 chunks +24 lines, -0 lines 0 comments Download
M src/wifi/helper/qos-wifi-mac-helper.cc View 3 chunks +24 lines, -0 lines 0 comments Download
M src/wifi/model/aarf-wifi-manager.cc View 1 2 2 chunks +9 lines, -2 lines 0 comments Download
A src/wifi/model/ampdu-subframe-header.h View 1 chunk +61 lines, -0 lines 0 comments Download
A src/wifi/model/ampdu-subframe-header.cc View 1 chunk +118 lines, -0 lines 0 comments Download
A src/wifi/model/ampdu-tag.h View 1 chunk +71 lines, -0 lines 0 comments Download
A src/wifi/model/ampdu-tag.cc View 1 chunk +104 lines, -0 lines 0 comments Download
M src/wifi/model/block-ack-agreement.h View 3 chunks +3 lines, -1 line 0 comments Download
M src/wifi/model/block-ack-agreement.cc View 1 chunk +10 lines, -1 line 0 comments Download
M src/wifi/model/block-ack-cache.h View 1 chunk +5 lines, -0 lines 0 comments Download
M src/wifi/model/block-ack-cache.cc View 1 chunk +5 lines, -0 lines 0 comments Download
M src/wifi/model/block-ack-manager.h View 7 chunks +32 lines, -2 lines 0 comments Download
M src/wifi/model/block-ack-manager.cc View 12 chunks +147 lines, -5 lines 0 comments Download
M src/wifi/model/dcf-manager.cc View 1 2 2 chunks +8 lines, -8 lines 0 comments Download
M src/wifi/model/edca-txop-n.h View 6 chunks +19 lines, -2 lines 0 comments Download
M src/wifi/model/edca-txop-n.cc View 18 chunks +235 lines, -17 lines 0 comments Download
M src/wifi/model/interference-helper.cc View 1 2 1 chunk +2 lines, -2 lines 0 comments Download
M src/wifi/model/mac-low.h View 1 2 14 chunks +111 lines, -4 lines 0 comments Download
M src/wifi/model/mac-low.cc View 1 2 48 chunks +725 lines, -92 lines 0 comments Download
M src/wifi/model/mac-tx-middle.h View 1 chunk +1 line, -0 lines 0 comments Download
M src/wifi/model/mac-tx-middle.cc View 1 chunk +25 lines, -0 lines 0 comments Download
M src/wifi/model/minstrel-wifi-manager.cc View 1 2 1 chunk +1 line, -1 line 0 comments Download
A src/wifi/model/mpdu-aggregator.h View 1 2 1 chunk +61 lines, -0 lines 0 comments Download
A src/wifi/model/mpdu-aggregator.cc View 1 2 1 chunk +77 lines, -0 lines 0 comments Download
A src/wifi/model/mpdu-standard-aggregator.h View 1 2 1 chunk +62 lines, -0 lines 0 comments Download
A src/wifi/model/mpdu-standard-aggregator.cc View 1 2 1 chunk +155 lines, -0 lines 0 comments Download
M src/wifi/model/wifi-mac.cc View 1 1 chunk +2 lines, -2 lines 0 comments Download
M src/wifi/model/wifi-mac-queue.h View 1 2 4 chunks +4 lines, -2 lines 0 comments Download
M src/wifi/model/wifi-mac-queue.cc View 1 2 5 chunks +31 lines, -2 lines 0 comments Download
M src/wifi/model/wifi-phy.h View 1 2 8 chunks +23 lines, -6 lines 0 comments Download
M src/wifi/model/wifi-phy.cc View 1 2 9 chunks +50 lines, -9 lines 0 comments Download
M src/wifi/model/wifi-preamble.h View 1 2 1 chunk +2 lines, -1 line 0 comments Download
M src/wifi/model/wifi-remote-station-manager.h View 1 2 1 chunk +1 line, -1 line 0 comments Download
M src/wifi/model/wifi-remote-station-manager.cc View 1 2 1 chunk +5 lines, -3 lines 0 comments Download
M src/wifi/model/yans-wifi-channel.h View 1 2 2 chunks +2 lines, -2 lines 0 comments Download
M src/wifi/model/yans-wifi-channel.cc View 1 2 2 chunks +8 lines, -5 lines 0 comments Download
M src/wifi/model/yans-wifi-phy.h View 1 2 3 chunks +5 lines, -2 lines 0 comments Download
M src/wifi/model/yans-wifi-phy.cc View 1 2 12 chunks +69 lines, -15 lines 0 comments Download
M src/wifi/test/tx-duration-test.cc View 1 2 2 chunks +3 lines, -2 lines 0 comments Download
M src/wifi/wscript View 2 chunks +8 lines, -0 lines 0 comments Download

Messages

Total messages: 82
Daniel L.
My initial review for the "modified" files. Also, don't forget to credit yourself in the ...
7 years ago (2013-10-11 15:25:05 UTC) #1
gbadawy
Hi Daniel, Thanks for your review please find my comments below. I updated the code ...
7 years ago (2013-10-11 18:35:10 UTC) #2
kusoof
Hi, I'm not sure whether this is the right list to post to, but if ...
7 years ago (2013-10-15 15:36:36 UTC) #3
tomh_tomh.org
On 10/15/2013 08:36 AM, Lynne Salameh wrote: > Hi, > > I'm not sure whether ...
7 years ago (2013-10-15 15:40:37 UTC) #4
kusoof
Hi Ghada, Thanks a lot for working on a much needed feature for ns-3. I'd ...
7 years ago (2013-10-15 17:14:08 UTC) #5
gbadawy
Hi Lynne, Thanks for helping. Adding 11n to NS3 is not an easy job and ...
7 years ago (2013-10-16 15:20:40 UTC) #6
Daniel L.
That is actually a very good point. I forgot to look into the pcap thing ...
7 years ago (2013-10-16 16:01:37 UTC) #7
kusoof
Hi Ghada, Indeed, you are right. The 802.11 spec does indicate that the aggregation functionality ...
7 years ago (2013-10-16 18:07:56 UTC) #8
Daniel L.
I think it is better to keep things in the MAC layer rather than doing ...
7 years ago (2013-10-16 18:18:43 UTC) #9
kusoof
Hi Daniel, > May be we can have MAC send a train of MPDUs? That ...
7 years ago (2013-10-16 19:05:33 UTC) #10
gbadawy
Hi Lynna and Daniel, On Wed, Oct 16, 2013 at 3:05 PM, <kusoof@gmail.com> wrote: > ...
7 years ago (2013-10-16 20:20:16 UTC) #11
gbadawy
Hi Lynne, On Wed, Oct 16, 2013 at 2:07 PM, <kusoof@gmail.com> wrote: > Hi Ghada, ...
7 years ago (2013-10-16 20:25:51 UTC) #12
kusoof
Hi Ghada, > Will the receiving node see a train of packets or 1 big ...
7 years ago (2013-10-16 20:48:55 UTC) #13
Daniel L.
On 2013/10/16 20:48:55, kusoof wrote: > Hi Ghada, > > > Will the receiving node ...
7 years ago (2013-10-16 21:26:21 UTC) #14
kusoof
> I think that de-aggregation should not be done at PHY. I think we should ...
7 years ago (2013-10-16 22:49:10 UTC) #15
Daniel L.
On 2013/10/16 22:49:10, kusoof wrote: > > I think that de-aggregation should not be done ...
7 years ago (2013-10-17 13:57:52 UTC) #16
kusoof
> How about something like this: > > MacLow::StartTransmission receives a list of Packets instead ...
7 years ago (2013-10-17 14:21:47 UTC) #17
gbadawy
On Thu, Oct 17, 2013 at 9:57 AM, <nikkipui@gmail.com> wrote: > On 2013/10/16 22:49:10, kusoof ...
7 years ago (2013-10-17 14:22:40 UTC) #18
kusoof
> > MacLow::StartTransmission should only receive 1 packet from EdcaTxopN as > it already does. ...
7 years ago (2013-10-17 14:30:32 UTC) #19
Daniel L.
On 2013/10/17 14:21:47, kusoof wrote: > > How about something like this: > > > ...
7 years ago (2013-10-17 14:36:58 UTC) #20
Daniel L.
On 2013/10/17 14:30:32, kusoof wrote: > > > MacLow::StartTransmission should only receive 1 packet from ...
7 years ago (2013-10-17 14:43:59 UTC) #21
gbadawy
On Thu, Oct 17, 2013 at 10:21 AM, <kusoof@gmail.com> wrote: > How about something like ...
7 years ago (2013-10-17 14:52:18 UTC) #22
kusoof
> The concern I have when gluing them together is that I think the receiving ...
7 years ago (2013-10-17 15:02:22 UTC) #23
gbadawy
Hi, On Thu, Oct 17, 2013 at 11:02 AM, <kusoof@gmail.com> wrote: > > The concern ...
7 years ago (2013-10-18 13:49:23 UTC) #24
kusoof
> > I will update my code to include the changes that we talked about ...
7 years ago (2013-10-18 13:53:27 UTC) #25
Daniel L.
On 2013/10/18 13:49:23, gbadawy wrote: > Hi, > I will update my code to include ...
7 years ago (2013-10-18 13:54:47 UTC) #26
kusoof
Hi Daniel and Ghada, Just a minor bug I discovered when I tried to implement ...
7 years ago (2013-10-21 16:10:04 UTC) #27
kusoof
I missed a spot. See the additional diffs. -Lynne https://codereview.appspot.com/14549044/diff/8001/src/wifi/model/mac-low.cc File src/wifi/model/mac-low.cc (right): https://codereview.appspot.com/14549044/diff/8001/src/wifi/model/mac-low.cc#newcode2287 src/wifi/model/mac-low.cc:2287: ...
7 years ago (2013-10-21 16:20:22 UTC) #28
kusoof
Another minor bug related to the previous one. https://codereview.appspot.com/14549044/diff/8001/src/wifi/model/block-ack-agreement.cc File src/wifi/model/block-ack-agreement.cc (right): https://codereview.appspot.com/14549044/diff/8001/src/wifi/model/block-ack-agreement.cc#newcode125 src/wifi/model/block-ack-agreement.cc:125: uint16_t ...
7 years ago (2013-10-21 16:22:24 UTC) #29
kusoof
https://codereview.appspot.com/14549044/diff/8001/src/wifi/model/block-ack-manager.cc File src/wifi/model/block-ack-manager.cc (right): https://codereview.appspot.com/14549044/diff/8001/src/wifi/model/block-ack-manager.cc#newcode566 src/wifi/model/block-ack-manager.cc:566: NS_ASSERT (it != m_agreements.end ()); Should insert: //Check the ...
6 years, 12 months ago (2013-11-05 19:17:21 UTC) #30
kusoof
Hi, I've managed to run the attached test case (see tcp_test.cc) with the code patches. ...
6 years, 12 months ago (2013-11-05 19:27:33 UTC) #31
jinjing81
https://codereview.appspot.com/14549044/diff/8001/src/wifi/model/mac-low.cc File src/wifi/model/mac-low.cc (right): https://codereview.appspot.com/14549044/diff/8001/src/wifi/model/mac-low.cc#newcode2506 src/wifi/model/mac-low.cc:2506: WifiTxVector dataTxVector = GetDataTxVector (m_currentPacket, &m_currentHdr); The dataTxVector is ...
6 years, 11 months ago (2013-11-18 19:48:43 UTC) #32
jinjing81
https://codereview.appspot.com/14549044/patch/8001/9028 there are several typos on "Msdu", should be "Mpdu"
6 years, 11 months ago (2013-11-19 00:11:29 UTC) #33
gbadawy
Yep you are correct I removed it On Mon, Nov 18, 2013 at 2:48 PM, ...
6 years, 11 months ago (2013-11-19 17:48:08 UTC) #34
gbadawy
These were corrected On Mon, Nov 18, 2013 at 7:11 PM, <jinjing81@gmail.com> wrote: > https://codereview.appspot.com/14549044/patch/8001/9028 ...
6 years, 11 months ago (2013-11-19 17:49:00 UTC) #35
jinjing81
Thanks for your hard work, gbadawy! One suggestion is to enable pcap in current patch. ...
6 years, 10 months ago (2013-12-17 02:23:19 UTC) #36
yannis
Hello, I 'm testing the A-mpdu and I have some queries. 1. The m_sentMpdus shouldn't ...
6 years, 7 months ago (2014-03-25 16:18:51 UTC) #37
gbadawy
On Tue, Mar 25, 2014 at 12:18 PM, <selinis.g@gmail.com> wrote: > Hello, > > I ...
6 years, 7 months ago (2014-03-25 18:15:41 UTC) #38
yannis
On 2014/03/25 18:15:41, gbadawy wrote: > On Tue, Mar 25, 2014 at 12:18 PM, <mailto:selinis.g@gmail.com> ...
6 years, 7 months ago (2014-03-25 19:16:43 UTC) #39
gbadawy
On Tue, Mar 25, 2014 at 3:16 PM, <selinis.g@gmail.com> wrote: > On 2014/03/25 18:15:41, gbadawy ...
6 years, 7 months ago (2014-03-25 19:26:50 UTC) #40
yannis
On 2014/03/25 19:26:50, gbadawy wrote: > On Tue, Mar 25, 2014 at 3:16 PM, <mailto:selinis.g@gmail.com> ...
6 years, 7 months ago (2014-03-25 20:15:04 UTC) #41
yannis
Is there any solution for the infinite loop on the BlockAckManager::GetSeqNumOfRetryPacket() ? I get inside ...
6 years, 7 months ago (2014-04-01 17:24:12 UTC) #42
gbadawy
On Tue, Mar 25, 2014 at 4:15 PM, <selinis.g@gmail.com> wrote: > On 2014/03/25 19:26:50, gbadawy ...
6 years, 7 months ago (2014-04-01 17:54:09 UTC) #43
gbadawy
On Tue, Apr 1, 2014 at 1:24 PM, <selinis.g@gmail.com> wrote: > Is there any solution ...
6 years, 7 months ago (2014-04-01 18:01:15 UTC) #44
gbadawy
On Tue, Apr 1, 2014 at 1:53 PM, Ghada Badawy <gbadawy@gmail.com> wrote: > > > ...
6 years, 7 months ago (2014-04-01 18:08:56 UTC) #45
yannis
On 2014/04/01 18:08:56, gbadawy wrote: > On Tue, Apr 1, 2014 at 1:53 PM, Ghada ...
6 years, 7 months ago (2014-04-01 19:43:29 UTC) #46
yannis
I'm testing under saturated condition, with multiple different applications installed and with various nodes on ...
6 years, 7 months ago (2014-04-03 18:49:59 UTC) #47
gbadawy
On Thu, Apr 3, 2014 at 2:49 PM, <selinis.g@gmail.com> wrote: > I'm testing under saturated ...
6 years, 7 months ago (2014-04-03 19:30:14 UTC) #48
S. Deronne
I made a patch for all reviewers, including those features: - Add IsSupportedMcs in YansWifiPhy ...
6 years, 6 months ago (2014-04-18 13:10:34 UTC) #49
Tom Henderson
On 2014/04/18 13:10:34, S. Deronne wrote: > I made a patch for all reviewers, including ...
6 years, 6 months ago (2014-04-20 22:01:36 UTC) #50
S. Deronne
> This is a very large patch addressing multiple issues; can it be broken into ...
6 years, 6 months ago (2014-04-23 08:28:39 UTC) #51
Daniel L.
I think your suggested chunks you are good. I think it’s better to have one ...
6 years, 6 months ago (2014-04-24 17:58:52 UTC) #52
S. Deronne
Thanks Daniel, I will proceed this way then. But on which code should I apply ...
6 years, 6 months ago (2014-04-24 18:24:21 UTC) #53
Daniel L.
I think from ns-3-dev sounds better. On Apr 24, 2014, at 2:24 PM, sebastien.deronne@gmail.com wrote: ...
6 years, 6 months ago (2014-04-24 18:48:09 UTC) #54
S. Deronne
On 2014/04/24 18:48:09, Daniel L. wrote: > I think from ns-3-dev sounds better. > > ...
6 years, 6 months ago (2014-04-24 19:05:34 UTC) #55
jinjing81
Hi, Sebastien, Thanks for your patch. I looked through your patches and have a question ...
6 years, 6 months ago (2014-04-25 02:06:08 UTC) #56
jinjing81
On 2014/04/24 19:05:34, S. Deronne wrote: > On 2014/04/24 18:48:09, Daniel L. wrote: > > ...
6 years, 6 months ago (2014-04-25 02:16:29 UTC) #57
S. Deronne
On 2014/04/25 02:06:08, jinjing81 wrote: > Hi, Sebastien, > > Thanks for your patch. I ...
6 years, 6 months ago (2014-04-25 07:22:53 UTC) #58
S. Deronne
On 2014/04/25 02:16:29, jinjing81 wrote: > On 2014/04/24 19:05:34, S. Deronne wrote: > > On ...
6 years, 6 months ago (2014-04-25 07:28:43 UTC) #59
jinjing81
Sorry I mean patch 3 from Ghada. Thought patch 2 is the latest... On Friday, ...
6 years, 6 months ago (2014-04-25 14:37:25 UTC) #60
jinjing81
On 2014/04/25 07:22:53, S. Deronne wrote: > On 2014/04/25 02:06:08, jinjing81 wrote: > > Hi, ...
6 years, 6 months ago (2014-04-25 16:48:48 UTC) #61
gbadawy
Hi All, Sebastien I have a question why do you need IsMcsSupported in the Phy ...
6 years, 6 months ago (2014-04-25 17:14:58 UTC) #62
S. Deronne
Hi Ghada, In fact this chunk is not dependent on A-MPDU aggregation. Since the addition ...
6 years, 6 months ago (2014-04-25 20:18:49 UTC) #63
gbadawy
On Fri, Apr 25, 2014 at 4:18 PM, <sebastien.deronne@gmail.com> wrote: > Hi Ghada, > > ...
6 years, 6 months ago (2014-04-28 18:24:59 UTC) #64
S. Deronne
On 2014/04/28 18:24:59, gbadawy wrote: > On Fri, Apr 25, 2014 at 4:18 PM, <mailto:sebastien.deronne@gmail.com> ...
6 years, 6 months ago (2014-04-29 08:09:08 UTC) #65
gbadawy
On Tue, Apr 29, 2014 at 4:09 AM, <sebastien.deronne@gmail.com> wrote: > On 2014/04/28 18:24:59, gbadawy ...
6 years, 6 months ago (2014-04-29 12:58:18 UTC) #66
Daniel L.
I personally think it might be the best if we can get everything working based ...
6 years, 6 months ago (2014-04-29 22:46:22 UTC) #67
yannis
Hi All, - Daniel I have the same question as Ghada,for the implementation of IsModeSupported. ...
6 years, 5 months ago (2014-05-15 17:24:53 UTC) #68
yannis
Hi All, I made a patch, based on Daniel's modifications on Ghada's 3rd patch. -BlockAckManager::PeekNextPacket, ...
6 years, 5 months ago (2014-05-26 22:47:16 UTC) #69
yannis
Something I forgot to mention. While the number of aggregated packets is increasing, the probability ...
6 years, 5 months ago (2014-05-27 17:57:28 UTC) #70
Yongsen
Hi all, I was trying to patch the A-MPDU on NS3.19, but I got some ...
6 years, 5 months ago (2014-06-05 07:01:57 UTC) #71
S. Deronne
Thanks Yannis for your patch, I will try to have a look at this shortly. ...
6 years, 4 months ago (2014-06-12 20:34:55 UTC) #72
Yongsen
On 2014/05/27 17:57:28, yannis wrote: > Something I forgot to mention. While the number of ...
6 years, 4 months ago (2014-06-24 10:48:54 UTC) #73
Tom Henderson
On 2014/06/24 10:48:54, Yongsen wrote: > On 2014/05/27 17:57:28, yannis wrote: > > Something I ...
6 years, 4 months ago (2014-06-24 13:56:34 UTC) #74
Tom Henderson
Hi all, Can we make a final push to finish MPDU aggregation off in the ...
6 years, 1 month ago (2014-09-25 20:04:52 UTC) #75
gbadawy
On 2014/09/25 20:04:52, Tom Henderson wrote: > Hi all, > > Can we make a ...
6 years, 1 month ago (2014-09-26 12:35:01 UTC) #76
Tom Henderson
> Since the original patch was based on ns3-dev 10 months ago I will have ...
6 years ago (2014-11-01 14:46:27 UTC) #77
s.fakhrali
On 2014/11/01 14:46:27, Tom Henderson wrote: > > Since the original patch was based on ...
5 years, 11 months ago (2014-11-21 14:34:39 UTC) #78
Gigi
On 2014/11/01 14:46:27, Tom Henderson wrote: > > Since the original patch was based on ...
5 years, 11 months ago (2014-12-06 01:38:03 UTC) #79
Gigi
On 2014/12/06 01:38:03, GigiIggytech wrote: > On 2014/11/01 14:46:27, Tom Henderson wrote: > > > ...
5 years, 11 months ago (2014-12-06 02:13:46 UTC) #80
Gigi
Has anyone tried enabling RTS/CTS? I found that if RTS/CST is enabled then the aggregation ...
5 years, 8 months ago (2015-02-26 19:58:08 UTC) #81
S. Deronne
5 years, 8 months ago (2015-02-26 22:28:04 UTC) #82
The final code has been pushed in ns-3.22, so please do no longer use this
rietveld.

If you encounter some bugs, please either post a message on ns-3-users google
group or open a bug in Bugzilla.

In the latest version, RTS/CTS together with A-MPDU aggregation is fully
supported.
Anyway, some bugs can still happen, so feel free to indicate when issues are
found.
Sign in to reply to this message.

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