RE: [Pce] Last Call: <draft-ietf-pce-pcep-service-aware-12.txt> (Extensions to the Path Computation Element Communication Protocol (PCEP) to compute service aware Label Switched Path (LSP).) to Proposed Standard

[Dhruv Dhody <[e-mail]>]

Hi Adrian, 

Thanks for your review and especially for providing suggested text.
I have attached the working copy and diff. 


> -----Original Message-----
> From: Pce [mailto:[e-mail]] On Behalf Of Adrian Farrel
> Sent: 30 August 2016 19:44
> To: [e-mail]
> Cc: [e-mail]; [e-mail];
> [e-mail]
> Subject: Re: [Pce] Last Call: <draft-ietf-pce-pcep-service-aware-12.txt>
> (Extensions to the Path Computation Element Communication Protocol (PCEP)
> to compute service aware Label Switched Path (LSP).) to Proposed Standard
> 
> I just read this document for IETF last call.
> 
> I feel a little bad that I didn't review the document in WG last call since
> I normally participate in the PCE WG, but the timing wasn't good.
> 
> The document is both well written and clear.  I support advancing this
> document, but I have a few small comments.
> 
> Thanks,
> Adrian
> 
> ---
> 
> Requirement 4 has
> 
>    4.  A PCE is not required to support service aware path computation.
>        Therefore, it MUST be possible for a PCE to reject a PCReq
>        message with a reason code that indicates service-aware path
>        computation is not supported.
> 
> It may be better to phrase this in terms of legacy systems since (obviously),
> a PCE that does not support this spec must still be able to reject requests.
> Perhaps...
> 
>    4.  A PCE that supports this specification is not required to provide
>        service aware path computation to any PCC at any time.
>        Therefore, it MUST be possible for a PCE to reject a PCReq
>        message with a reason code that indicates service-aware path
>        computation is not supported.  Furthermore, a PCE that does not
>        support this specification will either ignore or reject such
>        requests using pre-existing mechanisms, therefore the requests
>        MUST be identifiable to legacy PCEs and rejections by legacy
>        PCEs MUST be acceptable within this specification.
> 
> This is not a change to the requirement and doesn't change your protocol
> solution.
> 
[Dhruv] Updated!  
> ---
> 
> 4.1.1 has...
> 
>    - A path delay metric for the P2P path P = Sum {D(Lpi), (i=1...K)}.
> 
> Is it worth noting that the path delay metric is not the effective delay
> along the path? That is, delay on the path will also be introduced by delays
> in the nodes along the path. However, information about the delays introduced
> by nodes along the path is not currently made available through the routing
> protocols, and is unlikely to be consistent as network load varies.
> 
> Note that your definition of path delay is good. I am not proposing any change
> to the definition, just and explanation of what it is not.
> 
[Dhruv] Agreed, I can add a note with reference to [RFC7823] for this. 
> ---
> 
> In several places in Section 4 you refer to specific bits (P bit, B bit...).
> I think it would be helpful to understand that these are bits in the flags
> field of the Metric object. Maybe an easy way to deal with this is...
> 
> OLD
>    The METRIC object is defined in section 7.8 of [RFC5440], comprising
>    metric-value, metric-type (T field) and flags.  This document defines
>    the following types for the METRIC object.
> NEW
>    The METRIC object is defined in section 7.8 of [RFC5440], comprising
>    metric-value, metric-type (T field) and a flags field comprising a
>    number of bit-flags (B bit, P bit).  This document defines the
>    following types for the METRIC object.
> END
> 
[Dhruv] Done!
> ---
> 
> I think you need to add (a very small amount of) text to describe how this
> all works for initiated LSPs. Just like Section 5.
> 
[Dhruv] Added! 
> ---
> 
> 4.1.1.1
> 
>    [RFC7471] and [RFC7810] define the "Unidirectional Link Delay Sub-
>    TLV" in a 24-bit field.
> 
> I think this should be...
> 
>    [RFC7471] and [RFC7810] define the "Unidirectional Link Delay Sub-
>    TLV" to report link delay in microseconds in a 24-bit field.
> 
> Similarly in 4.1.2.1
> 
[Dhruv] Done! 
> ---
> 
> 4.1.6 has
> 
>    This section defines the following optional types for the METRIC
>    object for P2MP TE LSPs.
> 
> 4.1 does not say whether the P2P metrics are optional or mandatory.
> I suspect you mean...
> - Implementations of this spec must implement TBD1, TBD2, and TBD3
> - Implementations of this spec that support P2MP (which is an optional
>   feature) must support TBD8, TBD9, and TBD10.
> 
[Dhruv] I will remove the word optional from 4.1.6 to keep it consistent with P2P, and avoid the confusion. 
> ---
> 
> In 4.1.6.1 when you wrote
> 
>    - The P2P path delay metric of the path to destination Dest_j is
>    denoted by LM(Dest_j).
> 
> I wonder whether you were thinking "LM means Link delay Metric"?
> A bit odd that you have actually written "LM means P2P path delay metric"
> 
> Ditto LVM in 4.1.6.2
> 
[Dhruv] Yes that is odd, Updated to PDM and PDVM for path delay metric and path delay variation metric. 
> ---
> 
> 4.2.3.1
> 
>    A PCC SHOULD request the PCE to factor in the bandwidth utilization
>    during path computation by including a BU object in the PCReq
>    message.
> 
> I don't understand the use of "SHOULD" here. I suspect you mean...
> 
>    A PCC that wants the PCE to factor in the bandwidth utilization
>    during path computation includes a BU object in the PCReq message.
> 
> What you have is a statement that unless there is a good reason why not,
> a PCC is supposed to request the PCE to factor in the bandwidth utilization
> during path computation.
> 
[Dhruv] Good catch! 
> ---
> 
> 4.2.3.1
> 
> I found the following clumsy and suggest a change...
> 
> OLD
>    Multiple BU objects MAY be inserted in a PCReq or a PCRep message for
>    a given request but there MUST be at most one instance of the BU
>    object for each type.  If, for a given request, two or more instances
>    of a BU object with the same type are present, only the first
>    instance MUST be considered and other instances MUST be ignored.
> NEW
>    A PCReq or PCRep message MAY contain multiple BU objects so long as
>    each is for a different bandwidth utilization type.  If a message
>    contains more than one BU object with the same bandwidth utilization
>    type, the first MUST be processed by the receiver and subsequent
>    instances MUST be ignored.
> END
> 
[Dhruv] Updated!
> ---
> 
> 4.2.3.1 should not define legacy behavior. A legacy node will not see this
> spec and cannot be influenced by what you write. So...
> 
> OLD
>    If a PCE receives a PCReq message containing a BU object, and the PCE
>    does not understand or support the BU object, and the P bit is clear
>    in the BU object header then the PCE SHOULD simply ignore the BU
>    object.
> 
>    If the PCE does not understand the BU object, and the P bit is set in
>    the BU object header, then the PCE MUST send a PCErr message
>    containing a PCEP-ERROR Object with Error-Type = 3 (Unknown object)
>    and Error-value = 1 (Unrecognized object class) as per [RFC5440].
> NEW
>    The behavior of a PCE that does not understand an object that it
>    receives on PCReq message is defined in [RFC5440] and depends on the
>    setting of the P bit in the object header (P bit clear means ignore
>    the object, P but set means return an "Unknown object" error).
> END
> 
[Dhruv] I have updated it as - 
     If the BU object is unknown/unsupported, the PCE MUST follow
     procedures defined in [RFC5440]. That is, if the P bit is set, the
     PCE sends a PCErr message with error type 3 or 4 (Unknown / Not
     supported object) and error value 1 or 2 (unknown / unsupported
     object class / object type), and the related path computation
     request MUST be discarded.  If the P bit is cleared, the PCE is
     free to ignore the object.
