[CCAMP] Mahesh Jethanandani's Discuss on draft-ietf-ccamp-otn-topo-yang-18: (with DISCUSS and COMMENT)

[Mahesh Jethanandani via Datatracker <[e-mail]>]

Mahesh Jethanandani has entered the following ballot position for
draft-ietf-ccamp-otn-topo-yang-18: Discuss

When responding, please keep the subject line intact and reply to all
email addresses included in the To and CC lines. (Feel free to cut this
introductory paragraph, however.)


Please refer to https://www.ietf.org/about/groups/iesg/statements/handling-ballot-positions/ 
for more information about how to handle DISCUSS and COMMENT positions.


The document, along with other ballot positions, can be found here:
https://datatracker.ietf.org/doc/draft-ietf-ccamp-otn-topo-yang/



----------------------------------------------------------------------
DISCUSS:
----------------------------------------------------------------------

I am not an expert in OTN technology, so some of these DISCUSSes could be my
lack of understanding of the technology. Therefore, it will be good to have
some discussion around them.

But before that:

Section 1, paragraph 1

The document needs to be explicit whether this is a YANG 1.0 or a YANG 1.1
model. It also needs to state here, just as it has done in the YANG model
itself, whether it supports the NMDA architecture.

Section 2.2, paragraph 3
>             augment /nw:networks/nw:network/nt:link/tet:te
>                 /tet:te-link-attributes:
>           +--rw otn-link
>             +--rw odtu-flex-type?   l1-types:odtu-flex-type
>             +--rw tsg?              identityref
>             +--rw distance?         uint32

Is a user expected to specify the parameters above or this somehow learnt or
derived from a certain link attribute, e.g. odtu-flex-type? If the latter,
should these nodes not be state (ro) nodes? This DISCUSS applies to most of the
remaining rw nodes in the module, e.g., link bandwidth etc.

Section 2.2, paragraph 6
>         augment /nw:networks/nw:network/nw:node/nt:termination-point
>                   /tet:te:
>           +--rw client-svc!
>              +--rw supported-client-signal*   identityref

As an example, two paragraphs up in the draft, it says that the OTN topology
models is reporting on access link. The word "reporting" tells me that this
state (ro) data.  If it is, why are these nodes rw?

Section 6, paragraph 1

Please use the latest template from
https://www.ietf.org/archive/id/draft-ietf-netmod-rfc8407bis-11.html, Section
3.7.1. In particular, and as pointed out by the YANG doctor (thanks Radek), and
in the SECDIR review (thanks Watson), it is not enough to just say that all
writeable nodes are vulnerable without describing them using a XPath, or a
name, and describing why or how are they vulnerable.


----------------------------------------------------------------------
COMMENT:
----------------------------------------------------------------------

The remaining review is split between COMMENT and NIT

-------------------------------------------------------------------------------
COMMENT
-------------------------------------------------------------------------------
My overall comment is on the lack of instance data examples in the model. There
are several published YANG models that carry such examples, e.g. BGP YANG
model. Without such an example, it is difficult if not impossible to understand
how to use the model.

Section 3, paragraph 0
> 3.  YANG Tree for OTN topology

Please move the complete tree diagram to the Appendix or to an external site,
per the recommendation in draft-ietf-netmod-rfc8407bis, Section 3.4.

Section 4, paragraph 0
> 4.  The YANG Code
s/YANG Code/YANG Model/

Also, you need to provide a list of all the references cited in the YANG module
here. For example, you cite RFC 8345 in the YANG module below. That needs to be
called out as a reference before the YANG model with text such as "This YANG
module references A YANG Data Model for Network Topologies [RFC 8345], YANG
Data Model for Traffic Engineering [RFC 8795], ...

Section 4, paragraph 18
>      grouping label-range-info {
>        description
>          "OTN technology-specific label range related information with
>          a presence container indicating that the label range is an
>          OTN technology-specific label range.
>
>          This grouping SHOULD be used together with the
>          otn-label-start-end and otn-label-step groupings to provide
>          OTN technology-specific label information to the models which
>          use the label-restriction-info grouping defined in the module
>          ietf-te-types.";
>        uses l1-types:otn-label-range-info {
>          refine otn-label-range {
>            presence
>              "Indicates the label range is an OTN label range.
>
>              This container MUST NOT be present if there are other
>              presence containers or attributes indicating another type
>              of label range.";
>          }
>        }
>      }

I agree with the YANG Doctor (Radek), that unless there is a strong reason of
resuability of the grouping by other modules, the grouping should be removed,
and the code inlined with where it is being used.

No reference entries found for these items, which were mentioned in the text:
[RFCYYYY] and [odu-type].

Possible DOWNREF from this Standards Track doc to [ITU-T_G.709]. If so, the
IESG needs to approve it.

-------------------------------------------------------------------------------
NIT
-------------------------------------------------------------------------------

All comments below are about very minor potential issues that you may choose to
address in some way - or ignore - as you see fit. Some were flagged by
automated tools (via https://github.com/larseggert/ietf-reviewtool) so there
will likely be some false positives. There is no need to let me know what you
did with these suggestions.

Section 2.2, paragraph 1
>    There are a few characteristics augmenting to the generic TE
>    topology.

s/augmenting to the/augmenting the/

Section 2.2, paragraph 3
>    Three OTN technology-specific parameters are specified to augment the
>    generic TE link attributes.

s/specified to augment/specified that augment/

Section 1, paragraph 3
> d information pertaining to an Optical Transport Networks (OTN) electrical l
>                             ^^^^^^^^^^^^^^^^^^^^
Uncountable nouns are usually not used with an indefinite article. Use simply
"Optical Transport".

Section 1, paragraph 7
> ms used in this document are listed as follow. * TS: Tributary Slot. * TSG: T
>                                     ^^^^^^^^^
Did you mean "as follows"?

Section 2.2, paragraph 5
> hat kind of signal can be carried as follow. The same presence container is
>                                   ^^^^^^^^^
Did you mean "as follows"?