[Pce] Shepherd's Review of draft-ietf-pce-pcep-stateful-pce-gmpls-17

[Dhruv Dhody <[e-mail]>]

Hi Authors,

Thank you for your work on this document. I have finished my Shepherd
review and have some concerns that I would like to see resolved before
shipping this to the AD.

In case the format of this email is off, see
https://notes.ietf.org/draft-ietf-pce-pcep-stateful-pce-gmpls-17?view
<https://notes.ietf.org/#Major>Major

   -

   RFC 8779 added the Routing Granularity (RG) flag in the RP object for
   Explicit Label Control (ELC). Since RP object is not used in stateful
   messages how is the RG indicated in the stateful PCEP messages?
   -

   How does one identify if the LSP is a GMPLS LSP? Is it based on the
   presence of any of the generalized object types? If yes - we should add
   text and perhaps make generalized end-point mandatory for all messages. Or
   should there be some other way to signal this?
   -

   Several issues with LSP exclusion subobject (Section 7.2.2)
   - What is the role of the Attribute field (which identifies interface,
      node, or SRLG) but in this instance, it is the LSP? Do you
really need this
      field?
      - Symbolic Path Name is unique in the context of a PCC, but no PCC
      address is encoded in the sub-object. Is the use limited to an LSP
      originating from the same ingress and thus sharing the PCC? This is
      under-specified.
   -

   Error Handling
   - The error case in section 8.1 is not clear. How is it specific to
      GMPLS? If such error handling is needed it should be done
outside the scope
      of this document.
      - Creating a new Error-Type (LSP state information missing) seems to
      be an overkill. Please reuse Error-Type=19 Invalid Operation for instance.
      - RFC 8779 does not mandate LABEL-REQUEST TLV whereas this document
      does. The reason needs to be stated clearly.
      - It should be clearly stated that all the error checks for GMPLS
      objects and TLVs for PCReq/PCRep in RFC 8779 are also applicable to
      PCRpt/PCUpd/PCInitiate messages.
      - Error handling is missing in case of stateful PCEP message for
      GMPLS LSP, without the capability advertisement by both PCEP peers.
   -

   Section 11 - Please expand this section by adding all subsections as per
   RFC 6123 (the usual practice for PCE WG documents). See RFC 8779 for
   guidance.
   -

   Section 12 - Please expand the security consideration with reference to
   RFC8253, RFC8281, RFC8779 etc. Also, do list security issues specific to
   the stateful control of GMPLS LSP.

<https://notes.ietf.org/#Minor0>Minor

   - Abstract - I have concern over the use of the term
   “technology-agnostic” here. How about -

OLD:
   The PCE communication Protocol (PCEP) has been
   extended to support stateful PCE functions where the PCE retains
   information about the paths already present in the network, but
   those extensions are technology-agnostic.
NEW:
   The PCE communication Protocol (PCEP) has been
   extended to support stateful PCE functions where the Stateful PCE
   maintains information about paths and resource usage within a
   network, but these extensions do not cover all requirements for
   GMPLS networks.
END


   - Introduction - Some suggested changes

--
OLD:
   Such a
   PCE is usually referred as a stateless PCE. To request path
   computation services to a PCE, [RFC5440] defines the PCE
   communication Protocol (PCEP) for interaction between a Path
   Computation Client (PCC) and a PCE, or between two PCEs.  PCEP as
   specified in [RFC5440] mainly focuses on MPLS networks and the PCEP
   extensions needed for GMPLS-controlled networks are provided in
   [RFC8779].
NEW:
   A PCE that only maintains TED is referred to as a stateless PCE.
   [RFC5440] describes the Path Computation Element Communication
   Protocol (PCEP) for interaction between a Path Computation
   Client (PCC) and a PCE, or between two PCEs, enabling computation
   of TE LSPs. PCEP is further extended to support GMPLS-controlled
   networks as per [RFC8779].
END
--
OLD:
   In order for these applications to able to exploit the
   capability of stateful PCEs, extensions to PCEP are required.
NEW:
   In order for these applications to be able to exploit the
   capability of stateful PCEs, extensions to PCEP for GMPLS are
   required.
END
--
OLD:
   [RFC8231] provides the fundamental extensions needed for stateful
   PCE to support general functionality.
NEW:
   [RFC8231] specifies a set of extensions to PCEP to enable stateful
   control of TE LSPs where they are configured on the PCC, and
   control over them could be delegated to the PCE.
