Hi Mahesh,
Thanks for the detailed review, suggestions and clarification questions.
All the NITS and editorial issues are gratefully acknowledged, and your final comments on condition and augmentations will need a little further investigation and discussion.
Please see comments inline "DK>>".
BR, Dan.
-----Original Message-----
From: Mahesh Jethanandani via Datatracker <noreply@xxxxxxxx>
Sent: 11 March 2021 17:50
To: yang-doctors@xxxxxxxx
Cc: ccamp@xxxxxxxx; draft-ietf-ccamp-flexigrid-yang.all@xxxxxxxx; last-call@xxxxxxxx
Subject: Yangdoctors last call review of draft-ietf-ccamp-flexigrid-yang-09
Reviewer: Mahesh Jethanandani
Review result: On the Right Track
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/
DK>> Done.
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?
DK>> Done. We have updated this text and moved an example into the sister document, which describes the flexi-grid media-channel (combined with the flexi-grid topo).
Also, please s/also provide/also configure/ as you configure using a YANG model.
DK>> Done.
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.
DK>> Done.
s/granularity are also provided/granularity is also configured/
DK>> Done.
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?
DK>> Done. Fixed replication of XXXX/YYYY fixed in the latest version.
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.
DK>> Done. We will include a tree in the next version.
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@xxxxxxxxxxxxxxx:1: warning: unexpected latest revision "2020-10-21" in ietf-flexi-grid-topology@xxxxxxxxxxxxxxx, should be "2020-02-22"
DK>> Done. Date updated in the next version.
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.
DK>> We are investigating this issue further; other TE model documents use the condition "when" statement in the same manner.
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?
DK>> We follow the convention used in other CCAMP YANG model documents, and observations were also discussed NETMOD WG on https://mailarchive.ietf.org/arch/msg/netmod/-6L8KYVVhFiS8KpmH2fUpBmdSy8/. We also observed the example provided in RFC8795, as per https://datatracker.ietf.org/doc/html/rfc8795#appendix-C.
DK>> Thanks again for your review and comments!
BR, Dan.
--
last-call mailing list
last-call@xxxxxxxx
https://www.ietf.org/mailman/listinfo/last-call
