Re: [Netconf] mbj's WGLC review of subscribed-notifications-16

[Martin Bjorklund <[e-mail]>]

Hi,

One problematic thing below.

Martin Bjorklund <[e-mail]> wrote:
> Hi,
> 
> Thank you for addressing my comments.  More inline.
> 
> 
> "Eric Voit (evoit)" <[e-mail]> wrote:
> > Hi Martin,
> > 
> > Much incorporated.  Latest is at:
> > https://github.com/netconf-wg/rfc5277bis/blob/master/draft-ietf-netconf-subscribed-notifications-17.txt
> > 
> > Specific responses in-line...
> > 
> > > From: Martin Bjorklund, September 12, 2018 5:35 AM
> > > 
> > > "Eric Voit (evoit)" <[e-mail]> wrote:
> > > > Thanks for the comments Martin,  thoughts in-line...
> > > >
> > > > > From: Martin Bjorklund, September 10, 2018 1:42 PM
> > > 
> > > [...]
> > > 
> > > > > o  Section 2.1
> > > > >
> > > > >   The text says:
> > > > >
> > > > >     It is out of the scope of this document
> > > > >     to identify [...] b) how event records are defined
> > > > >
> > > > >   Is this really what we want?  Shouldn't this document specify that
> > > > >   an event record is an instance of a YANG "notification" statement?
> > > > >   Do we want to support event records that are not defined in YANG?
> > > > >
> > > > >   This was discussed in the thread "comments on
> > > > >   draft-ietf-netconf-subscribed-notifications-12", and it seems that
> > > > >   noone really have any use case for non-YANG-defined event records,
> > > > >   accessed with YANG-defined operations.
> > > >
> > > > Yes this was discussed previously.  My reading was that others wanted
> > > > to support subscription to non-YANG events.
> > > > E.g., see Andy & Tianran's thoughts in
> > > > https://www.ietf.org/mail-archive/web/netconf/current/msg14764.html
> > > 
> > > Yes, but read on in that thread, see
> > > https://www.ietf.org/mail-archive/web/netconf/current/msg14766.html
> > > for a clarification.
> > > 
> > > AFAIK, noone has presented a use case for sending non-YANG-defined
> > > data
> > > here.
> > 
> > As anything can be YANG wrapped, per the thread above.  I am fine with
> > this.
> > 
> > > > >   I think that this document should state that an event record is an
> > > > >   instance of a YANG "notification" statement.  If not, it is unclear
> > > > >   both how subtree/xpath filters and xml/json/... encodings would work.
> > > >
> > > > Applying xpath filters against XML based event records seems viable to
> > > > me.
> > > 
> > > Where does it say that it has to be XML?  Whatabout mixed-content?
> > > What if such a notification is sent when the encoding is JSON?
> > > 
> > > If you want to make this specification open for other data modeling
> > > languages,
> > > I think you need to carefully define how that is supposed to work;
> > > essentially
> > > similar to how we handle transports; we have a set of requirement on
> > > what a
> > > transport document must define.  IMO this is super complicated and
> > > overkill.
> > > This draft is complex enough.
> > > Don't add even more complexity.
> > 
> > If you are just saying that we can put everything into a YANG wrapper,
> > that is fine with me.  To clarify this, I have tweaked the first
> > sentence of Section 2.1 to:
> > "An event stream is a named entity on a publisher which exposes a
> > continuously updating set of YANG encoded event records."
> 
> Howabout:
> 
> OLD:
> 
>    An event stream is a named entity on a publisher which exposes a
>    continuously updating set of event records.  Each event stream is
>    available for subscription.  It is out of the scope of this document
>    to identify a) how streams are defined (other than the NETCONF
>    stream), b) how event records are defined/generated, and c) how event
>    records are assigned to streams.
> 
> NEW:
> 
>    An event stream is a named entity on a publisher which exposes a
>    continuously updating set of event records.  An event record is
>    defined with the "notification" YANG statement.  Each event stream is
>    available for subscription.  It is out of the scope of this document
>    to identify a) how streams are defined (other than the NETCONF
>    stream), b) how event records are generated, and c) how event
>    records are assigned to streams.
> 
> > And I have tweaked the first sentence of the third paragraph to:
> > "As YANG encoded event records are created by a system,..."
> 
> I think the original sentence is better.
> 
> Also, some tweaks are needed to the filter leafs in the YANG module:
> 
> OLD:
> 
>           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.
> 
> NEW:
> 
>           The subtree filter is applied to the representation of
>           individual, delineated event records as contained within the
>           event stream.  The top-level element in the event record is
>           the name of the YANG notification.
> 
> and
> 
> OLD:
> 
>            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.
> 
> NEW:
> 
> 
>            The XPath expression is evaluated on the representation of
>            individual, delineated event records as contained within
>            the event stream.  The top-level element in the event
>            record is the name of the YANG notification, and the root
>            node has this top-level element as the only child.


