Subject: [yang-doctors] to augment or to clone?

From: Andy Bierman <[email-masked]>
Date: Thu, 4 Oct 2018 12:15:02 -0700

Hi,

I am reviewing the ietf-ntp module.

(BTW, rfcstrip does not extract this module correctly.
Not sure I have the latest version, but the last 2 closing right braces get
left out)

This module replicates the /interfaces/interface subtree under the /ntp
container.

IMO this is correct YANG but not good for overall management.

The best current practice seems to be that the interface list should
be augmented (in this case with a presence container 'ntp').

Each protocol could have a top-level container and interface-level
containers.
Not just 1 top-level container that copies other tables inside it.

Is there any YD consensus on this issue?

Andy

---

From: Ladislav Lhotka <[email-masked]>
Date: Fri, 05 Oct 2018 10:32:05 +0200

> IMO this is correct YANG but not good for overall management.

Agreed.

> be augmented (in this case with a presence container 'ntp').

I think routing protocol models such as draft-ietf-isis-yang-isis-cfg
provide a good example to follow: they both augment the interface list
and then also refer to interfaces from other places via
if:interface-ref (i.e. leafref).

Lada

> _______________________________________________
> yang-doctors mailing list
> [email-masked]
> https://www.ietf.org/mailman/listinfo/yang-doctors

-- 
Ladislav Lhotka
Head, CZ.NIC Labs
PGP Key ID: 0xB8F92B08A9F76C67

---

From: Juergen Schoenwaelder <[email-masked]>
Date: Fri, 5 Oct 2018 11:45:58 +0200

> if:interface-ref (i.e. leafref).

How does this line up with this:

https://www.ietf.org/mail-archive/web/yang-doctors/current/msg01379.html

/js

-- 
Juergen Schoenwaelder           Jacobs University Bremen gGmbH
Phone: +49 421 200 3587         Campus Ring 1 | 28759 Bremen | Germany
Fax:   +49 421 200 3103         <https://www.jacobs-university.de/>

---

From: Ladislav Lhotka <[email-masked]>
Date: Fri, 05 Oct 2018 12:47:10 +0200

> https://www.ietf.org/mail-archive/web/yang-doctors/current/msg01379.html

I thought that Andy meant that the entire /interfaces/interface subtree is
replicated elsewhere, but it is not the case.

The protocol interfaces are another layer, so it is indeed cleaner to keep them
and their protocol-related parameters under the protocol.

Additional parameters for an L3 interface defined under /if:interfaces (clns-mtu 
in ietf-isis) are added via augment.

Lada

---

From: "Acee Lindem (acee)" <[email-masked]>
Date: Fri, 5 Oct 2018 15:17:24 +0000

Hi Lada, Andy, 

On 10/5/18, 4:32 AM, "yang-doctors on behalf of Ladislav Lhotka" <[email-masked] on behalf of [email-masked]> wrote:

    
    Agreed.

In the case of NTP, I don't see any reason for the NTP model to augment "/if:interfaces/if:interface". Rather, I think NTP specific config/state should be modeled with an interface list in the NTP model with each interface having an "if:interface-ref". 

The IS-IS clns-mtu leaf is a special case where an IOS Network Layer (CLNS) parameter is required for proper IS-IS operation. One could argue that there should be a separate ISO CLNS model but that would be overkill. Perhaps, we should move this single parameter into the IS-IS interface list - I'll discuss with my co-authors. 

Thanks,
Acee

    Lada

---

From: Robert Wilton <[email-masked]>
Date: Fri, 5 Oct 2018 17:04:46 +0100

Hi Acee,

> The IS-IS clns-mtu leaf is a special case where an IOS Network Layer (CLNS) parameter is required for proper IS-IS operation. One could argue that there should be a separate ISO CLNS model but that…
I thin that ISIS clns-mtu leaf is fine under if:interfaces/interface.  
This really feels more like an "interface property" rather than a 
"protocol property".

Thanks,
Rob

---

From: Andy Bierman <[email-masked]>
Date: Fri, 5 Oct 2018 09:59:45 -0700

On Fri, Oct 5, 2018 at 3:47 AM, Ladislav Lhotka <[email-masked]> wrote:

> > > > Not sure I have the latest version, but the last 2 closing right
> braces
> > > > get
> > > > left out)
> > > >

