Dear all,

Now implemented in the yangcatalog.org, we have the tree-type metadata.
from https://github.com/xorrkaz/netmod-yang-catalog, to be published soon in https://datatracker.ietf.org/doc/draft-clacla-netmod-model-catalog/

     leaf tree-type {
         type enumeration {
           enum split {
             description
               "This module uses a split config/operational state layout.";
           }
           enum nmda-compatible {
             description
               "This module is compatible with the Network Management
   Datastores
                Architecture (NMDA) and combines config and operational
   state nodes.";
           }
           enum transitional-extra {
             description
               "This module is derived as a '-state' module to allow
   for transitioning
                to a full NMDA-compliant tree structure.";
           }
           enum openconfig {
             description
               "This module uses the Openconfig data element layout.";
           }
           enum unclassified {
            description
              "This module does not have a data element tree, or it
   does not belong to any category.";
           }
           enum not-applicable {
             description
               "This module is submodule.";
           }
         }
         description
           "The type of data element tree used by the module as it
   relates to the
            Network Management Datastores Architecture.";
         reference
           "draft-dsdt-nmda-guidelines Guidelines for YANG Module
   Authors (NMDA)


Happy to see that the two following I2RS modules are "nmda-compatible"
https://yangcatalog.org:8443/search/modules/ietf-network,2017-06-30,ietf
https://yangcatalog.org:8443/search/modules/ietf-network-topology,2017-06-30,ietf
The following two are: transactional-extra
https://yangcatalog.org:8443/search/modules/ietf-network-state,2017-06-30,ietf
https://yangcatalog.org:8443/search/modules/ietf-network-state,2017-06-30,ietf

We'll work on a nicer GUI later one. The key point for now: the tree-type metadata is populated.

Regards, Benoit
Reviewer: Kent Watsen
Review result: Almost Ready


YANG Doctor review of draft-ietf-i2rs-yang-network-topo-14 (by Kent Watsen)


4 modules are defined in the draft:
- ietf-netw...@2017-06-30.yang
- ietf-network-topol...@2017-06-30.yang
- ietf-network-st...@2017-06-30.yang
- ietf-network-topology-st...@2017-06-30.yang

No validation errors from either `pyang` or `yanglint`.

0 examples are defined in the draft.

The -state modules appear to have all the correct changes
from their base modules.

Regarding ietf-netw...@2017-06-30.yang:
- prefix “nd”, should be “nw”?  (in IANA Considerations also)
- “import ietf-inet-types” should have a ‘reference’ to RFC 6991
- remove “WG Chairs”, per the template in rfc6087bis-13 Appendix C.
- s/Editor/Author/? (this is a preference choice)
- defines ‘network-ref’ that is unused within module (it’s used by
   ietf-network-topology)
- leafref paths don’t include prefix (rfc6087bis S4.2)

Regarding ietf-network-topol...@2017-06-30.yang:
- prefix “lnk” should be “nt” or maybe “nwtp”? (in IANA Considerations also)
- “import ietf-inet-types” should have a ‘reference’ to RFC 6991
- “import ietf-network” should have a ‘reference’ to RFC XXXX
- remove “WG Chairs”, per the template in rfc6087bis-13 Appendix C.
- s/Editor/Author/? (this is a preference choice)
- defines grouping link-ref and tp-ref that are unused within module
- mandatory true for source/dest-node/tp?
- replace “tp” with “term-pt”?  (both in “-tp” and “tp-“ uses)

Regarding ietf-networ-sta...@2017-06-30.yang:
- similar comments to ietf-netw...@2017-06-30.yang:

Regarding ietf-network-topology-st...@2017-06-30.yang:
- similar comments to ietf-network-topol...@2017-06-30.yang.


Comments on draft:

1) The document still has references to “server-provided”.  Not the YANG
leaf of course, but in general text.  For instance “The model does allow
to layer a network that is configured on top of one that is server-provided.”
I think that the statement is more about values being in <operational>
than how it was learned.  All uses of “server-provided” should be examined
for correctness.

