[Pce] Shepherd's Review of draft-ietf-pce-pce-initiated-lsp-07

[Julien Meuric]

Dear authors of draft-ietf-pce-pce-initiated-lsp,

Please find below my shepherd's review of the aforementioned I-D.

_Summary_

The document does not need much work to move forward. As discussed with 
Ina during the IETF week, a few items deserve to be highlighted:
- the choice of zero as wildcard PLSP-ID to remove all LSPs initiated by 
a PCE is unsafe;
- the use of the END-POINTS object is misaligned with RFC 5440's 
definition and not suited to SR.


_Detailed Comments_
--
Header
---
- Like with draft-ietf-pce-stateful-pce, adding "Individual Contributor" 
after Ed's name helps getting rid of the odd empty line.

--
Abstract
---
- s/provide stateful control/provide active control/
--
Section 1.
---
- s/Path Computation Element Protocol PCEP/Path Computation Element 
communication Protocol (PCEP)/

--
Section 3.
---
- s/provides stateful control/provides active control/
- s/is one of a software-driven/is a software-driven/
- s/is one of dynamically/is dynamically/
- s/is that of demand/is demand/
- s/Operation overview/Operation Overview/
- s/PCE provisioned/PCE-provisioned/
- s/it also generates/it MUST also generate/
- s/PCC also sets/PCC MUST also set/
- s/PCE may update/PCE MAY update/
- s/PCC initiated LSPs/PCC-initiated LSPs/
--
Section 4.
---
- s/PCE provisioned/PCE-provisioned/
--
Section 5.
---
- s/LSP instantiation and deletion/LSP Instantiation and Deletion/
- OLD:
A Path Computation LSP Initiate Message (also referred to as PCInitiate 
message) is a PCEP message...

  NEW:
A Path Computation LSP Initiate Message is referred to as PCInitiate 
message: it is a PCEP message...


