Subject: [Netconf] review of draft-ietf-netconf-monitoring-03

From: Juergen Schoenwaelder <[email-masked]>
Date: Fri, 23 Jan 2009 14:00:28 +0100

a) The abstract does (a) not summarize all the things covered in the
   data model and (b) is too long an detailed for the things
   covered. I propose to compose a new short abstract and to move most
   of the existing text into section 1 before 1.1, extending it as
   needed.

b) The document assumes that capabilities / schemas can dynamically
   change and it kind of assumes that applications poll the
   /netconf/schema tree. I am wondering where there is not a
   notification defined so that I can subscribe and get notified
   if the capabilities / schemas change.

c) Terminology:

   Operation:  This term is used to refer to NETCONF protocol
      operations.  Specifically within this document, operation refers
      to NETCONF protocol operations defined in support of NETCONF
      monitoring.

   I fail to see value in giving the term operation a more
   constrainted meaning. I prefer to have the term used with the
   normal RFC 4741 meaning.

d) Terminology:

   Schema:  This term is used to refer to a data model fragment,
      independent of which data modeling language is used in the data
      model.

   What is a "data model fragment"? I suggest to replace this phrase
   with "a machine readable data model definition".

e) I would prefer if section 2. would focus on the XML instance
   document structure and be more data modeling language agnostic. I
   believe this improves readability since instance document structure
   is really the most important to get right. I will detail concrete
   changes in the following comments.

f) Change title of 2.1 to: The /netconf Subtree

g) Turn the first two sentence fragements into full sentences:

   The /netconf subtree is the root of the monitoring data model.  It
   acts as a container for the other monitored data.

h) I suggested to rename "configurations" to "datastores" since this
   seems to be the more appropriate term. So things become:

   datastores: (type:  ConfigurationDatastore)
    List of NETCONF datastores on the server.
    Includes all supported datastore types (running, candidate, startup).

i) Wording change (use plural). OLD:

  schemas (type:  SchemaEntry)
    List of schemas supported on the server.
    Includes all the information required to identify the schema and
    to support its retrieval.

   NEW:

  schemas (type:  SchemaEntry)
    List of schemas supported on the server.
    Includes all the information required to identify schemas and
    to support their retrieval.

j) Clarification needed:

  sessions (type:  ManagementSession)
    List of all active sessions on the device including NETCONF
    and other sessions (eg: CLI).

   I think this needs clarification. I doubt you want to report a
   normal user login on a Unix server, or was that the intention?

k) Change title of 2.1.1 to: The /netconf/capabilities Subtree

l) Can capabilities announced during <hello> be dropped without
   closing the session? If not, then the text should be changed.

   OLD:

   The list MUST include all capabilities exchanged during session
   setup still applicable at the time of the request.

   NEW:

   The list MUST include all capabilities exchanged during session setup.

m) Change title of 2.1.2 to: The /netconf/datastores Subtree

n) Make the first sentence in 2.1.2 a full sentence.

o) Section 2.1.2 needs to be expanded. It talks about lockedNodes and
   such things and this requires to first read the data model in order
   to understand this text. This might require to introduce a
   subsection describing the /netconf/datastore/locks subtree.

p) Change title of 2.1.3 to: The /netconf/schemas Subtree

q) I suggest to rename schema/identifier to schema/name. After all,
   it contains a name. See also s) below.

r) Terminology consistency change:

   OLD:

   They are also used in the <get-schema> RPC.

   NEW:

   They are also used in the <get-schema> operation.

s) The description leaves it open whether the identifier (or name)
   contains a module name or a file name. This is not good for
   interoperability. Furthermore, the YANG description tells me again
   something different. I suggest that it must be the module name for
   data modeling languages that have a concept of a module name and it
   may be a filename in other cases.

t) Editorial:

   OLD:

   Each MUST be reported

   NEW:

   Each version MUST be reported

u) You make the assumption that a schema can only define one
   namespace.  This might be OK but I wanted to spell this out so that
   the WG takes this design decision not by accident.

