Re: [Rift] AD Review of draft-ietf-rift-rift-12 (Part 1)

[Jordan Head <[e-mail]>]

Hi Alvaro,

As discussed, I have added myself as the 6th author and will start a separate thread with you, the chairs, and the shepherd (Sandy).

My replies to your direct comments for part 1 are inline as jhead>>

Part 2 coming shortly.

Thank you

On 7/8/22, 8:00 AM, "RIFT on behalf of Alvaro Retana" <[e-mail] on behalf of [e-mail]> wrote:

    [External Email. Be cautious of content]


    On May 10, 2021 at 5:08:45 PM, Tony Przygienda wrote:


    Tony/authors:

    Hi!

    After clearing some of my queue, I'm back on this document...still
    will be interleaving with others.  :-(


    Alvaro.


    ...
    > > Part 1 includes the introductory text through the end of §4.1 (Overview).
    > >
    > > While I appreciate the overview of the topologies and the protocol, I
    > > think that this extended introductory material is at times too
    > > long/wordy and complex (for the average reader, sometimes being forced
    > > to make assumptions or jump ahead), but also incomplete. To be more
    > > specific:
    >
    > reader's guide upfront should help that, i.e. guide readers depending on
    > what they need to correct section, tell them what needs reading

    It does.  Thanks!

jhead>> Great! I'm glad this makes things easier.

    I'm appending at the bottom some comments on the new section and other
    new text, based on -15.


    > > - The description of the general topology (§4.1.2) gets in to significant
    > > detail and complexity, including the way in which the concepts are depicted
    > > in the figures. (ASCII art is not the best, you might want to take
    > > advantage of using SVI.) I noticed that none of the figures/sections in
    > > §4.1.2 are referenced elsewhere (but the simple Figures 2 and 3 are), which
    > > makes me think about the value of the in-depth treatment if it will not be
    > > explicitly considered later on.
    >
    > Jordan is preparing very nice SVGs. We will retain the ASCII form (it's
    > actually easier on the mind _once_ one understands it is the feeling
    > by most authors) while the SVG will allow an "intuitive first understanding".

    That's fine.

    However, there are some differences between the figures.  I realize
    that some of the details won't be able to fit in the ASCII version --
    but please try to keep them as close as possible.

    The most important thing is for the descriptions to apply to whichever
    version of the document is being used.

    - Figure 1: the SVG shows 32-bit addresses (A.A.A.A/32), but the ASCII
    only has room for A/32.  It is not obvious that A/32 and A.A.A.A/32
    are the same.

    The text describing the figure says this: "...the notation A/32 is
    used to indicate a loopback route to node A and 0/0 is the usual
    notation for a default route."  But A/32 is not in the SVG...  Bottom
    line: be mindful in checking that the text and the figures match.  In
    this case it looks like the easy fix is to use A/32 in the SVG.

jhead>> Fixed. I've relabeled SVG to align with ASCII as you indicated.

    - Figure 2: There are some subtle differences like the indication of
    directionality: for example, the ASCII explicitly shows an arrow ("^")
    to indicate that P111/2 is going up, but the SVG doesn't (it uses a
    partial line from the node sending the information).  The reader's
    digest mentions that the indication of direction are of "paramount
    importance", so this seems like a good thing to be consistent about.

jhead>> Some background context is required to address this.
jhead>> The PDF version(s) you've most recently reviewed are the result of how XML2RFC renders PDFs (to show SVGs). It basically takes the newer HTML format and pushes it into PDF.
jhead>> XML2RFC has some bugs/limitations when rendering the SVGs for this type of PDF, I've seen various presentations of this:
jhead>> One way this has presented was with what you saw here (a lack of arrows to indicate directionality), when the SVG does in fact have the arrows.
jhead>> The more optimal format that does render things correctly, is the newer HTML variant. We were previously unable to leverage this format as the draft upload tool didn't like any XML > 3M.
jhead>> I've spent some of this time to get the expanded XML document to under the 3M limit so that we can use the newer HTML format to view the SVGs.

