Steven - Thanx for the reply - inline.
> -----Original Message----- > From: homenet [mailto:[email protected]] On Behalf Of Steven > Barth > Sent: Wednesday, July 08, 2015 11:10 PM > To: Alia Atlas; Les Ginsberg (ginsberg) > Cc: [email protected]; [email protected] Group; draft-ietf-homenet- > [email protected]; [email protected] > Subject: Re: [homenet] RtgDir review: draft-ietf-homenet-dncp-07.txt > > Hello Les, > > thanks for your review. Please find detailed replies inline. > > > Cheers, > > Steven & Markus > > > > Major Issues: > > > > > > > > My biggest concern is that the document - and its companion HNCP - are > > not yet mature enough to be doing last call. > > Please note that DNCP does not depend on nor reference HNCP in any way. > It is a separate draft and it can and should be used not only by HNCP. > Furthermore while DNCP has been submitted to the IESG, HNCP has not and > is still in WGLC. Therefore I'd kindly ask you to keep issues separate from > one > another. > [Les:] We are not understanding each other. Let me try to explain more clearly than I did the first time. DNCP has two dependencies: 1)The (routing) protocol which uses it 2)The profile "protocol" which completes the specification of what information needs to be sent by DNCP. You are of course correct that HNCP is only one of many possible profile "protocols" - I mention it specifically because it is clearly the one being used in current prototyping/testing. To have high confidence that the DNCP specification is complete we would like to have experience w multiple routing protocols/profile protocols. So... In the best of all worlds we would have development experience w multiple routing protocols using multiple profile protocols in combination w DNCP. Second best would be multiple implementations of a single routing protocol/single profile protocol using DNCP. A distant third is a single implementation of a routing protocol/single profile protocol using DNCP. It seems to me that we are somewhere between #2 and #3 today - and HNCP itself is still under construction. This makes me concerned that we don't really know what gaps/problems might exist in DNCP because we simply don't have enough experience with it yet. HTH > > > > What is being defined here is a > > "state synchronization protocol" which is used within the context of a > > "parent protocol" (most interestingly a routing protocol for the > > homenet > > context) and which depends upon another configuration protocol > > (presumably HNCP) to fully define the behavior. > > I don't believe this to be an accurate summary. Again DNCP does not depend > on HNCP, rather the reverse is true. DNCP even defines some extensions > that are not used or intended to be used in HNCP. > > Neither of the two protocols depend on any routing protocol for correct > operation. DNCP does not even mention routing anywhere in the draft. > HNCP in fact enables routing protocols to operate in typical home network > scenarios. > > > > Judging from the review comments provided by others (notably Thomas > > Clausen's detailed review) and the continued discussion on the mailing > > list it has not yet been demonstrated that the specification is clear > > enough and robust enough for implementations to meet all the > requirements and interoperate. > > > > This is not to suggest that you are on the wrong track - but given the > > dependencies pushing this to last call seems - to put it politely - > > very "ambitious". I would prefer to see more implementation experience > > before the document moves to a state where it is presumed to be > complete. > > We addressed Thomas' comments to the best of our knowledge. Please let > us know, if any of the issues still remain in the current revisions. Also, a > second independent and interoperable implementation of DNCP and (parts > of) HNCP exists: > https://github.com/jech/shncpd > > Comments of the author regarding clarity of the draft are already integrated > in the current revisions, some more are queued for the next. > > > > I still have some trouble calling this a protocol. This is more of a > > process - or part of a process - which comprises a routing protocol. > Again "comprises a routing protocol" is simply incorrect. > > > > The process defined > > here serves to support reliable distribution and synchronization of "state" > > in an efficient manner under a limited set of conditions. I don't want > > to quibble too much about the term "protocol" - but I would prefer > something like: > > > > "a generic set of procedures which - when supplemented by a specific > > profile - define a means of maintaining state synchronization" > > It is very hard to find a wording for this that everyone agrees with. > For now it was changed to "DNCP is an abstract protocol, that must be > combined with a specific profile to make a complete implementable > protocol." based on a suggestion in Ole Troan's review. > > > > > Section 2 Terminology > > > > > > The term "neighbor" is not defined - but used frequently in the document. > > The term "peer" is defined as: > > "another DNCP node with which a DNCP node communicates using a > > particular local and remote endpoint pair." > > > > What I am used to is that the definition above for "peer" is usually > > associated with the term "neighbor", whereas the term "peer" is more > > generic - it is associated with a node in the network which performs > > the same functions in the protocol - but is not necessarily a neighbor. > > The terms "peer" and "neighbor" are used relatively interchangeably in this > document. The terminology has been adapted to reflect that. > > > > Section 4.5 illustrates why I find this confusing as it says > > > > "When receiving a Node Endpoint TLV... the remote node MUST be added > as a peer > > on the endpoint and a Neighbor TLV (Section 7.3.2) MUST be created > > for it." > > ??? > > I think this particular case should actually not be confusing, since all terms > used here are defined in the Terminology or as TLVs. However, we may have > a look at it again. [Les:] Well "neighbor" is not defined - though yours would not be the only specification to omit that definition. And I could intuit using "peer" to identify other nodes operating the protocol who are not necessarily neighbors - but that can't be done using your definition of "peer". > > > > Section 4.4 - final bullet on Page 11 > > > > o Any other TLV: TLVs not recognized by the receiver MUST be > > silently ignored. > > > > Does "ignore mean "discard"? (This is one traditional meaning) > > > > If so this seems inappropriate as it is part of the database sent by > > the node and therefore needs to be retained in order to keep a > > consistent database. Perhaps "store but do not process" is a more > > accurate behavioral description? > > TLVs that are part of the database are sent within the Node State TLV (Node > Data field). Said Node State TLV is mentioned just before "Any other TLV" > in the same list and thus not meant to be part of "other TLV"s. The text now > states this explicitly. > [Les:] But there can be TLVs within the Node State TLV - and these cannot be discarded - though they could be ignored. So there seem to be two classes of "unrecognized TLVs" - those within the Node State TLV and those not associated with the Node State TLV. ?? > > > Section 6.1 Keep-alives > > > > Are keep alives sent in multicast-listener mode? >From the text in > > 6.1.2 and > > 6.1.3 it seems "no" - but I am not certain. > > The are no multicast keep-alives in this case, please see next comment as > well. > > > > Section 6.2 Support For Dense Broadcast Links > > > > If a node is in Multicast-listen+unicast mode does it bear any > > responsibility for publishing state data in the event the node with > > highest node identifier does not have the latest information? I > > presume yes - but the text does not discuss this point. > > Yes, it is implied by "SHOULD treat the endpoint as > a unicast endpoint connected to the node that has the highest node > identifier". > > So effectively, the node with lower identifier (multicast-listen aside) > treats it > as a unicast connection to the node with highest identifier, including state > synchronisation, keep-alives etc. > > Maybe we can explicitly list some of those consequences, if that makes it > more clear. > > > > Also, does multicast-listener mode affect the way neighbors are > advertised? > > It seems not - so what you are preventing w multicast-listener mode is > > redundant state updates - but there is no change to the set of > > neighbors advertised (N*(N-1))? > > The initial paragraph states that that the main reason for this extension is > to > avoid lots of redundant Neighbor TLVs. A later paragraph also states that > usage of this extension results in only a subset of the topology being > advertised. > So I think this point should be pretty clear already. > As for the underlying logic, please see the comment before. > > > > Section 6.3 Node Data Fragmentation > The extension and thereby this section has been removed as requested by > another reviewer. > > > > Section 7 TLVs > > It says "padding bytes with value zero"". > > > > Is "0' a MUST - or is this the usual "SHOULD be transmitted as zero > > and ignored on receipt"? > > It actually says "Padding bytes with value zero MUST be added" so I guess it > should be pretty unambiguous already. > > Making this a non-MUST would make some things like ordering and hashing > of TLV sets a bit awkward. > > > > Section 7.2.1 Node Endpoint TLV > > When is this sent and how often? > > This is described in detail in 4.2, a reference has been added. > [Les:] Ahhh - thanx for the pointer. I searched for "Node Endpoint" - but Section 4.2 uses simply "Endpoint" - so I did not find a match. If you could use the full TLV name in Section 4.2 that would be helpful. Thanx. Les > > > _______________________________________________ > homenet mailing list > [email protected] > https://www.ietf.org/mailman/listinfo/homenet _______________________________________________ homenet mailing list [email protected] https://www.ietf.org/mailman/listinfo/homenet
