Re: [core] Yangdoctors last call review of draft-ietf-core-sid-15

Alexander Pelov <a@ackl.io> Fri, 09 April 2021 08:29 UTC

Return-Path: <alexander@ackl.io>
X-Original-To: core@ietfa.amsl.com
Delivered-To: core@ietfa.amsl.com
Received: from localhost (localhost [127.0.0.1]) by ietfa.amsl.com (Postfix) with ESMTP id 91A333A17D0 for <core@ietfa.amsl.com>; Fri, 9 Apr 2021 01:29:33 -0700 (PDT)
X-Virus-Scanned: amavisd-new at amsl.com
X-Spam-Flag: NO
X-Spam-Score: -1.897
X-Spam-Level:
X-Spam-Status: No, score=-1.897 tagged_above=-999 required=5 tests=[BAYES_00=-1.9, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, HTML_MESSAGE=0.001, RCVD_IN_DNSWL_BLOCKED=0.001, SPF_HELO_NONE=0.001, SPF_PASS=-0.001, URIBL_BLOCKED=0.001] autolearn=unavailable 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 VbupSvhVFJrN for <core@ietfa.amsl.com>; Fri, 9 Apr 2021 01:29:27 -0700 (PDT)
Received: from mail-io1-xd36.google.com (mail-io1-xd36.google.com [IPv6:2607:f8b0:4864:20::d36]) (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 A4BB63A17CD for <core@ietf.org>; Fri, 9 Apr 2021 01:29:27 -0700 (PDT)
Received: by mail-io1-xd36.google.com with SMTP id a11so3021249ioo.0 for <core@ietf.org>; Fri, 09 Apr 2021 01:29:27 -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 :cc; bh=pRzD2ejc0JllcOYMA4q1fGU/6iVm4FSdMWB7uM6XfOA=; b=FGxKog4OdDxO3Tk4yxZsCtVOjsAN8gC2ybgTC8tpamM9wHwuaoWIH+zeF9+qGRcx4S nKs08V3NuEdo1V0NBDQtqBujlxgSBHudRyZd8x7UYUltafm8FB1P6Wlq+oRMnprNeJvK CcC2nucvvkaP+wZHaCJAaDR2ydKM6y2kItGI2q168TK/2pkU9nMa4S16I5064sJztrtS MhFp8NcRiVBKots7OelOHY4caWZPhvqItjgXc50yJ1ZKEyW1FXw25P9uIgLwU3gEGkLA 6HVIm/sMO7yZSrvIJFyN17/kAJTvDdD0t6EwICJerXB9pE/OoK2byC4Y2XZAug3G5GQC 9V0g==
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=pRzD2ejc0JllcOYMA4q1fGU/6iVm4FSdMWB7uM6XfOA=; b=cG4NN9Lt91fj1/rc+40WmEcfyeyyTLWW9CN2vdwhyKOgXASCt8sizwgOmIdRytN+tq 12ApwJt16dWW+dG9fYFONtNF6vPKMmToVTCrr+unsNKsbGEG1EOv4rN6RjrqNuBnFmqE gjQA8xRjk8AbVoBZb/yGTxLiMZ8ZMGDLuTbW2KeAxU4Oh75aKQD8haQBea0M6jkmaaEa YAnPcuiPWN8U79qcg0NDFQGvpf7LS71hCRqJBU7O43+Pa2sqVDjmSEULJwioIGgLkEy0 Zp28BdMmCYS9U2tp5N1V16A8eb0kX4m2Bbk/q0AN4gG9AWvTC913Q4GX6fSUznjV/uHb 2w6Q==
X-Gm-Message-State: AOAM530LzNDp0IHpNIrDwEsBJISGfgB9GyM8kHpBIfhqYtsHDnynAKgN vuQ8zhSTBaqb8OlAif/zbJCSESNYUko9XFqez6JUOw==
X-Google-Smtp-Source: ABdhPJz2fzt7+mupENvhYMfjcem4Lp/QQ4IB5qB+23ZdrRa1qWEbHdSm07VNWYtUAPxhiK14XOfl+eL4x0OtOZV6J4c=
X-Received: by 2002:a05:6602:2102:: with SMTP id x2mr10927756iox.83.1617956966042; Fri, 09 Apr 2021 01:29:26 -0700 (PDT)
MIME-Version: 1.0
References: <161671562340.18744.12200188901217754567@ietfa.amsl.com> <14851067-3EF0-468F-97C7-0EC12A6ED1AC@ericsson.com>
In-Reply-To: <14851067-3EF0-468F-97C7-0EC12A6ED1AC@ericsson.com>
From: Alexander Pelov <a@ackl.io>
Date: Fri, 9 Apr 2021 10:29:14 +0200
Message-ID: <CACQW0EqGQXank6L21nV3hrv+6TsQtATX4MEHRda4ypi-=dnuoQ@mail.gmail.com>
To: Francesca Palombini <francesca.palombini@ericsson.com>
Cc: Xufeng Liu <xufeng.liu.ietf@gmail.com>, "yang-doctors@ietf.org" <yang-doctors@ietf.org>, "core@ietf.org" <core@ietf.org>, "draft-ietf-core-sid.all@ietf.org" <draft-ietf-core-sid.all@ietf.org>, "last-call@ietf.org" <last-call@ietf.org>
Content-Type: multipart/alternative; boundary="0000000000003923c305bf85f697"
Archived-At: <https://mailarchive.ietf.org/arch/msg/core/MGk6r65xHRfEV-ydosYnV1IDv4Y>
Subject: Re: [core] Yangdoctors last call review of draft-ietf-core-sid-15
X-BeenThere: core@ietf.org
X-Mailman-Version: 2.1.29
Precedence: list
List-Id: "Constrained RESTful Environments \(CoRE\) Working Group list" <core.ietf.org>
List-Unsubscribe: <https://www.ietf.org/mailman/options/core>, <mailto:core-request@ietf.org?subject=unsubscribe>
List-Archive: <https://mailarchive.ietf.org/arch/browse/core/>
List-Post: <mailto:core@ietf.org>
List-Help: <mailto:core-request@ietf.org?subject=help>
List-Subscribe: <https://www.ietf.org/mailman/listinfo/core>, <mailto:core-request@ietf.org?subject=subscribe>
X-List-Received-Date: Fri, 09 Apr 2021 08:29:34 -0000

Dear Xufeng,

Thank you for the detailed feedback. We will take into account all the
remarks you have made and will modify accordingly.

Thank you, Francesca, for following on Xufeng's review.

Best regards,
Alexander


On Fri, Mar 26, 2021 at 5:18 PM Francesca Palombini <
francesca.palombini@ericsson.com> wrote:

> Xufeng,
>
> Thank you very much for the detailed review!
>
> Authors: please address Xufeng's comments together with the other Last
> Call comments received.
>
> Thanks,
> Francesca
>
> On 26/03/2021, 00:40, "Xufeng Liu via Datatracker" <noreply@ietf.org>
> wrote:
>
>     Reviewer: Xufeng Liu
>     Review result: Ready with Nits
>
>     This is a review of the YANG module in draft-ietf-core-sid-15.
>
>     Minor Issues, Nits, and Questions:
>
>     1) This module uses the yang-data extension in RFC8040, which was the
> best
>     choice a few years ago when this draft started. However, RFC8791 has
> been
>     published so that the YANG structure extension is available now. Has
> the YANG
>     structure extension been considered to replace the yang-data extension?
>
>     2) The container sid-file is missing in the tree diagram in Section 4.
> RFC8340
>     specifies the tree format to represent such a yang-data definition. If
> the YANG
>     structure extension in RFC8791 is used, RFC8791 describes how the tree
> diagram
>     looks like for such a YANG structure. Also, the top container would
> not be
>     needed, because a YANG structure is encoded as a 'container'.
>
>     3) In the container sid-file, the leaf module-name is optional. What
> is the
>     assumption when it is not specified? It would be beneficial to clarify
> in the
>     description statement.
>
>     4) In the container sid-file, the leaf sid-file-version is optional.
> The
>     description says that this number starts at zero. Let’s say that there
> are two
>     .sid files, one of which does not have the version number and the
> other one has
>     version number 0. Are they the same? If so, would it be better to have
> a
>     default statement with a default value of 0?
>
>     5) For “list dependency-revision”, the key is module-name. The
> “mandatory
>     true” statement is not necessary for this leaf because it is a key.
>
>     6) Under the “list dependency-revision”, the leaf revision-identifier
> is
>     specified as “mandatory”. What would this leaf be specified when a
> dependent
>     module does not have a revision?
>
>     7) “list assigment-ranges” should be “list assignment-ranges”. A
> letter ‘n’ is
>     missing in the current YANG module because of a typo.
>
>     8) For “list assignment-ranges”, the key is entry-point. The
> “mandatory true”
>     statement is not necessary for this leaf because it is a key.
>
>     9) As a convention, the node names of “list assignment-ranges” and
> “list items”
>     should be in the singular form, the same way as “list
> dependency-revision”, so
>     that they would be “list assignment-range” and “list item”.
>
>     10) Since a YANG SID value is globally unique, would it be beneficial
> to have a
>     statement in the YANG file to describe the requirement formally? This
> can be
>     easily done by adding the following statement under “list items”:
>             unique "sid";
>
>     11) The format of the YANG file needs to be adjusted. Some of the
> lines in the
>     .yang file are longer than 69 characters. For example, at line 108 is:
>            o  Any subsequent schema node name is in the
> namespace-qualified form
>     To examine, the command “pyang --ietf --max-line-length 69 FILE” can
> be used.
>     Before publishing, an RFC editor would normalize the format by using
> the
>     command “pyang -f yang --keep-comments --yang-line-length 69 FILE”. It
> would be
>     helpful to run this command now since it may change the lines to be
> longer than
>     the limit of 69 characters.
>
>     12) In the example in Appendix A, the four "module-revision"
> statements contain
>     “.yang” after the date, not following the pattern rule of the
>     revision-identifier. It seems that the sid generation tool did not
> take out the
>     extension “.yang”.
>
>     13) In Appendix A, the 5th item is:
>      o  iana-crypt-hash@2014-04-04.yang (defined in [RFC7317])
>     but in  RFC7317, the revision of  iana-crypt-hash is 2014-08-06
>
>     14) The following is a thought that may have been discussed before and
> decided
>     by the WG, but I’d mention it here anyway just in case: Since the
> scope of a
>     .sid file is for a single YANG file (module), many of the data nodes
> start with
>     the namespace of this particular module. The “schema-node-path”
> currently
>     requires an identifier string to always start with a namespace.
> Because of this
>     requirement, there are many repeated namespace strings in the
> identifiers of
>     the items. If we assume that the default starting namespace is the
> module
>     associated with the .sid file, we may shorten the .sid file?
>
>     - Xufeng
>
>
>
>
>