[Pce] Shepherd review of draft-ietf-pce-segment-routing-12

[Dhruv Dhody <[e-mail]>]

Hi,

I am assigned the shepherded for this document. I have previously given my input and liked the direction of the update with the -12 version, but also found some new issues.

Disclaimer - I discussed some of the points with the authors F2F during the IETF meeting as well.

Major:
*******

(1) Though I appreciated the details in the new section 6.2.2. I feel the right place for this should be in 'draft-ietf-spring-segment-routing-mpls' because the source of the SID stack could be PCEP, BGP or Yang. The text related to the interpretation of the SID stack rightly belong in that draft and in this I-D should just refer it (and specify the PCEP specific error handling and procedures only).  I hope the chairs and AD of the WG could conclude on that. We could initiate a separate thread to discuss this point with spring chairs/AD.

(2) The change of the field ST (SID type) to NT (NAI type) was done in the last update. This change was done in the -12 version with the expectation that this is just a name change and would have no other impact. But since NAI is optional and might not be included, and in that case one would not be able to interpret what the SID represents when NAI is not provided. Though, it might be possible to find that information via searching the local database, there is no apparent benefit in making this change and limiting the knowledge of the SID type only when NAI is included.

Changes would be needed in various places including Section 6.2.1.

(3) There is no mention of the SR policy in the draft. This is a source of confusion, so a reference and relationship between them should be added in the Introduction.

(4) I think we should enhance the security consideration section 9 a little bit more. Since a label stack can be pushed directly for the PCE, this would have a bigger security impact then a path that gets further signaled by RSVP-TE and verified in control plane. We should talk about MSD information as well.

Minor:
*******

(1) I am not comfortable with the use of term LSDB, you would note no other SR documents uses it either.  SR architecture also allow future innovation in control plane especially in the centralized mode. May be we should use the term SR-DB introduced by the policy draft? Or a generic term such as local database?

(2) In section 3, it describes the 3 representations for SR-ERO, I have following suggestion -
OLD:
   o  An ordered set of IP addresses representing network nodes/links:
      In this case, the PCC needs to convert the IP addresses into the
      corresponding MPLS labels by consulting its Link State Database
      (LSDB).

   o  An ordered set of SIDs, with or without the corresponding IP
      addresses.

   o  An ordered set of MPLS labels and IP addresses: In this case, the
      PCC needs to convert the IP addresses into the corresponding SIDs
      by consulting its LSDB.
NEW:
   o  An ordered set of IP addresses representing network nodes/links.
      In this case, the PCC needs to convert the IP addresses into the
      corresponding MPLS labels by consulting its Link State Database
      (LSDB).

   o  An ordered set of SID index values, with or without the corresponding IP
      addresses. The PCC needs to convert the index into the corresponding
      MPLS label stack as per [I-D.ietf-spring-segment-routing-mpls].

   o  An ordered set of MPLS labels, with or without corresponding IP
      address. The top label in the label stack received from the PCE is
      from the point of view of the ingress PCC and it must follow the
      procedures as described in [I-D.ietf-spring-segment-routing-mpls]
      while pushing the label stack.

The idea behind this is to specify that PCE should prepare the label stack with the top label from the point of view of Ingress and label stack might change. The change could be an outgoing label change as per the SRGB of neighbor towards which the packet is sent; or it could be an adjacency label pop identifying the out-interface.
- In section 6.2.1, we should remove this condition requiring NAI MUST be present for the first SR-ERO sub-object and corresponding error-code.
- In Section 6.2.2.1, we should update the procedure and the next-hop is not identified from the NAI

Regarding the text in section 6.2.2, once the agreement is met to move the text to the SR-MPLS document, the text needs to be modified accordingly from the point of view of a generic MPLS Control Plane Client (MCC). And we should simply refer to it saying PCC is one such client.
Note that 'only NAI' in SR-ERO would be a unique feature for PCEP and thus section 6.2.2.3 would still be needed.


