Martin Bjorklund <m...@tail-f.com> writes: > 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.
IMO, "published" should be removed unless its meaning is explained. Lada > > 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 -- Ladislav Lhotka, CZ.NIC Labs PGP Key ID: E74E8C0C _______________________________________________ netmod mailing list netmod@ietf.org https://www.ietf.org/mailman/listinfo/netmod
