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

Issue 279540043: DHCP Implementation in ns-3.24

Can't Edit
Can't Publish+Mail
Start Review
Created:
1 year, 2 months ago by Mohit Tahiliani
Modified:
4 days, 17 hours ago
Reviewers:
adadeepak8, mattator, barnes26, Tommaso Pecorella
CC:
ns-3-reviews_googlegroups.com, deeptir96_gmail.com
Visibility:
Public.

Description

This patch is an extension and a port of Radu Lupu's DHCP Patch (provided for ns-3.12). The patch provided here works with ns-3.24. Following additions have been done: 1. Ported ns-3.12 DHCP code to work with ns-3.24 2. DHCP ACK has been implemented (it was pending in ns-3.12's patch) 3. DHCP header is now compatible with Wireshark 4. Two test cases have been provided We have taken permission from Radu Lupu to use his code of ns-3.12 and port it to ns-3.24. Any suggestions / reviews would be much appreciated. Thanks and Regards, Ankit Deepak and Deepti Rajagopal P.S.: 1. Patch set 6 onwards the code has been ported to work with ns-3.25 2. Patch set 8 onwards the code has been ported to work with ns-3.26

Patch Set 1 #

Total comments: 20

Patch Set 2 : Modified DHCP patch for ns-3.24 #

Total comments: 18

Patch Set 3 : Modified DHCP patch for ns-3.24 #

Total comments: 81

Patch Set 4 : Modified DHCP patch with changes in client #

Total comments: 26

Patch Set 5 : Latest DHCP modifications #

Total comments: 16

Patch Set 6 : DHCP patch shifted to internet-apps #

Patch Set 7 : Modified DHCP Patch (Aug 2016) #

Total comments: 6

Patch Set 8 : Revised patch (on ns-3.26) #

Total comments: 2

Patch Set 9 : Revised patch (added Router Option) #

Total comments: 4

Patch Set 10 : Modified patch (bug fix and improved Doxygen comments) #

Total comments: 7
Unified diffs Side-by-side diffs Delta from patch set Stats (+2586 lines, -0 lines) Patch
A src/internet-apps/doc/dhcp.rst View 1 2 3 4 5 1 chunk +66 lines, -0 lines 0 comments Download
A src/internet-apps/examples/dhcp-example.cc View 1 2 3 4 5 6 7 8 1 chunk +165 lines, -0 lines 0 comments Download
A src/internet-apps/examples/wscript View 1 2 3 4 5 1 chunk +8 lines, -0 lines 0 comments Download
A src/internet-apps/helper/dhcp-helper.h View 1 2 3 4 5 6 7 8 9 1 chunk +121 lines, -0 lines 0 comments Download
A src/internet-apps/helper/dhcp-helper.cc View 1 2 3 4 5 6 7 8 1 chunk +90 lines, -0 lines 0 comments Download
A src/internet-apps/model/dhcp-client.h View 1 2 3 4 5 6 7 8 9 1 chunk +183 lines, -0 lines 0 comments Download
A src/internet-apps/model/dhcp-client.cc View 1 2 3 4 5 6 7 8 9 1 chunk +456 lines, -0 lines 7 comments Download
A src/internet-apps/model/dhcp-header.h View 1 2 3 4 5 6 7 8 9 1 chunk +303 lines, -0 lines 0 comments Download
A src/internet-apps/model/dhcp-header.cc View 1 2 3 4 5 6 7 8 1 chunk +508 lines, -0 lines 0 comments Download
A src/internet-apps/model/dhcp-server.h View 1 2 3 4 5 6 7 8 1 chunk +127 lines, -0 lines 0 comments Download
A src/internet-apps/model/dhcp-server.cc View 1 2 3 4 5 6 7 8 9 1 chunk +381 lines, -0 lines 0 comments Download
A src/internet-apps/test/dhcp-test.cc View 1 2 3 4 5 6 7 8 1 chunk +163 lines, -0 lines 0 comments Download
M src/internet-apps/wscript View 1 2 3 4 5 2 chunks +15 lines, -0 lines 0 comments Download

Messages

