Reviewer: Dale Worley Review result: Ready with Nits 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-pim-igmp-mld-yang-10 Reviewer: Dale R. Worley Review Date: 2019-02-05 IETF LC End Date: 2019-02-08 IESG Telechat date: not known Summary: This draft is basically ready for publication, but has nits that should be fixed before publication. I do not have the expertise to review the contents of the Yang module itself. Fortunately, the Yang Doctor can review that. Minor issues: This draft has a number of exposition issues that should be fixed. Abstract This document defines a YANG data model that can be used to configure and manage Internet Group Management Protocol (IGMP) and Multicast Listener Discovery (MLD) devices. Both here and in the Introduction, it would be better to say "devices that implement IGMP and MLD" or something like that, since IGMP and MLD are protocols, not classes of devices. Table of Contents 2. Design of Data model......................................... 4 2.1. Scope of model ......................................... 4 2.2. Optional capabilities .................................. 4 2.3. Position of address family in hierarchy ................ 5 3. Module Structure ............................................ 5 3.1. IGMP Configuration and Operational state ............... 5 3.2. MLD Configuration and Operational State ................ 8 It looks like the current style would capitalize "model", "capabilities", "state", etc. 1. Introduction This model will support the core IGMP and MLD protocols, as well as many other features mentioned in separate IGMP and MLD RFCs. "will support" needs clarifying. Does the model defined by this document "support many other features", or is this a prediction that the model will eventually be extended to do so? Indeed, the Introduction should make a clear statement of what is and is not supported by this version of the model. 1.3. Prefixes in Data Node Names Otherwise, names are prefixed using the standard prefix associated with the The tail of this sentence is missing. 2. Design of Data model 2.1. Scope of model The model covers IGMPv1 [RFC1112], IGMPv2 [RFC2236], IGMPv3 [RFC3376] and MLDv1 [RFC2710], MLDv2 [RFC3810]. This should be stated in the Introduction as well. The configuration of IGMP and MLD features, and the operational state fields and RPC definitions are not all included in this document of the data model. As written, this says that the model covers some unspecified subset of IGMP and MLD features. Is it possible to characterize what is included and what is not? Otherwise, the reader would have to work through the model to check on every specific item they were interested in. This model can be extended, though the structure of what has been written may be taken as representative of the structure of the whole model. What does this mean? Like any Yang model, this model can be extended, by anyone who chooses to do so. But how does "what has been written" represent or constrain the structure of an extended model? 2.2. Optional capabilities The main design goals of this document are that any major now-existing implementation may be said to support the basic model, [...] Probably more correct to say "[...] may be said to support the facilities described by the basic model [...]". There is also value in widely-supported features being standardized, to save work for individual vendors, [...] And probably more importantly, so that the features can be accessed in a standardized way. 2.3. Position of address family in hierarchy The current document contains IGMP and MLD as separate schema branches in the structure. The reason for this is to make it easier for implementations which may optionally choose to support specific address families. And the names of objects may be different between the IPv4 (IGMP) and IPv6 (MLD) address families. It seems like the qualification of IGMP == IPv4 and MLD == IPv6 should be done in the first sentence rather than the last. 3.1. IGMP Configuration and Operational state It seems like this section has a first part which applies to IGMP and MLD equally (though it only talks about IGMP), and a second part which is a summary of the IGMP module. Perhaps they should be split into two sections? Interface-global: Only including configuration data nodes that IGMP configuration attributes are applicable to all the interfaces whose interface-level corresponding attributes are not existing, with same attributes' value for these interfaces. This sentence seems to have either extra words or missing words. "SSM" seems to show up a lot but isn't defined. Is it part of IGMP/MLD? 3.2. MLD Configuration and Operational State 3.3. IGMP and MLD RPC IGMP and MLD RPC clears the specified IGMP and MLD group membership. This is awkwardly phrased. Perhaps, "IGMP and MLD each have one RPC which clears the group membership [database? table?] for that protocol." 4. IGMP and MLD YANG Modules The use of empty lines isn't consistent in the module definition. 5. Security Considerations These subtrees are all under /rt:routing/rt:control-plane-protocols /rt:control-plane-protocol/igmp: Is the trailing "/igmp:" meaningful? And parallel cases later in the section. As far as I can see, "igmp:" etc. is part of the root node name of the subtree, not attached to the path above it. Unauthorized access to any data node of these subtrees can adversely affect the membership records of multicast routing subsystem on the local device. -- and some similar cases. The scope of the phrase "these subtrees" is unclear. 6. IANA Considerations This document registers the following namespace URIs in the IETF XML registry [RFC3688]: -------------------------------------------------------------------- URI: urn:ietf:params:xml:ns:yang:ietf-igmp-mld Registrant Contact: The IESG. XML: N/A, the requested URI is an XML namespace. -------------------------------------------------------------------- RFC 3688 section 3.2 includes: ns -- XML Namespaces [W3C.REC-xml-names] are named by a URI. [...] Thus, the registered document will be either the specification or a reference to it. [...] It seems to me that the "XML" field of this registration should be: XML: RFC XXXX to provide the name of the registered specification of the namespace.