jhead>> The offending PDF format is not the same one that the upload tool automatically renders, so this will no longer be a concern.
jhead>> We have an issue open with the tools folks for the 3M draft upload limit.

    The SVG has an "All TIEs" indication on the side, covering the whole
    topology.  But the ASCII seems to indicate that "All TIEs" go from
    Spin112 to ToF 22 only.

jhead>> Fixed. I've adjusted the ASCII image to depict the "All TIEs" to appropriately match the SVG.

    - Figure 3: the figures are completely different!

jhead>> Fixed. I reworked the ASCII variant to better align with the SVG's labels and structure.
jhead>> Section 4.1.3 "Fallen Leaf Problem" references this figure, so I slightly reworked some text for that to match.

    - Figure 7: the nodes are not tagged in the ASCII version.

jhead>> Fixed - ASCII has labels to match SVG.

    - Figure 8: the nodes have a different tag.

jhead>> Fixed - ASCII has labels to match SVG.

    - Figure 13: the figures are completely different!

jhead>> Fixed. I've reworked the ASCII to better align with the SVG (and in turn the newly reworked ASCII Figure 3).

    > We will check figure references and make sure they are all refered to
    > after we have the negative section reordered (example front) and SVG done.

    I think that Figures 16 and 28 are still not referenced anywhere.

jhead>> For Figure 16, I've changed this text:
jhead>>     "If partial_neighbors isn't empty, then the following procedure describes how to identify prefixes to disaggregate."
jhead>> To this, which now references Figure 16:
jhead>>     "If partial_neighbors isn't empty, then the procedure in Figure 16 describes how to identify prefixes to disaggregate."

jhead>> For Figure 28, I've changed this text:
jhead>>     "This section specifies the precise, normative ZTP FSM and can be omitted unless the reader is pursuing an implementation of the protocol."
jhead>> To this, which now references Figure 28:
jhead>>     "This section specifies the precise, normative ZTP FSM and can be omitted unless the reader is pursuing an implementation of the protocol. For additional clarity, a graphical representation of the ZTP FSM is depicted in Figure 28."

    ...
    > > 273 2. Introduction
    ...
    > > [nit] s/Fat-Tree/Fat Tree/g To be consistent with the terminology section.
    >
    > done, this was the only instance

    [] Not considering the references, I still count 5 instances.

jhead>> Fixed all non-reference instances.

    ...
    > > [minor] s/NOT/not/g This is not one of the rfc2119 keywords, so it
    > > should not be capitalized. I know you're doing it for emphasis, but
    > > it will be eventually changed -- so let's take care of it now.
    >
    > replaced whole document non-normative NOT with *not* and AND with *and*

    Ok.

    There are still one "NOT" (in the Node TIE definition) and one "AND"
    (in 4.2.2.1) left.

    ...

jhead>> Fixed the "NOT" in the Node TIE definition.
jhead>> Ideally, I'd like to leave the "AND" in 4.2.2.1 as the section uses logical AND and logical OR in a programming context that is helpful for implementers. I don't think there is any confusion in terms of normative language here.
jhead>> If it's a hard requirement to change, I will.

    *****
    What follows are comments on -15 covering the same sections (through
    the end of §4.1 (Overview)).
    *****


    [Line numbers from idnits.]

    ...
    166     1.  Introduction
    ...
    199        The protocol does further provide

    [nit] Add a ":".

jhead>> Done.

    201        *  optional fully automated construction of fat-tree topologies based
    202           on detection of links without any configuration (Section 4.2.7)
    203           while allowing for traditional configuration and arbitrary mix of
    204           both types of nodes as well,

    [] "traditional" carries some non-inclusive connotations.  Consider an
    alternative, suggestions:

          - classic
          - classical
          - common
          - conventional
          - customary
          - fixed
          - habitual
          - historic
          - long-established
          - popular
          - prescribed
          - regular
          - rooted
          - time-honored
          - universal
          - widely used
          - widespread

