Subject: Re: [Pce] WG Last Call on draft-ietf-pce-stateful-pce-app-02 and draft-ietf-pce-stateful-pce-09

From: Cyril Margaria <[email-masked]>
Date: Mon, 10 Nov 2014 18:53:05 -1000

Hi,

For the error handling on PCUpd, and instantiation draft, I think it would
be usefull to align the error procedures and capabilities.
in draft-ietf-pce-stateful-pce-10 the error is returned via PCRpt,
for the same kind of error its returned using a PCErr in
draft-ietf-pce-pce-initiated-lsp-02 ( error-type 24 (LSP instantiation
error) , error-value =1 (Unacceptable instantiation parameter) )

It would mean moving the definition  from instantion to stateful and
changing the type of error for one specific error type, this is not a too
big change,

BR
Cyril

On 27 October 2014 03:57, Cyril Margaria <[email-masked]> wrote:

> Hi Ina,
>
> Thanks, see inline for the open points.
>
> On 27 October 2014 01:57, Ina Minei <[email-masked]> wrote:
>
>> Thank you for the careful review, please see inline ###.
>>
>> [snip]
>>> I Have the following comment for draft-ietf-pce-stateful-pce-09:
>>>
>>> Section 2
>>> The document references the following timers:
>>>    - State Timeout Interval
>>>    - Redelegation Timeout Interval
>>>
>>> RFC5440 defines an Appendix B. PCEP Variables, having a dedicated section
>>> for those variables would be better, as they are integral part of the
>>> extensions.
>>>
>>
>> ### They are discussed in the main part of the draft, their use etc is
>> introduced as early as page 4 of the doc. The suggested values for these
>> timers are added in the operational section in the appendix, where they
>> logically belong and I don't think we want to move them.
>>
>
> I think it will be easier for YANG/MIB module designers to have a small
> appendix for those, with recommended values repeated there. The rest of the
> document does not need to  be changed in that regards.
>
>
>
>>
>>>
>>> Section 5.4
>>>
>>> After the first paragraph, add:
>>> The State synchronization start with a LSP state report having the SYNC
>>> Flag in the LSP Object set to 1.
>>>
>>> Reason: This would allow for the PCC to fully resend its database after
>>> the Initialization phase, and clarify the PCE operation.
>>>
>>
>> ### This is covered in the current text and also clearly shown in figure
>> 1. .
>>
>
> It is implicit in the text,, I think making it explicit would be better
> for implementations.
>
>
>>
>>>
>>> Section 5.6.2
>>> OLD
>>> If the PCC decides that the LSP parameters proposed in the PCUpd message
>>> are unacceptable, it MUST report this error by including the
>>> LSP-ERROR-CODE TLV (Section 7.3.3
>>> <
>>> https://tools.ietf.org/html/draft-ietf-pce-stateful-pce-09#section-7.3.3
>>> >)
>>> with LSP error-value=³Unacceptable parameters" in the LSP object in the
>>> PCRpt message to the PCE
>>>
>>> NEW
>>> If the PCC decides that the LSP parameters proposed in the PCUpd message
>>> are unacceptable, it MUST report this error by including the
>>> LSP-ERROR-CODE TLV (Section 7.3.3
>>> <
>>> https://tools.ietf.org/html/draft-ietf-pce-stateful-pce-09#section-7.3.3
>>> >)
>>> with LSP error-value=³Unacceptable parameters" in the LSP object in the
>>> PCRpt message to the PCE. The PCC MAY/SHOULD include the objects that
>>> were
>>> not accepted in the PCRpt message.
>>>
>>>
>>> Reason: If the PCC includes the objects (current PCC values) that caused
>>> the PCUpd to be rejected, it would help the PCE avoid resending them. A
>>> PCErr would allow to include the objects, a new error type would be
>>> needed
>>> but error handling from PCE side should be rather easy. Another
>>> possibility is having the LSP-ERROR-CODE containing a list of
>>> Object-Class, OT .
>>>
>>> ### While I agree in principle, I think if we go down this road we
>> should also include the reason
>> why the object was rejected to make this useful. Unless others feel
>> strongly, I would not add this.
>>
>
> I think having the mechanism would be aldready usefull with existing error
> messages.
> Having WG feedback in also welcomed.
>
>
>
>>
>> Section 7.3.
>>> Nits: Using Synchronize would be better aligned with other bits
>>> definition
>>>
>>> S bit: Add: On PCUpd the R flag SHOULD (MUST?) be set to 0 on
>>> transmission
>>> and MUST be ignored on receipt.
>>>
>>
>>
>>> R bit: Add: On PCUpd the R flag SHOULD (MUST?) be set to 0 on
>>> transmission
>>> and MUST be ignored on receipt
>>>   (or it is allowed on PCUpd?)
>>>
>> O bit: Add: On PCUpd the O Field SHOULD (MUST?) be set to 0 on
>>> transmission and MUST be ignored on receipt.
>>>
>>
>> ### This would preclude use of these bits in future documents, so  I
>> prefer not to add this.
>>
>
> Reserved bit are usually defined as follows  "Unassigned bits are
> considered reserved. They MUST be set to zero on transmission and MUST be
> ignored on receipt."
>
> So restricting those values on PCUpd in this document does not preclude
> another document indicating how to use them when supporting that other
> document (It will be likely negotiated). Moreover this allows the new
> defining document to make sure that those bits have a specific value when
> using the stateful document.
>
>
>
>
>>
>>
>>> Section 7.3.3.
>>>   The error value ŒLSP preempted¹ could seem a bit redundant with ŒRSVP
>>> signaling error¹ and the corresponding RSVP preempted error code, I
>>> believe the error code ŒLSP preempted¹ should be seen when a PCC-local
>>> administrative preemption is made, and the RSVP signaling error should be
>>> used otherwise (the error node can be of value for the PCE)
>>>
>> ###  I think there is value to keep preemption separate from signaling
>> error, and I prefer to leave them
>> distinct.
>>
>
> My comment was not much on removing it, but have the text scope them
> better, as they are mutually exclusive, implementation wise I would like to
> know when to send the PCEP preempted, and the signaling preempted.
>
>
>