Re: 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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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 <daedulus@xxxxxxxxxxxxx> 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" <xufeng.liu.ietf@xxxxxxxxx>
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 <daedulus@xxxxxxxxxxxxx>
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" <daedulus@xxxxxxxxxxxxx>
>
> 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" <iesg-secretary@xxxxxxxx>
> > To: "IETF-Announce" <ietf-announce@xxxxxxxx>
> > Cc: <pim-chairs@xxxxxxxx>; <pim@xxxxxxxx>;
> > <draft-ietf-pim-igmp-mld-yang@xxxxxxxx>
> > 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
> > > ietf@xxxxxxxxx mailing lists by 2019-02-08. Exceptionally, comments
> may
> > be
> > > sent to iesg@xxxxxxxx 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..


[Index of Archives]     [IETF Annoucements]     [IETF]     [IP Storage]     [Yosemite News]     [Linux SCTP]     [Linux Newbies]     [Mhonarc]     [Fedora Users]

  Powered by Linux