Hi Benoit, Benoit Claise <bcla...@cisco.com> wrote: > Dear all, > > Here is part 1 of my AD review. > > I found this useful: > http://tools.ietf.org/rfcdiff/rfcdiff.pyht?url1=http://www.ietf.org/rfc/rfc6020.txt&url2=http://www.ietf.org/id/draft-ietf-netmod-rfc6020bis-11.txt > > - Do we want to mention RESTCONF in the abstract? From the new charter: > > The NETMOD working group has defined the data modeling language > YANG, which can be used to specify network management data models > that are transported over such protocols as NETCONF and RESTCONF. > > OLD: > > YANG is a data modeling language used to model configuration data, > state data, remote procedure calls, and notifications for network > management protocols like the Network Configuration Protocol > (NETCONF). > > NEW: > > YANG is a data modeling language used to model configuration data, > state data, remote procedure calls, and notifications for network > management protocols transported over such protocols as Network > Configuration Protocol (NETCONF) and RESTCONF. This document specifies > the YANG mappings to NETCONF.
The first paragraph in the introduction mentions other protocols; RESCTONF and CoMI. My personal opinion is that this is sufficient, but I'd like to hear what others think. > - Section 1.1 > Can we want to stress the backwards incompatible changes from YANG > version 1 in a specific paragraph? Ok, this is a good idea. I'll move them to a separate list. > I see > o Changed the rules for the interpretation of escaped characters in > double quoted strings. This is an backwards incompatible change > from YANG version 1. A module that uses a character sequence that > is now illegal must change the string to match the new rules. See > Section 6.1.3 > > <https://tools.ietf.org/html/draft-ietf-netmod-rfc6020bis-11#section-6.1.3> > for details. > > o An unquoted string cannot contain any single or double quote > characters. This is an backwards incompatible change from YANG > version 1. > > > There is section 12, but this is not exactly that. > > Have we made an analysis of the 38 RFC-produced YANG modules? Any > facing issues with 1.1 compilation? I have tested to put "yang-version 1.1;" into all RFC-published modules, and then ran pyang on them w/o any issues. > - Section 1.1 > Since this document introduces the NETCONF mapping, the protocol > change must be included in section 1.1 > Example: no NETCONF capability exchange in YANG 1.1, we use > exclusively the YANG library > Any other ones? > - Section 3 > o anydata: A data node that can contain an unknown set of nodes that > can be modelled by YANG. > > NEW > > o anydata: A data node that can contain an unknown set of nodes that > can be modelled by YANG, except anyxml, but for which the data > model is not known at module design time. Ok, but I'd say: NEW: o anydata: A data node that can contain an unknown set of nodes that can be modelled by YANG, except anyxml. This is just a summary; the details are to be found in later sections. > - Terminology: > The following terms are defined in [RFC6241 > <https://tools.ietf.org/html/rfc6241>]: > > ... > > o configuration datastore: a configuration datastore is an > instantiated data tree with configuration data > > o datastore: an instantiated data tree > > RFC6241 has different definition for "configuration datastore" and > "datastore". > I would just provide the pointer to the RFC 6241 definitions. > If you intend to provide an adapted definition for the YANG mappings, > then you should say so. How about: OLD: o configuration datastore: a configuration datastore is an instantiated data tree with configuration data o datastore: an instantiated data tree NEW: o configuration datastore: When modelled with YANG, a configuration datastore is an instantiated data tree with configuration data o datastore: When modelled with YANG, an instantiated data tree > - Section 4.1 > > YANG models can describe constraints to be enforced on the data, > restricting the appearance or value of nodes based on the presence or > value of other nodes in the hierarchy. > > I was looking for an example of appearance. > NEW? > YANG models can describe constraints to be enforced on the data, > restricting the appearance (for example, with the "when" statement) > or value of nodes based on the presence or value of other nodes in > the hierarchy. This is very early in the document, and the text tries to give a very high level function overview. I am not sure that mentioning "when" at this time actually helps a first time reader. I would prefer to leave it as it is. > - section 4.2.2.3, Container Nodes > > A container is used to group related nodes in a subtree. A container > has only child nodes and no value. A container may contain any > number of child nodes of any type (leafs, lists, containers, leaf- > lists, and actions). > And notification, right? This should be added Ok. > container-stmt = container-keyword sep identifier-arg-str optsep > (";" / > "{" stmtsep > ;; these stmts can appear in any order > [when-stmt] > *if-feature-stmt > *must-stmt > [presence-stmt > > <https://tools.ietf.org/html/draft-ietf-netmod-rfc6020bis-11#ref-presence-stmt>] > [config-stmt] > [status-stmt] > [description-stmt] > [reference-stmt] > *(typedef-stmt / grouping-stmt) > *data-def-stmt > *action-stmt > *notification-stmt > "}") stmtsep > > - Examples > I guess that no examples are supposed to compile, right? > Please add a sentence saying so. I honestly does not think that this is an issue, but how about this: NEW: 3.1. A Note on Examples Throughout this document there are many examples of YANG statements. These examples are supposed to illustrate certain features, and are not supposed to be complete, valid YANG modules. > When Andy's proposal will be ready (TAG: EXAMPLE-BEGINS => the YANG > example compiles, NO TAG: => no supposed to compile), this document > will already be compliant. > > - Section 4.2.4 > > +---------------------+-------------------------------------+ > | Name | Description | > +---------------------+-------------------------------------+ > | binary | Any binary data | > | bits | A set of bits or flags | > | boolean | "true" or "false" | > | decimal64 | 64-bit signed decimal number | > | empty | A leaf that does not have any value | > | enumeration | Enumerated strings | > | identityref | A reference to an abstract identity | > | instance-identifier | References a data tree node | > > Editorial: What the difference between: A reference or references? Be > consistent No difference. I'll change to A reference. > - Section4.2.7 > - <https://tools.ietf.org/html/draft-ietf-netmod-rfc6020bis-11#section-4.2.7>. > - Choices > > To be consistent with > > When a node from one case is created in the data tree, all nodes from > all other cases are implicitly deleted. > > > This text in section 7.9 should be updated. > OLD: > Since only one of the choice's cases can be valid at any time, the > creation of a node from one case implicitly deletes all nodes from > all other cases. If a request creates a node from a case, the server > will delete any existing nodes that are defined in other cases inside > the choice. > > NEW: > Since only one of the choice's cases can be valid at any time_in the > data tree_, the > creation of a node from one case implicitly deletes all nodes from > all other cases. If a request creates a node from a case, the server > will delete any existing nodes that are defined in other cases inside > the choice. Ok. > - Section 4.2.9 > <rpc message-id="101" > xmlns="urn:ietf:params:xml:ns:netconf:base:1.0"> > <activate-software-image xmlns="http://example.com/system";> > <image-name>example-fw-2.3</image-name> > </activate-software-image> > </rpc> > > Editorial: Alignment Fixed. > - Editorial: is there a logic in the numbering? > <rpc message-id="101" > <rpc message-id="102" When there are two rpcs in the same example, I use 101, 102. I double checked, and fixed two occurences where 101 was used instead of 102. > - section 4.2.9 > > Operations can also be tied to a > data node. Such operations are defined with the "action" statement. > > From the definition: > o data node: A node in the schema tree that can be instantiated in a > data tree. One of container, leaf, leaf-list, list, anydata, and > anyxml. > > And I see in section 7.15: > The "action" statement is used to define an operation connected to a > specific container or list data node. > > > I believe it should be clarified in 4.2.9 > NEW: > Operations can also be tied to a > container or list data node. Such operations are defined with the > "action" statement. Ok. > - Section 5.1 > Editorial > OLD: > o A module or submodule belonging to that module can reference > definitions in the module and all submodules included by the > module. > > NEW? > o A module, or submodule belonging to that module, can reference > definitions in the module and all submodules included by that > module. Yes, fixed. > - The following examples (as far as my quick check goes) are the only > - one that misses "yang-version 1.1" > > module example-augment { > namespace "urn:example:augment"; > prefix mymod; > import ietf-interfaces { > prefix if; > } > > module example-a { > namespace urn:example:a; > prefix a; Fixed. > - Section 5.6.4 > If a server implements a module A that imports a module B, and A uses > any node from B in an "augment" or "path" statement that the server > supports, then the server MUST implement a revision of module B that > has these nodes defined. This is regardless of if module B is > imported by revision or not. > > "imported by revision or not" I'm, confused because I read the > document in sequence. > From 5.1 and 5.1.1, it doesn't look like we have two options (import > by revision or not) > And the example shows the two possibilities: > import b { > revision-date 2015-01-01; > } > import c; > > I found my answer in section 7.1.5: > When the optional "revision-date" substatement is present, any > typedef, grouping, extension, feature, and identity referenced by > definitions in the local module are taken from the specified revision > of the imported module. It is an error if the specified revision of > the imported module does not exist. If no "revision-date" > substatement is present, it is undefined from which revision of the > module they are taken. > > You should write a sentence or two (imported by revision or not) about > in section 5.1 or 5.1.1 OLD: Published modules evolve independently over time. In order to allow for this evolution, modules need to be imported using specific revisions. When a module is written, it uses the current revisions of other modules, based on what is available at the time. NEW: Published modules evolve independently over time. In order to allow for this evolution, modules can be imported using specific revisions. When a module is written, it can import the current revisions of other modules, based on what is available at the time. And then a new paragraph last in the same section (5.1.1): NEW: If a module is not imported with a specific revision, it is undefined which excat revision is used. > - section 5.6.4 > A server MUST NOT implement more than one revision of a module. > > You should add that the server may contain more than one version for > import reasons. > This would be in line with > https://tools.ietf.org/html/draft-ietf-netconf-yang-library-05 > > This mandatory list contains one entry for each YANG data model > module supported by the server. There MUST be an entry in this list > for each revision of each YANG module that is used by the server. It > is possible for multiple revisions of the same module to be imported, > in addition to an entry for the revision that is implemented by the > server. I started to write some text to address this issue, but it felt out of place. This section is about modules that are implemented. Other modules can be listed according to yang-library; but that is a yang-library issue, not YANG 1.1. So I prefer to leave this section as it is. > - section 5.6.4 > ietf-yang-library comes out of the blue in section 5.6.4 > > If a server implements a module A that imports a module C without > specifying the revision date of module C, and the server does not > implement C (e.g., if C only defines some typedefs), the server MUST > list module C in the "/modules-state/module" list from > "ietf-yang-library" [I-D.ietf-netconf-yang-library > > <https://tools.ietf.org/html/draft-ietf-netmod-rfc6020bis-11#ref-I-D.ietf-netconf-yang-library>], > and it MUST set > the leaf "conformance-type" to "import" for this module. > > You should say a few words about it in section 4. > Alternatively, the content in 5.6.5 should come before 5.6.4 Agreed. I switched the order of 5.6.4 and 5.6.5, and added a forward reference: A NETCONF server announces the modules it implements (Section 5.6.5) > - Some statements in the ABNF section contains links. Intended? Talking > - with Martin, he submitted only the .txt version. > So it seems that the issues resides in the ietf submission tool > chain. To be solved during AUTH48, I guess. > > Example: > > container-stmt = container-keyword sep identifier-arg-str optsep > (";" / > "{" stmtsep > ;; these stmts can appear in any order > [when-stmt] > *if-feature-stmt > *must-stmt > [presence-stmt > > <https://tools.ietf.org/html/draft-ietf-netmod-rfc6020bis-11#ref-presence-stmt>] > ... > > > refine-stmt = refine-keyword sep refine-arg-str optsep > "{" stmtsep > ;; these stmts can appear in any order > *if-feature-stmt > *must-stmt > [presence-stmt] > [default-stmt] > [config-stmt] > [mandatory-stmt] > [min-elements-stmt] > [max-elements-stmt > > <https://tools.ietf.org/html/draft-ietf-netmod-rfc6020bis-11#ref-max-elements-stmt>] > [description-stmt] > [reference-stmt] > "}" stmtsep Ok. /martin _______________________________________________ netmod mailing list netmod@ietf.org https://www.ietf.org/mailman/listinfo/netmod
