Early Review of draft-ietf-teas-yang-rsvp-te-07

Request Review of draft-ietf-teas-yang-rsvp-te-07
Requested rev. 07 (document currently at 09)
Type Early Review
Team YANG Doctors (yangdoctors)
Deadline 2019-12-15
Requested 2019-11-17
Requested by Lou Berger
Authors Vishnu Beeram, Tarek Saad, Rakesh Gandhi, Xufeng Liu, Igor Bryskin, Himanshu Shah
Draft last updated 2020-01-13
Completed reviews Yangdoctors Early review of -07 by Ebben Aries (diff)
Requesting early review in preparation for WG last call
Assignment Reviewer Ebben Aries 
State Completed
Review review-ietf-teas-yang-rsvp-te-07-yangdoctors-early-aries-2020-01-13
Posted at https://mailarchive.ietf.org/arch/msg/yang-doctors/1G_1C8Hkeeys_Ny3rNdBL0zIbio
Reviewed rev. 07 (document currently at 09)
Review result On the Right Track
Review completed: 2020-01-13


First, I'd like to apologize for the delay in getting this review out.
Overall, the draft/modules are in fairly good shape but have some
comments/clarifications below.

2 modules in this draft:
- ietf-rsvp-te@2019-07-06.yang
- ietf-rsvp-te-mpls@2019-07-06.yang

YANG compiler errors or warnings (pyang 2.1.1, yanglint 1.5.5, confdc 7.2.1)
- ietf-rsvp-te: Line 262 - when statement should have a description
  substatement per RFC6087, Section 4.12
- ietf-rsvp-te-mpls: Line 121 - when statement should have a description
  substatement per RFC6087, Section 4.12

Module ietf-rsvp-te@2019-07-06.yang:
- Remove WG Chairs from contact information per
- There are 2 augments to rsvp-te-interface-attributes of an empty state
  container.  What are the intentions behind this and is it meant for future
  proofing other modules?

Module ietf-rsvp-te-mpls@2019-07-06.yang:
- Remove WG Chairs from contact information per
- The leaf 'percent-value'.  What is the intention of specifying a range here
  that covers the entire uint32 space?

General comments on the draft/modules:
- Consistent use of 'units' statement for various data nodes
  (soft-preemption-timeout is one such example)
- When 'units' is in use, rather than define it as "Bytes per second", I
  suggest replacing w/ the common usage of "bps"
- Various leaf descriptions have "XX" placeholders that need updating
- In the IANA considerations, this appears possibly cut/pasted from another
  module set and needs to be corrected to reflect the actual 2 modules
  contained within this draft.
- It seems these modules are agnostic of IPv4/IPv6 transport yet existing
  implementations are going to be geared towards IPv4.  What if any
  considerations need to be called out or adjusted here as an implementation
  would either need to resolve this via deviations or backend validations.
- Lack of defined RPCs.  For all the cases around RSVP signalled LSPs, is
  there an intention to add relevant RPCs in this draft? (Same w/
- Am I missing any generic per-LSP counters defined throughout these models?
- Are there any uses where features/if-feature could be used throughout these