Sorry for the lengthy delay in reviewing this. Besides responding to the detailed comments, it would help to provide some simple examples of how this can be used. Also, a documentation file (.rst) for the model library is needed. I had a broader question about whether this should be implemented as a "Box" (bump in the wire) or as packet treatment on a specialized NetDevice. Couldn't you accomplish everything you need by just making this a special NetDevice and avoid e.g. the point-to-point variant? Also, it seems like it would be trivial to make this also handle UDP or any layer-4 protocol that matches the standard flowspec-- any reason why it is limited to TCP? http://codereview.appspot.com/4875052/diff/1/src/delaybox/delaybox-net-device.cc File src/delaybox/delaybox-net-device.cc (right): http://codereview.appspot.com/4875052/diff/1/src/delaybox/delaybox-net-device... src/delaybox/delaybox-net-device.cc:64: &DelayBoxNetDeviceT<ND>::m_delayBoxDropTrace)) // an example demonstrating use of these trace sources would be helpful http://codereview.appspot.com/4875052/diff/1/src/delaybox/delaybox-net-device... src/delaybox/delaybox-net-device.cc:89: dest).Bind (protocolNumber); This is an unconventional usage; I'm surprised that it works. Why not use a callback with explicit parameters? Callback<bool, Ptr<Packet>, const Address&, uint16_t> ? http://codereview.appspot.com/4875052/diff/1/src/delaybox/delaybox-net-device... src/delaybox/delaybox-net-device.cc:90: if (m_delayBox->Delay (send, static_cast<Ptr<const Packet> > (packet))) is this static_cast needed? http://codereview.appspot.com/4875052/diff/1/src/delaybox/delaybox-net-device.h File src/delaybox/delaybox-net-device.h (right): http://codereview.appspot.com/4875052/diff/1/src/delaybox/delaybox-net-device... src/delaybox/delaybox-net-device.h:60: * DelayBox is a NetDevice that should be placed on a node in between the source and destination nodes. Can't this instead be used without an intermediate node? The test suite seems to use it that way. Could you then do away with the DelayBoxNetDeviceT type of template class? I am not sure what the DelayBoxPointToPointNetDevice, for instance, buys you. http://codereview.appspot.com/4875052/diff/1/src/delaybox/delaybox.cc File src/delaybox/delaybox.cc (right): http://codereview.appspot.com/4875052/diff/1/src/delaybox/delaybox.cc#newcode140 src/delaybox/delaybox.cc:140: // TODO: instead of digging through headers here, we should take them as parameters to this function. Doesn't seem robust to handling non-TCP (Ipv4) packets; have you tested it with e.g. an IPv6 packet? http://codereview.appspot.com/4875052/diff/1/src/delaybox/delaybox.cc#newcode253 src/delaybox/delaybox.cc:253: != m_rules.end () // match both ports it might help for debugging to break each of these cases out into separate tests, so you could put some debug log statements that express the specific condition that matched. http://codereview.appspot.com/4875052/diff/1/src/delaybox/delaybox.h File src/delaybox/delaybox.h (right): http://codereview.appspot.com/4875052/diff/1/src/delaybox/delaybox.h#newcode55 src/delaybox/delaybox.h:55: * \brief For internal use. If you want this to be internal (not visible at the public API), suggest to either 1) make a private member of DelayBoxRule, or 2) put in a separate header file that is not exported by the wscript Same comment applies for other \internal classes below http://codereview.appspot.com/4875052/diff/1/src/delaybox/delaybox.h#newcode101 src/delaybox/delaybox.h:101: return m_delayRV; we do not put implementation of methods into headers, unless necessary for inline or templating. Please put into the .cc file. http://codereview.appspot.com/4875052/diff/1/src/delaybox/delaybox.h#newcode152 src/delaybox/delaybox.h:152: * \param send A callback which sends the packet when called. Incomplete doxygen for parameters. Putting a TcpHeader here seems very specialized; what if it is not TCP? http://codereview.appspot.com/4875052/diff/1/src/delaybox/tcp-flow-classifier.cc File src/delaybox/tcp-flow-classifier.cc (right): http://codereview.appspot.com/4875052/diff/1/src/delaybox/tcp-flow-classifier... src/delaybox/tcp-flow-classifier.cc:44: const uint8_t TCP_PROT_NUMBER = 6; this constant also available as TcpL4Protocol::PROT_NUMBER http://codereview.appspot.com/4875052/diff/1/src/delaybox/tcp-flow-classifier.h File src/delaybox/tcp-flow-classifier.h (right): http://codereview.appspot.com/4875052/diff/1/src/delaybox/tcp-flow-classifier... src/delaybox/tcp-flow-classifier.h:69: * \param out_flowId If not null, the packet's flow id will be written here on success. A better C++ design would be to pass by non-const reference instead of by pointer; then you don't have to worry about it being null. Plus, we frown on having raw pointers in our public API. http://codereview.appspot.com/4875052/diff/1/src/delaybox/tcp-flow-classifier... src/delaybox/tcp-flow-classifier.h:101: */ remove until implemented?