Subject: [Int-dir] IntDir review: draft-ietf-homenet-dhcp-07

From: Ole Troan <[email-masked]>
Date: Tue, 7 Jul 2015 20:50:11 +0200

I have been selected as the Internet Directorate reviewer for this
draft. The purpose of the review is to provide assistance to the
Internet ADs.

Although these comments are primarily for the use of the Internet ADs,
it would be helpful if you could consider them along with any other
IETF Last Call comments that you receive, and strive to resolve them
through discussion or by updating the draft.

Document: draft-ietf-homenet-dncp-07.txt
Reviewer: Ole Troan
Review Date: July 7 16, 2015

The document now reads much better. I do get the impression that the text is
reverse engineered from code in a few places though.

Given that it is an "Abstract protocol" specification, that must be
combined with a profile specification to be a fully implementable, it
is somewhat difficult to predict if the specification is complete or
not. Juliusz Chroboczek is writing an independent implementation, and
I'd recommend incorporating the very informative replies the authors have made to his
comments on the homenet list into the document.

General comments:
=================
=> Replace no affiliation with "Independent". If that's the case.

=> It is unclear to me how multiple instances of DNCP is run on a
   link. Is that something that must be specified in the profile
   document, and each profile must support multiple instances?
   Given draft-stenberg-shsp, and the way it "hijacks" the HNCP
   profile, it appears that more formal multiple instance support
   would be needed.

=> I can't help being left with the impression that fragmentation
   (section 6.3) is underspecified. Can fragmentation be left out of
   the protocol for now (and profiles requiring large TLVs can require
   a transport layer supporting segmentation and reassembly?)

=> I do find the text on transport modes somewhat confusing, U, M+U
and MulticastListen+U. I'd like to see more descriptive text

Abstract:
=========

OLD:
   This document describes the Distributed Node Consensus Protocol
   (DNCP), a generic state synchronization protocol which uses Trickle
   and Merkle trees.  DNCP leaves some details unspecified or provides
   alternative options.  Therefore, only profiles which specify those
   missing parts define actual implementable DNCP-based protocols.

NEW:
   This document describes the Distributed Node Consensus Protocol
   (DNCP), a generic state synchronization protocol that uses Trickle
   and Merkle trees. DNCP is an abstract protocol, that must be
   combined with a specific profile to make a complete implementable
   protocol.

=> The purpose of that change would be to make it clear what this
is. A framework? A component that protocols can be built out of?

=> Add a reference to Merkle trees?

Section 4.2:
============

=> This is confusing:
o If using a stream transport, the TLV MUST be sent at least once,
  and it SHOULD be sent only once.

OLD:
*  If only unreliable unicast transport is employed, Trickle state
   is kept per each peer and it is used to send Network State TLVs
   every now and then, as specified in Section 4.3.

NEW:
*  If only unreliable unicast transport is used, Trickle state
   is kept per peer and it is used to send Network State TLVs
   intermittently, as specified in Section 4.3.

s/every now and then/now and then/

s/employed/used/

Section 4.3:
============
=> reference to rfc6206?

   o  the endpoint is in Multicast+Unicast transport mode, in which case
      the TLV MUST be sent over multicast.

   o  the endpoint is NOT in Multicast+Unicast transport mode, and the
      unicast transport is unreliable, in which case the TLV MUST be
      sent over unicast.

=> What do an implementation do if the endpoint is not in M+U
transport mode, and the unicast transport is reliable?

(I do find the transport modes confusing, and I'm not sure I
understand the MulticastListen mode).

s/when using also secure unicast/when using secure unicast/

Section 4.4:
============
=> What is meant by: "link with shared bandwidth"?

  o  Any other TLV: TLVs not recognized by the receiver MUST be
      silently ignored.

=> doesn't that mean it isn't stored in the Merkle tree? and the
hashes don't compute?

Section 4.6:
============
=> First mention of a topology graph.

Section 5:
==========

o  Endpoint identifier: the 32-bit opaque value uniquely identifying
      it within the local node.

=> "it"? I think I'm still confused what is an endpoint, what is a
peer and what is a node.

o  Range of addresses: the DNCP nodes that are allowed to connect.

=> How does a range of addresses look like and how is it used?
   I find only one occurence in the document.

Section 6.1:
============
   A DNCP profile MAY specify either per-endpoint or per-peer keep-alive
   support.

=> Again I'm confused over the usage of endpoint versus peer.
What's the difference between per-peer and per-endpoint keepalives?

Section 6.2:
============
s/actually uses/uses/

---

From: Steven Barth <[email-masked]>
Date: Tue, 07 Jul 2015 23:52:50 +0200

Hello Ole, hello Les,

thank you both for your reviews.
We will hopefully get back to you with a detailed reply and a list of changes
we made based on your reviews by next week (due to vacations etc).

Cheers,

Steven

---

From: Terry Manderson <[email-masked]>
Date: Wed, 8 Jul 2015 06:36:34 +0000

Thanks for your review Ole.

Cheers
Terry

---

From: Terry Manderson <[email-masked]>
Date: Wed, 8 Jul 2015 06:37:37 +0000

Thanks Steven,

