[Pce] AD review of draft-ietf-pce-local-protection-enforcement-09

[John Scudder <[e-mail]>]

Hi Andrew and all,

Thanks for your replies to my previous review, and for the new version, it clears up most of my concerns. In reviewing the changes, I did come upon some other things, some of which were introduced in 09 and others which were things I’d overlooked on my first pass. I’m sorry not to have gotten these the first time through, but better late than never.

Once we’ve closed on these points [*] I think we’ll be ready for IETF Last Call.

Thanks,

—John

[*] Keeping in mind that “we’ve considered your input but decided to keep the text as written” is usually a way to resolve any given point. :-) 

--- draft-ietf-pce-local-protection-enforcement-09.txt	2023-05-15 20:21:43.000000000 -0400
+++ draft-ietf-pce-local-protection-enforcement-09-jgs-comments.txt	2023-05-16 14:19:03.000000000 -0400
@@ -130,6 +130,9 @@
    PCEP Extensions for Segment Routing ([RFC8664]) extends support in
    PCEP for Segment Routed paths.  The path list is encoded with Segment
    Identifiers, each of which offer local protection.  The PCE may
+---
+jgs: Should that be "each of which might offer"?
+---
    discover the protection eligibility for a Segment Identifier (SID)
    via BGP-LS [RFC9085] and take the protection into consideration as a
    path constraint.
@@ -156,6 +159,17 @@
 3.  Terminology
 
    This document uses the following terminology:
+---
+jgs: While I don't insist, I think the document will have a smoother
+approval path if the terms below were not rendered in ALL CAPS. If the
+WG feels that ALL CAPS is necessary or preferable, so be it, I'll take
+it forward that way -- I don't have a strong preference about this
+myself, but I know for certain that some reviewers will see it as 
+unnecessarily confusing vs. RFC 2119 keywords. 
+
+Another option would be to use initial caps as many specs do with terms
+they define, e.g. "Protection Mandatory", "Unprotected Mandatory", etc.
+---
 
    PROTECTION MANDATORY: The Path MUST have protection eligibility on
    all links.
@@ -173,10 +187,59 @@
    PROTECTION PREFERRED: The Path SHOULD have protection eligibility on
    all links but MAY contain links which do not have protection
    eligibility.
+---
+jgs: I'm sorry to not have raised this in my first round of review, but
+I just noticed it. It's clear to me what your intent is (or, what I
+think it is!), which is probably why I didn't pick up on this earlier,
+but I think if subjected to a close reading of the RFC 2119 definitions
+of SHOULD and MAY this doesn't work right.
+
+Let's recall how SHOULD and MAY are defined in RFC 2119:
+
+```
+3. SHOULD   This word, or the adjective "RECOMMENDED", mean that there
+   may exist valid reasons in particular circumstances to ignore a
+   particular item, but the full implications must be understood and
+   carefully weighed before choosing a different course.
+
+5. MAY   This word, or the adjective "OPTIONAL", mean that an item is
+   truly optional.  One vendor may choose to include the item because a
+   particular marketplace requires it or because the vendor feels that
+   it enhances the product while another vendor may omit the same item.
+   An implementation which does not include a particular option MUST be
+   prepared to interoperate with another implementation which does
+   include the option, though perhaps with reduced functionality. In the
+   same vein an implementation which does include a particular option
+   MUST be prepared to interoperate with another implementation which
+   does not include the option (except, of course, for the feature the
+   option provides.)
+```
+
+So if we did a "macro expansion" of your sentence, using those
+definitions, we'd find that the Path has to have protection eligibility
+on all links unless the implementor has understood and carefully weighed
+the implications of not doing so... but optionally (and this is truly
+optional) some vendors might choose to include links that don't have
+protection eligibility (and might choose this regardless of whether a
+protection-elegible link was also available).
+
+I bet this isn't what you mean to say. If it is, let's have a
+discussion. But I'm guessing what you really intend is that the Path
+will be built using protection-eligible links, unless it's not possible
+to form a Path in that way, in which case gaps may be closed using
+whatever links are available. If the latter is the case, let's find some
+wording that says that, without abusing RFC 2119 terms. A minimal fix
+would be to simply swap out the 2119 SHOULD/MAY for normal lower-case
+should/may, but I'd encourage you to do a little more if you're up for
+it. 
+---
 
    UNPROTECTED PREFERRED: The Path SHOULD NOT have protection
    eligibility on all links but MAY contain links which have protection
    eligibility.
+---
+jgs: same comment applies here.
+---
 
    PCC: Path Computation Client.  Any client application requesting a
    path computation to be performed by a Path Computation Element.
@@ -346,12 +409,29 @@
    consider the protection eligibility as an UNPROTECTED PREFERRED
    constraint but MAY consider protection eligibility as an UNPROTECTED
    MANDATORY constraint.
+---
+jgs: See my comment at https://mailarchive.ietf.org/arch/msg/pce/Ai6XH04JtVZCIPNTLLszytFC8FU/
+
+Repeated here for convenience, my suggestion in that comment --
+
+NEW:
+   When both L flag and E flag are set to 0, then the PCE SHOULD
+   consider the protection eligibility as an UNPROTECTED PREFERRED
+   constraint but MAY consider protection eligibility as an UNPROTECTED
+   MANDATORY constraint. An example of when the latter behavior might
+   be chosen is if the PCE has some means (outside the scope of this
+   document) to detect that it’s interacting with a legacy PCC that 
+   expects the legacy behavior.
+
+Or something to that effect -- an example and/or explanation of the
+circumstances under which the SHOULD might reasonably be disregarded.
+---
 
    When L flag is set to 0 and E flag is set to 1, then the PCE MUST
    consider the protection eligibility as an UNPROTECTED MANDATORY
    constraint.
 
-   If a PCE is unable to infer protection status of a resource, PCE MAY
+   If a PCE is unable to infer the protection status of a resource, the PCE MAY
    use local policy to define protected status assumptions.  When
    computing a Segment Routed path, It is RECOMMENDED that a PCE assume
    a Node SID is protected.  It is also RECOMMENDED that a PCE assume an
@@ -397,6 +477,11 @@
    behaviour specified in [RFC5440] and will ignore the E flag.  Thus,
    it may compute a path which may not respect the enforcement
    constraint.
+---
+jgs: this is only a small point, but I like to avoid "may" where possible
+for avoidance of confusion with MAY, so I might reword as "Thus, it could
+compute a path which might not respect..."
+---
 
    For PCC-initiated LSPs, the PCC SHOULD ignore the E flag value
    received from the PCE in a PCUpd message as it may be communicating
@@ -406,6 +491,13 @@
    from the PCE in a PCUpd message.  The PCE SHOULD ignore the E flag
    value received from the PCC in a PCRpt message as it may be
    communicating with a PCC which does not support this document.
+---
+jgs: While I have my RFC 2119 eyes peeled, I guess in the case of the
+two SHOULD above, you are deliberately leaving the door open for a 
+PCE implementation which either has a strict-mode knob, or which is
+a greenfield implementation and doesn't want to bother with 
+support of legacy behaviors. Or...?
+---
 
 6.  Implementation Status