[Pce] Benjamin Kaduk's Discuss on draft-ietf-pce-stateful-pce-p2mp-12: (with DISCUSS and COMMENT)

[Benjamin Kaduk via Datatracker <[e-mail]>]

Benjamin Kaduk has entered the following ballot position for
draft-ietf-pce-stateful-pce-p2mp-12: 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-stateful-pce-p2mp/



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

This is a comparatively minor point, but I don't think some of the
message layout is quite fully specified.  In particular, Section 7.2
discusses some new TLVs (in a confusing way, referring to the class of
two TLVs as a single nonexistent TLV) but does not always say which
object the TLVs are allowed to appear within.  (The first paragraph
seems to talk of the TLV appearing in a PCRpt, which is a message and as
such would contain only objects and not TLVs directly.)  The second
paragraph does mention the LSP object, which leads the reader to infer
that this TLV is only intended to appear in the LSP object, and only in
the PCRpt and PCUpd messages explicitly mentioned, but we probably
shouldn't force readers to make that leap.


----------------------------------------------------------------------
COMMENT:
----------------------------------------------------------------------

I've made a lot of comments about grammar nits (tagged as such), but
there are also some more substantial comments to look for.

Section 1

   [RFC4875] describes how to set up point-to-multipoint (P2MP) Traffic
   Engineering Label Switched Paths (TE LSPs) for use in Multiprotocol
   Label Switching (MPLS) and Generalized MPLS (GMPLS) networks.  The
   PCE has been identified as a suitable application for the computation
   of paths for P2MP TE LSPs ([RFC5671]).

nit: some readers might wonder who has done this identification.

Section 3.1

   [RFC8051] presents several use cases, demonstrating scenarios that
   benefit from the deployment of a stateful PCE including optimization,
   recovery, etc., which are equally applicable to P2MP TE LSPs.
   [RFC8231] defines the extensions to PCEP for P2P TE LSPs.  [...]

nit: maybe "the extensions to PCEP needed for stateful operation of P2P
TE LSPs"?  Just "the extensions" is pretty generic.

Section 5.1

   Path Computation State Report (PCRpt):  Each P2MP TE LSP State Report
      in a PCRpt message can contain actual P2MP TE LSP path attributes,

Is this "can contain" or just "contains"?

Section 5.6.3.1

   The Instantiation operation of P2MP TE LSPs is same as defined in

nit: "the same as"

   section 5.3 of [RFC8281] including handling of PLSP-ID, SYMBOLIC-

nit: comma after "[RFC8281]"

   PATH-NAME TLV etc.  Rules of processing and error codes remains

nit: comma after TLV

   unchanged.  The N (P2MP) flag (Section 7.1) MUST be set in LSP object

nit: "The rules for processing and use of error codes remain unchanged."
nit: "in the LSP object"

   in PCInitiate message by PCE to specify the instantiation is for P2MP

nit: "PCInitiate messages by the PCE to specify that the instantiation"

   TE LSPs.  Like the PLSP-ID as per [RFC8281], the P2MP-LSP-IDENTIFIERS

nit: parentheses around "as per [RFC8281]"

   TLV SHOULD NOT be included in the LSP object in PCIntiitate message

nit: "PCInitiate messages"

   (as it is generated by PCC and carried in PCRpt message instead) and
   MUST be ignored on receipt.

Section 6.1

Was it a conscious decision to depart from RFC 8231 and inline objects
in the definition of <state-report> (as opposed to keeping the <path>
intermediate construction)?

   The P2MP END-POINTS object defined in [RFC8306] is mandatory for
   specifying address of P2MP leaves grouped based on leaf types.

nit: "addresses of P2MP leaves, grouped by leaf type"

   When reporting the status of a P2MP TE LSP, the destinations MUST be
   grouped in END-POINTS object based on the operational status (O field
   in S2LS object) and leaf type (in END-POINTS).  This way, leaves of
   the same type that share the same operational status are grouped
   together.  [...]

