Re: [Lsr] AD review of draft-ietf-lsr-ospfv3-srv6-extensions-09

[Lizhenbin <[e-mail]>]

Hi John,
Thanks very much for your review. We will check the comments and reply soon.


Best Regards,
Zhenbin (Robin)



________________________________

李振斌 Li Zhenbin
Mobile: [phone]
Email: [e-mail]

发件人：John Scudder <[e-mail]>
收件人：draft-ietf-lsr-ospfv3-srv6-extensions <[e-mail]>;lsr <[e-mail]>
抄 送：Huzhibo <[e-mail]>;Lizhenbin <[e-mail]>;Ketan Talaulikar <[e-mail]>;Peter Psenak <[e-mail]>
时 间：2023-04-21 20:28:29
主 题：AD review of draft-ietf-lsr-ospfv3-srv6-extensions-09

Hi Authors, WG,

Thanks for this spec.

I’ve supplied my questions and comments in the form of an edited copy of the draft. Minor editorial suggestions I’ve made in place without further comment, more substantive questions and comments are done in-line and prefixed with “jgs:”. You can use your favorite diff tool to review them; I’ve attached the iddiff output for your convenience if you’d like to use it. I’ve also pasted a traditional diff below in case you want to use it for in-line reply.

Thanks,

—John

--- draft-ietf-lsr-ospfv3-srv6-extensions-09.txt        2023-04-21 11:29:59.000000000 -0400
+++ draft-ietf-lsr-ospfv3-srv6-extensions-09-jgs-comments.txt   2023-04-21 14:21:35.000000000 -0400
@@ -116,9 +116,9 @@

 1.  Introduction

-   Segment Routing (SR) architecture [RFC8402] specifies how a node can
-   steer a packet through an ordered list of instructions, called
-   segments.  These segments are identified through Segment Identifiers
+   The Segment Routing (SR) architecture [RFC8402] specifies how a node can
+   steer a packet using an ordered list of instructions, called
+   segments.  These segments are identified using Segment Identifiers
    (SIDs).

    Segment Routing can be instantiated on the IPv6 data plane through
@@ -144,7 +144,7 @@
    1.  An SRv6 Capabilities TLV to advertise the SRv6 features and SRH
        operations supported by an OSPFv3 router

-   2.  Several sub-TLVs are defined to advertise various SRv6 Maximum
+   2.  Several sub-TLVs to advertise various SRv6 Maximum
        SID Depths.

    3.  An SRv6 Locator TLV using an SRv6 Locator Link-State
@@ -179,6 +179,9 @@
    be advertised by an SRv6-enabled router.

    This TLV SHOULD be advertised only once in the OSPFv3 Router
+---
+jgs: What's the reason the above is a SHOULD and not a MUST?
+---
    Information LSA.  When multiple SRv6 Capabilities TLVs are received
    from a given router, the receiver MUST use the first occurrence of
    the TLV in the OSPFv3 Router Information LSA.  If the SRv6
@@ -194,6 +197,10 @@
    defined flooding scopes (link, area, or Autonomous System (AS)).  For
    the purpose of SRv6 Capabilities TLV advertisement, area-scoped
    flooding is REQUIRED.
+---
+jgs: I infer that link and AS scoped flooding is OPTIONAL. While this
+is implicit, might it be worth adding something to that effect?
+---

    The format of OSPFv3 SRv6 Capabilities TLV is shown below:

@@ -208,6 +215,14 @@
     +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+

                       Figure 1: SRv6 Capabilities TLV
+---
+jgs: In this and all subsequent figures, might it be worth filling in
+the type value in the figure, as in "Type = 20" in this case? FWIW I
+was first prompted to think about this by your Figure 3, where you
+embed the literal "1" for the U-bit instead of just relying on body
+text to tell the reader how to set it. ISTM if it's worth embedding one
+constant in the figure, it's probably worth doing so whenever feasible.
+---

    Where:

@@ -245,6 +260,12 @@

    The SRv6 Capabilities TLV may contain optional Sub-TLVs.  No Sub-TLVs
    are defined in this specification.
+---
+jgs: Thank you for setting up registry creation for this flags field.
+However, I noticed that the registry says the O-flag has value 0x0001,
+whereas in the diagram you show it as having value 0x4000. Please
+correct one or the other, to be consistent.
+---

 3.  Advertisement of Supported Algorithms

@@ -254,7 +275,7 @@
 4.  Advertisement of Maximum SRv6 SID Depths

    An SRv6-enabled router may have different capabilities and limits
