Last Call Review of draft-ietf-pim-igmp-mld-yang-10
review-ietf-pim-igmp-mld-yang-10-genart-lc-worley-2019-02-05-00

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.