END
--
OLD:
   Section 3 provides
   General context of Stateful PCE and PCEP for GMPLS are provided in
   Section 3, and PCE initiation requirement for GMPLS is provided in
   section 4. Protocol extensions are included in section 5, as a
   solution to address such requirements.
NEW:
   Section 3 provides
   a general context of the usage of Stateful PCE and PCEP for GMPLS.
   The requirements including PCE-initiation for GMPLS LSPs is
   provided in Section 4. PCEP message extensions are specified in
   Section 6, as a solution to address such requirements.
END
--


   - The document uses the term “remote-initiated” at various places (which
   does not align with RFC 8281), please use PCE-initiated.
   - Please consider adding a terminology section
   - Section 3 - Some suggested changes

--
OLD:
   This section is built on the basis of Stateful PCE in [RFC8231] and
   PCEP for GMPLS in [RFC8779].
NEW:
   This section is built on the basis of Stateful PCE specified in
   [RFC8231] and PCEP extension for GMPLS specified in [RFC8779].
END
--
OLD:
   For passive stateful PCEs, PCReq/PCRep messages are used to convey
   path computation instructions.  GMPLS-technology specific Objects
   and TLVs are defined in [RFC8779], so this document just points at
   that work and only adds the stateful PCE aspects where applicable.
NEW:
   For passive stateful PCEs, PCReq/PCRep messages are used to request
   for path computation.  GMPLS-technology specific Objects
   and TLVs are defined in [RFC8779], this document builds on it and
   adds the stateful PCE aspects where applicable.
END
--


   - Section 4
      - The 2nd requirement does not state what is new here for GMPLS –
      could be removed?
      - The last requirement for protection - isn’t it already satisfied by
      RFC 8754. I also don’t see any extension in this document for this
      requirement either – can it be removed?
   - Section 5.1 - Suggested change

--
OLD:
   Capability Advertisement has been specified in [RFC8231], and can be
   achieved by using the "STATEFUL-PCE-CAPABILITY" in the PCEP TLV Type
   Indicators. Another GMPLS-CAPABILITY TLV in the PCEP TLV Type
   Indicators has been defined in [RFC8779].  According to [RFC8779],
   IANA created a registry to manage the value of the GMPLS-CAPABILITY
   TLV's
   field.  New bits, LSP-UPDATE-CAPABILITY (TBD1) and LSP-
   INSTANTIATION-CAPABILITY (TBD2), are introduced as flags to indicate
   the capability for LSP update and remote LSP initiation in GMPLS
   networks.
NEW:
   Capability Advertisement has been specified in [RFC8231], and can be
   achieved by using the "STATEFUL-PCE-CAPABILITY TLV" in the Open
   message. Another GMPLS-CAPABILITY TLV has been defined in [RFC8779].
   A subregistry to manage the Flag field of the GMPLS-CAPABILITY TLV
   is created by the IANA as requested by [RFC8779]. New bits,
   LSP-UPDATE-CAPABILITY (TBD1) and LSP-INSTANTIATION-CAPABILITY (TBD2),
   are introduced as flags to indicate the capability for LSP update
   and LSP initiation in GMPLS networks.
END
--


   - Section 5.2 - Suggested change

--
OLD:
   PCCs need to report the attributes of LSPs to the PCE to enable
   stateful operation of a GMPLS network.  This process is known as
   LSP state synchronization.
NEW:
   After the session between the PCC and a stateful PCE is
   initialized, the PCE must learn the state of a PCC's LSPs
   (including its attributes) before it can perform path
   computations or update LSP attributes in a PCC. This
   process is known as LSP state synchronization.
END
--


   - Section 6 - Suggested change

--
OLD:
   This section describes how the PCEP messages are extended by using
   Routing Backus-Naur Form (RBNF) [RFC5511] formats. Contents in this
   section are for informative purpose.
NEW:
   This section uses the Routing Backus-Naur Form (RBNF) [RFC5511] to
   illustrate the PCEP messages. The RBNF in this section is
   reproduced for informative purposes.
END
--


   - Section 6.1 - The use of RBNF from RFC 8231 misses the use of the
   END-POINTS object which is one of the requirements for GMPLS and using the
   RBNF from RFC 8623 would be quite confusing. If you must include RBNF it
   could be added as an example in the appendix, but I suggest removing it
   from here. You could instead add text like this -

The format of the PCRpt message is specified in [RFC8231] and extended in
[RFC8623] to include the END-POINTS object. Some RBNF constructs usage for
GMPLS is described as follows:


   - Suggested change

