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

["Reshad Rahman (rrahman)" <[e-mail]>]

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" <[e-mail]> wrote:

    On Thu, 2018-02-08 at 12:39 +0100, Martin Bjorklund wrote:
    > "Acee Lindem (acee)" <[e-mail]> wrote:
    > > Hi Lada,
    > 
    > > 
    > 
    > > On 2/8/18, 4:42 AM, "yang-doctors on behalf of Ladislav Lhotka" <yang-docto
    > [e-mail] on behalf of [e-mail]> wrote:
    > 
    > > 
    > 
    > >     On Thu, 2018-02-08 at 09:20 +0100, Martin Bjorklund wrote:
    > 
    > >     > Hi,
    > 
    > >     > 
    > 
    > >     > "Reshad Rahman (rrahman)" <[e-mail]> 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)" <[e-mail]>
    > 
    > >     > > Date: Monday, February 5, 2018 at 6:25 PM
    > 
    > >     > > To: Xufeng Liu <[e-mail]>, "[e-mail]"
    > 
    > >     > > <[e-mail]>
    > 
    > >     > > Cc: "[e-mail]" <[e-mail]>, "Mahesh Sivakumar
    > 
    > >     > > (masivaku)" <[e-mail]>, "[e-mail]"
    > 
    > >     > > <[e-mail]>, "[e-mail]"
    > 
    > >     > > <[e-mail]>, "[e-mail]"
    > 
    > >     > > <[e-mail]>, "[e-mail]"
    > 
    > >     > > <[e-mail]>, "[e-mail]"
    > 
    > >     > > <[e-mail]>, "[e-mail]"
    > 
    > >     > > <[e-mail]>, "Acee Lindem (acee)" <[e-mail]>
    > 
    > >     > > 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 <[e-mail]>
    > 
    > >     > > Date: Monday, February 5, 2018 at 9:38 AM
    > 
    > >     > > To: "[e-mail]" <[e-mail]>
    > 
    > >     > > Cc: "Reshad Rahman (rrahman)" <[e-mail]>,
    > 
    > >     > > "[e-mail]" <[e-mail]>, "Mahesh Sivakumar
    > 
    > >     > > (masivaku)" <[e-mail]>, "[e-mail]"
    > 
    > >     > > <[e-mail]>, "[e-mail]"
    > 
    > >     > > <[e-mail]>, "[e-mail]"
    > 
    > >     > > <[e-mail]>, "[e-mail]"
    > 
    > >     > > <[e-mail]>, "[e-mail]"
    > 
    > >     > > <[e-mail]>, "[e-mail]"
    > 
    > >     > > <[e-mail]>
    > 
    > >     > > 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: [e-mail] [mailto:[e-mail]]
    > 
    > >     > > Sent: Monday, February 5, 2018 3:36 AM
    > 
    > >     > > To: Xufeng Liu <[e-mail]>
    > 
    > >     > > Cc: [e-mail]; [e-mail]; [e-mail];
    > 
    > >     > > [e-mail]; [e-mail];
    > 
    > >     > > [e-mail]; [e-mail];
    > 
    > >     > > [e-mail]; [e-mail]
    > 
    > >     > > 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
    > 
    > >     > > 原始邮件
    > 
    > >     > > 发件人： <[e-mail]<mailto:[e-mail]>>;
    > 
    > >     > > 收件人： <[e-mail]<mailto:[e-mail]>>;张征00007940;
    > 
    > >     > > <[e-mail]<mailto:[e-mail]>>;
    > 
    > >     > > <[e-mail]<mailto:[e-mail]>>;
    > 
    > >     > > <[e-mail]<mailto:[e-mail]>>;
    > 
    > >     > > <[e-mail]<mailto:[e-mail]
    > m>>;
    > 
    > >     > > <[e-mail]<mailto:[e-mail]>>;徐本崇10065053;
    > 
    > >     > > <[e-mail]<mailto:tanmoy.kundu@alcatel-lucent.
    > com>>;
    > 
    > >     > > <[e-mail]<mailto:[e-mail]>>;
    > 
    > >     > > 日 期 ：2018年02月03日 01:21
    > 
    > >     > > 主 题 ：RE: Hi all, about the modification of MSDP YANG
    > 
    > >     > > Hi Sandy and Reshad,
    > 
    > >     > > 
    > 
    > >     > > The reason that we used to augment
    > 
    > >     > > /rt:routing/rt:control-plane-protocols, instead of
    > 
    > >     > > /rt:routing/rt:control-plane-protocols/rt:control-plane-protocol, is
    > 
    > >     > > that we do not allow multiple instances of MSDP.
    > 
    > >     > > 
    > 
    > >     > > Thanks,
    > 
    > >     > > - Xufeng
    > 
    > >     > > 
    > 
    > >     > > From: Reshad Rahman (rrahman) [mailto:[e-mail]]
    > 
    > >     > > Sent: Friday, February 2, 2018 12:08 PM
    > 
    > >     > > To: [e-mail]<mailto:[e-mail]>; Xufeng
    > Liu
    > 
    > >     > > <[e-mail]<mailto:[e-mail]>>;
    > 
    > >     > > [e-mail]<mailto:[e-mail]>; Mahesh Sivakumar
    > 
    > >     > > (masivaku) <[e-mail]<mailto:[e-mail]>>;
    > 
    > >     > > [e-mail]<mailto:[e-mail]>;
    > 
    > >     > > [e-mail]<mailto:[e-mail]
    > >;
    > 
    > >     > > [e-mail]<mailto:[e-mail]>;
    > 
    > >     > > [e-mail]<mailto:[e-mail]>;
    > 
    > >     > > [e-mail]<mailto:tanmoy.kundu@alcatel-lucent.c
    > om>;
    > 
    > >     > > [e-mail]<mailto:[e-mail]>
    > 
    > >     > > Subject: Re: Hi all, about the modification of MSDP YANG
    > 
    > >     > > 
    > 
    > >     > > Hi Sandy,
    > 
    > >     > > 
    > 
    > >     > > I don’t know what warning you are getting now but from a quick look
    > at
    > 
    > >     > > the revision you sent I see couple of issues.
    > 
    > >     > > 
    > 
    > >     > >      identity msdp {
    > 
    > >     > >        base "rt:routing-protocol";  <== should be rt:control-plane-
    > protocol
    > 
    > >     > >        description "MSDP";
    > 
    > >     > >      }
    > 
    > >     > > <snip>
    > 
    > >     > >      /*
    > 
    > >     > >       * Data nodes
    > 
    > >     > >       */
    > 
    > >     > >      augment
    > 
    > >     > >      "/rt:routing/rt:control-plane-protocols/rt:control-plane-
    > protocol" {
    > 
    > >     > >         when "rt:type = 'MSDP'" { <== should be "rt:type =
    > 'msdp:msdp'"
    > 
    > >     > > 
    > 
    > >     > > 
    > 
    > >     > > HTH,
    > 
    > >     > > Reshad.
    > 
    > >     > > 
    > 
    > >     > > From: "[e-mail]<mailto:[e-mail]>"
    > 
    > >     > > <[e-mail]<mailto:[e-mail]>>
    > 
    > >     > > Date: Friday, February 2, 2018 at 4:37 AM
    > 
    > >     > > To: "[e-mail]<mailto:[e-mail]>"
    > 
    > >     > > <[e-mail]<mailto:[e-mail]>>,
    > 
    > >     > > "[e-mail]<mailto:[e-mail]>"
    > 
    > >     > > <[e-mail]<mailto:[e-mail]>>, "Mahesh
    > Sivakumar
    > 
    > >     > > (masivaku)" <[e-mail]<mailto:[e-mail]>>,
    > 
    > >     > > "[e-mail]<mailto:[e-mail]>"
    > 
    > >     > > <[e-mail]<mailto:[e-mail]>>,
    > 
    > >     > > "[e-mail]<mailto:[e-mail]
    > m>"
    > 
    > >     > > <[e-mail]<mailto:[e-mail]
    > m>>,
    > 
    > >     > > "[e-mail]<mailto:[e-mail]>"
    > 
    > >     > > <[e-mail]<mailto:[e-mail]>>,
    > 
    > >     > > "[e-mail]<mailto:[e-mail]>"
    > 
    > >     > > <[e-mail]<mailto:[e-mail]>>,
    > 
    > >     > > "[e-mail]<mailto:tanmoy.kundu@alcatel-lucent.
    > com>"
    > 
    > >     > > <[e-mail]<mailto:tanmoy.kundu@alcatel-lucent.
    > com>>,
    > 
    > >     > > "[e-mail]<mailto:[e-mail]>"
    > 
    > >     > > <[e-mail]<mailto:[e-mail]>>
    > 
    > >     > > Cc: "Reshad Rahman (rrahman)"
    > 
    > >     > > <[e-mail]<mailto:[e-mail]>>
    > 
    > >     > > Subject: FW: Hi all, about the modification of MSDP YANG
    > 
    > >     > > 
    > 
    > >     > > 
    > 
    > >     > > Hi all,
    > 
    > >     > > 
    > 
    > >     > > 
    > 
    > >     > > 
    > 
    > >     > > I deleted some groupings and make the model more clear.
    > 
    > >     > > 
    > 
    > >     > > And I updated the decription of (peer-as, up-time, expire).  Please
    > 
    > >     > > review it.
    > 
    > >     > > 
    > 
    > >     > > 
    > 
    > >     > > 
    > 
    > >     > > A warning is still existing about rt:type:
    > 
    > >     > > 
    > 
    > >     > > 5, - augment of control-plane-protocols is incorrect. There should
    > be
    > 
    > >     > > an identity msdp with
    > 
    > >     > > 
    > 
    > >     > > base "rt:routing-protocol" and then augment
    > 
    > >     > > 
    > 
    > >     > > "/rt:routing/rt:control-plane-protocols/rt:control-plane-protocol"
    > 
    > >     > > with a when
    > 
    > >     > > 
    > 
    > >     > > statement. Take a look at OSPF YANG for an example.
    > 
    > >     > > 
    > 
    > >     > > [Sandy]: Added the identity and modify the augmentation, but it
    > seems
    > 
    > >     > > like there is no MSDP register in rt:type.
    > 
    > >     > > 
    > 
    > >     > > How can we register it?
    > 
    > >     > > 
    > 
    > >     > > 
    > 
    > >     > > 
    > 
    > >     > > Thanks,
    > 
    > >     > > 
    > 
    > >     > > Sandy
    > 
    > >     > > 原始邮件
    > 
    > >     > > 发件人：张征00007940
    > 
    > >     > > 收件人： <[e-mail]<mailto:[e-mail]>>;
    > 
    > >     > > <[e-mail]<mailto:[e-mail]>>;
    > 
    > >     > > <[e-mail]<mailto:[e-mail]>>;
    > 
    > >     > > <[e-mail]<mailto:[e-mail]>>;
    > 
    > >     > > <[e-mail]<mailto:[e-mail]
    > m>>;
    > 
    > >     > > <[e-mail]<mailto:[e-mail]>>;徐本崇10065053;
    > 
    > >     > > <[e-mail]<mailto:tanmoy.kundu@alcatel-lucent.
    > com>>;
    > 
    > >     > > <[e-mail]<mailto:[e-mail]>>;
    > 
    > >     > > 抄送人： <[e-mail]<mailto:[e-mail]>>;
    > 
    > >     > > 日 期 ：2018年01月29日  17:04
    > 
    > >     > > 主 题 ：Hi all, about the modification of MSDP YANG
    > 
    > >     > > 
    > 
    > >     > > Hi all,
    > 
    > >     > > 
    > 
    > >     > > 
    > 
    > >     > > 
    > 
    > >     > > YANG doctor Reshad had finished the early review about MSDP YANG.
    > 
    > >     > > 
    > 
    > >     > > I finished the preliminary modification version, please review it.
    > 
    > >     > > 
    > 
    > >     > > I think some advices from Reshad should be discussed:
    > 
    > >     > > 
    > 
    > >     > > 
    > 
    > >     > > 
    > 
    > >     > > 1, - Not sure why peer-as is needed. Don't see it in RFC3618.
    > 
    > >     > > 
    > 
    > >     > > 2, - leaf up-time, what's meant by "up time" in the description? Is
    > it
    > 
    > >     > > time it's
    > 
    > >     > > 
    > 
    > >     > > been created?
    > 
    > >     > > 
    > 
    > >     > > 3, - description for leaf expire seems wrong.
    > 
    > >     > > 
    > 
    > >     > > [Sandy]: These items (peer-as, up-time, expire) doesn't existed in
    > 
    > >     > > RFC3618, are these unnecessary? Please write down your
    > 
    > >     > > 
    > 
    > >     > > description if you insist on it. If nobody insist on it, should we
    > 
    > >     > > delete them?
    > 
    > >     > > 
    > 
    > >     > > 
    > 
    > >     > > 
    > 
    > >     > > 4, - Groupings are used for data which is used only once. Is this
    > done
    > 
    > >     > > on purpose or
    > 
    > >     > > 
    > 
    > >     > > was the intention to use those groupings more than once?
    > 
    > >     > > 
    > 
    > >     > > [Sandy]: These eight groupings are used only once, should we change
    > 
    > >     > > them to container?
    > 
    > >     > > 
    > 
    > >     > > authentication-container;
    > 
    > >     > > 
    > 
    > >     > > global-config-attributes;
    > 
    > >     > > 
    > 
    > >     > > peer-config-attributes;
    > 
    > >     > > 
    > 
    > >     > > peer-state-attributes;
    > 
    > >     > > 
    > 
    > >     > > sa-cache-state-attributes;
    > 
    > >     > > 
    > 
    > >     > > statistics-container
    > 
    > >     > > 
    > 
    > >     > > statistics-error
    > 
    > >     > > 
    > 
    > >     > > statistics-queue
    > 
    > >     > > 
    > 
    > >     > > 
    > 
    > >     > > 
    > 
    > >     > > 5, - augment of control-plane-protocols is incorrect. There should
    > be
    > 
    > >     > > an identity msdp with
    > 
    > >     > > 
    > 
    > >     > > base "rt:routing-protocol" and then augment
    > 
    > >     > > 
    > 
    > >     > > "/rt:routing/rt:control-plane-protocols/rt:control-plane-protocol"
    > 
    > >     > > with a when
    > 
    > >     > > 
    > 
    > >     > > statement. Take a look at OSPF YANG for an example.
    > 
    > >     > > 
    > 
    > >     > > [Sandy]: Added the identity and modify the augmentation, but it
    > seems
    > 
    > >     > > like there is no MSDP register in rt:type.
    > 
    > >     > > 
    > 
    > >     > > How can we register it?
    > 
    > >     > > 
    > 
    > >     > > 
    > 
    > >     > > 
    > 
    > >     > > 
    > 
    > >     > > 
    > 
    > >     > > Most of the suggestion is adopted. The modification detail pls see
    > 
    > >     > > below:
    > 
    > >     > > 
    > 
    > >     > > 
    > 
    > >     > > 
    > 
    > >     > > - Too many features (17)! Every piece of config has an if-feature
    > 
    > >     > > - statement.
    > 
    > >     > > 
    > 
    > >     > > Some of the configs (timers?) should be part of most/basic
    > 
    > >     > > implementations, for
    > 
    > >     > > 
    > 
    > >     > > other config (e.g. authentication) I can see why a feature would be
    > 
    > >     > > used.
    > 
    > >     > > 
    > 
    > >     > > [Sandy]: Modified the three timers (connect-retry, hold, keepalive)
    > to
    > 
    > >     > > fixed format.
    > 
    > >     > > 
    > 
    > >     > > 
    > 
    > >     > > 
    > 
    > >     > > -“import ietf-yang-types” should have a reference to RFC6991 (see
    > 
    > >     > > -section 4.7 of
    > 
    > >     > > 
    > 
    > >     > > rfc6087bis-15)
    > 
    > >     > > 
    > 
    > >     > > - “import ietf-inet-types” should have a reference to RFC6991
    > 
    > >     > > 
    > 
    > >     > > - “import ietf-routing” should have a reference to RFC8022
    > 
    > >     > > 
    > 
    > >     > > - “import ietf-interfaces” should have a reference to RFC7223
    > 
    > >     > > 
    > 
    > >     > > - "import ietf-ip" should have a reference to RFC7277
    > 
    > >     > > 
    > 
    > >     > > - "import ietf-key-chain" should have a reference to RFC8177
    > 
    > >     > > 
    > 
    > >     > > [Sandy]: Added all the references above.
    > 
    > >     > > 
    > 
    > >     > > 
    > 
    > >     > > 
    > 
    > >     > > - organization s/"...PIM( Protocols for IP Multicast ) Working
    > 
    > >     > > 
    > 
    > >     > > Group"/"...PIM (Protocols for IP Multicast) Working Group"?
    > 
    > >     > > 
    > 
    > >     > > - Remove WG Chairs from contact information as per Appendix C of
    > 
    > >     > > - rfc6087bis-15
    > 
    > >     > > 
    > 
    > >     > > - No copyright in the module description, see Appendix of 6087bis-15 
    > for
    > 
    > >     > > - a module description
    > 
    > >     > > 
    > 
    > >     > > example
    > 
    > >     > > 
    > 
    > >     > > - Module description must contain reference to RFC, see Appendix C
    > of
    > 
    > >     > > 
    > 
    > >     > > rfc6087bis-15
    > 
    > >     > > 
    > 
    > >     > > [Sandy]: Removed WG chairs and add copyright from Appendix of
    > 
    > >     > > rfc6087bis. Added reference to RFC3618.
    > 
    > >     > > 
    > 
    > >     > > 
    > 
    > >     > > 
    > 
    > >     > > - grouping authentication-container. key-chain and password both
    > 
    > >     > > 
    > 
    > >     > > use if-feature peer-key-chain.
    > 
    > >     > > 
    > 
    > >     > > [Sandy]: Removed the if-feature peer-key-chain from password.
    > 
    > >     > > 
    > 
    > >     > > 
    > 
    > >     > > 
    > 
    > >     > > - grouping connect-source. The name is not very
    > 
    > >     > > 
    > 
    > >     > > descriptive. Should this be something along the lines of
    > 
    > >     > > tcp-connection-source?
    > 
    > >     > > 
    > 
    > >     > > [Sandy]: Changed the name "connect-source" to "tcp-connection-
    > source".
    > 
    > >     > > 
    > 
    > >     > > 
    > 
    > >     > > 
    > 
    > >     > > - grouping global-state-attributes has nothing
    > 
    > >     > > 
    > 
    > >     > > [Sandy]: Deleted the grouping.
    > 
    > >     > > 
    > 
    > >     > > 
    > 
    > >     > > 
    > 
    > >     > > - Some of the descriptions are
    > 
    > >     > > 
    > 
    > >     > > pretty terse. e.g. for rpf-peer it says "RPF peer.". In a case like
    > 
    > >     > > this
    > 
    > >     > > 
    > 
    > >     > > consider adding more descriptive text or a reference to the proper
    > 
    > >     > > section in
    > 
    > >     > > 
    > 
    > >     > > RFC3618
    > 
    > >     > > 
    > 
    > >     > > [Sandy]: Added more description.
    > 
    > >     > > 
    > 
    > >     > > 
    > 
    > >     > > 
    > 
    > >     > > - peer-as (Autonomous System Number) is defined as type string,
    > should
    > 
    > >     > > 
    > 
    > >     > > be of type as-number in ietf-inet-types?
    > 
    > >     > > 
    > 
    > >     > > [Sandy]: Modified to inet types.
    > 
    > >     > > 
    > 
    > >     > > 
    > 
    > >     > > 
    > 
    > >     > > - keepalive-interval depends on holdtime-interval.
    > 
    > >     > > 
    > 
    > >     > > There should be "if-feature peer-timer-holdtime" under leaf
    > 
    > >     > > keepalive-interval
    > 
    > >     > > 
    > 
    > >     > > or change the must statement to (assuming we keep the 2 features):
    > 
    > >     > > 
    > 
    > >     > >   must "(not ../holdtime-interval) or (. > 1 and . <
    > 
    > >     > >   ../holdtime-interval)".
    > 
    > >     > > 
    > 
    > >     > > [Sandy]: Modified the features to fixed format.
    > 
    > >     > > 
    > 
    > >     > > 
    > 
    > >     > > 
    > 
    > >     > > - leaf up-time: s/sa cache/SA cache/
    > 
    > >     > > 
    > 
    > >     > > - leaf peer-learned-from, change description from "The address of
    > peer
    > 
    > >     > > - that we learned
    > 
    > >     > > 
    > 
    > >     > > this SA from ." to "The address of the peer that we learned this SA
    > 
    > >     > > from."
    > 
    > >     > > 
    > 
    > >     > > [Sandy]: Modified.
    > 
    > >     > > 
    > 
    > >     > > 
    > 
    > >     > > 
    > 
    > >     > > - RPC leaf group, I thought we had a type for IP multicast address?
    > If
    > 
    > >     > > - not, it should be done?
    > 
    > >     > > 
    > 
    > >     > > [Sandy]: Yes. Added the rt-type reference to RFC8294.
    > 
    > >     > > 
    > 
    > >     > > 
    > 
    > >     > > 
    > 
    > >     > > - s/msdp/MSDP/
    > 
    > >     > > 
    > 
    > >     > > - In rpc msdp-clear-peer, s/Clears the session to the peer./Clears
    > 
    > >     > > 
    > 
    > >     > > the TCP connection to the peer./
    > 
    > >     > > 
    > 
    > >     > > - In rpc msdp-clear-sa-cache, why have the enum '*' for
    > 
    > >     > > - source-addr. Can't the same technique as for peer-address be
    > 
    > >     > > 
    > 
    > >     > > 
    > 
    > >     > > 
    > 
    > >     > > used?
    > 
    > >     > > 
    > 
    > >     > > - msdp prefix not needed in rpc names
    > 
    > >     > > 
    > 
    > >     > > [Sandy]: Done.
    > 
    > >     > > 
    > 
    > >     > > 
    > 
    > >     > > 
    > 
    > >     > > - MSDP peers are configured in a mesh-group, did the authors
    > consider
    > 
    > >     > > - adding state per mesh-group, e.g. all the
    > 
    > >     > > 
    > 
    > >     > > peers in a particular mesh-group?
    > 
    > >     > > 
    > 
    > >     > > [Sandy]: IMO it is unnecessary because the states of peers is not
    > 
    > >     > > unified in a mesh-group.
    > 
    > >     > > 
    > 
    > >     > > 
    > 
    > >     > > 
    > 
    > >     > > General:
    > 
    > >     > > 
    > 
    > >     > > - Per Appendix B of rfc6087bis-15: "that all YANG modules containing
    > 
    > >     > > 
    > 
    > >     > > imported items are cited as normative reference". So RFCs 6991,
    > 7223,
    > 
    > >     > > 
    > 
    > >     > > 7277, 8022 and 8177 should be included in the normative reference
    > 
    > >     > > 
    > 
    > >     > > section.
    > 
    > >     > > 
    > 
    > >     > > [Sandy]: Added.
    > 
    > >     > > 
    > 
    > >     > > 
    > 
    > >     > > 
    > 
    > >     > > - Section 3 "the irrelevant information", add a
    > reference/explanation
    > 
    > >     > > - for what
    > 
    > >     > > 
    > 
    > >     > > the irrelevant information is. s/the irrelevant
    > information/irrelevant
    > 
    > >     > > 
    > 
    > >     > > information/?
    > 
    > >     > > 
    > 
    > >     > > [Sandy]: Changed the description.
    > 
    > >     > > 
    > 
    > >     > > 
    > 
    > >     > > 
    > 
    > >     > > - Section 5 should give a brief description of what the RPCs do.
    > 
    > >     > > 
    > 
    > >     > > [Sandy]: Added some description.
    > 
    > >     > > 
    > 
    > >     > > 
    > 
    > >     > > 
    > 
    > >     > > - Section 6 any plans for notifications? If not, just say so.
    > 
    > >     > > 
    > 
    > >     > > [Sandy]: Done.
    > 
    > >     > > 
    > 
    > >     > > 
    > 
    > >     > > 
    > 
    > >     > > - Need Security
    > 
    > >     > > 
    > 
    > >     > > Considerations, see sections 3.7 and 6 of rfc6087bis-15
    > 
    > >     > > 
    > 
    > >     > > [Sandy]: Added security consideration section.
    > 
    > >     > > 
    > 
    > >     > > 
    > 
    > >     > > 
    > 
    > >     > > - Need IANA Considerations, see section 3.8 of rfc6087bis-15
    > 
    > >     > > 
    > 
    > >     > > [Sandy]: Added IANA considerations.
    > 
    > >     > > 
    > 
    > >     > > 
    > 
    > >     > > 
    > 
    > >     > > - Need license in YANG module,
    > 
    > >     > > 
    > 
    > >     > > see appendix B of rfc6087bis-15
    > 
    > >     > > 
    > 
    > >     > > [Sandy]: Added the YANG module description.
    > 
    > >     > > 
    > 
    > >     > > 
    > 
    > >     > > 
    > 
    > >     > > Thanks,
    > 
    > >     > > 
    > 
    > >     > > Sandy
    > 
    > >     > > 
    > 
    > >     > > 
    > 
    > >     > > 
    > 
    > >     > > 
    > 
    > >     > > 
    > 
    > >     > > 
    > 
    > >     > 
    > 
    > >     > _______________________________________________
    > 
    > >     > yang-doctors mailing list
    > 
    > >     > [e-mail]
    > 
    > >     > https://www.ietf.org/mailman/listinfo/yang-doctors
    > 
    > >     -- 
    > 
    > >     Ladislav Lhotka
    > 
    > >     Head, CZ.NIC Labs
    > 
    > >     PGP Key ID: 0xB8F92B08A9F76C67
    > 
    > >     
    > 
    > >     _______________________________________________
    > 
    > >     yang-doctors mailing list
    > 
    > >     [e-mail]
    > 
    > >     https://www.ietf.org/mailman/listinfo/yang-doctors
    > 
    > >     
    > 
    > > 
    > 
    -- 
    Ladislav Lhotka
    Head, CZ.NIC Labs
    PGP Key ID: 0xB8F92B08A9F76C67