Subject: [netmod] AD review of draft-ietf-netmod-ip-cfg

From: Benoit Claise <[email-masked]>
Date: Mon, 09 Dec 2013 17:51:42 +0100

Dear NETMOD WG,

Here is my AD review for draft-ietf-netmod-ip-cfg.
Nothing serious about this draft, but a couple of editorial changes could improve the draft.

-
draft-ietf-netmod-interfaces-cfg-14 abstract

    This document defines a YANG data model for the management of network
    interfaces.  It is expected that interface type specific data models
    augment the generic interfaces data model defined in this document.
    The data model includes configuration data and state data (status
    information and counters for the collection of statistics).

This draft draft-ietf-netmod-ip-cfg-11 abstract is a little light in comparison

Abstract

    This document defines a YANG data model for management of IP
    implementations.

You should express:
    The data model includes configuration data and state data (status
    information but no counters for the collection of statistics).

- Introduction

  Parameters to manage IP routing are defined in
    [I-D.ietf-netmod-routing-cfg].

This is maybe interesting, but it seems way more interesting to speak about the connection with
draft-ietf-netmod-interfaces-cfg, which is a normative reference.

-
add state data in the terminology

-
Below ...
    The data model defines two state containers per interface, "ipv4" and
    "ipv6", representing the IPv4 and IPv6 address families.  In each
    container, there is a leaf "forwarding" that indicates if IP packet
    forwarding is enabled on that interface.  In each container there is
    also a list of all addresses in use, and a list of known mappings
    from IP addresses to link-layer addresses.

... it might be interesting to add a sentence or two on the difference between the the /interfaces/ and /interfaces-state/.
At first glance they look similar, but there are not:
  * the origin has been added in multiple containers
  * the IPv6 status has been added (and not IPv4), because they're the SLAAC state
  * the IPv6 neighbor state has been added.

-
Appendix A.  Example: NETCONF <get> reply

You might want to mention that there are no neighbor for IPv4, or add one.

Regards, Benoit (OPS AD)

---

From: Martin Bjorklund <[email-masked]>
Date: Tue, 10 Dec 2013 12:26:30 +0100 (CET)

Hi,

>    information but no counters for the collection of statistics).

Ok.  I'd prefer to mention what we do, and not what we don't do, so I
suggest:

NEW:

  This document defines a YANG data model for management of IP
  implementations.  The data model includes configuration data and
  state data.

> draft-ietf-netmod-interfaces-cfg, which is a normative reference.

I agree.  So remove the reference to routing:

OLD:

The data model covers configuration of per-interface IPv4 and IPv6
parameters, and mappings of IP addresses to link-layer addresses.  It
also provides information about which IP addresses are operationally
used, and which link-layer mappings exist.

Parameters to manage IP routing are defined in
^I-D.ietf-netmod-routing-cfg^.

NEW:

The data model covers configuration of per-interface IPv4 and IPv6
parameters, and mappings of IP addresses to link-layer addresses.  It
also provides information about which IP addresses are operationally
used, and which link-layer mappings exist.  Per-interface parameters
are added through augmentation of the interface data model defined in
^I-D-ietf-netmod-interfaces-cfg^.

> add state data in the terminology

Ok.

> ... it might be interesting to add a sentence or two on the difference
> between the the /interfaces/ and /interfaces-state/.
>  * the IPv6 status has been added (and not IPv4), because they're the
>  * SLAAC state

I am not sure that this is the right way to view these structures.
They have very different semantics; which is why they are separate.
One is the "manual config", and the other one reflects what is
currently in operational use.  So I do not know what/if to add.

> You might want to mention that there are no neighbor for IPv4, or add

Ok, I'll add one.

/martin

---

From: Benoit Claise <[email-masked]>
Date: Wed, 11 Dec 2013 17:48:31 +0100

Martin,
>    implementations.  The data model includes configuration data and
fine.
> ^I-D-ietf-netmod-interfaces-cfg^.
Fine.
> currently in operational use.
Sure, I understand that.

