Re: [dtn-interest] IRSG review of draft-irtf-dtnrg-tcp-clayer-06.txt

[Dirk Kutscher <[e-mail]>]

Hi Simon,

Very good -- I agree with what you proposed in your reply -- looking forward to the new revision.

Best regards,
Dirk


> -----Original Message-----
> From: Simon Perreault [mailto:[e-mail]]
> Sent: Mittwoch, 17. Juli 2013 13:33
> To: Dirk Kutscher
> Cc: Internet Research Steering Group ([e-mail]); draft-irtf-dtnrg-tcp-
> [e-mail]; [e-mail]
> Subject: Re: IRSG review of draft-irtf-dtnrg-tcp-clayer-06.txt
> 
> Thanks for your very detailed review! I agree with almost all your proposed
> changes. I'll publish a new revision.
> 
> Le 2013-07-16 22:41, Dirk Kutscher a écrit :
> > - section 3:
> >   <comment>
> >    The protocol overview needs to say that a TCPCL can be used to
> >    transmit several bundles, but that multiple bundles MUST be
> >    transmitted consecutively (i.e., no inter-leaving of bundle
> >    segments). In addition, I think, you also have to say that the
> >    blocks of a particular bundle MUST be sent sequentially (no change
> >    of block order).
> >   </comment>
> 
> Agreed. I would also add that interleaving can be accomplished with bundle
> fragmentation.
> 
> > - section 3:
> >   <oldtext>
> >    This protocol provides bundle conveyance over a TCP connection and
> >    specifies the encapsulation of bundles as well as procedures for TCP
> >    connection setup and teardown.
> >    </oldtext>
> >
> >    <comment>
> >     "The service of this protocol is the transmission of DTN bundles
> >      over TCP. This document specifies the encapsulation of bundles,
> >      procedures for TCP setup and teardown, and a set of messages and
> >      node requirements."?
> >    </comment>
> 
> Agreed.
> 
> > - section 3.1:
> >
> >   <comment>
> >    The first paragraph is not very clear. You want to say that there
> >    are different specific messages for sending and receiving
> >    operations (in addition to connection setup/teardown).  TCPCL is
> >    symmetric, i.e., both sides can start sending data segments in a
> >    connection, and one side's bundle transfer does not have to
> >    complete before the other side can start sending data segments on
> >    its own. Hence, the protocol allows for a bi-directional mode of
> >    communication.
> >   </comment>
> 
> Agreed.
> 
> > - section 3.2:
> >
> >   <oldtext>
> >    Note that the sending node may transmit multiple
> >    DATA_SEGMENT messages without necessarily waiting for the
> >    corresponding ACK_SEGMENT responses.  This enables pipelining of
> >    messages on a channel.
> >   </oldtext>
> >
> >   <comment>
> >    This is a too important property to just mention it on the side of
> >    an example description. You should mention this in the protocol
> >    overview. You should also mention that there is no explicit flow
> >    control on the TCPCL layer.
> >   </comment>
> 
> Agreed.
> 
> > - section 3.2:
> >
> >   <oldtext>
> >     However, interleaving data segments from different bundles is not
> >     allowed.
> >   </oldtext>
> >
> >   <comment>
> >    Again, this is too important to not mention it earlier.
> >   </comment>
> 
> Agreed.
> 
> > -section 4:
> >
> >  <oldtext>
> >    The manner in which a bundle node makes the decision to establish
> >    such a connection is implementation-dependent.
> >  </oldtext>
> >
> >  <comment>
> >   I think you want to say that the implementation can decide, possibly
> >   case-by-case, at runtime (unless it is fixed). Perhaps better so
> >   say: "It is up to the implementation to decide how and when
> >   connection setup is triggered. For example..."
> >  </comment>
> 
> Agreed.
> 
> > -section 4:
> >  <oldtext>
> >    Therefore, the node MUST retry
> >    the connection setup only after some delay and it SHOULD use a
> >    (binary) exponential backoff mechanism to increase this delay in case
> >    of repeated failures.
> >  </oldtext>
> >
> >  <comment>
> >   You should specify what is meant by "some delay". Is it completely
> >   up to the implementation? Do you want to RECOMMEND a minimum
> delay?
> >  </comment>
> 
> How about recommending 1 second minimum?
> 
> > - section 4.1:
> >
> >  <oldtext>
> >    magic:  A four byte field that always contains the byte sequence 0x64
> >         0x74 0x6e 0x21, i.e., the text string "dtn!".
> >  </oldtext>
> >
> >
> >  <newtext>
> >    magic:  A four byte field that always contains the byte sequence 0x64
> >         0x74 0x6e 0x21, i.e., the text string "dtn!" in US-ASCII.
> >  </newtext>
> 
> Agreed.
> 
> > - section 4.1:
> >
> >  <oldtext>
> >    version:  A one byte field value containing the current version of
> >         the protocol.
> >  </oldtext>
> >
> >
> >  <newtext>
> >    version:  A one byte field value containing the value 3 (current version of
> >         the protocol).
> >  </newtext>
> 
> Agreed.
> 
> > - section 5.2
> >
> >  <oldtext>
> >   However, a node MUST be able to receive a bundle that has been
> >    transmitted in any segment size.
> >  </oldtext>
> >
> >  <comment>
> >
> >   How feasible is this, for example for constrained devices? Even if a
> >   node does not intend to store and reassemble bundles, there may be
> >   certain limits for the maximum size of segments that it can
> >   receive. Has there been a discussion to make this
> >   negotiable/configurable? How should a node react if it cannot
> >   receive/process any more bytes for a transmitted segment?
> >  </comment>
> 
> The intent here is not to override a node's intrinsic limitation on bundle sizes.
> The intent is that if a node is able to receive, over TCP, a bundle of size X with
> segment size Y, then it MUST also be able to do it with any segment size <= X.
> I'll adjust the text.
> 
> > - section 5.2
> >
> >  <oldtext>
> >    Following the message header, the length field is an SDNV containing
> >    the number of bytes of bundle data that are transmitted in this
> >    segment.  Following this length is the actual data contents.
> >  </oldtext>
> >
> >
> >  <comment>
> >   This should be mentioned earlier in section 5.2  </comment>
> 
> Agreed.
> 
> > - section 5.3
> >
> >   <oldtext>
> >    To transmit an acknowledgment, a node first transmits a message
> >    header with the ACK_SEGMENT type code and all flags set to zero, then
> >    transmits an SDNV containing the cumulative length of the received
> >    segment(s) of the current bundle.
> >   </oldtext>
> >
> >
> >   <newtext>
> >    To transmit an acknowledgment, a node first transmits a message
> >    header with the ACK_SEGMENT type code and all flags set to zero,
> >    then transmits an SDNV containing the cumulative length in bytes of
> >    the received segment(s) of the current bundle.
> >   </newtext>
> 
> Agreed.
> 
> > - section 5.4
> >
> >   <oldtext>
> >    Note: If a bundle transmission if aborted in this way, the receiver
> >    may not receive a segment with the 'E' flag set to '1' for the
> >    aborted bundle.  The beginning of the next bundle is identified by
> >    the 'S' bit set to '1', indicating the start of a new bundle.
> >   </oldtext>
> >
> >   <comment>
> >
> >    Should this be turned into a general rule? I.e.: "if a receiver
> >    receives a segment for a new bundle without having seen the final
> >    segment of a previous bundle, it SHOULD disregard all segments of
> >    the incompletely received bundle."
> 
> I don't think so. The received segments are valid. The receiver can do
> whatever it wants with them.
> 
> >    (wording may need some work)
> >
> >    It's probably good to use "SHOULD" here, because the node may
> >    already have forwarded the segments...
> >   </comment>
> >
> >
> > - section 6.1
> >
> >   <oldtext>
> >    The format of the shutdown message is as follows:
> >
> >                         1 1 1 1 1 1 1 1 1 1 2 2 2 2 2 2 2 2 2 2 3 3
> >     0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1
> >    +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
> >    |  0x5  |0|0|R|D| reason (opt)  | reconnection delay (opt)      |
> >    +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
> >
> >                Figure 8: Format of bundle shutdown messages
> >
> >   </oldtext>
> >
> >   <comment>
> >    Is it really efficient to have variable length messages here by
> >    making the reason code optional? How about always requiring it and
> >    then allow for a default value (0x00)?
> >   </comment>
> 
> I think it's a backward compatibility wart that we need to live with.
> That is, earlier versions of the protocol did not send those two fields.
> (not 100% sure)
> 
> > - section 6.1
> >
> >   <oldtext>
> >    If either node terminates a connection prematurely in this manner, it
> >    SHOULD send a SHUTDOWN message and MUST indicate a reason code
> unless
> >    the incoming connection did not include the magic string.
> >   </oldtext>
> >
> >   <comment>
> >    Why not send a reason like "illegal syntax"?
> >   </comment>
> 
> Wasn't there talks of a draft registering a bunch of additional reason codes?
> 
> > - section 6.7
> >
> >   <comment>
> >
> >   Regarding the peer entity authentication issue, I think you have to
> >   use a stronger statement, like "a node SHALL NO" use the endpoint
> >   identifier conveyed in the TCPCL connection message to derive a peer
> >   node's entity unless it can ascertain it via other means."
> 
> Agreed.
> 
> >   If it is possible to do that through the Bundle Security Protocol,
> >   you could be a bit more specific on how the Bundle Authentication
> >   Block would be used. I assume one TCPCL peer node would become a
> >   security source in an RFC 6257 sense?
> >
> >   BTW, you are using the term "Bundle Authentication Header" -- I
> >   think it's "Bundle Authentication Block".
> >
> >   In general, it could be worthwhile stating that TCPCL does not
> >   provide any security services and that RFC 6257's mechanisms should
> >   be used.
> 
> Agreed.
> 
> >   BTW, is the use of TLS for TCPCL defined or excluded? You may want
> >   to say something about why it's not applicable
> >   </comment>
> 
> Why wouldn't it be applicable? I think we *want* it to be applicable!
> 
> > - section 10.2
> >
> >   <oldtext>
> >    [RFC6256]  Eddy, W. and E. Davies, "Using Self-Delimiting Numeric
> >               Values in Protocols", RFC 6256, May 2011.
> >   </oldtext>
> >
> >   <comment>
> >     Isn't that a normative reference for this spec?
> >   </comment>
> 
> No, because that's an informational RFC.
> 
> > EDITORIAL COMMENTS
> > ------------------
> >
> > General:
> >
> > - make usage and capitalization of Bundle Protocol (sometimes referred
> >   to as "Bundling Protocol", "bundle protocol" etc.) consistent
> 
> Agreed.
> 
> > - section 2.1
> >    <oldtext>
> >      Bundle payload --  A bundle payload (or simply "payload") is the
> >         application data whose conveyance to the bundle's destination is
> >         the purpose for the transmission of a given bundle.
> >    </oldtext>
> >
> >    <comment>
> >      IMO this can be said in a clearer way: "... application
> >      data that is transmitted to a BP application at a Bundle
> >      Endpoint"?
> >    </comment>
> 
> We should just remove this and refer to RFC 5050 section 3.1.
> 
> > - section 2.1:
> >    <oldtext>
> >    Bundle node --  A bundle node (or simply a "node") is any entity that
> >         can send and/or receive bundles.  The particular instantiation
> >         of this entity is deliberately unconstrained, allowing for
> >         implementations in software libraries, long-running processes,
> >         or even hardware.  One component of the bundle node is the
> >         implementation of a convergence layer adapter.
> >    </oldtext>
> >
> >    <comment>
> >      I don't think the second sentence ("The particular
> >      instantiation...") is really needed.  This holds for most network
> >      elements these days.
> >    </comment>
> 
> We should just remove this and refer to RFC 5050 section 3.1.
> 
> > - section 2.1:
> >    <oldtext>
> >    Convergence layer adapter --  A convergence layer adapter (CLA) sends
> >         and receives bundles utilizing the services of some 'native'
> >         link, network, or internet protocol.  This document describes
> >         the manner in which a CLA sends and receives bundles when using
> >         the TCP protocol for inter-node communication.
> >    </oldtext>
> >
> >    <comment>
> >       Does it make make sense to define "Convergence Layer" first,
> >       before defining CLA? Is the CLA definition needed at all
> >       (assuming CL is defined)?
> >    </comment>
> 
> We should just remove this and refer to RFC 5050 section 3.1.
> 
> > - section 2.2:
> >   <oldtext>
> >    TCPCL Connection --  [...] The lifetime of a TCPCL connection is one-to-one
> >         with the lifetime of an underlying TCP connection.
> > 	[...]
> >    </oldtext>
> >
> >    <comment>
> >       "The lifetime of a TCPCL connection is bound to the
> >       lifetime of the underlying TCP connection."?
> >    </comment>
> 
> Agreed.
> 
> > -section 4:
> >
> >  <oldtext>
> >    Once a TCP connection is established, each node MUST immediately
> >    transmit a contact header over the TCP connection.  The format of the
> >    contact header is described in Section 4.1).
> >  </oldtext>
> >
> >  <newtext>
> >    Once a TCP connection is established, each node MUST immediately
> >    transmit a contact header over the TCP connection.  The format of the
> >    contact header is described in Section 4.1.
> >  </newtext>
> 
> Agreed.
> 
> > - section 4.1:
> >
> >  <oldtext>
> >    local EID:  An octet string containing the EID of some singleton
> >         endpoint in which the sending node is a member, in the canonical
> >         format of <scheme name>:<scheme-specific part>.  A eight byte
> >         EID is shown the clarity of the figure.
> >  </oldtext>
> >
> >
> >  <newtext>
> >    local EID:  An octet string containing the EID of some singleton
> >         endpoint in which the sending node is a member, in the canonical
> >         format of <scheme name>:<scheme-specific part>.  A eight byte
> >         EID is shown for clarity of the figure.
> >  </newtext>
> 
> Agreed.
> 
> > - section 4.2:
> >
> >
> >  <oldtext>
> >         The reactive fragmentation enabled parameter is set to true iff
> >         the corresponding flag is set in both contact headers.
> >
> >         The bundle refusal capability may only be used iff both peers
> >         indicate support for it in their contact header and segment
> >         acknowledgement has been enabled.
> >  </oldtext>
> >
> >
> >  <newtext>
> >         The reactive fragmentation enabled parameter is set to true iff
> >         the corresponding flag is set in both contact headers.
> >
> >         The bundle refusal capability is set to true if the
> >         corresponding flag is set in both contact headers and if
> >         segment acknowledgment has been enabled.
> >  </newtext>
> >
> >  <comment>
> >   I'd raher use the same or similar wording to not irritate the implementer.
> >  </comment>
> 
> Agreed.
> 
> > - section 5.1
> >
> >  <oldtext>
> >   Table 2: TCPCL Header Types
> >  </oldtext>
> >
> >
> >  <newtext>
> >   Table 2: TCPCL Message Types
> >  </newtext>
> 
> Agreed.
> 
> > - section 5
> >
> >   <comment>
> >    For the different subsections for the individual messages, please
> >    use the message type names from table 2 in the headings, for
> >    example: "Bundle Data Transmission (DATA_SEGMENT)". Also, I'd
> >    recommend to align the order of the subsections with the order of
> >    message types in table 2.
> >   </comment>
> 
> Agreed.
> 
> >   <comment>
> >    I would also recommend to use a consistent structure for each of
> >    the subsections, i.e., perhaps
> >      - purpose
> >      - syntax
> >      - procedures / node requirements
> >   </comment>
> 
> Agreed.
> 
> > - section 5.3
> >
> >   <oldtext>
> >    If so, then
> >    the receiver MUST transmit a bundle acknowledgment header when it
> >    successfully receives each data segment.
> >   </oldtext>
> >
> >
> >   <oldtext>
> >    If so, then
> >    the receiver MUST transmit a bundle acknowledgment message when it
> >    successfully receives each data segment.
> >   </oldtext>
> 
> Agreed.
> 
> > - section 5.6
> >
> >   <oldtext>
> >    If no message (keepalive or other) has been received for at least
> >    twice the keepalive interval, then either party may terminate the
> >    session by transmitting a one byte message type code of SHUTDOWN (as
> >    described in Table 2) and closing the TCP connection.
> >   </oldtext>
> >
> >
> >   <newtext>
> >    If no message (keepalive or other) has been received for at least
> >    twice the keepalive interval, then either party MAY terminate the
> >    session by transmitting a one byte SHUTDOWN message  (as
> >    described in Table 2) and by closing the TCP connection.
> >   </newtext>
> 
> Agreed.
> 
> > - section 6.1
> >
> >   <oldtext>
> >    In case acknowledgments have been
> >    negotiated, it is advisable to acknowledge all received data segments
> >    first and then shut down the connection.
> >   </oldtext>
> >
> >
> >   <newtext>
> >    In case acknowledgments have been
> >    negotiated, a node SHOULD  acknowledge all received data segments
> >    first and then shut down the connection.
> >   </newtext>
> 
> Agreed.
> 
> > - section 6.1
> >
> >   <oldtext>
> >    This may, for example, be used to notify
> >    that the node is currently not capable of or willing to communicate.
> >   </oldtext>
> >
> >   <newtext>
> >    This may, for example, be used to notify
> >    that the node is currently not able or willing to communicate.
> >   </newtext>
> 
> Agreed.
> 
> > - section 10.2
> >
> >   <oldtext>
> >    [refs.dtnsecurity]
> >               Symington, S., Farrell, S., and H. Weiss, "Bundle Security
> >               Protocol Specification", Internet Draft, work in progress
> >               draft-irtf-dtnrg-bundle-security-03.txt, April 2007.
> >   </oldtext>
> >
> >   <comment>
> >      This is RFC 6257.
> >   </comment>
> 
> Agreed.
> 
> Simon