Last Call Review of draft-ietf-spring-segment-routing-mpls-18
review-ietf-spring-segment-routing-mpls-18-rtgdir-lc-vainshtein-2019-03-10-00

Hello,

I have been selected as the Routing Directorate reviewer for this draft. The
Routing Directorate seeks to review all routing or routing-related drafts as
they pass through IETF last call and IESG review, and sometimes on special
request. The purpose of the review is to provide assistance to the Routing ADs.
For more information about the Routing Directorate, please see
​http://trac.tools.ietf.org/area/rtg/trac/wiki/RtgDir<http://trac.tools.ietf.org/area/rtg/trac/wiki/RtgDir>

Although these comments are primarily for the use of the Routing ADs, it would
be helpful if you could consider them along with any other IETF Last Call
comments that you receive, and strive to resolve them through discussion or by
updating the draft.

Document: review of draft-ietf-spring-segment-routing-mpls-18
Reviewer: Alexander (“Sasha”) Vainshtein
Review Date: 10-Mar-19
IETF LC End Date: 07-Mar-2019
Intended Status: Proposed Standard

Summary: I have some minor concerns about this document that I think should be
resolved before publication.

Comments:

I have done an early RTG-DIR review of the -14 version of the draft half a year
ago, and the issues I’ve raised then have been resolved in the subsequent
versions one way or another). Therefore this review has been intentionally
focused on the changes done to the draft in the few recent versions.

In my previous review I have noticed that the draft was not easy reading for
me. Since then readability of the draft has been improved. However, there are
still several places in the new text that are still difficult to parse.

I did not run the nits checker on the draft, so my list of nits is probably
incomplete.

Just as with my earlier review, I send this one also to the MPLS WG list – and
for the same reasons.

I tried to discuss my review privately with the authors, but they did not
respond.

Major Issues: No major issues found.

Minor Issues:

1.    The text in Section 1 states that “a network operator SHOULD configure at
least one node segment per routing instance, topology, algorithm” and continues
that “An implementation MAY check that an IGP node-SID is not associated with a
prefix that is owned by more than one router within the same routing domain, If
so, it SHOULD NOT use this Node-SID, MAY use another one if available, and
SHOULD log an error”. This looks somewhat controversial to me because:

a.    The check of the Node SID not being owned by more than one router in the
routing domain is defined as purely optional. According to RFC 2119,
implementations that choose to implement such a check must be able to
interoperate with implementations that do not implement it

b.    The recommended handling of the results of this check (fully aligned with
the text in Section 3.2 pf RFC 8402 that prohibits using prefixes owned by more
than one router in the domain as Node-SODs) strongly suggests that the prefix
that is owned by more than one router in the domain is unusable as the Node SID

I see two possibilities to resolve this controversy: either make the check in
question a “real requirement” (i.e., replace MAY with SHOULD or even MUST), or
explain why it is safe enough not to implement such a check (i.e., how
implementations that support this check and implementations that do not support
it can interoperate within a given routing domain). The first of these options
seems to me aligned with Section 3.2 in RFC 8402 that says that “An IGP
Node-SID MUST NOT be associated with a prefix that is owned by more than one
router within the same routing domain”.

2.       I have  a problem with the highlighted part of the following text in
Section 2.5:
   An implementation MUST NOT allow the MCCs belonging to the same
   router to assign the same incoming label to more than one SR FEC. An
   implementation that allows such behavior is considered as faulty.
   Procedures defined in this document equally applies to this case,
   both for incoming label collision (Section
   2.5<https://tools.ietf.org/html/draft-ietf-spring-segment-routing-mpls-18#section-2.5>)
   and the effect on outgoing label programming (Section
   2.6<https://tools.ietf.org/html/draft-ietf-spring-segment-routing-mpls-18#section-2.6>).

a.    The Section in question deals with incoming label collision (in fact, the
text that immediately follows the problematic fragment states that “The
objective of the following steps is to deterministically install in the MPLS
Incoming Label Map, also known as label FIB, a single FEC with the incoming
label "L1"”

b.    As a consequence, any mention of  outgoing label programming, looks out
of context (even accompanied by a forward reference to Section 2.6)

c.    Section 2.6 covers the impact of incoming label collision on programming
of outgoing labels in quite a generic way. Therefore I think that the 
highlighted part of the quoted fragment can be safely removed (complete with
the grammar mistake).

d.    I also do not see any value in stating that an implementations that
violates a mandatory requirement of the spec is faulty – isn’t that
self-evident?

3.    The highlighted text in Section 2.8 is not accurate:

   For Local SIDs, the MCC is responsible for downloading the correct

   label value to FIB. For example, an IGP with SR extensions [I-D.ietf-

   isis-segment-routing-extensions, I-D.ietf-ospf-segment-routing-

   extensions] allocates and downloads the MPLS label corresponding to

   an Adj-SID [RFC8402<https://tools.ietf.org/html/rfc8402>].

a.  IGP with SR extensions may indeed dynamically allocate and download MPLS
labels acting as local Adj-SIDs

b.  However, these labels can be allocated by configuration (e.g. as mentioned
in the tie-breaking rules in Section 2.5.1 and in the example in Section A.2.3
in the draft), in which case IGP with SR extensions would only responsible for
its advertisement and installation.

NITS:

 :

1.    In section 2.5:

a.    In the sentence “Procedures defined in this document equally applies to
this case” the noun is in plural but the verb is in singular. (If this sentence
is removed as suggested above, this nit disappears)

b.    The same problem exists in the sentence “An incoming label collision
occurs if the SIDs of the set of FECs {FEC1, FEC2,..., FECk} maps to the same
incoming SR MPLS label "L1"”

2.       In section 2.10.1 the preposition “to” between the words “according”
and “MPLS” is missing in the fragment “Push the calculated label according the
MPLS label pushing rules specified in [RFC3032]”.

3.       Problems with references:

a.       As reported by
Sergey<https://mailarchive.ietf.org/arch/msg/spring/C_W3KBcL2AWxmlB7Sp53_PvqbQA>,
there are two occurrences of references to RFC 8042 “OSPF Two-Part Metric”
instead of RFC 8402. Lots of thanks to Sergey for catching this

b.       Reference to RFC 8174 mistakenly contains a link to  RFC 7274.

Hopefully these notes will be useful.
Regards,
Sasha

Office: +972-39266302
Cell:      +972-549266302
Email:   Alexander.Vainshtein@ecitele.com

___________________________________________________________________________

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.
___________________________________________________________________________