Re: [pim] Yangdoctors early review of draft-ietf-pim-igmp-mld-snooping-yang-05

[Hongji Zhao <[e-mail]>]

Hi Reshad,

Regarding the main issues/questions:

  *     igmp-snooping-instances (mld- also) are top level containers, I believe they should be under rt:control-plane-protocol (RFC8349). I should have raised this in the previous review.
        ----I would like to change the structure like this. What do you think of that?
augment /rt:routing/rt:control-plane-protocols/rt:control-plane-protocol:
                    +--rw igmp-snooping-instance* [name]
                             +--rw name                                 string
                        ...
                    +--rw mld-snooping-instance* [name]
                               +--rw name                                 string
                ...


Minor comments, suggestions and nits:
-              Leaf exclude-lite, in description should there an explanation or reference of what exclude lite means?
            ------------- I would like to change the description like this. What do you think of that?
                     leaf exclude-lite {
                               …
                               description
                                 "Track the Lightweight IGMPv3 and MLDv2 protocol report";
                               reference "RFC5790";
                     }

-              Grouping statistics-sent-received:
4.            Counter names, e.g. “pim” is too short, it should be something along the lines of num-pim-messages or num-pim.
-------------Is  pim-hello ok? Here we just want to count the pim hello packet which is related to multicast router interface.
              +--ro statistics
    |              +--ro received
    |              |  +--ro query?                  yang:counter64
    |              |  +--ro membership-report-v1?   yang:counter64
    |              |  +--ro membership-report-v2?   yang:counter64
    |              |  +--ro membership-report-v3?   yang:counter64
    |              |  +--ro leave?                  yang:counter64
    |              |  +--ro non-member-leave?       yang:counter64
    |              |  +--ro pim-hello?              yang:counter64

-              RPCs clear-xxx-snooping-groups, rename to clear-xxx-snooping-cache (as per description)?
              ----------------what about modify the description? I think clear-xxx-snooping-groups is better.
                                rpc clear-igmp-snooping-groups {
                                 if-feature rpc-clear-groups;
                                  description
                                   "Clear the specified IGMP snooping group.";
  input {
…

-              Examples (Appendix A)
2.            Were the examples validated with a tool? You can use yanglib
-------------- I used json-yang-master to validate the examples, but didn’t use libyang.
                How to use libyang? Do you mean the yanglint?  Do you have some example about it?


Regarding other comments I have addressed and I will update the draft together with the comments above.  Thanks a lot!


BR/Hongji
赵宏吉