Path Computation Element (PCE) Communication Protocol (PCEP)
draft-ietf-pce-pcep-19

Referencing other documents that use BNF without a reference to the
original definition of BNF is probably a worse solution than no
reference.  I think it would be better to reference a real definition
of the language, such as one of the older references here:

 http://en.wikipedia.org/wiki/Backus%E2%80%93Naur_Form

However, the change makes it clear the authors want to use a formal
language without definition so it's not worth my time to argue the
point further.

My previous DISCUSS:

This document uses a formal language (BNF) without providing a reference
to a document that defines that formal language.  Please provide a
reference.

Section 4.2.1., paragraph 4:
> Successive retries are permitted but an implementation should make
> use of an exponential back-off session establishment retry procedure.

  s/should make/SHOULD make/


Section 4.2.2., paragraph 4:
> Once the PCC has selected a PCE, it sends the PCE a path computation
> request to the PCE (PCReq message) that contains a variety of objects
> that specify the set of constraints and attributes for the path to be
> computed.

  Can a PCC send a second path computation request over the same TCP
  connection to a PCE when the answer to an earlier one is still
  outstanding? Can multiple TCP connections exist between the same PCC
  and PCE?


Section 4.2.4., paragraph 1:
> There are several circumstances in which a PCE may want to notify a
> PCC of a specific event.  For example, suppose that the PCE suddenly
> gets overloaded, potentially leading to unacceptable response times.

  Can such notifications occur at any time, i.e., while another message
  is being sent? If so, how are they framed within the TCP byte stream?


Section 6.2., paragraph 2:
> characteristics.  If both parties agree on such characteristics the
> PCEP session is successfully established.  TOTO

  TOTO?


Section 7.3., paragraph 13:
> A sends an Open message to B with Keepalive=10 seconds and
> Deadtimer=30 seconds.  This means that A sends Keepalive messages (or
> ay other PCEP message) to B every 10 seconds and B can declare the
> PCEP session with A down if no PCEP message has been received from A
> within any 30 second period.

  I'd be nice if the example followed the recommended values/fomulas
  above and used Keepalive=30 and DeadTimer=4*Keepalive (or whatever the
  defaults will be after addressing my comments above.)


Section 7.3., paragraph 14:
> SID (PCEP session-ID - 8 bits): unsigned PCEP session number that
> identifies the current session.  The SID MUST be incremented each
> time a new PCEP session is established and is used for logging and
> troubleshooting purposes.  There is one SID number in each direction.

  What's the start value? Is it incremented for each connection to any
  PCEP peer or only for connections to the same PCEP peer? Does it
  matter when SID rolls over? The document doesn't discuss what the SID
  is used for at all.


Section 7.4.1., paragraph 14:
> Request-ID-number (32 bits).  The Request-ID-number value combined
> with the source IP address of the PCC and the PCE address uniquely
> identify the path computation request context.  The Request-ID-number
> MUST be incremented each time a new request is sent to the PCE.  The
> value 0x0000000 is considered as invalid.  If no path computation
> reply is received from the PCE, and the PCC wishes to resend its
> request, the same Request-ID-number MUST be used.  Conversely,
> different Request-ID-number MUST be used for different requests sent
> to a PCE.  The same Request-ID-number MAY be used for path
> computation requests sent to different PCEs.  The path computation
> reply is unambiguously identified by the IP source address of the
> replying PCE.

  It's redundant to identify requests by source and destination IP
  address, given that those are constant for requests going over the
  same TCP connection. Likewise, replies are implicitly identified by
  the TCP connection they arrive over.


Section 8.3., paragraph 1:
> PCEP includes a keepalive mechanism to check the liveliness of a PCEP
> peer and a notification procedure allowing a PCE to advertise its
> congestion state to a PCC.

  s/congestion/overload/ for consistency, here and throughout the
  document


Section 9.1., paragraph 1:
> PCEP uses a well-known TCP port. IANA is requested to assign a port
> number from the "System" sub-registry of the "Port Numbers" registry.

  Does "uses a well-known TCP port" mean that messages from the PCC to
  the PCE must come from that registered source port, or can they come
  from any port? (The former implies that only a single PCEP connection
  can exist between a PCC and a PCE. It also weakens security a bit,
  because an attacker doesn't need to guess the source port anymore.)


Section 10.3.1., paragraph 2:
> o  A PCE should avoid promiscuous TCP listens for PCEP TCP connection
>    establishment.  It should use only listens that are specific to
>    authorized PCCs.

  Authorized by what? TCP has no feature to restrict listens based on
  credentials.

Section 10.3.1., paragraph 4:
> o  The use of access-list on the PCE so as to restrict access to
>    authorized PCCs.

  Is redundant with the first bullet (or I don't understand what it
  means).


Appendix A., paragraph 46:
> If the system receives an Open message from the PCEP peer before the
> expiration of the OpenWait timer, the system first examines all of
> its sessions that are in the OpenWait or KeepWait state.  If another
> session with the same PCEP peer already exists (same IP address),
> then the system performs the following collision resolution
> procedure:

  The goal of this procedure seems to be to guarantee that there is only
  a single active PCEP connection between two peers, but it's
  cumbersome. It'd be much easier to require a peer to not initiate a
  connection to a peer it already has one established with, and to
  require it to immediately close a new TCP connection coming from a
  peer it has an active PCEP connection with. This handles everything at
  the TCP layer without needing to involve the PCEP state machine.

Version 14 resolved my discusses almost completely. However the overload discuss I had needs commenting:

It is quite unclear if the overload protection provided does actually work. It has similarities with the one for SIP that has been found to not work. The only way of really learning this is to do simulation for which I am not going to hold up the document.

There was agreement to document the BNF used in its own document.

Updated for version -19: RFC 2385 should be listed
as a normative reference, not informative.

Note: I support Lars discuss with respect to preference of draft-ietf-tcpm-tcp-auth-opt
over TCP MD5.

Section 7.5

It is unclear if "PCE chain broken" in a No Path object is really a
meaningful "Nature of Issue". What action can/should a PCC take?
Contact a different PCE?

Editorial nit:
4.2.1
s/a pair a PCEP peers/a pair of PCEP peers/