prot-04 nits

[Wes Hardaker <wjhns1@hardakers.net>]

[This is 1 out of 2 messages about the recent -04 document.  I tried
to separate my comments into concerns and nits, though a few of the
items due lie in gray areas.  Hopefully the split should still be
useful though.  I haven't read the appendixes nor the transport
documents recently.  Hopefully I can get through those by the
deadline, but the WG last call across both the IETF, DSOM and
Thanksgiving has made it a bit tough to read everything thoroughly]

1.1:  "during any session" -> "during any authorized session"

1.1: "Session specific attributes affect only the session in which they
     are changed."

     It'll likely be possible to change attributes of a "target"
     session?  (kill-session is almost changing attributes, but I could
     see other more true changes being allowed in the future).

1.1: for the application stack diagram, it'd be helpful if you added
     the stack numbers in the left hand box to match the numbers
     below.  The first time I looked at it, it took a sec to realize
     the numbered list below was counting bottom up.

1.2: "These capabilities must" -> MUST?  (actually this sentence is
somewhat redundant with the first, but I'm not complaining)

1.2: "The capability URI" -> "A capability URI"

1.3: The document in general discusses configuration and state.
Discussions within the IETF in the last number of years has included
"control" objects as well.  I'd at least mention them, even if you are
either excluding them or lumping them in to the other category.

1.3: "file would be too large"  -> remove "too".  subjective.  Maybe
"unnecessarily" would be a better choice.

1.3: "Note that the NETCONF protocol is concerned only with information
required to get the system software into its desired running state."
This is not true.  state information retrieval has already been
defined (and even contracts earlier sentences about "state").

1.3: "If a local database of user authentication..."  I'd say this
sentence is beyond scope of the document.  You're talking about the
data model here.

2.1: last p: reference the future "lock" section near the lock word.

2.2: Security and Privacy is an indeterminate title.  How about
"Security, Authentication and Integrity" instead?

2.3: Authentication: "...entity can be trusted." -> "identity has been
sufficiently proven".  Trust is not implied at all by merely
authentication.

2.3: "should use RADIUS" -> "should allow the use of RADIUS"

2.3: "whose permissions and capabilities are know" -> "whose
permissions are known".  Authentication has nothing to do with
capability negotiation.  I actually think better wording could be used
in this paragraph in general.  Something that says based on the
authentication it should be possible to determine the authorization
level of the incoming session (it's 5:30AM.  I can't think of better
wording at the moment)

somewhere: A brief intro on http vs urn's in the xmlns tags should be
mentioned somewhere so the reader understands why one is used in some
places and not in others.

4.1, last example: "example.net/house" -> "example.net/world"

4.3 "The <rpc-error> includes"  should this be a MUST?

4.3 error-tag: ... See list below for allowed values -> see appendix A

4.3: error-message:  "the error condition." -> "the error condition
intended for human consumption."  (IE, don't expect it to be machine
parsible).

4.3: error-info: the list below -> the list in appendex A

5.1: I'd be tempted to say that though some netconf operations work on
things like state, they're not considered to be part of the
configuration datastore

5.1: Section 8.3 and Section 8.7 -> "Capability sections 8.3 and 8.7"
for clarity.

6.1-6.3: various parts of 6.2 and 6.3 are very redundant with 6.1.
EG: last P in 6.2 is same as 4th in 6.1, 6.3 is the 5th P in 6.1...
Most of the text could use a bit of clean-up.  It doesn't read as well
as the rest of the document (which is very well written, so my
expectations have come to be a bit high at this point).

6 general: A diagram to help match terminology against parameters
would be helpful up front.  By the end of the section, my head was
spinning with all the terms regarding matching nodes vs leaves vs ....

6.1 last p: "or modified or acted upon?" somewhere for operations that
    don't just "select"?

6.5: "the only the content match nodes, plus"...  remove "only"

6.6: This section probably needs to be rewritten.  I could barely grok
it after staring at it for a while.

6.7: agent -> server.  (I don't think the rest of the doc talks about
"agent"s much.  I may be wrong).

