I have reviewed this document as part of the YANG doctors directorate's ongoing effort to review all IETF documents being processed by the IESG. These comments were written with the intent of improving the operational aspects of the IETF drafts. Comments that are not addressed in last call may be included in AD reviews during the IESG review. Document editors and WG chairs should treat these comments just like any other last call comments. Summary: This document includes the augmentations to the ietf-te-topology model for WSON in the model ietf-wson-topology.yang and the new Optical Layer-0 types in ietf-layer0-types.yang. I'm not a Subject Matter Expert (SME) in this area so my comments are mainly related to YANG and the draft. While the information content of this draft is sound, there are a number of minor issues that need to be resolved. Therefore I would characterise the review state as "On the Right Track". Major Issues: None Minor Issues: 1. The document has 7 authors. This will have to be ok'ed by the responsible AD. 2. There is no consistency in the usage of periods in the YANG descriptions. 3. The type descriptions in ietf-layer0-types are rather terse and many times just repeat part of the leaf name, e.g., "Section.". Also, optical domain abbreviations are not expanded. 4. The indentation in ietf-layer0-types is not consistent. I tried to fix. 5. When conditions starting with "Augement maximum LSP bandwidth of TE link template" and ending with "Augment unreserved bandwidth of TE link template" in ietf-layer0-types are commented out. 6. If the model ietf-wson-topology.yang is rendered, the "/* .... */" comments would be better served being in the augmentation descriptions rather than in comments. Most of augmentation descriptions currently say "WSON label.". 5. When conditions starting with "Augment label hop of underlay primary path of TE link template" and ending "Augment label restrictions end of TE link template" in ietf-wson-topology.yang are link template" and ending with "Augment unreserved bandwidth of TE link template" in ietf-layer0-types are commented out. 6. The descriptions at the module level could contain more information than just a one liner and the copyright. 7. The "Security Considerations" section in the draft doesn't follow the template in https://trac.ietf.org/trac/ops/wiki/yang-security-guidelines and doesn't enummarate the corresponding leaves their delta exposures. Nits: 1. Normally revision description start with the "Initial Version". 2. Changes references to "Yang" to "YANG". Suggested Edits: diff -c draft-ietf-ccamp-wson-yang-15.txt.orig draft-ietf-ccamp-wson-yang-15.txt *** draft-ietf-ccamp-wson-yang-15.txt.orig 2018-11-13 15:03:36.000000000 -0500 --- draft-ietf-ccamp-wson-yang-15.txt 2018-11-13 16:03:35.000000000 -0500 *************** *** 17,23 **** October 22, 2018 ! A Yang Data Model for WSON Optical Networks draft-ietf-ccamp-wson-yang-15 --- 17,23 ---- October 22, 2018 ! A YANG Data Model for WSON Optical Networks draft-ietf-ccamp-wson-yang-15 *************** *** 101,107 **** This document provides a YANG data model for the routing and wavelength assignment (RWA) Traffic Engineering (TE) topology in wavelength switched optical networks (WSONs). The YANG model ! described in this document is a WSON technology-specific Yang model based on the information model developed in [RFC7446] and the two --- 101,107 ---- This document provides a YANG data model for the routing and wavelength assignment (RWA) Traffic Engineering (TE) topology in wavelength switched optical networks (WSONs). The YANG model ! described in this document is a WSON technology-specific YANG model based on the information model developed in [RFC7446] and the two *************** *** 1213,1219 **** description "This module contains a collection of YANG definitions for ! RWA WSON. Copyright (c) 2018 IETF Trust and the persons identified as authors of the code. All rights reserved. --- 1213,1219 ---- description "This module contains a collection of YANG definitions for ! RWA WSON. Copyright (c) 2018 IETF Trust and the persons identified as authors of the code. All rights reserved. *************** *** 1237,1243 **** Internet-Draft WSON YANG Model October 2018 ! "RFC XXX: A Yang Data Model for WSON Optical Networks "; } /* --- 1237,1243 ---- Internet-Draft WSON YANG Model October 2018 ! "RFC XXX: A YANG Data Model for WSON Optical Networks "; } /* *************** *** 1275,1281 **** } leaf payload-type { type string; ! description "the payload type supported by this client tp"; reference "http://www.iana.org/assignments/gmpls-sig-parameters /gmpls-sig-parameters.xhtml"; --- 1275,1281 ---- } leaf payload-type { type string; ! description "the payload type supported by this client TP"; reference "http://www.iana.org/assignments/gmpls-sig-parameters /gmpls-sig-parameters.xhtml"; *************** *** 1292,1304 **** description ! "Indicating if it is a client-facing TP."; } } grouping wson-ttp-attributes { description ! "WSON tunnel termination point (e.g.tranponder) attributes"; leaf-list supported-operational-modes { --- 1292,1304 ---- description ! "Indicates if it is a client-facing TP."; } } grouping wson-ttp-attributes { description ! "WSON tunnel termination point (e.g., tranponder) attributes"; leaf-list supported-operational-modes { *************** *** 1348,1355 **** type boolean; description "Indicates if the TTP, or transponder, is tunable. Tunable ! transponders are assumed to be fully tunable to any of the ! 96 channels within DWDM C-band."; } leaf max-subcarrier-channel-num { --- 1348,1355 ---- type boolean; description "Indicates if the TTP, or transponder, is tunable. Tunable ! transponders are assumed to be fully tunable to any of the ! 96 channels within DWDM C-band."; } leaf max-subcarrier-channel-num { *************** *** 1359,1366 **** default 1; description "Indicate the maximum number of subcarrier channels for ! super-channel transponders. When the value equals 1 it ! represents regular single-channel transponder."; } } --- 1359,1366 ---- default 1; description "Indicate the maximum number of subcarrier channels for ! super-channel transponders. When the value equals 1 it ! represents regular single-channel transponder."; } } *************** *** 1482,1488 **** } /* Augment bandwidth path constraints of connectivity-matrices information- ! source */ augment "/nw:networks/nw:network/nw:node/tet:te/" + "tet:information-source-entry/tet:connectivity-matrices/" + "tet:path-constraints/tet:te-bandwidth/tet:technology" { --- 1482,1488 ---- } /* Augment bandwidth path constraints of connectivity-matrices information- ! source */ augment "/nw:networks/nw:network/nw:node/tet:te/" + "tet:information-source-entry/tet:connectivity-matrices/" + "tet:path-constraints/tet:te-bandwidth/tet:technology" { *************** *** 1497,1503 **** } /* Augment bandwidth path constraints of connectivity-matrix information- ! source */ augment "/nw:networks/nw:network/nw:node/tet:te/" + "tet:information-source-entry/tet:connectivity-matrices/" --- 1497,1503 ---- } /* Augment bandwidth path constraints of connectivity-matrix information- ! source */ augment "/nw:networks/nw:network/nw:node/tet:te/" + "tet:information-source-entry/tet:connectivity-matrices/" *************** *** 2185,2191 **** } /* Augment label restrictions start of connectivity-matrices information- ! source */ augment "/nw:networks/nw:network/nw:node/tet:te/" + "tet:information-source-entry/" + "tet:connectivity-matrices/tet:label-restrictions/" --- 2185,2191 ---- } /* Augment label restrictions start of connectivity-matrices information- ! source */ augment "/nw:networks/nw:network/nw:node/tet:te/" + "tet:information-source-entry/" + "tet:connectivity-matrices/tet:label-restrictions/" *************** *** 2209,2216 **** Internet-Draft WSON YANG Model October 2018 ! /* Augment label restrictions end of connectivity-matrices information- ! source */ augment "/nw:networks/nw:network/nw:node/tet:te/" + "tet:information-source-entry/" + "tet:connectivity-matrices/tet:label-restrictions/" --- 2209,2216 ---- Internet-Draft WSON YANG Model October 2018 ! /* Augment Restrictions end of connectivity-matrices information- ! source */ augment "/nw:networks/nw:network/nw:node/tet:te/" + "tet:information-source-entry/" + "tet:connectivity-matrices/tet:label-restrictions/" *************** *** 2228,2234 **** } /* Augment label hop of underlay primary path of connectivity-matrices ! information-source */ augment "/nw:networks/nw:network/nw:node/tet:te/" + "tet:information-source-entry/tet:connectivity-matrices/" + "tet:underlay/tet:primary-path/tet:path-element/tet:type/" --- 2228,2234 ---- } /* Augment label hop of underlay primary path of connectivity-matrices ! information-source */ augment "/nw:networks/nw:network/nw:node/tet:te/" + "tet:information-source-entry/tet:connectivity-matrices/" + "tet:underlay/tet:primary-path/tet:path-element/tet:type/" *************** *** 2245,2251 **** } /* Augment label hop of underlay backup path of connectivity-matrices ! information-source */ augment "/nw:networks/nw:network/nw:node/tet:te/" + "tet:information-source-entry/tet:connectivity-matrices/" + "tet:underlay/tet:backup-path/tet:path-element/tet:type/" --- 2245,2251 ---- } /* Augment label hop of underlay backup path of connectivity-matrices ! information-source */ augment "/nw:networks/nw:network/nw:node/tet:te/" + "tet:information-source-entry/tet:connectivity-matrices/" + "tet:underlay/tet:backup-path/tet:path-element/tet:type/" *************** *** 2269,2275 **** } /* Augment label hop of route-exclude of connectivity-matrices information- ! source */ augment "/nw:networks/nw:network/nw:node/tet:te/" + "tet:information-source-entry/tet:connectivity-matrices/" + "tet:optimizations/tet:algorithm/tet:metric/" --- 2269,2275 ---- } /* Augment label hop of route-exclude of connectivity-matrices information- ! source */ augment "/nw:networks/nw:network/nw:node/tet:te/" + "tet:information-source-entry/tet:connectivity-matrices/" + "tet:optimizations/tet:algorithm/tet:metric/" *************** *** 2289,2295 **** } /* Augment label hop of route-include of connectivity-matrices information- ! source */ augment "/nw:networks/nw:network/nw:node/tet:te/" + "tet:information-source-entry/tet:connectivity-matrices/" + "tet:optimizations/tet:algorithm/tet:metric/" --- 2289,2295 ---- } /* Augment label hop of route-include of connectivity-matrices information- ! source */ augment "/nw:networks/nw:network/nw:node/tet:te/" + "tet:information-source-entry/tet:connectivity-matrices/" + "tet:optimizations/tet:algorithm/tet:metric/" *************** *** 2309,2315 **** } /* Augment label hop of path-route of connectivity-matrices information- ! source */ Lee, et al. Expires August 2018 [Page 42] --- 2309,2315 ---- } /* Augment label hop of path-route of connectivity-matrices information- ! source */ Lee, et al. Expires August 2018 [Page 42] *************** *** 2334,2340 **** } /* Augment ingress label restrictions of connectivity-matrix information- ! source */ augment "/nw:networks/nw:network/nw:node/tet:te/" + "tet:information-source-entry/tet:connectivity-matrices/" + "tet:connectivity-matrix/" --- 2334,2340 ---- } /* Augment ingress label restrictions of connectivity-matrix information- ! source */ augment "/nw:networks/nw:network/nw:node/tet:te/" + "tet:information-source-entry/tet:connectivity-matrices/" + "tet:connectivity-matrix/" *************** *** 2349,2355 **** } /* Augment ingress label restrictions start of connectivity-matrix ! information-source */ augment "/nw:networks/nw:network/nw:node/tet:te/" + "tet:information-source-entry/tet:connectivity-matrices/" + "tet:connectivity-matrix/" --- 2349,2355 ---- } /* Augment ingress label restrictions start of connectivity-matrix ! information-source */ augment "/nw:networks/nw:network/nw:node/tet:te/" + "tet:information-source-entry/tet:connectivity-matrices/" + "tet:connectivity-matrix/" *************** *** 2375,2381 **** } /* Augment ingress label restrictions end of connectivity-matrix ! information-source */ augment "/nw:networks/nw:network/nw:node/tet:te/" + "tet:information-source-entry/tet:connectivity-matrices/" + "tet:connectivity-matrix/" --- 2375,2381 ---- } /* Augment ingress label restrictions end of connectivity-matrix ! information-source */ augment "/nw:networks/nw:network/nw:node/tet:te/" + "tet:information-source-entry/tet:connectivity-matrices/" + "tet:connectivity-matrix/" *************** *** 2394,2400 **** } /* Augment egress label restrictions of connectivity-matrix information- ! source */ augment "/nw:networks/nw:network/nw:node/tet:te/" + "tet:information-source-entry/tet:connectivity-matrices/" + "tet:connectivity-matrix/" --- 2394,2400 ---- } /* Augment egress label restrictions of connectivity-matrix information- ! source */ augment "/nw:networks/nw:network/nw:node/tet:te/" + "tet:information-source-entry/tet:connectivity-matrices/" + "tet:connectivity-matrix/" *************** *** 2409,2415 **** } /* Augment egress label restrictions start of connectivity-matrix ! information-source */ augment "/nw:networks/nw:network/nw:node/tet:te/" + "tet:information-source-entry/tet:connectivity-matrices/" + "tet:connectivity-matrix/" --- 2409,2415 ---- } /* Augment egress label restrictions start of connectivity-matrix ! information-source */ augment "/nw:networks/nw:network/nw:node/tet:te/" + "tet:information-source-entry/tet:connectivity-matrices/" + "tet:connectivity-matrix/" *************** *** 2434,2440 **** } /* Augment egress label restrictions end of connectivity-matrix information- ! source */ augment "/nw:networks/nw:network/nw:node/tet:te/" + "tet:information-source-entry/tet:connectivity-matrices/" + "tet:connectivity-matrix/" --- 2434,2440 ---- } /* Augment egress label restrictions end of connectivity-matrix information- ! source */ augment "/nw:networks/nw:network/nw:node/tet:te/" + "tet:information-source-entry/tet:connectivity-matrices/" + "tet:connectivity-matrix/" *************** *** 2452,2458 **** } /* Augment label hop of underlay primary path of connectivity-matrix ! information-source */ augment "/nw:networks/nw:network/nw:node/tet:te/" + "tet:information-source-entry/tet:connectivity-matrices/" + "tet:connectivity-matrix/" --- 2452,2458 ---- } /* Augment label hop of underlay primary path of connectivity-matrix ! information-source */ augment "/nw:networks/nw:network/nw:node/tet:te/" + "tet:information-source-entry/tet:connectivity-matrices/" + "tet:connectivity-matrix/" *************** *** 2470,2476 **** } /* Augment label hop of underlay backup path of connectivity-matrix ! information-source */ augment "/nw:networks/nw:network/nw:node/tet:te/" --- 2470,2476 ---- } /* Augment label hop of underlay backup path of connectivity-matrix ! information-source */ augment "/nw:networks/nw:network/nw:node/tet:te/" *************** *** 2495,2501 **** } /* Augment label hop of route-exclude of connectivity-matrix information- ! source */ augment "/nw:networks/nw:network/nw:node/tet:te/" + "tet:information-source-entry/tet:connectivity-matrices/" + "tet:connectivity-matrix/" --- 2495,2501 ---- } /* Augment label hop of route-exclude of connectivity-matrix information- ! source */ augment "/nw:networks/nw:network/nw:node/tet:te/" + "tet:information-source-entry/tet:connectivity-matrices/" + "tet:connectivity-matrix/" *************** *** 2516,2522 **** } /* Augment label hop of route-include of connectivity-matrix information- ! source */ augment "/nw:networks/nw:network/nw:node/tet:te/" + "tet:information-source-entry/tet:connectivity-matrices/" + "tet:connectivity-matrix/" --- 2516,2522 ---- } /* Augment label hop of route-include of connectivity-matrix information- ! source */ augment "/nw:networks/nw:network/nw:node/tet:te/" + "tet:information-source-entry/tet:connectivity-matrices/" + "tet:connectivity-matrix/" *************** *** 2543,2550 **** } } ! /* Augment label hop of path-route of connectivity-matrix information-source ! */ augment "/nw:networks/nw:network/nw:node/tet:te/" + "tet:information-source-entry/tet:connectivity-matrices/" + "tet:connectivity-matrix/" --- 2543,2550 ---- } } ! /* Augment label hop of path-route of connectivity-matrix information- ! source */ augment "/nw:networks/nw:network/nw:node/tet:te/" + "tet:information-source-entry/tet:connectivity-matrices/" + "tet:connectivity-matrix/" *************** *** 2618,2625 **** } } ! /* Augment label hop of underlay primary path of local-link-connectivities ! */ augment "/nw:networks/nw:network/nw:node/tet:te/" + "tet:tunnel-termination-point/" + "tet:local-link-connectivities/" --- 2618,2625 ---- } } ! /* Augment label hop of underlay primary path of local-link- ! connectivities */ augment "/nw:networks/nw:network/nw:node/tet:te/" + "tet:tunnel-termination-point/" + "tet:local-link-connectivities/" *************** *** 2643,2649 **** } ! /* Augment label hop of underlay backup path of local-link-connectivities */ augment "/nw:networks/nw:network/nw:node/tet:te/" + "tet:tunnel-termination-point/" + "tet:local-link-connectivities/" --- 2643,2650 ---- } ! /* Augment label hop of underlay backup path of local-link- ! connectivities */ augment "/nw:networks/nw:network/nw:node/tet:te/" + "tet:tunnel-termination-point/" + "tet:local-link-connectivities/" *************** *** 2783,2789 **** } /* Augment label hop of underlay primary path of local-link-connectivity ! (LLC) */ augment "/nw:networks/nw:network/nw:node/tet:te/" + "tet:tunnel-termination-point/" + "tet:local-link-connectivities/" --- 2784,2790 ---- } /* Augment label hop of underlay primary path of local-link-connectivity ! (LLC) */ augment "/nw:networks/nw:network/nw:node/tet:te/" + "tet:tunnel-termination-point/" + "tet:local-link-connectivities/" *************** *** 2809,2815 **** } /* Augment label hop of underlay backup path of local-link-connectivity ! (LLC) */ augment "/nw:networks/nw:network/nw:node/tet:te/" + "tet:tunnel-termination-point/" + "tet:local-link-connectivities/" --- 2810,2816 ---- } /* Augment label hop of underlay backup path of local-link-connectivity ! (LLC) */ augment "/nw:networks/nw:network/nw:node/tet:te/" + "tet:tunnel-termination-point/" + "tet:local-link-connectivities/" *************** *** 3190,3196 **** } units THz; description ! "The DWDM frequency in THz, e.g. 193.12500"; reference "RFC6205"; } --- 3191,3197 ---- } units THz; description ! "The DWDM frequency in THz, e.g., 193.12500"; reference "RFC6205"; } *************** *** 3201,3207 **** } units GHz; description ! "The DWDM frequency in GHz, e.g. 193125.00"; reference "RFC6205"; } --- 3202,3208 ---- } units GHz; description ! "The DWDM frequency in GHz, e.g., 193125.00"; reference "RFC6205"; } *************** *** 3276,3282 **** description "This WA method selects the wavelength that has the largest residual capacity on the most loaded ! link along the route (in muli-fiber networks)."; } identity layer0-grid-type { --- 3277,3283 ---- description "This WA method selects the wavelength that has the largest residual capacity on the most loaded ! link along the route (in multi-fiber networks)."; } identity layer0-grid-type { *************** *** 3323,3335 **** identity bw-otu1e { base layer0-bandwidth-type; description ! "OTU1e(11.04G)"; } identity bw-otu1f { base layer0-bandwidth-type; description ! "OTU1f(11.27G)"; } identity bw-otu2 { --- 3324,3336 ---- identity bw-otu1e { base layer0-bandwidth-type; description ! "OTU1e (11.04G)"; } identity bw-otu1f { base layer0-bandwidth-type; description ! "OTU1f (11.27G)"; } identity bw-otu2 { *************** *** 3538,3544 **** type frequency-thz; description "The DWDM fixed-grid channel frequency in THz, ! e.g. 193.12500"; reference "RFC6205"; } --- 3539,3545 ---- type frequency-thz; description "The DWDM fixed-grid channel frequency in THz, ! e.g., 193.12500"; reference "RFC6205"; } *************** *** 3555,3561 **** type uint32; units nm; description ! "The CWDM wavelength in nanometer, e.g. 1511"; reference "RFC6205"; } --- 3556,3562 ---- type uint32; units nm; description ! "The CWDM wavelength in nanometer, e.g., 1511"; reference "RFC6205"; } *************** *** 3573,3591 **** choice single-or-super-channel { description "single of super channel"; case single { ! leaf channel-freq { ! type frequency-thz; description "The DWDM fixed-grid channel frequency in THz, ! e.g. 193.12500"; ! } } case super { leaf-list subcarrier-channels { ! type frequency-thz; ! description ! "List of subcarrier channels for super channel."; ! } } } --- 3574,3592 ---- choice single-or-super-channel { description "single of super channel"; case single { ! leaf channel-freq { ! type frequency-thz; description "The DWDM fixed-grid channel frequency in THz, ! e.g., 193.12500"; ! } } case super { leaf-list subcarrier-channels { ! type frequency-thz; ! description ! "List of subcarrier channels for super channel."; ! } } } *************** *** 3602,3608 **** Internet-Draft WSON YANG Model October 2018 ! "The CWDM wavelength in nanometer, e.g. 1511"; reference "RFC6205"; } --- 3603,3609 ---- Internet-Draft WSON YANG Model October 2018 ! "The CWDM wavelength in nanometer, e.g., 1511"; reference "RFC6205"; } *************** *** 3630,3636 **** description "Label step information for WSON"; choice layer0-grid-type { description ! " WSON grid-type: DWDM, CWDM, etc."; case dwdm { leaf wson-dwdm { type identityref { --- 3631,3637 ---- description "Label step information for WSON"; choice layer0-grid-type { description ! "WSON grid-type: DWDM, CWDM, etc."; case dwdm { leaf wson-dwdm { type identityref { *************** *** 3712,3718 **** type frequency-thz; description "The DWDM flex-grid channel central frequency ! in THz, e.g. 193.12500."; reference "RFC7698"; } --- 3713,3719 ---- type frequency-thz; description "The DWDM flex-grid channel central frequency ! in THz, e.g., 193.12500."; reference "RFC7698"; } *************** *** 3723,3729 **** leaf central-frequency { type frequency-thz; description ! "Flex-grid central frequency in THz, e.g. 193.12500."; reference "RFC7698"; } --- 3724,3730 ---- leaf central-frequency { type frequency-thz; description ! "Flex-grid central frequency in THz, e.g., 193.12500."; reference "RFC7698"; } *************** *** 3731,3737 **** leaf slot-width { type frequency-ghz; description ! "Flex-grid The DWDM slot width in GHz, e.g. 50, 100, 150."; reference "RFC7698"; } --- 3732,3738 ---- leaf slot-width { type frequency-ghz; description ! "Flex-grid The DWDM slot width in GHz, e.g.., 50, 100, 150."; reference "RFC7698"; } *************** *** 3748,3756 **** list subcarrier-channels { key central-frequency; uses flex-grid-channel; ! description "List of subcarrier channels for flex-grid ! super channel."; Lee, et al. Expires August 2018 [Page 69] --- 3749,3757 ---- list subcarrier-channels { key central-frequency; uses flex-grid-channel; ! description "List of subcarrier channels for flex-grid ! super channel."; Lee, et al. Expires August 2018 [Page 69] *************** *** 3767,3773 **** grouping flex-grid-label-restriction { description "Flex Grid-specific label restriction."; ! uses layer0-label-restriction; container flex-grid { description "flex-grid definition"; --- 3768,3774 ---- grouping flex-grid-label-restriction { description "Flex Grid-specific label restriction."; ! uses layer0-label-restriction; container flex-grid { description "flex-grid definition";