[Pce] WGLC review of draft-ietf-pce-sr-p2mp-policy-19

[Dhruv Dhody <[e-mail]>]

Hi,

As a WG participant...

I thank the authors for addressing my previous comments and working to
prepare this I-D for publication.

I want to highlight a few remaining points that have not been updated to
gauge the group's feedback on these items as well as some newly spotted
issues:

1.  Appendix A (
https://www.ietf.org/archive/id/draft-ietf-pce-sr-p2mp-policy-19.html#appendix-A)
I still find this appendix with a bit level encodings problematic and an
unnecessary deviation from standard norms. I prefer the existing example
mechanisms used in previous P2MP RFCs, such as RFC 8623 (
https://www.rfc-editor.org/rfc/rfc8623.html#section-6.6) This standard
approach allows objects and TLVs to evolve without the examples going out
of sync.

Hooman previously responded to this, noting that this style of packet
sample has helped many readers implement the draft, but mentioned a
willingness to remove them if there is a large objection from the WG. I
remain unconvinced that the benefit outweighs the maintenance risk and
would appreciate feedback from other WG participants.

2.  Appendix B: This section requires additional textual explanation to
accompany the figures. Some issues include:

  - The first PCRpt message, which is unsolicited, should not have SRP-ID=1.
  - In the case of PCInitiate with PLSP-ID=1, the text states "PLSP-ID:
value MUST be set to zero and will be assigned by PCC."
  - RFC 9960 and Section 4.3.4 recommend installing downstream state before
the root, so why is the root RS downloaded first in the PCE-Initiated
workflow?
  - The MBB workflow seems to mishandle both the instance-ID (as it is
always b) and the CC-ID. Please check this thoroughly.

3.  Introduction: The text states, "As per [RFC9960] a P2MP service can be
realized by two types of a P2MP Trees, Ingress Replication or a P2MP tree."
It is confusing to state there are two types of P2MP trees and then call
one of those types simply a "P2MP tree."

4.  Terminology: Some descriptions feel too loose for a Proposed Standard

  - The use of the term "variant" is confusing. We should be explicit that
a new object type is being defined in this document. We do not typically
refer to each object-type as a variant; they are still the same object.
  - The text claims that the Instance-ID is equivalent to the LSP ID, it is
better to state that they serve a similar purpose during MBB.
  - The text states "[RFC3209] Defines the instance-ID", but I could not
find that term there.
  - The text states "These PTIs are equivalent to sub-lsps (instance-IDs)."
However, a sub-lsp in P2MP is S2L—how is that the same as an instance-id
which indicates the PTI?

5.  Section 5.1 Figure: The Length=4 value indicated in the figure is
incorrect.

6.  Section 5.1 Text: "Upon the receipt of an Open message, the receiving
PCEP peer MUST determine whether the suggested PCEP session characteristics
(leaf-types) are acceptable." Since there is no way to indicate support for
leaf-types, did you intend to reference a different characteristic here?
Also, the number of instances is listed as something to negotiate. The text
states a maximum of two instances, but the required behavior when the value
is >2 or 0 needs more clarity.

7.  Section 4.3.1: "The PCInitiate message sent to the root MUST set the
Tree-ID to 0. If not the PCC must send a PCEP Error message (PCErr) with
Error-Type = X2 (SR P2MP Policy General Error) and Error value = 1 (PCInit
Invalid Root-ID)." Did you mean "Tree-ID" instead of "Root-ID" in the error
description?

8.  In <sr-p2mp>::=<ENDPOINT>; change <ENDPOINT> to <END-POINTS>.

9. Section 5.5.2: "A SR P2MP PCRpt MAY mix different types of Leaf nodes by
including several P2MP END-POINTS objects." This is not compatible with our
RBNF, which specifies <sr-p2mp>::=<ENDPOINT> (allowing only one).

10.  Section 4.3.3.1: "If not the PCC must send a PCEP Error message
(PCErr) with Error-Type = X2 (SR P2MP Policy General Error) and Error value
= 2 (Invalid PLSP-ID)." Why not reuse Error-Type 19 (Invalid Operation) and
Error-Value 8 (Non-zero PLSP-ID in LSP Initiate Request)? There are other
errors that can be easily reused instead of defining new ones.

11. As Samuel noted, the Instance-ID length and allocation responsibility
need to be fixed.

Thanks!
Dhruv