Subject: [Pce] Rtgdir last call review of draft-ietf-pce-hierarchy-extensions-09

From: Mike McBride <[email-masked]>
Date: Mon, 25 Feb 2019 15:14:21 -0800

Reviewer: Mike McBride
Review result: Has Nits

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-hierarchy-extensions-09
Reviewer: Mike McBride
Review Date: 25 February 2019
IETF LC End Date: N/A (in preparation for IETF LC)
Intended Status: Standards Track

Summary:

This document is near ready for publication. It has nits that should be
considered prior to publication.

Comments:

Great job on the draft and congrats on near publication. It was a tad difficult
to follow certain areas of the document and I therefore offer suggestions to
improve the readability.

Major issues:

No major issues found.

Minor Issues:

No minor issues found.

Nits for your consideration:

1. Page 4:

Replace:
A child PCE may be responsible for a single domain or multiple domains, it is
used to compute the intra-domain path based on its own domain topology
information.

With:
A child PCE may be responsible for single or multiple domains and is used to
compute..."

2. Page 4:

Replace:
In addition, the parent PCE may be requested to provide only the sequence of
domains to a child PCE so that alternative inter-domain path computation
procedures, including Per Domain (PD) [RFC5152] and Backwards Recursive Path
Computation (BRPC) [RFC5441] may be used.

With:
The parent PCE may be requested to provide only the sequence of domains to a
child PCE so that alternative inter-domain path computation procedures,
including Per Domain (PD) [RFC5152] and Backwards Recursive Path Computation
(BRPC) [RFC5441], may be used.

3. Page 5:

Replace:
This could be done via

With:
Move formatting to the right to remain consistent in this section. Under
"Learning".

4. Page 5:

Replace:
The hierarchical relationship model is described in [RFC6805]. It is applicable
to environments with small groups of domains where visibility from the ingress
LSRs is limited. As highlighted in [RFC7399] applying the hierarchical PCE
model to large groups of domains such as the Internet is not considered
feasible or desirable.

With:
Move formatting to the right to remain consistent in this section. Under
"Stateful".

5. Page 10:

Replace:
The Domain-ID TLV when used in the OPEN object, identify the domains served by
the PCE.

With:
Add a comma after TLV
s/identify/identifies

6. Page 12:

Replace:
The usage of Domain-ID TLV carried in an OPEN object is used to indicate a
(list of) managed domains and is described in Section 3.3.1. This TLV when
carried in an RP object, indicates the destination domain ID.

With:
Use of commas. Comma after 'Domain-ID TLV' and 'object'. And after 'This TLV'.

7. Page 12:

Replace:
If a Domain-id TLV is used in the RP object, and the destination is not
actually in the indicated domain, then the parent PCE should respond with a
NO-PATH object and NO-PATH VECTOR TLV should be used, and a new bit number is
assigned to indicate "Destination not found in the indicated domain" (see
Section 3.7).

With:
End the sentence after the second 'used' and then start the new sentence with
'And a new bit...'

8. Page 14:

Replace:
The domain count metric type of the METRIC object encodes the number of domain
crossed in the path.

With:
change the second 'domain' to 'domains' or maybe 'domain's'.

9. Page 14:

Replace:
A PCC or child PCE MAY use these metric in PCReq message for an inter-domain
path computation meeting the number of domain or border nodes crossing
requirement.

With:
Change 'these' to 'the'

10. Page 15:

Replace:
The Parent PCE MAY use these metric in a PCRep message along with a NO-PATH
object in the case where the PCE cannot compute a path meeting this constraint.

With:
change 'these' to 'the' or change to 'PCRep messages'.

11. Page 16:

Replace:
The child PCE MAY also report its list of domain IDs to the parent PCE by
specifying them in the Domain-ID TLVs in the OPEN object carried in the Open
message during the PCEP session initialization procedure.

With:
This is a tough sentence to follow. With the following punctuation changes is
the intention still correct?: "The child PCE MAY also report its list of domain
IDs, to the parent PCE, by specifying them in the Domain-ID TLVs in the OPEN
object. This object is carried in the OPEN message during the PCEP session
initialization procedure."

