Re: [yang-doctors] How to restrict to have single control-plane-protocol instance

Martin Bjorklund <mbj@tail-f.com> Thu, 08 February 2018 08:20 UTC

Return-Path: <mbj@tail-f.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 6C5C912422F for <yang-doctors@ietfa.amsl.com>; Thu, 8 Feb 2018 00:20:17 -0800 (PST)
X-Virus-Scanned: amavisd-new at amsl.com
X-Spam-Flag: NO
X-Spam-Score: -1.91
X-Spam-Level:
X-Spam-Status: No, score=-1.91 tagged_above=-999 required=5 tests=[BAYES_00=-1.9, SPF_PASS=-0.001, T_RP_MATCHES_RCVD=-0.01, URIBL_BLOCKED=0.001] autolearn=ham autolearn_force=no
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 jKjh8m2j0DJH for <yang-doctors@ietfa.amsl.com>; Thu, 8 Feb 2018 00:20:13 -0800 (PST)
Received: from mail.tail-f.com (mail.tail-f.com [46.21.102.45]) by ietfa.amsl.com (Postfix) with ESMTP id 3E0BB120726 for <yang-doctors@ietf.org>; Thu, 8 Feb 2018 00:20:13 -0800 (PST)
Received: from localhost (unknown [173.38.220.45]) by mail.tail-f.com (Postfix) with ESMTPSA id E18D81AE046C; Thu, 8 Feb 2018 09:20:11 +0100 (CET)
Date: Thu, 08 Feb 2018 09:20:11 +0100 (CET)
Message-Id: <20180208.092011.1084955794834494213.mbj@tail-f.com>
To: rrahman@cisco.com
Cc: yang-doctors@ietf.org, Xufeng_Liu@jabil.com, zhang.zheng@zte.com.cn
From: Martin Bjorklund <mbj@tail-f.com>
In-Reply-To: <ED0403DF-840E-447B-A76D-7CDFF5E25C4B@cisco.com>
References: <ED0403DF-840E-447B-A76D-7CDFF5E25C4B@cisco.com>
X-Mailer: Mew version 6.7 on Emacs 24.5 / Mule 6.0 (HANACHIRUSATO)
Mime-Version: 1.0
Content-Type: Text/Plain; charset=utf-8
Content-Transfer-Encoding: base64
Archived-At: <https://mailarchive.ietf.org/arch/msg/yang-doctors/K_b47wwBXWbP-Q9OEONY4M9AX4Q>
Subject: Re: [yang-doctors] How to restrict to have single control-plane-protocol instance
X-BeenThere: yang-doctors@ietf.org
X-Mailman-Version: 2.1.22
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, 08 Feb 2018 08:20:17 -0000

Hi,

"Reshad Rahman (rrahman)" <rrahman@cisco.com> wrote:
> Hi YDs,
> 
> MSDP YANG authors want to enforce single-instance of MSDP
> control-plane protocol. The when “rt:type = ‘msdp’“ allows multiple
> control-pane-protocol instances as long as they have different
> rt:name. The only workaround I thought of is to have a when statement
> on the name in the top level container. This would still multiple
> control-plane-protocol instance of type msdp but restricts the name to
> a fixed name (msdp-protocol in this case) for the top level msdp
> container to exist. Any suggestions on how to do this better?

Hard-coding a name like this is IMO a bad idea.

Better would be to simply state in text that there MUST only be one
instance of this type.

