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

["Acee Lindem (acee)" <[e-mail]>]

Hi Rob, 

See one inline. 

On 12/1/22, 8:36 AM, "pim on behalf of Robert Wilton via Datatracker" <[e-mail] on behalf of [e-mail]> wrote:

    Robert Wilton has entered the following ballot position for
    draft-ietf-pim-igmp-mld-proxy-yang-08: 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/about/groups/iesg/statements/handling-ballot-positions/ 
    for more information about how to handle 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-proxy-yang/



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

    Hi,

    My discuss covers two of comments below that I would like to have some
    discussion on to resolve (further details are in my comments below):

    (1) The addition/default of the "enable" leaf.

Do you recall our discussions on the routing protocols models? We decided on "enabled"
 rather than "enable".  I was in favor of the latter but was in the minority and we'd
already published models using the former. 

Thanks,
Acee

    (2) Name of the interface_name list key rather than just name.

    Regards,
    Rob


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

    Hi,

    Thanks for this document.

    I note the formatting of the .txt file is strange (e.g., my commenting script
    isn't able to parse it because it seems to have extra line breaks in unexpected
    places).  I also note that there is no XML file uploaded.  So, sorry but my
    comments are in a slightly more raw format!

    1. Introduction:

     The "Network Management
    Datastore Architecture" (NMDA) adds the ability to inspect the current
    operational values for configuration, allowing clients to use identical
    paths for retrieving the configured values and the operational values.

    ## I don't think that this paragraph is required.  Other IETF YANG RFCs just
    cite NMDA.

    2. Design of Data Model

     This document provides freedom
    for vendors to adapt the data model to their product implementations.
    For example, some vendors could support configuring IGMP Robustness
    Variable under the interface which enabled IGMP Proxy. They could make
    their own augmentation.

    ## I'm not sure that this paragraph is necessary, since this applies to all
    YANG models and isn't specific to what is defined here.

    2.2. Optional Capabilities

    There is also value in widely supported features being standardized, to
    provide a standardized way to access these features, to save work for
    individual vendors, and so that mapping between different vendors'
    configuration is not needlessly complicated. Therefore, this model
    declares a number of features representing capabilities that not all
    deployed devices support.

    # I don't think that this paragraph is accurate for the YANG contained in this
    draft.  The model below only defines two features, one covering IGMP Proxy and
    one covering MLD Proxy.

    The extensive use of feature declarations should also substantially
    simplify the capability negotiation process for a vendor's IGMP / MLD
    Proxy implementations.

    # Again, I don't think that this paragraph is accurate and should be removed.

    The YANG data model defined in this document conforms to the Network
    Management Datastore Architecture (NMDA) [RFC8342]. The operational
    state data is combined with the associated configuration data in the
    same hierarchy [RFC8407].

    ## I think that this paragraph is redundant and can be removed.

    The igmp-version represents version of IGMP protocol, and default value
    is 2.

    ### represents version -> represents the version, and default -> and the default

     If the value of enable is true, it means IGMP Proxy is enabled.

    ## I would make this a separate paragraph.  I'm also not entirely sure why we
    need this leaf, since I would expect IGMP proxy to be enabled on an interface
    simply because an interface entry exists in the list.   Please can you give me
    an explanation as to why it needed, and if it is needed whether it would be
    better to default to true rather than false.

              leaf interface-name {
                type if:interface-ref;
                must "not( current() = /rt:routing"+
                  "/rt:control-plane-protocols/pim-base:pim"+
                  "/pim-base:interfaces/pim-base:interface"+
                  "/pim-base:name )" {
                  description
                    "The upstream interface for IGMP proxy
                     must not be configured to use PIM.";
                }
                description "The upstream interface name.";

    # It would be more consistent with other IETF models to just use "name" for the
    interface name.  Is there a good reason to not be consistent here?  Both here,
    and for downstream-interface and also for MLD.



    _______________________________________________
    pim mailing list
    [e-mail]
    https://www.ietf.org/mailman/listinfo/pim