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

From: Joerg Ott <[email-masked]>
Date: Mon, 10 Sep 2018 07:40:40 +0200

Hi Rick,

sorry for the delay, I was on vacation access to this email.
Yes, I believe those issues need fixing before proceeding.
And we are in the process of doing this telling from the
discussion.

Best,
Jörg

On 21.08.18 14:55, Rick Taylor wrote:
> Hi Joerg,
> 
> Thanks for the detailed review.
> 
> As you are one of the authors, are you suggesting that the author team
> no longer considers the document ready for publication, and that the
> team wish to abort the WG Last Call to give you all time to address
> your comments?
> 
> If so, can I impress upon you the urgency of getting at least one
> Convergence Layer document onto the Standards Track as soon as possible
> - TCP-CL being the current best candidate.
> 
> To the WG: If someone has another interoperable Convergence Layer
> protocol that they wish to submit to the working group, it would be
> very welcome.  TCP-CL is not the 'One CL to rule them All' just what
> was seen as a simple first CL to publish alongside BPbis and BPSec. If
> another CL document was to appear in such a state that it could be
> quickly adopted and peer-reviewed then the TCP-CL authors would have
> more time to address any perceived short-comings in the draft, and the
> publication of BPbis would not be further delayed.
> 
> Sorry for the top-post.
> 
> Cheers,
> 
> Rick
> 
> On Mon, 2018-08-13 at 22:36 +0200, Joerg Ott wrote:
>> [Now including the DTN WG list]
>>
>> Hi everybody,
>>
>> I am guilty of having had less than adequate time lately to look at
>> the
>> detailed evolution of this draft.  I did a detailed review that
>> yielded
>> a number of comments.
>>
>> As a meta note up front: I am not super-happy that the document more
>> than doubled in size, while the functionality did not grow that much
>> (luckily).  There seem to be lots of precautions taken for future-
>> proofing, while some of those could be addressed by another rev.
>> I specifically also went back to v3 to compare the changes.
>>
>> I am also puzzled about the generous choice of field sizes given that
>> the bundle protocol itself uses encodings that aim at wire
>> efficiency.
>>
>> I don't wanna bring up everything at once but here are a few bits
>> that
>> I really think need to be fixed.  So let me keep the discussion
>> focused
>> for now (we can look at nits later on):
>>
>> 1. Section 4.3 seems to suggest on p21 (a node may adapt its
>> operation
>>      to an older version of the protocol) the two endpoints might
>> choose
>>      using different protocols versions, which is surely problematic.
>>      The outcome of this negotiation must be unambiguous.
>>
>> 2. Section 4.4 discusses TLS but this isn't enough.  TLS has lots of
>>      parameters, TLS clients may and servers must have certificates.
>>      All this interaction needs to be spelled out.  What about the APN
>>      to be used for TLS connection setup.  And, of course, we should
>> use
>>      TLS 1.3.
>>
>> 3. Splitting XFER_INIT from XFER_SEGMENT is not useful.  Nodes would
>> not
>>      want to wait for a reply but use pipelining.  Hence, creating a
>>      SEGMENT must follow INIT dependency appears unnecessarily complex
>>      in terms of state space.
>>
>>      Simple suggestion: Just keep session transfer but, if the S bit
>> is
>>      set, include the total bundle length field and the bundle
>> transfer
>>      id.  You achieve the same, save a few bytes, and the sender won't
>>      wait transmitting anyway.  And one possible error condition goes
>>      away.
>>
>>      The only reason LENGTH was a separate message before is that we
>>      needed/wanted to be backwards compatible to earlier versions of
>> the
>>      draft and could not grow the header of the DATA_SEGMENT message.
>>
>> 4. Section 5.1.1 Keep alive has lost its function.  According to the
>>      current text, a keep-alive should be transmitted when the timer
>>      expires.  Fine, but this too late because the other entity will
>>      time out the connection.  The earlier guidance was that the
>> session
>>      timeout occurs if no data was received for _twice_ the keep alive
>>      interval.  But this part seems to have gone.  The text would need
>>      to be explicit about when to schedule sending keep-alive messages
>>      and when to time out.
>>
>> 5. The extension items (both for setup and transfer) are generously
>>      dimensioned but not a single one is defined.  I would say that
>>      8 bits parameter space and 16 bits length should be sufficient.
>>      It is also useful to define the encoding only in one place since
>>      the two are largely identical.
>>
>> 6. Echoing the SESS_TERM message as an ACK is a good idea.  Should
>>      we include a nonce here to make sure that an ACK is different
>> from
>>      an independently issued SESS_TERM?
>>
>> 7. Session termination in section 6.1 got more complicated.  The text
>>      now alllows sending even more segments, whereas the v3 spec
>>      essentially said that no data must follow afterwards.  If I
>> receive
>>      a SESS_TERM, I should be sure that I can release data processing
>>      resources.
>>
>> 8. Why not always have the reason code and define one to be
>> unpsecified
>>      in SESS_TERM rather than having two-stage processing?
>>
>>
>> Major editorial:
>>
>> We have too many terms and definitions.  Why are we discussing a
>> TCPCL
>> entity with a complex figure 2, when there is nowhere inside the
>> document any need visible for discussing more than a single TCPCL
>> connection at a time.  This is rather confusing.
>>
>> We have many state diagrams (fundamentally a good idea) but they are
>> hard to follow and sometimes very simple.  They are also not referred
>> to in the mandatory text.  Maybe we can have fewer states and a
>> stronger relation between text and diagrams?  Btw, not all acronyms
>> are
>> defined here.
>>
>> "CAN_TLS" is not the best name.
>>
>> Cheers,
>> Jörg
>>
>> _______________________________________________
>> dtn mailing list
>> [email-masked]
>> https://www.ietf.org/mailman/listinfo/dtn
> _______________________________________________
> dtn mailing list
> [email-masked]
> https://www.ietf.org/mailman/listinfo/dtn
>