YANG Doctor LC comments #135

italobusi · 2023-05-15T16:14:33Z

YANG model updates to address YD last call comments: fix #133

Aligned YANG description with latest terminology (e2e-MC instead of NMC): fix #137

Added YANG description to actual-gain, out-voa, in-voa and loss attributes: fix #141

Updated the data type of the total-output-power attribute: fix #142

Aligned the measurement units for the PMD attributes: fix #143

Added references for imported modules: fix #146

Addressed Tom Petch comment on 1 versus one: fix #147

Pending merge of PR ietf-ccamp-wg/ietf-ccamp-layer0-types-ext-RFC9093-bis#70

Co-authored-by: sergio belotti sergio.belotti@nokia.com

ietf-optical-impairment-topology.yang

Co-authored-by: italobusi <italo.busi@huawei.com>

sergiobelotti · 2023-05-29T12:39:02Z

See the reply from YANG doctor to our proposed modifications https://mailarchive.ietf.org/arch/msg/ccamp/mOSVChPuvpWdH24xTvY_0FLKjuM/

There are just a few minor updates to do before merging

Line 657: i seems like a really bad name, I would suggest index or at least idx.
[Belotti, Sergio-Italo Busi] The usage of “empty” type in the module is described on top of the module, in the description :… ” Within this module, if the value of a mandatory attribute is unknown, it MUST be reported using the empty type. If an optional attribute is applicable but its value is unknown, it MUST be reported using the empty type.”.

MV: Okay, I suppose, but then it should be consistent, no specific
empty type explanations because that is what actually confused me. I
still think it more user-friendly to have it repeated for every leaf,
though.

[Belotti, Sergio-Italo Busi] This model is augmenting ietf- te-topology , that is also augmenting ietf-network. For this augmentation related to the type of network both are following the guideline in section 4.3 of RFC8345 that, among other things says: “….First, a new network type needs to be defined; this is done by defining a presence container that represents the new network type. The new network type is inserted, by means of augmentation, below the network-types container……”

So the augmentation with adding an empty presence container has its own meaning.
The other augmentation you mention have a different xpath , so they cannot be put together with the first augmentation.
MV: Alright, makes sense, so add a reference to the other RFC for it to
be clear to everyone.

MV: My point is that otsi has the exact same meaning or clarity as

otsi-information, in my opinion. That makes information redundant. I
cannot help with coming up with a better name as this is not my
expertise at all.

Co-authored-by: sergio belotti <sergio.belotti@nokia.com>

…MC): fix #137 Co-authored-by: sergio belotti <sergio.belotti@nokia.com>

…ext-RFC9093-bis#70 Co-authored-by: sergio belotti <sergio.belotti@nokia.com>

…butes: fix #141 Co-authored-by: sergio belotti <sergio.belotti@nokia.com>

Co-authored-by: sergio belotti <sergio.belotti@nokia.com>

…yer0-types-ext-RFC9093-bis#70 Co-authored-by: sergio belotti <sergio.belotti@nokia.com>

…ietf-ccamp-optical-impairment-topology-yang into yang-doctor

Co-authored-by: sergio belotti <sergio.belotti@nokia.com>

…ietf-ccamp-layer0-types-ext-RFC9093-bis#74 Co-authored-by: sergio belotti <sergio.belotti@nokia.com>

EstherLerouzic · 2023-07-06T07:22:04Z

ietf-optical-impairment-topology.tree

@@ -182,13 +182,13 @@ module: ietf-optical-impairment-topology
                |           |  +--ro lower-frequency    frequency-thz
                |           |  +--ro upper-frequency    frequency-thz
                |           +--ro actual-gain
-                |           |       l0-types:ratio-in-db-or-null
+                |           |       l0-types:gain-in-db-or-null
                |           +--ro tilt-target
                |           |       l0-types:decimal-2-digits-or-null


tilt target isusually a value in dB I suggest to add the unit in the yang description, and/or to change the type into ratio-in-db-or-null: because it can be positive or negative

We have removed the ratio-in-db-or-null data type from l0-types: maybe better to keep it as a decimal-2-digits-or-null type and add units "dB"

I assume the values can be positive or negative so we do not need to define any range

Addressed in 62ec336

