Subject: Re: [dtn] I-D Action: draft-ietf-dtn-tcpclv4-05.txt

From: Marco Kaulea <[email-masked]>
Date: Tue, 16 Jan 2018 21:19:32 +0100

Hi All,
My comments are addressed in the new version.
But while checking I found two different points:
    1. 4.3 Paragraph 3 Spelling Mistake: TLS nqion -> TLS negotiation

    2. 4.1 Page 11, Parser Ambiguity: The Header Extension Length could
differ from the length of the present extension headers,
        for example when the length of the final extension header exceeds
the remaining length as specified by the Header Extension Length field.
        It should maybe clarified that this is invalid and the contact
header should be discarded.

Regards, Marco

On Sat, Dec 30, 2017 at 7:48 PM, Marc Blanchet <[email-masked]>
wrote:

> Thanks Brian,
> Would the reviewers confirm that this new version address their comments?
>
> Regards, Marc.
>
> On 29 Dec 2017, at 15:26, Brian Sipos wrote:
>
> All,
>
> This message is late in coming. The last submitted draft of the TCPCLv4
> spec. is intended to address all of the comments received to-date.
> Specifically, those from Victoria Pritchard, Marius Feldmann, Matt
> Wronkiewicz, and Marco Kaulea. The latest two draft revisions have been
> correcting logical and typographical issues, and dealing with edge cases;
> the changes have not affected the nominal sequencing of the protocol or the
> message contents.
>
> At this point, any further comments are welcome but I believe that the
> current spec. is implementable, in a robust way, as-written and provides a
> path for nodes to migrate from similarly-scoped TCPCLv3 implementations.
>
>

---

From: "Marc Blanchet" <[email-masked]>
Date: Tue, 16 Jan 2018 15:27:20 -0500

thanks. Victoria?

Marc.

---

From: Victoria Pritchard <[email-masked]>
Date: Thu, 18 Jan 2018 00:17:48 +0000

Hi all,

I'm not sure all my comments have been addressed...I was hoping to see more
text added to the draft which answered some of my questions. I've emailed a
response to the previous thread, and done a new review for draft 05. This
includes all the fussy comments, which I apologise for in advance, but
hopefully they are helpful in the end:

Abstract:
"Specifically, the TCPCLv4 uses CBOR-encoded BPv7 bundles
   as its service data unit being transported and provides a reliable
   transport of such bundles."
Not sure why this needs to be here...does TCPCL need to know it is CBOR
encoded BPv7 bundles? Isn't bundle data opaque?

Section 1
store-and- forward  - extra space before "forward"
a "convergence- layer adapter" - extra space before "layer"

2.1
"This section contains definitions that are interpreted to be specific
   to the operation of the TCPCL protocol, as described below."
That's a lot of words...how about:
"This section contains definitions specific to the TCPCL protocol:"

"TCPCL Node:  A TCPCL node refers to "
I think "A TCPCL node refers to" is unnecessary. Same for other definitions.
an negotiating - a negotiating

"Therefore, a TCPCL session is
      initiated after a bundle node establishes a TCP connection to for
      the purposes of bundle communication. "
This sentence ("to for") doesn't make sense. Also, I think this much detail
is not necessary in the definitions. It should be enough to say that the
TCPCL session is built on top of a TCP connection and cannot exist without
an established TCP connection.

"the procedures and mechanisms
      (described below) "
I don't think the text in brackets is required. Also not sure the text on
Transfer ID is needed here.

>From the definitions, a session is lively when idle. Is that the intended
definition? Also, the word "lively" is not used elsewhere. The word
liveliness (liveliqness) is only used once.

Reason codes:
I feel like the definitions section is not the place to justify why reason
codes are used. The first sentence in this definition should be enough.

3.1 - 2nd paragraph
"The
   choice of the length to use for segments is an implementation matter,
   but each segment must be no larger than the receiving node's maximum
   receive unit (MRU) (see the field "Segment MRU" of Section 4.1).  The
   first segment for a bundle MUST set the 'START' flag, and the last
   one MUST set the 'end' flag in the XFER_SEGMENT message flags."
I think this is too much detail for the overview. If you want, you could
mention that the first and last segment are marked, but I wouldn't give
specifics about flags and segment MRU.

3.1 - 3rd paragraph
For the overview, probably could rephrase this:
"Multiple bundles can be transmitted consecutively on a single TCPCL
connection. Segments from different bundles are never interleaved."
Followed by the sentence starting "Bundle interleaving".

3.1 - 4th paragraph
"A feature of this protocol is for the receiving node to send
   acknowledgments as bundle data segments arrive (XFER_ACK)."
The "(XFER_ACK)" should probably go after the word 'acknowledgments'.
The text starting "For each data segment that is received," is again too
much detail for the overview. I think the point you want to make here is
what acks are used for...The way this paragraph is worded, the sole reason
for acknowledgments is to trigger bundle fragmentation.

However it is not clear what process has responsibility for fragmentation.
I think it would be worth saying that "if the session is interrupted, the
BP can perform reactive fragmentation to avoid re-sending"...
However, in your response to previous comments, you said "In BPbis there is
no longer the concept of a CL-supplied transfer fragment information." Does
that mean TCPCL wont pass on information to BP on how much of a bundle was
received? In which case, what is the point in acknowledgments?

