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

From: Martin Bjorklund <[email-masked]>
Date: Wed, 15 Nov 2017 20:33:09 +0100 (CET)

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.

o  Section 1.2

  What is "RPC subscription state 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.

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

o  Section 3

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

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.

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.

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.

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?  Will
  it buffer events during this time?  What happens if the call home
  setup due to authentication issues, will it still try to
  re-establish?

  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)

o  Section 9.2

  s/yang/YANG/

  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.

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

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)

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.

      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.

/martin

---

From: "Eric Voit (evoit)" <[email-masked]>
Date: Mon, 20 Nov 2017 19:41:37 +0000

> From: Martin Bjorklund [mailto:[email-masked]]

<snip>

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

> The same argument applies to XPath.  How would this be interoperable

> otherwise?  I suggest you simply remove the sentence

>       left to each implementation.

Removed

> > >   resumed if it is modified?

> > Yes.

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

> available knobs.

> How does it reduce the available knobs?

For dynamic subscriptions, a successful execution of a modify-subscription RPC includes an implicit resume.   This is eliminates the need for an additional "resume" message.   And this should be the most common case.   I have tweaked text to clarify this behavior as follows:

"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 status.)  The publisher MAY immediately suspend this newly modified subscription through the "subscription-suspended" notification before any event records are sent."

I have also clarified the modify-subscription RPC and subscription-resumed notification accordingly.

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

One of the goals for configured subscriptions is that the publisher does not expose information about independent receivers, or activities which occurred when the subscription is not connected.   A result of this is that it is best that a subscription-modified not go out to a currently suspended receiver.    Instead, when a modified subscription resumes for a particular receiver, the full current state of the subscription is provided if it has been modified.

As an FYI: consider that for configured subscriptions, one of the receivers might not be transport-connected when a configuration is being modified.  Even though there are multiple updates, only the last subscription-modified notification need be sent when any suspension ends.

> With the current solution, the server must send three notifications if a

> suspended subscription is modified, and it is still suspended:

>   subscription-modified

This notification is only sent for configured subscriptions

>   subscription-resumed

>   subscription-suspended

Per the text above, these three in a row should never happen.  I have updated the text to clarify as follows...

"If the modification involved changing the policies for the subscription, the publisher sends to currently active receivers a subscription-modified notification.  For any suspended receivers, a subscription-modified notification will be delayed until the subscription is resumed.  (Note: it is this same subscription-modified notification that informs the receiver that the subscription has been resumed, no additional subscription-resumed need be sent.)"

And I have tweaked the state machine to better express the relationship as follows...

                    .-------.

                    | start |

                    '-------'

                        |

                     create

                        |

                        v

        .-------------------------------------------------------------.

  .---. |  Subscription              .------------.                   |

modify \|                            | connecting |-----------.       |

  '---->|      subscription-started--| receiver   |           |       |

        |              |    .------->|            |<-------.  |       |

        |              V    |        '------------'        |  V       |

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

        |             |active  |--subscription-suspended->|suspended| |

        |subscription-|receiver|                          |receiver | |

        |  -modified  |        |<--subscription-resumed,--|         | |

        |       '---->'--------'   subscription-modified  '---------' |

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

                        |

                      delete

                        |

                        v

                    .-------.

                    |  end  |

                    '-------'

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

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

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

There is text around the transport specific behaviors, I would love feedback if you feel anything is missing.   Certainly a NETCONF session can have a subscription-started sent using a subscription-id which has been previously reported.

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

The name for "configured" is understandable in this context, is defined in the yang model, and makes tree readability easier.

Eric