Co-authored-by: sergio belotti sergio.belotti@nokia.com

EstherLerouzic · 2023-07-06T07:34:49Z

ietf-optical-impairment-topology.tree

          |     +--ro roadm-inband-crosstalk?
-          |     |       l0-types:ratio-in-db-or-null
+          |     |       l0-types:loss-in-db-or-null


no, inband crosstalk is not a loss, I prefer to keep ratio-in-db-or-null

We have removed the ratio-in-db-or-null data type from l0-types: maybe better to defined it as a decimal-2-digits-or-null type with units "dB"

I assume the values can be positive or negative so we do not need to define any range

hello, crosstalk is usually a positive value . if you remove the ratio-in-db, then definig decimal-2-digits is ok for me.
I think that using loss in the type for this one is too far from its physical meaning.

Addressed in 62ec336

Co-authored-by: sergio belotti sergio.belotti@nokia.com

EstherLerouzic · 2023-07-06T07:35:05Z

ietf-optical-impairment-topology.tree

          |     +--ro roadm-inband-crosstalk?
-          |     |       l0-types:ratio-in-db-or-null
+          |     |       l0-types:loss-in-db-or-null


no, inband crosstalk is not a loss, I prefer to keep ratio-in-db-or-null

@EstherLerouzic: we have removed ratio-in-db-or-null, adding
Gain-in-db
Gain-in-db-or-null
Loss-in-db
Loss-in-db-or-null.

In this case what do you recommend?

then decimal-2, and put db in the units?

Addressed in 62ec336

Co-authored-by: sergio belotti sergio.belotti@nokia.com

EstherLerouzic · 2023-07-06T07:35:14Z

ietf-optical-impairment-topology.tree

                +--ro roadm-inband-crosstalk?
-                |       l0-types:ratio-in-db-or-null
+                |       l0-types:loss-in-db-or-null


no, inband crosstalk is not a loss, I prefer to keep ratio-in-db-or-null

The same as above

same comment

Addressed in 62ec336

Co-authored-by: sergio belotti sergio.belotti@nokia.com

EstherLerouzic · 2023-07-06T07:37:09Z