3.1 - 5th paragraph
Again I think this should be reworded for the overview:
"Transmission of a bundle can be interrupted by the receiver at any time by
sending a Transfer Refuse (XFER_REFUSE) message. This enables"...

3.1 - 6th paragraph
liveliqness - typo

3.1 - 7th paragraph
Again, can be simplified for the overview.
"A SHUTDOWN message is sent to close the TCPCL session (see Section 6.1).
In-progress transfers can be completed but no further transfers are
initiated."
With XFER_REFUSE, you mention cases when this may be used. Is it worth also
doing this when you say you can use SHUTDOWN as a refusal for session setup?

3.1 - 8th paragraph
Seems like this would work better before paragraph 7.

3.2
What does TCPCL do with received ACKs? What happens if it has moved on to
sending another bundle but ACKs never come back for the first one?

4
"(49152--65535)" - are there meant to be two dashes? Also not sure this
last sentence in the 2nd paragraph is needed.

"The default initial delay
   SHOULD be at least 1 second"
This point is made twice in this paragraph, one with SHOULD and one with
RECOMMENDED.

'MAY decide to re-attempt', and 'MUST retry after some delay', I think
could be phrased better. Maybe something like "An implementation MAY decide
to re-attempt to establish the connection, but any retry attempt MUST occur
after a delay, so that the target is not overwhelmed with repeated
connection attempts."

If the TCPCL cannot complete the bundle transfer, is there feedback to BP?
The draft says "The node MAY declare failure". Should it be a MUST or a
SHOULD? Also, should it be more specific? e.g. "TCPCL MAY declare failure
to the higher layer (i.e. the BP)". I still think it should be clear what
the TCPCL is responsible for (and what it is not responsible for), so when
the draft says "the node MAY", I'd prefer it was clear if that was
something TCPCL MAY be expected to do, e.g. the bit about finding an
alternate route? Also, does this bit about declaring failure only apply
during session establishment, or in general (in which case it shouldn't be
in this section)?

After the reference to Section 4.2, there should be a full stop.

In the last paragraph in Section 4, if it sends a SHUTDOWN instead, does it
also close the TCP connection? Which side closes?
"After receiving the contact header from the other node, either node
   MAY also refuse the session by sending a SHUTDOWN message.  If
   session setup is refused, a reason MUST be included in the SHUTDOWN
   message."
Could be updated to:
"After receiving the contact header from the other node, either node
   MAY also refuse the session by sending a SHUTDOWN message with a reason
code."

I would move the TCP specific parts of this section 4 into a "TCP
connection" sub-section as 4.1.

Section 4.1
"Header Extension Length Header Extension Items" - missing an 'and'?
"The encoding of each Header Extension Item is
      identical form " - This doesn't make sense to me.

Section 4.1.1
Should be consistent on either capital or lowercase i for Item.

Under Flags, "an node" -> "a node"
"an extensions" -> "an extension's"
"until the a full contact exchange" - don't need 'the' and 'a'

Figure 4 caption
"Extention " -> "Extension"

Section 4.2
This might work nicely as bullet points or a numbered list.

I would include all the session parameters here, I'd consider
transmit_segment_MRU, keepalive_send_interval, tls_enabled, etc. to be
session parameters too.

"the node MAY either terminate the session (with a
   reason code of "Version mismatch"). "
This sentence should continue with an "or" and a second half instead of a
full stop and the new sentence beginning with "Otherwise".

The notes about keepalive being zero should go in the "established session
operation" section 5.2.1. Here you only need to say to take the minimum.

For the TLS section, I would put the bit about local security policy after
the first sentence. I also think the wording here is unnecessarily
confusing...It could say
"Enable TLS: Negotiation of the Enable TLS parameter is performed by
      taking the logical AND of the two contact headers' CAN_TLS flags. A
local security policy is then applied to determine if the negotiated value
of Enable TLS is acceptable. If not, the node SHALL shutdown the session
with reason code of "Contact Failure" Note that this is different from a
"TLS Failure" after an agreed-upon Enable TLS state).

Section 4.3
"within each TLS nqion " - typo.

I find this a bit jumbled, and I suggest re-organising as follows:
"
4.3.  Session Security

   This version of the TCPCL supports establishing a Transport Layer
   Security (TLS) session within an existing TCP connection. When TLS is
used it affects the entire TCPCL session. There is no mechanism provided to
downgrade the session to a non-TLS session. If this is desired, the entire
TCPCL session MUST be shutdown and a new non-TLS session established.

   TLS is negotiated using the Contact Header as described in Section 4.2.
   After negotiating an Enable TLS parameter of true, the session nodes
   SHALL begin a TLS handshake in accordance with [RFC5246].  The
   parameters within each TLS session are implementation dependent but any
   TCPCL node SHOULD follow all recommended best practices of [RFC7525]. By
convention, TCPCL uses the node which initiated the underlying TCP
connection as the "client" role of the TLS handshake request.

   The TLS handshake is considered to be part of the TCPCL session
establishment. Specifics about sensitive data exposure are discussed in
Section 7.

