Re: [netmod] [core] js review of draft-ietf-core-sid-12

Ivaylo Petrov <ivaylo@ackl.io> Wed, 15 April 2020 13:27 UTC

Return-Path: <ivaylo@ackl.io>
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 A5BF23A0795 for <netmod@ietfa.amsl.com>; Wed, 15 Apr 2020 06:27:54 -0700 (PDT)
X-Virus-Scanned: amavisd-new at amsl.com
X-Spam-Flag: NO
X-Spam-Score: -1.898
X-Spam-Level:
X-Spam-Status: No, score=-1.898 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, SPF_PASS=-0.001, URIBL_BLOCKED=0.001] autolearn=ham autolearn_force=no
Authentication-Results: ietfa.amsl.com (amavisd-new); dkim=pass (2048-bit key) header.d=ackl-io.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 2eSGpwicQeZu for <netmod@ietfa.amsl.com>; Wed, 15 Apr 2020 06:27:52 -0700 (PDT)
Received: from mail-wr1-x433.google.com (mail-wr1-x433.google.com [IPv6:2a00:1450:4864:20::433]) (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 9ACBE3A082F for <netmod@ietf.org>; Wed, 15 Apr 2020 06:27:49 -0700 (PDT)
Received: by mail-wr1-x433.google.com with SMTP id d27so9960886wra.1 for <netmod@ietf.org>; Wed, 15 Apr 2020 06:27:49 -0700 (PDT)
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=ackl-io.20150623.gappssmtp.com; s=20150623; h=mime-version:references:in-reply-to:from:date:message-id:subject:to; bh=MPsGZp5jkhumT8uMvLA60Hq3iygUFDHYDVfalIc/G1M=; b=b/nkBFHjBtUGCTwtQPGUNiC3AqeaiLhcbeP+O6tm+DXxmSFpOR/1vGsqGZzN3VVIP7 mfWj8V82aRJnv2fT13dGPV85GtLje3vQ4hL/iM6P3h8ltH69y43dE/Fey0AcARipjp4U bhbuxoIKRtPABavCFUGO+5UiaWZtCiZwINC4VEybor1ND6zYuFcRheaJsBdbo4yBo5X5 s49Z40pZdXVQww+hkfNQYsn+V6NnYDZTdfiZ0jesXmr+fIjb9Iu52MrFFVY+l/FAEmY3 uXV38QLa2mwpJ+cV2v0BEx5BExeJyKr/fnZ8XmKyBgUA6dWhInWfxol7gCcgCYBeiiPU Kt2Q==
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; bh=MPsGZp5jkhumT8uMvLA60Hq3iygUFDHYDVfalIc/G1M=; b=h2WTmSodE5azANsUlpebobTOJwwTpRm8Y9OZdXZuiPb/dSNXgC0yqyW1qSw4ZHP7SJ 3KSwSDkz6EmkMaauFAHC8QOyX7WmlQi6NeIYEqINT1xmEJBajO5yrXgkLzsrXyxXBhjp Fmb8o6aHHCbXW5IsGpcIArKW6D4t4pKIfRntUopAY53UIP6Qg396hkHKIeyM6Mf6dnvj 6jMlkoFgaCYUsDmoOumi8BgfpwSmQt2zFCTuE+vghArJ1xlu1bgRNMBZswo7YLjW2xgN fo80HX2AjsRSqnZ6K/XYawRWf1B7sftSfpRMFWiqbKlgrDMrraMx5GxKnDj1cpgBxfKz F2FQ==
X-Gm-Message-State: AGi0PubiYsN/7Gu1z2D3Ce5JqTUZLx+djMQQz5pYCRVxnAJUaV3mmuX+ 40PGGw5nv5/Nr6m6LIRYbTuN2QzrRLOMsoLmnQrqtw==
X-Google-Smtp-Source: APiQypLbwlQH1/dAvHyIgldnMKLTXMwInP49WJ1CY75GDYuF1krgUmGfydI7XDBe8cI1E+2jnqIBFj6SiNCzlW0GB8U=
X-Received: by 2002:adf:e681:: with SMTP id r1mr7645390wrm.213.1586957267803; Wed, 15 Apr 2020 06:27:47 -0700 (PDT)
MIME-Version: 1.0
References: <20200330213129.m2azrbeaxrtgivfc@anna.jacobs.jacobs-university.de>
In-Reply-To: <20200330213129.m2azrbeaxrtgivfc@anna.jacobs.jacobs-university.de>
From: Ivaylo Petrov <ivaylo@ackl.io>
Date: Wed, 15 Apr 2020 15:27:21 +0200
Message-ID: <CAJFkdRz445b4n86ug=v1ruYYWbDjwnEJwUNCZvEzENu_gMV0bg@mail.gmail.com>
To: Juergen Schoenwaelder <j.schoenwaelder@jacobs-university.de>, core <core@ietf.org>, NetMod WG <netmod@ietf.org>
Content-Type: multipart/alternative; boundary="00000000000038b8e805a354486b"
Archived-At: <https://mailarchive.ietf.org/arch/msg/netmod/SqrtoPqD0MksT0Wp1aXqln1_Q2w>
Subject: Re: [netmod] [core] js review of draft-ietf-core-sid-12
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: Wed, 15 Apr 2020 13:27:57 -0000

Hello Juergen,

Thank you for your review and your comments! They really help us improve
this document. Please find my answers below (prefixed with [IP]). Note that
the diff after handing your comments and those of Esko Dijk and Tom Petch
can be found at [1].

Best regards,
Ivaylo

[1]:
https://tools.ietf.org/rfcdiff?url1=draft-ietf-core-sid&url2=http://core-wg.github.io/yang-cbor/draft-ietf-core-sid-latest.txt

On Mon, Mar 30, 2020 at 11:32 PM Juergen Schoenwaelder <
j.schoenwaelder@jacobs-university.de> wrote:

> Hi,
>
> I have reviewed draft-ietf-core-sid-12. I will try to review yang-cbor
> tomorrow as well. (I am probably more interested in CBOR encoding for
> RESTCONF than the usage of CoAP itself, hence my priority on these two
> IDs under last call.)
>
> - Avoid the use of the word 'template', which is not a well defined
>   term and may be used with varying interpretations.
>
>    o  data nodes (Note: including those parts of a YANG template as
>       defined by the 'yang-data' extension.)
>
>   It is not clear to me what is meant with "those parts of a YANG
>   template as defined by the 'yang-data' extension." I think this
>   could benefit from a rewording.
>

[IP]: Reworded.

- The ID seems to assume that semantics of yang items never change.
>   This is true so far but NETMOD has chartered work that might change
>   this property. So what happens if the semantics of a YANG item
>   changes?
>
>    SIDs are assigned permanently, items introduced by a new revision of
>    a YANG module are added to the list of SIDs already assigned.
>
>   If a YANG module changes in a non-backwards compatible way, I assume
>   a new sid range must be allocated? Strictly speaking, this question
>   does not have to be answered today but it very likely needs an
>   answer in the future...
>

[IP]: We will not be able to clearly answer this before there is more
information how the YANG items semantics can change. For now it looks like
assigning new range would be a good solution, but maybe there will be some
other solutions that will be even more optimal. What looks logical is that
at least every semantic of an item should have a separate SID.

- The definition of 'path' is not very precise, i.e., it does not
>   spell out how module namespaces work, only by example.. I do not
>   know whether a definition can be imported from somewhere.
>

[IP]: I tried to improve it. Please let me know if now it is clear enough.

- s/RESCONF/RESTCONF/
>

[IP]: Fixed.

- Is it CoRECONF or CORECONF? And I find the term CORECONF confusing.
>   We have two protocols called NETCONF and RESTCONF and now we add
>   another protocol called CoMI and we call CoMI together with YANG
>   CBOR and SIDs CORECONF?
>
>   1) NETCONF  + YANG + XML      serialization + path naming -> ?
>   2) RESTCONF + YANG + XML|JSON serialization + path naming -> ?
>   3) CoMI     + YANG + CBOR     serialization + SID naming  -> CORECONF
>
>   We do not have a term for 1) and 2) and then we have a term for 3)
>   which, however, looks more like the protocol names used in 1) and
>   2). This comment is not specific to this ID, but the asymmetry
>   showed up while reading the SID document, I had to look at other IDs
>   to understand how things are named. And the SID document says
>
>    YANG is a language designed to model data accessed using one of the
>    compatible protocols (e.g.  NETCONF [RFC6241], RESCONF [RFC8040] and
>    CoRECONF [I-D.ietf-core-comi]).
>
>   Then I read the CoMI abstract. It first says CoMI is "a CoAP
>   Management Interface", it then says "The complete solution composed
>   of CoMI, [I-D.ietf-core-yang-cbor] and [I-D.ietf-core-sid] is called
>   CORECONF." and finally it states that "CORECONF extends the set of
>   YANG based protocols, NETCONF and RESTCONF, with the capability to
>   manage constrained devices and networks.". So I am confused, is
>   CORECONF a protocol as stated in this document? Or is CoMI a
>   protocol? (What is then the difference between a "Management
>   Interface" and a management protocol?) I am not sure whether I get
>   to review comi, hence I mention my confusion here as I hit it while
>   reviewing the sid document.
>

[IP]: Currently this is indeed somewhat confusing. The proposed change from
Michael Richardson was to at least have CORECONF in the title of the CoMI
document. I am wondering if that might still leave some of the confusion.
For me the simple solution is in this document to refer to CoMI, not
CORECONF and let CoMI draft define what CORECONF actually is. Unless you
think this will still not resolve the issue, this is going to be my way
forward.

- This description makes little sense to me:
>
>   typedef sid-file-version-identifier {
>     type uint64;
>     description
>       "Optional attribute that gives information about the .sid file
>        version.";
>   }
>
>   This is a type definition. Why does the description talk about an
>   optional attribute? The type should not state whether something
>   using the type is optional or not. (And I would prefer to avoid
>   'attribute', better use YANG defined terms or just describe that
>   this type represents a version number for a SID file.)
>

[IP]: I believe now it should be more clear.

- sid range - was it not said before that it is 63 bits? Is the idea
>   that sids with the highest bit set are legal but undefined or
>   reserved?  Or should there be a range restriction?
>
>   typedef sid {
>     type uint64;
>     description
>       "YANG Schema Item iDentifier";
>     reference
>       "[I-D.ietf-core-sid] YANG Schema Item iDentifier (SID)";
>   }
>

[IP]: I set the range accordingly.

- schema-node-path
>
>       "Identifies a schema-node path string for use in the
>        SID registry. This string format follows the rules
>        for an instance-identifier, as defined in RFC 7959,
>        except that no predicates are allowed.
>
>   RFC 7959 seems to be a typo, I assume you mean RFC 7951
>

[IP]: Yes, thank you for spotting this!

  s/Identifies a schema-node path string/A schema-node path"
>
>   It is a bit confusing to define a schema-node path by way of
>   reference to an instance identifier. I understand that you borrow
>   the namespace encoding from the way JSON encode instance identifiers
>   but this type really represents what RFC 7950 calls an absolute
>   schema node identifier, no? Is the term schema-node path actually
>   needed or is it the same as absolute schema node identifier? Or is
>   the difference between the two how namespaces are represented?
>

[IP]: I might have misunderstood something, but my understanding is that
the prefix related to a module could be changed during an import, whereas
here we really want to use the module name as a more stable identifier. The
difference between absolute schema node identifier and schema-node path is
that we mandate the use of module name and not prefix as defined in RFC
7950.

- dependency-revision
>
>         list dependency-revision {
>         key "module-name";
>
>         description
>           "Information about the revision of each YANG module that the
>           module in
>           'module-name' includes used during the .sid file generation.";
>
>    The sentence can probably be rewritten to make it clearer.
>
>    Are the SIDs assigned to a module dependent on the modules listed
>    in the 'dependency-revision'? (Is this a good name for the list?)
>    Why is it necessary to know which revisions were used to resolve
>    imports without revision?
>

[IP]: I reformulated it to

Information about the used revision during the .sid file generation
of each YANG module that the module in 'module-name' imported.


The idea is that if this information is not available later on and
different versions of the modules are used, slightly different results
might be produced due to augmented items for example.

- Please avoid wrapped entry point numbers in the table in 6.3.3.
>

[IP]: Fixed.

- The handling of early allocations sounds complex, I have some doubts
>   this process will work in practice...
>

[IP]: We are working on reformulating it in a more precise manner.

- Incomplete sentence:
>
>   RFC7120] also says
>
>   Or is the following paragraph a quote? In that case, add a colon.
>
> - There are likely more normative references, e.g., RFC 6991 and RFC
>   8040.
>

[IP]: Fixed.

- Why does the example in appendix A not have / need
>   dependency-revisions?
>

[IP]: Added now.

- I have not run any tools to validate things. I just read the I-D.


> /js
>
> --
> Juergen Schoenwaelder           Jacobs University Bremen gGmbH
> Phone: +49 421 200 3587         Campus Ring 1 | 28759 Bremen | Germany
> Fax:   +49 421 200 3103         <https://www.jacobs-university.de/>
>
> _______________________________________________
> core mailing list
> core@ietf.org
> https://www.ietf.org/mailman/listinfo/core
>