Subject: Yangdoctors last call review of draft-ietf-netconf-subscribed-notifications-10

From: Andy Bierman <[email-masked]>
Date: Thu, 15 Mar 2018 15:00:51 -0700

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]

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>

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?
   >> Is this a normative reference to notification-messages?

  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>".

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

   YANG notifications
   >> term not defined

   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 (b) a server MAY add additional 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)

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

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
  >> suggest moving the normative (MUST NOT) text into sentences instead
     of bullet 3

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

2.4 Dynamic Subscription

  Dynamic subscriptions are managed via RPC
  >> should really be protocol operations, not RPC

   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?
   >> Do not agree text elsewhere about streams allows for extensions
      that ignore streams.  An augment cannot remove the 'identifier'
      leaf.

2.4.1.  Dynamic Subscription State Model

  >> this figure should really be named 'Figure 1' (so other
     figures will need to renumber)

  o  A delete or kill RPC will end the subscription.

  >> write out full names for <delete-subscription> or
     <kill-subscription> operation
  >> what about <edit-config> "delete" operation for configured
     subscriptions?

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

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

   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
   >> MUST NOT accept future start-time different than 5277, but OK
      because that was a bad requirement

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

   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

   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

2.4.6.  RPC Failures

  RPC error codes
  >> this term is not used in NETCONF

   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.

   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

   >> this section is very unclear wrt to specific standard error fields
      such as <error-tag>, <error-app-tag>, and <error-info>;

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?

2.5.1.  Configured Subscription State Model

     State model for a configured subscription.
     >> no Figure number

       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

   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.
  >> not sure why terms like CONNECTING are all-caps in the diagram if
     if term is lowercase in the text.

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?

   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.

   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

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

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

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

2.6.  Event Record Delivery

  s/RPC operations/protocol operations/

      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
   >> 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>)
  >> why distinguish between event records and state change notifications?
     seems the receiver has to process every <notification> sent to it

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

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

2.7.  Subscription State Notifications

   >> it should be clear if state change events are saved in the replay
     buffer; IMO they should be included

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.
    For a configured subscription, that data can be read from
    the configuration datastore.
  >> 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;
  >> Suggest that filter-ref-change be an explicit flag within
     the subscription-modified event

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?
   >> for configured subscriptions, seems more intuitive to reject
     <edit-config> that contains filter-ref to unknown filter
     (for example)

   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"

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

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.

   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

2.9.  Advertisement

  >> this section mentions some YANG features but not all

3.3.  Subscriptions Container

  +--ro subscriptions
     +--ro subscription* [identifier]
        +--ro identifier                       subscription-id

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

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.

 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; not clear how a client knows about other event types
     sent as subscription state, but not defined in this document

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

4C) identities for error handling

  >> need to specify how these identities are used
     with <rpc-error> fields.

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

4D) references

        "RFC-7540, section 5.3.1";

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

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

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. An example
      is "notification-origin-info" and "receiver-info".

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".
    >> this conflicts with:
       If replay-start-time doesn't exist, stop-time must be for a future
       time.
    >> 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

4H) protocol + receivers

   >> receiver list contains an address and port
      assume the "protocol" field applies to all receivers
      type transport = (netconf, http2, http1,1)
      *** Do not see how this is interoperable
      *** 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

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
   >> If not done, then list this difference in sec. 1.4

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

4K) modify-subscription/subscription-id

      leaf identifier {
        type subscription-id;
        description
          "Identifier to use for this subscription.";
      }
   >> should have "mandatory true"
   >> description should say "subscription to modify"
      looks like establish-subscription cut-and-paste

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

4M) dependency leaf

  >> what happens if the subscription-id is invalid at configure time?
     Is the edit-config rejected?
  >> what happens to this subscription if the referenced
     subscription is deleted?
  >> what happens if there are loops configured
     (e.g, dependency.1 = 2; dependency.2 = 3; dependency.3 = 1)

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.
  >> what happens to any notifications pending for the receiver if
     the reset action is invoked?
  >> the subscription-started is not a receiver-specific event.
      A new event called <receiver-reset> should be sent instead
      of <subscription-started> when this action is invoked
  >> what happens if the state=suspended?  The server is forced
     to activate again? And then go immediately from "connecting"
     to "suspended" state?

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?

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

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

