Re: [netmod] AD review of draft-ietf-netmod-nmda-diff-07

Andy Bierman <andy@yumaworks.com> Fri, 05 March 2021 16:36 UTC

Return-Path: <andy@yumaworks.com>
X-Original-To: netmod@ietfa.amsl.com
Delivered-To: netmod@ietfa.amsl.com
Received: from localhost (localhost [127.0.0.1]) by ietfa.amsl.com (Postfix) with ESMTP id B7DE93A27D2 for <netmod@ietfa.amsl.com>; Fri, 5 Mar 2021 08:36:03 -0800 (PST)
X-Virus-Scanned: amavisd-new at amsl.com
X-Spam-Flag: NO
X-Spam-Score: -1.887
X-Spam-Level:
X-Spam-Status: No, score=-1.887 tagged_above=-999 required=5 tests=[BAYES_00=-1.9, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, HTML_MESSAGE=0.001, SPF_HELO_NONE=0.001, T_SPF_PERMERROR=0.01, URIBL_BLOCKED=0.001] autolearn=unavailable autolearn_force=no
Authentication-Results: ietfa.amsl.com (amavisd-new); dkim=pass (2048-bit key) header.d=yumaworks-com.20150623.gappssmtp.com
Received: from mail.ietf.org ([4.31.198.44]) by localhost (ietfa.amsl.com [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id 1G5Q9JeA1K07 for <netmod@ietfa.amsl.com>; Fri, 5 Mar 2021 08:36:00 -0800 (PST)
Received: from mail-lj1-x22a.google.com (mail-lj1-x22a.google.com [IPv6:2a00:1450:4864:20::22a]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by ietfa.amsl.com (Postfix) with ESMTPS id 16D803A27BE for <netmod@ietf.org>; Fri, 5 Mar 2021 08:36:00 -0800 (PST)
Received: by mail-lj1-x22a.google.com with SMTP id r25so3511669ljk.11 for <netmod@ietf.org>; Fri, 05 Mar 2021 08:35:59 -0800 (PST)
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=yumaworks-com.20150623.gappssmtp.com; s=20150623; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=hUUB/S8SOJrf+x+O0JfuZEDDgk+jOPZ7TVW0CONWCMQ=; b=pinVNJ+4yntrBvW+Ei4RaEgYL6m3foTRQfcsrU2x0FHrQC8lHZ5REoDMDaHDaif35j vu2uI8PGJ/XU186juKVtIXtVWzDbYWbBdRmlDt1W2hGKe01+TTzmlw3boJS53WR8Ig6w 3B+u7Uv4z8fGVfaXGmtJ7PyeJUiE2yOHkdIkdafswCdlFr2W0i0X9dYoIPBT7uUe+4P3 /Og2+y6uG/ikSvRryIEfWJfR4QBbXBmQIlz3Q4Sr6MplMTeI+hRbwaZMFDzAceMuQEft xvlNqXM9liOkZFHGie7HbeI4m/BHQmN7Fvpj6xKUBCHSJvK+o9Dyw47I7IQRRAKq/Ngo PEQQ==
X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=hUUB/S8SOJrf+x+O0JfuZEDDgk+jOPZ7TVW0CONWCMQ=; b=moQo7S5TPd7ZPmQpDKjJ6yZ8tg5/aFBYHnE4RxcVkTWjpL+69J6R8Kr0301abyAhep MSaAQIhUNZvBULi89kJlK7a5xL6UAbeBaO3hgM288ckXoLbLPTrFXp4MYe2yzF46BkCa z+HVWT/tmFGAvLHeADSnp5np1SrJs72kX5UVNasuA9H0dbO6dzj4cxXwG9WU+u2P9Kq+ OKIFOoTKAwHNks/cp/ak3I38eHejyVtt95YgjnUyqr28WKHl1bfKcMd8EZfHKPcM2p3U +3hiUmqFbfJumXmAnuK6Y/koh8vErO4kOBzBYQ2i0Xa4kFyVi6Jv2pSBQPhAkApDldk8 Wf7g==
X-Gm-Message-State: AOAM533TqHeVSg+gnOD75t03Rb4UswKycqfO8rXCMyp4Ic2upFwxBZBj DAA20CZsM9/FN7aSPFGMqwMlZBeJe4AyFMRYH5ksig==
X-Google-Smtp-Source: ABdhPJy2RTQANMpDLVIDyiaGR7NdQ1bsLAzZSFbB4M+T7SLJ9fXx57GHwuyCKhC9EBd4LkUsM37dM7tOgPifXFYT3m8=
X-Received: by 2002:a2e:9d41:: with SMTP id y1mr5942757ljj.91.1614962153176; Fri, 05 Mar 2021 08:35:53 -0800 (PST)
MIME-Version: 1.0
References: <MN2PR11MB43662C6DC8C0E541D42DBF7CB5140@MN2PR11MB4366.namprd11.prod.outlook.com> <CAA8XPEHqN-z=K2q0-DqEE=EJvCAHMH8X9-eUxnfYpacLj8r8Gg@mail.gmail.com> <CABCOCHTEJKvchg7OtuJgJ=VjAGdtH0we=5WDWUFfhkcLBfQ2uw@mail.gmail.com> <MN2PR11MB43667D00F54AB5879D3036C9B5969@MN2PR11MB4366.namprd11.prod.outlook.com>
In-Reply-To: <MN2PR11MB43667D00F54AB5879D3036C9B5969@MN2PR11MB4366.namprd11.prod.outlook.com>
From: Andy Bierman <andy@yumaworks.com>
Date: Fri, 5 Mar 2021 08:35:42 -0800
Message-ID: <CABCOCHTZLQ7ktEbHJn61pfBM-2-U_jQSoG=ajTG-PCXWFtnLFg@mail.gmail.com>
To: "Rob Wilton (rwilton)" <rwilton@cisco.com>
Cc: joel jaeggli <joelja@gmail.com>, "draft-ietf-netmod-nmda-diff.all@ietf.org" <draft-ietf-netmod-nmda-diff.all@ietf.org>, "netmod@ietf.org" <netmod@ietf.org>
Content-Type: multipart/alternative; boundary="0000000000007753a705bcccadc5"
Archived-At: <https://mailarchive.ietf.org/arch/msg/netmod/AJxlxWfuULiaC1sDEevqSjYApwo>
Subject: Re: [netmod] AD review of draft-ietf-netmod-nmda-diff-07
X-BeenThere: netmod@ietf.org
X-Mailman-Version: 2.1.29
Precedence: list
List-Id: NETMOD WG list <netmod.ietf.org>
List-Unsubscribe: <https://www.ietf.org/mailman/options/netmod>, <mailto:netmod-request@ietf.org?subject=unsubscribe>
List-Archive: <https://mailarchive.ietf.org/arch/browse/netmod/>
List-Post: <mailto:netmod@ietf.org>
List-Help: <mailto:netmod-request@ietf.org?subject=help>
List-Subscribe: <https://www.ietf.org/mailman/listinfo/netmod>, <mailto:netmod-request@ietf.org?subject=subscribe>
X-List-Received-Date: Fri, 05 Mar 2021 16:36:05 -0000

On Fri, Mar 5, 2021 at 5:58 AM Rob Wilton (rwilton) <rwilton@cisco.com>
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 <andy@yumaworks.com>
> *Sent:* 30 October 2020 01:43
> *To:* joel jaeggli <joelja@gmail.com>
> *Cc:* Rob Wilton (rwilton) <rwilton@cisco.com>om>;
> draft-ietf-netmod-nmda-diff.all@ietf.org; netmod@ietf.org
> *Subject:* Re: AD review of draft-ietf-netmod-nmda-diff-07
>
>
>
>
>
>
>
> On Thu, Oct 29, 2020 at 6:09 PM joel jaeggli <joelja@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) <rwilton@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?
>
> *[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
>
>