Re: [yang-doctors] Yangdoctors early review of draft-ietf-lisp-yang-11

"Alberto Rodriguez Natal (natal)" <natal@cisco.com> Tue, 20 August 2019 18:30 UTC

Return-Path: <natal@cisco.com>
X-Original-To: yang-doctors@ietfa.amsl.com
Delivered-To: yang-doctors@ietfa.amsl.com
Received: from localhost (localhost [127.0.0.1]) by ietfa.amsl.com (Postfix) with ESMTP id 338FA12018D; Tue, 20 Aug 2019 11:30:14 -0700 (PDT)
X-Virus-Scanned: amavisd-new at amsl.com
X-Spam-Flag: NO
X-Spam-Score: -14.5
X-Spam-Level:
X-Spam-Status: No, score=-14.5 tagged_above=-999 required=5 tests=[BAYES_00=-1.9, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, RCVD_IN_DNSWL_HI=-5, SPF_PASS=-0.001, URIBL_BLOCKED=0.001, USER_IN_DEF_DKIM_WL=-7.5] autolearn=ham autolearn_force=no
Authentication-Results: ietfa.amsl.com (amavisd-new); dkim=pass (1024-bit key) header.d=cisco.com header.b=BZESNRc5; dkim=pass (1024-bit key) header.d=cisco.onmicrosoft.com header.b=lKi/XsWo
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 WFd4ApLam99h; Tue, 20 Aug 2019 11:30:11 -0700 (PDT)
Received: from alln-iport-7.cisco.com (alln-iport-7.cisco.com [173.37.142.94]) (using TLSv1.2 with cipher DHE-RSA-SEED-SHA (128/128 bits)) (No client certificate requested) by ietfa.amsl.com (Postfix) with ESMTPS id 1990A12011C; Tue, 20 Aug 2019 11:30:11 -0700 (PDT)
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=cisco.com; i=@cisco.com; l=8048; q=dns/txt; s=iport; t=1566325811; x=1567535411; h=from:to:cc:subject:date:message-id:references: in-reply-to:content-id:content-transfer-encoding: mime-version; bh=1P/aJy0HcERVvNaZEmdLzH0CKnkSCHUi5AuZXQDaovk=; b=BZESNRc5Fp5txFD0SlF6WT6Csa8Z/yXuECp9Kh1qdwYLPog7+8GrkCHL 7aBLqaHEWp/mYEhxRBcaanBq6Ks1dC5Lo0gCote5neXzmT3c9pbTzCypt a3Vr1Gd4gNZO9yl0dJBd0KLOnDbxr8xNge+zLtD6wIKwm7yLtPo5gROaY c=;
IronPort-PHdr: =?us-ascii?q?9a23=3ALJ8ZBh+JMbmaU/9uRHGN82YQeigqvan1NQcJ65?= =?us-ascii?q?0hzqhDabmn44+/bB7E/fs4iljPUM2b8P9Ch+fM+4HYEW0bqdfk0jgZdYBUER?= =?us-ascii?q?oMiMEYhQslVciMFUT/BPXrdCc9Ws9FUQwt8g=3D=3D?=
X-IronPort-Anti-Spam-Filtered: true
X-IronPort-Anti-Spam-Result: =?us-ascii?q?A0AWAACnO1xd/5NdJa1dCRoBAQEBAQI?= =?us-ascii?q?BAQEBBwIBAQEBgVUDAQEBAQsBgURQA21VIAQLKoQfg0cDinyCN4EjlmeBLoE?= =?us-ascii?q?kA1QJAQEBDAEBHw4CAQGEPwIXgj4jNgcOAgUBAQQBAQECAQYEbYUnDIVLAgE?= =?us-ascii?q?DEhERDAEBJRIBDwIBCA4MAiYCAgIwFRACBAENBSKDAAGBagMdAQKhGgKBOIh?= =?us-ascii?q?hc4EygnsBAQWFCRiCFAmBDCgBiiWBQxiBf4ERJwwTghc1PoQQCgMRFoMLMoI?= =?us-ascii?q?EIo8XhTKWI20JAoIdhmiJXIN1G4IxhzCOZY1bgTaGLZArAgQCBAUCDgEBBYF?= =?us-ascii?q?WATGBWHAVOyoBgkEJgjkMF4NPilNygSmLQgIkB4IlAQE?=
X-IronPort-AV: E=Sophos;i="5.64,408,1559520000"; d="scan'208";a="310687535"
Received: from rcdn-core-11.cisco.com ([173.37.93.147]) by alln-iport-7.cisco.com with ESMTP/TLS/DHE-RSA-SEED-SHA; 20 Aug 2019 18:30:08 +0000
Received: from XCH-RCD-009.cisco.com (xch-rcd-009.cisco.com [173.37.102.19]) by rcdn-core-11.cisco.com (8.15.2/8.15.2) with ESMTPS id x7KIU5wW015050 (version=TLSv1.2 cipher=AES256-SHA bits=256 verify=FAIL); Tue, 20 Aug 2019 18:30:07 GMT
Received: from xhs-aln-002.cisco.com (173.37.135.119) by XCH-RCD-009.cisco.com (173.37.102.19) with Microsoft SMTP Server (TLS) id 15.0.1473.3; Tue, 20 Aug 2019 13:30:03 -0500
Received: from xhs-rtp-001.cisco.com (64.101.210.228) by xhs-aln-002.cisco.com (173.37.135.119) with Microsoft SMTP Server (TLS) id 15.0.1473.3; Tue, 20 Aug 2019 13:30:03 -0500
Received: from NAM02-BL2-obe.outbound.protection.outlook.com (64.101.32.56) by xhs-rtp-001.cisco.com (64.101.210.228) with Microsoft SMTP Server (TLS) id 15.0.1473.3 via Frontend Transport; Tue, 20 Aug 2019 14:30:03 -0400
ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=NZKBvm6qnsXJD99qEGvmTtHFrk9dj0BTcPnigqWQ8LQnjhwyqkKdNz+wDR0caVyfShXese47MBJvBZbc5jvubn9FBCh2clsRomKrHThaodgvzjmmiNnQo8qXaPR/K3aksVmjIwiZTaxvIUgD4Bu+BupwLQdSgcuLMhNOar09EM96xZ4fF1/vUp6+OkSq/EX8dYIL5zwfZQK+pds6WC0ZVQDvMLIKG3aV8VtozROKU8mV03pSFsV69YhaXxzQGsXgJQQhalCW50OSqi0WYuhLrctHXC6k1NUtG5ncy6BnkuNmN7RERq9zi2kEhHiaJIL4pLncnC+aL1d9Ic/8gbHLiw==
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=1P/aJy0HcERVvNaZEmdLzH0CKnkSCHUi5AuZXQDaovk=; b=X7qjJUyeocKeip6pqrahqwija96wT4+DVbIaCSUgw22jHHGF7qQI1ckInYH+7Dk9DUX/gJCLWDFKOWNYAK044VYfI1trB9SD8I4WePuxQbi7LD4XRc2MnSxrlym5B3AFlRuUDUYQSsofa04lGJ3iUHXb4gUw5bnOTHPg0ilS7TOcAKsTreos3fzdwxVIonQ5K+l/4sB9ZTG7WHLbTZ/pUhJORHetvcJ1ibvEihyGRgH5CwqYNjjE1q6iD/Vvey//Hm6cX1QqZeDc5QS14h8V7ltNlTBLC0h21O3amTJ0CodEhNmVxgDNM1P5RJ2OdermN5Vwjl27C7VV7l+deaVIFg==
ARC-Authentication-Results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=cisco.com; dmarc=pass action=none header.from=cisco.com; dkim=pass header.d=cisco.com; arc=none
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=cisco.onmicrosoft.com; s=selector2-cisco-onmicrosoft-com; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=1P/aJy0HcERVvNaZEmdLzH0CKnkSCHUi5AuZXQDaovk=; b=lKi/XsWo1lPnLbSW4w4UumT8rXe8VYiAQWRASRZyY/sorzKCWECi+vWecX1JaCQgqm7HEN479cBNgFxFC/fnHRixkBMCkkOi5zeIhKAF+ubrn+h1oiGpM/pFKHWFUEy1s9cVr5XHXiSB9Y4NQmz96tITPegSDkT6yMfd134vUPs=
Received: from BYAPR11MB3014.namprd11.prod.outlook.com (20.177.225.13) by BYAPR11MB3207.namprd11.prod.outlook.com (20.177.127.156) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.2178.16; Tue, 20 Aug 2019 18:30:00 +0000
Received: from BYAPR11MB3014.namprd11.prod.outlook.com ([fe80::59ae:773d:d7b0:9ca]) by BYAPR11MB3014.namprd11.prod.outlook.com ([fe80::59ae:773d:d7b0:9ca%7]) with mapi id 15.20.2178.018; Tue, 20 Aug 2019 18:30:00 +0000
From: "Alberto Rodriguez Natal (natal)" <natal@cisco.com>
To: Christian Hopps <chopps@chopps.org>, "yang-doctors@ietf.org" <yang-doctors@ietf.org>
CC: "lisp@ietf.org" <lisp@ietf.org>, "draft-ietf-lisp-yang.all@ietf.org" <draft-ietf-lisp-yang.all@ietf.org>
Thread-Topic: Yangdoctors early review of draft-ietf-lisp-yang-11
Thread-Index: AQHVQ1Nu85KNK2Kykkek9GiFQm7DE6cEDo0A
Date: Tue, 20 Aug 2019 18:30:00 +0000
Message-ID: <69983428-3A0C-4DAB-857E-75586DCE27D0@cisco.com>
References: <156410535896.17429.16464147399003551309@ietfa.amsl.com>
In-Reply-To: <156410535896.17429.16464147399003551309@ietfa.amsl.com>
Accept-Language: en-US
Content-Language: en-US
X-MS-Has-Attach:
X-MS-TNEF-Correlator:
user-agent: Microsoft-MacOutlook/10.1b.0.190715
authentication-results: spf=none (sender IP is ) smtp.mailfrom=natal@cisco.com;
x-originating-ip: [2001:420:301:1310:a410:cab3:320c:8b02]
x-ms-publictraffictype: Email
x-ms-office365-filtering-correlation-id: 82367bd7-68cb-4313-0cd3-08d7259c6956
x-microsoft-antispam: BCL:0; PCL:0; RULEID:(2390118)(7020095)(4652040)(8989299)(4534185)(4627221)(201703031133081)(201702281549075)(8990200)(5600148)(711020)(4605104)(1401327)(2017052603328)(7193020); SRVR:BYAPR11MB3207;
x-ms-traffictypediagnostic: BYAPR11MB3207:
x-ms-exchange-purlcount: 1
x-ms-exchange-transport-forked: True
x-microsoft-antispam-prvs: <BYAPR11MB32075E966E58DD668E20FD89B6AB0@BYAPR11MB3207.namprd11.prod.outlook.com>
x-ms-oob-tlc-oobclassifiers: OLM:10000;
x-forefront-prvs: 013568035E
x-forefront-antispam-report: SFV:NSPM; SFS:(10009020)(4636009)(396003)(346002)(376002)(366004)(39860400002)(136003)(51914003)(199004)(189003)(6306002)(6506007)(8676002)(229853002)(81166006)(71190400001)(6436002)(6486002)(6512007)(486006)(8936002)(256004)(186003)(14444005)(71200400001)(86362001)(11346002)(476003)(2616005)(64756008)(66446008)(36756003)(305945005)(66946007)(46003)(446003)(76116006)(66476007)(66556008)(7736002)(2906002)(99286004)(6116002)(76176011)(81156014)(53936002)(33656002)(102836004)(54906003)(14454004)(25786009)(478600001)(58126008)(4326008)(2501003)(110136005)(6246003)(316002)(5660300002)(53376002); DIR:OUT; SFP:1101; SCL:1; SRVR:BYAPR11MB3207; H:BYAPR11MB3014.namprd11.prod.outlook.com; FPR:; SPF:None; LANG:en; PTR:InfoNoRecords; A:1; MX:1;
received-spf: None (protection.outlook.com: cisco.com does not designate permitted sender hosts)
x-ms-exchange-senderadcheck: 1
x-microsoft-antispam-message-info: fRneVnT9Zb5yJKs81cz1PsMSVpeTXnCiMuX9jfydSLHwOMoj6DBqKds4Gztb0mh/x1dU/GSTvYq2WfluJ2ojBADNlF7WLoU8xTaTXBOg4eMUwie3jOv4qdqTivORkk2fNXCAE0gH/mF+ZKwtP/RcW1gEXrtDs5nEEjHvWk3P4ShS/VeG4pvI8LI57/AHkAxzp4jqh/g7JShwY/y8EvF4qsGb8nixFKvIMd1qQGUGBE/e/nPRXOi2bbGwT+/kRO+jppiakWloyua82cTHOhpZuLu2+T53OU0muCZsezJS3XaJ/vDk2ovME+Fwv9xSaR9WwEJwm32gzVz71VDOBRpp8/MH+jOZ2zmlhf1hITj8P4rPHJ4A8qRryrw9J8/tt7ukcRRgpHIxQw6kUEj+2kQyTb9l5q1PmJYJ//x06Xp1XwE=
Content-Type: text/plain; charset="utf-8"
Content-ID: <FC731247F7637B4094EE6D8EF79827BA@namprd11.prod.outlook.com>
Content-Transfer-Encoding: base64
MIME-Version: 1.0
X-MS-Exchange-CrossTenant-Network-Message-Id: 82367bd7-68cb-4313-0cd3-08d7259c6956
X-MS-Exchange-CrossTenant-originalarrivaltime: 20 Aug 2019 18:30:00.7429 (UTC)
X-MS-Exchange-CrossTenant-fromentityheader: Hosted
X-MS-Exchange-CrossTenant-id: 5ae1af62-9505-4097-a69a-c1553ef7840e
X-MS-Exchange-CrossTenant-mailboxtype: HOSTED
X-MS-Exchange-CrossTenant-userprincipalname: Q/YmDPTAqWQfRJFV2s+8z38XYV8hrPebMViUId3BvBWI5mcW7wWneT8YtQLFmFqu
X-MS-Exchange-Transport-CrossTenantHeadersStamped: BYAPR11MB3207
X-OriginatorOrg: cisco.com
X-Outbound-SMTP-Client: 173.37.102.19, xch-rcd-009.cisco.com
X-Outbound-Node: rcdn-core-11.cisco.com
Archived-At: <https://mailarchive.ietf.org/arch/msg/yang-doctors/rDTKBL64Cmiw0oK_nKq2CJC2TAw>
Subject: Re: [yang-doctors] Yangdoctors early review of draft-ietf-lisp-yang-11
X-BeenThere: yang-doctors@ietf.org
X-Mailman-Version: 2.1.29
Precedence: list
List-Id: Email list of the yang-doctors directorate <yang-doctors.ietf.org>
List-Unsubscribe: <https://www.ietf.org/mailman/options/yang-doctors>, <mailto:yang-doctors-request@ietf.org?subject=unsubscribe>
List-Archive: <https://mailarchive.ietf.org/arch/browse/yang-doctors/>
List-Post: <mailto:yang-doctors@ietf.org>
List-Help: <mailto:yang-doctors-request@ietf.org?subject=help>
List-Subscribe: <https://www.ietf.org/mailman/listinfo/yang-doctors>, <mailto:yang-doctors-request@ietf.org?subject=subscribe>
X-List-Received-Date: Tue, 20 Aug 2019 18:30:14 -0000

