Subject: Comments on draft-ietf-netconf-notification-06

From: Andy Bierman <[email-masked]>
Date: Sun, 11 Mar 2007 11:43:40 -0700

Hi,

I am not sure how many of these comments are duplicates
from other reviewers.  We'll sort that out before the
WG meeting at IETF #68.

In general, I think the draft is almost done, and hopefully
we can resolve all the minor open issues soon.  There comments are
in page order, and range from typos to design flaws...

- pg 3, ToC: Suggest title change of sec. 3.4
     'Subscriptions not Configuration Data'

- pg 3, ToC: Sec. 5 title : Capitalize examples

- pg 3, ToC: No entry for IANA Considerations (no section either)

- pg 4, middle of page: Move 'Figure 1' above the para, and directly below
         the figure

- p4 4. sec. 1.1 Terms
    Managed Object is unclear, not used anywhere, should be removed

- p5 Terms
    Operation: 2nd sentence is not true.  Term used as stated only in
sentence 1.

- p5 sec1.2: Put first para in Terms section as 'Event'

- pg 7, bottom: missing period after parameter

- pg 7 - 8: General comments on create-subscription

   - need to specify conceptual data types for all parameters in this
section

   - need to show usage examples in this section

   - need to combine entire section 6 (Replay) into this section since it is
     confusing to introduce just the parameters but not the feature.

   - once again, we are creating parameter dependencies that XSD cannot
represent.
     (Just as happy to rant against XSD than change things though ;-)

- pg 8, Start Time and Stop Time comments

   - What if a legal start-time (or start/stop pair) specifies some time in
     the future?

   - What if the timezone is given and it is not the same as the agent's
timezone?

   - What if a time change (e.g., daylight savings time) happens?
     - Explain how both parameters handle forward and backward boundary
conditions

- pg 8, sec 2.1.1, Negative Response
   This section is correct, but inconsistent with the text in sec. 6.1,
para 3.
   There is no mention of a 'warning-and-continue' mode of operation here.
   (More reason why sec 6. create-sub. details needs to be integrated
into this section.)

- pg 9, sec 2.3, Terminating the Subscription

   - Does the <kill-session> have to come from another session? (I think so)

   - IMO, since we do not have an endless RPC reply model, it is not a
burden
     to the agent to accept a <close-session> from the session getting
notifications

   - Does sentence 2 mean after the last replayed notification that meets
     the stop time criteria, the session is terminated?

   - last sentence, capitalize Netconf to NETCONF

- pg 10, sec 3.1.2, cap. exchange example
   IMO, this section should be removed.  Sec. 3.1.1 defines the
capability identifier.
   Just say that RFC 4741 defines the capability exchange.

- pg 11, diagram
   This is a good diagram but there should be text following it explaining
   all the boxes, why they are there, and where in this document the
relevant
   definitions can be found

- pg 11, 3.2.1-2, typos
   - s/via NETCONF protocol/via the NETCONF protocol/

   - missing period after last sentence

   - s/i.e. the notification/i.e., the notification/

- pg 11, sec 3.2.3, Default Event Stream
  Mentions the "NETCONF" notification event stream but this is not
defined anywhere.
  Need to put this in the IANA considerations section that will be added.

- pg 12, typos
   - para 1,   s/e.g. SNMP, syslog, etc./e.g., SNMP, syslog/

   - para 3, missing space after <get> and before operation

- pg 12, Name Retrieval comments
   - Need introduction and formal description of the data model for
retrieving
     stream names

   - Can we use just one namespace for all NETCONF data model objects,
     put the definition in IANA, and use it for notification data?

   - agree with comment to change <eventStreams> to <event-streams> for
     better consistency with rest of this doc, and RFC 4741.

- pg 13, bad XML style
   <description> end tags on new line introduces significant whitespace
   into the element content

- pg 13, sec 3.2.5.2, redundant section
   This section is confusing to read, and does not really add any value,
   since the previous section just defined event subscription.

- pg 14-15, XSD comments
   - should define the new verbs to be in the NETCONF base namespace and use
     the capability to determine whether to use it (like all other NC
operations)
   - do not need to import these 3 namespaces
   - should define 1 namespace for data models related to the NETCONF
