Skip to main content

Last Call Review of draft-ietf-ccamp-flexigrid-yang-09
review-ietf-ccamp-flexigrid-yang-09-yangdoctors-lc-jethanandani-2021-03-11-00

Request Review of draft-ietf-ccamp-flexigrid-yang-08
Requested revision 08 (document currently at 16)
Type Last Call Review
Team YANG Doctors (yangdoctors)
Deadline 2021-02-19
Requested 2021-01-25
Requested by Daniele Ceccarelli
Authors Universidad Autonoma de Madrid , Daniel Perdices Burrero , Daniel King , Young Lee , Haomian Zheng
I-D last updated 2021-03-11
Completed reviews Yangdoctors Last Call review of -09 by Mahesh Jethanandani (diff)
Rtgdir Last Call review of -13 by Dhruv Dhody (diff)
Comments
the draft is tightly connected to https://tools.ietf.org/html/draft-ietf-ccamp-wson-yang-28. 
Having it reviewed by the same YANG doctor (Acee Lindem) could be helpful...if possible...
Thanks!
Assignment Reviewer Mahesh Jethanandani
State Completed
Request Last Call review on draft-ietf-ccamp-flexigrid-yang by YANG Doctors Assigned
Posted at https://mailarchive.ietf.org/arch/msg/yang-doctors/0BLfi59FprbpRAWeeRmTCz6qMcY
Reviewed revision 09 (document currently at 16)
Result On the Right Track
Completed 2021-03-11
review-ietf-ccamp-flexigrid-yang-09-yangdoctors-lc-jethanandani-2021-03-11-00
I am not an expert in traffic engineering as it relates to optical networks. If
my understanding of the traffic engineering for optical networks is incorrect,
feel free to educate me and everyone else. This review is looking at the draft
from a YANG perspective. With that said, I have marked it as on the right path,
because of some of the points discussed below.

Summary:

This document defines a YANG module for managing flexi-grid optical networks.
The model defined in this document specifies a flexi-grid traffic engineering
database that is used to describe the topology of a flexi-grid network.  It is
based on and augments existing YANG models that describe network and traffic
engineering topologies.

Nits

Section 1 - Introduction
s/[G.694.1] and G.872 [G.872] provides/[G.694.1] and G.872 [G.872] provide/
s/flexi-grid elements../flexi-grid elements/

Comments:

Section 4 - Example of Use

Thank you first of all for putting together an example to help folks understand
the different terminologies used in the document. However, I am confused whose
perspective does the example present. The paragraph starts by saying "In order
to configure a network media channel ...", which to me sounds like a client
*configuring* the network media channel. How does configuring of nodes A and E
provide connectivity towards the node interface? Or did you mean to say that
the act of configuring nodes A and E will somehow bring up the connection
towards the node interface?

Also, please s/also provide/also configure/ as you configure using a YANG model.

Generally, drafts and RFCs are not written from a first person view. Therefore
statement such as the following should be rewritten.

OLD:
      Then, we also define the links 1 to 5 that interconnect nodes,
      indicating which flexi-grid labels are available.

NEW:
      This example configures links 1 to 5 that interconnect nodes,
      with flexi-grid labels that are available.

or something similar.

s/granularity are also provided/granularity is also configured/

I believe XXXX is assigned to draft-ietf-ccamp-layer0-types:

     import ietf-layer0-types {
       prefix "l0-types";
       reference
         "RFC XXXX: A YANG Data Model for Layer 0 Types";
     }

yet the YANG model says:

        This version of this YANG module is part of RFC XXXX; see
        the RFC itself for full legal notices.";

and
          "RFC XXXX: A YANG Data Model for Flexi-Grid Optical Networks";
        // RFC Ed.: replace XXXX with actual RFC number, update date
       // information and remove this note

What gives?

RFC editors prefer that all instructions for substitution are provided in one
location instead of having them sprinkled all over the document.

Section 5 - Tree Structure

Having 17 pages of a tree diagram without any explanation for the different
sections of the tree diagram is hardly helpful. On the other hand an abridged
version of the tree diagram explaining the different sections would be more
helpful normative text. Suggest that --tree-depth and --tree-path options be
used in pyang to reduce the size of the tree and to break it up into smaller
pieces that can then be explained. The full tree diagram can be added as
non-normative text in the Appendix.

Section 6 - YANG model

The revision statement is incorrect. It should be the date indicated in the
name of the module 2020-02-20 or the model file should be renamed to reflect
the latest date:

ietf-flexi-grid-topology@2020-02-22.yang:1: warning: unexpected latest revision
"2020-10-21" in ietf-flexi-grid-topology@2020-02-22.yang, should be "2020-02-22"

The document says - "An application example is provided towards the end of the
document to better understand the utility of this YANG model." However, no such
example exists in the document. With no example, how is an operator supposed to
know how to use the module. Besides, there is no way to know that this model is
even valid.

What is not clear with the model is the first container and the when statement.
As another YANG doctor explained, the when-stmt applies to *instances* of a
model, not to the schema. Therefore a when-stmt that points back to a node that
is part of schema and is a presence container does not achieve what I think the
authors intended, i.e. a when condition.

The second general comments has to do with the 80 augment statements. As an
operator, I would find it very tedious to configure this model, what with 80
different XPaths to follow. Would it not be better to gather all the augmenting
(flexi-grid) parameters together and use leafrefs to link these data structures
to the ietf-network module?

General:

A idnits run of the draft reveals a few issues.  Checking boilerplate required
by RFC 5378 and the IETF Trust (see
  https://trustee.ietf.org/license-info):
  ----------------------------------------------------------------------------

     No issues found here.

  Checking nits according to https://www.ietf.org/id-info/1id-guidelines.txt:
  ----------------------------------------------------------------------------

     No issues found here.

  Checking nits according to https://www.ietf.org/id-info/checklist :
  ----------------------------------------------------------------------------

     No issues found here.

  Miscellaneous warnings:
  ----------------------------------------------------------------------------

     No issues found here.

  Checking references for intended status: Informational
  ----------------------------------------------------------------------------

  == Missing Reference: 'RFCXXXX' is mentioned on line 167, but not defined

  == Missing Reference: 'RFCYYYY' is mentioned on line 168, but not defined

     Summary: 0 errors (**), 0 flaws (~~), 2 warnings (==), 0 comments (--).