Re: [Netconf] Comments on Yang-push-18 //RE: I-D Action: draft-ietf-netconf-yang-push-18.txt

[Alexander Clemm <[e-mail]>]

Hi Rohit, 

thank you for the comments!  I am in the process of incorporating them into -19, which I hope to post by Thursday.

Replies to your items inline.

--- Alex

> -----Original Message-----
> From: Netconf [mailto:[e-mail]] On Behalf Of Rohit R
> Ranade
> Sent: Tuesday, August 28, 2018 10:01 PM
> To: [e-mail]
> Subject: [Netconf] Comments on Yang-push-18 //RE: I-D Action: draft-ietf-
> netconf-yang-push-18.txt
> 
> Hi All,
> 
> 
> 1. In Section 3.3
>   The whole para from "To avoid flooding receivers .. the last change will still
> be sent." looks like a repetition
>   of Section 3.1 , description of "Dampening period"
> 

<ALEX> 3.1 describes the model, while 3.3 discusses the considerations behind it and provides additional explanation.  While the explanation may be redundant, we felt it is still useful to explain the rational.  Hence we would like to keep it.  
</ALEX>

> 2. Section 3.3, Point 4
>   "insert into the YANG patch record the last change made for any object
> which otherwise wouldn't have appeared."
>    I am not able to understand this statement clearly. Can this statement be
> simplified ?

<ALEX> 
Rephrasing it as follows: " insert into the YANG patch record the last change made, even if the new value is no different from the original value (since changes that were made in the interim were canceled out)."
</ALEX>

> 
> 3. Section 3.5.2
>   The para "A publisher will indicate a change to the effect ... this by a
>    "move" operation."
>    This is already defined in the YANG Patch RFC right ? What is the need to
> define this again in this RFC ?

<ALEX> The patch is generated by the publisher, not by the client.  In that sense, we use it a bit different from RFC 8072.  We thought it will be useful in the interest of greater clarity to explain it.  So, while it would be possible to editorially tighten, again we would like to keep it.  
</ALEX>
> 
> 4. Section 3.5.2
>    "To support this, it is valid to encode a YANG patch operation so that its
> application would result
>    in no change between the previous and current state.  This indicates
>    that some churn has occurred on the object.  An example of this would
>    be a patch that does a "create" operation for a datastore node where
>    the receiver believes one already exists, or a "replace" operation
>    which replaces a previous value with the same value."
> 
>     Some points here need more clarification.
>     a. If there is no change, but only a churn then why is it important to send
> this object in a YANG push ? What is the benefit ?

<ALEX>It indicates the fact that churn has occurred.  Basically, it allows a client to recognize that the value has not been static/unchanged, but did undergo some kind of oscillation.  The benefits to only indicate the last change is per the dampening period (reduce bandwidth and CPU).  
</ALEX>

>     b. If there was a "create" and "delete" on object X, within a dampening
> period, then what will be the patch record operation ?

<ALEX>
We would send a delete.  (even if the object did not exist originally).  We should indicate this actually.  Adding the following to the same item: 
" In case the changes involve creating a new datastore node, then deleting it, the YANG patch record will indicate deletion of the datastore node.  Similarly, in case the changes involve deleting a new datastore node, then recreating it, the YANG patch record will indicate creation of the datastore node."
</ALEX>

>     c. s/patch that does a "create" operation /patch that indicates a "create"
> operation  ? Patch can indicate the operation not do it.

<ALEX> changed "does" do "indicates" /ALEX>

> 
> 5. Section 3.7
>   "the counter MUST be reset to '1' the after passing a maximum value of
> '4294967295'"
>   s/reset to '1' the after passing/reset to '1' after passing
> 

<ALEX> changed </ALEX>

> 6. Section 3.7 Figure 2: Push example for on change
>    The "yang-patch" should be under the namespace
> "urn:ietf:params:xml:ns:yang:ietf-yang-patch"
> 

<ALEX> changed </ALEX>

> 7. In the ietf-yang-push, tree diagram
>    "
>              +--ro datastore-changes?
>                  +--ro ypatch:yang-patch
>                 +--ro patch-id        string
>    "
>     There looks to be an indentation problem for "ypatch:yang-patch"
> 

<ALEX> changed (apparently somewhere a tab character sneaked into the .xml, which was not rendered correctly) </ALEX>

> 8. resynch-subscription-error
>    I feel "sync" is a better abbreviation than "synch" for synchronization,
> because when reading this name, we may
>    not pronounce this correctly or read correctly.  I also feel "re-sync" is better
> when compared to "resync"
> 

