Early Review of draft-ietf-rift-rift-08
review-ietf-rift-rift-08-rtgdir-early-hardwick-2019-10-31-00
review-ietf-rift-rift-08-rtgdir-early-hardwick-2019-10-31-00
Hello I have been selected to do a Routing Directorate “early review” of this draft: https://datatracker.ietf.org/doc/draft-ietf-rift-rift/ The routing directorate will, on request from the working group chair, perform an “early” review of a draft before it is submitted for publication to the IESG. The early review can be performed at any time during the draft’s lifetime as a working group document. The purpose of the early review depends on the stage that the document has reached. As this document has advanced to working group last call, my focus for the review was to determine whether the document is ready to be published. Please consider my comments along with the other working group last call comments. For more information about the Routing Directorate, please see http://trac.tools.ietf.org/area/rtg/trac/wiki/RtgDir Document: draft-ietf-rift-rift Reviewer: Jon Hardwick Review Date: 31 Oct 2019 Intended Status: Standards Track Summary Thanks for writing this document. It is a very interesting approach and I really enjoyed getting to grips with the ideas presented in the draft! Unfortunately, I have some concerns about the document and think it needs more work before being submitted to the IESG. The problem is that I found the document hard to read, for several reasons. * It is very light in its use of normative RFC-2119 style language. An implementer would have to fill in quite a few gaps and/or make assumptions about various passages. * The definition of the protocol and some of the normative behaviour is deferred to the appendices, whereas I would expect to encounter it early on in the text, with an in-line discussion of the purposes of the messages and fields. * It sometimes refers to concepts or terms that are either not defined or have not yet been introduced to the reader, suggesting an ordering issue within the text. I think that the document needs to be refactored somewhat to solve the ordering issues, use more normative language, eliminate any text that is not actually relevant to the implementation and deployment of the protocol, and pull together the normative definition of the protocol into a contiguous block early on in the document. The other issue is that, because the document is large and I found it rather hard going, I did not have time do a thorough review beyond section 5.3. I’d therefore have to recommend another directorate review once we have concluded on the issues I’m raising below. Details Here are comments on the sections that I was able to review in detail before I ran out of time. Abstract Is it possible to reformat this as a list of items on multiple lines? It would read more clearly. Section 2 "an optimal approach does not seem however": this appears to be a value judgment rather than consensus opinion, appearing as it does without citation, and may be perceived as treading on the toes of other standardization efforts currently in progress at the IETF. I suggest you simply state the facts: "RIFT approaches this problem using a mixture of..." Section 2.1 The form of words in the Requirements Language boilerplate has changed recently - see RFC 8174. Section 3.1 ZTP - expand acronym on first use. There is potential for confusion between N-TIE and Node TIE! I'd prefer "North TIE" for the former. An example of confusion: is the "South Node TIE" referred to in the definition of "South Reflection" the same as the S-TIE referred to in the definition of "TIE"? "The document sometimes calls them flood leaders as well." But it would be better if you just used one term. Section 4 Personally I could live without this section Merge PEND1 with NONREQx (or explain the distinction) Section 5.1.3 - 5.1.5 This discussion is not possible to follow properly until you have been introduced to positive & negative disaggregation and southern reflection. As such I wonder if it really belongs in a section called "overview". Section 5.2.2 A node configured with "undefined" PoD membership MUST, after building first northbound three way adjacencies to a node being in a defined PoD, advertise that PoD as part of its LIEs. In case that adjacency is lost, from all available northbound three way adjacencies the node with the highest System ID and defined PoD is chosen. It seems odd that the choice of advertised pod is at first non-deterministic (race to the first adjacency) and then, only if this initial adjacency is lost, the choice of pod becomes deterministic. Why not make it deterministic the whole time? Section 5.2.3.2 In the example TIEs, "Spine21" should be "ToF 21" to agree with the nomenclature of figure 2. Ditto in table 4 (section 5.2.3.4) In Spine 111's Node-S-TIE, I am not sure that the links(...) should be given for each neighbor. Section 5.2.3.5 "It should only set it in the southbound direction." - SHOULD? Section 5.2.3.8 Define N-SPF on first use Section 5.2.4 "A node has three sources" - I see only two listed. "We use simple, familiar SPF algorithms here..." - is the use of those algorithms supposed to be normative? Or are you just giving an example and leaving me to choose my own algorithm? If SPF is normative then you need to specify it using normative language or include a normative reference to it. Section 5.2.4.1 Please define the terms "south prefix" and "north prefix" "Supersuming" is not a word I recognise. Use "or a non-default prefix which contains this south prefix" "the node does not..." -> "the computing node does not..." Section 5.2.4.2 "S-SPF uses northbound adjacencies in node N-TIEs to verify backlink connectivity" - this statement needs to be recast into normative language using RFC 2119 terms. "A node MUST verify backlink connectivity ... Else it MUST NOT include the link.... Etc." Same comment applies in many places throughout the document. Section 5.2.4.3 What is a `"ring protection" scheme`? Are E-W links permitted between planes? Not sure what this is telling me: "Using south prefixes over horizontal links is optional..." - is that OPTIONAL as in RFC 2119? Do you mean that my implementation can ignore them? Or not advertise them? Or that the network operator does not have to cable them? Section 5.2.4.4 "Even though a ToF node could be tempted to use those links during southbound SPF this MUST NOT be attempted since it may lead in, e.g. anycast cases to routing loops." This is too verbose and obtuse. I cannot see how anycast cases lead to routing loops and I don't know if I need to understand why or not. Suggest: "A ToF node MUST NOT include east-west links in its south-SPF calculation." This section gives the impression that E-W links at the ToF will never be used for forwarding data - is that true? They are used for control plane only? "An implementation could try ... but the details are outside this specification" - so why mention it? Section 5.2.5.1 "A DAG computation" - expand DAG. "Neither is it necessary for the receiving node to reflect the disaggregated prefixes back over its adjacencies to nodes at the level from which it was received." Please restate this using RFC 2119 language. How can we guarantee that a same-level node does not have a next hop to a given prefix that is unknown to the node doing the computation? If X reaches P via N1 and N2, Y (at the same level as X) can reach P via N3 but X does not know this and assumes Y cannot reach P because Y is not adjacent to N1 and N2, then X unnecessarily disaggregates P positively. For instance if X's link to N3 has failed and Y's links to N1 and N2 have failed. "Each entry is a list of south neighbor of X and a list of nodes of X.level that can't reach that neighbor" Think this should say "Each entry in the set is a south neighbor of X and a list of nodes of X.level that can't reach that neighbor" "X does not to disaggregate any prefixes" -> ""X does not disaggregate any prefixes."" "The PoD containing the prefix will prefer southbound anyway." - I didn't understand the point. Is it necessary for me to understand it? Please expand or delete the sentence if it's not necessary. Section 5.2.6 "such as mobility per section 5.3.3 necessary" - delete "necessary". "ties are broken based upon type first and then distance and further attributes" - I don't see mention of further attributes in the proposed algorithm. "The nexthop adjacencies for a negative prefix are inherited from the longest prefix that aggregates it" - suggest changing to "longest positive prefix" "all entries of the father" -> "all entries of the parent" Section 5.2.7.3 "we have to decide whether node Y is at the same level as I, J or at the same level as Y and consequently, X is south of it." I could not parse this. I think you might mean this: "we have to decide whether node Y is at the same level as I, J (and consequently X is south of it) or at the same level as X." Section 5.2.7.4 How does a ToF node know what value to advertise in its LEVEL_VALUE?