Re: [RTG-DIR] Rtgdir last call review of draft-ietf-pce-rfc6006bis-02

[Dhruv Dhody <[e-mail]>]

Hi Ben,

Thanks for your review. Please see inline.

On Sunday 02 July 2017 05:45 PM, Ben Niven-Jenkins wrote:
> Reviewer: Ben Niven-Jenkins
> Review result: Has Issues
>
>   Hello,
>
> I have been selected as the Routing Directorate reviewer for this draft. The
> Routing Directorate seeks to review all routing or routing-related drafts as
> they pass through IETF last call and IESG review, and sometimes on special
> request. The purpose of the review is to provide assistance to the Routing ADs.
> For more information about the Routing Directorate, please see
> ​http://trac.tools.ietf.org/area/rtg/trac/wiki/RtgDir
>
> Although these comments are primarily for the use of the Routing ADs, it would
> be helpful if you could consider them along with any other IETF Last Call
> comments that you receive, and strive to resolve them through discussion or by
> updating the draft.
>
> Document: draft-ietf-pce-rfc6006bis-02
> Reviewer: Ben Niven-Jenkins
> Review Date: 2nd July 2017
> Intended Status: Standards Track
>
> Summary: I have some minor concerns about this document that I think should be
> resolved before publication.
>
> Comments: The document was generally well written and readable.
>
> Major Issues: No major issues found.
>
> Minor Issues:
> 1) Section 3.2 SERO & SRRO objects - In Section 6.5 you have them listed with
> Object-Type 0: Reserved, whereas in section 3.2 you start at 1. you should be
> consistent and list them the same in section 3.2 as you do in 6.5?
>
> Also in Section 6.5 the reference is to [This I-D] whereas in section 3.2 it is
> to [RFC6006].
[Dhruv]: Updated.
> 2) Section 3.10 says “When adding new leaves to or removing old leaves from the
> existing P2MP tree, by supplying a list of existing leaves, it SHOULD be
> possible to optimise the existing P2MP tree.” I don’t see why you have used a
> capitalised SHOULD here as you are simply making a statement rather than
> placing a requirement on an implementation.
[Dhruv]: Ok. Updated.
> 3) Section 5 says “PCEP implementations SHOULD consider the additional security
> provided by Transport Layer Security (TLS) [I-D.ietf-pce-pceps].”
>
> Use of SHOULD says to me you expect the majority of implementations to
> implement I-D.ietf-pce-pceps. So should the reference to I-D.ietf-pce-pceps be
> normative?

[Dhruv]:Hmm, you may be right.
It just that other documents have put this down as Informative usually. 
PCEPS is also in publication process, so normative reference will most 
likely not block progress. I am not sure if we should deviate in this 
document. Thoughts?

> 4) Section 6.5 - PCEP Objects. Should you specify the meaning of Object-Types
> 0, 1 & 2 for the END-POINTS object, like you do for the other objects in this
> section?

[Dhruv]: END-POINTS is an existing object defined in RFC5440. This 
document defines new object-types for the END-POINTS object. Thus I 
don't think there is a reason to mention 0,1 & 2.
> Nits:
> Section 3.9 says
> “The only difference is that the user MUST insert the list of RROs and SRROs
> after each type of END-POINTS in the PCReq message”
>
> and Section 3.10 also says
>
> “To add new leaves, the user MUST build a P2MP request using END-POINTS with
> leaf type 1.”
>
> “To remove old leaves, the user must build a P2MP request using END-POINTS with
> leaf type 2.
>
> “For old leaves, the user MUST provide the old path as a list of RROs that
> immediately follows each END-POINTS object.”
>
> You haven’t used or defined the term “user” up until now. By user do you really
> mean PCC? If not I think you should explain what/who this user is.
[Dhruv]: Updated to PCC.

The working copy and diff are attached.

Thanks for your review.

Regards,
Dhruv

>