Subject: [Pce] Comments on PCEP implementation

From: Martín Germán - INCO <[email-masked]>
Date: Mon, 7 May 2007 20:34:49 -0300

Hi,
We've been following the WG since the early BOF in 2005. We proposed a 
management interface for the PCE Architecture before Paris meeting in 
2005: http://www.potaroo.net/ietf/idref/draft-grampin-pce-mgmnt-if/

Now we're finishing an implementation of PCEP (complete except from 
notifications), a PCC and a PCE. Some doubts has arisen in the 
development process, which we would like to share with the WG. They're 
listed hereafter:

===

General edit:

In some body format the bits are group by 10 but in others are group by 8 (or bytes). We think it is better to group in all body formats the bits by 8 (or bytes). For example: Figure 21 and Figure 22.

It doesn't say something like "Unassigned bits are considered as reserved and MUST be set to zero on transmission" when speaking of the Flags in several sections (e.g.: section 7.10).

===

In section 4.2.6:

"In case of TCP connection failure, the PCEP session is immediately terminated."

But in section 5 says:

".it may be desirable to systematically open and close the TCP connection for each PCEP request (for instance when sending a path computation request is a rare event)."

We think that these 2 sentences are in contradiction, because every time you close a TCP connection, according to the first you close the PCEP session. So, when you open the TCP connection again, you need to open the PCEP session to.

===

In section 6.4:

". the RP and the END-POINTS objects (see section Section 7)." section is twice.

===

In section 6.4:

Says "The special case of two BANDWIDTH objects is discussed in details in Section 7.6." but the BNF permits only one.

===

In section 6.5:

"If the path computation request can be satisfied (the PCE finds a set of path(s) that satisfy the set of constraint(s)), the set of computed path(s) specified by means of ERO object(s) is inserted in the PCRep message. The ERO object is defined in Section 7.8."

In "ERO object" object is twice (it happens twice).

The BNF allowed a NO-PATH with a ERO. If there's no path possible there isn't a ERO.

===

In section 6.6:

"<request-id-list> :== <RP><request-id-list> and

<notification-list> := <NOTIFICATION><notification-list>"

The "[" and "]" are missing before <request-id-list> and <notification-list>, and after <request-id-list> and <notification-list>.

===

In section 6.8:

"The Message-Type field of the PCEP common header for the Open message is set to 7 (To be confirmed by IANA)." It should say "Close message" instead of "Open message".

"The Close message MUST contain exactly one CLOSE object (see Section 6.8)."

This reference is little recursive.

===

In section 7.2:

"SID (PCEP session-ID - 8 bits): specifies a 2 octet." We think it should say 1 octet.

===

In section 7.3:

"The P flag of the RP object MUST be set in PCReq and PCReq" It should say "...in PCReq and PCRep".

===

In section 7.3.1:

"Reserved (8 bits): ." In the body format there are 10 bits for Reserved.

"Flags: 18 bits" In the body format there are 22 bits for Flags.

===

In section 7.4:

"Reserved: ." It doesn't say the number of Reserved bits (16 bits).

"The only TLV currently defined is the NO-PATH-VECTOR TLV defined below." It should say "above".

===

In section 7.7:

It doesn't say how many bits are Flags.

===

In section 7.10:

 "Reserved (8 bits):" In the body format there are 10 bits for Reserved.

===

In section 7.11:

It says "IRO object" (again repeating object) four times in the section.