- s/and may contain/and MAY contain/
- OLD :
If the SRP object is missing, the PCC MUST send a PCErr with error-type 
6 (Mandatory Object missing) and error-value=10 (SRP Object missing) 
(per [I-D.ietf-pce-stateful-pce]. If the LSP object is missing, the PCC 
MUST send a PCErr with error-type 6 (Mandatory Object missing) and 
error-value=8 (LSP Object missing) (per [I-D.ietf-pce-stateful-pce]).

  NEW:
Missing SRP and LSP objects in PCInitiate MUST trigger the same PCErr 
procedures as specified in [I-D.ietf-pce-stateful-pce] for PCUpd.


- s//[]/  (see discussion below)
- s/5.3. LSP instantiation/5.3. LSP Instantiation/
- s/LSP Initiate Message/PCInitiate message/
- s/results in a PCErr/MUST result in a PCErr/

- The use of the END-POINTS objects has puzzled me for multiple reasons:
 * RFC 5440 defines the object as a pair of IDs allowing to identify 
the two points to be interconnected by the ERO to be filled in, whereas 
the draft uses it to push the IDs of the signaling ends;
 * The signaling source ID to be used should rather be up to the PCC, 
the requirement on pushing it from the PCE is not obvious;
 * The ERO may include some IDs that could be used as default 
source/destination IDs, which also makes the need for a destination ID 
less obvious;

 * To address these, I see 3 options:
  1- Giving up the use of a specific object and fully rely on the ERO;
  2- Defining new "SignalingRemoteID" types (possibly within the 
END-POINTS object class) to (optionally) convey the info;
  3- Rephrase the text to turn the unwanted "redefinition" of the 
END-POINTS object into a wording more consistent with RFC 5440, e.g.:

OLD:
   The END-POINTS Object is mandatory for an instantiation request of an
   RSVP-signaled LSP.  It contains the source and destination addresses
   for provisioning the LSP.  If the END-POINTS Object is missing, the
   PCC MUST send a PCErr message with Error-type=6 (Mandatory Object
   missing) and Error-value=3 (END-POINTS Object missing).
NEW:
   For an instantiation request of an RSVP-signaled LSP, the destination
   address may be needed. The PCC may determine it from a provided object
   (e.g., ERO) or a local decision. Alternatively, the END-POINTS object
   MAY be included to explicitly convey the destination addresses to be
   used in the RSVP-TE signaling. The source address may be either
   specified or left up to the PCC decision using the 0.0.0.0 value. For
   LSPs to be setup by other means (e.g., Segment Routing), the END-POINTS
   object SHOULD be omitted.

 * In case you go for option 2, you still need to be more explicit on 
the non-RSVP case; i.e., the new text should say: "The  MAY 
be included for instantiation request of an RSVP-TE-signaled LSP, and 
SHOULD be omitted otherwise."


- s/echo the SRP-id-number/echo the SRP-ID-number/
- The 2nd paragraph on page 11 ("On succesful completion...") duplicates 
the 2nd one on page 10 ("The PCE MAY include...") and should be dropped 
(or at least the 2nd half of it).

- s/succesful completion/successful completion/
- s/5.3.1. The Create flag/5.3.1. The Create Flag/
- On Figure 3, the "O" would be better in the middle of the 3-bit field.
- Once defined, the phrases "Create flag"/"C Flag"/"C-flag" are 
alternatively used, please pick one and use it everywhere (I personally 
like "C-flag" but "R flag" seems common 

Re: [Pce] draft-ietf-pce-stateful-pce : clarifying the End Of Synchronization marker

[Olivier Dugeon]

Hello Ina,

The beginning of our proposal seems OK for me, but the "/MUST include an empty 
ERO/" part seems in contradiction with our proposal that specifically mention 
that an ERO could not be empty. As it concerns the end of the synchronisation, 
I think that it is not necessary to include such ERO. The LSP-Identifiers TLV 
with special values of all zeroes is sufficient. I would propose to replace the 
following text:

"/The PCRpt message MUST include an empty ERO as its intended path and SHOULD 
NOT include the optional RRO object for its actual path./"

by

"/The PCRpt message MUST NOT include any ERO and RRO objects for its actual 
path./"

Indeed, the end marker of synchronisation is composed by a specific LSP with a 
PLSP-ID equal to 0 and a LSP-IDENTIFIERS TLV with all zeroes. IMHO, this 
corresponds to a "/zero LSP/" or "/null LSP/", where there is no need to attach 
and ERO or RRO. So, it is not necessary to convey them in the PCRpt message. 
From an algorithm point of view, as the code must include a special test to 
detect this end marker, it is not a problem to check that the ERO and RRO are 
not present, and when they must be present, this is check by the another branch 
in the code.

Regards

Olivier


Le 28/07/2016 20:15, Ina Minei a écrit :
> Stephane, 
>
> Thank you for the detailed feedback. How about the following text?
>
> The end of synchronization marker is a PCRpt message with the SYNC Flag set 
> to 0 for an LSP Object with PLSP-ID equal to the reserved value 0 (see 
> Section 7.3). In this case, the LSP Object SHOULD NOT include the 
> SYMBOLIC-PATH-NAME TLV and SHOULD include the LSP- IDENTIFIERS TLV with the 
> special value of all zeroes. The PCRpt message MUST include an empty ERO as 
> its intended path and SHOULD NOT include the optional RRO object for its 
> actual path. If the PCC has no state to synchronize, it SHOULD only send the 
> end of synchronization marker.
>
> On Mon, Jun 27, 2016 at 5:20 AM,  > wrote:
>
> Hi,
>
> Thanks for the feedback.
>
> > The intent here is to use a minimal PCRpt message, hence we explicitly 
> exclude SYMBOLIC-PATH-NAME TLV and RRO. ERO is kept empty for the same case.
> > I think we have not precluded other TLVs from appearing in EOS to allow 
> future extensions.
> > I do not think LSP-IDENTIFIERS TLV should be carried here, as it serves 
> no purpose and is not required -- section 7.3.1's MUST condition does not 
> trigger, as
> > PLSP-ID=0 is a reserved value and does not identify an LSP.
>
> Even if you think that LSP-ID should not be carried, it's not explicitly 
> mentioned in the draft, so it's authorized.
> Why not restricting EOS to the minimal case, and let potential future 
> extensions to modify it ? To you forsee anycase that could require 
> modification of EOS content ?
>
> At least the text should use normative words.
>
> Best Regards,
>
> Stephane
>
> -Original Message-
> From: Robert Varga [mailto:n...@hq.sk ]
> Sent: Monday, June 27, 2016 14:02
> To: LITKOWSKI Stephane OBS/OINIS; pce@ietf.org 
> Subject: Re: [Pce] draft-ietf-pce-stateful-pce : clarifying the End Of 
> Synchronization marker
>
> On 06/21/2016 05:18 PM, stephane.litkow...@orange.com 
>  wrote:
> > Hi,
> >
> > Doing some interop testing between two vendors we falled into 
> misinterpretation of the current text of the End Of Sync marker content.
> >
> > Here is the current text :
> >
> > "The end of synchronization marker is a PCRpt message with the SYNC
> >Flag set to 0 for an LSP Object with PLSP-ID equal to the reserved
> >value 0 (see Section 7.3).  The LSP Object does not include the
> >SYMBOLIC-PATH-NAME TLV in this case, it will include an empty ERO as
> >its intended path and will not include the optional RRO object in the
> >path.  If the PCC has no state to synchronize, it will only send the
> >end of synchronization marker."
> >
> > The current text, IMO, has the following issues :
> > - it uses non normative wording : "does not include", "will include" , 
> "will not include". How do we need to interpret it ? MUST, SHOULD, MAY ?
> > - it does not precise if it can include or not some other objects : can 
> it include an LSP-Identifier object (with all fields to 0) ?
>
> The intent here is to use a minimal PCRpt message, hence we explicitly 
> exclude SYMBOLIC-PATH-NAME TLV and RRO. ERO is kept empty for the same case.
>
> I think we have not precluded other TLVs from appearing in EOS to allow 
> future extensions.
>
> I do not think LSP-IDENTIFIERS TLV should be carried here, as it serves 
> no purpose and is not required -- section 7.3.1's MUST condition does not 
> trigger, as PLSP-ID=0 is a reserved value and does not identify an LSP.
>
> > It