On Thu, Oct 29, 2020 at 6:09 PM joel jaeggli <joe...@gmail.com> wrote:
> Rob, > > These seem like reasonable suggestions. > > Lets see what the authors say. > > Thanks for this > joel > > On Thu, Oct 29, 2020 at 6:47 AM Rob Wilton (rwilton) <rwil...@cisco.com> > wrote: > >> Hi, >> >> Here is my AD review for draft-ietf-netmod-nmda-diff-07. Apologies for >> the delay. >> >> Thank you for writing this document, I think that it is useful, and looks >> like it is in good shape. >> >> >> Main comments: >> >> 1. Should there be any text about how to find out what datastores are >> supported by a device? E.g., pointing them to either YANG library, or >> protocol specific mechanisms in the case of RESTCONF. >> >> Do you have a section in mind and suggested text? > 2. It might be helpful to add a comment about potential issues that could >> arise by comparing <running> to <operational>, i.e., additional differences >> could be reported due to inactive configuration and template processing >> between <running> and <operational>. >> >> Do you have a section in mind and suggested text? You mean if there are differences between <running> and <intended> then a diff between <running> and <operational> will not be the same as a diff between <intended> and <operational>.? 3. I would prefer if 'exclude=origin' was in the reverse sense and perhaps >> called 'report-origin' instead. With the reverse sense it seems to be >> safer if new datastores are defined, where otherwise the behaviour could >> end being under specified. >> > IMO the WG already designed the features so if the functional requirements have changed then the draft should go back to the WG for changes and new WG consensus calls. >> 4. Should there be an option to filter on origin metadata? E.g., only >> include values that come from intended. Otherwise, things like IP >> addresses learned from DHCP may always turn up as differences. >> > IMO the WG already designed the features so if the functional requirements have changedthen the draft should go back to the WG for changes and new WG consensus calls. >> 5. I'm not that keen on the "Possible Future Extensions" section of an >> RFC. Personally, I would prefer that this section is deleted, but if you >> wish to retain it, then please can you move it to an appendix. >> >> OK with me to remove it Andy > >> I've also included some minor comments inline below, and some nits at the >> end: >> >> Abstract >> >> This document defines an RPC operation to compare management >> datastores that comply with the NMDA architecture. >> >> The abstract is perhaps somewhat terse. Perhaps: >> >> This document defines a YANG RPC operation to compare the >> contents of network management datastores that comply with >> the NMDA architecture and return the differences in the >> YANG-Patch format. >> >> >> 1. Introduction >> >> The revised Network Management Datastore Architecture (NMDA) >> [RFC8342] introduces a set of new datastores that each hold YANG- >> defined data [RFC7950] and represent a different "viewpoint" on the >> data that is maintained by a server. New YANG datastores that are >> introduced include <intended>, which contains validated >> configuration >> data that a client application intends to be in effect, and >> <operational>, which contains at least conceptually operational >> state >> data (such as statistics) as well as configuration data that is >> actually in effect. >> >> I would suggest deleting "at least conceptually", since the <operational> >> datastore does contain all operational state, but it may be implemented >> as a virtual construct that spans multiple nodes (e.g., linecards) and >> processes. >> >> >> NMDA introduces in effect a concept of "lifecycle" for management >> data, allowing to clearly distinguish between data that is part of >> a >> configuration that was supplied by a user, configuration data that >> has actually been successfully applied and that is part of the >> operational state, and overall operational state that includes both >> applied configuration data as well as status and statistics. >> >> "allowing to clearly distinguish" => distinguishing" >> "status and statistics" => "status information and statistics" >> >> >> As a result, data from the same management model can be reflected >> in >> multiple datastores. Clients need to specify the target datastore >> to >> be specific about which viewpoint of the data they want to access. >> This way, an application can differentiate whether they are (for >> example) interested in the configuration that has been applied and >> is >> actually in effect, or in the configuration that was supplied by a >> client and that is supposed to be in effect. >> >> Perhaps reword the last sentence to match the logical data flow in the >> server: >> >> For example, a client application can differentiate whether they are >> interested in the configuration supplied to a server and that is >> supposed to be in effect, or the configuration that has been applied >> and is >> actually in effect on the server. >> >> >> When configuration that is in effect is different from >> configuration >> that was applied, many issues can result. It becomes more >> difficult >> to operate the network properly due to limited visibility of actual >> status which makes it more difficult to analyze and understand what >> is going on in the network. Services may be negatively affected >> (for >> example, breaking a service instance resulting in service is not >> properly delivered to a customer) and network resources be >> misallocated. >> >> Perhaps change "actual status" to "actual operational status". >> >> I also suggest changing the last sentence to: >> >> Services may be negatively affected (e.g., degrading or breaking a >> customer service) or network resources may be misallocated. >> >> >> 3. Definitions: >> >> It should probably define that <intended>, <operational>, (and perhaps >> <running>) are used to indicate names of datastores. >> >> It should also explain that <compare> is used as the name of a YANG RPC. >> >> >> 4. Data Model Overview >> >> At the core of the solution is a new management operation, >> <compare>, >> that allows to compare two datastores for the same data. >> >> Suggest rewording this first sentence to: >> >> The core of the solution is a new management operation, <compare>, >> that compares the data tree contents of two datastores. >> >> o target: The target identifies the datastore to compare against >> the >> source. >> >> Suggest adding an example ", e.g., <operational>." >> >> o filter-spec: This is a choice between different filter >> constructs >> to identify the portions of the datastore to be retrieved. It >> acts as a node selector that specifies which data nodes are >> within >> the scope of the comparison and which nodes are outside the >> scope > >
_______________________________________________ netmod mailing list netmod@ietf.org https://www.ietf.org/mailman/listinfo/netmod
