+1
On 2018-09-11, 10:11 AM, "yang-doctors on behalf of Martin Bjorklund" <yang-doctors-bounces@xxxxxxxx on behalf of mbj@xxxxxxxxxx> wrote:
Hi,
Xufeng Liu <xufeng.liu.ietf@xxxxxxxxx> wrote:
> 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.
But there was no clear outcome of that discussion. In fact, it seems
that most people in that thread argued to NOT do a special augment,
but instead augement the "control-plane-protocol" list, and state that
there can only be a single entry of this type.
/martin
>
> 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>>;
> > >>
> ================================
>
> 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.
> >
> >
> >
_______________________________________________
yang-doctors mailing list
yang-doctors@xxxxxxxx
https://www.ietf.org/mailman/listinfo/yang-doctors
