Hi Magnus,
Thanks for your review of this work. Here are our responses to your
Discusses.
Cheers,
Adrian
> Discuss:
> 1. Section 6.2:
>
>> Any message received
>> prior to an Open message MUST trigger a protocol error condition and
>> the PCEP session MUST be terminated.
>
> How? To my understanding no Session context has been created due to
> failure. So it is meant that one should indicate that error and then close
> the TCP connection?
It may be a semantic nicity to say that the Session is not created until it
has been fully established.
We should refine the text in the I-D to say that the "Any message received
prior to an Open message MUST trigger a protocol error condition causing a
PCErr message to be sent with Error-Type 'PCEP session establishment
failure' and Error-Value 'reception of an invalid Open message or a non Open
message' and the PCEP session establishment attempt MUST be terminated by
closing the TCP connection."
> 2. I can't find any information about how messages must be processed
> in relation to each other. There is text on various places talking about
> "pending requests" which seem to me to imply that requests can be
> handled in parallel. However, that doesn't seem to be explicitly stated.
> Also the inter message type relations in that case should be clarified.
> Both within a message type and between the different type of messages.
There is no interdependence between PCReq messages. These messages are
simply containers for Path Computation Requests. These requests may be made
dependent using the SVEC object (section 7.13).
We could clarify that...
A PCEP implementation SHOULD process messages in the order in which they are
received, but MAY queue path computation requests for processing, and MAY
re-order path computation requests according to their relative priorities as
encoded in the RP Object (Section 7.4.1).
> 3. Section 7.3:
>
>> 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.
>
> How do it needs to be incremented? With current formulation any
> incremention, 1, 10 or 100 would be okay.
>
> Secondly, what happens at wraparound?
>
> Based on Lars discuss further clarification may be need if reconnecting or
> multiple simultaneous sessions are allowed.
I replied to Lars' similar issue as follows...
I asked similar questions during WG last call.
The answers are:
| Start where you like, the value is not important for the protocol.
| The requirement is that the SID is 'sufficiently different' to avoid
| confusion between instances of sessions to the same peer.
| Thus, "incremented" is more like implementation advice than a strict
| definition. In particular, incremented by 255 would be fine :-)
| However, the usage (for logging and troubleshooting) might suggest that
| incrementing by one is a helpful way of looking at things.
| SID roll-over is not particularly a problem.
|
| Implementation could use a single source of SIDs across all peers, or one
| source for each peer. The former might constrain the implementation to
only
| 255 concurrent sessions. The latter potentially requires more state.
> 4. Section 7.4.1:
>
> Once more it is unclear if the request-id-number must be incremented with
> one or larger values are allowed. Also if any specific start value is
> assumed.
>
> Please do also clarify if this value has long term validity and should be
> maintained over multiple PCEP sessions.
>
> Wraparound may also be needed to be specified if the PCEP session
> becomes very long lived, or the space are valid over subsequent sessions.
You are correct that this section is not specified tightly enough. In fact,
it includes text influenced by implmentation rather than requirements.
The requirement needs to say not only that "different Request-ID-number MUST
be used for different requests sent to a PCE" but also what the of scope of
this ID is in space and time (e.g. session lifetime, until response
received, etc.) and whether monotonic increasing is needed (in which case
wrapping is an issue).
I don't believe that "incrementing" the ID is required at all. What is
required is disambiguation of requests.
It is not clear why a retransmission of a request would be allowed/needed
except after a TCP connection failure, nor is it clear what happens when
there is a race between retransmission and response: does the PCE ignore the
second request with the same ID, or does it send a second response
(potentially different) that the PCC must resolve? Actually, it is not clear
what a PCE should do if it receives a request-ID that is smaller than one
previously received: are there different cases if the request-ID has been
previously used, and if it is new but smaller?
I also see in 7.4.2...
If the PCC receives a PCRep message that contains a RP object
referring to an unknown Request-ID-Number, the PCC MUST send a PCErr
message with Error-Type="Unknown request reference".
I wonder why this is the case. It seems to increase DoS leverage when
ignore/discard would be adequate.
Lastly, the text states "The value 0x0000000 is considered as invalid" but
does not describe how to handle the receipt by a PCE of this value.
The authors will need to do some work here!
> 5. Section 7.7:
>
>> Bandwidth: 32 bits. The requested bandwidth is encoded in 32 bits in
>> IEEE floating point format, expressed in bytes per second. Refer to
>> Section 3.1.2 of [RFC3471] for a table of commonly used values.
>
> Over what time constraints are this value intended to be valid? Due to
> that transmission of packets can be bursty there is need for more
> information. Shouldn't this be a token bucket to better model the data
> moving capability of the request?
Perhaps I don't understand your question, but in MPLS-TE and GMPLS we are
reserving bandwidth. In this case, it is usual to complete the Sender_Tspec
using just the Peak Data Rate field of Int-Serv objects (see [RFC2210]) to
indicate the capacity to be reserved. (Note other non-packet technologies
vary this, but the effect is similar.)
> 6. Section 7.7:
>
> Reference for 32-bit "IEEE floating point format"? Please add it also on
> the
> other places where you use this format in definitions.
Yes.
I believe the reference is...
[IEEE] "IEEE Standard for Binary Floating-Point
Arithmetic", IEEE Standard 754-1985, 1985
(ISBN 1-5593-7653-8).
> 7. section 7.11:
>
> Please be explicit about which of the documents you use the include-any,
> exclude-any, and include-all. Having browsed through RFC 4090 and 3209
> I can't find a definition of what "attribute filters" are. So please
> include a
> reference to the definition of these also.
Section 4.7.4 of RFC3209 gives a detailed description of the use of resource
affinities.
> 8. The SyncTimer:
>
> How can this timer value be local only? The request sender MUST know
> how much time it has to supply all the request before it failing due to
> the
> timer.
It is expected that a PCC that has a set of requests to send will get on
with it in a relatively timely manner. The SyncTimer is a house-keeping
timer not a protocol timer. Thus its value on a PCE is set large enough to
ensure that the PCC has a good chance of achieving transmission, but that if
there is some problem (or a broken PCC) the PCE will clean up its resources.
> So if a PCE uses a non default
No default value is defined
> then a failure may occur and the PCC doesn't know what value is
> the one used. This value should either be explicit signalled at session
> opening or at least be included in the error message.
It is not required that the PCC know the value of the PCE's SyncTimer. In
general, if the PCC is told that the SyncTomer has expired on the PCE (and
that the previous requests have been discarded) there will be little it can
do except, maybe, simplify its request list (or lease a faster CPU from a
friend).
It should be clear that the setting of house-keeping timers is entirely an
issue for the operator, and that setting a small value is a great risk.
> 9. Section 7.14, Overload PCE indication.
>
>> Optionally, a TLV named OVERLOADED-DURATION may be included in
>> the NOTIFICATION object that specifies the period of time
>> during whichno further request should be sent to the PCE. Once
>> this period of time has elapsed, the PCE should no longer be
>> considered in congested state.
>
> This design seems to suffer from the same issue as SIP has with its
> overload
> protection. Especially if I understand it that a PCC to PCE request can
> result
> in the PCE sending its own request further to the next PCE that may be
> overloaded. In that case it seems that this mechanism only can stop the
> PCC
> from sending any request, rather than the ones that affect a particular
> upstream
> PCE.
The congestion indication is a single hop notification. So, in the case that
a downstream PCE was congested, it would inform its upstream neighbor
(acting as a PCC). The upstream neighbor does not need to report the
congestion to the originating PCC, it can simply select a different
downstream PCE.
Tools to allow an originating PCC to discover the well-being of downstream
PCEs are defined in draft-ietf-pce-monitoring-01.txt
> Secondary, for real safety against PCC->PCE congestion a mandatory
> exponential back-off for requests should be specified. That way a PCE
> that simply stops responding to request will shed load. Seems that with
> TCP this would need to be based on the response time on any request
> and some limitation on how many outstanding request the requestor may
> have.
>
> I am also worried about the lack of mandatory to implement responses
> as this likely will make it hard to determine what will happen in
> real-life
> with multiple implementations making different choices.
I think it is important to understand that the objective is not to decongest
the PCE, but to allow the PCC to know about congestion so that it can take
action. In many cases, there may be nothing the PCC can do because it has no
choice of PCE. In other cases it will be able to select a different PCE.
Note that a PCE that intends to not service requests because it is
overloaded MUST report this to the PCC, so silence is not an option.
It is true that in a significantly busy system, the congestion of a PCE may
cause multiple PCCs to switch to another PCE making it congested and
decongesting the first PCE. However, the use of the Overloaded-Duration TLV
is designed to allow this effect to be dampened. It is regarded as a network
operation and planning issue how a PCC selects which PCE to use under normal
circumstances. In view of this, the behavior when that PCE is overloaded is
clearly also an operator-dependent feature and we would expect that not all
PCCs would automatically swap to the same secondary PCE.
> 10. Discuss Discuss:
>
> This is in fact a extension on Chris's Discuss regarding the normative
> definition of the formal language used in this document.
> What tools has been used, if any, to determine the validity of the
> BNF? Has the shepherd or AD ensured that that these checks where
> run on the current version?
We are discussing with Chris what sort of reference he requires.
As in a separate thread with you, the lack of any automated check was
described in the proto write-up. The BNF used is particularly trivial.
Having looked at this in some detail and experimented with Chris' tool at
http://www.apps.ietf.org/abnf.html I conclude that the format of BNF used
here owes more to the core definition of BNF that is consistently used in
Routing Area RFCs, and very little to anything described in RFC2234 etc.
Harald's perl script is not suitable for me to run.
I found a BNF parser from Siemens at Sourceforge but it needs to be built
before it can be run. Same for other BNF syntax checkers on the Web.
The full totality of material specified in BNF in this I-D is reproduced
below. As you will see, it is trivial to check this by eye.
<Open Message> ::= <Common Header>
<OPEN>
<Keepalive Message> ::= <Common Header>
<svec-list> ::= <SVEC> [<svec-list>]
<metric-list> ::= <METRIC> [<metric-list>]
<request> ::= <RP>
<END-POINTS>
[<LSPA>]
[<BANDWIDTH>]
[<metric-list>]
[<RRO> [<BANDWIDTH>] ]
[<IRO>]
[<LOAD-BALANCING>]
<request-list> ::= <request> [<request-list>]
<PCReq Message> ::= <Common Header>
[<svec-list>]
<request-list>
<attribute-list> ::= [<LSPA>]
[<BANDWIDTH>]
[<metric-list>]
[<IRO>]
<path> ::= <ERO> <attribute-list>
<path-list> ::= <path> [<path-list>]
<response> ::= <RP>
[<NO-PATH>]
[<attribute-list>]
[<path-list>]
<response-list> ::= <response> [<response-list>]
<PCRep Message> ::= <Common Header>
<response-list>
<request-id-list> ::= <RP> <request-id-list>
<notification-list> ::= <NOTIFICATION> <notification-list>
<notify> ::= [<request-id-list>]
<notification-list>
<notify-list> ::= <notify> [<notify-list>]
<PCNtf Message> ::= <Common Header>
<notify-list>
<error-object-list> ::= <PCEP-ERROR> [<error-object-list>]
<request-id-list> ::= <RP> [<request-id-list>]
<error> ::= [<request-id-list>]
<error-object-list>
<error-list> ::= <error> [<error-list>]
<PCErr Message> ::= <Common Header>
( <error-object-list> [<Open>] ) | <error>
[<error-list>]
<Close Message>::= <Common Header>
<CLOSE>
_______________________________________________
Pce mailing list
[email protected]
https://www.ietf.org/mailman/listinfo/pce