---

From: Martin Bjorklund <[email-masked]>
Date: Fri, 16 Mar 2018 09:18:48 +0100 (CET)

Hi,

Andy Bierman <[email-masked]> wrote:

[...]

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

I agree, and I have pointed this out in earlier reviews.

If a client sends a too early time, and the server doesn't send the
optional hint, the client will have to guess the time.  Not very
robust.

If the motivation is that the client should be informed that he might
have missed some notifs b/c the replay-start-time is too early, this
information can be passed in the rpc-reply from establish-subscription
instead.

>       because that was a bad requirement

I have also pointed out in earlier reviews that the requirement that
the replay-start-time cannot be in the future is problematic:

  I know that this text is also present in RFC 5277, but I think it
  needs to be changed.  Which current time?  Probably the server's,
  but how would a client know that?  This is a problem that we faced
  when implementing 5277.   I think we should remove this
  requirement, since it doesn't add any value anyway.

IMO, if the server gets a replay-start-time that is later than the
latest stored notif, it will send a <replay-completed> notif, and then
move on.  This is a very simple behavior that doesn't rely on
synchronized clocks or anything like that.

[...]

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

+1

[...]

>     the configuration datastore.

I agree.  Removing these redundant parameters would also simplify the
models quite a bit.

[...]

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

+1

/martin

---

From: "Eric Voit (evoit)" <[email-masked]>
Date: Wed, 21 Mar 2018 22:17:25 +0000

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

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

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

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

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

>    >> Not really true for the NETCONF stream

Now says:

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

>    >> term not defined

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

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

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

>    >> s/Two/The two/

Updated

>       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].

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

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

>   to say a server implementation MUST...

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

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

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

>       leaf.

****

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

>      figures will need to renumber)

Done

>      <kill-subscription> operation

Done

>      subscriptions?

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

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

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

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

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

>       because that was a bad requirement

Yes

>    >> <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).

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

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

Done

>   >> this term is not used in NETCONF

Changed to:

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

Agree.

Does the tweaked text above solve your concern?

>       include one or more hints

****

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

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

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

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

>      >> no Figure number

Fixed

>       might determine that the subscription is no

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

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

>      if term is lowercase in the text.

Valid state text converted to all-caps.

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

>   >> 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.)

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

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

>      the transport session

Agree

>       just drop the transport session

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

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

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

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

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

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

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

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

>      after modification

Updated the Section 2.7.2 text to explicitly cover this text.

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

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

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

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

>      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.)

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

Made explicit via the following text:

   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 example)

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

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

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

Done

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

>    >> include tree diagram and remove this text

Done

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

All are now listed.

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

Fixed.  (I found better tooling.)

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

>      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

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

This is desired behavior.

>      with <rpc-error> fields.

This is done in the transport documents.

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

>         title included

Removed the reference to HTTP2.

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

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

>        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.'

>        time.

Fixed with the comment above.

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

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

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

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

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

>      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.";

  }

>    >> should have "mandatory true"

Fixed

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

>   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.";

>      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.";

>      subscription is deleted?

Text is above

>      (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.

>      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).

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

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

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

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

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

>      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

---

From: "Eric Voit (evoit)" <[email-masked]>
Date: Wed, 21 Mar 2018 23:49:44 +0000

Hi Martin,

> From: Martin Bjorklund, March 16, 2018 4:19 AM
> 
> Hi,
> 

In our past discussions, it looked like you were ok after reading Yves requirement here:

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

Beyond this functional requirement, the design pattern used is that an establish-subscription RPC must send the exact parameters accepted by the publisher.

> If a client sends a too early time, and the server doesn't send the optional
> hint, the client will have to guess the time.  Not very robust.
> 
> If the motivation is that the client should be informed that he might have
> missed some notifs b/c the replay-start-time is too early, this information
> can be passed in the rpc-reply from establish-subscription instead.

