Adrian Farrel skrev:
> 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."
> 

Okay, sounds good.

>> 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).

Thanks, I think that is good to clarify for interoperability.

> 
>> 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.

I saw it. From my perspective, it works but having a unclear definition
has both some risk of interoperability issues and reduce the utility of
the session ID. But, I did want to understand that you actually had
considered these issues.

> 
>> 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.

Agreed. One way of doing that, especially if you are allowed to answer
the requests out of order then you need unique identifiers in some
context. So your proposal for clarification seems to cover what needs to
happen.

> 
> 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?

Yes, all valid questions needing resolution.

> 
> 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!

Agreed, looking forward to updated text.

> 
>> 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.)

I have worked a bit with specifying bandwidth limits for applications.
As packets are discrete and lower layers have some behavior regarding to
how they send them. Thus, there are some temporal affects on bit-rate.
That is why you need to say something about over what time interval the
bit-rate is valid. I think what you are talking about with bandwidth is
something that delivers an average over some reasonable long time scale
> 100 ms) in relation to the link layers being used. That is why I 
would like to see a clearer definition of what is meant with bandwidth.
I have to reflect that even RFC 2210 definition is not super clear.
Especially when one are talking about something that are trying to
specify a flow that are a smaller fraction of the total data moving
capability of the path. In these cases the token bucket rate is what you
are allowed send on average as long you trying to utilize it to 100%,
the bucket size the burstiness allowed and the Peak DAta rate is limited
by the link interface. Even that is not clear only in bits/s instead it
has timing that depends on the actual interface technology and how that
works.

That is why I would prefer that one at least reference what definition
one is using. Section 3.1.2 in RFC 3471 is also not providing a definition.

> 
>> 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).
> 

Great.

>> 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.

Thanks, my lack of RSVP-TE knowledge is showing. But I still think a ref
would be good in the document.

> 
>> 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.

Okay, maybe some explanation of that is needed. Both in regards to
setting this timer and what response times that you really are expecting.

> 
>> 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.

Good, that should fix this.

> 
>> 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.

You mean for accepted requests. But for any heavily loaded PCE it could
in fact refuse to receive data.

> 
> 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.

Thanks, this clarifies a lot on how you where considering this going to
work. To, me it seems that it still a system that only is intended to
work by good capacity engineering and the potential for complete 
collapse in failure or high load cases does still exist. I don't quite 
find that acceptable, are you willing with living with such a state?

> 
>> 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.

Good that you are working on that. I am using Bill's web parser that is
quite good for checking RFC 2234 ABNF.

http://www.fenron.com/~fenner/abnf.cgi


> 
> 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.

I don't agree that it is trivial, but I agree that it is possible.
However, that doesn't help me if I don't know the rules. Which is why
you need a definition of the language.

> 
> 
> <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>
> 
> 
> 


Cheers

Magnus Westerlund

IETF Transport Area Director & TSVWG Chair
----------------------------------------------------------------------
Multimedia Technologies, Ericsson Research EAB/TVM
----------------------------------------------------------------------
Ericsson AB                | Phone +46 8 4048287
Färögatan 6                | Fax   +46 8 7575550
S-164 80 Stockholm, Sweden | mailto: [EMAIL PROTECTED]
----------------------------------------------------------------------


_______________________________________________
Pce mailing list
[email protected]
https://www.ietf.org/mailman/listinfo/pce

Reply via email to