jhead>> Good suggestion.
jhead>> Changed "traditional configuration" to "conventional configuration methods".

    ...
    208        *  automatic pruning and load balancing of topology flooding
    209           exchanges over a sufficient subset of links which resolves the
    210           traditional problem of link-state protocol struggling with densely
    211           meshed graphs due to high volume of flooding traffic
    212           (Section 4.2.3.9),

    [major] "...which resolves the traditional problem of link-state
    protocol struggling"   Comparing with other protocols -- and the
    problem is not described anyway.

    Suggestion>
       *  automatic pruning and load balancing of topology flooding
          exchanges over a sufficient subset of links (Section 4.2.3.9),

jhead>> I think the problem is stated but could be a bit clearer.
jhead>> Let's use your suggestion and simply add a clarifying statement as I think it carries weight here.
jhead>> "automatic pruning and load balancing of topology flooding exchanges over a sufficient subset of links (Section 4.2.3.9). This resolves scaling and convergence challenges caused by increased levels of flooding when typical link-state protocols are used in densely meshed graphs."

    214        *  automatic aggregation (Section 4.2.3.8) and consequently automatic
    215           disaggregation (Section 4.2.5) of prefixes on link and node
    216           failures to prevent black-holing and suboptimal routing,

    [] "black-holing" and other variations ("blackhole") are used in
    several parts.  Please consider using more inclusive terminology.
    Perhaps something like "dropped" or "lost traffic".

    https://urldefense.com/v3/__https://www.ietf.org/about/groups/iesg/statements/on-inclusive-language/__;!!NEt6yMaO-gk!F8z-OUNjxZdzzL3TRm6dzxBn8veU3zsnYW2XjkLyjJPIKbxGWAjwLgD9PnG5rk9F1bwnfeguxZ8FuPsozOvV$

jhead>> Done.

    ...
    223        *  re-balancing of traffic towards the spines based on bandwidth
    224           available (Section 4.3.7.1) and finally

    [nit] s/ and finally/, and finally

jhead>> Done.

    ...
    272     1.1.  Requirements Language

    274        The key words "MUST", "MUST NOT", "REQUIRED", "SHALL", "SHALL NOT",
    275        "SHOULD", "SHOULD NOT", "RECOMMENDED", "NOT RECOMMENDED", "MAY", and
    276        "OPTIONAL" in this document are to be interpreted as described in BCP
    277        14 RFC 2119 [RFC2119] RFC 8174 [RFC8174] when, and only when, they
    278        appear in all capitals, as shown here.

    [major] The boilerplate us not exactly what rfc8174 says, so idnits is
    complaining.

          The key words "MUST", "MUST NOT", "REQUIRED", "SHALL", "SHALL
          NOT", "SHOULD", "SHOULD NOT", "RECOMMENDED", "NOT RECOMMENDED",
          "MAY", and "OPTIONAL" in this document are to be interpreted as
          described in BCP 14 [RFC2119] [RFC8174] when, and only when, they
          appear in all capitals, as shown here.

    The difference seems to be in how the documents are referenced:  s/BCP
    14 RFC 2119 [RFC2119] RFC 8174 [RFC8174]/BCP 14 [RFC2119] [RFC8174]

jhead>> Fixed. Will confirm when I upload -16.

    280     2.  A Reader's Digest

    [minor] Given that readers may come to this section before any other
    part of the document, please expand all acronyms on first mention.

jhead>> Fixed. Only offending items were "RIFT", "LIE", and "ZTP".

    ...
    288        The indications to direction (i.e. "top", "bottom", etc.) referenced
    289        in the Section 1 are of paramount importance.  RIFT requires a
    290        topology with a sense top and bottom in order to properly achieve a
    291        sorted topology.  Clos, Fat-Tree, and other similarly structured
    292        networks are conducive to such requirements.  RIFT does allow for
    293        further relaxation of these constraints, they will be mentioned later
    294        in this section.

    [nit] s/indications to direction/indications of direction

jhead>> Fixed.

    [nit] s/sense top/sense of top

