Early Review of draft-ietf-i2rs-yang-l3-topology-10
review-ietf-i2rs-yang-l3-topology-10-yangdoctors-early-lhotka-2017-07-26-00

Request Review of draft-ietf-i2rs-yang-l3-topology
Requested rev. no specific revision (document currently at 16)
Type Early Review
Team YANG Doctors (yangdoctors)
Deadline 2017-07-07
Requested 2017-06-22
Requested by Susan Hares
Other Reviews Rtgdir Early review of -01 by Tony Przygienda (diff)
Opsdir Telechat review of -08 by Ron Bonica (diff)
Secdir Telechat review of -08 by Hilarie Orman (diff)
Genart Last Call review of -08 by Paul Kyzivat (diff)
Yangdoctors Early review of -02 by Carl Moberg (diff)
Rtgdir Early review of -10 by Christian Hopps (diff)
Rtgdir Early review of -10 by Michael Richardson (diff)
Secdir Last Call review of -13 by Hilarie Orman (diff)
Genart Last Call review of -13 by Paul Kyzivat (diff)
Comments
We've done one full round of QA reviews.  This round to make sure the latest model fits the latest in the NETMOD revised datastores structure.   Pelase note the base topology moidel may impact other modesl.
Review State Completed
Reviewer Ladislav Lhotka
Review review-ietf-i2rs-yang-l3-topology-10-yangdoctors-early-lhotka-2017-07-26
Posted at https://mailarchive.ietf.org/arch/msg/yang-doctors/ZdwwMUpvXaBiCQN6A3hm76joS7Q
Reviewed rev. 10 (document currently at 16)
Review result Ready with Issues
Draft last updated 2017-07-26
Review completed: 2017-07-26

Review
review-ietf-i2rs-yang-l3-topology-10-yangdoctors-early-lhotka-2017-07-26

This YANG module and I-D belong to an extensible suite of data models
addressing multi-layered network topology. It is an interesting, even
if somewhat atypical, application of YANG.

The entire network topology suite seems well thought out, and it is
apparent that the document has already undergone several rounds of
discussions and iterations. It is also positive that the document
deals with possible overlaps with other YANG modules such as
ietf-interfaces or ietf-hardware. Of course, an ultimate acid test of
the network topology suite will come with applications to real-life
network.

My review below raises several issues that need to be addressed
(especially #1). Some of them are related to design decisions
described in draft-ietf-i2rs-yang-network-topo. Apart from that, I
believe the document is ready to be published.

*** Comments to draft-ietf-i2rs-yang-network-topo-14:

1. With YANG 1.1, the network type information looks like a perfect
   candidate for identities (with multiple inheritance). Instead, it
   is modelled with presence containers, which is cumbersome and
   redundant. I don't see any reasons for doing so, if there are any,
   then they should be explained in sec. 4.4.8.

2. The document defines "supporting network" and then says "A
   supporting network is in effect an underlay network." In the
   subsequent text "underlay network" is used almost exclusively. So
   perhaps the term "supporting network" should be dropped altogether?

3. The text should be better aligned with the terminology of
   draft-ietf-netmod-revised-datastores. In particular,
   "system-controlled" should be used instead of "server-provided".

4. Is it necessary to use URIs for identifying all objects in the
   data model. URIs are difficult to use, so applications are likely
   to introduce some abbreviations and keep an internal mapping, which
   could cause problems, e.g. when matching objects in notifications
   with a GUI representation. In my view, it should be sufficient to
   use URIs for network-id and plain strings for other IDs, because
   all other objects are encapsulated inside a network, so their name
   conflicts shouldn't matter.

*** Comments to draft-ietf-i2rs-yang-l3-topology-10

5. The type of "router-id" should be "yang:dotted-quad" and not
   "inet:ip-address" because the latter means both IPv4 and IPv6
   address, possibly also with a zone index.

6. Both prefix and link attributes include the "metric" parameter. It
   should be explained what they mean. In the context of ietf-routing
   the option of including "metric" as a general route parameter was
   discussed and finally rejected because different routing protocol use
   metrics with different semantics and properties. I wonder if it is
   the same case here. 

*** Formal issues and typos

7. Both documents should refer to draft-ietf-netmod-yang-tree-diagrams
   rather than describe the notation of tree diagrams in the text.

8. Sec. 7 in draft-ietf-i2rs-yang-l3-topology-10: s/moodel/model/