Skip to main content

Last Call Review of draft-ietf-opsawg-l2nm-07
review-ietf-opsawg-l2nm-07-yangdoctors-lc-lhotka-2021-10-05-00

Request Review of draft-ietf-opsawg-l2nm-06
Requested revision 06 (document currently at 19)
Type Last Call Review
Team YANG Doctors (yangdoctors)
Deadline 2021-10-01
Requested 2021-09-16
Requested by Joe Clarke
Authors Mohamed Boucadair , Oscar Gonzalez de Dios , Samier Barguil , Luis Angel Munoz
I-D last updated 2021-10-05
Completed reviews Rtgdir Early review of -01 by Yingzhen Qu (diff)
Secdir Last Call review of -07 by Chris M. Lonvick (diff)
Rtgdir Last Call review of -10 by Himanshu C. Shah (diff)
Yangdoctors Last Call review of -07 by Ladislav Lhotka (diff)
Genart Last Call review of -15 by Dale R. Worley (diff)
Secdir Last Call review of -15 by Chris M. Lonvick (diff)
Rtgdir Telechat review of -16 by Yingzhen Qu (diff)
Assignment Reviewer Ladislav Lhotka
State Completed
Request Last Call review on draft-ietf-opsawg-l2nm by YANG Doctors Assigned
Posted at https://mailarchive.ietf.org/arch/msg/yang-doctors/v4_PQ4OJ8E-AqBJHRiS0g1WiS-E
Reviewed revision 07 (document currently at 19)
Result Ready w/issues
Completed 2021-10-05
review-ietf-opsawg-l2nm-07-yangdoctors-lc-lhotka-2021-10-05-00
**** General comments

The ietf-l2vpn-ntw module with about 400 data nodes represents an impressive
amount of work. Its size, however, raises some concerns in terms of
manageability. For example, if the ITU-T Y-1731 recommendation ever gets
updated (I don't know how likely this is), the module will have to be updated.
I would therefore suggest to consider factoring out such parts into separate
modules.

Apart from that, the YANG modules are in a good shape.

**** Specific comments

***** Instance data examples

The appendices A.1--A.6 contain examples of instance data for six different use
cases. This would be laudable, but only if the examples were valid according to
the data model schema. It seems that the examples were not updated during the
data model development. It is necessary to validate the examples using the
available tools such as yanglint or Yangson, and correct all errors. Here is a
non-exhaustive list of problems in the example of appendix A.1: - The top-level
container "ietf-l2vpn-ntw:l2vpn-ntw" is missing, which complicates the use of
validation tools. - "vpn-service" list contains "vpn-description" leaf, not
"description". - "global-parameters-profile" list contains "svc-mtu" leaf, not
"mtu". - The value of "global-parameters-profile/rd-suffix" should be uint16,
not string (e.g. 1 rather than "1"). Similarly, "vpn-target/id" is uint8, not
string. - Container "vpn-targets" encapsulating "vpn-target" list doesn't exist
in the schema. - "vpn-target/route-targets" is defined as a list, but its
instance value is given as ["0:65550:1"] (it probably used to be a leaf-list).
- Inside "vpn-service" list, the schema defines "vpn-nodes" container
encapsulating "vpn-node" list. This container is missing in the example.

***** RFC 8792 line folding

My reading of sec. 7.1.1 in RFC 8792 is that each text chunk that uses the
single backslash strategy should be preceded with the header

NOTE: '\' line wrapping per RFC 8792

**** Nits

- The naming of groupings is inconsistent: "cfm-802-grouping" versus e.g.
"y-1731". I'd suggest to remove the "-grouping" suffix in the former.