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

[John Scudder <[e-mail]>]

Hi Authors, WG,

Thanks for this spec. I can see the need to clear up this ambiguity. 

I’ve supplied most of 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, most of my 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. 

In addition to my various inline comments, I have a general question/comment. The spec, as far as I can tell, is technology-agnostic: it introduces the E flag and codifies the interpretation of the E+L flags. It doesn’t restrict this to any particular data plane. That seems like the right choice. But, the draft also opens with several paragraphs of discussion of Segment Routing in the Introduction, and its normative language is SR-specific (search for “SID” in the document to see what I mean, SIDs are an SR construct).

Is the applicability of the document intended to be specific to Segment Routing? If so I think this needs to be made clearer. Or, if the document is NOT intended to be restricted to SR, then revisions are needed everywhere after the Introduction where the string ‘SID’ occurs, to make it technology-agnostic. (Less importantly, one might also ask why the Introduction needs to contain a tutorial on SR.)

Thanks,

—John

--- draft-ietf-pce-local-protection-enforcement-08.txt	2023-04-20 14:28:28.000000000 -0400
+++ draft-ietf-pce-local-protection-enforcement-08-jgs-comments.txt	2023-04-20 15:25:51.000000000 -0400
@@ -148,15 +148,21 @@
    calculated to include other segment identifiers which are not
    applicable to having their protection state advertised, as they may
    only be locally significant for each router processing the SID, such
-   as Node SIDs, it may not be possible for PCE to include the
+   as Node SIDs, it may not be possible for the PCE to include the
    protection constraint as part of the path calculation.
 
-   It is desirable for an operator to define the enforcement, or
+   It is desirable for an operator to be able to define the enforcement, or
    strictness of the protection requirement when it can be applied.
+---
+jgs: I don't follow what you mean by "... or strictness of the protection
+requirement when it can be applied." I'm getting hung up on the "when it 
+can be applied" part. Would it be correct to just delete those words? Or 
+if it wouldn't, can you help me understand what work they're doing? Thanks.
+---
 
    This document updates [RFC5440] by further describing the behaviour
-   with Local Protection Desired Flag (L flag) and extends on it with
-   the introduction of Enforcement Flag (E flag).
+   with the Local Protection Desired Flag (L flag) and extends on it with
+   the introduction of the Enforcement Flag (E flag).
 
 
 
@@ -286,6 +292,25 @@
    the PCE a preference on which SID to select, as the behaviour of the
    LSP would differ during a local failure depending on which SID is
    selected.
+---
+jgs: I thought this paragraph was going to answer a question I have,
+when it began with "... cases where there is simply no requirement to
+enforce protection or no protection ..." That is, sometimes the 
+operator may simply _not care_ at all, they just want (for example)
+the lowest-latency path. 
+
+But then my hopes were dashed, because at the end of the paragraph
+you segue (without explaining why) into "... give the PCE a preference".
+But isn't the case you introduced, the one where there literally is no
+preference? 
+
+As this is written, it seems as though there is missing support for the
+true "don't care" case. If this was discussed in the WG and the consensus
+was that, er, the WG doesn't care about the "don't care" case, I'd like
+to know that. If I'm confused and somehow "don't care" is properly 
+captured by one (both?!?) of these options, I'd like to know that, and
+understand how. If I'm terribly confused, I'd like to know that, too. :-)
+---
 
 5.  Protection Enforcement Flag (E flag)
 
@@ -359,6 +384,16 @@
    consider the protection eligibility as an UNPROTECTED PREFERRED
    constraint but MAY consider protection eligibility as an UNPROTECTED
    MANDATORY constraint.
+---
+jgs: It's not self-evident why this SHOULD/MAY is written this way. The
+obvious choice would have been to make the SHOULD a MUST and omit the
+MAY -- indeed, if 'enforcement' is set to false, it's hard to see a
+justification for making enforcement mandatory! So it seems as though
+there must be some backstory here. Can you help me understand? (Or
+better yet, make the SHOULD a MUST...)
+
+See also my comment just below.
+---
 
    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
@@ -368,10 +403,27 @@
    they indicate the preference of selection of a SID if PCE has an
    option of either protected or unprotected available on a link.  The
    definition of UNPROTECTED PREFERRED is primarily as a guidance on how
-   PCE should behave when L bit is not set, maintaining compatibility
+   PCE should behave when the L bit is not set, maintaining compatibility
    with existing known implementations prior to this document.  When
    presented with either option, PCE SHOULD select the SID which has a
    protection state matching the state of the L flag.
+---
+jgs: I'm afraid I had a hard time making sense of this paragraph. :-( I
+think maybe it's mashing together a couple different and only
+loosely-related thoughts?
+
+One of them is (perhaps?) the answer to my question above, and in effect
+is saying "some implementations do one thing and some do another and we
+didn't want to make any of them noncompliant so we threw in a MAY",
+which I'm not sure is a strong enough reason (we can discuss further).
+
+The final sentence seems like it doesn't belong, and in fact it seems
+like it should be removed. As far as I can tell it's redundant with the
+two "... and the E flag ... set to 0" paragraphs above. It also subtly
+conflicts with the first of those paragraphs. It's better to specify
+things just once, for this reason -- I suggest you remove the final
+sentence.
+---
 
    The protection enforcement constraint can only be applied to resource
    selection in which the protection state or eligibility for protection
@@ -394,7 +446,7 @@
 Internet-Draft           Protection Enforcement            November 2022
 
 
-5.1.  Backwards Compatibility
+5.1.  Backward Compatibility
 
    Considerations in the message passing between the PCC and the PCE for
    the E flag bit which are not supported by the entity are outlined in
@@ -419,18 +471,33 @@
    the E flag set to 0 for PCE-initiated LSPs even if set to 1 in the
    prior PCInitiate or PCUpd.
 
-   For a PCC which does support this document, it MAY set E flag to 1
+   For a PCC which does support this document, it MAY set the E flag to 1
    depending on local configuration.  If communicating with a PCE which
    does not yet support this document, the PCE follows the behaviour
    specified in [RFC5440] and will ignore the E flag.  Thus, it will not
    compute a path respecting the enforcement constraint.
+---
+jgs: It *will* not compute a path respecting the constraint? Or it
+*might* not? All the introductory material, about the ambiguity of
+how implementations handle the L flag, makes me think it *might*
+not, or alternately, it cannot be guaranteed to respect the 
+constraint. As you've written it, it says that is guaranteed not to
+respect the constraint, which I think is probably wrong.
+---
 
    For PCC-initiated LSPs, the PCC SHOULD ignore the E flag value
    received from the PCE in a PCUpd message.
+---
+jgs: What would it mean for the PCC not to ignore the E flag? Is there
+any reason for this to be SHOULD and not MUST?
+---
 
    For PCE-initiated LSPs, the PCC MAY process the E flag value received
-   from the PCE in a PCUpd message.  PCE SHOULD ignore the E flag value
+   from the PCE in a PCUpd message.  The PCE SHOULD ignore the E flag value
    received from the PCC in a PCRpt message.
+---
+jgs: same question about this SHOULD.
+---
 
 6.  Implementation Status