Subject: Re: [Pce] Benjamin Kaduk's No Objection on draft-ietf-pce-segment-routing-14: (with COMMENT)

From: Benjamin Kaduk <[email-masked]>
Date: Sat, 5 Jan 2019 18:39:43 -0600

On Fri, Dec 21, 2018 at 04:59:28PM +0000, Jonathan Hardwick wrote:
> Hi Benjamin
> 
> Sorry for the delayed response.  Please find replies to your comments below.

And my apologies as well for waiting until after the holidays to
counter-respond.

> Best regards
> Jon
> 
> 
> Abstract
> 
>                    This document specifies extensions to the Path
>    Computation Element Communication Protocol (PCEP) that allow a
>    stateful PCE to compute and initiate Traffic Engineering (TE) paths,
>    as well as a PCC to request a path subject to certain constraints and
>    optimization criteria in SR networks.
> 
> Why are we talking about TE paths now instead of SR?
> 
> [Jon] TE is the primary application for SR paths instantiated by a PCE.
> 
> Section 1
> 
>    Several types of segment are defined.  A node segment represents an
>    ECMP-aware shortest-path to a specific node, and is always identified
>    uniquely within the SR/IGP domain.  [...]
> 
> A path to a node is only going to be unique if the starting node is also included, is it not?
> 
> [Jon] I suggest we reword this to make it clearer:
> 
> OLD
>    A node segment represents an
>    ECMP-aware shortest-path to a specific node, and is always identified
>    uniquely within the SR/IGP domain.
> NEW
>    A node segment uniquely identifies a specific node in the SR domain.
>    Each router in the SR domain associates a node segment with an
>    ECMP-aware shortest path to the node that it identifies.
> END

That works for me, thanks.

> Section 3
> 
> The sentences:
> 
>    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).
> 
> Appear virtually unchanged in both the start of the first paragraph and the middle of the second paragraph; is this duplication really needed?
> 
> [Jon] No - we'll remove the redundant text.
> 
>    A PCC MAY include an RRO containing the recorded LSP in PCReq and
>    PCRpt messages as specified in [RFC5440] and [RFC8231], respectively.
>    This document defines a new RRO subobject for SR networks.  The
>    methods used by a PCC to record the SR-TE LSP are outside the scope
>    of this document.
> 
> It's not entirely clear that we need to define a standard container to carry site-local data when a site-local container could also be used.
> 
> [Jon] Sorry, I don't understand your comment.  The RRO is an existing PCEP object whose subobjects are used to describe the actual path taken by an LSP.  We are adding subobjects to represent SIDs.

I'm going from memory, and it's been some time since I read the document
(so this could well be wrong), but I think I was thinking something like
"we're defining a new type of subobject to represent SIDs, but the contents
of that subobject are interpreted in a site-local manner.  How is this
different from just having site-local subobjects, with both the subobject
type and its contents being site-local matters?"  But this was just a
non-blocking comment to start with, so if it doesn't make sense (or any
other reason, really), feel free to drop the discussion -- I won't mind.

> Section 5.2
> 
> Please expand SRP (and RP, for that matter) on first usage.
> (Interestingly, https://www.rfc-editor.org/materials/abbrev.expansion.txt
> does not seem to have the correct expansion for this usage.)
> 
> [Jon] Will do.
> 
> Section 5.3.1
> 
>       *  C: If the M bit and the C bit are both set to 1, then the TC,
>          S, and TTL fields in the MPLS label stack entry are specified
>          by the PCE.  However, a PCC MAY choose to override these values
>          according its local policy and MPLS forwarding rules.  If the M
>          bit is set to 1 but the C bit is set to zero, then the TC, S,
>          and TTL fields MUST be ignored by the PCC.  The PCC MUST set
>          these fields according to its local policy and MPLS forwarding
>          rules.  [...]
> 
> Must be ignored in incoming messages received from where?
> 
> [Jon] There are two types of entity in PCEP - PCC (client) and PCE (server).  In any exchange of messages, one speaker takes the role of PCC and the other PCE.  Thus, "MUST be ignored by the PCC" implies that the messages are coming from the PCE.

Okay.

> Isn't the 'F' bit fully redundant with NT?
> 
> [Jon] The F bit determines whether NAI is appended, or not.  NT gives the type of the NAI that is appended.  The NT field has no meaning if F=0.