protocol,
     not a separate namespace for every bit of monitoring data.
   - do not need 'stream' parameter; it is already in the
<create-subscription> RPC
   - do not need the <lastModified> read-only timestamp.
     This is just general metadata associated with the configuration
database.
     We should have a general <config-changed> notification if this is
so important.
     IMO, it is not important, and also a bad idea to copy this aspect
of MIB design.
   - can we use short prefixes, like 'nc' instead of 'netconf'?

- pg 16, sec 3.4
   Not clear why this section is here.  It is actually incorrect.
   If a proprietary configuration data model (or future standard) exists,
   then this section is wrong.

- pg 17, sec 3.5
   First sentence is wrong; multiple filters cannot be combined.

- pg 17, 3.5.2
   Term 'just-in-time filtering' is used here without any explanation
   what it is, or why it matters in this document.

- pg 18, message flow diagram
   Should mention (and show) that many rpc/rpc-reply sequences could
   occur before the create-subscription.  In fact, text suggests use
   of get(event-streams) first.

- pg 19, XSD comments
   Apply same changes wrt/ namespaces, imports, and prefixes

- pg 20, XSD typo
   Indentation of <xs:choice> is wrong

- pg 22-26, sec. 6
   Move entire section to a non-normative appendix and state clearly that
   the data models shown are examples, not standards
   (bugs found by others in examples not confirmed)

- pg 27, sec 6 Replay
   Should move all <create-subscription> details to sec 2.1.1

- pg 27, sec 6.1, para 2
   Is this warning-then-continue mode really needed?  Why not just send
   the replay-complete notification right away, and not special-case any
   parameters.  Bad filter data is just a no-match, not an error.

- pg 28, error-tags
   We cannot update RFC 4741 to add redundant specialized error codes.
   We already have 'invalid-value'.  IMO, these should not even be errors.
   If the supplied timestamps are well-formed, but produce no output,
   then this is an empty data set.

- pg 28, error handling design
   The manager and agent procedures can both be simplified here.
   If the parameters are well-formed, but produce no matches,
   then this is no different than if the filter removed everything
   and there was no output.  In both cases, just sent the replay-complete
   notification right away, instead of any warnings or errors.
   If a stop-time is specified, then the normal termination procedure
   is followed after the replay-complete is sent.

- pg 29, XSD comments
   - do not need a special namespace for the replay-complete notification

   - suggest simple name like 'replay-complete' instead
     of 'replayCompleteNotification'.  This appears in instance documents

   - eventClass element introduces data model details into this document
     that the WG has not agreed to

   - eventClass is an empty element with no data type or content.  Doubt
     that is what is intended.

   - do not need statistics collection and reporting in this notification

   - do not need a timeGenerated timestamp here.  The manager's receive
     timestamp is good enough, considering that the content or time
     of this notification is irrelevant.  The manager just needs a marker
     to signal the end of replay.  (That was the functional requirement.)

   - agree with Martin that a simple empty notification element called
     <replay-complete> is sufficient:

        <notification>
          <replay-complete/>
        </notification>

   - even if numberEventsReplayed was useful, it should be a bounded integer
     like 'unsignedInt' or 'unsignedLong', not integer.  2^^64 log entries
     is a large enough amount to cover most implementations ;-)

- pg 30, Security Considerations

  - remove 'use of <kill-session>, only want to document the threats from
    this document, not RFC 4741.
  - fully specify the exact security threat, exact parameter names, use
cases, etc.
  - fully specify which data model content is writable and identify any
threats
    to services due to notification delivery on a NETCONF session. (Are
there any?
  - explain that the notification content is outside the scope of this
document
    but care must be taken just as with SNMP Notifications
  - explain how notifications are handled by the agent in the presence
of access control.
    Is the entire notification dropped if only some of the included data
is not allowed,
    or just the classified data removed from the notification?
  - capitalize netconf to NETCONF in last sentence

- pg 30, missing IANA Considerations section
  - need to determine exact IANA requests and add them here

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