Re: [Netconf] Yangdoctors last call review of draft-ietf-netconf-subscribed-notifications-10

["Eric Voit (evoit)" <[e-mail]>]

Hi Andy,



Thanks very much for the excellent comments.   Thoughts in-line...



Also where changes were made, you can see them in the working copy at:

https://github.com/netconf-wg/rfc5277bis/blob/master/draft-ietf-netconf-subscribed-notifications-11.txt

(there are two agreed changes from the WG session to be embedded, but the comments below are in there.)





> From: Andy Bierman, March 15, 2018 6:01 PM

>

> Reviewer: Andy Bierman

> Review result: Almost Ready

>

>

> 1.2 Terminology

>

>    Notification message: A set of transport encapsulated information

>    intended for a receiver indicating that one or more event(s) have

>    occurred.  A notification message may bundle multiple event records.

>    This includes the bundling of multiple, independent RFC 7950 YANG

>    notifications.

>

>   >> Cannot find any text that supports this claim; find the contrary:

>     from 2.6:

>        This notification

>        message MUST be encoded as one-way notification element

>        of [RFC5277]



The reason for this more inclusive term is to permit future notification messages which allowing bundling.  This is as per adopted NETCONF draft:

draft-ietf-netconf-notification-messages



I believe there are advantages in using the more inclusive term now, rather than doing a future retrofit to this draft when notification-messages completes.   I.e., I don't see it harming anything in the specification with the expansive term.  The alternative is to change the term definition of notification message when the other draft completes, and that itself might be disruptive to implementations.



> 1.3 Solution Overview

>

>    o  Configured subscriptions can be modified by any configuration

>       client with write permission on the configuration of the

>       subscription.  Dynamic subscriptions can only be modified via an

>       RPC request made by the original subscriber.

>

>   >> what about a filter-ref that changes via another <edit-config>



The subscription parameters are not modified in that case.  So to take into account your point, I tweaked the text to:



Dynamic subscriptions can only be modified via an RPC request made by the original subscriber, or a change to configuration data referenced by the subscription.

> 1.4 Relationship to RFC 5277

>

>    o  the one way operation of [RFC5277] is still used.  However this

>       operation will no longer be required with the availability of

>       [I.D.draft-ietf-netconf-notification-messages].

>

>    >> But the new message format will be optional, so this message format

>       will remain mandatory-to-implement, right?



This is likely the best approach.  So I just removed the last sentence.



>    >> Is this a normative reference to notification-messages?



No, informational.  And now it is removed.

>   o  a publisher MAY implement both the data model and RPCs defined in

>       [RFC5277] and this new document concurrently, in order to support

>       old clients.  However the use of both alternatives on a single

>       transport session is prohibited.

>

>    >> this constraint is not mentioned in the YANG module; expect text

>      in establish-subscription:

>        "This operation MUST fail if the session is already being

>         used for a RFC 5277 subscription via <create-subscription>".



As the constraint is specific to NETCONF, there is relevant text in a section within draft-ietf-netconf-netconf-event-notifications:



4.  Compatibility with RFC-5277's create-subscription



   A publisher is allowed to concurrently support configured

   subscriptions and dynamic subscription RPCs of

   [I-D.draft-ietf-netconf-subscribed-notifications] at the same time as

   [RFC5277]'s "create-subscription" RPC.  However a single NETCONF

   transport session MUST NOT support both this specification and a

   subscription established by [RFC5277]'s "create-subscription" RPC.

   To protect against any attempts to use a single NETCONF transport

   session in this way:



   o  A solution must reply with the [RFC6241] error "operation-not-

      supported" if a "create-subscription" RPC is received on a NETCONF

      session where any other

      [I-D.draft-ietf-netconf-subscribed-notifications] or [RFC5277]

      subscription exists.

   o  It is a prohibited to send updates or state change notifications

      for a configured subscription on a NETCONF session where the

      create-subscription RPC has successfully [RFC5277] created

      subscription.

   o  A "create-subscription" RPC MUST be rejected if any

      [I-D.draft-ietf-netconf-subscribed-notifications] or

      [RFC5277]subscription is active across that NETCONF transport

      session.



Let me know if this doesn’t cover your concern.



> 2.1: Event Streams

>

>    It is out of the scope of this document

>    to identify a) how streams are defined, b) how event records are

>    defined/generated, and c) how event records are assigned to streams.

>

>    >> Not really true for the NETCONF stream



Now says:



a) how streams are defined (other than the NETCONF stream),



>    YANG notifications

>    >> term not defined



Made it simply "notifications", as the RFC 7950 is always close by, context seems sufficient.

>    Beyond the NETCONF stream, implementations are free to add additional

>    event streams.

>

>    >> please reword so it is clear (a) a server MUST support the NETCONF

>       stream and



This mandatory is asserted in "draft-ietf-netconf-netconf-event-notifications".  See the section titled: "Mandatory XML, stream and datastore support"



As a supporting reason, IoT devices might not have need for the IoT stream



>   (b) a server MAY add additional streams



Tweaked the text to:



There is only one reserved event stream name within this document: "NETCONF".  The "NETCONF" event stream contains all NETCONF XML event record information supported by the publisher, except for where it has been explicitly indicated that this the event record MUST be excluded from the "NETCONF" stream.  The "NETCONF" stream will include individual notifications as per [RFC7950] section 7.16.  Each of these notifications will be treated as a distinct event record. Beyond the "NETCONF" stream, implementations MAY define additional event streams.



