[Rift] Genart early review of draft-ietf-rift-rift-08

Robert Sparks via Datatracker <noreply@ietf.org> Mon, 28 October 2019 18:59 UTC

Return-Path: <noreply@ietf.org>
X-Original-To: rift@ietf.org
Delivered-To: rift@ietfa.amsl.com
Received: from ietfa.amsl.com (localhost [IPv6:::1]) by ietfa.amsl.com (Postfix) with ESMTP id 04B1412094B; Mon, 28 Oct 2019 11:59:12 -0700 (PDT)
MIME-Version: 1.0
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: 7bit
From: Robert Sparks via Datatracker <noreply@ietf.org>
To: <gen-art@ietf.org>
Cc: draft-ietf-rift-rift.all@ietf.org, rift@ietf.org
X-Test-IDTracker: no
X-IETF-IDTracker: 6.108.0
Auto-Submitted: auto-generated
Precedence: bulk
Reply-To: Robert Sparks <rjsparks@nostrum.com>
Message-ID: <157228915192.16080.15649375711620871809@ietfa.amsl.com>
Date: Mon, 28 Oct 2019 11:59:12 -0700
Archived-At: <https://mailarchive.ietf.org/arch/msg/rift/_XVgxCvRkFxKDXt1VZTHwGIVeiE>
Subject: [Rift] Genart early review of draft-ietf-rift-rift-08
X-BeenThere: rift@ietf.org
X-Mailman-Version: 2.1.29
List-Id: Discussion of Routing in Fat Trees <rift.ietf.org>
List-Unsubscribe: <https://www.ietf.org/mailman/options/rift>, <mailto:rift-request@ietf.org?subject=unsubscribe>
List-Archive: <https://mailarchive.ietf.org/arch/browse/rift/>
List-Post: <mailto:rift@ietf.org>
List-Help: <mailto:rift-request@ietf.org?subject=help>
List-Subscribe: <https://www.ietf.org/mailman/listinfo/rift>, <mailto:rift-request@ietf.org?subject=subscribe>
X-List-Received-Date: Mon, 28 Oct 2019 18:59:20 -0000

Reviewer: Robert Sparks
Review result: Not Ready

Summary: Not ready for publication as a Proposed Standard

I found this document very difficult to read. On my first past through, I was
fairly certain it didn't specify the protocol well enough to actually implement
it, until I got deep into the appendices. Even after a second pass, I am not
comfortable that a implementation could be produced without having guess at
some behavior, which will lead to interoperability problems.

I strongly suggest restructuring the draft to pull the actual definition of the
protocol from the appendix and into the early part of the document text. You
may also want to use the new v3 format so that you can take advantage of its
support for SVG to help convey your state diagrams.

At the very least, the document needs a strong editorial scrub. There are many
sentences that are missing important articles. There are many sentences that
are very complex and would be better written as two or three sentences. There
are many sections that too deeply assume the reader already understands the
problem space.

In the remainder of the review, I'm not going to call out each overly-complex
sentence individually, but will attempt to point at a few that also had other
issues.

Some high-level points before going into a document-order set of comments:

** The IANA considerations section does not provide a clear set of instructions
for IANA to follow.

* The notion of a "three-way adjacency" is not made clear in the document. It
really takes looking at a rendering of the dot for the state machines before
this term becomes well-defined.

* There are a few places where there are internal references in the document
that are not clear. These center around the word "Paragraph". For instance, in
Req4, there is a reference to "Paragraph 17". Does that mean "Req 17"?  

* K_LEAF suddently appears without support or definition in 5.1.2. That and
the other K_ terms should be discussed in the terminology section.

* It's unclear whether HH symbol means one or two RJ45 jacks. The diagrams
appear to use the symbol inconsistently.


=== Issues in document order ===

In the introduction:

"The incumbent protocols precondition normally extensive configuration or
provisioning during bring up and re-dimensioning which is only viable for a set
of organizations with according networking operations skills and budgets." did
not parse for me until my second read-through, where I realized "precondition
normally" was intended to mean "normally require". "bring up" and
"re-dimensioning" are almost jargon in this context.