4.3.1.  TLS Handshake Result

   If a TLS handshake cannot negotiate a TLS session, both nodes of the
   TCPCL session SHALL start a TCPCL shutdown with reason "TLS Failure".

   After a TLS session is successfully established, both TCPCL nodes
   SHALL re-exchange TCPCL Contact Header messages.  Any information
   cached from the prior Contact Header exchange SHALL be discarded.
   This re-exchange avoids man-in-the-middle attack in identical fashion
   to [RFC2595].  Each re-exchange header CAN_TLS flag SHALL be
   identical to the original header CAN_TLS flag from the same node.
   The CAN_TLS logic (TLS negotiation) SHALL NOT apply during header re-
   exchange.  This reinforces the fact that there is no TLS downgrade
   mechanism.
 "
...end of suggested text

Table 3 - Section 5.3.5 is not a linked reference like all the other
references.

5.2.1
I would simplify the second paragraph to "The Keepalive Interval is
negotiated during session establishment as described in Section..."

In this bit:
"either party MAY terminate the
   session by transmitting a one-octet SHUTDOWN message (as described in
   Section 6.1) with reason code "Idle Timeout", and by closing the
   session."
Is the final 'session' in this sentence the TCP session? Or should that be
"TCP connection"?

5.3
"All of the message" - messages

"This requirement is
   imposed on the agent using the TCPCL rather than TCPCL itself."
Not sure how to interpret this. Surely it's TCPCL which creates the
segments, and knows which bundle the data came from, and therefore it's
TCPCL which ensures only data from a single segment goes into a bundle?

5.3.1
"a Transfer ID number" - dont need the word "number"?

"used to correlate messages originating from sender and
   receiver of a bundle" - is it better to say "correlate messages and
acknowledgements about the transfer of the bundle"

"Resource Exhaustion" doesn't appear in table 13.

5.3.2
"Total Bundle Length" - consistency with capital letters

The wording of the sentence that mentions the 'START' bit is strange. Are
the XFER_SEGMENT messages created before the XFER_INIT? Or is this
paragraph meant to emphasise that an XFER_INIT cannot be sent until the
previous transfer is finished?

The last paragraph should go into the XFER_REFUSE section. Also this is the
only time BUNDLE_REFUSE is used.

5.3.4
"a Bundle Protocol agent" - should that be "the TCPCL"? Does TCPCL, once it
receives an ack, pass this information to a higher layer?

"endpoing" - typo

I still don't understand why you would send segments of varying sizes, the
examples of 100, 200, 500, 1000 are given. As a sender, you only know the
maximum segment MRU from the contact header. Is there a reason the sender
might choose to vary sizes of consecutive segments?

Still don't understand what you do with acks, or what you do when an
expected ack is not received...

5.3.5
"an mechanism" - should be "a mechanism"

"bundle header or any bundle data is inspected" - does this require a
signal from a higher layer? TCPCL wouldn't look into the bundle header or
data since it treats it as opaque.

The "Note:" at the end - Wouldn't the beginning of the next bundle actually
be marked by an XFER_INIT before you see an XFER_SEGMENT with START bit
set?

6.1
"an node" -> "a node"

The first paragraph says after sending a SHUTDOWN, the sender may send
further acks, but also says it should send acks before sending the
SHUTDOWN, and then the second paragraph says once the SHUTDOWN is sent, any
further data received on the TCP connection should be ignored. So it should
ignore any new segments received or any other kind of message the remote
peer sends, but it can ack any that were received before the shutdown was
sent?

Is reconnection delay in seconds? should say that in the field description.
"delay until further TCPCL sessions" -> "delay before reattempting to
establish a TCPCL session"?

The fields describe the reason code, then the paragraph after table 7 says
it's possible to convey additional information, but it means the reason
code, so I think this makes more sense if you remove the word "additional",
or change it to "optional".

The sentence beginning "In contrast" is probably unnecessary.

The next paragraph states that a session shutdown can occur immediately
after TCP connection establishment, then in the same paragraph says that
the contact header must always be sent... is it better to say the shutdown
can occur after contact header exchange?

Also, the next paragraph says "If either node terminates a session
prematurely in this manner, it
   SHOULD send a SHUTDOWN message"
Does "in this manner" mean prematurely straight after TCP establishment? If
so, don't start a new paragraph.

Overall in these 2 paragraphs there's a bad mixture of MAY and SHOULD and
MUST, and it's confusing. It could be simplified like this:
"A session shutdown MAY occur immediately after contact header exchange
(prior to any further data exchange).  This MAY, for example, be used to
notify that the node is currently not able or willing to communicate. If
the magic string was not present in the contact header, a node SHOULD close
the TCP connection without sending a SHUTDOWN message. Otherwise, a node
SHOULD send a SHUTDOWN message. If a SHUTDOWN message is sent, it MUST
indicate a reason code. "

The text about the 'D' bit at the bottom of page 27 is already described
underneath Table 8, so can be removed.

The final paragraph here says about terminating before a message has
completed being sent, but elsewhere it says you cannot interrupt the
sending of a message. This is confusing. I think it might be better to
start with the final sentence:
"If a large XFER_SEGMENT is currently being transmitted but a session
SHUTDOWN is required, a SHUTDOWN message cannot be sent until the entire
XFER_SEGMENT is transmitted. However, in this case, the TCP connection can
be terminated mid-XFER_SEGMENT to achieve the TCPCL session SHUTDOWN
without waiting for the large segment transmission to complete."

Am I right in thinking only one side needs to send a SHUTDOWN, and it can
then close the TCP connection when it likes? And after it sends a SHUTDOWN,
it can still send acks, but must ignore anything it receives?