Chris, 

Thanks a lot for the review and apologies for the delayed response. We went through your comments and they make good sense to us. Please see inline for some replies from our end.

Thanks,
Alberto

On 7/25/19, 6:42 PM, "Christian Hopps via Datatracker" <noreply@ietf.org>; wrote:

    Reviewer: Christian Hopps
    Review result: On the Right Track
    
    
    * draft-ietf-lisp-yang-11 early yang-doctor review.
    
    ** Minor
    
       - Enumerations are not extensible and so should only be used when all the
         values are known and will not need to be added to. So, auth-algorithm-type
         should use identities and not an enumeration, as it almost for sure will
         need to be added to in the future. An example of this is present in RFC8177
         (keychains) which has identities derived from a base identity
         "crypto-algorithm".

[AR] Agreed, we will update.

       - "type string" is a very inclusive UTF-8 string, along with all legal UTF-8
         characters it includes tabs, spaces, newlines and carriage returns. This
         may not be what you actually want for things like "eid-id"s or
         "auth-key-id". You probably want to use a more restricted typedef variation
         of string (using a pattern to restrict its values).
    
[AR] Good point, we’ll make this more restrictive.

       - Node name consistency, you probably should be consistent with the name for
         nodes of the same type. For example type "eid-id", sometimes the name is
         "id" other times "eid-id" is used.
    
