[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

My review of and comments for: draft-ietf-netconf-notification-03 .txt

So I did finally find some time to review.
I did not check if any of my comments overlap with comments from others,
so appology if there are duplicates.

1. I see:

     1.3  Motivation

     The motivation for this work is to enable the sending of asynchronous
     messages that are consistent with the data model (content) and
     security model used within a Netconf implementation.

   I find it somewhat strange to read about consistency with "the data model" 
   and "security model". Because I do not recall we have defined any.
   I understand we want to send notifications over NETCONF (and if netconf
   is secured via (for example) ssh or some such, then we want to do so too,
   but is that a security model? I know we want to at least send notification 
   if some config erorr occurs as a result of our NETCONF config changes etc.
   In that sense, the content is probably "consistent" with what we use
   in the Netconf protocol to configure a device/system/server.
   Anyway, not sure how serious we need to specify our motivation, but I would
   prefer to not suggest we have done/completed more than what we really did. 

2. Consistency.
   In 1.1 we define "Managed Entity" (also sometimes called "NetConf server")
   In 1.2 we speak about a "Netconf client" without having defined the term.
   In 1.2 and in 1.4, we also speak about "an agent", where I think we mean
   "Netconf server". In any event, we did not define "agent"
   In 1.4 we speak about "a manager" which I think means "Netconf client"

   I expect that most of us on this WG mailing list understand what is meant
   with all these terms. But it might be better to try and be consistent in
   the terms we use for the "entities" on each side of the connection.

   More consistency
   Either use "subscription ID" or "subscription Id" But pls be consistent.

   More consistency
   In the protocol spec, the scetion headers for the operations have:

     7.1.  <get-config>

         Description:

   While this notification spec has:

     2.1.1  create-subscription

         <create-subscription>
  
         Description:

   Maybe we better try to be consistent. Makes it easier for novices.

   More consistency
   On page 13 (in the reply) you use 
              <sessionEventStream>
              <eventStreams>

                   ....

               </sessionEventStreams>
              </eventStreams>

   It seems to me that the end-tags are in the wrong order
   Also, sessnioEventStream is singular while end-tag is plural
   In the get-config sessionEventStream is all singular.
   On page 14, sessionEventStreamns is always plural.
   I have not checked other places.

   Indentation and alignment of indented tags could be better
   so it is easier to check examples.

3. Requirements (sect 1.4)
   I think that instead of:
      The requirements for this solution are as follows:
   I would state:
      The following are the requirements we have addressed in this solution:
   But maybe that is just personal taste.

4. Comments on some bullets in sect 1.4:
      o  solution should provide a filtering mechanism
   Do we mean that filtering moust occur/be provided at the notification
   origin/source? If so, pls be explicit.
      o  solution should support replay of locally logged notifications
   I guess "locally" mean on the "managed entity" ??
   But that is not 100% clear.
 
5. Do we consider notifications a capability.
   Not sure what sect 3.1 tries to say. But I wonder if this:

     "urn:ietf:params:xml:ns:netconf:notification:1.0"

     For Example


      <hello xmlns="urn:ietf:params:xml:ns:netconf:base:1.0">
        <capabilities>
          <capability>
            urn:ietf:params:xml:ns:netconf:base:1.0
          </capability>
          <capability>
            urn:ietf:params:xml:ns:netconf:capability:startup:1.0
          </capability>
          <capability>
            urn:ietf:params:xml:ns:netconf:notification:1.0
          </capability>
        </capabilities>
        <session-id>4</session-id>
      </hello>

   Would not be better defined as a capability and so the URU would be:

     "urn:ietf:params:xml:ns:netconf:capability:notification:1.0"

   And then I guess we would need to update the document to describe
   notifications as a capability, that ius, use the template to do so.
   Just wondering and thinking aloud here.
   If the WG feels I am just rambling... then I will shut up on this.

6. Page 13. 
   I think that in thebeginning there is a missing <rpc...> tag

7. Pages 13-15
   Not sure I  understand (not even sure we need) the differentiation
   between sessionEventStreams and systemEventStreams.
   I do see that the SAME streams themselves get listed in the 
   example responses to both a get for sessionEventStreams and for 
   systemEventStreams?? So what is the difference (if any)?

8. W.r.t. 3.2.5.3  Stream Retrieval Schema
   Mmm.. in the protocol doc, we have done the Schema in an appendix.
   Why do we have it in the middle of the document in this case?
   The schema itself and its indentation is kind of unreadable if
   you ask me. I'll need to take another look at that later.

9. I will have to study sect 3.4 in much much more detail before
   I can comment. It looks awfully complex to me.

10. I will have to study sect 4 in more detail before I can comment.

11. Why is this capability:

     7.3  Capability Identifier
 
       The Event Notification Replay capability is identified by following
       capability string:

       http://ietf.org/netconf/notificationReplay/1.0

    represented with a URI as suggested by the capability template?

    I also am not sure we agreed that we need/want a replay capability, did we?

12. I believe that this piece of text

      The IETF has been notified of intellectual property rights claimed in
      regard to some or all of the specification contained in this
      document.  For more information consult the online list of claimed
      rights.

    Is NOT sipposed to be in the RFC.

I get these problems with citations/references. Not sure if teh tool reports
them correctly, but the authors/editors may want to check:

!! Missing Reference for citation: [3]
  P004 L042:    document are to be interpreted as described in RFC 2119 [3].

!! Missing Reference for citation: [NETCONF-PROTO]
  P004 L008:    NETCONF [NETCONF-PROTO] can be conceptually partitioned into four
  P004 L046:    Managed Entity: A node, which supports NETCONF[NETCONF-PROTO] and has

!! Missing Reference for citation: [NETCONF-SSH]
  P027 L016:    over SSH transport mapping [NETCONF-SSH]
  P027 L024:    As the examples in [NETCONF-SSH] illustrate, a special character

!! Missing Reference for citation: [NETCONF]
  P038 L022:    [NETCONF]  Enns, R., "NETCONF Configuration Protocol",

!! Missing Reference for citation: [URI]
  P038 L045:    [URI]      Berners-Lee, T., Fielding, R., and L. Masinter, "Uniform

!! Missing Reference for citation: [XML]
  P004 L044:    Element: An XML Element[XML].
  P038 L049:    [XML]      World Wide Web Consortium, "Extensible Markup Language

!! Missing Reference for citation: [refs.RFC2026]
  P038 L053:    [refs.RFC2026]

!! Missing Reference for citation: [refs.RFC2119]
  P039 L009:    [refs.RFC2119]

!! Missing Reference for citation: [refs.RFC2223]
  P039 L013:    [refs.RFC2223]

!! Missing Reference for citation: [refs.RFC3080]
  P039 L017:    [refs.RFC3080]

Bert

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