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

["Rob Wilton (rwilton)" <[e-mail]>]

Hi Hongji,

Having discussed this with Eric, I would also be happy with the suggestion that he proposes (i.e. of having a single combined tree that covers both IGMP snooping and MLD snooping), so it is another option for you to please consider.

Regards,
Rob


> -----Original Message-----
> From: Eric Vyncke (evyncke) <evyncke=[e-mail]>
> Sent: 14 July 2020 12:28
> To: Rob Wilton (rwilton) <[e-mail]>; Hongji Zhao
> <[e-mail]>; The IESG <[e-mail]>;
> [e-mail]
> Cc: Stig Venaas <[e-mail]>; [e-mail]; [e-mail];
> [e-mail]
> Subject: Re: Robert Wilton's Discuss on draft-ietf-pim-igmp-mld-snooping-
> yang-17: (with DISCUSS and COMMENT)
> 
> It seems that similar issues keep repeating for IPv4 and IPv6 (e.g. RFC
> 8652)... Either two trees in one module or two modules (one per address
> family).
> 
> While I am not a YANG expert, I would assume that network operators would
> prefer a unified models (of course with specific leaves & containers) when
> operating a dual-stack network rather than duplicating their automation
> effort.
> 
> As long as IPv6 is covered, I am OK (per my COMMENT on this I-D) but
> unsure whether 2 modules/2 trees should be the 'way to go'... For SNMP,
> RFC 4292/4293 was made protocol version independent which is a big plus
> IMHO for operations.
> 
> -éric
> 
> -----Original Message-----
> From: iesg <[e-mail]> on behalf of "Rob Wilton (rwilton)"
> <rwilton=[e-mail]>
> Date: Tuesday, 14 July 2020 at 10:58
> To: Hongji Zhao <[e-mail]>, The IESG <[e-mail]>,
> "[e-mail]" <[e-mail]>
> Cc: Stig Venaas <[e-mail]>, "[e-mail]" <pim-
> [e-mail]>, "[e-mail]" <[e-mail]>, "draft-ietf-pim-igmp-mld-
> [e-mail]" <[e-mail]>
> Subject: RE: Robert Wilton's Discuss on draft-ietf-pim-igmp-mld-snooping-
> yang-17: (with DISCUSS and COMMENT)
> 
>     Hi Hongji,
> 
>     There are 3 choices here:
> 
>     (1) Put both models in the same YANG module, use common groupings, and
> then use top level features to enable/disable the functionality.  I.e.,
> this is what the draft currently does.
> 
>     Benefits:
>      - One module
>      - Use of groupings ensure the contents are aligned.
> 
>     Downsides:
>      - Undesirable top level IGMP/MLD features
>      - Somewhat unclear description statements (since the descriptions
> reference both IGMP and MLD even when they only apply to one protocol).
> 
> 
>     (2) Split the work into 3 YANG models:
>       (i) Common IGMP/MLD groupings/features, i.e. the same shared
> groupings that you have now.
>       (ii) IGMP snooping module (importing shared groupings)
>       (iii) MLD snooping module (importing shared groupings)
> 
>     Benefits:
>      - Avoids top level IGMP/MLD features
>      - Use of groupings ensures the contents are aligned
> 
>     Downsides:
>       - Somewhat unclear description statements (since the descriptions
> reference both IGMP and MLD even when they only apply to one protocol).
>       - Note, the 'refine' statement could be used where the groupings are
> used (RFC 7950 sec 7.13.2) to potentially improve the descriptions.
> Otherwise possibly some of the description statements could be slightly
> reworded to avoid referencing back to IGMP or MLD.
>       - Some features would need to belong the common module unless the
> data nodes using those features were taken out of the common groupings
> 
>     (3) Split the work into 2 separate modules
>       (i) IGMP snooping module
>       (ii) MLD snooping module
> 
>     Benefits:
>      - Avoids top level IGMP/MLD features
>      - Description statements are clear (since they always only refer to
> the protocol they act on)
>      - Features are controllable for each protocol separately (not sure if
> this is a benefit or a downside)
> 
>     Downsides:
>      - Duplicate definitions, more potential to get out of sync in future,
> although given that they are both defined in the same document, that
> doesn't seem too likely.
> 
> 
>     My preference is for (3).  I believe that makes it much easier/clearer
> to an operator to see whether one/both IGMP and MLD snooping is supported.
> 
>     However, this is just a preference, and I also want to see IETF YANG
> models completed and standardized.  I.e., I will not block progression of
> this document if the authors & Alvaro thinks that the existing module
> structure is sufficient, or choose to do (2) instead.
> 
>     Regards,
>     Rob
> 
> 
> 
>     > -----Original Message-----
>     > From: Hongji Zhao <[e-mail]>
>     > Sent: 14 July 2020 04:26
>     > To: Rob Wilton (rwilton) <[e-mail]>; The IESG
> <[e-mail]>
>     > Cc: [e-mail]; pim-
> [e-mail];
>     > [e-mail]; Stig Venaas <[e-mail]>; [e-mail]
>     > Subject: RE: Robert Wilton's Discuss on draft-ietf-pim-igmp-mld-
> snooping-
>     > yang-17: (with DISCUSS and COMMENT)
>     >
>     > Hi Robert,
>     >
>     > Thanks a lot for your comments.
>     >
>     > In this YANG model, four of six groupings are common for both igmp
>     > snooping and mld snooping.
>     > If it is splitted into two separate YANG models, there will be a lot
> of
>     > duplicated contents.  Is it ok?   Thank you very much.
>     >
>     >
>     >
>     > BR/Hongji
>     >
>     > -----Original Message-----
>     > From: Robert Wilton via Datatracker <[e-mail]>
>     > Sent: Thursday, July 9, 2020 5:55 PM
>     > To: The IESG <[e-mail]>
>     > Cc: [e-mail]; pim-
> [e-mail];
>     > [e-mail]; Stig Venaas <[e-mail]>; [e-mail];
>     > [e-mail]
>     > Subject: Robert Wilton's Discuss on draft-ietf-pim-igmp-mld-
> snooping-yang-
>     > 17: (with DISCUSS and COMMENT)
>     >
>     > Robert Wilton 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:
>     > --------------------------------------------------------------------
> --
>     >
>     > Hi,
>     >
>     > I appreciate that this YANG model has already passed a YANG doctor
> review,
>     > but this discuss is to understand the reasoning as to why both IGMP
>     > snooping and MLD snooping are in the same YANG module, yet have top
> level
>     > features to separate their functionality.
>     >
>     >     4. IGMP and MLD Snooping YANG Module
>     >
>     >       feature igmp-snooping {
>     >         description
>     >           "Support IGMP snooping.";
>     >         reference
>     >           "RFC 4541";
>     >       }
>     >
>     >       feature mld-snooping {
>     >         description
>     >           "Support MLD snooping.";
>     >         reference
>     >           "RFC 4541";
>     >       }
>     >
>     > It seems strange to me to have the entire YANG Model split under two
>     > separate feature statements. I believe that it would have been
> better to
>     > split this into two separate YANG models, both following the same
>     > structure.  Possibly, a common YANG module could have been used to
> share
>     > groupings and definitions, but even then duplicating the contents of
> the
>     > model so that the description statements could be correct/accurate
> would
>     > be more helpful.
>     >
>     >
>     > --------------------------------------------------------------------
> --
>     > COMMENT:
>     > --------------------------------------------------------------------
> --
>     >
>     > Thank you for your work on this document and YANG model.  I also
> have a
>     > few minor comments/suggestions that may improve the document and
> YANG
>     > module:
>     >
>     >     2. Design of Data Model
>     >
>     >     An IGMP/MLD snooping switch [RFC4541] analyzes IGMP/MLD packets
> and
>     > sets
>     >     up forwarding tables for multicast traffic. If a switch does not
> run
>     >     IGMP/MLD snooping, multicast traffic will be flooded in the
> broadcast
>     >     domain. If a switch runs IGMP/MLD snooping, multicast traffic
> will be
>     >     forwarded based on the forwarding tables to avoid wasting
> bandwidth.
>     > The
>     >     IGMP/MLD snooping switch does not need to run any of the
> IGMP/MLD
>     >     protocols. Because the IGMP/MLD snooping is independent of the
>     > IGMP/MLD
>     >     protocols, the data model defined in this document does not
> augment,
>     > or
>     >     even require, the IGMP/MLD data model defined in [RFC8652].
>     >     The model covers considerations for Internet Group Management
> Protocol
>     >     (IGMP) and Multicast Listener Discovery (MLD) Snooping Switches
>     >     [RFC4541].
>     >
>     > It wasn't clear to me what the final sentence was trying to say.
> Perhaps
>     > it should be merged with the penultimate sentence in the paragraph?
>     >
>     >     The YANG module includes IGMP and MLD Snooping instance
> definition,
>     >     using instance in the scenario of BRIDGE [dot1Qcp] and L2VPN
> [draft-
>     >     ietf-bess-l2vpn-yang]. The module also includes actions for
> clearing
>     >     IGMP and MLD Snooping group tables.
>     >
>     > I find the use of the terminology of "scenario of " to be somewhat
>     > strange.  I would probably have referred to these a "L2 forwarding
>     > pradigms" or "L2 forwarding instances".  If this terminology is
> changed
>     > then it would need to be fixed elsewhere in this document and the
> YANG
>     > model.
>     >
>     >     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.
>     >
>     > With NMDA, the server also has the option of not returning a value
> for a
>     > given item of operational data (RFC 8342, section 5,3, paragraph 4).
>     > Although this doesn't conform to the data model, the semantics are
> well
>     > defined - i.e. the client cannot infer anything about the value that
> has
>     > not been returned.
>     >
>     >     2.3. Position of Address Family in Hierarchy
>     >
>     >     IGMP Snooping only supports IPv4, while MLD Snooping only
> supports
>     > IPv6.
>     >     The data model defined in this document can be used for both
> IPv4 and
>     >     IPv6 address families.
>     >
>     >     This document defines IGMP Snooping and MLD Snooping as separate
>     > schema
>     >     branches in the structure. The benefits are:
>     >
>     >     *  The model can support IGMP Snooping (IPv4), MLD Snooping
> (IPv6), or
>     >     both optionally and independently. Such flexibility cannot be
> achieved
>     >     cleanly with a combined branch.
>     >
>     >     *  The separate branches for IGMP Snooping and MLD Snooping can
>     >     accommodate their differences better and cleaner. The two
> branches can
>     >     better support different features and node types.
>     >
>     > I would suggest rewording this first sentence to something like:
>     >
>     > "Having separate branches for IGMP Snooping and MLD Snooping allows
> minor
>     > differences in their  behavior to be modelled more simply and
> cleanly".
>     >
>     >     3. Module Structure
>     >
>     >     A configuration data node is marked as mandatory only when its
> value
>     >     must be provided by the user. Where nodes are not essential to
>     > protocol
>     >     operation, they are marked as optional.  Some other nodes are
>     > essential
>     >     but have a default specified, so that they are also optional and
> need
>     >     not be configured explicitly.
>     >
>     > This paragraph seems to just describe standard YANG modelling and
> can be
>     > removed.
>     >
>     >     3.1. IGMP Snooping Instances
>     >
>     >     The value of scenario in igmp-snooping-instance is bridge or
> l2vpn.
>     > When it
>     >     is bridge, igmp-snooping-instance will be used in the BRIDGE
>     >
>     > As per previous comments, this first sentence does not read well for
> me.
>     >
>     >     The values of bridge-mrouter-interface, l2vpn-mrouter-interface-
> ac,
>     >     l2vpn-mrouter-interface-pw are filled by the snooping device
>     > dynamically.
>     >     They are different from static-bridge-mrouter-interface,
>     >     static-l2vpn-mrouter-interface-ac, and static-l2vpn-mrouter-
> interface-
>     > pw
>     >     which are configured
>     >
>     > Ideally, these static nodes would not have been necessary, instead
> relying
>     > on the NMDA split between configuration and state, but that would
> probably
>     > require the default model to always allow them to be statically
>     > configured.  In NMDA, features can be implemented per-datastore but
> it is
>     > not clear how well that would work here.
>     >
>     >       units one-tenth-second;
>     >
>     > Perhaps "units deciseconds" would be better?
>     >
>     >   grouping igmp-snooping-statistics {
>     >     description
>     >       "The statistics attributes for IGMP snooping.";
>     >
>     >       leaf num-query {
>     >         type yang:counter64;
>     >         description
>     >           "The number of Membership Query messages.";
>     >         reference
>     >           "RFC 2236";
>     >       }
>     >
>     > For these counters, rather than "num-XXX", I think that they would
> be
>     > better as "XXX-count", or if these relate to the number of packets
> "XXX-
>     > pkts" (as per RFC 8343).
>     >
>     >            container statistics {
>     >              description
>     >                "The interface statistics for IGMP snooping";
>     >
>     >              container received {
>     >                description
>     >                  "Statistics of received IGMP snooping packets.";
>     >
>     >                uses igmp-snooping-statistics;
>     >              }
>     >              container sent {
>     >                description
>     >                  "Statistics of sent IGMP snooping packets.";
>     >
>     >                uses igmp-snooping-statistics;
>     >              }
>     >            }
>     >
>     > Should the descriptions for received and sent be for "snooped IGMP
>     > packets"?
>     > The equivalent MLD structure probably also needs a similar fix.
>     >
>     >
>