search

haomianzheng / IETF-ACTN-YANG-Model

IETF Optical YANG models in ACTN Architecture
4 stars 4 forks source link

Mahesh Jethanandani's Discuss on draft-ietf-ccamp-otn-topo-yang-18 #176

Closed italobusi closed 2 months ago

italobusi commented 4 months ago

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.

Authors: You can refer to RFC7062 for some framework description. We have added few sentences in the Introduction clarifying that the readers is assumed to be familiar with OTN and with the TE topology model, providing some references.

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.

Authors: The conformance to the NMDA architecture is written at the end of section 1. We have added a paragraph indicating that the model is using YANG 1.1

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.

Authors: According to RFC8345, a network topology can be system controlled or not. This also applies to the OTN network topology.

   In this data model, a network is categorized as either system
   controlled or not.  If a network is system controlled, then it is
   automatically populated by the server and represents dynamically
   learned information that can be read from the operational state
   datastore.  The data model can also be used to create or modify
   network topologies that might be associated with an inventory model
   or with an overlay network.  Such a network is not system controlled;
   rather, it is configured by a client.

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?

Authors: We have improved the text to support the case where the OTN network topology is not system controlled.

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.

Authors: Ok

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.

Authors: Examples added

Section 3, paragraph 0

  1. 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.

Authors: fixed

Section 4, paragraph 0

  1. The YANG Code s/YANG Code/YANG Model/

Authors: fixed

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

Authors: fixed

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.

Authors: This grouping is not intended to be re-used by other modules but it is used multiple times within this module

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

Authors: The [odu-type] is not used as a reference but only as a list key in the YANG tree.

Regarding [RFCYYYY], we have followed the common convention used in multiple drafts is to:

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

Authors: ITU-T G.709 is a normative ITU-T Recommendation so it should not be a DOWNREF.

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/

Authors: Fixed

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/

Authors: Rephrased as "Three OTN technology-specific parameters that augment the generic TE link attributes are specified."

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".

Authors: Fixed

Section 1, paragraph 7

ms used in this document are listed as follow. TS: Tributary Slot. TSG: T ^^^^^^^^^ Did you mean "as follows"?

Authors: Fixed

Section 2.2, paragraph 5

hat kind of signal can be carried as follow. The same presence container is ^^^^^^^^^ Did you mean "as follows"?

Authors: Fixed

See: https://mailarchive.ietf.org/arch/msg/ccamp/rX6IRAY29N71Ix0EcV_wjmR2kFY/

italobusi commented 2 months ago

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

The [odu-type] is not used as a reference but only as a list key in the YANG tree.

Not sure what to do with [RFCYYYY].

It seems that the common convention used in multiple drafts is to:

italobusi commented 2 months ago

See: https://mailarchive.ietf.org/arch/msg/ccamp/PS-xOlSQgWAzujvfrgkYbcdLkbU/