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

2018-10-21 Thread Naiming Shen (naiming) 

Hi Stewart,

Thanks for detailed review and comments, please see some
replies inline. the modified version diff is attached, please reivew.

thanks.
- Naiming

Title: Diff: draft-ietf-isis-reverse-metric-15.txt - draft-ietf-isis-reverse-metric-15-rev.txt
 
 

 
 
 
 
 
 
 
   
   draft-ietf-isis-reverse-metric-15.txt   draft-ietf-isis-reverse-metric-15-rev.txt  
   
  Networking Working Group N. Shen Networking Working Group N. Shen
  Internet-Draft Cisco Systems Internet-Draft Cisco Systems
  Intended status: Standards Track   S. Amante Intended status: Standards Track   S. Amante
  
  Expires: April 19, 2019  Apple, Inc. Expires: April 24, 2019  Apple, Inc.
M. Abrahamsson   M. Abrahamsson
  T-Systems Nordic T-Systems Nordic
  
  October 16, 2018 October 21, 2018
   
 IS-IS Routing with Reverse MetricIS-IS Routing with Reverse Metric
  
 draft-ietf-isis-reverse-metric-15  draft-ietf-isis-reverse-metric-15-rev
   
  Abstract Abstract
   
 This document describes a mechanism to allow IS-IS routing to quicklyThis document describes a mechanism to allow IS-IS routing to quickly
 and accurately shift traffic away from either a point-to-point orand accurately shift traffic away from either a point-to-point or
 multi-access LAN interface during network maintenance or othermulti-access LAN interface during network maintenance or other
 operational events.  This is accomplished by signaling adjacent IS-ISoperational events.  This is accomplished by signaling adjacent IS-IS
 neighbors with a higher reverse metric, i.e., the metric towards theneighbors with a higher reverse metric, i.e., the metric towards the
 signaling IS-IS router.signaling IS-IS router.
   
   
  skipping to change at page 1, line 38 skipping to change at page 1, line 38
 Internet-Drafts are working documents of the Internet EngineeringInternet-Drafts are working documents of the Internet Engineering
 Task Force (IETF).  Note that other groups may also distributeTask Force (IETF).  Note that other groups may also distribute
 working documents as Internet-Drafts.  The list of current Internet-working documents as Internet-Drafts.  The list of current Internet-
 Drafts is at http://datatracker.ietf.org/drafts/current/.Drafts is at http://datatracker.ietf.org/drafts/current/.
   
 Internet-Drafts are draft documents valid for a maximum of six monthsInternet-Drafts are draft documents valid for a maximum of six months
 and may be updated, replaced, or obsoleted by other documents at anyand may be updated, replaced, or obsoleted by other documents at any
 time.  It is inappropriate to use Internet-Drafts as referencetime.  It is inappropriate to use Internet-Drafts as reference
 material or to cite them other than as "work in progress."material or to cite them other than as "work in progress."
   
  
 This Internet-Draft will expire on April 19, 2019.This Internet-Draft will expire on April 24, 2019.
   
  Copyright Notice Copyright Notice
   
 Copyright (c) 2018 IETF Trust and the persons identified as theCopyright (c) 2018 IETF Trust and the persons identified as the
 document authors.  All rights reserved.document authors.  All rights reserved.
   
 This document is subject to BCP 78 and the IETF Trust's LegalThis document is subject to BCP 78 and the IETF Trust's Legal
 Provisions Relating to IETF DocumentsProvisions Relating to IETF Documents
 (http://trustee.ietf.org/license-info) in effect on the date of(http://trustee.ietf.org/license-info) in effect on the date of
 publication of this document.  Please review these documentspublication of this document.  Please review these documents
 carefully, as they describe your rights and restrictions with respectcarefully, as they describe your rights and restrictions with respect
 to this document.  Code Components extracted from this document mustto this

[Lsr] Genart last call review of draft-ietf-isis-reverse-metric-15

2018-10-17 Thread Stewart Bryant 
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

.

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


 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.

   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.

===

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.

===

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

   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.

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

=

   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.

=

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,
=
   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.

==

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

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