Re: [Gen-art] Genart last call review of draft-ietf-isis-reverse-metric-15

Alissa Cooper <alissa@cooperw.in> Tue, 20 November 2018 20:42 UTC

Return-Path: <alissa@cooperw.in>
X-Original-To: gen-art@ietfa.amsl.com
Delivered-To: gen-art@ietfa.amsl.com
Received: from localhost (localhost [127.0.0.1]) by ietfa.amsl.com (Postfix) with ESMTP id 154D3130DC6; Tue, 20 Nov 2018 12:42:04 -0800 (PST)
X-Virus-Scanned: amavisd-new at amsl.com
X-Spam-Flag: NO
X-Spam-Score: -2.7
X-Spam-Level:
X-Spam-Status: No, score=-2.7 tagged_above=-999 required=5 tests=[BAYES_00=-1.9, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, RCVD_IN_DNSWL_LOW=-0.7, SPF_PASS=-0.001, URIBL_BLOCKED=0.001] autolearn=ham autolearn_force=no
Authentication-Results: ietfa.amsl.com (amavisd-new); dkim=pass (2048-bit key) header.d=cooperw.in header.b=YB8lCXPQ; dkim=pass (2048-bit key) header.d=messagingengine.com header.b=lrKm81FN
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 erj5W1UPba1Y; Tue, 20 Nov 2018 12:42:00 -0800 (PST)
Received: from out4-smtp.messagingengine.com (out4-smtp.messagingengine.com [66.111.4.28]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by ietfa.amsl.com (Postfix) with ESMTPS id 82FD612D4EF; Tue, 20 Nov 2018 12:42:00 -0800 (PST)
Received: from compute7.internal (compute7.nyi.internal [10.202.2.47]) by mailout.nyi.internal (Postfix) with ESMTP id 5D4FA21B13; Tue, 20 Nov 2018 15:41:59 -0500 (EST)
Received: from mailfrontend2 ([10.202.2.163]) by compute7.internal (MEProxy); Tue, 20 Nov 2018 15:41:59 -0500
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=cooperw.in; h= content-type:mime-version:subject:from:in-reply-to:date:cc :content-transfer-encoding:message-id:references:to; s=fm1; bh=6 /XWoD5hQm3rUJ8j8Q/OTtrI0Y0ebrj24dt5f5iBtdU=; b=YB8lCXPQFF3DZ7zKG aTwwcXfoY90nDHWCaCbn7YfHfKmS3vyuLbOI9ie8tI99AlvVIfT91JeYoSH0hc62 XKlPWpFf+OAsqLNv2PhswifOhF4bpt0LfZ2F6Vzb5uasIJRZ0mm5fUwXmSCQI3BZ IWjtODyjsJETs0W+PrhjNjTLAGWoTTFQP3msILv7BFls49ihpIa3q/5P3YXxLUiP zk0xVLmo/cR8FpTQLsypn0l3GIRiT4L4XOzyNLIuwp9ryYnegX8jR71LQPfaUqrs q5KFPiAzvLfx9z4FA1mSt7HNVXuPRVJrQr1lZ6ZKA04FktOOWcLMP1wOTPMrlXUg lwJwA==
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d= messagingengine.com; h=cc:content-transfer-encoding:content-type :date:from:in-reply-to:message-id:mime-version:references :subject:to:x-me-proxy:x-me-proxy:x-me-sender:x-me-sender :x-sasl-enc; s=fm1; bh=6/XWoD5hQm3rUJ8j8Q/OTtrI0Y0ebrj24dt5f5iBt dU=; b=lrKm81FNFA5sS6ZQg4ZIBndnhKZDsvJ0YlM8ZHa1jc4m/KwP31n6GwzXC LXAyljvNJT9qr67JxKBAsRt2bEdVWxEF/6ScW/W/gO3dQSfzpbrMRMa0s6HlSqrz JRzStEyl0FvAo3wXmKN0uW3WAvzHBdORWWuO7ODHCPnEuN31L95ykvbR2oS89HXx ONqakBOZhcOSDNiY52CtKwk14xP6iRC/tXxlrZlRUIYQJrtmNirf5FBt9ANk7T7X NcEP72YBf4uOg0v0Mzw61Zm0mPJ+UXqsMJ9okzdsPoq1wUKlKg9FMbKXciyzws7G oUmk9Rf2oSGRjgeVLTmLT7xcMHe6A==
X-ME-Sender: <xms:l3H0W36nO2lX50PZJwsyKn-IraeYnsIstE_hJNClUerd9N6IuZppbA>
X-ME-Proxy: <xmx:l3H0W3Zg9Syzf67avxa9g2-JTTcWOBoCFx9SmXAyJG47nhNR2Nx4gw> <xmx:l3H0W_uF2b1OIEx5qsAztp62jGQqLZgYBqihpTZxSGzGltcndORUDg> <xmx:l3H0WxNd19eERjvTX-htl-T3G7XVlWozDTnK3pvyQ2EQTRutSOsmdQ> <xmx:l3H0W-7RBTvoAGzzrzEBbucOM50XM7DBXq8Pn94yw2d_5zOkDUnbXA> <xmx:l3H0W_SshlVkYs4ZwAk142mYv7fScu_QntWZWpSRbgAl9O0JB2UbuQ> <xmx:l3H0W_DQIIIxgwM5zg7kiVd7NzrQJdD4FYvIJ9ixu5sjWkqLjZrT-A>
Received: from alcoop-m-c46z.fios-router.home (pool-108-51-101-98.washdc.fios.verizon.net [108.51.101.98]) by mail.messagingengine.com (Postfix) with ESMTPA id A21A1102A0; Tue, 20 Nov 2018 15:41:58 -0500 (EST)
Content-Type: text/plain; charset="utf-8"
Mime-Version: 1.0 (Mac OS X Mail 11.5 \(3445.9.1\))
From: Alissa Cooper <alissa@cooperw.in>
In-Reply-To: <C12506C4-15EB-41CE-A1C7-9D771C56FF9B@cisco.com>
Date: Tue, 20 Nov 2018 15:41:57 -0500
Cc: "lsr@ietf.org" <lsr@ietf.org>, "gen-art@ietf.org" <gen-art@ietf.org>, "draft-ietf-isis-reverse-metric.all@ietf.org" <draft-ietf-isis-reverse-metric.all@ietf.org>
Content-Transfer-Encoding: quoted-printable
Message-Id: <6BDEE760-BD1F-4CFA-A5DD-F359D6ACCA1B@cooperw.in>
References: <153979199224.27586.17724630615593051025@ietfa.amsl.com> <C12506C4-15EB-41CE-A1C7-9D771C56FF9B@cisco.com>
To: "Naiming Shen (naiming)" <naiming@cisco.com>, Stewart Bryant <stewart.bryant@gmail.com>
X-Mailer: Apple Mail (2.3445.9.1)
Archived-At: <https://mailarchive.ietf.org/arch/msg/gen-art/Bv8mgSDMRdIV_lLCzinwlDMrsyc>
Subject: Re: [Gen-art] Genart last call review of draft-ietf-isis-reverse-metric-15
X-BeenThere: gen-art@ietf.org
X-Mailman-Version: 2.1.29
Precedence: list
List-Id: "GEN-ART: General Area Review Team" <gen-art.ietf.org>
List-Unsubscribe: <https://www.ietf.org/mailman/options/gen-art>, <mailto:gen-art-request@ietf.org?subject=unsubscribe>
List-Archive: <https://mailarchive.ietf.org/arch/browse/gen-art/>
List-Post: <mailto:gen-art@ietf.org>
List-Help: <mailto:gen-art-request@ietf.org?subject=help>
List-Subscribe: <https://www.ietf.org/mailman/listinfo/gen-art>, <mailto:gen-art-request@ietf.org?subject=subscribe>
X-List-Received-Date: Tue, 20 Nov 2018 20:42:04 -0000

Stewart, thanks for your review. Naiming, thanks for your responses. I have entered a No Objection ballot.

Alissa

> On Oct 21, 2018, at 5:21 PM, Naiming Shen (naiming) <naiming@cisco.com> wrote:
> 
> 
> Hi Stewart,
> 
> Thanks for detailed review and comments, please see some
> replies inline. the modified version diff is attached, please reivew.
> 
> thanks.
> - Naiming
> 
> <draft-ietf-isis-reverse-metric-15-rev-from-.diff.html>
> 
>> On Oct 17, 2018, at 8:59 AM, Stewart Bryant <stewart.bryant@gmail.com> wrote:
>> 
>> Reviewer: Stewart Bryant
>> Review result: Ready with Issues
>> 
>> I am the assigned Gen-ART reviewer for this draft. The General Area
>> Review Team (Gen-ART) reviews all IETF documents being processed
>> by the IESG for the IETF Chair.  Please treat these comments just
>> like any other last call comments.
>> 
>> For more information, please see the FAQ at
>> 
>> <https://trac.ietf.org/trac/gen/wiki/GenArtfaq>.
>> 
>> Document: draft-ietf-isis-reverse-metric-15
>> Reviewer: Stewart Bryant
>> Review Date: 2018-10-17
>> IETF LC End Date: 2018-10-17
>> IESG Telechat date: 2018-11-21
>> 
>> Summary: Generally a well written document, but some earlier text on what a
>> reverse metric is and what it does would be very helpful to the reader. Also
>> the reader is left with the impression that the use of this gives disruption
>> free network changes, and yet it does not discuss microloops.
>> 
>> Major issues: None
>> 
>> Minor issues:
>> 1.2.  Distributed Forwarding Planes
>> 
>> <snip>
>> Temporarily signaling
>>  the 'Reverse Metric' over this link to discourage the traffic via the
>> 
>> SB> I know it's always chicken and egg, but it would be useful if a
>> SB> clearer definition of reverse metric were provided before you
>> SB> explained its use.
> 
> <NS> I have added a paragraph at the beginning of the Introduction section
> 
>> 
>>  corresponding line-card will help to reduce the traffic loss in the
>>  network.  In the meantime, the remote PE routers will select a
>>  different set of PE routers for the BGP best path calculation or use
>>  a different link towards the same PE router on which a line-card is
>>  resetting.
>> 
>> SB> Remember that when you change paths you have to deal with
>> SB> microloops.
> 
> <NS> Well, this ‘reverse metric’ is just like a normal IS-IS metric change.
> <NS> and microloops is implied. This document does not create new
> <NS> condition or trying to address the issue. It assumes final convergence
> <NS> state will be reached for the mentioned use cases.
> 
>> 
>> =======
>> 
>> 1.5.  IS-IS Reverse Metric
>> 
>>  This document uses the routing protocol itself as the transport
>>  mechanism to allow one IS-IS router to advertise a "reverse metric"
>>  in an IS-IS Hello (IIH) PDU to an adjacent node on a point-to-point
>>  or multi-access LAN link.  This would allow the provisioning to be
>>  performed only on a single node, setting a "reverse metric" on a link
>>  and have traffic bidirectionally shift away from that link gracefully
>>  to alternate, viable paths.
>> 
>> SB> Again you need to be much clearer what a reverse metric is before
>> SB> you cite the use cases and advantages.
> 
> <NS> added a new paragraph.
> 
>> 
>> ===========
>> 
>> 3.1.  Processing Changes to Default Metric
>> 
>>  It is important to use the same IS-IS metric type on both ends of the
>>  link and in the entire IS-IS area or level.
>> 
>> SB> Isn't the point about links redundant given that it needs to be the
>> SB> same in the area/the level
> 
> <NS>  This sentence is talking about it does not deal with the case of
> <NS> one side of the link uses IS-IS wide metric, but the other side
> <NS> of the link uses a narrow metric. It is saying it’s broken and we
> <NS> don’t support such a mixup, thus ‘reverse-metric’ also does not handle.
> 
>> 
>>  On the receiving side of
>>  the 'reverse-metric' TLV, the accumulated value of configured metric
>>  and the reverse-metric needs to be limited to 63 in "narrow" metric
>>  mode and to (2^24 - 2) in "wide" metric mode.
>>  This applies to both
>>  the Default Metric of Extended IS Reachability TLV and the Traffic
>>  Engineering Default Metric sub-TLV in LSP or Pseudonode LSP for the
>>  "wide" metric mode case.  If the "U" bit is present in the flags, the
>>  accumulated metric value is to be limited to (2^24 - 1) for both the
>>  normal link metric and Traffic Engineering metric in IS-IS "wide"
>>  metric mode.
>> 
>> SB> A clarifying note explaining the different usage of 2^24 - 1 and
>> SB> 2^24 - 2 would be helpful to the reader.
> 
> <NS> Yes, added a sentence in the TLV definition part to describe them
> <NS> in some detail.
> 
>> 
>> =========
>> 3.2.  Multi-Topology IS-IS Support on Point-to-point links
>> 
>>  The Reverse Metric TLV is applicable to Multi-Topology IS-IS (M-ISIS)
>>  [RFC5120].  On point-to-point links, if an IS-IS router is configured
>>  for M-ISIS, it MUST send only a single Reverse Metric TLV in IIH PDUs
>>  toward its neighbor(s) on the designated link.
>> 
>> SB> Might you not want to use this on a topology by topology basis?
>> SB> For example you might want to bring up important typologies first.
> 
> <NS> this document does not support that. The main reason is that the
> <NS> pnode LSP is shared by all the topologies, and it’s not per topology
> <NS> pnode definition. For backwards competibility of this draft, it follows
> <NS> the same logic. Otherwise we need a flag day in the network to use
> <NS> ‘reverse-metric’.
> 
>> 
>> =========
>> 
>>  its
>>  inbound metric value to be the maximum and this puts the link of this
>>  new node in the last resort position without impacting the other IS-
>>  IS nodes on the same LAN.
>> 
>> SB> It is only down in S3.4 that you provide this simple definition of
>> SB> reverse metric as the "inbound metric". It would be helpful to have this
>> SB> earlier in the text.
> 
> <NS> yes, indeed. a new paragraph is added.
> 
>> 
>> =========
>> 
>> It is RECOMMENDED also that the CSPF does the immediate CSPF
>>  re-calculation when the Traffic Engineering metric is raised to (2^24
>>  - 2) to be the last resort link.
>> 
>> SB> Again it would help the reader if "link of last resort" was earlier
>> SB> in the text,
> 
> <NS> ‘link of last resort’ is only for certain use cases, but not all. added
> <NS> also in the TLV definition section.
> 
>> =========
>>  It is RECOMMENDED that implementations provide a capability to
>>  disable any changes by Reverse Metric mechanism through neighbor's
>>  Hello PDUs.
>> 
>> SB> Changes of what? That sentence does not seem to read very well.
> 
> <NS> added the “IS-IS metric changes”.
> 
>> 
>> ==========
>> 
>>  If an implementation enables this mechanism by default, it is
>>  RECOMMENDED that it be disabled by the operators when not explicitly
>>  using it.
>> 
>> SB> Why not RECOMMEND that it be disabled by default, or perhaps
>> SB> strengthen that to MUST be disabled by default.
> 
> <NS> As also replied also to Alvaro’s AD review comments, one of the main
> <NS> use case is for operational maintenance window to divert the traffic
> <NS> into certain links by using the ‘reverse-metric’ mechanism. If an operator
> <NS> has to track down all the nodes on the LAN side of the links, and to
> <NS> manually enable the feature one by one correctly, then it defeats the
> <NS> purpose of this to simplify the operational use case here.
> 
>> =========
>> 
>> it is highly RECOMMENDED that operators configure
>> authentication of IS-IS PDUs to mitigate use of the Reverse Metric
>> TLV as a potential attack vector.
>> 
>> SB> Not sure that you can qualify RFC2119 RECOMMENDED
> 
> <NS> changed to lower case.
> 
>> 
>> =========
>> 
>>> From the IANA section
>> 
>> SB> Why is 18 chosen in an otherwise empty registry?
> 
> <NS> Originally, it inherits the same sub-TLV from IS-IS TE sub-TLV RFC
> <NS> RFC 5305, and the Traffic Engineering Metric sub-TLV is 18.
> <NS> now this ‘reverse-metric’ TLV has it’s own registry, but we keep the
> <NS> number the same.
> 
>> 
>> =========
>> Regarding Appendix A and I think Appendix B
>> 
>> SB> As noted earlier you really need to talk about microloops. There
>> SB> is no disruption free lunch available.
>> 
>> ========
>> 
>> Nits/editorial comments:
>> 
>>> From ID-nits
>> ** Downref: Normative reference to an Informational RFC: RFC 5443
>> This is correctly dealt with in the LC
>> 
>> == Outdated reference: A later version (-07) exists of
>>    draft-shen-isis-spine-leaf-ext-03
>> I am sure the RFC Editor will address, but could usefully be fixed in any
>> respin.
>> 
> <NS> Good catch. corrected.
> 
> 
> 
> _______________________________________________
> Gen-art mailing list
> Gen-art@ietf.org
> https://www.ietf.org/mailman/listinfo/gen-art