>    If access control permissions are in use to secure publisher content,

>    then for event records to be sent to a receiver, that receiver MUST

>    be allowed access to all the event records on the stream.  If

>    subscriber permissions change during the lifecycle of a subscription

>    and stream access is no longer permitted, then the subscription MUST

>    be terminated.

>

>    >> known issues with 5277 compatibility (this text will be changed)



This was discussed in another thread.  We now have event based permissions filtering.  So considering the updates proposed elsewhere, let me know if there are still concerns here.



> 2.2 Event Stream Filters

>

>    This document defines an extensible filtering mechanism.  Two

>    optional stream filtering syntaxes supported are [XPATH] and subtree

>    [RFC6241].  A filter always removes a complete event record; a subset

>    of information is never stripped from an event record.

>

>    >> s/Two/The two/



Updated



>    >> Suggest mention a filter is a boolean test on the content of an

>       event message. A 'false' result causes the event message to

>       be excluded from delivery to a receiver.



Tweaked the text to:



This document defines an extensible filtering mechanism.  The filter itself is a boolean test which is placed on the content of an event record. A 'false' filtering result causes the event message to be excluded from delivery to a receiver. A filter never results in information being stripped from within an event record prior to that event record being encapsulated within a notification message. The two optional stream filtering syntaxes supported are [XPATH] and subtree [RFC6241].



> 2.3 QoS

>

>    o  a "dependency" upon another subscription.  Notification messages

>       MUST NOT be sent prior to other notification messages containing

>       update record(s) for the referenced subscription.

>

>   >> this is unclear, and perhaps belongs in the YANG Push draft

>      instead of this draft



All QoS was moved to this draft as per Martin's comments several months ago.  As there is nothing specific to yang-push, and as QoS is an optional feature, this seems a reasonable approach.



>   >> suggest moving the normative (MUST NOT) text into sentences instead

>      of bullet 3



Removed the normative text from Bullet 3.   There is now text on dependencies below the bullets:



If a subscription has a dependency, then any buffered notification messages containing event records selected by the parent subscription SHOULD be dequeued prior to the notification messages of the dependent subscription.  If notification messages have dependencies on each other, the notification message queued the longest MUST go first. If a dependency included within an RPC references a subscription which does not exist or is no longer visible to that subscriber, that dependency may be silently removed.



>   A subscription's weighting MUST work identically

>   A subscription's dependency MUST work identically...

>

>   >>> this wording is not that clear. Can it be reworded

>   to say a server implementation MUST...



Removed the "identically" and the references to HTTP2 QoS in favor of the text proposed above.



> 2.4 Dynamic Subscription

>

>   Dynamic subscriptions are managed via RPC

>   >> should really be protocol operations, not RPC



Made it:



Dynamic subscriptions are managed via protocol operations (in the form of [RFC7950], Section 7.14 RPCs) made against targets located within the publisher.



>    These RPCs have been designed

>    extensibly so that they may be augmented for subscription targets

>    beyond event streams.

>

>    >> not sure what this means.  Can't any YANG module be augmented?



This is true.   The ‘designed extensibly’ text request was made by Martin to set the stage for the augmentations made in yang-push.



>    >> Do not agree text elsewhere about streams allows for extensions

>       that ignore streams.  An augment cannot remove the 'identifier'

>       leaf.



****



I don't understand this question.  I don't know where the draft text suggests ignoring or removing.

> 2.4.1.  Dynamic Subscription State Model

>

>   >> this figure should really be named 'Figure 1' (so other

>      figures will need to renumber)



Done

>   o  A delete or kill RPC will end the subscription.

>

>   >> write out full names for <delete-subscription> or

>      <kill-subscription> operation



Done



>   >> what about <edit-config> "delete" operation for configured

>      subscriptions?



That is discussed in the "Deleting a Configured Subscription" section later on.



> 2.4.2.  Establishing a Subscription

>

>    And it MUST be possible to support the

>    interleaving of RPC requests made on independent subscriptions.

>

>    >> should not be separate sentence;

>    >> term "independent subscriptions" is not defined or clear



Based on other's feedback, now says...



The transport selected by the subscriber to reach the publisher MUST be able to support multiple "establish-subscription" requests made within the same transport session. In addition, the transport session MUST support the pipelining of RPC requests made for different subscriptions.



> 2.4.2.1.  Replay Subscription

>

>    If the "replay-start-

>    time" contains a value that is earlier than content stored within the

>    publisher's replay buffer, then the subscription MUST be rejected,

>    and the leaf "replay-start-time-hint" MUST be set in the reply.

>

>    >> this is a significant and bad change from RFC 5277 behavior



There were several discussions on why this behavior was not desirable for some users.  E.g., see thread with Yves at:



https://www.ietf.org/mail-archive/web/netconf/current/msg12151.html



As the design pattern used is that an establish-subscription RPC must send the exact parameters accepted by the publisher, and as you cannot send back alternative parameters as part of an accepted establish-subscription, this feels like a good change.



>    >> the start-time says "send all events that you have stored

>       since this time" The server sends its oldest event and does

>       not reject the request.  This draft incorrectly interprets

>       the request as "the server MUST have an event stored at least

>       this old"



Tweaked the text to:



If the "replay-start-time" contains a value that is earlier than what a publisher's retained history supports, then the subscription MUST be rejected and the leaf "replay-start-time-hint" MUST be set in the reply.



And



If the "replay-start-time" is later the scope of time covered by the replay buffer, then the publisher MUST send a "replay-completed" notification...



