Subject: [Netconf] tRE: Martin's thoughts on subscribed-notifications

From: "Eric Voit (evoit)" <[email-masked]>
Date: Thu, 16 Nov 2017 00:25:33 +0000

Hi Martin,

Thanks for the suggestions.   In-line...

> From: Martin Bjorklund, November 15, 2017 2:33 PM
> 
> Hi,
> 
> I have reviewed draft-ietf-netconf-subscribed-notifications-07,
> specifically to ensure that my earlier comments are addressed.  Here are my
> new comments:
> 
> 
> 
> o  General
> 
>   The document uses the term "RPC" to refer to the operations that
>   affect dynamic subscriptions.  I think that this term is not quite
>   correct.  For example the text says:
> 
>       a configured subscription cannot be modified or deleted using
>       RPC.
> 
>   But this is not correct, I can delete a configured subscription with
>   the <edit-config> rpc.

Will append the clarification:
"using RPCs from the document."

> o  Section 1.2
> 
>   What is "RPC subscription state signaling messages"?

Will change "signaling messages" to  "Notifications".   In this case the notifications act as signaling messages.

> o  Section 2.2
> 
>      The subsets of these filtering syntaxes supported are
>      left to each implementation.
> 
>    Juergen has already pointed out that this is not good design - how
>    will the client know which subset of subtree filtering a certain
>    device supports?   I think that if the server doesn't support
>    subtree filtering as defined, then it should not advertise the
>    "subtree" feature.  It can always advertise another feature.

Agree,  that was always the intent.  The words just need to match to that.  How about:
" If [RFC6241] filtering is supported, the full set of Section 6 must be enabled.  If XPATH filtering is supported, the subset of XPATH functionality is left to the implementation "

> o  Section 2.3
> 
>   The state diagram shows an arrow "modify" from "suspended" to
>   "active".  Is this really correct?  Will a suspended subscription be
>   resumed if it is modified?  

Yes. 
The subscription could immediately be suspended, but this reduces the available knobs.
 
> If so, this is not correct:
>     o  Suspend and resume state changes are driven by internal process
>        and prioritization.  There are no external controls over suspend
>        and resume.

How about: "there are no direct controls over suspend and resume other than modifying a subscription."  

Right before Figure 2, I will also add the sentence to clarify:
Modification of a configured subscription will place any suspended receivers into the connecting receiver state.

> o  Section 3
> 
>   The data model is not yet converted to NMDA single tree (and there
>   is no issue that tracks this).

We have an umbrella issue for this:
YP#11: Shift to NMDA compliant structure

We authors are willing to shift to NMDA constructs (allowing side-by-side comparison) once all other issues are closed and a firm WGLC start date is set for yang push. 

> o  Section 4.1.1
> 
>      Not all streams will support replay.  Those that do MUST include
>      they do via the "replay-support" object.
> 
>   The second sentence still looks odd.  I propose:
> 
>     If a stream supports replay, the "replay-support" leaf is present
>     in the "/streams/stream" list entry for the stream.

Will make this change

> o  Section 4.4
> 
>      An operator may find subscription
>      identifiers which may be used with "kill-subscription" by searching
>      for the ip address of a receiver within the yang tree.
> 
>   s/within the yang tree/in the "subscription" list"/
> 
>   or something along those lines.

Will make this change

> o  Section 5
> 
>      o  One or more receiver IP addresses (and corresponding ports)
>         intended as the destination for notification messages for each
>         subscription.  In addition, the transport for each destination MAY
>         be defined.
> 
>    Since the transport is mandatory in the data model, the last
>    sentence should be removed.

Will make this change

> o  Section 5.1
> 
>   The text in -07 is an improvement over -05.  But there are still
>   some things that needs to be specified.
> 
>   The text needs to discuss what's supposed to happen if a transport
>   session goes down. Will the server re-establish the session?  

As there are transport specific behaviors.  For NETCONF, see Section 5.2 of draft-ietf-netconf-netconf-event-notifications.

