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

Andy Bierman <andy@yumaworks.com> Fri, 26 February 2021 15:36 UTC

Return-Path: <andy@yumaworks.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 56A7B3A0C9E for <teas@ietfa.amsl.com>; Fri, 26 Feb 2021 07:36:22 -0800 (PST)
X-Virus-Scanned: amavisd-new at amsl.com
X-Spam-Flag: NO
X-Spam-Score: -1.887
X-Spam-Level:
X-Spam-Status: No, score=-1.887 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, T_SPF_PERMERROR=0.01, URIBL_BLOCKED=0.001] autolearn=unavailable autolearn_force=no
Authentication-Results: ietfa.amsl.com (amavisd-new); dkim=pass (2048-bit key) header.d=yumaworks-com.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 K4e9bB6unEUN for <teas@ietfa.amsl.com>; Fri, 26 Feb 2021 07:36:19 -0800 (PST)
Received: from mail-lf1-x12c.google.com (mail-lf1-x12c.google.com [IPv6:2a00:1450:4864:20::12c]) (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 B11523A0C9A for <teas@ietf.org>; Fri, 26 Feb 2021 07:36:18 -0800 (PST)
Received: by mail-lf1-x12c.google.com with SMTP id m22so14431584lfg.5 for <teas@ietf.org>; Fri, 26 Feb 2021 07:36:18 -0800 (PST)
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=yumaworks-com.20150623.gappssmtp.com; s=20150623; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=iXj2a+qYWUi934uD7Swj4VFeKpcYhagNfCzEPp5sN8Y=; b=xLw4UmUZyuu89gwz9dwreyi2azaurncOK42EKn98ULCsWbnR3ODOJoff8UdZXc+ADh zJiiBDNdWDznmdhJ9a8m8clZFM2whILRJ4FNrMBawDStmhLMUbnYPG2WxN8178vFU58l TjoezJCrV7E9Z2f6DbguexVYyg242RcLjoyBzR0Zj8JgrhYclgi5sFqAd6LjtlD3OkwB JWr/6tS6oOWe8RzMAjnLaeZGHkryjpiBHthHdjHbxm6SmuwBs6+IWx3KkuFyBBG72aNk UCsPOFmTjPxVL1TAEfBh3zNBO+q3iCSMhneOiQg37LcI+5e6p96LUQnAyaVdJPMLazV3 i/tQ==
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=iXj2a+qYWUi934uD7Swj4VFeKpcYhagNfCzEPp5sN8Y=; b=TySx9xmquK6bDKoIXmyVAYslxI3hgODyJ8IinSH75RfcwNvaNBJMCMHLABaSkIUSBJ bSzrBlPMUOAsC4HGla65CDFsN9fr1qkFRlUKrEQbwKtsamsqHy6+VIuRh9ozi++skbaT 5LzljNR0VRrtDN5kbMyWHCyz9YVOEoRUe5rH9nJ98L1LyxKfR7O+868gZ8ZN+tgBVDR+ mEg4eAj9WiCXsjPl5KF+zvgsCBvM9Q1KinkjtbVzsES/UUHmS4Ds+l92TsQYJB0717oy yL+ovb161qxOsN+pug2XnnlA4uOuCMBlKJBJ1ivQgBjtJCNXsQ8l2ZDHNKadquF7k2OQ VvwQ==
X-Gm-Message-State: AOAM531CKKKrY3bY5oLuW0NmsgIm1ggE8Hx2a9v0AIzABBabrsThXGje Vd6f4Lq1mH2D4lCmXGUD6yDPYuvo6by0t8sGvTUxuQ==
X-Google-Smtp-Source: ABdhPJxNwjFOkVZbwZM25idD7nJAvbBPZMIVXDHMcpQWBoWaZKXlmEtKIo0yda2zwOhGL94KYDTij8Om0+dUCJW9VjE=
X-Received: by 2002:ac2:43cf:: with SMTP id u15mr2056670lfl.568.1614353776814; Fri, 26 Feb 2021 07:36:16 -0800 (PST)
MIME-Version: 1.0
References: <160847918971.3738.423965928103853275@ietfa.amsl.com> <CAB75xn7KxxgNT++hLcNKbRAVuQ7zMbkGcfZMtB0JJARPYmaR5w@mail.gmail.com> <DB7PR07MB5546393B71017CB63D734A7BA29D9@DB7PR07MB5546.eurprd07.prod.outlook.com>
In-Reply-To: <DB7PR07MB5546393B71017CB63D734A7BA29D9@DB7PR07MB5546.eurprd07.prod.outlook.com>
From: Andy Bierman <andy@yumaworks.com>
Date: Fri, 26 Feb 2021 07:36:05 -0800
Message-ID: <CABCOCHQUPtuHHzgSVfZZEEKy7Avt=wA1zma_BQVHMCpRgzQj_A@mail.gmail.com>
To: tom petch <ietfa@btconnect.com>
Cc: Dhruv Dhody <dhruv.ietf@gmail.com>, "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>
Content-Type: multipart/alternative; boundary="00000000000068c53305bc3f07e2"
Archived-At: <https://mailarchive.ietf.org/arch/msg/teas/W2hZAEFh73HGgLpglh04xa82mBQ>
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 15:36:22 -0000

On Fri, Feb 26, 2021 at 4:07 AM tom petch <ietfa@btconnect.com> wrote:

> From: Teas <teas-bounces@ietf.org> on behalf of Dhruv Dhody <
> dhruv.ietf@gmail.com>
> Sent: 20 February 2021 13:25
>
> Hi Andy,
>
>
I agree with your comments.
Perhaps we should not be using 2 letter node names since they are likely to
be ambiguous over time.

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?
>
>
>
>