Re: [mpls] [spring] RTG-DIR Last Call review of draft-ietf-spring-segment-routing-mpls-18

[Ahmed Bashandy <[e-mail]>]

thanks for the comment

Mirrored SID  in Section 2.5 refers to RFC8402 section 5.1, which AFAIK 
is correct. What link needs to be updated?

Ahmed


On 3/29/19 9:13 PM, Przemyslaw Krol wrote:
> Hi Ahmed,
>
> Cosmetic minor nit:
> 2.5. Incoming Label Collision
> [...]
> both links in the '(Mirrored SID)' section need a cleanup / update
>
> thanks,
> pk
>
>
> On Fri, Mar 29, 2019 at 2:14 AM Ahmed Bashandy 
> <[e-mail] <mailto:[e-mail]>> wrote:
>
>     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: [phone]
>>
>>     Cell:      [phone]
>>
>>     Email: [e-mail]
>>     <mailto:[e-mail]>
>>
>>
>>     ___________________________________________________________________________
>>
>>     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
>     [e-mail] <mailto:[e-mail]>
>     https://www.ietf.org/mailman/listinfo/spring
>
>
>
> -- 
> Przemyslaw "PK" Krol | 	 Strategic Network Engineer 	ing 
> |[e-mail] <mailto:[e-mail]> 	
>