-   related to SRH processing and this needs to be advertised to other
+   related to SRH processing and these need to be advertised to other
    OSPFv3 routers in the SRv6 domain.

    [RFC8476] defines the means to advertise node and link specific
@@ -269,6 +290,10 @@
    OSPFv3.  These MSD Types are allocated under the IGP MSD Types
    registry maintained by IANA that are shared by IS-IS and OSPF.  They
    are described below:
+---
+jgs: Please update the reference for srv6-extensions throughout, to
+refer to [RFC9352].
+---



@@ -310,8 +335,8 @@
 4.4.  Maximum End D MSD Type

    The Maximum End D MSD Type specifies the maximum number of SIDs
-   present in an SRH when performing decapsulation.  These include, but
-   not limited to, End.DX6, End.DT4, End.DT46, End with USD, End.X with
+   present in an SRH when performing decapsulation.  These include, but are
+   not limited to, End.DX6, End.DT4, End.DT46, End with USD, and End.X with
    USD as defined in [RFC8986].  If the advertised value is zero or no
    value is advertised, then the router cannot apply any behavior that
    results in decapsulation and forwarding of the inner packet when the
@@ -354,20 +379,58 @@
    in the SRv6 Locator TLV, the calculation of its reachability, and the
    installation in the forwarding plane follows the OSPFv3 [RFC5340]
    specifications for the respective route types.
+---
+jgs: in the above paragraph you talk about "route types" and define them
+in terms of RFC 5340's "specifications for the respective route types".
+RFC 5340 isn't written in terms of route types, it talks about LSAs and
+LSA types. These are used to compute routes, but are not themselves
+referred to as routes. Please align your terminology with RFC 5340,
+especially since you lean heavily on RFC 5340 to define your behavior.
+---

-   Locators associated with algorithms 0 and 1 SHOULD be advertised
+   Locators associated with algorithms 0 and 1 SHOULD also be advertised
    using the respective OSPFv3 Extended LSA types with extended TLVs
    [RFC8362] so that routers that do not support SRv6 will install a
    forwarding entry for SRv6 traffic matching those locators.  When
    operating in Extended LSA sparse-mode [RFC8362], these locators
    SHOULD be also advertised using the respective legacy OSPFv3 LSAs
    [RFC5340].
+---
+jgs: I interpolated an "also" into the first sentence, to make it
+clearer that such locators should be advertised in *both* the SRv6 Locator
+LSA *and* the traditional LSA. Please check that this is correct.
+---
+---
+jgs: While I see the attraction of using the "respective... LSA types"
+style of writing here, because it relieves you of writing (and the
+reader of reading) quite a few words, I'm a little concerned that it
+leaves something to the imagination; the reader has to glean what you
+intended. I encourage you to think about whether you can improve on
+this without overly burdening the document, for example I wonder if
+you could have a paragraph earlier on that says something along the
+lines of,
+
+       In this specification we introduce a new LSA type, the SRv6 Locator
+       LSA. This has analogous use to other LSA types -- OSPFv3 Extended
+       LSAs, Intra-Area-Prefix-LSAs, and E-Intra-Area-Prefix-LSAs. In the
+       text that follows, we sometimes refer to "respective LSA types";
+       this means [and then say just what it is that you mean].
+
+Or maybe there's an even simpler solution. Maybe I'm trying to read too
+much into "the respective". If we simply removed those words, would the
+meaning still be as you intend it? As in, "Locators associated with
+algorithms 0 and 1 SHOULD also be advertised using OSPFv3 Extended
+LSA types with extended TLVs"?
+---

    When SRv6 Locators are also advertised as Intra-Area-Prefix-LSAs and/
    or E-Intra-Area-Prefix-LSAs, the SRv6 Locator MUST be considered as a
    prefix associated with the router and the referenced LSA type MUST
    point to the Router LSA of the advertising router as specified in
    Section 4.4.3.9 of [RFC5340].
