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

[Alvaro Retana <[e-mail]>]

Rob:

Hi!

The authors published a revision (-09), can you please take a look and
confirm your concerns have been addressed?

Thanks!

Alvaro.


The IETF datatracker page for this Internet-Draft is:
https://datatracker.ietf.org/doc/draft-ietf-pim-igmp-mld-proxy-yang/

Diff from previous version:
https://author-tools.ietf.org/iddiff?url2=draft-ietf-pim-igmp-mld-proxy-yang-09




On December 5, 2022 at 11:14:06 PM, Hongji Zhao ([e-mail])
wrote:

Hi Robert,

Thanks a lot for your comments. Please check inline.

BR/Hongji

-----Original Message-----
From: Robert Wilton via Datatracker <[e-mail]>
Sent: 2022年12月1日 21:36
To: The IESG <[e-mail]>
Cc: [e-mail]; [e-mail];
[e-mail]; [e-mail]; [e-mail]; [e-mail]
Subject: Robert Wilton's Discuss on draft-ietf-pim-igmp-mld-proxy-yang-08:
(with DISCUSS and COMMENT)

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.
[Authors] It could provide an option, and the customer could choose true or
false flexibly. It is similar as IGMP YANG model, OSPF YANG model etc. In
order to be consistent with other IETF models, the leaf needs to be changed
to "enabled". The default value will be true.

(2) Name of the interface_name list key rather than just name.
[Authors] Accepted. We will change "interface_name" into "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!
[Authors] I used WORD to edit, and then print out .txt file. So there may
be some format issues. The RFC editor will create a new XML file and solve
the format issues.

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.
[Authors] Accepted. We will remove it.

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.
[Authors] We will remove it.

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.
[Authors] How about reword as below?
2.2 Optional Features
This model declares two features representing capabilities that not all
deployed devices support. One feature is igmp-proxy, and the other feature
is mld-proxy. Either or both features could be implemented, which could
provide more choices for vendors.


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.
[Authors] Accepted

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.
[Authors] Accepted

The igmp-version represents version of IGMP protocol, and default value
is 2.
### represents version -> represents the version, and default -> and the
default
[Authors] Accepted

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.
[Authors] It could provide an option, and the customer could choose true or
false flexibly. It is similar as IGMP YANG model, OSPF YANG model etc. In
order to be consistent with other IETF models, the leaf needs to be changed
to "enabled". The default value will be true.


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.
[Authors] Accepted. We will change all the "interface_name" into "name".