On Thu, Apr 11, 2019 at 01:23:37PM +0100, Adrian Farrel wrote:
> As a general response, Ben, I would say that PCEP is only catching up with 
> GMPLS in this document. Thus, everything it is doing wrt bandwidth has been 
> built for some while in GMPLS RSVP-TE implementations.
> 
> It is probable that more careful references to the GMPLS signalling RFCs 
> would address some of your points. Specifically RFC 3473 and RFC 7025. But I 
> think section 2.3 already does this, so I'm not quite sure where to make this 
> clarification.

(I only see it referencing 7025 but not 3473.)

And it looks like only the LOAD-BALANCING object, that tries to enforce
that the component TE-LSPs each contribute some minmium bandwidth, is
going to require much thinking.  Any other text changes on this point ought
to be fairly mechanical.

> FWIW, I have never heard of anyone applying Intserv approaches to GMPLS. 
> Reservations are not statistical, and in most cases the resources are 
> physical quantities and assigned directly.

My main complaint here is that we're going from a scalar bandwidth number,
that can use regular arithmetic comparisons (trichotomy) and have a clear
sense of whether a minimum threshold value is achieved, to a more
complicated structure (a mathematician might think of it as a vector
space), where we no longer have a single clear metric or trichotomy
comparison between different vectors in the space.  If we said that, even
though the generalized bandwidth structure is this larger "vector" thing,
the load-balancing object was going to still just use a single scalar for
minimum bandwidth (and we defined how to compute the scalar bandwidth from
a given generalized bandwidth structure/vector), this issue would
evaporate.  The problem arises when we take the same generalized bandwidth
structure and try to use it (the structure/vector) as the minimum threshold
value.  Do I compare the components piece by piece and require each
individual component to surpass the stated minimum?  That is well-defined
in a procedural sense but may not make practical sense for all the
generalized bandwidth structures defined now or in the future.

-Ben

