Subject: Re: [pim] Hi Reshad, the issues about igmp snooping model are addressed. Thanks a lot! Re: [yang-doctors] Yangdoctors early review of draft-ietf-pim-igmp-mld-snooping-yang-03

From: Hongji Zhao <[email-masked]>
Date: Tue, 11 Sep 2018 02:18:13 +0000

Hi Reshad,

Thanks for your reply.

In IGMP Snooping, the mrouter interface (i.e. Multicast router interface)  is the interface which connects to the multicast router. So there may be more than one mrouter interfaces in a l2vpn-instance or BRIDGE.
As you say,  an interface refers to l2vpn-instance and l2vpn-instance has lists of ACs and PWs for the same mrouter-interface functionality.

Here is how to configure and show Internet Group Management Protocol (IGMP) snooping for IPv4 multicast traffic on the Cisco 7600 series routers.
Gi1/1,Gi2/1,Fa3/48 are all the mrouter interfaces in vlan 1.

Displaying Multicast Router Interfaces
When you enable IGMP snooping, the router automatically learns to which interface the multicast
routers are connected.
To display multicast router interfaces, perform this task:
This example shows how to display the multicast router interfaces in VLAN 1:
Router# show ip igmp snooping mrouter vlan 1
vlan ports
-----+----------------------------------------
1 Gi1/1,Gi2/1,Fa3/48,Router

In the current igmp snooping model, we can configure static-mrouter-interface and static-l2-multicast-group either under the igmp-snooping-instance or under the augment /if:interfaces/if:interface. The purpose is to provide the customer more than one way to configure static-mrouter-interface and static-l2-multicast-group.
Maybe it looks in a mess.
If removing the static-mrouter-interface and static-l2-multicast-group under augment /if:interfaces/if:interface is better, I can remove them.

BR/Hongji
赵宏吉

From: Reshad Rahman (rrahman) <[email-masked]>
Sent: Monday, September 10, 2018 6:25 AM
To: Hongji Zhao <[email-masked]>
Cc: [email-masked]; [email-masked]; YANG Doctors <[email-masked]>
Subject: Re: Hi Reshad, the issues about igmp snooping model are addressed. Thanks a lot! Re: [yang-doctors] Yangdoctors early review of draft-ietf-pim-igmp-mld-snooping-yang-03

Hi Hongji,

I’ve taken a look at the latest rev, thanks for changing the model to augment l2vpn-instance and bridge, I believe this improves the model substantially. Before posting review comments on the latest version, I’d like to get clarifications/closure on the following (this was my 2nd main comment from initial review, email attached).

With the augment of if:interface, we have IGMP/MLD snooping data both under l2vpn-instance or bridge (via the augment) and under if:interface. To me such a segmentation of the IGMP/MLD snooping data is undesirable, is it not possible to have the data for static-mrouter-interface and static-l2-multicast-group under l2vpn-instance or bridge? For example, looking at static-mrouter-interface, we have leaf-lists of ACs/PWs and then under interface we have an l2vpn-instance name (should be leafref btw), so looks like interface refers to l2vpn-instance and l2vpn-instance has lists of ACs and PWs for the same mrouter-interface functionality. I don’t get it.

Regards,
Reshad.

2)            If:interface is augmented and has the name of the l2vpn-instance. This config seems redundant since under l2vpn-instance (draft-ietf-bess-l2vpn-yang)
there is already an interface-ref for AC (Access Circuit). Why not augment the L2VPN endpoint or AC?
--------Augmenting if:interface in this model is used to configure static multicast router interface (static-mrouter-interface) and static l2 multicast forward table.
                    They are the concepts in IGMP/MLD Snooping. Augmenting if:interface could cover BRIDGE and L2VPN scenario at the same time.

From: Hongji Zhao <[email-masked]<mailto:[email-masked]>>
Date: Monday, August 6, 2018 at 11:36 PM
To: "Reshad Rahman (rrahman)" <[email-masked]<mailto:[email-masked]>>
Cc: "[email-masked]<mailto:[email-masked]>" <[email-masked]<mailto:[email-masked]>>, "[email-masked]<mailto:[email-masked]>" <[email-masked]<mailto:[email-masked]>>
Subject: Hi Reshad, the issues about igmp snooping model are addressed. Thanks a lot! Re: [yang-doctors] Yangdoctors early review of draft-ietf-pim-igmp-mld-snooping-yang-03

Hi Reshad,

I have updated the draft-ietf-pim-igmp-mld-snooping-yang according to your comments and upload the latest version.

