[RTG-DIR] Rtgdir early review of draft-ietf-pce-segment-routing-ipv6-16

[Yingzhen Qu via Datatracker <[e-mail]>]

Reviewer: Yingzhen Qu
Review result: Has Issues

Hello
I have been selected to do a routing directorate “early” review of this draft.
 draft-ietf-pce-segment-routing-ipv6-16 - Path Computation Element
 Communication Protocol (PCEP) Extensions for Segment Routing leveraging the
 IPv6 dataplane

For more information about the Routing Directorate, please see
http://trac.tools.ietf.org/area/rtg/trac/wiki/RtgDir

Document: draft-ietf-pce-segment-routing-ipv6-16
Reviewer: Yingzhen
Review Date: 2023-04-10

Summary:
I have some minor concerns about this document that I think should be resolved
before it is submitted to the IESG. Comments: The line numbers are from idnits.

The following warning generated by idnits should be fixed:

  ** The abstract seems to contain references ([RFC8664]), which it
     shouldn't.  Please replace those with straight textual mentions of the
     documents in question.

  Checking references for intended status: Proposed Standard
  ----------------------------------------------------------------------------

     (See RFCs 3967 and 4897 for information about using normative references
     to lower-maturity documents in RFCs)

  == Unused Reference: 'RFC7855' is defined on line 1090, but no explicit
     reference was found in the text

  == Unused Reference: 'RFC8354' is defined on line 1101, but no explicit
     reference was found in the text

  == Unused Reference: 'RFC8666' is defined on line 1112, but no explicit
     reference was found in the text

  == Unused Reference: 'RFC8667' is defined on line 1116, but no explicit
     reference was found in the text

  == Outdated reference: draft-ietf-lsr-isis-srv6-extensions has been
     published as RFC 9352

  == Outdated reference: A later version (-21) exists of
     draft-ietf-pce-pcep-yang-20

# Detailed questions and nits:

120        As defined in [RFC8402], Segment Routing (SR) architecture allows
121        that the source node to steer a packet through a path indicated by an
Nits:
allows that the source node to

127        is called SR-MPLS.  When SR architecture is applied to the IPv6 data
Nits:
When the SR architecture

131        A SR path can be derived from an IGP Shortest Path Tree(SPT), but SR-
s/A SR path/An SR path

207        SR Path:  IPv6 Segment List (List of IPv6 SIDs representing a path in
208           IPv6 SR domain in the context of this document)
Should this be SRv6 Path?

228        LSP State Report (PCRpt) messages defined in defined in [RFC8231].
Nits: extra words

[RFC8664] also defines a new Record Route Object(RR0) called SR-RRO
s/RR0/RRO (cap “O” not number ”0”)

233        This document define new subobjects "SRv6-ERO" and "SRv6-RRO" in the
s/define/defines

296        capability.  If a PCEP speaker includes PST=3 in the PST List of the
297        PATH-SETUP-TYPE-CAPABILITY TLV then it MUST also include the SRv6-
298        PCE-CAPABILITY sub-TLV inside the PATH-SETUP-TYPE-CAPABILITY TLV.

324        SRv6-PCE-CAPABILITY sub-TLV and the presence of the sub-TLV indicates
325        the support for the SRv6 paths in PCEP.
Question:
A bit confusion here: If the presence of the SRv6-PCE-CAPABILITY sub-tlv
indicates the support of the SRv6 paths, does it mean this SRv6-PCE-CAPABILITY
sub-tlv can exist without PST=3?

353     4.2.  The RP/SRP Object

355        In order to indicate that the path is for SRv6, any RP or SRP object
356        MUST include the PATH-SETUP-TYPE TLV specified in [RFC8408].  This
357        document defines a new Path Setup Type (PST=3) for SRv6.
Suggested text:
This document defines a new Path Setup Type (PST=3) for SRv6. In order to
indicate that the path is for SRv6, any RP or SRP object MUST include the
PATH-SETUP-TYPE TLV as specified in [RFC8408], where PST is set to 3.

362        inclusion in the in ERO.
Nits

462        associated with the SRv6 SIDs.  This information is optional.  This
463        information is optional.
Nits: Duplicate sentences

467        SRv6 SID: SRv6 Identifier is an 128-bit IPv6 addresses representing
s/addresses/address

473        At least one of the SRv6-SID or the NAI MUST be included in the
s/one of the/one

533        PCE could also be used for verification and the automation for
534        securing the SRv6 domain by provisioning filtering rules at SR domain
535        boundaries as described in Section 5 of [RFC8754].
Suggested text:
PCE can also be utilized to verify and automate the security of the SRv6 domain
by provisioning filtering rules at the domain boundaries as described in
Section 5 of [RFC8754].

545        To allow for future compatibility, any optional element added to the
546        SRv6-ERO subobject in future MUST specify the order of the optional
547        element and request IANA to allocate a flag to indicate its presence
548        from the subregistry created in Section 9.2.
Suggested text:
In order to ensure future compatibility, any optional elements added to the
SRv6-ERO subobject in the future must specify their order and request the
Internet Assigned Numbers Authority (IANA) to allocate a flag to indicate their
presence from the subregistry created in Section 9.2.

594        The ordering of optional elements in the SRv6-RRO subobject as same
s/as/is

616        The number of SRv6 SIDs that can be imposed on a packet depends on
617        the PCC's IPv6 data plane capability
major:
I don’t think this sentence is accurate. Although it’s true that the head end
node can impose a large number of SIDs based on its capability, for the path
setup, PCE has to consider all the endpoint nodes capabilities to ensure the
SRH can be processed appropriately. The section related with MSD needs some
clarifications.

641        Furthermore, whenever a PCE learns the other SRv6 MSD types that may
s/the other/any other

646        OA PCE MUST NOT send SRv6 paths with a number of SIDs exceeding that
647        SRv6 MSD value (based on the SRv6 MSD Type).
s/OA/A

664        TLV in an outbound message to a PCC.  Si SRv6-PCE-CAPABILITY sub-TLVs
665        in an Open message, it processes only themilarly, a PCC MUST ignore
666        N,X flag and any (MSD-Type,MSD-Value) pair received from a PCE.  If a
667        PCE receives multiple first sub-TLV received.
Please rewrite these two sentences.

671        The ERO processing remains as per [RFC5440] and [RFC8664].
Suggested text:
The processing of ERO remains unchanged in accordance with both [RFC5440] and
[RFC8664].

676        recognize the SRv6-ERO or SRv6-RRO subobjects, it will respond
677        according to the rules for a malformed object per [RFC5440].
Suggested text:
It should respond according to the rules for a malformed object as described in
[RFC5440].

712        consider the entire ERO invalid and send a PCErr message with Error-
Question:
Why no “MUST” for this PCErr message? I understand the "MUST" can be for both
considering the ERO invalid and sending a PCErr message, but better to keep the
language consistent through the document.

719        consider the entire ERO invalid and send a PCErr message with Error-
720        Type = 4 ("Not supported object") and Error-value = 4 ("Unsupported
Same as above.

729        In case a PCEP speaker receives the SRv6-ERO subobject, when the PST
s/the SRv6-ERO/ an SRv6-ERO

767     6.  Security Considerations
Please consider to add “Section 2.5 of RFC6952” and  RFC8253.

Thanks,
Yingzhen