Thanks for thorough (and VERY clear) the review
See inline #Ahmed
Ahmed
On 6/15/18 11:08 PM, Alexander Vainshtein wrote:
Re-sending to correct SPRING WG list.
Sincere apologies for the delay caused by a typo.
Thumb typed by Sasha Vainshtein
------------------------------------------------------------------------
*From:* Alexander Vainshtein
*Sent:* Sunday, June 10, 2018 10:43:52 AM
*To:* [email protected];
[email protected]
*Cc:* [email protected]; [email protected]; '[email protected]';
'[email protected]'; Jonathan Hardwick
([email protected]); [email protected]
*Subject:* RE: RtgDir Early review:
draft-ietf-spring-segment-routing-mpls-13
Explicitly adding Shraddha who is the shepherd of this draft.
Regards,
Sasha
Office: +972-39266302
Cell: +972-549266302
Email: [email protected]
*From:* Alexander Vainshtein
*Sent:* Friday, June 8, 2018 5:43 PM
*To:* '[email protected]' <[email protected]>;
'[email protected]'
<[email protected]>
*Cc:* '[email protected]' <[email protected]>; [email protected];
[email protected]; '[email protected]' <[email protected]>
*Subject:* RtgDir Early review: draft-ietf-spring-segment-routing-mpls-13
Hello,
I have been selected to do a routing directorate “early” review of
this draft:
https://datatracker.ietf.org/doc/draft-ietf-spring-segment-routing-mpls/
The routing directorate will, on request from the working group chair,
perform an “early” review of a draft before it is submitted for
publication to the IESG. The early review can be performed at any time
during the draft’s lifetime as a working group document. The purpose
of the early review depends on the stage that the document has
reached. As this document is currently in the WG Last call, my focus
for the review was to determine whether the document is ready to be
published. Please consider my comments along with the other working
group last call comments.
For more information about the Routing Directorate, please see
http://trac.tools.ietf.org/area/rtg/trac/wiki/RtgDir
*Document*: draft-ietf-spring-segment-routing-mpls-13
*Reviewer*: Alexander (“Sasha”) Vainshtein
([email protected]
<mailto:[email protected]>)
*Review Date*: 08-Jun-18
*Intended Status*: Proposed Standard.
*Summary*:
I have some minor concerns about this document that I think should be
resolved before it is submitted to the IESG.
*Comments*:
I consider this draft as an important companion document to the
Segment Routing Architecture
<https://tools.ietf.org/html/draft-ietf-spring-segment-routing-15>
draft that, ideally, should augment definitions of the Segment Routing
(SR) notions and constructs given there with details specific for the
SR instantiation that uses the MPLS data plane (SR-MPLS). Many issues
raised in my review reflect either gaps that should be, but have not
been, closed, or inconsistencies between the two drafts.
Since RFC 8287 <https://tools.ietf.org/html/rfc8287> is already
published as a Standards Track RFC, I expect such augmentation to be
backward compatible with this document (or to provide clear
indications of required updates to this document). And I include the
MPLS WG into distribution list.
This draft was not easy reading for me. In particular, the style of
Section 2.5 that discusses at length and in some detail multiple
“corner cases” resulting, presumably, from misconfiguration, before it
explains the basic (and relatively simple) “normal” behavior, looks
problematic to me.
The WG Last Call has been extended by one week. Nevertheless, I am
sending out my comments
*Major Issues*: None found
#Ahmed: thanks a lot
*Minor Issues*: Quite a few but, hopefully, easy to resolve.
1.*Encapsulation of SR-MPLS packets*:
a.RFC 3032 (referenced by the draft) and RFC 5332 (*/not mentioned in
the draft/*) depend two encapsulations of labeled packets - one for
Downstream-allocated labels and another for Upstream-allocated ones.
#Ahmed: RFC5332 is for multicast. As mentioned in Section 6 of
draft-ietf-spring-segment-routing-15, multicast is outside the scope of
SR. Hence the RFC was not referred to in the SR-MPLS draft
b.From my POV the ST-MPLS only uses Downstream-allocated labels – but
I expect the draft to state that explicitly, one way or another. (If
Upstream-allocated labels are relevant for SR-MPLS, I would see it as
a major gap, so I hope that this is not the case).
#Ahmed: I will add a statement in section 2.2 to mention that it is
down-stream allocated as you mentioned
2.*Label spaces in SR-MPLS*:
a.RFC 3031 (referenced by the draft) defines per-platform and
per-interface label spaces, and RFC 5331 (*/not mentioned in the
draft/*) adds context-specific label spaces and context labels.
b.The draft does not say which of these are or are not relevant for
SR-MPLS
c.From my POV:
i.Labels representing all kinds of SIDs mentioned in the draft MUST be
allocated from the per-platform label space only
ii.At the same time, instantiation of Mirror Segment IDs defined in
Section 5.1 of the Segment Routing Architecture draft using MPLS data
plane clearly calls for context labels and context-specific label spaces
d.I expect the draft to provide a clear-cut position on these aspects
of SR-MPLS.
#Ahmed: I will add a statement to section 2.2 to say that the it is
per-platform. Regarding the function "mirroring", SR attaches a
*function* to each SID. The "mirroring" function is already described in
Section 5.1 of draft-ietf-spring-segment-routing and is not specific to
the MPLS forwarding plane. Hence there is no need to re-mention it here
because this document is trying to be as specific as possible to the
MPLS forwarding plane. General functions attached to SID are described
in the segment routing architecture document or future documents.
Furture documents proposing new SR function must be as specific and
clear as possible
3.*SR-MPLS and hierarchical LSPs*:
a.SR LSPs that include more than one segment are hierarchical LSPs
from the POV of the MPLS data plane. Therefore some (possibly, all) of
the models for handling TTL and TC bits that have been defined in RFC
3443 (*/not mentioned in the draft/*) should apply to SR-MPLS
b.RFC 8287 (*/not referenced in the draft/*) specifically discussed
operation of the LSP Traceroute function for SR LSPs in the case when
Pipe/Short Pipe model for TTL handling is used
c.I expect the draft to provide at least some guidelines regarding
applicability of each specific model defined in RFC 3443 (separately
for TTL and TC bits) to SR-MPLS.
#Ahmed: BY design, the instantiation of SR over the MPLS forwarding
plane (and hence this draft) does not modify the MPLS forwarding plan
behavior as it is mentioned in the first sentence in Section 1. So the
TTL behavior specified in rfc3443 is already implied and there is no
need to re-mention it here just like all aspects of MPLS forwarding.
RFC8287 is OAM-specific. SR-OAM is handled in a separate document so is
outside the scope of this draft
4.*Inferring network layer protocol in SR-MPLS*:
a.I wonder if the draft could provide any details on the situation
when a label that represents some kind of SID is the bottom-of-stack
label to be popped by the egress LER
#ahmed: This is part of the "Next" function. It is described in detail
in this document.
b.For the reference, RFC 3032 says that “the identity of the network
layer protocol must be inferable from the value of the label which is
popped from the bottom of the stack, possibly along with the
contents of the network layer header itself”
c.From my POV the following scenario indicates relevance of this
expectation for SR-MPLS:
i.IS-IS is used for distributing both IPv4 and IPv6 reachability in a
given domain
ii.An IS-IS adjacency over some dual-stack link is established, and a
single Adj-SID for this adjacency is advertised
iii.The node that has assigned and advertised this Adj-SID receives a
labeled packet with the label representing this Adj-SID being both the
top and bottom-of-stack label
iv.The implementers must be given unambiguous instructions for
forwarding the unlabeled packet via the dual-stack link as an Ipv4 or
an IPv6 packet.
#Ahmed: If you take a look at the SR-ISIS , SR-OSPFv2 and SR-OSFv3
drafts, you will see all 3 protocol advertise different adj-SIDS for
IPv4 next-hop and IPv6 next-hop. For example, ISIS uses the "F-Flag"
(section 2.2.1 in draft-ietf-isis-segment-routing-extensions-18) to
specify whether the adj-SID is for IPv4 and IPv6. Similarly, the SR-ISIS
draft attaches a prefix-SID to the prefix advertisement and hence
implies the identity of the protocol underneath the bottom most label.
For any other "function" attached to a SID, it is part of the
specification of this function to describe what happens when the SID is
represented by a label in the MPLS forwarding plane and this label is
the bottom most label
5.*Resolution**of Conflicts*: Are the
a.Are the conflict resolution procedures listed in section 2.5
mandatory to implement?
b.If they are mandatory to implement, are they also mandatory to
deploy, or can the operators simply treat any detected conflict as
requiring human intervention and preventing normal operation of SR-MPLS?
#Ahmed: They are recommended. I will modify the paragraph after the
first 3 bullets in Section 2.5 to say that it is recommeded.
c.For the reference, the IETF capitalized MUST appears just in a few
places in Section 2.5, and each appearance has very narrow context:
i.For MCCs where the "Topology" and/or "Algorithm" fields are not
defined, the numerical value of zero MUST be used for these two fields
ii.If the same set of FECs are attached to the same label "L1", then
the tie-breaking rules MUST always select the same FEC irrespective of
the order in which the FECs and the label "L1" are received. In other
words, the tie-breaking rule MUST be deterministic.
iii.An implementation of explicit SID assignment MUST guarantee
collision freeness on the same router
From my POV, it is not possible to infer the answer to my question
from these statements. Some explicit statement is required.
#Ahmed: I agree with you POV and as mentioned in my reply to items (a)
and (b), I will modify the paragraph to say that it is RECOMMENDED to
answer you questions in items (a) and (b)
d.The tie-breaking rules in section 2.5.1 include some specific values
for encoding FEC types and address families – but these values are not
supposed to appear in any IANA registries (because the draft does not
request any IANA actions). Can you please clarify what is so special
about these values?
#Ahmed: There is no significance to the values but there is a
significance to the order among them. I will modify the text to clarify that
e.I also doubt that comparison of FECs that represent IPv4 and IPv6
prefix SIDs makes much sense (for conflict resolution or else),
because, among other things, there are valid scenarios when an IPv4
/32 prefix is embedded in an IPv6 /128 one.
#Ahmed: A prefix-SID is assigned to a prefix. An IPv6 prefix that embeds
an IPv4 prefix is different from the IPv4 prefix. The specifications of
SR extensions to ISIS, OSPFv2, OSPFv3, and BGP treat IPv4 and IPv6
prefixes separately, including the IPV6 prefixes with embedded IPv4
ones. Besides not all IPv6 prefixes embed IPv4 prefix in them. Hence the
distinction between IPv4 and IPv6 prefixes is quite clear
f.Section 2.5.1 defines 3 types of SR-MPLS FECs, but I am not sure all
SID types defined in the Segment Routing Architecture draft can be
unambiguously mapped to one of these types. Problematic examples
include at least the following:
i.Parallel Adjacency SID
ii.Mirror SID
Explicit mapping of SID types to SR-MPLS FEC types would be most
useful IMO. If some SID types cannot be mapped to SR-MPLS FECs, this
must be explicitly stated in the draft.
#Ahmed:
Parallel adjacency SID are handled in the type "(next-hop, outgoing
interface)"
Mirror SID is a type of binding-SID as mentioned in Section 5.1 in the
SR architecture draft (draft-ietf-spring-segment-routing-15). Also as
described in Section 2.4 draft-ietf-isis-segment-routing-extensions-18
(also see the equivalent in the OSPFv2 and OSPFv3 draft), a binding SID
is identified by a prefix. Hence it is covered by the type "(Prefix,
Routing Instance, Topology, Algorithm)"
6.*Node SIDs in SR-MPLS*:
a.Node SIDs are explicitly defined and discussed in the Segment
Routing Architecture draft but are not mentioned even once in this draft
b.AFAIK, the common implementation practice today includes assignment
of at least one Node SID to every node in the SR-MPLS domain
c.Is there a requirement to assign at least one Node SID per {routing
instance, topology, algorithm} in SR-MPLS? If not, can the authors
explain expected behavior of such a node? (See also my comment about
routing instances below).
#Ahmed: A Node-SID is a special case of prefix-SID. So there nothing
specific about it from the MPLS forwarding plane point of view.
Similarly from a standard tracks draft point of view, there is no
requirement to assign a SID to every prefix just like there is no
requirement to bind every prefix to an LDP label. Common and/or
recommended practices or description of deployment scenarios are more
befitting to BCP or informational drafts. This draft is a standards
track draft
If a {routing instance, topology, algorithm} is not assigned a SID, then
this FEC is totally irrelavant to this draft and hence describing how a
node treats it is totally outside the scope of this draft
7.*SRGB Size in SR-MPLS*:
a.The draft correctly treats the situation when an index assigned to
some global SID cannot be mapped to a label using the procedure in
Section 2.4 as a conflict.
b.At the same time the draft does not define any minimum size of SRGB
(be it defined as a single contiguous block or as a sequence of such
blocks) that MUST be implemented by all SR-capable nodes
c.I suspect that lack of such a definition could be detrimental to
interoperability of SR-MPLS solutions. AFAIK, the IETF has been
following, for quite some time, a policy that some reasonable
MUST-to-implement defaults should be assigned for all configurable
parameters exactly in order to prevent this.
#Ahmed: This document specifies how the SRGB is used and the behavior of
routers when a prefix-SID index maps to a label inside and/or outside
the SRGB. The actual size of the SRGB is a task in partitioning the
label space, which is very specific to a particular deployment scenario.
So IMO it is outside the scope of a standards track document. Now that
SR-MPLS is deployed in many places, I expect the community to gain
sufficient experience to recommend (or not recommend) a particular
minimum/maximum size for the SRGB is some future informational or BCP
draft/rfc
8.*Algorithms and Prefix SIDs*:
a.The draft mentions Algorithms (as part of SR-MPLS Prefix FEC) in,
but it does not explicitly link them with the Prefix-SID algorithms
defined in section 3.1.1 of the Segment Routing Architecture draft
#Ahmed: I will just add the reference [I-D.ietf-spring-segment-routing]
right beside the first time "Algorithm" is mentioned
b.From my POV, the draft should explicitly state that the default
Prefix-SID algorithm MUST be implemented in all SR-MPLS-compliant routers.
#Ahmed: The specification of what path calculation method should or must
be supported is a routing protocol property not a forwarding plane
property. In fact, the choice of a path calculation method or algorithm
is completely orthogonal to the routed protocol. Hence mandating the
support of a particular routing algorithm is beyond the scope of this
document.
c.The Segment Routing Architecture draft states (in section 3.1.3)
that “Support of multiple algorithms applies to SRv6”. But neither
draft states whether multiple algorithms apply to SR-MPLS. Can you
please clarify this point?
#Ahmed: The last paragraph of Section 3.1.2 titled SR-MPLS in
draft-ietf-spring-segment-routing-15 discusses the support of multiple
algorithms. So it is implied that the concept of algorithm applies to
SR-MPLS. Hence there is no need to re-mention it here
9.*Routing instances and the context for Prefix-SIDs*:
a.The Segment Routing Architecture draft states in Section 3.1 that
the “context for an IGP-Prefix segment includes the prefix, topology,
and algorithm”
b.This draft seems to define (in section 2.5) the context for the
Prefix SID as “Prefix, Routing Instance, Topology, Algorithm” where ”a
routing instance is identified by a single incoming label downloader
to FIB” (but the notion of the label downloader to FIB is not defined).
c.These two definitions look different to me.
d.At the very least I would expect alignment between the definitions
of context for the Prefix-SID between the two drafts. Preferably, the
definition given in the Segment Routing Architecture draft should be
used in both drafts.
#Ahmed: The context of the section 2.5 is limited to the resolution of
local label collision. The use of "routing instance" in section 2.5 is
just there for tie-breaking if there is local label collision.
10.*Example of PUSH operation in Section 2.10.1*:
a.The first para of this section begins with the sentence “Suppose an
MCC on a router "R0" determines that PUSH or CONTINUE operation is
to be applied to an incoming packet whose active SID is the global SID
"Si"”. In the context of SR-MPLS this means (to me) that the incoming
packet is a labeled packet and its top label matches the global SID “Si”.
b.However, the example for PUSH operation in the next para of this
section is the case of an (unlabeled) IP packet with the destination
address covered by the IP prefix for which “Si” has been assigned.
c.From my POV:
i.Mapping unlabeled packets to SIDs is indeed out of scope of the
draft. Therefore an example of a PUSH operation that is applied to a
labeled packet (with the active SID inferred from the top label in the
stack) is preferable.
ii.Valid examples of PUSH operation applied to a labeled incoming
packet can be found in Sections 4.2 or 4.3 of the TI-LFA
<https://tools.ietf.org/html/draft-bashandy-rtgwg-segment-routing-ti-lfa-04>
draft
#Ahmed: I do not understand your concern here:)
*Nits*:
1.I concur with Adrian regarding numerous nits he has reported in his
WG LC Comment
<https://mailarchive.ietf.org/arch/msg/spring/FRhO2lgR8r4VlKP2ZN2dZwHU5BY>.
I would like to thank Adrian for an excellent review that have saved
me lots of hard work.
#Ahmed: I also agree that Adrian's review is exceptional. I believe I
addressed all his comments in the latest version.
2.In addition, I’d like to report the following nits:
a.Ti-LFA in Section 2.11.1 should be TI-LFA (as in the TI-LFA
<https://tools.ietf.org/html/draft-bashandy-rtgwg-segment-routing-ti-lfa-04>
draft)
#Ahmed: Already done in the latest version
b.TI-LFA draft is referenced in the text of Section 2.11.1, but there
is no matching reference in the “References” section (probably,
Informational)
#Ahmed: Already done in the latest version
c.“zero Algorithm” in Section 2.5 (immediately above Section 2.5.1)
must be replaced with “default algorithm”. Similarly, “non-zero
Algorithm” should be replaced with “non-default algorithm”
#Ahmed: Will be done in the next version
3.I think that RFC 3443 and RFC 5332 should be listed as Normative
references in this draft while RFC 5331 and RFC 8277 should be listed
as Informative references. This would improve the readability of the
draft without any impact on its advancement.
#Ahmed RFC5331 describes upstream label assignment. As you mentioned
above (and I will modify the draft to indicate that) SR-MPLS behavior is
similar to downstream label assignment. RFC 3443 describes TTL behavior.
This is an MPLS forwarding behavior. As mentioned in the draft, SR-MPLS
does not modify at the MPLS forwarding behavior
Hopefully, these comments will be useful.
#Ahmed: They are certainly quite useful. Thanks a lot
Regards,
Sasha
Office: +972-39266302
Cell: +972-549266302
Email: [email protected]
<mailto:[email protected]>
___________________________________________________________________________
This e-mail message is intended for the recipient only and contains
information which is
CONFIDENTIAL and which may be proprietary to ECI Telecom. If you have
received this
transmission in error, please inform us by e-mail, phone or fax, and
then delete the original
and all copies thereof.
___________________________________________________________________________
_______________________________________________
spring mailing list
[email protected]
https://www.ietf.org/mailman/listinfo/spring