My (implicit) proposal was to assign a value for the NT field that means
"no NAI is present", and to treat that value of NT as meaning that F=0,
with any other value of NT meaning F=1, in which case the F bit does not
add any new information.

> Section 5.3.2
> 
> Do we need to say anything about which node is the reference point for evaluating "local" and "remote" w.r.t. IP addresses?  (Maybe we don't, since these are always unidirectional adjacencies, right?)
> 
> [Jon] Effectively, yes.  An adjacency SID is always given within the context of a particular node - it is meaningful only on that node.  So "local" and "remote" are defined by that context.

Thanks for confirming!

> Section 6.1
> 
>    If a PCC sets the N flag to 1, then the PCE MAY send an SR-ERO
>    subobject containing NAI and no SID (see Section 6.2).  Otherwise,
>    the PCE MUST NOT send an SR-ERO subobject containing NAI and no SID.
> 
> Sets the N flag in what message?
> 
> [Jon] This section is discussing capability exchange on the Open message.

Okay, I must have been reading too fast.

>                                                  If a PCE receives an
>    SR-PCE-CAPABILITY sub-TLV with the L flag set to 1 then it MUST
>    ignore the MSD field and MUST assume that the sender can impose a SID
>    stack of any depth.  [...]
> 
> nit: this second MUST seems like overkill; "and assumes" would probably work fine.  (Similarly for the following case.)
> 
> [Jon] OK
> 
> It's interesting that the routing protocols' MSD value supersedes the PCE-obtained one, given that all three (IS-IS, OSPF, and BGP-LS) documents have text to the effect of "PCEP provides a way to get this, but if PCEP is not used you can use our thing", which a naive reader would take to mean that PCEP is intended to be the primary distribution mechanism.
> 
> [Jon] Probably needs to be fixed in the other documents.  PCEP is a fairly blunt tool for specifying the MSD, which it can only do per node.  The IGPs can do better by specifying the MSD per network interface.

Ah.  Unfortunately, at least the IS-IS document is already published as an
RFC (8491) and thus immutable.  (I didn't check the others.)  I don't know
whether it would be appropriate to add this explanation into this document
since the other documents cannot (all) be changed.

> Section 7
> 
> Is there some history regarding making it MUST-level mandatory to treat top-level SR-CAPABILITY-TLV in OPEN for backwards-compatibility (as opposed to SHOULD-level but leaving open the option of a strict implementation)?
> (The guidance for what to do when both forms appear seems good to have at MUST-level, to me.)
> 
> [Jon] The impact of not doing this is that you might not interoperate with a currently fielded implementation, which seemed serious enough to me for a MUST to apply.  But I am open to discussion if you feel otherwise.

I don't have a particular argument for changing it, other than the abstract
possibility of an implementation that does not want to maintain the extra
compatibility code and will accept the resulting breakage.  Since I don't
know how likely that would be, this remains an abstract thought and
probably doesn't merit further discussion.

> Section 9
> 
> RFC 8281's security considerations incorporate those of RFC 8231 by reference; we could save the reader a step and also mention it at the toplevel here.
> 
> [Jon] OK
> 
> I don't think it's a good idea to just refer to "the security mechanisms of [RFC 5440] and [RFC8281]", since there are qualitative differences between the TCP authentication schemes and the full-on encryption provided by TLS and IPsec.  (Also, what exactly are the security mechanisms of RFC8281 supposed to be -- a quick look only sees the new guidance to apply resource
> limits?)  The second paragraph only requires integrity protection to avoid the vulnerability mentioned there, but the third paragraph requires confidentiality protection to preent the attack.
> 
> [Jon] RFC 8281 pulls in RFC 8231 in its security section, as you note above.  RFC 8231 says
> 
>    As a general precaution, it is RECOMMENDED that these PCEP extensions
>    only be activated on authenticated and encrypted sessions across PCEs
>    and PCCs belonging to the same administrative authority, using
>    Transport Layer Security (TLS) [PCEPS], as per the recommendations
>    and best current practices in [RFC7525].

That still doesn't seem to give the reader much guidance about which cases
require encryption and which cases can safely proceed with just integrity
protection (even though I'm happy to see the general recommendation to use
TLS!).

-Benjamin

> Section 10.6
> 
> RFC 8408 does not "request" the creation of the registry; IANA has already created the registry.  I would just say "[RFC8408] created a sub-registry [...]" but the smaller change to "requested" would be okay, too.
> 
> [Jon] OK.

---

From: Jeff Tantsura <[email-masked]>
Date: Sun, 6 Jan 2019 10:48:30 -0800

Hi John/Ben,

Happy New Year!

Both OSPF and IS-IS MSD documents have been published.
Wrt PCE - they merely state that if there’s no PCEP session between nodes
advertising and receiving this information, the receiving node has no other
means to learn the MSD of the advertising node, since it is local to the
node (while IGPs flood it to everyone).
Having this in the IGP drafts and PCE draft stating that if both protocols
advertise MSD, then IGP one should be preferred seemed clear enough (this
had been discussed).
>From granularity prospective - IGPs (BGP-LS) provide more details indeed as
per interface MSD could be advertised (PCEP could do node MSD only).

Hope this clarifies,
Jeff

On Sat, Jan 5, 2019 at 16:39 Benjamin Kaduk <[email-masked]> wrote:

> > Appear virtually unchanged in both the start of the first paragraph and
> the middle of the second paragraph; is this duplication really needed?
> >
> > It's not entirely clear that we need to define a standard container to
> carry site-local data when a site-local container could also be used.
> >
> > [Jon] Sorry, I don't understand your comment.  The RRO is an existing
> PCEP object whose subobjects are used to describe the actual path taken by
> an LSP.  We are adding subobjects to represent SIDs.
>
> > (Interestingly,
> https://www.rfc-editor.org/materials/abbrev.expansion.txt
> > [Jon] There are two types of entity in PCEP - PCC (client) and PCE
> (server).  In any exchange of messages, one speaker takes the role of PCC
> and the other PCE.  Thus, "MUST be ignored by the PCC" implies that the
> messages are coming from the PCE.
>
> Okay.
>
> > [Jon] The F bit determines whether NAI is appended, or not.  NT gives
> the type of the NAI that is appended.  The NT field has no meaning if F=0.
>
> > Do we need to say anything about which node is the reference point for
> evaluating "local" and "remote" w.r.t. IP addresses?  (Maybe we don't,
> since these are always unidirectional adjacencies, right?)
> >
> > [Jon] Effectively, yes.  An adjacency SID is always given within the
> context of a particular node - it is meaningful only on that node.  So
> "local" and "remote" are defined by that context.
>
> Thanks for confirming!
>
> > Section 6.1
> >
> > nit: this second MUST seems like overkill; "and assumes" would probably
> work fine.  (Similarly for the following case.)
> >
> > [Jon] OK
> >
> > It's interesting that the routing protocols' MSD value supersedes the
> PCE-obtained one, given that all three (IS-IS, OSPF, and BGP-LS) documents
> have text to the effect of "PCEP provides a way to get this, but if PCEP is
> not used you can use our thing", which a naive reader would take to mean
> that PCEP is intended to be the primary distribution mechanism.
> >
> > [Jon] Probably needs to be fixed in the other documents.  PCEP is a
> fairly blunt tool for specifying the MSD, which it can only do per node.
> The IGPs can do better by specifying the MSD per network interface.
>
> > Is there some history regarding making it MUST-level mandatory to treat
> top-level SR-CAPABILITY-TLV in OPEN for backwards-compatibility (as opposed
> to SHOULD-level but leaving open the option of a strict implementation)?
> > [Jon] The impact of not doing this is that you might not interoperate
> with a currently fielded implementation, which seemed serious enough to me
> for a MUST to apply.  But I am open to discussion if you feel otherwise.
>
> > RFC 8281's security considerations incorporate those of RFC 8231 by
> reference; we could save the reader a step and also mention it at the
> toplevel here.
> >
> > [Jon] OK
> >
> > I don't think it's a good idea to just refer to "the security mechanisms
> of [RFC 5440] and [RFC8281]", since there are qualitative differences
> between the TCP authentication schemes and the full-on encryption provided
> by TLS and IPsec.  (Also, what exactly are the security mechanisms of
> RFC8281 supposed to be -- a quick look only sees the new guidance to apply
> resource
> > limits?)  The second paragraph only requires integrity protection to
> avoid the vulnerability mentioned there, but the third paragraph requires
> confidentiality protection to preent the attack.
> >
> > RFC 8408 does not "request" the creation of the registry; IANA has
> already created the registry.  I would just say "[RFC8408] created a
> sub-registry [...]" but the smaller change to "requested" would be okay,
> too.
> >
> > [Jon] OK.
>