Re: [Lsr] RtgDir Review: draft-ietf-isis-segment-routing-msd-15

"Les Ginsberg (ginsberg)" <ginsberg@cisco.com> Mon, 24 September 2018 06:11 UTC

Return-Path: <ginsberg@cisco.com>
X-Original-To: lsr@ietfa.amsl.com
Delivered-To: lsr@ietfa.amsl.com
Received: from localhost (localhost [127.0.0.1]) by ietfa.amsl.com (Postfix) with ESMTP id 02CFB130DD8; Sun, 23 Sep 2018 23:11:41 -0700 (PDT)
X-Virus-Scanned: amavisd-new at amsl.com
X-Spam-Flag: NO
X-Spam-Score: -14.511
X-Spam-Level:
X-Spam-Status: No, score=-14.511 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_HI=-5, SPF_PASS=-0.001, T_DKIMWL_WL_HIGH=-0.01, USER_IN_DEF_DKIM_WL=-7.5] autolearn=ham autolearn_force=no
Authentication-Results: ietfa.amsl.com (amavisd-new); dkim=pass (1024-bit key) header.d=cisco.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 IuWwfizt0cIE; Sun, 23 Sep 2018 23:11:38 -0700 (PDT)
Received: from alln-iport-4.cisco.com (alln-iport-4.cisco.com [173.37.142.91]) (using TLSv1.2 with cipher DHE-RSA-SEED-SHA (128/128 bits)) (No client certificate requested) by ietfa.amsl.com (Postfix) with ESMTPS id 5AE06130DC5; Sun, 23 Sep 2018 23:11:38 -0700 (PDT)
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=cisco.com; i=@cisco.com; l=11664; q=dns/txt; s=iport; t=1537769498; x=1538979098; h=from:to:cc:subject:date:message-id:references: in-reply-to:content-transfer-encoding:mime-version; bh=zY0VWFVhlRTOECUP3HiRNTm2TXIVXdPD1rSJ9vEKGA0=; b=iCS5JpHB2f+y0Z5CGXRbVG9D05r6aLg8sRg7crFxSoDVJfcWU7EY7j+G WT13iAX/PekmQoryoG7V08kcagugPjZu2Nox0wJgwlzPzwgG0A7Rx/r1Z oYQ/ra3WKlnYQ3Q/OojK91gYf/BOu8coJNUsyPUqbIDu5MjMlxV4fAj0C I=;
X-IronPort-Anti-Spam-Filtered: true
X-IronPort-Anti-Spam-Result: =?us-ascii?q?A0BIAADBf6hb/5JdJa1ZGQEBAQEBAQE?= =?us-ascii?q?BAQEBAQcBAQEBAQGBUYIOZX8oCoNqiBWMKYINeIJFkxIUgWYLI4RJAheDNCE?= =?us-ascii?q?0GAEDAQECAQECbRwMhTgBAQEBAyMRRQwEAgEIEQMBAQEDAiYCAgIwFQgIAgQ?= =?us-ascii?q?BDQUIE4MHggEPoWWBLooGgQuJbReBQT+BEQGCXTWDEAsCgSUkPoJbglcCnH8?= =?us-ascii?q?JAoZBiV4fgUVKhAeJFot6iGgCERSBJR04gVVwFYMnCYIcF4hahT5vAYwUgR4?= =?us-ascii?q?BAQ?=
X-IronPort-AV: E=Sophos;i="5.54,296,1534809600"; d="scan'208";a="175691802"
Received: from rcdn-core-10.cisco.com ([173.37.93.146]) by alln-iport-4.cisco.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 24 Sep 2018 06:11:37 +0000
Received: from XCH-ALN-003.cisco.com (xch-aln-003.cisco.com [173.36.7.13]) by rcdn-core-10.cisco.com (8.15.2/8.15.2) with ESMTPS id w8O6BakF019347 (version=TLSv1.2 cipher=AES256-SHA bits=256 verify=FAIL); Mon, 24 Sep 2018 06:11:37 GMT
Received: from xch-aln-001.cisco.com (173.36.7.11) by XCH-ALN-003.cisco.com (173.36.7.13) with Microsoft SMTP Server (TLS) id 15.0.1395.4; Mon, 24 Sep 2018 01:11:36 -0500
Received: from xch-aln-001.cisco.com ([173.36.7.11]) by XCH-ALN-001.cisco.com ([173.36.7.11]) with mapi id 15.00.1395.000; Mon, 24 Sep 2018 01:11:35 -0500
From: "Les Ginsberg (ginsberg)" <ginsberg@cisco.com>
To: Julien Meuric <julien.meuric@orange.com>, "rtg-ads@ietf.org" <rtg-ads@ietf.org>
CC: "draft-ietf-isis-segment-routing-msd@ietf.org" <draft-ietf-isis-segment-routing-msd@ietf.org>, "rtg-dir@ietf.org" <rtg-dir@ietf.org>, "lsr@ietf.org" <lsr@ietf.org>
Thread-Topic: RtgDir Review: draft-ietf-isis-segment-routing-msd-15
Thread-Index: AQHUSRr9GtZ+0d9dH0WUMIG4HP6jZqT++7dQ
Date: Mon, 24 Sep 2018 06:11:35 +0000
Message-ID: <da219cdbe1db4edab2afb4dc03aa8656@XCH-ALN-001.cisco.com>
References: <cb33dfed-f14a-e079-78a7-83659aac7ffb@orange.com>
In-Reply-To: <cb33dfed-f14a-e079-78a7-83659aac7ffb@orange.com>
Accept-Language: en-US
Content-Language: en-US
X-MS-Has-Attach:
X-MS-TNEF-Correlator:
x-ms-exchange-transport-fromentityheader: Hosted
x-originating-ip: [10.24.93.154]
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: base64
MIME-Version: 1.0
X-Outbound-SMTP-Client: 173.36.7.13, xch-aln-003.cisco.com
X-Outbound-Node: rcdn-core-10.cisco.com
Archived-At: <https://mailarchive.ietf.org/arch/msg/lsr/OgUllFyiTm_dxTAJHOc6FT-W3ds>
Subject: Re: [Lsr] RtgDir Review: draft-ietf-isis-segment-routing-msd-15
X-BeenThere: lsr@ietf.org
X-Mailman-Version: 2.1.29
Precedence: list
List-Id: Link State Routing Working Group <lsr.ietf.org>
List-Unsubscribe: <https://www.ietf.org/mailman/options/lsr>, <mailto:lsr-request@ietf.org?subject=unsubscribe>
List-Archive: <https://mailarchive.ietf.org/arch/browse/lsr/>
List-Post: <mailto:lsr@ietf.org>
List-Help: <mailto:lsr-request@ietf.org?subject=help>
List-Subscribe: <https://www.ietf.org/mailman/listinfo/lsr>, <mailto:lsr-request@ietf.org?subject=subscribe>
X-List-Received-Date: Mon, 24 Sep 2018 06:11:41 -0000

