Subject: Re: [pim] Suresh Krishnan's No Objection on draft-ietf-pim-igmp-mld-yang-13: (with COMMENT)

From: Liuyisong <[email-masked]>
Date: Thu, 6 Jun 2019 06:17:04 +0000

Hi

        I have gone  through the parameters in IGMP & MLD yang model, and identified  the differences of the parameters based on version.

Thanks
Yisong

From: Xufeng Liu [mailto:[email-masked]]
Sent: Thursday, May 30, 2019 7:19 PM
To: Suresh Krishnan <[email-masked]<mailto:[email-masked]>>
Cc: The IESG <[email-masked]<mailto:[email-masked]>>; [email-masked]<mailto:[email-masked]>; Stig Venaas <[email-masked]<mailto:[email-masked]>>; Alvaro Retana <[email-masked]<mailto:[email-masked]>>; [email-masked]<mailto:[email-masked]>; [email-masked]<mailto:[email-masked]>
Subject: Re: Suresh Krishnan's No Objection on draft-ietf-pim-igmp-mld-yang-13: (with COMMENT)

Hi Suresh,

Thanks for the review and comments. Some of the version check could be done in YANG, while the concern was the complexity added to the model, with a cost to the usability. The authors will examine these cases and address them using either approach as suggested. Maybe some of them use YANG and others use explanation descriptions. In the case of the explanation description, the system can do the validation at the backend and provide feedback.

Thanks,
- Xufeng

On Wed, May 29, 2019 at 6:04 PM Suresh Krishnan via Datatracker <[email-masked]<mailto:[email-masked]>> wrote:
Suresh Krishnan has entered the following ballot position for
draft-ietf-pim-igmp-mld-yang-13: No Objection

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/

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

I do have a general concern with the document in relation to its handling of
multiple protocol versions. There are features in the yang models that should
be conditional but they do not seem to be. Here are some examples.

* The source specific features are to be used with IGMPv3 and MLDv2 and will
not work with the earlier versions * The router alert check is not optional for
MLD or IGMPv3, but is required to be disabled for compatibility with earlier
versions of IGMP. I would also make this feature conditional on the IGMP
version. If not you need to rethink the defaults for this.

I would like to understand the authors' views on how they plan to address the
potential consistency issues due to these features being unbound in the model.
I would be fine if it is either addressed with yang constructs or with some
explanatory text to this point.

---

From: Xufeng Liu <[email-masked]>
Date: Fri, 7 Jun 2019 21:22:22 -0400

Thank Yisong for providing the valuable information. Some of my comments
are:

1. For operational states, we do not need to add constraints. As specified
in RFC8342, semantic constraints MAY be violated in the <operational>
datastore. The server implementations usually do not need to perform such
validations. Moreover, these leaves are not mandatory, so the server does
not need to return any values when the protocol version does not provide.
Such nodes include the “leave” nodes under “statistics”, and the “source”
sub-tree.

2. Based on RFC2236, the IGMPv2 spec requires the presence of the IP Router
Alert option. Should the default value of “require-router-alert” be “true”
for IGMPv2?

3. Should we restrict “ssm-map” sub-tree to IGMPv3/MLDv2 only?

4. We cannot avoid the “source-addr” in the “static-group” sub-tree because
it is a key for the list, but we may add a description to indicate that
(S,G) is only supported by IGMPv3/MLDv2.

Additional proposal:

As the discussions on Benjamin’s review comments, it is proposed to replace
“exclude-lite” with “lite-exclude-filter".

Thanks,

- Xufeng

On Thu, Jun 6, 2019 at 2:17 AM Liuyisong <[email-masked]> wrote:

> Hi
>
>
>
>         I have gone  through the parameters in IGMP & MLD yang model, and
> identified  the differences of the parameters based on version.
>
>
>
> Thanks
>
> Yisong
>
>
>
> *From:* Xufeng Liu [mailto:[email-masked]
> <[email-masked]>]
> *Sent:* Thursday, May 30, 2019 7:19 PM
> *To:* Suresh Krishnan <[email-masked]>
> *Cc:* The IESG <[email-masked]>; [email-masked];
> Stig Venaas <[email-masked]>; Alvaro Retana <[email-masked]>;
> [email-masked]; [email-masked]
> *Subject:* Re: Suresh Krishnan's No Objection on
> draft-ietf-pim-igmp-mld-yang-13: (with COMMENT)
>
>
>
> Hi Suresh,
>
>
>
> Thanks for the review and comments. Some of the version check could be
> done in YANG, while the concern was the complexity added to the model, with
> a cost to the usability. The authors will examine these cases and address
> them using either approach as suggested. Maybe some of them use YANG and
> others use explanation descriptions. In the case of the explanation
> description, the system can do the validation at the backend and provide
> feedback.
>
>
>
> Thanks,
>
> - Xufeng
>
>
>
> On Wed, May 29, 2019 at 6:04 PM Suresh Krishnan via Datatracker <
> [email-masked]> wrote:
>