jhead>> Fixed.

    296        Operators and implementors alike must understand if multi-plane IP
    297        fabrics are of interest or not.  Section 3.2 illustrates an example
    298        of both single-plane in Figure 2 and multi-plane fabric in Figure 3.
    299        Multi-plane fabrics require understanding of additional RIFT concepts
    300        (e.g.  negative disaggregation in Section 4.2.5.2) that are otherwise
    301        unnecessary in context of strictly single-plane fabrics.  Overview
    302        (Section 4.1) and Section 4.1.2 aim to provide enough context to
    303        determine if multi-plane fabrics are of interest to the reader.  The
    304        Fallen Leaf part (Section 4.1.3), and additionally Section 4.1.4 and
    305        Section 4.1.5 describe further considerations that are specific to
    306        multi-plane fabrics.

    [nit] s/Overview/The Overview

jhead>> Fixed.

    308        The fundamental protocol concepts are described starting in the
    309        specification part (Section 4.2), but some sub-sections are not quite
    310        as relevant unless dealing with implementation of the protocol.  The
    311        protocol transport (Section 4.2.1) is of particular importance for
    312        two reasons.  First, it introduces RIFT's packet formats in the form
    313        of a normative Thrift model given in Appendix B.3.  Second, the
    314        Thrift model component is a prelude to understanding the RIFT's
    315        inherent security features as defined in the security segment
    316        (Section 7).  The normative schema defining the Thrift model can be
    317        found in both Appendix B.2 and Appendix B.3.  Furthermore, while a
    318        detailed understanding of Thrift and the models are not required
    319        unless implementing RIFT, they may provide additional useful
    320        information for other readers.

    [minor] "RIFT's inherent security features as defined in the security
    segment (Section 7)"    §7 makes sense, but the model, mechanisms,
    etc. are specified in §4.4.  Should there be a pointed to that too?

jhead>> Great catch, thank you for that. Text includes pointers to the "Security Model" (4.4) and now reads as:
jhead>> "...RIFT's inherent security features as defined in both security models part (Section 4.4) and the security segment (Section 7)."
jhead>> Quick note, this paragraph is describing general understanding/fundamentals for the average reader, I intentionally left out the "Security Association Challenges" (4.5) as it's a rather complex implementation-specific challenge.
jhead>> Additionally, I fixed Thrift reference that was broken.

    [nit] s/understanding...are not required/understanding...is not required

jhead>> Fixed.

    322        If implementing RIFT to support multi-plane topologies Section 4.2
    323        should be reviewed in its entirety in conjunction with previously
    324        mentioned Thrift schemas.  Sections not relevant to single-plane
    325        implementations will be noted later in the section.  Special
    326        attention should be paid to the LIE definitions part (Section 4.2.2)
    327        as it not only outlines basic neighbor discovery and adjacency
    328        formation, but also provides necessary context for RIFT's ZTP
    329        (Section 4.2.7) and mis-cabling detection capabilities that allow it
    330        to automatically detect and build the underlay topology with a
    331        negligible configuration.  These specific capabilities are detailed
    332        in Section 4.2.7.

    [nit] s/with previously mentioned/with the previously mentioned

jhead>> Fixed.

    [nit] s/with a negligible configuration/with negligible configuration

jhead>> Fixed.

    334        For other readers, the following sections provide a more detailed
    335        understanding of the fundamental properties and highlight some
    336        additional benefits of RIFT such as link state packet formats, highly
    337        efficient flooding, synchronization, loop-free path computation and
    338        link-state database maintenance - Section 4.2.3, Section 4.2.3.2,
    339        Section 4.2.3.3, Section 4.2.3.4, Section 4.2.3.6, Section 4.2.3.7,
    340        Section 4.2.3.8, Section 4.2.4, Section 4.2.4.1, Section 4.2.4.2,
    341        Section 4.2.4.3, Section 4.2.4.4.  RIFT's unique ability to perform
    342        weighted unequal-cost load balancing of traffic across all available
    343        links is outlined in Section 4.3.7 with an accompanying example.

    [] Let's stay away from "marketing"-like adjectives: highly, unique, etc..

jhead>> Fixed.

    ...
    356        Section 5 contains a set of comprehensive examples that continue to
    357        highlight just how efficiently RIFT handles failures by containing
    358        impact to only the required set of nodes.  It should also help cement
    359        some of RIFT's core concepts in the reader's mind.

    [] s/continue to highlight just how efficiently RIFT handles failures
    by containing impact/show how RIFT contains the impact of failures

