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?