Comments on draft-ietf-netconf-notification-06

[Andy Bierman <ietf@andybierman.com>]

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