[Netconf] last call on draft-ietf-netconf-subscribed-notifications-16

["Tim Jenkins (timjenki)" <[e-mail]>]

Greetings,

I have read draft-ietf-netconf-subscribed-notifications-16 and support it going to IESG.

I have a number of comments, mostly extremely minor in nature, and none need affect forward progress of subscribed notifications.

Tim

==>

Comments on https://tools.ietf.org/html/draft-ietf-netconf-subscribed-notifications-16

Section 1.2, Notification message definition: remove the () around event(s)

Section 1.2. No definition for just "stream"; the word stream is used alone (without 'event' in front of it in a few places)

Should this paragraph be in the Terminology section?
   All YANG tree diagrams used in this document follow the notation
   defined in [RFC8340].

Section 1.3, paragraph numbered 1: change "would have enabled" to "may have enabled". Use of would implies use of the suggested hints would succeed, and this cannot be guaranteed. (This would then be more consistent with same text in Section 2.4.2.)

Section 1.3, paragraph number 2: It's sort of a circular definition:
      "Configured subscriptions, which allow the management of
       subscriptions via a configuration so that a publisher can send
       notification messages to a receiver of a configured subscription."
       Suggest rephrasing and removing the "receiver of a configured subscription" part.

Section 1.3, second last paragraph: Partly repeats second bulleted paragraph, which explicitly lists exception to this statement "Similarly, a subscription established via RPC cannot be modified through configuration operations". I would suggest removing duplicate statements from these paragraphs regarding what can be changed.

Section 1.4, replace "the RPC operations in this draft replaces" with "the RPC operations in this draft replace"

Section 1.4, replace "single transport session
      to intermix of notification messages" with "single transport session
      to intermix notification messages"

Section 2.3: Second bulleted paragraph seem to imply that the weighting is per-receiver. Perhaps rephrase or refer instead to description elsewhere.

Section 2.3: Is dscp optional? Not stated here, but is for others. Seems it is.

Section 2.4.1: Does not show the transition described in second last paragraph of Section 2.1, or the transition described in second o paragraph of section 2.4.1. But the second doesn't exist based on section 2.4.3; it explicitly says to change state and them moved immediately back if needed, so perhaps add something to second o paragraph of section 2.4.1":

   o  Failed "modify-subscription" RPCs will leave the subscription in
      its previous state, with no visible change to any streaming
      updates. The transition to the receiver active state and back to
      receiver suspended state might not be not externally visible.

Section 2.4.2, third o paragraph: Remove "Note: " since statement is definitive about behaviour (it's not a note).

Section 2.4.5: Should there be some mention about permissions?

Section 2.4.6: Should there be a permissions failure error for kill-subscription? Or is that more generic? Meaning, is command execution permission enforcement not part of this specification?

Section 2.5: Should this
      "an ability to send notification messages to more than one receiver
      (note that receivers are unaware of the existence of any other
      receivers.)"
be changed to something like this
      "an ability to send notification messages to more than one receiver.
      Receivers MUST not be aware of the existence of other receivers.
      "?

2.5.1, receiver state: I think I prefer the state "receiver disconnected" over "receiver timeout" since there are more reasons a receiver would be disconnected. While the text says the states are only meaningful when the subscription state is valid, in practise receivers of invalid subscriptions will have state as well, and "timeout" isn't really true. Further, an implementation may choose to not even create connections under overload conditions, so the receiver won't be just "suspended" nor will it be "timeout".

Additionally, should it not be stated that the "connecting" state is only for when the publisher is actively trying to connect to the receiver? This sentence seems to imply it's more than that "In addition, a configured
   subscription's receiver MUST be moved to connecting if transport
   connectivity cannot be achieved, or ..." Or maybe the "cannot" confuses me: cannot before timeout is really what is meant?

Section 2.5.2

Last paragraph says that replay is supported on configured subscriptions. So why this statement in section 2.5? "On a receiver of a
   configured subscription, support for dynamic subscriptions is
   optional except where replaying missed event records is required.
"

Section 2.5.3, second paragraph: Receivers would only be put into the connecting state if the protocol specific callhome mechanism was invoked. Could they not also go instead to the active state if the connection needed already existed?

Section 2.5.3, maybe change
"Also note that if multiple modifications have occurred during the
   suspension, only the latest one need be sent to the receiver."
to
"Also note that if multiple modifications have occurred during the
   suspension, only the "subscription-modified" notification describing the latest one need be sent to the receiver.
"

Section 2.5.5: maybe change "re-initialization" to "action" - it's not really a re-initialization

Section 2.5.6, first paragraph, last sentence: event -> events

Section 2.7.2, paragraph numbered 2: this case would apply to both dynamic and configured subscriptions; suggest rephrasing to describe the situation and then saying that it applies to both

Section 2.8, maybe change
"This leaf also
   indicates if current state of that subscription is valid, invalid,
   and concluded."
to
"This leaf also
   indicates if the current state of that subscription is valid, invalid,
   or concluded."

Section 2.8, paragraph 3: remove "configured and dynamic" from first sentence

Section 2.8, paragraph 3: Is it not a potential security leak to have event notifications suppressed by permissions change the difference between the two counts?

Section 2.9: Change
"In addition support for optional features "encode-xml",
   "encode-json", "configured" "supports-vrf", "qos", "xpath",
   "subtree", "interface-designation", "dscp", and "replay" MUST be
   indicated if supported."
to
"If supported, the optional features "encode-xml",
   "encode-json", "configured" "supports-vrf", "qos", "xpath",
   "subtree", "interface-designation", "dscp", and "replay" MUST be
   indicated.
"

Section 3, third sentence: remove first word "Or"

Section 4, features for encoding: I don't see the point of global features for showing support for encodings. The encodings that are supported are tied to the protocols that are supported, and it's unlikely any encoding will be supported over all protocols.

Section 4, feature "replay": Given that the streams table shows which streams support reply, is there a need for this feature?

Section 4, DATA NODES: leaf "replay-log-creation-time" has
        when "../replay-support";
        if-feature "replay";
but leaf "replay-log-aged-time" has only
        if-feature "replay";

Should they be the same?

Also, is the
        if-feature "replay";
needed if
        when "../replay-support";
is used when it is using
        if-feature "replay";
already?

Section 4, "leaf configured-subscription-state": The description for "enum valid" looks wrong.

Section 4, "leaf count-sent": Should this be reset to zero on receiver state transition to active as opposed to subscription transition to valid? Just because the subscription is valid doesn't mean the receiver's being sent anything if the connection isn't up.

Section 5.4: "discovered,take" to "discovered, take"


--
Cisco Systems Canada Co.
2000 Innovation Drive
Kanata, ON, Canada, K2K 3E8
Preferences <http://www.cisco.com/offer/subscribe/?sid=000478326>
Unsubscribe <http://www.cisco.com/offer/unsubscribe/?sid=000478327>
Privacy <http://www.cisco.com/web/siteassets/legal/privacy.html>