>    If a "stop-time" parameter is included, it MAY also be earlier than

>    the current time and MUST be later than the "replay-start-time".  The

>    publisher MUST NOT accept a "replay-start-time" for a future time.

>

>    >> MUST be later (if the start-time) if supplied

>    >> MAY be before current time?  Inconsistent with start-time

>       MUST have events that exist



Reworded to:



If a "replay-start-time" has been provided, a "stop-time" parameter may also be included.  This "stop-time" MAY be earlier than the current time, but MUST be later than the "replay-start-time", should that object also be provided.



>    >> MUST NOT accept future start-time different than 5277, but OK

>       because that was a bad requirement



Yes



> 2.4.3.  Modifying a Subscription

>

>    Dynamic subscriptions established via RPC can only be modified via

>    RPC using the same transport session used to establish that

>    subscription.  Subscriptions created by configuration operations

>    cannot be modified via this RPC.

>

>    >> should say <modify-subscription> only accepted for the

>       session that issued <establish-subscription>

>    >> <edit-config> by anybody can modify a filter used by filter-ref





As other configuration references are included (e.g., stream) I am hoping to be more generic.   Based on feedback, I am also trying to use "configuration" rather than <edit-config>.



As a result of gathering feedback from several people, the text now says:



The "modify-subscription" operation permits changing the terms of an existing dynamic subscription established on that transport session via "establish-subscription".  Dynamic subscriptions can be modified any number of times. If the publisher accepts the requested modifications, it acknowledges success to the subscriber, then immediately starts sending event records based on the new terms.



Subscriptions created by configuration cannot be modified via this RPC.  However configuration may be used to modify objects referenced by the subscription (such as a referenced filter).



>    If the publisher accepts the requested modifications on a currently

>    suspended subscription, the subscription will immediately be resumed

>    (i.e., the modified subscription is returned to an active state.)

>    The publisher MAY immediately suspend this newly modified

>    subscription through the "subscription-suspended" notification before

>    any event records are sent.

>

>    >> Not sure why this is useful to go to active and immediately

>       back to suspended.

>    >> text in YANG module not the same:

>         A successful modify-subscription

>        will return a suspended subscription to an active state.

>

>

> 2.4.4.  Deleting a Subscription

> 2.4.5.  Killing a Subscription

>

>   >> Do not see any need for 2 RPC operations



Delete is specific not specific only to the user credentials, instead it is also specific to the transport session.   Plus  kill subscription should only be enabled for a very specific set of operational users.



So based on the potential for mis-configuration of permissions, and based delete being associated with transport session rather than permissions, it seemed more secure to do this.



>    The tree structure of "kill-subscription" is almost identical to

>    "delete-subscription", with only the name of the RPC and yang-data

>    changing.

>

>    >> just include the tree diagram and remove this text



Done



> 2.4.6.  RPC Failures

>

>   RPC error codes

>   >> this term is not used in NETCONF



Changed to:



Whenever an RPC is unsuccessful, the publisher returns relevant information as part of the RPC error response. RPC error information returned will use existing transport layer RPC structures, such as those seen with NETCONF in [RFC6241] Appendix A, or with RESTCONF in [RFC8040] Section 7.1. These structures MUST be able to encode subscription specific errors identified below and defined within this document's YANG model.  As a result of this mixture, how subscription errors are encoded within an RPC error response is transport dependent. Following are valid errors which can occur for each RPC:

>    RPC error codes returned

>    include both existing transport layer RPC error codes, such as those

>    seen with NETCONF in [RFC6241] Appendix A, as well as subscription

>    specific errors such as those defined within this document's YANG

>    model.  As a result of this mixture, how subscription errors are

>    encoded within an RPC error response is transport dependent.

>

>    >> Not sure this text is correct...

>       NETCONF and RESTCONF define transport-independent error handling.

>       Do not see any reason a YANG data model should define any new

>       error handling for these protocols. Existing <error-app-tag>

>       and <error-info> extensions should be sufficient.



Agree.



Does the tweaked text above solve your concern?





>    There are elements of the RPC error mechanism which are transport

>    independent.  Specifically, references to specific identities within

>    the YANG model MUST be returned as part of the error responses

>

>    >> conflicts with previous text:

>       The contents of the resulting RPC error response MAY

>       include one or more hints



****



I don't see the conflict.  Do you still see it existing based on the revised text?



>    >> this section is very unclear wrt to specific standard error fields

>       such as <error-tag>, <error-app-tag>, and <error-info>;



This information is included in the transport specific documents.



E.g., draft-ietf-netconf-netconf-event-notifications,  Section 8.



> 2.5.  Configured Subscriptions

>

>    o  Optional parameters to identify an egress interface, a host IP

>       address, a VRF (as defined by the network instance name within

>       [I-D.draft-ietf-rtgwg-ni-model]), or an IP address plus VRF out of

>       which notification messages are to be pushed from the publisher.

>       Where any of this info is not explicitly included, or where just

>       the VRF is provided, notification messages MUST egress the

>       publisher's default interface towards that receiver.

>

>      >> this seems like the wrong place to put normative text about

>         VRFs. Is this mandatory-to-implement for all servers?



VRF is identified as an optional feature "supports-vrf".

> 2.5.1.  Configured Subscription State Model

>

>      State model for a configured subscription.

>      >> no Figure number



Fixed



>        Second, the

>    publisher itself might itself determine that the subscription in no

>    longer supportable.  In either case, a "subscription-terminated"

>

>    >> s/might itself determine that the subscription in no/