Julien -

Thanx for your detailed review - and your patience in waiting for a response (I returned from vacation only a few days ago).

I have published V16 of the draft which addresses your comments except as noted inline below.


> -----Original Message-----
> From: Julien Meuric <julien.meuric@orange.com>
> Sent: Monday, September 10, 2018 8:28 AM
> To: rtg-ads@ietf.org
> Cc: draft-ietf-isis-segment-routing-msd@ietf.org; rtg-dir@ietf.org;
> lsr@ietf.org
> Subject: RtgDir Review: draft-ietf-isis-segment-routing-msd-15
> 
> 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: draft-ietf-isis-segment-routing-msd-15
> Reviewer: Julien Meuric (with some inputs from Bruno Decraene) Review
> Date: 2018-09-07 IETF LC End Date: 2018-09-12 Intended Status: ST
> 
> *Summary:*
> I have some minor concerns about this document that I think should be
> resolved before publication.
> 
> *Comments:"
> The document is well focused on a solution to a clearly scoped problem.
> The defined encodings are simple and clear. All the same, there are a few
> loose wordings, implicit assumptions or fuzzy zones which require
> clarification before publication.
> 
> *Minor issues:*
> 
> - The abstract uses the acronym "SID", however:
>     * It should be expanded at first use,
>     * It should be defined, e.g. by adding a (normative) reference to RFC 8402
> (at least in the introduction),
>     * The SR context is missing and should be explicitly mentioned (adding a
> phrase such as "in a Segment Routing-enabled network" would be enough).

[Les:] SID has been added to the terminology section and a reference to RFC 8402 added.
However, I don’t think "SR context" is missing. The first sentence of the Introduction starts with

" When Segment Routing (SR) paths are computed..."

> 
> - OLD
>    Path Computation Element Protocol (PCEP) SR extensions draft
>    [I-D.ietf-pce-segment-routing] signals MSD in SR Path Computation
>    Element (PCE) Capability TLV and METRIC Object.
>   NEW
>    The Path Computation Element communication Protocol (PCEP) SR
>    extension document [I-D.ietf-pce-segment-routing] defines how to
>    signal MSD using the SR-PCE-CAPABILITY sub-TLV (per node) and
>    the METRIC object (per request).
>
[Les:] I have updated this and included a reference to RFC 5440 where METRIC object was first defined.

 
> - The introduction says that the solution to complement BGP-LS lies in IS-IS
> (the OSPF draft claiming the counterpart on its side). This is a bit rough, the
> sentence 2 paragraph below already does the necessary scoping job: "This
> document defines an extension to <your favorite IGP
> here>". I suggest to temper the early sentence by rephrasing the
> beginning of page 3 into: "MSD capabilities should be advertised by every
> router in the network involved in the IGP."
> 
[Les:] The draft says 

