Subject: [Pce] Rtgdir last call review of draft-ietf-pce-rfc6006bis-02

From: Ben Niven-Jenkins <[email-masked]>
Date: Sun, 02 Jul 2017 05:15:36 -0700

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].

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.

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?

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?

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.

---

From: Dhruv Dhody <[email-masked]>
Date: Sun, 2 Jul 2017 21:38:39 +0530

Hi Ben,

Thanks for your review. Please see inline.

> Also in Section 6.5 the reference is to [This I-D] whereas in section 3.2 it is
[Dhruv]: Updated.
> placing a requirement on an implementation.
[Dhruv]: Ok. Updated.
> implement I-D.ietf-pce-pceps. So should the reference to I-D.ietf-pce-pceps be

[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?

> 0, 1 & 2 for the END-POINTS object, like you do for the other objects in this

[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.
> 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

---

From: Ben Niven-Jenkins <[email-masked]>
Date: Sun, 2 Jul 2017 18:20:23 +0100

Hi Dhruv,

> On 2 Jul 2017, at 17:08, Dhruv Dhody <[email-masked]> wrote:
> 
> Hi Ben,
> 
> 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?

I don’t have a strong opinion, I point it out as something that struck me as a possible oversight. If the authors & ADs are happy for it to be an informative reference, that’s fine with me.

> [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.

Fair enough.

Ben

---

From: Ben Niven-Jenkins <[email-masked]>
Date: Sun, 2 Jul 2017 18:20:23 +0100

Hi Dhruv,

Fair enough.

Ben