[dtn] TCPCLv4 comments

[Rick Taylor <[e-mail]>]

Hi All,

Just a quick review of TCPCLv4.  Sorry it's very late.

Overall, I like it, but I have some comments which I have tried to
divide into 'issues', 'protocol queries' and 'verbiage'.

This has turned into quite a big list, but I don't think it's
insurmountable.

Cheers,

Rick


****** Issues ******

* I have a problem with TLS being optional.  As I understand it, the
major use-case of TCPCL is to forward bundles across the Internet,
which is the one network which does necessitate privacy.  Having just
lost the fight with the IESG for optional TLS in another TCP based
protocol, I get the feeling that TLS will need to be mandatory, or you
will need to have really strong reasons not to do it in this document,
which I can't currently find. 

* I am concerned the absence of an extension mechanism without a
version/document uplift.  

* I would like to see Neighbour Discovery information propagated via
TCPCL, and without an extension mechanism it really ought to be in this
version.

* There needs to be WG discussion on the benefits of back-compatibility 
of TCPCLv4 with earlier versions as it affects some of the design
decisions taken here.


****** Protocol Queries *******

* Section 3, para 7: Can you define a min/max and recommended interval for KEEPALIVE please.

* Section 4, para 2: What source port should be used for the TCP connection?  Can a node re-use an existing connection in the other direction? What happens to the negotiation phase if it can?

* Section 4.1, 'EID' field: I don't understand why you have differentiated between a Zero (Null) EID, and no EID present, and I'm not sure of the purpose of no EID - is it just for pre-TLS negotiation?

* Section 4.1, 'EID' field: Please align the EID definition with BPbis, which just uses octets, not UTF-8.

* Section 4.2, para 3: When a reversion to an earlier version occurs, is there any indication to the other peer that it could use a newer version?  A kind of Upgrade reason code?  Is that even useful?

* Section 5.1: I am worried that compressing the message type and flags into 8 bits is overly restrictive.  Perhaps 16/16 would give nice alignment (given later message layouts) and future flexibility.

* Section 5.4.2, para 3:  I'm not sure that LENGTH being informative is a good idea.  If I say 12Mb I should send 12Mb.  I believe that a TCPCL instance knows how big a bundle is at time of transmission, or are you trying to allow streaming of bundles somehow?

* Section 5.4.2:  I don't really see why the LENGTH message is a separate message with explicit rules about it immediately preceding a DATA_SEGMENT.  Why not have a DATA_START message with a 'total length' field, and a DATA_NEXT message with an 'end' flag?  (On the wire, they look almost like that already)

* Section 5.4.3: Why does the DATA_SEGMENT have a U64 length, when the total bundle length is also U64.  Why not use U32 per data segment and avoid overflow issues?  4Gb seems a reasonable maximum size for a segment.

* Section 6.1: Some discussion about TCP connection half-close states is probably worthwhile to keep the pendants happy, i.e. should I call shutdown(SD_SEND) then close()?

**** Verbiage Comments ****

* Section 2.1 para 3, 2nd sentence: 'bundle node' is suddenly mentioned
as something we should know about already.  Perhaps a reference to
BP(bis) is required, or a 'Bundle Node' paragraph in this section?

* Section 3, para 2: I would like more clarity around the use of the
BPbis Node EID, rather than an application EID, perhaps using the same
language as BPbis.  I understand what is meant here, but I'm not sure
how obvious it would be to a less familiar reader.

* Section 3, para 3: "... finally the octet range of the bundle
 data."  That seems like an odd way of saying "a bit of the bundle".
 Can you try re-phrasing?  The rest of the paragraph talks of
'segments'.  I seem to remember some discussion over the use of the
word 'chunk'?

* Section 3, para 4: "If multiple bundles are transmitted..." -> "If
multiple bundles are to be transmitted...".  Is there any restriction
on the ordering?  

* Section 3.1: We have lost the prefix 'bundle' from 'node', which I
think makes reading easier, but perhaps this needs to be consistent
with Section 2.1?

* Section 3.2, para 2: Do you need to repeat the "no interleaving" and
"pipelined ack" rules here?  You've only just said it above, and
results in seemingly normative text in two places.

* Section 3.2:  I wonder if this would make more sense in an Appendix.
 I would like to read more about the protocol before a worked example.
 Perhaps with a second more complex example?

* Section 4, para 3: the acronym BP is undefined, please replace with
the full text.

* Section 4, para 6: I would expect this paragraph to be in section
4.2.

* Section 4.1, 'magic' field: I would just say UTF-8 and remove mention
of US-ASCII for simplicity.

* Section 4.2, para 6: "... minimum of this two..." -> "...minimum of
the two..."

* Section 5.1: There seems to be a terminology mismatch between
DATA_SEGMENT and REFUSE_BUNDLE.  I would go for either
DATA_SEGMENT/REFUSE_DATA or BUNDLE_SEGMENT/REFUSE_BUNDLE, or just
REFUSE. The same applies to LENGTH.

* Section 5.2.2 (and others): I much prefer the message layout diagram
style used in Section 4.1.

* Section 5.3.2: I would move this to an Appendix.

* Sections 5.4, para 1: "All of the message in this section..." -> "All
of the messages in this section..."

* Section 5.4, para 2: This information might be more useful in one of
the earlier introduction/overview sections?

* Section 5.4, para 3: This paragraph seems out of place, perhaps merge
with the earlier rules in Section 3.2?