>  Will it buffer events during this time?  

Events are buffered, if available.  If events are lost (rather than just delayed) due to the loss of transport connectivity, a new subscription-started must be sent.  This indicates the discontinuity to the receiver.

Note I will add these two sentences directly above to section 5.1 to clear up any confusion. 

On a side note: I will tweak the text of " 8.4. subscription-suspended" to 
" This indicates that a publisher has suspended the sending of event records to a receiver, and indicates the possible loss of events."
And delete the sentence from both 8.4 and 8.5 saying...
"Suspensions are notified to the subscriber..."
.
>  What happens if the call home
>   setup due to authentication issues, will it still try to
>   re-establish?

For NETCONF, see Section 5.2 of draft-ietf-netconf-netconf-event-notifications.
 
>   The new text says:
> 
>      Note that is possible to configure replay on a configured
>      subscription.  This is to allows a configured subscription to
>      exist on a system so that event records generated during boot can
>      be buffered and pushed as soon as the transport session is
>      established.
> 
>   I assume this means that the reciever must be prepared to receive
>   the same events multiple times?  (if the server reboots before the
>   replay buffer has been overwritten)

No, boot time is considered.   The proper operation is documented in the YANG model..

  notification subscription-started {
    uses subscription-policy {
      refine "target/stream/replay-start-time"
           "Indicates the time that a replay using for the streaming of
           buffered event records.  This will be populated with the most
           recent of the following: replay-log-creation-time,
           replay-log-aged-time, replay-start-time, or the most recent
           publisher boot time.";

> o  Section 9.2
>   s/yang/YANG/

Will fix

>   Also, the text says:
> 
>     In addition support for optional features: encode-xml,
>     encode-json, configured, and replay MUST also be indicated if
>     supported.
> 
>   I think you should remove this sentence; it is redundant.  If you
>   decide to keep it, write:
> 
> 
>     In addition support for optional features: "encode-xml",
>     "encode-json", "configured", "replay", "xpath", and "subtree" MUST
>     also be indicated if supported.

Will do this one.  

>   BTW, why did you rename "configured-subscriptions" to "configured"?
>   "configured" looks a bit short.

Due to the YANG tree.   The longer feature name causes make a significant number of the lines exceed the character limit for a row.   And things become very difficult to read.

> o  10 - extension
> 
>      extension subscription-state-notif {
> 
>   I think you should spell out "notification"; i.e., use
>   subscription-state-notification.
> 
>   (You replied "will do" to this comment in my previous review, so I
>   suspect you simply forgot this one)

Yes.  This time I really will do :-)

> o  10 - filter-spec
> 
>   I think both the subtree and XPath filters are not correctly
>   defined.  I provided text about the XPath filter, but you didn't
>   include it, so here it is again, updated with your text about
>   events.

Thanks for keeping on about this.  The definitions you provide are an upgrade, and I will include.
 
>       anydata subtree-filter {
>         description
> 
> 
>   OLD:
> 
>           "Event stream evaluation criteria encoded in a syntax of a
>            supported type of an RFC 6241, Section 6 filter.  The subtree
>            filter is applied to the representation of individual,
>            delineated event records as contained within the event
>            stream.  For example, if the notification message contains an
>            instance of a notification defined in YANG, then the top-
>            level element is the name of the YANG notification.  If the
>            stream filter matches an event record from the stream, the
>            event record should be included in a notification message
>            to the receiver(s).";
>       }
> 
>   NEW:
> 
>           "Event stream evaluation criteria encoded in the syntax of
>            a subtree filter as defined in RFC 6241, Section 6.
> 
>            The subtree filter is applied to the representation of
>            individual, delineated event records as contained within
>            the event stream.  For example, if the notification message
>            contains an instance of a notification defined in YANG,
>            then the top-level element is the name of the YANG
>            notification.
> 
>            If the subtree filter returns a non-empty node set, the
>            filter matches the event record, and the it is included in
>            the notification message sent to the receivers.";
> 
>         reference "RFC 6241, Section 6.";
>       }
> 
>   and:
> 
>       leaf xpath-filter {
>         type yang:xpath1.0;
>         description
> 
>   OLD:
> 
>           "Event stream evaluation criteria encoded in a syntax of xpath
>            1.0 and applied against an event stream. The result of
>            applying XPath expression is converted to a boolean value
>            using the standard XPath 1.0 rules.  If the boolean value is
>            'true', the stream filter matches an event record within the
>            stream, and the notification message should be sent to the
>            receiver(s).";
> 
>   NEW:
> 
>           "Event stream evaluation criteria encoded in the syntax of
>            an XPath 1.0 expression.
> 
>            The XPath expression is evaluated on the representation of
>            individual, delineated event records as contained within
>            the event stream.  For example, if the notification message
>            contains an instance of a notification defined in YANG,
>            then the top-level element is the name of the YANG
>            notification, and the root node has this top-level element
>            as the only child.
> 
>            The result of the XPath expression is converted to a
>            boolean value using the standard XPath 1.0 rules.  If the
>            boolean value is 'true', the filter matches the the event
>            record, and the it is included in the notification message
>            sent to the receivers.
> 
>            The expression is evaluated in the following XPath context:
> 
>              o  The set of namespace declarations are those in scope on
>                 the 'xpath-filter' leaf element where th
> 
>              o  The set of variable bindings is empty.
> 
>              o  The function library is the core function library, and
>                 the XPath functions defined in section 10 in RFC 7950.
> 
>              o  The context node is the root node.
> 
>         reference
>           "http://www.w3.org/TR/1999/REC-xpath-19991116
>            RFC 7950, Section 10.";
>       }
> 
> 
> 
> o  10 - grouping notification-origin-info
> 
>   In your reply tpo my previous review, you wrote that you would
>   remove the term "push source".  It is still used.

Yes, will update.

Eric

> /martin

---

From: Martin Bjorklund <[email-masked]>
Date: Thu, 16 Nov 2017 11:10:02 +0100 (CET)

Hi,

> " If [RFC6241] filtering is supported, the full set of Section 6 must be enabled.  If XPATH filtering is supported, the subset of XPATH functionality is left to the implementation "

The same argument applies to XPath.  How would this be interoperable
otherwise?  I suggest you simply remove the sentence

> The subscription could immediately be suspended, but this reduces the available knobs.

How does it reduce the available knobs?  I was thinking that if a
modify-subscription rpc was sent to a suspended subscription, it
wouldn't by itself change the suspension status; if the server thinks
it can be resumed it will resume.

With the current solution, the server must send three notifications if
a suspended subscription is modified, and it is still suspended:

  subscription-modified
  subscription-resumed
  subscription-suspended

> We authors are willing to shift to NMDA constructs (allowing side-by-side comparison) once all other issues are closed and a firm WGLC start date is set for yang push. 

The sooner you get it in the better, since it will speed up
everything.

> As there are transport specific behaviors.  For NETCONF, see Section 5.2 of draft-ietf-netconf-netconf-event-notifications.

Ok, makes sense.  I suggest you add a sentence along the lines of:

  How transport failures are dealt with is out of scope for this
  document.  It is expected that transport-specific documents describe
  how failures are handled.

> Events are buffered, if available.  If events are lost (rather than just delayed) due to the loss of transport connectivity, a new subscription-started must be sent.  This indicates the discontinui…

I assume this should also be covered by the transport specific
documents.

>            replay-log-aged-time, replay-start-time, or the most recent

Ok.

> Due to the YANG tree.   The longer feature name causes make a significant number of the lines exceed the character limit for a row.   And things become very difficult to read.

I think we should pick descriptive names that make sense, and not
names that make the tree diagram or whatever look nice.  I suggest you
keep the original name.

/martin