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

Issue 313270043: Add NetDeviceQueueInterface methods to implement flow control (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
2 months, 3 weeks ago by Stefano Avallone
Modified:
3 hours, 16 minutes ago
CC:
ns-3-reviews_googlegroups.com
Visibility:
Public.

Description

Add NetDeviceQueueInterface methods to implement flow control Please find a description in this mailing list post: http://mailman.isi.edu/pipermail/ns-developers/2017-January/013761.html

Patch Set 1 #

Patch Set 2 : #

Patch Set 3 : csma: Add support for flow control and BQL #

Patch Set 4 : #

Total comments: 2

Patch Set 5 : #

Unified diffs Side-by-side diffs Delta from patch set Stats (+709 lines, -119 lines) Patch
M examples/routing/global-injection-slash32.cc View 1 2 3 4 1 chunk +2 lines, -0 lines 0 comments Download
M examples/routing/global-routing-slash32.cc View 1 2 3 4 1 chunk +2 lines, -0 lines 0 comments Download
M examples/routing/static-routing-slash32.cc View 1 2 3 4 1 chunk +2 lines, -0 lines 0 comments Download
M src/csma/model/csma-net-device.h View 1 2 3 4 3 chunks +9 lines, -0 lines 0 comments Download
M src/csma/model/csma-net-device.cc View 1 2 3 4 3 chunks +45 lines, -1 line 0 comments Download
M src/lte/test/lte-simple-net-device.cc View 1 2 3 4 1 chunk +1 line, -1 line 0 comments Download
M src/network/model/net-device.h View 1 2 3 4 1 chunk +19 lines, -24 lines 0 comments Download
M src/network/utils/net-device-queue-interface.h View 1 2 3 4 4 chunks +204 lines, -0 lines 0 comments Download
M src/network/utils/net-device-queue-interface.cc View 1 2 3 4 1 chunk +6 lines, -0 lines 0 comments Download
M src/network/utils/simple-net-device.h View 1 2 3 4 2 chunks +5 lines, -0 lines 0 comments Download
M src/network/utils/simple-net-device.cc View 1 2 3 4 3 chunks +43 lines, -1 line 0 comments Download
M src/point-to-point/model/point-to-point-net-device.h View 1 2 3 4 1 chunk +1 line, -0 lines 0 comments Download
M src/point-to-point/model/point-to-point-net-device.cc View 1 2 3 4 4 chunks +26 lines, -89 lines 0 comments Download
M src/point-to-point/test/point-to-point-test.cc View 1 2 3 4 1 chunk +0 lines, -2 lines 0 comments Download
A src/traffic-control/test/tc-flow-control-test-suite.cc View 1 2 3 4 1 chunk +342 lines, -0 lines 0 comments Download
M src/traffic-control/wscript View 1 2 3 4 1 chunk +2 lines, -1 line 0 comments Download

Messages

Total messages: 18
Stefano Avallone
2 months, 3 weeks ago (2017-01-04 15:43:30 UTC) #1
Tom Henderson
Looks OK to me; this issue needs cc ns-3-reviews@googlegroups.com. Also consider to add some paragraph ...
2 months, 1 week ago (2017-01-15 23:35:30 UTC) #2
Stefano Avallone
Thanks a lot for the review. Just a note for other reviewers: I am soon ...
2 months, 1 week ago (2017-01-17 10:11:49 UTC) #3
Stefano Avallone
Changes with respect to the first version: - updated to match the second version of ...
2 months ago (2017-01-19 22:24:10 UTC) #4
Stefano Avallone
The third version adds support for flow control and BQL to CsmaNetDevice.
2 months ago (2017-01-19 22:28:54 UTC) #5
Stefano Avallone
Changes with respect to the third version: - removed the "workaround" of keeping track of ...
2 months ago (2017-01-24 17:16:06 UTC) #6
Luciano Chaves
Hi, I've just reviewed this code online and it seems ok to me. I've just ...
1 month, 3 weeks ago (2017-02-01 12:45:39 UTC) #7
Stefano Avallone
On 2017/02/01 12:45:39, Luciano Chaves wrote: > I would like to apply the patch on ...
1 month, 3 weeks ago (2017-02-01 13:51:40 UTC) #8
Stefano Avallone
https://codereview.appspot.com/313270043/diff/60001/src/network/model/net-device.h File src/network/model/net-device.h (right): https://codereview.appspot.com/313270043/diff/60001/src/network/model/net-device.h#newcode384 src/network/model/net-device.h:384: * - when the device queues have been created ...
1 month, 3 weeks ago (2017-02-01 13:51:52 UTC) #9
Luciano Chaves
Hi, I’ve tried to test the code from your github branch, but there’s something wrong ...
1 month, 2 weeks ago (2017-02-04 20:55:55 UTC) #10
Stefano Avallone
Hi Luciano, I cannot reproduce this error here. Could you please tell me which compiler ...
1 month, 2 weeks ago (2017-02-06 10:57:36 UTC) #11
Luciano Chaves
Hi Stefano, I’ve just repeated all steps from scratch and the error still persists. This ...
1 month, 2 weeks ago (2017-02-06 12:53:55 UTC) #12
Stefano Avallone
On 2017/02/06 12:53:55, Luciano Chaves wrote: > Hi Stefano, > > I’ve just repeated all ...
1 month, 2 weeks ago (2017-02-07 08:55:53 UTC) #13
Luciano Chaves
Hi Stefano, The error is happening for all the scripts that I’ve tried on this ...
1 month, 2 weeks ago (2017-02-07 17:03:34 UTC) #14
Luciano Chaves
Sorry, the link is missing the www... http://www.lrc.ic.unicamp.br/~luciano/files/debian8-ns3.ova <http://www.lrc.ic.unicamp.br/~luciano/files/debian8-ns3.ova> []’s -- Luciano Jerez Chaves Assistant ...
1 month, 2 weeks ago (2017-02-07 17:05:27 UTC) #15
Stefano Avallone
Hi Luciano, I am pretty sure the culprit is g++ 4.9.2. I have read of ...
1 month, 2 weeks ago (2017-02-07 22:08:45 UTC) #16
Stefano Avallone
I updated and reviewed the whole stack of patches. Examples and tests now run fine ...
1 month, 2 weeks ago (2017-02-10 18:05:05 UTC) #17
Luciano Chaves
1 month, 1 week ago (2017-02-14 15:10:50 UTC) #18
Hi,

I finally got it working here! I’ve tested your changes with some simulation
scripts that I have and everything seems to be ok (of course, with some small
changes on the commands used to change the mode and maxsize attributes that now
belongs to the QueueBase class.

Thank you for your hard work on this :)

--
Luciano Jerez Chaves
Assistant Professor at DCC/UFJF
PhD candidate at LRC/UNICAMP
http://www.lrc.ic.unicamp.br/~luciano <http://www.lrc.ic.unicamp.br/~luciano>
> On 10 Feb 2017, at 16:05, stavallo@gmail.com wrote:
> 
> I updated and reviewed the whole stack of patches. Examples and tests
> now run fine with g++-4.9.2.
> 
> Thanks,
> Stefano
> 
> 
> https://codereview.appspot.com/313270043/

Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld 80a51fa-tainted