In the Terminology section:

It would be good to introduce S-TIE and N-TIE as top level terms in this
section.

The words for Adjacency are not really a definition, but taken as such they are
circular.

The definition for Metric is not useful.

In the first paragraph of 5.1.1, The construction of 'never ... (we'll talk
about exceptions later)' is awkward. It's also unclear where the the "later" is
pointing. The last sentence is grammatically incorrect ("We omit ... out"), and
talks of omimitting discussion "for the moment". Please make it clear where you
return to the discussion.

The third paragraph of 5.1.2 does not parse. I suspect there is a missing word
somewhere in "the introduced Section 3.1".

The nested numbered lists in section 5.2.3.9 are perhaps not the best tool for
describing the algorithms you want to convey. On page 46, steps 4.3.3 and 4.4
are comments, not actions. Numbering them as you number actions is confusing.

The last Step 7 in section 5.2.3.9 speaks of "sine flooding reduction". What is
that?

The reference in 5.2.7.2's first paragraph to [EUI64] looks like a normative
reference to me. 

Something is wrong at the bottom of page 68. One branch of the sentence reduces
to "we have to decide whether node Y is ... at the same level as Y".

In 5.3.4.1 rule 1, you only consider nodes to which the reciever has a
bi-directional adjacency. As opposed to what? Does this mean adjacencies in
TwoWay or ThreeWay state? Or something else?

There is a dangling reference to [PGP reference] at the end of 5.3.11.

Section 7.3 is really not useful for the protocol specification. Consider
removing it.

In section 8.4 the sentence "The attack vector packet number present is
relatively benign." does not parse.

It's particularly unclear what you are trying to achive with the
"DirectionMaxValue" registry entry defined in 9.2.11.1 Are you trying to say
the registry is not allowed any more values? If so, just say that in the
instructions to IANA. I don't see where the codepoint is used by the protocol,
so I suggest it not be added to the registry.

It's a leap to get from the description of Sequence Number Binary Arithmetic in
Appendix A to the "rollover" mechanic the paragraphs that reference Appendix A
are looking for. Please clarify. 

Also in Appendix A, I question the sentence "The >: relationship is symmetric
but not transitive". Symmetric says "if A>:B then  B>:A". 

=== Nits === 

The document has many lines that exceed the maximum line width for txt
formatted RFCs.

The definition section uses terms before it defines them. For those that are
acronyms (see the first use of PoD and ZTP for example) an expansion would
help. The short-hand "E-W" isn't saving much over "East-West" and the latter
would be much clearer in the terminology section.

In section 3.2 at " We will use topology in Figure 2 (called commonly a fat
tree/network in modern IP fabric considerations [VAHDAT08] as homonym to the
original definition of the term [FATTREE]) in all further considerations.".
This would better be conveyed as two seperate sentences. I don't think
"homonym" (same spelling or pronunciation, but different meanings) is really
the word you are looking for.

On page 17, you have a block labelled "Following list represents
non-requirements", followed by a single PEND requirement, and then "Finally,
following are the non-requirements" before the NONREQs. The description of the
first block is incorrect?

The last sentence in the paragraph immediately following Figure 10 has no verb.

The second paragraph after Figure 10 includes a sentence that begins "If proves
convenient". I suspect it meant to be "It proves convenient"

In section 5.2.2 the phrase "if a link has entered three way IPv4 and/or IPv6
with a neighbor on an adjacency" should be reconstructed. I _think_ you mean
has an adjacency with a neighbor that is in the three-way state"?

In section 5.2.3.5, the pronouns in "It should only set it" can become
confused.Please rework the sentence to avoid the pronouns. 

There is a stray double-quote at the end of the first sentence in 5.2.7.2

At 5.2.8 "(with some finer distinctions not explained further)" adds no useful
information to the document. Please remove it.

At section 6.1, please explicitly point to figure 2 when mentioning the example
topology.

The term "ToRs" appears without definition or reference at section 7.1