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

["Susan Hares" <[e-mail]>]

Carl: 

Thank you for the yang doctor's review of draft-ietf-i2rs-yang-l3-topology-02.txt. 

Sue 

-----Original Message-----
From: Carl Moberg (camoberg) [mailto:[e-mail]] 
Sent: Monday, July 4, 2016 7:55 AM
To: [e-mail]; YANG Doctors
Subject: YANG doctor review of draft-ietf-i2rs-yang-l3-topology-02

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
[e-mail]