Thanks Bo, the update looks great!! On Tue, 17 May 2022 at 3:31 PM, Wubo (lana) <[email protected]> wrote:
> *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 > <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