[AR] Good catch, we’ll update.

       - TTL is limited to minute units. This may be overly restrictive. Couldn't
         there be some use (perhaps not common) e.g., perhaps when debugging, or in
         future versions of the protocol where seconds granularity might be useful?
         Changing these nodes later is non-backwards compatible and thus very
         painful to do.
    
[AR] That's a fair comment. We used minutes in the model, however, since RFC6833bis defines the TTLs in minutes. Do you think it would be reasonable to leave the TTL in minutes and aligned with 6833bis? 

    ** geo coordinates
    
       - It might be worth considering using the grouping in the geo-location module
         for specifying coordinates. The only drawback here would be if geo-location
         causes the publication to be delayed b/c lisp-yang finishes first. In any
         case the description for the coordinate nodes should echo more of the info from
         the LCAF RFC, in particular that the coordinate system used is WGS-84).
    
[AR] Thanks for pointing us to the geo-location module. After some discussion on this, we believe we prefer to follow (if you think it's ok) the format on RFC8060 since any other format would need to map to 8060. We would definitely update the description of the nodes to reflect more info from 8060 as you suggested.

       - I found a lisp geo draft and it seems to specify a bit more detail than is
         covered in this module (e.g., the kilometer bit, radius, uncertainty). Not
         sure if that would be appropriate to add or not.
    
[AR] We probably want to follow RFC 8060 here as well.

    ** Nits
    
      - Invalid example XML for LISP Map-Server.. The config namespace should not be
        "http://tail-f.com/ns/config/1.0".
    
[AR] Yes, agreed.

      - Correct module vs model language.
        - OLD: <t>This module is the base LISP module that is augmented in multiple
        models to represent various LISP device roles.</t>
        - NEW: <t>This is the base LISP module.  It is further augmented by the
        LISP device role specific modules defined elsewhere in this document.</t>
    
[AR] Yes, agreed.

      - Yang comments need some grammar fixes.
        - e.g., 'This augments *the* LISP decices list ...'
    
[AR] Agreed, will fix.

      - I got some warning from validation tools, but I'm not sure if they are
       valid, please double check though.
    
[AR] Thanks for the heads up, we will check and fix those.

        - Pyang nits:
          #+begin_src bash
            for f in *.yang; do echo $f; \
            pyang --ietf --max-line-length 69 $f ; \
            done
            ietf-lisp-address-types@2019-02-23.yang
            ietf-lisp-etr@2019-02-23.yang
            ietf-lisp-etr@2019-02-23.yang:86: warning: line length 70 exceeds 69 characters
            ietf-lisp-etr@2019-02-23.yang:106: warning: line length 70 exceeds 69 characters
            ietf-lisp-etr@2019-02-23.yang:157: warning: line length 71 exceeds 69 characters
            ietf-lisp-etr@2019-02-23.yang:164: warning: line length 70 exceeds 69 characters
            ietf-lisp-etr@2019-02-23.yang:171: warning: line length 72 exceeds 69 characters
            ietf-lisp-etr@2019-02-23.yang:179: warning: line length 72 exceeds 69 characters
            ietf-lisp-itr@2019-02-23.yang
            ietf-lisp-itr@2019-02-23.yang:125: warning: line length 70 exceeds 69 characters
            ietf-lisp-mapresolver@2019-02-23.yang
            ietf-lisp-mapresolver@2019-02-23.yang:91: warning: line length 72 exceeds 69 characters
            ietf-lisp-mapserver@2019-03-05.yang
            ietf-lisp-mapserver@2019-03-05.yang:204: warning: line length 70 exceeds 69 characters
            ietf-lisp@2019-03-05.yang
            ietf-lisp@2019-03-05.yang:431: warning: line length 70 exceeds 69 characters
            ietf-lisp@2019-03-05.yang:485: warning: line length 71 exceeds 69 characters
            ietf-lisp@2019-03-05.yang:490: warning: line length 71 exceeds 69 characters
          #+end_src
    
[AR] Thanks a lot for including the pyang arguments you used, will help us to fix the nits.