Subject: Yangdoctors early review of draft-ietf-teas-yang-rsvp-10

From: Ebben Aries via Datatracker <[email-masked]>
Date: Fri, 03 May 2019 17:10:32 -0700

Reviewer: Ebben Aries
Review result: On the Right Track

2 modules in this draft:
- [email-masked]
- [email-masked]

No YANG compiler errors or warnings (pyang 2.0, yanglint 1.1.16, confdc 6.6.1)

Module [email-masked]:
- Remove WG Chairs from contact information per
  https://tools.ietf.org/html/rfc8407#appendix-B
- Use of 'state' containers.  It is stated in Section 2.3 that 'Derived state
  data is contained under a "state" container...'.  My only comments here are:
  a) Should use caution when using 'state' containers in NMDA compliant
  modules.  Rob put together a nice doc here that I won't reiterate:
  https://github.com/netmod-wg/FAQ/wiki/NMDA-Modelling-FAQ
  Using such nomenclature locks you into r/o nodes only.
  b) In some cases, the hierarchy is a bit redundant (statistics/state).
  Other NMDA compliant modules will not introduce another 'state' hierarchy
  for instance (see ietf-interfaces)
- leaf 'packet-len' is abbreviated while most other leafs are not
- authentication-key is of type string.  Is this leaf mean to be clear-text?
  There is nothing associated with this type that would imply special
  treatment in handling.
- crypto-algorithm: Are all identities from ietf-key-chain supported?
- Pluralization on counters:  e.g. 'in-error' vs. 'in-errors'. Maintain
  consistency with other modules (see ietf-interfaces)
- Normative references are missing for some of the modules imported.  These
  must be added per https://tools.ietf.org/html/rfc8407#section-3.9

Module [email-masked]:
- Remove WG Chairs from contact information per
  https://tools.ietf.org/html/rfc8407#appendix-B
- Use of 'state' containers.  It is stated in Section 2.3 that 'Derived state
  data is contained under a "state" container...'.  My only comments here are:
  a) Should use caution when using 'state' containers in NMDA compliant
  modules.  Rob put together a nice doc here that I won't reiterate:
  https://github.com/netmod-wg/FAQ/wiki/NMDA-Modelling-FAQ
  Using such nomenclature locks you into r/o nodes only.
  b) In some cases, the hierarchy is a bit redundant (statistics/state).
  Other NMDA compliant modules will not introduce another 'state' hierarchy
  for instance (see ietf-interfaces)
- Pluralization on counters:  e.g. 'in-error' vs. 'in-errors'. Maintain
  consistency with other modules (see ietf-interfaces)
- 9 features are defined with no 'if-feature' statements.  Where are these
  feature conditions meant to be used?
- Normative references are missing for some of the modules imported.  These
  must be added per https://tools.ietf.org/html/rfc8407#section-3.9

General comments on the draft/modules:
- The state container and list key designs appear very familiar to that of
  OpenConfig modules however not consistent with IETF module design.
  Consistency is key from each producing entity (e.g. IETF in this case)
- The draft mentions RPCs and Notifications however none are defined within
  the modules
- Section 2.3: Has examples of RPCs and Notifications that are non-existant in
  the modules
- Abstract: s/RVSP/RSVP/
- Abstract: s/remote procedural/remote procedure/
- Section 2: s/extensiion/extension/
- Section 4: Update the security considerations section to adhere to
  https://tools.ietf.org/html/rfc8407#section-3.7 and
  https://trac.ietf.org/trac/ops/wiki/yang-security-guidelines
- Various (missing) wording/pluralization cleanup throughout that I'll defer
  to the RFC Editor.  A once over proofread should solve this.