Skip to main content

Last Call Review of draft-ietf-opsawg-yang-vpn-service-pm-07
review-ietf-opsawg-yang-vpn-service-pm-07-yangdoctors-lc-krejci-2022-04-27-00

Request Review of draft-ietf-opsawg-yang-vpn-service-pm
Requested revision No specific revision (document currently at 15)
Type Last Call Review
Team YANG Doctors (yangdoctors)
Deadline 2022-04-20
Requested 2022-04-06
Requested by Joe Clarke
Authors Bo Wu , Qin Wu , Mohamed Boucadair , Oscar Gonzalez de Dios , Bin Wen
I-D last updated 2022-04-27
Completed reviews Rtgdir Early review of -08 by Dhruv Dhody (diff)
Yangdoctors Early review of -05 by Ladislav Lhotka (diff)
Yangdoctors Last Call review of -07 by Radek Krejčí (diff)
Tsvart Last Call review of -11 by Bob Briscoe (diff)
Genart Last Call review of -12 by Elwyn B. Davies (diff)
Secdir Last Call review of -12 by Daniel Migault (diff)
Comments
I know I requested early review a while ago, but I'm bumping this back to the top of the queue since we're now closing WG LC.  There were some late additions involving model changes that I think would benefit from RTG review.  The module itself seems mostly okay to me, but another pair of YANG Doc eyes would be helpful.
Assignment Reviewer Radek Krejčí
State Completed
Request Last Call review on draft-ietf-opsawg-yang-vpn-service-pm by YANG Doctors Assigned
Posted at https://mailarchive.ietf.org/arch/msg/yang-doctors/Vv0ZNu15L214ci-sV7k5Zu63d54
Reviewed revision 07 (document currently at 15)
Result Ready w/nits
Completed 2022-04-27
review-ietf-opsawg-yang-vpn-service-pm-07-yangdoctors-lc-krejci-2022-04-27-00
The draft addresses/fixes previous comments.

The draft, as well as the module, is well written and the only issue I've found
is kind of unclear use for the
/nw:networks/nw:network/nt:link/pm-attributes/vpn-pm-type choice. I don't
understand the logic of having one case config true and the second one config
false. Does it mean that the second one is the default? Then it should be
stated in the choice. I'm not an expert in the area, but I understand the
choice as a way for clients to select the type of performance monitoring. Then
it is kind of confusing that I can actually select only one of the available
types. What about having config true presence container in the second case and
holding config false leaf(s) there, wouldn't it be more clear?