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

Issue 123190043: LTP final code review

Can't Edit
Can't Publish+Mail
Start Review
Created:
7 years, 2 months ago by Ruben Martinez
Modified:
7 years, 2 months ago
CC:
ns-3-reviews_googlegroups.com
Visibility:
Public.

Description

The current ltp protocol |ns3| implementation supports the following features: * Basic data transmission between two nodes. * Retransmission based reliability. * Notices to client service instance. * Udp Convergence layer adapter with link state cue support. * Static Ltp engine id to IP address conversion. * All types of Ltp Headers and Content Headers. * Self-Delimiting Numeric Values (SDNV) support in headers. The implementation has the following limitations: * Only supports one concurrent transmission session per destination. * Only a single timer of each type can be run concurrently in a single session (only a checkpoint or report can be handled at the same time). * Segment reception offsets are assumed to be ordered. * Upon loss of a red data segment, the whole red part is retransmitted, not just the specific segment. * Checkpoint and Red Data Segment retransmission are handled together The LTP model DOES NOT support the following features: * Asynchronous checkpoints. * Cancellation Requests. * Handling of System Error conditions. * Management Information Base, containing times of operation of remote ltp engines. * LTP security and authentication extensions.

Patch Set 1 #

Total comments: 41
Unified diffs Side-by-side diffs Delta from patch set Stats (+22678 lines, -0 lines) Patch
A src/ltp-protocol/doc/figures/ltp-protocol-structure.eps View 1 chunk +14373 lines, -0 lines 0 comments Download
A src/ltp-protocol/doc/ltp-protocol.rst View 1 chunk +277 lines, -0 lines 10 comments Download
A src/ltp-protocol/examples/ltp-protocol-example.cc View 1 chunk +170 lines, -0 lines 5 comments Download
A src/ltp-protocol/examples/wscript View 1 chunk +6 lines, -0 lines 0 comments Download
A src/ltp-protocol/helper/ltp-protocol-helper.h View 1 chunk +151 lines, -0 lines 4 comments Download
A src/ltp-protocol/helper/ltp-protocol-helper.cc View 1 chunk +259 lines, -0 lines 3 comments Download
A src/ltp-protocol/model/ltp-convergence-layer-adapter.h View 1 chunk +194 lines, -0 lines 0 comments Download
A src/ltp-protocol/model/ltp-convergence-layer-adapter.cc View 1 chunk +149 lines, -0 lines 0 comments Download
A src/ltp-protocol/model/ltp-header.h View 1 chunk +531 lines, -0 lines 0 comments Download
A src/ltp-protocol/model/ltp-header.cc View 1 chunk +1245 lines, -0 lines 0 comments Download
A src/ltp-protocol/model/ltp-ip-resolution-table.h View 1 chunk +190 lines, -0 lines 0 comments Download
A src/ltp-protocol/model/ltp-ip-resolution-table.cc View 1 chunk +196 lines, -0 lines 0 comments Download
A src/ltp-protocol/model/ltp-protocol.h View 1 chunk +453 lines, -0 lines 16 comments Download
A src/ltp-protocol/model/ltp-protocol.cc View 1 chunk +1049 lines, -0 lines 0 comments Download
A src/ltp-protocol/model/ltp-queue-set.h View 1 chunk +90 lines, -0 lines 0 comments Download
A src/ltp-protocol/model/ltp-queue-set.cc View 1 chunk +143 lines, -0 lines 0 comments Download
A src/ltp-protocol/model/ltp-session-state-record.h View 1 chunk +615 lines, -0 lines 0 comments Download
A src/ltp-protocol/model/ltp-session-state-record.cc View 1 chunk +852 lines, -0 lines 2 comments Download
A src/ltp-protocol/model/ltp-session-state-record-impl.h View 1 chunk +125 lines, -0 lines 0 comments Download
A src/ltp-protocol/model/ltp-udp-convergence-layer-adapter.h View 1 chunk +173 lines, -0 lines 0 comments Download
A src/ltp-protocol/model/ltp-udp-convergence-layer-adapter.cc View 1 chunk +262 lines, -0 lines 0 comments Download
A src/ltp-protocol/test/ltp-protocol-channel-loss-test-suite.cc View 1 chunk +387 lines, -0 lines 0 comments Download
A src/ltp-protocol/test/ltp-protocol-test-suite.cc View 1 chunk +742 lines, -0 lines 1 comment Download
A src/ltp-protocol/wscript View 1 chunk +46 lines, -0 lines 0 comments Download

Messages

Total messages: 3
Tom Henderson
First set of comments (on .rst documentation, examples, tests, and helper). Most are minor, but ...
7 years, 2 months ago (2014-08-21 14:58:21 UTC) #1
Tom Henderson
some additional comments (ltp-protocol.h) https://codereview.appspot.com/123190043/diff/1/src/ltp-protocol/model/ltp-protocol.h File src/ltp-protocol/model/ltp-protocol.h (right): https://codereview.appspot.com/123190043/diff/1/src/ltp-protocol/model/ltp-protocol.h#newcode65 src/ltp-protocol/model/ltp-protocol.h:65: /* Default constructor */ not ...
7 years, 2 months ago (2014-08-21 23:08:21 UTC) #2
bpswenson
7 years, 2 months ago (2014-08-24 17:22:59 UTC) #3
patch applied cleanly to ns-3-dev

Compile error
Missing file. Don't see it created in patch.  Need to be added to hg?  Not
seeing it in a wscript.

[1488/1970] cxx: src/lte/model/lte-anr.cc -> build/src/lte/model/lte-anr.cc.1.o
../src/ltp-protocol/model/ltp-header.cc:22:22: fatal error: ns3/sdnv.h: No such
file or directory
 #include "ns3/sdnv.h"
                      ^
compilation terminated.

https://codereview.appspot.com/123190043/diff/1/src/ltp-protocol/examples/ltp...
File src/ltp-protocol/examples/ltp-protocol-example.cc (right):

https://codereview.appspot.com/123190043/diff/1/src/ltp-protocol/examples/ltp...
src/ltp-protocol/examples/ltp-protocol-example.cc:54: std::cout <<
"ClientServiceNotification - Session ID: " << id.GetSessionNumber () << " Code :
" << code << std::endl;
Probably want to switch these couts to NS_LOG_INFO and enable with the verbose
parameter that currently doesn't appear to be used.

https://codereview.appspot.com/123190043/diff/1/src/ltp-protocol/helper/ltp-p...
File src/ltp-protocol/helper/ltp-protocol-helper.cc (right):

https://codereview.appspot.com/123190043/diff/1/src/ltp-protocol/helper/ltp-p...
src/ltp-protocol/helper/ltp-protocol-helper.cc:219: void
LtpProtocolHelper::SetBaseLtpEngineId (uint64_t id)
For consistency purposes please either stick with

void
LtpProtocolHelper::

or 

void LtpProtocolHelper

https://codereview.appspot.com/123190043/diff/1/src/ltp-protocol/model/ltp-se...
File src/ltp-protocol/model/ltp-session-state-record.cc (right):

https://codereview.appspot.com/123190043/diff/1/src/ltp-protocol/model/ltp-se...
src/ltp-protocol/model/ltp-session-state-record.cc:542:
SessionStateRecord::GetClaimsUpperBound (uint32_t serialNum)
const method?

https://codereview.appspot.com/123190043/diff/1/src/ltp-protocol/model/ltp-se...
src/ltp-protocol/model/ltp-session-state-record.cc:559:
SessionStateRecord::GetClaimsLowerBound (uint32_t serialNum)
const method?
Sign in to reply to this message.

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