[Lsr] AD Review for draft-ietf-ospf-yang-21
Alvaro Retana <aretana.ietf@gmail.com> Wed, 05 June 2019 22:26 UTC
Return-Path: <aretana.ietf@gmail.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 D1D22120025; Wed, 5 Jun 2019 15:26:29 -0700 (PDT)
X-Virus-Scanned: amavisd-new at amsl.com
X-Spam-Flag: NO
X-Spam-Score: 0.812
X-Spam-Level:
X-Spam-Status: No, score=0.812 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, RAZOR2_CF_RANGE_51_100=1.886, RAZOR2_CHECK=0.922, RCVD_IN_DNSWL_NONE=-0.0001, SPF_HELO_NONE=0.001, SPF_PASS=-0.001, UNPARSEABLE_RELAY=0.001, URIBL_BLOCKED=0.001] autolearn=no 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 JibSRXeYJZgh; Wed, 5 Jun 2019 15:26:26 -0700 (PDT)
Received: from mail-ed1-x52f.google.com (mail-ed1-x52f.google.com [IPv6:2a00:1450:4864:20::52f]) (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 00BF3120094; Wed, 5 Jun 2019 15:26:25 -0700 (PDT)
Received: by mail-ed1-x52f.google.com with SMTP id z25so193573edq.9; Wed, 05 Jun 2019 15:26:25 -0700 (PDT)
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=from:mime-version:date:message-id:subject:to:cc; bh=fB2ISORUfRkMFZ2FgXX51GDlRcSg9C+G2432VmfCDjY=; b=BVrNasjVIoWMtPi4aN9tEZnlSfu47ArphvzjVKRIIkQ5BlpfnKpRr7GpFWent68hY4 diUXChT9isNMpH/4W0eqeEwjbeSxA9c9WRHzc+ayTxRwd05ht9OxtS8VgmxofzSDOkNF 9qFTYAV2NVlaOs39k/ow6cggd1C8oeeUd6oY4/jX4RmqHEKNol4Ys2d139ioT56a66If wEhrxeLg6kM5QovWlpxXvttr9fUqq8CLNIiDwba58GtKc2a7CZ4i+0CUsbIkbugmplzw zKayj8gszgsIClPYEJblH56vDwLEAoLmsjBFSYKpgr3qK7QoIki8gxeCLawShpJNacow m2CQ==
X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:mime-version:date:message-id:subject:to:cc; bh=fB2ISORUfRkMFZ2FgXX51GDlRcSg9C+G2432VmfCDjY=; b=rjCezMx8s6ZxugHNuvurFLioY82Kmyk90Yu8eWZ+MmFsH2K50wV82M9pt/NeSY1ix6 rzAWrn++IJCJPRJIAsrrcOEQ/iCAgTtL75NrmeIX+2T/rJkZhPXdAR1RVR347kUnZHkc eh7hAtOVEJFEyMUGks3cbSEhblADo7rzStMZbU1Z7LzZLCgHgidx9RkTnMPJaTFnoTSA LjcD3wNMdMZOtSvYVsq6Sc/9J3SkoKTSSdlwycHSNvCL/un5MhQOyXsxt/V813InKNzT l71OIulCfg3RNgRcDbKMD32EUXdKL2e1PQOb28vePafRjz+BBeClhH+b9l5PteaBaNcE dEEw==
X-Gm-Message-State: APjAAAVJkqP8LF6/HEzMMT3IgmXaqSNKZWl/FvNcq1baB29Om7uJQnRv IeYdszcdf2vnn+tl5XAoI0ydEwLI2KN2W+nYSBO+fQ==
X-Google-Smtp-Source: APXvYqynLpkGFdlz2KZSManFXVZEo1i5PecUGMtysIEkUuBqkD/QgsEcRAtB24u8xxm3l4aSoSDI+hBEoLLPIIJ/5J8=
X-Received: by 2002:aa7:c545:: with SMTP id s5mr45700787edr.217.1559773584155; Wed, 05 Jun 2019 15:26:24 -0700 (PDT)
Received: from 1058052472880 named unknown by gmailapi.google.com with HTTPREST; Wed, 5 Jun 2019 15:26:23 -0700
From: Alvaro Retana <aretana.ietf@gmail.com>
MIME-Version: 1.0
Date: Wed, 05 Jun 2019 15:26:23 -0700
Message-ID: <CAMMESsztO1a4fnT2Gx2GDKcYVLtWS52WZ=HmPdQ9VFqSEtvG7Q@mail.gmail.com>
To: draft-ietf-ospf-yang@ietf.org
Cc: stephane.litkowski@orange.com, lsr-chairs@ietf.org, lsr@ietf.org
Content-Type: multipart/alternative; boundary="00000000000069e934058a9b165d"
Archived-At: <https://mailarchive.ietf.org/arch/msg/lsr/LKvR2rfkPHJui_GMRerIdDNHkug>
Subject: [Lsr] AD Review for draft-ietf-ospf-yang-21
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: Wed, 05 Jun 2019 22:26:30 -0000
Dear authors: I just finished reading this document. I have some comments in-line, including a couple that I consider major -- I'll wait for whose to be addressed before moving forward. The datatracker reports 4 errors in the model, mostly related to ietf-bfd-types. Please take a look. Thanks! Alvaro. [Line numbers from id-nits.] ... 81 1. Overview ... 92 This document defines a YANG data model that can be used to configure 93 and manage OSPF and it is an augmentation to the core routing data 94 model. If fully conforms to the Network Management Datastore 95 Architecture (NDMA) [RFC8342]. A core routing data model is defined 96 in [RFC8349], and it provides the basis for the development of data 97 models for routing protocols. The interface data model is defined in 98 [RFC8343] and is used for referencing interfaces from the routing 99 protocol. The key-chain data model used for OSPF authentication is 100 defined in [RFC8177] and provides both a reference to configured key- 101 chains and an enumeration of cryptographic algorithms. [nit] s/If fully conforms/It fully conforms 103 Both OSPFv2 [RFC2328] and OSPFv3 [RFC5340] are supported. In 104 addition to the core OSPF protocol, features described in other OSPF 105 RFCs are also supported. These includes demand circuit [RFC1793], 106 traffic engineering [RFC3630], multiple address family [RFC5838], 107 graceful restart [RFC3623] [RFC5187], NSSA [RFC3101], and OSPF(v3) as 108 a PE-CE Protocol [RFC4577], [RFC6565]. These non-core features are 109 optional in the OSPF data model. [nit] s/OSPF(v3) as a PE-CE Protocol/use as a PE-CE Protocol Using "OSPF(v3)" is confusing because it is not clear whether OSPFv3-only is meant or both, but v2 is not mentioned... ... 138 2.1. OSPF Operational State 140 The OSPF operational state is included in the same tree as OSPF 141 configuration consistent with Network Management Datastore 142 Architecture [RFC8342]. Consequently, only the routing container in 143 the ietf-routing model [RFC8349] is augmented. The routing-state 144 container is not augmented. [nit] s/consistent with Network/consistent with the Network 146 2.2. Overview ... 154 module: ietf-ospf 155 augment /rt:routing/rt:control-plane-protocols/ 156 rt:control-plane-protocol: 157 +--rw ospf 158 . 159 . 160 +--rw operation-mode? identityref 161 +--rw af? identityref 162 . 163 . 164 +--rw areas 165 | +--rw area* [area-id] 166 | +--rw area-id area-id-type 167 | . 168 | . 169 | +--rw virtual-links 170 | | +--rw virtual-link* [transit-area-id router-id] 171 | | . 172 | | . 173 | +--rw sham-links {pe-ce-protocol}? 174 | | +--rw sham-link* [local-id remote-id] 175 | | . 176 | | . 177 | +--rw interfaces 178 | +--rw interface* [name] 179 | . 180 | . 181 +--rw topologies {multi-topology}? 182 +--rw topology* [name] 183 . 184 . 186 The ospf module is intended to match to the vendor specific OSPF 187 configuration construct that is identified by the local identifier 188 'name'. The field 'version' allows support for OSPFv2 and OSPFv3. [minor] The 'version' field doesn't appear in the tree above. §2.3 says the same thing with more details. Perhaps take the sentence out of this paragraph. 190 The ospf container includes one OSPF protocol engine instance. The 191 instance includes OSPF router level configuration and operational 192 state. [??] One of the things that is confusing me is the instance concept in the model. I understand instances in OSPF, and I see how the module is organized around instances ("grouping instance-..."), including even per-instance Router-ID. But I don't see anywhere the definition of an instance-id (except attached to an interface (§2.7) and specific to OSPFv3 (§3))...if wanting to configure (or read from) a specific instance, how does the client differentiate? [YANG is not my thing, so this may be obvious to everyone else. If it is, a quick pointer would be very appreciated.] ... 209 2.4. Optional Features ... 221 3. explicit-router-id: Support explicit per-instance Router-ID 222 specification. [minor] Is there a reference? Should the document point to rfc5340/rfc6549 here? ... 234 8. ttl-security: Support OSPF Time to Live (TTL) security check 235 suppression [RFC5082]. [minor] s/suppression/support Right? 237 9. nsr: Support OSPF Non-Stop Routing (NSR). [minor] Reference? ... 242 11. admin-control: Support Administrative control of the protocol 243 state. [minor] This seems like a "basic" feature: enabling/disabling the protocol. Why was it decided to make it optional? Are there implementations that don't support enabling/disabling? ... 261 17. ospfv2-authentication-trailer: Support OSPFv2 Authentication 262 trailer as specified in [RFC5709] or [RFC7166]. [minor] s/RFC7166/RFC7474 ... 267 19. ospfv3-authentication-trailer: Support OSPFv3 Authentication 268 trailer as specified in [RFC7474]. [minor] s/RFC7474/RFC7166 ... 300 2.5. OSPF Router Configuration/Operational State ... 308 module: ietf-ospf ... 336 +--rw enable? boolean {admin-control}? [nit] Perhaps the admin-control should be moved closer to the start of this branch so it is clear what is being enabled/disabled. ... 464 2.6. OSPF Area Configuration/Operational State ... 570 | | +--rw hello-interval? uint16 571 | | +--rw dead-interval? uint32 572 | | +--rw retransmit-interval? uint16 [minor] Why aren't rt-types:timer-value-* used for the *-timer/interval definitions? I would have thought this was exactly the reason for the definitions in rfc8294. ... 1054 2.9. OSPF RPC Operations ... 1061 o clear-neighbor: restart a particular set of OSPF neighbor. [minor] Should the description be something like: "reset a set of OSPF neighbors on a particular interface"? 1063 rpcs: 1064 +---x clear-neighbor 1065 | +---w input 1066 | +---w routing-protocol-name 1067 | + -> /rt:routing/control-plane-protocols/ 1068 | + control-plane-protocol/name 1069 | +---w interface? if:interface-ref ... 1076 3. OSPF YANG Module ... 1143 Editor: Derek Yeung 1144 <mailto:derek@arrcus.com> 1145 Author: Acee Lindem 1146 <mailto:acee@cisco.com> 1147 Author: Yingzhen Qu 1148 <mailto:yingzhen.qu@huawei.com> 1149 Author: Jeffrey Zhang 1150 <mailto:zzhang@juniper.net> 1151 Author: Ing-Wher Chen 1152 <mailto:ingwherchen@mitre.org> 1153 Author: Dean Bogdanovic 1154 <mailto:ivandean@gmail.com> 1155 Author: Kiran Agrahara Sreenivasa 1156 <mailto:kk@employees.org"; [minor] This list is not the same as what is in the front page of this document. ... 1685 typedef if-state-type { 1686 type enumeration { ... 1712 enum backup { 1713 value "6"; 1714 description 1715 "Interface Backup Designated Router (BDR) state."; 1716 } [nit] s/backup/bdr To match the other instances where a BDR is referenced. ... 2729 container network { 2730 when "derived-from-or-self(../../header/type, " 2731 + "'ospfv3-network-lsa')" { 2732 description 2733 "Only applies to Network LSAs."; 2734 } 2735 description "Network LSA."; [nit] Perhaps network-lsa could be a better name. ... 3053 grouping lsa-common { 3054 description 3055 "Common fields for OSPF LSA representation."; 3056 leaf decoded-completed { 3057 type boolean; 3058 description 3059 "The OSPF LSA body is fully decoded."; 3060 } [minor] What is decoded-completed indicating? Does it indicate that no errors where found, IOW, "successful decoding"? Does it mean that the LSA has been decoded (but not an indication of success, or not)? Is it a time-based indication; IOW, is there a chance that an LSA has not been processed when retrieving the information...would that result in false?? ... 3271 grouping instance-fast-reroute-state { 3272 description "IPFRR state data grouping"; [nit] s/IPFRR/IP-FRR That is how the term is used everywhere else in this document. ... 3541 leaf hello-interval { 3542 type uint16 { 3543 range "1..65535"; 3544 } 3545 description 3546 "Interval between hello packets (seconds). It must 3547 be the same for all routers on the same network. 3548 Different networks, implementations, and deployments 3549 will use different hello-intervals. A sample value 3550 for a LAN network would be 10 seconds."; 3551 } [minor] Some implementations support having a sub-second hello-interval...but with a timer in seconds that can't be done. ... 3664 leaf ospfv2-key { 3665 type string; 3666 description 3667 "OSPFv2 authentication key. The 3668 length of the key may be dependent on the 3669 cryptographic algorithm. In cases where it is 3670 not, a key length of at least 32 octets should 3671 be supported to allow for interoperability 3672 with strong keys."; [major] Is there a reference for the part about the "key length of at least 32 octets should be supported to allow for interoperability with strong keys"? ... 3719 leaf ospfv3-key { 3720 type string; 3721 description 3722 "OSPFv2 authentication key. The 3723 length of the key may be dependent on the 3724 cryptographic algorithm. In cases where it is 3725 not, a key length of at least 32 octets should 3726 be supported to allow for interoperability 3727 with strong keys."; [major] The same comment from ospfv2-key applies here. [nit] s/OSPFv2 authentication key/OSPFv3 authentication key ... 3856 leaf priority { 3857 type uint8 { 3858 range "1..255"; 3859 } 3860 description "Neighbor priority for DR election."; 3861 } [minor] A range is included in this definition, but not in other priority statements before...should a range be included in those other definitions? [minor] Why is 0 not applicable here? ... 3936 leaf dead-timer { 3937 type uint32; 3938 units "seconds"; 3939 config false; 3940 description "This timer tracks the remaining time before 3941 the neighbor is declared dead."; 3942 } [major] For *-timer: Is tracking the remaining time in seconds enough? I would assume that ms would be the right unit. Why seconds? ... 4390 container preference { 4391 description "Route preference config state."; [minor] Maybe it's just me, but I don't understand what this piece of the model does. It seems like it can be used to configure which type of route is preferred: external over intra-area, for example...but that doens't make too much sense to me. Is that the intended use? Can you please give me an example of when this functionality could be used? 4392 choice scope { 4393 description 4394 "Options for expressing preference 4395 as single or multiple values."; 4396 case single-value { 4397 leaf all { 4398 type uint8; 4399 description 4400 "Preference for intra-area, inter-area, and 4401 external routes."; 4402 } 4403 } 4404 case multi-values { 4405 choice granularity { 4406 description 4407 "Options for expressing preference 4408 for intra-area and inter-area routes."; 4409 case detail { 4410 leaf intra-area { 4411 type uint8; 4412 description 4413 "Preference for intra-area routes."; 4414 } 4415 leaf inter-area { 4416 type uint8; 4417 description 4418 "Preference for inter-area routes."; 4419 } 4420 } 4421 case coarse { 4422 leaf internal { 4423 type uint8; 4424 description 4425 "Preference for both intra-area and 4426 inter-area routes."; 4427 } 4428 } 4429 } 4430 leaf external { 4431 type uint8; 4432 description 4433 "Preference for AS external routes."; 4434 } 4435 } 4437 } 4438 } ... 4551 container stub-router { 4552 if-feature stub-router; 4553 description "Set maximum metric configuration"; 4555 choice trigger { 4556 description 4557 "Specific triggers which will enable stub 4558 router state."; 4559 container always { 4560 presence 4561 "Enables unconditional stub router support"; 4562 description 4563 "Unconditional stub router state (advertise 4564 transit links with max metric"; [minor] s//MaxLinkMetric [rfc6897] ... 5537 4. Security Considerations ... 5552 There are a number of data nodes defined in ietf-ospf.yang module 5553 that are writable/creatable/deletable (i.e., config true, which is 5554 the default). These data nodes may be considered sensitive or 5555 vulnerable in some network environments. Write operations (e.g., 5556 edit-config) to these data nodes without proper protection can have a 5557 negative effect on network operations. For OSPF, the ability to 5558 modify OSPF configuration will allow the entire OSPF domain to be 5559 compromised including peering with unauthorized routers to misroute 5560 traffic or mount a massive Denial-of-Service (DoS) attack. The 5561 security considerations of OSPFv2 [RFC2328] and [RFC5340] apply to 5562 the ietf-ospf.yang module as well. [minor] s/OSPFv2 [RFC2328] and [RFC5340]/OSPFv2 [RFC2328] and OSPFv3 [RFC5340] [major] How do the "security considerations of OSPFv2 [RFC2328] and [RFC5340] apply to the ietf-ospf.yang module"? rfc2328/rfc5340 only talk about authentication of the protocol exchange... ... 5626 7.1. Normative References ... 5679 [RFC4750] Joyal, D., Ed., Galecki, P., Ed., Giacalone, S., Ed., 5680 Coltun, R., and F. Baker, "OSPF Version 2 Management 5681 Information Base", RFC 4750, DOI 10.17487/RFC4750, 5682 December 2006, <https://www.rfc-editor.org/info/rfc4750>. ... 5731 [RFC5643] Joyal, D., Ed. and V. Manral, Ed., "Management Information 5732 Base for OSPFv3", RFC 5643, DOI 10.17487/RFC5643, August 5733 2009, <https://www.rfc-editor.org/info/rfc5643>. ... 5745 [RFC5880] Katz, D. and D. Ward, "Bidirectional Forwarding Detection 5746 (BFD)", RFC 5880, DOI 10.17487/RFC5880, June 2010, 5747 <https://www.rfc-editor.org/info/rfc5880>. 5749 [RFC5881] Katz, D. and D. Ward, "Bidirectional Forwarding Detection 5750 (BFD) for IPv4 and IPv6 (Single Hop)", RFC 5881, 5751 DOI 10.17487/RFC5881, June 2010, <https://www.rfc- 5752 editor.org/info/rfc5881>. [minor] I think these references can be Informative. ... 5880 7.2. Informative References [major] All the references in this section (except maybe rfc905) point at functionality in the model...as the references above, these should be Normative as well. These documents are already in the Downref registry: rfc5309, rfc5443, rfc5714, rfc6987. The others are not, but I don't expect any issues in the IETF Last Call.
- [Lsr] AD Review for draft-ietf-ospf-yang-21 Alvaro Retana
- Re: [Lsr] AD Review for draft-ietf-ospf-yang-21 Acee Lindem (acee)
- Re: [Lsr] AD Review for draft-ietf-ospf-yang-21 Alvaro Retana
- Re: [Lsr] AD Review for draft-ietf-ospf-yang-21 Acee Lindem (acee)
- Re: [Lsr] AD Review for draft-ietf-ospf-yang-21 Alvaro Retana
- Re: [Lsr] AD Review for draft-ietf-ospf-yang-21 Acee Lindem (acee)
- Re: [Lsr] AD Review for draft-ietf-ospf-yang-21 Alvaro Retana
- Re: [Lsr] AD Review for draft-ietf-ospf-yang-21 Les Ginsberg (ginsberg)
- Re: [Lsr] AD Review for draft-ietf-ospf-yang-21 Jeffrey Haas
- Re: [Lsr] AD Review for draft-ietf-ospf-yang-21 Acee Lindem (acee)
- Re: [Lsr] AD Review for draft-ietf-ospf-yang-21 Acee Lindem (acee)
- Re: [Lsr] AD Review for draft-ietf-ospf-yang-21 Alvaro Retana
- Re: [Lsr] AD Review for draft-ietf-ospf-yang-21 Acee Lindem (acee)