A YANG Data Model for the Internet Group Management Protocol (IGMP) and Multicast Listener Discovery (MLD)
draft-ietf-pim-igmp-mld-yang-15
Yes
(Alvaro Retana)
No Objection
Warren Kumari
(Alissa Cooper)
(Deborah Brungard)
(Ignas Bagdonas)
(Martin Vigoureux)
Note: This ballot was opened for revision 11 and is now closed.
Roman Danyliw
No Objection
Comment
(2019-05-29 for -13)
Sent
(1) I support Ben Kaduk’s DISCUSS about privacy considerations. (2) Thanks for Section 1.3. This upfront cross-reference was very helpful! (3) Section 2.2. On the other hand, operational state parameters are not so widely designated as features, as there are many cases where the defaulting of an operational state parameter would not cause any harm to the system, and it is much more likely that an implementation without native support for a piece of operational state would be able to derive a suitable value for a state variable that is not natively supported. I struggled to understand this very dense single sentence paragraph. -- What does it mean for “operational state parameters are not so widely designated as features”? -- What is a “defaulting of an operational state parameter”? (4) Section 3. Does the sentence “This model augments the core routing data model specification in [RFC8349]” suggest that this draft should update [RFC8349]? (5) Reference Nits ** Section 1. I would have expected a reference to NMDA (i.e., [RFC8342]). This is done later in the draft but not the first time it is mentioned (here). ** idnits returned the following valid reference issues: == Missing Reference: 'I-D.ietf-netconf-yang-push' is mentioned on line 178, but not defined == Missing Reference: 'RFC 8446' is mentioned on line 1910, but not defined == Missing Reference: 'RFC8341' is mentioned on line 1912, but not defined == Unused Reference: 'RFC5246' is defined on line 2085, but no explicit reference was found in the text == Unused Reference: 'RFC6536' is defined on line 2099, but no explicit reference was found in the text == Unused Reference: 'RFC5790' is defined on line 2145, but no explicit reference was found in the text ** Downref: Normative reference to an Informational RFC: RFC 3569 ** Obsolete normative reference: RFC 5246 (Obsoleted by RFC 8446) ** Obsolete normative reference: RFC 6536 (Obsoleted by RFC 8341) (6) Editorial Nits ** Section 2.1. The sentence “Even though there is no protocol specific notifications are defined in this model, the subscription and push mechanism defined in [I-D.ietf-netconf-subscribed- notifications] and [I-D.ietf-netconf-yang-push]” doesn’t parse. Likely remove the “are”. ** Section 2.1. Per the sentence “Depending on the implementation choices, some systems may not allow some of the advanced parameters configurable”, what is a “advanced parameters configurable”? I think there is a typo there. ** Section 2.3. Per “The current document defines …”, shouldn’t this just be “The document defines …” since when published as an RFC there would be no notion of versioning as suggested by “current”? ** Section 2.3. s/supports and only supports/only supports/ ** Section 4. Typo. s/refered/referred/ ** Section 4. Missing space. /Querier.In RFC3376,/Querier. In RFC3376,/
Warren Kumari
No Objection
Éric Vyncke
No Objection
Comment
(2019-05-20 for -12)
Sent
Thanks for the work everyone has put into this document. I only have a couple of comments (but one important one about the 2 branches) and a couple of nits. == COMMENTS == -- Section 2.1.1 and section 2.1.2 -- Those sections are about configuration parameters not covered at global or interface level. But, what about operational states, can the reader assume that they are all covered by this document ? It is really unclear. -- Section 2.3 -- As I am not a multicast expert, I did not put a DISCUSS on this one. But, are MLD and IGMP so different? Why having TWO different branches for each address family... For SNMP, RFC 4292/4293 was made protocol version independent which is a big plus IMHO for operations. In any case, there should be more explanations why there are two branches than the one paragraph/two sentences in section 2.3. Moreover, it seems that the two schema branches are quite similar so having one protocol version independent branch appears to be doable. == NITS == -- Section 1 -- Add a reference to NMDA (expanding the acronym is not really sufficient, state RFC 8342) ? Expand CLI even if well-known.
Alvaro Retana Former IESG member
Yes
Yes
(for -11)
Unknown
Alissa Cooper Former IESG member
No Objection
No Objection
(2019-05-29 for -13)
Sent for earlier
Barry Leiba Former IESG member
No Objection
No Objection
(2019-05-28 for -13)
Sent
I support Ben's DISCUSS and will be watching that discussion.
Benjamin Kaduk Former IESG member
(was Discuss)
No Objection
No Objection
(2019-05-29 for -13)
Sent for earlier
Other than my Discuss point (which seems to be resolvable with no change), basically all I have is editorial nits -- thanks for the well-written document! Section 2.1 Even though there is no protocol specific notifications are defined in this model, the subscription nit: s/there is// -- the "are defined in this model" takes care of the grammatical necessities here. and push mechanism defined in [I-D.ietf-netconf-subscribed- notifications] and [I-D.ietf-netconf-yang-push] can be used by the user to subscribe notifications on the data nodes in this model. nit: "subscribe to notifications" The model contains all basic configuration parameters to operate the protocols listed above. Depending on the implementation choices, nit: "all the basic" Section 4 grouping interface-common-config-attributes { [...] leaf query-interval { [...] description "The Query Interval is the interval between General Queries sent by the Querier.In RFC3376, Querier's Query Interval(QQI) is represented from the Querier's Query Interval Code in query message as follows: nit: one or two (not zero) spaces after the end of the sentence. nit: "In RFC3376, the Querier's Query Interval (QQI)" leaf exclude-lite { if-feature intf-exclude-lite; side-note: I misparsed this (I think) the first few times I read it, since "exclude lite" can be taken as an imperative command to not use the lite version. But it seems this is really just an ordinary feature-enablment tag about whether to use the EXCLUDE filtering that is available in the lite version of the protocol. I don't know whether reordering to "lite-exclude" would be a net win or net loss due to causing some other confusion, though. grouping interface-state-attributes-igmp { [...] list group { [...] list source { [...] list host { [...] leaf host-address { type inet:ipv4-address; description "The IPv6 address of the host."; nit: "ipv6-address" grouping interface-state-group-attributes-igmp-mld { nit: I thought most of the other shared groupings just didn't use a suffix, as opposed to using the "-igmp-mld" combined suffix. (Similarly for interface-state-source-attributes-igmp-mld and interface-state-host-attributes-igmp-mld, which does cause me to wonder if I'm not perceiving "most" correctly.) Section 5 igmp-mld:global This subtree specifies the configuration for the IGMP attributes at the global level on an IGMP instance. Modifying the configuration can cause IGMP membership deleted or reconstructed on all the interfaces of an IGMP instance. nit: "to be deleted or reconstructed" (Similarly for the following paragraphs.) The description of the considerations for unauthorized read access are fairly generic and do not specify specific potential harms, but I will not insist on any changes here.
Deborah Brungard Former IESG member
No Objection
No Objection
(for -13)
Not sent
Ignas Bagdonas Former IESG member
No Objection
No Objection
(for -14)
Not sent
Magnus Westerlund Former IESG member
No Objection
No Objection
(2019-05-28 for -13)
Not sent
One very minor question about data tree representation: Should this figure have vertical bars from rw ssm-map down to the plus for rw ssm-map-source-addr? +--rw ssm-map* | [ssm-map-source-addr ssm-map-group-policy] | {intf-ssm-map}? | +--rw ssm-map-source-addr ssm-map-ipv4-addr-type | +--rw ssm-map-group-policy string +--rw static-group* [group-addr source-addr] | {intf-static-group}? | +--rw group-addr | | rt-types:ipv4-multicast-group-address | +--rw source-addr | rt-types:ipv4-multicast-source-address Thus making the unattached brances being attached?
Martin Vigoureux Former IESG member
No Objection
No Objection
(for -13)
Not sent
Mirja Kühlewind Former IESG member
No Objection
No Objection
(2019-05-24 for -13)
Sent
Two quick questions: 1) Not sure about the current practice about YANG models but shouldn’t this document eventually update RFC8349? 2) Also maybe it would make sense to discuss the sensitivity of explicit-tracking separately in the security consideration section?
Suresh Krishnan Former IESG member
No Objection
No Objection
(2019-05-29 for -13)
Sent
I do have a general concern with the document in relation to its handling of multiple protocol versions. There are features in the yang models that should be conditional but they do not seem to be. Here are some examples. * The source specific features are to be used with IGMPv3 and MLDv2 and will not work with the earlier versions * The router alert check is not optional for MLD or IGMPv3, but is required to be disabled for compatibility with earlier versions of IGMP. I would also make this feature conditional on the IGMP version. If not you need to rethink the defaults for this. I would like to understand the authors' views on how they plan to address the potential consistency issues due to these features being unbound in the model. I would be fine if it is either addressed with yang constructs or with some explanatory text to this point.