7
"security mandated" -> "security is mandated"
endpoing -> endpoint

8.3, 8.4, 8.5, 8.6, 8.7
initialized -> initialize

9.
memo doesnt seem like the right word.

Hope this is helpful,

Kind regards,
Victoria.

---

From: Rick Taylor <[email-masked]>
Date: Mon, 22 Jan 2018 14:17:24 +0000

Brian,

Are you intending to address Vicky's other comments, or do you think
the text is good as it stands?  What is the opinion of other WG
members?

Can I remind people that TCP-CL is one of the critical documents that
must leave the working group before we continue on to other work.

Cheers,

Rick

On Thu, 2018-01-18 at 00:17 +0000, Victoria Pritchard wrote:
> Hi all,
>
> I'm not sure all my comments have been addressed...I was hoping to
> see more text added to the draft which answered some of my questions.
> I've emailed a response to the previous thread, and done a new review
> for draft 05. This includes all the fussy comments, which I apologise
> for in advance, but hopefully they are helpful in the end:
>
> Abstract:
> Not sure why this needs to be here...does TCPCL need to know it is
> CBOR encoded BPv7 bundles? Isn't bundle data opaque?
>
> Section 1
> This sentence ("to for") doesn't make sense. Also, I think this much
> detail is not necessary in the definitions. It should be enough to
> say that the TCPCL session is built on top of a TCP connection and
> cannot exist without an established TCP connection.
>
> I don't think the text in brackets is required. Also not sure the
> text on Transfer ID is needed here.
>
> From the definitions, a session is lively when idle. Is that the
> intended definition? Also, the word "lively" is not used elsewhere.
> The word liveliness (liveliqness) is only used once.
>
> Reason codes:
> I feel like the definitions section is not the place to justify why
> reason codes are used. The first sentence in this definition should
> be enough.
>
> 3.1 - 2nd paragraph
> "The
> I think this is too much detail for the overview. If you want, you
> could mention that the first and last segment are marked, but I
> wouldn't give specifics about flags and segment MRU.
>
> 3.1 - 3rd paragraph
> The text starting "For each data segment that is received," is again
> too much detail for the overview. I think the point you want to make
> here is what acks are used for...The way this paragraph is worded,
> the sole reason for acknowledgments is to trigger bundle
> fragmentation.
>
> However it is not clear what process has responsibility for
> fragmentation. I think it would be worth saying that "if the session
> is interrupted, the BP can perform reactive fragmentation to avoid
> re-sending"...
> However, in your response to previous comments, you said "In BPbis
> there is no longer the concept of a CL-supplied transfer fragment
> information." Does that mean TCPCL wont pass on information to BP on
> how much of a bundle was received? In which case, what is the point
> in acknowledgments?
>
> 3.1 - 5th paragraph
> "Transmission of a bundle can be interrupted by the receiver at any
> time by sending a Transfer Refuse (XFER_REFUSE) message. This
> enables"...
>
> 3.1 - 6th paragraph
> liveliqness - typo
>
> 3.1 - 7th paragraph
> "A SHUTDOWN message is sent to close the TCPCL session (see Section
> 6.1). In-progress transfers can be completed but no further transfers
> are initiated."
> With XFER_REFUSE, you mention cases when this may be used. Is it
> worth also doing this when you say you can use SHUTDOWN as a refusal
> for session setup?
>
> 3.1 - 8th paragraph
> What does TCPCL do with received ACKs? What happens if it has moved
> on to sending another bundle but ACKs never come back for the first
> one?
>
> 4
> "(49152--65535)" - are there meant to be two dashes? Also not sure
> this last sentence in the 2nd paragraph is needed.
>
> This point is made twice in this paragraph, one with SHOULD and one
> with RECOMMENDED.
>
> 'MAY decide to re-attempt', and 'MUST retry after some delay', I
> think could be phrased better. Maybe something like "An
> implementation MAY decide to re-attempt to establish the connection,
> but any retry attempt MUST occur after a delay, so that the target is
> not overwhelmed with repeated connection attempts."
>
> If the TCPCL cannot complete the bundle transfer, is there feedback
> to BP? The draft says "The node MAY declare failure". Should it be a
> MUST or a SHOULD? Also, should it be more specific? e.g. "TCPCL MAY
> declare failure to the higher layer (i.e. the BP)". I still think it
> should be clear what the TCPCL is responsible for (and what it is not
> responsible for), so when the draft says "the node MAY", I'd prefer
> it was clear if that was something TCPCL MAY be expected to do, e.g.
> the bit about finding an alternate route? Also, does this bit about
> declaring failure only apply during session establishment, or in
> general (in which case it shouldn't be in this section)?
>
> In the last paragraph in Section 4, if it sends a SHUTDOWN instead,
> does it also close the TCP connection? Which side closes?
>    MAY also refuse the session by sending a SHUTDOWN message with a
> reason code."
>
> transmit_segment_MRU, keepalive_send_interval, tls_enabled, etc. to
> be session parameters too.
>
> This sentence should continue with an "or" and a second half instead
> of a full stop and the new sentence beginning with "Otherwise".
>
> The notes about keepalive being zero should go in the "established
> session operation" section 5.2.1. Here you only need to say to take
> the minimum.
>
> For the TLS section, I would put the bit about local security policy
> after the first sentence. I also think the wording here is
> unnecessarily confusing...It could say
>       taking the logical AND of the two contact headers' CAN_TLS
> flags. A local security policy is then applied to determine if the
> negotiated value of Enable TLS is acceptable. If not, the node SHALL
> shutdown the session with reason code of "Contact Failure" Note that
> this is different from a "TLS Failure" after an agreed-upon Enable
> TLS state).
>
> Section 4.3
>    Security (TLS) session within an existing TCP connection. When TLS
> is used it affects the entire TCPCL session. There is no mechanism
> provided to downgrade the session to a non-TLS session. If this is
> desired, the entire TCPCL session MUST be shutdown and a new non-TLS
> session established.
>
>    TCPCL node SHOULD follow all recommended best practices of
> [RFC7525]. By convention, TCPCL uses the node which initiated the
> underlying TCP connection as the "client" role of the TLS handshake
> request.
>
> establishment. Specifics about sensitive data exposure are discussed
> in Section 7.
>
> Is the final 'session' in this sentence the TCP session? Or should
> that be "TCP connection"?
>
> 5.3
> segments, and knows which bundle the data came from, and therefore
> it's TCPCL which ensures only data from a single segment goes into a
> bundle?
>
> 5.3.1
>    receiver of a bundle" - is it better to say "correlate messages
> and acknowledgements about the transfer of the bundle"
>
> The wording of the sentence that mentions the 'START' bit is strange.
> Are the XFER_SEGMENT messages created before the XFER_INIT? Or is
> this paragraph meant to emphasise that an XFER_INIT cannot be sent
> until the previous transfer is finished?
>
> The last paragraph should go into the XFER_REFUSE section. Also this
> is the only time BUNDLE_REFUSE is used.
>
> 5.3.4
> "a Bundle Protocol agent" - should that be "the TCPCL"? Does TCPCL,
> once it receives an ack, pass this information to a higher layer?
>
> "endpoing" - typo
>
> I still don't understand why you would send segments of varying
> sizes, the examples of 100, 200, 500, 1000 are given. As a sender,
> you only know the maximum segment MRU from the contact header. Is
> there a reason the sender might choose to vary sizes of consecutive
> segments?
>
> signal from a higher layer? TCPCL wouldn't look into the bundle
> header or data since it treats it as opaque.
>
> The "Note:" at the end - Wouldn't the beginning of the next bundle
> actually be marked by an XFER_INIT before you see an XFER_SEGMENT
> with START bit set?
>
> 6.1
> "an node" -> "a node"
>
> The first paragraph says after sending a SHUTDOWN, the sender may
> send further acks, but also says it should send acks before sending
> the SHUTDOWN, and then the second paragraph says once the SHUTDOWN is
> sent, any further data received on the TCP connection should be
> ignored. So it should ignore any new segments received or any other
> kind of message the remote peer sends, but it can ack any that were
> received before the shutdown was sent?
>
> The fields describe the reason code, then the paragraph after table 7
> says it's possible to convey additional information, but it means the
> reason code, so I think this makes more sense if you remove the word
> "additional", or change it to "optional".
>
> The next paragraph states that a session shutdown can occur
> immediately after TCP connection establishment, then in the same
> paragraph says that the contact header must always be sent... is it
> better to say the shutdown can occur after contact header exchange?
>
> Does "in this manner" mean prematurely straight after TCP
> establishment? If so, don't start a new paragraph.
>
> Overall in these 2 paragraphs there's a bad mixture of MAY and SHOULD
> and MUST, and it's confusing. It could be simplified like this:
> "A session shutdown MAY occur immediately after contact header
> exchange (prior to any further data exchange).  This MAY, for
> example, be used to notify that the node is currently not able or
> willing to communicate. If the magic string was not present in the
> contact header, a node SHOULD close the TCP connection without
> sending a SHUTDOWN message. Otherwise, a node SHOULD send a SHUTDOWN
> message. If a SHUTDOWN message is sent, it MUST indicate a reason
> code. "
>
> The text about the 'D' bit at the bottom of page 27 is already
> described underneath Table 8, so can be removed.
>
> sending of a message. This is confusing. I think it might be better
> to start with the final sentence:
> SHUTDOWN is required, a SHUTDOWN message cannot be sent until the
> entire XFER_SEGMENT is transmitted. However, in this case, the TCP
> connection can be terminated mid-XFER_SEGMENT to achieve the TCPCL
> session SHUTDOWN without waiting for the large segment transmission
> to complete."
>
> Am I right in thinking only one side needs to send a SHUTDOWN, and it
> can then close the TCP connection when it likes? And after it sends a
> SHUTDOWN, it can still send acks, but must ignore anything it
> receives?
>
> 7
> On 16 January 2018 at 20:27, Marc Blanchet <[email-masked]
> > wrote:
> > thanks. Victoria?
> > Marc.
> >     2. 4.1 Page 11, Parser Ambiguity: The Header Extension Length
> > could differ from the length of the present extension headers,
> >         for example when the length of the final extension header
> > exceeds the remaining length as specified by the Header Extension
> > Length field.
> >         It should maybe clarified that this is invalid and the
> > contact header should be discarded.
> >
> > Regards, Marco
> >
> >
> > On Sat, Dec 30, 2017 at 7:48 PM, Marc Blanchet <marc.blanchet@viage
> > nie.ca> wrote:
> > > Thanks Brian,
> > > This message is late in coming. The last submitted draft of the
> > > TCPCLv4 spec. is intended to address all of the comments received
> > > to-date. Specifically, those from Victoria Pritchard, Marius
> > > Feldmann, Matt Wronkiewicz, and Marco Kaulea. The latest two
> > > draft revisions have been correcting logical and typographical
> > > issues, and dealing with edge cases; the changes have not
> > > affected the nominal sequencing of the protocol or the message
> > > contents.
> > > At this point, any further comments are welcome but I believe
> > > that the current spec. is implementable, in a robust way, as-
> > > written and provides a path for nodes to migrate from similarly-
> > > scoped TCPCLv3 implementations.
> > >
> >
> >
>
> _______________________________________________
> dtn mailing list
> [email-masked]
> https://www.ietf.org/mailman/listinfo/dtn