6.8.2: type=subtree should be mentioned above somewhere first ideally
(I know it's mentioned later)

6.8.3: The <top> node used in the examples worries me.  Is each
standard/company going to define its own "top"?

6.8.6: siibling -> sibling

6.8.8: schema/2.0 -> schema/1.2

6.8.8: "same results" -> "similar results".  The results won't be
exactly identical, right?

7: move the "get" operation up the list under the "get-config"
operation for better grouping.

7: is "operation not supported" allowed even for the netconf core
operations?  I'm not sure there is requirements anywhere (MUST/SHOULD)
for the operational sets that are required to be implemented (simply
defining them doesn't state requirement for implementing them).

7.1: get-config requires? the source parameter.  If so, many of the
(previous, eg) examples don't contain it.

7.1: positive response:  says element returned is contained within
<config> but many of the examples fail to meet this (frequently they
use "data" including in the example in this section!)

7.2: "The target configuration is not simply" -> "... not necessarily simply"

7.2: should probably mention that the operation tag is allowed to be
used repeatedly in the tree?  It's only sort "sort of" obvious.

7.2 replace/create: It would be useful to explicitly talk about how
    "containers" (eg users) vs "objects" (a user) is treated with
    this.  IE, if operation=replace is specified at the users level,
    then *all* the users are replaced.  If specified at the user
    level, then just a single user is replaced.  This is important for
    the reader to understand.

7 general: it would be useful if potential error codes were specified in
the operation lists...  Right now its sort of open in many places.

7.2 delete:  what happens if you do this:

    <users>
      <user>
        <name operation="delete">Fred</name>
      </user>
    </users>

    That'll just delete the name, technically, not the user.  Is that
    legal?  This is where a protocol without a data-model usage case
    runs into problems.  The concept of containers is important when
    talking about operations that affect instances of those containers.

7.2 replace:  Can I do a name change on the search index?  IE, if the
    interface name is configurable can I change it?  There is no
    operation to support this thus I must do a delete and create (or
    equivelent), right?

7.2 delete: can I delete all interfaces with MTU of 1500?  Does
    multiple matches in a data model that may not have defined MTU as
    a unique index allowed within the protocol?

7.2 "Delete the interface" -> "Delete the configuration for the interface".

7.3 "Otherwise, a new one is created" -> "Otherwise, a new one is
    created if allowed."

7.3 "A device may choose not to support remote to remote copy
    operations" -> add "(URL to URL)".  (though see my other message
    on the concern with this first).

7.5 "human users" -> "console users"

7.5: delete "lack of network connectivity".  Let the transport deal
     with that.  If the transport looses it, thats a valid signal.  I don't
     think netconf should be directly tied to a lower level than the
     transport.

7.5: "If the lock is already held, the <error-tag> element will be
     LOCK_DENIED":  It'd be better to distinguish between denied due
     to access not allowed vs denied vs in-use by someone else.

7.5: most of the other examples have wording separation between them.
     This section doesn't so I'd recommend adding "Example showing an
     error when a lock is already in use:" in between the two examples
     for better readability.

7.7: examples don't contain the <top> node that others do.  This is
     inconsistent throughout the doc.  I just wrote it down here.

7.8: need to mention that replies to previous queries are still sent.
     (processing is mentioned, but not the replies to those operations)

8: "... standards bodies or published as proprietary by vendors" ->
   add "etc, ..." or something at the end.  Current wording is too
   limiting to the types of organizations that are likely to publish
   specs.

8: it is not clear from the sentences why you specified two urns (one
   with an embedded xml:ns tag and one without).

8: "each peer needs to understand only those capabilities that it
   might use..."  This is not entirely true for #validate.  It affects
   the results of the on-error parameter if available.  In some cases
   you'll potentially get errors back and in others not.  Sort of
   requires that you understand if the capability is there so you can
   prepare for or expect errors back.  Not sure you want to change
   anything though.

8.3.1: "creating a manipulating" -> creating and manipulating

8.3.1: I'd use a specific example operation in the example for
"operation" rather than the generic term.  It doesn't match the rest
of the examples in the document by making it generic.

8.3.1: for example, through another capability:  Add "EG, lock")

8.3.4.1: why doesn't the text refer to "<running>" here?  A commit is
really a copy-config from <candidate> to <running> even if
#writable-running isn't available and thus I'd make that statement so
the understanding is clear.

8.3.4.2: I'd state that discard-changes is essentially a copy from
running to candidate.

8.4.5.1: confirmed-timeout: please please please at least use seconds
rather than minutes here.  You're making an assumption about
operational environments.  seconds allows for rapid decisions, but
still allows for longer time periods as well (2^32 seconds is a long
time).

8.5.1: "to be inadvertently altered or removed":  This isn't true is
it?  Isn't the roll back only applied to the current operation?  IE, a
single edit-config in which case it is only rolled back to the start
of the edit-config and has nothing to do with what other people have
done either before or after the current operation?

8.6.1: "at least for syntax errors".  So the exact operation results
are indeterminate?  Some devices do something things with this
capability and others do others.

8.7.1: if startup == running in the rest of the document without this
capability, it should probably be mentioned before this point when
first discussing running.

8.9.5.1: example needs top too?

7.7: get should explicitly specify that it only works on running?

7.5: second bullet: committed -> committed or rolled back


-- 
"In the bathtub of history the truth is harder to hold than the soap,
 and much more difficult to find."  -- Terry Pratchett

--
to unsubscribe send a message to netconf-request@ops.ietf.org with
the word 'unsubscribe' in a single line as the message text body.
archive: <http://ops.ietf.org/lists/netconf/>