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

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

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]>
Sent: 13 September 2022 07:38
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,



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.





Minor level comments:



(1) p 0, sec



   The data model for network topologies defined in RFC 8345 introduces

   vertical layering relationships between networks that can be

   augmented to cover network and service topologies.  This document

   defines a YANG module for performance monitoring (PM) of both

  networks and VPN services that can be used to monitor and manage

   network performance on the topology at higher layer or the service

   topology between VPN sites.



"the topology at higher layer" doesn't scan particularly well to me, please can you tweak it.



Bo: Thanks for pointing this out. Is this better that we simply change to “the underlay topology”?



Yes, perhaps something like this:



NEW:

   The data model for network topologies defined in RFC 8345 introduces

   vertical layering relationships between networks that can be

   augmented to cover network and service topologies.  This document

   defines a YANG module for performance monitoring (PM) of both

   underlay networks and overlay VPN services that can be used to monitor

  and manage network performance on the topology of both layers.







(3) p 4, sec 3.  Network and VPN Service Performance Monitoring Model Usage



   As shown in Figure 1, in the context of the layering model

   architecture described in [RFC8309], the network and VPN service

   performance monitoring (PM) model can be used to expose a set of

   performance information to the above layer.  Such information can be

   used by an orchestrator to subscribe to performance data.



Perhaps rephase?  I.e., is it the performance data that is being used to create a subscription based on the performance data, or is it just that the model makes the performance data readily available, which can then be subscribed do?



Bo: Thanks for the suggestion. How about:

The model makes the performance data readily available, which can then be subscribed by the client application, such as an orchestrator.



I think that you can probably get away with deleting the last 2 sentences of that paragraph and rewording it slightly.  The document already talks more about the specifics in sections 3.1 and 3.2 anyway.  Hence, I propose:



OLD:



   As shown in Figure 1, in the context of the layering model

   architecture described in [RFC8309], the network and VPN service

   performance monitoring (PM) model can be used to expose a set of

   performance information to the above layer.  Such information can be

   used by an orchestrator to subscribe to performance data.  The

   network controller will then notify the orchestrator about

   corresponding parameter changes.



NEW:



As shown in Figure 1, in the context of the layering model

architecture described in [RFC8309], the network and VPN service

performance monitoring (PM) model can be used to expose operational

performance information to the layer above, e.g., to an orchestrator

or other client application, via standard network management APIs.









(5) p 5, sec 3.1.  Collecting Data via Pub/Sub Mechanism



   Some applications such as service-assurance applications, which must

   maintain a continuous view of operational data and state, can use the

   subscription model specified in [RFC8641] to subscribe to the

   specific network performance data or VPN service performance data

   they are interested in, at the data source.  For example, network or

   VPN topology updates may be obtained through on-change notifications

   [RFC8641].  For dynamic PM data, various notifications can be

   specified to obtain more complete data.



Can you elaborate a bit on what is meant by dynamic PM data please.



Bo: Thanks for pointing this out. How about we change:

For dynamic PM data, e.g. VRF routes or MAC entries, link metrics, and interface metrics, various notifications can be specified to obtain more complete data.



Looks good.  Thanks.





(6) p 5, sec 3.1.  Collecting Data via Pub/Sub Mechanism



  A periodic notification

   [RFC8641] can be specified to obtain real-time performance data, a

   replay notification defined in [RFC5277] or [RFC8639] can be

   specified to obtain historical data



If this data is coming from a device then ideally it would not hold on to much historical data.

Bo: Is it better that we change to “can be specified to obtain historical data in a limited period of time.”? E.g. in some implementation, a controller can store PM data for a year?



Okay.  Perhaps something like:



A periodic notification [RFC8641] can be specified to obtain real-time performance data.

For devices/controllers that maintain historical performance data for a period of time, a replay

notification [RFC5277] or [RFC8639] can be used to obtain the historical data,





(7) p 6, sec 4.1.  Layering Relationship between Multiple Layers of Topology



      Figure 3: Example of Topology Mapping Between VPN Service

                   Topology and Underlying Network



Note, I don't find this diagram brilliantly clear, it is hard to see when the dotted lines go but the explanatory text is clear (and probably sufficient).



Bo: Thanks. We can remove the lines if it doesn't help.



I’m ambivalent on this one, and hence I’m happy to leave it to the authors discretion.  You could leave them in and see if you get similar comments during the IETF LC or IESG reviews.





