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

Ahmed Bashandy <abashandy.ietf@gmail.com> Fri, 29 March 2019 01:13 UTC

Return-Path: <abashandy.ietf@gmail.com>
X-Original-To: mpls@ietfa.amsl.com
Delivered-To: mpls@ietfa.amsl.com
Received: from localhost (localhost [127.0.0.1]) by ietfa.amsl.com (Postfix) with ESMTP id C4E9912006A; Thu, 28 Mar 2019 18:13:44 -0700 (PDT)
X-Virus-Scanned: amavisd-new at amsl.com
X-Spam-Flag: NO
X-Spam-Score: -1.988
X-Spam-Level:
X-Spam-Status: No, score=-1.988 tagged_above=-999 required=5 tests=[BAYES_00=-1.9, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, FREEMAIL_FROM=0.001, HTML_MESSAGE=0.001, RCVD_IN_DNSWL_NONE=-0.0001, SPF_PASS=-0.001, T_KAM_HTML_FONT_INVALID=0.01, URIBL_BLOCKED=0.001] autolearn=ham autolearn_force=no
Authentication-Results: ietfa.amsl.com (amavisd-new); dkim=pass (2048-bit key) header.d=gmail.com
Received: from mail.ietf.org ([4.31.198.44]) by localhost (ietfa.amsl.com [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id PUySVVOMcF-3; Thu, 28 Mar 2019 18:13:41 -0700 (PDT)
Received: from mail-pl1-x632.google.com (mail-pl1-x632.google.com [IPv6:2607:f8b0:4864:20::632]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by ietfa.amsl.com (Postfix) with ESMTPS id 7A99A1200B6; Thu, 28 Mar 2019 18:13:41 -0700 (PDT)
Received: by mail-pl1-x632.google.com with SMTP id g12so179968pll.11; Thu, 28 Mar 2019 18:13:41 -0700 (PDT)
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=subject:to:cc:references:from:message-id:date:user-agent :mime-version:in-reply-to:content-language; bh=6mXQZ4LZ/hO5N6avoB0nP6y8u+tFyjcrEUY2VtL+R6Y=; b=K8Ji65cDMz0RIoxKItpaObO8KdRejFhgc6cRGwV7BrpwQiOrYGQkknb7sQQaVvu6FS Ndj+EFkVmCatT0FvVq5qgqAUhoCidD5V3k6AFKjcY2YU3yRlkX1+YKaNuIUZDJCNTii3 w4MzWw/Xh7X1JVVNMTSkY3HTneBJXPg6sbHv39NLc/epapI3Jcqudv/1+oX9SDVDsfwq jDdD7NPGAG2NckLBWd7cZTOT3Bn/pRLIwWIsLEB1YjlrdnAsM87rTNUz1nbag+9Zi/7V 3pYY41bpqtvCA9gto2L4Vmc6m8/GkQObE3UGwSK3lewH7pEVcRma7npDaH9OJnOZIMCZ 54vg==
X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:subject:to:cc:references:from:message-id:date :user-agent:mime-version:in-reply-to:content-language; bh=6mXQZ4LZ/hO5N6avoB0nP6y8u+tFyjcrEUY2VtL+R6Y=; b=NwuiI6Sy/5DwWW82/GTuL0co2jvIrijqK/wDadwyPV51CSTvb6RHXVbx3VlM5NwY+n pKIAElRDc2ltvvsfNI+qT8BKUFWOTOmbnhnvtMqN1cM2BZQ6hUB8o1fUMM+p6Hu2fVgb d2BFsk+O55fL0fFMf3yyj5DIkgXrUOF7OuhMwJi4w3vdG8CAjdzBoHZynUiF1gNK6IM6 dP1sLid3VSbJNYyWVZbtOQ/D/akYkzUqAgy5s9VlhKwU4lzIZnmhNuFIK97NRFwn6aHO VIDCrrnvMiTT0zd0/GkWa2a6cMj0LYWYX6gpD3PY4+e1Ni0vSJyIu0mQekpvK3TJY949 0Nng==
X-Gm-Message-State: APjAAAVqfjPAEcZ/NmZSlUyT8LYR/MGNYgn85D+abbNGH9IxYh8Y62ov 9bdv/QKBztDoJKSITrIId0I=
X-Google-Smtp-Source: APXvYqx8YmVO06JmixYtWLHP8RNLKmI6FW8KTHa2t57QsyqV49tRyMOuVvAoiRdHn1fLH7vWZHMODQ==
X-Received: by 2002:a17:902:a5c9:: with SMTP id t9mr44612552plq.196.1553822020960; Thu, 28 Mar 2019 18:13:40 -0700 (PDT)
Received: from Arrcus-Ahmeds-MacBook-Pro.local (adsl-70-234-233-188.dsl.rcsntx.sbcglobal.net. [70.234.233.188]) by smtp.gmail.com with ESMTPSA id i79sm537164pfj.28.2019.03.28.18.13.39 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Thu, 28 Mar 2019 18:13:40 -0700 (PDT)
To: Alexander Vainshtein <Alexander.Vainshtein@ecitele.com>, "rtg-ads@ietf.org" <rtg-ads@ietf.org>
Cc: "rtg-dir@ietf.org" <rtg-dir@ietf.org>, "spring@ietf.org" <spring@ietf.org>, "mpls@ietf.org" <mpls@ietf.org>, "draft-ietf-spring-segment-routing-mpls.authors@ietf.org" <draft-ietf-spring-segment-routing-mpls.authors@ietf.org>, Min Ye <amy.yemin@huawei.com>
References: <VI1PR03MB3839B5FA07EADE57084F8E389D4F0@VI1PR03MB3839.eurprd03.prod.outlook.com>
From: Ahmed Bashandy <abashandy.ietf@gmail.com>
Message-ID: <65361777-db63-9a7a-9199-dd04425b4785@gmail.com>
Date: Thu, 28 Mar 2019 18:13:39 -0700
User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.13; rv:60.0) Gecko/20100101 Thunderbird/60.6.0
MIME-Version: 1.0
In-Reply-To: <VI1PR03MB3839B5FA07EADE57084F8E389D4F0@VI1PR03MB3839.eurprd03.prod.outlook.com>
Content-Type: multipart/alternative; boundary="------------8CCF32B57D81BFE5451EA093"
Content-Language: en-US
Archived-At: <https://mailarchive.ietf.org/arch/msg/mpls/lWfW9LpBHnbsST8BitLFSTefNLQ>
Subject: Re: [mpls] RTG-DIR Last Call review of draft-ietf-spring-segment-routing-mpls-18
X-BeenThere: mpls@ietf.org
X-Mailman-Version: 2.1.29
Precedence: list
List-Id: Multi-Protocol Label Switching WG <mpls.ietf.org>
List-Unsubscribe: <https://www.ietf.org/mailman/options/mpls>, <mailto:mpls-request@ietf.org?subject=unsubscribe>
List-Archive: <https://mailarchive.ietf.org/arch/browse/mpls/>
List-Post: <mailto:mpls@ietf.org>
List-Help: <mailto:mpls-request@ietf.org?subject=help>
List-Subscribe: <https://www.ietf.org/mailman/listinfo/mpls>, <mailto:mpls-request@ietf.org?subject=subscribe>
X-List-Received-Date: Fri, 29 Mar 2019 01:13:45 -0000

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