Re: [OPSAWG] AD review of draft-ietf-opsawg-yang-vpn-service-pm-09

["Rob Wilton (rwilton)" <[e-mail]>]

Hi Bo, authors,

Please see inline. Again, I have removed sections where we have agreement.  I think that there is just one area that I’m still slightly confused by relating to the network vs service PM, for which I’ve added some further questions inline.



From: Wubo (lana) <[e-mail]>
Sent: 14 September 2022 09:25
To: Rob Wilton (rwilton) <[e-mail]>; [e-mail]
Cc: [e-mail]
Subject: 答复: AD review of draft-ietf-opsawg-yang-vpn-service-pm-09

Hi Rob,

Thank again for your deep review. Please find our response inline for the open points.

Best regards,
Bo


发件人: Rob Wilton (rwilton) [mailto:[e-mail]]
发送时间: 2022年9月13日 17:24
收件人: Wubo (lana) <[e-mail]<mailto:[e-mail]>>; [e-mail]<mailto:[e-mail]>
抄送: [e-mail]<mailto:[e-mail]>
主题: RE: AD review of draft-ietf-opsawg-yang-vpn-service-pm-09

Hi Bo,

Thanks.  I’ve made some further comments for a few points inline.  I’ve snipped those that we already have agreement on.


From: Wubo (lana) <[e-mail]<mailto:[e-mail]>>
Sent: 13 September 2022 07:38
To: Rob Wilton (rwilton) <[e-mail]<mailto:[e-mail]>>; [e-mail]<mailto:[e-mail]>
Cc: [e-mail]<mailto:[e-mail]>
Subject: 答复: AD review of draft-ietf-opsawg-yang-vpn-service-pm-09


Hi Rob,



Many thanks for your thoughtful review. Please see inline.



Thanks,



Bo



-----邮件原件-----
发件人: Rob Wilton (rwilton) [mailto:[e-mail]]
发送时间: 2022年9月9日 18:43
收件人: [e-mail]<mailto:[e-mail]>
抄送: [e-mail]<mailto:[e-mail]>
主题: AD review of draft-ietf-opsawg-yang-vpn-service-pm-09



Hi,



Here are my AD review comments for draft-ietf-opsawg-yang-vpn-service-pm-09, apologies for the delay.



I think that this document is in good shape and hence most of my comments are only minor or nits.







(11) p 8, sec 4.2.  Network Level



   For network performance monitoring, the container of "networks" in

   [RFC8345] is not extended.



I'm confused by what this sentence is meant to convey - did you mean augmented?  In particular, it isn't clear to me how you express PM for the physical (or underlay networks).  Is what you are trying to express that the "service-type" container is present for VPN service performance monitoring and absence otherwise?  Probably more words required here, and in the YANG module.



Bo: Thanks for pointing this out. Your understanding is exactly what we're trying to convey. How about we change to



As VPN Network PM YANG module includes two types of PM augmentation, the underlay networks PM is augmented on [RFC8345] when the "service-type" presence container is not defined

, and the VPN PM is augmented on [RFC8345] when the "service-type" presence container is defined.



For the underlay network performance monitoring, the container of "networks" in

   [RFC8345] is not augmented.



I think that I would still find that slightly confusing.  Perhaps:



NEW:



4.2.  Network Level



The model can be used for performance monitoring both for the network and the VPN services.



When the “service-type” presence container is absent, then it indicates

performance monitoring of the network itself.



When the “service-type” presence container is present, then it indicates

performance monitoring of the VPN service specified by the “service-type”

leaf, e.g. , L3VPN or Virtual Private LAN Service (VPLS).  The values are taken

from [RFC9181].  When a network topology instance contains the L3VPN or

other L2VPN network type, it represents a VPN instance that can perform

performance monitoring.


Bo 2: Thanks for the good suggestion. The text looks good.



One extra question:



Does this model allow you to gather PM data from both the network and L2VPN services at the same time?  If so, is there, or should there be, any text is the document that describes how to do this?


Bo2: In the current model design, the underlay network and L2VPN are separate network instances and the PM data cannot be gathered at the same time.

RW2:
Okay.  I would like to dig into this one a bit more, to understand whether this is a real limitation or not, and to ensure that I understand the model correctly:

I’m not really concerned about whether the data can be gathered at the same time (i.e., in the same request), but I would have thought that it is likely that some operators may want to do PM at both the network and overlay at the same time.

If you take the diagram in 4.1, that shows an underlay network with two VPN1 and VPN2 service overlays, then am I right to assume that they will be modelled as 3 separate list entries in the /nw:networks/nw:network/ list, one for the underlay network, and one for each of the VPN services?

