Thanks a lot for the review
I uploaded version 19 of the draft, which, IMO, addresses all your comments
See the reply "#Ahmed"
On 3/10/19 9:55 AM, Alexander Vainshtein wrote:
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
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*”.
#Ahmed: I replaced the MAY with SHOULD
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?
#Ahmed: I removed the highlighted text because I agree with what you
said in item (d) that it has no value
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. **
#Ahmed: I removed the highlighted word "allocated"
*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"*”
#Ahmed: The sentence is removed as you suggested
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]*”.
#Ahmed added the missing "to"
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
#Ahmed: Corrected, thanks again
b.Reference to RFC 8174 mistakenly contains a link to RFC 7274.
#Ahmed: Corrected
Hopefully these notes will be useful.
#Ahmed: VERY useful
Regards,
Sasha
Office: +972-39266302
Cell: +972-549266302
Email: [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