>       might determine that the subscription is no



Updated.  (Also removed other "itself" nearby.)



>    A configured subscription's receiver MUST be moved to a suspended

>    state if there is transport connectivity between the publisher and

>    receiver, but notification messages are not being generated for that

>    receiver.  A configured subscription receiver MUST be returned to an

>    active state from the suspended state when notification messages are

>    again being generated and a receiver has successfully been sent a

>    "subscription-resumed" or a "subscription-modified".

>

>   >> This seems very implementation-specific and unclear why it provides

>      any value.  Supposed a subscription is activated for some fault events

>      that should rarely be generated.



This is about the ability to generate, rather than the actual generation.   Based on this, I changed text to:



...but notification messages are not able to be generated for that receiver...



>   >> not sure why terms like CONNECTING are all-caps in the diagram if

>      if term is lowercase in the text.



Valid state text converted to all-caps.



> 2.5.2.  Creating a Configured Subscription

>

>        In this case, when there is

>    something to transport for an active subscription, transport specific

>    call-home operations will be used to establish the connection.  When

>

>    >> is this normative or is callhome optional-to-implement?



A specific call home mechanism is normative in each transport document.  But configured subscriptions themselves are optional to implement.



>    With active configured subscriptions, it is allowable to buffer event

>    records even after a "subscription-started" has been sent.  However

>    if events are lost (rather than just delayed) due to replay buffer

>    overflow, a new "subscription-started" must be sent.  This new

>    "subscription-started" indicates an event record discontinuity.

>

>   >> this is confusing to send multiple "subscription-started" events.



This gets to the promise theory basis of the draft.  You must tell the user when you have violated the promise that you gave.   Coming up with an alternative notification for such a discontinuity is of course technically possible.  But a client doesn't really care if the subscription process restarted, if there was buffer overflow on the publisher, or if something else happened.  They care to know there was a discontinuity, and “subscription-started” identifies that.  (More below on this.)



>    To see an example at subscription creation using configuration

>    operations over NETCONF, see Appendix A of

>    [I-D.draft-ietf-netconf-netconf-event-notifications].

>

>    >> IMO the examples should be moved to this draft



****



But that makes this draft very NETCONF specific.  As this isn't normative text, is such a move absolutely required?



> 2.5.3.  Modifying a Configured Subscription

>

>        If a receiver is removed, the

>    state change notification "subscription-terminated" is sent to that

>    receiver if that receiver is "active" or "suspended" .

>

>    >> how are events sent to suspended receivers? They have to go through

>      the state machine from CONNECTING -> ACTIVE (or TIMEOUT) first?



This is correct, events record are only sent to an ACTIVE receiver.  But state change notifications do not contain event records.  So I added the sentence:


Event records are only sent to ACTIVE receivers.



>      Seems there should be no requirement to ever send an event

>      to a suspended receiver. Should be OK to terminate receiver and drop

>      the transport session



Agree



> 2.5.4.  Deleting a Configured Subscription

>

>    Immediately after a subscription is successfully deleted, the

>    publisher sends to all receivers of that subscription a state change