---

From: "Templin, Fred L" <[email-masked]>
Date: Tue, 23 Jan 2018 21:58:57 +0000

Hi, I read the document and overall it looks very good. See below
for comments:

Thanks - Fred

---

1) The abstract calls it "Bundle Protocol, version 7 (BPv7)"
   but the rest of the document calls it BP. Should it be BPv7
   everywhere, or just BP? Suggestion: add a discussion of BP
   versions in the introduction and tell how TCPCL applies to
   the different versions, and then just use "BP" throughout
   the rest of the document only noting version number
   differences as necessary.

2) The abstract calls it "TCPCLv4" but the rest of the document
   for the most part calls it TCPCL. Should it be TCPCLv4
   everywhere, or just TCPCL? Suggestion: add a discussion of
   TCPCL versions in the introduction, and then just use "TCPCL"
   throughout the rest of the document only noting version
   number differences as necessary.

3) Introduction, second paragraph, citing [I-D.ietf-dtn-bpbis]
   in two consecutive sentences seems repetitive and therefore
   unnecessary. Suggest changing the second sentence to say:

     "BPv7 requires the services of a "convergence- layer adapter"
     (CLA) to send and receive bundles using the service of some
     "native" link, network, or Internet protocol."

4) Section 5.3, "Transfer ID" identifies the bundle to be
   transferred. How are all of the Segments of the bundle
   identified? Doesn't there need to be a segment offset
   value that tells the byte offset from the beginning of
   the bundle? Section 5.3.3 says:

    "Once a transfer of a bundle has commenced, the node MUST
     only send segments containing sequential portions of that
     bundle until it sends a segment with the 'END' bit set."

   but this does not seem to make any provisions for out of
   order delivery or segment loss. Is the idea to take
   advantage of TCP's reliable in-order delivery properties?

