Re: [Last-Call] [yang-doctors] [Rift] Yangdoctors last call review of draft-ietf-rift-yang-03

Michal Vaško <mvasko@cesnet.cz> Thu, 09 September 2021 09:26 UTC

Return-Path: <mvasko@cesnet.cz>
X-Original-To: last-call@ietfa.amsl.com
Delivered-To: last-call@ietfa.amsl.com
Received: from localhost (localhost [127.0.0.1]) by ietfa.amsl.com (Postfix) with ESMTP id E1D273A23FD; Thu, 9 Sep 2021 02:26:52 -0700 (PDT)
X-Virus-Scanned: amavisd-new at amsl.com
X-Spam-Flag: NO
X-Spam-Score: -2.099
X-Spam-Level:
X-Spam-Status: No, score=-2.099 tagged_above=-999 required=5 tests=[BAYES_00=-1.9, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, DKIM_VALID_EF=-0.1, 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 (1024-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 1zH88LNrIl25; Thu, 9 Sep 2021 02:26:47 -0700 (PDT)
Received: from kalendar.cesnet.cz (kalendar.cesnet.cz [78.128.211.34]) (using TLSv1.2 with cipher ADH-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ietfa.amsl.com (Postfix) with ESMTPS id A1AD73A2405; Thu, 9 Sep 2021 02:26:44 -0700 (PDT)
Received: by kalendar.cesnet.cz (Postfix, from userid 110) id 4CB1960068; Thu, 9 Sep 2021 11:26:37 +0200 (CEST)
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=cesnet.cz; s=kalendar; t=1631179597; bh=jC1qErbNEhx/qIJ3CSXnXVOkUbDyupjSKzCof3wCiZ4=; h=From:In-Reply-To:Date:Cc:To:Subject; b=ShmH+1vGZApL21UDEZPv1ckT4ihvnAdjNOp4ov2A7fH9A6NVRWhuz/Ek9i9zc/adp Qf8vSFgq6I+MxLYVA1i1DqEjwSUqxblMvCrpDF+nECRRMdA8OJblhw5jV145o2ip8z X6UDXvWDeQCfhtoscGtIrwke6E7GnAhYVLnOwedk=
From: Michal Vaško <mvasko@cesnet.cz>
In-Reply-To: <202109091709262207934@zte.com.cn>
Content-Type: text/plain; charset="utf-8"
X-Forward: 2001:67c:1220:80c:b5:55d3:81d5:8636
Date: Thu, 09 Sep 2021 11:26:37 +0200
Cc: noreply@ietf.org, draft-ietf-rift-yang.all@ietf.org, yang-doctors@ietf.org, rift@ietf.org, last-call@ietf.org
To: zhang.zheng@zte.com.cn
MIME-Version: 1.0
Message-ID: <137d-6139d380-3-67131e80@232159592>
User-Agent: SOGoMail 5.2.0
Content-Transfer-Encoding: quoted-printable
Archived-At: <https://mailarchive.ietf.org/arch/msg/last-call/uA9ZaHz2vj6j2KJUvJYT2uOyCzQ>
Subject: Re: [Last-Call] [yang-doctors] [Rift] Yangdoctors last call review of draft-ietf-rift-yang-03
X-BeenThere: last-call@ietf.org
X-Mailman-Version: 2.1.29
Precedence: list
List-Id: IETF Last Calls <last-call.ietf.org>
List-Unsubscribe: <https://www.ietf.org/mailman/options/last-call>, <mailto:last-call-request@ietf.org?subject=unsubscribe>
List-Archive: <https://mailarchive.ietf.org/arch/browse/last-call/>
List-Post: <mailto:last-call@ietf.org>
List-Help: <mailto:last-call-request@ietf.org?subject=help>
List-Subscribe: <https://www.ietf.org/mailman/listinfo/last-call>, <mailto:last-call-request@ietf.org?subject=subscribe>
X-List-Received-Date: Thu, 09 Sep 2021 09:26:53 -0000

Hi Sandy,
firstly, sorry for the chaotic review. I have organized it properly but some formatting was lost, I will know next time.

As for your question, I do not see any node "algorithm-type" of type leaf, only choice. This choice includes 2 empty cases "spf" and "all-path". This YANG construct as it is has no purpose and cannot even be instantiated in YANG data, unless some YANG data nodes were added into the cases by augments. I suppose that is not the intention and you simply wanted to write a configuration node that could be set to either "spf" or "all-path" value. In that case you want to use enumeration like this:

leaf algorithm-type {
  type enumeration {
    enum spf {
      description "The algorithm is SPF.";
    }
    enum all-path {
      description "The algorithm is all-path.";
    }
  }
  description "The possible algorithm types.";
}

If you need the possible "algorithm-type" values to be dynamic - allowed new to be added from other YANG modules, you can even use type "identityref" and define the values as identities.

Regards,
Michal

On Thursday, September 09, 2021 11:09 CEST, <zhang.zheng@zte.com.cn> wrote: 
 
> Hi Michal, 
> Thank you again for your review! 
> We are do the updating work. 
> But we are not sure that we got the meaning of the following comment:
> "algorithm-type - empty cases - redundant on their own, are expected to
> be augmented?"
> The algorithm-type leaf in the model is not empty. Could you please explain this comment more detailedly?
> Thank you very much!
> Best regards,
> Sandy
> 
> ------------------原始邮件------------------
> 发件人:MichalVaškoviaDatatracker
> 收件人:yang-doctors@ietf.org;
> 抄送人:draft-ietf-rift-yang.all@ietf.org;last-call@ietf.org;rift@ietf.org;
> 日 期 :2021年07月08日 18:17
> 主 题 :[Rift] Yangdoctors last call review of draft-ietf-rift-yang-03
> Reviewer: Michal Vaško
> Review result: Almost Ready
> 
> Generally, use references where make sense (features, nodes) and use units
> and/or standard types (ietf-yang-types) for leaves (such as grouping
> neighbor-node/bandwidth). All links are invalid, better to use references
> anyway because the module will be used outside the RFC.
> 
> Specific problems:
> 
> - description - copyright 2020
> - typedef ieee802-1as-timestamp-type - reference in description, put separately
> - grouping address-families
> - list with a single key can be leaf-list
> - would make sense if meant to be augmented with new nodes
> - grouping node-flag
> - used only once, makes sense if meant to be reused by other modules
> - consider using bits type in the leaf
> - grouping base-node-info/pod - redundant description, use union of number and
> "undefined", or leave out for undefined since it is not mandatory - augment
> rift/rx-lie-multicast-address,tx-lie-multicast-address
> - default value in description - should be defined in YANG
> - consider using
> refine on
> "addresses"
> rx-flood-port - redundant default in description, is obvious in YANG
> algorithm-type - empty cases - redundant on their own, are expected to
> be augmented? HAL - use lowercase
> database/tie/negative_disaggregation_prefixes
> - use hyphen instead of underscore
> - consider abbreviated/shorter node names
> - applicable for the following nodes as well
> 
> 
> _______________________________________________
> RIFT mailing list
> RIFT@ietf.org
> https://www.ietf.org/mailman/listinfo/rift