Skip to main content

Last Call Review of draft-ietf-ccamp-layer0-types-03

Request Review of draft-ietf-ccamp-layer0-types-02
Requested revision 02 (document currently at 09)
Type Last Call Review
Team YANG Doctors (yangdoctors)
Deadline 2019-12-04
Requested 2019-11-12
Requested by Daniele Ceccarelli
Authors Haomian Zheng , Young Lee , Aihua Guo , Victor Lopez , Daniel King
Draft last updated 2019-12-13
Completed reviews Yangdoctors Last Call review of -01 by Robert Wilton (diff)
Yangdoctors Last Call review of -03 by Robert Wilton (diff)
Rtgdir Last Call review of -06 by Emmanuel Baccelli (diff)
Genart Last Call review of -07 by Reese Enghardt (diff)
This draft is connected with Would it be possible to have them reviewed together?
Assignment Reviewer Robert Wilton
State Completed
Review review-ietf-ccamp-layer0-types-03-yangdoctors-lc-wilton-2019-12-13
Posted at
Reviewed revision 03 (document currently at 09)
Result Almost Ready
Completed 2019-12-13
Comments on the document:

1) I would delete the "overview" paragraph at the top of section 2, and just
promote section 2.1 as section 2.

2) 2.1. Layer 0 Types Module Contents:

The descriptions are good, but I would suggest formatting these as a table, or
more tightly link the definition to it's description.


"Operational-mode: A type that represents operational mode as defined in

Instead of:


A type that represents operational mode as defined in [ITU-Tg6982]."

3) I would define the module as YANG version "1.1" (because the language
behaviour is generally better specified) and then reference only RFC 7950 in
the introduction.

4) typo in the Security Considerations "layer0 => layer 0", and also in the
title of section 3.

5) I have suggested changing the module prefix to "l0-types" rather than
layer2-types.  If you make this change then the IANA considerations would need
to be updated, along with section 1.2.

Comments on the YANG module:

1) I would suggest changing the YANG prefix to "l0-types" to help keep it short
(particularly for the identities).

2) I would suggest making the module "yang-version 1.1;", because the behaviour
of YANG 1.1 is better specified.  In fact, I would recommend that all IETF YANG
modules are defined as YANG 1.1.

3) It is actually necessary to define frequency-thz at all?  I think that the
discussion in the WG suggested that it might be better if the frequencies are
always defined in Ghz and then converted in the client as necessary.

4) frequency-ghz: Is 3 fraction digits sufficient for future expansion?  E.g.
It would seem to support a flex grid 3.125Ghz channel spacing, but not 1.5625

5) We should aim for consistency of the identity names in the layer-1 types
module.  E.g. perhaps OTU, ODU and OPU should be capitalized.

6) The models uses identities for bandwidth, but I wonder whether defining a
numerical typedef might be a better choice (e.g. more efficient and perhaps
easier for programs to work with).  Here, I have constrained the values that
are allowed, but they could also be unconstrained:

typedef channel-bandwidth {
  type decimal64 {
    fraction-digits 2;
      "2.66|10.70|11.04|11.09|11.27|11.31|43.01|44.57|44.58|100.00 ... max"
      "Bandwidth carried by a single wavelength channel"
  units "Gb/s";

Or another alternative would be to use Mb/s, which would then allow them to be
specified as a union of specific values and an arbitrary bandwidth value.

7) Same comment as for bandwidth applies to the channel spacing identities. 
I.e. I wonder whether these wouldn't be better defined using a decimal64 type.

typedef dwdm-channel-spacing {
  type decimal64 {
    fraction-digits 2;
      "Bandwidth carried by a single wavelength channel"
  units "Ghz";

8) Same comment as for bandwidth and dwdm-channel-spacing could also be applied
to flexi-grid-channel-spacing, flexi-slot-width-granularity,

9) In grouping layer0-label-range-info
I would rename this grouping to l0-layer-range-info, change "layer0" => "layer
0" in the description.  Also "priority" could do with a more detailed
description as to what it means, etc.

10) In grouping flexi-grid-label-start-end, I think that the type should be
"flexi-n" rather than "int16".

Typos: "girds" => "grids", "attrtibutes" => "attributes"

Spacing/indenting needs to be fixed:
 - In "grouping wson-label-hop", just before case cwdm
 - In "grouping flexi-grip-label-hop", should have a blank line before, and
 "case super" block/fields indentation doesn't look right. - In some of the
 typedef definitions, the "{" should move from the start of the following line
 to the typedef line. In general, as a starting point, after all other markups
 have been made then it might be a good idea to use pyang to format the YANG
 file for you, e.g., "pyang -f yang --yang-line-length 69", but probably with
 some more blank lines, otherwise it is a bit dense.