Hi Dhruv, Thanks for the helpful review. The rev-09 has submitted to resolve your comments, please check whether you have further comments. https://www.ietf.org/rfcdiff?url2=draft-ietf-opsawg-yang-vpn-service-pm-09
Please also see inline for the replies.. Thanks, Bo From: Dhruv Dhody [mailto:[email protected]] Sent: Thursday, May 12, 2022 1:12 PM To: [email protected]; [email protected] Cc: [email protected]; [email protected] Subject: RtgDir Early review: draft-ietf-opsawg-yang-vpn-service-pm Hello I have been selected to do a routing directorate “early” review of this draft. The routing directorate will, on request from the working group chair, perform an “early” review of a draft before it is submitted for publication to the IESG. The early review can be performed at any time during the draft’s lifetime as a working group document. The purpose of the early review depends on the stage that the document has reached. As this document is post-working group last call, my focus for the review was to determine whether the document is ready to be published. Please consider my comments along with the other last call comments. For more information about the Routing Directorate, please see http://trac.tools.ietf.org/area/rtg/trac/wiki/RtgDir Document: draft-ietf-opsawg-yang-vpn-service-pm Reviewer: Dhruv Dhody Review Date: 2022-05-12 IETF LC End Date: Unknown Intended Status: Standards Track Summary: I have some minor concerns about this document that I think should be resolved before publication. Comments: The document is easy to read. The YANG model seems to be useful for VPN service assurance and is well-formatted. Major Issues: None Minor Issues: Earlier vpn-pm-type was a choice, it was changed in the -08 version and now both inter-vpn-access-interface and underlay-tunnel can be configured together. In the draft text, it states that "usually only one of the two methods is used". But the YANG allows both of them to be configured at the same time. +--rw vpn-pm-type +--rw inter-vpn-access-interface | +--rw inter-vpn-access-interface? empty +--rw underlay-tunnel! +--ro vpn-underlay-transport-type? identityref It is not clear to me how to interpret the measured PM delay value? Is it the VPN access interface or for the underlay tunnel? Maybe you should allow only one of them to be configured at a time? [Bo Wu] We think both measurements are not conflicting and should allow. How about adding a new read-only leaf "pm-type", that tells if the reported values are based on which pm-type. Query: The vpn-underlay-transport-type in underlay-tunnel identifies the type of underlay tunnel between the PEs. But isn't it possible to have multiple types of tunnels and even load balancing between them? Is the assumption that each instance of underlay tunnel would have its own link? [Bo Wu] The assumption is the latter case. How about adding some text to clarify: “In the case of multiple types of tunnels between a single pair of VPN nodes, a separate link for each type of tunnel can be created.” Nits: - Expand on first use: LMAP - Section 1: s/to display the// - Section 3.1: s/VPN service assurance model/VPN Service Performance Monitoring/ - Section 4.1: s/and node 3 (N3)5/and node 3 (N3)/ - Section 4.2: Is this text correct - "For network performance monitoring, the container of "networks" in [RFC8345<https://www.ietf.org/archive/id/draft-ietf-opsawg-yang-vpn-service-pm-08.html#RFC8345>] does not need to be extended."? Or is "not" there by mistake? [Bo Wu] Thanks. All fixed. YANG Model: 1) identity pe { base node-type; description "Provider Edge (PE) node type. A PE is the name of the device or set of devices at the edge of the provider network with the functionality that is needed to interface with the customer."; } What do you mean by "the name of the device"? [Bo Wu] Thanks, will remove “the name of”. 2) In the grouping entry-summary, the description says it for "VPN or network" but for all the leaves (maximum-routes, total-active-routes, mac-num etc) it says VPN only! Please update to "VPN or network". [Bo Wu] Thanks. Fixed. 3) leaf packet-loss-count { type yang:counter64; description "Total received packet drops count."; } Not sure about the description, it reads as if a packet is received but dropped, and I don't think that is what you mean? [Bo Wu] How about “Total number of lost packets.”? 4) leaf reference-time { type yang:date-and-time; config false; description "Indicates the time when the statistics are collected."; } I am not sure what collected means here. Do you mean when the current counters were last updated? Or when it was last aggregated/filtered at the controller? [Bo Wu] It means the former. How about replace the name to “last-updated”, and also the description “Indicates the time when the statistics were last updated” 5) leaf inbound-nunicast { type yang:counter64; description "The total number of inbound non-unicast (i.e., broadcast or multicast) packets."; } suggest changing the name to inbound-non-unicast (check other instances as well) [Bo Wu] OK. Thanks. 6) leaf network-access-id { type vpn-common:vpn-id; description "References to an identifier for the VPN network access, e.g. L3VPN or VPLS."; } The description of network-access-id is not clear. Is it name of VPN? or the site-network-access-id in LxSM? [Bo Wu] OK. Will remove the “e.g. L3VPN or VPLS.”. Thanks! Dhruv
_______________________________________________ OPSAWG mailing list [email protected] https://www.ietf.org/mailman/listinfo/opsawg