If so, presumably, this means that you could gather “network PM statistics” for the underlying network list entry, separately from “service PM statistics” for each of the VPN service entries?  I.e., presumably this would mean that it is possible to enable PM on both the network underlay and service VPNs at the same time?

If what I assume above is correct then for this:

     augment /nw:networks/nw:network/nw:network-types:
       +--rw service-type!
          +--rw service-type?   identityref

I wonder why you need the service-type presence container at all?  This would only be useful if there is an intention to augment it with other extra attributes (either in a standard, vendor, or operator model) in future.  Otherwise, it would be possible to just make service-type a leaf, and having the leaf existence determine whether it represents a service VPN.  If you do want to keep the presence contain then I would suggest calling it “service” rather than “service-type” since that would arguably make more sense if it was augmented in future.

I have a somewhat similar question for this:


     augment /nw:networks/nw:network:

       +--rw vpn-pm-attributes

          +--rw vpn-id?                 vpn-common:vpn-id

          +--rw vpn-service-topology?   identityref

Is vpn-service-topology specific to it being a service? If so, then renaming it to vpn-topology and putting it under the service-type/service presence container may make more sense.



How about we make such changes:

==

4.2.  Network Level



The model can be used for performance monitoring both for the network and the VPN services. However, the module does not allow to gather the performance monitoring data simultaneously for both cases. Concretely:

* When the “service-type” presence container is absent, then it indicates

performance monitoring of the network itself.



* When the “service-type” presence container is present, then it indicates

performance monitoring of the VPN service specified by the “service-type”

leaf, e.g. , L3VPN or Virtual Private LAN Service (VPLS).  The values are taken

from [RFC9181].  When a network topology instance contains the L3VPN or

other L2VPN network type, it represents a VPN instance that can perform

performance monitoring.

==



RW2:

I think that it would be helpful to have a bit more clarity on my questions above first.





(15) p 10, sec 4.4.  Link and Termination Point Level



  The performance data of a link is a collection of counters and gauges

   that report the performance status.

  augment /nw:networks/nw:network/nt:link:

    +--rw pm-attributes

       +--rw low-percentile?            percentile

       +--rw intermediate-percentile?   percentile

       +--rw high-percentile?           percentile

       +--rw measurement-interval?      uint32

       +--ro pm* [pm-type]

       |  +--ro pm-type          identityref

       |  +--ro pm-attributes

       |     +--ro start-time?                        yang:date-and-time

       |     +--ro end-time?                          yang:date-and-time

       |     +--ro pm-source?                         identityref

       |     +--ro one-way-pm-statistics

       |     |  +--ro loss-statistics

       |     |  |  +--ro packet-loss-count?   yang:counter64

       |     |  |  +--ro loss-ratio?          percentage

       |     |  +--ro delay-statistics

       |     |  |  +--ro unit-value?                      identityref

       |     |  |  +--ro min-delay-value?                 yang:gauge64

       |     |  |  +--ro max-delay-value?                 yang:gauge64

       |     |  |  +--ro low-delay-percentile?            yang:gauge64

       |     |  |  +--ro intermediate-delay-percentile?   yang:gauge64

       |     |  |  +--ro high-delay-percentile?           yang:gauge64

       |     |  +--ro jitter-statistics

       |     |     +--ro unit-value?                       identityref

       |     |     +--ro min-jitter-value?                 yang:gauge64

       |     |     +--ro max-jitter-value?                 yang:gauge64

       |     |     +--ro low-jitter-percentile?            yang:gauge64

       |     |     +--ro intermediate-jitter-percentile?   yang:gauge64

       |     |     +--ro high-jitter-percentile?           yang:gauge64



I presume that it is intentional delay and jitter statistics can have different units, rather than always being aligned to the same units?



Bo: Agree. Will change the jitter to gauge32.



I think that my previous comment wasn’t clear enough, yang:guage64 might be okay.  My question was more about whether it is correct to have separate “unit-value” identityref values for delay-statistics independently from jitter-statistics?  I’m not saying that this is necessary wrong, but I just wanted to ensure that the authors had proactively thought about this and had consciously decided that it makes sense for delay values to be use different units from jitter values.


Bo2: Thanks for the question. On the “unit-value”, the authors agree that the same “unit-value” is sufficient for most cases. Though considering to meet the precision requirements of some scenarios, e.g. 5G cases, we think this may be useful. As such, current YANG model defines default “unit-value” as "lime:milliseconds" for both delay and jitter values.

RW2:
Okay.

And for the yang:guage64 for jitter, we think we still need to keep it as is after more thought.

RW2:
Okay.



Regards,
Rob