(2) In section 5.3.1, it says -
    An SR-ERO subobject consists of a 32-bit header followed by the SID
   and/or the NAI associated with the SID.

      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=36   |     Length    |  NT   |     Flags     |F|S|C|M|
     +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
     |                         SID (optional)                        |
     +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
     //                   NAI (variable, optional)                  //
     +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+

I think it is wrong to consider the first 32-bit as a header as per RFC 3209 specification.

    0                   1
    0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5
   +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-------------//----------------+
   |L|    Type     |     Length    | (Subobject contents)          |
   +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-------------//----------------+

(3) Section 6.1, the N flag indicates the capability to get SID from NAI, so it should be applicable when only NAI is sent.
OLD:
   If a PCC sets the N flag to 1, then the PCE MAY send NAI to the PCC
   within the SR-ERO subobject (see Section 6.2).  Otherwise, the PCE
   MUST NOT send NAI to the PCC.
NEW:
   If a PCC sets the N flag to 1, then the PCE MAY send NAI only to the PCC
   within the SR-ERO subobject (see Section 6.2).  Otherwise, the PCE
   MUST NOT send NAI only to the PCC.

(4) Section 6.2.1, it says -
   If a PCC receives an SR-ERO subobject in which the S bit is set to
   zero and the M bit is set to 1 (that is, it represents an MPLS label
   value), its value (20 most significant bits) MUST be larger than 15,
   unless it is a special purpose label, such as an Entropy Label
   Indicator (ELI).

Since all labels < 15 are special-purpose label, the above sentence isn't correct. Do you mean ELI is the only special purpose label that is allowed? Or we should simply say the label should be valid from PCC's point of view without worrying about special-purpose label.


Suggestions:
**************

(1) In section 3, it says -
   An PCE can update an LSP that is initially established via RSVP-TE
   signaling to use an SR-TE path, by sending a PCUpd to the PCC that
   delegated the LSP to it ([RFC8231]).  Similarly, an LSP initially
   created with an SR-TE path can be updated to use RSVP-TE signaling,
   if necessary.  This capability is useful when a network is migrated
   from RSVP-TE to SR-TE technology.

We should also add text for LSPs that are not delegated and in control with PCC, where PCC will trigger the migration and this should be reported to the PCE via the PCRpt message.

(2) In section 5.1, can we describe the value part of the sub-TLV in the order, currently we have MSD, Reserved and then flags.
Also, we should state if future extension can define new flags (and create a corresponding IANA registry) and unassigned bits in the flags MUST be set to 0.
Since MSD value 0 as a special purpose, we should mention that and provide reference to section 5.5

(3) In section 5.3.1 (M bit in Flag field), we should reference SR I-D while describing the interpretation of index.
OLD:
      *  M: If this bit is set to 1, the SID value represents an MPLS
         label stack entry as specified in [RFC3032].  Otherwise, the
         SID value is an administratively configured value which acts as
         an index into an MPLS label space.
NEW:
      *  M: If this bit is set to 1, the SID value represents an MPLS
         label stack entry as specified in [RFC3032].  Otherwise, the
         SID value is an administratively configured value which represents
         an index into an MPLS label space (either SRGB or SRLB) as per
         [RFC8402].

(4) In section 5.3.1, can we add a little more description for SID.
OLD:
SID  is the Segment Identifier.
NEW:
SID is the Segment Identifier. According to the M bit, it contains
either:

  * A 4 octet index defining the offset into an MPLS label space as
    per [RFC8402].
  * A 4 octet MPLS label stack entry where the 20 rightmost bits are
    used for encoding the label value as per [RFC3032].

(5) In section 5.3.2, It would be nice to explicitly mention the length of the NAI field in each case.

(6) Section 5.4, we should add little bit more meat to this section to clearly state what does RRO represents in case of SR-TE LSP. Currently we simply say that -

   A PCC can record an SR-TE LSP and report the LSP to a PCE via the
   RRO.

