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

Francesca Palombini <francesca.palombini@ericsson.com> Fri, 26 March 2021 16:18 UTC

Return-Path: <francesca.palombini@ericsson.com>
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 C264D3A2244; Fri, 26 Mar 2021 09:18:35 -0700 (PDT)
X-Virus-Scanned: amavisd-new at amsl.com
X-Spam-Flag: NO
X-Spam-Score: -3.052
X-Spam-Level:
X-Spam-Status: No, score=-3.052 tagged_above=-999 required=5 tests=[BAYES_00=-1.9, DKIMWL_WL_HIGH=-0.251, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, DKIM_VALID_EF=-0.1, RCVD_IN_DNSWL_LOW=-0.7, RCVD_IN_MSPIKE_H2=-0.001, SPF_PASS=-0.001, URIBL_BLOCKED=0.001] autolearn=ham autolearn_force=no
Authentication-Results: ietfa.amsl.com (amavisd-new); dkim=pass (1024-bit key) header.d=ericsson.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 FJAyt2gCV6VS; Fri, 26 Mar 2021 09:18:31 -0700 (PDT)
Received: from EUR04-VI1-obe.outbound.protection.outlook.com (mail-eopbgr80055.outbound.protection.outlook.com [40.107.8.55]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ietfa.amsl.com (Postfix) with ESMTPS id 876873A2242; Fri, 26 Mar 2021 09:18:30 -0700 (PDT)
ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=QfNyNDKJDzdlzLz2DJhIchNTgbLVAedRCdKisNaY8PWD6agJlDvVT6QR8Eo8HbVTgvfF6d4ZNSAqXIaIUz47EA24fZaPenUJt2C1sQsBkqv8HdYcwknowuLfd5qaZPSpOk03SheIT5C3hw7PkZFCVWWrOd8nq6sezNC/birWxj4wHTHyfIYk9Y8G3a4sIBs4uArTl4ebHzbmUFMqTFkDtxzFhhRDuzpFvnYH8UfypuqMcH8THqna6c5WQp0BR1D8LxLovJy+T8hXRWSS3qenPH+RJLjE4hvkX5zGmQeBiTT9HE8K6tUm1aQfI1Y9PtmoinHrd1nn1iwt1pSMMkoPjQ==
ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=GBaVOo3p0KDrrY/FmzQ/rHmeNWMR9RR0FMIEjo5qqMQ=; b=HsfNK7xq/ouBapocUy+L3XvTx+GO8BX5lMgmmIFlykhzhQ29+aRJypQeksUKZE/9cFSxy8tWTZX/dUKzUQGlSKczKQKKWFiTVIGU9EHeaHTpd/LBvYbMx84zXlT5PbxmIP5onefu12NHTPlowbWh3nQD/dOlMreseQxXV5BUVkT43KpJcfoKSbR/BbCZiPgoS0HUgnTDK+U4ex3mLaQn6gI2rWzbM8g5yxS6KxFBqDcdQie1tLHuSMIREl0RSJ5JvAwg4FhbYZABEVQjdfRayNLErKEpD1Umz1YnzFo3q0/QNoICwf6RZ3GD8yKvfBS0PHKGD8nbvVPbUxEMxX3PUw==
ARC-Authentication-Results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=ericsson.com; dmarc=pass action=none header.from=ericsson.com; dkim=pass header.d=ericsson.com; arc=none
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=ericsson.com; s=selector1; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=GBaVOo3p0KDrrY/FmzQ/rHmeNWMR9RR0FMIEjo5qqMQ=; b=Hh9TkiPYCOjdBL6coXa8AecuHvsxJDcV5fm31H7IjXIVHW9/tcr8sW0gn7Hi4PMsgB+xLHW7mZWjQSUGf4pYVUXzXF+w4p/KsrmupEjalFN05btZ7Qoz5OxJn0ozFFf+OE5ExDI1gHiDSSWpPVESv4dRjjf9jh/bCGhVGTFTFz0=
Received: from HE1PR07MB4217.eurprd07.prod.outlook.com (2603:10a6:7:96::33) by HE1PR07MB3452.eurprd07.prod.outlook.com (2603:10a6:7:2f::13) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.3977.10; Fri, 26 Mar 2021 16:18:27 +0000
Received: from HE1PR07MB4217.eurprd07.prod.outlook.com ([fe80::e922:5ae8:48bb:b796]) by HE1PR07MB4217.eurprd07.prod.outlook.com ([fe80::e922:5ae8:48bb:b796%3]) with mapi id 15.20.3977.024; Fri, 26 Mar 2021 16:18:27 +0000
From: Francesca Palombini <francesca.palombini@ericsson.com>
To: Xufeng Liu <xufeng.liu.ietf@gmail.com>, "yang-doctors@ietf.org" <yang-doctors@ietf.org>
CC: "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>
Thread-Topic: Yangdoctors last call review of draft-ietf-core-sid-15
Thread-Index: AQHXIdBDDthO/EBbFE+UdCEW7n1Qj6qWhG8A
Date: Fri, 26 Mar 2021 16:18:27 +0000
Message-ID: <14851067-3EF0-468F-97C7-0EC12A6ED1AC@ericsson.com>
References: <161671562340.18744.12200188901217754567@ietfa.amsl.com>
In-Reply-To: <161671562340.18744.12200188901217754567@ietfa.amsl.com>
Accept-Language: en-GB, en-US
Content-Language: en-GB
X-MS-Has-Attach:
X-MS-TNEF-Correlator:
user-agent: Microsoft-MacOutlook/16.47.21031401
authentication-results: gmail.com; dkim=none (message not signed) header.d=none;gmail.com; dmarc=none action=none header.from=ericsson.com;
x-originating-ip: [2001:1ba8:147a:c100:d0d6:e209:7a1:6bf0]
x-ms-publictraffictype: Email
x-ms-office365-filtering-correlation-id: 36bf2075-7e11-4fc3-a32f-08d8f072c9c7
x-ms-traffictypediagnostic: HE1PR07MB3452:
x-microsoft-antispam-prvs: <HE1PR07MB34527BB8CEB6E1D15858ADD198619@HE1PR07MB3452.eurprd07.prod.outlook.com>
x-ms-oob-tlc-oobclassifiers: OLM:10000;
x-ms-exchange-senderadcheck: 1
x-microsoft-antispam: BCL:0;
x-microsoft-antispam-message-info: IB+7jrCJeHrRDUpEI53JqyhyDJ2lAJY0fZHz0xnSS1Yfp1OJj3A1Lpe77P0hCTTbaxo37XAczzTWffUIVEKTFA4u5cp0sCwVEBPqfRSIuqV1J8eNCr4KcoeCb5seH0ZMAZZYiHmbvsIwd0c4Wt8EDzOlVfPVFk8Zth3tKakzIjJU1Roqfs0TUbpVyRG6pxsgpCCUUUjICqiQF0/WBOXLwdNfFwYORAoT6Jevw7lN4VDdWTzOQ7YrgYpHZmBvRNqQMtF+qErkNL5b3a8BKMCiyWqg07DyuZQI0M1sY6EaLNMU2ME0+nZdHfc30VjCmMh19ruZJh7B1kbc42LfKzBwwGymTTUW3neGImKEU23APi9xPHrtLViUMrDrBBnd9epOnpbv02LaGSBzvCLupQOgFObGWNUtDH3KVI+ufk1CfbgSQoXDald0mfxx5D8TJnVTUV3lmbAwC3QreX2fidSGprADV7uWhtHtqgX4ZhzjRj1OM7LSKuHjbmlKPq69j+RMIKdtwdmKj1J6w1bg6iHXeL/maDFmU6dsKlWDer1ZMZ4rQS4H/IM6UUUz080ooG+fHFau/T4ufcypS/im0NEAOn1h0Zdeeu3/PhuI+MnIjnxWVe/hMPJGhJGbOBKti6kepTQgyN4Iq2hPnIcbOfteuHlHPGx27ZGX8vHLKKl2ySdNRAJ+wDCgbtIn32oyDFE2
x-forefront-antispam-report: CIP:255.255.255.255; CTRY:; LANG:en; SCL:1; SRV:; IPV:NLI; SFV:NSPM; H:HE1PR07MB4217.eurprd07.prod.outlook.com; PTR:; CAT:NONE; SFS:(4636009)(136003)(346002)(366004)(39860400002)(396003)(376002)(66446008)(33656002)(66946007)(86362001)(2616005)(2906002)(8936002)(478600001)(66476007)(83380400001)(66556008)(64756008)(76116006)(186003)(71200400001)(4326008)(8676002)(6506007)(5660300002)(36756003)(44832011)(6486002)(38100700001)(110136005)(54906003)(316002)(6512007)(45980500001); DIR:OUT; SFP:1101;
x-ms-exchange-antispam-messagedata: =?utf-8?B?Tis3YVh2R2pFd3ltMmZ5OTdXcWVvWUFIczk4S2s3TjVzTk00UDhWR1YrOXoz?= =?utf-8?B?SVkyV1RrY1N4MHJuQnhPQmRkY2M3UTV3SmRKSXdtTVl6TGRVU3Q4U2pDZC9V?= =?utf-8?B?dWxDR3Q2RjFUd1VFcVMrN3RXWS9zTVB3YWFHbUpBU2dOckR3YkdVcVdwSDM4?= =?utf-8?B?eTI3TlJHamlpUkRhWEgyaG9tREFCaUk5OStmUWdyTEJ0VmVqbkNtVFVPUHk5?= =?utf-8?B?L3REaWdjbk9OWDlLMUc1VEtybU9CUkd3S2VCcG9HZ0dHb3NmNi9PTlpVcy82?= =?utf-8?B?ZGdOeDNHeTVORDloMWhGT2V1NzQrZFNUVXdYbmt6WUNNN0dwUVNOTlpzQ1Z3?= =?utf-8?B?dXNWODIvQVEyMXRxaUFrSDBZN0JldUhXZmlnVk4wT0p3bzNUYmJwaXFqN1Nu?= =?utf-8?B?eHU3d0pmMFZhMDVkd0tyU0NJSHljMkwvRzQ2aGJiYmpLaUlGWHpzaTRFSG9j?= =?utf-8?B?S2hTNXhUU25WS1NTRzlPeGNyaDlGcTF1Y1pOVi9FdGlTc1hxVzl0SndrM0hG?= =?utf-8?B?U3liVDN5UllQT2FVb3Fvak5nM3JnRWFqY3EyQXUrWTl2TU9mV0k2anZOQTF1?= =?utf-8?B?cVJTaWFLV3ZmSE95MkRoVkpXcDlMUU5JcFkxeFNHVGVDMTYxeTZnNWRJcXlR?= =?utf-8?B?S2REbUttYmh6NGRwSmZCV0p4cWc2NzJJbnlZQ25ERFBTcXZiOGpHZHVNZzJN?= =?utf-8?B?NVorQ3p6R3huckRJajFNajlSWS9BeE81MlFBbGJQK0lheUpNSWNhY3RVOVFW?= =?utf-8?B?djh1T0ZZSHJMa1U1cHFha2k2WHluWEY2L3FCOE91KzBVQTFJNHdXYitxSlFS?= =?utf-8?B?Wm9ZTzB1allUdFloY3RlRi9mZ2QzeE1SVHFuQkZUbmdVVzIwcEFVa3J2Wlgy?= =?utf-8?B?UEYxWThFazNKUkJEb0FLa0pJdFRRalRZc1JRL3pSclFZY1dLblZTbVZXblho?= =?utf-8?B?TEhXSE8xTEJoNzFVZFMwblN1ZGdPc1lmT05yTU9LT3c1dHJWSG5JOXNYTTBL?= =?utf-8?B?L2JDd25WNExzWGtjYTVZZjZLV1RyU0tQcFVKY29lalQraHhHUHoyUWVGeitl?= =?utf-8?B?QlRmNEIyaFFLZWc2WWRSWWhBYXRVMlNwRTEvY05xOVEzOWRnSzVtemF6dFNH?= =?utf-8?B?OWdOT2JlWHpBNkt3ZlllTjVKYmVEaklWMHhXaXQ1N2VCL1BUaEU4TFNhb3VO?= =?utf-8?B?NDFwWmVJSm8rbkhSdStEcW9naTVZRDNUYjlkZWRzMG5oTzdEd1cwblU1QXZL?= =?utf-8?B?dFhPZ3JsbVVpaXdNc1BTeFpQSUVNbkw5OEZ2YTVUeGFPUFMwOHIxWFZoN1h6?= =?utf-8?B?R1NIV3lDblJUbCs1R0JkM25IdEEzODR3ODVENlpuTGhTdUVuYit6b3VFZXJ4?= =?utf-8?B?NjBHNis5VHNyTHdvQ2FlTjA3bDNEMVNKcW9nNDJzWlE2S1lIbUVUckdXZzRl?= =?utf-8?B?b3R4Q2JYQVN6RHIrU2tkTFZvb3RJTHN6N0twKzJnOE42bE9pZitiWjVsNnZG?= =?utf-8?B?NXg1N2R1MDJERkhEc3F5dGhuMEFMWUZoUUltWGZFZEN2Znc0dDJtVUl0Smh3?= =?utf-8?B?eTBObVV6Uzk1dFhFZTJ6TWszMEtMQzY4R2FTZnp2MnVxZVBYVkMyWndDUlVS?= =?utf-8?B?NExEUTJZVTdINVp6cTBUNldzenJOS1hyKzRadnNKSkZreHRUdDloVnQ1M0Ew?= =?utf-8?B?N04wQ0ZVZFQxdnU4NFl1NnVnTjNtZ2lkdmVhSi90bUNRU3NTaXBoY2Y2Q0NX?= =?utf-8?B?QWtLdWQ4UGVZYTF3RlBJZzZ0ZUZodEROcVpLS0h2ZkxWb0EycHI3MGN0SFJE?= =?utf-8?B?c0xydHR6aEE5Q2E2MkFkYXJVTFVaVk5lTzhLTFNVd0xIdEppdmpJQ1kxUzYr?= =?utf-8?B?ai9ubkt3dldaUERCd1lRMzFnSnJnZXQwZysxNlRXN0JQMkE9PQ==?=
x-ms-exchange-transport-forked: True
Content-Type: text/plain; charset="utf-8"
Content-ID: <622F150154BE58408B70555A93E28D19@eurprd07.prod.outlook.com>
Content-Transfer-Encoding: base64
MIME-Version: 1.0
X-OriginatorOrg: ericsson.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-AuthSource: HE1PR07MB4217.eurprd07.prod.outlook.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 36bf2075-7e11-4fc3-a32f-08d8f072c9c7
X-MS-Exchange-CrossTenant-originalarrivaltime: 26 Mar 2021 16:18:27.3328 (UTC)
X-MS-Exchange-CrossTenant-fromentityheader: Hosted
X-MS-Exchange-CrossTenant-id: 92e84ceb-fbfd-47ab-be52-080c6b87953f
X-MS-Exchange-CrossTenant-mailboxtype: HOSTED
X-MS-Exchange-CrossTenant-userprincipalname: laF3HAkz78xMgLEcNh/8QNAnfCNM4Q3dkM5keSs4Jy5qb0IsiQzVU2f+7P4z4Cv614T4OPJXKa2kBDj9MkdIx0qMVQQM394xBw6H6fWVO0a/6u7S4Y3SGHRlrDLQ4UNT
X-MS-Exchange-Transport-CrossTenantHeadersStamped: HE1PR07MB3452
Archived-At: <https://mailarchive.ietf.org/arch/msg/core/quYMJ7CwOHk0bPqSQxBXYkW44Dc>
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, 26 Mar 2021 16:18:36 -0000

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