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

[Martin Bjorklund <[e-mail]>]

"Acee Lindem (acee)" <[e-mail]> wrote:
> Hi Lada,
> 
> On 2/8/18, 4:42 AM, "yang-doctors on behalf of Ladislav Lhotka" <[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.


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]>>;
>     > > <[e-mail]<mailto:[e-mail]>>;徐本崇10065053;
>     > > <[e-mail]<mailto:[e-mail]>>;
>     > > <[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:[e-mail]>;
>     > > [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]>"
>     > > <[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:[e-mail]>"
>     > > <[e-mail]<mailto:[e-mail]>>,
>     > > "[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]>>;
>     > > <[e-mail]<mailto:[e-mail]>>;徐本崇10065053;
>     > > <[e-mail]<mailto:[e-mail]>>;
>     > > <[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
>     
>