On 3/2/22 20:31, Mark Michelson wrote: > Hi Dumitru, > Hi Mark,
> This looks good to me. I only have a small nit below. > > Acked-by: Mark Michelson <[email protected]> > Thanks! > On 2/28/22 16:06, Dumitru Ceara wrote: >> Commit 974618c61de8 ("ovn-ic: physical: Support multicast_group flooding >> on IC transit switches.") added support for statically forwarding IP >> multicast traffic between AZs. While this works, it's a waste of >> resources to flood all IP multicast traffic to all instances of the >> transit switch in all AZs. >> >> Instead, we now extend the OVN IGMP/MLD support, in order to make it >> function across availability zones. >> >> Until now, all IGMP/MLD control packets were punted to ovn-controller >> in the ingress pipeline of the logical switch that had multicast >> snooping enabled. We change this behavior for transit switches and >> punt the packets both in the ingress pipeline (in the source AZ) but >> also in the egress pipeline for remote ports (in the destination AZ). >> >> This ensures that both OVN deployments (in the source and destination >> AZs) dynamically learn the same IGMP groups on the transit switch. >> There is, however, a catch: there's no guarantee that multicast groups >> are learnt in the same order on all AZs so they might end up having >> different tunnel keys. In order to avoid the potential mismatch between >> local and remote AZ multicast group tunnel keys we change multicast >> group processing for "remote" ports and perform the group expansion on >> the source node. >> >> In order for the end-to-end solution to work, and for IP multicast >> traffic to flow between logical switch ports that are behind gateway >> routers in different AZs we also fix the logical router port static >> multicast flooding (options:mcast_flood) implementation as it was always >> dropping IGMP/MLD packets (these are always generated with TTL=1). >> >> Reported-at: https://bugzilla.redhat.com/show_bug.cgi?id=2042952 >> Signed-off-by: Dumitru Ceara <[email protected]> >> --- >> controller/physical.c | 28 +++++ >> northd/northd.c | 93 ++++++++++++++++- >> northd/ovn-northd.8.xml | 48 +++++++-- >> tests/ovn-northd.at | 43 ++++++++ >> tests/ovn.at | 226 ++++++++++++++++++++++++++++++++++++++++ >> 5 files changed, 430 insertions(+), 8 deletions(-) >> >> diff --git a/controller/physical.c b/controller/physical.c >> index 033828d577..02fcd5ea8b 100644 >> --- a/controller/physical.c >> +++ b/controller/physical.c >> @@ -1382,6 +1382,26 @@ get_vxlan_port_key(int64_t port_key) >> return port_key; >> } >> +/* Encapsulate and send to a single remote chassis. */ >> +static void >> +tunnel_to_chassis(enum mf_field_id mff_ovn_geneve, >> + const char *chassis_name, >> + const struct hmap *chassis_tunnels, >> + const struct sbrec_datapath_binding *datapath, >> + uint16_t outport, struct ofpbuf *remote_ofpacts) >> +{ >> + const struct chassis_tunnel *tun >> + = chassis_tunnel_find(chassis_tunnels, chassis_name, NULL); >> + if (!tun) { >> + return; >> + } >> + >> + put_encapsulation(mff_ovn_geneve, tun, datapath, outport, false, >> + remote_ofpacts); >> + ofpact_put_OUTPUT(remote_ofpacts)->port = tun->ofport; >> +} >> + >> +/* Encapsulate and send to a set of remote chassis. */ >> static void >> fanout_to_chassis(enum mf_field_id mff_ovn_geneve, >> struct sset *remote_chassis, >> @@ -1487,6 +1507,14 @@ consider_mc_group(struct ovsdb_idl_index >> *sbrec_port_binding_by_name, >> &remote_ofpacts); >> put_resubmit(OFTABLE_CHECK_LOOPBACK, &remote_ofpacts); >> } >> + } if (!strcmp(port->type, "remote")) { >> + if (port->chassis) { >> + put_load(port->tunnel_key, MFF_LOG_OUTPORT, 0, 32, >> + &remote_ofpacts); >> + tunnel_to_chassis(mff_ovn_geneve, port->chassis->name, >> + chassis_tunnels, mc->datapath, >> + port->tunnel_key, &remote_ofpacts); >> + } >> } else if (!strcmp(port->type, "localport")) { >> put_load(port->tunnel_key, MFF_LOG_OUTPORT, 0, 32, >> &remote_ofpacts); >> diff --git a/northd/northd.c b/northd/northd.c >> index 294a59bd7e..4c95933b87 100644 >> --- a/northd/northd.c >> +++ b/northd/northd.c >> @@ -1792,6 +1792,12 @@ lsp_is_router(const struct >> nbrec_logical_switch_port *nbsp) >> return !strcmp(nbsp->type, "router"); >> } >> +static bool >> +lsp_is_remote(const struct nbrec_logical_switch_port *nbsp) >> +{ >> + return !strcmp(nbsp->type, "remote"); >> +} >> + >> static bool >> lrport_is_enabled(const struct nbrec_logical_router_port *lrport) >> { >> @@ -5838,8 +5844,48 @@ build_empty_lb_event_flow(struct ovn_lb_vip >> *lb_vip, >> } >> static void >> -build_pre_lb(struct ovn_datapath *od, struct hmap *lflows) >> +build_interconn_mcast_snoop_flows(struct ovn_datapath *od, >> + const struct shash *meter_groups, >> + struct hmap *lflows) >> +{ >> + struct mcast_switch_info *mcast_sw_info = &od->mcast_info.sw; >> + if (!smap_get(&od->nbs->other_config, "interconn-ts") >> + || !mcast_sw_info->enabled) { >> + return; >> + } >> + >> + struct ovn_port *op; >> + >> + LIST_FOR_EACH (op, dp_node, &od->port_list) { >> + if (!lsp_is_remote(op->nbsp)) { >> + continue; >> + } >> + /* Punt IGMP traffic to controller. */ >> + char *match = xasprintf("inport == %s && ip4 && ip.proto == 2", > > FYI, you can use "igmp" in place of "ip4 && ip.proto == 2" in logical > flow matches. > Good point, I'll clean it up everywhere and post v2. Thanks, Dumitru _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
