Re: [Lsr] AD Review of draft-ietf-isis-segment-routing-msd-13

"Les Ginsberg (ginsberg)" <ginsberg@cisco.com> Wed, 15 August 2018 22:51 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 3A11B130DED; Wed, 15 Aug 2018 15:51:43 -0700 (PDT)
X-Virus-Scanned: amavisd-new at amsl.com
X-Spam-Flag: NO
X-Spam-Score: -14.509
X-Spam-Level:
X-Spam-Status: No, score=-14.509 tagged_above=-999 required=5 tests=[BAYES_00=-1.9, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, HTML_MESSAGE=0.001, RCVD_IN_DNSWL_HI=-5, SPF_PASS=-0.001, T_DKIMWL_WL_HIGH=-0.01, URIBL_BLOCKED=0.001, 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 WTOwuPC_YZqp; Wed, 15 Aug 2018 15:51:39 -0700 (PDT)
Received: from alln-iport-8.cisco.com (alln-iport-8.cisco.com [173.37.142.95]) (using TLSv1.2 with cipher DHE-RSA-SEED-SHA (128/128 bits)) (No client certificate requested) by ietfa.amsl.com (Postfix) with ESMTPS id 6393D130DD9; Wed, 15 Aug 2018 15:51:39 -0700 (PDT)
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=cisco.com; i=@cisco.com; l=108990; q=dns/txt; s=iport; t=1534373499; x=1535583099; h=from:to:cc:subject:date:message-id:references: in-reply-to:mime-version; bh=V8K6T5eU6FSV1JoaOiWzPUFNafLQ9G2Xmrl2ROgqJIM=; b=Wpevdz91G485QfPNejKi+hvN4O6zcL5wqtbA1Oh9QC51NdoErQSt1IVs xk9PUihBMAgSuq6g6ig7Ebr3uC1uLsCemG5LE2UEHTG3Hd0mb2yyeRnHH HCFgLpyX/Gcz6gVLOtTdfp06BSOzN5P9sp67K2/+d9NHvJ6D199IWFzC/ c=;
X-IronPort-Anti-Spam-Filtered: true
X-IronPort-Anti-Spam-Result: A0DNAACgrXRb/5pdJa1TChoBAQEBAQIBAQEBCAEBAQGCV0kvY38oCoNkiAqMNIINiFGNQhSBZgMIJYRHAheDHSE0GAECAQECAQECbRwMhTcBAQEBAxoJCj4OEAIBBgIOAwQBASEBCQICAh8RHQgCBAENBQiDG4EdTAMVD494mhoBgTCBLociDYMsBTSIYBeBQT+BEQGDEoJWRQICgScNQg+CW4JVAogFhG0ThTWIBisJAocYhTGDCB2BOoQuiESLZ4cTAhEUgSQdOIFScBU7gmmCJReIWYU+bwEBjSSBGwEB
X-IronPort-AV: E=Sophos;i="5.53,244,1531785600"; d="scan'208,217";a="157582035"
Received: from rcdn-core-3.cisco.com ([173.37.93.154]) by alln-iport-8.cisco.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 15 Aug 2018 22:51:38 +0000
Received: from XCH-ALN-004.cisco.com (xch-aln-004.cisco.com [173.36.7.14]) by rcdn-core-3.cisco.com (8.15.2/8.15.2) with ESMTPS id w7FMpb2Y001528 (version=TLSv1.2 cipher=AES256-SHA bits=256 verify=FAIL); Wed, 15 Aug 2018 22:51:37 GMT
Received: from xch-aln-001.cisco.com (173.36.7.11) by XCH-ALN-004.cisco.com (173.36.7.14) with Microsoft SMTP Server (TLS) id 15.0.1320.4; Wed, 15 Aug 2018 17:51:37 -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.1320.000; Wed, 15 Aug 2018 17:51:37 -0500
From: "Les Ginsberg (ginsberg)" <ginsberg@cisco.com>
To: Alvaro Retana <aretana.ietf@gmail.com>, "draft-ietf-isis-segment-routing-msd@ietf.org" <draft-ietf-isis-segment-routing-msd@ietf.org>
CC: "lsr-chairs@ietf.org" <lsr-chairs@ietf.org>, "lsr@ietf.org" <lsr@ietf.org>, Christian Hopps <chopps@chopps.org>
Thread-Topic: AD Review of draft-ietf-isis-segment-routing-msd-13
Thread-Index: AQHUNNoQmHI5FO69m0uobgW9g1VMU6TBXRPA
Date: Wed, 15 Aug 2018 22:51:36 +0000
Message-ID: <d2de842b864a4a7a98f646c748828fe6@XCH-ALN-001.cisco.com>
References: <CAMMESsxptarNYpLnNHA3mB+QBzb=RV0si1NNScPZdNJw4UyLog@mail.gmail.com>
In-Reply-To: <CAMMESsxptarNYpLnNHA3mB+QBzb=RV0si1NNScPZdNJw4UyLog@mail.gmail.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.68.145]
Content-Type: multipart/alternative; boundary="_000_d2de842b864a4a7a98f646c748828fe6XCHALN001ciscocom_"
MIME-Version: 1.0
X-Outbound-SMTP-Client: 173.36.7.14, xch-aln-004.cisco.com
X-Outbound-Node: rcdn-core-3.cisco.com
Archived-At: <https://mailarchive.ietf.org/arch/msg/lsr/Lo0GZKlorOzPABopw7n_G4otkn0>
Subject: Re: [Lsr] AD Review of draft-ietf-isis-segment-routing-msd-13
X-BeenThere: lsr@ietf.org
X-Mailman-Version: 2.1.27
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: Wed, 15 Aug 2018 22:51:43 -0000