>    notification stating the subscription has ended (i.e., "subscription-

>    terminated").

>

>    >> should only be for active receivers. For suspended receivers,

>       just drop the transport session



It is possible to have multiple configured subscriptions to a receiver, so you cannot drop the transport on delete.

> 2.5.5.  Resetting a Configured Receiver

>

>    It is possible that a configured subscription to a receiver needs to

>    be reset.  This re-initialization may be useful in cases where a

>    publisher has timed out trying to reach a receiver.  When such a

>    reset occurs, a transport session will be initiated if necessary, and

>    a new "subscription-started" notification will be sent.

>

>    >> this section does not define how a configuration is "reset".

>       There is no such protocol operation in NETCONF or RESTCONF.

>       Should mention the "reset" action in the YANG module here



Text now says:



It is possible that a configured subscription to a receiver needs to be reset.  This is accomplished via the "reset" action within the YANG model at "/subscriptions/subscription/receivers/receiver/reset". This re-initialization may be useful in cases where a publisher has timed out trying to reach a receiver. When such a reset occurs, a transport session will be initiated if necessary, and a new "subscription-started" notification will be sent.



> 2.6.  Event Record Delivery

>

>   s/RPC operations/protocol operations/



Now says:



Whether dynamic or configured, once a subscription has been set up, the publisher streams event records via notification messages per the terms of the subscription.



>       In all

>    cases, a single transport session MUST be able to support the

>    interleaving of event records, RPCs, and state change notifications

>    from independent subscriptions.

>

>    >> this is a significant change from RFC 5277, and should be noted

>       in sec 1.4



In Section 1.4, placed the bullet:



unlike [RFC5277], this document enables a single transport session to intermix of notification messages and RPCs for different subscriptions.



>    >> this text is confusing. The receiver would never get <rpc>,

>       just <notification> (for configured subscriptions). The

>       <rpc-reply> message (dynamic subscriptions only) is only

>       sent to 1 receiver (the client that sent <rpc>)



Text proposed below.   This is mostly about capability to support both



>   >> why distinguish between event records and state change notifications?

>      seems the receiver has to process every <notification> sent to it



Dynamic subscriptions are mandatory.  Based on this, simplified to:



In all cases, a single transport session MUST be capable of supporting the intermixing of RPCs and notifications from different subscriptions.



>    A notification message is sent to a receiver when an event record is

>    able to traverse the specified filter criteria.  This notification

>

>    >> traverse is odd terminology; expect "when the event record

>       is selected by...



Changed to:



A notification message is sent to a receiver when an event record is not blocked by either the specified filter criteria or receiver permissions.

>       A subscription's events MUST NOT be sent to a receiver

>    until after a corresponding RPC response or state-change notification

>    has been passed receiver indicating that events should be expected.

>

>    >> sentence is mangled; even after fixing that, not clear

>      this text is needed because already stated elsewhere.



Agree --un-mangled it:

A subscription's events MUST NOT be sent to a receiver until after a corresponding RPC response (in the case of a dynamic subscription) or state-change notification (in the case of a configured subscription) has been passed receiver.



As for needing the text, I agree that the same idea is expressed elsewhere.  What is not expressed elsewhere is the normative "MUST NOT".  If people want to remove the sentence, that is fine, but I figured that the normative statement in this section might turn out to be useful to some.





>    >> Think you mean that <subscription-modified> is delivered

>      to receivers immediately after the last event before modification

>      (ie. event placed in FILO queue) and before any events

>      after modification



Updated the Section 2.7.2 text to explicitly cover this text.





> 2.7.  Subscription State Notifications

>

>    >> it should be clear if state change events are saved in the replay

>      buffer; IMO they should be included



Elsewhere I have explicitly added that they are *not* included.   They cannot be included because state notifications are receiver specific.



> 2.7.1.  subscription-started

> 2.7.2.  subscription-modified

>

>   >> what is the value of returning all the input or configuration

>     parameters in these notifications?   For a dynamic subscription,

>     the only receiver just sent that info and does not need it.



It is possible that you could only send the contents of the leafref for dynamic subscriptions.  However this introduces complexity, as you have either must have a new notification type or a different expectation of what would be populated with a dynamic subscription.



>     For a configured subscription, that data can be read from

>     the configuration datastore.



This assumes that the receiver can do a read.  For IoT, this might not be the case.



Beyond that, if the parameters change multiple times, you might not have done such a read in time to know the parameters in process during a transient period.



Finally, a receiver might have lost state, so why not refresh the full set?  There is little cost to refreshing the full view of the subscription.



>   >> the subscription-modified event can be sent if the underlying

>      filter of a filter-ref changes; there will not appear to

>      be any parameter change at all;



****



The contents of the filter provided by reference are available in the notification.   Both the subscription-started and subscription modified populated in this notification. See this in the YANG model:



      refine "target/stream/stream-filter/within-subscription" {

        description

          "Filter applied to the subscription.  If the

          'stream-filter-ref' is populated, the filter within the

          subscription came from the 'filters' container.  Otherwise it

          is populated in-line as part of the subscription.";

      }



Of course it is possible to not overload the filter objects in the notification.   But the current model is structure this way for several reasons:



In an earlier version of the YANG model we included the full referenced filter container.   However this added lots of complexity:

- needs new top level containers within two notification (filter and subscription).   This seems excessive for the case.



By overloading, the receiver gets everything it needs from one object, making interpretation simpler



We of course could go to the more verbose tree structure if necessary.  It just adds complexity.



>   >> Suggest that filter-ref-change be an explicit flag within

>      the subscription-modified event



We can do that, but it seems better to restructure the notification per above so that we don't have a different behavior from all the other objects in the notification.   (i.e., Comparison to the previous parameters is possible for anything, without requiring another get.)

> 2.7.3.  subscription-terminated

>

>       Identities within the YANG model

>    corresponding to such a loss include: "filter-unavailable", "no-such-

>    subscription", and "stream-unavailable".

>

>    >> text is confusing: do you mean these things were valid

>      when the subscription was activated, but became invalid later?



Made explicit via the following text:



2.7.3.  subscription-terminated



   A publisher MAY terminate pushing subscribed event records to a

   receiver.  This notification indicates that no further notification

   messages should be expected from the publisher.  A publisher may

   terminate a subscription for the following reasons:



   1.  Configuration which removes a configured subscription, or a

       "kill-subscription" RPC.  These are identified via the reason

       "no-such-subscription".



   2.  A referenced filter is no longer accessible.  This is identified

       by "filter-unavailable".



   3.  The stream referenced by a subscription is no longer accessible

       by the receiver.  This is identified by "stream-unavailable".



   4.  A suspended subscription has exceeded some timeout.  This is

       identified by "suspension-timeout".



   Each of the reasons above correspond one-to-one with a "reason"

   identityref specified within the YANG model.





>    >> for configured subscriptions, seems more intuitive to reject

>      <edit-config> that contains filter-ref to unknown filter

>      (for example)



That should be the desired behavior.  Hopefully the text above shows there was no attempt to specify otherwise.



>    Note: subscribers themselves can terminate existing subscriptions

>    established via a "delete-subscription" RPC.  In such cases, no

>    "subscription-terminated" state change notifications are sent.

>    However if a "kill-subscription" RPC is sent, or some other event

>    other than reaching the subscription's stop time results in the end

>    of a subscription, then this state change notification MUST be sent.

>

>    >> this text is confusing; seems to contradict text in sec. 2.5.1

>    about sending "subscription-terminated"



Made the text less confusing by saying:



Note: this state change notification MUST be sent to a dynamic subscription's receiver when the "kill-subscription" RPC is successful, or other event other than reaching the subscription's stop time results in the end of a subscription.



> 2.7.4.  subscription-suspended

>

>    The tree structure of "subscription-suspended" is almost identical to

>    "subscription-terminated", with only the name of the notification

>    changing.

>    >> just include the tree diagram and remove this text



Done



> 2.7.7.  replay-completed

>

>    This notification indicates that all of the event records prior to

>    the current time have been sent.  This includes new event records

>    generated since the start of the subscription.  This notification

>    MUST NOT be sent for any other reason.

>

>    >> this text (prior to the current time) is confusing

>      The RFC 5277 definition was more clear and different behavior.

>      The replay-completed is sent before an event with a timestamp

>      later than (1) stop-time or (2) subscription start time.

>      So this event is before the "new event records", not after.



Adopted this text:



This notification indicates that all of the event records prior to the current time have been passed to a receiver. It is sent before any notification message containing an event record with a timestamp later than (1) the "stop-time" or (2) the subscription's start time.



>    The tree structure of "replay-completed" is almost identical to

>    "subscription-resumed", with only the name of the notification

>    changing.

>    >> include tree diagram and remove this text



Done



> 2.9.  Advertisement

>

>   >> this section mentions some YANG features but not all



All are now listed.



> 3.3.  Subscriptions Container

>

>   +--ro subscriptions

>      +--ro subscription* [identifier]

>         +--ro identifier                       subscription-id

>

>   >> tree diagram shows read-only but data nodes are config=true



Fixed.  (I found better tooling.)



> 4.  Data Model

>

> 4A) message encoding

