Document: draft-ietf-ccamp-dwdm-if-param-yang Title: A YANG data model to manage configurable DWDM optical interfaces Reviewer: Dhruv Dhody Review result: Has Issues # RTGDIR review of draft-ietf-ccamp-dwdm-if-param-yang Hello, I have been selected as the Routing Directorate reviewer for this draft. The Routing Directorate seeks to review all routing or routing-related drafts as they pass through the IETF last call and IESG review, and sometimes on special request. The purpose of the review is to assist the Routing ADs. For more information about the Routing Directorate, please see https://wiki.ietf.org/en/group/rtg/RtgDir Although these comments are primarily for the use of the Routing ADs, it would be helpful if you could consider them along with any other IETF Last Call comments that you receive, and strive to resolve them through discussion or by updating the draft. Document: draft-ietf-ccamp-dwdm-if-param-yang Reviewer: Dhruv Dhody Review Date: 2025-08-11 IETF LC End Date: Unknown Intended Status: Proposed Standard ## Summary: * I have some minor concerns about this document that I think should be resolved before publication. ## Minor - Suggest adding JSON examples in the document. - Incomplete section 3.3, as indicated in the editor note - While the introduction has some description of key fields, it would be much better to include a separate section that explains the key design elements in the YANG model with YANG tree snippets. - Section 5, please describe the sensitivities that are specific to current-wdm-if-parameters (and not just state that they exist). ## YANG Model - To fix the YANG formatting, please run the command "pyang --ietf --lint -f yang --keep-comments --yang-line-length 69" - Update the description in the revision statement to "Initial revision." - There are some comments (such as "// uses wdm-if-fec-tca-thresholds;" and comment before wdm-if-tca-types) that should be removed. - Update the description for "rx-power-tca" as it uses tx in the description. - tca-name: In the description, it gives an example 'Low TX Power', is this just a human-readable string for more information, or does it have any other significance? - raise-threshold and clear-threshold are of type int32, is that suitable for all wdm-if-tca-types? I see the use of decimal for some thresholds in RFC9093bis. Should there be any checks for the values for the raise and clear threshold? For instance, what happens if raise==clear? What if only clear-threshold is set without raise-threshold? Also, I would have liked some examples where these things could be explained better. - I also suggest writing a better description statement for raise-threshold and clear-thresold. The current clear-threshold should also include clear description on how to handle when raise-threshold > clear-threshold etc. - q-margin, cur-q-factor: is int32 correct type? Should this be decimal64? - explicit-transceiver-mode-id: Add a length statement to bound the string. - In configured-mode, it is 'rw' and it is referring to mode-id, which is 'ro'; this is against the rules of RFC 7950. I am sure this would show up in yanglint. Please make sure you have run all validation tools. ```` leaf configured-mode { type union { type empty; type leafref { path "../../supported-modes/supported-mode/mode-id"; } } augment /if:interfaces/if:interface: +--rw wdm-interface +--ro supported-modes! | +--ro supported-mode* [mode-id] | +--ro mode-id string ```` ## Nits - Marking all five authors as editors is a bit unusual. It is useful to mark the person who is holding the pen as editor or none at all. - DWDM does not have a * in the RFC Editor abbreviation list and thus needs to be expanded on first use. - s/Yang/YANG/g - Remove references from Abstract. - Expand on first use: FEC, QPSK, OSNR, QAM16, TCA, BER - There are two copyright notices; please remove the one following the abstract. Update the year in the copyright of the YANG model. - Avoid using the word draft (and memo, and RFC) when referring to the I-D, suggest using 'document' so that it makes sense even after publication as RFC. - Add reference for G.698.2 - s/as a mean to/as a means to/ - s/is the way and OpenROADM compliant/is the way, an OpenROADM compliant/ Thanks! Dhruv -- last-call mailing list -- last-call@xxxxxxxx To unsubscribe send an email to last-call-leave@xxxxxxxx