Alvaro –

A very thorough review – thanx.

Jeff has the pen – but I think he is on holiday at the moment – so there may be a short delay as regards a new version.
I will confine myself to comments on the non-editorial issues.
Inline.

From: Alvaro Retana <aretana.ietf@gmail.com>
Sent: Wednesday, August 15, 2018 1:53 PM
To: draft-ietf-isis-segment-routing-msd@ietf.org
Cc: lsr-chairs@ietf.org; lsr@ietf.org; Christian Hopps <chopps@chopps.org>
Subject: AD Review of draft-ietf-isis-segment-routing-msd-13

Dear authors:

I just finished reading this document.  I have several comments and concerns that I included inline below.

One item that I want to highlight here is the lack of specific procedures defined to handle the cases of multiple advertisements (in both §2 and §3).  Please take a look at my specific comments below -- in short, a clear specification is required for proper interoperability.  I will wait for (at least) this item to be addressed before starting the IETF LC.

Thanks!

Alvaro.



[The line numbers came from the idnits output.]

...
76       1.  Introduction
...
95         links in the network MSD is relevant, MSD capabilites should be
96         advertised by every IS-IS router in the network.

[nit] s/capabilites/capabilities

...
109       or SIDs associated with another dataplane e.g., IPv6.  Although MSD
110       advertisements are associated with Segment Routing, the
111       advertisements MAY be present even if Segment Routing itself is not
112       enabled.

[minor] Given that you're using Normative language...  It would be nice if you expanded on the use of the MSD in a non-SR network.  Something simple such as "a SID and a label are the same thing" would be enough.

114     1.1.  Conventions used in this document

116     1.1.1.  Terminology

[minor] Except for BMI/MSD, the other terms are not definitions, just expansions.  Some of the abbreviations are already included in the RFC Editor Abbreviations List [1].  In general, it would be better to just expand on first use (BGP-LS, for example, is used *before* this section) than to have this section with expansions.

[1] https://www.rfc-editor.org/materials/abbrev.expansion.txt

...
147     2.  Node MSD Advertisement
...
156              0                   1
157              0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5

159             +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
160             |    Type       |   Length      |
161             +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
162             |   MSD-Type    | MSD Value     |
163             +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
164             //     ...................     //
165             +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
166             |   MSD-Type    | MSD Value     |
167             +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+

169                            Figure 1: Node MSD Sub-TLV

171       Type: 23 (allocated by IANA via the early assignment process)