5) A few minor spelling and grammatical errors that will be
   flagged and corrected by the RFC editor.

> -----Original Message-----
> From: I-D-Announce [mailto:[email-masked]] On Behalf Of [email-masked]
> Sent: Thursday, December 14, 2017 5:43 PM
> To: [email-masked]
> Cc: [email-masked]
> Subject: I-D Action: draft-ietf-dtn-tcpclv4-05.txt
> 
> 
> A New Internet-Draft is available from the on-line Internet-Drafts directories.
> This draft is a work item of the Delay/Disruption Tolerant Networking WG of the IETF.
> 
>         Title           : Delay-Tolerant Networking TCP Convergence Layer Protocol Version 4
>         Authors         : Brian Sipos
>                           Michael Demmer
>                           Joerg Ott
>                           Simon Perreault
> 	Filename        : draft-ietf-dtn-tcpclv4-05.txt
> 	Pages           : 37
> 	Date            : 2017-12-14
> 
> Abstract:
>    This document describes a revised protocol for the TCP-based
>    convergence layer (TCPCL) for Delay-Tolerant Networking (DTN).  The
>    protocol revision is based on implementation issues in the original
>    TCPCL Version 3 and updates to the Bundle Protocol contents,
>    encodings, and convergence layer requirements in Bundle Protocol
>    Version 7.  Specifically, the TCPCLv4 uses CBOR-encoded BPv7 bundles
>    transport of such bundles.  Several new IANA registries are defined
>    for TCPCLv4 which define some behaviors inherited from TCPCLv3 but
>    with updated encodings and/or semantics.
> 
> 
> The IETF datatracker status page for this draft is:
> https://datatracker.ietf.org/doc/draft-ietf-dtn-tcpclv4/
> 
> There are also htmlized versions available at:
> https://tools.ietf.org/html/draft-ietf-dtn-tcpclv4-05
> https://datatracker.ietf.org/doc/html/draft-ietf-dtn-tcpclv4-05
> 
> A diff from the previous version is available at:
> https://www.ietf.org/rfcdiff?url2=draft-ietf-dtn-tcpclv4-05
> 
> 
> Please note that it may take a couple of minutes from the time of submission
> until the htmlized version and diff are available at tools.ietf.org.
> 
> Internet-Drafts are also available by anonymous FTP at:
> ftp://ftp.ietf.org/internet-drafts/
> 
> I-D-Announce mailing list
> [email-masked]
> https://www.ietf.org/mailman/listinfo/i-d-announce
> Internet-Draft directories: http://www.ietf.org/shadow.html
> or ftp://ftp.ietf.org/ietf/1shadow-sites.txt

---

From: Brian Sipos <[email-masked]>
Date: Wed, 24 Jan 2018 14:11:34 +0000

