Subject: [Pce] WG Last Call for draft-ietf-pce-segment-routing-11

From: Julien Meuric <[email-masked]>
Date: Mon, 15 Jan 2018 10:38:10 +0100

Dear PCE WG,

Best wishes for this new year, full of interoperable specifications. Let
us begin by resuming our work in progress.

This message starts a 2-week WG last call for
draft-ietf-pce-segment-routing-11. Please send your feedback on the I-D
to the PCE mailing list by Monday January 29.

Regards,

Jon & Julien

---

From: "Adrian Farrel" <[email-masked]>
Date: Thu, 18 Jan 2018 22:54:31 -0000

Hi,

I reviewed this draft as part of the WG last call.

It is important work and needs to be published as an RFC.

However, there are some places where the clarity could be improved and
so increase the chances of interoperable implementations.

I have clustered the nits at the end of this email.

Thanks,
Adrian

=========

Section 3

   In SR networks, an ingress node of an SR path appends all outgoing
   packets with an SR header consisting of a list of SIDs (or MPLS
   labels in the context of this document).

Conventionally, "append" means to add to the end (although it can mean
to arbitrarily add). I think you want "prepend" although the sentence
is still clumsy and might be better as...

   In an SR network, the ingress node of an SR path prepends an SR 
   header to all outgoing packets.  The SR header consists of a list of
   SIDs (or MPLS labels in the context of this document).

---

Section 3 (same paragraph)

I think this is not quite true. Well, it is true that signaling is not
needed, but not true that the header has all the necessary 
information in all cases - the IGP may need to resolve a path to a
Node SID. Perhaps...

   The header has all
   necessary information so that in combination with the information 
   distributed by the IGP, the packets can be guided from the ingress
   node to the egress node of the path, and hence there is no need for
   any signaling protocol.

---

Section 3 para 2 has the same "append" problem, and also overstates what
is carried in the ERO. Suggest...

OLD
   In a PCEP session, LSP information is carried in the Explicit Route
   Object (ERO), which consists of a sequence of subobjects.  Various
   types of ERO subobjects have been specified in [RFC3209], [RFC3473],
   and [RFC3477].  In SR networks, an ingress node of an SR path appends
   all outgoing packets with an SR header consisting of a list of SIDs
   (or MPLS labels in the context of this document).  SR-TE LSPs
   computed by a PCE can be represented in one of the following forms:
NEW
   In PCEP messages, LSP route information is carried in the Explicit 
   Route Object (ERO), which consists of a sequence of subobjects.  
   Various ERO subobjects have been specified in [RFC3209], [RFC3473],
   and [RFC3477].  In SR networks, an ingress node of an SR path 
   prepends an SR header to all outgoing packets.  The SR header 
   consists of a list of SIDs (or MPLS labels in the context of this
   document).  SR-TE paths computed by a PCE can be represented in one
   of the following forms:
END

However, I wonder why you have picked just those three RFCs to reference.
Many other RFCs also define ERO subobjects. The full list is
RFC3209
RFC3473
RFC3477
RFC4874
RFC5520
RFC5553
RFC7570
RFC7898
RFC7897
...but listing them all would be odd.

---

Section 3

   o  An ordered set of IP address(es) representing network nodes/links:
      In this case, the PCC needs to convert the IP address(es) into the
      corresponding MPLS labels by consulting its Traffic Engineering
      Database (TED).

The PCC does not have a TED. It does have an LSDB as distributed by the 
IGP and this will contain the SRGBs and SIDs that allow labels to be
computed.

But I am surprised that this work is offloaded to the PCC since the PCE
has all the information to do this task. Why did the WG want to add this
option?

And then...

   o  An ordered set of SID(s)

s/SID(s)/SIDs/

This list of SIDs would need to be converted to labels by the PCC by 
applying the SRGB offsets. Again, why make the PCC do this?

And finally...

   o  An ordered set of both MPLS label(s) and IP address(es): In this
      case, the PCC needs to convert the IP address(es) into the
      corresponding SID(s) by consulting its TED.

