Re: [yang-doctors] Yangdoctors early review of draft-ietf-i2rs-rib-data-model-09
"Reshad Rahman (rrahman)" <rrahman@cisco.com> Sat, 24 February 2018 18:45 UTC
Return-Path: <rrahman@cisco.com>
X-Original-To: yang-doctors@ietfa.amsl.com
Delivered-To: yang-doctors@ietfa.amsl.com
Received: from localhost (localhost [127.0.0.1]) by ietfa.amsl.com (Postfix) with ESMTP id 328A0127275; Sat, 24 Feb 2018 10:45:38 -0800 (PST)
X-Virus-Scanned: amavisd-new at amsl.com
X-Spam-Flag: NO
X-Spam-Score: -14.53
X-Spam-Level:
X-Spam-Status: No, score=-14.53 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, RCVD_IN_MSPIKE_H3=-0.01, RCVD_IN_MSPIKE_WL=-0.01, SPF_PASS=-0.001, T_RP_MATCHES_RCVD=-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 GcaYDycZY-E2; Sat, 24 Feb 2018 10:45:35 -0800 (PST)
Received: from rcdn-iport-3.cisco.com (rcdn-iport-3.cisco.com [173.37.86.74]) (using TLSv1.2 with cipher DHE-RSA-SEED-SHA (128/128 bits)) (No client certificate requested) by ietfa.amsl.com (Postfix) with ESMTPS id D842112008A; Sat, 24 Feb 2018 10:45:34 -0800 (PST)
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=cisco.com; i=@cisco.com; l=15840; q=dns/txt; s=iport; t=1519497934; x=1520707534; h=from:to:cc:subject:date:message-id:references: in-reply-to:content-id:content-transfer-encoding: mime-version; bh=hNkELq/v3xiwiBFQEb+BusKXUOUUPcwDqQvuO+59X1k=; b=RaRFtkUmipR5mxx3suHfjAJQ9TY2Bt96arNlX23v4LtPgEl3Pyqmj6Fi CXTr2CTkQAm0kpvloCrLFHlhLcpypa68bqwX78gUVSlXtm/cfqs40JIw5 xqmDNaSyKitfbqwGNjvrmlveWgYSClMkzP+z/tNZhl6qRCK0C5zELjOqg 0=;
X-IronPort-Anti-Spam-Filtered: true
X-IronPort-Anti-Spam-Result: A0AyAQD/sZFa/4sNJK1dGQEBAQEBAQEBAQEBAQcBAQEBAYNPZnAoCoNKiiKNfIFbJ4EWlgUUggIKGAuFEAIagh5UGAECAQEBAQEBAmsohSMBAQEDAQEBIREzBwsMBAIBCA4DBAEBAQICIwMCAgIlCxQBCAgCBAENBYUICBCrVYIniGOCHQEBAQEBAQEBAQEBAQEBAQEBAQEBAR2BD4YzgVeBZikMgWqBDoMuAQGBNhUmDAuCdjGCFCAFoWEJAoc3hzuFLIIGIkCFBINlhw+VSAIRGQGBLgEeOIFRcBUZISoBghgJgjocFoFld4oVASUHgQOBFwEBAQ
X-IronPort-AV: E=Sophos;i="5.47,389,1515456000"; d="scan'208";a="349557354"
Received: from alln-core-6.cisco.com ([173.36.13.139]) by rcdn-iport-3.cisco.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 24 Feb 2018 18:45:33 +0000
Received: from XCH-ALN-005.cisco.com (xch-aln-005.cisco.com [173.36.7.15]) by alln-core-6.cisco.com (8.14.5/8.14.5) with ESMTP id w1OIjX1c022106 (version=TLSv1/SSLv3 cipher=AES256-SHA bits=256 verify=FAIL); Sat, 24 Feb 2018 18:45:33 GMT
Received: from xch-rcd-005.cisco.com (173.37.102.15) by XCH-ALN-005.cisco.com (173.36.7.15) with Microsoft SMTP Server (TLS) id 15.0.1320.4; Sat, 24 Feb 2018 12:45:32 -0600
Received: from xch-rcd-005.cisco.com ([173.37.102.15]) by XCH-RCD-005.cisco.com ([173.37.102.15]) with mapi id 15.00.1320.000; Sat, 24 Feb 2018 12:45:32 -0600
From: "Reshad Rahman (rrahman)" <rrahman@cisco.com>
To: Ebben Aries <exa@juniper.net>, Amit Dass <amit.dass@ericsson.com>
CC: "yang-doctors@ietf.org" <yang-doctors@ietf.org>, "draft-ietf-i2rs-rib-data-model.all@ietf.org" <draft-ietf-i2rs-rib-data-model.all@ietf.org>, "i2rs@ietf.org" <i2rs@ietf.org>
Thread-Topic: [yang-doctors] Yangdoctors early review of draft-ietf-i2rs-rib-data-model-09
Thread-Index: AQHTql74NZDcOf6W7Ui/Yk7TkcQS86OzPusAgADtfID//87pgA==
Date: Sat, 24 Feb 2018 18:45:32 +0000
Message-ID: <80B87B5B-1659-474E-B7AD-120133EB9540@cisco.com>
References: <151626438402.10775.7250014155413667607@ietfa.amsl.com> <HE1PR0702MB3580F99E1E4BBB98E84D1DF593CF0@HE1PR0702MB3580.eurprd07.prod.outlook.com> <20180224023114.hvp3qkqlva27ntio@smtp.juniper.net> <20180224164113.q2a4crmy6o77peim@smtp.juniper.net>
In-Reply-To: <20180224164113.q2a4crmy6o77peim@smtp.juniper.net>
Accept-Language: en-US
Content-Language: en-US
X-MS-Has-Attach:
X-MS-TNEF-Correlator:
user-agent: Microsoft-MacOutlook/10.a.0.180210
x-ms-exchange-messagesentrepresentingtype: 1
x-ms-exchange-transport-fromentityheader: Hosted
x-originating-ip: [10.86.249.164]
Content-Type: text/plain; charset="utf-8"
Content-ID: <9A6A10A434CBC14BACC04396986EE8F5@emea.cisco.com>
Content-Transfer-Encoding: base64
MIME-Version: 1.0
Archived-At: <https://mailarchive.ietf.org/arch/msg/yang-doctors/yrSDZHdO2AyGGbAyNlbojDovq7I>
Subject: Re: [yang-doctors] Yangdoctors early review of draft-ietf-i2rs-rib-data-model-09
X-BeenThere: yang-doctors@ietf.org
X-Mailman-Version: 2.1.22
Precedence: list
List-Id: Email list of the yang-doctors directorate <yang-doctors.ietf.org>
List-Unsubscribe: <https://www.ietf.org/mailman/options/yang-doctors>, <mailto:yang-doctors-request@ietf.org?subject=unsubscribe>
List-Archive: <https://mailarchive.ietf.org/arch/browse/yang-doctors/>
List-Post: <mailto:yang-doctors@ietf.org>
List-Help: <mailto:yang-doctors-request@ietf.org?subject=help>
List-Subscribe: <https://www.ietf.org/mailman/listinfo/yang-doctors>, <mailto:yang-doctors-request@ietf.org?subject=subscribe>
X-List-Received-Date: Sat, 24 Feb 2018 18:45:38 -0000
Regarding reference to 7223, should this be replaced by 7223bis (I got that comment for another draft)? On 2018-02-24, 11:41 AM, "yang-doctors on behalf of Ebben Aries" <yang-doctors-bounces@ietf.org on behalf of exa@juniper.net> wrote: On Feb 23 19:31 PM, Ebben Aries wrote: > Amit, > > I took your latest pre -11 .xml revision that you attached so below is a > re-review against -10 and pre (-11). Comments below > > > Compilation error that was introduced between -10 and -11 > - ietf-i2rs-rib@2017-12-05.yang:1224: error: unexpected keyword "range" > > leaf hop-limit { > type uint8; > range "1..255"; > description > "The path hop limit header."; > } > - Also, by introducing a range on a uint8, are you saying that a hop-limit=0 > would never be in use for any use-case here? > > There are formatting and alignment issues all throughout the document that > need fixing (Mahesh pointed this out as well) > > 'ietf-interfaces' import carries the wrong RFC reference (RFC 7227) > - This should rather be RFC 7277 > I meant RFC 7223 rather import ietf-interfaces { prefix if; reference "RFC 7223: A YANG Data Model for Interface Management"; } > There are still ongoing discussions among yang-doctors on how references in > imports should be treated. At a minimum for now, lets carry the expanded > syntax w/ the title of the RFC as such: (As Mahesh previous mentioned, this is > subject to change pending further discussion) > - reference "RFC 6991: Common YANG Data Types"; > - reference "RFC 7277: A YANG Data Model for IP Management"; > - reference "RFC 6991: Common YANG Data Types"; > > Module description still does not carry full copyright/distribution section as > specified in: https://urldefense.proofpoint.com/v2/url?u=https-3A__tools.ietf.org_html_draft-2Dietf-2Dnetmod-2Drfc6087bis-2D15-23appendix-2DC&d=DwICAg&c=HAkYuh63rsuhr6Scbfh0UjBXeMK-ndb3voDTXcWzoCI&r=GIehbDpQlo31lSi6WbnEkA&m=oqu2ooyPcpQ1BsLe7O0eVojE-sP-YgMoHTpQoRTH954&s=nZJnXRNh9vlo9VZexb3zXgyY6mCzlN1ztsrmPjezh50&e= > > There still appears to be one remaining 'Rib' reference > - s/Routing Instance and Rib/Routing Instance and RIB/ > > Section 1.2 - Tree Diagrams > - The text here is an instruction template rather than what the actual text > should be. It should rather read: > > A simplified graphical representation of the data model is used in > this document. The meaning of the symbols in these diagrams is > defined in [I-D.ietf-netmod-yang-tree-diagrams]. > > You did not address my previous comment/question re: NMDA (Tom mentioned this > as well) > > You did not address my previous comment/question re: Section 2.5 (route-add > RPC) > > I still have concern to modules introducing large amounts of > features/if-features as previously noted but will defer to other's comments on > this as well > > Formatting nits: > - L3605: s/route:A/route: A/ > - L3610: s/from\/to RIB/from\/to a RIB/ > - L3610: s/lead to suboptimal/lead to a suboptimal/ > - L3611: s/possibly/possible/ > - L3626: s/his/their/ > > Tom's previous comment re: ENABLE_IP_RPF_CHECK has not been addressed and > exists in both -10 and (pre) -11 versions > > Thx > > /ebben > > On Feb 20 15:24 PM, Amit Dass wrote: > > Hi Ebben, > > > > I have updated the draft based on your comments. Could you please have a look at the same and provide your feedback? > > > > https://urldefense.proofpoint.com/v2/url?u=https-3A__datatracker.ietf.org_doc_draft-2Dietf-2Di2rs-2Drib-2Ddata-2Dmodel_&d=DwIGaQ&c=HAkYuh63rsuhr6Scbfh0UjBXeMK-ndb3voDTXcWzoCI&r=GIehbDpQlo31lSi6WbnEkA&m=Bd9RZ2mCWMYNPyhFBtSbqwApeVo8_SOmuA3FNG3trxA&s=h0scwYvh7UsfradRz7u1meraj47zZhr6vH3qiHJXiZ4&e= > > > > Best regards, > > Amit > > > > -----Original Message----- > > From: Ebben Aries [mailto:exa@juniper.net] > > Sent: Thursday, January 18, 2018 9:33 AM > > To: yang-doctors@ietf.org > > Cc: i2rs@ietf.org; draft-ietf-i2rs-rib-data-model.all@ietf.org; ietf@ietf.org > > Subject: Yangdoctors early review of draft-ietf-i2rs-rib-data-model-09 > > > > Reviewer: Ebben Aries > > Review result: On the Right Track > > > > 1 module in this draft: > > - ietf-i2rs-rib@2017-12-05.yang > > > > No YANG validation errors or warnings (from pyang 1.7.3 and yanglint 0.14.59) > > > > 0 examples are provided in this draft (section 3.12 of > > draft-ietf-netmod-rfc6087bis-15) > > > > Module ietf-i2rs-rib@2017-12-05.yang: > > - yang-version statement missing - should be 1.1 > > - prefix 'iir' is recommended for this module, would 'rib' suffice better? > > - import "ietf-inet-types" should reference RFC 6991 per (not as a comment) > > https://urldefense.proofpoint.com/v2/url?u=https-3A__tools.ietf.org_html_draft-2Dietf-2Dnetmod-2Drfc6087bis-2D15-23section-2D4.7&d=DwIGaQ&c=HAkYuh63rsuhr6Scbfh0UjBXeMK-ndb3voDTXcWzoCI&r=GIehbDpQlo31lSi6WbnEkA&m=Bd9RZ2mCWMYNPyhFBtSbqwApeVo8_SOmuA3FNG3trxA&s=dYhxw9bO0Uk78WJ5uTBmc5ZcElOT446ETFp7DXU546s&e= > > - import "ietf-interfaces" should reference RFC 7223 per > > https://urldefense.proofpoint.com/v2/url?u=https-3A__tools.ietf.org_html_draft-2Dietf-2Dnetmod-2Drfc6087bis-2D15-23section-2D4.7&d=DwIGaQ&c=HAkYuh63rsuhr6Scbfh0UjBXeMK-ndb3voDTXcWzoCI&r=GIehbDpQlo31lSi6WbnEkA&m=Bd9RZ2mCWMYNPyhFBtSbqwApeVo8_SOmuA3FNG3trxA&s=dYhxw9bO0Uk78WJ5uTBmc5ZcElOT446ETFp7DXU546s&e= > > - import "ietf-yang-types" should reference RFC 6991 per > > https://urldefense.proofpoint.com/v2/url?u=https-3A__tools.ietf.org_html_draft-2Dietf-2Dnetmod-2Drfc6087bis-2D15-23section-2D4.7&d=DwIGaQ&c=HAkYuh63rsuhr6Scbfh0UjBXeMK-ndb3voDTXcWzoCI&r=GIehbDpQlo31lSi6WbnEkA&m=Bd9RZ2mCWMYNPyhFBtSbqwApeVo8_SOmuA3FNG3trxA&s=dYhxw9bO0Uk78WJ5uTBmc5ZcElOT446ETFp7DXU546s&e= > > - Since this module imports "ietf-interfaces", a normative references must be > > added per > > https://urldefense.proofpoint.com/v2/url?u=https-3A__tools.ietf.org_html_draft-2Dietf-2Dnetmod-2Drfc6087bis-2D15-23section-2D3.9&d=DwIGaQ&c=HAkYuh63rsuhr6Scbfh0UjBXeMK-ndb3voDTXcWzoCI&r=GIehbDpQlo31lSi6WbnEkA&m=Bd9RZ2mCWMYNPyhFBtSbqwApeVo8_SOmuA3FNG3trxA&s=IaSTmgPV-jaH8vLp0nasBDWLSQ9-dDRqYwAkIvsNfkY&e= > > - prefix "if" in the import "ietf-interfaces" can remove quotes to remain > > consistent with other imports > > - Remove WG Chairs from contact information per > > https://urldefense.proofpoint.com/v2/url?u=https-3A__tools.ietf.org_html_draft-2Dietf-2Dnetmod-2Drfc6087bis-2D15-23appendix-2DC&d=DwIGaQ&c=HAkYuh63rsuhr6Scbfh0UjBXeMK-ndb3voDTXcWzoCI&r=GIehbDpQlo31lSi6WbnEkA&m=Bd9RZ2mCWMYNPyhFBtSbqwApeVo8_SOmuA3FNG3trxA&s=1U0JCnUyX_pqexjCB9KgzvhWsdLDQMN_iOgkNv29dEs&e= > > - Module description must contain most recent copyright notice per > > https://urldefense.proofpoint.com/v2/url?u=https-3A__tools.ietf.org_html_draft-2Dietf-2Dnetmod-2Drfc6087bis-2D15-23appendix-2DC&d=DwIGaQ&c=HAkYuh63rsuhr6Scbfh0UjBXeMK-ndb3voDTXcWzoCI&r=GIehbDpQlo31lSi6WbnEkA&m=Bd9RZ2mCWMYNPyhFBtSbqwApeVo8_SOmuA3FNG3trxA&s=1U0JCnUyX_pqexjCB9KgzvhWsdLDQMN_iOgkNv29dEs&e= > > - Module description should contain note to RFC Ed. and placeholder reference > > to RFC when assigned > > https://urldefense.proofpoint.com/v2/url?u=https-3A__tools.ietf.org_html_draft-2Dietf-2Dnetmod-2Drfc6087bis-2D15-23appendix-2DC&d=DwIGaQ&c=HAkYuh63rsuhr6Scbfh0UjBXeMK-ndb3voDTXcWzoCI&r=GIehbDpQlo31lSi6WbnEkA&m=Bd9RZ2mCWMYNPyhFBtSbqwApeVo8_SOmuA3FNG3trxA&s=1U0JCnUyX_pqexjCB9KgzvhWsdLDQMN_iOgkNv29dEs&e= > > - Add placeholder reference and note to RFC Ed. for RFC when assigned > > https://urldefense.proofpoint.com/v2/url?u=https-3A__tools.ietf.org_html_draft-2Dietf-2Dnetmod-2Drfc6087bis-2D15-23appendix-2DC&d=DwIGaQ&c=HAkYuh63rsuhr6Scbfh0UjBXeMK-ndb3voDTXcWzoCI&r=GIehbDpQlo31lSi6WbnEkA&m=Bd9RZ2mCWMYNPyhFBtSbqwApeVo8_SOmuA3FNG3trxA&s=1U0JCnUyX_pqexjCB9KgzvhWsdLDQMN_iOgkNv29dEs&e= > > - Security Considerations should be updated to reflect new template at > > https://urldefense.proofpoint.com/v2/url?u=https-3A__trac.ietf.org_trac_ops_wiki_yang-2Dsecurity-2Dguidelines&d=DwIGaQ&c=HAkYuh63rsuhr6Scbfh0UjBXeMK-ndb3voDTXcWzoCI&r=GIehbDpQlo31lSi6WbnEkA&m=Bd9RZ2mCWMYNPyhFBtSbqwApeVo8_SOmuA3FNG3trxA&s=tfCcnSFoI3jjNNzCj6pgkp94JZ-quOQEqMlv9hITBog&e= > > - Section 1.2 should be replaced with reference to > > draft-ietf-netmod-yang-tree-diagrams-02 rather (as-is in other i2rs YANG > > drafts in progress) per > > https://urldefense.proofpoint.com/v2/url?u=https-3A__tools.ietf.org_html_draft-2Dietf-2Dnetmod-2Drfc6087bis-2D15-23section-2D2.5.1&d=DwIGaQ&c=HAkYuh63rsuhr6Scbfh0UjBXeMK-ndb3voDTXcWzoCI&r=GIehbDpQlo31lSi6WbnEkA&m=Bd9RZ2mCWMYNPyhFBtSbqwApeVo8_SOmuA3FNG3trxA&s=DjKFw7t6_CU_2_03rwIHxgvz5_oC5mjX_rMUC5UuV6M&e= > > - This module contains '12' features. While it is understood the purpose of > > these features in the module, take precaution as to complexity for clients > > if they need to understand >= quantity of features per module in use on a > > network-element. > > - A few comments exist that are either unecessary or redundant. Encode the > > comment intent rather in description fields if need be. > > - Per NMDA, which datastores are targeted for the module? Will all RPC > > operations be acting upon the dynamic/ephemeral datastore? It is not clear > > to me if the intention is to be persistent or ephemeral > > > > General comments/Nits: > > - references to 'def' could be expanded out to 'definition' > > - references to 'decap' could be expanded out to 'decapsulation' for > > readability (across definitions and descriptions) > > - Follow consistent capitalization of 'RIB' throughout document text. Mixed > > use of 'Rib' and 'rib' exists (Outside of YANG node lowercase definitions). > > - Is it necessary to prefix all nodes under the nexthop container with > > "nexthop-"? > > - Section 2.5 - route-add RPC - text mentions it is required that the nh-add > > RPC be called as a pre-requisite however if the nh already exists and the > > nexthop-id is known, this should not be necessary. In addition, the text > > reads 'or return' which should rather be a result of querying the > > appropriate node in the data tree. > > - In 'IANA Considerations' - s/This document requests to register/This > > document registers/ > > > > _______________________________________________ > yang-doctors mailing list > yang-doctors@ietf.org > https://urldefense.proofpoint.com/v2/url?u=https-3A__www.ietf.org_mailman_listinfo_yang-2Ddoctors&d=DwICAg&c=HAkYuh63rsuhr6Scbfh0UjBXeMK-ndb3voDTXcWzoCI&r=GIehbDpQlo31lSi6WbnEkA&m=oqu2ooyPcpQ1BsLe7O0eVojE-sP-YgMoHTpQoRTH954&s=GTV9rhtmIT5nIY6w8DKgR9QiRak314uAIzfjhtj_z-k&e= _______________________________________________ yang-doctors mailing list yang-doctors@ietf.org https://www.ietf.org/mailman/listinfo/yang-doctors
- [yang-doctors] Yangdoctors early review of draft-… Ebben Aries
- Re: [yang-doctors] Yangdoctors early review of dr… Amit Dass
- Re: [yang-doctors] Yangdoctors early review of dr… Susan Hares
- Re: [yang-doctors] Yangdoctors early review of dr… Amit Dass
- Re: [yang-doctors] Yangdoctors early review of dr… Amit Dass
- Re: [yang-doctors] Yangdoctors early review of dr… Mahesh Jethanandani
- Re: [yang-doctors] [i2rs] Yangdoctors early revie… t.petch
- Re: [yang-doctors] Yangdoctors early review of dr… Ebben Aries
- Re: [yang-doctors] Yangdoctors early review of dr… Ebben Aries
- Re: [yang-doctors] Yangdoctors early review of dr… Reshad Rahman (rrahman)