ietf-optical-impairment-topology.yang

      description "Polarization Dependent Loss (PDL)";
    }      
    leaf roadm-inband-crosstalk {
-      type l0-types:ratio-in-db-or-null;
+      type l0-types:loss-in-db-or-null;


no, inband crosstalk is not a loss, I prefer to keep ratio-in-db-or-null

The same as above

same comment

Addressed in 62ec336

Co-authored-by: sergio belotti sergio.belotti@nokia.com

EstherLerouzic · 2023-07-06T07:46:54Z

ietf-optical-impairment-topology.tree

-    |        |              |  +--ro penalty-value
-    |        |              |          union
+    |        |              +--ro max-pdl-penalty* []
+    |        |              |  +--ro max-pdl-value


I'd like to remove the max here because it is misleading (and not similar to pmd, cd penalties):
and we miss a max-pdl attribute:
| | +--ro max-polarization-dependant-loss ? loss-in-db-or-null
| | +--ro pdl-penalty* []
| | | +--ro pdl-value loss-in-db-or-null
| | | +--ro penalty-value union

Addressed in ietf-ccamp-wg/ietf-ccamp-layer0-types-ext-RFC9093-bis@0038f5e and in ietf-ccamp-wg/ietf-ccamp-layer0-types-ext-RFC9093-bis@54a7c33

Co-authored-by: sergio belotti sergio.belotti@nokia.com

EstherLerouzic · 2023-07-06T07:47:30Z

ietf-optical-impairment-topology.tree

@@ -99,7 +94,7 @@ module: ietf-optical-impairment-topology
    |        |              +--ro out-of-band-osnr?
    |        |              |       snr
    |        |              +--ro tx-polarization-power-difference?
-    |        |              |       power-in-db
+    |        |              |       power-in-dbm


if its a power difference it is in db

We have removed the power-in-db data type from l0-types: maybe better to defined it as a decimal-2-digits-or-null type with units "dB"

I assume the values can be positive or negative so we do not need to define any range

yes decimal-2-digits-or-null type with units "dB" works fine

Addressed in ietf-ccamp-wg/ietf-ccamp-layer0-types-ext-RFC9093-bis@0038f5e

Co-authored-by: sergio belotti sergio.belotti@nokia.com

ju7ien · 2023-07-06T14:56:18Z

ietf-optical-impairment-topology.tree

    |        +--ro configured-mode?               union
+    |        +--ro line-coding-bitrate?           identityref
    |        +--ro tx-channel-power?              union
    |        +--ro rx-channel-power?              union
    |        +--ro rx-total-power?                union


tx-channel-power, rx-channel-power, rx-total-power should be of type power-in-dbm-or-null

Addressed in ietf-ccamp-wg/ietf-ccamp-layer0-types-ext-RFC9093-bis@54a7c33

Co-authored-by: sergio belotti sergio.belotti@nokia.com

ju7ien

Some in-dB/dBm leaf types to be adjusted.

Co-authored-by: sergio belotti <sergio.belotti@nokia.com>

…l-impairment-topology-yang#135 Co-authored-by: sergio belotti <sergio.belotti@nokia.com>

italobusi · 2023-07-07T09:40:18Z

Some in-dB/dBm leaf types to be adjusted.

Addressed in 62ec336

Co-authored-by: sergio belotti sergio.belotti@nokia.com

EstherLerouzic

looks good to me!
thanks !

…ssues #139 and #141 addressed This I-D update is addressing PR #135 and Open Issues #139 and #141, - major changes: - latest YANG model tree view and YANG code updated based on PR #135 - Reference to RFC8776 replaced by I-D.ietf-teas-rfc8776-update ( #139 ) - Paragraph added to the Optical Amplifier section addressing #141 - some pure editorial changes

* I-D Text Proposal for Open Issues #132 and #136 This I-D update provides text proposals addressing: - #132 - #136 + a few pure editorial changes * I-D Update: Note added addressing comment on Open Issues #132 from 2023-06-20 This I-D update is addressing: - the latest comment on #132 describing the agreement reached during the weekly call on 2023-06-20: #132 (comment) * I-D Update: addressing PR #138 review comments This I-D update is addressing PR #138 review comments - major change: - Some Figures illustrating protection architectures were updated showing the two directions separately. - The text was aligned with the updated figures - The note added added by the previous commit was removed (see 3ceaa84) - Table entry for RFC9093 was removed (RFC9093bis is needed for the YANG model in this I-D) * I-D Update: latest YANG model update incorporated ( PR #135 ), Open Issues #139 and #141 addressed This I-D update is addressing PR #135 and Open Issues #139 and #141, - major changes: - latest YANG model tree view and YANG code updated based on PR #135 - Reference to RFC8776 replaced by I-D.ietf-teas-rfc8776-update ( #139 ) - Paragraph added to the Optical Amplifier section addressing #141 - some pure editorial changes

italobusi added 3 commits May 15, 2023 17:55

Updated YANG file to address YD last call: fix #133

9bd5f2e

YANG model updates to address YD last call comments: fix #130

a512a3d

Normalize all the line endings

a88b90f

sergiobelotti requested review from EstherLerouzic and ju7ien May 16, 2023 09:40

italobusi commented May 23, 2023

View reviewed changes

ietf-optical-impairment-topology.yang Outdated Show resolved Hide resolved

Update ietf-optical-impairment-topology.yang

9e914d7

Co-authored-by: italobusi <italo.busi@huawei.com>

italobusi and others added 6 commits June 23, 2023 10:22

Addressed YD replies: #135 (comment)

52ea15a

Co-authored-by: sergio belotti <sergio.belotti@nokia.com>

Aligned YANG description with latest terminology (e2e-MC instead of N…

f87b6b4

…MC): fix #137 Co-authored-by: sergio belotti <sergio.belotti@nokia.com>

Aligned with latest changes in ietf-ccamp-wg/ietf-ccamp-layer0-types-…

97be1f9

…ext-RFC9093-bis#70 Co-authored-by: sergio belotti <sergio.belotti@nokia.com>

Added YANG description to actual-gain, out-voa, in-voa and loss attri…

0361dd4

…butes: fix #141 Co-authored-by: sergio belotti <sergio.belotti@nokia.com>

Updated the data type of the total-output-power attribute: fix #142

d6f4757

Co-authored-by: sergio belotti <sergio.belotti@nokia.com>

Aligned the measurement units for the PMD attributes: fix #143

7e145d1

Co-authored-by: sergio belotti <sergio.belotti@nokia.com>

italobusi mentioned this pull request Jul 3, 2023

Tom Petch all comments #144

Open

9 tasks

italobusi and others added 6 commits July 3, 2023 12:03

Added references for imported modules: see #144

154edd1

Co-authored-by: sergio belotti <sergio.belotti@nokia.com>

Create minute-2023-06-27.md

346580f

Aligned with the latest l0-types model in ietf-ccamp-wg/ietf-ccamp-la…

9500b7f

…yer0-types-ext-RFC9093-bis#70 Co-authored-by: sergio belotti <sergio.belotti@nokia.com>

Merge branch 'yang-doctor' of https://github.com/ietf-ccamp-wg/draft-…

26a00d8

…ietf-ccamp-optical-impairment-topology-yang into yang-doctor

Addressed Tom Petch comment on 1 versus one: fix #147

3cbc41b

Co-authored-by: sergio belotti <sergio.belotti@nokia.com>

Aligned with new names for power-in-db data types: see ietf-ccamp-wg/…

0302432

…ietf-ccamp-layer0-types-ext-RFC9093-bis#74 Co-authored-by: sergio belotti <sergio.belotti@nokia.com>

sergiobelotti requested a review from egriseri July 5, 2023 16:16

EstherLerouzic reviewed Jul 6, 2023

View reviewed changes

ju7ien reviewed Jul 6, 2023

View reviewed changes

ju7ien requested changes Jul 6, 2023

View reviewed changes

Addressed PR review comments

62ec336

Co-authored-by: sergio belotti <sergio.belotti@nokia.com>

italobusi requested review from EstherLerouzic and ju7ien July 7, 2023 09:40

EstherLerouzic approved these changes Jul 7, 2023

View reviewed changes

ju7ien approved these changes Jul 7, 2023

View reviewed changes

Delete minutes-2022-03-08.md

e1ec49c

italobusi merged commit 58cef8c into master Jul 7, 2023
2 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

YANG Doctor LC comments #135

YANG Doctor LC comments #135

italobusi commented May 15, 2023 •

edited

sergiobelotti commented May 29, 2023 •

edited

EstherLerouzic Jul 6, 2023

italobusi Jul 6, 2023 •

edited

italobusi Jul 7, 2023

EstherLerouzic Jul 6, 2023

italobusi Jul 6, 2023 •

edited

EstherLerouzic Jul 6, 2023

italobusi Jul 7, 2023

EstherLerouzic Jul 6, 2023

sergiobelotti Jul 6, 2023

EstherLerouzic Jul 6, 2023

italobusi Jul 7, 2023

EstherLerouzic Jul 6, 2023

sergiobelotti Jul 6, 2023

EstherLerouzic Jul 6, 2023

italobusi Jul 7, 2023

EstherLerouzic Jul 6, 2023

sergiobelotti Jul 6, 2023

EstherLerouzic Jul 6, 2023

italobusi Jul 7, 2023

EstherLerouzic Jul 6, 2023

italobusi Jul 7, 2023

EstherLerouzic Jul 6, 2023

italobusi Jul 6, 2023

EstherLerouzic Jul 6, 2023

italobusi Jul 7, 2023

ju7ien Jul 6, 2023

italobusi Jul 7, 2023

ju7ien left a comment

italobusi commented Jul 7, 2023

EstherLerouzic left a comment

YANG Doctor LC comments #135

YANG Doctor LC comments #135

Conversation

italobusi commented May 15, 2023 • edited

sergiobelotti commented May 29, 2023 • edited

Choose a reason for hiding this comment

italobusi Jul 6, 2023 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

italobusi Jul 6, 2023 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ju7ien left a comment

Choose a reason for hiding this comment

italobusi commented Jul 7, 2023

EstherLerouzic left a comment

Choose a reason for hiding this comment

italobusi commented May 15, 2023 •

edited

sergiobelotti commented May 29, 2023 •

edited

italobusi Jul 6, 2023 •

edited

italobusi Jul 6, 2023 •

edited