Re: [Netconf] review of draft-ietf-netconf-yang-push-11

[Martin Bjorklund <[e-mail]>]

Hi,

I'll reply to the on-change discussion here.

Alexander Clemm <[e-mail]> wrote:
> Yes, thank you for the comments!  
> 
> As the mail is long, rather than commenting inline, let me make just a
> few additional replies in addition to Eric's:
> 
> - On the topic of how to mark on-change notifiable YANG objects:
> 
> Yes, clearly what is on-change notifiable and what is not could differ
> by implementation.  We thought that asking implementors to define
> implementation-specific modules that would indicate the on-change
> notifiability through deviations would make sense, since this is
> really one of the function of deviations - indicate what is
> implementation-specific.

Actually, I think it mis-uses the deviation statement a bit.  From the
spec:

   Deviations define the way a server or class of servers deviate from a
   standard.

   [...]

   Server deviations are strongly discouraged and MUST only be used as a
   last resort.  Telling the application how a server fails to follow a
   standard is no substitute for implementing the standard correctly.  A
   server that deviates from a module is not fully compliant with the
   module.


We have discussed an "annotate" statement before, which would be used
to add information to schema nodes.  (we actually have such a
statement as a vendor-specific extension, and it is used a lot).  But
even if we had that, I'm not sure that would be the best solution in
this case.

> In that sense, the current solution does
> make sense to me and seems to be in the spirit of how
> implementation-specific YANG modules should be used.
> 
> That said, the other proposal to have a "meta-module" that contains a
> list of objects for which on-change notification is supported could be
> certainly be an alternative.  However, the notifiability should be
> indicated per node at the schema level, not for each individual
> instance.  We would not want to have to enumerate individual list
> entries, for example. Also, are there additional considerations to
> accommodate schema-mount in that case?
> 
> Another thought, perhaps on-change notifiability is something that
> could instead also be included in the YANG library.  Each module in
> the library would then list the nodes for which on-change is
> notifiable on a given server.

That's also an option.

I think we want somehting that is available on-line in runtime, but
that also can be stored in an off-line file (ala AGENT-CAPABILITIES).
Augmentation of yang library probably solved both these cases.


> Deferring this until later might also an option.

I think that's my preferred option.  At least if we want to finish
this document sooner rather than later...


> I am sure there will
> be other annotations/markings for objects that we will come up with
> our time.  However, one issue is that by default we did not want to
> make assumptions about on-change notifiability being supported.
> Rather, when it is supported, it would be specifically indicated; the
> default would be that it's not supported.  If we defer the solution
> until later, then this won't work.

Well, there's alread a feature to indicate *some* support.  If we
defer, we'd just write that how an implementation communicates which
nodes it supports for on-change is out of scope.

> If we wanted to defer, it would
> seem what we should do is still propose a solution informally in an
> appendix, and indicate that this may be later replaced by another
> solution.  We would put the YANG extension into an own separate
> module.

I don't think that's a good solution.  I don't think we should
encourage the extension-statement solution.

As one data point, in our implementation, all config nodes in the
conventional datastores would be available for on-change.  For
operational state, it is up to the instrumentation.


> - on the Subscription Policy term (3.6).  Sure, we don't need to
> - introduce a new term, but really what the parameters describe is in
> - fact a policy.  We could of course call them "datastore subscription
> - specific configuration parameters", or "datastore subscription
> - specific behavior configuration" , but this sounds a bit awkward to
> - me.

I would actually prefer more concrete words like the ones you
propose.


> - on the "no-such-datastore" eror tag: true, invalid-value would cover
> - this, but why be general when we can be specific?

I'll reply to this in my reply to Eric.


/martin



