On Thu, Jul 11, 2019 at 10:57 AM Dumitru Ceara <dce...@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 <dce...@redhat.com>
> Acked-by: Mark Michelson <mmich...@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
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