Yes this could be done.  But this doesn't follow the design pattern of making the client explicitly ask for what they want.  Consistent design patterns do matter.

> I have also pointed out in earlier reviews that the requirement that the
> replay-start-time cannot be in the future is problematic:

Yes.  The thinking both of you passed along drove this modification.

> IMO, if the server gets a replay-start-time that is later than the latest stored
> notif, it will send a <replay-completed> notif, and then move on.  This is a
> very simple behavior that doesn't rely on synchronized clocks or anything
> like that.

The text which I suggested back to Andy :

   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 immediately after the after a successful establish-
   subscription RPC response.

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

But those examples are transport specific.  This will make the document less applicable for other transports.

> I agree.  Removing these redundant parameters would also simplify the

I replied on the thinking here to Andy. 

Basically it is possible that you could only send the contents of the leafref for dynamic subscriptions.  However this introduces complexity, as should have a notification type and set a different expectation of what would be populated with a dynamic subscription.  So in the end, we can do it.  But it makes the model more complicated (although the tree gets smaller.)   

Also 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 on a configured subscription, you might not be quick enough to do a read in time to know the parameters during a transient period.

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

So in the end, this a complex simplification drives error cases and more variations to process for the receiver.

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

Per response to Andy:

Thanks,
Eric

---

From: Martin Bjorklund <[email-masked]>
Date: Fri, 23 Mar 2018 12:04:27 +0100 (CET)

Hi,

One comment below.

"Eric Voit (evoit)" <[email-masked]> wrote:
> The reason for this more inclusive term is to permit future
> notification messages which allowing bundling.  This is as per adopted
> NETCONF draft:
> 
> 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.

But later in this draft you state that there will be an update to this
document when the notification messages draft is done.

> I.e., I don't see it harming
> anything in the specification with the expansive term.

I think it will be confusing to readers to see the statement that this
document support bundling, then it says that notifs MUST be encoded as
5277 notifications.

I suggest you remove:

  A notification message may bundle multiple event records.  This
  includes the bundling of multiple, independent RFC 7950 YANG
  notifications.

[...]

/martin

---

From: Martin Bjorklund <[email-masked]>
Date: Fri, 23 Mar 2018 12:36:31 +0100 (CET)

Hi,

"Eric Voit (evoit)" <[email-masked]> wrote:
> Beyond this functional requirement, the design pattern used is that an
> establish-subscription RPC must send the exact parameters accepted by
> the publisher.
> 
> 
> > optional
> > have
> Yes this could be done.  But this doesn't follow the design pattern of
> making the client explicitly ask for what they want.  Consistent
> design patterns do matter.

Well, "what they want" depends on the semantics of this leaf!  If we
keep the old semantics, then if the client passes this parameter with
some start time, it "explicitly asked for what it wanted".

Anyway, if the objective is to ensure that no notifs are sent unless
the replay-start-time exactly matches the event-time of a notification
in the buffer, then we can add a parameter to ensure that.

In all cases, if the client receives a notif with a time later than
what it asked for, it knows that it might have lost some notifs.

  leaf replay-exact-start-time {
    if-feature "replay";
    when "../replay-start-time";
    type empty;
    description
      "If this parameter is present, and the server does not have any
       stored event record with 'eventTime' equal to the requested
       'replay-start-time', then the server MUST reject the request.";
  }

> > the
> > latest stored

Good.

> But those examples are transport specific.  This will make the
> document less applicable for other transports.

I don't think it will.  Just write that the example is using NETCONF
XML or RESTCONF JSON or whatever.  Compare with the RESTCONF draft, it
has plenty of examples despite the fact that it supports two
encodings.

I think having the examples in this draft is the right thing to do.

