[ovs-dev] [PATCH ovn] ovn-northd: Fix IGMP_Group port extraction.

Mark Michelson mmichels at redhat.com
Thu Feb 27 15:27:51 UTC 2020


Acked-by: Mark Michelson <mmichels at redhat.com>

Should this be backported to branch-20.03 as well?

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 <yinxu at redhat.com>
> Reported-at: https://bugzilla.redhat.com/1805652
> Fixes: 79308138891a ("ovn-northd: Add static IP multicast flood configuration")
> Signed-off-by: Dumitru Ceara <dceara at redhat.com>
> ---
>   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);
>       }
>   
> 



More information about the dev mailing list