>

>   feature encode-json {

>     description

>       "This feature indicates that JSON encoding of notification

>        messages is supported.";

>   }

>

>   feature encode-xml {

>     description

>       "This feature indicates that XML encoding of notification

>        messages is supported.";

>   }

>

>   identity encodings {

>     description

>       "Base identity to represent data encodings";

>   }

>

>   identity encode-xml {

>     base encodings;

>     if-feature "encode-xml";

>     description

>       "Encode data using XML";

>   }

>

>   identity encode-json {

>     base encodings;

>     if-feature "encode-json";

>     description

>       "Encode data using JSON";

>   }

>

>   typedef encoding {

>     type identityref {

>       base encodings;

>     }

>     description

>       "Specifies a data encoding, e.g. for a data subscription.";

>   }

>

>     leaf encoding {

>       type encoding;

>       mandatory true;

>       description

>         "The type of encoding for the subscribed data.";

>     }

>

>   >> IMO all YANG definitions related to message encoding should

>      be removed because they are in conflict with existing protocols.

>      NETCONF defines XML encoding. HTTP already defines

>      media type handling for message encoding (Accept, Content-Type)

>      There is no definition how to use JSON with NETCONF.



****



It is true that it is possible to populate unsupported mixtures of protocol and encoding.  However:

(a) for configured subscriptions, we must be able to select different encodings for a single type of transport

(b) checking what is an invalid/unsupported combination for a platform is quite easy



While it is possible to build a structure which enforces valid combinations with YANG, this would add complexity, especially as vendor custom encodings will also become new identities under the base encoding.   If there is some YANG structure which exists for such enforcement of protocol and encoding (which would be something likely common with other solutions), do you have a link?

>  4B) extension

>

>   extension subscription-state-notification {

>     description

>       "This statement applies only to notifications. It indicates that

>        the notification is a subscription state notification. Therefore

>        it does not participate in a regular event stream and does not

>        need to be specifically subscribed to in order to be received.

>        This statement can only occur as a substatement to the YANG

>        'notification' statement.";

>   }

>

>   >> not really clear what it means for other modules to use this

>      extension



It means, that the notification would not placed into an event stream, and it may be sent to a receiver without an explicit subscription.  (If other drafts extend the state machine for subscriptions, this could be a reasonable use of the extension.)



>    not clear how a client knows about other event types

>      sent as subscription state, but not defined in this document



I think that would be up to the other documents to define.  And the receiver to understand.



In any case -- in this draft (or another draft) without the proposed extension, it would be necessary to hardcode notification processing so that each of these don't go onto the NETCONF stream

>   >> All tool implementations MAY ignore any extension, which

>      includes a notification receiver; implies a receiver MUST

>      accept (and discard) event types that it does not expect.



This is desired behavior.



> 4C) identities for error handling

>

>   >> need to specify how these identities are used

>      with <rpc-error> fields.



This is done in the transport documents.



>   >> it needs to be clear that protocol and YANG error handling will

>      be processed before any application-level error handling.

>      Any YANG validation errors will cause the initial <edit-config>

>      to fail. In this case, error responses will conform to the

>      protocol or YANG error handling procedures.



Agree.   Added the following to lead off the RPC errors section...



Whenever an RPC is unsuccessful, the publisher returns relevant information as part of the RPC error response.  Transport level error processing MUST be done before RPC error processing described in this section.  In all cases, RPC error information returned will use existing transport layer RPC structures, such a...



> 4D) references

>

>         "RFC-7540, section 5.3.1";

>

>      >> the NETMOD WG decided all references need to have the document

>         title included



Removed the reference to HTTP2.



> 4E) dependency leaf

>

>     leaf dependency {

>       if-feature "qos";

>       type subscription-id;

>       description

>         "Provides the Subscription ID of a parent subscription which

>          has absolute priority should that parent have push updates

>          ready to egress the publisher. In other words, there should be

>          no streaming of objects from the current subscription if

>          the parent has something ready to push.";

>

>    >> it is unclear how message delivery on individual subscriptions

>       is supposed to be implemented to meet the requirements in the

>       description-stmt.  The purpose of this leaf is not clear either.

>       A tree of subscriptions is not described anywhere.

>       No examples are provided which show how parent and dependent

>       subscriptions are used together



Changed the text away from a reference to HTTP2, to a functional description in the QoS section:



If a subscription has a dependency, then any buffered notification messages containing event records selected by the parent subscription SHOULD be dequeued prior to the notification messages of the dependent subscription.  If notification messages have dependencies on each other, the notification message queued the longest MUST go first.



> 4F) groupings

