Subject: Re: [pim] Last Call: <draft-ietf-pim-igmp-mld-yang-10.txt> (A YANG data model for Internet Group Management Protocol (IGMP) and Multicast Listener Discovery (MLD)) to Proposed Standard

From: Xufeng Liu <[email-masked]>
Date: Mon, 29 Apr 2019 11:37:27 -0400

Hi Tom,

Thanks for the review. We have posted the updated
https://tools.ietf.org/html/draft-ietf-pim-igmp-mld-yang-11 to address
these issues, along with other review comments.

Best regards,
- Xufeng

On Sat, Jan 26, 2019 at 11:57 AM tom petch <[email-masked]> wrote:

> Some initial  thoughts on this I-D
>
> Prefer 'Abstract' before 'Copyright' and 'Status'
>
[Xufeng]: Changed the order.

>
>    | acl       | ietf-access-control-list | [I-D.ietf-acl-yang] |
> better to have
>    | acl       | ietf-access-control-list | [RFC YYYY] |
> Note to RFC Editor please replace YYYY with number assigned to draft-
> ietf-netmod-acl-model
>
[Xufeng]: Fixed.

>
> YANG module needs the Copyright statement
>
[Xufeng]: Fixed.

>
> 4. IGMP and MLD YANG Modules
> suggests that there are two modules but I only see one
>
[Xufeng]: The text is wrong. Fixed. There is only one module.