+---
+jgs: what would be expected to happen/be done if the MUST is violated?
+---

    In cases where a locator advertisement is received both in a prefix
    reachability advertisement (i.e., via legacy OSPFv3 LSAs and/or
@@ -382,7 +445,7 @@

    SRv6 SIDs are advertised as Sub-TLVs in the SRv6 Locator TLV except
    for SRv6 End.X SIDs/LAN End.X SIDs which are associated with a
-   specific Neighbor/Link and are therefore advertised as Sub-TLVs of E-
+   specific Neighbor/Link and are therefore advertised as Sub-TLVs of the E-
    Router-Link TLV.


@@ -399,7 +462,7 @@
    Reachability to SRv6 SIDs depends upon the existence of a covering
    locator.

-   Adherence to the rules defined in this section will assure that SRv6
+   Adherence to the rules defined in this section will ensure that SRv6
    SIDs associated with a supported algorithm will be forwarded
    correctly, while SRv6 SIDs associated with an unsupported algorithm
    will be dropped.  NOTE: The drop behavior depends on the absence of a
@@ -408,6 +471,30 @@
    For forwarding to work correctly, the locator associated with SRv6
    SID advertisements must be the longest prefix match installed in the
    forwarding plane for those SIDs.  To ensure correct forwarding,
+---
+jgs: I don't think the above is strictly correct -- you've stated a
+sufficient condition for forwarding to work correctly, but not a
+necessary one. For example, suppose the locator in question is some /64.
+If there were also /128s in the forwarding plane, for the individual
+SIDs, pointing to the same router, surely forwarding would still work?
+One can also posit innumerable more baroque cases where forwarding
+still works, either by design or by accident.
+
+If you agree with this, a simple rewrite to assuage my mania for
+precision might go like,
+
+   If the locator associated with SRv6 SID advertisements is the longest
+   prefix match installed in the forwarding plane for those SIDs, this
+   will ensure correct forwarding.
+
+And then start the next sentence with "Network operators should..." as
+it stands.
+
+I also wonder if this advice to network operators should go in an
+Operational Considerations section or at least in its own subsection.
+It seems misplaced in the current section which otherwise seems to be
+speaking to implementors.
+---
    network operators should take steps to make sure that this
    requirement is not compromised.  For example, the following
    situations should be avoided:
@@ -428,11 +515,18 @@
    represents a Flexible Algorithm, the procedures described in section
    14.2 of [I-D.ietf-lsr-flex-algo] are followed for the programming of
    those specific SRv6 Locators.
+---
+jgs: Please update all references to lsr-flex-algo to point to [RFC9350].
+---
+---
+jgs: Instead of "the same applies" may I suggest "analogous procedures
+apply"?
+---

    Locators associated with Flexible Algorithms SHOULD NOT be advertised
    in the base OSPFv3 prefix reachability advertisements.  Advertising
    the Flexible Algorithm locator in a regular prefix reachability
-   advertisement would make them available for non-Flexible Algorithm
+   advertisement would make it available for non-Flexible Algorithm
    forwarding (i.e., algorithm 0).

    The procedures for OSPFv3 Flexible Algorithm for SR-MPLS, as
@@ -456,6 +550,11 @@
    such the same value can be advertised by multiple routers.  It is
    useful for other routers to know that the advertisement is for an
    anycast identifier.
+---
+jgs: "It is useful" is an interesting claim to drop in without further
+explanation or support. I expect it *is* useful or you wouldn't have said
+it, but can you cite a reference here?
+---

    A new bit in OSPFv3 PrefixOptions [RFC5340] is defined to advertise
    the anycast property:
@@ -473,6 +572,9 @@

    When the prefix/SRv6 Locator is configured as anycast, the AC-bit
    SHOULD be set.  Otherwise, this flag MUST be clear.
+---
+jgs: Why SHOULD and not MUST?
+---

    The AC-bit MUST be preserved when re-advertising the prefix/SRv6
    Locator across areas.
@@ -484,6 +586,11 @@
    The same prefix/SRv6 Locator can be advertised by multiple routers.
    If at least one of them sets the AC-bit in its advertisement, the
    prefix/SRv6 Locator SHOULD be considered as anycast.
+---
+jgs: Again, why SHOULD and not MUST, or alternately, why a 2119 keyword
+at all and not "is", for example? If kept as SHOULD, what would be the
+implications and consequences of disregarding the SHOULD?
+---

    A prefix/SRv6 Locator that is advertised by a single node and without
    an AC-bit is considered node-specific.
@@ -516,7 +623,19 @@
 7.  SRv6 Locator LSA

    The SRv6 Locator LSA has a function code of 42 while the S1/S2 bits
+---
+jgs: Congratulations on getting 42, the best number, assigned for your
+code point. :-)
+---
    are dependent on the desired flooding scope for the LSA.  The
