Re: [Pce] AD review of draft-ietf-pce-segment-routing-ipv6-20

[Dhruv Dhody <[e-mail]>]

Hi Cheng,

In Section 9.2 (New SRv6-ERO NAI Type Registry) where you define a new
registry, please add -

The allocation policy for this new registry is by IETF Review [RFC8126].


Thanks!
Dhruv

On Wed, Jan 31, 2024 at 10:33 PM Cheng Li <c.l=[e-mail]>
wrote:

> Hi John,
>
> Thank you so much for your comments, we think they are very helpful.
> We have modified the draft to address your comments accordingly, please
> review it again.
> If they can address your comments, we will update the draft very soon.
>
> Thanks,
> Cheng
>
>
>
> -----Original Message-----
> From: John Scudder <[e-mail]>
> Sent: Thursday, January 25, 2024 12:49 AM
> To: [e-mail]
> Cc: [e-mail]
> Subject: AD review of draft-ietf-pce-segment-routing-ipv6-20
>
> Hi Authors, WG,
>
> Thanks for this document.
>
> 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.
>
> One further point regarding “minor editorial suggestions”. While I did
> make a few, I didn’t do exhaustive copy-editing of this draft, and of
> course, the RFC Editor will do their usual comprehensive job. However, I
> did notice a pervasive need for a few particular kinds of changes (for
> example definite articles, agreement in number) that mar what’s otherwise a
> high-quality document and are likely to bother some reviewers. It’s
> optional, but it wouldn’t be a bad idea for you to pass the document
> through a grammar checker before we send it for IETF Review. Your call.
>
> Thanks,
>
> —John
>
> --- draft-ietf-pce-segment-routing-ipv6-20.txt  2024-01-24
> 14:39:13.000000000 -0500
> +++ draft-ietf-pce-segment-routing-ipv6-20-jgs-comments.txt     2024-01-24
> 18:40:57.000000000 -0500
> @@ -29,12 +29,15 @@
>     A Segment Routed Path can be derived from a variety of mechanisms,
>     including an IGP Shortest Path Tree (SPT), explicit configuration, or
>     a PCE.
> +--
> +jgs: I think it would be appropriate to expand PCE.
> +--
>
>     Since SR can be applied to both MPLS and IPv6 forwarding planes, a
> -   PCE should be able to compute SR-Path for both MPLS and IPv6
> +   PCE should be able to compute an SR path for both MPLS and IPv6
>     forwarding planes.  The PCEP extension and mechanisms to support SR-
>     MPLS have been defined.  This document describes the extensions
> -   required for SR support for IPv6 data plane in the Path Computation
> +   required for SR support for the IPv6 data plane in the Path
> + Computation
>     Element communication Protocol (PCEP).
>
>  Status of This Memo
> @@ -151,7 +154,7 @@
>     [RFC8754].
>
>     An SR path can be derived from an IGP Shortest Path Tree (SPT), but
> -   SR-TE (Segment Routing Traffic Engineering) paths may not follow IGP
> +   SR-TE (Segment Routing Traffic Engineering) paths might not follow
> + IGP
>     SPT.  Such paths may be chosen by a suitable network planning tool,
>     or a PCE and provisioned on the ingress node.
>
> @@ -186,7 +189,7 @@
>     stateful PCE for computing one or more SR-TE paths taking into
>     account various constraints and objective functions.  Once a path is
>     chosen, the stateful PCE can initiate an SR-TE path on a PCC using
> -   PCEP extensions specified in [RFC8281] using the SR specific PCEP
> +   PCEP extensions specified in [RFC8281] and the SR-specific PCEP
>     extensions specified in [RFC8664].  [RFC8664] specifies PCEP
>     extensions for supporting a SR-TE LSP for the MPLS data plane.  This
>     document extends [RFC8664] to support SR for the IPv6 data plane.
> @@ -228,6 +231,16 @@
>
>     The message formats in this document are specified using Routing
>     Backus-Naur Format (RBNF) encoding as specified in [RFC5511].
> +--
> +jgs: as far as I can tell, no they are not. Either remove this
> +paragraph, and the normative reference, or help me understand why I am
> +wrong.
> +
> +Speaking of RBNF, the fact this spec doesn't use it seems to put it out
> +of step with some of the other PCE document set. I'm assuming it's the
> +consensus of the WG that this is fine. (I don't have a problem with the
> +style used here.)
> +--
>
>     Further, following terms are used in the document,
>
> @@ -240,6 +253,11 @@
>     SID:  Segment Identifier.
>
>     SRv6:  Segment Routing for IPv6 forwarding plane.
> +--
> +jgs: in the introduction you expanded this as "Segment Routing over
> +IPv6 data plane", which seems like a better expansion to me. But either
> +way, please pick one and stick with it.
> +--
>
>     SRH:  IPv6 Segment Routing Header.
>
> @@ -255,6 +273,14 @@
>     Basic operations for PCEP speakers are as per [RFC8664].  SRv6 Paths
>     computed by a PCE can be represented as an ordered list of SRv6
>     segments of 128-bit value.
> +--
> +jgs: probably just "128 bit SRv6 segments", or if you feel that's not
> +explicit enough, "SRv6 segments, each of which is 128 bits in length."
> +(The problem with the current version is that first, 128 bits is the
> +length of the segment, not the value of it, and second, there's
> +possible ambiguity as to whether 128 bits is the length of the list, or
> +the length of each individual segment.)
> +--
>
>     [RFC8664] defined a new Explicit Route Object (ERO) subobject denoted
>     by "SR-ERO subobject" capable of carrying a SID as well as the @@
> -271,6 +297,16 @@
>
>     This document defines new subobjects "SRv6-ERO" and "SRv6-RRO" in the
>     ERO and the RRO respectively to carry the SRv6 SID (IPv6 Address).
> +--
> +jgs: it's not at all clear to me that it's formally correct to say an
> +SRv6 SID is the same as an IPv6 address; for example, the abstract of
> +draft-ietf-6man-sids-05 tells me that "Segment Identifiers (SIDs) used
> +by SRv6 *can resemble* IPv6 addresses and behave like them while
> +exhibiting *slightly different* behaviors in some situations" (my
> +emphasis).
> +
> +Probably the best fix is just to remove the text inside the parentheses.
> +--
>     SRv6-capable PCEP speakers MUST be able to generate and/or process
>     these subobjects.
>
> @@ -304,6 +340,15 @@
>     necessary information to guide the packets from the ingress node to
>     the egress node of the path, and hence there is no need for any
>     signaling protocol.
> +--
> +jgs: that last clause can't possibly be right as written. SR networks
> +aren't completely devoid of any and all signaling... consider that PCEP
> +itself is a signaling protocol of sorts. While it would be possible to
> +expand and rewrite the clause more precisely to be formally correct, I
> +think the easiest fix is just to delete it, since it's not directly
> +relevant to the subject at hand. (I see this language may have been
> +inherited from RFC 8664; I think it's wrong there too.)
> +--
>
>     For the use of an IPv6 control plane but an MPLS data plane,
>     mechanism remains the same as specified in [RFC8664].
> @@ -412,6 +457,10 @@
>           SRv6-SID.
>
>        -  Unassigned bits MUST be set to 0 and ignored on receipt.
> +--
> +jgs: I think this should be "set to 0 on transmission and ignored on
> +receipt", right?
> +--
>
>        A pair of (MSD-Type, MSD-Value): Where MSD-Type (1 octet) is as
>        per the IGP MSD Type registry created by [RFC8491] and populated @@
> -426,12 +475,29 @@
>     to ensure those midpoints are able to correctly process their
>     segments and for the tailend to dispose of the SRv6 encapsulation.
>     The SRv6 MSD capabilities of these other nodes MAY be learned as part
> +--
> +jgs: The above RFC 2119 style "MAY" should probably be "may", or better
> +still "might", I don't think you're stating a requirement here (or in
> +this case, since "MAY", a non-requirement). I assume you'd take the
> +position that it's beyond the scope of this specification to say how
> +the capabilities of the other nodes have to be learned.
> +--
>     of the topology information via IGP/BGP-LS or via PCEP if the PCE
>     also happens to have PCEP sessions to those nodes.
> +--
> +jgs: you need a reference for BGP-LS.
> +--
>
>     It is RECOMMENDED that the SRv6 MSD information be not included in
>     the SRv6-PCE-Capability sub-TLV in deployments where the PCE is able
>     to obtain this via IGP/BGP-LS as part of the topology information.
> +--
> +jgs: By using the RFC 2119 style "RECOMMENDED" you're making this an
> +almost-MUST. Do you intend that? If so, please consider providing
> +guidance as to when the requirement can be disregarded.
> +
> +But I think probably you mean "recommended".
> +--
>
>  4.2.  The RP/SRP Object
>
> @@ -478,6 +544,12 @@
>        +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
>
>                      Figure 2: SRv6-ERO Subobject Format
> +--
> +jgs: the fields marked "optional" aren't really completely optional,
> +are they? I'm not aware of a totally standard pattern for this kind of
> +field, but I have seen the term "conditional" used in such a context,
> +or even "conditional, see below".
> +--
>
>     The fields in the SRv6-ERO subobject are as follows:
>
> @@ -511,6 +583,13 @@
>     below) then the NT field has no meaning and MUST be ignored by the
>     receiver.  This document reuses NT types defined in [RFC8664], but
>     assigns them new meanings appropriate to SRv6.
> +--
> +jgs: I feel very uncomfortable about being told that you're using an
> +existing registry, but assigning different semantics to
> +already-allocated code points. I think the most straightforward fix to
> +this would be to create a new "PCEP SRv6-ERO NAI Types" registry,
> +populated with values 0, 2, 4 and 6, defined however you like.
> +--
>
>        If NT value is 0, the NAI MUST NOT be included.
>
> @@ -531,6 +610,11 @@
>        identify the IPv6 Adjacency and used with the SRv6 Adj-SID.
>
>        SR-MPLS specific NT types are not valid in SRv6-ERO.
> +--
> +jgs: if you take my suggestion of creating a new registry, the above
> +line becomes unnecessary. There would be no SR-MPLS specific NT types
> +in your new registry.
> +--
>
>     Flags: Used to carry additional information pertaining to the
>     SRv6-SID.  This document defines the following flag bits.  The other
> @@ -575,9 +659,23 @@
>     is recommended to signal it always if possible.  It could be used for
>     maintainability and diagnostic purpose.  If behavior is not known,
>     the opaque value '0xFFFF' is used [RFC8986].
> +--
> +jgs: please be explicit that the behavior values are taken from the
> +registry "SRv6 Endpoint Behaviors".
> +
> +It sure seems strange to me to choose to use a reserved value called
> +"opaque" to signal "behavior not known" instead of having a registered
> +value called "behavior not known". But, it's probably too late to
> +change this now.
> +--
>
>     SRv6 SID: SRv6 Identifier is an 128-bit IPv6 address representing the
>     SRv6 segment.
> +--
> +jgs: but is it in fact an address? See my comment with reference to
> +draft-ietf-6man-sids-05 earlier on. An easy fix would be to replace
> +"IPv6 address" with "value".
> +--
>
>     NAI: The NAI associated with the SRv6-SID.  The NAI's format depends
>     on the value in the NT field, and is described in [RFC8664].
> @@ -650,7 +748,7 @@
>     validation of SRv6 SIDs being instantiated in the network and checked
>     for conformance to the SRv6 SID allocation scheme chosen by the
>     operator as described in Section 3.2 of [RFC8986].  In the future,
> -   PCE can also be utilized to verify and automate the security of the
> +   PCE might also be utilized to verify and automate the security of
> + the
>     SRv6 domain by provisioning filtering rules at the domain boundaries
>     as described in Section 5 of [RFC8754].  The details of these
>     potential applications are outside the scope of this document.
> @@ -811,7 +909,7 @@
>  5.2.1.  SRv6 ERO Validation
>
>     If a PCC does not support the SRv6 PCE Capability and thus cannot
> -   recognize the SRv6-ERO or SRv6-RRO subobjects.  It should respond
> +   recognize the SRv6-ERO or SRv6-RRO subobjects, it should respond
>     according to the rules for a malformed object as described in
>     [RFC5440].
>
> @@ -832,6 +930,11 @@
>        MUST be 48, otherwise the Length MUST be 64.
>
>     *  NT types (1,3, and 5) are not valid for SRv6.
> +--
> +jgs: If you take my suggestion to set up a separate registry, you won't
> +need the above bullet, because NT types 1, 3, and 5 won't exist in that
> +registry.
> +--
>
>     *  If T bit is 1, then S bit MUST be zero.
>
> @@ -963,10 +1066,15 @@
>  7.2.  Information and Data Models
>
>     The PCEP YANG module is out of the scope of this document and defined
> -   in other drafts.  An augmented YANG module for SRv6 is also specified
> -   in another draft that allows for SRv6 capability and MSD
> +   in other documents.  An augmented YANG module for SRv6 is also
> specified
> +   in another document that allows for SRv6 capability and MSD
>     configurations as well as to monitor the SRv6 paths set in the
>     network.
> +--
> +jgs: please provide informative references to the "other documents" and
> +"another document" (which I've edited from "other drafts" and "another
> +draft").
> +--
>
>  7.3.  Liveness Detection and Monitoring
>
>
>
> _______________________________________________
> Pce mailing list
> [e-mail]
> https://www.ietf.org/mailman/listinfo/pce
>