But you can also add a must statement that enforces this:

   augment "/rt:routing/rt:control-plane-protocols/"
         + "rt:control-plane-protocol" {
      when 'derived-from-or-self(rt:type, "msdp:msdp"'  {
     container msdp {
       must 'count(/rt:routing/rt:control-plane-protocols/'
          + '      rt:control-plane-protocol['
          + '        derived-from-or-sel(../rt:type, "msdp:msdp")]) <= 1'";


In general, you should be careful with the usage of "count", since it
will loop through *all* instances in the list every time.  If the list
is big, this can have a performance impact.

Also note that I use derived-from-or-self instead of equality for the
identity.


/martin


> 
> Regards,
> Reshad.
> 
>   augment "/rt:routing/rt:control-plane-protocols/"
>         + "rt:control-plane-protocol" {
>      when "rt:type = ‘msdp’"  {
>       description
>         "….”;
>     }
>     description "….";
> 
>     container msdp {
>       when "../rt:name = ‘msdp-protocol’"  {
>         description
>           "….";
>       }
>       description "MSDP top level container.";
> 
> 
> From: "Reshad Rahman (rrahman)" <rrahman@cisco.com>
> Date: Monday, February 5, 2018 at 6:25 PM
> To: Xufeng Liu <Xufeng_Liu@jabil.com>om>, "zhang.zheng@zte.com.cn"
> <zhang.zheng@zte.com.cn>
> Cc: "anish.ietf@gmail.com" <anish.ietf@gmail.com>om>, "Mahesh Sivakumar
> (masivaku)" <masivaku@cisco.com>om>, "guofeng@huawei.com"
> <guofeng@huawei.com>om>, "pete.mcallister@metaswitch.com"
> <pete.mcallister@metaswitch.com>om>, "liuyisong@huawei.com"
> <liuyisong@huawei.com>om>, "xu.benchong@zte.com.cn"
> <xu.benchong@zte.com.cn>cn>, "tanmoy.kundu@alcatel-lucent.com"
> <tanmoy.kundu@alcatel-lucent.com>om>, "zzhang_ietf@hotmail.com"
> <zzhang_ietf@hotmail.com>om>, "Acee Lindem (acee)" <acee@cisco.com>
> Subject: Re: Hi all, about the modification of MSDP YANG
> 
> Hi Sandy and Xufeng,
> 
> I understand that you want only 1 MSDP instance but I don’t think that
> justifies /rt:routing/rt:control-plane-protocols. If we do that we
> will end up with all single-instance protocols under
> /rt:routing/rt:control-plane-protocols and all the multi-instance ones
> under
> /rt:routing/rt:control-plane-protocols/rt:control-plane-protocol.
> 
> I am not sure what’s the best way to enforce single-instance, I can
> check with the other YDs on this topic. One way it can be done is as
> follows (I’ve added the when statement in bold to existing BFD model),
> it enforces that the protocol name is ‘bfdv1’. So multiple instances
> with rt:type=bfd-types:bfdv1 could be created, but only one of these
> instances can have the bfd container. This is probably not the best
> way but the point is that IMO protocols have to go under
> /rt:routing/rt:control-plane-protocols/rt:control-plane-protocol.
> 
> Regards,
> Reshad.
> 
>   augment "/rt:routing/rt:control-plane-protocols/"
>         + "rt:control-plane-protocol" {
>      when "rt:type = 'bfd-types:bfdv1'"  {
>       description
>         "This augmentation is only valid for a control-plane protocol
>          instance of BFD (type 'bfdv1').";
>     }
>     description "BFD augmentation.";
> 
>     container bfd {
>       when "../rt:name = 'bfdv1'"  {
>         description
>           "This augmentation is only valid for a control-plane protocol
>            instance of BFD (type 'bfdv1').";
>       }
>       description "BFD top level container.";
> 
> From: Xufeng Liu <Xufeng_Liu@jabil.com>
> Date: Monday, February 5, 2018 at 9:38 AM
> To: "zhang.zheng@zte.com.cn" <zhang.zheng@zte.com.cn>
> Cc: "Reshad Rahman (rrahman)" <rrahman@cisco.com>om>,
> "anish.ietf@gmail.com" <anish.ietf@gmail.com>om>, "Mahesh Sivakumar
> (masivaku)" <masivaku@cisco.com>om>, "guofeng@huawei.com"
> <guofeng@huawei.com>om>, "pete.mcallister@metaswitch.com"
> <pete.mcallister@metaswitch.com>om>, "liuyisong@huawei.com"
> <liuyisong@huawei.com>om>, "xu.benchong@zte.com.cn"
> <xu.benchong@zte.com.cn>cn>, "tanmoy.kundu@alcatel-lucent.com"
> <tanmoy.kundu@alcatel-lucent.com>om>, "zzhang_ietf@hotmail.com"
> <zzhang_ietf@hotmail.com>
> Subject: RE: Hi all, about the modification of MSDP YANG
> 
> Hi Sandy,
> 
> Thanks for the updates.
> 
> In RFC8022bis, the rt:type is defined under
> /rt:routing/rt:control-plane-protocols/rt:control-plane-protocol. If
> we augment /rt:routing/rt:control-plane-protocols, the “when”
> statement will not be valid, because it cannot find the rt:type. I
> don’t think that we need the “when” statement. The container with
> “presence” will serve the purpose of the identity. We can simply take
> out the “when” statement and the definition of the MSDP identity.
> 
> Thanks,
> - Xufeng
> 
> From: zhang.zheng@zte.com.cn [mailto:zhang.zheng@zte.com.cn]
> Sent: Monday, February 5, 2018 3:36 AM
> To: Xufeng Liu <Xufeng_Liu@jabil.com>
> Cc: rrahman@cisco.com; anish.ietf@gmail.com; masivaku@cisco.com;
> guofeng@huawei.com; pete.mcallister@metaswitch.com;
> liuyisong@huawei.com; xu.benchong@zte.com.cn;
> tanmoy.kundu@alcatel-lucent.com; zzhang_ietf@hotmail.com
> Subject: RE: Hi all, about the modification of MSDP YANG
> 
> 
> Hi Xufeng and Reshad,
> 
> 
> 
> I am sorry for forgetting the point. I updated the YANG model.
> 
> If no one has comments on it I'd like to submit the new version. :-)
> 
> 
> 
> Thanks,
> 
> Sandy
> 原始邮件
> 发件人: <Xufeng_Liu@jabil.com<mailto:Xufeng_Liu@jabil.com>>;
> 收件人: <rrahman@cisco.com<mailto:rrahman@cisco.com>>;张征00007940;
> <anish.ietf@gmail.com<mailto:anish.ietf@gmail.com>>;
> <masivaku@cisco.com<mailto:masivaku@cisco.com>>;
> <guofeng@huawei.com<mailto:guofeng@huawei.com>>;
> <pete.mcallister@metaswitch.com<mailto:pete.mcallister@metaswitch.com>>;
> <liuyisong@huawei.com<mailto:liuyisong@huawei.com>>;徐本崇10065053;
> <tanmoy.kundu@alcatel-lucent.com<mailto:tanmoy.kundu@alcatel-lucent.com>>;
> <zzhang_ietf@hotmail.com<mailto:zzhang_ietf@hotmail.com>>;
> 日 期 :2018年02月03日 01:21
> 主 题 :RE: Hi all, about the modification of MSDP YANG
> Hi Sandy and Reshad,
> 
> The reason that we used to augment
> /rt:routing/rt:control-plane-protocols, instead of
> /rt:routing/rt:control-plane-protocols/rt:control-plane-protocol, is
> that we do not allow multiple instances of MSDP.
> 
> Thanks,
> - Xufeng
> 
> From: Reshad Rahman (rrahman) [mailto:rrahman@cisco.com]
> Sent: Friday, February 2, 2018 12:08 PM
> To: zhang.zheng@zte.com.cn<mailto:zhang.zheng@zte.com.cn>; Xufeng Liu
> <Xufeng_Liu@jabil.com<mailto:Xufeng_Liu@jabil.com>>;
> anish.ietf@gmail.com<mailto:anish.ietf@gmail.com>; Mahesh Sivakumar
> (masivaku) <masivaku@cisco.com<mailto:masivaku@cisco.com>>;
> guofeng@huawei.com<mailto:guofeng@huawei.com>;
> pete.mcallister@metaswitch.com<mailto:pete.mcallister@metaswitch.com>;
> liuyisong@huawei.com<mailto:liuyisong@huawei.com>;
> xu.benchong@zte.com.cn<mailto:xu.benchong@zte.com.cn>;
> tanmoy.kundu@alcatel-lucent.com<mailto:tanmoy.kundu@alcatel-lucent.com>;
> zzhang_ietf@hotmail.com<mailto:zzhang_ietf@hotmail.com>
> Subject: Re: Hi all, about the modification of MSDP YANG
> 
> Hi Sandy,
> 
> I don’t know what warning you are getting now but from a quick look at
> the revision you sent I see couple of issues.
> 
>      identity msdp {
>        base "rt:routing-protocol";  <== should be rt:control-plane-protocol
>        description "MSDP";
>      }
> <snip>
>      /*
>       * Data nodes
>       */
>      augment
>      "/rt:routing/rt:control-plane-protocols/rt:control-plane-protocol" {
>         when "rt:type = 'MSDP'" { <== should be "rt:type = 'msdp:msdp'"
> 
> 
> HTH,
> Reshad.
> 
> From: "zhang.zheng@zte.com.cn<mailto:zhang.zheng@zte.com.cn>"
> <zhang.zheng@zte.com.cn<mailto:zhang.zheng@zte.com.cn>>
> Date: Friday, February 2, 2018 at 4:37 AM
> To: "xufeng_liu@jabil.com<mailto:xufeng_liu@jabil.com>"
> <xufeng_liu@jabil.com<mailto:xufeng_liu@jabil.com>>,
> "anish.ietf@gmail.com<mailto:anish.ietf@gmail.com>"
> <anish.ietf@gmail.com<mailto:anish.ietf@gmail.com>>, "Mahesh Sivakumar
> (masivaku)" <masivaku@cisco.com<mailto:masivaku@cisco.com>>,
> "guofeng@huawei.com<mailto:guofeng@huawei.com>"
> <guofeng@huawei.com<mailto:guofeng@huawei.com>>,
> "pete.mcallister@metaswitch.com<mailto:pete.mcallister@metaswitch.com>"
> <pete.mcallister@metaswitch.com<mailto:pete.mcallister@metaswitch.com>>,
> "liuyisong@huawei.com<mailto:liuyisong@huawei.com>"
> <liuyisong@huawei.com<mailto:liuyisong@huawei.com>>,
> "xu.benchong@zte.com.cn<mailto:xu.benchong@zte.com.cn>"
> <xu.benchong@zte.com.cn<mailto:xu.benchong@zte.com.cn>>,
> "tanmoy.kundu@alcatel-lucent.com<mailto:tanmoy.kundu@alcatel-lucent.com>"
> <tanmoy.kundu@alcatel-lucent.com<mailto:tanmoy.kundu@alcatel-lucent.com>>,
> "zzhang_ietf@hotmail.com<mailto:zzhang_ietf@hotmail.com>"
> <zzhang_ietf@hotmail.com<mailto:zzhang_ietf@hotmail.com>>
> Cc: "Reshad Rahman (rrahman)"
> <rrahman@cisco.com<mailto:rrahman@cisco.com>>
> Subject: FW: Hi all, about the modification of MSDP YANG
> 
> 
> Hi all,
> 
> 
> 
> I deleted some groupings and make the model more clear.
> 
> And I updated the decription of (peer-as, up-time, expire).  Please
> review it.
> 
> 
> 
> A warning is still existing about rt:type:
> 
> 5, - augment of control-plane-protocols is incorrect. There should be
> an identity msdp with
> 
> base "rt:routing-protocol" and then augment
> 
> "/rt:routing/rt:control-plane-protocols/rt:control-plane-protocol"
> with a when
> 
> statement. Take a look at OSPF YANG for an example.
> 
> [Sandy]: Added the identity and modify the augmentation, but it seems
> like there is no MSDP register in rt:type.
> 
> How can we register it?
> 
> 
> 
> Thanks,
> 
> Sandy
> 原始邮件
> 发件人:张征00007940
> 收件人: <xufeng_liu@jabil.com<mailto:xufeng_liu@jabil.com>>;
> <anish.ietf@gmail.com<mailto:anish.ietf@gmail.com>>;
> <masivaku@cisco.com<mailto:masivaku@cisco.com>>;
> <guofeng@huawei.com<mailto:guofeng@huawei.com>>;
> <pete.mcallister@metaswitch.com<mailto:pete.mcallister@metaswitch.com>>;
> <liuyisong@huawei.com<mailto:liuyisong@huawei.com>>;徐本崇10065053;
> <tanmoy.kundu@alcatel-lucent.com<mailto:tanmoy.kundu@alcatel-lucent.com>>;
> <zzhang_ietf@hotmail.com<mailto:zzhang_ietf@hotmail.com>>;
> 抄送人: <rrahman@cisco.com<mailto:rrahman@cisco.com>>;
> 日 期 :2018年01月29日  17:04
> 主 题 :Hi all, about the modification of MSDP YANG
> 
> Hi all,
> 
> 
> 
> YANG doctor Reshad had finished the early review about MSDP YANG.
> 
> I finished the preliminary modification version, please review it.
> 
> I think some advices from Reshad should be discussed:
> 
> 
> 
> 1, - Not sure why peer-as is needed. Don't see it in RFC3618.
> 
> 2, - leaf up-time, what's meant by "up time" in the description? Is it
> time it's
> 
> been created?
> 
> 3, - description for leaf expire seems wrong.
> 
> [Sandy]: These items (peer-as, up-time, expire) doesn't existed in
> RFC3618, are these unnecessary? Please write down your
> 
> description if you insist on it. If nobody insist on it, should we
> delete them?
> 
> 
> 
> 4, - Groupings are used for data which is used only once. Is this done
> on purpose or
> 
> was the intention to use those groupings more than once?
> 
> [Sandy]: These eight groupings are used only once, should we change
> them to container?
> 
> authentication-container;
> 
> global-config-attributes;
> 
> peer-config-attributes;
> 
> peer-state-attributes;
> 
> sa-cache-state-attributes;
> 
> statistics-container
> 
> statistics-error
> 
> statistics-queue
> 
> 
> 
> 5, - augment of control-plane-protocols is incorrect. There should be
> an identity msdp with
> 
> base "rt:routing-protocol" and then augment
> 
> "/rt:routing/rt:control-plane-protocols/rt:control-plane-protocol"
> with a when
> 
> statement. Take a look at OSPF YANG for an example.
> 
> [Sandy]: Added the identity and modify the augmentation, but it seems
> like there is no MSDP register in rt:type.
> 
> How can we register it?
> 
> 
> 
> 
> 
> Most of the suggestion is adopted. The modification detail pls see
> below:
> 
> 
> 
> - Too many features (17)! Every piece of config has an if-feature
> - statement.
> 
> Some of the configs (timers?) should be part of most/basic
> implementations, for
> 
> other config (e.g. authentication) I can see why a feature would be
> used.
> 
> [Sandy]: Modified the three timers (connect-retry, hold, keepalive) to
> fixed format.
> 
> 
> 
> -“import ietf-yang-types” should have a reference to RFC6991 (see
> -section 4.7 of
> 
> rfc6087bis-15)
> 
> - “import ietf-inet-types” should have a reference to RFC6991
> 
> - “import ietf-routing” should have a reference to RFC8022
> 
> - “import ietf-interfaces” should have a reference to RFC7223
> 
> - "import ietf-ip" should have a reference to RFC7277
> 
> - "import ietf-key-chain" should have a reference to RFC8177
> 
> [Sandy]: Added all the references above.
> 
> 
> 
> - organization s/"...PIM( Protocols for IP Multicast ) Working
> 
> Group"/"...PIM (Protocols for IP Multicast) Working Group"?
> 
> - Remove WG Chairs from contact information as per Appendix C of
> - rfc6087bis-15
> 
> - No copyright in the module description, see Appendix of 6087bis-15 for
> - a module description
> 
> example
> 
> - Module description must contain reference to RFC, see Appendix C of
> 
> rfc6087bis-15
> 
> [Sandy]: Removed WG chairs and add copyright from Appendix of
> rfc6087bis. Added reference to RFC3618.
> 
> 
> 
> - grouping authentication-container. key-chain and password both
> 
> use if-feature peer-key-chain.
> 
> [Sandy]: Removed the if-feature peer-key-chain from password.
> 
> 
> 
> - grouping connect-source. The name is not very
> 
> descriptive. Should this be something along the lines of
> tcp-connection-source?
> 
> [Sandy]: Changed the name "connect-source" to "tcp-connection-source".
> 
> 
> 
> - grouping global-state-attributes has nothing
> 
> [Sandy]: Deleted the grouping.
> 
> 
> 
> - Some of the descriptions are
> 
> pretty terse. e.g. for rpf-peer it says "RPF peer.". In a case like
> this
> 
> consider adding more descriptive text or a reference to the proper
> section in
> 
> RFC3618
> 
> [Sandy]: Added more description.
> 
> 
> 
> - peer-as (Autonomous System Number) is defined as type string, should
> 
> be of type as-number in ietf-inet-types?
> 
> [Sandy]: Modified to inet types.
> 
> 
> 
> - keepalive-interval depends on holdtime-interval.
> 
> There should be "if-feature peer-timer-holdtime" under leaf
> keepalive-interval
> 
> or change the must statement to (assuming we keep the 2 features):
> 
>   must "(not ../holdtime-interval) or (. > 1 and . <
>   ../holdtime-interval)".
> 
> [Sandy]: Modified the features to fixed format.
> 
> 
> 
> - leaf up-time: s/sa cache/SA cache/
> 
> - leaf peer-learned-from, change description from "The address of peer
> - that we learned
> 
> this SA from ." to "The address of the peer that we learned this SA
> from."
> 
> [Sandy]: Modified.
> 
> 
> 
> - RPC leaf group, I thought we had a type for IP multicast address? If
> - not, it should be done?
> 
> [Sandy]: Yes. Added the rt-type reference to RFC8294.
> 
> 
> 
> - s/msdp/MSDP/
> 
> - In rpc msdp-clear-peer, s/Clears the session to the peer./Clears
> 
> the TCP connection to the peer./
> 
> - In rpc msdp-clear-sa-cache, why have the enum '*' for
> - source-addr. Can't the same technique as for peer-address be
> 
> 
> 
> used?
> 
> - msdp prefix not needed in rpc names
> 
> [Sandy]: Done.
> 
> 
> 
> - MSDP peers are configured in a mesh-group, did the authors consider
> - adding state per mesh-group, e.g. all the
> 
> peers in a particular mesh-group?
> 
> [Sandy]: IMO it is unnecessary because the states of peers is not
> unified in a mesh-group.
> 
> 
> 
> General:
> 
> - Per Appendix B of rfc6087bis-15: "that all YANG modules containing
> 
> imported items are cited as normative reference". So RFCs 6991, 7223,
> 
> 7277, 8022 and 8177 should be included in the normative reference
> 
> section.
> 
> [Sandy]: Added.
> 
> 
> 
> - Section 3 "the irrelevant information", add a reference/explanation
> - for what
> 
> the irrelevant information is. s/the irrelevant information/irrelevant
> 
> information/?
> 
> [Sandy]: Changed the description.
> 
> 
> 
> - Section 5 should give a brief description of what the RPCs do.
> 
> [Sandy]: Added some description.
> 
> 
> 
> - Section 6 any plans for notifications? If not, just say so.
> 
> [Sandy]: Done.
> 
> 
> 
> - Need Security
> 
> Considerations, see sections 3.7 and 6 of rfc6087bis-15
> 
> [Sandy]: Added security consideration section.
> 
> 
> 
> - Need IANA Considerations, see section 3.8 of rfc6087bis-15
> 
> [Sandy]: Added IANA considerations.
> 
> 
> 
> - Need license in YANG module,
> 
> see appendix B of rfc6087bis-15
> 
> [Sandy]: Added the YANG module description.
> 
> 
> 
> Thanks,
> 
> Sandy
> 
> 
> 
> 
> 
>