Last Call Review of draft-ietf-mpls-ldp-yang-06
review-ietf-mpls-ldp-yang-06-genart-lc-enghardt-2019-09-25-00

Request Review of draft-ietf-mpls-ldp-yang-06
Requested rev. 06 (document currently at 07)
Type Last Call Review
Team General Area Review Team (Gen-ART) (genart)
Deadline 2019-09-27
Requested 2019-09-13
Requested by Tarek Saad
Authors Kamran Raza, Rajiv Asati, Xufeng Liu, Santosh Esale, Xia Chen, Himanshu Shah
Draft last updated 2019-09-25
Completed reviews Yangdoctors Early review of -01 by Dean Bogdanović (diff)
Yangdoctors Early review of -02 by Jan Lindblad (diff)
Genart Last Call review of -06 by Theresa Enghardt (diff)
Rtgdir Last Call review of -06 by Yingzhen Qu (diff)
Yangdoctors Last Call review of -06 by Jan Lindblad (diff)
Comments
This document describes a YANG data model for Multi-Protocol Label Switching (MPLS) Label Distribution Protocol (LDP).
Assignment Reviewer Theresa Enghardt
State Completed
Review review-ietf-mpls-ldp-yang-06-genart-lc-enghardt-2019-09-25
Posted at https://mailarchive.ietf.org/arch/msg/gen-art/qa5ExaZTQfioyONEFO8MJmm_CQg
Reviewed rev. 06 (document currently at 07)
Review result Ready with Issues
Review completed: 2019-09-25

Review
review-ietf-mpls-ldp-yang-06-genart-lc-enghardt-2019-09-25

I am the assigned Gen-ART reviewer for this draft. The General Area
Review Team (Gen-ART) reviews all IETF documents being processed
by the IESG for the IETF Chair.  Please treat these comments just
like any other last call comments.

For more information, please see the FAQ at

<https://trac.ietf.org/trac/gen/wiki/GenArtfaq>.

Document: draft-ietf-mpls-ldp-yang-06
Reviewer: Theresa Enghardt
Review Date: 2019-09-25
IETF LC End Date: 2019-10-04
IESG Telechat date: Not scheduled for a telechat

Summary:

This draft is basically ready for publication, but has some minor issues that should be fixed before publication.

Major issues: None.

Minor issues:

Section 1.1:

Why is LDP IPv6 grouped in the "extended" category and not in the "base" category, which the draft states to be the "minumum requirements for a typical base LDP deployment" and "suffice for small deployments"? Are typical, small deployments usually IPv4-only, and is this expected to remain true? 
Please consider briefly explaining this design decision.

What does "igp sync" refer to? Is this the same as "igp-synchronization-delay" in the extended model? Please consider expanding this abbreviation and/or providing a reference.

Section 3:

Could you provide references for the "widely deployed non-RFC features", which are part of the extended model, please?

"GR session is in recovery state" - What does "GR" refer to?

Section 10:

In the Security Considerations, it would be great if you could provide some examples of writable/creatable/deletable data nodes which may be considered sensitive or vulnerable, and what negative effects on network operations one could expect if an attacker wrote to them.

Nits/editorial comments:

The document doesn't use any RFC 2119 keywords, yet has text resembling RFC 2119 boilerplate text. Please consider removing the RFC 2119 boilerplate text.

The document contains a few typos and grammar issues.
To improve readability, please check for consistency of upper/lower case terms, for the use of definite and indefinite articles, and consider running a spellchecker.


Some examples:

Section 1.1:

"The configuration and state items are divided into following two broad categories" --> "The configuration and state items are divided into the following two broad categories" 

"This is worth higlighting " --> "It is worth highlighting"


Section 3:

"yang" - should this be all caps?

"rpc" - should this be all caps?

"grapically" --> "graphically"


Section 5.2.1:

"This container falls under global tree" --> "This container falls under the global tree"

"The example of former is interface hello timers, and example of latter is enabling hellos for a given AF under an interface." --> "The example of the former is interface hello timers, and an example of the latter is enabling hellos for a given AF under an interface."

"A peer is uniquely identified using its LSR Id and hence LSR Id is the key for peer list" [missing punctuation]