At this point, we'd like to say that we agree we the ones that suggest that the XRO should be included in this draft, mainly for 3 reasons: it's a useful, very used (it's a common constraint in a request) and easy to compute constraint.

===

In section: 7.12.1:

"(since PCEP allows for the bundling of multiple path computation requests within a single PCRep message)" it should say "PCReq" instead of "PCRep".

===

In section 7.12.2:

"Flags: ." It doesn't say the number of Flag bits (24 bits).

===

In section 7.13:

"Flags: ." It doesn't say the number of Flag bits (8 bits).

===

In section 7.15:

"Reserved: ." It doesn't say the number of Reserved bits (20 bits).

"Flags: ." It doesn't say the number of Flag bits (4 bits).

===

In section 7.16:

"Reason (4 bits): ." In the body format there are 8 bits for Reason.

"Reserved: ." It doesn't say the number of Reserved bits (16 bits).

"Flags (4 bits): ." In the body format there are 8 bits for Flags.

The reason value 3 ("PCEP session characteristics negotiation failure") is never used, because you send an Error message with an ERROR object with Error-Type = 1and corresponding Error-Value.

===

In section 9.5:

"Error-value=2: RRO object missing for a reoptimization request (R bit of the RP object set)", repeats object.

===

In section 10:

We think that the variables used to "control" TCP connection (e.g.: TCPConnect) are useless, along with the paragraph "It is expected that an implementation.", because you rely the transport on TCP, or is matter of PCEP to specify how TCP should work?

The item "Starts the KeepWait timer" in the Idle and TCPPending state are useless, because you never wait for them, the timer that matters is OpenWait.

In the UP State, says "If the system detects that the PCEP peer tries to setup a second TCP connection, it stops the TCP connection establishment and sends a PCErr with Error-Type=10.", the Error-Type should be 9. The PCEP peer never expect this message when attempting to established a PCEP session, sending this message causes that the peer sends an Error message back.

===

Best regards,
Alberto, Martín & Eduardo

_______________________________________________
Pce mailing list
[email-masked]
https://www1.ietf.org/mailman/listinfo/pce

---

From: "LE ROUX Jean-Louis RD-CORE-LAN" <[email-masked]>
Date: Tue, 8 May 2007 23:51:54 +0200

Hi Martin
 
Thanks for the comments,
 
Please see inline,

________________________________

	De : Martín Germán - INCO [mailto:[email-masked]] 
	Envoyé : mardi 8 mai 2007 01:35
	À : [email-masked]
	Objet : [Pce] Comments on PCEP implementation

	Hi,

	===

	General edit:

	Good catch, you are right, this will be updated in next revision 

	OK 

	===

	In section 4.2.6:

	But in section 5 says:

	"...it may be desirable to systematically open and close the TCP connection for each PCEP request (for instance when sending a path computation request is a rare event)."

	Yes the wording is bad, this text corresponds to an old version of the draft where the notion of PCEP session was not defined. 

	This actualy applies to the PCEP session and not the TCP connection. We wil update (replace TCP connection by PCEP session in section 5.)

	===

	In section 6.4:

	"... the RP and the END-POINTS objects (see section Section 7)." section is twice. 

	OK 

	===

	In section 6.4:

	OK will be updated 

	===

	In section 6.5:

	Right,

	I agree, but we may want to account for less constrained path procedures that will be defined in a separate draft. You may have the ERO of a less constrained path even if there

	is a NO-PATH object. 

	===

	In section 6.6:

	Right

	===

	In section 6.8:

	Right 

	Right ! 

	===

	In section 7.2:

	"SID (PCEP session-ID - 8 bits): specifies a 2 octet..." We think it should say 1 octet. 

	Yes 

	===

	In section 7.3:

	Right 

	===

	In section 7.3.1:

	"Reserved (8 bits): ..." In the body format there are 10 bits for Reserved. 

	Yes will be updated to 8 bits in the body format

	Right, and this will be actually updated to 24 bits. 

	===

	In section 7.4:

	"Reserved: ..." It doesn't say the number of Reserved bits (16 bits). 

	OK

	OK 

	===

	In section 7.7:

	8 bits, will be added. 

	===

	In section 7.10:

	OK will be updated to 8 bits in the body 

	===

	In section 7.11:

	OK

	there are many other "easy" constraints that could be added. At some point in time we need to stop augmenting the base spec... 

	===

	In section: 7.12.1:

	Right, 

	===

	In section 7.12.2:

	"Flags: ..." It doesn't say the number of Flag bits (24 bits). 

	right,  

	===

	In section 7.13:

	"Flags: ..." It doesn't say the number of Flag bits (8 bits). 

	right, 

	===

	In section 7.15:

	"Reserved: ..." It doesn't say the number of Reserved bits (20 bits).

	"Flags: ..." It doesn't say the number of Flag bits (4 bits). 

	OK 

	===

	In section 7.16:

	"Reason (4 bits): ..." In the body format there are 8 bits for Reason.

	"Reserved: ..." It doesn't say the number of Reserved bits (16 bits).

	"Flags (4 bits): ..." In the body format there are 8 bits for Flags. 

	OK this will be updated 

	You are right this will be removed

	===

	In section 9.5:

	OK 

	===

	In section 10:

	We think that the variables used to "control" TCP connection (e.g.: TCPConnect) are useless, along with the paragraph "It is expected that an implementation...", because you rely the transport on TCP, or is matter of PCEP to specify how TCP should work? 

	Maybe the naming is confusing, these are definitely not TCP variables but PCEP variables. We may rename them ConnectWait, ConnectRetry and MaxConnectRetry.

	But note that these variables must be discussed here, this is an application issue, not a TCP issue (see the same approach in RFC 1771 for instance).

	right,

	Good catch, we should remove it.

	===

	Thanks a lot for these really useful comments 
	 
	Best Regards
	 
	JL 
	
	Best regards,