... as they say 'watching the thread' :-)

Cheers
Terry

On 8/07/2015 7:52 am, "Steven Barth" <[email-masked]> wrote:

>_______________________________________________
>homenet mailing list
>[email-masked]
>https://www.ietf.org/mailman/listinfo/homenet

---

From: Steven Barth <[email-masked]>
Date: Sun, 12 Jul 2015 17:05:33 +0200

Hello Ole,

thanks again for your review and sorry for the delay.
Please find replies inline.

Cheers,

Steven & Markus

> comments on the homenet list into the document.
-07 already included some of them and we've staged a few more for -08.

> General comments:
Done.

>    profile, it appears that more formal multiple instance support
Hmm, I think this is up to the specific protocol. Reuse of profiles
is in theory possible but for standardisation would require either
one protocol updating the other OR distinct TLV-spaces. The latter
sounds a bit awkward e.g. IANA-wise. In any case , when sharing
transport details such as port numbers, then a shared registry would
need to be done anyway. SHSP should probably not be seen as a good
example here. Since DNCP does not define defaults here an extra
identifier seems unncessary and could or should probably be
specified by the derived protocol.

>    a transport layer supporting segmentation and reassembly?)
Agreed.

> and MulticastListen+U. I'd like to see more descriptive text
Okay, we will prepare something for -08.

>    combined with a specific profile to make a complete implementable
Done.

> => Add a reference to Merkle trees?
I'm not certain what would be a good source to quote here, maybe Merkle's
paper from '87 or the ‘92 patent? At least there doesn't seem to be
a really appropriate reference.

> Section 4.2:
I staged "If using a stream transport, the TLV MUST be sent at least
          once per connection, but SHOULD NOT be sent more than once."
for now.

> s/every now and then/now and then/
All done.

> transport mode, and the unicast transport is reliable?
Section 4.2 states "If only reliable unicast transport is employed, Trickle is not
used at all." for unicast mode and ML+U mode references that as well.

> understand the MulticastListen mode).
These modes, especially the listen one is only used for Dense Broadcast Link
optimization. It is essentially the same as unicast, however the node
listens for multicast traffic to detect the presence or abscence of a node
with higher identifier on the underlying multicast-capable link.

> s/when using also secure unicast/when using secure unicast/
Done.

> Section 4.4:
I changed it to "multiple access link" for now, I think that makes it
more clear.

> => doesn't that mean it isn't stored in the Merkle tree? and the
"Any other TLV" was intended do be applied to top-level TLVs only.
Actual data TLVs are part of Node State TLVs and thus not meant here.
Nevertheless I proposed
"TLVs not recognized by the receiver MUST be silently ignored
          unless they are sent as part of the payload of one of the TLVs
          above (such as TLVs in the Node Data field of a Node State TLV)."

> Section 4.6:
It should be defined in the topology.

> Section 5:

Changed to "Endpoint identifier: the 32-bit opaque value uniquely
        identifying the endpoint within the local node."

> => "it"? I think I'm still confused what is an endpoint, what is a

I propose the following endpoint definition:
      "a locally configured communication endpoint of a DNCP node, such
      as a network socket. An endpoint may be bound to a set of predefined
      unicast Addresses representing remote DNCP nodes to individually connect
      to or to accept connections from whereby communication with each node is
      separated (e.g., an individual unicast UDP message flow per remote node).
      An endpoint may also be bound to a whole network interface, then
      multicast communication is used (in addition to individual unicast flows)
      to send certain messages to all DNCP nodes connected therewith at once
      as well as to automatically discover new DNCP nodes."

>    I find only one occurence in the document.
I changed it to "Set of addresses: the DNCP nodes from which connections
        are accepted."
This should also align a bit better with the new endpoint definition.

An example here would be a DNCP node which just listens on a port for incoming connections
then that set would be, e.g., all IPv6 addresses for this node. On the other end the node
connecting to it would have the addresses of the first node included in this set.

> Section 6.1:
I changed it to "A DNCP profile MAY specify either per-endpoint (sent using multicast
        to all DNCP nodes connected therewith) or per-peer (sent using unicast
        to each peer individually) keep-alive support."

> Section 6.2:

Done.

---

From: Brian E Carpenter <[email-masked]>
Date: Mon, 13 Jul 2015 08:16:05 +1200

> Hello Ole,

...
> a really appropriate reference.

His PhD thesis seems to be the best formal reference:

www.merkle.com/papers/Thesis1979.pdf
http://dl.acm.org/citation.cfm?id=909000

If you want to actually know what a Merkle tree is:
https://en.wikipedia.org/wiki/Merkle_tree

   Brian

---

From: Brian E Carpenter <[email-masked]>
Date: Mon, 13 Jul 2015 08:24:20 +1200

...
> sounds a bit awkward e.g. IANA-wise. 

I don't see it as 'awkward', it just means being very systematic.

> In any case , when sharing
> need to be done anyway. 

That is the assumption in the current design of GDNP, which isn't even
a DNCP profile but simply a protocol assumed to co-exist with DNCP.

Whether reliance on a fiddly IANA registry is a good thing is of course
a valid question, which we will definitely have to debate in Anima.

    Brian