Re: [Pce] Spencer Dawkins' Discuss on draft-ietf-pce-pce-initiated-lsp-10: (with DISCUSS and COMMENT)

[Spencer Dawkins at IETF]

Hi, Jon,

On Wed, Oct 4, 2017 at 8:48 AM, Jonathan Hardwick <
jonathan.hardw...@metaswitch.com> wrote:

> Hi Spencer
>
> Many thanks for these comments.  I'm picking up this thread and replying
> as PCE working group chair, as the authors are unavailable.  I am very
> sorry for the delay.
>
> Please see my proposed resolutions inline below, marked with "Jon>"
>

I'd clear based on an update that heads this direction. Want to let me know
when the working group has had a chance to review the proposed text?

And thanks.

Spencer


> Best regards
> Jon
>
> 
>
> --
> DISCUSS:
> --
>
> This ballot position would be Please Educate Me, if that was a choice, but
> that's not a choice. I'm sure we can clear this quickly. And I found this
> document very easy to read as a reviewer - thanks for that.
>
> I found a couple of places where SHOULDs seemed at least under-specified,
> and this one looked important.
>
> In this text,
>
>   LSP State Synchronization procedures are described in section 5.4 of
>[I-D.ietf-pce-stateful-pce].  During State Synchronization, a PCC
>reports the state of its LSPs to the PCE using PCRpt messages,
>setting the SYNC flag in the LSP Object.  For PCE-initiated LSPs, the
>PCC MUST also set the Create Flag in the LSP Object and MAY include
>the SPEAKER-ENTITY-ID TLV identifying the PCE that requested the LSP
>creation.  At the end of state synchronization, the PCE SHOULD
>compare the reported PCE-Initiated LSPs with its configuration.  For
>any mismatch, the PCE SHOULD send a PCInitiate message to initiate
>any missing LSPs and/or remove any LSPs that are not wanted.
>
> I’m having a hard time understanding why a PCE would not compare reported
> PCE-Initiated LSPs with its configuration, which is allowed by the first
> SHOULD. Does that mean you thought it was important to TRY to synchronize,
> but you’re not curious enough to check whether that worked?
>
> I can imagine reasons why you wouldn't try to fix the LSPs that weren't
> synchronized, at least not immediately, but you might also give guidance
> about one or more reasons why you wouldn't try, to help implementers
> understand what not doing what the SHOULD means, and make informed choices
> for their implementations.
>
>
> Jon> I definitely agree with you that, having received a snapshot from the
> PCC, there is no reason that the PCE would not then compare the PCC's state
> with its local copy i.e. the first SHOULD ought to be a MUST.  The
> intention of the second SHOULD was to leave the PCE with some flexibility
> for dealing with clients that are out of sync.  For example, perhaps the
> clients are slow, or perhaps the operator's policy is to prefer not to
> disrupt existing flows so long as the main characteristics of the flows are
> correct.  These are just my invented examples, but we expect there might be
> valid operational reasons along these lines, so we wanted to the draft to
> allow the server the flexibility to choose whether it updates the flows, or
> not.
>
> Here is my proposed fix.
>
> OLD
>== as quoted above ===
> NEW
>LSP State Synchronization procedures are described in section 5.4 of
>[RFC8231].  During State Synchronization, a PCC
>reports the state of its LSPs to the PCE using PCRpt messages,
>setting the SYNC flag in the LSP Object.  For PCE-initiated LSPs, the
>PCC MUST also set the Create Flag in the LSP Object and MAY include
>the SPEAKER-ENTITY-ID TLV identifying the PCE that requested the LSP
>creation.  At the end of state synchronization, the PCE MUST
>compare the reported PCE-Initiated LSPs with its configuration.  For
>any mismatch, the PCE SHOULD send a PCInitiate message to initiate
>any missing LSPs and/or remove any LSPs that are not wanted.  Under
>some circumstances, depending on the deployment,  it might be preferable
>for a PCE not to send this PCInitiate immediately, or at all.  For
>example, the PCC may be a slow device, or the operator might prefer
>not to disrupt active flows.
>
>
> --
> COMMENT:
> --
>
> In this text,
>
>   The State Timeout Interval timer ensures that a PCE crash does not
>result in automatic and immediate disruption for the services using
>PCE-initiated LSPs.  PCE-initiated LSPs are not removed immediately
>upon PCE failure.  Instead, they are cleaned up on the expiration of
>this timer.  This allows for network cleanup without manual
>intervention.  The PCC SHOULD support removal of PCE-initiated LSPs
>as one of the behaviors applied on expiration of the State Timeout
>Interval timer.  The behavior SHOULD be picked based on local policy,
>and can result either in LSP 