173       Length: variable (minimum of 2, multiple of 2 octets) and represents
174       the total length of value field.

[nit] ...in octets (?).

176       Value: field consists of one or more pairs of a 1 octet MSD-Type and
177       1 octet MSD-Value.

[nit] There is no "Value" field illustrated above.  You might want to reword a little.

[nit] The figure says "MSD Value", but the text talks about "MSD-Value".

...
191       If there exist multiple Node MSD advertisements for the same MSD-Type
192       originated by the same router, the procedures defined in [RFC7981]
193       apply.

[major] Does this text refer to multiple node MSD sub-TLVs (inside the same, or different, IS-IS Router CAPABILITY TLV), or to the same MSD-Type (included multiple times in a node MSD sub-TLV), or both?

[Les:] It really doesn’t matter. If you have two advertisements for the same MSD type from the same source then the procedures defined in RFC 7981 apply. It does not matter whether you find the advertisements in the same sub-TLV, in the same Router Capabilities TLV but different sub-TLVs, or in different Router Capabilities TLVs(sic).



[major] The only relevant text I can find in rfc7981 is this:

   Where a receiving system has two copies of an IS-IS Router CAPABILITY
   TLV from the same system that have conflicting information for a
   given sub-TLV, the procedure used to choose which copy shall be used
   is undefined.

[Les:] Your searching skills are excellent. ☺
RFC 7981 declined to define procedures for reasons which are explained in the three paragraphs prior to the one you have quoted.
If you have a problem with that please raise it in the context of RFC 7981 – not in the context of this draft.

I then don't know how to handle the multiple advertisements.  Please point me in the right direction.

195     3.  Link MSD Advertisement

197       The link MSD sub-TLV is defined for TLVs 22, 23, 25, 141, 222, and
198       223 to carry the MSD of the interface associated with the link.  MSD
199       values may be learned via a hardware API or may be provisioned.

[nit] A reference to the appropriate RFCs would be nice.

[Les:] You are asking for an RFC reference for each of the mentioned TLV types (22, 23,…)???
Given that information is readily available here:  https://www.iana.org/assignments/isis-tlv-codepoints/isis-tlv-codepoints.xhtml why should we clutter a draft with duplicate info??


201             0                   1
202             0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5

204             +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
205             |    Type       |   Length      |
206             +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
207             |   MSD-Type    | MSD Value     |
208             +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
209             //     ...................     //
210             +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
211             |   MSD-Type    | MSD Value     |
212             +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+

214                            Figure 2: Link MSD Sub-TLV

216       Type: 15 (allocated by IANA via the early assignment process)

218       Length: variable (minimum of 2, multiple of 2 octets) and represents
219       the total length of value field.

[nit] ...in octets (?).

221       Value: consists of one or more pairs of a 1 octet MSD-Type and 1
222       octet Value.

[nit] There is no "Value" field illustrated above.  You might want to reword a little.

[nit] The figure says "MSD Value", but the text talks about "Value".

...
235       If multiple Link MSD advertisements for the same MSD Type and the
236       same link are received, the procedure used to select which copy is
237       used is undefined.

[major] Does this text refer to multiple link MSD sub-TLVs (inside the same, or different, IS-IS Router CAPABILITY TLV), or to the same MSD-Type (included multiple times in a link MSD sub-TLV), or both?

[Les:] As with node MSD, it does not matter. What matters is that you have duplicate advertisements for the same link and MSD type.
Ohhh…and these advertisements are not in Router Capability TLV. ☺


[major] Without a procedure "it is unlikely that multiple implementations of the specification would interoperate" [2].

[Les:] The issue is not interoperability but that you do not know which one is correct. So no matter which one you choose you might use a value that is either not supported by the advertising node or limits label imposition unnecessarily.
I really don’t think there is an interoperability issue here.


[2] https://www.ietf.org/blog/discuss-criteria-iesg-review/


239     4.  Using Node and Link MSD Advertisements

[major] After reading this section, I still don't know how do use the advertisements.  What should a receiver do with the values?  Maybe the use is constrained to a controller...maybe the exact operation is our of the scope of this document.  Either way, please say something.

