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

Benjamin Kaduk via Datatracker <noreply@ietf.org> Thu, 11 April 2019 11:45 UTC

Return-Path: <noreply@ietf.org>
X-Original-To: pce@ietf.org
Delivered-To: pce@ietfa.amsl.com
Received: from ietfa.amsl.com (localhost [IPv6:::1]) by ietfa.amsl.com (Postfix) with ESMTP id 701BE120048; Thu, 11 Apr 2019 04:45:55 -0700 (PDT)
MIME-Version: 1.0
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: 7bit
From: Benjamin Kaduk via Datatracker <noreply@ietf.org>
To: "The IESG" <iesg@ietf.org>
Cc: draft-ietf-pce-gmpls-pcep-extensions@ietf.org, Julien Meuric <julien.meuric@orange.com>, pce-chairs@ietf.org, julien.meuric@orange.com, pce@ietf.org
X-Test-IDTracker: no
X-IETF-IDTracker: 6.95.0
Auto-Submitted: auto-generated
Precedence: bulk
Reply-To: Benjamin Kaduk <kaduk@mit.edu>
Message-ID: <155498315544.12722.1073746104492266680.idtracker@ietfa.amsl.com>
Date: Thu, 11 Apr 2019 04:45:55 -0700
Archived-At: <https://mailarchive.ietf.org/arch/msg/pce/xzV_PtEbZl0VaE-CbQRhdGfoqEw>
Subject: [Pce] Benjamin Kaduk's Discuss on draft-ietf-pce-gmpls-pcep-extensions-14: (with DISCUSS and COMMENT)
X-BeenThere: pce@ietf.org
X-Mailman-Version: 2.1.29
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: <https://mailarchive.ietf.org/arch/browse/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: Thu, 11 Apr 2019 11:45:56 -0000

Benjamin Kaduk has entered the following ballot position for
draft-ietf-pce-gmpls-pcep-extensions-14: Discuss

When responding, please keep the subject line intact and reply to all
email addresses included in the To and CC lines. (Feel free to cut this
introductory paragraph, however.)


Please refer to https://www.ietf.org/iesg/statement/discuss-criteria.html
for more information about IESG DISCUSS and COMMENT positions.


The document, along with other ballot positions, can be found here:
https://datatracker.ietf.org/doc/draft-ietf-pce-gmpls-pcep-extensions/



----------------------------------------------------------------------
DISCUSS:
----------------------------------------------------------------------

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