This text is problematic b/c it doesn't work well with inline
notifications.

Consider the example in section 7.16.3 in RFC 7950:

       container interfaces {
         list interface {
           key "name";
           leaf name {
             type string;
           }
           notification interface-enabled {
             leaf by-user {
               type string;
             }
           }
         }
       }
       
and corresponding XML example:

     <notification
       xmlns="urn:ietf:params:xml:ns:netconf:notification:1.0">
       <eventTime>2008-07-08T00:01:00Z</eventTime>
       <interfaces xmlns="urn:example:interface-module">
         <interface>
           <name>eth1</name>
           <interface-enabled>
             <by-user>fred</by-user>
           </interface-enabled>
         </interface>
       </interfaces>
     </notification>

What, in this example, is the "event record"?  I assume it is this
part:

       <interfaces xmlns="urn:example:interface-module">
         <interface>
           <name>eth1</name>
           <interface-enabled>
             <by-user>fred</by-user>
           </interface-enabled>
         </interface>
       </interfaces>

But the element that is the name of the YANG notification is this:

           <interface-enabled>
             <by-user>fred</by-user>
           </interface-enabled>


(With draft-ietf-netconf-notification-message, the "event record"
would be placed in the <notification-contents> anydata)


A problem is that the containment hierarchy is defined in RFC 7950 in
the XML encoding text.  There is no text in RFC 7951 or RFC 8040 for
how to do this in JSON for RESTCONF.  It is natural to do a pure
JSON-translation of the XML example, but this isn't specified
anywhere.


So I propose the following tweaked changes:


OLD:

   An event stream is a named entity on a publisher which exposes a
   continuously updating set of event records.  Each event stream is
   available for subscription.  It is out of the scope of this document
   to identify a) how streams are defined (other than the NETCONF
   stream), b) how event records are defined/generated, and c) how event
   records are assigned to streams.

NEW:

   An event stream is a named entity on a publisher which exposes a
   continuously updating set of event records.  An event record is an
   intantiation of a "notification" YANG statement.  If the
   "notification" is defined as a child to a data node, the
   intantiation includes the hierarchy of nodes that identifies the
   data node in the datastore (see Section 7.16.2 of RFC 7950).  Each
   event stream is available for subscription.  It is out of the scope
   of this document to identify a) how streams are defined (other than
   the NETCONF stream), b) how event records are generated, and c) how
   event records are assigned to streams.


and in the YANG module:

OLD:

          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.

NEW:

          The subtree filter is applied to the representation of
          individual, delineated event records as contained within the
          event stream.

and

OLD:

           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.

NEW:


           The XPath expression is evaluated on the representation of
           individual, delineated event records as contained within
           the event stream.

> (I think it would be useful with some filtering examples, like in
> section 5 of RFC 5277, but I think you have all examples in
> netconf-notif, so they would go into that draft.  (re-reading an old
> thread I found that examples for filters were requested before)).

And I strongly think that we should add examples in an appendix, even
though this is a protocol and encoding neutral document.  Maybe one
example with NETCONF XML and one with RESTCONF JSON.  One example can
show a subtree filter and a top-level notif, and the other an XPath
filter with an inline notif.



/martin