[Roll] Yangdoctors early review of draft-ietf-roll-mpl-yang-01

[Radek Krejčí <[e-mail]>]

Reviewer: Radek Krejčí
Review result: Not Ready

This document specifies 4 YANG modules for Multicast Protocol for Low power and
lossy Networks (MPL) implementations. The document is not NMDA compliant and
the separated modules are not correctly linked together (not even via leafrefs,
but I suggest to redesign them as augments).

document

- all the modules are YANG 1.1, but you refer to RFC 6020 which defines YANG
1.0, fix the references to RFC 7950 - there is no note about NMDA, the document
is NOT NMDA compliant because of the separated status data which are actually
connected with the specific mpl domains (see below)

Section 1.1

- there is no note about SID term, you should refer draft-ietf-core-sid

Section 1.1.1

- instead of text description, refer to the Tree diagram specification (follow
rfc6087bis sec. 3.4)

Section 2

- consider to split description of the modules to their tree diagrams, the
intro of the section could be just about motivation to split all the data into
a separated modules

Section 3

- as mentioned for section 1.1, there is no explanation of SID term, no
reference, nothing

Section 4

OLD
  This section describes four yang modules.  The model is based
NEW
  This section describes four yang modules.  The models are based

- RFC 7223 was obsoleted by 8343

- division between the schemas is not clear to me. Isn't the mpl-domain kind of
core module, while the others extend it? In mpl-ops, I see mpl-parameter list
connected with domainID from mpl-domain. Despite it should be at least leafref,
I believe that it should actually augment the domains list from mpl-domain.
Similarly, you can have the mpl-domain's top level domain (or better rename it
to something like mpl) container as a top-level container holding all the
mpl-related data together. This is also the reason why the document is NOT NMDA
compliant - instead of augmenting domains list, the status data are held in a
separate data tree.

- please fix indentation in the models. It makes reading of the model very
difficult. If statement (mainly descriptions) carry into a subsequent line,
please indent also those consequent lines. Also indent opening { of a node
definition (e.g. domainID{ -> domainID {)

- update copyright notice (year)

- you mix several naming conventions: domainID, SEED_SET_ENTRY_LIFETIME,
life-time - please follow rfc6087bis sec 4.3.1

Section 5

- draft must register schema modules, so there are definitely IANA
considerations. Besides this, I believe that also SID numbers are supposed to
be registered via IANA.

Section 7

- update and maintain the changelog! The current information are taken from
draft-vanderstok-roll-mpl-yang.

[e-mail]

/domain/single
  - calling the whole switch as one of its case does not seem descriptive

/domain/single/mpl-domain/mpl-domain/addresses/interfaces
  - where, in RFC 6206, is interface name defined? Probably should be RFC 8343
  - why not use leafref to ietf-interfaces?

[e-mail]

/mpl-ops/SEED_SET_ENTRY_LIFETIME (and several others)
  - use default statement
  - consider using units statement

/mpl-ops/mpl-parameter/domainID
  - should be leafref, but actually the whole mpl-parameter should be an
  augment of domains list in mpl-domain

/mpl-ops/mpl-parameter/CONTOL_MESSAGE_I* (and possibly others)
  - aren't there some limitations? Such as min < max? Then there should be must
  statement with the appropriate constraint specification.

[e-mail]

/mpl-seeds/local
  - fix upper cases in description or better rewrite the description as a
  sentence

/mpl-seeds/buffered-messages/t I
  - there is no SE-LIFETIME in the modules nor draft

[e-mail]

/mpl-statistics/nr-of-copies-forwarded
  - there is no number-of-copies-received in the modules nor draft

/mpl-statistics/reset-statistics
  - according to the location of the action, it is supposed to reset just the
  statistics for the specific buffer set (instance of the mpl-statistics list),
  right? Maybe it should be clarified in description, now there is "all
  statistics" which can be confusing according to the place of the action.