jhead>> Fixed.

    ...
    367        More information related to RIFT can be found in the "RIFT
    368        Applicability" [APPLICABILITY] document, which discusses alternate
    369        topologies upon which RIFT may be deployed, use cases where it is
    370        applicable, and presents operational considerations that complement
    371        this document.

    [major] As we discussed, putting complementary information in that
    other document is fine, but it should be a Normative reference.

jhead>> Applicability document is now a Normative reference.

    ...
    375     3.1.  Terminology
    ...
    401        Level:
    402           Clos and Fat Tree networks are topologically partially ordered
    403           graphs and 'level' denotes the set of nodes at the same height in
    404           such a network, where the bottom level (leaf) is the level with
    405           lowest value.  A node has links to nodes one level down and/or one
    406           level up.  Under some circumstances, a node may have links to
    407           nodes at the same level and a leaf may have links to nodes
    408           multiple levels higher.  RIFT counts levels from top-of-fabric
    409           (ToF) numerically down.  Level 0 always implies a leaf in RIFT but
    410           a leaf does not have to be level 0.  Level in RIFT can be
    411           configured or automatically derive its level via ZTP as explained
    412           in Section 4.2.7.  As final footnote: Clos terminology uses often
    413           the concept of "stage" but due to the folded nature of the Fat
    414           Tree it is not used from this point on to prevent
    415           misunderstandings.

    [nit] s/Level in RIFT can be configured or automatically derive its
    level via ZTP/Level in RIFT can be configured or automatically derived
    via ZTP

jhead>> Fixed.

    [nit] s/As final footnote/As a final footnote

 jhead>> Fixed.

    ...
    456        Top-of-fabric Plane or Partition:
    457           In large fabrics top-of-fabric switches may not have enough ports
    458           to aggregate all switches south of them and with that, the ToF is
    459           'split' into multiple independent planes.  Section 4.1.2 explains
    460           the concept in more detail.  A plane is subset of ToF nodes that
    461           see each other through south reflection or E-W links.

    [nit] s/is subset/is a subset

jhead>> Fixed.

    ...
    635        System ID:
    636           Each RIFT node identifies itself by a valid, network wide unique
    637           number when trying to build adjacencies or describing its
    638           topology.  RIFT System IDs can be auto-derived or configured.

    [major] What is a "valid" System ID?  I found the answer in §4.2.2,
    but given that we don't want to make this section an index, perhaps
    simplify the definition here and leave the validity for later:

    Suggestion>
       Each RIFT node identifies itself by a network-wide unique number...

jhead>> Agreed, validity isn't relevant here. However, I think phrasing this reads a bit better in the context of the definition:
jhead>> "RIFT nodes identify themselves with a unique network-wide number when trying to build adjacencies or describe their topology."

    ...
    719     4.  RIFT: Routing in Fat Trees

    721        Remainder of this documents presents the detailed specification of a
    722        protocol optimized for Routing in Fat Trees (RIFT) that in most
    723        abstract terms has many properties of a modified link-state protocol
    724        when distributing information northbound and a distance vector
    725        protocol when distributing information southbound.  While this is an
    726        unusual combination, it does quite naturally exhibit the desirable
    727        properties desired.

    [nit] s/Remainder of this documents/The remainder of this document