In the new version I augmented the l2vpn-instance (from draft-ietf-bess-l2vpn-yang) and ieee802-dot1q-bridge module instead of the old reference style. I also addressed your other comments.

Could you please review it again? Thanks a lot!

https://datatracker.ietf.org/doc/draft-ietf-pim-igmp-mld-snooping-yang/

BR/Hongji

赵宏吉

FYI, easier to read https://datatracker.ietf.org/doc/review-ietf-pim-igmp-mld-snooping-yang-03-yangdoctors-early-rahman-2018-06-28/ than the email below.

On 2018-06-28, 5:42 PM, "yang-doctors on behalf of Reshad Rahman" <[email-masked] on behalf of rrahman=[email-masked]<mailto:[email-masked]%20on%20behalf%20of%20rrahman=[email-masked]>> wrote:

    Reviewer: Reshad Rahman

    Review result: On the Right Track

    YANG Doctor review of draft-ietf-pim-igmp-mld-snooping-yang-03 (by Reshad

    Rahman)

1)      The YANG model has a new container+list for bridges and

    “l2vpn-instances”.  Why not augment l2vpn-instance (from

    draft-ietf-bess-l2vpn-yang)? …

    I will have to re-review once the issues are addressed.

---

From: "Reshad Rahman (rrahman)" <[email-masked]>
Date: Thu, 13 Sep 2018 18:21:31 +0000

Hi Hongji,

Yes I believe it’s best to take out the augment of if:interfaces and have everything in the l2vpn-instance or bridge. Also, right now I believe  the model is inconsistent because PWs which can be used for mrouter-interface do not have any config outside of l2vpn-instance (but AC interfaces used as mrouter-interface do have mrouter config under if:interface ).

Regards,
Reshad.

From: Hongji Zhao <[email-masked]>
Date: Monday, September 10, 2018 at 10:18 PM
To: "Reshad Rahman (rrahman)" <[email-masked]>
Cc: "[email-masked]" <[email-masked]>, "[email-masked]" <[email-masked]>, YANG Doctors <[email-masked]>
Subject: RE: Hi Reshad, the issues about igmp snooping model are addressed. Thanks a lot! Re: [yang-doctors] Yangdoctors early review of draft-ietf-pim-igmp-mld-snooping-yang-03

Hi Reshad,

Thanks for your reply.

BR/Hongji
赵宏吉

Hi Hongji,

Regards,
Reshad.

Hi Reshad,

BR/Hongji

赵宏吉

    Reviewer: Reshad Rahman

    Rahman)

---

From: Hongji Zhao <[email-masked]>
Date: Fri, 14 Sep 2018 03:46:59 +0000

Hi Reshad,

Do you mean it’s best to take out the whole augmentation of if:interfaces?

And besides it, do you have any other comments about the latest version of igmp snooping model?

Thanks a lot!

BR/Hongji
赵宏吉

Sent: Friday, September 14, 2018 2:22 AM

Hi Hongji,

Regards,
Reshad.

Cc: "[email-masked]<mailto:[email-masked]>" <[email-masked]<mailto:[email-masked]>>, "[email-masked]<mailto:[email-masked]>" <[email-masked]<mailto:[email-masked]>>, YANG Doctors <[email-masked]<mailto:[email-masked]>>

Hi Reshad,

Thanks for your reply.

BR/Hongji
赵宏吉

From: Reshad Rahman (rrahman) <[email-masked]<mailto:[email-masked]>>
Sent: Monday, September 10, 2018 6:25 AM
To: Hongji Zhao <[email-masked]<mailto:[email-masked]>>
Cc: [email-masked]<mailto:[email-masked]>; [email-masked]<mailto:[email-masked]>; YANG Doctors <[email-masked]<mailto:[email-masked]>>
Subject: Re: Hi Reshad, the issues about igmp snooping model are addressed. Thanks a lot! Re: [yang-doctors] Yangdoctors early review of draft-ietf-pim-igmp-mld-snooping-yang-03

Hi Hongji,

Regards,
Reshad.

Hi Reshad,

BR/Hongji

赵宏吉

    Reviewer: Reshad Rahman

    Rahman)

---

From: "Reshad Rahman (rrahman)" <[email-masked]>
Date: Fri, 14 Sep 2018 12:48:06 +0000

Hi Hongji,

Yes I don’t see the purpose of augmenting if:interfaces, I believe all mrouter related data could be in l2vpn-instance or bridge. If this is not the case I’d like to understand why.

I have no further major comments, will do detailed review once this major comment is resolved.

Regards,
Reshad.

Date: Thursday, September 13, 2018 at 11:47 PM

