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

[Benjamin Kaduk <[e-mail]>]

On Tue, May 28, 2019 at 10:18:54AM -0400, Xufeng Liu 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?

I replied over there, but it seems to be about right.

> 
> 
> >
> >
> > ----------------------------------------------------------------------
> > 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?

I think "lite-exclude-filter" would be better, but please don't feel
obligated to change anything, especially if there are already implementations
that would be affected.

Thanks!

-Ben

> >
> >      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.
> >
> >
> >