Rick, I would like to make edits for these comments.

I also very much appreciate the recommended replacement wording, it helps a lot to have an idea of improved text rather than just a note on existing confusing wording.

For many of the comments, I believe that there is no change needed in the requirement text itself but I agree with shifting some actual requirements from the overview section into the appropriate message definition section(s).

________________________________
From: Rick Taylor <[email-masked]>
Sent: Monday, January 22, 2018 9:17:24 AM
To: [email-masked]; [email-masked]
Cc: [email-masked]; Brian Sipos
Subject: Re: [dtn] I-D Action: draft-ietf-dtn-tcpclv4-05.txt

Brian,

Cheers,

Rick

---

From: Brian Sipos <[email-masked]>
Date: Sat, 27 Jan 2018 19:38:23 +0000

I've done some editing of the draft, not yet submitted, but I'm also
replying inline below.

The summary of my response to these comments is that the SHUTDOWN
state/sequence needs some more effort put into it to capture the
desired and allowed behavior.

> CBOR encoded BPv7 bundles? Isn't bundle data opaque?
The bundle itself is opaque to the CL. This text was included as the
result of an earlier comment to tie together the CL with its service
data (the bundle).

> a "convergence- layer adapter" - extra space before "layer"
Will fix this.

> "This section contains definitions specific to the TCPCL protocol:"
Simplified this phrase.

> I think "A TCPCL node refers to" is unnecessary. Same for other
I've attempted to simplify these.

> an negotiating - a negotiating
Will fix this.

> cannot exist without an established TCP connection.
Agreed, and simplified.

> text on Transfer ID is needed here.
There was earlier some confusion about whether a transfer ID is unique
to the session or to the node. This is just solidifying the requirement
(from another section).

> The word liveliness (liveliqness) is only used once.
Yes, you are correct; once established the session is live as long as
its TCP connection is up and messages are getting through. I've changed
this to be simply "live" vs "idle" to allow term searching.

> reason codes are used. The first sentence in this definition should
Yes, it really does not add to the definition. There is not any other
part of the spec which discusses codes in general. I don't think it's
controversial, so I'm taking the extra sentences out.

> wouldn't give specifics about flags and segment MRU.
I'm attempting to leave Section 3 requirement-free and to move these
into Sections 4 and 5.

> Followed by the sentence starting "Bundle interleaving".
Agreed with edit.

> how much of a bundle was received? In which case, what is the point
This is getting into some of the earlier WG discussion; I've added in
the Introduction section details about what services this CL provides
to the BP agent, including intermediate progress.
I have moved the actual requirements into Section 5.

> time by sending a Transfer Refuse (XFER_REFUSE) message. This
Removed duplicate requirement with Section 5.

> worth also doing this when you say you can use SHUTDOWN as a refusal
I've attempted to simplify the Section 3 language here.

> Seems like this would work better before paragraph 7.
I'm leaving the general protocol paragraph after the SHUTDOWN sequence
paragraph.

> on to sending another bundle but ACKs never come back for the first
The BP agent can be informed of any transmitted or received ACKs. I've
added discussion of these services in new Section 1.1. It's not part of
the on-the-wire protocol however so not a spec requirement.

> this last sentence in the 2nd paragraph is needed.
I agree that it doesn't buy much, but it's also just informational.

> not overwhelmed with repeated connection attempts."
I believe this language is inherited from TCPCLv3. I've attempted to
clean it up based on your recommendations.

