Roberto, WG, Sorry for the long delay -- I have been traveling. Yesterday I finally managed to submit an updated YANG Doctor LC review of draft-ietf-ccamp-dwdm-if-param-yang, which by necessity also covers draft-ietf-ccamp-rfc9093-bis. Things are a lot better now that I have the correct set of dependencies, but there are still a few blocking problems in both wdm-interface and layer0-types. I'm not sure any automatic mail went out with the link to the updated review, so here it is. The link will take you to the updated review (2025-08-23) despite the old date embedded in the URL. https://datatracker.ietf.org/doc/review-ietf-ccamp-dwdm-if-param-yang-13-yangdoctors-lc-lindblad-2025-07-31/ Best Regards, /Jan On Monday, August 4th, 2025 at 11:07, Roberto Manzotti <manzoro.ietf@xxxxxxxxx> wrote: > 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
