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

[Dhruv Dhody <[e-mail]>]

Hi Haomian,

Thanks for the update. Feel free to reach out if my comments need more
clarifications! Please see inline...

On Wed, Mar 30, 2022 at 3:42 PM Zhenghaomian <[e-mail]>
wrote:

> Hi Dhruv,
>
>
>
> Thank you for shepherding the documents with detailed review and comments.
> I am having some clarification questions in the major issues, while most of
> the minor ones could be accepted. Please check them inline with [Haomian]…
>
>
>
> BTW I was trying to use hedgedoc but the link fails exactly today…
>
>
>

[Dhruv] Working for me as of now! Though hedgedoc was down yesterday.



> Best wishes,
>
> Haomian
>
>
>
> *发件人:* Dhruv Dhody [mailto:[e-mail]]
> *发送时间:* 2022年2月19日 2:52
> *收件人:* [e-mail]
> *抄送:* [e-mail]
> *主题:* Shepherd's Review of draft-ietf-pce-pcep-stateful-pce-gmpls-17
>
>
>
> 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
> 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?
>
> [Haomian] When you say ‘RP object is not used in stateful messages’, are
> you referring to RFC8231 or other later documents? I assume the RP objects
> and RG flag could be valid in the stateful GMPLS PCEP messages. What is the
> problem here?
>
[Dhruv]: Let me clarify my comment first - RFC 8779 added RG flags in RP
Object. The RP object is not carried in the PCRpt/PCUpd/PCInitiate
messages. What is the way the RG flag is indicated in these messages? Are
they not needed or are they missing?

Hope the problem is clearer now?

·      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?
>
> [Haomian] I assume the question is to differentiate the GMPLS LSP with
> non-GMPLS LSP. Yes it’s done by checking the capabilities included in
> Generalized END-POINTS. The scope of this document is to compute the path
> in a GMPLS network, i.e., we are not targeting on solving the problem for a
> hybrid LSP. The current statement “Stateful PCEP messages MUST include the END-POINTS
> with Generalized Endpoint object type, containing the LABEL-REQUEST TLV.”
> looks sufficient.
>
[Dhruv]: I would suggest going more explicit - "The presence of the
END-POINTS object with Generalized Endpoint Object Type indicates that the
stateful PCEP message is for a GMPLS LSP."

> ·      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?
>
> o   [Haomian] Yes its description was removed due to unnecessary, and the
> one on the TLV should be removed as well. Then we are having a 16 bit
> reservation for flags.
>
[Dhruv]: OK! Which TLV?

>
>    - 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.
>
> o   [Haomian] Not quite clear about the question. If there is only one
> LSP to exclude, it’s unique in the context of PCC address. If there are
> multiple LSPs, the sub-object would be present multiple times in the XRO.
>
[Dhruv]: It is possible for LSP1 (A to B) and LSP2 (C to B) to share the
same symbolic-path-name NAME1 since symbolic path name are unique in the
context of head node A and C. At the PCE, just the symbolic path name=NAME1
is ambiguous as one also needs to know head node A fo C to differentiate
between LSP1 and LSP2. In my comment, I asked if the PCC sending the
message is assumed to be the headnode? If not encoding needs to change or
more clarification text needs to be added.

Is it clear now?

>
>
> ·      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.
>
> o   [Haomian] not an expert in packet, I think the TDM-based
> re-optimization is more challenging because there is a risk on losing bits,
> which is not going to happen in packet networks. Shall we restrict the
> re-optimization to ‘GMPLS re-optimization’?
>
[Dhruv]: But the error is all about the case when PCReq message for
reoptimization refers to an LSP that does not exist which is generic. I
think maybe best is to add a note saying - "Note that this error message
could also be used by non-GMPLS LSPs".