2) This sentence doesn’t make sense to me, how can a server-provided model
(you mean data?) access any information in conventional datastores?:
    “An implementation's security policy MAY further restrict what
    information the server-provided model is allowed to access in
    standard configuration data-stores,”
Either way, this text likely should be moved to the Security
Considerations section.

3) Should the last paragraph in Section 1 be removed now?  Is the module
expected to continue to update?

4) S4.1, P3: the text here says new data nodes are augmented in, but
the YANG module itself says that only presence containers are allowed.

5) are the mandatory statement values set appropriately everywhere?
(e.g., source-node?, source-tp?)

6) network-type is a presence container, not an identity? No where in
the draft is there an example showing it being used.

7) Section 4.4.8., why not use identities?

8) S4.4.9 is a duplicate of some text in S4.1

9) This statement is true, but I think misleading in context. “YANG
requires data nodes to be designated as either configuration or
operational data, but not both”. It seems that the solution depends
entirely on config true nodes, and NMDA to present the operational
value of those config true nodes.  Right?

10) When using <CODE BEGINS>, you should have a note to the RFC
Editor to change the date to the date of publication, both in the
filename as well as in the module’s ’revision’ statement.

11) IANA Considerations has comments “(RFC form)” - are these supposed
to be notes to RFC Editor to replace with final RFC designation?

12) Your Security Considerations section should follow the template here:
https://trac.ietf.org/trac/ops/wiki/yang-security-guidelines

13) create examples for the use-cases in Appendix A?  Note, every draft
should have examples of its YANG modules...


Nits:

- why reference RFC6991 in the Introduction?
- replace “allows to define” with “enables the definition of”
- in a few places, you refer to “network.yang”, but the
   file is called “ietf-network.yang”.
- in a few places, you refer to “network-topology.yang”,
   but the file is called “ietf-network-topology.yang”.
- replace “- X1 and X2 - mapping onto a single L3 network element”
   with “(X1 and X3) mapping onto a single L3 network element (Y2)”
- replace “data-store” with “datastore”
- replace “model” with “data model” where it’s not already.
- replace “the <intended> datastore” with just “<intended>”
- replace “the intended datastore” with just “<intended>”
- update Section 2 to also include a reference to RFC 8174 (see RFC 8174)
- pull the “datastore” term from the revised-datastores draft?
- NETCONF and YANG don’t need to be terms/defined, since references
   to their RFCs are provided when they’re used.
- s/the network.yang module/the “ietf-network” YANG module/
- your tree-diagram notation in S4.1 isn’t complete.  And you duplicate
   it in S4.2.  Create a top-level section called “tree diagram notation”
   that both sections reference?
- s/allows to represent/allows representation of/
- s/another container/a presence container called/
- s/allows to define more/allows definition of more/
- s/allows also to represent/allows representation of/
- s/configuration and intended datastores/conventional datastores/
- s/and show up only/and thus only appear/
- /ietf-network:networks/network/network-id - being the list’s key,
   I was expecting this to the first element defined.
- for the various leafrefs in both models, I see a lot of longish
   relative paths; suggest you change these to absolute paths if possible
- /nd:networks/nd:network:/link/link-id is the list key but not the
   first leaf listed
- incomplete sentence: “augmentations can in turn against augmenting
   modules”
- s/need to specified/need to be specified/
- s/if the link-to-links mapping known/if the link-to-links mapping
   are known/
- s/each link known/each link are known/
- s/the operational datastore/<operational>/
- s/into the data model without relying/into <operational> without relying/
- s/in the following two companion modules/the following two companion modules/
- s/that represent a state model/to represent the operational state/

Thanks,
Kent



_______________________________________________
yang-doctors mailing list
yang-doct...@ietf.org
https://www.ietf.org/mailman/listinfo/yang-doctors

_______________________________________________
i2rs mailing list
i2rs@ietf.org
https://www.ietf.org/mailman/listinfo/i2rs

Reply via email to