Additional audience for this review, per S.M.

---------- Forwarded message ----------
From: Joshua Bell <josh@lindenlab.com>
Date: Mon, Feb 7, 2011 at 10:03 AM
Subject: apps-team review of draft-ietf-netconf-4741bis-07
To: apps-review@ietf.org, rpe@juniper.net, mbj@tail-f.com,
j.schoenwaelder@jacobs-university.de, andy.bierman@brocade.com
Cc: SM <sm+ietf@elandsys.com>


== Boilerplate ==

I have been selected as the Applications Area Review Team reviewer for this
draft (for background on apps-review, please see
http://www.apps.ietf.org/content/applications-area-review-team).

Please resolve these comments along with any other Last Call comments you
may receive. Please wait for direction from your document shepherd or AD
before posting a new version of the draft.

Document: draft-ietf-netconf-4741bis-07
Title: Network Configuration Protocol (NETCONF)
Reviewer: Joshua Bell
Review Date: 2011-02-04
IETF Last Call Date: 2011-02-07

== Summary ==

This draft is almost ready for publication but has a few issues that should
be fixed before publication. The issues I note are not significant
detractions from the overall high quality and readability of the I-D.

The I-D is an update to RFC 4741 and has therefore "stood the test of time".
I was not familiar with the previous RFC and thus reviewed the document as
"new to me", then went back and did a cursory review of my feedback to see
what applied to the previous document as well.

== Major Issues ==

none

== Minor Issues unique to the I-D ==

Section 3 XML Considerations includes:

   All NETCONF messages MUST be well-formed XML, encoded in UTF-8.  If a
   peer receives an 'rpc' message that is not well-formed XML, it SHOULD
   reply with an 'operation-failed' error.  If a reply cannot be sent
   for any reason, the server MUST close the session.


The behavior when an 'rpc' message is received that is well-formed XML but
specifies an encoding other than UTF-8 is not strictly defined. This could
occur where the encoding is defined via an XML declaration, such as <?xml
version="1.0" encoding="EUC-JP" ?> (XML declarations are explicitly
permitted a few lines down in the I-D). I suggest that the I-D specify that
if a non-UTF-8 encoding is specified the peer SHOULD reply with an
'operation-failed' error, if this is the expected behavior.

Section 8.9.1 XPath Capability Description includes:

   The data model used in the XPath expression is the same as that used
   in XPath 1.0 [2], with the same extension for root node children as
   used by XSLT 1.0 [11] (section 3.1).  Specifically, it means that the
   root node may have any number of element nodes as its children.


It is unclear how this applies. My understanding is that in XSLT this
wording refers to the results tree of a transform, and allows for a
transform which e.g. outputs a non-well-formed XML document with multiple
top-level elements (e.g. "<elem/><elem/><elem/>") rather than a well-formed
document with a single root element. However, this section of the I-D is
describing the input data model to the XPath filter expression, and which
explicitly yields a node-set as a result rather than a result tree. If this
text is intended to describe the XML returned by the <get/> or <get-config/>
operation, it should be stated separately from the description of the XPath
data model.

On a possibly related note, the next bit which describes how the node-set
selected by the XPath expression yields the output tree seems
underspecified:

   The response message contains the subtrees selected by the filter
   expression.  For each such subtree, the path from the data model root
   node down to the subtree, including any elements or attributes
   necessary to uniquely identify the subtree, are included in the
   response message.


It is unclear (to me) exactly what the output should be when the node-set
results of the XPath expression contains multiple nodes. For example
(simplified
relative to the examples in the I-D). given the filter:

   select="/users/user[name='fred' or name='barney']

Would this yield:

           <users>

             <user>
               <name>fred</name>
             </user>

             <user>
               <name>barney</name>
             </user>

</users>


or:


           <users>
             <user>
               <name>fred</name>
             </user>
           </users>


           <users>
             <user>
               <name>barney</name>
             </user>
           </users>


The former is strongly implied by the non-XPath-based filter logic defined
in section 6 (i.e. the text "Specific data instances are not duplicated in
the response in the event that the request contains multiple filter subtree
expressions that select the same data."), but the latter is hinted at by the
note about the "XSLT extension for root node children" called out above. In
either case, more clarity in the normative text would be worthwhile, as well
as an example.


== Minor Issues in common with RFC 4741 ==

Section 4.1 and 4.2 define the usage of the "message-id" attribute. Since
the value is an arbitrary string selected by the sender, it is possible that
XML processing would modify this value as part of attribute-value
normalization: http://www.w3.org/TR/2008/REC-xml-20081126/#AVNormalize - I
suggest adding the requirement that senders ensure that message-ids are
already normalized per these rules so that they round-trip cleanly.

Section 4.2 is where the <data> element first appears but only in examples;
unlike 4.3 which explicitly calls out <rpc-error> and 4.4 which explicitly
calls out <ok>; <data> is also not identified in the XML Schema in Appendix
B. This leads to the impression that <data> is just an example element, but
it is defined normatively as part of the positive response in sections 7.1
and 7.7


== Nits in common with RFC 4741 ==

Section 5.1 has odd bulleting around the paragraph for "Running" (a single
bullet point). (This looks like residue from an earlier revision where
multiple datastores were defined but then made optional via capabilities.)

Attribute names are called out inconsistently within the document. E.g. "...
the select attribute...", "containing a 'type' attribute...", "... a
"message-id" attribute...". For improved readability this should be
standardized.

Finally, I did not manually verify all of the filter examples in Section 6.
>From a cursory inspection, the introduction of the namespace wildcarding
(Section 6.2.1) should not change the validity of the samples from RFC 4741.
If there is a sample dataset and utility that could be used to quickly
validate the filters in all cases it would be a good idea.

-- Josh