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

[Mahesh Jethanandani <[e-mail]>]

Hi Italo,

> On Jun 25, 2024, at 8:17 AM, Italo Busi <[e-mail]> wrote:
> 
> Dear Mahesh, 
> 
> Thank you for the review, the authors have updated the document to address your comments and posted the updated document as draft-ietf-ccamp-otn-topo-yang-19
> 
> See our feedbacks in line, marked as [Authors]
> 
> Thanks for the support and review. 
> 
> Authors, Haomian and Italo.
> 
>> -----Original Message-----
>> From: Mahesh Jethanandani via Datatracker <[e-mail] <mailto:[e-mail]>>
>> Sent: venerdì 26 aprile 2024 00:19
>> To: The IESG <[e-mail] <mailto:[e-mail]>>
>> Cc: [e-mail] <mailto:[e-mail]>; [e-mail] <mailto:[e-mail]>;
>> [e-mail] <mailto:[e-mail]>; [e-mail] <mailto:[e-mail]>; [e-mail] <mailto:[e-mail]>
>> Subject: Mahesh Jethanandani's Discuss on draft-ietf-ccamp-otn-topo-yang-18:
>> (with DISCUSS and COMMENT)
>> 
>> 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.
>> 
> 
> [Authors] 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. You can also refer to RFC7062 for some framework description.

Ok.

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

Thanks for adding that. 

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

Thanks for that reference. Reading RFC 8345, and in particular paragraph 3 of Section 4.4.10, it says:

   In general, a configured network topology will refer to an underlay
   topology and include layering information, such as the supporting
   node(s) underlying a node, supporting link(s) underlying a link, and
   supporting termination point(s) underlying a termination point.

You have provided the layering information for nodes, link, and termination points, but where is the reference to the underlay topology?

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

I am looking at the diffs, between -19 and -18, for the text that provides such a clarification, but do not see it. Can you point me to it?

> 
>> 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] We have updated the security considerations to follow the guidelines but since the sensitivity/vulnerability considerations for the different subtrees would be the same as those provided in section 8 of RFC8795, we think it is better to reference rather than copying that text.

I agree that the nodes defined in RFC 8795 do not need to copied here. However, the nodes that are defined in this draft, and do not exist in RFC 8795 need to have their own Security Considerations. By leaving the section blank for each of writeable/readable/actionable (RPCs) sections, I am not sure what is being implied. At the minimum, the document needs to say for each of the writeable/readable/actionable sections, that the nodes in this document have no security considerations.

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

Great. Was it validated using any of the tools, e.g. yanglint?

Thanks

> 
>> 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.
>> 
> 
> [Authors] fixed
> 
>> Section 4, paragraph 0
>>> 4.  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.
> 
> [Authors] Regarding [RFCYYYY], we have followed the common convention used in multiple drafts is to
> - reference RFC YYYY within the YANG code and add an RFC Editor Note to replace YYYY with the RFC number assigned to the corresponding draft;
> - use in the prefix table the text [RFCYYYY] and an RFC Editor Note to replace YYYY with the RFC number assigned to a draft
> 
>> 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


Mahesh Jethanandani
[e-mail]