Jan, Thanks for taking the time to recompile and for a new review of the model. We will for sure clarify better the dependency, but hopefully RFC9093bis will get published soon replacing the old layer0-types and this will avoid any other possible misunderstanding. The module set below is correct and the module should compile error free in pyang. In case you may have any additional comment on RFC9093bis please send them to the related Yangdoctors thread in this list. Thanks Regards Roberto -----Original Message----- From: Jan Lindblad <jan.lindblad+ietf@xxxxxxx> Sent: Monday, August 4, 2025 9:44 AM To: Roberto Manzotti <manzoro.ietf@xxxxxxxxx> Cc: yang-doctors@xxxxxxxx; ccamp@xxxxxxxx; draft-ietf-ccamp-dwdm-if-param-yang.all@xxxxxxxx; last-call@xxxxxxxx Subject: RE: [CCAMP]draft-ietf-ccamp-dwdm-if-param-yang-13 ietf last call Yangdoctors review Roberto, > Thanks for the detailed review we will work on your comment but unfortunately some of them are not fully applicable. > > We may have not explicitly called out in the document and in the model description (sorry for that) but this complete model is based and dependent on the new types and grouping definitions present in the reviewed ietf-layer0-types draft that is going through the IETF LC process. > The fact that your compilation of the model failed and the many errors raised during compilation are due to this. > > I can assure that the module compile w/o error with pyang, while we are aware of a problem compiling with by yanglint on the configured-mode leaf of which we are evaluating a possible solution. > > I know you have already spent some time and worked on a very detailed review, but it would be very helpful for us if you could have a look at the model after a proper compilation using l0-types from https://datatracker.ietf.org/doc/draft-ietf-ccamp-rfc9093-bis/. This because we would like to avoid overlooking some important comments erroneously considering them related to the usage of old l0-types. Thank you for the explanation, that helps. Sorry for accusing you of using an LLM, but that is how it appeared to me. I will definitely ask you to make very clear in the module that anyone who tries to implement this module will have to get the updated l0-types module. I will not be the last to fall in that trap. The module set I am using now is below. Let me know if that differs from the set you intend me to review: - ietf-wdm-interface@xxxxxxxxxxxxxxx - ietf-layer0-types@xxxxxxxxxxxxxxx - ietf-te-types@xxxxxxxxxxxxxxx - ietf-interfaces@xxxxxxxxxxxxxxx - ietf-routing-types@xxxxxxxxxxxxxxx - ietf-yang-types@xxxxxxxxxxxxxxx I will have another go at reviewing the module then. I note that the l0-types module is quite large now, and is not following certain conventions typical for "-types" modules, so I will inevitably have to comment on that module too. This may take a bit of time. Best Regards, /jan > -----Original Message----- > From: Jan Lindblad via Datatracker noreply@xxxxxxxx > > Sent: Thursday, July 31, 2025 5:35 PM > To: yang-doctors@xxxxxxxx > Cc: ccamp@xxxxxxxx; draft-ietf-ccamp-dwdm-if-param-yang.all@xxxxxxxx; > last-call@xxxxxxxx > Subject: [CCAMP]draft-ietf-ccamp-dwdm-if-param-yang-13 ietf last call > Yangdoctors review > > 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-m > ode/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 > > > _______________________________________________ > CCAMP mailing list -- ccamp@xxxxxxxx > To unsubscribe send an email to ccamp-leave@xxxxxxxx -- last-call mailing list -- last-call@xxxxxxxx To unsubscribe send an email to last-call-leave@xxxxxxxx
