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

["Adrian Farrel" <[e-mail]>]

Very belatedly...

Thanks Julien. Making all these changes in the next revision.

Adrian

> -----Original Message-----
> From: [e-mail] [mailto:[e-mail]] On Behalf Of Julien
> Meuric
> Sent: 03 July 2013 17:05
> To: [e-mail]
> Subject: Re: [Pce] WG Last Call of draft-ietf-pce-vendor-constraints-10
> 
> 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
> > [e-mail]
> > https://www.ietf.org/mailman/listinfo/pce
> >
> 
> _______________________________________________
> Pce mailing list
> [e-mail]
> https://www.ietf.org/mailman/listinfo/pce