> Cheers,
> Adrian
> 
> --------------
> 
> This document makes some well-needed extensions to existing PCEP
> concepts such as bandwidth, but I'm not convinced that the way they
> interact with existing PCEP functionality is sufficiently well specified
> to admit interoperable implementation.  Specifically, we introduce the
> generalized bandwidth structures and reuse that encoding for the
> generalized load balancing structures, which includes a notion of
> "minimum bandwidth specification".  But now that the bandwidth
> specification is a compound data structure instead of a scalar type,
> it's not guaranteed that we have a strict linear ordering with
> well-defined minimum.  If we consider the specific case of Intserv, do I
> insist upon all three of the minimum bucket rate, minimum bucket size,
> and minimum peak data rate?  Or perhaps I only care about the peak data
> rate and not the bucket size/rate.  We need more text in order to
> specify what "minimum" actually means/measures.
> 
> Similarly, I'm not sure all the referenced generalized bandwidth
> types/traffic parameters in Section 2.3 clearly indicate which
> structures/fields we are to incorporate by reference (see COMMENT).
> 
> Section 2.1.2 says:
> 
>    GMPLS-CAPABILITY TLV it is RECOMMENDED that the PCC does not make use
>    of the objects and TLVs defined in this document.
> 
> Why is this not "the PCC MUST NOT make use of the objects and TLVs
> defined in this document"?  Ignoring the peer's (non-)advertisement and
> plowing ahead seems like a recipe for non-interoperability.
> 
> Section 2.5.1 notes that:
> 
>      <p2mp-endpoints> ::=
>        <endpoint> [<endpoint-restriction-list>]
>        [<endpoint> [<endpoint-restriction-list>]]...
> 
> 
>    For endpoint type Point-to-Multipoint, several endpoint objects MAY
>    be present in the message and each represents a leave, exact meaning
>    depend on the endpoint type defined of the object.
> 
> If all <endpoint>s represent leaves, then how is the head node
> specified?
> 
> I couldn't find a full spcification for some of the fields in the XRO
> Label subobject (Section 2.7) by chasing the indicated references (see
> COMMENT).
> 
> 
> ----------------------------------------------------------------------
> COMMENT:
> ----------------------------------------------------------------------
> 
> Section 1
> 
> Please expand OTN and WSON on first use.
> 
> Section 1.4
> 
> It's very unclear to me what kind of support, from/by what entities/data
> structures, under what conditions, these tables are attempting to
> indicate.
> 
> We should probably be consistent whether we talk about just "FOO" or
> "FOO object" as the hanging text for these bulleted lists.
> 
>    From [RFC8282]:
> 
>    o  SWITCH-LAYER: address requirements (1, 2 and 3) for the TE-LSP and
>       indicates which layer(s) should be considered, can be used to
>       represent the RSVP-TE generalized label request.  [...]
> 
> nit: this looks like a comma splice.
> 
>    The PCEP extensions defined later in this document to cover the gap
>    are:
> 
>       Two new object types are introduced for the BANDWIDTH object
>       (Generalized bandwidth, Generalized bandwidth of existing TE-LSP
>       for which a reoptimization is requested).
> 
> I'm confused by this language "new object types are introduced for the
> BANDWIDTH object".  My understanding was that objects did not nest: that
> is, objects have a given structure and can sometimes contain TLVs, but
> do not contain other objects.  So, my current understanding is that new
> objects are introduced that can appear where the BANDWIDTH object would
> previously have appeared, but they are separate object (type)s from the
> RFC 5440 BANDWIDTH objects.  (This language is used in the next couple
> items as well.)  To be clear, this is at most an editorial
> consideration, essentially whether to use "introduced for" or something
> like "introduced akin to".
> 
> Section 2.1.2
> 
>                                                   If the PCE does not
>    include the GMPLS-CAPABILITY TLV in the OPEN message and the PCC does
>    include the TLV, it is RECOMMENDED that the PCC indicates a mismatch
>    of capabilities.  Moreover, in case that the PCC does not receive the
> 
> Indicate how, to whom?
> 
> Section 2.2
> 
> This granularity applies to all links in the path, right?  So I can't
> request label-level granularity for one hop and indicate that I only
> care about node-level granularity for the other hops?
> 
> Section 2.3
> 
> [similar comments apply here to what I mentioned at the end of Section
> 1.4]
> 
>    The Bw Spec Type correspond to the RSVP-TE SENDER_TSPEC (Object Class
>    12) C-Types
> 
> Should we ask IANA to update the SENDER_TSPEC registry to note that it
> is used for PCEP as well as RSVP?
> 
>    The encoding of the fields Generalized Bandwidth and Reverse
>    Generalized Bandwidth is the same as the Traffic Parameters carried
>    in RSVP-TE, it can be found in the following references.
> 
>                       Object Type Name      Reference
> 
>                       2           Intserv   [RFC2210]
>                       4           SONET/SDH [RFC4606]
>                       5           G.709     [RFC4328]
>                       6           Ethernet  [RFC6003]
>                       7           OTN-TDM   [RFC7139]
>                       8           SSON      [RFC7792]
> 
> It's quite confusing to have the table heading be just "object type"
> when this is the value in the field named "Bw Spec Type" and corresponds
> to class type values in the SENDER_TSPEC registry.
> 
> Also, I looked up the Intserv case, and RFC 2210 doesn't really give me
> a clear picture of what I'm supposed to encode as the "transport
> parameters".  I think it's supposed to be the 12-octet assembly
> consisting of the token bucket rate, token bucket size, and peak data
> rate, but I have very low confidence in that assessment.  On the other
> hand, RFC 4606 has a very nice data structure layout in Section 2.1,
> "SONET/SDH Traffic Parameters".  On the gripping hand, there's not a
> clear "bandwidth" number in that structure that I can apply a comparison
> to for load-balancing purposes.  It doesn't look like I'll have time to
> check the other four cases right now, but that will need to be done
> before final publication.
> 
> Section 2.4
> 
> I'm having trouble parsing:
> 
>    The LOAD-BALANCING object [RFC5440] is used to request a set of
>    maximum Max-LSP TE-LSP having in total the bandwidth specified in
>    BANDWIDTH, each TE-LSP having a minimum of bandwidth.
> 
> Is it intended to read:
> 
>    The LOAD-BALANCING object [RFC5440] is used to request allocation of a set 
> of
>    at most Max-LSP TE-LSPs, having in total the bandwidth specified in
>    BANDWIDTH, with each TE-LSP having at least a specified minimum bandwidth.
> 
> ?
> 
> [similar comments apply here to what I mentioned at the end of Section
> 1.4]
> 
>    Bandwidth Spec Length (16 bits): the total length of the Min
>    Bandwidth Spec field.  It is to be noted that the RSVP-TE traffic
>    specification MAY also include TLV different from the PCEP TLVs.  The
>    length MUST be strictly greater than 0.
> 
> It's not entirely clear to me why the note about different TLVs in
> RSVP-TE and PCEP belongs here.
> 
> Section 2.5.1
> 
>               Endpoints label restriction may not be part of the RRO or
>    IRO, they can be included when following [RFC4003] in signaling for
>    egress endpoint, but ingress endpoint properties can be local to the
>    PCC and not signaled.  [...]
> 
> nit: the first comma looks like a comma splice.
> 
>                       A PCE not supporting a given Endpoint Type SHOULD
>    respond with a PCErr with Error Type 4, Value TBD "Unsupported
>    endpoint type in END-POINTS Generalized Endpoint object type".  [...]
> 
> s/TBD/TBA-15/
> 
>                                              The TLVs present in the
>    request object body MUST follow the following [RFC5511] grammar:
> 
> It feels a bit like a type error to use RBNF to describe the layout
> of TLVs within a TLV block, as RBNF acts on objects.
> 
> Section 2.5.2.4
> 
>    The LABEL-REQUEST TLV indicates the switching capability and encoding
>    type of the following label restriction list for the endpoint.  Its
>    format and encoding is the same as described in [RFC3471] Section 3.1
>    Generalized label request.  [...]
> 
> Presumably the "Its" refers to just the value portion of the TLV?
> That should probably be stated explicitly.
> 
> Section 2.5.2.5
> 
> Is there any reason for the section title to not be "LABEL-SET TLV" for
> consistency with the other sections?
> 
>    A LABEL-SET TLV represents a set of possible labels that can be used
>    on an interface.  If the L bit is cleared, the label allocated on the
>    first endpoint MUST be within the label set range.  [...]
> 
> Is this MUST binding on the PCC that generates a request, or on the
> computed LSP returned by the PCE?
> 
>    A LABEL-SET TLV with the O and L bit set MUST trigger a PCErr message
>    with error type="Reception of an invalid object" error value="Wrong
>    LABEL-SET TLV present with O and L bit set".
> 
>    A LABEL-SET TLV with the O bit set and an Action Field not set to 0
>    (Inclusive list) or containing more than one subchannel MUST trigger
>    a PCErr message with error type="Reception of an invalid object"
>    error value="Wrong LABEL-SET TLV present with O bit and wrong
>    format".
> 
>    If a LABEL-SET TLV is present with O bit set, the R bit of the RP
>    object MUST be set, otherwise a PCErr message MUST be sent with error
>    type="Reception of an invalid object" error value="LABEL-SET TLV
>    present with O bit set but without R bit set in RP".
> 
> nit: I don't know if it makes more sense to use the TBA-25, TBA-26, and
> TBA-24 values in these descriptions.
> 
> Section 2.6
> 
>    The IRO as defined in [RFC5440] is used to include specific objects
>    in the path.  RSVP-TE allows to include label definition, in order to
>    fulfill requirement 13 of [RFC7025] the IRO needs to support the new
>    subobject type as defined in [RFC3473]:
> 
> nit: this looks like a comma splice.  (A similar construction appears in
> Section 2.7 as well.)
> 
> Section 2.7
> 
>       U (1 bit): see [RFC3471].
> 
>       C-Type (8 bits): the C-Type of the included Label Object as
>       defined in [RFC3471].
> 
>       Label: see [RFC3471].
> 
> Sorry, where exactly in RFC 3471?  I do not see discussion of a U bit
> or C-Type therein.  (Perhaps RFC 3473 was intended?  Though, RFC 3473
> seems to refer back to 3471 for the U parameter, again without section
> reference.)
> 
> Section 6
> 
> It seems that a malicious PCC might be able to effect a denial of
> service attack on the PCE by attempting to make many requests that
> consume lots of resources (whether on the PCE itself or in the managed
> network elements).
> 
>                  In addition Technology specific data plane mechanism
>    can be used (following [RFC5920] Section 5.8) to verify the data
>    plane connectivity and deviation from constraints.
> 
> nit: "In addition, technology-specific"
> 
> Appendix A
> 
> It's not entirely clear to me why this specific group of examples was
> chosen and no others.  (The appendix does not seem to be referenced from
> elsewhere in the document, so it appears fairly random to a reader
> making it that far.)
> 
> 

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

Reply via email to