Re: [Pce] Benjamin Kaduk's Discuss on draft-ietf-pce-gmpls-pcep-extensions-14: (with DISCUSS and COMMENT)

[Benjamin Kaduk <[e-mail]>]

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