v) The location description is not consistent with the schemas and the
   examples since it is actually a list. Well, the YANG says it is a
   leaf but the example shows the encoding of a leaf-list. This is
   screwed up (and probably the example is right).

w) The union of a string with the magic value "NETCONF" and a URI
   looks really ugly. I suggest that a location leaf always contains a
   URI.  If you need a mechanism to indicate retrieval via get-schema,
   create another empty leaf that is only present if retrieval is
   possible via NETCONF get-schema.

x) Change title of 2.1.4 to: The /netconf/sessions Subtree

y) I believe this needs better wording:

   username (type: xs:string)
     Session owner.

   I do not think NETCONF has a notion of session ownership. I think I
   know what you mean but it needs to be written down properly (and I
   am not sure to what extend the username is not transport specific).

z) I fail to see why sourceHost is included at all. The IP address or
   a DNS name is not sufficient to identify a client in all cases and
   the value of this diminishes greatly if you have NATs and so on.
   But the most important problem I have is that an IP address is not
   a transport endpoint.

A) Change title of 2.1.5 to: The /netconf/subscriptions Subtree

B) The description of SesionId is screwed up:

   sessionId (type: SessionId)
     Unique identifier for the notifications.
     MUST return the same value as returned in
     'sessions' to allow correlation.

   This is not an identifier for notifications. You probably want
   something like this:

   sessionId (type: SessionId)
     A unique identifier for a session carrying notifications.
     The value of sessionId MUST be the same as the value of
     the corresponding /netconf/sessions/sessionId to allow
     correlation.

C) For me, messagesSent does not belong into /netconf/subscriptions
   since it is a statistics counter. I know, it is right now the only
   session specific counter but that is not a good argument to be
   inconsistent in the organization of the tree. Other session
   counters might be added in the future.

D) Change title of 2.1.6 to: The /netconf/statistics Subtree

E) Add a tree layout figure similar to all the other sections.

F) The document assumes zero-based counters and no rollovers ever
   happen:

   Ie:  current time - startTime defines the collection interval for the
   metrics allowing for calculations such as averages.

   SNMP people might find this approach broken.

G) I tried to draw a case diagram for the counters and I failed. I
   believe the counters are not well thought out. Perhaps it helps
   to draw case diagrams and to organize suitable counters along
   the netconf protocol layers.

H) Change title of 3.1 to: The <get-schema> Operation

I) Change the first input argument to name (see above) and the
   description as follows:

   name (type: xs:string): Name of the schema to be retrieved.
   Typically the module name or filename of the schema.

J) Wording change:

   OLD:

   format (type: SchemaFormat): The data modeling language of the file/
   module.

   NEW:

   format (type: SchemaFormat): The data modeling language of the schema.

K) Why is the schema in the example returned in CDATA??

L) Rewording:

   OLD:

   A NETCONF client retrieves the list of supported XML Schema from a
   NETCONF server using the <get> RPC request. The list of available
   XML Schema is retrieved by requesting the <schema> subtree via a
   <get> operation.

   NEW:

   A NETCONF client retrieves the list of supported schema from a
   NETCONF server using the <get> operation by retrieving the
   /netconf/schema subtree via a <get> operation.

M) Editorial changes:

   OLD:
   
   ie.

   NEW:

   i.e.,

   OLD:

   http.i

   NEW:

   http

   OLD:

   <get-schema> RPC

   NEW:

   <get-schema> operation

N) I did not review the XML schema definitions. The pattern in the
   ipAddress schema differ from the pattern in yang-types, I am not
   sure at the moment this impacts semantics. Since the sourceHost
   definition seems not very useful in the first place, my suggestion
   would be to nuke sourceHost and with it the whole XML Schema
   definition of an IP address type.

O) The security considerations just say "be careful" which I think
   will not be sufficient to pass the security area.

