Hello Ole,

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


Cheers,

Steven & Markus


>
> 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.
-07 already included some of them and we've staged a few more for -08.


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


> => 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.
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.

>
> => 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?)
Agreed.


>
> => I do find the text on transport modes somewhat confusing, U, M+U
> and MulticastListen+U. I'd like to see more descriptive text
Okay, we will prepare something for -08.



> 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.
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:
> ============
>
> => This is confusing:
> o If using a stream transport, the TLV MUST be sent at least once,
>   and it SHOULD be sent only once.
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.

>
> 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?
All done.

>
>
>    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?
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.

>
> (I do find the transport modes confusing, and I'm not sure I
> 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:
> ============
> => What is meant by: "link with shared bandwidth"?
I changed it to "multiple access link" for now, I think that makes it
more clear.

>   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?
"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:
> ============
> => First mention of a topology graph.
It should be defined in the topology.


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

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
> peer and what is a node.

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."


> 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.
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:
> ============
>    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?
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:
> ============
> s/actually uses/uses/

Done.

_______________________________________________
homenet mailing list
homenet@ietf.org
https://www.ietf.org/mailman/listinfo/homenet

Reply via email to