Re: [Teas] Yangdoctors early review of draft-ietf-teas-actn-vn-yang-10

tom petch <ietfa@btconnect.com> Fri, 26 February 2021 12:07 UTC

Return-Path: <ietfa@btconnect.com>
X-Original-To: teas@ietfa.amsl.com
Delivered-To: teas@ietfa.amsl.com
Received: from localhost (localhost [127.0.0.1]) by ietfa.amsl.com (Postfix) with ESMTP id 75CDE3A1420; Fri, 26 Feb 2021 04:07:36 -0800 (PST)
X-Virus-Scanned: amavisd-new at amsl.com
X-Spam-Flag: NO
X-Spam-Score: -1.901
X-Spam-Level:
X-Spam-Status: No, score=-1.901 tagged_above=-999 required=5 tests=[BAYES_00=-1.9, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, 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=btconnect.onmicrosoft.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 sjtjca3wg3VS; Fri, 26 Feb 2021 04:07:34 -0800 (PST)
Received: from EUR03-VE1-obe.outbound.protection.outlook.com (mail-eopbgr50133.outbound.protection.outlook.com [40.107.5.133]) (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 1574A3A141E; Fri, 26 Feb 2021 04:07:33 -0800 (PST)
ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=K2rCxlL2WLC9icE9rsAArMdjIF7r3KZ0pKfjBWvk7zC7nRSKBWwYUHqzF4hUT1sfqpoxll83jDDa+TNLQw54xIMwAU8EfsukT8fLbr6D7nKqT6MgPf9RMIfHVhaKXc8SzarkeiydZktNdDH/l5meaC1SrqJo7ty4DzAFQtIL3Qi/+9L3I7rsw3tGpvvgMTwj+knWnxlT88vkqoTWpv38JrRNdKB2kFCRh/2+4o2OtUONbacsQd+vbdprxlxayOGXxSZodSJ0gbYFcFVbwOLN4SQFzNgui3dsqnjc0GHpFMt2+khpziyffHmMWzU94xD0euvyvg6nlR5PL7aTW6EB/A==
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=4HiwiXVYZVheMPRBYz1V9AYiKkOK1606et0AQTjCH0w=; b=PM6pSLuj5pa0zLxi5S6VyQxC0+vciCIamObaEoRf2IW+1oRPInjl7y4KY//qHgUEZvsse2YPpu2nDqJyo9lVYREjTouhsyA/k2DhaycpvL7DihLNI35YlWq1kLThnsTzIoHbtn0lMxCl6oKOWNLfNuXzjjnzWE5p20g6H+/LdaRqoGhWvL3juiVMzf2Fp36VH/+jsA74So7AS5GljATTkWtzUX9xQ4lysV5ogAlLf6b4UBt5ajkLCx5rFB5qO3cpfNhmqhkl4vrtio5UCT6HRUuCm7HMmttT8jTZM1DPdig9j8ZN6EgD583QFncZeFYumKGudS4gVZGDSrmRu1h+Gw==
ARC-Authentication-Results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=btconnect.com; dmarc=pass action=none header.from=btconnect.com; dkim=pass header.d=btconnect.com; arc=none
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=btconnect.onmicrosoft.com; s=selector2-btconnect-onmicrosoft-com; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=4HiwiXVYZVheMPRBYz1V9AYiKkOK1606et0AQTjCH0w=; b=jTsqERXsQK4nAETpwn9BLBrCFwz/enx6iPIbCw8q0IaWeoEMxkhul8RfLNPEmmq8OXg8pPF4YHDjAy0ZwnwjoYgsNWCt9gyEdjIeBtUyiykylESGuCzt7PWl3y0ntBpRJ3xnlIYLxzHO3eJ4s9UcjHLm5s6ghPZzyu9AL0nuOZY=
Received: from DB7PR07MB5546.eurprd07.prod.outlook.com (2603:10a6:10:73::23) by DB9PR07MB7227.eurprd07.prod.outlook.com (2603:10a6:10:1fb::17) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.3912.9; Fri, 26 Feb 2021 12:07:29 +0000
Received: from DB7PR07MB5546.eurprd07.prod.outlook.com ([fe80::e079:baec:373c:824f]) by DB7PR07MB5546.eurprd07.prod.outlook.com ([fe80::e079:baec:373c:824f%7]) with mapi id 15.20.3912.009; Fri, 26 Feb 2021 12:07:29 +0000
From: tom petch <ietfa@btconnect.com>
To: Dhruv Dhody <dhruv.ietf@gmail.com>, Andy Bierman <andy@yumaworks.com>
CC: "draft-ietf-teas-actn-vn-yang.all@ietf.org" <draft-ietf-teas-actn-vn-yang.all@ietf.org>, "yang-doctors@ietf.org" <yang-doctors@ietf.org>, "TEAS WG (teas@ietf.org)" <teas@ietf.org>
Thread-Topic: [Teas] Yangdoctors early review of draft-ietf-teas-actn-vn-yang-10
Thread-Index: AQHW1udOqtJn9w9suEmwFEi0PtRngaphaeEAgAlTXvY=
Date: Fri, 26 Feb 2021 12:07:29 +0000
Message-ID: <DB7PR07MB5546393B71017CB63D734A7BA29D9@DB7PR07MB5546.eurprd07.prod.outlook.com>
References: <160847918971.3738.423965928103853275@ietfa.amsl.com>, <CAB75xn7KxxgNT++hLcNKbRAVuQ7zMbkGcfZMtB0JJARPYmaR5w@mail.gmail.com>
In-Reply-To: <CAB75xn7KxxgNT++hLcNKbRAVuQ7zMbkGcfZMtB0JJARPYmaR5w@mail.gmail.com>
Accept-Language: en-GB, en-US
Content-Language: en-GB
X-MS-Has-Attach:
X-MS-TNEF-Correlator:
authentication-results: gmail.com; dkim=none (message not signed) header.d=none;gmail.com; dmarc=none action=none header.from=btconnect.com;
x-originating-ip: [86.146.121.140]
x-ms-publictraffictype: Email
x-ms-office365-filtering-correlation-id: a27078d7-873c-4cf8-d927-08d8da4f16b9
x-ms-traffictypediagnostic: DB9PR07MB7227:
x-microsoft-antispam-prvs: <DB9PR07MB72270ACA21E2D2CD31A1A7BCA29D9@DB9PR07MB7227.eurprd07.prod.outlook.com>
x-ms-oob-tlc-oobclassifiers: OLM:6790;
x-ms-exchange-senderadcheck: 1
x-microsoft-antispam: BCL:0;
x-microsoft-antispam-message-info: 4ilb49KFdWxc9kLas88IHJ0ixz2VyE0wYjREEEMpc3sLOH/ko1xFwJ0KvP9Qy5S7Zm2dktAj3AVRC22IM+nbdtJsbvJlaHmrfRy8yLtY+q4FPyn3+u7RrNJHKIJixaRLos7kQWo5M4o6fUQlvIVXZqBGSA4crUGVlkbe2fKkoFzR+ZN02Y9q4x4Or19fnjNMpBtlZceidBFF54B8PtwQsJS1Bz3/LWu3RXsZm60PPMLg9LTBm3oBK9RlPYpoDZqVRLgOxKLzRLVX42mjNoD6q4ISrp7S5TLxzl3Ihd5v5wRtd1YMP2GEBBsC4AR6bGmCb+ykpMVu2ISi2Y59gSW6GE96GXbBoxg7E+yCaVoW2z2W8LEmWPZHRhHKjdk58qKVdFfEJ04/nvq7tU4PXeclL5BSV/7DQZdQzKGmSEaz6H/S04tgad7tl8uT3kGHJuDGRGKCfWgZ2wX0wsF+yb24RBkaC1isK3kPbEkrCdR8ceI3eJ6ISdUE9KChoMlv5qDXPX6TfLBTq8Tcu72cclyM/mlhJzrKqgbWC3+dYxhrBWi4m2FCfMdq7ajQQKYGpn/9XmIUBjzGMzFSRFWK+tt57iTtA58mQJCj9jGhF1ttlxM=
x-forefront-antispam-report: CIP:255.255.255.255; CTRY:; LANG:en; SCL:1; SRV:; IPV:NLI; SFV:NSPM; H:DB7PR07MB5546.eurprd07.prod.outlook.com; PTR:; CAT:NONE; SFS:(366004)(346002)(376002)(136003)(396003)(39860400002)(9686003)(55016002)(83380400001)(2906002)(4326008)(316002)(110136005)(54906003)(8676002)(966005)(33656002)(76116006)(86362001)(66446008)(26005)(64756008)(8936002)(53546011)(66946007)(6506007)(478600001)(66556008)(5660300002)(91956017)(66476007)(71200400001)(52536014)(186003)(7696005); DIR:OUT; SFP:1102;
x-ms-exchange-antispam-messagedata: =?iso-8859-1?Q?mw0nlFV4UNThhMm6gJ64Rx1VWsi683sd2VKNCHMondtpU1zLJLJEQb14G8?= =?iso-8859-1?Q?rUb2X5fhTtyPWrQI/NuUk5dTRbF1KJSHRDzqG+3pFWRwpfF48dJtx0sOHn?= =?iso-8859-1?Q?hx9DcsVjC6wRwErRFo66rYmR285hZagm/PFdWjqBUJBh5g+/WjrlTVdMzq?= =?iso-8859-1?Q?gfSVZjVQmDzrIVTqxlUYxuBYOW0z31ugfp+dX2yul9bumdM0QHul2J10AR?= =?iso-8859-1?Q?+F8osNtLObhUCTauC2JYDQovkBQL3IqqXaH7uq/z8sx8RvIhb1ZguDlXlE?= =?iso-8859-1?Q?XZzgXhPhbCjEEU5AFOxGaeki1SzuPOYi8KPYdrCmhPsA2kRkDGdWjOiM+d?= =?iso-8859-1?Q?ADVLyccta6G+Rz7xwa5QS3AjA50xIiOKqExO0KjqSozFWFxrCOykRkUxkS?= =?iso-8859-1?Q?XmMCGlcUFgr9veX430P1WOqaXUD9e+QTj38sv7Oqc9V44sqLY9uEsge6Zw?= =?iso-8859-1?Q?+AjC0ecIs1TOQ+G9AAyzcv03Q/a62/Q9guXs6PB0U6zecYXXrVbj5sXzXL?= =?iso-8859-1?Q?xmiiw+su8uMN7NcNLLW95jpMJokFEwUimU/p18vT7FQ6iJ7/Fg1PIybnFS?= =?iso-8859-1?Q?SZpMzjKDVP6w4JthJ6HQWc1raaVWeWSV7D80SnnZOPxRcnZAMuVW9Gjq9X?= =?iso-8859-1?Q?5JOZIxovYs6AhJPVN3u1pcD9DpZg/j9tzQxWszrUCMSUETVCV4Hk5E6KQC?= =?iso-8859-1?Q?bEOX860HdJy0/RCQ44a0lQwfNauDMOr22iUAi9qJCpK7rbNWLyRF6bKewP?= =?iso-8859-1?Q?80UQw6tC0FeNJNPum8DcPIXJgMETGN7efexBZMC8iMnwWeGkb9HKWcT1bE?= =?iso-8859-1?Q?kzR2Wf0LV+fX/vj6zYIsml66Crt1Hps8liFGkGO7rR6cE2lj5EIGAxDd8f?= =?iso-8859-1?Q?oM947svnJZ9nFldtsc6y3oRBrwea1xnBAdH2Xiyi/T5kbpD10COk4996Io?= =?iso-8859-1?Q?6st4eZDQAxoCxXEWaW1yvBoQK9q/Rxb+mBcw4FEJOfxwQJjkFpYBrp1liJ?= =?iso-8859-1?Q?kw9Th+cMp0LP2n3prqd1+PyPsnFsy8cEMFlDxG1QPYRs+DWAKlpJiGi71t?= =?iso-8859-1?Q?jZi3MYW+taihrul6Y+UDkigJmJBHcnVcXm1e/SHfXeMWgcBBG+IAEQYB4k?= =?iso-8859-1?Q?OX0nJtWZ12GjvL3QNubMpFOVxUXNmNp57UVqHed2QQwfuQgKj6FEzkWtT4?= =?iso-8859-1?Q?nB5q0vGpf4ME0FBY14Pvug1Lh5IxMcF+eCaHqQTfh9+EfhIEAKUdIJdOiL?= =?iso-8859-1?Q?JAld9vc/rOkvGe3HHXhl/KYUaDTFWAt71LDbt4KhWM92zUFTbiDN0Yp5+H?= =?iso-8859-1?Q?JbcDV+B9Q++9OooylA/js0tAIhT8zADUtivCIu3PyGCvmxM=3D?=
x-ms-exchange-transport-forked: True
Content-Type: text/plain; charset="iso-8859-1"
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
X-OriginatorOrg: btconnect.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-AuthSource: DB7PR07MB5546.eurprd07.prod.outlook.com
X-MS-Exchange-CrossTenant-Network-Message-Id: a27078d7-873c-4cf8-d927-08d8da4f16b9
X-MS-Exchange-CrossTenant-originalarrivaltime: 26 Feb 2021 12:07:29.0325 (UTC)
X-MS-Exchange-CrossTenant-fromentityheader: Hosted
X-MS-Exchange-CrossTenant-id: cf8853ed-96e5-465b-9185-806bfe185e30
X-MS-Exchange-CrossTenant-mailboxtype: HOSTED
X-MS-Exchange-CrossTenant-userprincipalname: a8y4VE5YT/Z7zHy3mOAak6FHbd0fmb6F884tgfT5iODEWCJo8EOgM1yKyvgzYSSUQZz+pIwwRvdf53eqjfDRqg==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: DB9PR07MB7227
Archived-At: <https://mailarchive.ietf.org/arch/msg/teas/XSHGHgcyc9-tIJSwgXKvYY6bMVs>
Subject: Re: [Teas] Yangdoctors early review of draft-ietf-teas-actn-vn-yang-10
X-BeenThere: teas@ietf.org
X-Mailman-Version: 2.1.29
Precedence: list
List-Id: Traffic Engineering Architecture and Signaling working group discussion list <teas.ietf.org>
List-Unsubscribe: <https://www.ietf.org/mailman/options/teas>, <mailto:teas-request@ietf.org?subject=unsubscribe>
List-Archive: <https://mailarchive.ietf.org/arch/browse/teas/>
List-Post: <mailto:teas@ietf.org>
List-Help: <mailto:teas-request@ietf.org?subject=help>
List-Subscribe: <https://www.ietf.org/mailman/listinfo/teas>, <mailto:teas-request@ietf.org?subject=subscribe>
X-List-Received-Date: Fri, 26 Feb 2021 12:07:37 -0000

From: Teas <teas-bounces@ietf.org> on behalf of Dhruv Dhody <dhruv.ietf@gmail.com>
Sent: 20 February 2021 13:25

Hi Andy,

Thanks for your review. The new -11 version is out which takes your comments into consideration.

https://datatracker.ietf.org/doc/draft-ietf-teas-actn-vn-yang/
A diff from the previous version is available at:
https://www.ietf.org/rfcdiff?url2=draft-ietf-teas-actn-vn-yang-11

<tp>
I liked most of Andy's comments but not the one that leads to/vn/vn/vn-id or/ap/ap/ap-id or /ap/ap/vn-ap/vn-ap-id.  I agree that the original was not good but am not sure that repeated 'ap'  or 'vn' add clarity.

However, a more substantial comment relates to vn-compute where the compute status uses te-common-status which allows for up, down, testing, maintenance; mmmm.  This is not the only I-D to compute and they seem to be heading in different directions where success/failure and reasons therefore are concerned..

Ditto error-info which here is RYO while others import from  yang-te.

YANG is bad at having a list which users only want to use a part of (e.g. Hash algorithms) but the identifiers could be coordinated at least within one IETF WG..

Tom Petch



Thanks!
Dhruv

On Sun, Dec 20, 2020 at 9:16 PM Andy Bierman via Datatracker <noreply@ietf.org<mailto:noreply@ietf.org>> wrote:
Reviewer: Andy Bierman
Review result: Ready with Issues


Major Issues:

  None

Moderate Issues:

1)
   leaf /ap/access-point-list/access-point-id
   leaf /vn/vn-list/vn-id
   leaf /vn/vn-list/vn-member-list/vn-member-id

   These list keys use type inet:uri.
   You should consider the implementation complexity here.
   Will servers all correctly convert any arbitrary URI to its canonical
   representation? The draft should address this issue.

2)
  leaf /vn/vn-list/oper-status
  leaf /vn/vn-list/admin-status

  These objects use vn-status-type, vn-admin-state-type
  The use of identities for even simple "up/down" status types
  seems extreme. The conformance for an enumeration is clear
  (mandatory), but not for an identityref type.
    - E.g., Is is mandatory for a vendor to support vn-state-up,down?
      A vendor could write their own identities and ignore the standard
      identities.


3)
  rpc /vn-compute
  The procedure for this operation is not explained here.
  A full description or reference to normative test is needed.
   - what does the server do with the input?
   - what output is expected? Any variants based on the inputs
     should be explained.
   - any interoperability considerations wrt/ use of these
     common groupings in this RPC context?
   - what errors can occur? Specify any error-tags, etc. that
     the server MUST/SHOULD include in the response


Minor Issues:

4)
 - naming inconsistent within /ap and /vn
   access-point is spelled out and vn is not
   Suggest: shorten access-point to ap

5)
 - naming a list entry with the the suffix -list is redundant.
   YANG lists do not have any conceptual container or way to
   reference all the entries (if that what this naming intends)
   Suggest:
     s/access-point-list/ap/
     s/vn-list/vn/
     s/vn-member-list/vn-member/

6)
  - leaf /vn/vn-list/vn-member-list/src/multi-src
  - leaf /vn/vn-list/vn-member-list/dest/multi-dest
    Looks like these leafs should each have a default-stmt.
    What does it mean if the multi-src-dest feature is supported
    but these leafs are missing from the config?