Defined under container ntp:

    +--rw interfaces
     |  +--rw interface* [name]
     |     +--rw name                if:interface-ref
     |     +--rw broadcast-server!
     |     |  +--rw ttl?              uint8
     |     |  +--rw authentication
     |     |  |  +--rw (authentication-type)?
     |     |  |     +--:(symmetric-key)
     |     |  |        +--rw key-id?   ->
/ntp/authentication/authentication-keys/key-id
     |     |  +--rw minpoll?          ntp-minpoll
     |     |  +--rw maxpoll?          ntp-maxpoll
     |     |  +--rw port?             uint16 {ntp-port}?
     |     |  +--rw version?          ntp-version
     |     +--rw broadcast-client!
     |     +--rw multicast-server* [address]
     |     |  +--rw address           rt-types:ip-multicast-group-address
     |     |  +--rw ttl?              uint8
     |     |  +--rw authentication
     |     |  |  +--rw (authentication-type)?
     |     |  |     +--:(symmetric-key)
     |     |  |        +--rw key-id?   ->
/ntp/authentication/authentication-keys/key-id
     |     |  +--rw minpoll?          ntp-minpoll
     |     |  +--rw maxpoll?          ntp-maxpoll
     |     |  +--rw port?             uint16 {ntp-port}?
     |     |  +--rw version?          ntp-version
     |     +--rw multicast-client* [address]
     |     |  +--rw address    rt-types:ip-multicast-group-address
     |     +--rw manycast-server* [address]
     |     |  +--rw address    rt-types:ip-multicast-group-address
     |     +--rw manycast-client* [address]
     |        +--rw address           rt-types:ip-multicast-group-address
     |        +--rw authentication
     |        |  +--rw (authentication-type)?
     |        |     +--:(symmetric-key)
     |        |        +--rw key-id?   ->
/ntp/authentication/authentication-keys/key-id
     |        +--rw ttl?              uint8
     |        +--rw minclock?         uint8
     |        +--rw maxclock?         uint8
     |        +--rw beacon?           uint8
     |        +--rw minpoll?          ntp-minpoll
     |        +--rw maxpoll?          ntp-maxpoll
     |        +--rw port?             uint16 {ntp-port}?
     |        +--rw version?          ntp-version

I agree with Acee that the data under /ntp could use an interface-ref and
not
reproduce /interfaces/interface/name under the /ntp hierarchy

I also like for "show interface" to list everything configured on that
interface.
Having a bunch of separate trees to search for any config related to a
specific interface
is not as easy to use.

Lada

Andy

---

From: Ladislav Lhotka <[email-masked]>
Date: Fri, 05 Oct 2018 20:25:43 +0200

On Fri, 2018-10-05 at 15:17 +0000, Acee Lindem (acee) wrote:
> Hi Lada, Andy, 
> 
> On 10/5/18, 4:32 AM, "yang-doctors on behalf of Ladislav Lhotka" <
> [email-masked] on behalf of [email-masked]> wrote:
> 
> In the case of NTP, I don't see any reason for the NTP model to augment
> "/if:interfaces/if:interface". Rather, I think NTP specific config/state
> should be modeled with an interface list in the NTP model with each interface
> having an "if:interface-ref".

Yes, I agree. I didn't bother to check the ietf-ntp module myself and
misunderstood what Andy wrote. Sorry for the confusion. 

>  
> 
> The IS-IS clns-mtu leaf is a special case where an IOS Network Layer (CLNS)
> parameter is required for proper IS-IS operation. One could argue that there
> should be a separate ISO CLNS model but that would be overkill. Perhaps, we
> should move this single parameter into the IS-IS interface list - I'll discuss
> with my co-authors.

I think there is no need to move clns-mtu.

Thanks, Lada

---

From: Martin Bjorklund <[email-masked]>
Date: Mon, 08 Oct 2018 10:11:31 +0200 (CEST)

> > > > > Not sure I have the latest version, but the last 2 closing right

Thanks, now fixed (see https://github.com/mbj4668/rfcstrip)

> reproduce /interfaces/interface/name under the /ntp hierarchy

But it does use the if:interface-ref; the 'name' which is the key in
the /ntp/interfaces/interface list is an if:interface-ref.

> Having a bunch of separate trees to search for any config related to a

Yes, but OTOH have info related to a certain feature (NTP in this
case) augmented under a bunch of separate trees is also not easy to
use.  In order to see NTP-related info you have to do "show ntp",
"show interfaces interface ntp", ...

/martin