> Basically it is possible that you could only send the contents of the
> leafref for dynamic subscriptions.  However this introduces
> complexity, as should have a notification type and set a different
> expectation of what would be populated with a dynamic subscription.
> So in the end, we can do it.  But it makes the model more complicated
> (although the tree gets smaller.)
> 
> Beyond that, if the parameters change multiple times on a configured
> subscription, you might not be quick enough to do a read in time to
> know the parameters during a transient period.
> 
> Finally, a configured receiver might have lost state, so why not
> refresh the full set?  There is little cost to refreshing the full
> view of the subscription.
> 
> So in the end, this a complex simplification drives error cases and
> more variations to process for the receiver.
> 
> > [...]
> > 
> > > 4A) message encoding
> > >
> > >   feature encode-json {
> > >     description
> 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?

I replied to this issue in the other thread, so let's continue the
discussion there.

/martin

---

From: Andy Bierman <[email-masked]>
Date: Fri, 23 Mar 2018 05:04:09 -0700

On Fri, Mar 23, 2018 at 4:04 AM, Martin Bjorklund <[email-masked]> wrote:

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

There is no need for this spec to say anything about different notification
headers.
It is trivial to add a new parameter later to allow the client to request a
different message
format.

[...]
Andy

---

From: Andy Bierman <[email-masked]>
Date: Fri, 23 Mar 2018 05:07:10 -0700

>        'replay-start-time', then the server MUST reject the request.";

I would like to hear the use-case for this leaf.
The existing leaf (in 5277) supports a very common use-case, which is
a collector that connects periodically and retrieves all the events of
interest
since the last connect time.  The client has no idea when specific events
occurred on the device. It only knows its own last connect time.

Andy

> The

---

From: "Eric Voit (evoit)" <[email-masked]>
Date: Mon, 26 Mar 2018 06:09:30 +0000

From: Andy Bierman, March 23, 2018 8:04 AM

On Fri, Mar 23, 2018 at 4:04 AM, Martin Bjorklund <[email-masked]<mailto:[email-masked]>> wrote:
Hi,

One comment below.

I suggest you remove:

+1

There is no need for this spec to say anything about different notification headers.
It is trivial to add a new parameter later to allow the client to request a different message
format.

<Eric> I have removed this part of the notification message definition.

Eric

[...]

Andy

/martin

---

From: "Eric Voit (evoit)" <[email-masked]>
Date: Mon, 26 Mar 2018 06:09:31 +0000

> From: Martin Bjorklund, March 23, 2018 7:37 AM
> 
> Hi,
> 
> 
> > > If the motivation is that the client should be informed that he
> > > might have missed some notifs b/c the replay-start-time is too
> > > early, this information can be passed in the rpc-reply from
> > > establish-subscription instead.
> >
> Well, "what they want" depends on the semantics of this leaf!  If we keep
> the old semantics, then if the client passes this parameter with some start
> time, it "explicitly asked for what it wanted".
> 
> Anyway, if the objective is to ensure that no notifs are sent unless the
> replay-start-time exactly matches the event-time of a notification in the
> buffer, then we can add a parameter to ensure that.

The current definition of replay-start-time is:

"Used to trigger the replay feature and indicate that the replay should start at the time specified..."

To me, that means the replay-start-time has to be covered by the scope of the replay buffer.  It does not mean that it is required that the requested replay-start-time needs to exactly match the time of a buffered event.

> In all cases, if the client receives a notif with a time later than what it asked
> for, it knows that it might have lost some notifs.

Why would this mean it might have lost some notifs?    In the current embodiment, the replay will not start unless the subscriber asked for a time that is within the scope covered by the buffer.  I.e., a time later than both "replay-log-creation-time" and "replay-log-aged-time".

>        'replay-start-time', then the server MUST reject the request.";

Something like this parameter *might* be applicable if we choose to respond to a dynamic replay request with events later than those requested.  (i.e., in the establish-subscription success response.)    As noted in other threads, this is a legitimate way to approach the issue.  However if the WG chooses this way, this will result in an exception to the design pattern of requiring the subscriber to ask for what they are going to receive. In addition, we might end up sending a stream of information to the subscriber which is not sufficient, and therefore not verifiably relevant.

> > > latest stored notif, it will send a <replay-completed> notif, and
> > > then move on.
> > > This is a
> > > very simple behavior that doesn't rely on synchronized clocks or
> > > anything like that.
> >
> I don't think it will.  Just write that the example is using NETCONF XML or
> RESTCONF JSON or whatever.  Compare with the RESTCONF draft, it has
> plenty of examples despite the fact that it supports two encodings.
> 

For the RESTCONF draft, there isn't another natural matching transport document where the information naturally fits.  And by doing the split in this way means we can point to a fairly extensive list of examples in one place which is transport-relevant.

Eric

> > > I agree.  Removing these redundant parameters would also simplify
> > > the models quite a bit.
> >
> I replied to this issue in the other thread, so let's continue the discussion
> there.
> 
> 
> /martin

---

From: Martin Bjorklund <[email-masked]>
Date: Mon, 26 Mar 2018 09:47:58 +0200 (CEST)

"Eric Voit (evoit)" <[email-masked]> wrote:
> > keep
> To me, that means the replay-start-time has to be covered by the scope
> of the replay buffer.  It does not mean that it is required that the
> requested replay-start-time needs to exactly match the time of a
> buffered event.

The text is quite unclear:

  Used to trigger the replay feature and indicate that the
  replay should start at the time specified.

and:

  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

In lack of a clear definition, I assume that "content stored [in]
replay buffer" refers to event records, since I assume that nothing
else can be stored in the replay buffer?

Next question is what it means that a time value is earlier than
"content"?  Again, my assumption is that it means "earlier than the
'eventTime' of the event records".  Is this not what is intended?

>From what you write here though, I think that what you propose is
that:

  If "replay-start-time" is less than the latest of
  "replay-log-aged-time" and "replay-log-creation-time", then the
  request is rejected.

This must be clarified.  Also, ensure that the required behavior is
clearly defined in the YANG module, and not just in the text in the
document.

But I still think that there should be some way for the client to get
all buffered event records, just like what was supported in RFC 5277,
without extra round trips.  Note that if the system is quickly
generating notifs, the client might need many round trips before it
manages to replay anything.

> > what it asked
> Why would this mean it might have lost some notifs?  In the current
> embodiment, the replay will not start unless the subscriber asked for
> a time that is within the scope covered by the buffer.  I.e., a time
> later than both "replay-log-creation-time" and "replay-log-aged-time".

See above.  But the reason for rejection is that the client might have
lost some notifs.

> >        'replay-start-time', then the server MUST reject the request.";

If we add something like this, the leaf name and description text
needs to be tweaked for the clarified semantics of replay-start-time.

> Something like this parameter *might* be applicable if we choose to
> respond to a dynamic replay request with events later than those
> requested.  (i.e., in the establish-subscription success response.)
> As noted in other threads, this is a legitimate way to approach the
> issue.  However if the WG chooses this way, this will result in an
> exception to the design pattern of requiring the subscriber to ask for
> what they are going to receive.

I disagree.  The client explicitly asks the server to send all
buffered event records.

> In addition, we might end up sending a
> stream of information to the subscriber which is not sufficient, and
> therefore not verifiably relevant.

It is up to the client to define what is relevant.  Maybe I just want
to view the replay buffer for trouble shooting purposes.

/martin

> > XML or
> For the RESTCONF draft, there isn't another natural matching transport
> document where the information naturally fits.  And by doing the split
> in this way means we can point to a fairly extensive list of examples
> in one place which is transport-relevant.
> 
> Eric
> 
> > discussion

---

From: Martin Bjorklund <[email-masked]>
Date: Mon, 26 Mar 2018 10:25:19 +0200 (CEST)

Hi,

BTW, I noticed this:

      leaf replay-log-creation-time {
        if-feature "replay";
        type yang:date-and-time;
        description
          "The timestamp of the creation of the log used to support the
          replay function on this stream. Note that this might be
          earlier then the earliest available information contained in
          the log. This object is updated if the log resets for some
          reason. This object MUST be present if replay is supported.";
      }

I think the last sentence should be formalized, by adding:

   when "../replay-support";
   mandatory true;

/martin

Martin Bjorklund <[email-masked]> wrote:
> _______________________________________________
> Netconf mailing list
> [email-masked]
> https://www.ietf.org/mailman/listinfo/netconf
>