This mixture of the previous two cases seems to compound the level of
complexity. Can't the PCE just know it is making an SR path and supply
a list of MPLS labels corresponding to the SIDs?

---

5.1.1 ---> 9

You should probably set up a registry for other SR PCE Capability sub-
TLV flags.

---
                                                 
5.1.2 ----> 6

   If a PCEP speaker receives a PATH-SETUP-TYPE-CAPABILITY TLV with a
   PST list containing PST=1, but the SR-PCE-CAPABILITY sub-TLV is
   absent, then the PCEP speaker MUST send a PCErr message with Error-
   Type 10 (Reception of an invalid object) and Error-Value TBD1 (to be
   assigned by IANA) (Missing PCE-SR-CAPABILITY sub-TLV) and MUST then
   close the PCEP session.

Suppose an implementation receives an Open with PST=1 but doesn't 
understand (or doesn't support) that value. Is it still required to
perform this check? Hopefully not.

---

5.3.1

The figure and the following text can be explicit about the value 36 
since it has already been assigned.

   Type  is the type of the SR-ERO subobject.  This document defines the
      SR-ERO subobject type, and requests a new codepoint from IANA.

---

5.3.1 (under Length) has

      As mentioned earlier, an SR-ERO
      subobject MUST have at least SID or NAI.

This is good and does tie in with what is written in earlier sections.

However, 5.3.1 starts with

   An SR-ERO subobject consists of a 32-bit header followed by the SID
   and the NAI associated with the SID.  The SID is a 32-bit number.
   The size of the NAI depends on its respective type, as described in
   the following sections.

That makes it sound like they are both mandatory, and the figure doesn't
help.

A little rewording would go a long way, and you could add "(optional)" 
to the figure.

---

5.3.1

   SID Type (ST)  indicates the type of information associated with the
      SID contained in the object body.  When ST value is 0, SID MUST
      NOT be null and NAI MUST be null.  

Does "null" mean "all zeros" or "absent"?

See also the definition of the S and F flags.

---

5.3.1

      Other ST values are described
      later in this document.

It would be friendly to provide a list somewhere.
Do you need a registry?

---

5.3.1

   Flags  is used to carry any additional information pertaining to SID.

You need to say how to set the unused bits.
Do you need a registry?

---

      *  M: When this bit is set, the SID value represents an MPLS label
         stack entry as specified in [RFC5462] where only the label
         value is specified by the PCE.  Other fields (TC, S, and TTL)
         fields MUST be considered invalid, and PCC MUST set these
         fields according to its local policy and MPLS forwarding rules.

"considered invalid"? You probably mean "ignored".

RFC5462 is not the right reference. It only renamed EXP to TC and does 
not define the other fields. You should reference 3032 and you will pick
up TC through the updates clause. (Ditto other places where the ref is
made).

---

5.3.2 has text like
      Length is 8, 20, or 24 depending on either
      SID or NAI or both are included in the subobject.

But NAI must be present or we wouldn't be talking about NAI types.
So, you need (e.g.)
      Length is 20 or 24 depending on whether
      the SID is absent or also included in the subobject.

---

5.3.2

OLD
   'IPv6 Node ID'  is specified as an IPv6 address.  In this case, ST
      and Length are 2, and Length is 8, 20, or 24 depending on either
      SID or NAI or both are included in the subobject.
NEW
   'IPv6 Node ID'  is specified as an IPv6 address.  In this case, ST
      is 2, and Length is 8, 20, or 24 depending on either SID or NAI
      or both are included in the subobject.

---

5.3.3

   If both M and C bits of an SR-
   ERO subobject are set, and if a PCEP speaker finds erroneous setting
   in one or more of TC, S, and TTL fields, it MUST send a PCErr message
   with Error-Type = 10 ("Reception of an invalid object") and Error-
   Value = 4 ("Bad label format").

But didn't the definition of the C flag say that a not MAY overwrite 
those fields according to local policy? So this is MUST send PCErr or
overwrite the fields.

---

5.3.3

   If a PCC receives a stack of SR-ERO subobjects, and the number of
   stack exceeds the maximum number of SIDs that the PCC can impose on
   the packet, it MAY send a PCErr message with Error-Type = 10
   ("Reception of an invalid object") and Error-Value = 3 ("Unsupported
   number of Segment ERO subobjects").

And if it doesn't send the PCErr, what should it do?

---

5.3.3

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

"Not identical" could have so many meanings here!
- Presumably you don't mean that all SIDs have the same value
- You might be referring to all flags
- You might be referring to just the M and C flags
- You might mean that the ERO must contain all SR-ERO subobjects or
  no SR-ERO subobjects

Please clarify and possibly rename the error value.

(See also 5.4.1).

---

5.3.3

   When a PCEP speaker receives an SR-ERO subobject in which ST is 0,
   SID MUST be present and NAI MUST NOT be present(i.e., S-flag MUST be
   0, F-flag MUST be 1, and the Length MUST be 8).  Otherwise, it MUST
   consider the entire ERO object invalid and send a PCErr message with
   Error-Type = 10 ("Reception of an invalid object") and Error-Value =
   11 ("Malformed object").  The PCEP speaker MAY include the malformed
   SR-ERO object in the PCErr message as well.

This text is good.
But it makes me think: what about the ST values 1 through 5 if the NAI is
absent?

---

========

Nits

Update to the new BCP14 (RFC2119/8174) language and place it as 
Section 2 of the document as required by RFC7322.

---

Need to expand "SR" on first use in the body of the document.

---

Section 1 para 1 has two cases where "SR architecture" should be "the
SR architecture".

---

S1P1

OLD
and is always global within SR/IGP domain
NEW
and is always identified uniquely within the SR/IGP domain
END

---

S1P1
s/e.g./e.g.,/

---

S1P1

OLD
within SR/IGP domain
NEW
an within SR/IGP domain
END

--

draft-ietf-pce-pce-initiated-lsp is now RFC8281

---

5.2 Took some time to parse...
OLD
   In order to setup an SR-TE LSP using SR, RP or SRP object MUST
   include PATH-SETUP-TYPE TLV
NEW
   In order to setup an SR-TE LSP using SR, the RP or SRP object MUST
   include PATH-SETUP-TYPE TLV
END

---

5.3 and 5.4

Please avoid saying "ERO Object" and "RRO Object"

---

Throughout the document, please don't do the optional plural thing.
E.g., "may contain one or more TLV(s)"

English likes you to use the plural even when the singular is possible.
E.g., "may contain one or more TLVs"

> -----Original Message-----
> From: Pce [mailto:[email-masked]] On Behalf Of Julien Meuric
> Sent: 15 January 2018 09:38
> To: [email-masked]
> Subject: [Pce] WG Last Call for draft-ietf-pce-segment-routing-11
> 
> Dear PCE WG,
>

---

From: Jonathan Hardwick <[email-masked]>
Date: Fri, 19 Jan 2018 10:52:28 +0000

Adrian

Many thanks for your review.  The authors will need to discuss your comments, and then we'll get back to you.

Best regards
Jon

-----Original Message-----
From: Pce [mailto:[email-masked]] On Behalf Of Adrian Farrel
Sent: 18 January 2018 22:55
To: 'Julien Meuric' <[email-masked]>; [email-masked]
Subject: Re: [Pce] WG Last Call for draft-ietf-pce-segment-routing-11

Hi,

However, there are some places where the clarity could be improved and so increase the chances of interoperable implementations.

Thanks,
Adrian

=========

Section 3

Conventionally, "append" means to add to the end (although it can mean to arbitrarily add). I think you want "prepend" although the sentence is still clumsy and might be better as...

---

I think this is not quite true. Well, it is true that signaling is not needed, but not true that the header has all the necessary information in all cases - the IGP may need to resolve a path to a Node SID. Perhaps...

   The header has all

---

Section 3 para 2 has the same "append" problem, and also overstates what is carried in the ERO. Suggest...

OLD

---

Section 3

The PCC does not have a TED. It does have an LSDB as distributed by the IGP and this will contain the SRGBs and SIDs that allow labels to be computed.

But I am surprised that this work is offloaded to the PCC since the PCE has all the information to do this task. Why did the WG want to add this option?

And then...

s/SID(s)/SIDs/

This list of SIDs would need to be converted to labels by the PCC by applying the SRGB offsets. Again, why make the PCC do this?

And finally...

This mixture of the previous two cases seems to compound the level of complexity. Can't the PCE just know it is making an SR path and supply a list of MPLS labels corresponding to the SIDs?

---

5.1.1 ---> 9

You should probably set up a registry for other SR PCE Capability sub- TLV flags.

---
                                                 
5.1.2 ----> 6

Suppose an implementation receives an Open with PST=1 but doesn't understand (or doesn't support) that value. Is it still required to perform this check? Hopefully not.

---

5.3.1

The figure and the following text can be explicit about the value 36 since it has already been assigned.

---

5.3.1 (under Length) has

That makes it sound like they are both mandatory, and the figure doesn't help.

---

5.3.1

---

5.3.1

---

5.3.1

---

RFC5462 is not the right reference. It only renamed EXP to TC and does not define the other fields. You should reference 3032 and you will pick up TC through the updates clause. (Ditto other places where the ref is made).

---

5.3.2 has text like

---

5.3.2

OLD

---

5.3.3

But didn't the definition of the C flag say that a not MAY overwrite those fields according to local policy? So this is MUST send PCErr or overwrite the fields.

---

5.3.3

---

5.3.3

(See also 5.4.1).

---

5.3.3

This text is good.
But it makes me think: what about the ST values 1 through 5 if the NAI is absent?

---

========

Nits

Update to the new BCP14 (RFC2119/8174) language and place it as Section 2 of the document as required by RFC7322.

---

---

Section 1 para 1 has two cases where "SR architecture" should be "the SR architecture".

---

S1P1

OLD
and is always global within SR/IGP domain NEW and is always identified uniquely within the SR/IGP domain END

---

S1P1
s/e.g./e.g.,/

---

S1P1

OLD
within SR/IGP domain
NEW
an within SR/IGP domain
END

--

---

---

5.3 and 5.4

---

> Best wishes for this new year, full of interoperable specifications. 
> Let us begin by resuming our work in progress.
> 
> draft-ietf-pce-segment-routing-11. Please send your feedback on the 
> I-D to the PCE mailing list by Monday January 29.

_______________________________________________
Pce mailing list
[email-masked]
https://www.ietf.org/mailman/listinfo/pce

---

From: Dhruv Dhody <[email-masked]>
Date: Tue, 30 Jan 2018 09:19:33 +0000

Hi, 

   I had reviewed and given comments on the I-D in the past, which the
   authors had addressed. I found these additional nits/suggestions.
   Apologies for being late by a day.  

   Suggestions
   -----------

   Section 1

   (1) Though it is true that a child PCE act as a PCC towards the
   parent PCE, I feel it is not wise to say the opposite, that is a PCC
   is acting as a PCE in this context. I see no advantage to bring up the
   H-PCE in this context. I suggest we remove it. 

      A PCE, or a PCC operating as a PCE (in hierarchical PCE
      environment), computes paths for MPLS Traffic Engineering LSPs
      (MPLS-TE LSPs) based on various constraints and optimization
      criteria.

   (2) Since this document is related to MPLS data plane only, it would
   be nice to include a pointer to the SRv6 work in PCEP for the benefit
   of the readers.

   (3) Regarding first mention of PST
   OLD:
      This specification relies on the procedures specified in [I-
      D.ietf-pce-lsp-setup-type].
   NEW:
      This specification relies on the procedures specified in [I-
      D.ietf-pce-lsp-setup-type] for the path setup type for SR.

   Section 3

   (4) Regarding this text - 

      SR-TE LSPs

      o  An ordered set of SID(s).

   Each SR-ERO object can include both SID and NAI (IP address); this
   case is different from the case 3 above, I suggest if some text can
   be added to make things clearer. 

   Section 5.1.1

   (5) Why SHOULD in this text?

      A PCEP speaker SHOULD indicate its support of the function described
      in this document by sending a PATH-SETUP-TYPE-CAPABILITY TLV in the
      OPEN object with this new PST included in the PST list.

   Section 6

   (6) Regarding, 

      A PCEP speaker that does not support the SR PCEP capability cannot
      recognize the SR-ERO or SR-RRO subobjects. As such, it MUST send a
      PCEP error with Error-Type = 4 (Not supported object) and Error-Value
      = 2 (Not supported object Type) as per [RFC5440].

   RFC 5440 did not state the behavior for unknown sub-object. My
   suggestion would be - 

      A PCEP speaker that does not support the SR PCEP capability and
      thus cannot recognize the SR-ERO or SR-RRO subobjects, it will
      respond according to the rules for a malformed object as per
      [RFC5440].

   Section 7

   (7) Suggest to make Manageability Consideration section as per RFC
   6123

   (8) PCEP-Yang should be mentioned in section 7.2

   Section 8
 
   (9) Suggest we expand the security consideration section a bit based
   on recent DISCUSSes.

   Nits
   ----

   Section 5.3.1

   s/MUST not/MUST NOT/

   Section 5.3.3

   (2) 
   OLD:
      A PCEP speaker that does not recognize the SR-ERO subobject in PCRep,
      PCInitiate, PCUpd or PCRpt messages MUST reject the entire PCEP
      message and MUST send a PCErr message with Error-Type=3 ("Unknown
      Object") and Error-Value=2 ("Unrecognized object Type") or Error-
      Type=4 ("Not supported object") and Error-Value=2 ("Not supported
      object Type"), defined in [RFC5440].
   NEW:
      A PCEP speaker that does not recognize or support the SR-ERO
      subobject in PCRep, PCInitiate, PCUpd or PCRpt messages MUST
      reject the entire PCEP message and MUST send a PCErr message with
      Error-Type=3 ("Unknown Object") and Error-Value=2 ("Unrecognized
      object Type") or Error- Type=4 ("Not supported object") and Error-
      Value=2 ("Not supported object Type"), defined in [RFC5440].

   (3) I agree with Adrian that the ".. not identical" needs to change.
   Since you mean all subobject in ERO must be of SR-ERO type, we should
   just call it that! (also applicable for SR-RRO).

   Thanks! 
   Dhruv

> Sent: 15 January 2018 15:08
> To: [email-masked]
> This message starts a 2-week WG last call for draft-ietf-pce-segment-
> routing-11. Please send your feedback on the I-D to the PCE mailing list
> by Monday January 29.
> 
> Regards,
> 
> Jon & Julien
>

---

From: Julien Meuric <[email-masked]>
Date: Tue, 30 Jan 2018 10:38:56 +0100

Hi all,

This LC has ended.
Adrian, Dhruv (on HAST time), thanks a lot for your valuable reviews.
Authors, please address identified issues and update the I-D.

Cheers,

Julien

---

From: Cyril Margaria <[email-masked]>
Date: Tue, 30 Jan 2018 08:48:58 -0800

Hi,

I have the following (hopefully not too late) comments/questions:

Section 5.3 ERO Object

         S: When this bit is set, the SID value in the subobject body is
         null.  In this case, the PCC is responsible for choosing the
         SID value, e.g., by looking up its TED using the NAI which, in
         this case, MUST be present in the subobject.

- What is the associated procedure if the PCC cannot resolve the NAI to a
SID? Should there be associated error codes. For instance the PCC may not
be able to resolve the NAI  at all or the resolution may fail. In case the
PCC does not support the NAI resolution, having this capability part of the
capability exchange would improve interop, as the PCE can be capable to
provide the SID.
- If Both S and F are cleared, should the PCC do the NAI resolution and
verify that the SID match? Would error codes be needed)

Thanks,
Cyril