[Pce] Your IESG comments on draft-ietf-pce-pce-initiated-lsp

[Jonathan Hardwick]

Hi Alvaro


Many thanks for your comments.  I'm picking up this thread and replying as PCE 
working group chair, as the authors are unavailable.  I am very sorry for the 
delay.



For some reason I can't find the original email with your comments in, so I 
have scraped the text from the datatracker and pasted it below.  Please see my 
proposed resolutions inline below, marked with "Jon>"



Best regards

Jon


Just some minor comments:

(1) Section 3.2

   This document defines a new PCEP message, the LSP Initiate Request
   (PCInitiate) message, which a PCE can send to a PCE to request the
   initiaton or deletion of an LSP.

s/...PCE can send to a PCE.../...PCE can send to a PCC...

Jon> OK.


(2) Section 5.3: "The source address MAY be either specified or left up to the 
PCC decision using the 0.0.0.0 value."  These seem to be the only two possible 
options, so s/MAY/MUST.

Jon> OK.  So, including the feedback we got from Adam Roach, the new text is:

''The source address MUST either be specified or left for the PCC to choose by 
setting it to "0.0.0.0" (if the destination is an IPv4 address) or "::" (if the 
destination is an IPv6 address).''


(3) Also from Section 5.3: "...the END-POINTS object MAY be included to 
explicitly convey the destination...For LSPs to be setup by other means, the 
END-POINTS object MAY be omitted..."

You already wrote that "other setup methods are outside the scope".  Also, not 
including the END-POINTS object is not an indication of other types of LSPs, as 
its use is optional to start with.  Take out the last sentence.

Jon> Fine with me.

___
Pce mailing list
Pce@ietf.org
https://www.ietf.org/mailman/listinfo/pce

Re: [Pce] Eric Rescorla's No Objection on draft-ietf-pce-pce-initiated-lsp-10: (with COMMENT)

[Jonathan Hardwick]

Hi Eric



Many thanks for these comments.  I'm picking up this thread and replying as PCE 
working group chair, as the authors are unavailable.  I am very sorry for the 
delay.



Please see my proposed resolutions inline below, marked with "Jon>"



Best regards

Jon







--

COMMENT:

--



Document: draft-ietf-pce-pce-initiated-lsp-10.txt



Note: I reviewed this document on my experimental Phabricator instance.

You can see the comments inline at:



  https://mozphab-ietf.devsvcdev.mozaws.net/D20



Jon> This is a useful tool, thanks!





It may just be my unfamiliarity with this system, but it's not clear to me what 
the security model is here for the delegation. As I understand this document 
the PCC just tells the PCE that it has delegated the LSP to it, and then the 
PCE can make the LSP via the normal procedures. But what is it that tells the 
rest of the system that the PCC is allowed to manage that LSP. I didn't get 
that out of this document or out of a cursory look at RFC 8051.



Jon> The model is that the PCE makes the first move.  It instructs the PCC to 
initiate an LSP that the PCC has not previously heard of.  The PCC initiates 
the LSP and sends a PCRpt message delegating control over it to the PCE.  Once 
it receives the delegation, the PCE is free to make whatever changes it likes, 
or delete the LSP.





INLINE COMMENTS

Line 162

   A possible use case is a software-driven network, where applications

   request network resources and paths from the network infrastructure.

NIT: isn't the term here "software-defined network"



Jon> Indeed.  Will fix.





Line 218

   all state related to the LSP and sends a PCRpt for the removed state.

   See details in Section 5.4.

A diagram would sure help here.



Jon> How about this:



NEW

   The following diagram illustrates these message exchanges.



  +-+-++-+-+

  |PCC||PCE|

  +-+-++-+-+

||

|<--PCInitiate---| (Initiate LSP)

||

|---PCRpt, PLSP_ID=1, D=1--->| (Confirm initiation)

|.   |

|.   |

||

|<--PCUpd, PLSP_ID=1-| (Update LSP)

||

|---PCRpt, PLSP_ID=1, D=1--->| (Confirm update)

|.   |

|.   |

||

|<--PCInitiate, PLSP_ID=1, R=1---| (Delete LSP)

||

|---PCRpt, PLSP_ID=1, R=1--->| (Confirm update)





   Figure 1: Initiated LSP lifecycle

END NEW



Line 263

   Unassigned bits are considered reserved.  They MUST be set to 0 on

   transmission and MUST be ignored on receipt.

As I understand this text, you are merely adding a new code point to flags. I'm 
not sure it's necessary to reproduce the PDU, but if you do, you should clarify 
that th only change you are making is adding a new field. Perhaps on line 249 
"It is reproduced here with the addition of the new I bit"



Jon> Yes, this is correct.  I will update this section and the similar cases 
below to follow the form of the "good text" from line 436 that you cite below.



Line 278

   and the LSP objects, and MAY contain other objects, as discussed

   later in this section.

Is the syntax here supposed to be ABNF? If so, you need a citation to the 
syntax".



Jon> It's RBNF. It’s defined in [RFC5511], listed as a normative reference and 
cited from section 2.





Line 337

  create an LSP.  If set to 1, it indicates a request to remove an

  LSP.

I have the same comment here about repeating PDU.



Jon> Ack.





Line 436

   The LSP object is defined in [I-D.ietf-pce-stateful-pce] and included

   here for easy reference.

This is good text, and is what I would encourage the other places you replicate 
PDUs from other documents.



Jon> Ack.


___
Pce mailing list
Pce@ietf.org
https://www.ietf.org/mailman/listinfo/pce

Re: [Pce] Adam Roach's No Objection on draft-ietf-pce-pce-initiated-lsp-10: (with COMMENT)

[Jonathan Hardwick]

Hi Adam

Many thanks for these comments.  I'm picking up this thread and replying as PCE 
working group chair, as the authors are unavailable.  I am very sorry for the 
delay.

Please see my proposed resolutions inline below, marked with "Jon>"

Best regards
Jon



--
COMMENT:
--

Section 5.1 defines the PCInititate Message, and is generally pretty good about 
indicating where its component elements come from; however, it's missing 
pointers to , , and . I think you want to add: ", 
, and  are defined in [RFC5440]".

Jon> I will add this:  ' is defined in [RFC8231]. , and  
are defined in [RFC5440].'


Section 5.3 indicates that an indication that the PCC is supposed to pick the 
source address is signaled by using a source address of "0.0.0.0" -- 
presumably, if the destination is an IPv6 address, this would instead use "::", 
right? Please add text that addresses the IPv6 case.

Jon> Thanks, you are correct.  I will change it to this: 'The source address 
MAY either be specified or left for the PCC to choose by setting it to 
"0.0.0.0" (if the destination is an IPv4 address) or "::" (if the destination 
is an IPv6 address).'


I'm pretty certain that [I-D.ietf-pce-stateful-sync-optimizations] is a 
normative reference. Even though its use is optional, this document contains 
normative statements regarding its mechanism. See 
 for guidance, 
and "Note 1" of that statement in particular.

Jon> Oops! We will make it a normative reference.
___
Pce mailing list
Pce@ietf.org
https://www.ietf.org/mailman/listinfo/pce

Re: [Pce] Spencer Dawkins' Discuss on draft-ietf-pce-pce-initiated-lsp-10: (with DISCUSS and COMMENT)

[Jonathan Hardwick]

Hi Spencer

Many thanks for these comments.  I'm picking up this thread and replying as PCE 
working group chair, as the authors are unavailable.  I am very sorry for the 
delay.

Please see my proposed resolutions inline below, marked with "Jon>"

Best regards
Jon