P) Acknowledgements: I think the phrase "earlier draft of Schema"
   needs to be rewritten and I think it is more appropriate to
   acknowledge the authors of the earlier schema drafts rather than
   the whole WG.

Q) Appendix A introduction rewrite:

   OLD

   The following YANG module is included as a reference only.  It is
   based on YANG definition at the time of publishing and is subject to
   change as a result of netmod work underway to refine YANG.  Further
   details on YANG and updated non-normative reference to this model are
   available at:
   http://www.yang-central.org/twiki/bin/view/Main/YangExamples.

   NEW

   The following YANG module is included as a reference only.  It is
   based on YANG specification at the time of publishing and is subject to
   change as a result of NETMOD work underway to refine YANG.

   I suggest to remove the online reference since it is hard to
   guarantee that this is kept updated.

R) Yang meta information should in spririt follow RFC 4181. 

S) I think it is useful to write reference statements in the following
   format:

      reference "RFC 4741: NETCONF Configuration Protocol"

   Not every reader knows RFC numbers by heart.

T) I am concerned about the extensibility of some of the type
   definitions. Some should probably be maintained by IANA.  Andy
   recently published a YANG module for NETCONF and in the ideal
   world, this document would import some of the needed type from 
   the YANG module for NETCONF.

U) SSL should be TLS

V) What is WebUI? How does a NETCONF server determine whether a client
   is a CLI, NETCONF, or WebUI??

W) There are many statements without descriptions and I believe it
   would be useful to repeat text from section 2 in the data model so
   that it becomes selfcontained.

X) I prefer a naming style where short names that are meaningful in
   the context of an instance document are used. For example, I like

   <locks>
     <global>
        <sessionId>42</sessionId>
	<time>..</time>
     </global>
   </locks>

   <locks>
     <partial>
        <lock-id>12</lock-id>
	<sessionId>42</sessionId>
	<time>..</time>
	<select>/netconf</select>
	<node>..</node>
	<node>..</node>
	<node>..</node>
     </partia>
   </locks>

   over what the document actually produces.

Y) A description of lockId is needed to make it clear that it is the
   value returned by <partial-lock> and the partial locking draft must
   be clarified that the lock id is globally unique within a NETCONF
   server so that it can actually serve as a key here without having
   to scope it by the sessionId.

Z) I see another case where a common xpath data type would make sense.

+) In the statistics section, language needs to be more precise. What
   is a "correctly started session"? What are "sessions started towards
   the NETCONF peer"? 

/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/>
_______________________________________________
Netconf mailing list
[email-masked]
https://www.ietf.org/mailman/listinfo/netconf

---

From: WashamFan <[email-masked]>
Date: Sat, 24 Jan 2009 11:41:28 +0800

Hi, 

>     closing the session? If not, then the text should be changed.
According to para 2 in Abstract, and last sentence in sec 2.1.1, yes, announced capabilities can be dropped.

>     subsection describing the /netconf/datastore/locks subtree.
Last sentence in sec 2.1.2 says:
"The selection is the xpath expression which was used to evaluate the list of nodes to lock."
Is subtree expression ignored?

>  K) Why is the schema in the example returned in CDATA??
I think, returned data is a complete XSD document, so it should be in CDATA in order to avoid parsing error.
  
washam  
>  Fax:   +49 421 200 3103         <

---

From: WashamFan <[email-masked]>
Date: Sat, 24 Jan 2009 11:41:28 +0800

Hi, 

 

washam

---

From: Juergen Schoenwaelder <[email-masked]>
Date: Sat, 24 Jan 2009 07:19:57 +0100

> According to para 2 in Abstract, and last sentence in sec 2.1.1, yes, announced capabilities can be dropped.

I question that this document can define the semantics of the <hello>
message in this way. I believe RFC4741 is not clear enough about the
lifetime of capabilities announce in the <hello> message and it needs
more discussion on finally documentation in RFC4741bis what the
lifetime of capabilities announce in the <hello> messages is.
 
