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

Issue 850045: MPLS implementation

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

Patch Set 1 #

Total comments: 16
Unified diffs Side-by-side diffs Delta from patch set Stats (+3201 lines, -0 lines) Patch
M src/devices/point-to-point/point-to-point-net-device.cc View 2 chunks +2 lines, -0 lines 0 comments Download
A src/mpls-stack/examples/mpls1.cc View 1 chunk +151 lines, -0 lines 1 comment Download
A src/mpls-stack/mpls/forwarding-equivalence-class.h View 1 chunk +47 lines, -0 lines 1 comment Download
A src/mpls-stack/mpls/forwarding-equivalence-class.cc View 1 chunk +47 lines, -0 lines 1 comment Download
A src/mpls-stack/mpls/ip-to-mpls-routing-helper.h View 1 chunk +51 lines, -0 lines 0 comments Download
A src/mpls-stack/mpls/ip-to-mpls-routing-helper.cc View 1 chunk +83 lines, -0 lines 0 comments Download
A src/mpls-stack/mpls/ipv4-address-prefix.h View 1 chunk +70 lines, -0 lines 0 comments Download
A src/mpls-stack/mpls/ipv4-address-prefix.cc View 1 chunk +145 lines, -0 lines 0 comments Download
A src/mpls-stack/mpls/ipv4-host-address.h View 1 chunk +63 lines, -0 lines 2 comments Download
A src/mpls-stack/mpls/ipv4-host-address.cc View 1 chunk +90 lines, -0 lines 0 comments Download
A src/mpls-stack/mpls/ipv4-to-mpls-routing-protocol.h View 1 chunk +81 lines, -0 lines 0 comments Download
A src/mpls-stack/mpls/ipv4-to-mpls-routing-protocol.cc View 1 chunk +169 lines, -0 lines 1 comment Download
A src/mpls-stack/mpls/ipv6-to-mpls-routing-protocol.h View 1 chunk +84 lines, -0 lines 0 comments Download
A src/mpls-stack/mpls/ipv6-to-mpls-routing-protocol.cc View 1 chunk +186 lines, -0 lines 0 comments Download
A src/mpls-stack/mpls/label-information-base-helper.h View 1 chunk +73 lines, -0 lines 3 comments Download
A src/mpls-stack/mpls/label-information-base-helper.cc View 1 chunk +143 lines, -0 lines 0 comments Download
A src/mpls-stack/mpls/mpls-label-stack.h View 1 chunk +111 lines, -0 lines 1 comment Download
A src/mpls-stack/mpls/mpls-label-stack.cc View 1 chunk +306 lines, -0 lines 0 comments Download
A src/mpls-stack/mpls/mpls-lib-entry.h View 1 chunk +83 lines, -0 lines 0 comments Download
A src/mpls-stack/mpls/mpls-lib-entry.cc View 1 chunk +187 lines, -0 lines 0 comments Download
A src/mpls-stack/mpls/mpls-routing-protocol.h View 1 chunk +116 lines, -0 lines 2 comments Download
A src/mpls-stack/mpls/mpls-routing-protocol.cc View 1 chunk +641 lines, -0 lines 2 comments Download
A src/mpls-stack/mpls/mpls-stack-helper.h View 1 chunk +64 lines, -0 lines 1 comment Download
A src/mpls-stack/mpls/mpls-stack-helper.cc View 1 chunk +175 lines, -0 lines 1 comment Download
A src/mpls-stack/mpls/wscript View 1 chunk +32 lines, -0 lines 0 comments Download
M src/wscript View 1 chunk +1 line, -0 lines 0 comments Download

Messages

Total messages: 8
Lalith Suresh
The helper related code should go into src/helper/mpls-stack-helper.h/cc.
11 years, 5 months ago (2010-04-14 17:50:36 UTC) #1
Lalith Suresh
On 2010/04/14 17:50:36, Lalith Suresh wrote: > The helper related code should go into src/helper/mpls-stack-helper.h/cc. ...
11 years, 5 months ago (2010-04-14 18:02:29 UTC) #2
aachurin
On 2010/04/14 18:02:29, Lalith Suresh wrote: > On 2010/04/14 17:50:36, Lalith Suresh wrote: > > ...
11 years, 5 months ago (2010-04-18 07:06:00 UTC) #3
aachurin
On 2010/04/18 07:06:00, aachurin wrote: > On 2010/04/14 18:02:29, Lalith Suresh wrote: > > On ...
11 years, 5 months ago (2010-04-18 07:12:35 UTC) #4
Mathieu Lacage
I am short on time so I did not review completely the MplsRoutingProtocol class but ...
11 years, 4 months ago (2010-05-21 13:26:12 UTC) #5
Tom Henderson
Overall this code looks good. However, besides dealing with the recent review comments, there are ...
11 years, 4 months ago (2010-05-22 06:12:43 UTC) #6
Tom Henderson
On 2010/05/22 06:12:43, Tom Henderson wrote: > Overall this code looks good. However, besides dealing ...
11 years, 4 months ago (2010-05-22 06:15:29 UTC) #7
aachurin
11 years, 4 months ago (2010-05-22 11:22:41 UTC) #8
Thx!

I will modify the source in response to your comments in the near future


2010/5/22 <tomh.org@gmail.com>

> On 2010/05/22 06:12:43, Tom Henderson wrote:
>
>> Overall this code looks good.  However, besides dealing with the
>>
> recent review
>
>> comments, there are a few key missing pieces required to merge:  1)
>>
> all public
>
>> methods documented with Doxygen, 2) some kind of overview
>>
> documentation about
>
>> what this model is, what is its scope of applicability (what is
>>
> implemented from
>
>> the RFCs and what is not); see src/routing/aodv/aodv.h for an example,
>>
> 3) there
>
>> are no tests.  For tests, I would like to see at least one non-trivial
>>
> test that
>
>> exercises the key elements of the code, including showing how MPLS can
>>
> cause
>
>> packets to traverse the non-shortest-path from IP perspective, and
>>
> handle both
>
>> IPv4 and IPv6 flows.  The test could go in src/test; I could help with
>>
> setting
>
>> that up if needed.
>>
>
> Forgot to mention, Wireshark can decode MPLS headers so it would be nice
> to enable pcap on these flows and verify that Wireshark can read it.
>
>
> http://codereview.appspot.com/850045/show
>



-- 
Best regards,

Churin Andrey,
Lead Application Developer,
Vicman
Sign in to reply to this message.

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