Hi Martin
Thanks for the comments,
Please see inline,
________________________________
De : Martín Germán - INCO [mailto:[EMAIL PROTECTED]
Envoyé : mardi 8 mai 2007 01:35
À : [EMAIL PROTECTED]
Objet : [Pce] Comments on PCEP implementation
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.
Good catch, you are right, this will be updated in next revision
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).
OK
===
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.
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:
Says "The special case of two BANDWIDTH objects is discussed in details
in Section 7.6." but the BNF permits only one.
OK will be updated
===
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).
Right,
The BNF allowed a NO-PATH with a ERO. If there's no path possible there
isn't a ERO.
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:
"<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>.
Right
===
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".
Right
"The Close message MUST contain exactly one CLOSE object (see Section
6.8)."
This reference is little recursive.
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:
"The P flag of the RP object MUST be set in PCReq and PCReq" It should
say "...in PCReq and PCRep".
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
"Flags: 18 bits" In the body format there are 22 bits for Flags.
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
"The only TLV currently defined is the NO-PATH-VECTOR TLV defined
below." It should say "above".
OK
===
In section 7.7:
It doesn't say how many bits are Flags.
8 bits, will be added.
===
In section 7.10:
"Reserved (8 bits):" In the body format there are 10 bits for
Reserved.
OK will be updated to 8 bits in the body
===
In section 7.11:
It says "IRO object" (again repeating object) four times in the
section.
OK
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.
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:
"(since PCEP allows for the bundling of multiple path computation
requests within a single PCRep message)" it should say "PCReq" instead of
"PCRep".
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
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.
You are right this will be removed
===
In section 9.5:
"Error-value=2: RRO object missing for a reoptimization request (R bit
of the RP object set)", repeats object.
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).
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.
right,
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.
Good catch, we should remove it.
===
Thanks a lot for these really useful comments
Best Regards
JL
Best regards,
Alberto, Martín & Eduardo
_______________________________________________
Pce mailing list
[email protected]
https://www1.ietf.org/mailman/listinfo/pce
