From patchwork Mon Mar 16 11:12:03 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Numan Siddique X-Patchwork-Id: 1255400 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=openvswitch.org (client-ip=140.211.166.137; helo=fraxinus.osuosl.org; envelope-from=ovs-dev-bounces@openvswitch.org; receiver=) Authentication-Results: ozlabs.org; dmarc=none (p=none dis=none) header.from=ovn.org Received: from fraxinus.osuosl.org (smtp4.osuosl.org [140.211.166.137]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 48gtv31CDdz9sPJ for ; Mon, 16 Mar 2020 22:12:47 +1100 (AEDT) Received: from localhost (localhost [127.0.0.1]) by fraxinus.osuosl.org (Postfix) with ESMTP id 861EF87BBC; Mon, 16 Mar 2020 11:12:45 +0000 (UTC) X-Virus-Scanned: amavisd-new at osuosl.org Received: from fraxinus.osuosl.org ([127.0.0.1]) by localhost (.osuosl.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id wuGXwW_5IxPH; Mon, 16 Mar 2020 11:12:43 +0000 (UTC) Received: from lists.linuxfoundation.org (lf-lists.osuosl.org [140.211.9.56]) by fraxinus.osuosl.org (Postfix) with ESMTP id DF36887B5C; Mon, 16 Mar 2020 11:12:43 +0000 (UTC) Received: from lf-lists.osuosl.org (localhost [127.0.0.1]) by lists.linuxfoundation.org (Postfix) with ESMTP id C727FC1D7C; Mon, 16 Mar 2020 11:12:43 +0000 (UTC) X-Original-To: dev@openvswitch.org Delivered-To: ovs-dev@lists.linuxfoundation.org Received: from whitealder.osuosl.org (smtp1.osuosl.org [140.211.166.138]) by lists.linuxfoundation.org (Postfix) with ESMTP id 1DE97C013E for ; Mon, 16 Mar 2020 11:12:42 +0000 (UTC) Received: from localhost (localhost [127.0.0.1]) by whitealder.osuosl.org (Postfix) with ESMTP id 0C72E885F2 for ; Mon, 16 Mar 2020 11:12:42 +0000 (UTC) X-Virus-Scanned: amavisd-new at osuosl.org Received: from whitealder.osuosl.org ([127.0.0.1]) by localhost (.osuosl.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id 8IWKk8aSlDqn for ; Mon, 16 Mar 2020 11:12:40 +0000 (UTC) X-Greylist: domain auto-whitelisted by SQLgrey-1.7.6 Received: from relay11.mail.gandi.net (relay11.mail.gandi.net [217.70.178.231]) by whitealder.osuosl.org (Postfix) with ESMTPS id 9CF91885E7 for ; Mon, 16 Mar 2020 11:12:39 +0000 (UTC) Received: from nummac.local (unknown [27.7.184.93]) (Authenticated sender: numans@ovn.org) by relay11.mail.gandi.net (Postfix) with ESMTPSA id D978510002B; Mon, 16 Mar 2020 11:12:14 +0000 (UTC) From: numans@ovn.org To: dev@openvswitch.org Date: Mon, 16 Mar 2020 16:42:03 +0530 Message-Id: <20200316111203.1686979-1-numans@ovn.org> X-Mailer: git-send-email 2.24.1 In-Reply-To: <20200316111034.1686721-1-numans@ovn.org> References: <20200316111034.1686721-1-numans@ovn.org> MIME-Version: 1.0 Subject: [ovs-dev] [PATCH ovn 1/6] Refactor binding_run()to take two context argument - binding_ctx_in and binding_ctx_out. X-BeenThere: ovs-dev@openvswitch.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: ovs-dev-bounces@openvswitch.org Sender: "dev" From: Numan Siddique No functional changes are introduced in this patch. Signed-off-by: Numan Siddique --- controller/binding.c | 261 +++++++++++++++++------------------- controller/binding.h | 39 +++--- controller/ovn-controller.c | 120 ++++++++++------- 3 files changed, 215 insertions(+), 205 deletions(-) diff --git a/controller/binding.c b/controller/binding.c index c3376e25e..1e9f78249 100644 --- a/controller/binding.c +++ b/controller/binding.c @@ -441,12 +441,9 @@ static bool is_our_chassis(const struct sbrec_chassis *chassis_rec, const struct sbrec_port_binding *binding_rec, const struct sset *active_tunnels, - const struct shash *lport_to_iface, + const struct ovsrec_interface *iface_rec, const struct sset *local_lports) { - const struct ovsrec_interface *iface_rec - = shash_find_data(lport_to_iface, binding_rec->logical_port); - bool our_chassis = false; if (iface_rec || (binding_rec->parent_port && binding_rec->parent_port[0] && @@ -478,78 +475,74 @@ is_our_chassis(const struct sbrec_chassis *chassis_rec, * additional processing. */ static const struct sbrec_port_binding ** -consider_local_datapath(struct ovsdb_idl_txn *ovnsb_idl_txn, - struct ovsdb_idl_txn *ovs_idl_txn, - struct ovsdb_idl_index *sbrec_datapath_binding_by_key, - struct ovsdb_idl_index *sbrec_port_binding_by_datapath, - struct ovsdb_idl_index *sbrec_port_binding_by_name, - const struct sset *active_tunnels, - const struct sbrec_chassis *chassis_rec, - const struct sbrec_port_binding *binding_rec, +consider_local_datapath(const struct sbrec_port_binding *binding_rec, struct hmap *qos_map, - struct hmap *local_datapaths, - struct shash *lport_to_iface, - struct sset *local_lports, - struct sset *local_lport_ids, + const struct ovsrec_interface *iface_rec, + struct binding_ctx_in *b_ctx_in, + struct binding_ctx_out *b_ctx_out, const struct sbrec_port_binding **vpbs, size_t *n_vpbs, size_t *n_allocated_vpbs) { - const struct ovsrec_interface *iface_rec - = shash_find_data(lport_to_iface, binding_rec->logical_port); - - bool our_chassis = is_our_chassis(chassis_rec, binding_rec, active_tunnels, - lport_to_iface, local_lports); + bool our_chassis = is_our_chassis(b_ctx_in->chassis_rec, binding_rec, + b_ctx_in->active_tunnels, iface_rec, + b_ctx_out->local_lports); if (iface_rec || (binding_rec->parent_port && binding_rec->parent_port[0] && - sset_contains(local_lports, binding_rec->parent_port))) { + sset_contains(b_ctx_out->local_lports, + binding_rec->parent_port))) { if (binding_rec->parent_port && binding_rec->parent_port[0]) { /* Add child logical port to the set of all local ports. */ - sset_add(local_lports, binding_rec->logical_port); + sset_add(b_ctx_out->local_lports, binding_rec->logical_port); } - add_local_datapath(sbrec_datapath_binding_by_key, - sbrec_port_binding_by_datapath, - sbrec_port_binding_by_name, - binding_rec->datapath, false, local_datapaths); - if (iface_rec && qos_map && ovs_idl_txn) { + add_local_datapath(b_ctx_in->sbrec_datapath_binding_by_key, + b_ctx_in->sbrec_port_binding_by_datapath, + b_ctx_in->sbrec_port_binding_by_name, + binding_rec->datapath, false, + b_ctx_out->local_datapaths); + if (iface_rec && qos_map && b_ctx_in->ovs_idl_txn) { get_qos_params(binding_rec, qos_map); } } else if (!strcmp(binding_rec->type, "l2gateway")) { if (our_chassis) { - sset_add(local_lports, binding_rec->logical_port); - add_local_datapath(sbrec_datapath_binding_by_key, - sbrec_port_binding_by_datapath, - sbrec_port_binding_by_name, - binding_rec->datapath, false, local_datapaths); + sset_add(b_ctx_out->local_lports, binding_rec->logical_port); + add_local_datapath(b_ctx_in->sbrec_datapath_binding_by_key, + b_ctx_in->sbrec_port_binding_by_datapath, + b_ctx_in->sbrec_port_binding_by_name, + binding_rec->datapath, false, + b_ctx_out->local_datapaths); } } else if (!strcmp(binding_rec->type, "chassisredirect")) { if (ha_chassis_group_contains(binding_rec->ha_chassis_group, - chassis_rec)) { - add_local_datapath(sbrec_datapath_binding_by_key, - sbrec_port_binding_by_datapath, - sbrec_port_binding_by_name, - binding_rec->datapath, false, local_datapaths); + b_ctx_in->chassis_rec)) { + add_local_datapath(b_ctx_in->sbrec_datapath_binding_by_key, + b_ctx_in->sbrec_port_binding_by_datapath, + b_ctx_in->sbrec_port_binding_by_name, + binding_rec->datapath, false, + b_ctx_out->local_datapaths); } } else if (!strcmp(binding_rec->type, "l3gateway")) { if (our_chassis) { - add_local_datapath(sbrec_datapath_binding_by_key, - sbrec_port_binding_by_datapath, - sbrec_port_binding_by_name, - binding_rec->datapath, true, local_datapaths); + add_local_datapath(b_ctx_in->sbrec_datapath_binding_by_key, + b_ctx_in->sbrec_port_binding_by_datapath, + b_ctx_in->sbrec_port_binding_by_name, + binding_rec->datapath, true, + b_ctx_out->local_datapaths); } } else if (!strcmp(binding_rec->type, "localnet")) { /* Add all localnet ports to local_lports so that we allocate ct zones * for them. */ - sset_add(local_lports, binding_rec->logical_port); - if (qos_map && ovs_idl_txn) { + sset_add(b_ctx_out->local_lports, binding_rec->logical_port); + if (qos_map && b_ctx_in->ovs_idl_txn) { get_qos_params(binding_rec, qos_map); } } else if (!strcmp(binding_rec->type, "external")) { if (ha_chassis_group_contains(binding_rec->ha_chassis_group, - chassis_rec)) { - add_local_datapath(sbrec_datapath_binding_by_key, - sbrec_port_binding_by_datapath, - sbrec_port_binding_by_name, - binding_rec->datapath, false, local_datapaths); + b_ctx_in->chassis_rec)) { + add_local_datapath(b_ctx_in->sbrec_datapath_binding_by_key, + b_ctx_in->sbrec_port_binding_by_datapath, + b_ctx_in->sbrec_port_binding_by_name, + binding_rec->datapath, false, + b_ctx_out->local_datapaths); } } @@ -558,65 +551,63 @@ consider_local_datapath(struct ovsdb_idl_txn *ovnsb_idl_txn, || !strcmp(binding_rec->type, "localport") || !strcmp(binding_rec->type, "vtep") || !strcmp(binding_rec->type, "localnet")) { - update_local_lport_ids(local_lport_ids, binding_rec); + update_local_lport_ids(b_ctx_out->local_lport_ids, binding_rec); } - ovs_assert(ovnsb_idl_txn); - if (ovnsb_idl_txn) { - const char *vif_chassis = smap_get(&binding_rec->options, + ovs_assert(b_ctx_in->ovnsb_idl_txn); + const char *vif_chassis = smap_get(&binding_rec->options, "requested-chassis"); - bool can_bind = !vif_chassis || !vif_chassis[0] - || !strcmp(vif_chassis, chassis_rec->name) - || !strcmp(vif_chassis, chassis_rec->hostname); - - if (can_bind && our_chassis) { - if (binding_rec->chassis != chassis_rec) { - if (binding_rec->chassis) { - VLOG_INFO("Changing chassis for lport %s from %s to %s.", - binding_rec->logical_port, - binding_rec->chassis->name, - chassis_rec->name); - } else { - VLOG_INFO("Claiming lport %s for this chassis.", - binding_rec->logical_port); - } - for (int i = 0; i < binding_rec->n_mac; i++) { - VLOG_INFO("%s: Claiming %s", - binding_rec->logical_port, binding_rec->mac[i]); - } - sbrec_port_binding_set_chassis(binding_rec, chassis_rec); + bool can_bind = !vif_chassis || !vif_chassis[0] + || !strcmp(vif_chassis, b_ctx_in->chassis_rec->name) + || !strcmp(vif_chassis, b_ctx_in->chassis_rec->hostname); + + if (can_bind && our_chassis) { + if (binding_rec->chassis != b_ctx_in->chassis_rec) { + if (binding_rec->chassis) { + VLOG_INFO("Changing chassis for lport %s from %s to %s.", + binding_rec->logical_port, + binding_rec->chassis->name, + b_ctx_in->chassis_rec->name); + } else { + VLOG_INFO("Claiming lport %s for this chassis.", + binding_rec->logical_port); } - /* Check if the port encap binding, if any, has changed */ - struct sbrec_encap *encap_rec = sbrec_get_port_encap( - chassis_rec, iface_rec); - if (encap_rec && binding_rec->encap != encap_rec) { - sbrec_port_binding_set_encap(binding_rec, encap_rec); + for (int i = 0; i < binding_rec->n_mac; i++) { + VLOG_INFO("%s: Claiming %s", + binding_rec->logical_port, binding_rec->mac[i]); } - } else if (binding_rec->chassis == chassis_rec) { - if (!strcmp(binding_rec->type, "virtual")) { - if (*n_vpbs == *n_allocated_vpbs) { - vpbs = x2nrealloc(vpbs, n_allocated_vpbs, sizeof *vpbs); - } - vpbs[(*n_vpbs)] = binding_rec; - (*n_vpbs)++; - } else { - VLOG_INFO("Releasing lport %s from this chassis.", - binding_rec->logical_port); - if (binding_rec->encap) { - sbrec_port_binding_set_encap(binding_rec, NULL); - } - sbrec_port_binding_set_chassis(binding_rec, NULL); + sbrec_port_binding_set_chassis(binding_rec, b_ctx_in->chassis_rec); + } + /* Check if the port encap binding, if any, has changed */ + struct sbrec_encap *encap_rec = + sbrec_get_port_encap(b_ctx_in->chassis_rec, iface_rec); + if (encap_rec && binding_rec->encap != encap_rec) { + sbrec_port_binding_set_encap(binding_rec, encap_rec); + } + } else if (binding_rec->chassis == b_ctx_in->chassis_rec) { + if (!strcmp(binding_rec->type, "virtual")) { + if (*n_vpbs == *n_allocated_vpbs) { + vpbs = x2nrealloc(vpbs, n_allocated_vpbs, sizeof *vpbs); } - } else if (our_chassis) { - static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 1); - VLOG_INFO_RL(&rl, - "Not claiming lport %s, chassis %s " - "requested-chassis %s", - binding_rec->logical_port, - chassis_rec->name, - vif_chassis); + vpbs[(*n_vpbs)] = binding_rec; + (*n_vpbs)++; + } else { + VLOG_INFO("Releasing lport %s from this chassis.", + binding_rec->logical_port); + if (binding_rec->encap) { + sbrec_port_binding_set_encap(binding_rec, NULL); + } + sbrec_port_binding_set_chassis(binding_rec, NULL); } + } else if (our_chassis) { + static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 1); + VLOG_INFO_RL(&rl, + "Not claiming lport %s, chassis %s " + "requested-chassis %s", + binding_rec->logical_port, b_ctx_in->chassis_rec->name, + vif_chassis); } + return vpbs; } @@ -706,23 +697,9 @@ consider_localnet_port(const struct sbrec_port_binding *binding_rec, } void -binding_run(struct ovsdb_idl_txn *ovnsb_idl_txn, - struct ovsdb_idl_txn *ovs_idl_txn, - struct ovsdb_idl_index *sbrec_datapath_binding_by_key, - struct ovsdb_idl_index *sbrec_port_binding_by_datapath, - struct ovsdb_idl_index *sbrec_port_binding_by_name, - const struct ovsrec_port_table *port_table, - const struct ovsrec_qos_table *qos_table, - const struct sbrec_port_binding_table *port_binding_table, - const struct ovsrec_bridge *br_int, - const struct sbrec_chassis *chassis_rec, - const struct sset *active_tunnels, - const struct ovsrec_bridge_table *bridge_table, - const struct ovsrec_open_vswitch_table *ovs_table, - struct hmap *local_datapaths, struct sset *local_lports, - struct sset *local_lport_ids) +binding_run(struct binding_ctx_in *b_ctx_in, struct binding_ctx_out *b_ctx_out) { - if (!chassis_rec) { + if (!b_ctx_in->chassis_rec) { return; } @@ -733,9 +710,9 @@ binding_run(struct ovsdb_idl_txn *ovnsb_idl_txn, struct hmap qos_map; hmap_init(&qos_map); - if (br_int) { - get_local_iface_ids(br_int, &lport_to_iface, local_lports, - &egress_ifaces); + if (b_ctx_in->br_int) { + get_local_iface_ids(b_ctx_in->br_int, &lport_to_iface, + b_ctx_out->local_lports, &egress_ifaces); } /* Array to store pointers to local virtual ports. It is populated by @@ -752,16 +729,14 @@ binding_run(struct ovsdb_idl_txn *ovnsb_idl_txn, * later. This is special case for virtual ports is needed in order to * make sure we update the virtual_parent port bindings first. */ - SBREC_PORT_BINDING_TABLE_FOR_EACH (binding_rec, port_binding_table) { + SBREC_PORT_BINDING_TABLE_FOR_EACH (binding_rec, + b_ctx_in->port_binding_table) { + const struct ovsrec_interface *iface_rec + = shash_find_data(&lport_to_iface, binding_rec->logical_port); vpbs = - consider_local_datapath(ovnsb_idl_txn, ovs_idl_txn, - sbrec_datapath_binding_by_key, - sbrec_port_binding_by_datapath, - sbrec_port_binding_by_name, - active_tunnels, chassis_rec, binding_rec, + consider_local_datapath(binding_rec, sset_is_empty(&egress_ifaces) ? NULL : - &qos_map, local_datapaths, &lport_to_iface, - local_lports, local_lport_ids, + &qos_map, iface_rec, b_ctx_in, b_ctx_out, vpbs, &n_vpbs, &n_allocated_vpbs); } @@ -769,26 +744,29 @@ binding_run(struct ovsdb_idl_txn *ovnsb_idl_txn, * updated above. */ for (size_t i = 0; i < n_vpbs; i++) { - consider_local_virtual_port(sbrec_port_binding_by_name, chassis_rec, - vpbs[i]); + consider_local_virtual_port(b_ctx_in->sbrec_port_binding_by_name, + b_ctx_in->chassis_rec, vpbs[i]); } free(vpbs); - add_ovs_bridge_mappings(ovs_table, bridge_table, &bridge_mappings); + add_ovs_bridge_mappings(b_ctx_in->ovs_table, b_ctx_in->bridge_table, + &bridge_mappings); /* Run through each binding record to see if it is a localnet port * on local datapaths discovered from above loop, and update the * corresponding local datapath accordingly. */ - SBREC_PORT_BINDING_TABLE_FOR_EACH (binding_rec, port_binding_table) { + SBREC_PORT_BINDING_TABLE_FOR_EACH (binding_rec, + b_ctx_in->port_binding_table) { if (!strcmp(binding_rec->type, "localnet")) { consider_localnet_port(binding_rec, &bridge_mappings, - &egress_ifaces, local_datapaths); + &egress_ifaces, b_ctx_out->local_datapaths); } } shash_destroy(&bridge_mappings); if (!sset_is_empty(&egress_ifaces) - && set_noop_qos(ovs_idl_txn, port_table, qos_table, &egress_ifaces)) { + && set_noop_qos(b_ctx_in->ovs_idl_txn, b_ctx_in->port_table, + b_ctx_in->qos_table, &egress_ifaces)) { const char *entry; SSET_FOR_EACH (entry, &egress_ifaces) { setup_qos(entry, &qos_map); @@ -834,13 +812,16 @@ binding_evaluate_port_binding_changes( * - If a regular VIF is unbound from this chassis, the local ovsdb * interface table will be updated, which will trigger recompute. * - * - If the port is not a regular VIF, and not a "remote" port, - * always trigger recompute. */ - if (binding_rec->chassis == chassis_rec - || is_our_chassis(chassis_rec, binding_rec, - active_tunnels, &lport_to_iface, local_lports) - || (strcmp(binding_rec->type, "") && strcmp(binding_rec->type, - "remote"))) { + * - If the port is not a regular VIF, always trigger recompute. */ + if (binding_rec->chassis == chassis_rec) { + changed = true; + break; + } + + const struct ovsrec_interface *iface_rec + = shash_find_data(&lport_to_iface, binding_rec->logical_port); + if (is_our_chassis(chassis_rec, binding_rec, active_tunnels, iface_rec, + local_lports) || strcmp(binding_rec->type, "")) { changed = true; break; } diff --git a/controller/binding.h b/controller/binding.h index 924891c1b..d0b8331af 100644 --- a/controller/binding.h +++ b/controller/binding.h @@ -32,22 +32,31 @@ struct sbrec_chassis; struct sbrec_port_binding_table; struct sset; +struct binding_ctx_in { + struct ovsdb_idl_txn *ovnsb_idl_txn; + struct ovsdb_idl_txn *ovs_idl_txn; + struct ovsdb_idl_index *sbrec_datapath_binding_by_key; + struct ovsdb_idl_index *sbrec_port_binding_by_datapath; + struct ovsdb_idl_index *sbrec_port_binding_by_name; + const struct ovsrec_port_table *port_table; + const struct ovsrec_qos_table *qos_table; + const struct sbrec_port_binding_table *port_binding_table; + const struct ovsrec_bridge *br_int; + const struct sbrec_chassis *chassis_rec; + const struct sset *active_tunnels; + const struct ovsrec_bridge_table *bridge_table; + const struct ovsrec_open_vswitch_table *ovs_table; + const struct ovsrec_interface_table *iface_table; +}; + +struct binding_ctx_out { + struct hmap *local_datapaths; + struct sset *local_lports; + struct sset *local_lport_ids; +}; + void binding_register_ovs_idl(struct ovsdb_idl *); -void binding_run(struct ovsdb_idl_txn *ovnsb_idl_txn, - struct ovsdb_idl_txn *ovs_idl_txn, - struct ovsdb_idl_index *sbrec_datapath_binding_by_key, - struct ovsdb_idl_index *sbrec_port_binding_by_datapath, - struct ovsdb_idl_index *sbrec_port_binding_by_name, - const struct ovsrec_port_table *, - const struct ovsrec_qos_table *, - const struct sbrec_port_binding_table *, - const struct ovsrec_bridge *br_int, - const struct sbrec_chassis *, - const struct sset *active_tunnels, - const struct ovsrec_bridge_table *bridge_table, - const struct ovsrec_open_vswitch_table *ovs_table, - struct hmap *local_datapaths, - struct sset *local_lports, struct sset *local_lport_ids); +void binding_run(struct binding_ctx_in *, struct binding_ctx_out *); bool binding_cleanup(struct ovsdb_idl_txn *ovnsb_idl_txn, const struct sbrec_port_binding_table *, const struct sbrec_chassis *); diff --git a/controller/ovn-controller.c b/controller/ovn-controller.c index 2893eaac1..4930d5ffb 100644 --- a/controller/ovn-controller.c +++ b/controller/ovn-controller.c @@ -1003,34 +1003,11 @@ en_runtime_data_cleanup(void *data) } static void -en_runtime_data_run(struct engine_node *node, void *data) +init_binding_ctx(struct engine_node *node, + struct ed_type_runtime_data *rt_data, + struct binding_ctx_in *b_ctx_in, + struct binding_ctx_out *b_ctx_out) { - struct ed_type_runtime_data *rt_data = data; - struct hmap *local_datapaths = &rt_data->local_datapaths; - struct sset *local_lports = &rt_data->local_lports; - struct sset *local_lport_ids = &rt_data->local_lport_ids; - struct sset *active_tunnels = &rt_data->active_tunnels; - - static bool first_run = true; - if (first_run) { - /* don't cleanup since there is no data yet */ - first_run = false; - } else { - struct local_datapath *cur_node, *next_node; - HMAP_FOR_EACH_SAFE (cur_node, next_node, hmap_node, local_datapaths) { - free(cur_node->peer_ports); - hmap_remove(local_datapaths, &cur_node->hmap_node); - free(cur_node); - } - hmap_clear(local_datapaths); - sset_destroy(local_lports); - sset_destroy(local_lport_ids); - sset_destroy(active_tunnels); - sset_init(local_lports); - sset_init(local_lport_ids); - sset_init(active_tunnels); - } - struct ovsrec_open_vswitch_table *ovs_table = (struct ovsrec_open_vswitch_table *)EN_OVSDB_GET( engine_get_input("OVS_open_vswitch", node)); @@ -1051,19 +1028,6 @@ en_runtime_data_run(struct engine_node *node, void *data) = chassis_lookup_by_name(sbrec_chassis_by_name, chassis_id); ovs_assert(chassis); - struct ed_type_ofctrl_is_connected *ed_ofctrl_is_connected = - engine_get_input_data("ofctrl_is_connected", node); - if (ed_ofctrl_is_connected->connected) { - /* Calculate the active tunnels only if have an an active - * OpenFlow connection to br-int. - * If we don't have a connection to br-int, it could mean - * ovs-vswitchd is down for some reason and the BFD status - * in the Interface rows could be stale. So its better to - * consider 'active_tunnels' set to be empty if it's not - * connected. */ - bfd_calculate_active_tunnels(br_int, active_tunnels); - } - struct ovsrec_port_table *port_table = (struct ovsrec_port_table *)EN_OVSDB_GET( engine_get_input("OVS_port", node)); @@ -1091,16 +1055,72 @@ en_runtime_data_run(struct engine_node *node, void *data) engine_get_input("SB_port_binding", node), "datapath"); - binding_run(engine_get_context()->ovnsb_idl_txn, - engine_get_context()->ovs_idl_txn, - sbrec_datapath_binding_by_key, - sbrec_port_binding_by_datapath, - sbrec_port_binding_by_name, - port_table, qos_table, pb_table, - br_int, chassis, - active_tunnels, bridge_table, - ovs_table, local_datapaths, - local_lports, local_lport_ids); + b_ctx_in->ovnsb_idl_txn = engine_get_context()->ovnsb_idl_txn; + b_ctx_in->ovs_idl_txn = engine_get_context()->ovs_idl_txn; + b_ctx_in->sbrec_datapath_binding_by_key = sbrec_datapath_binding_by_key; + b_ctx_in->sbrec_port_binding_by_datapath = sbrec_port_binding_by_datapath; + b_ctx_in->sbrec_port_binding_by_name = sbrec_port_binding_by_name; + b_ctx_in->port_table = port_table; + b_ctx_in->qos_table = qos_table; + b_ctx_in->port_binding_table = pb_table; + b_ctx_in->br_int = br_int; + b_ctx_in->chassis_rec = chassis; + b_ctx_in->active_tunnels = &rt_data->active_tunnels; + b_ctx_in->bridge_table = bridge_table; + b_ctx_in->ovs_table = ovs_table; + + b_ctx_out->local_datapaths = &rt_data->local_datapaths; + b_ctx_out->local_lports = &rt_data->local_lports; + b_ctx_out->local_lport_ids = &rt_data->local_lport_ids; +} + +static void +en_runtime_data_run(struct engine_node *node, void *data) +{ + struct ed_type_runtime_data *rt_data = data; + struct hmap *local_datapaths = &rt_data->local_datapaths; + struct sset *local_lports = &rt_data->local_lports; + struct sset *local_lport_ids = &rt_data->local_lport_ids; + struct sset *active_tunnels = &rt_data->active_tunnels; + + static bool first_run = true; + if (first_run) { + /* don't cleanup since there is no data yet */ + first_run = false; + } else { + struct local_datapath *cur_node, *next_node; + HMAP_FOR_EACH_SAFE (cur_node, next_node, hmap_node, local_datapaths) { + free(cur_node->peer_ports); + hmap_remove(local_datapaths, &cur_node->hmap_node); + free(cur_node); + } + hmap_clear(local_datapaths); + sset_destroy(local_lports); + sset_destroy(local_lport_ids); + sset_destroy(active_tunnels); + sset_init(local_lports); + sset_init(local_lport_ids); + sset_init(active_tunnels); + } + + struct binding_ctx_in b_ctx_in; + struct binding_ctx_out b_ctx_out; + init_binding_ctx(node, rt_data, &b_ctx_in, &b_ctx_out); + + struct ed_type_ofctrl_is_connected *ed_ofctrl_is_connected = + engine_get_input_data("ofctrl_is_connected", node); + if (ed_ofctrl_is_connected->connected) { + /* Calculate the active tunnels only if have an an active + * OpenFlow connection to br-int. + * If we don't have a connection to br-int, it could mean + * ovs-vswitchd is down for some reason and the BFD status + * in the Interface rows could be stale. So its better to + * consider 'active_tunnels' set to be empty if it's not + * connected. */ + bfd_calculate_active_tunnels(b_ctx_in.br_int, active_tunnels); + } + + binding_run(&b_ctx_in, &b_ctx_out); engine_set_node_state(node, EN_UPDATED); } From patchwork Mon Mar 16 11:12:38 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Numan Siddique X-Patchwork-Id: 1255402 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=openvswitch.org (client-ip=140.211.166.138; helo=whitealder.osuosl.org; envelope-from=ovs-dev-bounces@openvswitch.org; receiver=) Authentication-Results: ozlabs.org; dmarc=none (p=none dis=none) header.from=ovn.org Received: from whitealder.osuosl.org (smtp1.osuosl.org [140.211.166.138]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 48gtyl1vnrz9sPJ for ; Mon, 16 Mar 2020 22:15:58 +1100 (AEDT) Received: from localhost (localhost [127.0.0.1]) by whitealder.osuosl.org (Postfix) with ESMTP id 958AB88628; Mon, 16 Mar 2020 11:15:56 +0000 (UTC) X-Virus-Scanned: amavisd-new at osuosl.org Received: from whitealder.osuosl.org ([127.0.0.1]) by localhost (.osuosl.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id pdDCe2SZIGwN; Mon, 16 Mar 2020 11:15:50 +0000 (UTC) Received: from lists.linuxfoundation.org (lf-lists.osuosl.org [140.211.9.56]) by whitealder.osuosl.org (Postfix) with ESMTP id 776FF88603; Mon, 16 Mar 2020 11:15:50 +0000 (UTC) Received: from lf-lists.osuosl.org (localhost [127.0.0.1]) by lists.linuxfoundation.org (Postfix) with ESMTP id 64ECEC1D7C; Mon, 16 Mar 2020 11:15:50 +0000 (UTC) X-Original-To: dev@openvswitch.org Delivered-To: ovs-dev@lists.linuxfoundation.org Received: from hemlock.osuosl.org (smtp2.osuosl.org [140.211.166.133]) by lists.linuxfoundation.org (Postfix) with ESMTP id C2FCCC013E for ; Mon, 16 Mar 2020 11:15:48 +0000 (UTC) Received: from localhost (localhost [127.0.0.1]) by hemlock.osuosl.org (Postfix) with ESMTP id A5ED1898CB for ; Mon, 16 Mar 2020 11:15:48 +0000 (UTC) X-Virus-Scanned: amavisd-new at osuosl.org Received: from hemlock.osuosl.org ([127.0.0.1]) by localhost (.osuosl.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id btoTfWhTAPvV for ; Mon, 16 Mar 2020 11:15:46 +0000 (UTC) X-Greylist: domain auto-whitelisted by SQLgrey-1.7.6 Received: from relay6-d.mail.gandi.net (relay6-d.mail.gandi.net [217.70.183.198]) by hemlock.osuosl.org (Postfix) with ESMTPS id B8E84898B8 for ; Mon, 16 Mar 2020 11:15:45 +0000 (UTC) X-Originating-IP: 27.7.184.93 Received: from nummac.local (unknown [27.7.184.93]) (Authenticated sender: numans@ovn.org) by relay6-d.mail.gandi.net (Postfix) with ESMTPSA id 6571BC0012; Mon, 16 Mar 2020 11:15:40 +0000 (UTC) From: numans@ovn.org To: dev@openvswitch.org Date: Mon, 16 Mar 2020 16:42:38 +0530 Message-Id: <20200316111238.1687066-1-numans@ovn.org> X-Mailer: git-send-email 2.24.1 In-Reply-To: <20200316111034.1686721-1-numans@ovn.org> References: <20200316111034.1686721-1-numans@ovn.org> MIME-Version: 1.0 Subject: [ovs-dev] [PATCH ovn 2/6] ovn-controller: Store the local port bindings in the runtime data I-P state. X-BeenThere: ovs-dev@openvswitch.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: ovs-dev-bounces@openvswitch.org Sender: "dev" From: Numan Siddique This patch adds a new data structure - 'struct local_binding' which represents a probable local port binding. A new object of this structure is creared for each interface present in the integration bridge (br-int) with the external_ids:iface-id set. This struct has 2 main fields - 'struct ovsrec_interface *' and 'struct sbrec_port_binding *'. These fields are updated during port claim and release. A shash of the local bindings is maintained with the 'iface-id' as the hash key in the runtime data of the incremental processing engine. This patch helps in the upcoming patch to add incremental processing support for OVS interface and SB port binding changes. Signed-off-by: Numan Siddique --- controller/binding.c | 686 ++++++++++++++++++++++-------------- controller/binding.h | 16 +- controller/ovn-controller.c | 49 ++- controller/pinctrl.c | 19 +- controller/pinctrl.h | 4 +- 5 files changed, 469 insertions(+), 305 deletions(-) diff --git a/controller/binding.c b/controller/binding.c index 1e9f78249..34bd475de 100644 --- a/controller/binding.c +++ b/controller/binding.c @@ -69,47 +69,6 @@ binding_register_ovs_idl(struct ovsdb_idl *ovs_idl) ovsdb_idl_add_column(ovs_idl, &ovsrec_qos_col_type); } -static void -get_local_iface_ids(const struct ovsrec_bridge *br_int, - struct shash *lport_to_iface, - struct sset *local_lports, - struct sset *egress_ifaces) -{ - int i; - - for (i = 0; i < br_int->n_ports; i++) { - const struct ovsrec_port *port_rec = br_int->ports[i]; - const char *iface_id; - int j; - - if (!strcmp(port_rec->name, br_int->name)) { - continue; - } - - for (j = 0; j < port_rec->n_interfaces; j++) { - const struct ovsrec_interface *iface_rec; - - iface_rec = port_rec->interfaces[j]; - iface_id = smap_get(&iface_rec->external_ids, "iface-id"); - int64_t ofport = iface_rec->n_ofport ? *iface_rec->ofport : 0; - - if (iface_id && ofport > 0) { - shash_add(lport_to_iface, iface_id, iface_rec); - sset_add(local_lports, iface_id); - } - - /* Check if this is a tunnel interface. */ - if (smap_get(&iface_rec->options, "remote_ip")) { - const char *tunnel_iface - = smap_get(&iface_rec->status, "tunnel_egress_iface"); - if (tunnel_iface) { - sset_add(egress_ifaces, tunnel_iface); - } - } - } - } -} - static void add_local_datapath__(struct ovsdb_idl_index *sbrec_datapath_binding_by_key, struct ovsdb_idl_index *sbrec_port_binding_by_datapath, @@ -469,171 +428,6 @@ is_our_chassis(const struct sbrec_chassis *chassis_rec, return our_chassis; } -/* Updates 'binding_rec' and if the port binding is local also updates the - * local datapaths and ports. - * Updates and returns the array of local virtual ports that will require - * additional processing. - */ -static const struct sbrec_port_binding ** -consider_local_datapath(const struct sbrec_port_binding *binding_rec, - struct hmap *qos_map, - const struct ovsrec_interface *iface_rec, - struct binding_ctx_in *b_ctx_in, - struct binding_ctx_out *b_ctx_out, - const struct sbrec_port_binding **vpbs, - size_t *n_vpbs, size_t *n_allocated_vpbs) -{ - bool our_chassis = is_our_chassis(b_ctx_in->chassis_rec, binding_rec, - b_ctx_in->active_tunnels, iface_rec, - b_ctx_out->local_lports); - if (iface_rec - || (binding_rec->parent_port && binding_rec->parent_port[0] && - sset_contains(b_ctx_out->local_lports, - binding_rec->parent_port))) { - if (binding_rec->parent_port && binding_rec->parent_port[0]) { - /* Add child logical port to the set of all local ports. */ - sset_add(b_ctx_out->local_lports, binding_rec->logical_port); - } - add_local_datapath(b_ctx_in->sbrec_datapath_binding_by_key, - b_ctx_in->sbrec_port_binding_by_datapath, - b_ctx_in->sbrec_port_binding_by_name, - binding_rec->datapath, false, - b_ctx_out->local_datapaths); - if (iface_rec && qos_map && b_ctx_in->ovs_idl_txn) { - get_qos_params(binding_rec, qos_map); - } - } else if (!strcmp(binding_rec->type, "l2gateway")) { - if (our_chassis) { - sset_add(b_ctx_out->local_lports, binding_rec->logical_port); - add_local_datapath(b_ctx_in->sbrec_datapath_binding_by_key, - b_ctx_in->sbrec_port_binding_by_datapath, - b_ctx_in->sbrec_port_binding_by_name, - binding_rec->datapath, false, - b_ctx_out->local_datapaths); - } - } else if (!strcmp(binding_rec->type, "chassisredirect")) { - if (ha_chassis_group_contains(binding_rec->ha_chassis_group, - b_ctx_in->chassis_rec)) { - add_local_datapath(b_ctx_in->sbrec_datapath_binding_by_key, - b_ctx_in->sbrec_port_binding_by_datapath, - b_ctx_in->sbrec_port_binding_by_name, - binding_rec->datapath, false, - b_ctx_out->local_datapaths); - } - } else if (!strcmp(binding_rec->type, "l3gateway")) { - if (our_chassis) { - add_local_datapath(b_ctx_in->sbrec_datapath_binding_by_key, - b_ctx_in->sbrec_port_binding_by_datapath, - b_ctx_in->sbrec_port_binding_by_name, - binding_rec->datapath, true, - b_ctx_out->local_datapaths); - } - } else if (!strcmp(binding_rec->type, "localnet")) { - /* Add all localnet ports to local_lports so that we allocate ct zones - * for them. */ - sset_add(b_ctx_out->local_lports, binding_rec->logical_port); - if (qos_map && b_ctx_in->ovs_idl_txn) { - get_qos_params(binding_rec, qos_map); - } - } else if (!strcmp(binding_rec->type, "external")) { - if (ha_chassis_group_contains(binding_rec->ha_chassis_group, - b_ctx_in->chassis_rec)) { - add_local_datapath(b_ctx_in->sbrec_datapath_binding_by_key, - b_ctx_in->sbrec_port_binding_by_datapath, - b_ctx_in->sbrec_port_binding_by_name, - binding_rec->datapath, false, - b_ctx_out->local_datapaths); - } - } - - if (our_chassis - || !strcmp(binding_rec->type, "patch") - || !strcmp(binding_rec->type, "localport") - || !strcmp(binding_rec->type, "vtep") - || !strcmp(binding_rec->type, "localnet")) { - update_local_lport_ids(b_ctx_out->local_lport_ids, binding_rec); - } - - ovs_assert(b_ctx_in->ovnsb_idl_txn); - const char *vif_chassis = smap_get(&binding_rec->options, - "requested-chassis"); - bool can_bind = !vif_chassis || !vif_chassis[0] - || !strcmp(vif_chassis, b_ctx_in->chassis_rec->name) - || !strcmp(vif_chassis, b_ctx_in->chassis_rec->hostname); - - if (can_bind && our_chassis) { - if (binding_rec->chassis != b_ctx_in->chassis_rec) { - if (binding_rec->chassis) { - VLOG_INFO("Changing chassis for lport %s from %s to %s.", - binding_rec->logical_port, - binding_rec->chassis->name, - b_ctx_in->chassis_rec->name); - } else { - VLOG_INFO("Claiming lport %s for this chassis.", - binding_rec->logical_port); - } - for (int i = 0; i < binding_rec->n_mac; i++) { - VLOG_INFO("%s: Claiming %s", - binding_rec->logical_port, binding_rec->mac[i]); - } - sbrec_port_binding_set_chassis(binding_rec, b_ctx_in->chassis_rec); - } - /* Check if the port encap binding, if any, has changed */ - struct sbrec_encap *encap_rec = - sbrec_get_port_encap(b_ctx_in->chassis_rec, iface_rec); - if (encap_rec && binding_rec->encap != encap_rec) { - sbrec_port_binding_set_encap(binding_rec, encap_rec); - } - } else if (binding_rec->chassis == b_ctx_in->chassis_rec) { - if (!strcmp(binding_rec->type, "virtual")) { - if (*n_vpbs == *n_allocated_vpbs) { - vpbs = x2nrealloc(vpbs, n_allocated_vpbs, sizeof *vpbs); - } - vpbs[(*n_vpbs)] = binding_rec; - (*n_vpbs)++; - } else { - VLOG_INFO("Releasing lport %s from this chassis.", - binding_rec->logical_port); - if (binding_rec->encap) { - sbrec_port_binding_set_encap(binding_rec, NULL); - } - sbrec_port_binding_set_chassis(binding_rec, NULL); - } - } else if (our_chassis) { - static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 1); - VLOG_INFO_RL(&rl, - "Not claiming lport %s, chassis %s " - "requested-chassis %s", - binding_rec->logical_port, b_ctx_in->chassis_rec->name, - vif_chassis); - } - - return vpbs; -} - -static void -consider_local_virtual_port(struct ovsdb_idl_index *sbrec_port_binding_by_name, - const struct sbrec_chassis *chassis_rec, - const struct sbrec_port_binding *binding_rec) -{ - /* pinctrl module takes care of binding the ports of type 'virtual'. - * Release such ports if their virtual parents are no longer claimed by - * this chassis. - */ - const struct sbrec_port_binding *parent = - lport_lookup_by_name(sbrec_port_binding_by_name, - binding_rec->virtual_parent); - if (!parent || parent->chassis != chassis_rec) { - VLOG_INFO("Releasing lport %s from this chassis.", - binding_rec->logical_port); - if (binding_rec->encap) { - sbrec_port_binding_set_encap(binding_rec, NULL); - } - sbrec_port_binding_set_chassis(binding_rec, NULL); - sbrec_port_binding_set_virtual_parent(binding_rec, NULL); - } -} - static void add_localnet_egress_interface_mappings( const struct sbrec_port_binding *port_binding, @@ -696,6 +490,360 @@ consider_localnet_port(const struct sbrec_port_binding *binding_rec, ld->localnet_port = binding_rec; } +enum local_binding_type { + BT_VIF, + BT_CHILD, + BT_VIRTUAL +}; + +struct local_binding { + struct ovs_list node; /* In parent if any. */ + char *name; + enum local_binding_type type; + const struct ovsrec_interface *iface; + const struct sbrec_port_binding *pb; + + struct ovs_list children; +}; + +static struct local_binding * +local_binding_create(const char *name, const struct ovsrec_interface *iface, + const struct sbrec_port_binding *pb, + enum local_binding_type type) +{ + struct local_binding *lbinding = xzalloc(sizeof *lbinding); + lbinding->name = xstrdup(name); + lbinding->type = type; + lbinding->pb = pb; + lbinding->iface = iface; + ovs_list_init(&lbinding->children); + return lbinding; +} + +static void +local_binding_add(struct shash *local_bindings, struct local_binding *lbinding) +{ + shash_add(local_bindings, lbinding->name, lbinding); +} + +static struct local_binding * +local_binding_find(struct shash *local_bindings, const char *name) +{ + return shash_find_data(local_bindings, name); +} + +static void +local_binding_destroy(struct local_binding *lbinding) +{ + struct local_binding *c, *next; + LIST_FOR_EACH_SAFE (c, next, node, &lbinding->children) { + ovs_list_remove(&c->node); + free(c->name); + free(c); + } + free(lbinding->name); + free(lbinding); +} + +void +local_bindings_destroy(struct shash *local_bindings) +{ + struct shash_node *node, *next; + SHASH_FOR_EACH_SAFE (node, next, local_bindings) { + struct local_binding *lbinding = node->data; + struct local_binding *c, *n; + LIST_FOR_EACH_SAFE (c, n, node, &lbinding->children) { + ovs_list_remove(&c->node); + free(c->name); + free(c); + } + } + + shash_destroy(local_bindings); +} + +static void +local_binding_add_child(struct local_binding *lbinding, + struct local_binding *child) +{ + struct local_binding *l; + LIST_FOR_EACH (l, node, &lbinding->children) { + if (l == child) { + return; + } + } + + ovs_list_push_back(&lbinding->children, &child->node); +} + +static struct local_binding * +local_binding_find_child(struct local_binding *lbinding, + const char *child_name) +{ + struct local_binding *l; + LIST_FOR_EACH (l, node, &lbinding->children) { + if (!strcmp(l->name, child_name)) { + return l; + } + } + + return NULL; +} + +void +binding_add_vport_to_local_bindings(struct shash *local_bindings, + const struct sbrec_port_binding *parent, + const struct sbrec_port_binding *vport) +{ + struct local_binding *lbinding = local_binding_find(local_bindings, + parent->logical_port); + ovs_assert(lbinding); + struct local_binding *vbinding = + local_binding_find_child(lbinding, vport->logical_port); + if (!vbinding) { + vbinding = local_binding_create(vport->logical_port, lbinding->iface, + vport, BT_VIRTUAL); + local_binding_add_child(lbinding, vbinding); + } else { + vbinding->type = BT_VIRTUAL; + } +} + +static void +claim_lport(const struct sbrec_port_binding *pb, + const struct sbrec_chassis *chassis_rec, + const struct ovsrec_interface *iface_rec) +{ + if (pb->chassis != chassis_rec) { + if (pb->chassis) { + VLOG_INFO("Changing chassis for lport %s from %s to %s.", + pb->logical_port, pb->chassis->name, + chassis_rec->name); + } else { + VLOG_INFO("Claiming lport %s for this chassis.", pb->logical_port); + } + for (int i = 0; i < pb->n_mac; i++) { + VLOG_INFO("%s: Claiming %s", pb->logical_port, pb->mac[i]); + } + + sbrec_port_binding_set_chassis(pb, chassis_rec); + } + + /* Check if the port encap binding, if any, has changed */ + struct sbrec_encap *encap_rec = + sbrec_get_port_encap(chassis_rec, iface_rec); + if (encap_rec && pb->encap != encap_rec) { + sbrec_port_binding_set_encap(pb, encap_rec); + } +} + +static void +release_lport(const struct sbrec_port_binding *pb) +{ + VLOG_INFO("Releasing lport %s from this chassis.", pb->logical_port); + if (pb->encap) { + sbrec_port_binding_set_encap(pb, NULL); + } + sbrec_port_binding_set_chassis(pb, NULL); + + if (pb->virtual_parent) { + sbrec_port_binding_set_virtual_parent(pb, NULL); + } +} + +static void +consider_port_binding_for_vif(const struct sbrec_port_binding *pb, + struct binding_ctx_in *b_ctx_in, + enum local_binding_type binding_type, + struct local_binding *lbinding, + struct binding_ctx_out *b_ctx_out, + struct hmap *qos_map) +{ + const char *vif_chassis = smap_get(&pb->options, "requested-chassis"); + bool can_bind = !vif_chassis || !vif_chassis[0] + || !strcmp(vif_chassis, b_ctx_in->chassis_rec->name) + || !strcmp(vif_chassis, b_ctx_in->chassis_rec->hostname); + + if (lbinding && lbinding->pb && can_bind) { + claim_lport(pb, b_ctx_in->chassis_rec, lbinding->iface); + + switch (binding_type) { + case BT_VIF: + lbinding->pb = pb; + break; + case BT_CHILD: + case BT_VIRTUAL: + { + /* Add child logical port to the set of all local ports. */ + sset_add(b_ctx_out->local_lports, pb->logical_port); + struct local_binding *child = + local_binding_find_child(lbinding, pb->logical_port); + if (!child) { + child = local_binding_create(pb->logical_port, lbinding->iface, + pb, binding_type); + local_binding_add_child(lbinding, child); + } else { + ovs_assert(child->type == BT_CHILD || + child->type == BT_VIRTUAL); + child->pb = pb; + child->iface = lbinding->iface; + } + break; + default: + break; + } + } + + add_local_datapath(b_ctx_in->sbrec_datapath_binding_by_key, + b_ctx_in->sbrec_port_binding_by_datapath, + b_ctx_in->sbrec_port_binding_by_name, + pb->datapath, false, b_ctx_out->local_datapaths); + update_local_lport_ids(b_ctx_out->local_lport_ids, pb); + if (lbinding->iface && qos_map && b_ctx_in->ovs_idl_txn) { + get_qos_params(pb, qos_map); + } + } else if (lbinding && lbinding->pb && !can_bind) { + static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 1); + VLOG_INFO_RL(&rl, + "Not claiming lport %s, chassis %s " + "requested-chassis %s", + pb->logical_port, + b_ctx_in->chassis_rec->name, + vif_chassis); + } + + if (pb->chassis == b_ctx_in->chassis_rec) { + if (!lbinding || !lbinding->pb || !can_bind) { + release_lport(pb); + } + } +} + +static void +consider_port_binding(const struct sbrec_port_binding *pb, + struct binding_ctx_in *b_ctx_in, + struct binding_ctx_out *b_ctx_out, + struct hmap *qos_map) +{ + + bool our_chassis = is_our_chassis(b_ctx_in->chassis_rec, pb, + b_ctx_in->active_tunnels, NULL, + b_ctx_out->local_lports); + + if (!strcmp(pb->type, "l2gateway")) { + if (our_chassis) { + sset_add(b_ctx_out->local_lports, pb->logical_port); + add_local_datapath(b_ctx_in->sbrec_datapath_binding_by_key, + b_ctx_in->sbrec_port_binding_by_datapath, + b_ctx_in->sbrec_port_binding_by_name, + pb->datapath, false, + b_ctx_out->local_datapaths); + } + } else if (!strcmp(pb->type, "chassisredirect")) { + if (ha_chassis_group_contains(pb->ha_chassis_group, + b_ctx_in->chassis_rec)) { + add_local_datapath(b_ctx_in->sbrec_datapath_binding_by_key, + b_ctx_in->sbrec_port_binding_by_datapath, + b_ctx_in->sbrec_port_binding_by_name, + pb->datapath, false, + b_ctx_out->local_datapaths); + } + } else if (!strcmp(pb->type, "l3gateway")) { + if (our_chassis) { + add_local_datapath(b_ctx_in->sbrec_datapath_binding_by_key, + b_ctx_in->sbrec_port_binding_by_datapath, + b_ctx_in->sbrec_port_binding_by_name, + pb->datapath, true, b_ctx_out->local_datapaths); + } + } else if (!strcmp(pb->type, "localnet")) { + /* Add all localnet ports to local_lports so that we allocate ct zones + * for them. */ + sset_add(b_ctx_out->local_lports, pb->logical_port); + if (qos_map && b_ctx_in->ovs_idl_txn) { + get_qos_params(pb, qos_map); + } + } else if (!strcmp(pb->type, "external")) { + if (ha_chassis_group_contains(pb->ha_chassis_group, + b_ctx_in->chassis_rec)) { + add_local_datapath(b_ctx_in->sbrec_datapath_binding_by_key, + b_ctx_in->sbrec_port_binding_by_datapath, + b_ctx_in->sbrec_port_binding_by_name, + pb->datapath, false, + b_ctx_out->local_datapaths); + } + } + + if (our_chassis || !strcmp(pb->type, "localnet")) { + update_local_lport_ids(b_ctx_out->local_lport_ids, pb); + } + + ovs_assert(b_ctx_in->ovnsb_idl_txn); + if (our_chassis) { + claim_lport(pb, b_ctx_in->chassis_rec, NULL); + } else if (pb->chassis == b_ctx_in->chassis_rec) { + release_lport(pb); + } +} + +static void +build_local_bindings_from_local_ifaces(struct binding_ctx_in *b_ctx_in, + struct binding_ctx_out *b_ctx_out) +{ + int i; + for (i = 0; i < b_ctx_in->br_int->n_ports; i++) { + const struct ovsrec_port *port_rec = b_ctx_in->br_int->ports[i]; + const char *iface_id; + int j; + + if (!strcmp(port_rec->name, b_ctx_in->br_int->name)) { + continue; + } + + for (j = 0; j < port_rec->n_interfaces; j++) { + const struct ovsrec_interface *iface_rec; + + iface_rec = port_rec->interfaces[j]; + iface_id = smap_get(&iface_rec->external_ids, "iface-id"); + int64_t ofport = iface_rec->n_ofport ? *iface_rec->ofport : 0; + + if (iface_id && ofport > 0) { + const struct sbrec_port_binding *pb + = lport_lookup_by_name( + b_ctx_in->sbrec_port_binding_by_name, iface_id); + struct local_binding *lbinding = + local_binding_find(b_ctx_out->local_bindings, iface_id); + if (!lbinding) { + lbinding = local_binding_create(iface_id, iface_rec, pb, + BT_VIF); + local_binding_add(b_ctx_out->local_bindings, lbinding); + } else { + lbinding->type = BT_VIF; + lbinding->pb = pb; + } + sset_add(b_ctx_out->local_lports, iface_id); + } + + /* Check if this is a tunnel interface. */ + if (smap_get(&iface_rec->options, "remote_ip")) { + const char *tunnel_iface + = smap_get(&iface_rec->status, "tunnel_egress_iface"); + if (tunnel_iface) { + sset_add(b_ctx_out->egress_ifaces, tunnel_iface); + } + } + } + } + + struct shash_node *node, *next; + SHASH_FOR_EACH_SAFE (node, next, b_ctx_out->local_bindings) { + struct local_binding *lbinding = node->data; + if (!sset_contains(b_ctx_out->local_lports, lbinding->name)) { + local_binding_destroy(lbinding); + shash_delete(b_ctx_out->local_bindings, node); + } + } +} + void binding_run(struct binding_ctx_in *b_ctx_in, struct binding_ctx_out *b_ctx_out) { @@ -705,49 +853,62 @@ binding_run(struct binding_ctx_in *b_ctx_in, struct binding_ctx_out *b_ctx_out) const struct sbrec_port_binding *binding_rec; struct shash bridge_mappings = SHASH_INITIALIZER(&bridge_mappings); - struct shash lport_to_iface = SHASH_INITIALIZER(&lport_to_iface); - struct sset egress_ifaces = SSET_INITIALIZER(&egress_ifaces); struct hmap qos_map; hmap_init(&qos_map); if (b_ctx_in->br_int) { - get_local_iface_ids(b_ctx_in->br_int, &lport_to_iface, - b_ctx_out->local_lports, &egress_ifaces); + build_local_bindings_from_local_ifaces(b_ctx_in, b_ctx_out); } - /* Array to store pointers to local virtual ports. It is populated by - * consider_local_datapath. - */ - const struct sbrec_port_binding **vpbs = NULL; - size_t n_vpbs = 0; - size_t n_allocated_vpbs = 0; + struct hmap *qos_map_ptr = + !sset_is_empty(b_ctx_out->egress_ifaces) ? &qos_map : NULL; /* Run through each binding record to see if it is resident on this * chassis and update the binding accordingly. This includes both - * directly connected logical ports and children of those ports. - * Virtual ports are just added to vpbs array and will be processed - * later. This is special case for virtual ports is needed in order to - * make sure we update the virtual_parent port bindings first. + * directly connected logical ports and children of those ports + * (which also includes virtual ports). */ SBREC_PORT_BINDING_TABLE_FOR_EACH (binding_rec, b_ctx_in->port_binding_table) { - const struct ovsrec_interface *iface_rec - = shash_find_data(&lport_to_iface, binding_rec->logical_port); - vpbs = - consider_local_datapath(binding_rec, - sset_is_empty(&egress_ifaces) ? NULL : - &qos_map, iface_rec, b_ctx_in, b_ctx_out, - vpbs, &n_vpbs, &n_allocated_vpbs); - } + if (!strcmp(binding_rec->type, "patch") || + !strcmp(binding_rec->type, "localport") || + !strcmp(binding_rec->type, "vtep")) { + update_local_lport_ids(b_ctx_out->local_lport_ids, binding_rec); + continue; + } - /* Now also update the virtual ports in case their parent ports were - * updated above. - */ - for (size_t i = 0; i < n_vpbs; i++) { - consider_local_virtual_port(b_ctx_in->sbrec_port_binding_by_name, - b_ctx_in->chassis_rec, vpbs[i]); + bool consider_for_vif = false; + struct local_binding *lbinding = NULL; + enum local_binding_type binding_type = BT_VIF; + if (!binding_rec->type[0]) { + if (!binding_rec->parent_port || !binding_rec->parent_port[0]) { + lbinding = local_binding_find(b_ctx_out->local_bindings, + binding_rec->logical_port); + } else { + lbinding = local_binding_find(b_ctx_out->local_bindings, + binding_rec->parent_port); + binding_type = BT_CHILD; + } + + consider_for_vif = true; + } else if (!strcmp(binding_rec->type, "virtual") && + binding_rec->virtual_parent && + binding_rec->virtual_parent[0]) { + lbinding = local_binding_find(b_ctx_out->local_bindings, + binding_rec->virtual_parent); + consider_for_vif = true; + binding_type = BT_VIRTUAL; + } + + if (consider_for_vif) { + consider_port_binding_for_vif(binding_rec, b_ctx_in, + binding_type, lbinding, b_ctx_out, + qos_map_ptr); + } else { + consider_port_binding(binding_rec, b_ctx_in, b_ctx_out, + qos_map_ptr); + } } - free(vpbs); add_ovs_bridge_mappings(b_ctx_in->ovs_table, b_ctx_in->bridge_table, &bridge_mappings); @@ -759,49 +920,39 @@ binding_run(struct binding_ctx_in *b_ctx_in, struct binding_ctx_out *b_ctx_out) b_ctx_in->port_binding_table) { if (!strcmp(binding_rec->type, "localnet")) { consider_localnet_port(binding_rec, &bridge_mappings, - &egress_ifaces, b_ctx_out->local_datapaths); + b_ctx_out->egress_ifaces, + b_ctx_out->local_datapaths); } } shash_destroy(&bridge_mappings); - if (!sset_is_empty(&egress_ifaces) + if (!sset_is_empty(b_ctx_out->egress_ifaces) && set_noop_qos(b_ctx_in->ovs_idl_txn, b_ctx_in->port_table, - b_ctx_in->qos_table, &egress_ifaces)) { + b_ctx_in->qos_table, b_ctx_out->egress_ifaces)) { const char *entry; - SSET_FOR_EACH (entry, &egress_ifaces) { + SSET_FOR_EACH (entry, b_ctx_out->egress_ifaces) { setup_qos(entry, &qos_map); } } - shash_destroy(&lport_to_iface); - sset_destroy(&egress_ifaces); hmap_destroy(&qos_map); } /* Returns true if port-binding changes potentially require flow changes on * the current chassis. Returns false if we are sure there is no impact. */ bool -binding_evaluate_port_binding_changes( - const struct sbrec_port_binding_table *pb_table, - const struct ovsrec_bridge *br_int, - const struct sbrec_chassis *chassis_rec, - struct sset *active_tunnels, - struct sset *local_lports) +binding_evaluate_port_binding_changes(struct binding_ctx_in *b_ctx_in, + struct binding_ctx_out *b_ctx_out) { - if (!chassis_rec) { + if (!b_ctx_in->chassis_rec) { return true; } bool changed = false; const struct sbrec_port_binding *binding_rec; - struct shash lport_to_iface = SHASH_INITIALIZER(&lport_to_iface); - struct sset egress_ifaces = SSET_INITIALIZER(&egress_ifaces); - if (br_int) { - get_local_iface_ids(br_int, &lport_to_iface, local_lports, - &egress_ifaces); - } - SBREC_PORT_BINDING_TABLE_FOR_EACH_TRACKED (binding_rec, pb_table) { + SBREC_PORT_BINDING_TABLE_FOR_EACH_TRACKED (binding_rec, + b_ctx_in->port_binding_table) { /* XXX: currently OVSDB change tracking doesn't support getting old * data when the operation is update, so if a port-binding moved from * this chassis to another, there is no easy way to find out the @@ -813,22 +964,33 @@ binding_evaluate_port_binding_changes( * interface table will be updated, which will trigger recompute. * * - If the port is not a regular VIF, always trigger recompute. */ - if (binding_rec->chassis == chassis_rec) { + if (binding_rec->chassis == b_ctx_in->chassis_rec) { + changed = true; + break; + } + + if (strcmp(binding_rec->type, "")) { changed = true; break; } - const struct ovsrec_interface *iface_rec - = shash_find_data(&lport_to_iface, binding_rec->logical_port); - if (is_our_chassis(chassis_rec, binding_rec, active_tunnels, iface_rec, - local_lports) || strcmp(binding_rec->type, "")) { + struct local_binding *lbinding = NULL; + if (!binding_rec->type[0]) { + if (!binding_rec->parent_port || !binding_rec->parent_port[0]) { + lbinding = local_binding_find(b_ctx_out->local_bindings, + binding_rec->logical_port); + } else { + lbinding = local_binding_find(b_ctx_out->local_bindings, + binding_rec->parent_port); + } + } + + if (lbinding) { changed = true; break; } } - shash_destroy(&lport_to_iface); - sset_destroy(&egress_ifaces); return changed; } diff --git a/controller/binding.h b/controller/binding.h index d0b8331af..6bee1d12e 100644 --- a/controller/binding.h +++ b/controller/binding.h @@ -31,6 +31,7 @@ struct ovsrec_open_vswitch_table; struct sbrec_chassis; struct sbrec_port_binding_table; struct sset; +struct sbrec_port_binding; struct binding_ctx_in { struct ovsdb_idl_txn *ovnsb_idl_txn; @@ -51,8 +52,10 @@ struct binding_ctx_in { struct binding_ctx_out { struct hmap *local_datapaths; + struct shash *local_bindings; struct sset *local_lports; struct sset *local_lport_ids; + struct sset *egress_ifaces; }; void binding_register_ovs_idl(struct ovsdb_idl *); @@ -60,11 +63,10 @@ void binding_run(struct binding_ctx_in *, struct binding_ctx_out *); bool binding_cleanup(struct ovsdb_idl_txn *ovnsb_idl_txn, const struct sbrec_port_binding_table *, const struct sbrec_chassis *); -bool binding_evaluate_port_binding_changes( - const struct sbrec_port_binding_table *, - const struct ovsrec_bridge *br_int, - const struct sbrec_chassis *, - struct sset *active_tunnels, - struct sset *local_lports); - +bool binding_evaluate_port_binding_changes(struct binding_ctx_in *, + struct binding_ctx_out *); +void local_bindings_destroy(struct shash *local_bindings); +void binding_add_vport_to_local_bindings( + struct shash *local_bindings, const struct sbrec_port_binding *parent, + const struct sbrec_port_binding *vport); #endif /* controller/binding.h */ diff --git a/controller/ovn-controller.c b/controller/ovn-controller.c index 4930d5ffb..8cc27cebf 100644 --- a/controller/ovn-controller.c +++ b/controller/ovn-controller.c @@ -958,6 +958,9 @@ struct ed_type_runtime_data { /* Contains "struct local_datapath" nodes. */ struct hmap local_datapaths; + /* Contains "struct local_bindings" nodes. */ + struct shash local_bindings; + /* Contains the name of each logical port resident on the local * hypervisor. These logical ports include the VIFs (and their child * logical ports, if any) that belong to VMs running on the hypervisor, @@ -969,6 +972,8 @@ struct ed_type_runtime_data { * _ */ struct sset local_lport_ids; struct sset active_tunnels; + + struct sset egress_ifaces; }; static void * @@ -981,6 +986,8 @@ en_runtime_data_init(struct engine_node *node OVS_UNUSED, sset_init(&data->local_lports); sset_init(&data->local_lport_ids); sset_init(&data->active_tunnels); + sset_init(&data->egress_ifaces); + shash_init(&data->local_bindings); return data; } @@ -992,6 +999,7 @@ en_runtime_data_cleanup(void *data) sset_destroy(&rt_data->local_lports); sset_destroy(&rt_data->local_lport_ids); sset_destroy(&rt_data->active_tunnels); + sset_init(&rt_data->egress_ifaces); struct local_datapath *cur_node, *next_node; HMAP_FOR_EACH_SAFE (cur_node, next_node, hmap_node, &rt_data->local_datapaths) { @@ -1000,6 +1008,7 @@ en_runtime_data_cleanup(void *data) free(cur_node); } hmap_destroy(&rt_data->local_datapaths); + local_bindings_destroy(&rt_data->local_bindings); } static void @@ -1072,6 +1081,8 @@ init_binding_ctx(struct engine_node *node, b_ctx_out->local_datapaths = &rt_data->local_datapaths; b_ctx_out->local_lports = &rt_data->local_lports; b_ctx_out->local_lport_ids = &rt_data->local_lport_ids; + b_ctx_out->egress_ifaces = &rt_data->egress_ifaces; + b_ctx_out->local_bindings = &rt_data->local_bindings; } static void @@ -1098,9 +1109,11 @@ en_runtime_data_run(struct engine_node *node, void *data) sset_destroy(local_lports); sset_destroy(local_lport_ids); sset_destroy(active_tunnels); + sset_destroy(&rt_data->egress_ifaces); sset_init(local_lports); sset_init(local_lport_ids); sset_init(active_tunnels); + sset_init(&rt_data->egress_ifaces); } struct binding_ctx_in b_ctx_in; @@ -1129,35 +1142,12 @@ static bool runtime_data_sb_port_binding_handler(struct engine_node *node, void *data) { struct ed_type_runtime_data *rt_data = data; - struct sset *local_lports = &rt_data->local_lports; - struct sset *active_tunnels = &rt_data->active_tunnels; - - struct ovsrec_open_vswitch_table *ovs_table = - (struct ovsrec_open_vswitch_table *)EN_OVSDB_GET( - engine_get_input("OVS_open_vswitch", node)); - struct ovsrec_bridge_table *bridge_table = - (struct ovsrec_bridge_table *)EN_OVSDB_GET( - engine_get_input("OVS_bridge", node)); - const char *chassis_id = chassis_get_id(); - const struct ovsrec_bridge *br_int = get_br_int(bridge_table, ovs_table); - - ovs_assert(br_int && chassis_id); - - struct ovsdb_idl_index *sbrec_chassis_by_name = - engine_ovsdb_node_get_index( - engine_get_input("SB_chassis", node), - "name"); - - const struct sbrec_chassis *chassis - = chassis_lookup_by_name(sbrec_chassis_by_name, chassis_id); - ovs_assert(chassis); - - struct sbrec_port_binding_table *pb_table = - (struct sbrec_port_binding_table *)EN_OVSDB_GET( - engine_get_input("SB_port_binding", node)); + struct binding_ctx_in b_ctx_in; + struct binding_ctx_out b_ctx_out; + init_binding_ctx(node, rt_data, &b_ctx_in, &b_ctx_out); - bool changed = binding_evaluate_port_binding_changes( - pb_table, br_int, chassis, active_tunnels, local_lports); + bool changed = binding_evaluate_port_binding_changes(&b_ctx_in, + &b_ctx_out); return !changed; } @@ -2113,7 +2103,8 @@ main(int argc, char *argv[]) ovnsb_idl_loop.idl), br_int, chassis, &runtime_data->local_datapaths, - &runtime_data->active_tunnels); + &runtime_data->active_tunnels, + &runtime_data->local_bindings); if (engine_node_changed(&en_runtime_data)) { update_sb_monitors(ovnsb_idl_loop.idl, chassis, &runtime_data->local_lports, diff --git a/controller/pinctrl.c b/controller/pinctrl.c index 8bf19776c..e9f83b26e 100644 --- a/controller/pinctrl.c +++ b/controller/pinctrl.c @@ -18,6 +18,7 @@ #include "pinctrl.h" +#include "binding.h" #include "coverage.h" #include "csum.h" #include "dirs.h" @@ -278,7 +279,8 @@ static void run_put_vport_bindings( struct ovsdb_idl_txn *ovnsb_idl_txn, struct ovsdb_idl_index *sbrec_datapath_binding_by_key, struct ovsdb_idl_index *sbrec_port_binding_by_key, - const struct sbrec_chassis *chassis) + const struct sbrec_chassis *chassis, + struct shash *local_bindings) OVS_REQUIRES(pinctrl_mutex); static void wait_put_vport_bindings(struct ovsdb_idl_txn *ovnsb_idl_txn); static void pinctrl_handle_bind_vport(const struct flow *md, @@ -2189,7 +2191,8 @@ pinctrl_run(struct ovsdb_idl_txn *ovnsb_idl_txn, const struct ovsrec_bridge *br_int, const struct sbrec_chassis *chassis, const struct hmap *local_datapaths, - const struct sset *active_tunnels) + const struct sset *active_tunnels, + struct shash *local_bindings) { ovs_mutex_lock(&pinctrl_mutex); if (br_int && (!pinctrl.br_int_name || strcmp(pinctrl.br_int_name, @@ -2206,7 +2209,7 @@ pinctrl_run(struct ovsdb_idl_txn *ovnsb_idl_txn, sbrec_port_binding_by_key, sbrec_mac_binding_by_lport_ip); run_put_vport_bindings(ovnsb_idl_txn, sbrec_datapath_binding_by_key, - sbrec_port_binding_by_key, chassis); + sbrec_port_binding_by_key, chassis, local_bindings); send_garp_rarp_prepare(sbrec_port_binding_by_datapath, sbrec_port_binding_by_name, br_int, chassis, local_datapaths, active_tunnels); @@ -4875,7 +4878,8 @@ run_put_vport_binding(struct ovsdb_idl_txn *ovnsb_idl_txn OVS_UNUSED, struct ovsdb_idl_index *sbrec_datapath_binding_by_key, struct ovsdb_idl_index *sbrec_port_binding_by_key, const struct sbrec_chassis *chassis, - const struct put_vport_binding *vpb) + const struct put_vport_binding *vpb, + struct shash *local_bindings) { /* Convert logical datapath and logical port key into lport. */ const struct sbrec_port_binding *pb = lport_lookup_by_key( @@ -4900,6 +4904,7 @@ run_put_vport_binding(struct ovsdb_idl_txn *ovnsb_idl_txn OVS_UNUSED, pb->logical_port, parent->logical_port); sbrec_port_binding_set_chassis(pb, chassis); sbrec_port_binding_set_virtual_parent(pb, parent->logical_port); + binding_add_vport_to_local_bindings(local_bindings, parent, pb); } } } @@ -4910,7 +4915,8 @@ static void run_put_vport_bindings(struct ovsdb_idl_txn *ovnsb_idl_txn, struct ovsdb_idl_index *sbrec_datapath_binding_by_key, struct ovsdb_idl_index *sbrec_port_binding_by_key, - const struct sbrec_chassis *chassis) + const struct sbrec_chassis *chassis, + struct shash *local_bindings) OVS_REQUIRES(pinctrl_mutex) { if (!ovnsb_idl_txn) { @@ -4920,7 +4926,8 @@ run_put_vport_bindings(struct ovsdb_idl_txn *ovnsb_idl_txn, const struct put_vport_binding *vpb; HMAP_FOR_EACH (vpb, hmap_node, &put_vport_bindings) { run_put_vport_binding(ovnsb_idl_txn, sbrec_datapath_binding_by_key, - sbrec_port_binding_by_key, chassis, vpb); + sbrec_port_binding_by_key, chassis, vpb, + local_bindings); } flush_put_vport_bindings(); diff --git a/controller/pinctrl.h b/controller/pinctrl.h index 8fa4baae9..bf2d02455 100644 --- a/controller/pinctrl.h +++ b/controller/pinctrl.h @@ -31,6 +31,7 @@ struct sbrec_chassis; struct sbrec_dns_table; struct sbrec_controller_event_table; struct sbrec_service_monitor_table; +struct shash; void pinctrl_init(void); void pinctrl_run(struct ovsdb_idl_txn *ovnsb_idl_txn, @@ -46,7 +47,8 @@ void pinctrl_run(struct ovsdb_idl_txn *ovnsb_idl_txn, const struct sbrec_service_monitor_table *, const struct ovsrec_bridge *, const struct sbrec_chassis *, const struct hmap *local_datapaths, - const struct sset *active_tunnels); + const struct sset *active_tunnels, + struct shash *local_bindings); void pinctrl_wait(struct ovsdb_idl_txn *ovnsb_idl_txn); void pinctrl_destroy(void); From patchwork Mon Mar 16 11:15:43 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Numan Siddique X-Patchwork-Id: 1255404 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=openvswitch.org (client-ip=140.211.166.133; helo=hemlock.osuosl.org; envelope-from=ovs-dev-bounces@openvswitch.org; receiver=) Authentication-Results: ozlabs.org; dmarc=none (p=none dis=none) header.from=ovn.org Received: from hemlock.osuosl.org (smtp2.osuosl.org [140.211.166.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 48gtyt0QhWz9sQt for ; Mon, 16 Mar 2020 22:16:06 +1100 (AEDT) Received: from localhost (localhost [127.0.0.1]) by hemlock.osuosl.org (Postfix) with ESMTP id E715B8995A; Mon, 16 Mar 2020 11:16:03 +0000 (UTC) X-Virus-Scanned: amavisd-new at osuosl.org Received: from hemlock.osuosl.org ([127.0.0.1]) by localhost (.osuosl.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id hRzaG-mYnTJ8; Mon, 16 Mar 2020 11:16:01 +0000 (UTC) Received: from lists.linuxfoundation.org (lf-lists.osuosl.org [140.211.9.56]) by hemlock.osuosl.org (Postfix) with ESMTP id 9026E8994D; Mon, 16 Mar 2020 11:16:01 +0000 (UTC) Received: from lf-lists.osuosl.org (localhost [127.0.0.1]) by lists.linuxfoundation.org (Postfix) with ESMTP id 72855C1D7E; Mon, 16 Mar 2020 11:16:01 +0000 (UTC) X-Original-To: dev@openvswitch.org Delivered-To: ovs-dev@lists.linuxfoundation.org Received: from whitealder.osuosl.org (smtp1.osuosl.org [140.211.166.138]) by lists.linuxfoundation.org (Postfix) with ESMTP id AE5C4C013E for ; Mon, 16 Mar 2020 11:16:00 +0000 (UTC) Received: from localhost (localhost [127.0.0.1]) by whitealder.osuosl.org (Postfix) with ESMTP id A368588623 for ; Mon, 16 Mar 2020 11:16:00 +0000 (UTC) X-Virus-Scanned: amavisd-new at osuosl.org Received: from whitealder.osuosl.org ([127.0.0.1]) by localhost (.osuosl.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id mdG2V7H7cAiz for ; Mon, 16 Mar 2020 11:15:58 +0000 (UTC) X-Greylist: domain auto-whitelisted by SQLgrey-1.7.6 Received: from relay6-d.mail.gandi.net (relay6-d.mail.gandi.net [217.70.183.198]) by whitealder.osuosl.org (Postfix) with ESMTPS id 2DE0888615 for ; Mon, 16 Mar 2020 11:15:58 +0000 (UTC) X-Originating-IP: 27.7.184.93 Received: from nummac.local (unknown [27.7.184.93]) (Authenticated sender: numans@ovn.org) by relay6-d.mail.gandi.net (Postfix) with ESMTPSA id A7450C0018; Mon, 16 Mar 2020 11:15:55 +0000 (UTC) From: numans@ovn.org To: dev@openvswitch.org Date: Mon, 16 Mar 2020 16:45:43 +0530 Message-Id: <20200316111543.1687608-1-numans@ovn.org> X-Mailer: git-send-email 2.24.1 In-Reply-To: <20200316111034.1686721-1-numans@ovn.org> References: <20200316111034.1686721-1-numans@ovn.org> MIME-Version: 1.0 Subject: [ovs-dev] [PATCH ovn 3/6] ovn-controller: I-P for SB port binding and OVS interface in runtime_data. X-BeenThere: ovs-dev@openvswitch.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: ovs-dev-bounces@openvswitch.org Sender: "dev" From: Numan Siddique This patch handles SB port binding changes and OVS interface changes incrementally in the runtime_data stage which otherwise would have resulted in calls to binding_run(). Prior to this patch, port binding changes were handled differently. The changes were only evaluated to see if they need full recompute or they can be ignored. With this patch, the runtime data is updated with the changes (both SB port binding and OVS interface) and ports are claimed/released in the handlers itself, avoiding the need to trigger recompute of runtime data stage. Signed-off-by: Numan Siddique --- controller/binding.c | 467 ++++++++++++++++++++++++++++++------ controller/binding.h | 7 +- controller/ovn-controller.c | 55 ++++- 3 files changed, 448 insertions(+), 81 deletions(-) diff --git a/controller/binding.c b/controller/binding.c index 34bd475de..ce4467f31 100644 --- a/controller/binding.c +++ b/controller/binding.c @@ -349,17 +349,6 @@ setup_qos(const char *egress_iface, struct hmap *queue_map) netdev_close(netdev_phy); } -static void -update_local_lport_ids(struct sset *local_lport_ids, - const struct sbrec_port_binding *binding_rec) -{ - char buf[16]; - snprintf(buf, sizeof(buf), "%"PRId64"_%"PRId64, - binding_rec->datapath->tunnel_key, - binding_rec->tunnel_key); - sset_add(local_lport_ids, buf); -} - /* * Get the encap from the chassis for this port. The interface * may have an external_ids:encap-ip= set; if so we @@ -490,6 +479,28 @@ consider_localnet_port(const struct sbrec_port_binding *binding_rec, ld->localnet_port = binding_rec; } +static void +update_local_lport_ids(struct sset *local_lport_ids, + const struct sbrec_port_binding *binding_rec) +{ + char buf[16]; + snprintf(buf, sizeof(buf), "%"PRId64"_%"PRId64, + binding_rec->datapath->tunnel_key, + binding_rec->tunnel_key); + sset_add(local_lport_ids, buf); +} + +static void +remove_local_lport_ids(const struct sbrec_port_binding *binding_rec, + struct sset *local_lport_ids) +{ + char buf[16]; + snprintf(buf, sizeof(buf), "%"PRId64"_%"PRId64, + binding_rec->datapath->tunnel_key, + binding_rec->tunnel_key); + sset_find_and_delete(local_lport_ids, buf); +} + enum local_binding_type { BT_VIF, BT_CHILD, @@ -545,18 +556,21 @@ local_binding_destroy(struct local_binding *lbinding) free(lbinding); } +static +void local_binding_delete(struct shash *local_bindings, + struct local_binding *lbinding) +{ + shash_find_and_delete(local_bindings, lbinding->name); + local_binding_destroy(lbinding); +} + void local_bindings_destroy(struct shash *local_bindings) { struct shash_node *node, *next; SHASH_FOR_EACH_SAFE (node, next, local_bindings) { struct local_binding *lbinding = node->data; - struct local_binding *c, *n; - LIST_FOR_EACH_SAFE (c, n, node, &lbinding->children) { - ovs_list_remove(&c->node); - free(c->name); - free(c); - } + local_binding_destroy(lbinding); } shash_destroy(local_bindings); @@ -651,6 +665,22 @@ release_lport(const struct sbrec_port_binding *pb) } } +static void +release_local_binding_children(struct local_binding *lbinding) +{ + struct local_binding *l; + LIST_FOR_EACH (l, node, &lbinding->children) { + release_lport(l->pb); + } +} + +static void +release_local_binding(struct local_binding *lbinding) +{ + release_local_binding_children(lbinding); + release_lport(lbinding->pb); +} + static void consider_port_binding_for_vif(const struct sbrec_port_binding *pb, struct binding_ctx_in *b_ctx_in, @@ -725,7 +755,6 @@ consider_port_binding(const struct sbrec_port_binding *pb, struct binding_ctx_out *b_ctx_out, struct hmap *qos_map) { - bool our_chassis = is_our_chassis(b_ctx_in->chassis_rec, pb, b_ctx_in->active_tunnels, NULL, b_ctx_out->local_lports); @@ -821,6 +850,8 @@ build_local_bindings_from_local_ifaces(struct binding_ctx_in *b_ctx_in, lbinding->pb = pb; } sset_add(b_ctx_out->local_lports, iface_id); + smap_replace(b_ctx_out->local_iface_ids, iface_rec->name, + iface_id); } /* Check if this is a tunnel interface. */ @@ -938,62 +969,6 @@ binding_run(struct binding_ctx_in *b_ctx_in, struct binding_ctx_out *b_ctx_out) hmap_destroy(&qos_map); } -/* Returns true if port-binding changes potentially require flow changes on - * the current chassis. Returns false if we are sure there is no impact. */ -bool -binding_evaluate_port_binding_changes(struct binding_ctx_in *b_ctx_in, - struct binding_ctx_out *b_ctx_out) -{ - if (!b_ctx_in->chassis_rec) { - return true; - } - - bool changed = false; - - const struct sbrec_port_binding *binding_rec; - SBREC_PORT_BINDING_TABLE_FOR_EACH_TRACKED (binding_rec, - b_ctx_in->port_binding_table) { - /* XXX: currently OVSDB change tracking doesn't support getting old - * data when the operation is update, so if a port-binding moved from - * this chassis to another, there is no easy way to find out the - * change. To workaround this problem, we just makes sure if - * any port *related to* this chassis has any change, then trigger - * recompute. - * - * - If a regular VIF is unbound from this chassis, the local ovsdb - * interface table will be updated, which will trigger recompute. - * - * - If the port is not a regular VIF, always trigger recompute. */ - if (binding_rec->chassis == b_ctx_in->chassis_rec) { - changed = true; - break; - } - - if (strcmp(binding_rec->type, "")) { - changed = true; - break; - } - - struct local_binding *lbinding = NULL; - if (!binding_rec->type[0]) { - if (!binding_rec->parent_port || !binding_rec->parent_port[0]) { - lbinding = local_binding_find(b_ctx_out->local_bindings, - binding_rec->logical_port); - } else { - lbinding = local_binding_find(b_ctx_out->local_bindings, - binding_rec->parent_port); - } - } - - if (lbinding) { - changed = true; - break; - } - } - - return changed; -} - /* Returns true if the database is all cleaned up, false if more work is * required. */ bool @@ -1028,3 +1003,347 @@ binding_cleanup(struct ovsdb_idl_txn *ovnsb_idl_txn, return !any_changes; } + +static void +add_local_datapath_peer_port(const struct sbrec_port_binding *pb, + struct binding_ctx_in *b_ctx_in, + struct binding_ctx_out *b_ctx_out, + struct local_datapath *ld) +{ + bool present = false; + for (size_t i = 0; i < ld->n_peer_ports; i++) { + if (ld->peer_ports[i].local == pb) { + present = true; + break; + } + } + + const char *peer_name = smap_get(&pb->options, "peer"); + if (strcmp(pb->type, "patch") || !peer_name) { + return; + } + + const struct sbrec_port_binding *peer; + peer = lport_lookup_by_name(b_ctx_in->sbrec_port_binding_by_name, + peer_name); + + if (!peer || !peer->datapath) { + return; + } + + if (!present) { + ld->n_peer_ports++; + if (ld->n_peer_ports > ld->n_allocated_peer_ports) { + ld->peer_ports = + x2nrealloc(ld->peer_ports, + &ld->n_allocated_peer_ports, + sizeof *ld->peer_ports); + } + ld->peer_ports[ld->n_peer_ports - 1].local = pb; + ld->peer_ports[ld->n_peer_ports - 1].remote = peer; + } + + struct local_datapath *peer_ld = + get_local_datapath(b_ctx_out->local_datapaths, + peer->datapath->tunnel_key); + if (!peer_ld) { + add_local_datapath__(b_ctx_in->sbrec_datapath_binding_by_key, + b_ctx_in->sbrec_port_binding_by_datapath, + b_ctx_in->sbrec_port_binding_by_name, + peer->datapath, false, + 1, b_ctx_out->local_datapaths); + return; + } + + for (size_t i = 0; i < peer_ld->n_peer_ports; i++) { + if (peer_ld->peer_ports[i].local == peer) { + return; + } + } + + peer_ld->n_peer_ports++; + if (peer_ld->n_peer_ports > peer_ld->n_allocated_peer_ports) { + peer_ld->peer_ports = + x2nrealloc(peer_ld->peer_ports, + &peer_ld->n_allocated_peer_ports, + sizeof *peer_ld->peer_ports); + } + peer_ld->peer_ports[peer_ld->n_peer_ports - 1].local = peer; + peer_ld->peer_ports[peer_ld->n_peer_ports - 1].remote = pb; +} + +static void +remove_local_datapath_peer_port(const struct sbrec_port_binding *pb, + struct local_datapath *ld, + struct hmap *local_datapaths) +{ + size_t i =0; + for (i = 0; i < ld->n_peer_ports; i++) { + if (ld->peer_ports[i].local == pb) { + break; + } + } + + if (i == ld->n_peer_ports) { + return; + } + + const struct sbrec_port_binding *peer = ld->peer_ports[i].remote; + + ld->peer_ports[i].local = ld->peer_ports[ld->n_peer_ports - 1].local; + ld->peer_ports[i].remote = ld->peer_ports[ld->n_peer_ports - 1].remote; + ld->n_peer_ports--; + + struct local_datapath *peer_ld = + get_local_datapath(local_datapaths, peer->datapath->tunnel_key); + if (peer_ld) { + remove_local_datapath_peer_port(peer, peer_ld, local_datapaths); + } +} + +static void +update_local_datapath_for_pb(const struct sbrec_port_binding *pb, + struct binding_ctx_in *b_ctx_in, + struct binding_ctx_out *b_ctx_out, + struct local_datapath *ld) +{ + if (!strcmp(pb->type, "patch")) { + add_local_datapath_peer_port(pb, b_ctx_in, b_ctx_out, ld); + } else if (!strcmp(pb->type, "localnet")) { + struct shash bridge_mappings = SHASH_INITIALIZER(&bridge_mappings); + add_ovs_bridge_mappings(b_ctx_in->ovs_table, b_ctx_in->bridge_table, + &bridge_mappings); + consider_localnet_port(pb, &bridge_mappings, b_ctx_out->egress_ifaces, + b_ctx_out->local_datapaths); + shash_destroy(&bridge_mappings); + } else if (!strcmp(pb->type, "l3gateway")) { + const char *chassis_id = smap_get(&pb->options, + "l3gateway-chassis"); + if (chassis_id && !strcmp(chassis_id, b_ctx_in->chassis_rec->name)) { + ld->has_local_l3gateway = true; + } + } + + if (!strcmp(pb->type, "patch") || + !strcmp(pb->type, "localport") || + !strcmp(pb->type, "vtep")) { + update_local_lport_ids(b_ctx_out->local_lport_ids, pb); + } +} + +static void +remove_pb_from_local_datapath(const struct sbrec_port_binding *pb, + const struct sbrec_chassis *chassis_rec, + struct binding_ctx_out *b_ctx_out, + struct local_datapath *ld) +{ + remove_local_lport_ids(pb, b_ctx_out->local_lport_ids); + if (!strcmp(pb->type, "patch") || + !strcmp(pb->type, "l3gateway")) { + remove_local_datapath_peer_port(pb, ld, b_ctx_out->local_datapaths); + } else if (!strcmp(pb->type, "localnet")) { + if (ld->localnet_port && !strcmp(ld->localnet_port->logical_port, + pb->logical_port)) { + ld->localnet_port = NULL; + } + } else if (!strcmp(pb->type, "l3gateway")) { + const char *chassis_id = smap_get(&pb->options, + "l3gateway-chassis"); + if (chassis_id && !strcmp(chassis_id, chassis_rec->name)) { + ld->has_local_l3gateway = false; + } + } +} + +/* Returns true if the ovs interface changes were handled successfully, + * false otherwise. + */ +bool +binding_handle_ovs_interface_changes(struct binding_ctx_in *b_ctx_in, + struct binding_ctx_out *b_ctx_out) +{ + if (!b_ctx_in->chassis_rec) { + return false; + } + + bool handled = true; + struct hmap qos_map = HMAP_INITIALIZER(&qos_map); + struct hmap *qos_map_ptr = + sset_is_empty(b_ctx_out->egress_ifaces) ? NULL : &qos_map; + + const struct ovsrec_interface *iface_rec; + OVSREC_INTERFACE_TABLE_FOR_EACH_TRACKED (iface_rec, + b_ctx_in->iface_table) { + const char *iface_id = smap_get(&iface_rec->external_ids, "iface-id"); + const char *old_iface_id = smap_get(b_ctx_out->local_iface_ids, + iface_rec->name); + if (iface_rec->type && iface_rec->type[0]) { + handled = false; + goto out; + } + + struct local_binding *lbinding = NULL; + struct local_binding *claim_lbinding = NULL; + const char *cleared_iface_id = NULL; + int64_t ofport = iface_rec->n_ofport ? *iface_rec->ofport : 0; + if (!ovsrec_interface_is_deleted(iface_rec)) { + if (iface_id) { + /* Check if iface_id is changed. If so we need to + * release the old port binding and associate this + * inteface to new port binding. */ + if (old_iface_id && strcmp(iface_id, old_iface_id)) { + cleared_iface_id = old_iface_id; + } + } else if (old_iface_id) { + cleared_iface_id = old_iface_id; + } + } else { + cleared_iface_id = iface_id; + iface_id = NULL; + } + + if (cleared_iface_id) { + lbinding = local_binding_find(b_ctx_out->local_bindings, + cleared_iface_id); + if (lbinding && lbinding->pb && + lbinding->pb->chassis == b_ctx_in->chassis_rec) { + release_local_binding(lbinding); + struct local_datapath *ld = + get_local_datapath(b_ctx_out->local_datapaths, + lbinding->pb->datapath->tunnel_key); + if (ld) { + remove_pb_from_local_datapath(lbinding->pb, + b_ctx_in->chassis_rec, + b_ctx_out, ld); + } + local_binding_delete(b_ctx_out->local_bindings, lbinding); + } + + sset_find_and_delete(b_ctx_out->local_lports, cleared_iface_id); + smap_remove(b_ctx_out->local_iface_ids, iface_rec->name); + } + + if (iface_id && ofport > 0) { + sset_add(b_ctx_out->local_lports, iface_id); + smap_replace(b_ctx_out->local_iface_ids, iface_rec->name, + iface_id); + claim_lbinding = + local_binding_find(b_ctx_out->local_bindings, iface_id); + if (!claim_lbinding) { + claim_lbinding = local_binding_create(iface_id, iface_rec, + NULL, BT_VIF); + local_binding_add(b_ctx_out->local_bindings, claim_lbinding); + } else { + claim_lbinding->iface = iface_rec; + } + + if (!claim_lbinding->pb || + strcmp(claim_lbinding->name, + claim_lbinding->pb->logical_port)) { + claim_lbinding->pb = + lport_lookup_by_name(b_ctx_in->sbrec_port_binding_by_name, + claim_lbinding->name); + } + + if (claim_lbinding->pb) { + consider_port_binding_for_vif(claim_lbinding->pb, b_ctx_in, + BT_VIF, claim_lbinding, + b_ctx_out, qos_map_ptr); + } + } + } + + if (qos_map_ptr && set_noop_qos(b_ctx_in->ovs_idl_txn, + b_ctx_in->port_table, + b_ctx_in->qos_table, + b_ctx_out->egress_ifaces)) { + const char *entry; + SSET_FOR_EACH (entry, b_ctx_out->egress_ifaces) { + setup_qos(entry, &qos_map); + } + } + + hmap_destroy(&qos_map); +out: + return handled; +} + +/* Returns true if the port binding changes resulted in local binding + * updates, false otherwise. + */ +bool +binding_handle_port_binding_changes(struct binding_ctx_in *b_ctx_in, + struct binding_ctx_out *b_ctx_out) +{ + bool updated = false; + struct hmap qos_map = HMAP_INITIALIZER(&qos_map); + struct hmap *qos_map_ptr = + sset_is_empty(b_ctx_out->egress_ifaces) ? NULL : &qos_map; + + const struct sbrec_port_binding *pb; + SBREC_PORT_BINDING_TABLE_FOR_EACH_TRACKED (pb, + b_ctx_in->port_binding_table) { + bool consider_for_vif = false; + struct local_binding *lbinding = NULL; + enum local_binding_type binding_type = BT_VIF; + bool is_deleted = sbrec_port_binding_is_deleted(pb); + if (!pb->type[0]) { + if (!pb->parent_port || !pb->parent_port[0]) { + lbinding = local_binding_find(b_ctx_out->local_bindings, + pb->logical_port); + } else { + lbinding = local_binding_find(b_ctx_out->local_bindings, + pb->parent_port); + binding_type = BT_CHILD; + } + + consider_for_vif = true; + } else if (!strcmp(pb->type, "virtual") && + pb->virtual_parent && pb->virtual_parent[0]) { + lbinding = local_binding_find(b_ctx_out->local_bindings, + pb->virtual_parent); + consider_for_vif = true; + binding_type = BT_VIRTUAL; + } + + if (is_deleted) { + if (lbinding) { + updated = true; + lbinding->pb = NULL; + + if (binding_type == BT_VIF) { + release_local_binding_children(lbinding); + } + } + + struct local_datapath *ld = + get_local_datapath(b_ctx_out->local_datapaths, + pb->datapath->tunnel_key); + if (ld) { + remove_pb_from_local_datapath(pb, b_ctx_in->chassis_rec, + b_ctx_out, ld); + } + } else { + if (consider_for_vif) { + if (lbinding) { + updated = true; + lbinding->pb = pb; + consider_port_binding_for_vif(pb, b_ctx_in, binding_type, + lbinding, b_ctx_out, + qos_map_ptr); + } + } else { + updated = true; + consider_port_binding(pb, b_ctx_in, b_ctx_out, qos_map_ptr); + struct local_datapath *ld = + get_local_datapath(b_ctx_out->local_datapaths, + pb->datapath->tunnel_key); + if (ld) { + update_local_datapath_for_pb(pb, b_ctx_in, b_ctx_out, ld); + } + } + } + } + + return updated; +} diff --git a/controller/binding.h b/controller/binding.h index 6bee1d12e..95ca0367d 100644 --- a/controller/binding.h +++ b/controller/binding.h @@ -56,6 +56,7 @@ struct binding_ctx_out { struct sset *local_lports; struct sset *local_lport_ids; struct sset *egress_ifaces; + struct smap *local_iface_ids; }; void binding_register_ovs_idl(struct ovsdb_idl *); @@ -63,10 +64,12 @@ void binding_run(struct binding_ctx_in *, struct binding_ctx_out *); bool binding_cleanup(struct ovsdb_idl_txn *ovnsb_idl_txn, const struct sbrec_port_binding_table *, const struct sbrec_chassis *); -bool binding_evaluate_port_binding_changes(struct binding_ctx_in *, - struct binding_ctx_out *); void local_bindings_destroy(struct shash *local_bindings); void binding_add_vport_to_local_bindings( struct shash *local_bindings, const struct sbrec_port_binding *parent, const struct sbrec_port_binding *vport); +bool binding_handle_ovs_interface_changes(struct binding_ctx_in *, + struct binding_ctx_out *); +bool binding_handle_port_binding_changes(struct binding_ctx_in *, + struct binding_ctx_out *); #endif /* controller/binding.h */ diff --git a/controller/ovn-controller.c b/controller/ovn-controller.c index 8cc27cebf..6841be29d 100644 --- a/controller/ovn-controller.c +++ b/controller/ovn-controller.c @@ -753,6 +753,7 @@ enum sb_engine_node { OVS_NODE(open_vswitch, "open_vswitch") \ OVS_NODE(bridge, "bridge") \ OVS_NODE(port, "port") \ + OVS_NODE(interface, "interface") \ OVS_NODE(qos, "qos") enum ovs_engine_node { @@ -974,6 +975,7 @@ struct ed_type_runtime_data { struct sset active_tunnels; struct sset egress_ifaces; + struct smap local_iface_ids; }; static void * @@ -988,6 +990,7 @@ en_runtime_data_init(struct engine_node *node OVS_UNUSED, sset_init(&data->active_tunnels); sset_init(&data->egress_ifaces); shash_init(&data->local_bindings); + smap_init(&data->local_iface_ids); return data; } @@ -1000,6 +1003,7 @@ en_runtime_data_cleanup(void *data) sset_destroy(&rt_data->local_lport_ids); sset_destroy(&rt_data->active_tunnels); sset_init(&rt_data->egress_ifaces); + smap_destroy(&rt_data->local_iface_ids); struct local_datapath *cur_node, *next_node; HMAP_FOR_EACH_SAFE (cur_node, next_node, hmap_node, &rt_data->local_datapaths) { @@ -1041,6 +1045,10 @@ init_binding_ctx(struct engine_node *node, (struct ovsrec_port_table *)EN_OVSDB_GET( engine_get_input("OVS_port", node)); + struct ovsrec_interface_table *iface_table = + (struct ovsrec_interface_table *)EN_OVSDB_GET( + engine_get_input("OVS_interface", node)); + struct ovsrec_qos_table *qos_table = (struct ovsrec_qos_table *)EN_OVSDB_GET( engine_get_input("OVS_qos", node)); @@ -1070,6 +1078,7 @@ init_binding_ctx(struct engine_node *node, b_ctx_in->sbrec_port_binding_by_datapath = sbrec_port_binding_by_datapath; b_ctx_in->sbrec_port_binding_by_name = sbrec_port_binding_by_name; b_ctx_in->port_table = port_table; + b_ctx_in->iface_table = iface_table; b_ctx_in->qos_table = qos_table; b_ctx_in->port_binding_table = pb_table; b_ctx_in->br_int = br_int; @@ -1083,6 +1092,7 @@ init_binding_ctx(struct engine_node *node, b_ctx_out->local_lport_ids = &rt_data->local_lport_ids; b_ctx_out->egress_ifaces = &rt_data->egress_ifaces; b_ctx_out->local_bindings = &rt_data->local_bindings; + b_ctx_out->local_iface_ids = &rt_data->local_iface_ids; } static void @@ -1110,10 +1120,12 @@ en_runtime_data_run(struct engine_node *node, void *data) sset_destroy(local_lport_ids); sset_destroy(active_tunnels); sset_destroy(&rt_data->egress_ifaces); + smap_destroy(&rt_data->local_iface_ids); sset_init(local_lports); sset_init(local_lport_ids); sset_init(active_tunnels); sset_init(&rt_data->egress_ifaces); + smap_init(&rt_data->local_iface_ids); } struct binding_ctx_in b_ctx_in; @@ -1139,17 +1151,47 @@ en_runtime_data_run(struct engine_node *node, void *data) } static bool -runtime_data_sb_port_binding_handler(struct engine_node *node, void *data) +runtime_data_ovs_interface_handler(struct engine_node *node, void *data) { + if (!engine_get_context()->ovnsb_idl_txn) { + return false; + } + struct ed_type_runtime_data *rt_data = data; struct binding_ctx_in b_ctx_in; struct binding_ctx_out b_ctx_out; init_binding_ctx(node, rt_data, &b_ctx_in, &b_ctx_out); - bool changed = binding_evaluate_port_binding_changes(&b_ctx_in, - &b_ctx_out); + engine_set_node_state(node, EN_UPDATED); + return binding_handle_ovs_interface_changes(&b_ctx_in, &b_ctx_out); +} + +static bool +runtime_data_noop_handler(struct engine_node *node OVS_UNUSED, + void *data OVS_UNUSED) +{ + return true; +} - return !changed; +static bool +runtime_data_sb_port_binding_handler(struct engine_node *node, void *data) +{ + if (!engine_get_context()->ovnsb_idl_txn) { + return false; + } + + struct ed_type_runtime_data *rt_data = data; + struct binding_ctx_in b_ctx_in; + struct binding_ctx_out b_ctx_out; + init_binding_ctx(node, rt_data, &b_ctx_in, &b_ctx_out); + if (!b_ctx_in.chassis_rec) { + return false; + } + + bool updated = binding_handle_port_binding_changes(&b_ctx_in, &b_ctx_out); + enum engine_node_state state = updated ? EN_UPDATED : EN_VALID; + engine_set_node_state(node, state); + return true; } /* Connection tracking zones. */ @@ -1891,7 +1933,10 @@ main(int argc, char *argv[]) engine_add_input(&en_runtime_data, &en_ovs_open_vswitch, NULL); engine_add_input(&en_runtime_data, &en_ovs_bridge, NULL); - engine_add_input(&en_runtime_data, &en_ovs_port, NULL); + engine_add_input(&en_runtime_data, &en_ovs_port, + runtime_data_noop_handler); + engine_add_input(&en_runtime_data, &en_ovs_interface, + runtime_data_ovs_interface_handler); engine_add_input(&en_runtime_data, &en_ovs_qos, NULL); engine_add_input(&en_runtime_data, &en_sb_chassis, NULL); From patchwork Mon Mar 16 11:15:57 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Numan Siddique X-Patchwork-Id: 1255405 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=openvswitch.org (client-ip=140.211.166.136; helo=silver.osuosl.org; envelope-from=ovs-dev-bounces@openvswitch.org; receiver=) Authentication-Results: ozlabs.org; dmarc=none (p=none dis=none) header.from=ovn.org Received: from silver.osuosl.org (smtp3.osuosl.org [140.211.166.136]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 48gtzG3W3rz9sPk for ; Mon, 16 Mar 2020 22:16:26 +1100 (AEDT) Received: from localhost (localhost [127.0.0.1]) by silver.osuosl.org (Postfix) with ESMTP id 835F120788; Mon, 16 Mar 2020 11:16:24 +0000 (UTC) X-Virus-Scanned: amavisd-new at osuosl.org Received: from silver.osuosl.org ([127.0.0.1]) by localhost (.osuosl.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id jlvkpnP1FCqO; Mon, 16 Mar 2020 11:16:21 +0000 (UTC) Received: from lists.linuxfoundation.org (lf-lists.osuosl.org [140.211.9.56]) by silver.osuosl.org (Postfix) with ESMTP id 7F7F221574; Mon, 16 Mar 2020 11:16:16 +0000 (UTC) Received: from lf-lists.osuosl.org (localhost [127.0.0.1]) by lists.linuxfoundation.org (Postfix) with ESMTP id 60726C1D7C; Mon, 16 Mar 2020 11:16:16 +0000 (UTC) X-Original-To: dev@openvswitch.org Delivered-To: ovs-dev@lists.linuxfoundation.org Received: from whitealder.osuosl.org (smtp1.osuosl.org [140.211.166.138]) by lists.linuxfoundation.org (Postfix) with ESMTP id 87678C013E for ; Mon, 16 Mar 2020 11:16:14 +0000 (UTC) Received: from localhost (localhost [127.0.0.1]) by whitealder.osuosl.org (Postfix) with ESMTP id 72BAB88680 for ; Mon, 16 Mar 2020 11:16:14 +0000 (UTC) X-Virus-Scanned: amavisd-new at osuosl.org Received: from whitealder.osuosl.org ([127.0.0.1]) by localhost (.osuosl.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id KE1MQ8WW01Fi for ; Mon, 16 Mar 2020 11:16:12 +0000 (UTC) X-Greylist: domain auto-whitelisted by SQLgrey-1.7.6 Received: from relay1-d.mail.gandi.net (relay1-d.mail.gandi.net [217.70.183.193]) by whitealder.osuosl.org (Postfix) with ESMTPS id 74C5788690 for ; Mon, 16 Mar 2020 11:16:12 +0000 (UTC) X-Originating-IP: 27.7.184.93 Received: from nummac.local (unknown [27.7.184.93]) (Authenticated sender: numans@ovn.org) by relay1-d.mail.gandi.net (Postfix) with ESMTPSA id 5E5E2240014; Mon, 16 Mar 2020 11:16:08 +0000 (UTC) From: numans@ovn.org To: dev@openvswitch.org Date: Mon, 16 Mar 2020 16:45:57 +0530 Message-Id: <20200316111557.1687679-1-numans@ovn.org> X-Mailer: git-send-email 2.24.1 In-Reply-To: <20200316111034.1686721-1-numans@ovn.org> References: <20200316111034.1686721-1-numans@ovn.org> MIME-Version: 1.0 Subject: [ovs-dev] [PATCH ovn 4/6] ovn-controller: I-P for datapath binding X-BeenThere: ovs-dev@openvswitch.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: ovs-dev-bounces@openvswitch.org Sender: "dev" From: Numan Siddique This patch adds partial support of incremental processing of datapath binding. If a datapath is deleted, then a full recompute is triggered. Signed-off-by: Numan Siddique --- controller/ovn-controller.c | 26 +++++++++++++++++++++++++- tests/ovn-performance.at | 8 +++++--- 2 files changed, 30 insertions(+), 4 deletions(-) diff --git a/controller/ovn-controller.c b/controller/ovn-controller.c index 6841be29d..07823f020 100644 --- a/controller/ovn-controller.c +++ b/controller/ovn-controller.c @@ -1194,6 +1194,29 @@ runtime_data_sb_port_binding_handler(struct engine_node *node, void *data) return true; } +static bool +runtime_data_sb_datapath_binding_handler(struct engine_node *node OVS_UNUSED, + void *data OVS_UNUSED) +{ + struct sbrec_datapath_binding_table *dp_table = + (struct sbrec_datapath_binding_table *)EN_OVSDB_GET( + engine_get_input("SB_datapath_binding", node)); + const struct sbrec_datapath_binding *dp; + struct ed_type_runtime_data *rt_data = data; + + SBREC_DATAPATH_BINDING_TABLE_FOR_EACH_TRACKED (dp, dp_table) { + if (sbrec_datapath_binding_is_deleted(dp)) { + if (get_local_datapath(&rt_data->local_datapaths, + dp->tunnel_key)) { + return false; + } + } + } + + engine_set_node_state(node, EN_VALID); + return true; +} + /* Connection tracking zones. */ struct ed_type_ct_zones { unsigned long bitmap[BITMAP_N_LONGS(MAX_CT_ZONES)]; @@ -1940,7 +1963,8 @@ main(int argc, char *argv[]) engine_add_input(&en_runtime_data, &en_ovs_qos, NULL); engine_add_input(&en_runtime_data, &en_sb_chassis, NULL); - engine_add_input(&en_runtime_data, &en_sb_datapath_binding, NULL); + engine_add_input(&en_runtime_data, &en_sb_datapath_binding, + runtime_data_sb_datapath_binding_handler); engine_add_input(&en_runtime_data, &en_sb_port_binding, runtime_data_sb_port_binding_handler); diff --git a/tests/ovn-performance.at b/tests/ovn-performance.at index a8a15f8fe..5c21f6dd7 100644 --- a/tests/ovn-performance.at +++ b/tests/ovn-performance.at @@ -239,8 +239,10 @@ for i in 1 2; do ovn_attach n1 br-phys 192.168.0.$i done +ovn-nbctl --wait=hv sync + # Add router lr1 -OVN_CONTROLLER_EXPECT_HIT( +OVN_CONTROLLER_EXPECT_NO_HIT( [hv1 hv2], [lflow_run], [ovn-nbctl --wait=hv lr-add lr1] ) @@ -251,7 +253,7 @@ for i in 1 2; do lrp=lr1-$ls # Add switch $ls - OVN_CONTROLLER_EXPECT_HIT( + OVN_CONTROLLER_EXPECT_NO_HIT( [hv1 hv2], [lflow_run], [ovn-nbctl --wait=hv ls-add $ls] ) @@ -414,7 +416,7 @@ for i in 1 2; do done # Delete router lr1 -OVN_CONTROLLER_EXPECT_HIT( +OVN_CONTROLLER_EXPECT_NO_HIT( [hv1 hv2], [lflow_run], [ovn-nbctl --wait=hv lr-del lr1] ) From patchwork Mon Mar 16 11:16:09 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Numan Siddique X-Patchwork-Id: 1255406 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=openvswitch.org (client-ip=140.211.166.138; helo=whitealder.osuosl.org; envelope-from=ovs-dev-bounces@openvswitch.org; receiver=) Authentication-Results: ozlabs.org; dmarc=none (p=none dis=none) header.from=ovn.org Received: from whitealder.osuosl.org (smtp1.osuosl.org [140.211.166.138]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 48gtzX1bfPz9sPk for ; Mon, 16 Mar 2020 22:16:40 +1100 (AEDT) Received: from localhost (localhost [127.0.0.1]) by whitealder.osuosl.org (Postfix) with ESMTP id A1F95886C2; Mon, 16 Mar 2020 11:16:38 +0000 (UTC) X-Virus-Scanned: amavisd-new at osuosl.org Received: from whitealder.osuosl.org ([127.0.0.1]) by localhost (.osuosl.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id egKT9OqPZmlt; Mon, 16 Mar 2020 11:16:34 +0000 (UTC) Received: from lists.linuxfoundation.org (lf-lists.osuosl.org [140.211.9.56]) by whitealder.osuosl.org (Postfix) with ESMTP id 7DB91886BF; Mon, 16 Mar 2020 11:16:32 +0000 (UTC) Received: from lf-lists.osuosl.org (localhost [127.0.0.1]) by lists.linuxfoundation.org (Postfix) with ESMTP id 736D1C1D7E; Mon, 16 Mar 2020 11:16:32 +0000 (UTC) X-Original-To: dev@openvswitch.org Delivered-To: ovs-dev@lists.linuxfoundation.org Received: from silver.osuosl.org (smtp3.osuosl.org [140.211.166.136]) by lists.linuxfoundation.org (Postfix) with ESMTP id E7766C1D85 for ; Mon, 16 Mar 2020 11:16:30 +0000 (UTC) Received: from localhost (localhost [127.0.0.1]) by silver.osuosl.org (Postfix) with ESMTP id D01A920478 for ; Mon, 16 Mar 2020 11:16:30 +0000 (UTC) X-Virus-Scanned: amavisd-new at osuosl.org Received: from silver.osuosl.org ([127.0.0.1]) by localhost (.osuosl.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id 1Br6bA2T2-RX for ; Mon, 16 Mar 2020 11:16:25 +0000 (UTC) X-Greylist: domain auto-whitelisted by SQLgrey-1.7.6 Received: from relay6-d.mail.gandi.net (relay6-d.mail.gandi.net [217.70.183.198]) by silver.osuosl.org (Postfix) with ESMTPS id 89FAC21538 for ; Mon, 16 Mar 2020 11:16:20 +0000 (UTC) X-Originating-IP: 27.7.184.93 Received: from nummac.local (unknown [27.7.184.93]) (Authenticated sender: numans@ovn.org) by relay6-d.mail.gandi.net (Postfix) with ESMTPSA id 5557DC0008; Mon, 16 Mar 2020 11:16:17 +0000 (UTC) From: numans@ovn.org To: dev@openvswitch.org Date: Mon, 16 Mar 2020 16:46:09 +0530 Message-Id: <20200316111609.1687734-1-numans@ovn.org> X-Mailer: git-send-email 2.24.1 In-Reply-To: <20200316111034.1686721-1-numans@ovn.org> References: <20200316111034.1686721-1-numans@ovn.org> MIME-Version: 1.0 Subject: [ovs-dev] [PATCH ovn 5/6] ofctrl_check_and_add_flow: Replace the actions of an existing flow if actions have changed. X-BeenThere: ovs-dev@openvswitch.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: ovs-dev-bounces@openvswitch.org Sender: "dev" From: Numan Siddique If ofctrl_check_and_add_flow(F') is called where flow F' has match-actions (M, A2) and if there already exists a flow F with match-actions (M, A1) in the desired flow table, then this function doesn't update the existing flow F with new actions actions A2. This patch is required for the upcoming patch in this series which adds incremental processing for OVS interface in the flow output stage. Since we will be not be clearing the flow output data if these changes are handled incrementally, some of the existing flows will be updated with new actions. One such example would be flows in physical table OFTABLE_LOCAL_OUTPUT (table 33). And this patch is required to update such flows. Otherwise we will have incomplete actions installed. Signed-off-by: Numan Siddique --- controller/ofctrl.c | 23 ++++++++++++++++------- 1 file changed, 16 insertions(+), 7 deletions(-) diff --git a/controller/ofctrl.c b/controller/ofctrl.c index 36e39ba06..2df7400be 100644 --- a/controller/ofctrl.c +++ b/controller/ofctrl.c @@ -667,14 +667,23 @@ ofctrl_check_and_add_flow(struct ovn_desired_flow_table *flow_table, ovn_flow_log(f, "ofctrl_add_flow"); - if (ovn_flow_lookup(&flow_table->match_flow_table, f, true)) { - if (log_duplicate_flow) { - static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 5); - if (!VLOG_DROP_DBG(&rl)) { - char *s = ovn_flow_to_string(f); - VLOG_DBG("dropping duplicate flow: %s", s); - free(s); + struct ovn_flow *existing_f = + ovn_flow_lookup(&flow_table->match_flow_table, f, true); + if (existing_f) { + if (ofpacts_equal(f->ofpacts, f->ofpacts_len, + existing_f->ofpacts, existing_f->ofpacts_len)) { + if (log_duplicate_flow) { + static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 5); + if (!VLOG_DROP_DBG(&rl)) { + char *s = ovn_flow_to_string(f); + VLOG_DBG("dropping duplicate flow: %s", s); + free(s); + } } + } else { + free(existing_f->ofpacts); + existing_f->ofpacts = xmemdup(f->ofpacts, f->ofpacts_len); + existing_f->ofpacts_len = f->ofpacts_len; } ovn_flow_destroy(f); return; From patchwork Mon Mar 16 11:16:19 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Numan Siddique X-Patchwork-Id: 1255407 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=openvswitch.org (client-ip=140.211.166.136; helo=silver.osuosl.org; envelope-from=ovs-dev-bounces@openvswitch.org; receiver=) Authentication-Results: ozlabs.org; dmarc=none (p=none dis=none) header.from=ovn.org Received: from silver.osuosl.org (smtp3.osuosl.org [140.211.166.136]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 48gtzx0BsSz9sPJ for ; Mon, 16 Mar 2020 22:17:01 +1100 (AEDT) Received: from localhost (localhost [127.0.0.1]) by silver.osuosl.org (Postfix) with ESMTP id 79B6920532; Mon, 16 Mar 2020 11:16:59 +0000 (UTC) X-Virus-Scanned: amavisd-new at osuosl.org Received: from silver.osuosl.org ([127.0.0.1]) by localhost (.osuosl.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id v10Wux3jBG2m; Mon, 16 Mar 2020 11:16:52 +0000 (UTC) Received: from lists.linuxfoundation.org (lf-lists.osuosl.org [140.211.9.56]) by silver.osuosl.org (Postfix) with ESMTP id A245620478; Mon, 16 Mar 2020 11:16:52 +0000 (UTC) Received: from lf-lists.osuosl.org (localhost [127.0.0.1]) by lists.linuxfoundation.org (Postfix) with ESMTP id 8A3F3C1D7C; Mon, 16 Mar 2020 11:16:52 +0000 (UTC) X-Original-To: dev@openvswitch.org Delivered-To: ovs-dev@lists.linuxfoundation.org Received: from whitealder.osuosl.org (smtp1.osuosl.org [140.211.166.138]) by lists.linuxfoundation.org (Postfix) with ESMTP id 5F61EC013E for ; Mon, 16 Mar 2020 11:16:51 +0000 (UTC) Received: from localhost (localhost [127.0.0.1]) by whitealder.osuosl.org (Postfix) with ESMTP id 5C2F988610 for ; Mon, 16 Mar 2020 11:16:51 +0000 (UTC) X-Virus-Scanned: amavisd-new at osuosl.org Received: from whitealder.osuosl.org ([127.0.0.1]) by localhost (.osuosl.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id KxZHD1hpEYVg for ; Mon, 16 Mar 2020 11:16:49 +0000 (UTC) X-Greylist: domain auto-whitelisted by SQLgrey-1.7.6 Received: from relay11.mail.gandi.net (relay11.mail.gandi.net [217.70.178.231]) by whitealder.osuosl.org (Postfix) with ESMTPS id 42F8688653 for ; Mon, 16 Mar 2020 11:16:49 +0000 (UTC) Received: from nummac.local (unknown [27.7.184.93]) (Authenticated sender: numans@ovn.org) by relay11.mail.gandi.net (Postfix) with ESMTPSA id 70DF610001B; Mon, 16 Mar 2020 11:16:27 +0000 (UTC) From: numans@ovn.org To: dev@openvswitch.org Date: Mon, 16 Mar 2020 16:46:19 +0530 Message-Id: <20200316111619.1687790-1-numans@ovn.org> X-Mailer: git-send-email 2.24.1 In-Reply-To: <20200316111034.1686721-1-numans@ovn.org> References: <20200316111034.1686721-1-numans@ovn.org> MIME-Version: 1.0 Subject: [ovs-dev] [PATCH ovn 6/6] ovn-controller: I-P for ct zone and OVS interface changes in flow output stage. X-BeenThere: ovs-dev@openvswitch.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: ovs-dev-bounces@openvswitch.org Sender: "dev" From: Numan Siddique This patch handles ct zone changes and OVS interface changes incrementally in the flow output stage. Any changes to ct zone can be handled by running physical_run() instead of running flow_output_run(). And any changes to OVS interfaces can be either handled incrementally (for OVS interfaces representing VIFs) or just running physical_run() (for tunnel and other types of interfaces). To better handle this, a new engine node 'physical_flow_changes' is added which handles changes to ct zone and OVS interfaces. After this patch, We don't trigger full recompute in flow output stage for runtime data changes. There's noop handler for this. With this we avoid calls to lflow_run() a lot. For the below commands the lflow_run counter without this patch is 16 and with this patch is 3. ----------------------- ovn-nbctl ls-add sw0 ovn-nbctl lsp-add sw0 sw0-port1 ovn-nbctl lsp-set-addresses sw0-port1 "50:54:00:00:00:01 192.168.0.2" ovn-nbctl ls-add sw1 ovn-nbctl lsp-add sw1 sw1-port1 ovn-nbctl lsp-set-addresses sw1-port1 "50:54:00:00:00:03 11.0.0.2" ovn-nbctl lr-add lr0 ovn-nbctl lrp-add lr0 lrp0 00:00:00:00:ff:01 192.168.0.1/24 ovn-nbctl lsp-add sw0 lrp0-attachment ovn-nbctl lsp-set-type lrp0-attachment router ovn-nbctl lsp-set-addresses lrp0-attachment 00:00:00:00:ff:01 ovn-nbctl lsp-set-options lrp0-attachment router-port=lrp0 ovn-nbctl lrp-add lr0 lrp1 00:00:00:00:ff:02 11.0.0.1/24 ovn-nbctl lsp-add sw1 lrp1-attachment ovn-nbctl lsp-set-type lrp1-attachment router ovn-nbctl lsp-set-addresses lrp1-attachment 00:00:00:00:ff:02 ovn-nbctl lsp-set-options lrp1-attachment router-port=lrp1 ovs-vsctl add-port br-int p1 -- \ set Interface p1 external_ids:iface-id=sw0-port1 ovs-vsctl add-port br-int p2 -- \ set Interface p2 external_ids:iface-id=sw1-port1 ovn-appctl -t ovn-controller coverage/read-counter lflow_run ------------------- Signed-off-by: Numan Siddique --- controller/binding.c | 22 --------- controller/lflow.c | 13 +++++ controller/lflow.h | 2 + controller/ovn-controller.c | 96 ++++++++++++++++++++++++++++++++++++- controller/ovn-controller.h | 22 +++++++++ controller/physical.c | 48 +++++++++++++++++++ controller/physical.h | 5 +- tests/ovn-performance.at | 14 +++--- 8 files changed, 190 insertions(+), 32 deletions(-) diff --git a/controller/binding.c b/controller/binding.c index ce4467f31..9dc1c3f38 100644 --- a/controller/binding.c +++ b/controller/binding.c @@ -501,22 +501,6 @@ remove_local_lport_ids(const struct sbrec_port_binding *binding_rec, sset_find_and_delete(local_lport_ids, buf); } -enum local_binding_type { - BT_VIF, - BT_CHILD, - BT_VIRTUAL -}; - -struct local_binding { - struct ovs_list node; /* In parent if any. */ - char *name; - enum local_binding_type type; - const struct ovsrec_interface *iface; - const struct sbrec_port_binding *pb; - - struct ovs_list children; -}; - static struct local_binding * local_binding_create(const char *name, const struct ovsrec_interface *iface, const struct sbrec_port_binding *pb, @@ -537,12 +521,6 @@ local_binding_add(struct shash *local_bindings, struct local_binding *lbinding) shash_add(local_bindings, lbinding->name, lbinding); } -static struct local_binding * -local_binding_find(struct shash *local_bindings, const char *name) -{ - return shash_find_data(local_bindings, name); -} - static void local_binding_destroy(struct local_binding *lbinding) { diff --git a/controller/lflow.c b/controller/lflow.c index 01214a3a6..512258cd3 100644 --- a/controller/lflow.c +++ b/controller/lflow.c @@ -847,3 +847,16 @@ lflow_destroy(void) expr_symtab_destroy(&symtab); shash_destroy(&symtab); } + +bool +lflow_evaluate_pb_changes(const struct sbrec_port_binding_table *pb_table) +{ + const struct sbrec_port_binding *binding; + SBREC_PORT_BINDING_TABLE_FOR_EACH_TRACKED (binding, pb_table) { + if (!strcmp(binding->type, "remote")) { + return false; + } + } + + return true; +} diff --git a/controller/lflow.h b/controller/lflow.h index f02f709d7..fa54d4de2 100644 --- a/controller/lflow.h +++ b/controller/lflow.h @@ -48,6 +48,7 @@ struct sbrec_dhcp_options_table; struct sbrec_dhcpv6_options_table; struct sbrec_logical_flow_table; struct sbrec_mac_binding_table; +struct sbrec_port_binding_table; struct simap; struct sset; struct uuid; @@ -153,6 +154,7 @@ void lflow_handle_changed_neighbors( const struct hmap *local_datapaths, struct ovn_desired_flow_table *); +bool lflow_evaluate_pb_changes(const struct sbrec_port_binding_table *); void lflow_destroy(void); void lflow_expr_destroy(struct hmap *lflow_expr_cache); diff --git a/controller/ovn-controller.c b/controller/ovn-controller.c index 07823f020..448e9908e 100644 --- a/controller/ovn-controller.c +++ b/controller/ovn-controller.c @@ -1360,6 +1360,10 @@ static void init_physical_ctx(struct engine_node *node, ovs_assert(br_int && chassis); + struct ovsrec_interface_table *iface_table = + (struct ovsrec_interface_table *)EN_OVSDB_GET( + engine_get_input("OVS_interface", node)); + struct ed_type_ct_zones *ct_zones_data = engine_get_input_data("ct_zones", node); struct simap *ct_zones = &ct_zones_data->current; @@ -1369,12 +1373,14 @@ static void init_physical_ctx(struct engine_node *node, p_ctx->mc_group_table = multicast_group_table; p_ctx->br_int = br_int; p_ctx->chassis_table = chassis_table; + p_ctx->iface_table = iface_table; p_ctx->chassis = chassis; p_ctx->active_tunnels = &rt_data->active_tunnels; p_ctx->local_datapaths = &rt_data->local_datapaths; p_ctx->local_lports = &rt_data->local_lports; p_ctx->ct_zones = ct_zones; p_ctx->mff_ovn_geneve = ed_mff_ovn_geneve->mff_ovn_geneve; + p_ctx->local_bindings = &rt_data->local_bindings; } static void init_lflow_ctx(struct engine_node *node, @@ -1637,6 +1643,8 @@ flow_output_sb_port_binding_handler(struct engine_node *node, void *data) * always logical router port, and any change to logical router port * would just trigger recompute. * + * - When a port binding of type 'remote' is updated by ovn-ic. + * * Although there is no correctness issue so far (except the unusual ACL * use case, which doesn't seem to be a real problem), it might be better * to handle this more gracefully, without the need to consider these @@ -1647,6 +1655,14 @@ flow_output_sb_port_binding_handler(struct engine_node *node, void *data) struct physical_ctx p_ctx; init_physical_ctx(node, rt_data, &p_ctx); + if (!lflow_evaluate_pb_changes(p_ctx.port_binding_table)) { + /* If this returns false, it means there is an impact on + * the logical flow processing because of some changes in + * port bindings. Return false so that recompute is triggered + * for this stage. */ + return false; + } + physical_handle_port_binding_changes(&p_ctx, flow_table); engine_set_node_state(node, EN_UPDATED); @@ -1782,6 +1798,70 @@ flow_output_port_groups_handler(struct engine_node *node, void *data) return _flow_output_resource_ref_handler(node, data, REF_TYPE_PORTGROUP); } +struct ed_type_physical_flow_changes { + bool ct_zones_changed; +}; + +static bool +flow_output_physical_flow_changes_handler(struct engine_node *node, void *data) +{ + struct ed_type_physical_flow_changes *pfc = + engine_get_input_data("physical_flow_changes", node); + struct ed_type_runtime_data *rt_data = + engine_get_input_data("runtime_data", node); + + struct ed_type_flow_output *fo = data; + struct physical_ctx p_ctx; + init_physical_ctx(node, rt_data, &p_ctx); + + engine_set_node_state(node, EN_UPDATED); + if (pfc->ct_zones_changed) { + pfc->ct_zones_changed = false; + physical_run(&p_ctx, &fo->flow_table); + return true; + } + + return physical_handle_ovs_iface_changes(&p_ctx, &fo->flow_table); +} + +static void * +en_physical_flow_changes_init(struct engine_node *node OVS_UNUSED, + struct engine_arg *arg OVS_UNUSED) +{ + struct ed_type_physical_flow_changes *data = xzalloc(sizeof *data); + return data; +} + +static void +en_physical_flow_changes_cleanup(void *data OVS_UNUSED) +{ + +} + +static void +en_physical_flow_changes_run(struct engine_node *node, void *data OVS_UNUSED) +{ + engine_set_node_state(node, EN_UPDATED); +} + +static bool +physical_flow_changes_ct_zones_handler(struct engine_node *node OVS_UNUSED, + void *data OVS_UNUSED) +{ + struct ed_type_physical_flow_changes *pfc = data; + pfc->ct_zones_changed = true; + engine_set_node_state(node, EN_UPDATED); + return false; +} + +static bool +flow_output_noop_handler(struct engine_node *node OVS_UNUSED, + void *data OVS_UNUSED) +{ + return true; +} + + struct ovn_controller_exit_args { bool *exiting; bool *restart; @@ -1904,6 +1984,7 @@ main(int argc, char *argv[]) ENGINE_NODE(runtime_data, "runtime_data"); ENGINE_NODE(mff_ovn_geneve, "mff_ovn_geneve"); ENGINE_NODE(ofctrl_is_connected, "ofctrl_is_connected"); + ENGINE_NODE(physical_flow_changes, "physical_flow_changes"); ENGINE_NODE(flow_output, "flow_output"); ENGINE_NODE(addr_sets, "addr_sets"); ENGINE_NODE(port_groups, "port_groups"); @@ -1923,16 +2004,27 @@ main(int argc, char *argv[]) engine_add_input(&en_port_groups, &en_sb_port_group, port_groups_sb_port_group_handler); + engine_add_input(&en_physical_flow_changes, &en_ct_zones, + physical_flow_changes_ct_zones_handler); + engine_add_input(&en_physical_flow_changes, &en_ovs_interface, + NULL); + engine_add_input(&en_flow_output, &en_addr_sets, flow_output_addr_sets_handler); engine_add_input(&en_flow_output, &en_port_groups, flow_output_port_groups_handler); - engine_add_input(&en_flow_output, &en_runtime_data, NULL); - engine_add_input(&en_flow_output, &en_ct_zones, NULL); + engine_add_input(&en_flow_output, &en_runtime_data, + flow_output_noop_handler); engine_add_input(&en_flow_output, &en_mff_ovn_geneve, NULL); + engine_add_input(&en_flow_output, &en_physical_flow_changes, + flow_output_physical_flow_changes_handler); engine_add_input(&en_flow_output, &en_ovs_open_vswitch, NULL); engine_add_input(&en_flow_output, &en_ovs_bridge, NULL); + engine_add_input(&en_flow_output, &en_ovs_interface, + flow_output_noop_handler); + engine_add_input(&en_flow_output, &en_ct_zones, + flow_output_noop_handler); engine_add_input(&en_flow_output, &en_sb_chassis, NULL); engine_add_input(&en_flow_output, &en_sb_encap, NULL); diff --git a/controller/ovn-controller.h b/controller/ovn-controller.h index 5d9466880..40c358cef 100644 --- a/controller/ovn-controller.h +++ b/controller/ovn-controller.h @@ -72,6 +72,28 @@ struct local_datapath { struct local_datapath *get_local_datapath(const struct hmap *, uint32_t tunnel_key); +enum local_binding_type { + BT_VIF, + BT_CHILD, + BT_VIRTUAL +}; + +struct local_binding { + struct ovs_list node; /* In parent if any. */ + char *name; + enum local_binding_type type; + const struct ovsrec_interface *iface; + const struct sbrec_port_binding *pb; + + struct ovs_list children; +}; + +static inline struct local_binding * +local_binding_find(struct shash *local_bindings, const char *name) +{ + return shash_find_data(local_bindings, name); +} + const struct ovsrec_bridge *get_bridge(const struct ovsrec_bridge_table *, const char *br_name); diff --git a/controller/physical.c b/controller/physical.c index 144aeb7bd..09a5d9b39 100644 --- a/controller/physical.c +++ b/controller/physical.c @@ -1780,6 +1780,54 @@ physical_run(struct physical_ctx *p_ctx, simap_destroy(&new_tunnel_to_ofport); } +bool +physical_handle_ovs_iface_changes(struct physical_ctx *p_ctx, + struct ovn_desired_flow_table *flow_table) +{ + const struct ovsrec_interface *iface_rec; + OVSREC_INTERFACE_TABLE_FOR_EACH_TRACKED (iface_rec, p_ctx->iface_table) { + if (!strcmp(iface_rec->type, "geneve") || + !strcmp(iface_rec->type, "patch")) { + return false; + } + } + + struct ofpbuf ofpacts; + ofpbuf_init(&ofpacts, 0); + + OVSREC_INTERFACE_TABLE_FOR_EACH_TRACKED (iface_rec, p_ctx->iface_table) { + const char *iface_id = smap_get(&iface_rec->external_ids, "iface-id"); + if (!iface_id) { + continue; + } + + const struct local_binding *lb = + local_binding_find(p_ctx->local_bindings, iface_id); + + if (!lb || !lb->pb) { + continue; + } + + if (ovsrec_interface_is_deleted(iface_rec)) { + ofctrl_remove_flows(flow_table, &lb->pb->header_.uuid); + } else { + if (!ovsrec_interface_is_new(iface_rec)) { + ofctrl_remove_flows(flow_table, &lb->pb->header_.uuid); + } + + consider_port_binding(p_ctx->sbrec_port_binding_by_name, + p_ctx->mff_ovn_geneve, p_ctx->ct_zones, + p_ctx->active_tunnels, + p_ctx->local_datapaths, + lb->pb, p_ctx->chassis, + flow_table, &ofpacts); + } + } + + ofpbuf_uninit(&ofpacts); + return true; +} + bool get_tunnel_ofport(const char *chassis_name, char *encap_ip, ofp_port_t *ofport) { diff --git a/controller/physical.h b/controller/physical.h index dadf84ea1..9ca34436a 100644 --- a/controller/physical.h +++ b/controller/physical.h @@ -49,11 +49,13 @@ struct physical_ctx { const struct ovsrec_bridge *br_int; const struct sbrec_chassis_table *chassis_table; const struct sbrec_chassis *chassis; + const struct ovsrec_interface_table *iface_table; const struct sset *active_tunnels; struct hmap *local_datapaths; struct sset *local_lports; const struct simap *ct_zones; enum mf_field_id mff_ovn_geneve; + struct shash *local_bindings; }; void physical_register_ovs_idl(struct ovsdb_idl *); @@ -63,7 +65,8 @@ void physical_handle_port_binding_changes(struct physical_ctx *, struct ovn_desired_flow_table *); void physical_handle_mc_group_changes(struct physical_ctx *, struct ovn_desired_flow_table *); - +bool physical_handle_ovs_iface_changes(struct physical_ctx *, + struct ovn_desired_flow_table *); bool get_tunnel_ofport(const char *chassis_name, char *encap_ip, ofp_port_t *ofport); #endif /* controller/physical.h */ diff --git a/tests/ovn-performance.at b/tests/ovn-performance.at index 5c21f6dd7..555434484 100644 --- a/tests/ovn-performance.at +++ b/tests/ovn-performance.at @@ -263,7 +263,7 @@ for i in 1 2; do ) # Add router port to $ls - OVN_CONTROLLER_EXPECT_HIT( + OVN_CONTROLLER_EXPECT_NO_HIT( [hv1 hv2], [lflow_run], [ovn-nbctl --wait=hv lrp-add lr1 $lrp 02:00:00:00:0$i:01 10.0.$i.1/24] ) @@ -271,15 +271,15 @@ for i in 1 2; do [hv1 hv2], [lflow_run], [ovn-nbctl --wait=hv lsp-add $ls $lsp] ) - OVN_CONTROLLER_EXPECT_HIT( + OVN_CONTROLLER_EXPECT_NO_HIT( [hv1 hv2], [lflow_run], [ovn-nbctl --wait=hv lsp-set-type $lsp router] ) - OVN_CONTROLLER_EXPECT_HIT( + OVN_CONTROLLER_EXPECT_NO_HIT( [hv1 hv2], [lflow_run], [ovn-nbctl --wait=hv lsp-set-options $lsp router-port=$lrp] ) - OVN_CONTROLLER_EXPECT_HIT( + OVN_CONTROLLER_EXPECT_NO_HIT( [hv1 hv2], [lflow_run], [ovn-nbctl --wait=hv lsp-set-addresses $lsp router] ) @@ -393,8 +393,8 @@ for i in 1 2; do lp=lp$i # Delete port $lp - OVN_CONTROLLER_EXPECT_HIT_COND( - [hv$i hv$j], [lflow_run], [>0 =0], + OVN_CONTROLLER_EXPECT_NO_HIT( + [hv$i hv$j], [lflow_run], [ovn-nbctl --wait=hv lsp-del $lp] ) done @@ -409,7 +409,7 @@ for i in 1 2; do ls=ls$i # Delete switch $ls - OVN_CONTROLLER_EXPECT_HIT( + OVN_CONTROLLER_EXPECT_NO_HIT( [hv1 hv2], [lflow_run], [ovn-nbctl --wait=hv ls-del $ls] )