241       When Link MSD is present for a given MSD type, the value of the Link
242       MSD MUST take preference over the Node MSD.  When a Link MSD type is
243       not signalled but the Node MSD type is, then the Node MSD type value
244       MUST be considered as the MSD value for that link.

[nit] s/signalled/signaled

...
258     5.  Base MPLS Imposition MSD

260       Base MPLS Imposition MSD (BMI-MSD) signals the total number of MPLS
261       labels a node is capable of imposing, including all
262       service/transport/special labels.

264       Absence of BMI-MSD advertisements indicates solely that the
265       advertising node does not support advertisement of this capability.

[major] The MSD Types are applicable for both nodes and links, right?  The description above only talks about nodes -- what about links?

[Les:] This section is not specific to link advertisements or node advertisements. It is talking about what it means when there is no applicable advertisement of BMI-MSD.
What is an applicable advertisement? That is explained in Section 4.
For a given link I either have an advertisement for that link or I have a node advertisement. If I have neither, I have no information and so you can infer that the node “declined to state”.

267     6.  IANA Considerations

269       This document requests IANA to allocate a sub-TLV type for the new
270       sub TLV proposed in Section 2 of this document from IS-IS Router
271       Capability TLV Registry as defined by [RFC7981].

[minor] The registry is called "Sub-TLVs for TLV 242 (IS-IS Router CAPABILITY TLV)". [3]

[3] https://www.iana.org/assignments/isis-tlv-codepoints/isis-tlv-codepoints.xhtml#isis-tlv-codepoints-242

...
303       This document requests creation of an IANA managed registry under a
304       new category of "Interior Gateway Protocol (IGP) Parameters" IANA
305       registries to identify MSD types as proposed in Section 2 and
306       Section 3.  The registration procedure is "Expert Review" as defined
307       in [RFC8126].  Suggested registry name is "IGP MSD Types".  Types are
308       an unsigned 8 bit number.  The following values are defined by this
309       document

[nit] s/under a new category/under the category

[major] This creation of the registry needs to include the "required documentation and review criteria, giving clear guidance to the designated expert" -- please see §4.5 in rfc8126.

[Les:] Guidance for Designated Experts – at least for IS-IS codepoints – has been defined in RFC 7170. Would it be sufficient to refer to that document and state that it applies in this case as well??
(I sure hope so. ☺ )

311          Value     Name                             Reference
312          -----     ---------------------            -------------
313          0         Reserved                         This document

[major] 0 is not Reserved, but has a specific meaning (from §2 and §3).

[Les:] I am confused by your comment.
What is being reserved is the value “0” as an MSD-Type (See Figures 1 and 2) – not 0 as an MSD-Value.
Please help me to understand what text in the draft is in conflict??

314          1         Base MPLS Imposition MSD         This document
315          2-250     Unassigned                       This document
316          251-254   Experimental                     This document
317          255       Reserved                         This document

319                      Figure 6: MSD Types Codepoints Registry

321     7.  Security Considerations

323       Security considerations as specified by [RFC7981] are applicable to
324       this document.

326       Advertisement of the additional information defined in this document
327       that is false, e.g., an MSD that is incorrect, may result in a path
328       computation failing, having a service unavailable, or instantiation
329       of a path that can't be supported by the head-end (the node
330       performing the imposition).

[major] rfc7981 says that "specifications based on this mechanism need to describe the security considerations around the disclosure and modification of their information".  I think that the paragraph above applies also to modification.  What about disclosure?

[Les:]I think the same issues apply to disclosure. How about if we added:

“The presence of this information also may inform an attacker of how to induce any of the aforementioned conditions.”

???

   Les


...
364     10.2.  Informative References

[major] rfc8126 should be Normative.

...
390       [RFC8126]  Cotton, M., Leiba, B., and T. Narten, "Guidelines for
391                  Writing an IANA Considerations Section in RFCs", BCP 26,
392                  RFC 8126, DOI 10.17487/RFC8126, June 2017,
393                  <https://www.rfc-editor.org/info/rfc8126>.