| Message ID | 20240315105606.107757-2-mheib@redhat.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [ovs-dev,1/2] northd: Don't skip transit switch LSP when creating mcast groups. | expand |
| Context | Check | Description |
|---|---|---|
| ovsrobot/apply-robot | warning | apply and check: warning |
| ovsrobot/github-robot-_Build_and_Test | fail | github build: failed |
| ovsrobot/github-robot-_ovn-kubernetes | success | github build: passed |
Bleep bloop. Greetings Mohammad Heib, I am a robot and I have tried out your patch. Thanks for your contribution. I encountered some error that I wasn't expecting. See the details below. checkpatch: WARNING: The subject, '<area>: <summary>', is over 70 characters, i.e., 82. Subject: IC: Tansit switch don't flood mcast traffic to router ports if matches igmp group. Lines checked: 105, Warnings: 1, Errors: 0 Please check this out. If you feel there has been an error, please email aconole@redhat.com Thanks, 0-day Robot
On 3/15/24 11:56, Mohammad Heib wrote: > Crrently ovn transit switch forward mcast traffic that match an igmp > group to all ports participating in this group and to all router ports > that are connected to this TS switch and have mcast_flood enabled. > > The above behavior can lead to packet duplicate if we have a VM in > a specific AZ that participates in igmp group because the gateway router > in this AZ will forward igmp membership report from the VM to the TS > which will be learned as an IGMP_group on the Tansit switch in different > AZs and every mcast traffic to that igmp group address from the different > AZs will be handled by the Tansit switch twice: > - First time TS will send the traffic according to the igmp group > which will reach the VM. > > - Second time TS will send the traffic to all router ports including > the router that exists on the VM AZ which will forward the traffic > to the VM again. > > To avoid this issue this patch adds flows that forward mcast traffic > that match igmp group to the igmp group ports only, this flows only > apply to Transit switches. > > Rreported-at: https://issues.redhat.com/browse/FDP-101 > Signed-off-by: Mohammad Heib <mheib@redhat.com> > --- Thanks, Mohammad! Applied to main and backported all the way to 23.06. Regards, Dumitru
diff --git a/northd/northd.c b/northd/northd.c index 98c837a20..0c5122d27 100644 --- a/northd/northd.c +++ b/northd/northd.c @@ -9362,8 +9362,14 @@ build_lswitch_destination_lookup_bmcast(struct ovn_datapath *od, } -/* Ingress table 25: Add IP multicast flows learnt from IGMP/MLD - * (priority 90). */ +/* Ingress table 27: Add IP multicast flows learnt from IGMP/MLD + * (priority 90). + * + * Ingress table 27: Transit switch add IP multicast flows learnt + * from IGMP/MLD to forward traffic explicitly to the ports that are + * part of the IGMP/MLD group, and ignore MROUTERAS Ports. + * (priority 95). + */ static void build_lswitch_ip_mcast_igmp_mld(struct ovn_igmp_group *igmp_group, struct lflow_table *lflows, @@ -9377,6 +9383,9 @@ build_lswitch_ip_mcast_igmp_mld(struct ovn_igmp_group *igmp_group, ds_clear(match); ds_clear(actions); + bool transit_switch = + ovn_datapath_is_transit_switch(igmp_group->datapath); + struct mcast_switch_info *mcast_sw_info = &igmp_group->datapath->mcast_info.sw; uint64_t table_size = mcast_sw_info->table_size; @@ -9422,7 +9431,7 @@ build_lswitch_ip_mcast_igmp_mld(struct ovn_igmp_group *igmp_group, } /* Also flood traffic to all multicast routers with relay enabled. */ - if (mcast_sw_info->flood_relay) { + if (mcast_sw_info->flood_relay && !transit_switch) { ds_put_cstr(actions, "clone { " "outport = \""MC_MROUTER_FLOOD "\"; " @@ -9440,7 +9449,8 @@ build_lswitch_ip_mcast_igmp_mld(struct ovn_igmp_group *igmp_group, igmp_group->mcgroup.name); ovn_lflow_add(lflows, igmp_group->datapath, S_SWITCH_IN_L2_LKUP, - 90, ds_cstr(match), ds_cstr(actions), NULL); + transit_switch? 95 : 90, ds_cstr(match), + ds_cstr(actions), NULL); } } diff --git a/northd/ovn-northd.8.xml b/northd/ovn-northd.8.xml index 17b414144..e25285d67 100644 --- a/northd/ovn-northd.8.xml +++ b/northd/ovn-northd.8.xml @@ -1933,6 +1933,13 @@ output; logical switch. </li> + <li> + Priority-95 flows for transit switches only that forward registered + IP multicast traffic to their corresponding multicast group , which + <code>ovn-northd</code> creates based on learnt + <ref table="IGMP_Group" db="OVN_Southbound"/> entries. + </li> + <li> Priority-90 flows that forward registered IP multicast traffic to their corresponding multicast group, which <code>ovn-northd</code>
Crrently ovn transit switch forward mcast traffic that match an igmp group to all ports participating in this group and to all router ports that are connected to this TS switch and have mcast_flood enabled. The above behavior can lead to packet duplicate if we have a VM in a specific AZ that participates in igmp group because the gateway router in this AZ will forward igmp membership report from the VM to the TS which will be learned as an IGMP_group on the Tansit switch in different AZs and every mcast traffic to that igmp group address from the different AZs will be handled by the Tansit switch twice: - First time TS will send the traffic according to the igmp group which will reach the VM. - Second time TS will send the traffic to all router ports including the router that exists on the VM AZ which will forward the traffic to the VM again. To avoid this issue this patch adds flows that forward mcast traffic that match igmp group to the igmp group ports only, this flows only apply to Transit switches. Rreported-at: https://issues.redhat.com/browse/FDP-101 Signed-off-by: Mohammad Heib <mheib@redhat.com> --- northd/northd.c | 18 ++++++++++++++---- northd/ovn-northd.8.xml | 7 +++++++ 2 files changed, 21 insertions(+), 4 deletions(-)