Re: [Rift] Comments on most recent version of the RIFT YANG data model.
Bruno Rijsman <brunorijsman@gmail.com> Thu, 18 February 2021 08:22 UTC
Return-Path: <brunorijsman@gmail.com>
X-Original-To: rift@ietfa.amsl.com
Delivered-To: rift@ietfa.amsl.com
Received: from localhost (localhost [127.0.0.1]) by ietfa.amsl.com (Postfix) with ESMTP id 6B1DB3A0D9D for <rift@ietfa.amsl.com>; Thu, 18 Feb 2021 00:22:51 -0800 (PST)
X-Virus-Scanned: amavisd-new at amsl.com
X-Spam-Flag: NO
X-Spam-Score: -2.096
X-Spam-Level:
X-Spam-Status: No, score=-2.096 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, FREEMAIL_FROM=0.001, HTML_MESSAGE=0.001, RCVD_IN_DNSWL_BLOCKED=0.001, 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 8iyExMSbXs1W for <rift@ietfa.amsl.com>; Thu, 18 Feb 2021 00:22:48 -0800 (PST)
Received: from mail-ed1-x52b.google.com (mail-ed1-x52b.google.com [IPv6:2a00:1450:4864:20::52b]) (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 D5F323A0DA5 for <rift@ietf.org>; Thu, 18 Feb 2021 00:22:47 -0800 (PST)
Received: by mail-ed1-x52b.google.com with SMTP id n1so2796750edv.2 for <rift@ietf.org>; Thu, 18 Feb 2021 00:22:47 -0800 (PST)
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=vibtyoWuNNf8hAUxrYBUbwTMHA1v6SjxkR8zgdrTB40=; b=hOGpUbpH1idsMXj3cYgfk/BnYK2DDy4c4mG1UpmSbgwXaRCp8uEHXie/jhUM4wp/fr r1+5xWZnIKaaWPhfjALFuzIhcsMMfPEf3iHaDxNyY24NobHlMH7ObhKNITiEwGOIbYyP M+xg/CV+HV58warHZMxq+Sc8A95eNpmRRI5V3gNAig2YQb+0G+rf24dHLjo3IhMH9/9v 9eW9wob0obm3TaFNTo2TpeME0MMJ0kA5tVL20mEMfOtQUy9RWgNLNvntw08DnBGuQe81 4unEF1yNMPEFbtZYPyqUOiwWdk7fi178YjmtfJcoHHE2gGXUd25KSsAx6f/1mf6ylZMY c1pA==
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=vibtyoWuNNf8hAUxrYBUbwTMHA1v6SjxkR8zgdrTB40=; b=R/qtoj9ryBvDKkDAYqWqWUkNDkf+XlMAi1ws5kJmMUVHSLbXYadACwJbRTuGTw+d1x IeSi4RuI2MkSO9jv1BlRt6GofdEub5MnoCJl47fF83OgsOXFF95mqjVi5fxb3CYpfL9o eZVQchARNafINXfIdlivXvdiyn13vK8GhwJbAYuY09I3gKaK3xQ43uPRehhkDY+aw5dL N8s25KgkrJhyNMOadXdpm+cJfuRqvkigYiY4rCTBW0i+B39VcYUhERv8jUI9Gr2Wb1dZ ZegssUGaaa/mI5SPlB4YYdzziZUpv+wZL0Mspf/cr2XyA9byoTaFthR2No8u/L+pUP6m mfkg==
X-Gm-Message-State: AOAM531+9iXh75eds0kGUWrGAFCCC3d/4qXLT0VHFFm/s8nUFMD/0fQM PwmSQoyCWMNp4L/yuMR5Tss=
X-Google-Smtp-Source: ABdhPJyOnFSOUh/sec7lZkVTS4Z6TpK2uvXI8Rhn2gkcb1S5VC+Jk4KSJmIRt1KJ5PsULR4Tnbykeg==
X-Received: by 2002:aa7:db0c:: with SMTP id t12mr2994105eds.33.1613636566255; Thu, 18 Feb 2021 00:22:46 -0800 (PST)
Received: from [192.168.68.59] (095-097-022-201.static.chello.nl. [95.97.22.201]) by smtp.gmail.com with ESMTPSA id y8sm2200637edd.97.2021.02.18.00.22.44 (version=TLS1_2 cipher=ECDHE-ECDSA-AES128-GCM-SHA256 bits=128/128); Thu, 18 Feb 2021 00:22:45 -0800 (PST)
From: Bruno Rijsman <brunorijsman@gmail.com>
Message-Id: <E325786A-A8DE-405B-8E7F-C5915CB7BD4D@gmail.com>
Content-Type: multipart/alternative; boundary="Apple-Mail=_19D12CEF-9BC1-4783-9752-774207A85102"
Mime-Version: 1.0 (Mac OS X Mail 13.4 \(3608.120.23.2.4\))
Date: Thu, 18 Feb 2021 09:22:44 +0100
In-Reply-To: <202102171636417529439@zte.com.cn>
Cc: wei.yuehua@zte.com.cn, "Jeffrey (Zhaohui) Zhang" <zzhang@juniper.net>, Jeff Tantsura <jefftant.ietf@gmail.com>, Antoni Przygienda <prz@juniper.net>, Tony Przygienda <tonysietf@gmail.com>, rift@ietf.org
To: "EXT-zhang.zheng@zte.com.cn" <zhang.zheng@zte.com.cn>
References: <FC8B0249-85DB-43A2-858B-B6306149121B@gmail.com> <202102171636417529439@zte.com.cn>
X-Mailer: Apple Mail (2.3608.120.23.2.4)
Archived-At: <https://mailarchive.ietf.org/arch/msg/rift/QmLDF6aOCzqO_dLvx8Z5MYIX1vE>
Subject: Re: [Rift] Comments on most recent version of the RIFT YANG data model.
X-BeenThere: rift@ietf.org
X-Mailman-Version: 2.1.29
Precedence: list
List-Id: Discussion of Routing in Fat Trees <rift.ietf.org>
List-Unsubscribe: <https://www.ietf.org/mailman/options/rift>, <mailto:rift-request@ietf.org?subject=unsubscribe>
List-Archive: <https://mailarchive.ietf.org/arch/browse/rift/>
List-Post: <mailto:rift@ietf.org>
List-Help: <mailto:rift-request@ietf.org?subject=help>
List-Subscribe: <https://www.ietf.org/mailman/listinfo/rift>, <mailto:rift-request@ietf.org?subject=subscribe>
X-List-Received-Date: Thu, 18 Feb 2021 08:22:52 -0000
Hi Sandy,
Thank you for the prompt update!
A few very last nitpicks:
+--ro protocol-major-version uint16
^^^ Should be a uint8: the Thrift datamodel has separate data types for (major) VersionType (i8) and MinorVersionType (i16). In both cases the Thrift data model has a note saying that the signed Thrift type should be interpreted as an unsigned.
+--ro time-of-current-state? uint64
^^^ Having a “time” as “uint64” is somewhat ambiguous. Suggest changing to either (a) seconds-in-current-state uint64 or (b) last-state-change date-and-time. The latter is better IMO.
+--ro kv-store
^^^ Rename kv-store to key-value (ties > key-value is consistent with ties > node, ties > prefix)
+--ro key? uint16
+--ro value? uint32
^^^ Change key type from uint16 to binary and change value data type from uint32 to binary. In the RIFT draft, keys are currently defined as strings (type KeyIdType) and values are also currently defined as strings (element 1 in KeyValueTIEElement). However in the e-mail with Tony on 30-Jan-2021 we agreed that string should be changed to binary.
Once again, I would like to call on other people on the mailing list to also review the Yang data model. Another pair of eyes is definitely needed.
— Bruno
> On Feb 17, 2021, at 9:36 AM, zhang.zheng@zte.com.cn wrote:
>
> Hi Bruno,
>
> I updated the model according to your comments.
>
> Could you please review it and give me more comments?
>
> Appreciate for your review!
>
> Thank you very much!
>
> Best regards,
>
> Sandy
>
> 原始邮件
> 发件人：BrunoRijsman
> 收件人：rift@ietf.org;张征00007940;魏月华00019655;
> 日 期 ：2021年02月04日 15:48
> 主 题 ：Comments on most recent version of the RIFT YANG data model.
> Here are my comments on the most recent version of the RIFT YANG data model.
>
> I think it is in pretty good shape now.
>
> I have not actually implemented the YANG data model and I am no YANG expert, so more eyeballs would be helpful.
>
> — Bruno
>
>
> module: ietf-rift
> augment /rt:routing/rt:control-plane-protocols/rt:control-plane-protocol:
> +--rw rift!
> +--rw node* [name]
> +--rw name string
> +--ro level? level
> +--rw system-id system-id
> +--rw pod? uint32
> +--rw configured-level? level
> +--rw overload? boolean
>
> >>> Suggest adding: ro protocol-major-version uint8
>
> +--ro protocol-minor-version uint16
> +--ro hierarchy-indications? enumeration
> +--rw flood-reduction? boolean
> +--rw nonce-increasing-interval? uint16
> +--rw maximum-nonce-delta? uint8 {nonce-delta-adjust}?
> +--rw rx-lie-multicast-address
> | +--rw ipv4? inet:ipv4-address
> | +--rw ipv6? inet:ipv6-address
> +--rw tx-lie-multicast-address
> | +--rw ipv4? inet:ipv4-address
> | +--rw ipv6? inet:ipv6-address
> +--rw lie-tx-port? inet:port-number
> +--rw global-link-capabilities
> | +--rw bfd? boolean
> | +--rw v4-forwarding-capable? boolean
> +--rw rx-flood-port? inet:port-number
> +--rw holdtime? rt-types:timer-value-seconds16
> +--rw tide-generation-interval? rt-types:timer-value-seconds16
> +--rw tie-security-key-id? uint32
> +--rw interface* [name]
> | +--ro link-id? linkid-type
> | +--rw name if:interface-ref
> | +--rw cost? uint32
> | +--rw advertised-source-addresses
> | | +--rw ipv4? inet:ipv4-address
> | | +--rw ipv6? inet:ipv6-address
> | +--ro direction-type? enumeration
> | +--ro was-the-last-lie-accepted? boolean
> | +--ro last-lie-reject-reason? string
> | +--ro advertised-in-lies
> | | +--ro you-are-flood-repeater? boolean
> | | +--ro not-a-ztp-offer? boolean
> | | +--ro you-are-sending-too-quickly? boolean
> | +--rw link-capabilities
> | | +--rw bfd? boolean
> | | +--rw v4-forwarding-capable? boolean
> | +--ro state enumeration
>
> Suggest adding per interface:
> - Number of flaps
> - Time duration in current state
>
> +--ro miscabled-links* linkid-type
> +--rw (algorithm-type)?
> | +--:(spf)
> | +--:(all-path)
> +--ro hal? level
> +--rw instance-label? uint32
>
> Suggest making this dependent on a {segment-routing} or maybe {label-switching} feature
>
> +--ro neighbor
> | +--ro nbrs* [system-id]
> | +--ro name? string
> | +--ro level? level
> | +--ro system-id system-id
> | +--ro pod? uint32
> | +--ro protocol-version? uint16
>
> protocol-minor-version
>
> | +--ro address-families
> | | +--ro address-family* [address-family]
> | | +--ro address-family iana-rt-types:address-family
> | +--ro received-source-addresses
> | | +--ro ipv4? inet:ipv4-address
> | | +--ro ipv6? inet:ipv6-address
> | +--ro link-id-pair* [remote-id]
> | | +--ro local-id? uint32
> | | +--ro remote-id uint32
> | | +--ro if-index? uint32
> | | +--ro if-name? if:interface-ref
> | +--ro cost? uint32
> | +--ro bandwidth? uint32
> | +--ro flood-reduction? boolean
> | +--ro sent-offer
> | | +--ro level? level
> | | +--ro not-a-ztp-offer? boolean
> | +--ro received-offer
> | | +--ro level? level
> | | +--ro not-a-ztp-offer? boolean
> | | +--ro best? boolean
> | | +--ro removed-from-consideration? boolean
> | | +--ro removal-reason? string
> | +--ro received-link-capabilities
> | | +--ro bfd? boolean
> | | +--ro v4-forwarding-capable? boolean
> | +--ro received-in-lies
> | | +--ro you-are-flood-repeater? boolean
> | | +--ro not-a-ztp-offer? boolean
> | | +--ro you-are-sending-too-quickly? boolean
> | +--ro tx-flood-port? inet:port-number
> | +--ro bfd-up? boolean
> | +--ro outer-security-key-id? uint8
> +--ro database
> | +--ro ties* [direction-type originator tie-type tie-number]
> | +--ro direction-type enumeration
> | +--ro originator system-id
> | +--ro tie-type enumeration
> | +--ro tie-number uint32
> | +--ro seq? uint64
> | +--ro origination-time? uint32
> | +--ro origination-lifetime? uint32
> | +--ro node
>
> Maybe I am misunderstanding something, but the list of Node TIE attributes below appears to be
> including several attributes that are not stored in the Node TIE, for example the
> "sent-offer" and "received-offer" attributes.
>
> | | +--ro name? string
> | | +--ro level? level
> | | +--ro system-id system-id
> | | +--ro pod? uint32
> | | +--ro flood-reduction? boolean
> | | +--ro overload? boolean
> | | +--ro startup-time? uint64
> | | +--ro neighbors* [system-id]
> | | | +--ro name? string
> | | | +--ro level? level
> | | | +--ro system-id system-id
> | | | +--ro pod? uint32
> | | | +--ro address-families
> | | | | +--ro address-family* [address-family]
> | | | | +--ro address-family iana-rt-types:address-family
> | | | +--ro received-source-addresses
> | | | | +--ro ipv4? inet:ipv4-address
> | | | | +--ro ipv6? inet:ipv6-address
> | | | +--ro link-id-pair* [remote-id]
> | | | | +--ro local-id? uint32
> | | | | +--ro remote-id uint32
> | | | | +--ro if-index? uint32
> | | | | +--ro if-name? if:interface-ref
> | | | +--ro cost? uint32
> | | | +--ro bandwidth? uint32
> | | | +--ro flood-reduction? boolean
> | | | +--ro sent-offer
> | | | | +--ro level? level
> | | | | +--ro not-a-ztp-offer? boolean
> | | | +--ro received-offer
> | | | | +--ro level? level
> | | | | +--ro not-a-ztp-offer? boolean
> | | | | +--ro best? boolean
> | | | | +--ro removed-from-consideration? boolean
> | | | | +--ro removal-reason? string
> | | | +--ro received-link-capabilities
> | | | | +--ro bfd? boolean
> | | | | +--ro v4-forwarding-capable? boolean
> | | | +--ro received-in-lies
> | | | | +--ro you-are-flood-repeater? boolean
> | | | | +--ro not-a-ztp-offer? boolean
> | | | | +--ro you-are-sending-too-quickly? boolean
> | | | +--ro tx-flood-port? inet:port-number
> | | | +--ro bfd-up? boolean
> | | | +--ro outer-security-key-id? uint8
> | | +--ro miscabled-links* linkid-type
> | +--ro prefix
> | +--ro prefix? inet:ip-prefix
> | +--ro (type)?
> | | +--:(prefix)
> | | +--:(positive-disaggregation)
> | | +--:(negative-disaggregation)
> | | +--:(external)
> | | +--:(positive-external-disaggregation)
> | | +--:(pgp)
> | +--ro metric? uint32
> | +--ro tags* uint64
> | +--ro monotonic-clock
> | | +--ro prefix-sequence-type
> | | +--ro timestamp ieee802-1as-timestamp-type
> | | +--ro transaction-id? uint8
> | +--ro loopback? boolean
> | +--ro directly-attached? boolean
> | +--ro from-link? linkid-type
>
> What made you model the kv-store separately? It seems more logical to me to model it as part
> of the TIE database (similar to how you model node TIEs and the various flavors of prefix TIEs)
>
> +--ro kv-store
> +--ro kvs* [kvs-index]
> +--ro kvs-index uint32
> +--ro kvs-tie
> +--ro direction-type? enumeration
> +--ro originator? system-id
> +--ro tie-type? enumeration
> +--ro tie-number? uint32
> +--ro seq? uint64
> +--ro origination-time? uint32
> +--ro origination-lifetime? uint32
> +--ro key-value
> +--ro key? uint16
> +--ro value? uint32
>
> notifications:
> +---n error-set
> +--ro tie-level-error
> | +--ro direction-type? enumeration
> | +--ro originator? system-id
> | +--ro tie-type? enumeration
> | +--ro tie-number? uint32
> | +--ro seq? uint64
> | +--ro origination-time? uint32
> | +--ro origination-lifetime? uint32
> +--ro neighbor-error
> +--ro neighbor* [system-id]
> +--ro name? string
> +--ro level? level
> +--ro system-id system-id
> +--ro pod? uint32
> +--ro protocol-version? uint16
> +--ro address-families
> | +--ro address-family* [address-family]
> | +--ro address-family iana-rt-types:address-family
> +--ro received-source-addresses
> | +--ro ipv4? inet:ipv4-address
> | +--ro ipv6? inet:ipv6-address
> +--ro link-id-pair* [remote-id]
> | +--ro local-id? uint32
> | +--ro remote-id uint32
> | +--ro if-index? uint32
> | +--ro if-name? if:interface-ref
> +--ro cost? uint32
> +--ro bandwidth? uint32
> +--ro flood-reduction? boolean
> +--ro sent-offer
> | +--ro level? level
> | +--ro not-a-ztp-offer? boolean
> +--ro received-offer
> | +--ro level? level
> | +--ro not-a-ztp-offer? boolean
> | +--ro best? boolean
> | +--ro removed-from-consideration? boolean
> | +--ro removal-reason? string
> +--ro received-link-capabilities
> | +--ro bfd? boolean
> | +--ro v4-forwarding-capable? boolean
> +--ro received-in-lies
> | +--ro you-are-flood-repeater? boolean
> | +--ro not-a-ztp-offer? boolean
> | +--ro you-are-sending-too-quickly? boolean
> +--ro tx-flood-port? inet:port-number
> +--ro bfd-up? boolean
> +--ro outer-security-key-id? uint8
>
>
>
> <rift20210216.yang><rift-tree20210217.txt><Diff_ rift-tree20210128.txt - rift-tree20210217.txt.html>
- [Rift] Comments on most recent version of the RIF… Bruno Rijsman
- Re: [Rift] Comments on most recent version of the… Bruno Rijsman