> "The selection is the xpath expression which was used to evaluate
> the list of nodes to lock."  Is subtree expression ignored?

I don't understand your question. The <partial-lock> select elements
carry xpath expressions, or if :xpath is not supported, instance
identifier expressions. I am not sure that was your question.
 
> I think, returned data is a complete XSD document, so it should be
> in CDATA in order to avoid parsing error.

I can't judge this but if such wrapping is needed for some data
modeling languages, then this needs to be spelled out. Right now, this
wrapping only appears in an example.

/js

---

From: Juergen Schoenwaelder <[email-masked]>
Date: Sat, 24 Jan 2009 07:19:57 +0100

/js

---

From: WashamFan <[email-masked]>
Date: Sat, 24 Jan 2009 17:55:44 +0800

>  > According to para 2 in Abstract, and last sentence in sec 2.1.1, 
> yes, announced capabilities can be dropped.
>  
Sorry for ambiguity. I meant if subtree filtering is used, how to deal with <select> elements, instance identifier expressions?

---

From: WashamFan <[email-masked]>
Date: Sat, 24 Jan 2009 17:55:44 +0800

---

From: Juergen Schoenwaelder <[email-masked]>
Date: Sat, 24 Jan 2009 12:23:00 +0100

On Sat, Jan 24, 2009 at 05:55:44PM +0800, WashamFan wrote:
> Sorry for ambiguity. I meant if subtree filtering is used, how to
> deal with <select> elements, instance identifier expressions?

Edit-config does not do subtree filtering. I still don't get the
question. And even if it would, I fail to see the issue since the
<partial-lock> locks a set of nodes and once <partial-lock> processing
is complete, the select / instance identifier expressions do not
really matter anymore from the processing point of view.

/js

---

From: Juergen Schoenwaelder <[email-masked]>
Date: Sat, 24 Jan 2009 12:23:00 +0100

/js

---

From: Martin Bjorklund <[email-masked]>
Date: Sun, 25 Jan 2009 12:23:02 +0100 (CET)

Hi,

Juergen Schoenwaelder <[email-masked]> wrote:
>    interoperability.

My idea was that each DML must define how it maps to this list.  Such
a section is included in the latest YANG draft, but maybe it should be
included here instead?

>    Furthermore, the YANG description tells me again

This may work as well.

>    the WG takes this design decision not by accident.

We can easily change this to a leaf-list, but OTOH the three current
DMLs all define one namespace only per module/file, and since the
SchemaFormat is hardcoded in this data model (as an enumeration), this
spec needs to be updated if a new DML comes along.  And this leaf can
be changed to a leaf-list at the same time, if necessary.

>    screwed up (and probably the example is right).

Yes, this should be a leaf-list in the YANG module.

>    possible via NETCONF get-schema.

That works as well.  I don't have any strong preference for either of
the alternatives.

>    the value of this diminishes greatly if you have NATs and so on.

Correct, but I think it's quite useful to have this information in
many cases (i.e. when the client is not behind a NAT).  In the worst
case the information is not useful for some sessions.

>    metrics allowing for calculations such as averages.

I'm not sure I understand this comment.  The YANG module uses the
yang:zero-based-counter32 type, which will rollover.  Or do you mean
that the problem is that the normative text does not define this
semantics?

> K) Why is the schema in the example returned in CDATA??

To avoid using &gt; in the example.  This is just an XML encoding
detail.

/martin

---

From: Martin Bjorklund <[email-masked]>
Date: Sun, 25 Jan 2009 12:23:02 +0100 (CET)

Hi,

This may work as well.

/martin

---

From: Juergen Schoenwaelder <[email-masked]>
Date: Sun, 25 Jan 2009 12:59:53 +0100

> case the information is not useful for some sessions.

First, I claim reporting an IP address is not sufficient to identify a
client. Second, I claim that there are well established ways to find
out which clients are connected to a TCP server. Third, I claim it
might be not easy to implement this leaf if all the netconf server has
is a file description provided by an ssh daemon and which might not
even be the socket connected to the client itself.

