Hi Adrian

Thanks for these comments

Please see inline 

> -----Message d'origine-----
> De : [EMAIL PROTECTED] [mailto:[EMAIL PROTECTED] De la 
> part de Adrian Farrel
> Envoyé : mardi 1 avril 2008 15:45
> À : [email protected]
> Objet : [Pce] Quesitons about OF draft
> 
> Hi,
> 
> A bunch of nits and a couple of technical question.
> 
> Cheers,
> Adrian
> 
> Please run idnits (http://tools.ietf.org/tools/idnits/) and 
> clean up any issues shown.

OK will be done

> ===
> Abstract
> s/series/set/
> s/criteria(s)/criteria/
> s/may support one or multiple/may support one or more/ 

>=== Abstract
>    ...it is desired for a Path Computation Client (PCC) to
>    automatically discover the set of objective functions 
> supported by a
>    PCE. Furthermore, it may be useful for a PCC to specify in a path
>    computation request the required objective function to be used...
> I think you might present the requirements in a slightly 
> different order. For example:
>    A Path Computation Client (PCC) may want a path to be computed for
>    one or more TE LSPs according to a specific objective 
> function. Thus,
>    the PCC needs to instruct the PCE to use the correct objective
>    function. Furthermore, it is possible that not all PCEs support the
>    same set of objective functions, therefore it is useful for the PCC
>    to be able to automatically discover the set of objective functions
>    supported by each PCE.

OK

> ===
> Abstract
>    Thus the aim of this
>    document is to define extensions to the PCE communication Protocol
>    (PCEP) in order to allow a PCC to discover the set of objective
>    functions supported by a PCE as well as to allow a PCC to 
> indicate in
>    a path computation request the required objective function 
> and a PCE
>    to indicate in a path computation reply the objective function that
>    was used for path computation.
> Can we break this up to make it a bit easier to read and 
> start a new paragraph? For example:
>    This document defines extensions to the PCE communication Protocol
>    (PCEP) to allow a PCE to indicate the set of objective functions it
>    supports. Extensions are also defined so that a PCC can indicate in
>    a path computation request the required objective function, and so
>    that a PCE can report in a path computation reply the objective
>    function that was used for path computation.

OK

> ===
>  1. Terminology
> The indentation of all of the section headers is broken.
> You need to have the Introduction as section 1. This means 
> more than just re-ordering the sections as any acronyms in 
> the Introduction will need to be expanded.

OK

> ===
> 1. Terminology
> s/criteria(s)/criteria/
> ===
> 2. Introduction
> s/A PCE serves/A PCE services/
> s/criteria(s)/criteria/
> s/candidate path(s)/candidate paths/
> s/non synchronized/non-synchronized/
> s/It defines PCEP extensions allowing a PCE advertising a 
> list/It defines PCEP extensions allowing a PCE to advertise a 
> list/ s/extensions so as to carry/extensions to carry/ s/It 
> thus complements/It complements/ s/parameters should rather 
> be discovered/parameters should be discovered/ s/Objective 
> functions pertain to this second/Objective functions are in 
> this second/ s/section 3/Section 3/ s/the list of objective 
> functions that it supports/to list the objective functions 
> that it supports/ s/section 4/Section 4/ s/applied by a PCE 
> or in a PCRep/applied by a PCE, or in a PCRep/ === 3.1. 
> OF-List TLV s/The OSPF OF-List TLV/The PCEP OF-List TLV/  (!) 
> In the figure, indent the bit numbering by one more space.
> In your figure, I suggst you label the final two bytes "padding".
> c/(see IANA section)/(see Section 6)/
> ===

OK

> 3.2. Elements of procedure
> s/A PCE MAY include and OF-List/A PCE MAY include an OF-List/
> 
>    The OF-List TLV MUST NOT appear more than once
>    in an OPEN object.
> Say what happens if more than one is present. I assume you 
> class this as an invalid Open message and would reject the 
> PCEP session establishment.

Yes in that case the session must be rejected. This will be mentionned.

> 
> s/The absence of the OF-List TLV in an OPEN object must/The 
> absence of the OF-List TLV in an OPEN object MUST/ === 4. 
> Objective Function in PCEP Path Computation request and reply
>    messages
> 
> Please capitalise the section heading.
> 
> At the end of section 4.2 you say that the OF object must not 
> be used in association with the No-Path object. I have 
> comments about that later, but if that remains the case, you 
> should mention it in this section.
> 
> OLD
>    A new flag is defined in the RP object, so as to indicate 
> in a PCReq
>    message that the PCE MUST provide in the PCRep message the 
> objective
>    function that was used during path computation.
> NEW
>    A new flag is defined in the RP object. The flag is used in a PCReq
>    message to indicate that the PCE MUST include an OF object in the
>    PCRep message to indicate the objective function that was 
> used during
>    path computation.

OK

> 
> s/Also new//Also, new/
> s/error type and value//error types and values/ === 4.1. OF 
> Object s/The OF Object-Types/The OF Object-Type/
> 
> Indent the bit number in the figure.
> 
> s/(see IANA section)/(see Section 6) /
> 
>    Optional TLVs may be defined so as to encode objective function
>    parameters.
> Perhaps say "in the future"?

OK

> ===
> 4.1.1. Elements of Procedure
> 
>    To request the use of a specific objective function to be 
> used by the
>    PCE a PCC MUST include an OF object in the PCReq message.
> This use of RFC2119 "MUST" is likely (on the basis of recent 
> experience) to cause confusion during IESG review. It is 
> actually a conditional "MUST", but could be misinterpretted. 
> I think you can safely rephrase as...
>    To request the use of a specific objective function by the 
> PCE, a PCC
>    includes an OF object in the PCReq message.

OK

> 
> OLD
>    [PCEP] specifies a bit flag referred to as the P bit in a PCEP
>    common header that can be set by a PCC to enforce a PCE to 
> take into
>    account the related information during the path computation.
> NEW
>    [PCEP] specifies a bit flag, referred to as the P bit, 
> carried in the
>    common PCEP object header. The P bit is set by a PCC to 
> mandate that
>    a PCE must take the information carried in the object into account
>    during the path computation.
> 
> 
>    If the
>    objective function is mandatory (required objective 
> function), the P
>    bit in the OF object MUST be set, else if it is optional (desired
>    objective function) the P bit MUST be cleared.
> I would prefer to see this text reversed so that we describe 
> the behavior of the PCE if the bit is set/clear. As you have 
> it at the moment you are mandating the PCC behavior based on 
> its desires. Either you should not use RFC 2119 language, or 
> you should re-word according to how you want the PCE to 
> behave. How about:
>    If the P bit is set in the OF object, the objective function is
>    mandatory (required objective function) and the PCE MUST use the
>    objective function during path computation. If the P bit 
> is clear in
>    the OF object, the objective function is optional (desired 
> objective
>    function) and the PEC SHOULD apply the function if it is supported,
>    but MAY choose to apply a different objective function according to
>    local capablities and policy.

OK

> 
> 
>         - If the OF object is unknown/unsupported, the PCE MUST follow
>           procedures defined in [PCEP], that is if the P bit 
> is set, it
>           sends a PCErr message with error type unknown/unsupported
>           object (type 3 and 4) and the related path 
> computation request
>           MUST be discarded.
> You need to describe the error-value to use.

OK

> 
>         - If the objective function is unknown / unsupported and the P
>           bit is set, the PCE MUST send a PCErr message with 
> a new PCEP
>           error type "objective function error" and error value
>           "unknown/unsupported objective function" (defined in this
>           document), and the related path computation request MUST be
>           discarded.
> My understanding of our discussion about the PCEP spec was 
> that you intended to have generic error-types and 
> error-values. This would be a case for using Error-type 3/4: 
> Unknown/Not supported Object and Error-
> value=4: Unrecognized/Unsupported parameter.

OK

> 
> ===
> 4.2. Carrying the OF object in a PCEP message
> 
> Please capitalise the section heading.
> 
>    The OF object MAY be carried within a PCReq message. An OF object
>    specifying an objective function that applies to a set of
>    synchronized path computation requests MUST be carried 
> just after the
>    corresponding SVEC object
> Again, there is a conditional "MUST" here. You need to say, 
> "If an objective function is to be applied to a set... .. the 
> OF object MUST be carried..."

OK

> 
> 
>    An OF object specifying an objective function that applies to an
>    individual path computation request (non synchronized case) MUST
>    follow the RP object for which it applies.
> 1. Your BNF shows it following, but not immediately following. I guess
>    that this is exactly what you meant to write.
>    Wouldn't it be better to put <OF> next to <metric-list> ?

I agree, this will be done

> 2. I don't like the fact that the OF is sometimes here, 
> sometimes there.
>    You need to facilitate several different cases.
>    a. Message contains just one request with an OF to apply
>    b. Message contains several unsynchronized requests each with an OF
>    c. Message contains several synchronized requests with an 
> OF to apply
>       to the set of computations
>    d. Message contains several synchronized requests with an 
> OF to apply
>       to the set of computations, and the message contains one or more
>       unsynchronized requests each with an OF to apply.
>    It seems to me that your handling of the synchronized requests is a
>    problem because it appears that you can have a separate OF for each
>    request in the set, but have no way to say the OF that 
> applies to the
>    whole set.

Not really.

>    So, I think you need...
>      <PCReq Message>::= <Common Header>
>                          [ [<OF>] <SVEC-list>]
>                          <request-list>

No. Recall that an SVEC comprises a set of synchronized requests.
A SVEC list is a list of SVEC, that is a list of sets of synchronized requests.
We need to be able to specify an OF for each SVEC...
So I think our proposed encoding is the right one.


>       where:
>          <svec-list>::=<SVEC>
>                        [<svec-list>]
> 
>          <request-list>::=<request>[<request-list>]
> 
>          <request>::= <RP>
>                       <END-POINTS>
>                       [<LSPA>]
>                       [<BANDWIDTH>]
>                       [<OF>]
>                       [<metric-list>]
>                       [<RRO>]
>                       [<IRO>]
>                       [<LOAD-BALANCING>]
> 
>    Further, this allows that you to have an OF per requests, and an
>    additional OF to cover the set of synchronized requests.

This is already what we have.

> 
> 
> Now, you also have...
>       <metric-list>::=<METRIC>[<metric-list>]
> This is lifted from [PCEP] and is fine. But don't you also 
> need a metric list for the synchronized OF?
> This would yield...
>      <PCReq Message>::= <Common Header>
>                          [ [<OF>] [<metric-list>] <SVEC-list>]
>                          <request-list>

Right, this will be added

> 
> 
> 
>    When the PCE wants to indicate to the PCC the objective 
> function that
>    was used for the synchronized computation of a set of paths, the
>    PCRep message MUST include the corresponding SVEC object directly
>    followed by the OF object
> Again, the conditional use of "MUST" is confusing. Since the 
> previous paragraph includes "MAY", I think you can use:
> s/PCRep message MUST include/PCRep message includes/
> 
> 
>    The format of the PCRep message is updated as follows:
> Same issues as for the PCReq.
> 
> And similarly, aren't some of the metrics you need to report 
> applicable to the set rather than the individual path? For 
> example, minimize the cost of the set of paths - report the 
> cost of the set.

OK will be added

> 
> s/<SVEC-list>/<svec-list>/
> 
> 
>     Note: The OF object MUST NOT be associated to a negative 
> reply, i.e.
>     a reply with a NO-PATH object.
> I can see that you have other ways of conveying a negative 
> response related to the OF (unknown, unsupported), 

If the OF is not supported this is not a negative response, this is an error.

> or the 
> specific issues resulting from trying to use the OF (metric, etc.).
> Now, suppose the request desires (not requires) an OF, and 
> the response says No-Path. Shouldn't the response also say 
> which OF was used to produce this result?

Strictly speaking the reason for no path is never an objective function, the 
reason is a constraint.

> 
> ===
> 4.3. New RP object flag
> 
> Please capitalise the section heading.
> 
> s/does not follow the recommendation/does not follow the request/
> 
>    Objective Function (OF) flag (1 bit): 0x200 (bit number 
> 16) Currently, PCEP is numbering these bits form the other 
> ned, so this is bit 10.
> 
> s/this indicates that the PCE has to/this indicates that the PCE MUST/
> 
> ===
> 4.3.1. Elements of procedure
> 
> Please capitalise the section heading.
> 
> s/it MUST set/it sets/
> s/the PCE has to proceed as follows/the PCE proceeds as follows/
> 
> ===
> 5. Objective Functions definition
> 
> Please capitalise the section heading.
> 
> s/be the IGP metric the TE metric or any/be the IGP metric, 
> the TE metric, or any/
> 
> ===
> 6.1. PCE Objective Function registry
> s/registry/Sub-Registry/
> 
>    The guidelines (using terms defined in [RFC2434]) for the
>    assignment of objective function code point values are as follows:
> 
> /Function code value in the range 1-32767/Function code 
> values in the range 1-32767/
> 
> ===
> 6.2. PCEP code points
> 
> Please capitalise the section heading.
> ===
> 6.2.3. PCEP Error values
> 
> Please capitalise the section heading.
> 
> See comment for section 4.1.1
> ===
> 6.2.4. RP Object flag
> 
> Please capitalise the section heading.
> ===
> 7.                    Security Considerations
> 
> Ecessive tabs!
> ===
> 8.2. Information and Data Models
> 
>    The PCEP MIB Module defined in [PCEP-MIB] MUST be extended 
> to include
>    Objective Functions.
> MUST or SHOULD?

SHOULD

> ===
> 8.5. Requirements on other protocols
> 
> Please capitalise the section heading.
> ===
> 8.6. Impact on network operations
> 
> Please capitalise the section heading.
> ===
> 10.1. Normative references
> 
> Please capitalise the section heading.
> 
>    [RFC2740] Coltun, R., Ferguson, D., and J. Moy, "OSPF for IPv6",
>    RFC 2740, December 1999.
> Unused reference
> 
>    [RFC3630] Katz, D., Yeung, D., Kompella, K., "Traffic Engineering
>    Extensions to OSPF Version 2", RFC 3630, September 2003.
> Unused reference
> 
>    [RFC3784] Li, T., Smit, H., "IS-IS extensions for Traffic
>    Engineering", RFC 3784, June 2004.
> Unused reference
> ===
> 10.2. Informative references
> 
> Please capitalise the section heading.
> 
>    RFC4674, October 2006.
> 
> ===
> 11. Author's Addresses:
> s/Author's Addresses:Authors' Addresses/ ===

Thanks a lot Adrian for this detailed review

We will submit soon a new version accounting for these comments.

Regards,

JL


> 
> 
> _______________________________________________
> Pce mailing list
> [email protected]
> https://www.ietf.org/mailman/listinfo/pce
> 
_______________________________________________
Pce mailing list
[email protected]
https://www.ietf.org/mailman/listinfo/pce

Reply via email to