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

[Nagendra Kumar via Datatracker <[e-mail]>]

Reviewer: Nagendra Kumar
Review result: Has Issues

Hi,

I have reviewed this document as part of the Operational directorate's ongoing
effort to review all IETF documents being processed by the IESG.  These
comments were written with the intent of improving the operational aspects of
the IETF drafts per guidelines in RFC5706.

Comments that are not addressed in last call may be included
in AD reviews during the IESG review.  Document editors and WG chairs should
treat these comments just like any other last call comments.

Overall Summary:

This draft is a standard track proposing a very new protocol and not an
extension of any existing protocol. As such, there are no backward
compatibility issues expected. Overall this is a well written document. Some
ambiguity that needs attention are listed below. I chose "Has Issues" primarily
to address the issue that may cause behavior inconsistency or traffic blakc
holes. More details below:

--> LIE with TTL more than 1 is clearly defined to be discarded. I couldn't
find a similar clarification when TIE is received with TTL more than 1?. If i
didn't miss it, I think it is better to be clarified for consistent behavior.

5.3.7.  Label Binding

   A node MAY advertise on its TIEs a locally significant, downstream
   assigned label for the according interface.  One use of such label is
   a hop-by-hop encapsulation allowing to easily distinguish forwarding
   planes served by a multiplicity of RIFT instances.

--> Read this more than a couple of times and not sure if "for the accordingly
interface" means assigning a locally unique label for each interface/adjacency
or just one label for the node and advertise the same label over all adjacency.

--> Since it is mentioned that the use of this label is for HbH encapsulation,
a node at Level n should install (or ignore) the label from node at Level (n-1)
in its Incoming Label Mapping (ILM) table?.

--> On the same line, a node at Level n should install (or ignore) the label
from node at Level (n-2)?.

--> I think some clarity is better to avoid traffic black holing due to
inconsistent behavior.

5.3.8.1.  Global Segment Identifiers Assignment

   Global segment identifiers are normally assumed to be provided by
   some kind of a centralized "controller" instance and distributed to
   other entities.  This can be performed in RIFT by attaching a
   controller to the Top-of-Fabric nodes at the top of the fabric where
   the whole topology is always visible, assign such identifiers and
   then distribute those via the KV mechanism towards all nodes so they
   can perform things like probing the fabric for failures using a stack
   of segments.

--> According to RFC 8402, Global Segment is a generic term that refers to a
segment from SR Global Block with instruction or forwarding semantic defined at
the domain level. But it does not have a default instruction associated with
it. I think the above should be more specific as Prefix SID or something
related to RIFT.

Few observations below:

Clos/Fat Tree:  This document uses the terms Clos and Fat Tree
      interchangeably whereas it always refers to a folded spine-and-
      leaf topology with possibly multiple PoDs and one or multiple ToF

--> ToF and PoD were not expanded in the first occurrence. ToF was explained
later.

--> While Southbound representation is explained in the terminology section,
northbound representation is not. I think it is better to define the same for
consistency.

Multiple
      adjacencies can be formed to a neighbor via parallel interfaces
      but such adjacencies are NOT sharing a neighbor structure.  Saying
      "neighbor" is thus equivalent to saying "a three way adjacency".

--> The above says "Multiple adjacencies can be formed to a neighbor.." where
the "neighbor" here is implicitly defining the remote node and then the
sentence further continues stating "Saying "neighbor" is thus equivalent to
saying "a three way adjacency"".  May be the first "neighbor" can be changed to
"remote node"?

mobility defined in Paragraph 17

--> I assume that you are referring to REQ#17?. I think it is better to refer
it as REQ#17. But I will leave that to the authors to decide.

 The solution should allow for access to link states of the
            whole topology to enable efficient support for modern
            control architectures like SPRING [RFC7855] or PCE
            [RFC4655].

--> RFC 7855 is an information RFC that outlines the problem statement and use
case. I think you should refer RFC 8402 here for the SR architecture.

"Future
   versions of this document may describe support for such tunneling in
   RIFT."

--> Section 5.3.3.4 talks about Overlay encapsulation and leave it a bit
ambiguous with the above statement. I think, it should be removed.

already to HAT nodes with level different than the adjacent

--> HAT was first expanded in page 66 while the abbreviation is used in/from
Page 34.

In Section 5.2.3.2, I hope the first node is "ToF21 S-TIEs" instead of "Spine21
S-TIEs"?. Similar references are throughout the example with Spine21 and
Spine22.

"scope of this document and may be covered in a separate document
   about policy guided prefixes [PGP reference]."

--> There is no PGP reference. This may need to be fixed or removed.