> ---
> 
> 4.2.3.1
> 
> Your use of passive voice may cause confusion. You write...
> 
>    The path computation
>    request MUST then be cancelled.
> 
> Does this mean:
> 1. The PCE must do not more processing on the request and the PCC must
>    not expect any further response
> 2. The PCC is required to issue a message to cancel the request
> 
> 
> There are two cases of this text in this section.
> 
[Dhruv] You are right, I have updated it to - 
     ... and the related path
     computation request MUST be discarded.

> ---
> 
> Section 5 does a nice job for stateful PCE. I think you also need to cover
> PCE initiated LSPs.
> 
[Dhruv] I have tried to handle it. 

> ---
> 
> Section 6 needs a reference to RFC 5511 so that the message formats can be
> understood.

[Dhruv] added.

Thanks again for your comments!
 
Dhruv

> 
> > -----Original Message-----
> > From: Pce [mailto:[e-mail]] On Behalf Of The IESG
> > Sent: 23 August 2016 18:59
> > To: IETF-Announce
> > Cc: [e-mail]; [e-mail]; draft-ietf-pce-pcep-service-
> > [e-mail]
> > Subject: [Pce] Last Call: <draft-ietf-pce-pcep-service-aware-12.txt>
> (Extensions
> > to the Path Computation Element Communication Protocol (PCEP) to
> > compute service aware Label Switched Path (LSP).) to Proposed Standard
> >
> >
> > The IESG has received a request from the Path Computation Element WG
> > (pce) to consider the following document:
> > - 'Extensions to the Path Computation Element Communication Protocol
> >    (PCEP) to compute service aware Label Switched Path (LSP).'
> >   <draft-ietf-pce-pcep-service-aware-12.txt> as Proposed Standard
> >
> > The IESG plans to make a decision in the next few weeks, and solicits
> > final comments on this action. Please send substantive comments to the
> > [e-mail] mailing lists by 2016-09-06. Exceptionally, comments may
> > be sent to [e-mail] instead. In either case, please retain the
> > beginning of the Subject line to allow automated sorting.
> 
> _______________________________________________
> Pce mailing list
> [e-mail]
> https://www.ietf.org/mailman/listinfo/pce