---

From: Xufeng Liu <[email-masked]>
Date: Sun, 9 Jun 2019 21:38:51 -0400

Attached please find the proposed changes:

* Protocol version checks have been added to the following parameters:
  - last-member-query-interval
  - max-group-sources
  - source-policy
  - explicit-tracking
  - lite-exclude-filter
  - ssm-map
* During the process of adding version checks, some default values were
found problematic and fixed:
  - last-member-query-interval
  - query-interval
  - query-max-response-time
  - require-router-alert
  - robustness-variable
  - version
* Descriptions have adjusted on the following parameters:
  - last-member-query-interval
  - query-interval
  - query-max-response-time
  - require-router-alert
  - robustness-variable
  - version
  - static-group
* “exclude-lite” has be changed to “lite-exclude-filter"

It would be appreciated if these changes can be reviewed.
Thanks,
- Xufeng

On Fri, Jun 7, 2019 at 9:22 PM Xufeng Liu <[email-masked]> wrote:

> 2. Based on RFC2236, the IGMPv2 spec requires the presence of the IP
> Router Alert option. Should the default value of “require-router-alert” be
> “true” for IGMPv2?
>
> 4. We cannot avoid the “source-addr” in the “static-group” sub-tree
> because it is a key for the list, but we may add a description to indicate
> that (S,G) is only supported by IGMPv3/MLDv2.
>
>
> Additional proposal:
>
> As the discussions on Benjamin’s review comments, it is proposed to
> replace “exclude-lite” with “lite-exclude-filter".
>
> Thanks,
>
> - Xufeng
>

---

From: Liuyisong <[email-masked]>
Date: Tue, 11 Jun 2019 01:20:30 +0000

Hi Xufeng

      I agree your comments except the point 3. I think the ssm-map feature is more commonly used for low-version protocol like IGMPv1,v2 and MLDv1 to be compatible with the multicast users, so we cannot restrict the ssm-map feature to only IGMPv3 and MLDv2.

Thanks
Yisong

From: Xufeng Liu [mailto:[email-masked]]
Sent: Saturday, June 08, 2019 9:22 AM
To: Liuyisong <[email-masked]>
Cc: [email-masked]; Guofeng <[email-masked]>
Subject: Re: Suresh Krishnan's No Objection on draft-ietf-pim-igmp-mld-yang-13: (with COMMENT)

Thank Yisong for providing the valuable information. Some of my comments are:

1.       For operational states, we do not need to add constraints. As specified in RFC8342, semantic constraints MAY be violated in the <operational> datastore. The server implementations usually do not need to perform such validations. Moreover, these leaves are not mandatory, so the server does not need to return any values when the protocol version does not provide. Such nodes include the “leave” nodes under “statistics”, and the “source” sub-tree.

2. Based on RFC2236, the IGMPv2 spec requires the presence of the IP Router Alert option. Should the default value of “require-router-alert” be “true” for IGMPv2?

4. We cannot avoid the “source-addr” in the “static-group” sub-tree because it is a key for the list, but we may add a description to indicate that (S,G) is only supported by IGMPv3/MLDv2.

Additional proposal:

As the discussions on Benjamin’s review comments, it is proposed to replace “exclude-lite” with “lite-exclude-filter".

Thanks,

- Xufeng

On Thu, Jun 6, 2019 at 2:17 AM Liuyisong <[email-masked]<mailto:[email-masked]>> wrote:
Hi

Thanks
Yisong

Hi Suresh,

Thanks,
- Xufeng

---

From: Xufeng Liu <[email-masked]>
Date: Mon, 10 Jun 2019 22:35:26 -0400

Hi Yisong,

Thanks for catching the error. I'll remove the constraint on the ssm-map.
Best,
- Xufeng

On Mon, Jun 10, 2019 at 9:20 PM Liuyisong <[email-masked]> wrote:

> Hi Xufeng
>
>
>
>       I agree your comments except the point 3. I think the ssm-map
> feature is more commonly used for low-version protocol like IGMPv1,v2 and
> MLDv1 to be compatible with the multicast users, so we cannot restrict the
> ssm-map feature to only IGMPv3 and MLDv2.
>
>
>
> Thanks
>
> Yisong
>
>
>
> *From:* Xufeng Liu [mailto:[email-masked]]
> *Sent:* Saturday, June 08, 2019 9:22 AM
> *To:* Liuyisong <[email-masked]>
> *Cc:* [email-masked]; Guofeng <[email-masked]>
> 1.       For operational states, we do not need to add constraints. As
> specified in RFC8342, semantic constraints MAY be violated in the
> <operational> datastore. The server implementations usually do not need to
> perform such validations. Moreover, these leaves are not mandatory, so the
> server does not need to return any values when the protocol version does
> not provide. Such nodes include the “leave” nodes under “statistics”, and
> the “source” sub-tree.
>