Subject: [spring] AD Review of draft-ietf-spring-segment-routing-ldp-interop-09

From: Alvaro Retana <[email-masked]>
Date: Wed, 20 Dec 2017 13:36:54 -0500

Dear authors:

I just finished reading this document.  I have some Major comments below
that I would like to see addressed before starting the IETF LC.

Thanks for your work on this document.

Alvaro.

Major:

M1. From Section 2: "An MCC, operating at node N, MUST ensure that the
incoming label it installs in the MPLS data plane of Node N has been
uniquely allocated to himself.”  I’m sure this sentence is not meant as a
new Normative statement for all MCCs, right?  I think that the “MUST” is
out of place since the text is really stating a fact.  s/MUST/must

M2. SRMS Definition and Operation

M2.1. Section 4.2.1. (SR to LDP Behavior) uses normative language to
describe the operation of the SRMS in ways that I think are not needed for
interoperability.

M2.1.1. "The SRMS MUST be configured by the operator in order to
advertise Node-SIDs
on behalf of non-SR nodes.”  Section 4.2 already says that "The mappings
advertised by one or more SRMSs result from local policy information
configured by the operator.”, so the sentence in 4.2.1 is at best
redundant.  In any case, what can be enforced from a specification point of
view by that “MUST”?  s/MUST/must

M2.1.2. "At least one SRMS MUST be present in the routing domain.
Multiple SRMSs
SHOULD be present for redundancy.”  These MUST|SHOULD seem to indicate a
statement of fact.  Again, from a specification point of view, what can be
enforced?  s/MUST|SHOULD/must|should. Note also that in 7.2 the text says
that "Multiple SRMSs can be provisioned in a network for redundancy.”,
which seems to be the right thing (no Normative language).

M2.2. Section 7.2. says that "a preference mechanism may also be used among
SRMSs so to deploy a primary/secondary SRMS scheme”…but no other details
are included.  This document is where the SRMS is first defined, so I would
expect this detail to also be included here.  I note that Section 3.1. (SID
Preference) of draft-ietf-spring-conflict-resolution contains the
preference specification. Please move that section to this document.

M2.3. Section 7.2 also says that: "When the SRMS advertise mappings, an
implementation SHOULD provide a mechanism through which the operator
determines which of the IP2MPLS mappings are preferred among the one
advertised by the SRMS and the ones advertised by LDP.”  First off, I think
that the SHOULD is out of place because it is not specifying any specific
action (the mechanism is not explicit). Second, this statement (with the
Normative SHOULD) is in conflict with (from 2.2. (IP2MPLS co-existence)):
"if both LDP and SR propose an IP to MPLS entry (IP2MPLS) for the same IP
prefix, then the LDP route SHOULD be selected.”  Solution: s/SHOULD/should

M3. Manageability Considerations

M3.1. The text in Section 7.1. (SR and LDP co-existence) is almost the same
as in Section 2.2. (IP2MPLS co-existence); the difference is that 7.1’s
first bullet says that "by default the LDP route MUST be selected”, while
2.2 uses SHOULD instead.  Which one is it?  Obviously, having the same text
is two places adds nothing to the document — please consolidate.

M3.2. [minor] The last bullet in 7.1/2.2 says that the "policy MAY be
locally defined.  There is no requirement that all routers use the same
policy.”  Given that in this case “all routers” really refers to the edge
nodes (at the IP2MPLS boundary), it seems like it makes sense that either
choice could be ok.  Maybe I’m wrong, but I’m guessing that giving
preference to LDP (MUST/SHOULD above) has to do with the assumption that it
is supported everywhere, while SR might not yet be…so it supports the
migration case in Section 3.  Is that a reasonable guess?  It would be
nice, to provide some justification for the default LDP preference so that
operators have a better idea of when it might be ok to use SR instead.

M4. Security Considerations.  I tend to agree that this document doesn’t
introduce anything new…but it does specify something different.  The base
SR-related advertisement by an IGP is done for the segments belonging to
the local node, but the SRMS lets a node (any node, multiple nodes) adverse
any mapping (for nodes that may be anywhere in the network) which may
result in conflicting advertisements (in the best case), or even false
ones.  Cryptographic authentication (any any other current security
mechanisms in IGPs) only verify that the information was not changed, but
it doesn’t validate the information itself, which can then lead to
conflicting and or false advertisements, which could “compromise traffic
forwarding”.  You should at least recognize that the risk exists, even if
no specific mitigation (except maybe strict configuration/programmability
control by the operator) can be mentioned.

M5. References:  These references don’t need to be Normative and can be
made Informative:
I-D.ietf-isis-segment-routing-extensions,
I-D.ietf-ospf-ospfv3-segment-routing-extensions,
I-D.ietf-ospf-segment-routing-extensions.
 OTOH, this one should be Normative: RFC5036

Minor:

P1. As with all the other SR-related documents, please take out “service
chain” from the text.

P2. Please add References for "RSVP-TE, BGP 3107, VPNv4”.   BTW, note that
rfc3107 has been obsoleted by rfc8277 — you make references to “BGP3107”
routes/label.

P3. Please define (or reference) the MPLS2MPLS, MPLS2IP and IP2MPLS
terminology (only IP2MPLS is expanded).

P4. From Section 3: “...the SR infrastructure is usable, e.g. for Fast
Reroute (FRR) or IGP Loop Free Convergence to protect existing IP and LDP
traffic.  FRR mechanisms are described in
[I-D.bashandy-rtgwg-segment-routing-ti-lfa].”
 draft-ietf-spring-resiliency-use-cases may be a better reference.

P5. Section 3: "However, any traffic switched through LDP entries will
still suffer from LDP-IGP synchronization.”  While that statement is true,
it seems out of place since there is no other discussion about LDP-IGP
synchronization anywhere — if you want to keep it, please add a reference.

P6. Sections 4 and 4.1.1 have very similar, redundant text.  To avoid
confusion, please consolidate it in one place.  This is what I’m referring
to:

4:
“  If the SR/LDP node operates in LDP ordered label distribution control
   mode (as defined in [RFC5036]), then the SR/LDP node MUST consider SR
   learned labels as if they were learned through an LDP neighbor and
   create LDP bindings for each Prefix-SID and Node-SID learned in the
   SR domain."

4.1.1:
"  A SR node having LDP neighbors MUST create LDP bindings for each
   Prefix-SID and Node-SID learned in the SR domain and, for each FEC,
   stitch the incoming LDP label to the outgoing SR label.  This has to
   be done in both LDP independent and ordered label distribution
   control modes as defined in [RFC5036]."

Note that this same text (as in 4.1.1 above) is repeated exactly in 4.2.1 —
where the SRMS is discussed.  To me, it seems out of place there (4.2.1) as
the behavior is true whether an SRMS is in use or not.  In line with the
above, it may be better to consolidate redundant text in one place —
Section 4 seems good to me.

Nits:

N1. s/This draft(s)/This document

N2. s/Ship-in-the-night/Ships-in-the-night

N3. Please don’t use “we”, use the 3rd person instead.  Just a personal
preference (= nit).

N4. s/R-LFA/RLFA

N5. Please put the reference for Option C when it is first mentioned.