Does this mean that it's an error for a PCRpt message to include more
than one END-POINTS object with a given value of leaf-type?  If so, it
may be worth stating that explicitly.

   For a delegated P2MP TE LSP configuration changes are reported via
   PCRpt message.  For example, adding of new leaves END-POINTS (leaf-
   type = 1) is used where as removing of old leaves (leaf-type = 2) is
   used.

nit: "is used, and removing of old leaves (leaf-type = 2) is used".

   Note that the compatibility with the [RFC8231] definition of <state-
   report> is preserved.  At least one instance of <END-POINTS> MUST be
   present in this message for P2MP LSP.

Interestingly, RFC 8231 does not spell the structure of the PCRpt
message using an <END-POINTS> object.

Section 6.2

   Note that the compatibility with the [RFC8231] definition of <update-
   request> is preserved.

If I'm reading this right, compatibility is only preserved when
<END-POINTS> is omitted (and the list only has a single endpoint/path
pair).  Perhaps some more text could be added about the nature of the
provided (and needed) compatibility?

Section 6.6.2

   The LSP State Report message is sent by a PCC to report or delegate
   the P2MP TE LSPs.  An example of a PCRpt message for a delegated P2MP
   TE LSPs is described below to add new leaves to an existing P2MP TE

nit: "TE LSP"

It might be handy to mention inline "for leaf type 1 (add)" and "leaf
type 2 (remove)" just to refresh the reader's memory.

Section 7.1

   The flags defined in this section (N, F, E flags) are used in PCRpt,
   PCUpd, or PCInitiate message.  In case of PCReq and PCRep message,
   these flags have no meaning and thus MUST be ignored.  The
   corresponding flags in the RP (Request Parameters) object are used as
   described in [RFC8231].

I'm not really finding much discussion of RP flags in RFC 8231 (as
opposed to SRP, in Section 7.2 thereof).  RFC 5440 does define a few
flags for the RP object, though.

Section 7.2

I strongly suggest changing the initial language to make it clear that
there is not a single P2MP-LSP-IDENTIFIER TLV, but rather a class of two
related TLVs that serve to identify P2MP LSPs, e.g., by referring to
"P2MP LSP Identification TLVs" (without hyphenation or all-caps).

   If the N bit is set on a PCRpt but the P2MP-LSP-IDENTIFIER TLV is

nit: the bit is set in the LSP object within the PCRpt, right?

   The P2MP-LSP-IDENTIFIERS TLV MAY be included in the LSP object in the
   PCUpd message for P2MP TE LSPs.  The special value of all zeros for
   this TLV is used to refer to all paths pertaining to a particular
   PLSP-ID.

Given that we haven't specialized into the IP version-specific TLVs yet,
I agree with the previous comments that we need greater clarity on what
the "value of all zeros" means, here.  Additionally, is that special
value supposed to be restricted to the given IP version or literally all
paths, so that there are two functionally equivalent encodings for these
semantics?

Section 9

   The PCEP extensions described in this document for stateful PCEs with
   P2MP capability MUST NOT be used if PCE has not advertised its
   stateful capability with P2MP as per Section 5.2.  If the PCEP
   Speaker on the PCC supports the extensions of this draft (understands
   the P2MP flag in the LSP object) but did not advertise this
   capability, then upon receipt of PCUpd message from the PCE, it
   SHOULD generate a PCErr with error-type 19 ("Invalid Operation"),
   error-value 12 (early allocated by IANA) ("Attempted LSP Update
   Request for P2MP if active stateful PCE capability for P2MP was not
   advertised") and terminate the PCEP session.  If the PCEP Speaker on
   the PCE supports the extensions of this draft (understands the P2MP
   flag in the LSP object) but did not advertise this capability, then

We should probably disambiguate these two "P2MP flag"s to "M flag" and
"N flag", respectively.  ("P flags" is used below.)

   If a Stateful PCE receives a P2MP TE LSP report message and the PCE
   does not understand the P2MP flag in the LSP object, and therefore
   the PCEP extensions described in this document, then the Stateful PCE
   would act as per [RFC8231].

Without a section reference, it's a little annoying to figure out
exactly what that behavior would be.

Section 14.2

One could perhaps argue that RFCs 4875, 7525, and 8253 should be
normative references.