[Last-Call] draft-ietf-ccamp-dwdm-if-param-yang-13 ietf last call Yangdoctors review

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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-mode/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


-- 
last-call mailing list -- last-call@xxxxxxxx
To unsubscribe send an email to last-call-leave@xxxxxxxx




[Index of Archives]     [IETF Annoucements]     [IETF]     [IP Storage]     [Yosemite News]     [Linux SCTP]     [Linux Newbies]     [Mhonarc]     [Fedora Users]

  Powered by Linux