+---
+jgs: The use of "while" above creates some ambiguity. Are you using it
+to simply stitch together two statements? If so consider rewriting as
+two shorter sentences, as in
+
+   The SRv6 Locator LSA has a function code of 42. The S1/S2 bits
+   are dependent on the desired flooding scope for the LSA.
+---
    flooding scope of the SRv6 Locator LSA depends on the scope of the
    advertised SRv6 Locator and is under the control of the advertising
    router.  The U-bit will be set indicating that the LSA should be
@@ -546,6 +665,13 @@
     |                             ...                               |

                          Figure 3: SRv6 Locator LSA
+---
+jgs: For consistency, if you don't elect to put the literal type values
+into your figures where relevant, perhaps consider removing the literal
+"1" above and replacing with "U"?
+
+Otherwise, indicate Function Code is 42?
+---

    The format of the TLVs within the body of the SRv6 Locator LSA is the
    same as the format used by [RFC3630].  The variable TLV section
@@ -568,13 +694,19 @@
     |              Type             |             Length            |
     +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
     |                            Value                              |
-                                   o
-                                   o
-                                   o
+                                   .
+                                   .
+                                   .
     |                                                               |
     +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+

                    Figure 4: SRv6 Locator LSA TLV Format
+---
+jgs: I suggested changing out the lower-case 'o' letters in your figure
+for periods because the column of ohs didn't scan as a vertical ellipsis
+for me when I read it, I had to stop and think about it for an instant.
+It's obviously not a big deal though.
+---

    The Length field defines the length of the value portion in octets
    (thus, a TLV with no value portion would have a length of 0).  The
@@ -594,6 +726,11 @@
    flooding scope, the LSA flooding scope MUST satisfy the application-
    specific requirements for all the locators included in a single SRv6
    Locator LSA.
+---
+jgs: Isn't the MUST above almost tautological? It doesn't really seem like
+something actionable for protocol conformance, would it be reasonable to
+replace it with "has to"?
+---

    When multiple SRv6 Locator TLVs are received from a given router in
    an SRv6 Locator LSA for the same Locator, the receiver MUST use the
@@ -658,14 +795,26 @@
               4 - AS External Type 2
               5 - NSSA External Type 1
               6 - NSSA External Type 2
+---
+jgs: I take it this list was created ab initio and isn't taken from
+some existing code point space. Should it have a registry or is it
+certain (or nearly certain) to never need extension?
+---

       Algorithm: 1-octet field.  The algorithm associated with the SRv6
       Locator.  Algorithm values are defined in the IGP Algorithm Type
       registry.
+---
+jgs: please add a reference to the registry
+---

       Locator Length: 1-octet field.  Specifies the length of the
       Locator prefix as the number of locator bits from the range
       (1-128).
+---
+jgs: is there any need for words about what would happen if the
+(implicitly forbidden) value 0 were present?
+---



@@ -694,6 +843,13 @@
    The following OSPFv3 Extended-LSA sub-TLVs corresponding to the
    Extended Prefix LSAs are also applicable for use as sub-TLVs of the
    SRv6 Locator TLV using code points as specified in Section 13.7:
+---
+jgs: Is there a need for another applicability column in the
+OSPFv3 Extended-LSA Sub-TLVs registry table, analogous to the L3BM
+column? The operative question is, when new sub-TLVs are introduced,
+should the authors consider the question of whether they're applicable
+for inclusion in the Extended Prefix LSA?
+---

    *  IPv6-Forwarding-Address sub-TLV [RFC8362]

@@ -766,12 +922,19 @@
       Length: 2-octet field.  The total length (in octets) of the value
       portion of the Sub-TLV including its further nested Sub-TLVs.

-      Reserved: 1-octet field.  It MUST be set to 0 on transmission and
-      MUST be ignored on receipt.
-
       Flags: 1-octet field.  It specifies the flags associated with the
       SID.  No flags are currently defined and this field MUST be set to
       0 on transmission and MUST be ignored on receipt.
+---
+jgs: I suggested reordering the "Flags" and "Reserved" descriptions to
+match the flow of the diagram. But more substantively, if you're going
+to define a flags field, I think you should also create an (unpopulated)
+registry for it. If you don't want to create the registry, just make
+the Reserved field 16 bits wide.
+---
+
+      Reserved: 1-octet field.  It MUST be set to 0 on transmission and
+      MUST be ignored on receipt.

       Endpoint Behavior: 2 octets field.  The endpoint behavior code
       point for this SRv6 SID as defined in [RFC8986].  Supported
@@ -795,8 +958,8 @@

    The SRv6 endpoint behaviors defined in [RFC8986] include certain
    behaviors that are specific to links or adjacencies.  The most basic
