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" <lho...@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 >"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 > > _______________________________________________ OSPF mailing list OSPF@ietf.org https://www.ietf.org/mailman/listinfo/ospf
