On 2/27/20 4:27 PM, Mark Michelson wrote: > Acked-by: Mark Michelson <[email protected]> >
Thanks Mark! I had however to send a v2 to fix an additional bug in the same function: https://patchwork.ozlabs.org/patch/1245917/ > Should this be backported to branch-20.03 as well? Yes, if possible. Thanks, Dumitru > > On 2/27/20 9:57 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. >> >> Reported-by: Ying Xu <[email protected]> >> Reported-at: https://bugzilla.redhat.com/1805652 >> Fixes: 79308138891a ("ovn-northd: Add static IP multicast flood >> configuration") >> Signed-off-by: Dumitru Ceara <[email protected]> >> --- >> northd/ovn-northd.c | 25 ++++++++++++++++++------- >> 1 file changed, 18 insertions(+), 7 deletions(-) >> >> diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c >> index d007ba6..e661380 100644 >> --- a/northd/ovn-northd.c >> +++ b/northd/ovn-northd.c >> @@ -3692,7 +3692,7 @@ 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++) { >> @@ -3712,6 +3712,10 @@ ovn_igmp_group_get_ports(const struct >> sbrec_igmp_group *sb_igmp_group, >> continue; >> } >> + if (ports == NULL) { >> + ports = xmalloc(sb_igmp_group->n_ports * sizeof *ports); >> + } >> + >> ports[(*n_ports)] = port; >> ovn_port_find(ovn_ports, >> sb_igmp_group->ports[i]->logical_port); >> if (ports[(*n_ports)]) { >> @@ -10617,6 +10621,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 +10640,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 [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
