Re: [Pce] PCE WG LC for draft-ietf-pce-pcep-extension-for-pce-controller

["Pengshuping (Peng Shuping)" <[e-mail]>]

Dear Adrian, 

Many thanks for your detailed review and comments. 

Please find the diff with our updates to the draft according to your comments. Hope we have addressed them properly. 

https://tools.ietf.org/tools/rfcdiff/rfcdiff.pyht?url1=draft-ietf-pce-pcep-extension-for-pce-controller-06&url2=https://raw.githubusercontent.com/dhruvdhody/ietf/master/draft-ietf-pce-pcep-extension-for-pce-controller-07.txt

We have also updated Chao Zhou's email address in the draft. 

Thank you!

Best regards, 
Shuping


> -----Original Message-----
> From: Adrian Farrel [mailto:[e-mail]]
> Sent: Monday, August 17, 2020 12:15 AM
> To: [e-mail]; [e-mail]
> Cc: [e-mail]
> Subject: RE: [Pce] PCE WG LC for
> draft-ietf-pce-pcep-extension-for-pce-controller
> 
> Hi Julien, WG, authors,
> 
> I'm listed as one of the eight Contributors to this document, although my
> affiliation should read "Old Dog Consulting".
> 
> I was a co-author of RFC 8283, so I am generally glad to see protocol work
> addressing this space.
> 
> This document is almost ready to progress, but there are quite a few nits
> that we should take care of before asking to advance the document.
> After that's done, I support this document being published as an RFC.
> 
> Thanks,
> Adrian
> 
> ===
> 
> Figures
> 
> If you make the table in Section 5.2 into "Figure 1" then you will not need to
> renumber Figure 2.
> 
> All figures should have a caption such as you have for Figure 2.
> 
> All figures should be mentioned explicitly in the text. Thus, instead of saying
> "as shown below" you should say "as shown in Figure 2".  This is important
> because:
> - it helps the RFC Editor check that you are using all of the figures
> - it protects the text from edits that might move it around in relation
>   to the figure.
> 
> ---
> 
> Abstract
> 
> s/(G)MPLS/MPLS and GMPLS/
> s/PCEP protocol extensions/PCEP extensions/
> 
> ---
> 
> Introduction
> 
> s/draft specify/document specifies/
> 
> ---
> 
> Introduction
> 
>    The extension for PCECC in Segment Routing (SR) is specified in a
>    separate draft [I-D.zhao-pce-pcep-extension-pce-controller-sr].
> 
> This paragraph is, of course, completely true. But it is absolutely unclear
> what it is doing here. There has been no previous discussion of SR in the
> document, and the only other mention of SR is in 5.5.9. I think we can
> probably do without this paragraph.
> 
> ---
> 
> 2.
> 
> OLD
>    Terminologies used in this document is same as described in the draft
>    [RFC8283].
> NEW
>    Terminology used in this document is that same as that described in
>    [RFC8283].
> END
> 
> ---
> 
> 3.
> 
> OLD
>    it is assumed that label range to be used NEW
>    it is assumed that the label range to be used END
> 
> OLD
>    A future extension could add this capability NEW
>    A future extension could add the capability END
> 
> OLD
>    This document also allow a case where the label space is maintained
>    by PCC itself
> NEW
>    This document also allows a case where the label space is maintained
>    by the PCC itself
> END
> 
> ---
> 
> 4.
> 
> OLD
>    Following key requirements associated PCECC should be considered
> when
>    designing the PCECC based solution:
> NEW
>    The following key requirements should be considered when
>    designing the PCECC based solution:
> END
> 
> ---
> 
> 5.3
> s/This document specify/This document specifies/ s//new CCI
> object-type/new CCI object-types/
> 
> ---
> 
> 5.4
> s/During PCEP Initialization Phase/During the PCEP Initialization Phase/
> s/Path is setup/Path is set up/ s/sub-TLV in PCC's/sub-TLV in a PCC's/ s/PCE's
> OPEN message/a PCE's OPEN message/
> 
> ---
> 
> 5.4
> 
>    If the PCEP
>    Speakers support the extensions of this draft but did not advertise
>    this capability attempts a PCECC operation then a PCErr message with
>    Error-Type=19(Invalid Operation) and Error-Value=TBD3 (Attempted
>    PCECC operations when PCECC capability was not advertised) will be
>    generated and the PCEP session will be terminated.
> 
> This is good. But you also need to include what happens if an
> implementation that does not understand these extensions receives one.
> I suspect you can do this with a reference to another document.
> 
> ---
> 
> There are a couple of cases of s/a LSP/an LSP/
> 
> ---
> 
> 5.5.1
> 
> s/based on PCECC mechanism/based on the PCECC mechanism/
> s/LSP-IDENTIFIER TLV MUST/An LSP-IDENTIFIER TLV MUST/ s/with D
> flags/with D flag/ s/and set up the/and sets up the/ s/include the
> central/includes the central/ s/The CC-ID is uniquely identify/The CC-ID
> uniquely identifies/ s/two CCI object/two CCI objects/ s/PCECC LSP is same
> as/PCECC LSP is the same as/
> s/receives PCRpt message/receives a PCRpt message/   (twice)
> s/The PCECC LSP are/The PCECC LSPs are/
> s/In case where/In the case where/
> s/label allocation are/label allocations are/ s/Similarly C bit/Similarly the C
> bit/
> 
> ---
> 
> 5.5.1
> 
> You have:
>    This PCUpd message is as per [RFC8231] SHOULD include the path
>    information as calculated by the PCE.
> 
> I think that you are not defining anything new here. You are just describing
> what 8231 already says. So how about...
> 
>    Per [RFC8231], this PCUpd message should include the path
>    information calculated by the PCE.
> 
> ---
> 
> 5.5.1
> 
>    The Ingress MAY further choose to deploy a
>    data plane check mechanism and report the status back to the PCE via
>    PCRpt message.
> 
> This is fine, but you should probably add some explanation of why an Ingress
> might choose to do this.
> 
> ---
> 
> 5.5.1
> 
>    It should be noted that in this example, the request is made to the
>    egress node with C bit set in the CCI object to indicate that the
>    label allocation needs to be done by the egress and it responds with
>    the allocated label to the PCE.
> 
> This is a little ambiguous. "...it responds" - what is it?
> Perhaps you should have:
> "...done by the egress, and the egress responds..."
> 
> ---
> 
> 5.5.2.1
> 
> s/to setup an LSP based/to set up an LSP based/
> s/included in LSP object/included in the LSP object/   (twice)
> s/New PCEP object/A new PCEP object/
> 
> ---
> 
> 5.5.2.2
> 
>    If the PCC receives a PCInitiate message but does not recognize the
>    label in the CCI, the PCC MUST generate a PCErr message with Error-
>    Type 19(Invalid operation) and Error-Value=TBD8, "Unknown Label" and
>    MUST include the SRP object to specify the error is for the
>    corresponding label cleanup (via PCInitiate message).
> 
> This is OK as a choice, but I wonder why you need to report this as an error.
> The intention of the request is to make sure that the label in the CCI is
> removed from the PCC. If the label is not recognised, isn't this the right result
> meaning that the answer should be a positive response?
> 
> I'm not saying you must change this, but please consider it.
> 
> ---
> 
> 5.5.3
> 
> s/In order to setup/In order to set up/
> 
> OLD
>    The
>    PCC responds with first PCRpt message with the status as "GOING-UP"
>    and assigned PLSP-ID.
> NEW
>    The
>    PCC responds with a PCRpt message with the status set to "GOING-UP"
>    and carrying the assigned PLSP-ID.
> END
> 
> s/from PCECC are send/from PCECC are sent/ s/setup operations are/setup
> operations are the/ s/LSP is same as/LSP is the same as/ s/the PCE SHOULD
> send the/the PCE SHOULD send a/ s/In case where/In the case where/
> s/label allocation are made/label allocations are made/
> 
> ---
> 
> 5.5.3
> 
>    In case where the label allocation are made by the PCC itself (see
>    Section 5.5.8), the procedure remains similar.
> 
> Do you mean "remains similar" or "is the same"?
> If it is only similar then you need to describe how it is different.
> 
> ---
> 
> 5.5.4                                                  `
> s/modification of PCECC/modification of a PCECC/ s/In case where/In the
> case where/ s/label allocation are made/label allocations are made/
> 
> ---
> 
> 5.5.4
> 
>    In case where the label allocation are made by the PCC itself (see
>    Section 5.5.8), the procedure remains similar.
> 
> Same question as 5.5.3
> 
> ---
> 
> 5.5.5
> 
> s/control over the/control over an/
> s/In case of/In the case of a/
> 
> ---
> 
> 5.5.9
> 
> s/the PCE via PCRpt message as per/the PCE via a PCRpt message as per/
> 
> ---
> 
> 5.5.9
> 
>    The PCECC capability MUST be exchanged on the PCEP session, before
>    PCE could allocate binding label.
> 
> Maybe turn this around to read...
> 
>    Before a PCE can allocate a binding label the PCECC capability MUST
>    be exchanged on the PCEP session.
> 
> ---
> 
> 6.1
> 
> OLD
>    the message has been extended as shown below NEW
>    this document extends the message as shown below END
> 
> ---
> 
> Either in 6.1 or in section 2, you should include something like
> 
>    The message formats in this document are specified using Routing
>    Backus-Naur Form (RBNF) encoding as specified in [RFC5511].
> 
> And obviously add a reference to 5511.
> 
> ---
> 
> 6.1
> 
> c/At max two instances/At most two instances/
> 
> ---
> 
> 7.1
> 
> s/defines a new optional TLVs/defines new optional TLVs/
> 
> ---
> 
> 7.1.1
> 
> OLD
>    No flags are assigned right now.
> NEW
>    No flags are defined in this document.
> END
> 
> ---
> 
> 7.3
> 
> OLD
>    Currently, the following flag bit is defined:
> NEW
>    Currently, the following flag bits are defined:
> END
> 
> s/A PCE set this bit/A PCE sets this bit/
> 
> ---
> 
> 11.3
> 
> Can you add to the definition of this registry what the size is? I think there
> are 32 bits available.
> 
> ---
> 
> 11.4
> 
> I think the name of this section is wrong. you are not creating a new registry
> in this section.
> 
> ---
> 
> 11.6
> 
> You should say how many bits this registry can track. I think it is 16.
> 
> ---
> 
> 11.7
> 
> It's not too important, but it might be nice to order this list by increasing
> error type.
> 
> -----Original Message-----
> From: Pce <[e-mail]> On Behalf Of [e-mail]
> Sent: 05 August 2020 17:19
> To: [e-mail]
> Subject: [Pce] PCE WG LC for
> draft-ietf-pce-pcep-extension-for-pce-controller
> 
> Hi all,
> 
> This message initiates a 3-week WG Last Call on
> draft-ietf-pce-pcep-extension-for-pce-controller-06. Please review and
> share your feedback, whatever it is, using the PCE mailing list.
> This LC will end on Wednesday August 26, 11:59pm (any timezone).
> 
> Please note that this I-D is related to
> draft-zhao-pce-pcep-extension-pce-controller-sr which is already in our WG
> adoption queue.
> 
> Thanks,
> 
> Dhruv & Julien
> 
> 
> ______________________________________________________________
> ______________
> _____________________________________________
> 
> Ce message et ses pieces jointes peuvent contenir des informations
> confidentielles ou privilegiees et ne doivent donc pas etre diffuses, exploites
> ou copies sans autorisation. Si vous avez recu ce message par erreur, veuillez
> le signaler a l'expediteur et le detruire ainsi que les pieces jointes. Les
> messages electroniques etant susceptibles d'alteration, Orange decline toute
> responsabilite si ce message a ete altere, deforme ou falsifie. Merci.
> 
> This message and its attachments may contain confidential or privileged
> information that may be protected by law; they should not be distributed,
> used or copied without authorisation.
> If you have received this email in error, please notify the sender and delete
> this message and its attachments.
> As emails may be altered, Orange is not liable for messages that have been
> modified, changed or falsified.
> Thank you.
> 
> _______________________________________________
> Pce mailing list
> [e-mail]
> https://www.ietf.org/mailman/listinfo/pce