Total messages: 34
Tommaso Pecorella
Hi, I'm reviewing the code, but before we go further it's mandatory to do the ...
1 year, 1 month ago (2015-12-28 00:13:41 UTC) #1
adadeepak8
On 2015/12/28 00:13:41, Tommaso Pecorella wrote: > Hi, > > I'm reviewing the code, but ...
1 year, 1 month ago (2016-01-05 09:47:11 UTC) #2
mattator
Dear Ankit, Thanks for the contribution. I am curious what's your motive for reimplementing DHCP ...
1 year, 1 month ago (2016-01-05 14:08:51 UTC) #3
adadeepak8
On 2016/01/05 14:08:51, mattator wrote: > Dear Ankit, > > Thanks for the contribution. > ...
1 year, 1 month ago (2016-01-06 08:50:09 UTC) #4
adadeepak8
Dear Tommaso Sir, I have updated the DHCP patch and uploaded it here. Almost all ...
1 year ago (2016-02-01 11:01:18 UTC) #5
Tommaso Pecorella
Second round, still only on headers (sorry, but I have little time and I prefer ...
1 year ago (2016-02-02 23:11:40 UTC) #6
adadeepak8
Dear Tommaso Sir, Except for the query related to deserialize function, I've incorporated all the ...
1 year ago (2016-02-04 10:33:41 UTC) #7
Tommaso Pecorella
Hi Ankit, thanks for your very quick replies. I hope to be able to be ...
1 year ago (2016-02-04 11:20:58 UTC) #8
adadeepak8
Dear Tommaso Sir, I have uploaded patch 3 with following changes: - created the examples ...
1 year ago (2016-02-08 11:32:57 UTC) #9
Tommaso Pecorella
Hi, here's another round of comments. I still need to check the server tho... expect ...
1 year ago (2016-02-10 23:40:02 UTC) #10
adadeepak8
Dear Sir, I have incorporated 14 suggestions out of 32, and working on the rest. ...
1 year ago (2016-02-12 11:03:08 UTC) #11
adadeepak8
Dear Sir, Patch 4 has been uploaded. Thank you very much for your time and ...
1 year ago (2016-02-12 11:39:11 UTC) #12
Tommaso Pecorella
Hi, I'm sorry if I seems to be extremely boring and utterly precise. The truth ...
1 year ago (2016-02-12 21:56:09 UTC) #13
adadeepak8
Dear Sir, Apologies for the delayed response; there were mid term exams. I have uploaded ...
11 months, 4 weeks ago (2016-03-01 14:10:02 UTC) #14
adadeepak8
Dear Sir, I've shifted the DHCP code to internet-apps in patch set 6, as suggested ...
9 months, 3 weeks ago (2016-05-06 06:47:55 UTC) #15
Tommaso Pecorella
Hi, sorry for the delay, my bad. Please find some more comments. The next review ...
9 months ago (2016-05-27 01:16:04 UTC) #16
adadeepak8
Dear Sir, Thank you for your suggestions. I am really sorry for the delay, I ...
8 months, 1 week ago (2016-06-16 20:36:27 UTC) #17
adadeepak8
Dear Sir, I am very sorry for the delay, but the next version incorporating all ...
5 months, 4 weeks ago (2016-08-29 04:09:42 UTC) #18
Tommaso Pecorella
Hi Ankit, I'm deeply sorry that it took this long. There are a couple of ...
3 months, 1 week ago (2016-11-12 13:10:02 UTC) #19
adadeepak8
Dear sir, I have updated the DHCP patch and uploaded it here. I have tried ...
3 months, 1 week ago (2016-11-15 16:38:14 UTC) #20
Tommaso Pecorella
This is the last comment. From my side I have no more things to add ...
3 months, 1 week ago (2016-11-18 22:59:16 UTC) #21
adadeepak8
Dear Sir, Sorry for the delay, I had my final examinations for the semester over ...
2 months, 3 weeks ago (2016-11-28 07:08:59 UTC) #22
Tommaso Pecorella
On 2016/11/28 07:08:59, adadeepak8 wrote: > Dear Sir, > > Sorry for the delay, I ...
2 months, 3 weeks ago (2016-11-29 15:02:04 UTC) #23
adadeepak8
Sir, While adding the router option, do I make the list of routers that the ...
2 months, 1 week ago (2016-12-12 13:05:00 UTC) #24
Tommaso Pecorella
1 month, 2 weeks ago (2017-01-09 11:24:06 UTC) #25
Tommaso Pecorella
Hi Ankit, (previous message was empty) I'd say that a single router is more than ...
1 month, 2 weeks ago (2017-01-09 11:25:56 UTC) #26
adadeepak8
Sir, I have completed the setting of default router for client. But, the users thread ...
1 month, 2 weeks ago (2017-01-10 16:45:20 UTC) #27
Tommaso Pecorella
On 2017/01/10 16:45:20, adadeepak8 wrote: > Sir, > > I have completed the setting of ...
1 month, 2 weeks ago (2017-01-10 17:05:59 UTC) #28
adadeepak8
Sir, I have included the router option and added two trace sources for new lease ...
1 month, 1 week ago (2017-01-11 05:12:44 UTC) #29
Tommaso Pecorella
The more you look at some code, the more bugs you find. Embarrassing. After the ...
1 month, 1 week ago (2017-01-13 00:43:32 UTC) #30
barnes26_llnl.gov
On Tuesday, January 10, 2017 at 9:06:00 AM UTC-8, Tommaso Pecorella wrote: > > I ...
1 month, 1 week ago (2017-01-13 18:47:44 UTC) #31
adadeepak8
Sir, I have uploaded the updated patch. I scanned the whole code once more and ...
1 month, 1 week ago (2017-01-16 07:47:59 UTC) #32
Tommaso Pecorella
I bet you're going to kill me. https://codereview.appspot.com/279540043/diff/180001/src/internet-apps/model/dhcp-client.cc File src/internet-apps/model/dhcp-client.cc (right): https://codereview.appspot.com/279540043/diff/180001/src/internet-apps/model/dhcp-client.cc#newcode187 src/internet-apps/model/dhcp-client.cc:187: } Remove ...
1 month, 1 week ago (2017-01-16 20:01:56 UTC) #33
Tommaso Pecorella
1 week ago (2017-02-16 23:41:34 UTC) #34
Hi,

I don't want to rush things, but... I'd really love to push this in the next
release :)

Cheers,

T.
Sign in to reply to this message.

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