Hi Jan,
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.
Thanks,
- Xufeng
================================
原始邮件
发件人：XufengLiu <Xufeng_Liu@xxxxxxxxx>
收件人：Acee Lindem (acee) <acee@xxxxxxxxx>Christian Hopps <chopps@xxxxxxxxxx>Martin Bjorklund <mbj@xxxxxxxxxx>
抄送人：张征00007940;yang-doctors@xxxxxxxx <yang-doctors@xxxxxxxx>
日 期 ：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@xxxxxxxxx]
> Sent: Monday, February 19, 2018 1:35 PM
> To: Christian Hopps <chopps@xxxxxxxxxx>; Martin Bjorklund <mbj@xxxxxxxxxx>
> Cc: Xufeng Liu <Xufeng_Liu@xxxxxxxxx>; zhang.zheng@xxxxxxxxxx; yang-
> doctors@xxxxxxxx
> Subject: Re: [yang-doctors] How to restrict to have single control-plane-
> protocol instance
>
>
>
> On 2/19/18, 5:02 AM, "Christian Hopps" <chopps@xxxxxxxxxx> wrote:
>
>
> Martin Bjorklund <mbj@xxxxxxxxxx> writes:
>
> > Hi,
> >
> > "Acee Lindem (acee)" <acee@xxxxxxxxx> 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@xxxxxxxxx>
> 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@xxxxxxxxx> 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@xxxxxxxxx]
> >> > Sent: Thursday, February 8, 2018 9:41 AM
> >> > To: Ladislav Lhotka <lhotka@xxxxxx>; Martin Bjorklund <mbj@tail-
> f.com>;
> >> > Acee Lindem (acee) <acee@xxxxxxxxx>
> >> > Cc: yang-doctors@xxxxxxxx; zhang.zheng@xxxxxxxxxx; Xufeng Liu
> >> > <Xufeng_Liu@xxxxxxxxx>
> >> > 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@xxxxxx> wrote:
> >> >
> >> > On Thu, 2018-02-08 at 12:39 +0100, Martin Bjorklund wrote:
> >> > > "Acee Lindem (acee)" <acee@xxxxxxxxx> wrote:
> >> > > > Hi Lada,
> >> > >
> >> > > >
> >> > >
> >> > > > On 2/8/18, 4:42 AM, "yang-doctors on behalf of Ladislav
> Lhotka"
> >> > <yang-docto
> >> > > rs-bounces@xxxxxxxx on behalf of lhotka@xxxxxx> wrote:
> >> > >
> >> > > >
> >> > >
> >> > > > On Thu, 2018-02-08 at 09:20 +0100, Martin Bjorklund wrote:
> >> > >
> >> > > > > Hi,
> >> > >
> >> > > > >
> >> > >
> >> > > > > "Reshad Rahman (rrahman)" <rrahman@xxxxxxxxx> 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@xxxxxxxxx>
> >> > >
> >> > > > > > Date: Monday, February 5, 2018 at 6:25 PM
> >> > >
> >> > > > > > To: Xufeng Liu <Xufeng_Liu@xxxxxxxxx>,
> >> > "zhang.zheng@xxxxxxxxxx"
> >> > >
> >> > > > > > <zhang.zheng@xxxxxxxxxx>
> >> > >
> >> > > > > > Cc: "anish.ietf@xxxxxxxxx" <anish.ietf@xxxxxxxxx>,
> "Mahesh
> >> > Sivakumar
> >> > >
> >> > > > > > (masivaku)" <masivaku@xxxxxxxxx>,
> "guofeng@xxxxxxxxxx"
> >> > >
> >> > > > > > <guofeng@xxxxxxxxxx>,
> "pete.mcallister@xxxxxxxxxxxxxx"
> >> > >
> >> > > > > > <pete.mcallister@xxxxxxxxxxxxxx>,
> "liuyisong@xxxxxxxxxx"
> >> > >
> >> > > > > > <liuyisong@xxxxxxxxxx>, "xu.benchong@xxxxxxxxxx"
> >> > >
> >> > > > > > <xu.benchong@xxxxxxxxxx>, "tanmoy.kundu@alcatel-
> >> > lucent.com"
> >> > >
> >> > > > > > <tanmoy.kundu@xxxxxxxxxxxxxxxxxx>,
> >> > "zzhang_ietf@xxxxxxxxxxx"
> >> > >
> >> > > > > > <zzhang_ietf@xxxxxxxxxxx>, "Acee Lindem (acee)"
> >> > <acee@xxxxxxxxx>
> >> > >
> >> > > > > > 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@xxxxxxxxx>
> >> > >
> >> > > > > > Date: Monday, February 5, 2018 at 9:38 AM
> >> > >
> >> > > > > > To: "zhang.zheng@xxxxxxxxxx"
> <zhang.zheng@xxxxxxxxxx>
> >> > >
> >> > > > > > Cc: "Reshad Rahman (rrahman)" <rrahman@xxxxxxxxx>,
> >> > >
> >> > > > > > "anish.ietf@xxxxxxxxx" <anish.ietf@xxxxxxxxx>,
> "Mahesh
> >> > Sivakumar
> >> > >
> >> > > > > > (masivaku)" <masivaku@xxxxxxxxx>,
> "guofeng@xxxxxxxxxx"
> >> > >
> >> > > > > > <guofeng@xxxxxxxxxx>,
> "pete.mcallister@xxxxxxxxxxxxxx"
> >> > >
> >> > > > > > <pete.mcallister@xxxxxxxxxxxxxx>,
> "liuyisong@xxxxxxxxxx"
> >> > >
> >> > > > > > <liuyisong@xxxxxxxxxx>, "xu.benchong@xxxxxxxxxx"
> >> > >
> >> > > > > > <xu.benchong@xxxxxxxxxx>, "tanmoy.kundu@alcatel-
> >> > lucent.com"
> >> > >
> >> > > > > > <tanmoy.kundu@xxxxxxxxxxxxxxxxxx>,
> >> > "zzhang_ietf@xxxxxxxxxxx"
> >> > >
> >> > > > > > <zzhang_ietf@xxxxxxxxxxx>
> >> > >
> >> > > > > > 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@xxxxxxxxxx
> [mailto:zhang.zheng@xxxxxxxxxx]
> >> > >
> >> > > > > > Sent: Monday, February 5, 2018 3:36 AM
> >> > >
> >> > > > > > To: Xufeng Liu <Xufeng_Liu@xxxxxxxxx>
> >> > >
> >> > > > > > Cc: rrahman@xxxxxxxxx; anish.ietf@xxxxxxxxx;
> >> > masivaku@xxxxxxxxx;
> >> > >
> >> > > > > > guofeng@xxxxxxxxxx;
> pete.mcallister@xxxxxxxxxxxxxx;
> >> > >
> >> > > > > > liuyisong@xxxxxxxxxx; xu.benchong@xxxxxxxxxx;
> >> > >
> >> > > > > > tanmoy.kundu@xxxxxxxxxxxxxxxxxx;
> zzhang_ietf@xxxxxxxxxxx
> >> > >
> >> > > > > > 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@xxxxxxxxx<mailto:Xufeng_Liu@xxxxxxxxx>>;
> >>
发件人：XufengLiu <Xufeng_Liu@xxxxxxxxx>
收件人：Acee Lindem (acee) <acee@xxxxxxxxx>Christian Hopps <chopps@xxxxxxxxxx>Martin Bjorklund <mbj@xxxxxxxxxx>
抄送人：张征00007940;yang-doctors@xxxxxxxx <yang-doctors@xxxxxxxx>
日 期 ：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@xxxxxxxxx]
> Sent: Monday, February 19, 2018 1:35 PM
> To: Christian Hopps <chopps@xxxxxxxxxx>; Martin Bjorklund <mbj@xxxxxxxxxx>
> Cc: Xufeng Liu <Xufeng_Liu@xxxxxxxxx>; zhang.zheng@xxxxxxxxxx; yang-
> doctors@xxxxxxxx
> Subject: Re: [yang-doctors] How to restrict to have single control-plane-
> protocol instance
>
>
>
> On 2/19/18, 5:02 AM, "Christian Hopps" <chopps@xxxxxxxxxx> wrote:
>
>
> Martin Bjorklund <mbj@xxxxxxxxxx> writes:
>
> > Hi,
> >
> > "Acee Lindem (acee)" <acee@xxxxxxxxx> 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@xxxxxxxxx>
> 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@xxxxxxxxx> 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@xxxxxxxxx]
> >> > Sent: Thursday, February 8, 2018 9:41 AM
> >> > To: Ladislav Lhotka <lhotka@xxxxxx>; Martin Bjorklund <mbj@tail-
> f.com>;
> >> > Acee Lindem (acee) <acee@xxxxxxxxx>
> >> > Cc: yang-doctors@xxxxxxxx; zhang.zheng@xxxxxxxxxx; Xufeng Liu
> >> > <Xufeng_Liu@xxxxxxxxx>
> >> > 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@xxxxxx> wrote:
> >> >
> >> > On Thu, 2018-02-08 at 12:39 +0100, Martin Bjorklund wrote:
> >> > > "Acee Lindem (acee)" <acee@xxxxxxxxx> wrote:
> >> > > > Hi Lada,
> >> > >
> >> > > >
> >> > >
> >> > > > On 2/8/18, 4:42 AM, "yang-doctors on behalf of Ladislav
> Lhotka"
> >> > <yang-docto
> >> > > rs-bounces@xxxxxxxx on behalf of lhotka@xxxxxx> wrote:
> >> > >
> >> > > >
> >> > >
> >> > > > On Thu, 2018-02-08 at 09:20 +0100, Martin Bjorklund wrote:
> >> > >
> >> > > > > Hi,
> >> > >
> >> > > > >
> >> > >
> >> > > > > "Reshad Rahman (rrahman)" <rrahman@xxxxxxxxx> 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@xxxxxxxxx>
> >> > >
> >> > > > > > Date: Monday, February 5, 2018 at 6:25 PM
> >> > >
> >> > > > > > To: Xufeng Liu <Xufeng_Liu@xxxxxxxxx>,
> >> > "zhang.zheng@xxxxxxxxxx"
> >> > >
> >> > > > > > <zhang.zheng@xxxxxxxxxx>
> >> > >
> >> > > > > > Cc: "anish.ietf@xxxxxxxxx" <anish.ietf@xxxxxxxxx>,
> "Mahesh
> >> > Sivakumar
> >> > >
> >> > > > > > (masivaku)" <masivaku@xxxxxxxxx>,
> "guofeng@xxxxxxxxxx"
> >> > >
> >> > > > > > <guofeng@xxxxxxxxxx>,
> "pete.mcallister@xxxxxxxxxxxxxx"
> >> > >
> >> > > > > > <pete.mcallister@xxxxxxxxxxxxxx>,
> "liuyisong@xxxxxxxxxx"
> >> > >
> >> > > > > > <liuyisong@xxxxxxxxxx>, "xu.benchong@xxxxxxxxxx"
> >> > >
> >> > > > > > <xu.benchong@xxxxxxxxxx>, "tanmoy.kundu@alcatel-
> >> > lucent.com"
> >> > >
> >> > > > > > <tanmoy.kundu@xxxxxxxxxxxxxxxxxx>,
> >> > "zzhang_ietf@xxxxxxxxxxx"
> >> > >
> >> > > > > > <zzhang_ietf@xxxxxxxxxxx>, "Acee Lindem (acee)"
> >> > <acee@xxxxxxxxx>
> >> > >
> >> > > > > > 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@xxxxxxxxx>
> >> > >
> >> > > > > > Date: Monday, February 5, 2018 at 9:38 AM
> >> > >
> >> > > > > > To: "zhang.zheng@xxxxxxxxxx"
> <zhang.zheng@xxxxxxxxxx>
> >> > >
> >> > > > > > Cc: "Reshad Rahman (rrahman)" <rrahman@xxxxxxxxx>,
> >> > >
> >> > > > > > "anish.ietf@xxxxxxxxx" <anish.ietf@xxxxxxxxx>,
> "Mahesh
> >> > Sivakumar
> >> > >
> >> > > > > > (masivaku)" <masivaku@xxxxxxxxx>,
> "guofeng@xxxxxxxxxx"
> >> > >
> >> > > > > > <guofeng@xxxxxxxxxx>,
> "pete.mcallister@xxxxxxxxxxxxxx"
> >> > >
> >> > > > > > <pete.mcallister@xxxxxxxxxxxxxx>,
> "liuyisong@xxxxxxxxxx"
> >> > >
> >> > > > > > <liuyisong@xxxxxxxxxx>, "xu.benchong@xxxxxxxxxx"
> >> > >
> >> > > > > > <xu.benchong@xxxxxxxxxx>, "tanmoy.kundu@alcatel-
> >> > lucent.com"
> >> > >
> >> > > > > > <tanmoy.kundu@xxxxxxxxxxxxxxxxxx>,
> >> > "zzhang_ietf@xxxxxxxxxxx"
> >> > >
> >> > > > > > <zzhang_ietf@xxxxxxxxxxx>
> >> > >
> >> > > > > > 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@xxxxxxxxxx
> [mailto:zhang.zheng@xxxxxxxxxx]
> >> > >
> >> > > > > > Sent: Monday, February 5, 2018 3:36 AM
> >> > >
> >> > > > > > To: Xufeng Liu <Xufeng_Liu@xxxxxxxxx>
> >> > >
> >> > > > > > Cc: rrahman@xxxxxxxxx; anish.ietf@xxxxxxxxx;
> >> > masivaku@xxxxxxxxx;
> >> > >
> >> > > > > > guofeng@xxxxxxxxxx;
> pete.mcallister@xxxxxxxxxxxxxx;
> >> > >
> >> > > > > > liuyisong@xxxxxxxxxx; xu.benchong@xxxxxxxxxx;
> >> > >
> >> > > > > > tanmoy.kundu@xxxxxxxxxxxxxxxxxx;
> zzhang_ietf@xxxxxxxxxxx
> >> > >
> >> > > > > > 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@xxxxxxxxx<mailto:Xufeng_Liu@xxxxxxxxx>>;
> >>
================================
On Mon, Aug 13, 2018 at 10:34 AM Jan Lindblad <janl@xxxxxxxxxx> 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.
