Subject: [Gen-art] Genart early review of draft-ietf-rift-rift-08

From: Robert Sparks via Datatracker <[email-masked]>
Date: Mon, 28 Oct 2019 11:59:12 -0700

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

---

From: Tony Przygienda <[email-masked]>
Date: Mon, 28 Oct 2019 13:51:58 -0700

Robert, reacting as editor here

thanks for your detailed review, answers inline

I'm raising couple questions myself in the answers, if you'd care to
clarify those further I can try to make the
draft cut-off for an updated version

> support for SVG to help convey your state diagrams.

As first observation, we have two interoperable implementations since a
bit, one completely open source which has been produced based on the spec.
It was in fact open source work that helped to refine the document content
to make sure we can have an implementation produced based on the text
without further "guessing things". As example the LIE FSM has been
implemented initially in open source without consulting authors of the spec
and interoperat'ed without a single defect (but discovered a protocol
underspecification in case of misconfiguration that was subsequently
added). Please refer to IETF proceedings for the according presentations if
necessary. We have a third implemenation progressing now where all
questions the implementor asked so far could be answered by pointing
directly @  the specification as written. This seems to answer to me the
"suspicion of specification maybe not being good enough to implement" as an
objective measuring stick as far I can imagine one.

As second observation regarding your comments, the majority of readers
seemed to prefer a format where the protocol is "described" in narrative
and the tight procedure appendices are left to people implementing the
protocol but I have no issue to pull the appendices directly into the
document under the according sections. I would simply add something along
the lines of "this section of interest to a reader implementing the spec".

I will try the v3 format again but last time we tried couple months ago it
wasn't workable. I see first RFC havs been just published using it (albeit
I don't see it using any drawings).

> are many sections that too deeply assume the reader already understands the

Point taken and a scrub will be performed along the lines you suggest.

> sentence individually, but will attempt to point at a few that also had

yes, thank you. that should suffice for a scrub.

> ** The IANA considerations section does not provide a clear set of

Ok, could you be a little bit more specific

9.1.  Requested Multicast and Port Numbers  seems to describe the
allocation points and registries.

9.2.  Requested Registries with Suggested Values requests new
registries with pre-allocated entries, maybe it's not clear that each
subsection is a new registry? I can point that out.

> this term becomes well-defined.

good catch, will be pulled up into "terminology" section and defined.

> Req4, there is a reference to "Paragraph 17". Does that mean "Req 17"?

anomaly of rfc2txt, when referencing a list no matter what the prefix it
will always generate
"paragraph" in the reference. I will see how I can fix that.

> the other K_ terms should be discussed in the terminology section.

the section 5.1.2. was written more as a narrative but no problem, I will
add a special terminology section and define the variables.

> appear to use the symbol inconsistently.

The document says verbatim *"we have chosen to use the "HH" symbol as ASCII
visualisation of a RJ45 jack."  *I will add "single" to it.

Figure 9 may cause confusing since a single H is used to visualize the
jack.  I also see possible inconsistency in Figure 13, will correct,
probably replacing the "HH" with "X"

> "re-dimensioning" are almost jargon in this context.

will try to simplify language where it looks possible.

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

terminology section already says:

TIE:  This is an acronym for a "Topology Information Element".  TIEs
   are exchanged between RIFT nodes to describe parts of a network
   such as links and address prefixes, in a fashion similar to ISIS
   LSPs or OSPF LSAs.

*We will talk about N-TIEs when talking about   TIEs in the northbound
representation and S-TIEs for the   southbound equivalent.*

but sure, I will break that out and define more explicitly

> The words for Adjacency are not really a definition, but taken as such

correct, we failed to root the defintions of interface/link/adjacency since
in p2p link-state
protocols they are pretty much equivalent (modulo tunnels). suggestions?
I'll introduce
3-way as clear defintion and define adjacency as "3-way state on a link"

> The definition for Metric is not useful.

ok, will be removed

> return to the discussion.

ok, I'll list the exceptions in detail here

> somewhere in "the introduced Section 3.1".

word missing

> are comments, not actions. Numbering them as you number actions is

