[ovs-dev] [PATCH v3 3/3] OVN: Add ovn-northd IGMP support

Dumitru Ceara dceara at redhat.com
Thu Jul 11 15:20:58 UTC 2019


On Thu, Jul 11, 2019 at 10:57 AM Dumitru Ceara <dceara at redhat.com> wrote:
>
> New IP Multicast Snooping Options are added to the Northbound DB
> Logical_Switch:other_config column. These allow enabling IGMP snooping and
> querier on the logical switch and get translated by ovn-northd to rows in
> the IP_Multicast Southbound DB table.
>
> ovn-northd monitors for changes done by ovn-controllers in the Southbound DB
> IGMP_Group table. Based on the entries in IGMP_Group ovn-northd creates
> Multicast_Group entries in the Southbound DB, one per IGMP_Group address X,
> containing the list of logical switch ports (aggregated from all controllers)
> that have IGMP_Group entries for that datapath and address X. ovn-northd
> also creates a logical flow that matches on IP multicast traffic destined
> to address X and outputs it on the tunnel key of the corresponding
> Multicast_Group entry.
>
> Signed-off-by: Dumitru Ceara <dceara at redhat.com>
> Acked-by: Mark Michelson <mmichels at redhat.com>
> ---
>  ovn/northd/ovn-northd.c |  460 ++++++++++++++++++++++++++++++++++++++++++++---
>  ovn/ovn-nb.xml          |   54 ++++++
>  tests/ovn.at            |  270 ++++++++++++++++++++++++++++
>  tests/system-ovn.at     |  119 ++++++++++++
>  4 files changed, 871 insertions(+), 32 deletions(-)
>

<snip>

> +
> +static void
> +build_mcast_groups(struct northd_context *ctx,
> +                   struct hmap *datapaths, struct hmap *ports,
> +                   struct hmap *mcast_groups,
> +                   struct hmap *igmp_groups)
> +{
> +    struct ovn_port *op;
> +
> +    hmap_init(mcast_groups);
> +    hmap_init(igmp_groups);
> +
> +    HMAP_FOR_EACH (op, key_node, ports) {
> +        if (!op->nbsp) {
> +            continue;
> +        }
> +
> +        if (lsp_is_enabled(op->nbsp)) {
> +            ovn_multicast_add(mcast_groups, &mc_flood, op);
> +        }
> +    }
> +
> +    const struct sbrec_igmp_group *sb_igmp, *sb_igmp_next;
> +
> +    SBREC_IGMP_GROUP_FOR_EACH_SAFE (sb_igmp, sb_igmp_next, ctx->ovnsb_idl) {
> +        /* If this is a stale group (e.g., controller had crashed,
> +         * purge it).
> +         */
> +        if (!sb_igmp->chassis || !sb_igmp->datapath) {
> +            sbrec_igmp_group_delete(sb_igmp);
> +            continue;
> +        }
> +
> +        struct ovn_datapath *od =
> +            ovn_datapath_from_sbrec(datapaths, sb_igmp->datapath);
> +        if (!od) {
> +            sbrec_igmp_group_delete(sb_igmp);
> +            continue;
> +        }
> +
> +        ovn_igmp_group_add(mcast_groups, igmp_groups, od, ports,
> +                           sb_igmp->address, sb_igmp->ports, sb_igmp->n_ports);
> +    }
> +}

Hi Ben, Mark,

While doing some scale testing I realized that walking the rows of the
IGMP_Group table in ovn-northd in the order we get them from the
database might create an issue: ovn_igmp_group_add will create a new
multicast_group for every unique IGMP group address and allocate a
tunnel-id for it. However, because rows are not processed in the order
they were added to the database, it can happen that multicast groups
that didn't actually change will get a different tunnel-id triggering
a change in the associated logical flows.

In order to avoid this I would need to reuse the tunnel-ids of
multicast groups that didn't change between different runs of the
ovn-northd loop. Until now I thought of two different approaches (both
with advantages and disadvantages):

1. Force ovn-northd to walk the IGMP table in a way that ensures that
IGMP groups are processed in the order they were added to the
database:

Add a column to the IGMP_Group table storing a free running counter
value (unique per ovn-controller instance) and add another compound
index [datapath + address + counter]. Every time an ovn-controller
adds an IGMP group it increments its own counter. Then have ovn-northd
walk the IGMP_Group table with SBREC_IGMP_GROUP_FOR_EACH_BYINDEX which
would give us a stable ordering of the entries.

Advantages:
- relatively straightforward to code and maintain

Disadvantages:
- extra column in SB DB
- populating the index in ovn-northd will take N log(N) operations if
i understand correctly the IDL index implementation (N = number of
IGMP_Group entries in the DB)

2. Maintain a cache (hashtable) of allocated multicast group
tunnel-ids between subsequent  runs of the ovn-northd loop:
- Once all IGMP_Group entries are processed and their corresponding
Multicast_Group entries are collected we'd need to store a mapping
(per datapath) between IGMP group address and multicast group
tunnel-id.
- Next time ovn-northd walks the IGMP_Group table, before allocating a
new tunnel-id for a multicast group entry it would check the "cache"
from the previous run. If there's already an entry it would reuse the
tunnel-id. If not, it will have to allocate a tunnel-id. Store the
(IGMP group address, tunnel-id) mapping for next run.

Advantages:
- Changes are all local to ovn-northd, no need to store additional
information in the DB.
- Should be faster on average when processing small batches of new
IGMP_Groups from the DB.

Disadvantages:
- We need a multicast_group tunnel-id allocator. I see there's already
similar code (allocate_tnlid) for datapath tunnel-keys but this will
walk the whole range of tunnel-ids until we find one that's not used.
I'm not completely sure about the worst case complexity of this
approach..
- The code to maintain the tunnel-ids across ovn-northd loop
iterations might end up complicated.

What do you guys think? Any alternatives?

Thanks,
Dumitru


More information about the dev mailing list