[Pce] Adoption poll review of draft-negi-pce-segment-routing-ipv6

["Adrian Farrel" <[e-mail]>]

Hi,

As this document is being polled for adoption, I thought it might be
useful to do a review.

My comments are not blocking for adoption, but can be addressed in a 
version if/when the draft becomes a WG document.

Thanks for the work.
Adrian

===

Abstract

No need to write "Segment Routing (SR)" twice.
s/Since, Segment/Since Segment/
s/This draft/This document/

---

Abstract

Add a final sentence saying "Base protocol extensions and mechanisms to
support MPLS-SR are described in a separate document."

---

You need to expand "ERO" on first use.
Try to avoid saying "ERO object" (that would be EROO? ;-)

---

Section 3.

   This document extends the "SR-ERO subobject" defined in
   [I-D.ietf-pce-segment-routing] to carry IPv6 SID(s) (IPv6 Addresses).
   SRv6-capable PCEP speakers MUST be able to generate and/or process
   this.

I don't think "extends" is the right word. I think you define a new
subobject type.

Similarly...

   o  Update the SR-ERO and SR-RRO sub-object for SRv6

I don't think "Update" is the right word. Again, you are defining a new
subobject type.

---

3.3.1.1

Will you need an IANA registry for the SRv6-PCE-CAPABILITY flags?

---

3.3.1.1 makes I-D.bashandy-isis-srv6-extensions a normative reference

---

3.3.1.2 Unclear why the PCE must set the L bit since the whole TLV can
be ignored by the PCC. Actually, unclear why the TLV must be present 
when sent by the PCE.

---

3.3.3.1

It is really confusing to show a field that is not there!

But, also, to model what is in I-D.ietf-pce-segment-routing, I think
you should keep the NAI as part of the core sub-object definition.

It might be better if this section looked something like the following.

3.3.3.1.  SR-ERO Subobject


   [I-D.ietf-pce-segment-routing] defines the SR-ERO subobject.  The
   Node or Adjacency Identifier Type (NAI Type or NT) field indicates
   the type and format of the NAI contained in the body of the 
   subobject.

   This document defines a new NT with value TBD3 to indicate an SRv6
   SID.

   When the NT indicates SRv6, then the SR-ERO subobject represent a 
   SRv6 segment.  In this case, the optional SID and NAI fields MUST NOT
   be included, but the subobject MUST include an SRv6I (SRv6
   Identifier) field.  The format of SR-ERO when the NT value is TBD3 is
   shown in Figure 2.


    0                   1                   2                   3
    0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1
   +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
   |L|    Type     |     Length    |  NT   |     Flags     |F|S|C|M|
   +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
   |                                                               |
   |                      SRv6 Identifier                          |
   |                         (128-bit)                             |
   |                                                               |
   +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
   | SRv6NT|         Flags           |       Function Code         |
   +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
   //                        NAI (variable)                       //
   +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+


                     Figure 2: SR-ERO Subobject Format

   For SRv6 segments, a new NT (NAI Type) is assigned by IANA as TBD3.

   The description of all the flags and fields is as per
   [I-D.ietf-pce-segment-routing].  For SRv6 segments (when NT is TBD3),
   M and C flag MUST NOT be set.  The S flag MUST be set.  The F bit
   MUST NOT be set.  If these flags are not set properly, the subobject
   MUST be considered malformed and the PCEP speaker react as per the
   error handling described in Section 3.3.3.2.

   The SRv6 Identifier is the 128 bit IPv6 addresses representing the 
   SRv6 segment.

   The SRv6NT is the SRv6 NAI Type which indicates the interpretation
   for NAI (Node or Adjacency Identifier).  Values are as per the NT
   field defined in [I-D.ietf-pce-segment-routing], but the value
   TBD3 MUST NOT be used.

   Flags is a 12 bit field.  No flag bits are currently defined in
   this document.  This MUST be set to 0 and ignored on receipt.

   Function Code is a 16 bit field representing supported functions
   associated with SRv6 SIDs.  This information is optional and 
   included only for maintainability.  That is, no action is required of
   the PCC based on this field.  The following function codes are 
   defined:

      0: Reserved

      1: End Function

      2: End.DX6 Function

      3: End.DT6 Function

      4: End.X Function

   The NAI field is modelled on [I-D.ietf-pce-segment-routing] and
   contains the NAI associated with the SRv6 Identifier.  Depending on
   the value of SRv6NT, the NAI can have different formats.

      When the SRv6NT value is 0, the NAI MUST NOT be included.

      When SRv6NT value is 1, the NAI is as per the 'IPv6 Node ID'
      format defined in [I-D.ietf-pce-segment-routing], which specify
      an IPv6 address.  This is used to identify the owner of the
      SRv6 Identifier.  This is optional, as the LOC (the locater
      portion) of the SRv6 SID serves a similar purpose.

      When SRv6NT value is 2, the NAI is as per the 'IPv6 Adjacency'
      format defined in [I-D.ietf-pce-segment-routing], which specify
      a pair of IPv6 addresses.  This is used to identify the IPv6
      Adjacency and used with the SRv6 Adj-SID.

      [Editor's Note - Add IPv6 unnumbered adjacency, once done by
      [I-D.ietf-pce-segment-routing]]

---

Similarly, in 3.3.4.1, it is confusing to include a field that isn't
there.  So maybe....

3.3.4.1.  SR-RRO Subobject

   For SRv6 segments, a new NT (NAI Type) is assigned by IANA as TBD3,
   the format of SR-RRO sub object remains the same as the SR-ERO
   subobject, but without the L flag [I-D.ietf-pce-segment-routing].

   When the NAI Type (NT) indicates SRv6, then the SR-RRO subobject
   represent a SRv6 segment and include a field SRv6I (SRv6 Identifier)
   in place of NAI (Node or Adjacency Identifier) defined in
   [I-D.ietf-pce-segment-routing].  The 32 bit SID MUST NOT be included.
   The format of SR-RRO subobject is reproduced with the SRv6I field as
   shown in Figure 4.


    0                   1                   2                   3
    0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1
   +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
   |      Type     |     Length    |  ST   |     Flags     |F|S|C|M|
   +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
   |                                                               |
   |                      SRv6 Identifier                          |
   |                         (128-bit)                             |
   |                                                               |
   +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
   | SRv6NT|         Flags           |       Function Code         |
   +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
   //                        NAI (variable)                       //
   +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+

                     Figure 4: SR-RRO Subobject Format

   The description of all fields and flags is as per SR-ERO subobject.

   Processing rules of SR-RRO subobject are identical to those of SR-ERO
   subobject.

   If a PCE detects that all subobjects of RRO are not identical, and if
   it does not handle such RRO, it MUST send a PCErr message with Error-
   Type = 10 ("Reception of an invalid object") and Error-Value = 10
   ("Non-identical RRO subobjects").


...But some of this text is odd!
- The processing of the RRO is identical to the ERO? Surely not.
- The RRO subobjects have to be identical? Identical to what?