Hi JP,

Thanks for the work.

> Thanks for the detailed review. We'll soon propose a text for the
> Manageability and Security sections. In the meantime, let me know
> if this addresses all your comments/suggestions.

>> 2.  Introduction
>>
>>    This document specifies procedures and extensions to the Path
>>    Computation Element Protocol (PCEP) ([I-D.ietf-pce-pcep]) in
>>    order to monitor the path computation chain and gather various
>>    performance metrics.
>>
>> I think you need to introduce the concept of a "path computation
>> chain" before talking about monitoring it. I would simply move this
>> paragraph to after the next paragraph, but even then the term is not
>> clearly explained. There is a good paragraph in section 3.1.
>
> Yes, I added the following paragraph:
>
>  As defined in [RFC4655], there are circumstances where more than one
>  PCE is involved in the computation of a TE LSP.  A typical example is
>  when the PCC requires the computation of a TE LSP where the head-end
>  and the tail-end of the TE LSP do not reside in adjacent domains and
>  there is no single PCE with the visibility of both the head-end and
>  tail-end domain.  We call path computation chain the set of PCEs
>  involved in the computation of a TE LSP.  As further discussion in
>  Section 3.1, the PCE chain may either be static (pre-configured) or
>  dynamically determined during the path computation process.

OLD
We call path computation chain the set of PCEs
involved in the computation of a TE LSP.  As further discussion in
NEW
We call the set of PCEs involved in the computation of a TE LSP a
"path computation chain".  As further discussed in

>> 3.1.  Path Computation Monitoring Request message (PCMonReq)
>>
>> You need to mention how this message will be handled by a "legacy"
>> PCE that does not recognise the message. Also how it would be
>> handled by a PCC.
>
> Suggestion:
>
> In PCEP,
>
> Add: "A PCEP implementation that received an unrecognized PCEP
> message MUST send a PCErr message with Error-value=2 (capability
> not supported).
>
> Then add a new Reasons value in the CLOSE object (still in [PCEP]):
>
> Reasons
>    Value        Meaning
>    5          Reception of an unacceptable number of
>                 unrecognized PCEP messages
>
> And in PCE-Monitoring: "As specified in [PCEP], a PCEP peer
> receiving an unrecognized PCEP message must send a PCErr
> message with Error-value=2 (capability not supported)."

I'm nearly OK with this :-)

You will also need to add (to [PCEP]) some text to describe use of the Close 
object with the new Reason value. This is mainly to point out that it is 
available, but also to help define "unacceptable number".

I guess there is still time to squeeze this into [PCEP] while we address 
IESG issues.

>> 3.2.  Path Monitoring Reply message (PCMonRep)
>>
>>    If the Monitoring
>>    object is missing, the receiving PCE MUST send an error message to
>>    the requesting PCC.
>>
>> Which error code?
>
> Oops, good catch thanks. New error-value added:
>
> Also reported in section 3.1

Good.

> Fixed to a 8-bit flag field.

OK

>>    The P bit MUST always be set in a PCMonRep message if also
>>    set in the corresponding PCMonReq message.
>>
>> No objection, but why? Doesn't the PROC-TIME object convey the same
>> information?
>
> Yes, we could also just ignore the value of the P bit of the MONITORING
> object in a PCMonRep or PCRep messages.

OK. Just write some text.

>> I note that the L and G bits are not defined in the PCMonRep.
>
> Right. It might be actually simpler to just ignore the values of the L, G, 
> P
> and C (not I of course) in PCMonRep and PCRep messages. Agree ?

OK