>

>    >> these grouping do not look reusable outside this module;

>       they make the module extra difficult to read, especially with

>       extensive use of 'refine' and 'augment' statements.



Understand.   Some specific fixes in next comment.   Stepping back, in general, we as YANG module author had a choice, either to repeat object definitions, or structure the groupings, augmentations, and refinements this way.



> An example is "notification-origin-info" and "receiver-info".



Pre-NMDA, the two grouping initially were reused.   With the NMDA structure, '-config' went away, and now these groupings are not reused.



As a result, I have removed these two groupings, an placed them in the object model.  You can see the result in the git version of this revised draft.   This version is linked above.



> 4G) stop-time

>

>     leaf stop-time {

>       type yang:date-and-time;

>       description

>         "Identifies a time after which notification messages for a

>         subscription should not be sent.  If stop-time is not present,

>         the notification messages will continue until the subscription

>         is terminated.  If replay-start-time exists, stop-time must be

>         for a subsequent time. If replay-start-time doesn't exist,

>         stop-time must be for a future time.";

>     }

>

>     >> sec 2.4.2.1 says:

>        If a "stop-time" parameter is included, it MAY also be earlier than

>        the current time and MUST be later than the "replay-start-time".



I have updated the text in.2.4.2.1 to say: 'than the "replay-start-time", should that object also be provided.'



>     >> this conflicts with:

>        If replay-start-time doesn't exist, stop-time must be for a future

>        time.



Fixed with the comment above.



>     >> what does "future time" mean for configured subscriptions?

>        Seems really broken to configure a stop-time, which is an

>        absolute data/time, not relative to any other point in time



Agree that it doesn't feel critical.  But it is feasible.



We could prohibit it in the model by making the node config-false.  But perhaps somebody wants to set up a shut-off time.



So should we restrict things to explicitly exclude that possibility?  I don’t see a reason why an implementation can easily enforce this should it chose to.   But if you really don't like such application flexibility, we likely should just go with config-false.



> 4H) protocol + receivers

>

>    >> receiver list contains an address and port



Based on WG feedback at IETF 101, this will not be keyed on "name" not address and port



>       assume the "protocol" field applies to all receivers

>       type transport = (netconf, http2, http1,1)



This is correct.  (Although note that per Martin's comments, protocol was renamed to transport.)



>       *** Do not see how this is interoperable



****



In draft-ietf-netconf-netconf-event-notifications, I have tweaked the text to:



In cases where a configured subscription has a receiver in the "CONNECTING" state as described in [draft-ietf-netconf-subscribed-notifications], section 2.6.1, and the "transport" for that subscription equals "NETCONF", but no NETCONF transport session exists to that receiver (or all existing NETCONF transport sessions are currently supporting [RFC5277] subscriptions), then the publisher MUST initiate a transport session via NETCONF call home ["RFC8071"], section 4.1 to that receiver.



>       *** No detailed procedures defined to establish the transport

>           session for any of these protocols

>       *** Not clear how callhome is used, eg. which one (SSH or TLS)

>       *** Not clear if NETCONF 1.0 or NETCONF 1.1 is used

>    >> there does not seem to be any mandatory-to-implement transport

>       so nothing a client can rely on



These details are in draft-ietf-netconf-netconf-event-notifications, and must be in other transport documents.  So when the subscriber initiates the session, it would have what is needed.



> 4I) lack of mandatory filtering

>

>       anydata stream-subtree-filter {

>         if-feature "subtree";

>

>    >> The subtree filtering was mandatory-to-implement in RFC 5277

>        This should be mandatory in this module.

>        The feature "subtree" should be removed



draft-ietf-netconf-netconf-event-notifications, section 5 defines XML as mandatory.  So we should be ok.



A driver here is IoT is interested in subscriptions to streams with no filter.



>    >> If not done, then list this difference in sec. 1.4



Per above, I think we are covered.



> 4J) source-interface

>

>   >> this object is mandatory-to-implement for configured subscriptions

>      Not clear that all servers have capability to select a

>      source-interface from the application level.  Suggest a feature

>      for this data node so it is optional



This makes sense.  Done.



  feature interface-designation {

    description

      " This feature indicates a publisher supports sourcing all receiver interactions for a configured subscription from a single designated egress interface.";

  }



> 4K) modify-subscription/subscription-id

>

>       leaf identifier {

>         type subscription-id;

>         description

>           "Identifier to use for this subscription.";

>       }

>    >> should have "mandatory true"



Fixed



>    >> description should say "subscription to modify"

>       looks like establish-subscription cut-and-paste



Updated the text to:



This RPC allows a subscriber to modify a dynamic subscription's parameters.  If successful, the changed subscription parameters remain in effect for the duration of the subscription, until the subscription is again modified, or until the subscription is terminated.



> 4L) <edit-config> delete on /subscriptions or /subscriptions/subscription

>

>   >> no mention of any requirements to send a <subscription-terminated>

>   if a configured subscription is deleted by a client



