Re: [pim] Yangdoctors last call review of draft-ietf-pim-igmp-mld-yang-07
Jan Lindblad <janl@tail-f.com> Tue, 11 September 2018 13:52 UTC
Return-Path: <janl@tail-f.com>
X-Original-To: pim@ietfa.amsl.com
Delivered-To: pim@ietfa.amsl.com
Received: from localhost (localhost [127.0.0.1]) by ietfa.amsl.com (Postfix) with ESMTP id E9776130DDB; Tue, 11 Sep 2018 06:52:10 -0700 (PDT)
X-Virus-Scanned: amavisd-new at amsl.com
X-Spam-Flag: NO
X-Spam-Score: -1.899
X-Spam-Level:
X-Spam-Status: No, score=-1.899 tagged_above=-999 required=5 tests=[BAYES_00=-1.9, HTML_MESSAGE=0.001, SPF_PASS=-0.001, 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 0FL6JXfmG3_M; Tue, 11 Sep 2018 06:52:04 -0700 (PDT)
Received: from mail.tail-f.com (mail.tail-f.com [46.21.102.45]) by ietfa.amsl.com (Postfix) with ESMTP id 21FEC130DF5; Tue, 11 Sep 2018 06:52:03 -0700 (PDT)
Received: from [10.147.40.194] (unknown [173.38.220.60]) by mail.tail-f.com (Postfix) with ESMTPSA id 420371AE03F5; Tue, 11 Sep 2018 15:52:01 +0200 (CEST)
From: Jan Lindblad <janl@tail-f.com>
Message-Id: <2579BF3E-4406-412A-958F-6D2C5CC1B3E9@tail-f.com>
Content-Type: multipart/alternative; boundary="Apple-Mail=_72D27405-E73C-4350-B10C-E175BD5A30C1"
Mime-Version: 1.0 (Mac OS X Mail 11.5 \(3445.9.1\))
Date: Tue, 11 Sep 2018 15:51:57 +0200
In-Reply-To: <CAEz6PPT6UFPUdFVm0ACM3vLh9vXt1dtJjtgrbEZ=hiDPmy=Q3g@mail.gmail.com>
Cc: yang-doctors@ietf.org, ietf <ietf@ietf.org>, pim@ietf.org, draft-ietf-pim-igmp-mld-yang.all@ietf.org
To: Xufeng Liu <xufeng.liu.ietf@gmail.com>
References: <153417087203.25020.10120277992606371332@ietfa.amsl.com> <CAEz6PPT6UFPUdFVm0ACM3vLh9vXt1dtJjtgrbEZ=hiDPmy=Q3g@mail.gmail.com>
X-Mailer: Apple Mail (2.3445.9.1)
Archived-At: <https://mailarchive.ietf.org/arch/msg/pim/2ETGh_pAR0Mly6hK_hJKAi1YFy0>
X-Mailman-Approved-At: Wed, 19 Sep 2018 16:07:35 -0700
Subject: Re: [pim] Yangdoctors last call review of draft-ietf-pim-igmp-mld-yang-07
X-BeenThere: pim@ietf.org
X-Mailman-Version: 2.1.29
Precedence: list
List-Id: Protocol Independent Multicast <pim.ietf.org>
List-Unsubscribe: <https://www.ietf.org/mailman/options/pim>, <mailto:pim-request@ietf.org?subject=unsubscribe>
List-Archive: <https://mailarchive.ietf.org/arch/browse/pim/>
List-Post: <mailto:pim@ietf.org>
List-Help: <mailto:pim-request@ietf.org?subject=help>
List-Subscribe: <https://www.ietf.org/mailman/listinfo/pim>, <mailto:pim-request@ietf.org?subject=subscribe>
X-List-Received-Date: Tue, 11 Sep 2018 13:52:11 -0000
Xufeng, > Thanks for the review and valuable comments. > > In regard to item #1, there was a discussion thread among the Yang Doctors, authors of RFC 8349, and Routing Area Yang Architecture Design Team, as attached below. The discussion occurred during the review of a draft with the same issue as this one. I see, didn't know. Good. If this has been discussed to conclusion, then you should of course go with that decision. As mentioned earlier, there are a few other singleton protocols mapped into this structure, e.g. static. I think it would make sense to treat this the same. Principle of least astonishment. Best Regards, /jan > ================================ > 原始邮件 > 发件人:XufengLiu <Xufeng_Liu@jabil.com <mailto:Xufeng_Liu@jabil.com>> > 收件人:Acee Lindem (acee) <acee@cisco.com <mailto:acee@cisco.com>>Christian Hopps <chopps@chopps.org <mailto:chopps@chopps.org>>Martin Bjorklund <mbj@tail-f.com <mailto:mbj@tail-f.com>> > 抄送人:张征00007940;yang-doctors@ietf.org <mailto:00007940%3Byang-doctors@ietf.org> <yang-doctors@ietf.org <mailto:yang-doctors@ietf.org>> > 日 期 :2018年02月20日 22:30 > 主 题 :RE: [yang-doctors] How to restrict to have singlecontrol-plane-protocol instance > Using "" as the name is better, but I am not sure that it is good enough. When we use ConfD to translate the model to a command line, if the option "tailf:cli-expose-key-name" is not used, we will have: > > edit routing control-plane-protocols control-plane-protocol type msdp name ''" > > If the option "tailf:cli-expose-key-name" is used, we will have: > > edit routing control-plane-protocols control-plane-protocol msdp ''" > > I am pretty sure that we would get a bug report on this, asking what is the purpose to have: name ''", and requesting a suppression on the term, but we do not have a good way to achieve. > > As a comparison, the option #3 will give: > > edit routing control-plane-protocols msdp > > This is the only acceptable solution so far. When a model is not usable by the end-user, other considerations (such as augmentation convenience) will not matter. > > Thanks, > - Xufeng > > > -----Original Message----- > > From: Acee Lindem (acee) [mailto:acee@cisco.com <mailto:acee@cisco.com>] > > Sent: Monday, February 19, 2018 1:35 PM > > To: Christian Hopps <chopps@chopps.org <mailto:chopps@chopps.org>>; Martin Bjorklund <mbj@tail-f.com <mailto:mbj@tail-f.com>> > > Cc: Xufeng Liu <Xufeng_Liu@jabil.com <mailto:Xufeng_Liu@jabil.com>>; zhang.zheng@zte.com.cn <mailto:zhang.zheng@zte.com.cn>; yang- > > doctors@ietf.org <mailto:doctors@ietf.org> > > Subject: Re: [yang-doctors] How to restrict to have single control-plane- > > protocol instance > > > > > > > > On 2/19/18, 5:02 AM, "Christian Hopps" <chopps@chopps.org <mailto:chopps@chopps.org>> wrote: > > > > > > Martin Bjorklund <mbj@tail-f.com <mailto:mbj@tail-f.com>> writes: > > > > > Hi, > > > > > > "Acee Lindem (acee)" <acee@cisco.com <mailto:acee@cisco.com>> wrote: > > >> All, > > >> > > >> As seems to be the modus operandi for YANG issues, we have 3 separate > > opinions as to how a protocol only supporting a single instance should be > > realized. > > >> > > >> 1. Augment the existing control plane protocols list (RFC 8022BIS) > > >> and specify in the description text that only a single instance is > > >> supported. > > >> 2. Augment the existing control plane protocols list (RFC 8022BIS) > > >> and use a YANG 1.1 must() restriction as discussed by Martin and > > >> Lada. > > >> 3. Augment the container one level up from the list for singleton > > >> protocols (suggested by Xufeng). > > > > > But I think there was also a proposal to require the single instance > > > to have a well-known name - but maybe this proposal is no longer on > > > the table. > > > > I actually liked this solution; however, instead of picking an arbitrary "well- > > known" value for name, I would specify the 0 length string instead. I think that > > reinforces the idea that this isn't actually a named instance. :) > > > > augment "/rt:routing/rt:control-plane-protocols/" > > + "rt:control-plane-protocol" { > > when "derived-from-or-self(rt:type, 'msdp:msdp') and rt:name = ''" { > > container msdp { > > > > One benefit of this solution is that it solves Xufeng's issue of what the client uses > > as the instance name. > > > > > > Thanks, > > Chris. > > > > > > > > > > > /martin > > > > > > > > >> and #3. For #3, one determent would be that the control plane protocols > > are in a location other than where they were originally envisioned and I don't > > relish pulling RFC8022BIS off the RFC queue to document. > > >> > > >> Thanks, > > >> Acee > > >> > > >> On 2/15/18, 8:39 AM, "Reshad Rahman (rrahman)" <rrahman@cisco.com <mailto:rrahman@cisco.com>> > > wrote: > > >> > > >> Hi Xufeng, > > >> > > >> I think the intent of 8022bis was to have all protocols under > > /rt:routing/rt:control-plane-protocols/rt:control-plane-protocol. I agree that > > forcing a name for a single-instance is cumbersome, but I think it is too late to > > change tree hierachy organization at this point. > > >> > > >> I will defer to other YDs and 8022bis authors on this. > > >> > > >> Regards, > > >> Reshad. > > >> > > >> On 2018-02-08, 9:48 AM, "Xufeng Liu" <Xufeng_Liu@jabil.com <mailto:Xufeng_Liu@jabil.com>> wrote: > > >> > > >> Hi All, > > >> > > >> I feel that such a solution is still not clean enough to outweigh the > > simple augmentation to "/rt:routing/rt:control-plane-protocols/". > > >> > > >> Some considerations are: > > >> > > >> - Name management: Neither the operator nor the implementation > > wants to deal with the artificial name, whether it is hardcoded, user-configured, > > or system-generated. When we implement such singleton protocol, we don't > > save a name anywhere. > > >> - The complexity of validation: The "when" statement is an > > unnecessary expense to the user and to the implementation, especially if we > > need to check all instances. > > >> - Data tree query: If the singleton "MSDP" is mixed with other protocol > > instances, it is less obvious or harder to search for. Depending on the > > implementation, it would be worse if the entire list needs to be iterated. > > >> - Tree hierarchy organization: I don't see too big a problem 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". If necessary, some of the names can be adjusted. > > >> > > >> Thanks, > > >> - Xufeng > > >> > > >> > > >> > -----Original Message----- > > >> > From: Reshad Rahman (rrahman) [mailto:rrahman@cisco.com <mailto:rrahman@cisco.com>] > > >> > Sent: Thursday, February 8, 2018 9:41 AM > > >> > To: Ladislav Lhotka <lhotka@nic.cz <mailto:lhotka@nic.cz>>; Martin Bjorklund <mbj@tail- > > f.com <http://f.com/>>; > > >> > Acee Lindem (acee) <acee@cisco.com <mailto:acee@cisco.com>> > > >> > Cc: yang-doctors@ietf.org <mailto:yang-doctors@ietf.org>; zhang.zheng@zte.com.cn <mailto:zhang.zheng@zte.com.cn>; Xufeng Liu > > >> > <Xufeng_Liu@jabil.com <mailto:Xufeng_Liu@jabil.com>> > > >> > Subject: Re: [yang-doctors] How to restrict to have single control- > > plane- > > >> > protocol instance > > >> > > > >> > Thanks for the suggestions. I agree that hard-coding the name is a > > bad idea, > > >> > glad that a cleaner way of doing this is possible. > > >> > - We can move the must statement up to restrict max of 1 control- > > plane- > > >> > protocol instance of type msdp? > > >> > - Acee/Lada, should a note be added to section 5.3 of 8022bis > > regarding how > > >> > to enforce single instance? How much of a concern is the > > performance > > >> > impact in this specific case? > > >> > > > >> > Regards, > > >> > Reshad. > > >> > > > >> > On 2018-02-08, 7:02 AM, "Ladislav Lhotka" <lhotka@nic.cz <mailto:lhotka@nic.cz>> wrote: > > >> > > > >> > On Thu, 2018-02-08 at 12:39 +0100, Martin Bjorklund wrote: > > >> > > "Acee Lindem (acee)" <acee@cisco.com <mailto:acee@cisco.com>> wrote: > > >> > > > Hi Lada, > > >> > > > > >> > > > > > >> > > > > >> > > > On 2/8/18, 4:42 AM, "yang-doctors on behalf of Ladislav > > Lhotka" > > >> > <yang-docto > > >> > > rs-bounces@ietf.org <mailto:rs-bounces@ietf.org> on behalf of lhotka@nic.cz <mailto:lhotka@nic.cz>> wrote: > > >> > > > > >> > > > > > >> > > > > >> > > > On Thu, 2018-02-08 at 09:20 +0100, Martin Bjorklund wrote: > > >> > > > > >> > > > > Hi, > > >> > > > > >> > > > > > > >> > > > > >> > > > > "Reshad Rahman (rrahman)" <rrahman@cisco.com <mailto: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. > > >> > > > > >> > > > > > >> > > > > >> > > > Instead of count(), it is possible to use the so-called > > Muenchian > > >> > > method: > > >> > > > > >> > > > > > >> > > > > >> > > > container msdp { > > >> > > > > >> > > > must "not(../preceding-sibling::rt:control-plane- > > protocol[" > > >> > > > > >> > > > + "derived-from-or-self(rt:type, 'msdp:msdp')])"; > > >> > > > > >> > > > .. > > >> > > > > >> > > > } > > >> > > > > >> > > > > > >> > > > > >> > > > It basically states that the control-plane-protocol containing > > the > > >> > > "msdp" > > >> > > > > >> > > > container must not be preceded with a control-plane- > > protocol entry > > >> > of > > >> > > the > > >> > > > > >> > > > msdp:msdp type (or derived). > > >> > > > > >> > > > > > >> > > > > >> > > > This looks like an elegant solution. > > >> > > > > >> > > > > >> > > "elegant" as in "less obvious" ;) It has the same time complexity > > as > > >> > > the count() solution. > > >> > > > >> > It should be faster on the average - it has to scan only preceding > > siblings of > > >> > the MSDP protocol instance whereas count() always has to check > > *all* > > >> > protocol > > >> > instances. > > >> > > > >> > It is true though that in XSLT this technique can be made > > considerably > > >> > more > > >> > efficient by using indexed keys. > > >> > > > >> > Lada > > >> > > > >> > > > > >> > > > > >> > > However, since the key for the control-plane-protocol list is > > "type > > >> > > name", won't it only work if the previous sibling has a "name" > > that > > >> > > is precedes the one being added? > > >> > > > > >> > > For each list entry that has this container, the expression is > > >> > > evaluated. It will scan all preceding entries and ensure that there > > >> > > are none with this type. So the order of the entries doesn't > > matter; > > >> > > if there are two with the same type, one of them has to be > > before the > > >> > > other. > > >> > > > > >> > > > > >> > > /martin > > >> > > > > >> > > > > >> > > > > > >> > > > > >> > > > Thanks, > > >> > > > > >> > > > Acee > > >> > > > > >> > > > > > >> > > > > >> > > > > > >> > > > > >> > > > Lada > > >> > > > > >> > > > > > >> > > > > >> > > > > > > >> > > > > >> > > > > 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 <mailto:rrahman@cisco.com>> > > >> > > > > >> > > > > > Date: Monday, February 5, 2018 at 6:25 PM > > >> > > > > >> > > > > > To: Xufeng Liu <Xufeng_Liu@jabil.com <mailto:Xufeng_Liu@jabil.com>>, > > >> > "zhang.zheng@zte.com.cn <mailto:zhang.zheng@zte.com.cn>" > > >> > > > > >> > > > > > <zhang.zheng@zte.com.cn <mailto:zhang.zheng@zte.com.cn>> > > >> > > > > >> > > > > > Cc: "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 <http://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>>, "Acee Lindem (acee)" > > >> > <acee@cisco.com <mailto: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 <mailto:Xufeng_Liu@jabil.com>> > > >> > > > > >> > > > > > Date: Monday, February 5, 2018 at 9:38 AM > > >> > > > > >> > > > > > To: "zhang.zheng@zte.com.cn <mailto:zhang.zheng@zte.com.cn>" > > <zhang.zheng@zte.com.cn <mailto:zhang.zheng@zte.com.cn>> > > >> > > > > >> > > > > > Cc: "Reshad Rahman (rrahman)" <rrahman@cisco.com <mailto:rrahman@cisco.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 <http://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>> > > >> > > > > >> > > > > > 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> > > [mailto: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 <mailto:Xufeng_Liu@jabil.com>> > > >> > > > > >> > > > > > Cc: rrahman@cisco.com <mailto:rrahman@cisco.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>; 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 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><mailto:Xufeng_Liu@jabil.com <mailto:Xufeng_Liu@jabil.com>>>; > > >> > ================================ > > On Mon, Aug 13, 2018 at 10:34 AM Jan Lindblad <janl@tail-f.com <mailto:janl@tail-f.com>> wrote: > Reviewer: Jan Lindblad > Review result: On the Right Track > > This is my YANG-doctor review of draft-ietf-pim-igmp-mld-yang-07. In the > spring, I did an early review of the -02 version. > > Most of the comments from the earlier review are still valid. In some ways the > document has progressed since -02, in many it has not, and in a few it has > deteriorated. In my judgement, the document is not ready for last call. Many > fundamentally important questions are still unresolved. Here are my review > comments in rough falling order of importance. > > #1 Improper augment of /rt:routing/rt:control-plane-protocols > > Quoted from section 3.1: > This model augments the core routing data model "ietf-routing" > specified in [RFC8349]. The IGMP model augments "/rt:routing/ > rt:control-plane-protocols" as opposed to augmenting "/rt:routing/ > rt:control-plane-protocols/rt:control-plane-protocol", as the latter > would allow multiple protocol instances, while the IGMP protocol is > designed to be enabled or disabled as a single protocol instance on > a network instance or a logical network element. > > The description above, and the actual augment statements in the YANG module > violate the principles described in RFC 8349, the ietf-routing.yang module it > augments. In RFC 8349, section 5.3. Control-Plane Protocol, the proper way of > augmenting the routing module is described. The fact that this is a singleton > protocol instance doesn't change this. Section 5.3 describes singleton cases as > well. > > #2 Incorrect vendor refinement model > > Quoted from section 2.2: > For the same reason, wide constant ranges (for example, timer > maximum and minimum) will be used in the model. It is expected that > vendors will augment the model with any specific restrictions that > might be required. Vendors may also extend the features list with > proprietary extensions. > > This is not acceptable. The principle suggested does not foster > interoperability and useful standards. It is also not possible to do what the > paragraph suggests in YANG. This was pointed out in the -02 review, and a > suggestion was given there on how to address the problem. > > #3 Top level structures not optional > > Quoted from section 2.3: > The current document contains IGMP and MLD as separate schema > branches in the structure. The reason for this is to make it easier > for implementations which may optionally choose to support specific > address families. And the names of objects may be different between > the IPv4 (IGMP) and IPv6 (MLD) address families. > > This problem was also pointed out in the -02 review. The author suggests that > implementing igmp and/or mld is optional. This is not reflected in the YANG > module, however. As currently modeled, both are currently mandatory to > implement. If-feature is used liberally in the module, and could be used here > as well. > > #4 Unclear meaning of optional leaves > > Quoted from section 3.1: > Where fields are not genuinely essential to protocol operation, they > are marked as optional. Some fields will be essential but have a > default specified, so that they need not be configured explicitly. > > In fact, in the current version of the module, every leaf is optional (except > keys, which cannot be optional). It is good to see the addition of defaults in > many cases, but many unclear cases remain. E.g. leaf /igmp/global/enable is of > type boolean. I understand what true and false implies for this leaf. But what > does it mean if it is not set at all? Either add a default or describe the > meaning in the description. Similarly, if the leaf version is not set on an > igmp or mld interface, or on the interface-global level, what does that mean? > Add default. require-router-alert? explicit-tracking? exclude-lite? Many of > these are used in NP-containers inheriting all the from the root, which makes > the use of mandatory highly discouraged in the current form. If the RFC 8349 > augmentation principles are followed, the concern around mandatory falls, and > some leafs with no sensible default could be marked mandatory instead. > > #5 All optional state > > All state data is optional, which means a conforming server could very well > decide not to implement it. E.g. discontinuity-time is optional. Should a > manager count on this being available? A situation where every leaf is optional > is as nice and flexible for server implementors as it is frustrating and > complicated for manager implementors to consume. A YANG model is an API > contract and should consider the needs of both sides. The way this has been > designed reveals that no representation for the consumer side of this model has > been involved in the design. I would suggest thinking through what is the most > essential state data for a manager, and make some leafs mandatory. > > #6 Abundant copy-paste > > There is abundant repetition in the YANG module. leaf version is defined 2 > times for igmp with identical definitions, and two more for mld with identical > definitions. leaf enable is defined once for the interface global-level, and > with identical definition on the interface local level. leaf > last-member-query-interval, query-interval and half a dozen other leaves are > defined twice with identical definitions. > > #7 Leaf interface in the rpc clear*groups on line 1124, 1094 has type string. > Should be a leafref? Describe what values are valid. #8 Leaf group-policy, > source-policy on line 486, 527, 624, 689: type string. Should be leafref? > Describe what values are valid. #9 Leaf group on line 705, 1101, 1131: Is any > ipv4/6 address ok, or only a multicast address? Model accordingly. > >
- [pim] Yangdoctors last call review of draft-ietf-… Jan Lindblad
- Re: [pim] Yangdoctors last call review of draft-i… Guofeng
- Re: [pim] [yang-doctors] Yangdoctors last call re… Reshad Rahman (rrahman)
- Re: [pim] Yangdoctors last call review of draft-i… Xufeng Liu
- Re: [pim] Yangdoctors last call review of draft-i… Jan Lindblad
- Re: [pim] [yang-doctors] Yangdoctors last call re… Martin Bjorklund
- Re: [pim] Yangdoctors last call review of draft-i… Guofeng
- Re: [pim] Yangdoctors last call review of draft-i… Jan Lindblad
- Re: [pim] [yang-doctors] Yangdoctors last call re… Reshad Rahman (rrahman)
- Re: [pim] Yangdoctors last call review of draft-i… Xufeng Liu
- Re: [pim] [yang-doctors] Yangdoctors last call re… Xufeng Liu