>> Question...
>>
>>    Monitoring-id-number (32 bits).  The monitoring-id-number value
>>    combined with the source IP address of the PCC and the PCE address
>>    uniquely identify the monitoring request context.
>>    "the PCE address"
>>
>> The request is not specifically targeted at a PCE, or may include a list 
>> of
>> PCEs.
>>
>> How does this work? Isn't the PCC ID and the monitoring-id-number
>> sufficient?
>
> You're quite right.
>
>> Later you say that the same monitoring-id-number can be used for requests
>> sent to different PCEs, but this seems to be ambiguous for chains of 
>> PCEs.
>> And you say "The path computation monitoring reply is unambiguously
>> identified by the IP source address of the replying PCE" but the replying
>> PCE might not be known by the source PCC.
>
> Agree. There are two ways to address this but I would propose just the 
> easy
> one: impose to the PCC to always increment the monitoring-id-number
> (otherwise PCE along the chain would have to check the PCE chain along 
> with
> the PCC ID to disambiguate the request) and indicate that the
> {monitoring-id-number,PCC ID} uniquely identifies the request.

Works for me with the work on "incrementing", below.

>> Question
>>    The monitoring-id-number MUST be incremented each time a new
>>    monitoring is sent to a PCE.
>>
>> Same problem as was raised by the IESG for PCEP. The requirement
>> is just that this number allows disambiguation of active 
>> requests/responses.
>> Why do you require "increment" and by what value should the increment
>> be made?
>
> Let's adopt a similar approach as for the SID:
>
> OLD:
> Monitoring-id-number (32 bits). The monitoring-id-number value combined
> with the source IP address of the PCC and the PCE address uniquely
> identify the monitoring request context. The monitoring-id-number MUST
> be incremented each time a new monitoring is sent to a PCE. The value
> 0x0000000 is considered as invalid. If no reply to a monitoring request is
> received from the PCE, and the PCC wishes to resend its path computation
> monitoring request, the same monitoring-id-number MUST be used.
> Conversely, different monitoring-id-number MUST be used for different
> requests sent to a PCE. The same monitoring-id-number may be used for
> path computation monitoring requests sent to different PCEs. The path
> computation monitoring reply is unambiguously identified by the IP source
> address of the replying PCE.
>
> NEW:
> Monitoring-id-number (32 bits): The monitoring-id-number value combined
> with the PCEP-ID of the PCC  identifies the monitoring request context. 
> The
> monitoring-id-number MUST be incremented each time a new monitoring is
> sent to a PCE. Each increment SHOULD have a value of 1 and may cause
> a wrap back to zero. The value 0x0000000 is considered as invalid. If no
> reply to a monitoring request is received from the PCE, and the PCC wishes
> to resend its path computation monitoring request, the same monitoring-id-
> number MUST be used. Conversely, a different monitoring-id-number
> MUST be used for different requests sent to a PCE. The path computation
> monitoring reply is unambiguously identified by the monitoring-id-number 
> and
> the PCEP-ID of the replying PCE.

If 0x0000000 is invalid, I suggest:
- you say so up front so that we can see to start at a non-zero value
- you say wrap to 1 (not to zero!)

Do we need to worry about PCC restart? On restart, a monitoring I-D might 
get re-used.

>>    No optional TLVs are currently defined.
>>
>> So, why do we have TLVs? Would it be better to put some of the
>> response objects into this object as TLVs? E.g., PROC-TIME,
>> CONGESTION, etc.
>
> That's another option but does not buy that much. The idea was to
> potentially allow for TLVs to be carried within the MONITORING
> object to express more complex monitoring requests.

OK

>> 4.2.  PCE-ID Object
>
> Actually, let's call PCEP-ID since it could also be used to identify a 
> PCC.

OK

>>  A set of PCE-ID objects may be inserted within a PCReq or a PCMonReq
>>  message to specify the PCE for which PCE state metrics are requested
>>  and in a PCMonRep or a PCRep message to record the IP address of the
>>  PCE reporting PCE state metrics or that was involved in the path
>>  computation chain.
>>
>> So...
>>
>> If I send a PCReq with a list of PCE-IDs because these are the PCEs I 
>> want to
>> process the request, can I control the list of PCEs that supply the 
>> monitoring
>> information? For example, I way want the processing time form just the 
>> last
>> PCE of a chain of PCEs.
>
> Nope ... We would use the same list to indicate the PCE chain and list of
> PCE for which we need monitoring data. We could have two list here, one of
> path computation, one for monitoring request but this would increase the
> complexity. Looks much simpler to me when the PCC wants to impose a PCE
> chain and requires monitoring data to use the same list.
>
> OK ?