> general (in which case it shouldn't be in this section)? 
BP agent services are in new Section 1.1, but it gets tricky to try to
impose protocol requirements between CL and BP-agent since these are
not on-the-wire visible.
I'm adding CL services to indicate session started, session
established, session shutdown, and session failed.

> After the reference to Section 4.2, there should be a full stop.
Fixed this.

>    MAY also refuse the session by sending a SHUTDOWN message with a
Agreed, simplified this.

> connection" sub-section as 4.1.
Sure, this helps to break up the large subsection.

> "Header Extension Length Header Extension Items" - missing an 'and'?
Fixed typo.

>       identical form " - This doesn't make sense to me.
Fixed typo.

> Should be consistent on either capital or lowercase i for Item.
Standardized on uppercase.

> "Extention " -> "Extension"
Fixed these typos.

> be session parameters too.
This makes sense, to try to make the list complete with the other
parameters. I don't think that its worth limiting the segment/transfer
maximum transmit unit though. There is no reason why a node could not
have a time-variable MTU depending on configuration, network
conditions, etc. The existing requirement on not sending larger than
the MRU is sufficient; but it's probably worth noting all of that in
this section.

> of a full stop and the new sentence beginning with "Otherwise".
Fixed this language.

> session operation" section 5.2.1. Here you only need to say to take
Agreed and moved requirements out of Section 3.

> this is different from a "TLS Failure" after an agreed-upon Enable
Fixed the language in this section, kept the cross-reference to the
other section.

> "within each TLS nqion " - typo.
Fixed typo.

>    exchange.  This reinforces the fact that there is no TLS downgrade
I agree and I've edited this section to flow better.

> Table 3 - Section 5.3.5 is not a linked reference like all the other
This may be some weird side effect of processing the XML source. I will
need to look into this.

> that be "TCP connection"?
I've removed the confusing language and now rely on normal SHUTDOWN
handling requirements.

> "All of the message" - messages
Fixed typo.

> it's TCPCL which ensures only data from a single segment goes into a
This requirement is a holdover from TCPCLv3 and is probably
unnecessary, as the CL is opaque to the data content. This is really
just saying "don't misuse a convergence layer". I'm not sure how much
value is in this requirement.

> "a Transfer ID number" - dont need the word "number"?
Simplified this.

> and acknowledgements about the transfer of the bundle"
I've rephrased this, as acks are themselves a type message.

> "Resource Exhaustion" doesn't appear in table 13.
Added this reason code.

> "Total Bundle Length" - consistency with capital letters
Standardized on capitalized form.

> until the previous transfer is finished?
I've rephrased this to try to be more clear, that XFER_INIT is always
the first message in any transfer.

> is the only time BUNDLE_REFUSE is used.
Agreed and moved.

> once it receives an ack, pass this information to a higher layer?
Yes, and I've added new "Convergence Layer Services" section to justify
the need for these ACKs.

> "endpoing" - typo
The term "endpoint" was deprecated in favor of "node" so this typo was
missed.

> there a reason the sender might choose to vary sizes of consecutive
Similar to earlier comment, I really don't think that there is value in
the protocol spec. limiting the transmit segment sizes (as long as they
are no larger than the segment MRU).
An implementation may choose to adapt segment sizes to available TCP
goodput, so that the session tends to have ACKs at a regular real-time
interval instead of a regular data-size interval.
This would also allow an implementation which has side information
(e.g. about the IP or link layer) to adapt its segment size to some
underlying layer packet size, which is the model of LTP (RFC 5326).

> expected ack is not received...
See new section "Convergence Layer Services".

> "an mechanism" - should be "a mechanism"
Fixed typo.

> header or data since it treats it as opaque.
Yes, see new section "Convergence Layer Services". And this brings up
the point that the reception intermediate progress (to BP agent) allows
Bundle header inspection before the entire bundle is finished.

> actually be marked by an XFER_INIT before you see an XFER_SEGMENT
The start/end bits are really a holdover from TCPCLv3 and are... not
strictly necessary any longer. They provide redundant information, so
not horrible to have but also unnecessary.

> received before the shutdown was sent?
This really needs cleaned up and made consistent... I will look into
SHUTDOWN sequencing further after these other edits.

> Is reconnection delay in seconds? should say that in the field
Yes, added this unit.

> establish a TCPCL session"?
I clarified this text.

> "additional", or change it to "optional".
Fixed text.

> better to say the shutdown can occur after contact header exchange?
I agree that the contact header really is mandatory first thing, as
it's not a true "message". I will edit this as part of SHUTDOWN
rewriting.

> establishment? If so, don't start a new paragraph. 
I've rewritten this section of the spec to avoid this language
altogether.

> described underneath Table 8, so can be removed.
Agreed and removed.

> SHUTDOWN, it can still send acks, but must ignore anything it
This definitely needs to be made more consistent. And yes, the ideal
situation is that a SHUTDOWN informs the peer that "no further
transfers will be allowed to start in my direction and I will not start
any more transfers in your direction". I wonder if this means a new
XFER_REFUSE reason code is needed for "Shutdown", or if this would be
more like a MSG_REJECT with reason "Message Unexpected". Need to spend
a bit more time on this unfortunately.

> "security mandated" -> "security is mandated"
Fixed typos

Fixed typos

> > > On Sat, Dec 30, 2017 at 7:48 PM, Marc Blanchet <marc.blanchet@via
> > > genie.ca> wrote:
> > > > Thanks Brian,
> > > > > This message is late in coming. The last submitted draft of
> > > > > the TCPCLv4 spec. is intended to address all of the comments
> > > > > received to-date. Specifically, those from Victoria
> > > > > Pritchard, Marius Feldmann, Matt Wronkiewicz, and Marco
> > > > > Kaulea. The latest two draft revisions have been correcting
> > > > > logical and typographical issues, and dealing with edge
> > > > > cases; the changes have not affected the nominal sequencing
> > > > > of the protocol or the message contents.
> > > > > written and provides a path for nodes to migrate from
> > > > > similarly-scoped TCPCLv3 implementations.
> 
>

---

From: Brian Sipos <[email-masked]>
Date: Mon, 29 Jan 2018 03:18:47 +0000

Marco,
The typo in #1 has been corrected from earlier comment.

For #2, I added an additional informative statement about this
possibility to the section which describes a contact header failure.

On Tue, 2018-01-16 at 21:19 +0100, Marco Kaulea wrote:
> On Sat, Dec 30, 2017 at 7:48 PM, Marc Blanchet <marc.blanchet@viageni
> e.ca> wrote:
> > Thanks Brian,

---

From: Brian Sipos <[email-masked]>
Date: Mon, 29 Jan 2018 03:18:54 +0000

Fred,
I'm editing the draft and also have replies to individual comments
inline below.

>    number differences as necessary.
For both of these, I identified the distinction and convention in the
Introduction section.

>      "native" link, network, or Internet protocol."
I've edited this to read better as suggested.

>    advantage of TCP's reliable in-order delivery properties?
The short answer is yes; because all bundle segments are sent in-order
and with no CL-based retransmission then the segment data received is
simply concatenated in-order to reassemble the bundle.

>    flagged and corrected by the RFC editor.
I have corrected a number of spelling errors in the last edit.