Updated



    list subscription {

      description

           ...

         If configuration operations or the 'kill-subscription' rpc are

         used to delete a subscription, a 'subscription-terminated'

         message is sent to any ACTIVE or SUSPENDED receivers.";



> 4M) dependency leaf

>

>   >> what happens if the subscription-id is invalid at configure time?

>      Is the edit-config rejected?



Added text to leaf "dependency":



                              If a dependency is asserted via configuration or via RPC, but

                               the referenced subscription-id does not exist, the dependency

                              is silently discarded.  If a referenced subscription is deleted

                              this dependency is removed.";



>   >> what happens to this subscription if the referenced

>      subscription is deleted?



Text is above



>   >> what happens if there are loops configured

>      (e.g, dependency.1 = 2; dependency.2 = 3; dependency.3 = 1)



Text in the QoS section defines this (described above).  Basically the oldest goes first.



> 4N) reset action + state=connecting

>

>               enum connecting {

>                 value 3;

>                 if-feature "configured";

>                 description

>                   "A subscription has been configured, but a

>                   subscription-started state change notification needs

>                   to be successfully received before notification

>                   messages are sent.";

>               }

>

>   >> The text for connecting state does not account for the <reset>

>      action.





Now says:

               If the 'reset' action is invoked for a receiver of an

               active configured subscription, the state must be

               moved to connecting.



Also tweaked the 2.5.1 state diagram text a little:



In addition, a configured subscription's receiver MUST be moved to CONNECTING if transport connectivity cannot be achieved, or if the receiver is reset via the "reset" configuration action (3), (4).



>   >> what happens to any notifications pending for the receiver if

>      the reset action is invoked?



Right now it is undefined.   I suspect we could force any buffered event records to be purged. But that will be hard, especially when we get into bundling.



>   >> the subscription-started is not a receiver-specific event.



It is a receiver specific event.   A design choice was that there is no visibility into the fact that peers for this subscription exist.   A result is simplified interactions from the perspective of the receiver.



>       A new event called <receiver-reset> should be sent instead

>       of <subscription-started> when this action is invoked



****



Per above, it is far simpler if this is not the case as the state machine would then need to split into multiple types of connecting states.   If you really want to expose a reset, we could send a subscription-terminated with a new error identity/reason for this (or a new event for subscription-reset), both of paths could safely then proceed to subscription-started).    But I don't think this is needed.



>   >> what happens if the state=suspended?  The server is forced

>      to activate again? And then go immediately from "connecting"

>      to "suspended" state?



Yes.  If you reset a suspended subscription, it will follow the path in 2.5.1.  Which even lets the receiver know that reset attempts are being made.

> 4O) reset action / output

>

>             output {

>               leaf time {

>                 type yang:date-and-time;

>                 mandatory true;

>                 description

>                   "Time a publisher returned the receiver to a

>                   connecting state.";

>               }

>

>   >> I do not see what value this output parameter has to the client

>      The client will not get this info until the response is received,

>      and then it knows the receiver has already been set to connecting

>      state.  What problem does this output leaf solve?



We can get rid of it.   It just provides the system effective time to the entity which performed the reset.  And this entity might be different from the receiver.  It doesn’t seem to hurt, and could help.

> 4P) configured-subscription-state

>

>

>           enum invalid {

>             value 2;

>             description

>               "The subscription as a whole is unsupportable with its

>               current parameters.";

>           }

>

>   >> it is not clear which specific conditions will cause the edit

>      to be accepted, but immediately put a configured subscription

>      into "invalid" state



You appear to be arguing at an edit-config should be rejected if syntactic and semantic checks pass, but other publisher determined constraints fail.  This could be done (see picture below), but I don’t think that is optimal.



The biggest reason to do this is peer visibility.  If you put in a syntactically and semantically correct subscription, but can't be installed for reasons such a temporary performance considerations, lots of receivers might care to do a read to see what is going on with that subscription.  If it is summarily rejected, that is not possible.  Nor can the publisher automatically transition to active should constraints ease.  Also without the visibility of an installed inactive subscription, you will be depending on trouble logs.   Finally this way, receivers only need to look to one place to see if a subscription was configured.



If we wanted to do it the other way it would be:



.........

: start :-create->[evaluate]

:.......:           |    |

     ^               no   |

     '---------------'    |

                          |

  .-----------------------'

  |           .---modify-----.----------------------------------.

  |           |              |                                  |

  |           V          .-------.         .......         .---------.

  .----[evaluate]--no--->|INVALID|-delete->: end :<-delete-|CONCLUDED|

  |                      '-------'         :.....:         '---------'

  |----[evaluate]--no-.      ^                ^                 ^

  |        ^          |      |                |                 |

yes       |          '->unsupportable      delete           stop-time

  |      modify         (subscription-   (subscription-   (subscription-

  |        |             terminated*)     terminated*)      concluded*)

  |        |                 |                |                 |

  |       (1)               (2)              (3)               (4)

  |   .---------------------------------------------------------------.

  '-->|                         VALID                                 |

      '---------------------------------------------------------------'



Which can’t support the points I try to make above.



>   >> this draft should explain in detail which conditions can occur

>      after the subscription was initially configured that will

>      cause this state variable to be set to "invalid"



Added the text to section 2.5.1:



Second, the publisher might determine that the subscription is no longer supportable.  This could be for reasons of an unexpected but sustained increase in stream events, degraded CPU capacity, a more complex referenced filter, or other higher priority subscriptions which have usurped resources. See (2) in the diagram.





Thanks again for the superb review.  There are lots of improvements already included per above.  And it is genuinely appreciated.

Eric