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

[Dhruv Dhody <[e-mail]>]

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!
- RFC8407bis is now RFC9907.
- I'm surprised by the claim that NO writable or readable nodes have any
security implications.

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]
>