<ALEX> Hmm.  This will also require to change the name of RPCs, and everywhere else the term "sync(h)" is used.  I agree that while not a major item, this would have been a better choice, so I am applying the change throughout.  </ALEX>  

> 9. Section 3.9.
>    "since the last notification message was sent to a particular each receiver.".
>    s/sent to a particular each receiver/sent to a particular receiver
> 

<ALEX> changed </ALEX>

> 
> 10. Section 3.9
>   "To accomplish this, implementations SHOULD support the
>    conceptual authorization model of [RFC8341], specifically section 3.2.4.".
>    The link to section 3.2.4 seems to be broken as it is taking the reader to the
> YANG Push draft and not to NACM
> 

<ALEX> Yes indeed, I see it.  Interestingly this is autogenerated in the .html version (and only there), the XML source does not include a link.  I am not sure how to change it.  Note for the RFC Editor?
</ALEX>

> 11. Section 3.9
>   I think here "access-protected data", is used to indicate that the receiver
> has "no access". I think this is misleading.
>   "access-protected data" means that there are NACM rules to protect the
> data. Maybe we can replace it with "access-denied data" ?
> 

<ALEX> Rephrased the passages in question as follows: " A publisher MUST allow for the possibility that a subscription's selection filter references non-existent data or data that the receiver is not allowed to access." and " A publisher MAY choose to reject an establish-subscription request which selects non-existent data or data that a receiver is not allowed to access. "  </ALEX>

> 12. Section 3.9
>   "If read access into previously accessible nodes has been lost due to
>    a receiver permissions change, this SHOULD be reported as a patch
>    "delete" operation for on-change subscriptions."
>   I think we should not have such mechanisms. If a user does not have access,
> we should not try to evaluate what was the last update sent
>   to such user, and how to send a subsequent patch which will stabilize the
> user's DB due to the access-control changes.
> 

<ALEX> I can see your point; that said I think one can argue to keep things as-is.  When a receiver can no longer access a datastore node, the node has effectively "disappeared" to the receiver.  By keeping it, the receiver will be under the illusion of no change even when the value does change.  This might cause a receiver to "not trust" a subscription any more and start polling, which is something we would absolutely want to avoid.  So, I prefer to keep things as they are.   Alternatively, an implementation may always delete the subscription / force subscription reestablishment.  
--- Alex
</ALEX>

> 
> With Regards,
> Rohit R Ranade
> 
> -----Original Message-----
> From: Netconf [mailto:[e-mail]] On Behalf Of internet-
> [e-mail]
> Sent: 29 August 2018 06:24
> To: [e-mail]
> Cc: [e-mail]
> Subject: [Netconf] I-D Action: draft-ietf-netconf-yang-push-18.txt
> 
> 
> A New Internet-Draft is available from the on-line Internet-Drafts directories.
> This draft is a work item of the Network Configuration WG of the IETF.
> 
>         Title           : YANG Datastore Subscription
>         Authors         : Alexander Clemm
>                           Eric Voit
>                           Alberto Gonzalez Prieto
>                           Ambika Prasad Tripathy
>                           Einar Nilsen-Nygaard
>                           Andy Bierman
>                           Balazs Lengyel
> 	Filename        : draft-ietf-netconf-yang-push-18.txt
> 	Pages           : 57
> 	Date            : 2018-08-28
> 
> Abstract:
>    Via the mechanism described in this document, subscriber applications
>    may request a continuous, customized stream of updates from a YANG
>    datastore.  Providing such visibility into updates enables new
>    capabilities based on the remote mirroring and monitoring of
>    configuration and operational state.
> 
> 
> The IETF datatracker status page for this draft is:
> https://datatracker.ietf.org/doc/draft-ietf-netconf-yang-push/
> 
> There are also htmlized versions available at:
> https://tools.ietf.org/html/draft-ietf-netconf-yang-push-18
> https://datatracker.ietf.org/doc/html/draft-ietf-netconf-yang-push-18
> 
> A diff from the previous version is available at:
> https://www.ietf.org/rfcdiff?url2=draft-ietf-netconf-yang-push-18
> 
> 
> Please note that it may take a couple of minutes from the time of submission
> until the htmlized version and diff are available at tools.ietf.org.
> 
> Internet-Drafts are also available by anonymous FTP at:
> ftp://ftp.ietf.org/internet-drafts/
> 
> _______________________________________________
> Netconf mailing list
> [e-mail]
> https://www.ietf.org/mailman/listinfo/netconf
> 
> _______________________________________________
> Netconf mailing list
> [e-mail]
> https://www.ietf.org/mailman/listinfo/netconf