Subject: [Pce] RtgDir review: draft-ietf-pce-pcep-service-aware-11

From: <[email-masked]>
Date: Sun, 07 Aug 2016 11:24:50 -0400

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-pcep-service-aware-11
Reviewer: Christian Hopps
Review Date: August 7th, 2016
IETF LC End Date: Unknown
Intended Status: Standards Track

Summary:
========

    This document is basically ready for publication, but has nits that
    should be considered prior to publication.

Comments:
=========

    This document is clearly written and easy to understand.

Major Issues:
=============

    No major issues found.

Minor Issues:
=============

    - [4.1.5.1] Indicates inserting second METRIC object is optional ("may"),
      but doesn't say MUST or MAY for the first METRIC object. The implication
      is that the first METRIC object is required in the reply, if this isn't
      the case then a MAY/SHOULD would be useful here as well.

Nits:
=====

    - [4.1.5.1] uppercase "may" to MAY.

    - [4.2.1] parenthetical reference uses ``"Utilized Bandwidth"'' to refer to
      the IS-IS and OSPF values which are actualy defined in their respective
      documents using the text: "Unidirectional Utilized Bandwidth". I suggest
      updating this to match the referenced documents (i.e., add
      "Unidirectional").

    - [4.2.1] It's clear to me that the first paragraph is defining what LBU is;
      however, it never actually says this explicitly incase you wanted to.

    - [4.3] I found the formatting a bit odd with the "Objective Function Code:"
      the "Name:" and "Description:" being the final lines in the section
      defining them, but it's also hard to misinterpret the text when you
      actually read it so I don't know if any changes are needed.

    - [4.3 last paragraph] an unnamed procedure is referred to in RFC5541 here,
      I believe its use is more thoroughly described earlier in the document
      under "[4.2.3.1] Elements of Procedure", if so perhaps the reference
      should be to the text earlier in this document?

    - [6.1, 6.2] It's interesting that this text is actually replacing the
      message definition from the original specification. I understand it's
      trying to show where the new <bu-list> fits in, but it also is sort of
      redefining the actual makup of the entire message format. Using this
      method of description, each future extension to a message format must comb
      through any and all previous extensions, and also incorporate their
      changes as well; however, as this document isn't actually replacing
      RFC5541 (and neither would other extensions), or the STATEFUL-PCE
      document, so there's no document pointer trail to follow to do this --
      it's messy. A simple way to avoid this would be to present the change to
      the message definition using a diff like format indicating where the new
      <bu-list> fits into the already defined format and leave the overall
      definition of the message format in RFC5541.

---

From: Dhruv Dhody <[email-masked]>
Date: Tue, 9 Aug 2016 16:33:54 +0000

Hi Chris, 

Thanks for your review. 
I have attached the working copy with diff. 
Please see inline. 

> -----Original Message-----
> From: Pce [mailto:[email-masked]] On Behalf Of [email-masked]
> Sent: 07 August 2016 20:55
> To: [email-masked]
> Cc: [email-masked]; [email-masked];
> [email-masked]
> Subject: [Pce] RtgDir review: draft-ietf-pce-pcep-service-aware-11
> 
> 
> Hello,
> 
> 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
> 
> 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.
> 
[Dhruv] The first is required and the second is optional. 
I am using language similar to the RFC5440 example [https://tools.ietf.org/html/rfc5440#section-7.8]. 

I have added Reference to RFC5440 to be clear that the mode of operations is as per that draft. 
  
>     - [4.1.5.1] uppercase "may" to MAY.
[Dhruv] Ok! 
>       updating this to match the referenced documents (i.e., add
[Dhruv] Done! 
>       however, it never actually says this explicitly incase you wanted to.
[Dhruv] Done! 
 
>       actually read it so I don't know if any changes are needed.
[Dhruv] I have made a formatting change and tried to make it similar to RFC5541. 

>       should be to the text earlier in this document?
[Dhruv] I added a section number 3.1.1 of RFC5541. The section 4.2.3.1 cannot be applied here. 
>       document, so there's no document pointer trail to follow to do this

[Dhruv] I understand how that could be useful, there was discussion in the past in the WG about the best way to align RBNF along various extensions but the WG did not conclude on it. I will discuss with Jon as document shepherded and figure out if any action needs to be taken here. 

Thanks again! 

Regards,
Dhruv

---

From: Jonathan Hardwick <[email-masked]>
Date: Tue, 16 Aug 2016 15:21:27 +0000

Hi Chris,

Many thanks for your comments.

Prompted by Dhruv, I'm replying to the following comment, as document shepherd.

>       message definition from the original specification. 
[...]
>       A simple way to avoid this would be to present the change to
>       <bu-list> fits into the already defined format

This happens any time we add a new object to a PCEP message.  As Dhruv mentioned, we've wrestled with this before in the PCE WG without having really solved the problem.  Since the objects in the message must be present in the order given by the RBNF, we have to show each object in its context rather than simply say "it goes anywhere in the <request> construct".

I think that presenting a diff would make the text more concise, but it would not solve the problem of authors (or implementers) having to comb through previous extensions.  A diff is necessarily relative to a fixed base version of the message format.  The next time we add an object, say after the <bu-list> construct, the authors would need to find this document in order to say that their new object comes after <bu-list>.  Otherwise we'd get into a mess of having multiple "branches" of PCEP message definitions, which would need to be merged by implementers, which would be unreliable.

I've checked several RFCs published for PCEP and RSVP (which also uses RBNF).  Almost all of these completely re-specify the message format when a new object is added.  I propose that we continue to do this, as it has not proved to be a problem so far.

Best regards
Jon

-----Original Message-----
From: Pce [mailto:[email-masked]] On Behalf Of Dhruv Dhody
Sent: 09 August 2016 17:34
To: [email-masked]; [email-masked]
Cc: [email-masked]; [email-masked]; [email-masked]
Subject: Re: [Pce] RtgDir review: draft-ietf-pce-pcep-service-aware-11

Hi Chris, 

Thanks for your review. 

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

[Dhruv] Ok! 
[Dhruv] Done! 
[Dhruv] Done! 

>       document, so there's no document pointer trail to follow to do 
> this
> --

Thanks again! 

Regards,
Dhruv