Re: [yang-doctors] Please review the update of MSDP YANG model ----- Fw:[pim]Yangdoctors last call review of draft-ietf-pim-msdp-yang-12

Hi Sandy,

I am fine with the change from RPC to action. I haven’t reviewed the latest update, I would suggest that you upload a new revision after the authors have agreed.

Regards,
Reshad.

From: "zhang.zheng@zte.com.cn" <zhang.zheng@zte.com.cn>
Date: Sunday, February 2, 2020 at 11:59 PM
To: "xufeng.liu.ietf@gmail.com" <xufeng.liu.ietf@gmail.com>, "daedulus@btconnect.com" <daedulus@btconnect.com>, "Reshad Rahman (rrahman)" <rrahman@cisco.com>
Cc: "sivakumar.mahesh@gmail.com" <sivakumar.mahesh@gmail.com>, "guofeng@huawei.com" <guofeng@huawei.com>, "anish.ietf@gmail.com" <anish.ietf@gmail.com>, "pete.mcallister@metaswitch.com" <pete.mcallister@metaswitch.com>, "zzhang_ietf@hotmail.com" <zzhang_ietf@hotmail.com>, "pim-chairs@ietf.org" <pim-chairs@ietf.org>, "aretana.ietf@gmail.com" <aretana.ietf@gmail.com>
Subject: Re:Please review the update of MSDP YANG model ----- Fw:[pim]Yangdoctors last call review of draft-ietf-pim-msdp-yang-12

Hi Xufeng,

Thank you very much for your review! Please see my replies inline with [Sandy].

About the change from rpc to action:

6) rpc clear-sa-cache: The rpc used to know which msdp instance to handle when there is only one instance of mspd. Now that the augmentation is on the control-plane-protocol list, we need to identify which msdp instance a rpc request is applied to. I’d suggest to use action instead of rpc. I have added the proposed changes in the YANG file. Please review.

I am not sure if this modification will cause the new round of YANG doctor reviewing. The RPC related parts should also be changed.

Hi Tom, Reshad, how do you think about the change from rpc to action?

Thank you very much for your reply!

Thanks,

Sandy

原始邮件
发件人：XufengLiu <xufeng.liu.ietf@gmail.com>
收件人：张征00007940;
抄送人：Mahesh Sivakumar <sivakumar.mahesh@gmail.com>;Guofeng <guofeng@huawei.com>;Anish Peter <anish.ietf@gmail.com>;Pete McAllister <pete.mcallister@metaswitch.com>;Zhang Z <zzhang_ietf@hotmail.com>;tom p. <daedulus@btconnect.com>;rrahman@cisco.com <rrahman@cisco.com>;pim-chairs@ietf.org <pim-chairs@ietf.org>;Alvaro Retana <aretana.ietf@gmail.com>;
日 期 ：2020年02月03日 08:40
主 题 ：Re: Please review the update of MSDP YANG model ----- Fw:[pim]Yangdoctors last call review of draft-ietf-pim-msdp-yang-12
Hi Sandy,

Thanks for the updates. Here are some items that I noticed. I have also attached proposed YANG file and the corresponding tree file.

1) Container msdp does not need to have “presence”, because all msdp data nodes are under list items now. I took the “presence” statement from YANG.

[Sandy] OK.

2) sa-filter: I feel that the policies here are more “routing policies” than “data plane policies”, so using https://tools.ietf.org/html/draft-ietf-rtgwg-policy-model seems to be better and RFC 8519. I have not made the changes yet.

[Sandy]: The modification to RFC8519 is according the comments from Yingzhen Qu: https://mailarchive.ietf.org/arch/msg/pim/DBhJ5DOt4hfCqy-RKKwHMYp95NA

3) Feature filter-policy is applied to list default-peer, so the naming seems mismatching. Should it be renamed to “default-peer”?

[Sandy] I'd like to provide general policy useage for peer/SA. I made a tiny change in feature description "Support policy configuration of peer/message filtering.". What do you think about it?

4) We have received the comment about “enable” vs “enabled” in other document reviews. Since most RFCs are using “enabled”, we may use “enabled” here too.

[Sandy]: I change the leaf "enable" in "peer-config-attributes" to leaf "enabled".

5) Did some format edits and fixed some end-marks that not matching or not needed for short sections.

[Sandy]: Thank you! I fixed some unmatched end-marks. Please review it.