OK

>> A PCE MUST use the same IP address as the address used in the PCE-
>> ADDRESS sub-TLV defined in [RFC5088] and [RFC5089] should a dynamic
>> discovery mechanism be used for PCE discovery.
>
>> Should that read "A PCC MUST ..."? After all, the PCE has no control over
>> the PCE-ID supplied in a request.
>
> Ah well, it is both in fact. The PCC MUST use the same IP address as the
> one used by the PCE to advertise itself and the PCE must also use this
> address in its replies.
>
> NEW Text:
>
>  When a dynamic discovery mechanism is used for PCE discovery, a PCE
>  advertises its PCE address in the PCE-ADDRESS sub-TLV defined in
>  [RFC5088] and [RFC5089].  A PCE MUST use this address in PCReq and
>  PCMonReq messages and a PCE MUST also use this address in PCRep and
>  PCMonRep messages.

Yes, but typo...

s/A PCE/A PCC/

>> 4.3.  PROC-TIME Object
>>
>>    The PROC-TIME object MUST be present within a PCMonRep or a PCRep
>>    message if the P bit of the MONITORING object carried within the
>>    corresponding PCMonReq or PCReq message is set.
>>
>> "MUST"?
>>
>> Compare with the I bit.
>>
>> Consider local policy that says "I am not going to answer that"
>
> Yes it should read: "If allowed by policy, the PCE inclused a PROC-TIME
> object is within a PCMonRep or a PCRep message if the P bit of the
> MONITORING object carried within the corresponding PCMonReq or
> PCReq message is set."

OK

>> Wouldn't it be more sensible to move the E bit into the Flags field so 
>> that
>> all time fields are the same format?
>
> Good suggestion indeed, bit moved.

OK

>> 4.4.  CONGESTION Object
>>
>>    C (Congestion) - 1 bit: when set, this indicates that PCE is
>>    congested, in which case the congestion duration may be non nul.
>>    When cleared this indicates that the PCE is not congested.
>>    Congestion duration - 16 bits: This field indicates in seconds the
>>    estimated congestion duration.
>>
>> Confused! What is the meaning of C-bit set, Congestion Duration = 0?
>
> OLD:
> C (Congestion) - 1 bit: when set, this indicates that PCE is congested, in
> which case the congestion duration may be non nul. When cleared this
> indicates that the PCE is not congested. Congestion duration - 16 bits: 
> This
> field indicates in seconds the estimated congestion duration.
>
> NEW:
> C (Congestion) - 1 bit: when set, this indicates that PCE is congested, in
> which case the congestion duration may be non nul. When cleared this
> indicates that the PCE is not congested and the "Congestion Duration" 
> field
> MUST be set to 0x0000. Congestion duration - 16 bits: This field indicates
> in seconds the estimated congestion duration.

OK

>> 4.5.  TIMESTAMP Object
>>
>>   A TIMESTAMP object will be specified in a further revision of this
>>   document
>>
>> Will you do this before WG last call, or will you delete this section?
>
> It has been deleted.

OK

>> 5.  Multi-destination monitoring
>>
>>   In a further revision of this document, a new object will be
>>   specified allowing a PCC or a user to gather PCE state metrics for a
>>   set of destinations using a single PCMonReq message.
>>
>> Need to resolve this TBD.
>
> Yes this was a placeholder for a potential addition, which may be useful 
> for
> a "targeted subnet" or more likely P2MP. That said, it is probably better 
> at
> this stage to stabilize the document and work on a potential P2MP 
> extensions
> once the P2MP PCEP ID will have evolved.

OK. I assume deleted.

>> 7.  Elements of procedure
>>
>> Wait a moment. You are defining a new error code to handle non-support
>> of a new feature. Surely you should use an existing error code as see in 
>> the
>> PCEP spec. For example, error-type=2
>
> Yes, this has been addressed in the earlier in this email. We should 
> delete
> that new Error-type here.

OK

Cheers,
Adrian 

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

Reply via email to