Re: [Pce] WG Last Call of draft-ietf-pce-vendor-constraints-10

Julien Meuric <julien.meuric@orange.com> Wed, 03 July 2013 16:04 UTC

Return-Path: <julien.meuric@orange.com>
X-Original-To: pce@ietfa.amsl.com
Delivered-To: pce@ietfa.amsl.com
Received: from localhost (localhost [127.0.0.1]) by ietfa.amsl.com (Postfix) with ESMTP id 0F77421F9CAD for <pce@ietfa.amsl.com>; Wed, 3 Jul 2013 09:04:57 -0700 (PDT)
X-Virus-Scanned: amavisd-new at amsl.com
X-Spam-Flag: NO
X-Spam-Score: -6.249
X-Spam-Level:
X-Spam-Status: No, score=-6.249 tagged_above=-999 required=5 tests=[BAYES_00=-2.599, HELO_EQ_FR=0.35, RCVD_IN_DNSWL_MED=-4]
Received: from mail.ietf.org ([12.22.58.30]) by localhost (ietfa.amsl.com [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id Qelm+NXNxULs for <pce@ietfa.amsl.com>; Wed, 3 Jul 2013 09:04:52 -0700 (PDT)
Received: from r-mail1.rd.orange.com (r-mail1.rd.orange.com [217.108.152.41]) by ietfa.amsl.com (Postfix) with ESMTP id 7596D21F9CB1 for <pce@ietf.org>; Wed, 3 Jul 2013 09:04:52 -0700 (PDT)
Received: from r-mail1.rd.orange.com (localhost.localdomain [127.0.0.1]) by localhost (Postfix) with SMTP id 8F723DE4005 for <pce@ietf.org>; Wed, 3 Jul 2013 18:06:33 +0200 (CEST)
Received: from ftrdsmtp2.rd.francetelecom.fr (unknown [10.192.128.47]) by r-mail1.rd.orange.com (Postfix) with ESMTP id 83882DE4002 for <pce@ietf.org>; Wed, 3 Jul 2013 18:06:33 +0200 (CEST)
Received: from ftrdmel10.rd.francetelecom.fr ([10.192.128.44]) by ftrdsmtp2.rd.francetelecom.fr with Microsoft SMTPSVC(6.0.3790.4675); Wed, 3 Jul 2013 18:04:49 +0200
Received: from [10.193.71.100] ([10.193.71.100]) by ftrdmel10.rd.francetelecom.fr with Microsoft SMTPSVC(6.0.3790.4675); Wed, 3 Jul 2013 18:04:49 +0200
Message-ID: <51D44BA0.4090800@orange.com>
Date: Wed, 03 Jul 2013 18:04:48 +0200
From: Julien Meuric <julien.meuric@orange.com>
Organization: Orange
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:17.0) Gecko/20130623 Thunderbird/17.0.7
MIME-Version: 1.0
To: pce@ietf.org
References: <51B82C91.3050304@orange.com> <51C8747F.50604@orange.com> <523C37072C291347B9730C9291CCA07D09269C@DB3PRD0411MB427.eurprd04.prod.outlook.com> <51CABA62.3080703@cttc.es>
In-Reply-To: <51CABA62.3080703@cttc.es>
Content-Type: text/plain; charset="ISO-8859-1"; format="flowed"
Content-Transfer-Encoding: 8bit
X-OriginalArrivalTime: 03 Jul 2013 16:04:49.0291 (UTC) FILETIME=[0B790DB0:01CE7807]
Subject: Re: [Pce] WG Last Call of draft-ietf-pce-vendor-constraints-10
X-BeenThere: pce@ietf.org
X-Mailman-Version: 2.1.12
Precedence: list
List-Id: Path Computation Element <pce.ietf.org>
List-Unsubscribe: <https://www.ietf.org/mailman/options/pce>, <mailto:pce-request@ietf.org?subject=unsubscribe>
List-Archive: <http://www.ietf.org/mail-archive/web/pce>
List-Post: <mailto:pce@ietf.org>
List-Help: <mailto:pce-request@ietf.org?subject=help>
List-Subscribe: <https://www.ietf.org/mailman/listinfo/pce>, <mailto:pce-request@ietf.org?subject=subscribe>
X-List-Received-Date: Wed, 03 Jul 2013 16:04:57 -0000

Hi.

Besides the previous comments already raised, mostly on section 2, I 
would add the following:

----------
Title + Abstract + Introduction
-----
s/Path Computation Element Protocol/Path Computation Element 
Communication Protocol/ [x3]
----------
Section 2
-----
s/flag is clear, then as defined in [RFC5440],/flag is clear then, as 
defined in [RFC5440],/
----------
Section 5
-----
Another protocol name without "communication", but no need to add it 
there since it is the current name of the registry...
----------
Section 6
-----
s/SHUOLD/SHOULD/ [already mentioned by Robert]
----------


Best regards,

Julien


Le 26/06/2013 11:54, Ramon Casellas a écrit :
> El 26/06/2013 9:40, Margaria, Cyril (Coriant - DE/Munich) escribió:
>> Support.
>>
>> I have a few remarks :
>>   1) The <svec-list> definition does include the definitions from 
>> RFC5541 and RFC5557, the RFC5557 did not include the [<metric-list>] 
>> element, should this fixed by RFC5557 errata to match the 
>> pce-vendor-constraints definition?
>>
>>   2) This document include the XRO in the svec-list, but not in the 
>> <request>, where should it go in the <request>?
>>
>>   3) the Path key expansion requests, nor monitoring are considered, 
>> is it OK? Basically should a new document take into account all the 
>> existing RFCs grammar element or should it cherry pick (and based on 
>> which criteria)
>>
>>   4) RFC5440/RFC6006/pce-vendor-constraints compatibility:
>>       RFC5440 indicates that the object order MUST be followed, 
>> RFC6006 did change the object order defined in RFC5440:
>>          - RRO list and RRO bandwidth should follow the <ENDPOINTS>, 
>> not the <metric-list>,
>>          - 3 BANDWITH objects are allowed
>>          - [<OF>] is before the [<LSPA>]  in RFC6006 but after the 
>> [<metric-list>] in RFC5541
>>      pce-vendor-constraints does  use the [<OF>] definition of 
>> RFC5541 (after [<metric-list>]), add the [<RRO>] (without 
>> [<BANDWIDTH>])  before the [<IRO>]( in contrary to RFC6006, which put 
>> the RROs after the endpoints)
>>
>> The object order are not consistent, which is not very good for 
>> implementation (need to support the different  variation)
>>
>>   I understand the need for a different grammar in RFC6006, my 
>> preference would be to define one p2p grammar and one p2mp grammar 
>> (as this is in the RP this is not perfect, but OK from implementation 
>> point of view) (as in gmpls-pcep-extensions).
>> For the other points  think this could  be covered by erratas in the 
>> original documents.
>
> Dear all,
>
> I share Cyril's comments. In my humble opinion, we are more and more 
> having a set of documents with grammars selecting only some objects 
> from existing documents and not covering them all and, once integrated 
> in a single implementation, it becomes harder to make sense of all 
> them (e.g. "bandwidths" made worse later with also 
> GENERALIZED-BANDWIDTHs). Ordering constraints are also a problem that 
> would need to be addressed.
>
> Below a minor review. The draft is quite clear and self-explanatory 
> (all my comments are minor) .
>
>
> OLD
>   The object can be present in two places within the PCReq message to
>   enable it to apply to a single path computation request or to a set
>   of synchronized requests
>
> Sorry to be picky, but I count three :-) within a SVEC, within an 
> individual request both as a direct constraint and within the endpoint 
> rro pair-list. Maybe just
>
> NEW
>    The object can be present to enable it to apply to a single path
>    computation request or to a set of synchronized requests.
>
>
>
> * I would, if not too much effort, separate the case where a PCE does 
> not parse/understand the VENDOR-INFORMATION object from the case it 
> does not support a given Enterprise number. In other words, the draft 
> could specify the procedures when a PCE finds a VERDOR-INFORMATION 
> object with an Enterprise number it does not understand (as done for 
> TLV, Section 2.1 deals with the case of "unknown object".) Maybe 
> something in the lines of "If the Enterprise Number is unknown to the 
> PCE, the PCE (...)".  I think it could be useful to have a dedicated 
> error code for that. Alternatively Just add the text "If the 
> Enterprise Number is unknown to the PCE, it MUST treat that object as 
> Unknown" but I like this less. The curent text "if the P flag is set, 
> the object will be treated as mandatory and the request will either be 
> processed using the contents of the object" somehow covers it 
> implicitly, thuogh but I would like to see it explicitly written.
>
> OLD
>
> - The PCE determines how to interpret the information in the Vendor
>   Information object by examining the Enterprise Number it contains.
>
> NEW
>
> - The PCE determines how to interpret the information in the Vendor
>   Information object by examining the Enterprise Number it contains.
>   If the Enterprise Number is unknown to the PCE, it MUST treat that
>   object as an Unknown object.
>
>
>
> Note that the TLV text currently states
>
> - The PCE determines how to interpret the Vendor Information TLV by
>   examining the Enterprise Number it contains.  If the Enterprise
>   Number is unknown to the PCE, it MUST treat the Vendor Information
>   TLV as an unknown TLV
>
>
>
>
> * <request> ::= lacks a [<XRO>] after [<IRO>] since XRO is mentioned
>
>
>
> * <request> ::= I would suggest moving the vendor-info-list after 
> endpoints (for both p2p and p2mp) My personal preference would be 
> after metric list and objective function. endpoint-rro-pair-list at 
> least includes one mandatory ENDPOINTS object, making the mandatory RP 
> and ENDPOINTS objects appear first.
>
>
> * The draft states "Thus, the PCReq message based on [RFC6006] is 
> encoded as follows". Much like RFC6006, the draft is ignoring the BNC 
> object in the grammar. Also it is not clear where in the PCReq it 
> should appear. RFC6006 also says that "The object can only be carried 
> in a PCReq message.  A Path Request may carry at most one Branch Node 
> Object". But I am not sure how to specify a branch node list and a 
> non-branch node list.
>
>
> * The draft just says "The Vendor Information object can be included 
> in a PCRep message in exactly the same way as any other object as 
> defined in [RFC5440]" I would suggest to provide a suggested 
> grammar/ordering which includes not only the vendor-information-list 
> but also othe RFC6006 extensions notably regarding 
> end-point-path-pair-list and ERO/SERO. As a bare minimum the draft 
> should refer to RFC6006 regarding to PCRep message instead of RFC5440.
>
>
> * As RFC6006, the draft is ignoring the UNREACH-DESTINATION object and 
> is not present in the PCRep grammar
>
>
>
> Other "philosophical" comments
> =============================
>
> Although I understand it is inherited from RFC6006,  it is unfortunate 
> that that we keep the name of endpoint-rro-pair-list, since it is more 
> and and more losing its meaning of a (endpoint, rro) pair list. 
> Dreaming on, I also believe it could useful to split into a P2P 
> grammar and a P2MP grammar, roughly as follows (as Cyril mentioned 
> this was also suggested for GMPLS extensions and clarifies RFC6006. In 
> any case, an implementation needs to parse the RP object to know if it 
> is a p2p or p2mp)
>
> <request> ::= <expansion> | <p2p_computation> | <p2mp_computation>
>
> <expansion> ::= <RP> <PATH-KEY>
>
> <p2p_computation> ::= <RP><ENDPOINTS> [<attributes>]
>
> <attributes> ::= CLASSTYPE LSPA BANDWIDTH metric-list 
> objective-functions vendor-info-list rro-bw-pair IRO BNC XRO 
> LOADBALANDING ... (all optional and parsed in any order for interworking)
>
> <p2mp_computation> ::= <RP><tree-list>[<attributes>]
>
> <tree-list> ::= <tree> [<tree-list>]
>
> <tree> ::= <ENDPOINTS> <rro_bw_pair> etc.
>
> Finally, as a a personal comment which I echoed when the draft was 
> polled, and althgouh I support this draft, I still hope that the 
> objects and TLVs defined in this document are not overused and that 
> objective functions, related metrics, and constraints in general are 
> defined following open processes.
>
>
> Thanks
> R.
> _______________________________________________
> Pce mailing list
> Pce@ietf.org
> https://www.ietf.org/mailman/listinfo/pce
>