Subject: [RTG-DIR] RtgDir review: draft-ietf-pce-association-group-08

From: Stig Venaas <[email-masked]>
Date: Fri, 5 Apr 2019 17:55:17 -0700

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-association-group-08.txt
Reviewer: Stig Venaas
Review Date: April 5th 2019
IETF LC End Date: April 5th 2019
Intended Status: Standards Track

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

Comments:
The document is in good shape and fairly easy to read. There are some
grammar errors and some language that could be improved somewhat. I'm
sure the RFC editor will help with that, but I've added some comments
below.

Major Issues:
No major issues found.

Minor Issues:
No minor issues found.

Nits:

In Introduction first paragraph
Typo: Generalzied

In section 3.2:
   can either be dependent or independent.  The SVEC object identify the
Should be identifies                                        ^^^^^^^^

   The motivation behind the association group defined in this document
   and the SVEC object are quite different, though some use case may
   overlap.  The PCEP extensions that defines new association type
   should clarify the relationship between SVEC object and association
   type, if any.

A few issues here. Perhaps it should be
   The motivation behind the association group defined in this document
   and the SVEC object are quite different, though some use cases may
   overlap.  PCEP extensions that define a new association type should
   clarify the relationship between the SVEC object and the association
   type, if any.

In section 3.3:
   For the operator-configured association, the association parameters
   such as the association identifier, association type, as well as the
   association source IP address is manually configured by the operator.
The last line should perhaps be
   association source IP address, are manually configured by the
   operator.

Right below:
This line
   the association identifier is allocated dynamically by the PCEP

should perhaps be
   the association identifier, are allocated dynamically by the PCEP

Also, right below:
   operator-configured association are known to the PCEP peer before

Should be "is known".

In this paragraph:
  The associations are properties of the LSP and thus could be stored
   in the LSP state database.  The dynamic association exist as long as
   the LSP state.  In case of PCEP session termination, the LSP state
   clean-up MUST also take care of associations.

the sentence "The dynamic association exist as long as the LSP state."
should perhaps be "The dynamic association exists as long as the LSP
state exists"?

In 3.4:
   A range of association identifier for each Association type
identifiers                     ^^^^^

In 6.1:
   association type are defined in separate documents.
               ^^^^^^^^^
In 6.3.1:
   <state-report-list> ::= <state-report>[<state-report-list>]
Should there be a space here?                        ^^^^^

In 6.3.2:
   <request-list>::= <request>[<request-list>]
Space here?                ^^^^^

In 6.3.3
   <response-list>::=<response>[<response-list>]
Space here?                  ^^^^^

In 6.4:
   attached to LSP state and the association exist till there is an
exists                                      ^^^^^^^

   in [RFC5440].  If a PCEP speaker understand the ASSOCIATION object
understands                         ^^^^^^^^^^^

   a new associations, it MUST return a PCErr message with Error-Type 26
^^^^^^^^^^^

In 7.2:
   31 (Early   Extended Association Id     [This.I-D]
I think it should say ID           ^^^^
If you agree, then this should also be fixed in the IANA registry. It
currently says Id.

In 7.4:
   There are no association type specified in this document, future
types                      ^^^^^^

Regards,
Stig

---

From: Dhruv Dhody <[email-masked]>
Date: Sat, 6 Apr 2019 18:03:22 +0530

Hi Stig,

Thank you for your review, especially for providing clear suggestions
to fix the text, much appreciated!

I have fixed them all, except -

>    <response-list>::=<response>[<response-list>]

While I agree the space would have been nice, but keeping it
consistent with existing PCEP RFCs (where the space is not used, such
as RFC8231, RFC5440) is also important. Thus I am leaving this as it
is.

See diff - https://www.ietf.org/rfcdiff?url2=draft-ietf-pce-association-group-09

Regards,
Dhruv