> 
> 
> --- Alex
> 
> > -----Original Message-----
> > From: Netconf [mailto:[e-mail]] On Behalf Of Eric Voit
> > (evoit)
> > Sent: Wednesday, November 29, 2017 9:37 AM
> > To: Martin Bjorklund <[e-mail]>; [e-mail]; Balazs
> > Lengyel
> > <[e-mail]> ([e-mail])
> > <[e-mail]>; Andy Bierman ([e-mail])
> > <[e-mail]>
> > Subject: Re: [Netconf] review of draft-ietf-netconf-yang-push-11
> > 
> > Hi Martin,
> > 
> > Thanks for the comments.   Many thoughts in-line...
> > 
> > > From: Martin Bjorklund, November 28, 2017 4:38 AM
> > >
> > > Hi,
> > >
> > >
> > > I have now reviewed draft-ietf-netconf-yang-push-11.  I have one
> > > somewhat more important comment, and several others.
> > >
> > > Important issue:
> > >
> > > o  3.10
> > >
> > >   I don't think the proposed YANG extension is the correct solution to
> > >   the stated problem, for several reasons:
> > 
> > In general we agree there are several alternatives to solve this
> > issue.  And
> > any mechanism supported by the WG would be fine.
> > 
> > My read of your proposal below is that it does reuse elements of 3.10.
> > For
> > example, I *think* you are proposing hierarchical inheritance of
> > on-change
> > property markings of the parent rather than making every instance.
> > Because
> > marking every instance within a routing table would be prohibitively
> > expensive.  So based on whatever commonality we can establish across
> > the
> > proposed alternatives, perhaps we can come to a common way to set this
> > information.    If not, we can back out on-change marking as something
> > implementation-specific for this current specification.
> > 
> > Stepping back a minute, it is good to think about who would use this
> > on-
> > change support data.  Application developers are going to make
> > subscription
> > requests.  And they are far more likely to want to design based on the
> > schema level information exposed rather than instance data.  Driven by
> > that
> > constraint, the current proposal is framed as is -- mostly since there
> > was no
> > interest in Balazs'
> > https://tools.ietf.org/html/draft-lengyel-netmod-schema-annotation-00
> > which was designed to support application developers needing this
> > information.  Without this draft, at the time this was proposed the
> > next-best
> > thing was an extension which works based on the established pattern of
> > deviations.
> > 
> > To cover this issue, I have re-opened YP#10 to track and resolve...
> > https://github.com/netconf-wg/yang-push/issues/10
> > 
> > Below are some specific pros/cons I see...
> > 
> > >     1.  In most cases, this is not a property of the data model, but
> > >         of the implementation, and possibly even the deployment.  So
> > >         having a YANG extension statement is not a good solution.
> > 
> > Fully agree this can be implementation level issue.  But then again,
> > so is any
> > deviation.
> > 
> > So to cover "property of the implementation" case, the proposed
> > extension
> > would typically be included in deviations file per implementation.
> > Such an
> > approach does mirror existing mechanisms for deviations, and has the
> > benefit of not requiring another a new totally separate file to
> > document the
> > desired behavior.
> > 
> > As for times this might be deployment specific, we saw potential
> > capacity
> > issues determining whether something might not be on-change
> > subscribable.   Here it would be up to the implementation to determine
> > whether to use the error "on-change-unsupported" from YANG Push, or
> > the
> > error "insufficient-resources" from subscribed notifications.  A
> > choice
> > between the two can be driven based on whether a deployment would
> > *ever* have capacity for the requested subscription.
> > 
> > >     2.  With NDMA, the same schema node is present in different
> > >         datastores.  It might be the case that an implementation
> > >         supports on-change for the node in a configuration datastore,
> > >         but not in operational.  Again, marking a node in the schema
> > >         is not a good solution.
> > 
> > As the current solution was developed before NMDA, we didn't consider
> > differences in on-change support for different datastores.  So this is
> > a real
> > consideration.
> > 
> > If for a minute we assume we are going to follow the current approach
> > in the
> > draft, the extension proposed in 3.10 could have a default of the
> > operational
> > datastore.  And we could add the option of including a list of
> > datastores in the
> > extension whenever on-change for something else is needed/supported..
> > 
> > >     3.  Since the on-change property is implementation dependent, it
> > >         means the information will be available to clients only in
> > >         deviation modules.  This is quite an expensive and complicated
> > >         way to pass the information to the clients.
> > 
> > Visibility to application developers was one reason we went down this
> > path.
> > Any solution needs to frame this visibility as a key element.
> > Considering this,
> > I actually see a need both for the schema based solution as framed,
> > and the
> > possibility of an instance based mechanism if someone wanted the lower
> > level of granularity.  Per above, I don't think instance data within
> > all instance
> > objects like routing entries is possible anyplace datastore size
> > becomes a
> > factor.
> > 
> > >   An alternative solution could be to have an ordered list of
> > >   instance-identifiers that list this property, per datastore, for
> > >   example:
> > >
> > >    <entry>
> > >      <path>/sys:system/sys:system-time<path>
> > >      <notifiable-on-change>false</notifiable-on-change>
> > >    </entry>
> > >    <entry>
> > >      <path>/sys:system<path>
> > >      <notifiable-on-change>true</notifiable-on-change>
> > >    </entry>
> > 
> > This could work.  Several questions which would need to be addressed
> > with
> > this approach:
> > 
> > (1) how would application developers easily know what objects are on-
> > change subscribable?
> > 
> > (2) If this is per-instance, is the property inherited from a parent?
> > If no, how
> > do we deal with the size of the resulting information?
> > 
> > (3) If this is per-instance, how is this information populated?  Don't
> > you need
> > schema level guidance recorded somewhere anyway?
> > 
> > (4) Are there any issues with things like Schema mount?
> > 
> > >   Yet another alternative would be to leave this to future work.
> > 
> > If necessary, we can push this out.   I am hoping we can come up with
> > something here, especially as the on-chance implementations I have
> > seen
> > have all used phased introduction of on-change for different schema
> > objects.
> > 
> > > Other comments:
> > >
> > >
> > > o  2
> > >
> > >   The document uses the term "YANG datastore" (it is even in the title
> > >   of the document).  This term is not used elsewhere, and it is not
> > >   defined in this document.  I suggest you change this to simply
> > >   "datastore".
> > 
> > In general, I am fine with making the change to "Datastore" throughout
> > the
> > document.  For the title, I think it better to keep YANG to allow
> > casual
> > readers easy access to the context.
> > 
> > >   Also, I think you should "import" the term "datastore" from
> > >   draft-ietf-netmod-revised-datastores, instead of having a slightly
> > >   different term in this document.
> > 
> > Can do.
> > 
> > >   It is also unfortunate that you use the term "data node" in a
> > >   different meaning than RFC 7950.  Maybe use "datastore node"
> > >   instead?
> > 
> > Can do.
> > 
> > >
> > > o  3.1
> > >
> > >   I'm not sure I understand the dampening period concept.  Let's
> > >   assume that the dampening period is 10s.  Then changes happen at
> > >   times:
> > >
> > >     2  3  4  11  13  14  15
> > >
> > >   From the description, it seems I would receive 4 notifications, from
> > >   times:
> > >
> > >     2  (containing only change from 2)
> > >     12 (containing change from 3,4,11)
> > >     13 (containing only change from 13)
> > >     23 (containing changes from 14,15)
> > 
> > Should be 2, 12, & 22.  I have updated the definition (below) in a way
> > which
> > hopefully clarifies the proper behavior
> > 
> >          +  Dampening period: In an on-change subscription, detected object
> > changes should be sent as quickly as possible.  However without
> > adequate
> > protections, a rapid series of object changes might exhaust of
> > resources in
> > the publisher or receiver.  In order to protect against that, a
> > dampening
> > period MAY be used to specify the interval which must pass before
> > successive update records for the same subscription are generated for
> > a
> > receiver.  The dampening period collectively applies to the set of all
> > data
> > nodes selected by a single subscription and sent to a single receiver.
> > This
> > means that when there is a change to a subscribed object, an update
> > record
> > containing that object is created either immediately when no dampening
> > period is in effect, or at the end of a dampening period.  A dampening
> > period
> > is reset every time a new notification message is passed to transport.
> > 
> > >   Is this correct?
> > >
> > >   In any case, I suggest the description in the YANG module is
> > >   clarified - currently the RFC text contains more details than the
> > >   YANG module.
> > 
> > I have updated the YANG module text as:
> > 
> > "Specifies the interval which must pass before successive update
> > records for
> > the same subscription are generated for a receiver.  The dampening
> > period
> > collectively applies to the set of all data nodes selected by a single
> > subscription and sent to a single receiver.  This means that when
> > there is a
> > change to a subscribed object, an update record containing that object
> > is
> > created either immediately when no dampening period is in effect, or
> > at the
> > end of a dampening period.  A dampening period is reset every time a
> > new
> > notification message is passed to transport.  A dampening period is
> > reset
> > every time a new notification message is passed to transport.  A zero
> > value
> > indicates no dampening period, and all subscribed object changes are
> > sent
> > immediately."
> > 
> > > o  3.2
> > >
> > >   The text says:
> > >
> > >      Therefore, in order to minimize the number of subscription
> > >      iterations between subscriber and publisher, dynamic
> > >      subscriptions SHOULD support a simple negotiation between
> > >      subscribers and publishers for subscription parameters.
> > >
> > >   To whom is this "SHOULD" directed?  I mean, dynamic subscriptions as
> > >   specified in this draft does indeed support simple "negotiation".
> > >   Maybe s/SHOULD support/supports/ ?
> > 
> > Yes.  Will remove the SHOULD, showing just "supports".
> > 
> > > o  3.4
> > >
> > >   The text says:
> > >
> > >     Please see [promise] for
> > >     more on the transactional basis underlying the publisher and
> > >     subscriber interactions within this document.
> > >
> > >   So I did that.  But I didn't find any text in the referenced
> > >   document that talked about "the transactional basis".
> > >
> > >   I suggest you remove this sentence from the draft.
> > 
> > As this is informative, I can remove.
> > 
> > > o  3.5
> > >
> > >   The text says:
> > >
> > >      A publisher MUST support XML encoding and MAY support other
> > >      encodings such as JSON encoding.
> > >
> > >   I don't think this is correct.  A RESTCONF sever is not required to
> > >   support XML.  I think you should remove this sentence.
> > 
> > ok
> > 
> > > o  3.5.1
> > >
> > >      In a periodic subscription, the data included as part of an update
> > >      corresponds to data that could have been simply retrieved using a get
> > >      operation and is encoded in the same way.
> > >
> > >   What is "a get operation"?  Do you mean RESTCONF GET or NETCONF
> > >   <get/> or something else?  Whatabout other protocols?
> > 
> > As YANG Push is intended to be transport independent, it is better not
> > to
> > provide a specific transport.   Instead, how about the following text:
> > 
> > In a periodic subscription, the update record MUST include the
> > datastore
> > nodes which would have been retrieved using an equivalent datastore
> > selection operation over that subscription's transport.
> > 
> > >   (In 3.6, you use simply "a GET" for probably the same thing.)
> > 
> > If you are good with the text above, I will change 3.6 to:
> > 
> > " to what is available with a datastore selection operations".
> > 
> > > o  3.6
> > >
> > >      Subscription policy specifies both the selection filters and the
> > >      datastores against which these selection filters will be applied.
> > >      The result is the push of information necessary to remotely maintain
> > >      an extract of the publisher's datastore.
> > >
> > >   It seems this paragraph defines the term "Subscription policy".  But
> > >   this term is not used in the document.
> > 
> > We don't intend to define a new term in this paragraph.  And you are
> > correct,
> > policy only appears elsewhere in the document as part of the YANG
> > model
> > group names.    (more below)
> > 
> > >  What does it means that the  result of a policy is "the push of
> > information"?
> > >
> > >   I think I don't understand what this paragraph tries to tell me.
> > 
> > What if I changes the first paragraph of Section 3.6 to:
> > 
> > An objective of YANG push is to allow the replication of a subset of a
> > publisher's datastore within a receiver.  The datastore selection
> > filter defines
> > this subset.  Only a single selection filter can be applied to a
> > subscription at a
> > time.  The selection filter types defined in this include:
> > 
> > > o  3.6
> > >
> > >      o  xpath: An xpath selection filter is an XPath expression which may
> > >         be meaningfully applied to a datastore.
> > >
> > >    What does "meaningfully applied" mean?
> > 
> > There are plenty of valid xpath expressions which don't select data
> > nodes.
> > How about:
> > 
> > " xpath: An xpath selection filter is an XPath expression which
> > references
> > data nodes. When applied to a datastore, it is the results of this
> > expression
> > which will be pushed."
> > 
> > > o  3.6
> > >
> > >      Selection filters are not intended to be used to filter objects
> > >      based on a non-key property.  Supporting non-key property
> > >      filtering so would have a number of implications that would
> > >      result in significant complexity.
> > >
> > >      [...]
> > >
> > >      the goal is to
> > >      provide equivalent capabilities to what is available with a GET.
> > >
> > >   In GET you can filter on "non-key properties".
> > >
> > >   I think you should remove the text about "non-key properties".  If
> > >   anything, I think you can allow an implementation to reject a filter
> > >   that would be too complex to implement / evaluate (in fact I think
> > >   the text already allows a server to reject such filters.)
> > 
> > The issue with non-key properties is how to deal with the creation and
> > deletion of objects when a non-key property goes in/out of the
> > selection.
> > Others were uncomfortable with things like passing along a patch
> > showing a
> > node was deleted, when in reality the property simply changed from
> > meeting the selection filter to failing the selection filter.  So
> > people wanted
> > to default to the simpler case of key properties for selection.
> > 
> > Based on that I can delete the words "but the goal is to provide
> > equivalent
> > capabilities to what is available with a GET".
> > 
> > >   With the current text, is this filter ok:
> > >
> > >      /interfaces/interface[contains(name, "eth")]
> > >
> > >   It filters on a key, so it should be ok, right?
> > 
> > Yes
> > 
> > > o  3.7
> > >
> > >   The XML examples are not using the correct XML namespace for the
> > >   nodes from the "ietf-interface" module.
> > >
> > >   The YANG Patch example also shows an interesting effect in the
> > >   "patch-id" and "edit-id" leafs.  I think the draft should mention
> > >   how implementations are suppose to fill in these leafs.
> > 
> > Will add how to populate.  In summary, sequential numbering of
> > "edit-id"
> > was from the RFC-8072.   And a null "patch-id" is because patch-id is
> > mandatory in RFC-8072, and was originally supposed to be used for
> > debugging of failed datastore write operations.  That really isn't the
> > case
> > here, plus we have subscription-id and other object available.
> > 
> > >   The "target" leaf is not specified correctly.  It should be:
> > >
> > >          <target>/ietf-interfaces:interfaces-state</target>
> > 
> > Good catch.  Making change.
> > 
> > > o  3.8
> > >
> > >   The example has:
> > >
> > >    <establish-subscription
> > >        xmlns="urn:ietf:params:xml:ns:yang:ietf-subscribed-notifications"
> > >        xmlns:yp="urn:ietf:params:xml:ns:yang:ietf-yang-push">
> > >       <yp:datastore>
> > >         <yp:source
> > > xmlns="urn:ietf:params:xml:ns:yang:ietf-datastores">
> > >
> > >   This doesn't match the YANG model; there is no container called
> > >   "datastore" in the model.
> > >
> > >   Further is has:
> > >
> > >         <yp:subtree-filter netconf:type="xpath"
> > >             xmlns:ex="http://example.com/sample-data/1.0"
> > >             select="/ex:foo"/>
> > >
> > >   a subtree-filter of type xpath?  I think ot should be:
> > >
> > >         <yp:xpath-filter xmlns:ex="http://example.com/sample-data/1.0">
> > >            /ex:foo
> > >         </yp:xpath-filter>
> > 
> > Yes
> > 
> > >   Also, it has:
> > >
> > >         <yp:source xmlns="urn:ietf:params:xml:ns:yang:ietf-datastores">
> > >           operational
> > >         </yp:source>
> > >
> > >   which should be:
> > >
> > >         <yp:source xmlns:ds="urn:ietf:params:xml:ns:yang:ietf-datastores">
> > >           ds:operational
> > >         </yp:source>
> > 
> > At the last IETF, Kent pointed us to YANG Lint which should allow us
> > to do
> > example checking in ways not previously known to us.  I will install
> > and
> > attempt to use these to clear the example items you have listed.
> > 
> > > o  3.9
> > >
> > >   This section lists three cases for which:
> > >
> > >      the error identity "data-unavailable" SHOULD be returned.
> > >
> > >   One of the cases is:
> > >
> > >     o  the authorization privileges of a receiver change over the course
> > >        of the subscription.
> > >
> > >   But how can a server know this when "establish-subscription" is sent?
> > 
> > It cannot know this at "establish-subscription".  So a publisher will
> > have to
> > track whether the permissions on subscribed objects change.  How is
> > left to
> > implementations.
> > 
> > > o  3.9
> > >
> > >      The contextual authorization model for data in YANG datastores is
> > >      the NETCONF Access Control Model [RFC6536bis], Section 3.2.4.
> > >
> > >   Did you mean s/contextual/conceptual/ ?
> > 
> > Will make that change.
> > 
> > > o  3.11.1
> > >
> > >
> > >   OLD:
> > >
> > >        If
> > >        this is not possible and the synch-on-start option is
> > > configured,
> > >
> > >   NEW:
> > >
> > >        If
> > >        this is not possible and the "no-synch-on-start" option is not
> > >        present for the subscription.
> > >
> > >
> > >   (fixes incorrect name of leaf, and also covers dynamic
> > >   subscriptions)
> > 
> > Yes, good catch.  Will fix.
> > 
> > > o  4.3.2
> > >
> > >      A subscription-id MUST be transported along with the subscribed
> > >      contents.
> > >
> > >   Then the leaf "subscription-id" should be mandatory.
> > 
> > The requirement is that a subscription-id must be in the notification
> > message, this doesn't mean that the subscription-id must be within the
> > two
> > new notifications.  The reason for this is that when we add support
> > for the
> > notification-messages draft, but having the subscription-id optional
> > in the
> > push-update and push-change-update, we can enable implementations
> > which do not duplicate this header item.  (And a duplication would be
> > forced
> > with the future header if we made this mandatory.)
> > 
> > >      A "time-of-update" which represents the time an update record
> > >      snapshot was generated.  A receiver MAY assume that a publisher's
> > >      objects have these pushed values at this point in time.
> > >
> > >   Should "time-of-update" be mandatory?
> > 
> > Two reasons it is not:
> > (a) Other vendors have worried they can only support message-time,
> > rather
> > than notification-time.
> > (b) Notification-time is the generalized name for "time-of-update" .
> > Having
> > "time-of-update" as optional allows for eventual migration to common
> > headers of draft-ietf-netconf-notification-messages without
> > information
> > duplication
> > 
> > >   How is "time-of-update" different from "eventTime" in the
> > >   notification?
> > 
> > I think that answer is above.  But there is some good guidance here
> > from
> > draft-ietf-netconf-notification-messages.  The new draft ultimately
> > will
> > define and support the following times:
> > 
> > message-time:
> >       "Header information consisting of time the message headers were placed
> > generated prior to being sent to transport";
> > 
> > notification-time
> >       "Header information consisting of the time an originating process
> >       created
> > the notification."
> > 
> > Notification-time should be equivalent to eventTime from RFC-5277.
> > But
> > some other vendors have said they really only have message-time, and
> > this is
> > what they are populating in eventTime even though it doesn't
> > explicitly fit
> > the definition.  With the new definitions, at least they should be
> > able to
> > populate what they explicitly have.
> > 
> > > o  4.3.2
> > >
> > >      If the application detects an informational discontinuity
> > >
> > >   What is an "informational discontinuity"?
> > 
> > Will change to:
> >   If the application detects any incompleteness in set of objects placed
> >   into a
> > "push-update" or "push-change-update"
> > 
> > > o  4.4.1 / 4.4.2
> > >
> > >   The XML examples are broken; compare with my comments for 3.8.
> > 
> > Will do.  I am hoping a new attempt using yanglint helps pick out
> > everything I
> > missed by hand previously.
> > 
> > > o  5
> > >
> > >   OLD:
> > >
> > >     "This module contains conceptual YANG specifications
> > >      for YANG push.";
> > >
> > >   NEW:
> > >
> > >     "This module contains YANG specifications for YANG push.";
> > 
> > ok
> > 
> > > o  5 - identities
> > >
> > >     identity qos-unsupported {
> > >       base sn:error;
> > >       description
> > >         "Subscription QoS parameters not supported on this platform.";
> > >     }
> > >
> > >   This identity is not mentioned anywhere in the text.  Instead of
> > >   having this identity, wouldn't it be better to define a feature for
> > >   "qos", and mark the nodes you have in mind with an if-feature?
> > 
> > It is possible to expose QoS explicitly as a feature.  I will make
> > that addition if
> > you are ok with my other QoS comment described below about subscribed-
> > notifications.
> > 
> > But even in that case if QoS is not supported, and someone includes
> > QoS
> > objects in an "establish-subscription" we need this identity as an
> > error.  This
> > error allow an explicit identification of what was wrong in the RPC
> > (such as a
> > DSCP provided is not supported by the Publisher).  I will include this
> > in the
> > Identity description.
> > 
> > >     identity on-change-unsupported {
> > >       base sn:error;
> > >       description
> > >         "On-change not supported.";
> > >     }
> > >
> > >   When will this identity be used?  There is already a feature
> > >   "on-change" and corresponding if-feature statements.
> > 
> > Per our discussion above, this can be used if an RPC asks for
> > on-change for an
> > object which is not available at a platform deployment (either marked
> > in the
> > schema, or a specific deployment doesn't support an object which is
> > included).  I will enhance the description based on the discussion on
> > this
> > earlier in this thread.
> > 
> > >     identity on-change-synch-unsupported {
> > >       base sn:error;
> > >       description
> > >         "On-change synch-on-start and resynchonization not supported.";
> > >     }
> > >
> > >   The leaf is called "no-sync-on-start", which implies that sync on
> > >   start is the default.  So when will this identity be used?
> > 
> > Can be used in two places:
> > 
> > (1) Will be used if an RPC asks to synch on start, but it can't be
> > supported for
> > any reason (e.g., no nodes identifiable within the selection filter
> > will ever be
> > support on-change).   Will enhance the definition.
> > 
> > (2) The resynch RPC is invoked on a periodic subscription-id, or on an
> > on-
> > change subscription which can't support synchronization.
> > 
> > >     identity reference-mismatch {
> > >      base sn:error;
> > >       description
> > >        "Mismatch in filter key and referenced yang subtree.";
> > >     }
> > >
> > >   I don't understand the description of this identity.  Please
> > >   clarify.
> > 
> > Will clarify description to explain that the key provided with the
> > filter does
> > not match the data type of the referenced yang subtree
> > 
> > >      identity datatree-size {
> > >
> > >   Should it be "result-too-big" or something?
> > 
> > Yes.   I will change the name.
> > 
> > >     identity no-such-datastore {
> > >
> > >   I think this one should be removed.  The normal "invalid-value"
> > >   error-tag covers this error.
> > >
> > >
> > >       identity custom-datastore {
> > >         base ds:datastore;
> > >         description
> > >           "A datastore with boundaries not defined within
> > >            draft-ietf-netmod-revised-datastores";
> > >       }
> > >
> > >   This identity needs to be removed.  If someone defines a custom
> > >   datastore, it would get a specific identity, and that identity can
> > >   be used as "source".
> > 
> > Yes, any new custom datastore will get a new identity, but if that new
> > identity uses this custom-datastore one as a base, then we have an
> > umbrella
> > identity which is a handle just for the custom ones.  That was the
> > intent of
> > this.  If you don't think that an identity which acts as a bundling
> > mechanism is
> > useful, we can remove.
> > 
> > > o  5 - "change-type"
> > >
> > >   The descriptions of the enums need to be improved.  This is about
> > >   reporting a change in a datastore.  The value "create" is described
> > >   as:
> > >
> > >         description
> > >           "Create a new data resource if it does not already exist.  If
> > >           it already exists, replace.";
> > >
> > >   Also, the description of the typedef has:
> > >
> > >       "RFC 8072 section 2.5, with a delta that it is ok to receive
> > >       ability create on an existing node, or receive a delete on a
> > >       missing node.";
> > >
> > >   But what does this mean?  The type is used to *exclude* some changes
> > >   from a yang patch record.
> > 
> > This is to identify churn within a datastore during a dampening
> > period.  Two
> > examples of when this is needed:
> > 
> > (1) For security applications, it is an absolute requirement that you
> > know if a
> > node was added and then removed during a dampening period.   For an
> > example, a hacker adds a "permit any any" which is then quickly
> > removed
> > before the dampening period ends.  Remote applications need to know
> > that
> > churn occurred.
> > 
> > (2) For network management applications when a physical interface is
> > going
> > down and then quickly back up.  If the up/down is transient, how do
> > you
> > know that this occurred if there is no notification that churn
> > occurred?
> > 
> > To support this, the YANG patch definition is loosened to allow the
> > new
> > creation on an existing node (based on what resulted from the last
> > patch), or
> > a delete on a missing node.  This acts as an indication that some
> > churn in the
> > datastore happened even though the data is in the same state as a per
> > the
> > previous update.  And if an application get to sees this indication
> > when
> > comparing to the previous state, they have the option of looking more
> > closely at some logs on the publisher to determine what actually
> > happened.
> > 
> > I can see that this behavior is under-described in the text.  I will
> > put a small
> > section between sections 3.10 & 3.11 describing this.  The title will
> > be
> > "Identification of a transient change within dampening period"
> > 
> > > o  5 - selection filter
> > >
> > >   The XPath expression is not properly defined.
> > >
> > >   OLD:
> > >
> > >           "This parameter contains an XPath expression identifying the
> > >           portions of the target datastore to retrieve.";
> > >
> > >   NEW:
> > >
> > >           "This parameter contains an XPath expression identifying the
> > >            portions of the target datastore to retrieve.
> > >
> > >            If the expression returns a node-set, all nodes in the
> > >            node-set are selected by the filter.  Otherwise, if the
> > >            expression does not return a node-set, the filter
> > >            doesn't select any nodes.
> > >
> > >            FIXME: (*)
> > >
> > >            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.
> > >
> > >              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 of the target
> > >                 datastore.
> > >
> > >
> > >   (*) - We need to describe when the filter is evaluated.  This is
> > >   also true for the subtree filter.  Is it evaluated when the
> > >   subscription is started and explicitly modified, or everytime a
> > >   change is detected?
> > >
> > >   [Side note: this description is also missing from the "xpath-filter"
> > >   leaf in "get-data" in draft-ietf-netconf-nmda-netconf.  I have
> > >   updated the description in that draft.]
> > 
> > I will update the descriptions to match your draft.
> > 
> > As for when the evaluation occurs, I am not sure what you mean.  Every
> > time
> > a change occurs, you need to see if the changed object would have
> > fallen
> > within the selection.  There is no attempt to support an
> > event-condition-
> > action context.
> > 
> > > o  5 - terminology
> > >
> > >   The term "agent" is used a couple of times.  Use "publisher" or
> > >   "server" instead.
> > 
> > Will update
> > 
> > > o  5 - dampening
> > >
> > >           leaf dampening-period {
> > >             type yang:timeticks;
> > >             mandatory true;
> > >
> > >    Should this instead be:
> > >
> > >           leaf dampening-period {
> > >             type yang:timeticks;
> > >             default "0";
> > >
> > >    So that the request is the same if the "on-change" feature is
> > >    support or not.
> > 
> > I like it, will update.
> > 
> > > o  5 - no-synch-on-start
> > >
> > >              When
> > >              present, pushing a full selection per the terms of the
> > >              selection filter MAY NOT be done for this subscription.
> > >
> > >    Did you mean s/MAY NOT/MUST NOT/ ?
> > >
> > >    MAY NOT is not a 2119 term.
> > 
> > Will change to MUST NOT.
> > 
> > > o  5 - QoS
> > >
> > >   Why are the qos parameters defined in yang push, and not in
> > >   subscribed notifications?  It seems that they are generic.
> > 
> > This question has come up before.  Basically current 5277 event
> > streams
> > haven't shown need for QoS to date.   We know that QoS has been
> > requested to handle congestion issues for YANG push.  So for
> > simplicities
> > sake, we bundled all QoS elements together in YANG push where we knew
> > there was demonstrable need.  If someone really is asking for these
> > QoS
> > constructs for events, we can revisit.
> > 
> > > o  5 - establish-subscription params
> > >
> > >   I think a "when" expression should be added to the first augment:
> > >
> > >     augment "/sn:establish-subscription/sn:input" {
> > >       when "sn:target/yp:datastore/yp:source";
> > 
> > So this is saying that the named datastore must exist before allowing
> > the
> > augment.   Works for me.
> > 
> > > o  5 - push-update
> > >
> > >   What does the presence of "updates-not-sent" mean in "push-update"?
> > >   "push-update" contains a snapshot, not a diff, so why is
> > >   "updates-not-sent" present?
> > 
> > This is to indicate a system error where not all objects can be
> > extracted from
> > the datastore.  (e.g., the routing table got *lots* bigger, and the
> > periodic
> > extract cannot be performed as expected.)  At least some
> > implementations
> > have seen this.   I will add some clarifying text.
> > 
> > > o  5 - push-change-update
> > >
> > >         "This contains an incremental set of datastore changes needed
> > >          to update a remote datastore starting at the time of the
> > >          previous update, per the terms of the subscription.
> > >
> > >    What is an "incremental set"?  Probably s/incremental set/set/
> > 
> > I can change to "set".
> > 
> > > o  General
> > >
> > >   Use double quotes for node names.  "push-update", "subscription-id"
> > >   etc.  Quotes are currently used inconsistently, and it makes the
> > >   text harder to read.
> > 
> > Will make the fix.
> > 
> > Thanks again for all the excellent thoughts!
> > Eric
> > 
> > >
> > > /martin
> > >
> > > _______________________________________________
> > > Netconf mailing list
> > > [e-mail]
> > > https://www.ietf.org/mailman/listinfo/netconf
> > 
> > _______________________________________________
> > Netconf mailing list
> > [e-mail]
> > https://www.ietf.org/mailman/listinfo/netconf
>