[yang-doctors] YANG doctor review of draft-ietf-i2rs-yang-l3-topology-02

"Carl Moberg (camoberg)" <camoberg@cisco.com> Mon, 04 July 2016 11:54 UTC

Return-Path: <camoberg@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 07E5B127058; Mon, 4 Jul 2016 04:54:50 -0700 (PDT)
X-Virus-Scanned: amavisd-new at amsl.com
X-Spam-Flag: NO
X-Spam-Score: -15.947
X-Spam-Level:
X-Spam-Status: No, score=-15.947 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, RP_MATCHES_RCVD=-1.426, SPF_PASS=-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 zhN3EItR7Ggd; Mon, 4 Jul 2016 04:54:48 -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 77E5B12D0A7; Mon, 4 Jul 2016 04:54:48 -0700 (PDT)
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=cisco.com; i=@cisco.com; l=5908; q=dns/txt; s=iport; t=1467633288; x=1468842888; h=from:to:subject:date:message-id:content-id: content-transfer-encoding:mime-version; bh=PTMi+ofzJsrym6zkaNPtn2sSByiWwuy5uiynygMJn60=; b=ieIsA6Q/9XpRxL+OIXbQkbW6L/VS6N2A2qaIRI2BuDSTXgAwGKeCe9SN q1lbBcLjM/CjRmV6rek4Fs1X25Id4ONhmJeACl34BNW8SbKJkP6taI4B+ loSvT8rTPz9Uzsc/6f7Vd0T1mYWUFwQuiucVtD9v8B26kKVPD133KHWdt M=;
X-IronPort-Anti-Spam-Filtered: true
X-IronPort-Anti-Spam-Result: =?us-ascii?q?A0A6AgC4TXpX/5hdJa1cgz6BUga3IIIPg?= =?us-ascii?q?XmGNoEZOBQBAQEBAQEBZSeEUyMRPhkBIgImAgQwFRIEARKIMKo+j0IBAQEBAQE?= =?us-ascii?q?EAQEBAQEigQGFJ4F4ihYrghIdBYYHkwwBi3SCUo8qkAkBHjaDcG6IDX8BAQE?=
X-IronPort-AV: E=Sophos;i="5.26,574,1459814400"; d="scan'208";a="292547586"
Received: from rcdn-core-1.cisco.com ([173.37.93.152]) by alln-iport-4.cisco.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 04 Jul 2016 11:54:47 +0000
Received: from XCH-RCD-012.cisco.com (xch-rcd-012.cisco.com [173.37.102.22]) by rcdn-core-1.cisco.com (8.14.5/8.14.5) with ESMTP id u64BslPY029400 (version=TLSv1/SSLv3 cipher=AES256-SHA bits=256 verify=FAIL); Mon, 4 Jul 2016 11:54:47 GMT
Received: from xch-rcd-015.cisco.com (173.37.102.25) by XCH-RCD-012.cisco.com (173.37.102.22) with Microsoft SMTP Server (TLS) id 15.0.1210.3; Mon, 4 Jul 2016 06:54:47 -0500
Received: from xch-rcd-015.cisco.com ([173.37.102.25]) by XCH-RCD-015.cisco.com ([173.37.102.25]) with mapi id 15.00.1210.000; Mon, 4 Jul 2016 06:54:47 -0500
From: "Carl Moberg (camoberg)" <camoberg@cisco.com>
To: "draft-ietf-i2rs-yang-l3-topology.all@ietf.org" <draft-ietf-i2rs-yang-l3-topology.all@ietf.org>, YANG Doctors <yang-doctors@ietf.org>
Thread-Topic: YANG doctor review of draft-ietf-i2rs-yang-l3-topology-02
Thread-Index: AQHR1ercuAtS/pjYnkezuhHJeFkSAg==
Date: Mon, 4 Jul 2016 11:54:47 +0000
Message-ID: <B7626001-CBFC-4879-9340-7C1D784AC6C6@cisco.com>
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.147.40.92]
Content-Type: text/plain; charset="utf-8"
Content-ID: <BF9B1EE4F0EF6E46B3671CDA2AC4CB33@emea.cisco.com>
Content-Transfer-Encoding: base64
MIME-Version: 1.0
Archived-At: <https://mailarchive.ietf.org/arch/msg/yang-doctors/Z7nM70pZHyN1Vesmp-wx8sGm7Iw>
Subject: [yang-doctors] YANG doctor review of draft-ietf-i2rs-yang-l3-topology-02
X-BeenThere: yang-doctors@ietf.org
X-Mailman-Version: 2.1.17
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, 04 Jul 2016 11:54:50 -0000