Hi Reshad,

Thanks a lot!

BR/Hongji
赵宏吉

Hi Hongji,

Regards,
Reshad.

Hi Reshad,

Thanks for your reply.

BR/Hongji
赵宏吉

Hi Hongji,

Regards,
Reshad.

Hi Reshad,

BR/Hongji

赵宏吉

    Reviewer: Reshad Rahman

    Rahman)

---

From: Hongji Zhao <[email-masked]>
Date: Tue, 18 Sep 2018 02:28:49 +0000

Hi Reshad,

In augment /if:interfaces/if:interface, besides mrouter related data, there are still some statistics related with igmp-mld-snooping.
If take out the whole augmentation of if:interfaces, the statistics will be lost.  What do you think of that?

augment /if:interfaces/if:interface:
    +--rw igmp-mld-snooping
       +--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?                    yang:counter64
          +--ro sent
             +--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?                    yang:counter64

BR/Hongji
赵宏吉

Sent: Friday, September 14, 2018 8:48 PM

Hi Hongji,

Regards,
Reshad.

Hi Reshad,

Thanks a lot!

BR/Hongji
赵宏吉

Hi Hongji,

Regards,
Reshad.

Hi Reshad,

Thanks for your reply.

BR/Hongji
赵宏吉

---

From: "Reshad Rahman (rrahman)" <[email-masked]>
Date: Sun, 9 Sep 2018 22:24:59 +0000

Hi Hongji,

Regards,
Reshad.

Cc: "[email-masked]" <[email-masked]>, "[email-masked]" <[email-masked]>

Hi Reshad,

BR/Hongji

赵宏吉

    Reviewer: Reshad Rahman

    Rahman)

Subject: Some explanations for you questions. Thanks! RE: [yang-doctors] Yangdoctors early review of draft-ietf-pim-igmp-mld-snooping-yang-03
From: Hongji Zhao <[email-masked]>
To: "Reshad Rahman (rrahman)" <[email-masked]>
Cc: "[email-masked]" <[email-masked]>, "[email-masked]" <[email-masked]>, "[email-masked]" <[email-masked]>, Xufeng Liu <[email-masked]>, Liuyisong <[email-masked]>
Date: Tue, 3 Jul 2018 06:54:15 +0000

Hi Reshad,

Below are the explanations for major issues.  Thank you very much! 

1)            The YANG model has a new container+list for bridges and “l2vpn-instances”.  Why not augment l2vpn-instance (from draft-ietf-bess-l2vpn-yang)? 

If all L2 features end up adding their own lists for “l2vpn-instances” this will be messy and there’ll be no easy way to look at all the configuration relevant to 

an l2vpn-instance, it’ll have to be done feature by feature.

--------  There are 2 reasons for not augment l2vpn-instance

   a) If augment l2vpn-instance, the igmp-snooping-instances need to defined as grouping. This igmp snooping model will not be an independent model, and couldn't be augmented furtherly;

   b) If augment l2vpn-instance and BRIDGE, this draft will rely on l2vpn-instance and BRIDGE.

 

3)            There doesn’t seem to be the capability to enable IGMP/MLD snooping on a subset of ACs or PW (i.e. not on the full l2vpn-instance)?

--------Generally IGMP/MLD Snooping is corresponding to l2vpn-instance. They are in the same level. AC or pw is lower than l2vpn-instance.

4)            I thought Bridge related YANG models belong to IEEE. But if we have to do the model for bridges in this draft, why not augment IEEE YANG models 

e.g.  ieee802-dot1q-bridge.yang (same comment as for l2vpn-instance)?

--------It is similar as Question 1.

- Section 2.2, 2nd paragraph needs rewording. Explanation of how reference also not super clear (add reference to 2.4?) , e.g. what does an igmp-snooping-instance correspond to

(to me it seems to be more a profile than the instances we have with routing protocols)? And is 1 instance usable in multiple l2vpn or BRIDGE instances? I believe it’s for 1 instance? 

 Anyway clarify that.

  ----------------There are config data and stata data in igmp-snooping-instance. So I think it is an instance. Generally there is no state data in profile.

                                                                  1 instance is usable in multiple l2vpn or BRIDGE instances, but we don't recommend.

BR/Hongji

赵宏吉

-----Original Message-----
From: Reshad Rahman (rrahman) <[email-masked]> 
Sent: Friday, June 29, 2018 5:57 AM
To: [email-masked]; [email-masked]
Cc: [email-masked]
Subject: Re: [yang-doctors] Yangdoctors early review of draft-ietf-pim-igmp-mld-snooping-yang-03

