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

[Hongji Zhao <[e-mail]>]

Hi Benjamin,

Thanks a lot for your comments. 

Regarding your discuss,  we plan to modify exclude-lite leaf as below. 
Is it ok? Thanks.

leaf lite-exclude-filter {
      if-feature lite-exclude-filter;
      type empty;
      description
           "For IGMP Snooping, the presence of this
            leaf enables the support of the simplified EXCLUDE filter 
            in the Lightweight IGMPv3 protocol, which simplifies the
            standard versions of IGMPv3.
            For MLD Snooping, the presence of this
            leaf enables the support of the simplified EXCLUDE filter
            in the Lightweight MLDv2 protocol, which simplifies the
            standard versions of MLDv2.";
      reference
           "RFC 5790: Lightweight Internet Group Management Protocol
            Version 3 (IGMPv3) and Multicast Listener Discovery
            Version 2 (MLDv2) Protocols";
    }

BR/Hongji

-----Original Message-----
From: Benjamin Kaduk via Datatracker <[e-mail]> 
Sent: Thursday, July 9, 2020 11:03 PM
To: The IESG <[e-mail]>
Cc: [e-mail]; [e-mail]; [e-mail]; Stig Venaas <[e-mail]>; [e-mail]; [e-mail]
Subject: Benjamin Kaduk's Discuss on draft-ietf-pim-igmp-mld-snooping-yang-17: (with DISCUSS and COMMENT)

Benjamin Kaduk has entered the following ballot position for
draft-ietf-pim-igmp-mld-snooping-yang-17: 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-snooping-yang/



----------------------------------------------------------------------
DISCUSS:
----------------------------------------------------------------------

Let's discuss whether we need to be a bit more clear about what behavior should be expected when the exclude-lite leaf has value true vs. false -- the apparent inconsistency between "exclude"
in the leaf name and the positive verb "track" in the description left me confused.


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

Is a gauge type better than an int for the various 'count' fields?

Do we want to put in explicit "discontinuity-time" elements for when the counters last reset?  (I see at least one "up-time" leaf but it's not entirely clear that the semantics match up.)

Section 3.2

    The mld-snooping-instance is the same as IGMP snooping except changing
    IPv4 addresses to IPv6 addresses. One mld-snooping-instance could be

The diff also shows some mechanical changes, like replacing "igmp-version" with "mld-version", and the name of statistics leafs that are tied to protocol version/elements.  There doesn't seem to be a clear need to call out every such difference, though.

Section 4

  feature exclude-lite {
    description
      "Support configuration of per instance exclude-lite.";
    reference
      "RFC 5790";

RFC 5790 does not use the keyword "EXCLUDE-lite" (or actually, "lite" at all), so a little bit more description of which functionality this refers to could be helpful.

    leaf-list bridge-mrouter-interface {
      when 'derived-from-or-self(../scenario,"ims:bridge")';
      type if:interface-ref;
      config false;
      description
        "The mrouter interface in BRIDGE forwarding. When switch
         receives IGMP/MLD queries from multicast router on an
         interface, this interface will become mrouter interface
         for IGMP/MLD snooping.";

nit: the grammar in this description should be revisited; maybe just missing articles.  Similarly for the next few entries.

          leaf last-reporter {
            type inet:ipv4-address;
            description
              "Address of the last host which has sent report
               to join the multicast group.";

I guess I'm not entirely sure what this is used for; it doesn't seem like it would provide a way to reliably get a stream of all hosts that sent a report to join (the "list host" with feature explicit-tracking would be needed for that, right?), and could be stale at any time, but I'm probably just missing something.
(Similarly for the MLD case.)

      container interfaces {
         config false;

         description
           "Interfaces associated with the IGMP snooping instance";

         list interface {
           key "name";

           description
             "Interfaces associated with the IGMP snooping instance";

Should we consider non-verbatim-equivalent descriptions for the container and the list?  (Likewise for MLD.)

Section 5

It's probably worth a brief note in the "readable data nodes" section about any privacy considerations of exposing multicast group membership (e.g., if IP addresses are associated with users, and multicast groups associated with certain types of content, then there is transitively an association between the user and the content, which could be privacy sensitive).

Section 7.1

It's not entirely clear that RFC 8407 needs to be normative.

It is, however, pretty clear that RFC 8652 does not need to be normative, since we reference it basically to say "we don't need to pull this in".

Section 7.2

RFC 6636 is referenced from the YANG module; doesn't that make it normative?

Appendix A

[Just noting that I did not carefully review the examples.]