--
OLD:
   GENERALIZED-BANDWIDTH object has been defined
   in [RFC8779] to address the limitation of the BANDWIDTH object, with
   supporting the following:
NEW:
   [RFC8779] extended the BANDWIDTH object encoding to allow the object
   to express the generalized bandwidth to allow:
END
--


   -

   Plus you should add text about END-POINTS, LSPA, IRO, XRO, and
   SWITCH-LAYER objects in this list to be consistent with section 5.2
   -

   Section 6.2 - same change as 6.1 and this statement is incorrect - “The
   SRP object is OPTIONAL”.
   -

   Section 6.3 - if you decide to move the RBNF for others then it would
   make sense to do it for PCInitiate as well.
   - The END-POINTS object type text is applicable for all messages and not
      just PCInitiate. Maybe repeat it for PCRpt and PCUpd as well.
      - Query: Is END-POINTS mandatory only for PCInitiate? Why? I am
      wondering if the rules for END-POINTS are only applicable for PCInitiate,
      if not we need to move to a commonplace! (Section 7.1 does not limit to
      PCInitiate )
   -

   Section 7.2.1 - Update the text for S and I flag to make sure it applies
   to GMPLS LSP only.
   -

   Suggested change when XRO itself is OPTIONAL (also update similar text
   in 8.2) -

--
OLD:
   This sub-object is OPTIONAL in the exclude route object (XRO) and
   can be present multiple times.  When a stateful PCE receives a PCReq
   message carrying this sub-object, it MUST search for the identified
   LSP in its LSP-DB and then exclude from the new path computation all
   resources used by the identified LSP.
NEW:
   This sub-object MAY be present multiple times in the exclude route
   object (XRO) to exclude resources from multiple LSPs. When a stateful
   PCE receives a PCEP message carrying this sub-object, it MUST search
   for the identified LSP in its LSP-DB and then exclude from the new
   path computation all resources used by the identified LSP.
END
--


   - Section 8.2 - Error is limited to PCReq, I suggest changing that to
   PCReq/PCRpt or just PCEP message. Also, update the error string to match
   with the general style -
   https://www.iana.org/assignments/pcep/pcep.xhtml#pcep-error-object
   - Section 10 - Please update the text in the IANA consideration to say
   that you are requesting the allocation and not that IANA has already made
   the allocation.
   - Section 10.2 - Please update

--
OLD:
   IANA maintains the "PCEP Parameters" registry containing a
   subregistry called "PCEP Objects".  This registry has a subregistry
   for the XRO (Exclude Route Object) listing the sub-objects that can
   be carried in the XRO.  IANA is requested to assign a further sub-
   object that can be carried in the XRO as follows:
NEW:
   IANA maintains the various XRO Subobjects types within the "XRO
   Subobjects" subregistry of the PCEP Numbers registry. IANA is
   requested to allocate a codepoint for another XRO subobject as
   follows:
END
--


   - Section 10.3 - You need to also add

   New values are to be assigned by Standards Action [RFC8126].
   Each bit should be tracked with the following qualities:

   o  Bit number (counting from bit 0 as the most significant bit)

   o  Capability description

   o  Defining RFC

<https://notes.ietf.org/#Nits>Nits

   - The title - s/Path Computation Element (PCE) Protocol/Path Computation
   Element Communication Protocol (PCEP)/ ; PCE is well known as per
   https://www.rfc-editor.org/materials/abbrev.expansion.txt
   - Expand on first use or expand in the terminology section
      - WSON
      - OTN
      - SONET
      - SDH
      - PCUpd
      - PCRpt
      - PCReq
      - PCRep
      - PCInitiate
      - TDM
      - L2SC
      - OTN-TDM
      - LSC
      - TSpec
      - WDM
      - LSPA
      - IRO
      - XRO
      - ERO
      - MEF
   - Add suitable references when mentioning
      - active stateful PCE and passive stateful PCE
      - delegation
      - explicit label control
      - bidirectional co-routed LSP
   - s/GMPLS CAPABILITY TLV/GMPLS-CAPABILITY TLV/
   - Section 5.2 - s/The END-POINTS object is carried/The END-POINTS object
   can be carried/
   - s/GENERALIZED-BANDWIDTH/Generalized bandwidth/
   - s/“label request” TLV/LABEL-REQUEST TLV/
   - s/unnumbered endpoint TLV/UNNUMBERED-ENDPOINT TLV/

Thanks,
Dhruv