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

["Acee Lindem (acee)" <[e-mail]>]

Hi Lada,

Apologies for the delay. We somewhat got hung up on 4 and 6. See inline.

On 12/6/17, 6:26 AM, "Ladislav Lhotka" <[e-mail]> 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
>"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.

We will add this. 


>
>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.

We will add comments to the description.
>
>3. Maybe the draft could mention that implementations should supply a
>   default routing domain as a system-controlled resource.

Isn’t this more of an RFC8022BIS statement? I guess we could state this as
an assumption. 
>
>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.

Why is this problematic? Is it because the types can be extended?

>
>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".

I agree - we will follow the RFC 6087BIS guidelines.


>
>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";

I guess I don’t see the identities as always being better. We will
consider this one. 


>
>7. The title of sec. 2.9 should be "OSPF Notifications" rather than
>   "OSPF notification".

Sure. 

Thanks,
Acee 
>
>