Hello Alia,
please find replies inline.
Cheers,
Steven & Markus
> ----------------------------------------------------------------------
> DISCUSS:
> ----------------------------------------------------------------------
>
> First, I have a number of specific comments. Some of these are hazards
> to technical interoperability; I've tried
> to include those in my discuss - but the general point is that it is very
> hard to tell a number of details because the
> structure is assumed. Having read this document, I do not think that I
> could properly implement DNCP from scratch.
Please note that an independent DNCP (or more specifically an HNCP)
implementation has been successfully developed based on
an earlier version of this draft and has been shown to be
interoperable with the reference implementation of the draft authors.
We used the implementer’s feedback afterwards to further refine the draft
to avoid possible ambiguities.
> a) What is a topology graph? When is it created, modified, or destroyed?
> Is it a conceptual artifact constructed
> from the various TLVs? I think a quick paragraph describing it would
> help.
The term “topology graph” is defined in the Terminology Section and based
on bidirectional peer reachability which is defined right afterwards. In the
latter definition it is mentioned that the process is solely based on published
TLVs so the topology graph is to my understanding already unambiguously
defined. It is solely up to the implementer if this is implemented as an actual
graph or not, as long as the outcome is consistent with what is described.
If you still think it is ambiguous please provide some ideas on how this
could be worded better.
> b) How are peer relationships discovered and established and destroyed?
> I really can't tell from the draft and even a quick scan of
> RFC 6206 doesn't give any hints.
This is explained in section “4.5 Adding and Removing Peers”. As for
methods for discovery, this is actually mentioned multiple times across
the document, e.g. in the second paragraph of the Overview or in the
terminology.
Could you please be more specific on what exactly is unclear here in your
opinion?
> c) It looks like the TLVs are sent one after another in a datagram or
> stream across a socket. The closest I see to any detail
> is "TLVs are sent across the transport as is".
Section “4.2 Data Transport” says
TLVs are sent across the transport as is, and they SHOULD be sent
together where, e.g., MTU considerations do not recommend sending
them in multiple batches. TLVs in general are handled individually
and statelessly, with one exception …
Could you please be more specific on what is unclear?
The section states that TLV handling is stateless except for the one exception
which is explained, so otherwise it does not really matter in what order
they are sent or how you split them up onto datagrams (if that is your
transport).
> d) As far as I can tell, Trickle is used to decide basically how often to
> send information - but the details of this and the interactions
> aren't clear to me.
This is explained in detail in section “4.3. Trickle-Driven Status Updates”.
The Trickle state for all Trickle instances is considered
inconsistent and reset if and only if the locally calculated network
state hash changes. This occurs either due to a change in the local
node's own node data, or due to receipt of more recent data from
another node. A node MUST NOT reset its Trickle state merely based
on receiving a Network State TLV (Section 7.2.2) with a network state
hash which is different from its locally calculated one.
Every time a particular Trickle instance indicates that an update
should be sent, the node MUST send a Network State TLV…
Could you please be more specific on what is missing here?
> I suspect that there are dependencies on the HNCP draft that would make
> this a lot easier to understand but since we want it to progress
> separately, the document does need to stand alone.
No, there are no dependencies. This was noted already in response to
reviews from Thomas Claussen and Les Ginsberg.
Please refer to section “9. DNCP Profile-Specific Definitions“ for the
comprehensive list of things we have intentionally left out in DNCP.
> 8) In Sec 4.6 " o The origination time (in milliseconds) of R's node
> data is greater
> than current time in - 2^32 + 2^15." Since origination time hasn't
> yet been introduced, I'm going
> on an assumption that it means when R's node data was originated from R.
> So - this requirement is
> saying that R's node data must have been generated in the future (but
> already known by the local node)???
The term “origination time” is actually explained in Section “5. Data Model”,
-10 will have a forward-reference here. About the time itself,
the text says greater than “current time - 2^32+2^15”; that threshold is always
in the past.
> 9) In Sec 4.6 "They MAY be removed immediately after the topology graph
> traversal" The DNCP nodes can
> be removed from what?? The topology graph? From some type of received
> TLV?? How would they be
> added back in later?
Maybe “forgotten (about)” may be a more intuitive term here. Each node stores
each other node’s data and metadata so “removing” the node essentially means
removing that information from the local node’s memory. If it is not removed
immediately the node can still keep it in its memory in case the remote node
becomes bidirectionally reachable again to avoid synchronizing the node data
(if it did not change in the meantime).
Clarified in -10.
> 11) In sec 6.1: "Trickle-driven status updates (Section 4.3) provide a
> mechanism for
> handling of new peer detection on an endpoint, as well as state
> change notifications." Nothing in Sec 4.3 talked about a mechanism
> for detecting
> new peers on an endpoint. It is entirely possible that Trickle does
> provide this (but what
> about the modes where Trickle isn't used/needed??) but there needs to be
> a description of how
> new peer detection is done - even if it's just a pointer to Trickle
> RFCs.
As noted in the reply for c) addition is based on TLV reception as noted in 4.3.
Actual discovery is mentioned multiple times in this draft, e.g. 2nd paragraph
of Overview. Summarizing: it boils down to either one node receives that TLV
over multicast or one of the node knows the other’s unicast address based
on configuration or some other protocol then the other node discovers the
former when that TLV is unicasted to it.
Could you please note how this could be improved?
> 12) In Sec 6.1.4: " On receipt of a Network State TLV (Section 7.2.2)
> which is consistent
> with the locally calculated network state hash, the Last contact
> timestamp for the peer MUST be updated." Could you add some
> rationale for why this
> is needed?
This is to indicate the node is still alive (6.1. is the section on
keep-alives).
The Last Contact timestamp is defined in 6.1.1 and 6.1.5 explains what
happens when it reaches a threshold.
> I suspect that part of my confusion is that the "locally
> calculated network state
> hash" could mean two different things. Is it the hash computed by the
> local node on the
> data received in the Network State TLV to validate that the Network State
> TLV is not
> corrupted?
Since there is no mention of any hashing to validate generic TLVs anywhere
in the RFC or any such hash values for comparison at all I cannot really follow
how one could come to that conclusion.
> Or is it the hash computed by the local node on its
> arbitrarily wide 1-hash tree
> that represents the local node's network state? Since the term is never
> defined, it's hard
> to guess here. The bottom of Sec 7.2.2 uses "current locally calculated
> network state hash"
> to refer to, I believe, the latter.
The term “network state hash” is actually defined in the terminology section
and hash tree section 4.1;
if the words “locally calculated” make it really ambiguous please let me know
how to improve it.
> 13) In Sec 6.2: "normally form peer relationships with all peers." Where
> is forming a peer
> relationship defined? Is this tied solely to Trickle instances? What
> about with reliable unicast
> where presumably Trickle isn't used between peers as the Overview states
> "If used in pure unicast mode with unreliable transport, Trickle is also
> used between peers"?
Please see replies to c) and 11).
> 14) In Sec 7: " For example, type=123 (0x7b) TLV with value 'x' (120 =
> 0x78) is
> encoded as: 007B 0001 7800 0000. If it were to have sub-TLV of
> type=124 (0x7c) with value 'y', it would be encoded as 007B 0009 7800
> 0000 007C 0001 7900 0000." In this case, the padding between the
> TLV's value
> and the sub-TLV is included but the padding after the sub-TLV is not.
> What would
> happen if there were multiple sub-TLVs?? Would the padding between those
> sub-TLVs
> be included?
Yes, that is correct, fixed in -10. The individual sub-TLVs themselves contain
length fields which will not include even their own padding, but the container
TLV’s length ends at the end of the last sub-TLV’s last non-padding byte
(and then padding is inserted if any).
> Also related :"In this case the length field includes the length of the
> original TLV, the
> length of the padding that are inserted before the embedded TLVs and the
> length of the
> added TLVs." Here, the phrase "length of the TLV" means different
> things. In the first case,
> "length of the original TLV" means the "length of the value in the
> encapsulating TLV". In
> the second case, "length of the added TLVs" appears to mean the length of
> the sub-TLVs
> including the type/length header. As I mentioned above, I can't tell
> what happens to the
> padding in between sub-TLVs.
Clarified in -10.
> ----------------------------------------------------------------------
> COMMENT:
> ----------------------------------------------------------------------
>
> 1) In Sec 4.1, I think it would be clearer if you moved the sentence
> "These leaves are ordered in ascending
> order of the respective node identifiers. " to after the first
> sentence with appropriate text changes. I was left
> confused by why the leaf would be represented by a node's sequence
> number. I think it's that a leaf represents
> a node and is ordered based upon that node's identifier. The value of
> a leaf is a tuple of the node's sequence
> number and the hash value...
Clarified as
“These leaves are ordered in ascending
order of the node identifiers of the nodes they represent.”
in -10.
> 2) In Sec 4.2, it says "As multicast is used only to identify potential
> new DNCP
> nodes and to send status messages which merely notify that a unicast
> exchange should be triggered, the multicast transport does not have
> to be secured." There aren't attacks from processing fake potential
> new DNCP node
> or triggering lots of unneeded/unterminated unicast exchanges?
This is already covered in the Security Considerations section.
> 3) In Sec 4.3, it says "Imin, Imax and k. Imin and Imax represent the
> minimum and maximum values for I"
> I believe this isn't quite accurate. Imax is a max multiplier of Imin
> for I and not a maximum value.
> I've seen this error in another draft also; I think it's important to be
> correct & clear here.
Addressed for -10. 6206 defines it as “The maximum interval size, Imax“,
which probably led to this. (I wonder why it is not maximum interval exponent,
or something..)
> 4) There are multiple references to different unintroduced TLVs. There
> is also the idea that
> each DNCP protocol can have its own TLVs. It'd be very useful in the
> Introduction to simply state
> what TLVs are required by DNCP and conceptually what they are for.
Section 7 is entirely devoted to that purpose. I do not think that it should
be repeated in the general introduction.
If there are particular forward references that are missing, please let us know.
> 5) In Sec 4.3, it says "The Trickle state for all Trickle instances is
> considered
> inconsistent and reset if and only if the locally calculated network
> state hash changes." but I have no idea yet what a Trickle instance
> is, when
> a Trickle instance is created or associated with a node? an
> interface? or something else?
Please see Section “5. Data Model”
“For each remote (peer, endpoint) pair detected on a local endpoint, a
DNCP node has:”
[...]
* Trickle instance: the particular peer's Trickle instance with
parameters I, T, and c (only on an endpoint in Unicast mode, when
using an unreliable unicast transport) .
we will add an xref there and add “trickle instance” to the terminology.
> 6) I think it would be more useful to describe generally at least what is
> in a DNCP profile earlier in the
> document. This may be bringing Section 9 forward or it may be listing it
> earlier. I'm seeing numerous
> forward references.
There would be numerous forward references in that case too, as it refers
to the protocol behavior and individual TLVs. (There are currently 2 forward
references in the terminology and 2 in the text; we probably cannot move it
before the terminology, and right after the terminology it would introduce
more forward references than it would eliminate.)
> 7) Start of Sec 4.6 talks about the topology graph - but there's been
> absolutely no introduction of
> what the topology graph is or how it was created, updated, etc.
See reply to a).
> 10) In Sec 5: This is a helpful section. It tells me that a Trickle
> instance is per interface. I don't see anything
> like a topology graph in it. It clarifies origination time slightly. It
> talks about "For each remote (peer, endpoint)
> pair detected on a local endpoint" but I still have no ideas how that
> detection is done. It implies some policy
> restrictions "Set of addresses: the DNCP nodes from which connections are
> accepted." but has no details on whether
> that is created via multicast messaging or via local configuration or
> something else.
See previous answers referring to peer detection.
> 15) Also, in Sec. 4.6, the terminology for the fields of the Peer TLV is
> different than defined
> in Sec 7.3.1 - just "Endpoint Identifier" instead of "Local Endpoint
> Identifier".
Addressed in -10.
> 16) In Sec 7.3.2: " Endpoint identifier is used to identify the
> particular endpoint for
> which the interval applies. If 0, it applies for ALL endpoints for
> which no specific TLV exists." Is this the Local Endpoint Identifier
> or the remote?
> The same question applies for Sec 7.2.1.
Both refer to the endpoint identifier of the (local) sending node. Given the
TLV is published network-wide within the particular node’s node state,
identifying that of remote nodes would be nontrivial and probably not unique
either.
_______________________________________________
homenet mailing list
[email protected]
https://www.ietf.org/mailman/listinfo/homenet