On Fri, Mar 5, 2021 at 10:18 AM Rob Wilton (rwilton) <[email protected]>
wrote:
> Hi Andy,
>
>
>
> I’m not sure which one you think is s a design change: Do you mean issue
> 3 or issue 4 below?
>
>
>
> I see that my response to issue 4 may not have been clear, so to clarify:
>
>
>
> By “okay”, I meant, that I am okay with how it is written in the current
> draft. My presumption is that this could be addressed as a future version
> of the module if this turns out be an issue, or vendors can define their
> own augmentation if needed.
>
>
>
> If you think issue 3 is a design change that requires WG consensus that I
> will leave it to the WG chairs to decide if they wish to issue a consensus
> call for it.
>
>
>
The change:
Current: default is to include origin attributes and client adds
exclude-origin leaf to turn this off
Proposed: default is to exclude origin attributes and client adds
report-origin leaf to turn this on
Also, report-origin has an if-feature because origin support in NMDA is
optional.
I have no objections to this proposal.
My point all along has been that this is not my decision to make, it is a
WG decision.
It does not seem that there are any objections to making this change.
Regards,
>
> Rob
>
>
>
Andy
>
>
> *From:* Andy Bierman <[email protected]>
> *Sent:* 05 March 2021 16:36
> *To:* Rob Wilton (rwilton) <[email protected]>
> *Cc:* joel jaeggli <[email protected]>;
> [email protected]; [email protected]
> *Subject:* Re: AD review of draft-ietf-netmod-nmda-diff-07
>
>
>
>
>
>
>
> On Fri, Mar 5, 2021 at 5:58 AM Rob Wilton (rwilton) <[email protected]>
> wrote:
>
> Hi Andy, authors,
>
>
>
>
>
>
>
> I think you mean to address this to the WG since the redesign issues need
> WG approval.
>
> I have no objections to any changes.
>
>
>
>
>
> Andy
>
>
>
> Sorry for the long delay in replying.
>
>
>
> Please see [RW] inline below …
>
>
>
>
>
> *From:* Andy Bierman <[email protected]>
> *Sent:* 30 October 2020 01:43
> *To:* joel jaeggli <[email protected]>
> *Cc:* Rob Wilton (rwilton) <[email protected]>;
> [email protected]; [email protected]
> *Subject:* Re: AD review of draft-ietf-netmod-nmda-diff-07
>
>
>
>
>
>
>
> On Thu, Oct 29, 2020 at 6:09 PM joel jaeggli <[email protected]> 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) <[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.
>
>
>
> Do you have a section in mind and suggested text?
>
> *[RW] *
>
> *Perhaps somewhere in section 4, either as part of the description of
> source, or perhaps before the parameters are described.*
>
>
>
> *Proposed text:*
>
> *“A client can discover which datastores a server supports by reading YANG
> library [RFC 8525] from the operational state datastore.”*
>
>
>
>
>
>
>
> 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>.?
>
>
>
> *[RW] *
>
> *My main concern is that if you have template expansion then comparing
> <running> and <operational> may not really give a meaningful comparison,
> since <running> is pre-template expansion, and <operational> (and
> <intended>) are both post template expansion.*
>
>
>
> *I would suggest putting some text in section 4 or perhaps the YANG
> module.*
>
>
>
> *Perhaps some text, something like: *
>
>
>
> * “Clients should to be aware that comparing <running> to <operational>
> will report differences due to any configuration transformation (e.g.,
> inactive configuration, or the expansion of templates) between the
> <running> and <intended> datastores. In these scenarios, clients may get a
> more useful result by comparing the <intended> and <operational> datastores
> instead.”*
>
>
>
>
>
>
>
>
>
> 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.
>
> *[RW] *
>
>
>
> *I don’t see this as really changing the functional requirements, but just
> changing the default sense and name of an API parameter. Although, given
> my comments below “with-origin” might be better than “report-origin”.*
>
>
>
> *In RFC 8526, the “with-origin” parameter is off by default, and origin
> metadata is only included when the parameter is included. This keyword is
> also under a feature.*
>
>
>
> *So, changing the parameter name to “with-origin” and putting it under
> ”if-feature ietf-netconf-nmda:origin”, and making the default off, would
> make the definition more consistent with RFC 8526.*
>
>
>
>
>
>
> 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.
>
>
>
> *[RW] *
>
>
>
> *Okay.*
>
>
>
> *Regards,*
>
> *Rob*
>
>
>
>
>
>
> 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
[email protected]
https://www.ietf.org/mailman/listinfo/netmod
