Re: [yang-doctors] Yangdoctors early review of draft-ietf-babel-yang-model-03

Mahesh Jethanandani <mjethanandani@gmail.com> Tue, 15 October 2019 22:43 UTC

Return-Path: <mjethanandani@gmail.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 AE6AE120045; Tue, 15 Oct 2019 15:43:37 -0700 (PDT)
X-Virus-Scanned: amavisd-new at amsl.com
X-Spam-Flag: NO
X-Spam-Score: -1.999
X-Spam-Level:
X-Spam-Status: No, score=-1.999 tagged_above=-999 required=5 tests=[BAYES_00=-1.9, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, FREEMAIL_FROM=0.001, RCVD_IN_DNSWL_NONE=-0.0001, SPF_HELO_NONE=0.001, SPF_PASS=-0.001] autolearn=ham autolearn_force=no
Authentication-Results: ietfa.amsl.com (amavisd-new); dkim=pass (2048-bit key) header.d=gmail.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 iV4tiP6cTxFw; Tue, 15 Oct 2019 15:43:35 -0700 (PDT)
Received: from mail-pg1-x543.google.com (mail-pg1-x543.google.com [IPv6:2607:f8b0:4864:20::543]) (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 A682F12003E; Tue, 15 Oct 2019 15:43:35 -0700 (PDT)
Received: by mail-pg1-x543.google.com with SMTP id r1so11829773pgj.12; Tue, 15 Oct 2019 15:43:35 -0700 (PDT)
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=mime-version:subject:from:in-reply-to:date:cc :content-transfer-encoding:message-id:references:to; bh=3ZG8r8nAbZsd1aEoNkjAGo+KatL6J4R3ZOxhlsPuj2E=; b=EIohfJL0q/PoLDNPSAbvRW38KzGrlLTq3Xg/3OuM/4PokyiuRwSlLQ5cTKfj1QJExE v2i51d3GXB1Xodkmk29Duxz/Hbn0SXOTMVNyEfrbKSyEZgle6+k3VwJfziFQeFLyD1+/ A2gKpOvYcoNMH/ySqaDMA/doNK0aqr1PrBowepKcA1ihzM6SBaJHsW+CZRY9LbZFH8ZF RHxWNx/olA5+WGBZtWioV8YktnC2wM/Y2w6Au4WvDBid5803QG1KEXz0vc2BGPv5IhDt uhuauXJy/96EX+0JVwGzFM9VbbRUeAKxxXVEqYc5JR14ZGJC43rS7hel6QRz9Uo+K9xN X7oA==
X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:subject:from:in-reply-to:date:cc :content-transfer-encoding:message-id:references:to; bh=3ZG8r8nAbZsd1aEoNkjAGo+KatL6J4R3ZOxhlsPuj2E=; b=XFoYR4fnpFtWuzAwKpVX8fRAFYXN6KncT4Og3faoomvYkHM/Psmyk7jyiM3qQJx88s uxv82E1e+Wlnf6LaZyzq6AlKbCGPuZ8mOb8nqzdGnffR5p4WJiHVHPKRzmkC5QYhz8Cf kUbtuhIVZ8brXy/rzTNEKde0PxyzDCVfZnWFtrAuogOTt/I24CiaMHo7Gwq9sotpQUdw qNjXh5aHGZYO1vuCus0EPlqd8SwYl8WgxxazrJJQFjTREWoNvPntdSg9WIINBHtz63OJ NKllEPH5vyUdPX3q61scbQ9CS+SGNuXVEhfprb8OVq6Pe37Cjkh9bM6/aZ7Q1bpzWHz/ l+ag==
X-Gm-Message-State: APjAAAVtFbXU904vx1hFpIHw+9HalMzUGF9WEGjlWW9WkUxWyiqvyNbS x/Yx2qiXiBOaUUQ4xrR20aQ=
X-Google-Smtp-Source: APXvYqwQPkF5hNiSQiHcZGsJiMe80tIytt/8kDbglxxOgAQOuKTKEZMFGFEyez6aRWls1sY+fWZ7Jw==
X-Received: by 2002:a63:e211:: with SMTP id q17mr41017271pgh.316.1571179414627; Tue, 15 Oct 2019 15:43:34 -0700 (PDT)
Received: from [10.1.244.27] ([66.170.99.95]) by smtp.gmail.com with ESMTPSA id 127sm29400726pfw.6.2019.10.15.15.43.33 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Tue, 15 Oct 2019 15:43:33 -0700 (PDT)
Content-Type: text/plain; charset=utf-8
Mime-Version: 1.0 (Mac OS X Mail 11.5 \(3445.9.1\))
From: Mahesh Jethanandani <mjethanandani@gmail.com>
In-Reply-To: <157106166738.24700.11185508261809138396@ietfa.amsl.com>
Date: Tue, 15 Oct 2019 15:43:32 -0700
Cc: yang-doctors@ietf.org, draft-ietf-babel-yang-model.all@ietf.org, babel@ietf.org
Content-Transfer-Encoding: quoted-printable
Message-Id: <3532A7A2-E9C2-43AA-A214-4E2AF5470251@gmail.com>
References: <157106166738.24700.11185508261809138396@ietfa.amsl.com>
To: =?utf-8?B?UmFkZWsgS3JlasSNw60=?= <rkrejci@cesnet.cz>
X-Mailer: Apple Mail (2.3445.9.1)
Archived-At: <https://mailarchive.ietf.org/arch/msg/yang-doctors/xX9xGEDAtJkUWHghVMIc9-LASzA>
Subject: Re: [yang-doctors] Yangdoctors early review of draft-ietf-babel-yang-model-03
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, 15 Oct 2019 22:43:38 -0000

Hi Radek,

Thank you first of all for your thorough review. Please see comments inline.

> On Oct 14, 2019, at 7:01 AM, Radek Krejčí via Datatracker <noreply@ietf.org>; wrote:
> 
> Reviewer: Radek Krejčí
> Review result: Ready with Nits
> 
> There is a single module in the draft: ietf-babel@2019-08-22.yang.
> 
> There are 2 warnings reported by pyang 2.0.2, but both warnings seem to be
> false.
> 
> ietf-babel@2019-08-22.yang:53: warning: RFC 8407: 3.1: The IETF Trust Copyright
> statement seems to be missing (see pyang --ietf-help for details).
> ietf-babel@2019-08-22.yang:208: warning: the module seems to use RFC 2119
> keywords, but the required text from RFC 8174 is not found (see pyang
> --ietf-help for details).

I have addressed them as part of my discussion with Martin.

> 
> The draft is well written with an informative overview of the YANG module
> sections. Maybe some set of example configuration data (some Appendix) would be
> useful.

I am not sure of the ask. Could you elaborate?

> 
> Regarding the YANG module, I have just a few notes:
> 
> - In multiple description texts, nodes are referenced by a path in format with
> dashes (e.g. babel-mac-default-apply). This format is confusing especially when
> some element in the path contains dash(es). Why the common format with slashes
> is not used?

Ok. I can see that the text can be confusing. The text is from the information model (I-d.ietf-babel-information-model), where the name of the node just happens to follow the path in the YANG model. I can modify the text to use the node names in the YANG model itself.

> 
> - Description and reference statements in containers and lists are placed as
> the last statement (after children nodes). To increase readability and allow
> people to decide if the content of the container/list is interesting for
> detailed view, please move the descriptions / references to the beginning.

Done.

> 
> - grouping routes
>  - just think if the grouping is reusable - in the module itself, the grouping
>  is used just once and needless creation of groupings decrease module
>  readability

It is a grouping so that it can be used with routing policy module.

> 
> - routes/routes/received-metric
>  -  ... indicate a the route ... -> indicate the route …

Fixed

> 
> - babel/interfaces/metric-algorithm
>  - the text "The value MUST be one of those listed in
>  'metric-comp-algorithm'." in description just duplicates the type's
>  information (identityref with metric-comp-algorithms base).

How about if it says “The value MUST be one of those listed in ‘metric-comp-algorithm’ typedef.”?

> 
> - babel/interfaces/mac-key-sets
>  - what does the babel-mac-key-sets in description refer to?

It now says:

            "List of references to the mac entries that apply
             to this interface. When an interface instance is
             created, all mac instances with default-apply 'true'
             will be included in this list.”


> 
> - babel/interfaces/stats/reset
>  - I'm confused, is it per-interface reset (as placed in the module) or
>  system-wide interfaces stats reset (as information model defines it?). If it
>  is the system-wide reset, why tha action cannot be placed in babel container?

The action statement defines an operation connected to a specific container. What would it mean to have an action statement on the whole babel container? Reset everything?

Even though the information model calls for a system-wide reset of stats, the model defines it at a per-interface level. It can be called recursively for every interface instance to realize a system-wide reset of stats.

> 
> - babel/mac/test/output/resulting-hash
>  - according to the content, it seems to me that the description is actually
>  the missing description of the test action.

> 
> - babel/dtls/test/output
>  - seems to me more as a description of the action itself (where the
>  description is missing)
> 

Fixed.

Thanks.

> 

Mahesh Jethanandani
mjethanandani@gmail.com