6) rpc clear-sa-cache: The rpc used to know which msdp instance to handle when there is only one instance of mspd. Now that the augmentation is on the control-plane-protocol list, we need to identify which msdp instance a rpc request is applied to. I’d suggest to use action instead of rpc. I have added the proposed changes in the YANG file. Please review.

7) Would you like to fix the author list by putting your name at the front, since you are doing most of the editing now?

Thanks,
- Xufeng

On Thu, Jan 30, 2020 at 5:53 AM <zhang.zheng@zte.com.cn<mailto:zhang.zheng@zte.com.cn>> wrote:

Hi co-authors,

I updated the MSDP YANG model according to the comments from Tom and Reshad.

IMO the fetures comments from Tom and Reshad are right. Most of the features are generated by implementation experience, not by some other RFCs' definition.

Since these leaves is optional, I deleted most of the features.

Please review the new version.

In case you don't agree to delete the features, please tell me to modify it.

Thank you very much!

Thanks,

Sandy

原始邮件
发件人：ReshadRahmanviaDatatracker <noreply@ietf.org<mailto:noreply@ietf.org>>
收件人：yang-doctors@ietf.org<mailto:yang-doctors@ietf.org> <yang-doctors@ietf.org<mailto:yang-doctors@ietf.org>>;
抄送人：last-call@ietf.org<mailto:last-call@ietf.org> <last-call@ietf.org<mailto:last-call@ietf.org>>;draft-ietf-pim-msdp-yang.all@ietf.org<mailto:draft-ietf-pim-msdp-yang.all@ietf.org> <draft-ietf-pim-msdp-yang.all@ietf.org<mailto:draft-ietf-pim-msdp-yang.all@ietf.org>>;pim@ietf.org<mailto:pim@ietf.org> <pim@ietf.org<mailto:pim@ietf.org>>;
日 期 ：2020年01月29日 11:06
主 题 ：[pim] Yangdoctors last call review of draft-ietf-pim-msdp-yang-12
Reviewer: Reshad Rahman
Review result: Almost Ready

YANG Doctor review of draft-ietf-pim-msdp-yang-12 by Reshad Rahman

1 module in this draft:
- ietf-msdp@2020-01-24.yang

No YANG validation errors or warnings

Thank you for addressing comments which were provided on rev-01 of the document.

- Page 21, the YANG module has the augmentation of
“/rt:…/rt:control-plane-protocol” but there is a missing when statement. This
means that on a device which supports this YANG model, the “msdp” container
node will appear in all instances of “rt:control-plane-protocol”, even the
non-MSDP ones. If you need an example of how to fix this with when statement,
please take a look at OSPF and BFD YANG models.

- There are no examples. Just a couple of simple XML examples would help a lot.

- There are IMO still too many (14?) features for a fairly straight-forward
YANG model. An explanation for this is provided in section 3.1, but this does
not comply with 4.17 of RFC8407:
   The set of YANG features defined in a module should be considered
   carefully.  Very fine granular features increase interoperability
   complexity and should be avoided.  A likely misuse of the feature
   mechanism is the tagging of individual leafs (e.g., counters) with
   separate features.

- Page 12, the if-feature was taken out of password (to address a comment
regarding duplicate if-feature), but I believe the remaining one should be
moved up from case key-chain to container authentication.

- There is an as-number leaf in the YANG model, but no such thing in RFC3618.
Do we need a reference here?

- Page 24, RPC clear-sa-cache has source-addr using type
ipv4-multi-cast-source-address. But in the operational model (container
sa-cache), source-addr is a union of either * or ipv4-address. Why the
difference? Same question, wrt inconsistency, for leaf group
(ipv4-multicast-group-address in RPC and ipv4-address in operational model).

- Page 22, rp-address is of type ip-address, should that be ipv4-address just
like rpf-peer? Or am I misunderstanding this?

- Many of the descriptions are still very terse, e.g. up-time, expire.

- I’m not an MSDP expert, but I believe adding, where appropriate in the YANG
module, more references to the appropriate sections of RFC3618 or other PIM
RFCs would improve the document. e.g. this could help for RPF-related nodes.

- Section 6: s/one new URIs/one new URI/

_______________________________________________
pim mailing list
pim@ietf.org<mailto:pim@ietf.org>
https://www.ietf.org/mailman/listinfo/pim