PCC reports the state of LSP to the PCE. RRO represents the actual path in the PCRpt message, thus the RRO for SR-TE represents the SID list applied by the PCC. This could be represented as MPLS label stack or as the list of indexes. In some cases, the ERO (intended path) and RRO (actual path) might be different based on local policy applied by the PCC. We should state that procedures as per [RFC8231] with no change.

Another query NAI only SR-RRO is of no use. I think we should not allow that!!

(7) Section 5.5, is METRIC object without bound allowed? I.e. a path computation request to optimize MSD valid?

(8) Section 6.2.1, it says -

   If a PCC receives an SR-ERO subobject in which the S bit is set to
   zero and the M bit is set to zero (that is, it represents an index
   value), then the SID MUST be a node-SID, an adjacency-SID or a
   binding-SID.  If the SID is not one of these types, the PCC MUST send
   a PCErr message with Error-Type = 10 ("Reception of an invalid
   object") and Error Value = TBD10 ("Bad SID type in SR-ERO").  If the
   SID is an Adjacency-SID then the L flag MUST NOT be set.  If the L
   flag is set for an Adjacency-SID then the PCC MUST send a PCErr
   message with Error-Type = 10 ("Reception of an invalid object") and
   Error-Value = 11 ("Malformed object").

Shouldn't we just say that PCC should support the SID type and leave the door open for any other SID type use for ex anycast SID or any other future SID type?

(8) Section 6.2.2.4, this error is not clear -

   o  If the PCC cannot find an SRGB in the LSDB, it MUST send a PCErr
      message with Error-Type = 10 ("Reception of an invalid object")
      and Error-Value = TBD5 ("Could not find SRGB").

Does it mean that a particular node does not support SR or did not advertise SRGB and thus PCC is unaware of the SRGB information of that node?

Nits:
*****

(1) https://www.ietf.org/tools/idnits?url=https://www.ietf.org/archive/id/draft-ietf-pce-segment-routing-12.txt - all easy to fix with a new update!
(2) s/ PATH_SETUP_TYPE/PATH-SETUP-TYPE/
(3) s/PATH_SETUP_TYPE_CAPABILITY/PATH-SETUP-TYPE-CAPABILITY/
(4) We have used the term octet mostly except for 2 instances of bytes. Suggest to use the same term.
(5) Section 8.1
OLD:
   o  When a PCC requests a path for an LSP, it can nominate a preferred
      path setup type by including it in the PATH-SETUP-TYPE TLV in the
      RP object of the PCInitiate message.
NEW:
   o  When a PCC requests a path for an LSP, it can nominate a preferred
      path setup type by including it in the PATH-SETUP-TYPE TLV in the
      RP object of the PCReq message.
(6) Section 8.3, for the first 2 bullets only L flag is mentioned. I think we should also include the N flag.
(7) The heading for section 10.1 is PCEP objects where as its content is about SE-ERO and SR-RRO sub-objects in RSVP registry!

Thanks again!

Looking forward to resolving the above points and moving the document quickly.

Regards,
Dhruv

Dhruv Dhody
Lead Architect
Network Business Line
Huawei Technologies India Pvt. Ltd.
Survey No. 37, Next to EPIP Area, Kundalahalli, Whitefield
Bengaluru, Karnataka - 560066
Tel: + 91-80-49160700 Ext 71583 II Email: [e-mail]<mailto:[e-mail]>
[Huawei-small]
This e-mail and its attachments contain confidential information from HUAWEI, which
is intended only for the person or entity whose address is listed above. Any use of the
information contained herein in any way (including, but not limited to, total or partial
disclosure, reproduction, or dissemination) by persons other than the intended
recipient(s) is prohibited. If you receive this e-mail in error, please notify the sender by
phone or email immediately and delete it!