Juergen Schoenwaelder <[email protected]> writes:

> Hi,
>
> I have read through draft-ietf-netmod-rfc6020bis-07 (except section 9.
> Built-In Types). Overall, the document is in a good shape. I spotted a
> number of editorial issues or a number of issues where we could try to
> seek a clearer separation of NETCONF specifics. (I plan to read
> section 9 as well but I assume this is less important since there
> likely are not many changes from RFC 6020 in this part.)
>
> I am indicating the paper number below for each comment/suggestion.
>
> * p8:
>
>   s/XML has been/XML have been/

And right after that: s/propsed/proposed/

Lada

>
>   s/how the data model/how a data model/
>
>   s/represented in/encoded in/
>
> * p9:
>
>   s/names means/names mean/
>
> * p11:
>
>   s/the module faithfully/a module faithfully/
>
>   I am also wondering why we use device and server. It seems we use
>   these terms interchangeably. If so, for clarity, I would suggest to
>   use a single term, that is s/device/server/ and perhaps explicitly
>   state that unless stated otherwise server means a server providing
>   access to a YANG defined data tree.
>
> * p12/p13
>
>   We import 7 terms from RFC 6241. Would it make sense to copy the
>   necessary text in order to avoid a too strict binding to RFC 6241?
>   In particular, 'client' and 'server' means NETCONF client and server
>   if we import from RFC 6241 but this may be a bit narrow given that
>   we have RESTCONF as well. In an ideal world, we would factor out
>   core architectural concepts but the best we can do is likely to
>   define core concepts inline, pointing out where they are the same.
>   The idea is to loosen the coupling to RFC 6241. Example:
>
>   OLD
>
>    o  datastore: an instantiated data tree
>
>   NEW
>
>    o  datastore: A conceptual place to store and access information.
>       A datastore might be implemented, for example, using files, a
>       database, flash memory locations, or combinations thereof.
>       [Matches the definition in RFC 6241.]
>
> * p13
>
>   Does the term 'mandatory node' really deserve its own subsection?  I
>   understand that there are references to section 3.1 but would
>   reference to section 3 not work as well?
>
> * p13
>
>   s/used for other/used with other/
>
> * p14
>
>   s/encoded in NETCONF operations/encoded on-the-wire/
>
> * p14
>
>   s/of NETCONF data models/of data models for network management
>   protocols such as NETCONF,/
>
>   And I suggest to remove the following sentence since it is not
>   really needed.
>
>       The data models described by YANG are designed to be easily
>       operated upon by NETCONF operations.
>
> * p15
>
>   s/read-only access/read-only access [RFC6643]/
>
> * p15
>
>   I am not sure why the last paragraph in section 4.1 is needed. In
>   the first setence, I would remove 'Like NETCONF' and in the second
>   sentence I am not sure what we are saying given that there is NACM.
>   I would definitely remove the second sentence and if the first
>   should be kept, I would likely move it up since it is a fairly
>   general statement. But I think removing the whole paragraph is the
>   simplest solution since it is not essential.
>
> * p13
>
>   Would it make sense to add a sentence right at the beginning of
>   section 4 stating that section 4 is intended to provide orientation
>   for the first time reader but that section 4 is not normative?
>
> * p15
>
>   With the previous in mind, I suggest to remove "This progressive
>   approach handles the inter-related nature of YANG concepts and
>   statements.  A detailed description of YANG statements and syntax
>   begins in Section 7." from the text right below 4.2. Note that
>   Section 7 is actually Section 6 (but Section 5 has also important
>   normative content).
>
> * p16
>
>   s/four types of nodes/four types of data nodes/
>
> * p16
>
>   Perhaps simplify:
>
>   OLD
>
>     A leaf instance contains simple data like an integer or a string.  It
>     has exactly one value of a particular type and no child nodes.
>
>   NEW
>
>     A leaf data node has exactly one value of a particular type and no
>     child nodes.
>
> * p16-p25
>
>   Should this be 'XML Encoding Example' instead "NETCONF XML Example'?
>   This does not seem to be NETCONF specific.
>
> * p25
>
>   s/XML representation/XML encoding/
>
> * p25
>
>   s/operations' names/operations' name/
>
> * p25
>
>   s/node in the data hierarchy/data node/
>
> * p28
>
>   Start section 5 by saying that this section defined language
>   concepts and that it is normative, e.g.
>
>     This normative section defines core language concepts.
>
> * p29
>
>   s/its contents are unchanged/its content is unchanged/
>
> * p37
>
>   s/following shows valid data/following XML encoding shows valid data/
>
> * p35
>
>   I think there should be a citation of I-D.ietf-netconf-yang-library
>   where this module first appears.
>
> * p46
>
>   The text concerning module names is a repetition from section 5.1.
>   To avoid introducing inconsistencies, perhaps replace the repeated
>   text with a reference to section 5.1?
>
> * p49
>
>   Is the text in section 7.1.3 correct when it says all identifiers
>   defined by the module? I mean, schema node identifiers are
>   namespaced by module names and their prefixes. And data node
>   identifiers are using the namespace in the XML encoding. Here is
>   an attempt of a rewrite:
>
>      The "namespace" statement defines the XML namespace that all data
>      node identifiers defined by the module are qualified by in the
>      XML encoding. Data node identifiers defined inside a grouping are
>      an exception q(see Section 7.13 for details).  The argument to
>      the "namespace" statement is the URI of the namespace.
>
> * p53
>
>   Another repetition of the module/submodule name requirements, does
>   it make sense to avoid repetition?
>
> * p61
>
>   Should the text in 7.5.4.1 and 7.5.4.2 say explicitly that we talk
>   about NETCONF when we refer to <rpc-error>? Or should the text try
>   to be more generic, saying that the respective fields will be
>   carried in error message in protocols that use YANG? It is actually
>   somewhat opaque what an error-app-tag is or how it should be used.
>
> * p63
>
>   Should the last paragraph in 7.5.7 be factored out into its own
>   subsection titled "NETCONF <get> and <get-config> Operations"?  The
>   text is not really anymore about XML encoding (which may be used
>   with RESTCONF). Or the text is actually about the encoding but
>   should be written to simply state that non-presence containers can
>   be pruned in encodings (without restricting this to NETCONF).
>
> * p67
>
>   Similar to the comment for p63. Perhaps the right way is to phrase
>   this in such a way that is simply says leaf nodes containing a
>   default value may not be present in the XML encoding. Simply remove
>   NETCONF <get> or <get-config> specifics.
>
> * p72
>
>   s/an unspecified order/an order determined by the system/
>
>   Note that a description clause may actually define how the system
>   has to order elements and hence 'unspecified order' seems wrong.
>
> * p97
>
>   The text in 7.14 talks about NETCONF specifics, namely the <rpc>
>   element and the "rpcOperation" substitution group. Perhaps move this
>   down into a specific section defining the mapping of YANG RPCs to
>   NETCONF <rpc>s.
>
> * p100
>
>   The XML encoding text starts with detailing NETCONF specifics.
>   Would it make sense to separate the XML encoding of the rpc and its
>   input/output from the specifics how the RPCs fit into NETCONF's
>   <rpc> system?
>
> * p102
>
>   The XML encoding section again mixes XML encoding of the node
>   hierarchy with NETCONF specifics how an action is invoked using
>   NETCONF's <rpc> system. I suggest to factor this into two different
>   sections.
>
> * p105
>
>   Again, the proposal is to separate the XML encoding of notifications
>   from the details how notifications work with RFC 5277.
>
> * p120
>
>   The text in section 7.21.1 talks about NETCONF specific operations.
>   Is this actually necessary? Can the purpose of the config statement
>   not be described by referring to general concepts such as
>   configuration datastores instead of using protocol specific
>   operations?
>
> * p123
>
>   Is there a special reason why the text uses 'notification instance
>   data tree' instead of just 'notification data tree'?
>
> * p123
>
>   "All leaf data values MUST match the type constraints..." may be
>   read as 'all data nodes that contain values' or 'all instantiations
>   of nodes defined by the leaf statement'.
>
> * p192
>
>   The Acknowledgements section may need some updates.
>
> /js
>
> -- 
> Juergen Schoenwaelder           Jacobs University Bremen gGmbH
> Phone: +49 421 200 3587         Campus Ring 1 | 28759 Bremen | Germany
> Fax:   +49 421 200 3103         <http://www.jacobs-university.de/>
>
> _______________________________________________
> netmod mailing list
> [email protected]
> https://www.ietf.org/mailman/listinfo/netmod

-- 
Ladislav Lhotka, CZ.NIC Labs
PGP Key ID: E74E8C0C

_______________________________________________
netmod mailing list
[email protected]
https://www.ietf.org/mailman/listinfo/netmod

Reply via email to