Subject: [netmod] js review of draft-ietf-netmod-arch-05.txt

From: Juergen Schoenwaelder <[email-masked]>
Date: Tue, 18 May 2010 09:39:50 +0200

Hi,

this is my WG last call review of <draft-ietf-netmod-arch-05.txt>. The
document has made very good progress since I reviewed it the last time
but I think there is still some minor work left to be done before it
is ready to go to the IESG. My comments are in document order, not
ordered by significance.

a) p1: s/operational data/state data/

   This aligns with RFC 4741 terminology.

b) p1: The abstract should IMHO also mention notifications, they are
       mentioned later on anyway.

c) p4: s/Origins of YANG/Origins of NETCONF and YANG/

d) p4: Did operators really ask for "low implementation costs"? I
       would understand if they ask for "low deployment costs" - low
       implementation costs is something vendors usually worry about.

e) p4: s/Existing data modeling/Existing general purpose data modeling/

f) p4: s/70 and 71/70/

   If you leave IETF 71 there, then the chronologic sequence breaks
   apart since the design team formed after IETF 70 produced results
   presented at IETF 71 as far as I recall.

g) p5: s/that easily/that is easily/

h) p5: s/brings NETCONF/brings NETCONF and YANG/

i) p6: s/Elements of YANG/Elements of the Architecture/

j) p6: Remove the following one sentence paragraph:

      YANG is focused on creating data models for NETCONF, and any
      understanding of the former must begin with the latter.

   Since the scope of the architecture document was broadened, this
   sentence is pretty much useless.

k) p7: I find section 2.1 a bit terse compared to section 2.2 I think
       it would not hurt to explain the different datastores NETCONF
       provides in one paragraph. It might also be helpful to explain
       the notion of capabilities in one paragraph. Furthermore, it
       might be useful to summarize the operations NETCONF provides,
       e.g. by creating a table with columns Operation Description,
       much like we do for YANG statements in section 2.2.

l) p8: I suggest to add a new section 2.2 "NETCONF Transport Mappings"
       that briefly reviews the transport mappings available. I think
       it is useful if this document provides a complete overview and
       the relevant pointers to related documents (not just RFC 4741).

m) p8: Similarily, I suggest to add a new 2.3 talking about NETCONF
       notifications. Perhaps we should also mention the NETCONF
       monitoring data model somewhere.

n) p8: s/data model nodes/data nodes/

       To be consistent with the YANG terminology (where the term
       "data node" is defined).

o) p8: s/data instances/data nodes/

p) p8: s/leaf data values/data nodes forming leafs of the data tree/

q) p9: I suggest to put the statements into canonical order (see
       section 12 of the YANG specification).

r) p9: I would replace the import with two imports since that is a
       more likely organization

        import ospf-types      { prefix ospf-types; }
        import interface-types { prefix if-types; }

       and then use ospf-types:area-id and if-types:interface-name.

s) p10: s/keys leafs/key leafs/

t) p10: s/as multiple/multiple/

u) p10: s/layer of hierarchy/hierarchy/

v) p10: s/rpc/input and output parameters of RPC operations/

w) p11: If you put YANG statements in quotes, be consistent:

        s/choice gives/"choice" statement defines/
	s/The feature/The "feature" statement/
	s/The "deviation" give/The "deviation" statement gives/

x) p11: s/this problem, rather than ignoring it/this limitation, rather
          than leaving it to be discovered via runtime errors/

y) p13: s/YANG Technology/YANG Translations/

z) p14: Add a new section "2.X Usage Guidelines" that briefly points
        to the usage guidelines document and explains that these
        guidelines must be followed by (IETF) standard data models.

A) p15: s/Building YANG-based Solutions/Building NETCONF- and
          YANG-based Solutions/

B) p16: s/against the data defined/against the data model defined/

C) p16: s/XML payload/XML payload template/

D) p16: s/Addressing Operation Problems/Addressing Operator Requirements/

E) p17: s/a the/the/

F) p17: This text does not make much sense to me:

   o  Network-wide configuration: A standard YANG module can be
      implemented in all devices, giving a common view of configuration
      data.

	I think operators associate with the term "network-wide
	configuration" the support to run network-wide configuration
	changes; instead of configuring individual boxes you
	concentrate on configuring the network. NETCONF kind of
	supports this kind of network wide configuration changes via
	the confirmed-commit capability. So perhaps the text should
	be:

   o  Network-wide configuration: NETCONF supports robust network-wide
      configuration transactions via the confirmed-commit capability.

