Hi,
thanks a lot for the thorough review. I uploaded version 11 to address
your comments. See my reply inline "#Ahmed".
Thanks
Ahmed
On 4/2/18 7:26 AM, Ahmed Bashandy (bashandy) wrote:
-------- Forwarded Message --------
Subject: AD Review of draft-ietf-spring-segment-routing-ldp-interop-09
Resent-Date: Wed, 20 Dec 2017 10:37:01 -0800
Resent-From: [email protected]
Resent-To: [email protected], [email protected],
[email protected], [email protected],
[email protected]
Date: Wed, 20 Dec 2017 13:36:54 -0500
From: Alvaro Retana <[email protected]>
To: [email protected]
CC: [email protected], Martin Vigoureux
<[email protected]>, [email protected]
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
#Ahmed: done
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).
#Ahmed
I removed these two statements. Instead I modified the 3rd paragraph in
Section 4.2 to indicate that there must be at least one SRMS server in
the domain
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.
Ahmed: agreed. But since section 7.2 is under the manageability
consideration, IMO it should really not contain much specification.
Instead, I modified section 4.2.2 specify how to prefer SRMS
advertisements and removed section 7.2 completely. Section 7.2 in
version 11 is section 7.3 in version 10
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
#Ahmed:
Agreed. I replaced SHOULD with 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.
#Ahmed: I removed section 2.2 as it is very similar to section 7.1 as
you correctly pointed out. for IP2MPLS, I referred to Section 7.1. I
also added justification for preferring LDP over SR as you suggested and
changed the MUST to SHOULD
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.
#Ahmed: Agreed. Added text to indicate what you 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
Ahmed: Agreed and done
Minor:
P1. As with all the other SR-related documents, please take out
“service chain” from the text.
#Ahmed: Agreed and done
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.
#Ahmed Removed VPNv4 and added references to the others
P3. Please define (or reference) the MPLS2MPLS, MPLS2IP and IP2MPLS
terminology (only IP2MPLS is expanded).
Ahmed: Done
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.
#Ahmed: Agreed and modified as suggested
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.
#Ahmed: Agree with your assessment and removed the statement
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]."
#Ahmed: Agree with your assessment and consolidated the text in section
4 and 4.1.1 into section 4.1.1
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.
#Ahmed: Agree with your assessment and removed the statement in Section
4.2.1
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.
#ahmed: Made the changes as suggested
_______________________________________________
spring mailing list
[email protected]
https://www.ietf.org/mailman/listinfo/spring
