Re: [netmod] WG Last Call: draft-ietf-netmod-intf-ext-yang-07

[Martin Bjorklund <[e-mail]>]

Hi,

Here is my (late) review of draft-ietf-netmod-intf-ext-yang-07.  It is
a well-written document and my comments are mostly minor.


o  Abstract

  OLD:

   The YANG data model in this document conforms to the Network
   Management Datastore Architecture (NMDA) defined in RFC 8342.

  NEW:

   The YANG modules in this document conform to the Network
   Management Datastore Architecture (NMDA) defined in RFC 8342.


o  Section 1

     One of the aims of this draft is to provide a standard namespace and
     path for these configuration items regardless of the underlying
     interface type.  For example a standard namespace and path for

  "standard namespace and path" sounds a bit clumsy.  In section 2 you
  use "standard definition", perhaps that can be use here.


o  General

  s/this internet draft/this document/g
  s/this draft/this document/g


o  Section 2

  It seems this short section mostly says what is already said in
  section 1.  Remove?


o  3

  The text says:

    o  A parent interface leaf useable for all types of sub-interface
       that are children of parent interfaces.

  I suggest you add before that bullet:

    o  A generic "sub-interface" identity that an interface identity
       defintion can derive from if it defines a sub-interface.


o  3.1

  The text says:

    E.g. in the
    case that the link state transition is suppressed then there is no
    change of the /if:interfaces-state/if:interface/oper-status or
    /if:interfaces-state/if:interfaces/last-change leaves for the
    interface that the feature is operating on.

  This should be:

    no change of the /if:interfaces/if:interface/oper-status or
    /if:interfaces/if:interfaces/last-change leaves for the
    interface that the feature is operating on.


o  3.2

  It took me some time to understand the dampening algorithm.  Why is
  it important to talk about nominal values and that a device doesn't
  have to use 1000 as the penalty, as long as they scale the given
  values?  Wouldn't it be easier to describe the algorithm w/o any
  nominal values, and then explain that an implementation is free to
  implement this algorithm in any way it wants (which of course is
  true for everything we do...)

  Otherwise, the text currently says:

   Implementations are not required to use a penalty of 1000 units in
   their dampening algorithm, but should ensure that the Suppress
   Threshold and Reuse Threshold values are scaled relative to the
   nominal 1000 unit penalty to ensure that the same configuration
   values provide consistent behaviour.

  Should "should" in this text be "SHOULD"?  Or perhaps "MUST"?


o  3.2.1

  The text says:

   When the accumulated penalty reaches the default or
   configured suppress threshold, the interface is placed in a dampened
   state.

  The term "dampended state" occurs twice, in 3.2.1 and 3.2.3.  It is
  not used in the YANG model.  I suspect the leaf "suppressed"
  reflects this.  Perhaps align naming.


o  4

  It would be useful with a sentence that describes the relationship
  to /if:interfaces/if:interface/if:phys-address.

  It seems that the mac-address leaf is useful when the mac address
  can be configured; otherwise if:phys-address should be sufficient,
  right?  Should the mac-address leaf have a feature, or can we expect
  all implementations to support configurable mac addresses?


o  4

  You add a container 'statistics' under 'ethernet-like', so we have:

  +--rw interfaces
     +--rw interface* [name]
        ...
        +--ro statistics
           ...
        +--rw ethlike:ethernet-like
           +--ro ethlike:statistics
              ...

  Did you consider augmenting the container if:statistics instead?  I
  think it can be useful to have all statistics in the same container
  in this case.


o  7.2

  Perhaps show the (related) 'if:oper-status' leaf as well.


o  7.3

  Perhaps show the (related) if:phys-address' leaf as well in the
  first and third examples.

  Before the second example, perhaps change:

   The following example shows an explicit MAC address being configured
   on interface eth0.

  to:

   The following example shows the intended configuration for
   interface eth0 with an explicit MAC address being configured.


o  YANG nits

  Both YANG modules list the WG chairs; we don't do that anymore.

  Both YANG modules have the IETF Trust Copyright statement, but not
  exactly as it should be (try: pyang --ietf and/or pyang --ietf-help)

  Many descriptions are full sentences w/o the ending ".".

  The reference in the revision statement should be changed to "RFC
  XXXX: <title>"



/martin