>
>       import ietf-inet-types {
> and other imports need a reference clause to identify the RFC; for acl
> you need such as
> RFC YYYY "Network Access Control List (ACL) YANG Data Model",
> Note to RFC Editor please replace YYYY with number assigned to draft-
> ietf-netmod-acl-model
> ( you can put all RFC Editor Notes in one place at the front - they
> prefer it this way)
>
[Xufeng]: Fixed.

>
>       revision 2019-01-03 {
> there must only be one revision clause stating 'Initial revision' with
> date of publication supplied by RFC Editor to match that on the file
> statement
>
[Xufeng]: Removed the extra revision statetments.

>
> several lines are too long e.g.
>              If QQIC >= 128, QQIC represents a floating-point value as
> follows:
>
[Xufeng]: Fixed these lines and other lines with the same problem.

>
> 5. Security Considerations
>    transport is TLS [RFC5246].
> this is now superseded
>
[Xufeng]: Updated.

>
> 6. IANA Considerations
>    Names registry [RFC7950]:
> this is a poor reference since all it does is tell you to go to RFC6020;
> better to tell the reader to go straight there.
>
[Xufeng]: Use RFC6020 now.

>
>    [I-D.ietf-netmod-rfc6087bis] Bierman, A., "Guidelines for Authors
> this is now out of date - RFC8407
> (and it tells you to do most of what I have listed above:-)
>
[Xufeng]: Updated.

>
> Tom Petch
>
> ----- Original Message -----
> From: "The IESG" <[email-masked]>
> To: "IETF-Announce" <[email-masked]>
> Cc: <[email-masked]>; <[email-masked]>;
> <[email-masked]>
> Sent: Friday, January 25, 2019 8:28 PM
>
> >
> > The IESG has received a request from the Protocols for IP Multicast WG
> (pim)
> > to consider the following document: - 'A YANG data model for Internet
> Group
> > Management Protocol (IGMP) and
> >    Multicast Listener Discovery (MLD)'
> >   <draft-ietf-pim-igmp-mld-yang-10.txt> as Proposed Standard
> >
> > The IESG plans to make a decision in the next few weeks, and solicits
> final
> > comments on this action. Please send substantive comments to the
> > [email-masked] mailing lists by 2019-02-08. Exceptionally, comments may
> be
> > sent to [email-masked] instead. In either case, please retain the
> beginning of
> > the Subject line to allow automated sorting.
> >
> > 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.
> >
> >
> >
> >
> > The file can be obtained via
> > https://datatracker.ietf.org/doc/draft-ietf-pim-igmp-mld-yang/
> >
> > IESG discussion can be tracked via
> > https://datatracker.ietf.org/doc/draft-ietf-pim-igmp-mld-yang/ballot/
> >
> >
> > No IPR declarations have been submitted directly on this I-D.
> >
> >
> >
> >
>
>

---

From: Xufeng Liu <[email-masked]>
Date: Mon, 29 Apr 2019 11:51:47 -0400

Hi Tom,

Thanks for the comments. We have posted the updated
https://tools.ietf.org/html/draft-ietf-pim-igmp-mld-yang-11 to address
these issues, along with other review comments.
Best regards,
- Xufeng

On Wed, Jan 30, 2019 at 7:27 AM tom petch <[email-masked]> wrote:

> Looking into the technical aspects of this, I am inclined to think that
> it is not ready to become an RFC, needing widespread revision; the
> problem is the language.  In places, I can guess what is meant, in
> others, I cannot.  An example of the latter is
> "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."
> which is differentiated from "Global level" and "Interface-level".  That
> combination of 'not existing' and 'same value' leaves me uncertain as to
> the meaning.
>
[Xufeng]: Rephrased a bit to see if the description is improved.

>
> (When these terms are used in the YANG descriptions, the hyphens are
> missing).
>
> 'Global level' (no hyphen) is defined as "configuration and operational
> state attributes for the entire routing system."  Is that the entire
> routing system of all routers and hosts? or just everything regardless
> of protocol in this router? or just IGMP in this router?  ...
>
[Xufeng]: Should be just the IGMP instance in this router. Fixed.

>
> SSM/ssm appears in dozens of places and is never expanded nor is there a
> single reference.  I imagine that this is RFC3569 in which case, that
> must be a Normative Reference.
>
[Xufeng]: Right. Added an entry in Sec 1.1, and added the references
([RFC3569] and [RFC4607])

>
> 'IGMP or MLD' appears in a dozen places and gives me the most heartache;
> I do not know what it means.  Thus
> "The maximum number of entries in IGMP or MLD.";
> could be a maximum for IGMP which is also a maximum for MLD, same value;
> or it could mean the maximum for IGMP or MLD combined, regardless of
> which.  I cannot tell which is meant
>
[Xufeng]: I think that the confusion comes from the fact that such a
grouping is used in either IGMP schema or MLD schema. It could be better to
split above statement into two separate statements: one for IGMP and one
for MLD. The document has been updated for all such occurrences.

>
> Likewise, with
> grouping global-config-attributes/ leaf enable /type boolean;
> you have enable or disable coupled with IGMP and MLD; that is four
> choices, while a boolean only has two values - does not compute.
>
> Ditto
>
[Xufeng]: Fixed in the same way as above.

> grouping interface-specific-config-attributes / leaf enable / type
> boolean
>
> Is interface specific the same as interface level?
>
[Xufeng]: Yes. For consistency, it has been renamed to “interface-level-...
”.

>
> Interestingly, where I was expecting ambiguity,
> leaf discontinuity-time
> ...
> description "The time on the most recent occasion at which any one
>               or more of the statistic counters suffered a
>               discontinuity. "
> is perfectly clear; that ' any one or more' leaves me in no doubt.
>
> IGMP and MLD do get expanded on first use but it would be helpful to
> have the RFC reference there - you give it for YANG (surely everyone
> knows those RFC numbers off by heart by now:-) but not for the less
> widely used MLD.
>
[Xufeng]: I think that you mean the first use in the YANG module, whose
description has be updated as such.

>
> And then there are many places where the English is understandable but
> quirky and might be left to the RFC Editor but here, probably worth
> changing sooner rather than later; e.g.
>
> - not all included in this document of the data model
> - including some with basic subsets of the IGMP and MLD protocols.
> - any major now-existing implementation may be said to support the basic
> model
> - operational state parameters are not so widely designated as features
> - Where fields are not genuinely essential to protocol operation
> - The MLD YANG model uses the same structure as IGMP YANG model
> - MLD module also defines in a three-level hierarchy structure
> - IGMP and MLD RPC clears
> - group-addrss?
> - definitions common for IGMP and MLD
> - whose are not existing in interface global level
> - it prunes off the group
> - that IGMP ro MLD can join
> - If present, IGMP/MLD-based explicit membership tracking function for
> multicast routers and IGMP/MLD proxy devices supporting IGMPv3/MLDv2.
> - lightweight IGMPv3 and MLDv2 protocols will run on the which simplify
> the
> - List of multicast membership hosts
> - The last host address which has sent the report to
> - the MLD attributes at all of the interfaces level on a device
>
[Xufeng]: Went through these, and did some adjustments. Please let us know
for any further issues.

> HTH:-(
> From: "tom petch" <[email-masked]>
>
> Sent: Saturday, January 26, 2019 4:57 PM
>
>

---

From: tom petch <[email-masked]>
Date: Tue, 30 Apr 2019 10:17:41 +0000

Yes, much revised, much clearer to me.

A few small glitches

"   The terminology for describing YANG data models is found in
   [RFC6020] and [RFC7950]."

The terminology keeps moving on.  If you look at RFC8341, you can see a
more expansive summary of the terminology; I have not been through and
checked it but, knowing the author, it is unlikely I could find fault
with it:-)  I am not saying use it, but it is worth thinking about.
YANG Guidelines says there must be a Terminology section but not what
must be in it.

ACL is now RFC8519 which you have in the list of prefixes but not in the
References for the I-D

s.3.1
" The identity "igmp" derived from  the "rt:control-plane-protocol" base
identity is defined to indicate   a control-plane-protocol instance is
for IGMP.
"

Not clear what this is saying; perhaps

The identity "igmp" is derived from
   the "rt:control-plane-protocol" base identity and indicates that
   a control-plane-protocol instance is IGMP.

s.3.2 likewise for mld

In passing, can there be more than one IGMP or MLD instance in a router
or not?  I do not know the answer to that; OSPF, e.g., can and I see
that, BGP can but I do not; this would affect the English in some places

"       leaf group-policy {
         type leafref {
           path "/acl:acls/acl:acl/acl:name";
"
could do with a reference to RFC8519; and since the RFC says

" length "1..64";
        }
        description
          "The name of the access list.  A device MAY further
           restrict the length of this name; space and special
           characters are not allowed.";
"

then I think the description needs to reflect that.  A device can
restrict the length but cannot be greater than the 64 of the RFC.  And
the RFC bans space and special characters - again, the device cannot
gainsay that (unless you are into deviations which are a bad thing IMO).

"     rpc clear-igmp-groups {
...
              If it is not specified, IGMP groups from all interfaces
              are cleared.";
"
Mmm is this wise?  it is fail danger, someone makes a mistake and all it
lost.  I wonder if there should be a default which the user has to
wilfully override in order to clear everything.  Something to ponder.

"     rpc clear-mld-groups { "
ditto

Reference

"   [RFC8407] Bierman, A., "Guidelines for Authors and Reviewers of YANG
             Data Model Documents", draft-ietf-netmod-rfc6087bis-
             20(work in progress), March 2018.
"
Um, nearly but not quite:-)

Tom Petch

---- Original Message -----
From: "Xufeng Liu" <[email-masked]>
Sent: Monday, April 29, 2019 4:51 PM

Hi Tom,

[Xufeng]: I think that the confusion comes from the fact that such a
grouping is used in either IGMP schema or MLD schema. It could be better
to
split above statement into two separate statements: one for IGMP and one
for MLD. The document has been updated for all such occurrences.

> Is interface specific the same as interface level?
[Xufeng]: Yes. For consistency, it has been renamed to
“interface-level-...
”.

> - the MLD attributes at all of the interfaces level on a device
[Xufeng]: Went through these, and did some adjustments. Please let us
know
for any further issues.

> > > The IESG has received a request from the Protocols for IP
Multicast
> WG