>
>    - Creating a new Error-Type (LSP state information missing) seems to
>       be an overkill. Please reuse Error-Type=19 Invalid Operation for instance.
>
> o   [Haomian] ok, we have changed ‘TBD5’ to ‘19’.
>
>    - RFC 8779 does not mandate LABEL-REQUEST TLV whereas this document
>       does. The reason needs to be stated clearly.
>
> o   [Haomian] I don’t think this is a problem in error handling. Instead
> we detect something to be changed in section 6.3. I don’t remember if
> people agreed on using only LABEL-REQUEST TLV in stateful GMPLS PCEP, but
> that’s my taking from reading the current text. So how about following?
>
> --
>
> Old:
>
> The END-POINTS object MUST contain a LABEL-REQUEST TLV per [RFC8779].
> The LABEL-REQUEST TLV is used to specify the switching type, encoding type
> and G-PID of the LSP being instantiated by the PCE.
>
> New:
>
> The END-POINTS object MAY contain a LABEL-REQUEST TLV and/or LABEL-SET TLV
> per [RFC8779].  In the literature of the GMPLS stateful PCE, only the
> LABEL-REQUEST TLV is used to specify the switching type, encoding type and
> G-PID of the LSP being instantiated by the PCE. The present of
> LABEL-REQUEST TLV is mandatory.
>
> --
>
[Dhruv]: The use of "and/or" is contradicting is one of them being
mandatory! Could you please clarify?

>
>    - 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.
>
> o   [Haomian] Ok, this is added to the beginning of section 8. The
> statement is “All the error handling specified in section 3 of [RFC8779] is
> applicable and MUST be supported for stateful PCE in GMPLS networks.”
>
>    - Error handling is missing in case of stateful PCEP message for GMPLS
>       LSP, without the capability advertisement by both PCEP peers.
>
> [Haomian] This is a valid observation. I think we can use Error-type 19
> with error-value 2/5/17 in this scenario. (Do we have a scenario using
> 17?)
>
> A new section 8.1 is proposed as follow.
> 8.1     Error Handling in PCEP Capabilities Advertisement
>
> When the PCEP capabilities advertisement failed, the stateful PCE SHOULD
> report the error ‘Invalid Operation’ (Error-type 19). Error-value in this
> case can be one of the following:
>
> - 2 when there is an attempt for LSP Update Request
>
> - 5 when there is an attempt for LSP State Report
>
> - 17 when the Stateful PCE capability was not advertised.
>
>
>

[Dhruv]: This would be incorrect! You need new error codes as those
indicate that the basic stateful flags are not set (and they are not about
the GMPLS). Use this as a reference -
https://datatracker.ietf.org/doc/html/rfc8623#section-9

·      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.
>
> ·       [Haomian] This part is accepted/done.
>
> ·      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.
>
> ·       [Haomian] This part is accepted/done. BTW I don’t see any
> specific security considerations specific to stateful GMPLS PCEP.
>
>
> 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?
>
> ·       [Haomian] restructured with the right referencing (BTW it’s
> RFC8745) in the first paragraph of section 4. Two requirements mentioned
> are 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.
>
> ·       [Haomian] are you suggesting moving all the section 6 RBNF to the
> appendix?
>
[Dhruv] Yes and If you move it to the appendix, then show the objects like
END-POINT otherwise there is no point in reproducing it from RFC 8231.

> 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:
>
> [Haomian] put this suggested text in section 5.2.
>
>
>
>
>    - 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
>
> [Haomian] To confirm, are you suggesting adding all the objects into RBNF?
> Or just describe in the text? It looks we are having difficulties on
> referencing too much. I would suggest we do the following: 1) confirm the
> correctness of RBNF, correct the mistake/incompleteness; 2) Only reference
> to what appears in the RBNF in the text. 3) we can add some sentence if we
> really need to have all related objects, like ‘The typical RBNF objects are
> skipped with only highlighting the changes in GMPLS network. ’
>

[Dhruv]: What I am asking is that since you have text for BANDWIDTH, why
not for other objects - adding just text makes sense to me!

> ·      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 )
>
> ·       [Haomian] No END-POINTS should apply for all stateful PCEP
> messages. I think the description in 7.1 first bullet is correct.
>
>
[Dhruv]: Looking forward to the updated text!

Thanks!
Dhruv

>
>
> ·      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
>
> 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
>