12. Page 16:

Replace:
When a specific child PCE sends a PCReq to a peer PCE that requires parental
activity and H-PCE capability flags TLV was not included in the session
establishment procedure as described above, the peer PCE should send a PCErr
message to the child PCE and specify the error-type=TBD8 (H-PCE error) and
error-value=1 (H-PCE capability was not advertised) in the PCEP-ERROR object.

With:
Again, a hard, and long, sentence to follow.  I'm not certain if its the child
or peer PCE that is doing the requiring, I'll go with the peer.  See if this is
still correct after adding some commas: "When a child PCE sends a PCReq to a
peer PCE, which requires parental activity and H-PCE capability flags TLV but
which were not included in the session establishment procedure described above,
the peer PCE should send a PCErr message to the child PCE and should specify
the error-type..."

13. Page 17:

Replace:
When a specific child PCE sends a PCReq to a peer PCE that requires parental
activity and the peer PCE does not want to act as the parent for it, the peer
PCE should send a PCErr message to the child PCE and specify the
error-type=TBD8 (H-PCE error) and error-value=2 (Parent PCE capability cannot
be provided) in the PCEP-ERROR object.

With:
strategically placed commas for clarity:
"When a specific child PCE sends a PCReq to a peer PCE, that requires parental
activity and the peer PCE does not want to act as the parent for it, the peer
PCE..."

14. Page 17:

Replace:
ERO

With:
first time ERO is being used in the document. Please call out Explicit Route
Object somewhere.

15. Page 17:

Replace:
o The parent PCE do not hear from a child PCE for a specified time;

With:
'does not' instead of 'do not'

that's it! ;-) I'm suggestion many changes but it'll go quick and make it much
more readable.

thanks,
mike

---

From: Dhruv Dhody <[email-masked]>
Date: Tue, 26 Feb 2019 04:33:45 +0000

Hi Mike, 

Thank you for your RTGDIR review! Especially thankful for providing suggested texts in your comments. I hope the authors will spin out a new version SOON.  

Just one point - 

> 4. Page 5:
> 
> Replace:
> The hierarchical relationship model is described in [RFC6805]. It is
> applicable to environments with small groups of domains where visibility
> from the ingress LSRs is limited. As highlighted in [RFC7399] applying the
> hierarchical PCE model to large groups of domains such as the Internet is
> not considered feasible or desirable.
> 
> With:

It is better if the text starts with "o" as a separate item in the outer list. As this is not under the Stateful PCE operations.  

Thanks! 
Dhruv (Shepherd/Co-Chair)

---

From: <[email-masked]>
Date: Tue, 26 Feb 2019 06:19:16 -0000

Hi Mike, 

Thanks for your detailed review and comments! 

We will squash those NITS and post an update shortly.

BR, Dan (and the other draft-ietf-pce-hierarchy-extensions authors)  

-----Original Message-----
From: Mike McBride <[email-masked]> 
Sent: 25 February 2019 23:14
To: [email-masked]
Cc: [email-masked]; [email-masked]
Subject: Rtgdir last call review of draft-ietf-pce-hierarchy-extensions-09

Reviewer: Mike McBride
Review result: Has Nits

Summary:

Comments:

Major issues:

No major issues found.

Minor Issues:

No minor issues found.

1. Page 4:

Replace:

With:

2. Page 4:

Replace:

With:

3. Page 5:

Replace:
This could be done via

With:

4. Page 5:

Replace:

With:

5. Page 10:

Replace:

With:
Add a comma after TLV
s/identify/identifies

6. Page 12:

Replace:

With:

7. Page 12:

Replace:

With:

8. Page 14:

Replace:

With:

9. Page 14:

Replace:

With:
Change 'these' to 'the'

10. Page 15:

Replace:

With:

11. Page 16:

Replace:

With:

12. Page 16:

Replace:

With:

13. Page 17:

Replace:

With:

14. Page 17:

Replace:
ERO

With:

15. Page 17:

Replace:

With:

thanks,
mike