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

[tom petch <[e-mail]>]

Xufeng

Looks good except ... sigh ...

"            The value space of this leaf is restricted to the existing
            policy instances defined by the refered schema [RFC8519].
            As specified by [RFC8519],  ..."
where [RFC8519] looks like an XML/HTML type anchor whereas the YANG
module per se must be plain text so it can be extracted as such ie I
would expect to see (RFC8519)
.
And referred should have two 'r'.

My take would be to leave these to be incorporated once the IESG has
come up with its comments but that is up to you and the WG Chairs.

Tom Petch

----- Original Message -----
From: "Xufeng Liu" <[e-mail]>
To: "tom petch" <[e-mail]>
Cc: <[e-mail]>; <[e-mail]>;
<[e-mail]>; <[e-mail]>
Sent: Thursday, May 09, 2019 3:10 PM

Hi Tom,

Thanks for the further comments. The fixes have been posted at
https://tools.ietf.org/html/draft-ietf-pim-igmp-mld-yang-12.

Best regards,
- Xufeng

On Tue, Apr 30, 2019 at 6:17 AM tom petch <[e-mail]>
wrote:

> 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.
>
[Xufeng]: Fortunately the YANG terms used in this document are
traditionally stable ones. I do agree that listing some terms would
benefit
the readers by the convenience and clarity. We have listed a few ones
that
are important to this document and have been used many times.

>
> ACL is now RFC8519 which you have in the list of prefixes but not in
the
> References for the I-D
>
[Xufeng]: Fixed the reference.

>
> 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.
>
[Xufeng]: Right. Fixed as suggested.

>
> s.3.2 likewise for mld
>
[Xufeng]: Fixed.

>
> 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
>
[Xufeng]: This model allows multiple instances because of the augmented
module ietf-routing and its convention in [RFC8349]. Usual
implementations
do not allow multiple instances, in which case the restriction may be
added
to the model during implementation. I went through the text, and the
descriptions seem to be ok. Two sentences have been added to make this
explicit.

>
>
> "       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).
>
[Xufeng]: Modified as suggested.

>
> "     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
>
[Xufeng]: Glad that you brought this up. Using unspecified argument to
clear all entries is convenient and often implemented. However, as you
mentioned, the fail danger does exist. We have modified the parameters
to
use explicit “all-interfaces” instead of empty interface name, and the
special value “*” to indicate all addresses.

>
> 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:-)
>
[Xufeng]: Fixed.

>
> Tom Petch
>
>
> ---- Original Message -----
> From: "Xufeng Liu" <[e-mail]>
> Sent: Monday, April 29, 2019 4:51 PM
>
> 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 <[e-mail]>
> 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:-(
> >
> > Tom Petch
> >
> >
> > ----- Original Message -----
> > From: "tom petch" <[e-mail]>
> >
> > Sent: Saturday, January 26, 2019 4:57 PM
> >
> >
> > > Some initial  thoughts on this I-D
> > >
> > > Prefer 'Abstract' before 'Copyright' and 'Status'
> > >
> > >    | 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
> > >
> > > YANG module needs the Copyright statement
> > >
> > > 4. IGMP and MLD YANG Modules
> > > suggests that there are two modules but I only see one
> > >
> > >       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)
> > >
> > >       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
> > >
> > > several lines are too long e.g.
> > >              If QQIC >= 128, QQIC represents a floating-point
value
> as
> > > follows:
> > >
> > > 5. Security Considerations
> > >    transport is TLS [RFC5246].
> > > this is now superseded
> > >
> > > 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.
> > >
> > >    [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:-)
> > >
> > > Tom Petch
> > >
> > > ----- Original Message -----
> > > From: "The IESG" <[e-mail]>
> > > To: "IETF-Announce" <[e-mail]>
> > > Cc: <[e-mail]>; <[e-mail]>;
> > > <[e-mail]>
> > > 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
> > > > [e-mail] mailing lists by 2019-02-08. Exceptionally,
comments
> > may
> > > be
> > > > sent to [e-mail] 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.
>
>