G) p17: s/as rpc methods/as RPC operations/

H) p17: s/the rpc or/the RPC operation or/

I) p17: s|CLI/Expect|command line interface/

J) p17: s/CLI operations/command line interface/

K) p17: s/should allow simpler modules/enables modules/

L) p17: s/DDL/data definition language/

M) p17: s/UTF-8 data/UTF-8 encoded unicode characters/

N) p17: s/YANG integrated rpc, notification, operational and
        configuration data to make internal references./YANG integrates
        RPC operations, notifications, configuration and state data,
        enabling internal references./

O) p18: "NETCONF runs as an SSH service" is not quite correct since
        SSH is just one of several possible transports. This might
	need some rephrasing, e.g.

	NETCONF runs over transport protocols secured by SSH or TLS,
    	allowing secure communications and authentication using
    	well-trusted technology.  The secure transports use the
    	existing key and credential management infrastructure,
    	reducing deployment costs.

P) p18: s/new NETCONF RPCs/new RPC operations/

Q) p18: s/RPC's input and output data/input and output parameters of
        RPC operations/

R) p18: s/Building YANG-based Solutions/Building NETCONF- and
        YANG-based Solutions/

S) p19: I suggest to restructure section headers as follows (since the
        text is organized according to the various roles involved in
	building solutions:

        3.3 Building NETCONF- and YANG-based Solutions

	3.3.1 Modeler
	3.3.2 Reviewer
	3.3.3 Device Developer
	3.3.4 Application Developer
	3.3.4.1 Hard Coded
	3.3.4.2 Bottom Up
	3.3.4.3 Top Down

	To keep things simple, I suggest to remove the current section
	titles 3.6.1 and 3.6.2 - they do not seem to be necessary.

T) p19: s/modeler's role constructing a model/modeler defines a data model/

U) p19: I suggest to remove this example:

   In addition, reviewers can encode review policies in scripts, such as
   XSLT.  A policy that leaf names can't have underscores can be coded
   as:

        <xsl:template match="leaf[contains(@name, '_')]">
            Error: leaf name contains underscore
        </xsl:template>

	The reason is that I do not understand how this example works
	(is this XSLT snippet applied to YIN?) nor is this the only
	way of doing things nor is this in particular NETCONF or YANG
	specific (you can always automate some review policies).

V) p19: s/, absorbs the zen of the model,//

W) p19: Should the section describing the device modeler not mention
        that the developer should write deviation statements in cases
        where his implementation puts restrictions on the data model?

X) p21: s/modules allows/modules allow/

Y) p22: s/ietf-bgpvpn/bgpvpn/

        I do not think we want to encourage people to put ietf-
        prefixes on data nodes.

Z) p22: s/uses either/using either/

0) p23: s/if it not/if it is not/

1) p24: I found section 4.2 confusing, mostly because the term
        deviation seems to be used with a meaning different from
        YANG's notion of a deviation. I would prefer if this text
        would talk about features and deviations in the YANG sense. At
        first reading, I thought "modeled deviations" refers to YANG
        features but then that later turned out to be not true. It is
        also not clear what makes an acceptable deviation, an
        unacceptable deviation, an allowed deviation etc. Are the
        words allowed and acceptable synonymous? I believe the
        simplest fix is to remove the bulleted list from section 4.2.

2) p24: s/it a state/it as state/

3) p26: s/interfaces usually comes/interfaces usually come/

4) p27: s/provide explicitly/explicitly/

5) p27: Add the following section:

   4.3.3.4 Introduction of Meta-Data Tags

      The NETCONF protocol might be extended to carry meta-data tags
      (for example encoded in XML attributes) that identify whether an
      XML element carries configuration data, operational state data,
      or statistical data. This approach allows responses to <get>
      operations to contain mixed data, making it easy for clients to
      separate configuration data from operational state data. Servers
      may be allowed to skip over any non-config data supplied by
      clients in <edit-config> or <copy-config> operations.

6) p27: Does section 4.3 belong into this document? Unfortunately, as
        long as the WGs have not converged to a solution, I believe it
        is necessary to keep this text since we right now leave it to
        the data modelers to figure out what to do. I believe this is
        actually a bad idea but lets be honest about it.

7) p28: Here is a rewrite of the security considerations:

   This document discusses an architecture for network management
   using NETCONF and YANG. It has no security impact on the Internet.

/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/>