All,

This is my review of draft-ietf-i2rs-yang-l3-topology-02 and the included
YANG modules. I am the assigned YANG doctor for this WG document.

 The reviewed document includes the following YANG modules:
  - A module for Layer 3 Unicast IGP topology (ietf-l3-unicast-igp-topology)
  - A module for OSPF topology (ietf-ospf-topology)
  - A module for IS-IS topology (ietf-isis-topology)

Overall, the document is in reasonable shape (though in need of some language
editing) but needs more attention around its fundamental scope and some
high-level issues.

My top-level feedback is that the relationships between the l3 unicast IGP
topology module and the protocol-specific ones are somewhat under-explained.
I don’t understand if the OSPF and IS-IS models are mere examples or if the
aim is to have them become normative. The modules themselves are not marked
as examples (no 'example-' prefix), but e.g. the OSPF module is referred to
as "[...] an example of how the general topology model can be refined […]"
in running text.

If the protocol-specific modules are to become part of the normative
specification, I believe there needs to be additional work together with the
OSPF and IS-IS WGs and YANG authors to align efforts. IMHO the main topic
being what kind of share or reuse should we do across the topology-centric
models and the respective device layer modules (ietf-ospf, ietf-isis).

If the protocol-specific modules are to be classified as examples, I would
suggest that needs to be clearly stated, and the modules marked up
accordingly.

I believe the above needs to be resolved before a more detailed review of the
OSPF and IS-IS modules and document sections can be done.

The rest of this review focuses on some feedback on the layer 3 unicast IGP
topology section of the draft and associated YANG module (ietf-network-
topology).

1. All data definitions are optional configuration (e.g. no config false,
default values or mandatory statements). This seems to open up for many odd
combinations of e.g. existing and non-existing leafs in node, link, and
termination-points. This is also true for the notifications. Is this by design
or something that needs additional work?

2. I might be missing something subtle here, but the 'network-ref', 'node-
ref', 'link-ref', and 'tp-ref' groupings seems to be copied from ietf-network
and ietf-network-topology. Unless there is some reasoning behind this, it
seems we could just drop them and use imported versions.

3. For the notifications, it is not clear to me what the semantics of the
*-attributes groupings are. Section 3.2 states that "[...] as a convenience to
applications, additional data of the affected node, or link, or termination
point (respectively) is included.". Does this mean that the (optional) content
of e.g. the igp-node-attributes grouping in the igp-node-event notification
always describes the running configuration of the referenced node at the time
of the notification? Also assuming that that point in time is after the
attribute- change event happened. I would suggest we need to be more explicit
in the running text as well as in the description field of the events.

4. Another one on the notifications. The extensibility model of the
notifications seems to be based on augmenting a presence container into the
top-level of each notification. E.g. the isis module examplifies this by
augmenting the isis-topology-type container into /l3t:igp-node-event. This
means that the resulting igp-node-event now has two topology type-containers,
one from the main module (called 'l3-unicast-igp-topology') and one from the
isis-module (called 'isis'). Are there any semantics implicit in the
combinationi (e.g. should isis-related notifications only include the isis-
container)?

5. A couple of smaller nits:
 - This has been discussed (and I haven't seen any formal conclusions), but I
   suggest we don't need value-statements for enums
 - The import prefixes are not aliged with the prefixes in the imported modules:
  + ietf-network is imported with prefix 'nw', while the module itself has 'nd'
  + ietf-network-topology is imported with prefix 'nt', while the module itself
    has 'lnk'

Regards,
--
Carl Moberg
camoberg@cisco.com