[Pce] BRPC last call comments

Hi,

Here are some last call comments on draft-ietf-pce-brpc-07.txt

Running idnits (http://tools.ietf.org/tools/idnits/) yields...
  ** There is 1 instance of too long lines in the document, the longest one
     being 5 characters in excess of 72.

  == Unused Reference: 'I-D.ietf-ccamp-inter-domain-rsvp-te' is defined on
     line 675, but no explicit reference was found in the text

  == Outdated reference: A later version (-11) exists of
     draft-ietf-pce-pcep-09

  ** Downref: Normative reference to an Informational RFC: RFC 4655

  == Outdated reference: draft-ietf-ccamp-inter-domain-pd-path-comp has been
     published as RFC 5152

  == Outdated reference: draft-ietf-ccamp-inter-domain-rsvp-te has been
     published as RFC 5151

  == Outdated reference: A later version (-03) exists of
     draft-ietf-pce-manageability-requirements-02

  == Outdated reference: A later version (-02) exists of
     draft-ietf-pce-path-key-01

====
General
Replace optional plurals "(s)" with normal plural "s" for correct grammar
Please fix indentation for bullets in sections 3, 4, and 7
Please ensure that all section headings use capitals
====
Document title
Please use capitalisation as...
 A Backward Recursive PCE-based Computation (BRPC) Procedure to Compute
     Shortest Inter-Domain Traffic Engineering Label Switched Paths
====
Document title
Should the title say "Shortest Constrained" rather than "Shortest"? The 
Abstract and the body of the text refered to "constrained".
====
Abstract
s/domain is referred to as a/domain is a/
s/as IGP areas and Autonomous Systems/as an IGP area or an Autonomous 
System/
s/in order to compute/to compute/
s/paths along a/paths across a/
s/determined sequence/predetermined sequence/
s/technique while preserving/technique. This technique preserves/
====
Section 1.
The RFC Editor requires that the first section is the Introduction.
Therefore...
- Swap section 1 and section 2
- Expand all acronyms on their first use in the Introduction
  This includes "(G)MPLS"
====
Terminology section
If you choose to include terms like ABR, can you also please include the 
acronym expansion.
====
Introduction
s/establishing inter-domain/establishing an inter-domain/
====
Introduction
You say...
   the entry
   boundary node of each domain (each node in charge of computing a
   section of an inter-domain TE LSP path is always along the path of
   such TE LSP).
This implies that the BN is not able to be a PCC and use a remote PCE. I 
don't think this limit applies.
Perhaps you mean to say "each node responsible for triggering the 
computation of a section...."
====
Introduction
s/Such path computation technique/This computation technique/
====
Introduction
I think you should add to the final paragraph the usual note about common 
understanding or metrics and objective functions (or the ability to provide 
suitable mappings) between the domains.
(This is exapnded on in section 3 bullet 3, but it is fundamental to the 
brpc paradigm.
====
Section 3, bullet 1
Why do you assume that OSPF-TE or ISIS-TE are being run.
I think your assumption is that each PCE has access to an up-to-date TED, 
and you might note that the normal way to achieve this would be by running 
OSPF-TE or ISIS-TE.
====
Section 3, final bullet
s/means that are/means that is/
====
Section 4
s/solve multi-constraints/solve multi-constraint/
====
Section 4.1
In section 3, final bullet, we see...
   - The domain path (set of domains traversed to reach the destination
   domain) is either administratively pre-determined or discovered by
   some means that are outside of the scope of this document.
But here in section 4.1...
   The sequence of domains to be
   traversed can either be determined a priori or during the path
   computation procedure.
I wonder if section 4.1 is trying to provide hints of how to do domain 
selection. Perhaps you could replace 4.1 with a simple statement that BRPC 
mechanisms might also be used to determine the sequence of domains, but that 
such procedures are out of scope of this document.
====
Section 4.2
Is "Definition of VSPT(i)" intended to be a subsection?
I find that the section leaps to rapidly into the definition of the VSPT. 
Shouldn't there be some discussion first (somewhere) about the sequence of 
exchange of information between PCC and PCEs? This isn't much information, 
but seeing the VSPT(i) described as "tree returned by PCE(i) to PCE(i-1)" 
makes me wonder how the PCEs are joined together.
====
Figure 1
s/BN-en((j), i)/BN-en(j,i)/
====
Section 4.2, paragraph after Figure 1
s/the number of entry BN/the number of entry BNs/
Delete "(identified by its TE Router-ID)"? This has nothing to do with the 
definition of the abstract VSPT.
====
Section 4.2
   Note that PCE(i) only considers the entry BNs that provide
   connectivity from domain(i-1).
According to the definition of "Entry BN of domain(n)" in the terminology 
section, this text is redundant.
Perhaps you mean to say...
   Note that PCE(i) only considers the entry BNs of domain(i). That is,
   only the BNs that provide connectivity from domain (i-1).
This, however, jars with the note in Figure 1 that "j <= [X-en(i)]" since 
the implication of that note and the subsequent text in 4.2 is that the set 
of entry nodes to domain(i) (X-en(i)) is actually considered to be the set 
of all BNs to domain(i).
You need to select a some terminology and then fix the text to be 
consistent.
Note, it is fine when you say:
   Furthermore, some BNs may be excluded according to policy
   constraints (either due to local policy or policies signaled in the
   path computation request).
This does allow j <= [X-en(i)]
====
Section 4.2
   The path computation request is then relayed until reaching a PCE(n)
   such that the TE LSP destination resides in the domain(n).  At each
   step of the process, the next PCE can either be statically configured
   or dynamically discovered via IGP/BGP extensions.
You need to be clear. Is it required that the PCEP request is forwarded in 
strict sequence PCE(i-1)-->PCE(i) for all i = 2 to n, or is it OK for the 
request to simply be forwarded direct to PCE(n)?
This leads to...
   If
   multiple PCEs are discovered, the PCE may select a subset of these
   PCEs based on some local policies or heuristics.
...should be clarified as...
   If PCE(i-1)
   discovers multiple PCEs for the adjacent domain(i), PCE(i) may
   select a subset of these PCEs based on some local policies or
   heuristics.
And hidden in here is the fact that PCE(i-1) may consult multiple PCE(i)s. 
This is fine, but is not exposed anywhere else in the document. In 
particular, the process does not say "PCE(i-1) MUST/SHOULD/MAY wait until it 
has heard responses from each PCE(i) that it has consulted."
====
Section 4.2
   Step i:

   - For i=n-1 to 2: PCE(i) concatenates the topology of domain(i)
   (using its TED) with the received VSPT(i+1).

I don't like "concatenates the topology". Don't you mean...
   Step i:

   - For i=n-1 to 2: PCE(i) computes the shortest constrained paths from
     each BN-en(i) to each BN-ex(i) that is identified in VSPT(i+1). It
     then concatenates the shortest of those paths to the paths in
     VSPT(i+1) to construct VSPT(i).

Then you can delete the following...
   Then PCE(i) computes VSPT(i) (MP2P (Multi-Point to Point) tree made
   of the shortest constrained paths between each BN-en(j,i) and the TE
   LSP destination).

====
Section 4.2
Replace...
   End
...with...
   Step n
====
Section 5
Is it assumed that PCE(i) knows which domain the requesting PCE(i-1) 
represents?
The reverse is obviously true because PCE(i-1) must select PCE(i) on this 
basis, but it seems to me that the PCReq could useful identify the 
neighboring (upstream) domain. This would be particularly useful in the case 
of a requesting PCE that represents multiple domains since it will allow 
PCE(i) to know which are the BN-en(i) nodes.
Conversely, if PCE(i) has computation capabilities for multiple domains, it 
will need to be told which for domain(i) it should act.
So we either need protocol extensions (requesting domain, computation 
domain) or we need to be told how to use existing protocol fields for this 
purpose.

Furthermore :-( how although the sequence of domains is known (a priori) by 
the PCC, we need some way to convey the sequence in the PCReq so that PCE(i) 
knows that the next domain in the sequence is domain(i+1).
If this can be achieved by existing protocol mechanism, you need to describe 
the procedures. If it can't be done, we need protocol extensions.
====
Section 6
s/from an ABR/from a BN/
s/PCERep/PCRep/
====
Section 9
Since the protocol extensions presented in this document are exactly that 
(protocol extensions) you must beware of backward compatibility issues. So 
when you have...
   If the BRPC procedure cannot be completed because a PCE along the
   domain path does not support the procedure, a PCErr message MUST be
   returned to the upstream PCE with a Error-Type "BRPC procedure
   completion failure".  The PCErr message MUST be relayed to the
   requesting PCC.
...you need to be aware that for PCE(i), not supporting BRPC (i.e. not 
recognising the VSPT bit) is going to be treated according to the main PCEP 
spec, and not according to any rules in this document.
But pcep-11 says...
   Unassigned bits are considered as reserved and MUST be set to zero on
   transmission.
...from which we assume that bits not understood are ignored and MAYBE 
forwarded in the next PCReq to PCE(i+1) or MAYBE dropped.

Ouch!
Not to late to fix the PCEP spec to say that an RP object carrying an 
unrecognised bit MUST be rejected with an Error Type of "Unknown Request 
Parameter" (or would you use "Capability Not Supported"?) and an Error Code 
to indicate the bit number. (You can't give names to the error codes because 
you don't know what the bits mean!)

That would leave you with the need to modify your document here as...
   If the BRPC procedure cannot be completed because a PCE along the
   domain path recognises but does not support the procedure, it MUST
   return a PCErr message to the upstream PCE with an Error-Type "BRPC
   procedure completion failure".

Note that if you do not fix the PCEP spec, you will have to handle the case 
where the PCE receives a PCRep from its downstream neighbor containing a 
VSPT that will be incomprehensible.

See also Section 14.1
====
Section 10.1
s/Such request MUST/Such requests MUST/
s/a set of diversely routed TE LSP/a set of diversely routed TE LSPs/
====
Section 13
s/meaningful, a metric normalization/meaningful, metric normalization/
s/thanks to the METRIC/using the METRIC/
====
Section 14.2
Not mandatory, but it would be useful to highlight what this MIB module 
actually needs to track.
Is it just the configuration widget in 14.1, or is there more stuff?
====
Section 14.4
Ha!
And how will you look at these TEDs?
Have a look at 8.4 of pcep-11
====
Appendix A
I think you can remove this now
====

Cheers,
Adrian 

_______________________________________________
Pce mailing list
Pce@ietf.org
https://www.ietf.org/mailman/listinfo/pce