Here is how I went through the draft: I read through the two structures 
(I like that high level structure very much) to quickly understand the 
YANG module content.

        The data model has the following structure for IP configuration per
        interface:

           +--rw if:interfaces
              +--rw if:interface* [name]
                 ...
                 +--rw ipv4!
                 |  +--rw enabled?            boolean
                 |  +--rw forwarding?         boolean
                 |  +--rw mtu?                uint16
                 |  +--rw address* [ip]
                 |  |  +--rw ip               inet:ipv4-address-no-zone
                 |  |  +--rw (subnet)
                 |  |     +--:(prefix-length)
                 |  |     |  +--rw ip:prefix-length?   uint8
                 |  |     +--:(netmask)
                 |  |        +--rw ip:netmask?         yang:dotted-quad
                 |  +--rw neighbor* [ip]
                 |     +--rw ip                    inet:ipv4-address-no-zone
                 |     +--rw link-layer-address    yang:phys-address
                 +--rw ipv6!
                    +--rw enabled?            boolean
                    +--rw forwarding?         boolean
                    +--rw mtu?                uint32
                    +--rw address* [ip]
                    |  +--rw ip               inet:ipv6-address-no-zone
                    |  +--rw prefix-length    uint8
                    +--rw neighbor* [ip]
                    |  +--rw ip                    inet:ipv6-address-no-zone
                    |  +--rw link-layer-address    yang:phys-address
                    +--rw dup-addr-detect-transmits?   uint32
                    +--rw autoconf
                       +--rw create-global-addresses?        boolean
                       +--rw create-temporary-addresses?     boolean
                       +--rw temporary-valid-lifetime?       uint32
                       +--rw temporary-preferred-lifetime?   uint32

        The data model defines two configuration containers per interface,
        "ipv4" and "ipv6", representing the IPv4 and IPv6 address families.
        In each container, there is a leaf "enabled" that controls if the
        address family is enabled on that interface, and a leaf "forwarding"
        that controls if IP packet forwarding for the address family is
        enabled on the interface.  In each container, there is also a list of
        configured addresses, and a list of configured mappings from IP
        addresses to link-layer addresses.

        The data model has the following structure for IP state per
        interface:

           +--ro if:interfaces-state
              +--ro if:interface* [name]
                 ...
                 +--ro ipv4!
                 |  +--ro forwarding?   boolean
                 |  +--ro mtu?          uint16
                 |  +--ro address* [ip]
                 |  |  +--ro ip               inet:ipv4-address-no-zone
                 |  |  +--ro (subnet)?
                 |  |  |  +--:(prefix-length)
                 |  |  |  |  +--ro prefix-length?   uint8
                 |  |  |  +--:(netmask)
                 |  |  |     +--ro netmask?         yang:dotted-quad
                 |  |  +--ro origin?          ip-address-origin
                 |  +--ro neighbor* [ip]
                 |     +--ro ip                    inet:ipv4-address-no-zone
                 |     +--ro link-layer-address?   yang:phys-address
                 |     +--ro origin?               neighbor-origin
                 +--ro ipv6!
                    +--ro forwarding?   boolean
                    +--ro mtu?          uint32
                    +--ro address* [ip]
                    |  +--ro ip               inet:ipv6-address-no-zone
                    |  +--ro prefix-length    uint8
                    |  +--ro origin?          ip-address-origin
                    |  +--ro status?          enumeration
                    +--ro neighbor* [ip]
                       +--ro ip                    inet:ipv6-address-no-zone
                       +--ro link-layer-address?   yang:phys-address
                       +--ro origin?               neighbor-origin
                       +--ro is-router?            empty
                       +--ro state?                enumeration

Browsing through the two structures, the first thing I realized is that 
they're similar.
The second thing I realized is that they're almost similar, not quite!
Then, logically, I started to wonder what the differences were, and I 
tried to compare the two with some page up/page down.
Hence my feedback:

    it might be interesting to add a sentence or two on the difference

If it's only me, leave that comment alone.
If some other people believe it makes sense...

Regards, Benoit

> So I do not know what/if to add.
>
>
>> -
> .