[CCAMP]Re: draft-ietf-ccamp-dwdm-if-param-yang-13 ietf last call Rtgdir review

[Roberto Manzotti <[e-mail]>]

Hi,

thanks for the follow up, please check in-line tagged as authors3>

Thanks
Best Regards
Roberto on behalf of co-authors

On 7/7/26 11:37, Dhruv Dhody wrote:
> Thanks!
>
> I browsed through the changes. Thanks for making them!
>
> Few things:
> - What is the value of a default statement on "config false" nodes in 
> YANG? The general practice is to limit this for configurations!
author3> we had from yangdoctor review a comment to check globally the 
optionality (mandatory true), provide default value and explain the 
behavior for missing leaf. maybe we overreacted and provided default 
also where not needed; we will recheck and remove the one not required

> - RFC8407bis is now RFC9907.
authors3> Thanks, will update all reference to the published RFC
> - I'm surprised by the claim that NO writable or readable nodes have 
> any security implications.
authros3> we followed the template of 8407bis (now RFC9907) that require 
to list writable or readable data nodes that are "particularly 
sensitive" with a related example. Of curse we have writable node that 
shall be protected with proper NACM, but checking the definition in 
RFC9907 we think that we do not have any "particularly  sensitive" 
writable or readable data node.
>
> Thanks!
> Dhruv
>
>
>
> On Tue, Jul 7, 2026 at 2:30 PM Roberto Manzotti 
> <[e-mail]> wrote:
>
>     Hello Dhruv,
>
>     thanks again for your review, we just posted revision 16 of the draft
>     that should address the last 2 open issue from your review; please
>     check
>     our reply in line below marked with authors2>>
>
>     Thanks
>     Best Regards
>     Roberto on behalf of co-authors
>
>     On 3/5/26 13:56, Roberto Manzotti wrote:
>     > Hello Dhruv,
>     >
>     > first of all thanks for your detailed review and sorry for
>     taking long
>     > to reply.
>     >
>     > We have uploaded since the review two new revision of the document:
>     > - 14: was addressing most of the Opsdir and YANG Doctor review
>     > comment, and some of your comment that was in common
>     > - 15: just uploaded to datatracker is addressing almost all of your
>     > comments with two exception that we are tracking with github
>     issue for
>     > the resolution in the next revision.
>     >
>     > Please find in-line below marked with "authors>" how we
>     addressed your
>     > comments.
>     >
>     > Thanks
>     > Regards
>     > Roberto on behalf of co-authors
>     >
>     >
>     > On 8/11/25 11:08, Dhruv Dhody via Datatracker wrote:
>     >> 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.
>     > authors> agree to add some Json example in the appendix, still
>     pending
>     > and tracked with github issue:
>     >
>     https://github.com/ietf-ccamp-wg/draft-ietf-ccamp-dwdm-if-param-yang/issues/2
>
>     >
>
>     authors2>>The original Appendix C (that now is Appendix D) has been
>     rewritten to provide an application example with a meaningful
>     scenario
>     and with a yanglint validated Json example
>
>     >>
>     >> - Incomplete section 3.3, as indicated in the editor note
>     > authors> We received the same comment also by OPSDIR, Section
>     3.3 has
>     > been fixed since rev. 14 and simplified by pointing
>     > the reader to the parameters description in the YANG model.
>     >>
>     >> - 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.
>     > authors> this is still pending and tracked by github issue:
>     >
>     https://github.com/ietf-ccamp-wg/draft-ietf-ccamp-dwdm-if-param-yang/issues/25
>
>     >
>     >
>     authors2>> We added a new Appendix C session that provide an
>     explanation
>     of the model using its tree rappresentation and explaining the key
>     design elements
>
>     >>
>     >> - Section 5, please describe the sensitivities that are specific to
>     >> current-wdm-if-parameters (and not just state that they exist).
>     > authors> actually after better reading the rfc8407bis we think that
>     > there are no sensitive writable data in the meaning described in
>     > rfc8407bis, so we replaced the text with
>     > "There are no particularly sensitive writable data nodes."
>     >>
>     >> ## YANG Model
>     >>
>     >> - To fix the YANG formatting, please run the command "pyang --ietf
>     >> --lint -f
>     >> yang --keep-comments --yang-line-length 69"
>     >> authors> fromatting fixed in rev. 15 using the suggested pyang
>     >> command and attributes
>     >> - Update the description in the revision statement to "Initial
>     >> revision."
>     > authors> Fixed in rev. 15
>     >>
>     >> - There are some comments (such as "//    uses
>     >> wdm-if-fec-tca-thresholds;" and
>     >> comment before wdm-if-tca-types) that should be removed.
>     > authors> already fixed as part of OPSDIR review in rev. 14
>     >>
>     >> - Update the description for "rx-power-tca" as it uses tx in the
>     >> description.
>     > authors> already fixed as part of OPSDIR review in rev. 14
>     >>
>     >> - 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?
>     > authors> yes it is a human readable description of the scope of the
>     > specific TCA;
>     > in rev. 15 we extended the description to better clarify and
>     provided
>     > a more meaningful example
>     >>
>     >> - 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.
>     > authors> the type has been already changed to l0-types:decimal-5
>     > in rev, 14.  In rev. 15 we also improved the definition of
>     > raise/clear-threshold:
>     > - both raise and clear are now mandatory
>     > - added conditions to avoid clear==raise
>     > - expanded both description to better clarify the behavior
>     > - Added a TCA example in the appendix
>     >>
>     >> - 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.
>     > authors> as indicated also in the previous bullet, we have improved
>     > the description of raise and clear threshold
>     >>
>     >> - q-margin, cur-q-factor: is int32 correct type? Should this be
>     >> decimal64?
>     > authors> the type of this two leaf has been changed to
>     > l0-types:decimal-2-or-unknown since rev.14
>     >>
>     >> - explicit-transceiver-mode-id: Add a length statement to bound
>     the
>     >> string.
>     > authors> adding "length 64" as part of rev. 15 update
>     >
>     authors2>> this has been changed in length "8..64" in rev. 16
>     >>
>     >> - 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
>     >> ````
>     > authors> was tracked also from YANG doctor review, it has been
>     fixed
>     > in rev.15 by following YANG doctor suggestion to use a weak
>     reference
>     > by adding "require-instance false;" to the leafref. In fact the
>     issue
>     > was showing up in yanglint, while now yanglint check is passing w/o
>     > errors.
>     >>
>     >> ## 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.
>     > authors> Fixed in rev. 15 by expanding the first usage in both the
>     > Abstract and the Introduction
>     >>
>     >> - DWDM does not have a * in the RFC Editor abbreviation list
>     and thus
>     >> needs to
>     >> be expanded on first use.
>     > authors> Fixed in rev. 15 by expanding the first usage in both the
>     > Abstract and the Introduction
>     >>
>     >> - s/Yang/YANG/g
>     > authors> same comment received also from OPSDIR, already fixed
>     in rev. 14
>     >>
>     >> - Remove references from Abstract.
>     > authors> same comment received also from OPSDIR, already fixed
>     in rev. 14
>     >>
>     >> - Expand on first use: FEC, QPSK, OSNR, QAM16, TCA, BER
>     > authors> some already fixed in rev. 14, the remaining are fixed in
>     > rev. 15 update
>     >>
>     >> - There are two copyright notices; please remove the one
>     following the
>     >> abstract. Update the year in the copyright of the YANG model.
>     > authors> Removing the first note and fixed as part of rev. 15
>     update.
>     >>
>     >> - 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.
>     > authors> fixed all instance of "draft" and "memo" usage and
>     changed to
>     > "document" as part of rev. 15 update.
>     >>
>     >> - Add reference for G.698.2
>     > authors> sorry we have not found where the reference is missing,
>     since
>     > the reference to G.698.2 is present already and was there also
>     in rev,
>     > 13.
>     > Could you share any specific instance in the document text where is
>     > missing?
>     >>
>     >> - s/as a mean to/as a means to/
>     > authors> same comment received also from OPSDIR, already fixed
>     in rev. 14
>     >>
>     >> - s/is the way and OpenROADM compliant/is the way, an OpenROADM
>     >> compliant/
>     > authors> same comment received also from OPSDIR, already fixed
>     in rev. 14
>     >>
>     >> Thanks!
>     >> Dhruv
>     >>
>     >>
>     >> _______________________________________________
>     >> CCAMP mailing list -- [e-mail]
>     >> To unsubscribe send an email to [e-mail]
>     >
>
>     _______________________________________________
>     CCAMP mailing list -- [e-mail]
>     To unsubscribe send an email to [e-mail]
>