Subject: [pim] Roman Danyliw's No Objection on draft-ietf-pim-igmp-mld-yang-13: (with COMMENT)

From: Roman Danyliw via Datatracker <[email-masked]>
Date: Wed, 29 May 2019 06:55:45 -0700

Roman Danyliw has entered the following ballot position for
draft-ietf-pim-igmp-mld-yang-13: No Objection

When responding, please keep the subject line intact and reply to all
email addresses included in the To and CC lines. (Feel free to cut this
introductory paragraph, however.)

Please refer to https://www.ietf.org/iesg/statement/discuss-criteria.html
for more information about IESG DISCUSS and COMMENT positions.

The document, along with other ballot positions, can be found here:
https://datatracker.ietf.org/doc/draft-ietf-pim-igmp-mld-yang/

----------------------------------------------------------------------
COMMENT:
----------------------------------------------------------------------

(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,/

---

From: Alvaro Retana <[email-masked]>
Date: Wed, 29 May 2019 12:38:09 -0400

On May 29, 2019 at 9:55:46 AM, Roman Danyliw via Datatracker (
[email-masked]) wrote:

Hi!

Just responding to this specific issue:

The reference to rfc3569 was added from a LC comment to define SSM in the
terminology.  The correct Normative reference is rfc4607 (Source-Specific
Multicast for IP) and not rfc3569 (An Overview of Source-Specific Multicast
(SSM)).  rfc4607 is already a Normative reference in the document.
Authors: in the next revision, please make the rfc3569 reference
Informative.

Thanks!

Alvaro.

---

From: Xufeng Liu <[email-masked]>
Date: Wed, 29 May 2019 23:39:58 -0400

Hi Roman,

Thanks for the review. We have posted an updated revision
https://tools.ietf.org/html/draft-ietf-pim-igmp-mld-yang-14, to address
these issues.

Regards,
- Xufeng

On Wed, May 29, 2019 at 9:55 AM Roman Danyliw via Datatracker <
[email-masked]> wrote:

> designated as features”?

[Xufeng]: In the modeling language YANG, an optional section (that can be a
group of parameters) may be marked as a YANG “feature”, meaning that this
section is “optionally” supported by an implementation. If an
implementation supports this “feature”, the name and status of this feature
are sent to the client. In the design of this model, we do not use many
such “features” for operational states. The result is that an
implementation will have to return the values of the states, without the
option of not implementing them.

> -- What is a “defaulting of an operational state
> parameter”?

[Xufeng]: As mentioned in the previous paragraph, an implementation needs
to return many of the states specified in this model without the vendor’s
choice. Usually the states that an implementation would want to opt-out are
these states that are not critical for the protocol to operate, and the
implementation usually has used a default value. Therefore, the
implementation may always return the default value when the client ask for
such a state, hence the phrase “defaulting of an operational state
parameter”.

If better wording can make this paragraph more comprehensible, please
advise.

> model specification in [RFC8349]” suggest that this draft should update
[Xufeng]: The suggestion is logically correct, but the current practice for
YANG model document is not to update the base model [RFC8349], but to make
a reference to it. I think that it is because of the modular and extensible
natures of YANG, there could be hundreds of models augmenting a base model,
both inside and outside of IETF.

> This is done later in the draft but not the first time it is mentioned
[Xufeng]: Added.

>   == Missing Reference: 'I-D.ietf-netconf-yang-push' is mentioned on line
[Xufeng]: Added.

>   == Missing Reference: 'RFC 8446' is mentioned on line 1910, but not
[Xufeng]: Added.

>   == Missing Reference: 'RFC8341' is mentioned on line 1912, but not
[Xufeng]: Added.

>      reference was found in the text
[Xufeng]: Removed.

>      reference was found in the text
[Xufeng]: Removed.

>      reference was found in the text
[Xufeng]: Fixed.

>   ** Downref: Normative reference to an Informational RFC: RFC 3569
[Xufeng]: Fixed as Alvaro suggested.

>   ** Obsolete normative reference: RFC 5246 (Obsoleted by RFC 8446)
[Xufeng]: Removed

>   ** Obsolete normative reference: RFC 6536 (Obsoleted by RFC 8341)
[Xufeng]: Removed

> [I-D.ietf-netconf-yang-push]” doesn’t parse.  Likely remove the “are”.
[Xufeng]: Fixed

> what is a “advanced parameters configurable”?  I think there is a typo
[Xufeng]: Fixed.

> notion of versioning as suggested by “current”?
[Xufeng]: Edited.

> ** Section 2.3.  s/supports and only supports/only supports/
[Xufeng]: Fixed.

> ** Section 4.  Typo.  s/refered/referred/
[Xufeng]: Fixed.

> ** Section 4. Missing space.  /Querier.In RFC3376,/Querier. In RFC3376,/
[Xufeng]: Fixed.