-
Notifications
You must be signed in to change notification settings - Fork 10
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
YANG model for ROADM optical impairments #28
Conversation
@sergiobelotti , I have compiled with pyang and a today's import of layer0-type yang model https://github.com/YangModels/yang/blob/master/experimental/ietf-extracted-YANG-modules/ietf-layer0-types%402019-11-28.yang:
standard-mode has been suppressed in this commit: YangModels/yang@f278c2f and probably replaced with operational-mode vendor-identifier has been suppressed in this commit YangModels/yang@d7c356d (which I find stange because operatinal-mode description in my opinion is not sufficient to ensure that tsp from different vendors are compatible/interoperable ...) I read the detail of your changes a give you more details regards |
@@ -248,15 +245,15 @@ module ietf-optical-impairment-topology { | |||
config false; | |||
description "Identifier of the carrier"; | |||
} | |||
} | |||
} | |||
} | |||
|
|||
grouping optical-fiber-data { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this grouping is not used
"sigma in the Gausian Noise Model"; | ||
} | ||
} | ||
|
||
|
||
grouping optical-channel-data { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this grouping is not used
units "ps/nm/km"; | ||
description "Cromatic Dispersion"; | ||
} | ||
leaf pdl { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If would suggest to use the same wording as roadm-xxx -> roadm-pdl
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
fraction-digits 5; | ||
} | ||
units "ps/nm/km"; | ||
description "Cromatic Dispersion"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Chromatic
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thanks, done
type decimal64 { | ||
fraction-digits 5; | ||
} | ||
units "ps/nm/km"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this one is in ps/nm not per km
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok
fraction-digits 8; | ||
range "0..max"; | ||
} | ||
units "ps/(km)^0.5"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this one should be in ps (not per km^1/2)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
type decimal64 { | ||
fraction-digits 2; | ||
} | ||
units db ; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
dB
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
type decimal64 { | ||
fraction-digits 2; | ||
} | ||
units db; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
dB
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
Hi Sergio, All, Thanks for the information, the previous email from Esther did not come to me. I added Esther in this loop as well. I tracked the changes in previous ietf-layer0-types versions, and share the following findings:
We understand the concern you have, and agree that these parameters are probably useful in the impairment models. Actually Italo and I have some future consideration about the versioning of ietf-layer0-types, which has been mentioned in the side meeting but may need discussion as well. Hopefully such explanation helps proceeding the work, please feel free to comments, thank you. BTW, is this issue open on the ccamp-wg github? I did not find #28 so cannot follow up in public way. Best wishes, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hope this helps!
I will try to build a json instance out of this and see if we miss anything
thanks a lot!
regards
with the inputs either routed to different output ports, | ||
or all but 1 blocked"; | ||
} | ||
leaf maxloss-exp { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same comment on unit db and naming roadm-maxloss-exp
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
fraction-digits 8; | ||
range "0..max"; | ||
} | ||
units "ps/(km)^0.5"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
unit in ps (same comment)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok done
type decimal64 { | ||
fraction-digits 5; | ||
} | ||
units "ps/nm/km"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
in ps/nm
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
units "ps/nm/km"; | ||
description "Cromatic Dispersion"; | ||
} | ||
leaf pdl { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
name roadm-pdl
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
fraction-digits 5; | ||
} | ||
units "ps/nm/km"; | ||
description "Cromatic Dispersion"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Chromatic
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
type decimal64 { | ||
fraction-digits 2; | ||
} | ||
units dB ; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
dBm
} | ||
} | ||
|
||
grouping roadm-drop-path { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same comments as for add
"This augment is only valid for Optical Impairment topology"; | ||
} | ||
description | ||
"node attributes augmentantion for optical-impairment RAODM node"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
augmentantion - > augmentation
RAODM -> ROADM
description | ||
"node attributes augmentantion for optical-impairment RAODM node"; | ||
|
||
list path-impairment { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
change to roadm-path-impairment ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I originally put exactly this name, but I received one comment realted to the fact,, in the choice you already have the mention of "roadm" (roadm-express-path, roadm-add roadm.drop) so no need still to highlight roadm word.
The simple way to proceed for the moment is to introduce definitions directly into ietf-optical-impairment-topology module like this: typedef operational-mode { typedef standard-mode { typedef vendor-identifier { If agreed I can proceed in this way. |
I agree with this ! |
Here is the json template created out of the yang |
added ROADM power optical impairments description
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is a good starting point and ready to be merge and used for the next I-D update
@sergiobelotti , @EstherLerouzic : I have just discovered that there is another option to resolve the layer0-types versioning issue, without the need to copy the definitions which have been removed:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here is my rapid review
"list of available baud-rates. Baud-rate is the unit for | ||
symbol rate or modulation rate in symbols per second or | ||
pulses per second. It is the number of distinct symbol | ||
changes (signaling events) made to the transmission medium |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
signal instead of signaling
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok
"String identifier of fiber type referencing a specification in | ||
a separate equipment catalog"; | ||
description "String identifier of fiber type referencing a specification in a | ||
separate equipment catalog"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
indent on 586
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok
with the inputs either routed to different output ports, | ||
or all but 1 blocked"; | ||
} | ||
leaf roadm-maxloss-exp { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
-exp is maybe too short (could be understoop as expecte as this word is also on the description
-> change to -expr ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
deleted exp. We have malloss in all, express, add and drop roadm path, and the context is defined by the container.
with the inputs either routed to different output ports, | ||
or all but 1 blocked. | ||
In the case of add path it is the total of the add block | ||
+ the egress amp crosstalk contributions."; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- egress WSS crosstalk ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
correct. Thanks to suggest.
assuming no additional add path loss is added. | ||
This is used to establish the minimum required transponder output power required | ||
to hit the ROADM egress target power levels and preventing to hit the WSS attenuation limits. | ||
If the add path contains an internal amplifier,this loss value should be based |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
blank after the comma
final "." on next line
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
not sure to have got the problem. Can you elaborate it?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is my understanding:
If the add path contains an internal amplifier, this loss value should be based
on worst case expected amplifier gain due to ripple or gain uncertainty.";
drop path amplifier,or simply,to reduce the power of a “strong” carrier(due to ripple,for example), | ||
then the use of the ROADM input power levels and the above drop losses is not appropriate. | ||
This parameter corresponds to the worst case per carrier power levels expected | ||
at the output of the drop block.To be noted that the net power levels from the drop block consider |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the sentence 'To be noted that ...' is a bit obscure when there is not the exact computation with it : maybe add that: a detail example of the comparison using these parameters is detailed in section xxx of the document xxx
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok. We will add correct referecne when a related text will be added in the draft, ok?
then the use of the ROADM input power levels and the above drop losses is not appropriate. | ||
This parameter corresponds to the best case per carrier power levels expected | ||
at the output of the drop block.To be noted that the net power levels from the drop block consider | ||
these power levels,and the power levels calculated with the drop loss attributes,if both are defined. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same comment:
the sentence 'To be noted that ...' is a bit obscure when there is not the exact computation with it : maybe add that: a detail example of the comparison using these parameters is detailed in section xxx of the document xxx
blank after ','
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok, as above
drop path amplifier,or simply,to reduce the power of a “strong” carrier(due to ripple,for example), | ||
then the use of the ROADM input power levels and the above drop losses is not appropriate. | ||
This parameter corresponds to the typical case per carrier power levels expected | ||
at the output of the drop block.To be noted that the net power levels from the drop block consider |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the sentence 'To be noted ... is different for ptyp. in Colin slides the ptyp determines the power of the group of carriers, if several carriers are extrated on the same drop path (drop block)... not clear to me if it is for the pmaxxnb_carriers or Pminxnb_carriers ... Maybe remove the sentence a say that a detailed description is for further work ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've removed the sentence. We will check further with Colin.
"Optical Signal-to-Noise Ratio (OSNR). | ||
Expected OSNR contribution of the drop path amplifier(if present) for the case of additional | ||
drop path loss (before this amplifier) in order to hit a target power level (per carrier). | ||
If both, the OSNR based on the ROADM input power level(Pcarrier=Pref+10Log(carrier_baudrate/ref_baud)+delta_power) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
long line: add spaces around operators ' + '
change worst to minimum value
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok
description | ||
"Drop path Noise Figure. | ||
If the drop path contains an amplifier,this is the noise figure of that amplifier, | ||
inferred to the ROADM input port.This permits to determine amplifier OSNR contribution |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
input port -> ingress port
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I will still provide minor comments later.
description | ||
"List of modulation types the OTSi supports"; | ||
description | ||
"List determining all the available modulations"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why has this been changed? Please undo.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok
leaf-list available-modulation-types { | ||
|
||
leaf-list available-modulation { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why has this been changed? Please undo.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok
} | ||
|
||
leaf configured-modulation-type { | ||
leaf modulation-type { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why has this been changed? Please undo.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok
description | ||
"Currently configured OTSi modulation type"; | ||
description | ||
"Modulation configured for the transponder"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why has this been changed? Please undo.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok
@@ -198,23 +230,23 @@ module ietf-optical-impairment-topology { | |||
description "configured baud-rate"; | |||
} | |||
|
|||
leaf-list available-FEC-types { | |||
leaf-list available-FEC { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why has this been changed? Please undo.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok
type identityref { | ||
base FEC; | ||
} | ||
config false; | ||
description "List determining all the available FEC"; | ||
} | ||
|
||
leaf configured-FEC-type { | ||
leaf FEC-type { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why has this been changed? Please undo.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok
} | ||
|
||
grouping sliceable-transponder-attributes { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The term "sliceable-transponder" is very unclear. The description does not provide more information what a sliceable transponder is. Therefore, a better term shall be used.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you propose something? Suggestion feature can be used.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we need more work on the transponder model (I have just created an open issue #29 )
For this version we either keep the current name or change it to transponder-attributes
If you change the grouping name you have also to change the code where you use this grouping
@@ -81,3 +81,52 @@ module: ietf-optical-impairment-topology | |||
augment /nw:networks/nw:network/nw:node/tet:te/tet:tunnel-termination-point: | |||
+--ro transponder-list* [carrier-id] | |||
+--ro carrier-id uint32 | |||
augment /nw:networks/nw:network/nw:node/tet:te/tet:te-node-attributes: | |||
+--ro path-impairment* [path-impairment-id] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As discussed: replace "path-impairment*" with "roadm-path-impairments*
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok
Co-Authored-By: italobusi <italo.busi@huawei.com>
@@ -55,66 +55,67 @@ module ietf-optical-impairment-topology { | |||
Provisions Relating to IETF Documents | |||
(http://trustee.ietf.org/license-info)."; | |||
|
|||
revision 2019-05-30 { | |||
revision 2020-03-03 { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shall we use 2020-03-09 as revision date? This will be the submission date of the draft.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes, modified with new commit
Modified version date
Adding modifications to the YANG model to introduce the attributes for optical impairments for the ROADM node.