On Fri, Feb 28, 2020 at 2:08 AM Mark Michelson <mmich...@redhat.com> wrote: > > Acked-by: Mark Michelson <mmich...@redhat.com>
Thanks Dumitru and Mark. I applied this patch to master and branch-20.03 Thanks Numan > > On 2/27/20 10:54 AM, Dumitru Ceara wrote: > > It can happen that all ports on which IGMP/MLD join reports were > > received for a multicast group are already configured to flood multicast > > traffic. In such cases we can safely skip the IGMP_Group record > > completely. Otherwise we risk trying to install an entry in the > > Southbound DB Multicast_Group table with 0 ports, triggering a > > transaction error. > > > > Also, remove the duplicated call to ovn_port_find() in function > > ovn_igmp_group_get_ports() and add a check for NULL port. > > > > Reported-by: Ying Xu <yi...@redhat.com> > > Reported-at: https://bugzilla.redhat.com/1805652 > > Fixes: 79308138891a ("ovn-northd: Add static IP multicast flood > > configuration") > > Signed-off-by: Dumitru Ceara <dce...@redhat.com> > > > > --- > > V2: Remove duplicate call to ovn_port_find(). > > --- > > northd/ovn-northd.c | 34 +++++++++++++++++++++++----------- > > 1 file changed, 23 insertions(+), 11 deletions(-) > > > > diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c > > index d007ba6..9abbec9 100644 > > --- a/northd/ovn-northd.c > > +++ b/northd/ovn-northd.c > > @@ -3692,13 +3692,17 @@ static struct ovn_port ** > > ovn_igmp_group_get_ports(const struct sbrec_igmp_group *sb_igmp_group, > > size_t *n_ports, struct hmap *ovn_ports) > > { > > - struct ovn_port **ports = xmalloc(sb_igmp_group->n_ports * sizeof > > *ports); > > + struct ovn_port **ports = NULL; > > > > *n_ports = 0; > > for (size_t i = 0; i < sb_igmp_group->n_ports; i++) { > > struct ovn_port *port = > > ovn_port_find(ovn_ports, > > sb_igmp_group->ports[i]->logical_port); > > > > + if (!port) { > > + continue; > > + } > > + > > /* If this is already a flood port skip it for the group. */ > > if (port->mcast_info.flood) { > > continue; > > @@ -3712,11 +3716,12 @@ ovn_igmp_group_get_ports(const struct > > sbrec_igmp_group *sb_igmp_group, > > continue; > > } > > > > - ports[(*n_ports)] = port; > > - ovn_port_find(ovn_ports, > > sb_igmp_group->ports[i]->logical_port); > > - if (ports[(*n_ports)]) { > > - (*n_ports)++; > > + if (ports == NULL) { > > + ports = xmalloc(sb_igmp_group->n_ports * sizeof *ports); > > } > > + > > + ports[(*n_ports)] = port; > > + (*n_ports)++; > > } > > > > return ports; > > @@ -10617,6 +10622,18 @@ build_mcast_groups(struct northd_context *ctx, > > continue; > > } > > > > + /* Extract the IGMP group ports from the SB entry. */ > > + size_t n_igmp_ports; > > + struct ovn_port **igmp_ports = > > + ovn_igmp_group_get_ports(sb_igmp, &n_igmp_ports, ports); > > + > > + /* It can be that all ports in the IGMP group record already have > > + * mcast_flood=true and then we can skip the group completely. > > + */ > > + if (!igmp_ports) { > > + continue; > > + } > > + > > /* Add the IGMP group entry. Will also try to allocate an ID for > > it > > * if the multicast group already exists. > > */ > > @@ -10624,12 +10641,7 @@ build_mcast_groups(struct northd_context *ctx, > > ovn_igmp_group_add(ctx, igmp_groups, od, &group_address, > > sb_igmp->address); > > > > - /* Extract the IGMP group ports from the SB entry and store them > > - * in the IGMP group. > > - */ > > - size_t n_igmp_ports; > > - struct ovn_port **igmp_ports = > > - ovn_igmp_group_get_ports(sb_igmp, &n_igmp_ports, ports); > > + /* Add the extracted ports to the IGMP group. */ > > ovn_igmp_group_add_entry(igmp_group, igmp_ports, n_igmp_ports); > > } > > > > > > _______________________________________________ > dev mailing list > d...@openvswitch.org > https://mail.openvswitch.org/mailman/listinfo/ovs-dev > _______________________________________________ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev