[Teas] Yangdoctors early review of draft-ietf-teas-yang-rsvp-10

Ebben Aries via Datatracker <noreply@ietf.org> Sat, 04 May 2019 00:10 UTC

Return-Path: <noreply@ietf.org>
X-Original-To: teas@ietf.org
Delivered-To: teas@ietfa.amsl.com
Received: from ietfa.amsl.com (localhost [IPv6:::1]) by ietfa.amsl.com (Postfix) with ESMTP id 47F061202E5; Fri, 3 May 2019 17:10:32 -0700 (PDT)
MIME-Version: 1.0
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: 7bit
From: Ebben Aries via Datatracker <noreply@ietf.org>
To: yang-doctors@ietf.org
Cc: ietf@ietf.org, teas@ietf.org, draft-ietf-teas-yang-rsvp.all@ietf.org
X-Test-IDTracker: no
X-IETF-IDTracker: 6.95.1
Auto-Submitted: auto-generated
Precedence: bulk
Reply-To: Ebben Aries <exa@arrcus.com>
Message-ID: <155692863223.7173.7717533907709205656@ietfa.amsl.com>
Date: Fri, 03 May 2019 17:10:32 -0700
Archived-At: <https://mailarchive.ietf.org/arch/msg/teas/lFF1dewiCvCrSJutG1skzWAMl3U>
Subject: [Teas] Yangdoctors early review of draft-ietf-teas-yang-rsvp-10
X-BeenThere: teas@ietf.org
X-Mailman-Version: 2.1.29
List-Id: Traffic Engineering Architecture and Signaling working group discussion list <teas.ietf.org>
List-Unsubscribe: <https://www.ietf.org/mailman/options/teas>, <mailto:teas-request@ietf.org?subject=unsubscribe>
List-Archive: <https://mailarchive.ietf.org/arch/browse/teas/>
List-Post: <mailto:teas@ietf.org>
List-Help: <mailto:teas-request@ietf.org?subject=help>
List-Subscribe: <https://www.ietf.org/mailman/listinfo/teas>, <mailto:teas-request@ietf.org?subject=subscribe>
X-List-Received-Date: Sat, 04 May 2019 00:10:32 -0000

Reviewer: Ebben Aries
Review result: On the Right Track

2 modules in this draft:
- ietf-rsvp@2019-02-18.yang
- ietf-rsvp-extended@2019-02-18.yang

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

Module ietf-rsvp@2019-02-18.yang:
- 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 ietf-rsvp-extended@2019-02-18.yang:
- 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.