On 2018-06-28, 5:42 PM, "yang-doctors on behalf of Reshad Rahman" <[email-masked]  on behalf of rrahman=[email-masked]> wrote:

    Reviewer: Reshad Rahman

    Rahman)

    1 module defined in this draft:

    - [email-masked]

    No YANG validation errors or warnings (from yang and yanglint).

    1 example are provided in this draft.

    Major issues perceived:

    draft-ietf-bess-l2vpn-yang)? If all L2 features end up adding their own lists

    for “l2vpn-instances” this will be messy and there’ll be no easy way to look at

    all the configuration relevant to an l2vpn-instance, it’ll have to be done

    feature by feature. 2)      If:interface is augmented and has the name of the

    l2vpn-instance. This config seems redundant since under l2vpn-instance

    (draft-ietf-bess-l2vpn-yang) there is already an interface-ref for AC (Access

    Circuit). Why not augment the L2VPN endpoint or AC? 3)      There doesn’t seem

    to be the capability to enable IGMP/MLD snooping on a subset of ACs or PW (i.e.

    not on the full l2vpn-instance)? 4)      I thought Bridge related YANG models

    belong to IEEE. But if we have to do the model for bridges in this draft, why

    not augment IEEE YANG models e.g.  ieee802-dot1q-bridge.yang (same comment as

    for l2vpn-instance)?

    There might be good reasons to justify the way the YANG model has been done,

    but if that's the case IMO there needs to be text which justifies the design of

    the YANG model.

    If the authors haven’t done so already I would suggest discussing with authors

    of draft-ietf-bess-l2vpn-yang, IETF102 would be a good opportunity and I can

    attend a meeting if needed.

    Other comments/questions/nits:

    - General: needs spelling verification

    - General: indentation of YANG model has to be fixed, also some descriptions

    are too long and wrap. - Add NMDA in abstract (that's what most drafts now do)

    - Section 1.1, add space after in "in[RFC6020]" - There are references for

    L2VPN/EVPN YANG but none for bridges - Section 2, add reference for IGMP -

    Section 2.2, 2nd paragraph needs rewording. Explanation of how reference also

    not super clear (add reference to 2.4?) , e.g. what does an

    igmp-snooping-instance correspond to (to me it seems to be more a profile than

    the instances we have with routing protocols)? And is 1 instance usable in

    multiple l2vpn or BRIDGE instances? I believe it’s for 1 instance? Anyway

    clarify that. - Section 2.2, 4th paragraph, instead of “routing system” should

    this be “snooping device”? - Section 2.5 "This model augment", should be

    "augments". - YANG model: s/to configure the igmp snooping/to configure IGMP

    snooping/ - YANG model, having a feature for supporting admin-enable seems like

    overkill. My first impression was that that's a lot of features for this model,

    but I guess that's debatable. - YANG model s/fowarding/forwarding/ - YANG

    model, for the lookup modes (IP-based and MAC-based, add reference). I don’t

    think adding a vendor-specific CLI-example in the YANG description is a good

    idea. - YANG model, use yang-version 1.1 and add reference to import statements

    (as per 6087bis) - YANG model, if per-instance-config feature is not supported,

    how are the IGMP/MLD instances configured? - YANG model, vlan-index-type, use

    of range 4096… not very clear. And vlan-id shouldn’t be uint32, uint16 is

    enough. There’s also ieee:vlanid - YANG model, as opposed to using regular

    address type for group/multicast addresses, is there a type already defined for

    group addresses? If not there should be (V4 and V6) - YANG model,

    host-filter-mode, add reference - Appendix A, fix diagram

    _______________________________________________

    yang-doctors mailing list

    [email-masked]

    https://www.ietf.org/mailman/listinfo/yang-doctors

---

From: "Reshad Rahman (rrahman)" <[email-masked]>
Date: Thu, 20 Sep 2018 12:26:00 +0000

Hi Hongji,

It looks like those statistics are needed only for interfaces (not for PWs) , so you could keep them under augment of if:interface, or put those stats under bridge-mrouter-interface and l2vpn-mrouter-interface-ac in the snooping-instance. My preference is for the latter.

Regards,
Reshad.

Date: Monday, September 17, 2018 at 10:28 PM

Hi Reshad,

BR/Hongji
赵宏吉

Hi Hongji,

Regards,
Reshad.

Hi Reshad,

Thanks a lot!

BR/Hongji
赵宏吉

Hi Hongji,

Regards,
Reshad.

Hi Reshad,

Thanks for your reply.

BR/Hongji
赵宏吉