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

Radek Krejčí <rkrejci@cesnet.cz> Thu, 17 October 2019 08:06 UTC

Return-Path: <rkrejci@cesnet.cz>
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 889BE120143; Thu, 17 Oct 2019 01:06:50 -0700 (PDT)
X-Virus-Scanned: amavisd-new at amsl.com
X-Spam-Flag: NO
X-Spam-Score: -4.298
X-Spam-Level:
X-Spam-Status: No, score=-4.298 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_MED=-2.3, SPF_HELO_NONE=0.001, SPF_NONE=0.001] autolearn=ham autolearn_force=no
Authentication-Results: ietfa.amsl.com (amavisd-new); dkim=pass (2048-bit key) header.d=cesnet.cz
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 kZatMLFvGFHW; Thu, 17 Oct 2019 01:06:48 -0700 (PDT)
Received: from office2.cesnet.cz (office2.cesnet.cz [IPv6:2001:718:1:101::144:244]) (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 4DFDA120115; Thu, 17 Oct 2019 01:06:48 -0700 (PDT)
Received: from pckrejci.nat9.vcit.vutbr.net (unknown [IPv6:2001:67c:1220:80c:d0:552c:73a5:18da]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by office2.cesnet.cz (Postfix) with ESMTPSA id 8CAC2400065; Thu, 17 Oct 2019 10:06:43 +0200 (CEST)
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=cesnet.cz; s=office2-2020; t=1571299604; bh=XkpZ1o1NLgo7CkoVZ5T68OxVp6S+5HuSAXVJXFhKonE=; h=Subject:To:Cc:References:From:Date:In-Reply-To; b=VsvejO0XcBh7cD9poGeVTH+EteCDePFsh6RTtQCcL6T8pyInwr3gKQ4Dm8D5TCAnB ma4mBpFLhfUIcE3hC+MQeChWC5pUYEe3E88CmaF67sZY5VSfnVoiZx/RUwrQh8EBmd 6IgWaxvsd3/0cqhxprCqMRb67P+7+t+q1+hEZpz3GY2sBvXV2uBhgfa9eb0nFIB6tZ 88p0maAhJ/qrKFG5/kXwWP/Kh4DWi0cuUGfXwTaIL2guBFZKVxv19nfB7V1cdrYH8K m/fAU9dcvkjFzhW7WGYw1U6gWPoZRcqidk7STo777G7n0RAVqCVtM/PSqvhRd7WT0H Ym8EWUZPS5dFw==
To: Mahesh Jethanandani <mjethanandani@gmail.com>
Cc: YANG Doctors <yang-doctors@ietf.org>, draft-ietf-babel-yang-model.all@ietf.org, babel@ietf.org
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>
From: =?UTF-8?B?UmFkZWsgS3JlasSNw60=?= <rkrejci@cesnet.cz>
Openpgp: preference=signencrypt
Autocrypt: addr=rkrejci@cesnet.cz; prefer-encrypt=mutual; keydata= mQGiBEKfHd4RBADDE8CtJpEtOraXBKfQg0KCRZu7BRALixoLqW98U+N9h+PJ+gCnFaKNmnYu fXWLYKTJRUlaoMGIJOZjHpr/zvwozSR+VJkxCsTyNYTF8vIfN3Iwrxy9e8CNy/O1GI50K/ld WWMDl+3M2NztiBFPrCT0b/U5ErsN7bTrf2XLEQRpZwCg95POGbJPqPAaaok2KU5e2u0/flsD /AyC0aRO66Ci0OGw0R5sCJmzZ5xE5eBUvfx0N0IC16aojrwRYM5yf+bULtBDd4wPI1R+VH/X P6OrDgzlDmutJthVtYfCcho3IhqnVo1R/UvJxjF3ATKbOnVHL4xwiLSrRDb6rKVyd1+Kc7cq +JABgFl+JP4xndytvvUXdVqhuSUFBACCDdDtxutkclBrvEp2guBIftuT4/oK3IWxgtevlGfY LZXwdD6pIWS1z6y6xthoFTsLWS1QCFk2ZXmAgvOV/lnW0iGHwO5kCfzvWJq7weeH2FGuBgq+ WInxhdIFD/QwiXV6EPUWzAoC5Fx4Cz5ySFSd6n0C1Mrzin3ABtPHRpUT8rQpUmFkZWsgS3Jl amNpIChDRVNORVQpIDxya3JlamNpQGNlc25ldC5jej6IYgQTEQIAIgUCTT/pkAIbAwYLCQgH AwIGFQgCCQoLBBYCAwECHgECF4AACgkQIMoxClN+p/31DwCfWVWX1IWaUa6+QbuVvZQIkb6m Rn8AoLRvdANGe/As/Nxabu+KKtrorkQ6uQENBEKfHeIQBACwORs231u+o9/pM7y85ZlZhnNY iJziZ4P5W9lD5cwcEUFgTt1upUmjjSMWr5x4HL6o5jZeKOQMxiYP+8qA8OPEM6fzemS1Uj9M 6RXUaoUZFrcKD6BvneyyKuGgNa9bQfTG0aDOqaxy4lYFNcHVeo9sXJ+6adVxlCo/GzZ6zznn nwADBQP+IZQoao7aCFkZOVk8F5AW9Iiz0hk1trdCw88vD5fPMqcLxOQEsKrHAjibTWyOy1il 9zgLyVjcBzOs+v6UvbcJRybyaITC7j4IFPr78euVup/AeL+A9ay+ZWKHMFzALD+VjLyYAiRL w2MBjdqAKbPh2Ei1HXJoOX5JTWWnMRsBey+ISQQYEQIACQUCQp8d4gIbDAAKCRAgyjEKU36n /YssAKDVrEroZMSci018ipG4q6w11TsriwCghwCwX0isavqXJTbw10hwJePlDns=
Message-ID: <eaff5648-33aa-c0c7-0b49-6bee275944c7@cesnet.cz>
Date: Thu, 17 Oct 2019 10:06:43 +0200
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.8.0
MIME-Version: 1.0
In-Reply-To: <1D84A1F2-973D-4FF4-B9DE-BC36AECB2069@gmail.com>
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: quoted-printable
Content-Language: en-US
Archived-At: <https://mailarchive.ietf.org/arch/msg/yang-doctors/U9rzm-y88SYHDD8zCS_-jyxbRf0>
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 08:06:51 -0000

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.

>
>>
>>>> - 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?
>> but that is what the information model says, right? System-wide reset of the stats.Or is the meaning different?
>>
>>> 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.
>> that's what I miss in the action description. There is what the reset is supposed to do by information model (system-wide reset) and that in YANG the reset action has to be placed where the action needs to be performed. So according to the description, the reset is in a wrong container because to perform system-wide reset, it probably don't need to be performed in a specific interface, right? There is no word about intention to change what the reset is supposed to do (reset of stats for the specific interface).
> Ok. How about this text?
>
>               The information model [RFC ZZZZ] defines reset
>                action as a system-wide reset of Babel statistics.
>                In YANG the reset action is associated with
>                container where the action is defined. In this case
>                the action is associated with the interfaces
>                container, so the action will reset statistics at an
>                interface level. 
>
>                Implementations that want to support a system-wide
>                reset of Babel statistics need to call this action
>                for every instance of the interface.

Perfect, thanks!

Radek