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

Issue 4440051: Fragmentation support for IPv4 (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
9 years, 10 months ago by Tommaso Pecorella
Modified:
9 years, 7 months ago
Reviewers:
Tom Henderson, Vedran Miletić, tomh
CC:
ns-3-reviews_googlegroups.com
Visibility:
Public.

Description

IMPORTANT ARP prevents packets longer than 3 fragments to be delivered unless the ARP cache is already valid. This is (so far) working as intended.

Patch Set 1 : Update the patch to apply to the latest ns-3-dev revision #

Total comments: 15

Patch Set 2 : Changes according to Tom's comments #

Total comments: 8

Patch Set 3 : Changed attribute name to something meaningful #

Total comments: 1

Patch Set 4 : Fix spaces, add assert about IP header length #

Total comments: 7

Patch Set 5 : Fixed some comments and removed commented lines #

Unified diffs Side-by-side diffs Delta from patch set Stats (+1607 lines, -11 lines) Patch
M src/flow-monitor/model/ipv4-flow-probe.h View 1 chunk +1 line, -0 lines 0 comments Download
M src/flow-monitor/model/ipv4-flow-probe.cc View 1 chunk +4 lines, -0 lines 0 comments Download
M src/internet/model/ipv4-l3-protocol.h View 1 2 3 6 chunks +112 lines, -0 lines 0 comments Download
M src/internet/model/ipv4-l3-protocol.cc View 1 2 3 6 chunks +346 lines, -9 lines 0 comments Download
M src/internet/model/ipv6-extension.h View 1 2 3 2 chunks +45 lines, -0 lines 0 comments Download
M src/internet/model/ipv6-extension.cc View 1 2 3 4 4 chunks +65 lines, -2 lines 0 comments Download
A src/internet/test/error-channel.h View 1 1 chunk +92 lines, -0 lines 0 comments Download
A src/internet/test/error-channel.cc View 1 1 chunk +153 lines, -0 lines 0 comments Download
A src/internet/test/error-net-device.h View 1 1 chunk +117 lines, -0 lines 0 comments Download
A src/internet/test/error-net-device.cc View 1 1 chunk +254 lines, -0 lines 0 comments Download
A src/internet/test/ipv4-fragmentation-test.cc View 1 2 3 4 1 chunk +415 lines, -0 lines 0 comments Download
M src/internet/wscript View 1 chunk +3 lines, -0 lines 0 comments Download

Messages

Total messages: 39
Tommaso Pecorella
9 years, 10 months ago (2011-04-21 00:57:21 UTC) #1
Tommaso Pecorella
The tentative is not poor anymore. I completed the fragmentation support (assuming I haven't forgot ...
9 years, 10 months ago (2011-04-21 11:24:02 UTC) #2
Tommaso Pecorella
9 years, 10 months ago (2011-04-22 17:48:30 UTC) #3
Tom Henderson
My main comment is that it would be nice to have a few unit/regression tests, ...
9 years, 10 months ago (2011-04-22 21:29:49 UTC) #4
Tommaso Pecorella
On 2011/04/22 21:29:49, Tom Henderson wrote: > My main comment is that it would be ...
9 years, 10 months ago (2011-04-22 22:31:57 UTC) #5
Tommaso Pecorella
http://codereview.appspot.com/4440051/diff/6002/src/internet/model/ipv4-l3-protocol.cc File src/internet/model/ipv4-l3-protocol.cc (right): http://codereview.appspot.com/4440051/diff/6002/src/internet/model/ipv4-l3-protocol.cc#newcode738 src/internet/model/ipv4-l3-protocol.cc:738: uint32_t fragmentSize = outInterface->GetDevice ()->GetMtu () & ~uint32_t (0x7); ...
9 years, 10 months ago (2011-04-23 00:52:31 UTC) #6
Tommaso Pecorella
9 years, 10 months ago (2011-04-24 13:56:13 UTC) #7
Tommaso Pecorella
New code (ironed out in some parts) and new bugs (found and killed ofc). I ...
9 years, 10 months ago (2011-04-24 14:16:05 UTC) #8
Tommaso Pecorella
I found something I forgot. When a packet is dropped due to timeout on fragments ...
9 years, 10 months ago (2011-04-29 20:05:29 UTC) #9
tomh_tomh.org
On 04/29/2011 01:05 PM, tommypec@gmail.com wrote: > I found something I forgot. > > When ...
9 years, 10 months ago (2011-05-02 04:24:23 UTC) #10
Tommaso Pecorella
Yo Tom, On 02/mag/2011, at 06.24, Tom Henderson wrote: > On 04/29/2011 01:05 PM, tommypec@gmail.com ...
9 years, 10 months ago (2011-05-02 10:15:48 UTC) #11
Tommaso Pecorella
9 years, 10 months ago (2011-05-03 00:16:25 UTC) #12
Tom Henderson
On 2011/05/02 10:15:48, Tommaso Pecorella wrote: > Yo Tom, > > > On 02/mag/2011, at ...
9 years, 10 months ago (2011-05-03 04:31:51 UTC) #13
Tommaso Pecorella
9 years, 10 months ago (2011-05-03 22:57:44 UTC) #14
Tommaso Pecorella
I know I shouldn't mix things together, but right now I'm a bit wasted from ...
9 years, 10 months ago (2011-05-03 23:16:09 UTC) #15
Tommaso Pecorella
Updated the patch with IPv6 fragmentation fixes
9 years, 10 months ago (2011-05-05 11:39:44 UTC) #16
Tommaso Pecorella
9 years, 9 months ago (2011-05-26 07:51:27 UTC) #17
Tom Henderson
sorry for the delay in reviewing. Main comments: - I am not sure that your ...
9 years, 9 months ago (2011-06-13 05:08:45 UTC) #18
Tommaso Pecorella
On 2011/06/13 05:08:45, Tom Henderson wrote: > sorry for the delay in reviewing. Main comments: ...
9 years, 9 months ago (2011-06-13 10:14:46 UTC) #19
Tommaso Pecorella
9 years, 8 months ago (2011-06-24 00:39:14 UTC) #20
Tommaso Pecorella
This is only to cleanup the comments and remove the unnecessary files (scratch and IPv4header.cc/.h). ...
9 years, 8 months ago (2011-06-24 00:44:29 UTC) #21
Vedran Miletić
Sorry for the delay on reviewing. Implementation looks good to me. Compared to Mathieu's yans ...
9 years, 8 months ago (2011-06-24 15:51:05 UTC) #22
Tommaso Pecorella
On 2011/06/24 15:51:05, Vedran Miletić wrote: > Sorry for the delay on reviewing. No problem ...
9 years, 8 months ago (2011-06-24 19:49:00 UTC) #23
Tommaso Pecorella
9 years, 8 months ago (2011-06-24 19:53:40 UTC) #24
Vedran Miletić
On 2011/06/24 19:49:00, Tommaso Pecorella wrote: > > API change that Tom mentioned can also ...
9 years, 8 months ago (2011-06-25 08:21:53 UTC) #25
Tommaso Pecorella
On 2011/06/25 08:21:53, Vedran Miletić wrote: > On 2011/06/24 19:49:00, Tommaso Pecorella wrote: > > ...
9 years, 8 months ago (2011-06-25 11:41:19 UTC) #26
Tommaso Pecorella
Let's not loose the spin... we're close to finalize this small but quite useful patch. ...
9 years, 8 months ago (2011-06-28 06:53:43 UTC) #27
Tom Henderson
Regarding uncrustify, you have a lot of trailing whitespace changes in ipv4-l3-protocol.cc which I suggest ...
9 years, 8 months ago (2011-06-28 17:45:56 UTC) #28
Tommaso Pecorella
On 2011/06/28 17:45:56, Tom Henderson wrote: > Regarding uncrustify, you have a lot of trailing ...
9 years, 8 months ago (2011-06-28 18:50:21 UTC) #29
tomh_tomh.org
On 06/28/2011 11:50 AM, tommypec@gmail.com wrote: > On 2011/06/28 17:45:56, Tom Henderson wrote: >> Regarding ...
9 years, 8 months ago (2011-06-28 18:57:18 UTC) #30
Tommaso Pecorella
On 2011/06/28 18:57:18, tomh_tomh.org wrote: > >> you could consider to make the program abort ...
9 years, 8 months ago (2011-06-28 19:30:08 UTC) #31
Tommaso Pecorella
9 years, 8 months ago (2011-06-28 20:59:48 UTC) #32
Tommaso Pecorella
On 2011/06/28 20:59:48, Tommaso Pecorella wrote: All done. Plan for the patches is (when everyone ...
9 years, 8 months ago (2011-06-28 21:04:15 UTC) #33
Tommaso Pecorella
Hi guys, shall we do a final round of checks or do you think this ...
9 years, 8 months ago (2011-07-05 07:50:19 UTC) #34
Vedran Miletić
Hi Tommaso, on the IPv4 side, I believe this is ready for prime time. As ...
9 years, 8 months ago (2011-07-06 12:08:59 UTC) #35
Tom Henderson
just a few more comments http://codereview.appspot.com/4440051/diff/41003/src/internet/model/ipv6-extension.cc File src/internet/model/ipv6-extension.cc (right): http://codereview.appspot.com/4440051/diff/41003/src/internet/model/ipv6-extension.cc#newcode375 src/internet/model/ipv6-extension.cc:375: // m_dropTrace (packet); I ...
9 years, 8 months ago (2011-07-08 17:17:44 UTC) #36
Tommaso Pecorella
http://codereview.appspot.com/4440051/diff/41003/src/internet/model/ipv6-extension.cc File src/internet/model/ipv6-extension.cc (right): http://codereview.appspot.com/4440051/diff/41003/src/internet/model/ipv6-extension.cc#newcode375 src/internet/model/ipv6-extension.cc:375: // m_dropTrace (packet); On 2011/07/08 17:17:45, Tom Henderson wrote: ...
9 years, 8 months ago (2011-07-08 18:49:47 UTC) #37
Tommaso Pecorella
http://codereview.appspot.com/4440051/diff/41003/src/internet/model/ipv6-extension.cc File src/internet/model/ipv6-extension.cc (right): http://codereview.appspot.com/4440051/diff/41003/src/internet/model/ipv6-extension.cc#newcode375 src/internet/model/ipv6-extension.cc:375: // m_dropTrace (packet); On 2011/07/08 17:17:45, Tom Henderson wrote: ...
9 years, 8 months ago (2011-07-09 01:20:46 UTC) #38
Tommaso Pecorella
9 years, 8 months ago (2011-07-09 01:25:49 UTC) #39

          
Sign in to reply to this message.

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