-   of these which is critical for link-state routing protocols like
-   OSPFv3 is the End.X behavior that is an instruction to forward to a
+   of these, which is critical for link-state routing protocols like
+   OSPFv3, is the End.X behavior that is an instruction to forward to a
    specific neighbor on a specific link.  These SRv6 SIDs and others
    that are defined in [RFC8986] which are specific to links or
    adjacencies need to be advertised to OSPFv3 routers within an area to
@@ -825,6 +988,9 @@
    one neighbor.  Thus multiple instances of the SRv6 End.X SID and SRv6
    LAN End.X SID Sub-TLVs MAY be advertised within the E-Router-Link TLV
    for a single link.
+---
+jgs: why is the SHOULD above not a MUST?
+---

    All End.X and LAN End.X SIDs MUST be subsumed by the subnet of a
    Locator with the matching algorithm which is advertised by the same
@@ -887,6 +1053,9 @@
             +-+-+-+-+-+-+-+-+
             |B|S|P| Reserved|
             +-+-+-+-+-+-+-+-+
+---
+jgs: this should have a registry
+---

       -  B-Flag: Backup Flag.  If set, the SID refers to a path that is
          eligible for protection.
@@ -916,6 +1085,9 @@
       Algorithm: 1-octet field.  The algorithm associated with the SRv6
       Locator from which the SID is allocated.  Algorithm values are
       defined in the IGP Algorithm Type registry.
+---
+jgs: please add a reference to the registry
+---

       Weight: 1-octet field.  Its value represents the weight of the
       End.X SID for load-balancing.  The use of the weight is defined in
@@ -994,6 +1166,9 @@
             +-+-+-+-+-+-+-+-+
             |B|S|P| Reserved|
             +-+-+-+-+-+-+-+-+
+---
+jgs: this should have a registry
+---

       -  B-Flag: Backup Flag.  If set, the SID refers to a path that is
          eligible for protection.
@@ -1081,6 +1256,10 @@
       Type: 2-octet field.  The value for this type is 30.

       Length: 2-octet field.  The value MUST be 4.
+---
+jgs: if you're filling in literal values in your figures, note you can
+fill in length as well as type
+---

       LB Length: 1-octet field.  SRv6 SID Locator Block length in bits.

@@ -1110,6 +1289,9 @@
       BGP-LS and then be monitored for conformance to the SRv6 SID
       allocation scheme chosen by the operator as described in
       Section 3.2 of [RFC8986].
+---
+jgs: add an informational reference for BGP-LS?
+---

    *  verification and the automation for securing the SRv6 domain by
       provisioning filtering rules at SR domain boundaries as described
@@ -1124,6 +1306,11 @@

    The details of these potential applications are outside the scope of
    this document.
+---
+jgs: that may be so, but having identified the possible use for securing
+the domain, you might do well to mention or refer to it in your Security
+Considerations section.
+---

 11.  Advertising Endpoint Behaviors

@@ -1163,9 +1350,14 @@
    domain.  In these deployments, stronger authentication mechanisms
    such as those specified in [RFC4552] or [RFC7166] SHOULD be used.

-   Implementations MUST assure that malformed TLV and Sub-TLV defined in
+   Implementations MUST ensure that malformed TLVs and Sub-TLVs defined in
    this document are detected and do not provide a vulnerability for
    attackers to crash the OSPFv3 router or routing process.  Reception
+---
+jgs: The preceding sentence smells a lot like "implementations MUST NOT
+contain bugs", see [RFC9225] for a more detailed treatment. I suggest
+removing it.
+---
    of malformed TLV or Sub-TLV SHOULD be counted and/or logged for
    further analysis.  Logging of malformed TLVs and Sub-TLVs SHOULD be
    rate-limited to prevent a Denial of Service (DoS) attack (distributed
@@ -1185,7 +1377,7 @@
    Routing behaviors to enable the creation of interoperable overlays;
    the security considerations from that document apply too.

-   The advertisement for an incorrect MSD value may have negative
+   The advertisement of an incorrect MSD value may have negative
    consequences, see [RFC8476] for additional considerations.

    Security concerns associated with the setting of the O-flag are
@@ -1246,6 +1438,10 @@

    *  Value: 0x0001.  Description: O-flag.  Reference: Section 2 of this
       document.
+---
+jgs: as noted earlier, Section 2 shows the O-flag with value 0x4000.
+Please reconcile.
+---

 13.5.  OSPFv3 Extended-LSA Sub-TLVs