Re: [yang-doctors] Yangdoctors last call review of draft-ietf-ospf-yang-09

"Acee Lindem (acee)" <acee@cisco.com> Mon, 08 January 2018 19:34 UTC

Return-Path: <acee@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 5FA66124B18; Mon, 8 Jan 2018 11:34:28 -0800 (PST)
X-Virus-Scanned: amavisd-new at amsl.com
X-Spam-Flag: NO
X-Spam-Score: -14.531
X-Spam-Level:
X-Spam-Status: No, score=-14.531 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, 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 2TsBt5nfSASH; Mon, 8 Jan 2018 11:34:26 -0800 (PST)
Received: from rcdn-iport-4.cisco.com (rcdn-iport-4.cisco.com [173.37.86.75]) (using TLSv1.2 with cipher DHE-RSA-SEED-SHA (128/128 bits)) (No client certificate requested) by ietfa.amsl.com (Postfix) with ESMTPS id E8EAE12426E; Mon, 8 Jan 2018 11:34:25 -0800 (PST)
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=cisco.com; i=@cisco.com; l=6348; q=dns/txt; s=iport; t=1515440066; x=1516649666; h=from:to:cc:subject:date:message-id:references: in-reply-to:content-id:content-transfer-encoding: mime-version; bh=FX0fsNKz8etBTIQsrrFde8OW/Pyoapfq+ofazDQPe1M=; b=fA77LiWVA1LxzcfyaxW31EGzLbc02atmWBtpp6hnhRdMl9DL8oGLUr/F KNvrEe6fTCJhVDQXYdXDF8gaqwHgfWRJIlLVZRuKDib2LLEv8BXaw9Jf/ tBM91Njn2ZdnqqE1P9a4Ew+F9f5+1wWCsl4UJQZFVFkxlgjK/l5kJP65f 0=;
X-IronPort-Anti-Spam-Filtered: true
X-IronPort-Anti-Spam-Result: A0DxAwBDxlNa/5pdJa1dGQEBAQEBAQEBAQEBAQcBAQEBAYM/ZnQnB4QAmH2CApk/ChgLhRgCGoQcQxQBAQEBAQEBAQFrKIUkAQEBAwEBIRE6CxACAQgSBgICJgICAiULFQIOAgQBDQWKMRCvGIInijQBAQEBAQEBAQEBAQEBAQEBAQEBAQEYBYEPgxGCFYM/gy6DLwGBboMXgkUgBaNeApU8lAmHX48LAhEZAYE7ATYiP4ERbxUZJIIqhFd4iViBFwEBAQ
X-IronPort-AV: E=Sophos;i="5.46,332,1511827200"; d="scan'208";a="340468022"
Received: from rcdn-core-3.cisco.com ([173.37.93.154]) by rcdn-iport-4.cisco.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 08 Jan 2018 19:34:24 +0000
Received: from XCH-RTP-015.cisco.com (xch-rtp-015.cisco.com [64.101.220.155]) by rcdn-core-3.cisco.com (8.14.5/8.14.5) with ESMTP id w08JYOYD002745 (version=TLSv1/SSLv3 cipher=AES256-SHA bits=256 verify=FAIL); Mon, 8 Jan 2018 19:34:24 GMT
Received: from xch-rtp-015.cisco.com (64.101.220.155) by XCH-RTP-015.cisco.com (64.101.220.155) with Microsoft SMTP Server (TLS) id 15.0.1320.4; Mon, 8 Jan 2018 14:34:23 -0500
Received: from xch-rtp-015.cisco.com ([64.101.220.155]) by XCH-RTP-015.cisco.com ([64.101.220.155]) with mapi id 15.00.1320.000; Mon, 8 Jan 2018 14:34:23 -0500
From: "Acee Lindem (acee)" <acee@cisco.com>
To: Ladislav Lhotka <lhotka@nic.cz>, Martin Bjorklund <mbj@tail-f.com>
CC: "yang-doctors@ietf.org" <yang-doctors@ietf.org>, "draft-ietf-ospf-yang.all@ietf.org" <draft-ietf-ospf-yang.all@ietf.org>
Thread-Topic: [yang-doctors] Yangdoctors last call review of draft-ietf-ospf-yang-09
Thread-Index: AQHTboUcIbs87wVZj0yzoeuFibmmLKM2hmeAgAALVgCANACPAA==
Date: Mon, 08 Jan 2018 19:34:23 +0000
Message-ID: <D67931B0.E8CE1%acee@cisco.com>
References: <151255960762.30655.17225294251460480729@ietfa.amsl.com> <20171206.124611.1073986528289329597.mbj@tail-f.com> <1512563206.2653.32.camel@nic.cz>
In-Reply-To: <1512563206.2653.32.camel@nic.cz>
Accept-Language: en-US
Content-Language: en-US
X-MS-Has-Attach:
X-MS-TNEF-Correlator:
x-ms-exchange-messagesentrepresentingtype: 1
x-ms-exchange-transport-fromentityheader: Hosted
x-originating-ip: [10.116.152.198]
Content-Type: text/plain; charset="utf-8"
Content-ID: <6B7A81479B12A74C964C07F225A0A3E0@emea.cisco.com>
Content-Transfer-Encoding: base64
MIME-Version: 1.0
Archived-At: <https://mailarchive.ietf.org/arch/msg/yang-doctors/YVyW1almBuzgTGiDb10ZNNIzmYM>
Subject: Re: [yang-doctors] Yangdoctors last call review of draft-ietf-ospf-yang-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: Mon, 08 Jan 2018 19:34:28 -0000

