Dynamic Subscription to YANG Events and Datastores over NETCONF
draft-ietf-netconf-netconf-event-notifications-22

Thank you for addressing my DISCUSS.

Thanks for the work everyone has put into this document. I have only a small number of comments and some nits.


== COMMENTS ==
- section 4, "MUST be supported" but by which party ?
- section 7, MUST all components (except 'error-severity') be part of the rpc-error ? If so, then make it clear
- section 8, see the subscriber as the ennemy, but, can also the exporter be a threat?


== NITS ==
- it would be clearer to group all authors by affiliation
- abstract providing references to subscribed notifications and YANG-push documents would be a plus
- section 3, expand RPC
- section 3, probably because I am not a native English speaker, but I cannot really parse "A solution MUST reply" esp the word "solution"

Section 6

   Notification messages transported over the NETCONF protocol MUST be
   encoded in a <notification> message as defined within [RFC5277],
   Section 4.  And per [RFC5277]'s "eventTime" object definition, the
   "eventTime" populated with the event occurrence time.

nit: I think the last sentence is actually a sentence fragment.

Section 7

This "Either it will correspond to [...] Or this 'error-tag' will
correspond to [...]" seems to preclude future extensions; do we want to
add some weakening language like "for the mechanisms specified in this
document"?

                                                  The specific identity
      to use depends on the RPC for which the error occurred.  Each
      error identity will be inserted as the "error-app-tag" following
      the form <modulename>:<identityname>.  An example of such as valid
      encoding would be "ietf-subscribed-notifications:no-such-
      subscription".  Viable errors for different RPCs are as follows:

            RPC                     use base identity
            ----------------------  ----------------------------
            establish-subscription  establish-subscription-error
            modify-subscription     modify-subscription-error
            delete-subscription     delete-subscription-error
            kill-subscription       delete-subscription-error
            resync-subscription     resync-subscription-error

This is probably just my lack of familiarity with the protocol, but the
text doesn't do much to indicate how the "base identity" concept in the
table corresponds to the "<modulename>:<identityname>" syntax or the
specific example given.  I think that this just means that the
<identityname> must be of the base type or derived from it, so maybe
"derive from" or "have" instead of "use" in the table heading would be
more clear.

      The yang-data included within "error-info" SHOULD NOT include the
      optional leaf "error-reason", as such a leaf would be redundant
      with information that is already placed within the
      "error-app-tag".

I'm not sure where this "error-reason" leaf is defined -- I don't  see
it in any of subscribed-notifications, yang-push, or RFC 6241.

Section 8

                                                                     The
   publisher MAY also suspend or terminate a subset of the active
   subscriptions on that NETCONF session.

I'd suggest saying/repeating why the publisher might do this, i.e., "MAY
also suspend or terminate [...], in order to reclaim resources and
preserve normal operation for the other subscriptions."

Appendix A.2

I'd suggest adding a note that the "id" values of 22, 23, and 39 are
just examples, and that actual values may not be small integers (akin to
my comment on the RESTCONF document).

Please consider the comment from the TSV-ART review about the example DSCP value (Thanks Wes!). I actually would also appreciate to add a comment that this is an internal value that depends on the network configuration (in order to avoid that people just randomly copy this example value and suddenly always use 10)!