Re: [pim] Benjamin Kaduk's Discuss on draft-ietf-pim-igmp-mld-yang-13: (with DISCUSS and COMMENT)

[Xufeng Liu <[e-mail]>]

Hi Benjamin,

We have posted an updated version
https://tools.ietf.org/html/draft-ietf-pim-igmp-mld-yang-14, to address
these nits. We are holding on the updates for the Discuss point for now,
since it is still being discussed.

Best regards,
- Xufeng

On Tue, May 28, 2019 at 10:18 AM Xufeng Liu <[e-mail]>
wrote:

> Hi Benjamin,
>
> Thanks for the review. A little more on the discussion below.
> Best regards,
> - Xufeng
>
> On Sun, May 26, 2019 at 10:15 PM Benjamin Kaduk via Datatracker <
> [e-mail]> wrote:
>
>> Benjamin Kaduk has entered the following ballot position for
>> draft-ietf-pim-igmp-mld-yang-13: Discuss
>>
>> 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/
>>
>>
>>
>> ----------------------------------------------------------------------
>> DISCUSS:
>> ----------------------------------------------------------------------
>>
>> I agree with Mirja that reiterating the privacy considerations of
>> explicit tracking of group membership (with pointer to the relevant
>> IGMP/MLD protocol documents) would be worthwhile.
>> Normally I would leave this as a Comment on the assumption that the
>> privacy considerations are already documented in the protocol
>> specification that documents explicit tracking, but I could not find
>> such a document (with privacy considerations listed) in a quick search;
>> I found RFC 6636 and draft-ietf-pim-explicit-tracking but probably
>> missed a few others.
>>
>> Let's talk about what the current state actually is, and where it's best
>> to document the privacy considerations (which is not necessarily this
>> document, a priori).
>>
> [Xufeng]: Understand the conern about the insufficient decription in
> RFC6636 and draft-ietf-pim-explicit-tracking. We are ok to add more
> description in this document, as proposed in the reply to Mirja's comment.
> Would the change be ok?
>
>
>
>>
>>
>> ----------------------------------------------------------------------
>> COMMENT:
>> ----------------------------------------------------------------------
>>
>> Other than my Discuss point, basically all I have is editorial nits --
>> thanks for
>> the well-written document!
>
>
> [Xufeng]: Thanks for these. We will fix them in the new revision, and post
> the updates soon.
>
>>
>> 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.
>>
> [Xufeng]: Understand the confusion. "exlude" has its special meaning
> (which a type of filter) here, but the leaf name does not show it. Would
> "lite-exclude-filter" or "exclude-filter-lite" a bitter better?
>
>>
>>      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.
>>
>>
>>