Hi Martin, 

We will incorporate these comments as well. I agree we could take another
pass at the descriptions.

Thanks,
Acee 

On 12/6/17, 7:26 AM, "Ladislav Lhotka" <lhotka@nic.cz> wrote:

>On Wed, 2017-12-06 at 12:46 +0100, Martin Bjorklund wrote:
>> Ladislav Lhotka <lhotka@nic.cz> wrote:
>> > Reviewer: Ladislav Lhotka
>> > Review result: Ready with Issues
>> > 
>> > The data model defined in this document is a massive piece of work: it
>> > consists of 11 YANG modules and defines around 1200 schema nodes. The
>> 
>> 11 YANG modules?  Isn't there just one?
>
>Yes, but I meant the complete data model including the existing modules
>that
>must be implemented along with "ietf-ospf", such as "ietf-interface",
>"ietf-
>routing" and "ietf-keychain". I think it is a higher league of data
>modelling
>compared to a stand-alone module - kudos to the authors.
>
>> 
>> 
>> 
>> I would like to add two quick comments to Lada's review.
>> 
>> o  Remove all "revision" statements except the one that says "initial
>>    revision".   The idea is to have one revision statement per
>>    published version (i.e., RFC).
>
>Right, the revisions make the module excessively long.
>
>> 
>> 
>> o  Many of the nodes have quite rudimentary descriptions, and the
>>    "reference" statement is rarely used.  For example:
>> 
>>          leaf lsa-id {
>>            type yang:dotted-quad;
>>            mandatory true;
>>            description "LSA ID.";
>>          }
>> 
>>    It seems as the description is put there just to keep the tools
>>    quiet.  I know that it is painful to write good descriptions, but
>>    it does help the consumer of the data model.
>
>Agreed.
>
>Thanks, Lada
>
>> 
>> 
>> /martin
>> 
>> 
>> > "ietf-ospf@2017-10-30" module is compatible with the NMDA
>>architecture.
>> > 
>> > **** Comments
>> > 
>> > 1. Unless there is a really compelling reason not to do so, the
>> >    "ietf-ospf" should declare YANG version 1.1. For one,
>> >    "ietf-routing" that is being augmented by "ietf-ospf" already
>> >    declares this version. Some of my suggestions below also assume
>> >    version 1.1.
>> > 
>> > 2. The "ietf-ospf" can work only with the new NMDA-compatible
>> >    revisions of some modules, such as "ietf-interfaces" and
>> >    "ietf-routing". I understand it is not desirable to import such
>> >    modules by revision, but at least it should be mentioned in a
>> >    description attached to every such import.
>> > 
>> > 3. Maybe the draft could mention that implementations should supply a
>> >    default routing domain as a system-controlled resource.
>> > 
>> > 4. In "when" expressions, the module uses literal strings for
>> >    identities. This is known to be problematic, the XPath functions
>> >    derived-from() or derived-from-or-self() should be used instead.
>> > 
>> > 5. Some enumerations, such as "packet-type" and "if-state-type"
>> >    define enum identifiers with uppercase letters and/or underscores,
>> >    for example "Database-Description" or "LONG_WAIT". RFC6087bis
>> >    recommends that only lowercase letters, numbers and dashes. I think
>> >    this convention should be observed despite the fact that the
>> >    current names are traditionally used in OSPF specs. The
>> >    "ietf-routing" module also defines "router-id" even though the
>> >    documents use "Router ID".
>> > 
>> > 6. The types of LSA headers are modelled as integers. While OSPF gurus
>> >    probably know these numbers by heart, it is not very
>> >    reader-frienly. So at least some references to documents defining
>> >    these numbers should be provided, but my suggestion is to consider
>> >    implementing them with identities. It seems it might also be useful
>> >    to define some "abstract" identities for these types. For example,
>> >    if "opaque-lsa" is defined, then the definition of container
>> >    "opaque" could simply use
>> > 
>> >      when "derived-from(../../header/type, 'ospf:opaque-lsa')";
>> > 
>> >    instead of
>> > 
>> >       when "../../header/type = 9 or "
>> >               + "../../header/type = 10 or "
>> >               + "../../header/type = 11";
>> > 
>> > 7. The title of sec. 2.9 should be "OSPF Notifications" rather than
>> >    "OSPF notification".
>> > 
>> > 
>> > _______________________________________________
>> > yang-doctors mailing list
>> > yang-doctors@ietf.org
>> > https://www.ietf.org/mailman/listinfo/yang-doctors
>> > 
>-- 
>Ladislav Lhotka
>Head, CZ.NIC Labs
>PGP Key ID: 0xB8F92B08A9F76C67