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

["Andrew Stone (Nokia)" <[e-mail]>]

Hi John, PCE WG

New version -09 has been posted [1]. Hopefully this helps clear up the discussion comments below.

Summary:

- Reduced the verbosity in the introduction regarding segment routing background
- Added comment regarding control/data plane applicability and prefixed SR specific comments
- Moved preferred content from section 5 to section 4 and consolidated the wording with 'do not care'
- Various nits that were caught in review

Thanks again for the review! 
Andrew

[1] https://author-tools.ietf.org/iddiff?url1=draft-ietf-pce-local-protection-enforcement-08&url2=draft-ietf-pce-local-protection-enforcement-09&difftype=--html



On 2023-04-24, 7:13 PM, "Andrew Stone (Nokia)" <[e-mail] <mailto:[e-mail]>> wrote:


Hi John,


Thank you very much for the review and including the inline diff, made it easier to follow. 


Good observation and point about it being technology-agnostic. It was motivated by SR, but as you point out the E+F flags descriptions and behavior clarifications may be applicable outside of SR in ways I'm unaware of or don't exist yet. The introduction was intended to build context in the purpose of the draft, not necessarily a tutorial on SR, but I can see how it may be a bit heavy for that purpose. Considering these two observations perhaps the SR specific discussions from the introduction would be better to move to section 4.2, and the introduction include statements about technology applicability? And where applicable, the use of 'SID' outside of that section can be amended where applicable? There are some recommendation text embedded in the document that should be kept (ex: 5.2: .....It is RECOMMENDED ..... assume a Node SID is protected ...RECOMMENDED ... assume an Adjacency SID is protected if the backup flag advertised ....). Perhaps those can be pre-ambled with "If the technology is SR then ..." ? Do you think this would help clarify what portions are agnostic vs SR specific?


Further replies in-line below with >>>>>>[Andrew]. >>>> blocks. 


Will adopt your editorial changes and address discussions for a new revision soon. 


Thanks again,
Andrew










On 2023-04-20, 3:35 PM, "John Scudder" <[e-mail] <mailto:[e-mail]> <mailto:[e-mail] <mailto:[e-mail]>>> wrote:
CAUTION: This is an external email. Please be very careful when clicking links or opening attachments. See the URL nok.it/ext for additional information.


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.
+---


>>>>>>>>>>>> 
[Andrew] "When it can be applied" was intended to suggest to a reader that there may be scenarios where the PCE is simply unaware whether a given resource is protected or not, or may even need to make assumptions on it's protected state because the information may not be available (knowledge if Node SID is protected by all nodes along the path). It's likely redundant and can be removed (PCE can't enforce what it doesn't know). Will adjust.
>>>>>>>>>>>>








@@ -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. :-)
+---


>>>>>>>>>>>>
[Andrew] Even if the operator does not care to enforce something, PCE still needs to make a selection on which SID to pick in a situation where two or more are available, which are logically equal, and without a consistent preference, operational behavior may differ between two PCEs. This section is to introduces the two options, without saying which to use. Even if the end-user doesn't care, PCE should still pick one behavior or the other. Section 5 goes a bit more into contrasting the two and gives recommendations. Perhaps the "do not care" text is throwing things off a bit, will do some adjustments to this paragraph.
>>>>>>>>>>>>








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.
+---


>>>>>>>>>>>> 
[Andrew] Backward friendly text with existing implementations. Similar discussion at [1] and [2]
[1]: https://mailarchive.ietf.org/arch/msg/pce/vPfrDDqtMm1aXTfCID9poG3H42E/ <https://mailarchive.ietf.org/arch/msg/pce/vPfrDDqtMm1aXTfCID9poG3H42E/> 
[2]: https://mailarchive.ietf.org/arch/msg/pce/4EX28antvCp_2CY--7RJmCvR-jo/ <https://mailarchive.ietf.org/arch/msg/pce/4EX28antvCp_2CY--7RJmCvR-jo/>
>>>>>>>>>>>>








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.
+---


>>>>>>>>>>>> 
[Andrew] This paragraph is an extension of the section 4.2 "do not care" case. I'll move the contrasting the two terms to section 4.2 with the related "do not care" adjustments, and keep this section to just actions (should do __). Good catch about the redundant statement, agreed and will drop. So this paragraph will be removed and some of the content moved to 4.2.
>>>>>>>>>>>>










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.
+---


>>>>>>>>>>>> 
[Andrew] Good catch, thanks - correct - it might compute a path violating the constraints. Will correct.
>>>>>>>>>>>>








For PCC-initiated LSPs, the P
CC 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.
+---


>>>>>>>>>>>> 
[Andrew] For the above two: I'm not aware of any spec or implementation behavior where PCC interpets the E flag echo however as an example, one risk is if PCC sends PcRpt+Delegation with E=1 and receives PcUpd with E=0, it *could* perform some kind of check and do a PcErr, breaking compatibility, meanwhile the goal is to maintain PCEP backwards compatibility. So the intention was to give recommendation on how the (PCC/PCE) should interpret an echo (i.e do not interpret it), to maintain backwards compatibility. The SHOULD is leaving a door open in case some future reason or use case someone may have justification for interpreting it. Should I add a remark on why interpreting it may break compatibility or leave as is? 
>>>>>>>>>>>>