any better suggestions? (I will move from numbering to number.latin.other
and so on). This is
an old technique in protocol specs to allow implementors to discuss very,
very precise statement
and their meaning (e.g. ISIS spec is all written like that). Otherwise
people start to talk about
"4th line in the algorithm X" which is far more confusing.

rfc2txt does not have a mechanism to "skip numbering for this item" when
genreeating
lists unless I'm oblivious to it so I'm limited by the tools IETF provides
to render documents.

> The last Step 7 in section 5.2.3.9 speaks of "sine flooding reduction".

some editorial slip on find/replace I assume, was "such" AFAIR, seems
slipped -04 or -05. missed by
good amount of readers ;-)

> The reference in 5.2.7.2's first paragraph to [EUI64] looks like a

I wasn't sure since the EUI64 is kind of a "side implementation note in
IEEE". Can move to normative.

> to "we have to decide whether node Y is ... at the same level as Y".

correct, sentence is illogical. I will correct

> TwoWay or ThreeWay state? Or something else?

I will define "bi-directional adjacency", it cannot hurt.
But the term is kind of well understood by anyone who
implemented OSPF/ISIS or is intimately familiar with the specs normally.

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

slipped. PGP has been folded out the main document and this got missed. I
probably remove the reference
since we cannot publish standard with reference to a draft while PGP work
is going slowly.

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

This question/observation/doubt was coming up many, many times and that's
why the document
introduced this section. I prefer to retain it to prevent the question
being asked over and over again
on the reading of the spec.

> relatively benign." does not parse.

yes

> so I suggest it not be added to the registry.

implementation specific basically. Can be removed but allows in
implementation to
use the codepoint to scale arrays for example.

If the registry takes

DirectionMaxValue

for e.g. version 3.0 schema this value could move. Alternately we
could split a registry
_per schema version_ but that seems a huge proliferation and replication.

I'm agnostic either way and would like to hear an opinion here

> are looking for. Please clarify.

