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

Mahesh Jethanandani <mjethanandani@gmail.com> Thu, 17 October 2019 22:33 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 0A7D3120808; Thu, 17 Oct 2019 15:33:31 -0700 (PDT)
X-Virus-Scanned: amavisd-new at amsl.com
X-Spam-Flag: NO
X-Spam-Score: -1.997
X-Spam-Level:
X-Spam-Status: No, score=-1.997 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, HTML_MESSAGE=0.001, RCVD_IN_DNSWL_NONE=-0.0001, SPF_HELO_NONE=0.001, SPF_PASS=-0.001, URIBL_BLOCKED=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 q12eAeQEqlLF; Thu, 17 Oct 2019 15:33:28 -0700 (PDT)
Received: from mail-pg1-x536.google.com (mail-pg1-x536.google.com [IPv6:2607:f8b0:4864:20::536]) (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 1628E12012A; Thu, 17 Oct 2019 15:33:28 -0700 (PDT)
Received: by mail-pg1-x536.google.com with SMTP id k20so2185428pgi.1; Thu, 17 Oct 2019 15:33:28 -0700 (PDT)
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=from:message-id:mime-version:subject:date:in-reply-to:cc:to :references; bh=emrjqcfVcb7HDNJMWrjw/8q0vhKs09cp0mkq09rqNS8=; b=Pyiv6D/6N/7rN9qda/uj0K9Cs9a3bZhKTCi6k068rWB5H4b+3cEsFC5MeM4fGQOg5g FotQ59le66d76oA/BkPCCDVeYDJ7Nc/ziKSKNTI1t6iPh/wNKKt3YQUXiz3Kqr4zZTsD Mm8GaCfYZY0we9IGhajYW0WqGijKn3MKAJjt0aGwH1bnFDOEtyUWKbl28Zi26yvpraNQ Tok71QwCsJOGIEjH9CRcCDDFuHriO0OkW0rVG2QFoN3YptmBtBKq8I2mMIQAXxpqcTqN Rt4LI0Gr7kTbQGrx+RIUUB/9T/rQQy6XGhEDgDJfOJvOB4VSn8Vg2fl/4U1n8AeYE2Ma clpw==
X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:message-id:mime-version:subject:date :in-reply-to:cc:to:references; bh=emrjqcfVcb7HDNJMWrjw/8q0vhKs09cp0mkq09rqNS8=; b=Hg2ily9Ed3MgJdVXQYrHI+uJOHZhSB2za5ciH5FEmpT93j21mzXKZuQmNnY+TghVit WpzYLUlKZAT/A8ERiKsbaCenRK0veiw/cP4I3guYVP0aL9kBLnu198SHpNok5e8LJB5G qvDwa0c4Q4DLLTV3nyJvtY6YLZ3wIvoDPr4wxVhpPGqnaTM17tdK5u7i7HtVPkhaKSrX /5TCHutszNskI27jLOG5kf5v3dgOiNUaQPdmFJ05Tp5pYQHjLQcR5OxEJCsu0DeYamFe dWbEqR4gPHfp3dfRN43Ynl3ZAoFNmZ6qHtOWkR8MHufNG63QIljqP/RYDz+tv8d5ehxY jeDw==
X-Gm-Message-State: APjAAAVEHOg8WEuDQy3VCCd5wUJ3t5BJpQZukAOVmGF61gxhoxEFTUm2 /9CUco+jaMBR8p0fOc+X5qaLP0sO
X-Google-Smtp-Source: APXvYqyzx8ctH5n5a0GH7GkZ0ZRJauuhB/kNapGB1mFXQ0LbXggtK7Tdb2vuOTzJ8jV/f8RlXpMMuw==
X-Received: by 2002:aa7:9295:: with SMTP id j21mr2730665pfa.87.1571351607453; Thu, 17 Oct 2019 15:33:27 -0700 (PDT)
Received: from [10.33.123.155] ([66.170.99.2]) by smtp.gmail.com with ESMTPSA id p17sm3426677pfn.50.2019.10.17.15.33.25 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Thu, 17 Oct 2019 15:33:25 -0700 (PDT)
From: Mahesh Jethanandani <mjethanandani@gmail.com>
Message-Id: <4FF6E800-5D47-491E-9D7C-906951F6C19B@gmail.com>
Content-Type: multipart/alternative; boundary="Apple-Mail=_D8F84213-6063-446D-895C-3C21BCCBF20E"
Mime-Version: 1.0 (Mac OS X Mail 11.5 \(3445.9.1\))
Date: Thu, 17 Oct 2019 15:33:24 -0700
In-Reply-To: <eaff5648-33aa-c0c7-0b49-6bee275944c7@cesnet.cz>
Cc: YANG Doctors <yang-doctors@ietf.org>, draft-ietf-babel-yang-model.all@ietf.org, babel@ietf.org
To: =?utf-8?B?UmFkZWsgS3JlasSNw60=?= <rkrejci@cesnet.cz>
References: <157106166738.24700.11185508261809138396@ietfa.amsl.com> <3532A7A2-E9C2-43AA-A214-4E2AF5470251@gmail.com> <e7f82974-b8f5-7efd-18ac-354af1954d5f@cesnet.cz> <1D84A1F2-973D-4FF4-B9DE-BC36AECB2069@gmail.com> <eaff5648-33aa-c0c7-0b49-6bee275944c7@cesnet.cz>
X-Mailer: Apple Mail (2.3445.9.1)
Archived-At: <https://mailarchive.ietf.org/arch/msg/yang-doctors/VmpHbfnT17oU7ad4AZCtv-JyscQ>
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: Thu, 17 Oct 2019 22:33:32 -0000

[Trimming the list further down]

> On Oct 17, 2019, at 1:06 AM, Radek Krejčí <rkrejci@cesnet.cz>; wrote:
> 
> Dne 16. 10. 19 v 20:30 Mahesh Jethanandani napsal(a):
>> [Trimming it down to open issues]
>> 
>>>>> - 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.”?
>>> I see no such typedef in the module (neither in the information model). This is probably again a confusion between information model and yang model. If it refers to information model, I don't think it is a good idea, but it should be at least explicitly stated. If it refers to the leaf's base identity, I don't think that it is necessary to duplicate this information (but I'm not strongly against it, it just make me believe that actually it is some additional restriction to the type itself), because there is already:
>>> 
>>> type identityref {
>>>  base metric-comp-algorithms;
>>> }
>>> 
>>> which also says that the value MUST be one of the identities based on metric-comp-algorithms. And technically, such identities can be added so the possible values do not need to be listed in some text/list/typedef. If you really want to limit the values to some static list, the type should be actually enumeration.
>> How about we say just that, i.e. “The value MUST be one of the identities based on metric-comp-algorithm”?
> 
> my point from the beginning is that it is actually duplicated information. The leaf specification can be read as:
> 
> leaf metric-algorithm {
>   type:  “The value MUST be one of the identities based on metric-comp-algorithms”;
>   it is mandatory;
>   description:  “... The value MUST be one of the identities based on metric-comp-algorithms”;
> }
> 
> So I don't think that it is necessary to have that sentence in the description text at all.
> 
> I've looked at the model again and I've realized, that there is actually babel/metric-comp-algorithms leaf-list. So despite the model defines set of identities, the server is actually allowed to implement just a subset of them, right? Maybe the intention of the description was to require the value to be one of the values of the leaf-list. Confusion is caused by naming - identity as well as the leaf-list have the same name and it is referenced in the text as metric-comp-algorithm. I see that you cannot use leafref from configuration to state data. So, I see 2 options. First, the sentence in the description should be:
> 
> "The value MUST be one of those listed by server as babel/metric-comp-algorithms leaf-list."
> 
> or, and I think this is the right solution, remove the babel/metric-comp-algorithms leaf-list and create features for each (there are 2) metric-comp-algorithms identity (add if-feature into them) so the server can say via YANG features mechanism which identities are actually implemented. Then the value of the metric-algorithm leaf will be limited to the metric-comp-algorithms identities with the feature enabled by the server.

I am ok with the second option. 

Except I do not know why pyang complains about this.

  identity two-out-of-three {
    if-feature two-out-of-three-supported;
    base "metric-comp-algorithms";
    description
      "2-out-of-3 algorithm.";
  }

  identity etx {
    if-feature etx-supported;
    base "metric-comp-algorithms";
    description
      "Expected Transmission Count.";
  }

yang/ietf-babel@2019-10-17.yang:88: error: keyword "base" not in canonical order (see RFC 6020, Section 12)
yang/ietf-babel@2019-10-17.yang:95: error: keyword "base" not in canonical order (see RFC 6020, Section 12)

Where the ABNF grammar says:

   identity-stmt       = identity-keyword sep identifier-arg-str optsep
                         (";" /
                          "{" stmtsep
                              ;; these stmts can appear in any order
                              *if-feature-stmt
                              *base-stmt
                              [status-stmt]
                              [description-stmt]
                              [reference-stmt]
                          "}") stmtsep


Mahesh Jethanandani
mjethanandani@gmail.com