Document: draft-ietf-ccamp-dwdm-if-param-yang Title: A YANG data model to manage configurable DWDM optical interfaces Reviewer: Jan Lindblad Review result: Not Ready This is my early YANG-Doctor review of draft-ietf-ccamp-dwdm-if-param-yang-13. This document has come a long way in the number of revisions. Counting the full revision history, there are 25 prior revisions. Despite the long history, the request is for an early review, which means problems are expected. That is also what I find. From a YANG perspective, I declare that the document is Not ready. Judging by the ample confabulation that is visible in the module, I expect this module has been created by an LLM with a rather weak understanding of the context. The resulting module does not compile, and it's not always easy to guess the authors' modeling intent. #1: Key concept supported-modes never defined In section 1 of the draft, it is stated that: "The key concept introduced by this YANG model in accordance with I-D [...] is the notion of a mode." and "To advertise the capability supported by an interface, a list of transceiver modes is provided by the device for each dwdm coherent module (supported-modes)." The intent to define a list of supported-modes is apparent also from the tree diagram in Appendix C, but the module does not actually have any definition of supported-modes. augment /if:interfaces/if:interface: +--rw wdm-interface +--ro supported-modes! | +--ro supported-mode* [mode-id] | +--ro mode-id string | +--ro (mode) | +--:(G.698.2) | | +--ro standard-mode? standard-mode #2: Incorrect pointer to template explicit-transceiver-mode Deep down under the interface configuration (/if:interfaces/if:interface/wdm-interface/supported-modes/supported-mode/mode/explicit-mode/explicit-mode), there is a leaf (explicit-transceiver-mode-ref) that is presumably meant to reference which mode, out of a list of available templates, the interface is configured to use. I don't quite understand the relation between the list of supported modes under each interface (/if:interfaces/if:interface/wdm-interface/supported-modes), and the list of templates common to all interfaces (/if:interfaces/wdm-if-templates), but in any case the explicit-transceiver-mode-ref is config true, and the list of templates is config false. Such config references to operational state are strictly forbidden in YANG. The design around this situation needs a thorough check. I can explain a bit more if you wish. leaf explicit-transceiver-mode-ref { type leafref { path "../../../../../../wdm-if:wdm-if-templates" + "/wdm-if:explicit-transceiver-modes" + "/wdm-if:explicit-transceiver-mode" + "/wdm-if:explicit-transceiver-mode-id"; } description "The reference to the explicit transceiver mode template."; } #3: Confabulated types and groupings As noted above, the YANG module does not compile. This is mainly due to the LLM confabulating convenient types in ietf-layer0-types (RFC 9093) that simply are not there. ietf-wdm-interface.yang:159: error: type "snr-or-null" not found in module "ietf-layer0-types" ietf-wdm-interface.yang:171: error: type "frequency-thz" not found in module "ietf-layer0-types" ietf-wdm-interface.yang:182: error: type "snr" not found in module "ietf-layer0-types" ietf-wdm-interface.yang:247: error: grouping "explicit-mode" not found in module "ietf-layer0-types" ietf-wdm-interface.yang:261: error: grouping "transceiver-capabilities" not found in module "ietf-layer0-types" ietf-wdm-interface.yang:263: error: node ietf-wdm-interface::supported-modes is not found ietf-wdm-interface.yang:299: error: grouping "common-transceiver-param" not found in module "ietf-layer0-types" Discussing them one by one: Line 159: leaf min-osnr-margin { type l0-types:snr-or-null; units "dB"; config false; description "OSNR margin to FEC threshold"; } Maybe declaring an snr type in your own module and using that would be a good solution? The -or-null aspect might be covered by the server simply not returning this leaf if it is null? Line 171: leaf central-frequency { type l0-types:frequency-thz; description "This parameter indicates the interface Central Frequency"; } Maybe defining a frequency type (or frequency-thz) in your own module and using that would be a good solution? If it is required to report, consider declaring it mandatory true; Line 182: leaf cur-osnr { type l0-types:snr; units "dB"; config false; description "OSNR margin to FEC threshold"; } If you declare an snr type (solution to line 159), that type could go here as well? Line 247: uses l0-types:explicit-mode; The grouping explicit-mode seems rather central to the interpretation of this module, yet there is no definition as to what it might contain. Provide a grouping in your own module with the relevant contents. Line 261, 263: container wdm-interface { description "Container for capabilities, configuration, current operational data for a DWDM interface"; uses l0-types:transceiver-capabilities { augment "supported-modes/supported-mode/mode/" + "explicit-mode/explicit-mode" { description "Augment the explicit-mode container with the proper leafref."; This construct assumes that there is a grouping called transceiver-capabilities in l0-types (there isn't), and that this structure defines supported-modes, supported-mode, mode, explicit-mode, explicit-mode (sic! explicit-mode occurs twice in the path). Define the transceiver-capabilities grouping in your own module, and fill it with the relevant contents, e.g. include a definition of supported-modes in there (assuming the design should have both supported-modes and templates). Line 299: uses l0-types:common-transceiver-param; There is no such grouping in l0-types. Define your own grouping with the relevant contents. If there are no common-tranceiver-params, you can delete this line. #4: Enumeration of open ended type // Need to verify if the enumeration is the proper approach for // the tca-types, since the list is not fully standardized adn // subject to change. typedef wdm-if-tca-types { type enumeration { enum laser-linewdt-tca { Good catch! Enumerations work poorly when the complete set of available values is not known at modeling time. A better approach is to use YANG identities. This allows adding and subclassing valid values over time. identity wdm-if-tca-identity; identity laser-linewdt-tca { base wdm-if-tca-identity; } And to reference these values you would use leaf my-wdm-if-tca { type identityref { base wdm-if-tca-identity; } } #5: Possibly unintended numeric type This is the modeling of leaf pre-fec-ber: leaf pre-fec-ber { type decimal64 { fraction-digits 18; } config false; description "Pre-FEC errored words"; } I don't know exactly what the typical way to represent "Pre-FEC errored words" is like, but as currently modeled, this will be a number between 0 and 1 with 18 decimals of precision, e.g. 0.123456789123456789. Is that the intent? #6: Unclear semantics for missing value The leaf configured-mode can take a reference to a supported-mode (not defined, as noted earlier), or it can be a type empty leaf. A type empty leaf is something similar to a boolean, it can be in one of two states, "exists" or "not exists". container current-wdm-if-parameters { leaf configured-mode { type union { type empty; type leafref { path "../../supported-modes/supported-mode/mode-id"; } } description "Reference to the configured mode for transceiver compatibility approach. The empty value is used to report that no mode has been configured and there is no default mode. When not present, the configured-mode is not reported by the server."; } If the modeling intent is to say that configured-mode could either be a reference to a supported-mode or not exist, you could remove the union and type empty, and just keep the leafref type. Since you are not declaring this leaf mandatory, it could simply not exists if it doesn't have any value. The description statement does not clarify the expected use of this leaf much. #7: Names There are several names of leafs/containers etc. in this module that are not conventional. E.g. you would typically not name a list in YANG "xxx-list". You would typically not have "explicit-mode" inside "explicit-mode". It would be nice if augmented container names explained which technology they are related to. E.g. /if:interfaces/if:interface/supported-modes might be better named "wdm-supported-modes", as many other technologies associated with interfaces might have modes as well. Also note that YANG and NETCONF should be referenced in upper case. Best Regards, /jan -- last-call mailing list -- last-call@xxxxxxxx To unsubscribe send an email to last-call-leave@xxxxxxxx