" MSD capabilities should be
   advertised by every Intermediate System to Intermediate System(IS-IS)
   router in the network."

which I believe meets your suggestion.

> - The wording of the following sentence is not clear: "Note that in a non-SR
> MPLS network, label depth is what is defined by the MSD advertisements." Is
> it intended to say: "Note that MSD advertisements are applicable beyond SR-
> enabled networks and may refer to label depth in MPLS networks."?
> 
[Les:] The draft says:

" Although MSD advertisements are associated with Segment Routing, the
   advertisements MAY be present even if Segment Routing itself is not
   enabled."

I have made this sentence and the following sentence (which you quoted) a separate paragraph - which hopefully makes this a bit clearer.

> - "SID" may deserve to join the terminology section.

[Les:] Done.

> 
> - s/SIDs a node or a link on a node can support./SIDs supported by a node or
> a link on a node./

[Les:] Done

> 
> - The same ASCII art is used for the 2 figures (sections 2 and 3). It would be
> nice to specialize each one a little bit by explicitly adding their respective
> codepoint in the type field (23 and 15).
>
[Les:] The figure is intended to define the class of information in each field - not the actual values. The actual values are specified in the text which follows the ASCII art.

 
> - Section 3 says "any other value represents that of the link when used as an
> outgoing link." Is it not excessive to be that prescriptive about outgoing vs.
> incoming? E.g., if an implementation was to advertise ERLD as a link MSD,
> that would rather refer to a capability of the incominglink. This specification
> could be left to each MSD-Type definition.
> 
[Les:] Good point - I removed reference to outgoing.

> - In section 4: s/the Link MSD MUST take preference/the Link MSD MUST
> take precedence/
> 
[Les:] Done.

> - In section 5, BMI-MSD is defined as "the total number of MPLS  labels which
> can be imposed" (which is OK when the incoming packet is unlabled). When
> the incoming packet is labeled (e.g. use of segment routing binding SID), if
> the incoming label is to be "replaced" by N outgoing labels, what processing
> model is assumed when advertising the MSD:
>  * one swap and one imposition of N-1 labels?
>  * one pop and one imposition of N labels?
> 
[Les:] What is being defined is the maximum number of SIDs/labels which can be "imposed". If popping or swapping affects the MSD which can be supported this needs to be accounted for in the advertisement. BMI-MSD is not being used to advertise a value specific to labeled or unlabeled packets nor a value which is "swap specific" or "pop specific". 

> - For the BMI-MSD use case, the draft seems to clearly target a value
> attached to the outgoing link. However, this capability on a router is highly
> hardware-dependent: some implementations may push a stack on the
> egress linecard while some may perform it on the ingress linecard. The I-D
> seems to make some implicit hardware assumption and the current link MSD
> advertisement is not suited to describe these possible combinations of
> hardware implementations. This needs to be addressed somehow.
> 
[Les:] If a platform does imposition on ingress, then it should advertise only a Node MSD - as there would be no way to advertise a value which is specific to the ingress-egress interface combination. So, link MSDs for BMI are associated with the use of that interface as the outgoing link.

> - Section 7 says that "an MSD that is incorrect, may result in [...] instantiation
> of a path that can't be supported". It would be more accurate to mention
> "instantiation attempt", as a proper state control mechanism is expected to
> deny unsupported instantiation (e.g. PCEP errors for "invalid objects" include
> "Unsupported number of SR-ERO subobjects").
>
[Les:] I replaced "instantiation" with "calculation".
 
> *Nits:*
> - s/(IS-IS) Router to advertise/(IS-IS) router to advertise/

[Les:] Done
> - s/link a given SR path/each node/link of a given SR path/

[Les:] Done
> - s/path doesn't exceed/path does not exceed/

[Les:] Done. You don't like contractions? :-)

> - s/and associated attributes and capabilities/as well as associated attributes
> and capabilities/

[Les:] I prefer the existing wording. 

> - s/it is expected, that/it is expected that/

[Les:] Done

> ---
> - s/an IANA managed registry/an IANA-managed registry/

[Les:] I am going to leave this to the RFC editor if you don’t mind. I am not fully convinced that a compound adjective is appropriate here.

> - s/defined by this document/defined by this document:/

[Les:] Done

> - s/path that can't be/path that cannot be/

[Les:] Done

    Les

> ---
> 
> Regards,
> 
> Julien