jhead>> Fixed.

    ...
    1361    4.1.5.  Addressing the Fallen Leaves Problem
    ...
    1369       When used for the operation of disaggregation, a positive South TIE
    1370       contained in `positive_disaggregation_prefixes`, as usual, indicates
    1371       reachability to a prefix of given length and all addresses subsumed
    1372       by it.  In contrast, a negative route advertisement contained in
    1373       `negative_disaggregation_prefixes` indicates that the origin cannot
    1374       route to the advertised prefix.

    [major] This paragraph was updated to include the "contained in
    `positive_disaggregation_prefixes`" and "contained in
    `negative_disaggregation_prefixes`" phrases.  This is the first time
    these elements (?) are mentioned, so I went looking for a
    description...and found this in the schema:

    /** TIE carrying prefixes */
    struct PrefixTIEElement {
        /** Prefixes with the associated attributes. */
        1: required map<common.IPPrefixType, PrefixAttributes> prefixes;
    } (python.immutable = "")

    ...

    /** Single element in a TIE. */
    union TIEElement {
        /** Used in case of enum common.TIETypeType.NodeTIEType. */
        1: optional NodeTIEElement     node;
        /** Used in case of enum common.TIETypeType.PrefixTIEType. */
        2: optional PrefixTIEElement          prefixes;
        /** Positive prefixes (always southbound). */
        3: optional PrefixTIEElement   positive_disaggregation_prefixes;
        /** Transitive, negative prefixes (always southbound) */
        5: optional PrefixTIEElement   negative_disaggregation_prefixes;
        /** Externally reimported prefixes. */
        6: optional PrefixTIEElement          external_prefixes;
        /** Positive external disaggregated prefixes (always southbound). */
        7: optional PrefixTIEElement
                positive_external_disaggregation_prefixes;
        /** Key-Value store elements. */
        9: optional KeyValueTIEElement keyvalues;
    } (python.immutable = "")

    Both elements point at PrefixTIEElement, as does "prefixes".  I also
    looked at PrefixAttributes (from PrefixTIEElement), but couldn't
    figure out where the distinction between positive and negative is
    made.

    I also found Figure 18 (Adding Routes from South TIE Positive and
    Negative Prefixes), which I realize is an example -- but I also
    couldn't figure out how positive and negative prefixes are
    advertised/treated differently.

    How/where is the distinction between them (and with "prefixes") made?
    This question is probably answered somewhere in the document, and
    there might be some confusion in my reading of the schema.

jhead>> The distinction is pointed out in the "Southbound and Northbound TIE Representation" section:
jhead>> First, "Table 2: TIE Types" points out that positive and negative TIEs are their own unique type of TIE.
jhead>>     "...Different TIE types are carried in `TIEElement`. Schema enum `common.TIETypeType` in `TIEID` indicates which elements MUST be present in the `TIEElement`..."

jhead>> Additionally, it's pointed out again in the context of how TIEs are flooded in the following "Flooding" section:
jhead>>     "...All valid TIE types are defined in `TIETypeType`. This enum indicates what TIE type the TIE is carrying..."

    After all this, what is the value of adding the references to
    `positive_disaggregation_prefixes` and
    `negative_disaggregation_prefixes` at this point in the text?

jhead>> I re-read this section and the contained reference flow several times and I don't think the terms "positive_disaggregation_prefixes" and "negative_disaggregation_prefixes" add much value since this section is simply describing a general approach to how the fallein leaf problem is addressed, rather than the normative procedures.
jhead>> I have removed these particular terms in favor of "common speak" for in this instance.

    ...
    6676    11.1.  Normative References
    ...
    6759       [VFR]      Erlebach et al., T., "Cuts and Disjoint Paths in the
    6760                  Valley-Free Path Model of Internet BGP Routing", Springer
    6761                  Berlin Heidelberg Combinatorial and Algorithmic Aspects of
    6762                  Networking, 2005.

    [major] This reference should be Informative.

jhead>> [VFR] is now an Informative Reference.

    6764    11.2.  Informative References

    6766       [APPLICABILITY]
    6767                  Wei, Y., Zhang, Z., Afanasiev, D., Thubert, P., and T.
    6768                  Przygienda, "RIFT Applicability", Work in Progress,
    6769                  Internet-Draft, draft-ietf-rift-applicability-10, 16
    6770                  December 2021, <https://urldefense.com/v3/__https://datatracker.ietf.org/doc/html/__;!!NEt6yMaO-gk!F8z-OUNjxZdzzL3TRm6dzxBn8veU3zsnYW2XjkLyjJPIKbxGWAjwLgD9PnG5rk9F1bwnfeguxZ8FuJItYoso$
    6771                  draft-ietf-rift-applicability-10>.

    [major] This reference should be Normative.

jhead>> Already fixed per previous comment.

    [EoR P1-15]


Juniper Business Use Only