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) <[email protected]> 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. > > 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>. > > 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. > > 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. > > 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. > > > 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 [email protected] https://www.ietf.org/mailman/listinfo/netmod