ok, will explain rollover in relation to Appendix albeit it's a well-known
technique
since many years in link-state protocols (explained e.g. in depth in John
Moy's book) though
lacking a rigorous implementation description such as Appendix A provides.

> but not transitive". Symmetric says "if A>:B then  B>:A".

yes, symmetric means "if A>:B then  B>:A" precisely what you say and the
relation holds.

non transitive means obviously that

A>:B and B>:C does NOT imply A>:C

Obvious example of the 3-bit arithmetic is

4 >: 2 & 2 >: 0 does NOT follow in 4 >: 0

That seems quite clear to me so I don't see easily what you question unless
you
care to explain more.

> The document has many lines that exceed the maximum line width for txt

ack, will correct

> would be much clearer in the terminology section.

will correct

> the word you are looking for.

"homonym" is precisely what is meant hear, the term has been overloaded to
mean two different things over time.

Will split into shorter sentences.

> first block is incorrect?

yes, sorry, slipped. we had pending? requirements which slid into non
requirements or has
been removed. this one is a left over.

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

correct, thanks

> convenient". I suspect it meant to be "It proves convenient"

correct

> has an adjacency with a neighbor that is in the three-way state"?

yes, should clarify via crisp definition of 3-way.

> confused.Please rework the sentence to avoid the pronouns.

ack

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

ack

> information to the document. Please remove it.

ack

> At section 6.1, please explicitly point to figure 2 when mentioning the

ack

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

ack

---

From: Robert Sparks <[email-masked]>
Date: Mon, 28 Oct 2019 16:16:07 -0500

On 10/28/19 3:51 PM, Tony Przygienda wrote:
> On Mon, Oct 28, 2019 at 12:00 PM Robert Sparks via Datatracker 
> <[email-masked] <mailto:[email-masked]>> wrote:
>
>     Reviewer: Robert Sparks
>     Review result: Not Ready
>
> As first observation, we have two interoperable implementations since 
> a bit, one completely open source which has been produced based on the 
> spec. It was in fact open source work that helped to refine the 
> document content to make sure we can have an implementation produced 
> based on the text without further "guessing things". As example the 
> LIE FSM has been implemented initially in open source without 
> consulting authors of the spec and interoperat'ed without a single 
> defect (but discovered a protocol underspecification in case of 
> misconfiguration that was subsequently added). Please refer to IETF 
> proceedings for the according presentations if necessary. We have a 
> third implemenation progressing now where all questions the 
> implementor asked so far could be answered by pointing directly @  the 
> specification as written. This seems to answer to me the "suspicion of 
> specification maybe not being good enough to implement" as an 
> seemed to prefer a format where the protocol is "described" in 
> narrative and the tight procedure appendices are left to people 
> implementing the protocol but I have no issue to pull the appendices 
> directly into the document under the according sections. I would 
> simply add something along the lines of "this section of interest to a 
> reader implementing the spec".
>
> I will try the v3 format again but last time we tried couple months 
> ago it wasn't workable. I see first RFC havs been just published using 
> it (albeit I don't see it using any drawings).
>
>

You can get AD help here. See section 2.2 of 
https://datatracker.ietf.org/doc/rfc8126/.

9.2 is where I am concerned.

Please add an explicit statement that you are asking IANA to create a 
new top-level registry named RIFT with the following subregistries under it.

>
> 9.1.  Requested Multicast and Port Numbers  seems to describe the allocation points and registries.
> 9.2.  Requested Registries with Suggested Values requests new registries with pre-allocated entries, maybe it's not clear that each subsection is a new registry? I can point that out.
>
>
> anomaly of rfc2txt, when referencing a list no matter what the prefix 
> it will always generate
> the section 5.1.2. was written more as a narrative but no problem, I 
> will add a special terminology section and define the variables.
>
>
> The document says verbatim /"we have chosen to use the "HH" symbol as 
> ASCII visualisation of a RJ45 jack." /I will add "single" to it.
>
>     LSPs or OSPF LSAs.*We will talk about N-TIEs when talking about TIEs in the northbound 
> correct, we failed to root the defintions of interface/link/adjacency 
> since in p2p link-state
> protocols they are pretty much equivalent (modulo tunnels). 
> suggestions?  I'll introduce
> any better suggestions? (I will move from numbering to 
> number.latin.other and so on). This is
> an old technique in protocol specs to allow implementors to discuss 
> very, very precise statement
number.latin.other will help.
>
> rfc2txt does not have a mechanism to "skip numbering for this item" 
> when genreeating
> lists unless I'm oblivious to it so I'm limited by the tools IETF 
> provides to render documents.
Do you mean xml2rfc when you say rfc2txt?
> I wasn't sure since the EUI64 is kind of a "side implementation note 
> in IEEE". Can move to normative.
>
>
> slipped. PGP has been folded out the main document and this got 
> missed. I probably remove the reference
> since we cannot publish standard with reference to a draft while PGP 
> work is going slowly.
>
>
> This question/observation/doubt was coming up many, many times and 
> that's why the document
> could split a registry _per schema version_ but that seems a huge 
> proliferation and replication.
I'll punt this to the group/AD.
> ok, will explain rollover in relation to Appendix albeit it's a 
> well-known technique
> since many years in link-state protocols (explained e.g. in depth in 
> John Moy's book) though
I will reread. 4 >: 2 and 2 >: 4 surprises me.
>
> That seems quite clear to me so I don't see easily what you question 
> unless you
> care to explain more.
>
>
>     === Nits ===
>

---

From: Tony Przygienda <[email-masked]>
Date: Mon, 28 Oct 2019 17:54:33 -0700

On Mon, Oct 28, 2019 at 2:16 PM Robert Sparks <[email-masked]> wrote:

> Please add an explicit statement that you are asking IANA to create a new
> top-level registry named RIFT with the following subregistries under it.
>

yes, no problem.

> number.latin.other will help.

ok, thanks

> Do you mean xml2rfc when you say rfc2txt?

yes, correct. sorry.

> for e.g. version 3.0 schema this value could move. Alternately we could split a registry

ack

> I will reread. 4 >: 2 and 2 >: 4 surprises me.

oh, sorry, that's the objection, I thought you objected to transitive and
focused on that. yes, you're correct, it's not symmetric, I was blind and
implied

4 >: 2  <=>  2 <: 4

which is not symmetry of course. I will correct.