Note that some popular SSH implementations do have mechanisms to mux
new "connections" into existing SSH sessions by opening new channels.
So even if the argument is that this leaf is needed to find out which
client is related to a specific NETCONF server session and if you add
a port number, there are still possible cases where it won't work or
not provide necessarily meaninful results.

Anyway, if the WG feels this leaf is really essential to have, then
just ignore my comments.

> To avoid using &gt; in the example.  This is just an XML encoding

It needs to be defined whether all models are supposed to be returned
in CDATA or if implementors can choose whatever they like or something
else. Right now, I believe this is under specified and this is what I
wanted to point out.

/js

---

From: Juergen Schoenwaelder <[email-masked]>
Date: Sun, 25 Jan 2009 12:59:53 +0100

/js

---

From: Martin Bjorklund <[email-masked]>
Date: Sun, 25 Jan 2009 16:10:13 +0100 (CET)

> else. Right now, I believe this is under specified and this is what I

IMO we should not specify which encoding servers should use, and it
doesn't really matter.  It is up to the server to use some legal XML
encoding, and clients should be prepared to handle the legal variants.
I don't think this object is different from any other object.  For
example, the <format> element could be encoded as:

  <format><![CDATA[YANG]]></format>

This is legal XML and should be supported.

/martin

---

From: Martin Bjorklund <[email-masked]>
Date: Sun, 25 Jan 2009 16:10:13 +0100 (CET)

/martin

---

From: Juergen Schoenwaelder <[email-masked]>
Date: Sun, 25 Jan 2009 16:43:07 +0100

> This is legal XML and should be supported.

I am not sure why different encodings increase interoperability. So
you are saying I can wrap any YANG data type values into <![CDATA[]]>
if I like to do so. Is that spelled out in the YANG draft somewhere?
And if CDATA is so beautiful, what happens if a description clause
contains the sequence ']]>'? Ah, the XML solution of using multiple
CDATA of course:

  <![CDATA[foo]]]]><![CDATA[>bar]]>

So if I follow you, I can write <level>42</level> as

  <level><![CDATA[4]]><![CDATA[2]]></level>

everywhere in an XML instance document for

  leaf level { type int8; }

since this is standard XML stuff? If the NETMOD/NETCONF WGs feel this
is the way to go, fine. But spell it out using scary examples so that
people implementing things know about this and get it right.

/js

---

From: Juergen Schoenwaelder <[email-masked]>
Date: Sun, 25 Jan 2009 16:43:07 +0100

/js

---

From: Martin Bjorklund <[email-masked]>
Date: Sun, 25 Jan 2009 21:06:22 +0100 (CET)

> people implementing things know about this and get it right.

I never said CDATA was beautiful.  But as you noted, this is the way
XML works.  Maybe scary examples should be added, but if so, the right
place is probably in section 3 ("XML Considerations") of 4741bis.  I
don't think the monitoring draft is the right place for this.

/martin

---

From: Martin Bjorklund <[email-masked]>
Date: Sun, 25 Jan 2009 21:06:22 +0100 (CET)

/martin

---

From: Andy Bierman <[email-masked]>
Date: Sun, 25 Jan 2009 12:20:41 -0800

> don't think the monitoring draft is the right place for this.

This is kind of stretching the Postel Principle to the limit.
I think the NETCONF/NETMOD documents should be silent on
XML like this, and not encourage its use.

> /martin

Andy

---

From: Andy Bierman <[email-masked]>
Date: Sun, 25 Jan 2009 12:20:41 -0800

Andy

---

From: WashamFan <[email-masked]>
Date: Sat, 31 Jan 2009 17:02:00 +0800

Hi,
Sorry, I made a mistake. Just ignore it.

washam

---

From: WashamFan <[email-masked]>
Date: Sat, 31 Jan 2009 17:02:00 +0800

Hi,

washam