(8) p 7, sec 4.1.  Layering Relationship between Multiple Layers of Topology



   Apart from the association between the VPN topology and the underlay

   topology, VPN Network PM can also provide the performance status of

   the underlay network and VPN services.  For example, network PM can

   provide link PM statistics and port statistics.  VPN PM can provide

   statistics on VPN access interfaces, the number of current VRF routes

   or L2VPN MAC entry of VPN nodes, and performance statistics on the

   logical point-to-point link between source and destination VPN nodes

   or between source and destination VPN access interfaces.  Figure 4

   illustrates an example of VPN PM and the difference between two VPN

   PM measurement methods.  One is the VPN tunnel PM and the other is

   inter-VPN-access interface PM.



By "VPN Network PM", do you mean the "VPN Network PM YANG module", or is this just referring to performance monitoring in general?



Bo: "VPN Network PM" mean "VPN Network PM YANG module". How about we rephrase:



Apart from the association between the VPN topology and the underlay

   topology, VPN Network PM YANG module can also provide the performance status of

   the underlay network and VPN services.  For example, network PM the module can

   provide link PM statistics and port statistics of a underlay network.  And it can also provide

   VPN PM statistics, which can be further split into PM for the VPN tunnel and PM at the VPN PE access node, as illustrated in the following diagram.

such as statistics on VPN access interfaces, the number of current VRF routes

   or L2VPN MAC entry of VPN nodes, and performance statistics on the

   logical point-to-point link between source and destination VPN nodes

   or between source and destination VPN access interfaces.  Figure 4

   illustrates an example of the module VPN PM and shows the difference between two VPN

   PM measurement methods.  One is including the VPN tunnel PM and the other is

   inter-VPN-access interface PM. // The newly added text are blue.



Figure 4 illustrates an example of VPN PM and two VPN PM measurement methods including the VPN tunnel PM and the inter-VPN-access interface PM. VPN PM can also provide

   statistics on VPN access interfaces, the number of current VRF routes or L2VPN MAC entry of VPN node





Yes, I think that this is clearer.  One nit: “a underlay” => “an underlay”.





(9) p 7, sec 4.1.  Layering Relationship between Multiple Layers of Topology



   Apart from the association between the VPN topology and the underlay

   topology, VPN Network PM can also provide the performance status of

   the underlay network and VPN services.  For example, network PM can

   provide link PM statistics and port statistics.  VPN PM can provide

   statistics on VPN access interfaces, the number of current VRF routes

   or L2VPN MAC entry of VPN nodes, and performance statistics on the

   logical point-to-point link between source and destination VPN nodes

   or between source and destination VPN access interfaces.  Figure 4

   illustrates an example of VPN PM and the difference between two VPN

   PM measurement methods.  One is the VPN tunnel PM and the other is

   inter-VPN-access interface PM.



I wonder if it would be better to move, and perhaps expand, the "Figure 4" explanation text to below the diagram?  If so, then I would add a joining sentence here, something like:

"VPN PM can be further split into PM for the VPN tunnel and PM at the VPN PE access node, as illustrated in the following diagram:"



Bo: Agree with the suggestion. Please take a look at the previous one, which has been combined.



Looks good.







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





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?







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





(18) p 24, sec 5.  Network and VPN Service Performance Monitoring YANG Module



     augment "/nw:networks/nw:network/nw:network-types" {

       description

         "Defines the service topologies types.";

       container service-type {

         presence "Indicates network service topology.";



Perhaps expand either in the presence statement, or the documentation what it means if this container isn't present.  I.e., does this mean that the topology represents the underlying network?



Bo: Thanks the suggestion. How about the change:

“VPN PM is indicated through this presence containers. When the container is not present, the topology represents the underlying network.”



Perhaps just:

“Presence of the container indicates a service topology, absence of the container indicates an underlay network.”









(22) p 38, sec Appendix A.  Illustrative Examples



   The following shows an example of a percentile measurement for a VPN

   link.



Please can you expand the description a bit.  I.e., that this is an example of data that could be returned for for link foo:vpn1-link1 between vpn-node1 and vpn-node3.





Bo: Thanks. Will rephrase as “This is an example of data that could be returned for a link foo:vpn1-link1 between vpn-node1 and vpn-node3”.



Thanks.



Regards,

Rob