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

Issue 306660043: Phase 1: ECN implementation for queue discs (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
4 years ago by Shravya
Modified:
3 years, 10 months ago
CC:
ns-3-reviews_googlegroups.com
Visibility:
Public.

Description

Explicit Congestion Notification (RFC 3168) is a mechanism for Active Queue Management (AQM) to notify endpoints of congestion that may be developing in a bottleneck queue, without resorting to packet drops. In ns-3, the current queue disciplines (e.g., RED) only support packet drops but not an ECN mode of operation. The patch here provides an implementation of ECN mechanism in RED QueueDisc and tests to verify its functionality. This patch has been developed as a part of the ns-3 summer project listed here: https://www.nsnam.org/wiki/ECN_support_for_qdiscs_in_ns-3 Any suggestions would be much appreciated. Regards, Shravya K. S P.S.: - This patch contains a fix for Bug 2485 - Phase II code is available for review here: https://codereview.appspot.com/314790043/

Patch Set 1 #

Total comments: 15

Patch Set 2 : Revised patch #

Total comments: 25

Patch Set 3 : revised patch #

Unified diffs Side-by-side diffs Delta from patch set Stats (+336 lines, -12 lines) Patch
M src/internet/doc/ipv4.rst View 1 chunk +23 lines, -0 lines 0 comments Download
M src/internet/doc/ipv6.rst View 1 1 chunk +24 lines, -0 lines 0 comments Download
M src/internet/model/ipv4-queue-disc-item.h View 1 chunk +6 lines, -0 lines 0 comments Download
M src/internet/model/ipv4-queue-disc-item.cc View 1 1 chunk +13 lines, -0 lines 0 comments Download
M src/internet/model/ipv6-header.h View 1 2 1 chunk +26 lines, -0 lines 0 comments Download
M src/internet/model/ipv6-header.cc View 1 2 2 chunks +32 lines, -0 lines 0 comments Download
M src/internet/model/ipv6-queue-disc-item.h View 1 chunk +6 lines, -0 lines 0 comments Download
M src/internet/model/ipv6-queue-disc-item.cc View 1 1 chunk +12 lines, -0 lines 0 comments Download
M src/test/ns3tc/pie-queue-disc-test-suite.cc View 2 chunks +7 lines, -0 lines 0 comments Download
M src/traffic-control/doc/queue-discs.rst View 1 chunk +9 lines, -5 lines 0 comments Download
M src/traffic-control/doc/red.rst View 1 2 2 chunks +27 lines, -1 line 0 comments Download
M src/traffic-control/model/queue-disc.h View 1 chunk +7 lines, -0 lines 0 comments Download
M src/traffic-control/model/red-queue-disc.h View 1 2 2 chunks +3 lines, -1 line 0 comments Download
M src/traffic-control/model/red-queue-disc.cc View 1 2 4 chunks +24 lines, -5 lines 0 comments Download
M src/traffic-control/test/codel-queue-disc-test-suite.cc View 2 chunks +7 lines, -0 lines 0 comments Download
M src/traffic-control/test/red-queue-disc-test-suite.cc View 1 2 5 chunks +110 lines, -0 lines 0 comments Download

Messages

Total messages: 13
n.p
Review for the first part. I have some other comments but I prefer to view ...
3 years, 11 months ago (2016-11-04 13:46:45 UTC) #1
Shravya
Hi Natale, Thank you very much for reviewing the code and giving suggestions. I have ...
3 years, 11 months ago (2016-11-05 07:00:49 UTC) #2
Stefano Avallone
Thanks for working on the support of ECN. Please find below my comments. https://codereview.appspot.com/306660043/diff/1/src/internet/doc/ipv6.rst File ...
3 years, 11 months ago (2016-11-06 22:44:58 UTC) #3
Shravya
Thank you for providing detailed reviews. I have attached a revised patch, and provided comments ...
3 years, 11 months ago (2016-11-14 06:00:14 UTC) #4
Stefano Avallone
Hi Shravya, thanks for updating the patch. I will discuss with Tom to decide what ...
3 years, 11 months ago (2016-11-14 09:42:04 UTC) #5
Mohit Tahiliani
Thanks for the reviews, Natale and Stefano. Shravya, I've added some minor comments; mainly related ...
3 years, 11 months ago (2016-11-14 14:17:51 UTC) #6
Stefano Avallone
Thanks Mohit, I also had noticed all the trailing spaces and the redundant lines in ...
3 years, 11 months ago (2016-11-14 14:29:35 UTC) #7
Mohit Tahiliani
On 2016/11/14 14:29:35, Stefano Avallone wrote: > Thanks Mohit, I also had noticed all the ...
3 years, 11 months ago (2016-11-14 14:32:49 UTC) #8
Tom Henderson
> On 2016/11/06 22:44:58, Stefano Avallone wrote: > > This test suite is very similar ...
3 years, 11 months ago (2016-11-14 15:30:49 UTC) #9
Stefano Avallone
On 2016/11/14 15:30:49, Tom Henderson wrote: > > I don't agree that these tests must ...
3 years, 11 months ago (2016-11-14 17:14:21 UTC) #10
Shravya
Dear Tom Sir and Stefano Sir, I have uploaded the revised patch with following changes: ...
3 years, 11 months ago (2016-11-17 14:35:34 UTC) #11
Stefano Avallone
Thanks for updating once again the patch. I have yet to review the new version ...
3 years, 11 months ago (2016-11-17 23:33:30 UTC) #12
Stefano Avallone
3 years, 10 months ago (2016-12-13 19:01:22 UTC) #13
This patch has been pushed to ns-3-dev. Could you please close this review?

Thanks,
Stefano
Sign in to reply to this message.

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