--
DISCUSS:
--

This ballot position would be Please Educate Me, if that was a choice, but 
that's not a choice. I'm sure we can clear this quickly. And I found this 
document very easy to read as a reviewer - thanks for that.

I found a couple of places where SHOULDs seemed at least under-specified, and 
this one looked important.

In this text,

  LSP State Synchronization procedures are described in section 5.4 of
   [I-D.ietf-pce-stateful-pce].  During State Synchronization, a PCC
   reports the state of its LSPs to the PCE using PCRpt messages,
   setting the SYNC flag in the LSP Object.  For PCE-initiated LSPs, the
   PCC MUST also set the Create Flag in the LSP Object and MAY include
   the SPEAKER-ENTITY-ID TLV identifying the PCE that requested the LSP
   creation.  At the end of state synchronization, the PCE SHOULD
   compare the reported PCE-Initiated LSPs with its configuration.  For
   any mismatch, the PCE SHOULD send a PCInitiate message to initiate
   any missing LSPs and/or remove any LSPs that are not wanted.

I’m having a hard time understanding why a PCE would not compare reported 
PCE-Initiated LSPs with its configuration, which is allowed by the first 
SHOULD. Does that mean you thought it was important to TRY to synchronize, but 
you’re not curious enough to check whether that worked?

I can imagine reasons why you wouldn't try to fix the LSPs that weren't 
synchronized, at least not immediately, but you might also give guidance about 
one or more reasons why you wouldn't try, to help implementers understand what 
not doing what the SHOULD means, and make informed choices for their 
implementations.


Jon> I definitely agree with you that, having received a snapshot from the PCC, 
there is no reason that the PCE would not then compare the PCC's state with its 
local copy i.e. the first SHOULD ought to be a MUST.  The intention of the 
second SHOULD was to leave the PCE with some flexibility for dealing with 
clients that are out of sync.  For example, perhaps the clients are slow, or 
perhaps the operator's policy is to prefer not to disrupt existing flows so 
long as the main characteristics of the flows are correct.  These are just my 
invented examples, but we expect there might be valid operational reasons along 
these lines, so we wanted to the draft to allow the server the flexibility to 
choose whether it updates the flows, or not.

Here is my proposed fix.

OLD
   == as quoted above ===
NEW
   LSP State Synchronization procedures are described in section 5.4 of
   [RFC8231].  During State Synchronization, a PCC
   reports the state of its LSPs to the PCE using PCRpt messages,
   setting the SYNC flag in the LSP Object.  For PCE-initiated LSPs, the
   PCC MUST also set the Create Flag in the LSP Object and MAY include
   the SPEAKER-ENTITY-ID TLV identifying the PCE that requested the LSP
   creation.  At the end of state synchronization, the PCE MUST
   compare the reported PCE-Initiated LSPs with its configuration.  For
   any mismatch, the PCE SHOULD send a PCInitiate message to initiate
   any missing LSPs and/or remove any LSPs that are not wanted.  Under
   some circumstances, depending on the deployment,  it might be preferable
   for a PCE not to send this PCInitiate immediately, or at all.  For
   example, the PCC may be a slow device, or the operator might prefer
   not to disrupt active flows.
   

--
COMMENT:
--

In this text,

  The State Timeout Interval timer ensures that a PCE crash does not
   result in automatic and immediate disruption for the services using
   PCE-initiated LSPs.  PCE-initiated LSPs are not removed immediately
   upon PCE failure.  Instead, they are cleaned up on the expiration of
   this timer.  This allows for network cleanup without manual
   intervention.  The PCC SHOULD support removal of PCE-initiated LSPs
   as one of the behaviors applied on expiration of the State Timeout
   Interval timer.  The behavior SHOULD be picked based on local policy,
   and can result either in LSP removal, or in reverting to operator-
   defined default parameters.

I found myself wondering why “The PCC SHOULD support removal of PCE-initiated 
LSPs” is a SHOULD, and not a MUST, but if it’s a SHOULD, you might say 
something about the effects of not supporting this, in order to help 
implementers make an informed decision about whether to support it.

In the same text, I found myself wondering if there were other