From patchwork Thu Apr 30 16:59:23 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Numan Siddique X-Patchwork-Id: 1280669 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 49ChSg1Wvxz9sSh for ; Fri, 1 May 2020 02:59:46 +1000 (AEST) Received: from localhost (localhost [127.0.0.1]) by hemlock.osuosl.org (Postfix) with ESMTP id 43707887C5; Thu, 30 Apr 2020 16:59:44 +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 fLSo0SIck5ET; Thu, 30 Apr 2020 16:59:42 +0000 (UTC) Received: from lists.linuxfoundation.org (lf-lists.osuosl.org [140.211.9.56]) by hemlock.osuosl.org (Postfix) with ESMTP id 52BF588721; Thu, 30 Apr 2020 16:59:42 +0000 (UTC) Received: from lf-lists.osuosl.org (localhost [127.0.0.1]) by lists.linuxfoundation.org (Postfix) with ESMTP id 3D7B9C0864; Thu, 30 Apr 2020 16:59:42 +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 8BEBDC016F for ; Thu, 30 Apr 2020 16:59:40 +0000 (UTC) Received: from localhost (localhost [127.0.0.1]) by silver.osuosl.org (Postfix) with ESMTP id 731A0204CD for ; Thu, 30 Apr 2020 16:59:40 +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 1wgoNa7R5Jas for ; Thu, 30 Apr 2020 16:59:38 +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 A477220387 for ; Thu, 30 Apr 2020 16:59:37 +0000 (UTC) X-Originating-IP: 125.99.245.47 Received: from nummac.local (unknown [125.99.245.47]) (Authenticated sender: numans@ovn.org) by relay6-d.mail.gandi.net (Postfix) with ESMTPSA id 00CA0C0008; Thu, 30 Apr 2020 16:59:32 +0000 (UTC) From: numans@ovn.org To: dev@openvswitch.org Date: Thu, 30 Apr 2020 22:29:23 +0530 Message-Id: <20200430165923.1975422-1-numans@ovn.org> X-Mailer: git-send-email 2.25.4 In-Reply-To: <20200430165741.1975097-1-numans@ovn.org> References: <20200430165741.1975097-1-numans@ovn.org> MIME-Version: 1.0 Subject: [ovs-dev] [PATCH v4 01/10] 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 Acked-by: Han Zhou Acked-by: Dumitru Ceara --- controller/binding.c | 263 +++++++++++++++++------------------- controller/binding.h | 39 ++++-- controller/ovn-controller.c | 120 +++++++++------- 3 files changed, 217 insertions(+), 205 deletions(-) diff --git a/controller/binding.c b/controller/binding.c index 20a89d07d..007a94866 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); - /* Ports of type "virtual" should never be explicitly bound to an OVS * port in the integration bridge. If that's the case, ignore the binding * and log a warning. @@ -490,78 +487,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); } } @@ -570,65 +563,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; } @@ -722,23 +713,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; } @@ -749,9 +726,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 @@ -768,16 +745,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); } @@ -785,26 +760,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); @@ -850,13 +828,18 @@ 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, "") && strcmp(binding_rec->type, + "remote"))) { 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 6ff897325..24c89e617 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 Thu Apr 30 16:59:34 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Numan Siddique X-Patchwork-Id: 1280670 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 49ChSr4kDtz9sSh for ; Fri, 1 May 2020 02:59:56 +1000 (AEST) Received: from localhost (localhost [127.0.0.1]) by whitealder.osuosl.org (Postfix) with ESMTP id 1B30487EDB; Thu, 30 Apr 2020 16:59:55 +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 MCzFFmYQbvlE; Thu, 30 Apr 2020 16:59:48 +0000 (UTC) Received: from lists.linuxfoundation.org (lf-lists.osuosl.org [140.211.9.56]) by whitealder.osuosl.org (Postfix) with ESMTP id 817D787F79; Thu, 30 Apr 2020 16:59:48 +0000 (UTC) Received: from lf-lists.osuosl.org (localhost [127.0.0.1]) by lists.linuxfoundation.org (Postfix) with ESMTP id 65122C016F; Thu, 30 Apr 2020 16:59:48 +0000 (UTC) X-Original-To: dev@openvswitch.org Delivered-To: ovs-dev@lists.linuxfoundation.org Received: from fraxinus.osuosl.org (smtp4.osuosl.org [140.211.166.137]) by lists.linuxfoundation.org (Postfix) with ESMTP id 133F1C016F for ; Thu, 30 Apr 2020 16:59:47 +0000 (UTC) Received: from localhost (localhost [127.0.0.1]) by fraxinus.osuosl.org (Postfix) with ESMTP id EB6E386D8D for ; Thu, 30 Apr 2020 16:59:46 +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 oCzvNbWyg6BR for ; Thu, 30 Apr 2020 16:59:44 +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 fraxinus.osuosl.org (Postfix) with ESMTPS id 10D7986DA3 for ; Thu, 30 Apr 2020 16:59:43 +0000 (UTC) X-Originating-IP: 125.99.245.47 Received: from nummac.local (unknown [125.99.245.47]) (Authenticated sender: numans@ovn.org) by relay1-d.mail.gandi.net (Postfix) with ESMTPSA id CD1BD240008; Thu, 30 Apr 2020 16:59:39 +0000 (UTC) From: numans@ovn.org To: dev@openvswitch.org Date: Thu, 30 Apr 2020 22:29:34 +0530 Message-Id: <20200430165934.1975474-1-numans@ovn.org> X-Mailer: git-send-email 2.25.4 In-Reply-To: <20200430165741.1975097-1-numans@ovn.org> References: <20200430165741.1975097-1-numans@ovn.org> MIME-Version: 1.0 Subject: [ovs-dev] [PATCH v4 02/10] 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 | 722 ++++++++++++++++++++++-------------- controller/binding.h | 16 +- controller/ovn-controller.c | 46 +-- controller/pinctrl.c | 1 + controller/pinctrl.h | 1 + 5 files changed, 467 insertions(+), 319 deletions(-) diff --git a/controller/binding.c b/controller/binding.c index 007a94866..66f4f65e3 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, @@ -441,25 +400,11 @@ 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 ovsrec_interface *iface_rec, const struct sset *local_lports) { - /* Ports of type "virtual" should never be explicitly bound to an OVS - * port in the integration bridge. If that's the case, ignore the binding - * and log a warning. - */ - if (iface_rec && !strcmp(binding_rec->type, "virtual")) { - static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 1); - VLOG_WARN_RL(&rl, - "Virtual port %s should not be bound to OVS port %s", - binding_rec->logical_port, iface_rec->name); - return false; - } - bool our_chassis = false; - if (iface_rec - || (binding_rec->parent_port && binding_rec->parent_port[0] && - sset_contains(local_lports, binding_rec->parent_port))) { + if (binding_rec->parent_port && binding_rec->parent_port[0] && + sset_contains(local_lports, binding_rec->parent_port)) { /* This port is in our chassis unless it is a localport. */ our_chassis = strcmp(binding_rec->type, "localport"); } else if (!strcmp(binding_rec->type, "l2gateway")) { @@ -481,175 +426,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) -{ - if (binding_rec->virtual_parent) { - 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) { - return; - } - } - - /* 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. - */ - 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, @@ -712,6 +488,374 @@ 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); + + /* Ports of type "virtual" should never be explicitly bound to an OVS + * port in the integration bridge. If that's the case, ignore the binding + * and log a warning. + */ + if (!strcmp(pb->type, "virtual") && lbinding && lbinding->iface && + binding_type == BT_VIF) { + static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 1); + VLOG_WARN_RL(&rl, + "Virtual port %s should not be bound to OVS port %s", + pb->logical_port, lbinding->iface->name); + lbinding->pb = NULL; + return; + } + + 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: + OVS_NOT_REACHED(); + } + + 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, + 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) { @@ -721,49 +865,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); @@ -775,49 +932,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 @@ -829,24 +976,31 @@ 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; } - 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, "") && strcmp(binding_rec->type, - "remote"))) { + if (strcmp(binding_rec->type, "")) { + changed = true; + break; + } + + struct local_binding *lbinding = NULL; + 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 24c89e617..fdfa60e9c 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; } diff --git a/controller/pinctrl.c b/controller/pinctrl.c index 3230bb386..824be5e7a 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" diff --git a/controller/pinctrl.h b/controller/pinctrl.h index 8fa4baae9..12fb3b080 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, From patchwork Thu Apr 30 16:59:41 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Numan Siddique X-Patchwork-Id: 1280671 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 49ChTK17j8z9sSg for ; Fri, 1 May 2020 03:00:21 +1000 (AEST) Received: from localhost (localhost [127.0.0.1]) by whitealder.osuosl.org (Postfix) with ESMTP id 94F3C87FF2; Thu, 30 Apr 2020 17:00:19 +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 qu4+4ZgBUjy1; Thu, 30 Apr 2020 17:00:00 +0000 (UTC) Received: from lists.linuxfoundation.org (lf-lists.osuosl.org [140.211.9.56]) by whitealder.osuosl.org (Postfix) with ESMTP id DB3D687FB5; Thu, 30 Apr 2020 17:00:00 +0000 (UTC) Received: from lf-lists.osuosl.org (localhost [127.0.0.1]) by lists.linuxfoundation.org (Postfix) with ESMTP id BFE67C0864; Thu, 30 Apr 2020 17:00:00 +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 176D3C016F for ; Thu, 30 Apr 2020 16:59:58 +0000 (UTC) Received: from localhost (localhost [127.0.0.1]) by hemlock.osuosl.org (Postfix) with ESMTP id 0755A887B9 for ; Thu, 30 Apr 2020 16:59:58 +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 UPeoD4q2w7T0 for ; Thu, 30 Apr 2020 16:59:56 +0000 (UTC) X-Greylist: domain auto-whitelisted by SQLgrey-1.7.6 Received: from relay12.mail.gandi.net (relay12.mail.gandi.net [217.70.178.232]) by hemlock.osuosl.org (Postfix) with ESMTPS id 81D30887BC for ; Thu, 30 Apr 2020 16:59:55 +0000 (UTC) Received: from nummac.local (unknown [125.99.245.47]) (Authenticated sender: numans@ovn.org) by relay12.mail.gandi.net (Postfix) with ESMTPSA id BBA9A200007; Thu, 30 Apr 2020 16:59:52 +0000 (UTC) From: numans@ovn.org To: dev@openvswitch.org Date: Thu, 30 Apr 2020 22:29:41 +0530 Message-Id: <20200430165941.1975527-1-numans@ovn.org> X-Mailer: git-send-email 2.25.4 In-Reply-To: <20200430165741.1975097-1-numans@ovn.org> References: <20200430165741.1975097-1-numans@ovn.org> MIME-Version: 1.0 Subject: [ovs-dev] [PATCH v4 03/10] 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 | 589 ++++++++++++++++++++++++++++++------ controller/binding.h | 9 +- controller/ovn-controller.c | 61 +++- tests/ovn-performance.at | 13 + 4 files changed, 580 insertions(+), 92 deletions(-) diff --git a/controller/binding.c b/controller/binding.c index 66f4f65e3..c959cce63 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 @@ -488,6 +477,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, @@ -543,18 +554,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); @@ -607,10 +621,11 @@ binding_add_vport_to_local_bindings(struct shash *local_bindings, } } -static void +static bool claim_lport(const struct sbrec_port_binding *pb, const struct sbrec_chassis *chassis_rec, - const struct ovsrec_interface *iface_rec) + const struct ovsrec_interface *iface_rec, + bool cant_update_sb) { if (pb->chassis != chassis_rec) { if (pb->chassis) { @@ -624,6 +639,9 @@ claim_lport(const struct sbrec_port_binding *pb, VLOG_INFO("%s: Claiming %s", pb->logical_port, pb->mac[i]); } + if (cant_update_sb) { + return false; + } sbrec_port_binding_set_chassis(pb, chassis_rec); } @@ -631,25 +649,74 @@ claim_lport(const struct sbrec_port_binding *pb, struct sbrec_encap *encap_rec = sbrec_get_port_encap(chassis_rec, iface_rec); if (encap_rec && pb->encap != encap_rec) { + if (cant_update_sb) { + return false; + } sbrec_port_binding_set_encap(pb, encap_rec); } + + return true; } -static void -release_lport(const struct sbrec_port_binding *pb) +static bool +release_lport(const struct sbrec_port_binding *pb, bool cant_update_sb) { VLOG_INFO("Releasing lport %s from this chassis.", pb->logical_port); if (pb->encap) { - sbrec_port_binding_set_encap(pb, NULL); + if (pb->encap) { + if (cant_update_sb) { + return false; + } + sbrec_port_binding_set_encap(pb, NULL); + } + } + + if (pb->chassis) { + if (cant_update_sb) { + return false; + } + sbrec_port_binding_set_chassis(pb, NULL); } - sbrec_port_binding_set_chassis(pb, NULL); if (pb->virtual_parent) { + if (cant_update_sb) { + return false; + } sbrec_port_binding_set_virtual_parent(pb, NULL); } + + return true; } -static void +static bool +release_local_binding_children(struct local_binding *lbinding, + bool cant_update_sb) +{ + struct local_binding *l; + LIST_FOR_EACH (l, node, &lbinding->children) { + if (!release_lport(l->pb, cant_update_sb)) { + return false; + } + } + + return true; +} + +static bool +release_local_binding(struct local_binding *lbinding, bool cant_update_sb) +{ + if (!release_local_binding_children(lbinding, cant_update_sb)) { + return false; + } + + if (!release_lport(lbinding->pb, cant_update_sb)) { + return false; + } + + return true; +} + +static bool consider_port_binding_for_vif(const struct sbrec_port_binding *pb, struct binding_ctx_in *b_ctx_in, enum local_binding_type binding_type, @@ -657,6 +724,12 @@ consider_port_binding_for_vif(const struct sbrec_port_binding *pb, struct binding_ctx_out *b_ctx_out, struct hmap *qos_map) { + if (pb->type[0] && strcmp(pb->type, "virtual")) { + /* The port binding type should be empty or 'virtual' + * to consider it for port binding here. */ + return true; + } + 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) @@ -673,11 +746,14 @@ consider_port_binding_for_vif(const struct sbrec_port_binding *pb, "Virtual port %s should not be bound to OVS port %s", pb->logical_port, lbinding->iface->name); lbinding->pb = NULL; - return; + return false; } if (lbinding && lbinding->pb && can_bind) { - claim_lport(pb, b_ctx_in->chassis_rec, lbinding->iface); + if (!claim_lport(pb, b_ctx_in->chassis_rec, lbinding->iface, + !b_ctx_in->ovnsb_idl_txn)) { + return false; + } switch (binding_type) { case BT_VIF: @@ -726,22 +802,26 @@ consider_port_binding_for_vif(const struct sbrec_port_binding *pb, if (pb->chassis == b_ctx_in->chassis_rec) { if (!lbinding || !lbinding->pb || !can_bind) { - release_lport(pb); + if (!release_lport(pb, !b_ctx_in->ovnsb_idl_txn)) { + return false; + } } } + + return true; } -static void +static bool 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, b_ctx_out->local_lports); + bool success = true; if (!strcmp(pb->type, "l2gateway")) { if (our_chassis) { sset_add(b_ctx_out->local_lports, pb->logical_port); @@ -789,12 +869,14 @@ consider_port_binding(const struct sbrec_port_binding *pb, 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); + success = claim_lport(pb, b_ctx_in->chassis_rec, NULL, + !b_ctx_in->ovnsb_idl_txn); } else if (pb->chassis == b_ctx_in->chassis_rec) { - release_lport(pb); + success = release_lport(pb, !b_ctx_in->ovnsb_idl_txn); } + + return success; } static void @@ -833,6 +915,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. */ @@ -950,60 +1034,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->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 @@ -1038,3 +1068,390 @@ 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, + bool *changed) +{ + if (!b_ctx_in->chassis_rec) { + return false; + } + + bool handled = true; + *changed = 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 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] && + strcmp(iface_rec->type, "internal")) { + /* Right now are not handling ovs_interface changes of + * other types. This can be enhanced to handle of + * types - patch and tunnel. */ + 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) { + + if (!release_local_binding(lbinding, + !b_ctx_in->ovnsb_idl_txn)) { + handled = false; + goto out; + } + 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); + *changed = true; + } + + 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) { + *changed = true; + 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 && + !strcmp(claim_lbinding->pb->type, "virtual")) { + claim_lbinding->pb = NULL; + } + } + + if (claim_lbinding->pb) { + if (!consider_port_binding_for_vif(claim_lbinding->pb, + b_ctx_in, BT_VIF, + claim_lbinding, + b_ctx_out, qos_map_ptr)) { + handled = false; + goto out; + } + } + } + } + + 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 *changed) +{ + 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; + + *changed = false; + + 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) { + lbinding->pb = NULL; + if (binding_type == BT_VIF && + !release_local_binding_children( + lbinding, !b_ctx_in->ovnsb_idl_txn)) { + handled = false; + break; + } + *changed = true; + } + + 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) { + lbinding->pb = pb; + bool claimed = (pb->chassis == b_ctx_in->chassis_rec); + if (!consider_port_binding_for_vif( + pb, b_ctx_in, binding_type, lbinding, b_ctx_out, + qos_map_ptr)) { + handled = false; + break; + } + bool now_claimed = (pb->chassis == b_ctx_in->chassis_rec); + if (!claimed && now_claimed) { + *changed = true; + } + } + } else { + if (!consider_port_binding(pb, b_ctx_in, b_ctx_out, + qos_map_ptr)) { + handled = false; + break; + } + 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); + } + *changed = 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); + } + } + } + } + + return handled; +} diff --git a/controller/binding.h b/controller/binding.h index 6bee1d12e..fda680723 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,14 @@ 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 *changed); +bool binding_handle_port_binding_changes(struct binding_ctx_in *, + struct binding_ctx_out *, + bool *changed); #endif /* controller/binding.h */ diff --git a/controller/ovn-controller.c b/controller/ovn-controller.c index fdfa60e9c..76983c181 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; @@ -1138,6 +1150,34 @@ en_runtime_data_run(struct engine_node *node, void *data) engine_set_node_state(node, EN_UPDATED); } +static bool +runtime_data_ovs_interface_handler(struct engine_node *node, void *data) +{ + 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 = false; + if (!binding_handle_ovs_interface_changes(&b_ctx_in, &b_ctx_out, + &changed)) { + return false; + } + + if (changed) { + engine_set_node_state(node, EN_UPDATED); + } + + return true; +} + +static bool +runtime_data_noop_handler(struct engine_node *node OVS_UNUSED, + void *data OVS_UNUSED) +{ + return true; +} + static bool runtime_data_sb_port_binding_handler(struct engine_node *node, void *data) { @@ -1145,11 +1185,21 @@ runtime_data_sb_port_binding_handler(struct engine_node *node, void *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 changed = false; + if (!binding_handle_port_binding_changes(&b_ctx_in, &b_ctx_out, + &changed)) { + return false; + } - bool changed = binding_evaluate_port_binding_changes(&b_ctx_in, - &b_ctx_out); + if (changed) { + engine_set_node_state(node, EN_UPDATED); + } - return !changed; + return true; } /* Connection tracking zones. */ @@ -1891,7 +1941,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); diff --git a/tests/ovn-performance.at b/tests/ovn-performance.at index a8a15f8fe..5dfc6f7ca 100644 --- a/tests/ovn-performance.at +++ b/tests/ovn-performance.at @@ -239,6 +239,19 @@ for i in 1 2; do ovn_attach n1 br-phys 192.168.0.$i done +# Wait for the tunnel ports to be created and up. +# Otherwise this may affect the lflow_run count. + +OVS_WAIT_UNTIL([ + test $(as hv1 ovs-vsctl list interface ovn-hv2-0 | \ +grep tunnel_egress_iface_carrier=up | wc -l) -eq 1 +]) + +OVS_WAIT_UNTIL([ + test $(as hv2 ovs-vsctl list interface ovn-hv1-0 | \ +grep tunnel_egress_iface_carrier=up | wc -l) -eq 1 +]) + # Add router lr1 OVN_CONTROLLER_EXPECT_HIT( [hv1 hv2], [lflow_run], From patchwork Thu Apr 30 16:59:54 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Numan Siddique X-Patchwork-Id: 1280675 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 49ChVH2NGLz9sSK for ; Fri, 1 May 2020 03:01:11 +1000 (AEST) Received: from localhost (localhost [127.0.0.1]) by whitealder.osuosl.org (Postfix) with ESMTP id C61B887FF6; Thu, 30 Apr 2020 17:01:09 +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 V1ymmj6Gb2as; Thu, 30 Apr 2020 17:01:03 +0000 (UTC) Received: from lists.linuxfoundation.org (lf-lists.osuosl.org [140.211.9.56]) by whitealder.osuosl.org (Postfix) with ESMTP id A7A8587F92; Thu, 30 Apr 2020 17:00:42 +0000 (UTC) Received: from lf-lists.osuosl.org (localhost [127.0.0.1]) by lists.linuxfoundation.org (Postfix) with ESMTP id 8C870C0864; Thu, 30 Apr 2020 17:00:42 +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 D8FF7C016F for ; Thu, 30 Apr 2020 17:00:40 +0000 (UTC) Received: from localhost (localhost [127.0.0.1]) by silver.osuosl.org (Postfix) with ESMTP id AD585251C1 for ; Thu, 30 Apr 2020 17:00:40 +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 3QQ-ZRSGetd2 for ; Thu, 30 Apr 2020 17:00:36 +0000 (UTC) X-Greylist: domain auto-whitelisted by SQLgrey-1.7.6 Received: from relay9-d.mail.gandi.net (relay9-d.mail.gandi.net [217.70.183.199]) by silver.osuosl.org (Postfix) with ESMTPS id 1EC93252CA for ; Thu, 30 Apr 2020 17:00:04 +0000 (UTC) X-Originating-IP: 125.99.245.47 Received: from nummac.local (unknown [125.99.245.47]) (Authenticated sender: numans@ovn.org) by relay9-d.mail.gandi.net (Postfix) with ESMTPSA id 3C494FF804; Thu, 30 Apr 2020 17:00:00 +0000 (UTC) From: numans@ovn.org To: dev@openvswitch.org Date: Thu, 30 Apr 2020 22:29:54 +0530 Message-Id: <20200430165954.1975589-1-numans@ovn.org> X-Mailer: git-send-email 2.25.4 In-Reply-To: <20200430165741.1975097-1-numans@ovn.org> References: <20200430165741.1975097-1-numans@ovn.org> MIME-Version: 1.0 Subject: [ovs-dev] [PATCH v4 04/10] 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 if that datapath is present in the 'local_datapaths' hmap of runtime data. Signed-off-by: Numan Siddique --- controller/ovn-controller.c | 25 ++++++++++++++++++++++++- tests/ovn-performance.at | 6 +++--- 2 files changed, 27 insertions(+), 4 deletions(-) diff --git a/controller/ovn-controller.c b/controller/ovn-controller.c index 76983c181..6bcc31516 100644 --- a/controller/ovn-controller.c +++ b/controller/ovn-controller.c @@ -1202,6 +1202,28 @@ 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; + } + } + } + + return true; +} + /* Connection tracking zones. */ struct ed_type_ct_zones { unsigned long bitmap[BITMAP_N_LONGS(MAX_CT_ZONES)]; @@ -1948,7 +1970,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 5dfc6f7ca..5cc1960b6 100644 --- a/tests/ovn-performance.at +++ b/tests/ovn-performance.at @@ -253,7 +253,7 @@ grep tunnel_egress_iface_carrier=up | wc -l) -eq 1 ]) # Add router lr1 -OVN_CONTROLLER_EXPECT_HIT( +OVN_CONTROLLER_EXPECT_NO_HIT( [hv1 hv2], [lflow_run], [ovn-nbctl --wait=hv lr-add lr1] ) @@ -264,7 +264,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] ) @@ -427,7 +427,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 Thu Apr 30 17:00:02 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Numan Siddique X-Patchwork-Id: 1280674 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 49ChTt4YC5z9sSM for ; Fri, 1 May 2020 03:00:50 +1000 (AEST) Received: from localhost (localhost [127.0.0.1]) by silver.osuosl.org (Postfix) with ESMTP id 4458C25611; Thu, 30 Apr 2020 17:00:48 +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 w4YhgW+q9DGW; Thu, 30 Apr 2020 17:00:47 +0000 (UTC) Received: from lists.linuxfoundation.org (lf-lists.osuosl.org [140.211.9.56]) by silver.osuosl.org (Postfix) with ESMTP id 1B4E925316; Thu, 30 Apr 2020 17:00:16 +0000 (UTC) Received: from lf-lists.osuosl.org (localhost [127.0.0.1]) by lists.linuxfoundation.org (Postfix) with ESMTP id E7FEFC088D; Thu, 30 Apr 2020 17:00:15 +0000 (UTC) X-Original-To: dev@openvswitch.org Delivered-To: ovs-dev@lists.linuxfoundation.org Received: from fraxinus.osuosl.org (smtp4.osuosl.org [140.211.166.137]) by lists.linuxfoundation.org (Postfix) with ESMTP id A69F9C088F for ; Thu, 30 Apr 2020 17:00:13 +0000 (UTC) Received: from localhost (localhost [127.0.0.1]) by fraxinus.osuosl.org (Postfix) with ESMTP id A3BA386E85 for ; Thu, 30 Apr 2020 17:00:13 +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 jRVmeMUxkbEz for ; Thu, 30 Apr 2020 17:00:12 +0000 (UTC) X-Greylist: domain auto-whitelisted by SQLgrey-1.7.6 Received: from relay10.mail.gandi.net (relay10.mail.gandi.net [217.70.178.230]) by fraxinus.osuosl.org (Postfix) with ESMTPS id 25C3886EB0 for ; Thu, 30 Apr 2020 17:00:11 +0000 (UTC) Received: from nummac.local (unknown [125.99.245.47]) (Authenticated sender: numans@ovn.org) by relay10.mail.gandi.net (Postfix) with ESMTPSA id 12976240010; Thu, 30 Apr 2020 17:00:07 +0000 (UTC) From: numans@ovn.org To: dev@openvswitch.org Date: Thu, 30 Apr 2020 22:30:02 +0530 Message-Id: <20200430170002.1975643-1-numans@ovn.org> X-Mailer: git-send-email 2.25.4 In-Reply-To: <20200430165741.1975097-1-numans@ovn.org> References: <20200430165741.1975097-1-numans@ovn.org> MIME-Version: 1.0 Subject: [ovs-dev] [PATCH v4 05/10] 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 485a857d1..3b257be35 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 Thu Apr 30 17:00: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: 1280672 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 49ChTQ55d1z9sSM for ; Fri, 1 May 2020 03:00:26 +1000 (AEST) Received: from localhost (localhost [127.0.0.1]) by fraxinus.osuosl.org (Postfix) with ESMTP id 1232386EB9; Thu, 30 Apr 2020 17:00:25 +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 l2cihWIJ9LIe; Thu, 30 Apr 2020 17:00:23 +0000 (UTC) Received: from lists.linuxfoundation.org (lf-lists.osuosl.org [140.211.9.56]) by fraxinus.osuosl.org (Postfix) with ESMTP id 2A65186DEC; Thu, 30 Apr 2020 17:00:23 +0000 (UTC) Received: from lf-lists.osuosl.org (localhost [127.0.0.1]) by lists.linuxfoundation.org (Postfix) with ESMTP id 08D04C088D; Thu, 30 Apr 2020 17:00:23 +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 C4431C016F for ; Thu, 30 Apr 2020 17:00:21 +0000 (UTC) Received: from localhost (localhost [127.0.0.1]) by hemlock.osuosl.org (Postfix) with ESMTP id AEC23887C1 for ; Thu, 30 Apr 2020 17:00:21 +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 9-e3UI6cZJBR for ; Thu, 30 Apr 2020 17:00:18 +0000 (UTC) X-Greylist: domain auto-whitelisted by SQLgrey-1.7.6 Received: from relay7-d.mail.gandi.net (relay7-d.mail.gandi.net [217.70.183.200]) by hemlock.osuosl.org (Postfix) with ESMTPS id 0F31D887BB for ; Thu, 30 Apr 2020 17:00:17 +0000 (UTC) X-Originating-IP: 125.99.245.47 Received: from nummac.local (unknown [125.99.245.47]) (Authenticated sender: numans@ovn.org) by relay7-d.mail.gandi.net (Postfix) with ESMTPSA id E02EE2000B; Thu, 30 Apr 2020 17:00:13 +0000 (UTC) From: numans@ovn.org To: dev@openvswitch.org Date: Thu, 30 Apr 2020 22:30:09 +0530 Message-Id: <20200430170009.1975692-1-numans@ovn.org> X-Mailer: git-send-email 2.25.4 In-Reply-To: <20200430165741.1975097-1-numans@ovn.org> References: <20200430165741.1975097-1-numans@ovn.org> MIME-Version: 1.0 Subject: [ovs-dev] [PATCH v4 06/10] 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. Signed-off-by: Numan Siddique --- controller/binding.c | 22 --------- controller/lflow.c | 13 ++++++ controller/lflow.h | 2 + controller/ovn-controller.c | 92 ++++++++++++++++++++++++++++++++++++- controller/ovn-controller.h | 22 +++++++++ controller/physical.c | 51 ++++++++++++++++++++ controller/physical.h | 5 +- 7 files changed, 182 insertions(+), 25 deletions(-) diff --git a/controller/binding.c b/controller/binding.c index c959cce63..6eab08e5c 100644 --- a/controller/binding.c +++ b/controller/binding.c @@ -499,22 +499,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, @@ -535,12 +519,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 6bcc31516..289a55dbd 100644 --- a/controller/ovn-controller.c +++ b/controller/ovn-controller.c @@ -1367,8 +1367,13 @@ 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", + engine_get_input("physical_flow_changes", node))); struct ed_type_ct_zones *ct_zones_data = - engine_get_input_data("ct_zones", node); + engine_get_input_data("ct_zones", + engine_get_input("physical_flow_changes", node)); struct simap *ct_zones = &ct_zones_data->current; p_ctx->sbrec_port_binding_by_name = sbrec_port_binding_by_name; @@ -1376,12 +1381,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, @@ -1644,6 +1651,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 @@ -1654,6 +1663,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); @@ -1789,6 +1806,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 need_physical_run; + bool ovs_ifaces_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->need_physical_run) { + pfc->need_physical_run = false; + physical_run(&p_ctx, &fo->flow_table); + return true; + } + + if (pfc->ovs_ifaces_changed) { + pfc->ovs_ifaces_changed = false; + return physical_handle_ovs_iface_changes(&p_ctx, &fo->flow_table); + } + + return true; +} + +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) +{ + struct ed_type_physical_flow_changes *pfc = data; + pfc->need_physical_run = true; + engine_set_node_state(node, EN_UPDATED); +} + +static bool +physical_flow_changes_ovs_iface_handler(struct engine_node *node OVS_UNUSED, + void *data OVS_UNUSED) +{ + struct ed_type_physical_flow_changes *pfc = data; + pfc->ovs_ifaces_changed = true; + engine_set_node_state(node, EN_UPDATED); + return true; +} + struct ovn_controller_exit_args { bool *exiting; bool *restart; @@ -1911,6 +1992,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"); @@ -1930,13 +2012,19 @@ 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, + NULL); + engine_add_input(&en_physical_flow_changes, &en_ovs_interface, + physical_flow_changes_ovs_iface_handler); + 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_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); 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..03eb677d1 100644 --- a/controller/physical.c +++ b/controller/physical.c @@ -1780,6 +1780,57 @@ 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; + } + + int64_t ofport = iface_rec->n_ofport ? *iface_rec->ofport : 0; + if (ovsrec_interface_is_deleted(iface_rec)) { + ofctrl_remove_flows(flow_table, &lb->pb->header_.uuid); + simap_find_and_delete(&localvif_to_ofport, iface_id); + } else { + if (!ovsrec_interface_is_new(iface_rec)) { + ofctrl_remove_flows(flow_table, &lb->pb->header_.uuid); + } + + simap_put(&localvif_to_ofport, iface_id, ofport); + 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 */ From patchwork Thu Apr 30 17:00:15 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Numan Siddique X-Patchwork-Id: 1280676 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 49ChVN3cznz9sSK for ; Fri, 1 May 2020 03:01:16 +1000 (AEST) Received: from localhost (localhost [127.0.0.1]) by whitealder.osuosl.org (Postfix) with ESMTP id 046778805A; Thu, 30 Apr 2020 17:01:15 +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 CQpMwFTiMvX2; Thu, 30 Apr 2020 17:01:06 +0000 (UTC) Received: from lists.linuxfoundation.org (lf-lists.osuosl.org [140.211.9.56]) by whitealder.osuosl.org (Postfix) with ESMTP id EE94487FED; Thu, 30 Apr 2020 17:00:44 +0000 (UTC) Received: from lf-lists.osuosl.org (localhost [127.0.0.1]) by lists.linuxfoundation.org (Postfix) with ESMTP id CA133C088F; Thu, 30 Apr 2020 17:00:44 +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 DA261C088F for ; Thu, 30 Apr 2020 17:00:42 +0000 (UTC) Received: from localhost (localhost [127.0.0.1]) by whitealder.osuosl.org (Postfix) with ESMTP id C3FFE87FA5 for ; Thu, 30 Apr 2020 17:00: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 xoTY5OtEElB3 for ; Thu, 30 Apr 2020 17:00:38 +0000 (UTC) X-Greylist: domain auto-whitelisted by SQLgrey-1.7.6 Received: from relay10.mail.gandi.net (relay10.mail.gandi.net [217.70.178.230]) by whitealder.osuosl.org (Postfix) with ESMTPS id 1D46088005 for ; Thu, 30 Apr 2020 17:00:21 +0000 (UTC) Received: from nummac.local (unknown [125.99.245.47]) (Authenticated sender: numans@ovn.org) by relay10.mail.gandi.net (Postfix) with ESMTPSA id 9A193240004; Thu, 30 Apr 2020 17:00:19 +0000 (UTC) From: numans@ovn.org To: dev@openvswitch.org Date: Thu, 30 Apr 2020 22:30:15 +0530 Message-Id: <20200430170015.1975741-1-numans@ovn.org> X-Mailer: git-send-email 2.25.4 In-Reply-To: <20200430165741.1975097-1-numans@ovn.org> References: <20200430165741.1975097-1-numans@ovn.org> MIME-Version: 1.0 Subject: [ovs-dev] [PATCH v4 07/10] I-P engine: Provide the option for an engine to store changed 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 With this patch, an engine node which is an input to another engine node can provide the tracked data. The parent of this engine node can handle this tracked data incrementally. At the end of the engine_run(), the tracked data of the nodes are cleared. Signed-off-by: Numan Siddique --- lib/inc-proc-eng.c | 30 +++++++++++++++++++++++++++++- lib/inc-proc-eng.h | 20 ++++++++++++++++++++ 2 files changed, 49 insertions(+), 1 deletion(-) diff --git a/lib/inc-proc-eng.c b/lib/inc-proc-eng.c index 9b1479a1c..8d63feac7 100644 --- a/lib/inc-proc-eng.c +++ b/lib/inc-proc-eng.c @@ -125,6 +125,11 @@ engine_cleanup(void) engine_nodes[i]->cleanup(engine_nodes[i]->data); } free(engine_nodes[i]->data); + + if (engine_nodes[i]->clear_tracked_data) { + engine_nodes[i]->clear_tracked_data(engine_nodes[i]->tracked_data); + } + free(engine_nodes[i]->tracked_data); } free(engine_nodes); engine_nodes = NULL; @@ -222,6 +227,23 @@ engine_node_changed(struct engine_node *node) return node->state == EN_UPDATED; } +void * +engine_get_tracked_data(struct engine_node *node) +{ + if (engine_node_valid(node)) { + return node->tracked_data; + } + + return NULL; +} + +void * +engine_get_input_tracked_data(const char *input_name, struct engine_node *node) +{ + struct engine_node *input_node = engine_get_input(input_name, node); + return engine_get_tracked_data(input_node); +} + bool engine_has_run(void) { @@ -370,7 +392,13 @@ engine_run(bool recompute_allowed) if (engine_nodes[i]->state == EN_ABORTED) { engine_run_aborted = true; - return; + break; + } + } + + for (size_t i = 0; i < engine_n_nodes; i++) { + if (engine_nodes[i]->clear_tracked_data) { + engine_nodes[i]->clear_tracked_data(engine_nodes[i]->tracked_data); } } } diff --git a/lib/inc-proc-eng.h b/lib/inc-proc-eng.h index 780c3cd22..4b5e69edb 100644 --- a/lib/inc-proc-eng.h +++ b/lib/inc-proc-eng.h @@ -123,6 +123,15 @@ struct engine_node { */ void *data; + /* A pointer to node internal tracked data. The tracke data can be + * used by en engine node to provide the changed data during the + * engine run if its an input to other engine node. This data can + * be accessed during the engine run using the function + * engine_get_tracked_data(). This data can be cleared by + * calling the clean_tracked_data() engine node function. + */ + void *tracked_data; + /* State of the node after the last engine run. */ enum engine_node_state state; @@ -149,6 +158,9 @@ struct engine_node { * doesn't store pointers to DB records it's still safe to use). */ bool (*is_valid)(struct engine_node *); + + /* Method to clear up tracked data if any. It may be NULL. */ + void (*clear_tracked_data)(void *tracked_data); }; /* Initialize the data for the engine nodes. It calls each node's @@ -183,6 +195,12 @@ struct engine_node * engine_get_input(const char *input_name, /* Get the data from the input node with for */ void *engine_get_input_data(const char *input_name, struct engine_node *); +void *engine_get_tracked_data(struct engine_node *); + +/* Get the tracked data from the input node with for */ +void *engine_get_input_tracked_data(const char *input_name, + struct engine_node *); + /* Add an input (dependency) for , with corresponding change_handler, * which can be NULL. If the change_handler is NULL, the engine will not * be able to process the change incrementally, and will fall back to call @@ -270,11 +288,13 @@ void engine_ovsdb_node_add_index(struct engine_node *, const char *name, struct engine_node en_##NAME = { \ .name = NAME_STR, \ .data = NULL, \ + .tracked_data = NULL, \ .state = EN_STALE, \ .init = en_##NAME##_init, \ .run = en_##NAME##_run, \ .cleanup = en_##NAME##_cleanup, \ .is_valid = en_##NAME##_is_valid, \ + .clear_tracked_data = NULL, \ }; #define ENGINE_NODE_CUSTOM_DATA(NAME, NAME_STR) \ From patchwork Thu Apr 30 17:00:20 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Numan Siddique X-Patchwork-Id: 1280673 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 49ChTf3xDKz9sSK for ; Fri, 1 May 2020 03:00:38 +1000 (AEST) Received: from localhost (localhost [127.0.0.1]) by hemlock.osuosl.org (Postfix) with ESMTP id D7B1F887D3; Thu, 30 Apr 2020 17:00:36 +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 XJgZGkwG-Mpz; Thu, 30 Apr 2020 17:00:31 +0000 (UTC) Received: from lists.linuxfoundation.org (lf-lists.osuosl.org [140.211.9.56]) by hemlock.osuosl.org (Postfix) with ESMTP id DAC4E887BC; Thu, 30 Apr 2020 17:00:31 +0000 (UTC) Received: from lf-lists.osuosl.org (localhost [127.0.0.1]) by lists.linuxfoundation.org (Postfix) with ESMTP id C9EECC0864; Thu, 30 Apr 2020 17:00:31 +0000 (UTC) X-Original-To: dev@openvswitch.org Delivered-To: ovs-dev@lists.linuxfoundation.org Received: from fraxinus.osuosl.org (smtp4.osuosl.org [140.211.166.137]) by lists.linuxfoundation.org (Postfix) with ESMTP id 978E9C0890 for ; Thu, 30 Apr 2020 17:00:30 +0000 (UTC) Received: from localhost (localhost [127.0.0.1]) by fraxinus.osuosl.org (Postfix) with ESMTP id 77AFF86EB4 for ; Thu, 30 Apr 2020 17:00:30 +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 ZMt1-rPbBq2M for ; Thu, 30 Apr 2020 17:00:28 +0000 (UTC) X-Greylist: domain auto-whitelisted by SQLgrey-1.7.6 Received: from relay7-d.mail.gandi.net (relay7-d.mail.gandi.net [217.70.183.200]) by fraxinus.osuosl.org (Postfix) with ESMTPS id 1979686D8D for ; Thu, 30 Apr 2020 17:00:27 +0000 (UTC) X-Originating-IP: 125.99.245.47 Received: from nummac.local (unknown [125.99.245.47]) (Authenticated sender: numans@ovn.org) by relay7-d.mail.gandi.net (Postfix) with ESMTPSA id 57C4120006; Thu, 30 Apr 2020 17:00:25 +0000 (UTC) From: numans@ovn.org To: dev@openvswitch.org Date: Thu, 30 Apr 2020 22:30:20 +0530 Message-Id: <20200430170020.1975791-1-numans@ovn.org> X-Mailer: git-send-email 2.25.4 In-Reply-To: <20200430165741.1975097-1-numans@ovn.org> References: <20200430165741.1975097-1-numans@ovn.org> MIME-Version: 1.0 Cc: Venkata Anil Subject: [ovs-dev] [PATCH v4 08/10] I-P engine: Handle runtime data changes in flow output engine 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 In order to handle runtime data changes incrementally, the flow outut runtime data handle should know the changed runtime data. Runtime data now tracks the changed data for any OVS interface and SB port binding changes. The tracked data contains a hmap of tracked datapaths (which changed during runtime data processing. The flow outout runtime_data handler in this patch doesn't do much with the tracked data. It returns false if there is tracked data available so that flow_output run is called. If no tracked data is available then there is no need for flow computation and the handler returns true. Next patch in the series processes the tracked data incrementally. Co-Authored-by: Venkata Anil Signed-off-by: Venkata Anil Signed-off-by: Numan Siddique --- controller/binding.c | 156 +++++++++++++++++++++++++++++++++--- controller/binding.h | 6 ++ controller/ovn-controller.c | 62 +++++++++++++- controller/ovn-controller.h | 16 +++- tests/ovn-performance.at | 24 +++--- 5 files changed, 237 insertions(+), 27 deletions(-) diff --git a/controller/binding.c b/controller/binding.c index 6eab08e5c..2223c7e66 100644 --- a/controller/binding.c +++ b/controller/binding.c @@ -69,13 +69,20 @@ binding_register_ovs_idl(struct ovsdb_idl *ovs_idl) ovsdb_idl_add_column(ovs_idl, &ovsrec_qos_col_type); } +static struct tracked_binding_datapath *tracked_binding_datapath_create( + struct hmap *tracked_dps, const struct sbrec_datapath_binding *, + bool is_new); +static struct tracked_binding_datapath *tracked_binding_datapath_find( + struct hmap *, const struct sbrec_datapath_binding *); + static void add_local_datapath__(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 sbrec_datapath_binding *datapath, bool has_local_l3gateway, int depth, - struct hmap *local_datapaths) + struct hmap *local_datapaths, + struct hmap *updated_dp_bindings) { uint32_t dp_key = datapath->tunnel_key; struct local_datapath *ld = get_local_datapath(local_datapaths, dp_key); @@ -92,6 +99,11 @@ add_local_datapath__(struct ovsdb_idl_index *sbrec_datapath_binding_by_key, ld->localnet_port = NULL; ld->has_local_l3gateway = has_local_l3gateway; + if (updated_dp_bindings && + !tracked_binding_datapath_find(updated_dp_bindings, datapath)) { + tracked_binding_datapath_create(updated_dp_bindings, datapath, true); + } + if (depth >= 100) { static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 1); VLOG_WARN_RL(&rl, "datapaths nested too deep"); @@ -124,7 +136,8 @@ add_local_datapath__(struct ovsdb_idl_index *sbrec_datapath_binding_by_key, sbrec_port_binding_by_datapath, sbrec_port_binding_by_name, peer->datapath, false, - depth + 1, local_datapaths); + depth + 1, local_datapaths, + updated_dp_bindings); } ld->n_peer_ports++; if (ld->n_peer_ports > ld->n_allocated_peer_ports) { @@ -147,12 +160,14 @@ add_local_datapath(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 sbrec_datapath_binding *datapath, - bool has_local_l3gateway, struct hmap *local_datapaths) + bool has_local_l3gateway, struct hmap *local_datapaths, + struct hmap *updated_dp_bindings) { add_local_datapath__(sbrec_datapath_binding_by_key, sbrec_port_binding_by_datapath, sbrec_port_binding_by_name, - datapath, has_local_l3gateway, 0, local_datapaths); + datapath, has_local_l3gateway, 0, local_datapaths, + updated_dp_bindings); } static void @@ -580,6 +595,71 @@ local_binding_find_child(struct local_binding *lbinding, return NULL; } +static struct tracked_binding_datapath * +tracked_binding_datapath_create(struct hmap *tracked_datapaths, + const struct sbrec_datapath_binding *dp, + bool is_new) +{ + struct tracked_binding_datapath *t_dp = xzalloc(sizeof *t_dp); + t_dp->dp = dp; + t_dp->is_new = is_new; + ovs_list_init(&t_dp->lports_head); + hmap_insert(tracked_datapaths, &t_dp->node, uuid_hash(&dp->header_.uuid)); + return t_dp; +} + +static struct tracked_binding_datapath * +tracked_binding_datapath_find(struct hmap *tracked_datapaths, + const struct sbrec_datapath_binding *dp) +{ + struct tracked_binding_datapath *t_dp; + size_t hash = uuid_hash(&dp->header_.uuid); + HMAP_FOR_EACH_WITH_HASH (t_dp, node, hash, tracked_datapaths) { + if (uuid_equals(&t_dp->dp->header_.uuid, &dp->header_.uuid)) { + return t_dp; + } + } + + return NULL; +} + +static void +tracked_binding_datapath_lport_add(struct hmap *tracked_datapaths, + const struct sbrec_port_binding *pb, + bool deleted) +{ + if (!tracked_datapaths) { + return; + } + + struct tracked_binding_datapath *tracked_dp = + tracked_binding_datapath_find(tracked_datapaths, pb->datapath); + if (!tracked_dp) { + tracked_dp = tracked_binding_datapath_create(tracked_datapaths, + pb->datapath, false); + } + struct tracked_binding_lport *lport = xmalloc(sizeof *lport); + lport->pb = pb; + lport->deleted = deleted; + ovs_list_push_back(&tracked_dp->lports_head, &lport->list_node); +} + +void +binding_tracked_dp_destroy(struct hmap *tracked_datapaths) +{ + struct tracked_binding_datapath *t_dp; + HMAP_FOR_EACH_POP (t_dp, node, tracked_datapaths) { + struct tracked_binding_lport *lport, *next; + LIST_FOR_EACH_SAFE (lport, next, list_node, &t_dp->lports_head) { + ovs_list_remove(&lport->list_node); + free(lport); + } + free(t_dp); + } + + hmap_destroy(tracked_datapaths); +} + void binding_add_vport_to_local_bindings(struct shash *local_bindings, const struct sbrec_port_binding *parent, @@ -668,6 +748,7 @@ release_lport(const struct sbrec_port_binding *pb, bool cant_update_sb) static bool release_local_binding_children(struct local_binding *lbinding, + struct hmap *tracked_dp_bindings, bool cant_update_sb) { struct local_binding *l; @@ -675,15 +756,23 @@ release_local_binding_children(struct local_binding *lbinding, if (!release_lport(l->pb, cant_update_sb)) { return false; } + if (tracked_dp_bindings) { + tracked_binding_datapath_lport_add(tracked_dp_bindings, l->pb, + true); + } } return true; } static bool -release_local_binding(struct local_binding *lbinding, bool cant_update_sb) +release_local_binding(struct local_binding *lbinding, + struct hmap *tracked_dp_bindings, + bool cant_update_sb) { - if (!release_local_binding_children(lbinding, cant_update_sb)) { + if (!release_local_binding_children(lbinding, + tracked_dp_bindings, + cant_update_sb)) { return false; } @@ -691,6 +780,10 @@ release_local_binding(struct local_binding *lbinding, bool cant_update_sb) return false; } + if (tracked_dp_bindings) { + tracked_binding_datapath_lport_add(tracked_dp_bindings, lbinding->pb, + true); + } return true; } @@ -748,6 +841,10 @@ consider_port_binding_for_vif(const struct sbrec_port_binding *pb, child = local_binding_create(pb->logical_port, lbinding->iface, pb, binding_type); local_binding_add_child(lbinding, child); + if (b_ctx_out->tracked_dp_bindings) { + tracked_binding_datapath_lport_add( + b_ctx_out->tracked_dp_bindings, pb, false); + } } else { ovs_assert(child->type == BT_CHILD || child->type == BT_VIRTUAL); @@ -763,7 +860,8 @@ consider_port_binding_for_vif(const struct sbrec_port_binding *pb, 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); + pb->datapath, false, b_ctx_out->local_datapaths, + b_ctx_out->tracked_dp_bindings); 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); @@ -807,7 +905,8 @@ consider_port_binding(const struct sbrec_port_binding *pb, b_ctx_in->sbrec_port_binding_by_datapath, b_ctx_in->sbrec_port_binding_by_name, pb->datapath, false, - b_ctx_out->local_datapaths); + b_ctx_out->local_datapaths, + b_ctx_out->tracked_dp_bindings); } } else if (!strcmp(pb->type, "chassisredirect")) { if (ha_chassis_group_contains(pb->ha_chassis_group, @@ -816,14 +915,16 @@ consider_port_binding(const struct sbrec_port_binding *pb, b_ctx_in->sbrec_port_binding_by_datapath, b_ctx_in->sbrec_port_binding_by_name, pb->datapath, false, - b_ctx_out->local_datapaths); + b_ctx_out->local_datapaths, + b_ctx_out->tracked_dp_bindings); } } 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); + pb->datapath, true, b_ctx_out->local_datapaths, + b_ctx_out->tracked_dp_bindings); } } else if (!strcmp(pb->type, "localnet")) { /* Add all localnet ports to local_lports so that we allocate ct zones @@ -839,7 +940,8 @@ consider_port_binding(const struct sbrec_port_binding *pb, b_ctx_in->sbrec_port_binding_by_datapath, b_ctx_in->sbrec_port_binding_by_name, pb->datapath, false, - b_ctx_out->local_datapaths); + b_ctx_out->local_datapaths, + b_ctx_out->tracked_dp_bindings); } } @@ -978,6 +1080,9 @@ binding_run(struct binding_ctx_in *b_ctx_in, struct binding_ctx_out *b_ctx_out) consider_port_binding_for_vif(binding_rec, b_ctx_in, binding_type, lbinding, b_ctx_out, qos_map_ptr); + if (lbinding && lbinding->pb && + lbinding->pb->chassis == b_ctx_in->chassis_rec) { + } } else { consider_port_binding(binding_rec, b_ctx_in, b_ctx_out, qos_map_ptr); @@ -1094,7 +1199,8 @@ add_local_datapath_peer_port(const struct sbrec_port_binding *pb, 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); + 1, b_ctx_out->local_datapaths, + b_ctx_out->tracked_dp_bindings); return; } @@ -1217,6 +1323,7 @@ binding_handle_ovs_interface_changes(struct binding_ctx_in *b_ctx_in, struct hmap *qos_map_ptr = sset_is_empty(b_ctx_out->egress_ifaces) ? NULL : &qos_map; + *b_ctx_out->local_lports_changed = false; const struct ovsrec_interface *iface_rec; OVSREC_INTERFACE_TABLE_FOR_EACH_TRACKED (iface_rec, b_ctx_in->iface_table) { @@ -1259,6 +1366,7 @@ binding_handle_ovs_interface_changes(struct binding_ctx_in *b_ctx_in, lbinding->pb->chassis == b_ctx_in->chassis_rec) { if (!release_local_binding(lbinding, + b_ctx_out->tracked_dp_bindings, !b_ctx_in->ovnsb_idl_txn)) { handled = false; goto out; @@ -1271,12 +1379,14 @@ binding_handle_ovs_interface_changes(struct binding_ctx_in *b_ctx_in, b_ctx_in->chassis_rec, b_ctx_out, ld); } + local_binding_delete(b_ctx_out->local_bindings, lbinding); *changed = true; } sset_find_and_delete(b_ctx_out->local_lports, cleared_iface_id); smap_remove(b_ctx_out->local_iface_ids, iface_rec->name); + *b_ctx_out->local_lports_changed = true; } if (iface_id && ofport > 0) { @@ -1284,6 +1394,7 @@ binding_handle_ovs_interface_changes(struct binding_ctx_in *b_ctx_in, sset_add(b_ctx_out->local_lports, iface_id); smap_replace(b_ctx_out->local_iface_ids, iface_rec->name, iface_id); + *b_ctx_out->local_lports_changed = true; claim_lbinding = local_binding_find(b_ctx_out->local_bindings, iface_id); if (!claim_lbinding) { @@ -1307,6 +1418,8 @@ binding_handle_ovs_interface_changes(struct binding_ctx_in *b_ctx_in, } if (claim_lbinding->pb) { + bool claimed = + (claim_lbinding->pb->chassis == b_ctx_in->chassis_rec); if (!consider_port_binding_for_vif(claim_lbinding->pb, b_ctx_in, BT_VIF, claim_lbinding, @@ -1314,6 +1427,14 @@ binding_handle_ovs_interface_changes(struct binding_ctx_in *b_ctx_in, handled = false; goto out; } + bool now_claimed = (claim_lbinding->pb && ( + claim_lbinding->pb->chassis == b_ctx_in->chassis_rec)); + if (!claimed && now_claimed) { + /* Add this to the updated tracked datapath bindings. */ + tracked_binding_datapath_lport_add( + b_ctx_out->tracked_dp_bindings, claim_lbinding->pb, + false); + } } } } @@ -1377,9 +1498,13 @@ binding_handle_port_binding_changes(struct binding_ctx_in *b_ctx_in, if (is_deleted) { if (lbinding) { lbinding->pb = NULL; + /* Add this to the deleted tracked datapath bindings. */ + tracked_binding_datapath_lport_add( + b_ctx_out->tracked_dp_bindings, pb, true); if (binding_type == BT_VIF && !release_local_binding_children( - lbinding, !b_ctx_in->ovnsb_idl_txn)) { + lbinding, b_ctx_out->tracked_dp_bindings, + !b_ctx_in->ovnsb_idl_txn)) { handled = false; break; } @@ -1407,6 +1532,11 @@ binding_handle_port_binding_changes(struct binding_ctx_in *b_ctx_in, bool now_claimed = (pb->chassis == b_ctx_in->chassis_rec); if (!claimed && now_claimed) { *changed = true; + + /* Add this to the updated tracked datapath + * bindings. */ + tracked_binding_datapath_lport_add( + b_ctx_out->tracked_dp_bindings, pb, false); } } } else { diff --git a/controller/binding.h b/controller/binding.h index fda680723..2f79dd63c 100644 --- a/controller/binding.h +++ b/controller/binding.h @@ -18,6 +18,9 @@ #define OVN_BINDING_H 1 #include +#include "openvswitch/hmap.h" +#include "openvswitch/uuid.h" +#include "openvswitch/list.h" struct hmap; struct ovsdb_idl; @@ -57,6 +60,8 @@ struct binding_ctx_out { struct sset *local_lport_ids; struct sset *egress_ifaces; struct smap *local_iface_ids; + struct hmap *tracked_dp_bindings; + bool *local_lports_changed; }; void binding_register_ovs_idl(struct ovsdb_idl *); @@ -74,4 +79,5 @@ bool binding_handle_ovs_interface_changes(struct binding_ctx_in *, bool binding_handle_port_binding_changes(struct binding_ctx_in *, struct binding_ctx_out *, bool *changed); +void binding_tracked_dp_destroy(struct hmap *tracked_datapaths); #endif /* controller/binding.h */ diff --git a/controller/ovn-controller.c b/controller/ovn-controller.c index 289a55dbd..94ad79295 100644 --- a/controller/ovn-controller.c +++ b/controller/ovn-controller.c @@ -978,6 +978,23 @@ struct ed_type_runtime_data { struct smap local_iface_ids; }; +struct ed_type_runtime_tracked_data { + struct hmap tracked_dp_bindings; + bool local_lports_changed; + bool tracked; +}; + +static void +en_runtime_clear_tracked_data(void *tracked_data) +{ + struct ed_type_runtime_tracked_data *data = tracked_data; + + binding_tracked_dp_destroy(&data->tracked_dp_bindings); + hmap_init(&data->tracked_dp_bindings); + data->local_lports_changed = false; + data->tracked = false; +} + static void * en_runtime_data_init(struct engine_node *node OVS_UNUSED, struct engine_arg *arg OVS_UNUSED) @@ -991,6 +1008,13 @@ en_runtime_data_init(struct engine_node *node OVS_UNUSED, sset_init(&data->egress_ifaces); shash_init(&data->local_bindings); smap_init(&data->local_iface_ids); + + struct ed_type_runtime_tracked_data *tracked_data = + xzalloc(sizeof *tracked_data); + hmap_init(&tracked_data->tracked_dp_bindings); + node->tracked_data = tracked_data; + node->clear_tracked_data = en_runtime_clear_tracked_data; + return data; } @@ -1093,6 +1117,8 @@ init_binding_ctx(struct engine_node *node, 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; + b_ctx_out->tracked_dp_bindings = NULL; + b_ctx_out->local_lports_changed = NULL; } static void @@ -1104,6 +1130,8 @@ en_runtime_data_run(struct engine_node *node, void *data) struct sset *local_lport_ids = &rt_data->local_lport_ids; struct sset *active_tunnels = &rt_data->active_tunnels; + en_runtime_clear_tracked_data(node->tracked_data); + static bool first_run = true; if (first_run) { /* don't cleanup since there is no data yet */ @@ -1154,9 +1182,13 @@ static bool runtime_data_ovs_interface_handler(struct engine_node *node, void *data) { struct ed_type_runtime_data *rt_data = data; + struct ed_type_runtime_tracked_data *tracked_data = node->tracked_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); + tracked_data->tracked = true; + b_ctx_out.tracked_dp_bindings = &tracked_data->tracked_dp_bindings; + b_ctx_out.local_lports_changed = &tracked_data->local_lports_changed; bool changed = false; if (!binding_handle_ovs_interface_changes(&b_ctx_in, &b_ctx_out, @@ -1189,6 +1221,10 @@ runtime_data_sb_port_binding_handler(struct engine_node *node, void *data) return false; } + struct ed_type_runtime_tracked_data *tracked_data = node->tracked_data; + tracked_data->tracked = true; + b_ctx_out.tracked_dp_bindings = &tracked_data->tracked_dp_bindings; + bool changed = false; if (!binding_handle_port_binding_changes(&b_ctx_in, &b_ctx_out, &changed)) { @@ -1870,6 +1906,29 @@ physical_flow_changes_ovs_iface_handler(struct engine_node *node OVS_UNUSED, return true; } +static bool +flow_output_runtime_data_handler(struct engine_node *node, + void *data OVS_UNUSED) +{ + struct ed_type_runtime_tracked_data *tracked_data = + engine_get_input_tracked_data("runtime_data", node); + + if (!tracked_data || !tracked_data->tracked) { + return false; + } + + if (!hmap_is_empty(&tracked_data->tracked_dp_bindings)) { + /* We are not yet handling the tracked datapath binding + * changes. Return false to trigger full recompute. */ + return false; + } + + if (tracked_data->local_lports_changed) { + engine_set_node_state(node, EN_UPDATED); + } + return true; +} + struct ovn_controller_exit_args { bool *exiting; bool *restart; @@ -2021,7 +2080,8 @@ main(int argc, char *argv[]) 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_runtime_data, + flow_output_runtime_data_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); diff --git a/controller/ovn-controller.h b/controller/ovn-controller.h index 40c358cef..6c40a6492 100644 --- a/controller/ovn-controller.h +++ b/controller/ovn-controller.h @@ -84,7 +84,6 @@ struct local_binding { enum local_binding_type type; const struct ovsrec_interface *iface; const struct sbrec_port_binding *pb; - struct ovs_list children; }; @@ -94,6 +93,21 @@ local_binding_find(struct shash *local_bindings, const char *name) return shash_find_data(local_bindings, name); } +/* Represents a tracked binding logical port. */ +struct tracked_binding_lport { + const struct sbrec_port_binding *pb; + struct ovs_list list_node; + bool deleted; +}; + +/* Represent a tracked binding datapath. */ +struct tracked_binding_datapath { + struct hmap_node node; + const struct sbrec_datapath_binding *dp; + bool is_new; + struct ovs_list lports_head; /* List of struct tracked_binding_lport. */ +}; + const struct ovsrec_bridge *get_bridge(const struct ovsrec_bridge_table *, const char *br_name); diff --git a/tests/ovn-performance.at b/tests/ovn-performance.at index 5cc1960b6..6df42ce3a 100644 --- a/tests/ovn-performance.at +++ b/tests/ovn-performance.at @@ -274,7 +274,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] ) @@ -282,15 +282,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] ) @@ -416,15 +416,15 @@ OVN_CONTROLLER_EXPECT_NO_HIT( [ovn-nbctl --wait=hv destroy Port_Group pg1] ) -for i in 1 2; do - ls=ls$i +OVN_CONTROLLER_EXPECT_HIT( + [hv1 hv2], [lflow_run], + [ovn-nbctl --wait=hv ls-del ls1] +) - # Delete switch $ls - OVN_CONTROLLER_EXPECT_HIT( - [hv1 hv2], [lflow_run], - [ovn-nbctl --wait=hv ls-del $ls] - ) -done +OVN_CONTROLLER_EXPECT_NO_HIT( + [hv1 hv2], [lflow_run], + [ovn-nbctl --wait=hv ls-del ls2] +) # Delete router lr1 OVN_CONTROLLER_EXPECT_NO_HIT( From patchwork Thu Apr 30 17:00:27 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Numan Siddique X-Patchwork-Id: 1280681 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 49ChW55yTpz9sSM for ; Fri, 1 May 2020 03:01:53 +1000 (AEST) Received: from localhost (localhost [127.0.0.1]) by silver.osuosl.org (Postfix) with ESMTP id 2E48925173; Thu, 30 Apr 2020 17:01:52 +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 JsksfvGo3IBS; Thu, 30 Apr 2020 17:01:39 +0000 (UTC) Received: from lists.linuxfoundation.org (lf-lists.osuosl.org [140.211.9.56]) by silver.osuosl.org (Postfix) with ESMTP id CB32B255FC; Thu, 30 Apr 2020 17:00:46 +0000 (UTC) Received: from lf-lists.osuosl.org (localhost [127.0.0.1]) by lists.linuxfoundation.org (Postfix) with ESMTP id B0097C088F; Thu, 30 Apr 2020 17:00:46 +0000 (UTC) X-Original-To: dev@openvswitch.org Delivered-To: ovs-dev@lists.linuxfoundation.org Received: from fraxinus.osuosl.org (smtp4.osuosl.org [140.211.166.137]) by lists.linuxfoundation.org (Postfix) with ESMTP id 63265C088D for ; Thu, 30 Apr 2020 17:00:45 +0000 (UTC) Received: from localhost (localhost [127.0.0.1]) by fraxinus.osuosl.org (Postfix) with ESMTP id 4D3C786EFE for ; Thu, 30 Apr 2020 17:00: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 b7fq_rqAYagx for ; Thu, 30 Apr 2020 17:00:43 +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 fraxinus.osuosl.org (Postfix) with ESMTPS id 4FBFE86EC4 for ; Thu, 30 Apr 2020 17:00:43 +0000 (UTC) X-Originating-IP: 125.99.245.47 Received: from nummac.local (unknown [125.99.245.47]) (Authenticated sender: numans@ovn.org) by relay1-d.mail.gandi.net (Postfix) with ESMTPSA id A4933240005; Thu, 30 Apr 2020 17:00:40 +0000 (UTC) From: numans@ovn.org To: dev@openvswitch.org Date: Thu, 30 Apr 2020 22:30:27 +0530 Message-Id: <20200430170027.1975845-1-numans@ovn.org> X-Mailer: git-send-email 2.25.4 In-Reply-To: <20200430165741.1975097-1-numans@ovn.org> References: <20200430165741.1975097-1-numans@ovn.org> MIME-Version: 1.0 Cc: Venkata Anil Subject: [ovs-dev] [PATCH v4 09/10] I-P: Use the tracked runtime data changes for flow calculation. 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: Venkata Anil This patch processes the logical flows of tracked datapaths and tracked logical ports. To handle the tracked logical port changes, reference of logical flows to port bindings is maintained. Co-Authored-by: Numan Siddique Signed-off-by: Venkata Anil Signed-off-by: Numan Siddique --- controller/lflow.c | 69 ++++++++++++++++++++- controller/lflow.h | 12 +++- controller/ovn-controller.c | 116 +++++++++++++++++++----------------- controller/physical.h | 3 +- tests/ovn-performance.at | 8 +-- 5 files changed, 146 insertions(+), 62 deletions(-) diff --git a/controller/lflow.c b/controller/lflow.c index 512258cd3..45bf4aa4b 100644 --- a/controller/lflow.c +++ b/controller/lflow.c @@ -258,7 +258,7 @@ lflow_resource_destroy_lflow(struct lflow_resource_ref *lfrr, /* Adds the logical flows from the Logical_Flow table to flow tables. */ static void add_logical_flows(struct lflow_ctx_in *l_ctx_in, - struct lflow_ctx_out *l_ctx_out) + struct lflow_ctx_out *l_ctx_out) { const struct sbrec_logical_flow *lflow; @@ -649,6 +649,8 @@ consider_logical_flow(const struct sbrec_logical_flow *lflow, int64_t dp_id = lflow->logical_datapath->tunnel_key; char buf[16]; snprintf(buf, sizeof(buf), "%"PRId64"_%"PRId64, dp_id, port_id); + lflow_resource_add(l_ctx_out->lfrr, REF_TYPE_PORTBINDING, buf, + &lflow->header_.uuid); if (!sset_contains(l_ctx_in->local_lport_ids, buf)) { VLOG_DBG("lflow "UUID_FMT " port %s in match is not local, skip", @@ -860,3 +862,68 @@ lflow_evaluate_pb_changes(const struct sbrec_port_binding_table *pb_table) return true; } + +bool +lflow_add_flows_for_datapath(const struct sbrec_datapath_binding *dp, + struct lflow_ctx_in *l_ctx_in, + struct lflow_ctx_out *l_ctx_out) +{ + bool handled = true; + struct hmap dhcp_opts = HMAP_INITIALIZER(&dhcp_opts); + struct hmap dhcpv6_opts = HMAP_INITIALIZER(&dhcpv6_opts); + const struct sbrec_dhcp_options *dhcp_opt_row; + SBREC_DHCP_OPTIONS_TABLE_FOR_EACH (dhcp_opt_row, + l_ctx_in->dhcp_options_table) { + dhcp_opt_add(&dhcp_opts, dhcp_opt_row->name, dhcp_opt_row->code, + dhcp_opt_row->type); + } + + + const struct sbrec_dhcpv6_options *dhcpv6_opt_row; + SBREC_DHCPV6_OPTIONS_TABLE_FOR_EACH (dhcpv6_opt_row, + l_ctx_in->dhcpv6_options_table) { + dhcp_opt_add(&dhcpv6_opts, dhcpv6_opt_row->name, dhcpv6_opt_row->code, + dhcpv6_opt_row->type); + } + + struct hmap nd_ra_opts = HMAP_INITIALIZER(&nd_ra_opts); + nd_ra_opts_init(&nd_ra_opts); + + struct controller_event_options controller_event_opts; + controller_event_opts_init(&controller_event_opts); + + struct sbrec_logical_flow *lf_row = sbrec_logical_flow_index_init_row( + l_ctx_in->sbrec_logical_flow_by_logical_datapath); + sbrec_logical_flow_index_set_logical_datapath(lf_row, dp); + + const struct sbrec_logical_flow *lflow; + SBREC_LOGICAL_FLOW_FOR_EACH_EQUAL ( + lflow, lf_row, l_ctx_in->sbrec_logical_flow_by_logical_datapath) { + if (!consider_logical_flow(lflow, &dhcp_opts, &dhcpv6_opts, + &nd_ra_opts, &controller_event_opts, + l_ctx_in, l_ctx_out)) { + handled = false; + break; + } + } + + dhcp_opts_destroy(&dhcp_opts); + dhcp_opts_destroy(&dhcpv6_opts); + nd_ra_opts_destroy(&nd_ra_opts); + controller_event_opts_destroy(&controller_event_opts); + return handled; +} + +bool +lflow_handle_flows_for_lport(const struct sbrec_port_binding *pb, + struct lflow_ctx_in *l_ctx_in, + struct lflow_ctx_out *l_ctx_out) +{ + int64_t dp_id = pb->datapath->tunnel_key; + char pb_ref_name[16]; + snprintf(pb_ref_name, sizeof(pb_ref_name), "%"PRId64"_%"PRId64, + dp_id, pb->tunnel_key); + bool changed = true; + return lflow_handle_changed_ref(REF_TYPE_PORTBINDING, pb_ref_name, + l_ctx_in, l_ctx_out, &changed); +} diff --git a/controller/lflow.h b/controller/lflow.h index fa54d4de2..53ecc45b4 100644 --- a/controller/lflow.h +++ b/controller/lflow.h @@ -49,6 +49,8 @@ struct sbrec_dhcpv6_options_table; struct sbrec_logical_flow_table; struct sbrec_mac_binding_table; struct sbrec_port_binding_table; +struct sbrec_datapath_binding; +struct sbrec_port_binding; struct simap; struct sset; struct uuid; @@ -73,7 +75,8 @@ struct uuid; enum ref_type { REF_TYPE_ADDRSET, - REF_TYPE_PORTGROUP + REF_TYPE_PORTGROUP, + REF_TYPE_PORTBINDING }; /* Maintains the relationship for a pair of named resource and @@ -118,6 +121,7 @@ void lflow_resource_clear(struct lflow_resource_ref *); struct lflow_ctx_in { struct ovsdb_idl_index *sbrec_multicast_group_by_name_datapath; + struct ovsdb_idl_index *sbrec_logical_flow_by_logical_datapath; struct ovsdb_idl_index *sbrec_port_binding_by_name; const struct sbrec_dhcp_options_table *dhcp_options_table; const struct sbrec_dhcpv6_options_table *dhcpv6_options_table; @@ -159,4 +163,10 @@ void lflow_destroy(void); void lflow_expr_destroy(struct hmap *lflow_expr_cache); +bool lflow_add_flows_for_datapath(const struct sbrec_datapath_binding *, + struct lflow_ctx_in *, + struct lflow_ctx_out *); +bool lflow_handle_flows_for_lport(const struct sbrec_port_binding *, + struct lflow_ctx_in *, + struct lflow_ctx_out *); #endif /* controller/lflow.h */ diff --git a/controller/ovn-controller.c b/controller/ovn-controller.c index 94ad79295..58ede3a54 100644 --- a/controller/ovn-controller.c +++ b/controller/ovn-controller.c @@ -1438,6 +1438,11 @@ static void init_lflow_ctx(struct engine_node *node, engine_get_input("SB_port_binding", node), "name"); + struct ovsdb_idl_index *sbrec_logical_flow_by_dp = + engine_ovsdb_node_get_index( + engine_get_input("SB_logical_flow", node), + "logical_datapath"); + struct ovsdb_idl_index *sbrec_mc_group_by_name_dp = engine_ovsdb_node_get_index( engine_get_input("SB_multicast_group", node), @@ -1485,6 +1490,8 @@ static void init_lflow_ctx(struct engine_node *node, l_ctx_in->sbrec_multicast_group_by_name_datapath = sbrec_mc_group_by_name_dp; + l_ctx_in->sbrec_logical_flow_by_logical_datapath = + sbrec_logical_flow_by_dp; l_ctx_in->sbrec_port_binding_by_name = sbrec_port_binding_by_name; l_ctx_in->dhcp_options_table = dhcp_table; l_ctx_in->dhcpv6_options_table = dhcpv6_table; @@ -1639,7 +1646,8 @@ flow_output_sb_mac_binding_handler(struct engine_node *node, void *data) } static bool -flow_output_sb_port_binding_handler(struct engine_node *node, void *data) +flow_output_sb_port_binding_handler(struct engine_node *node, + void *data) { struct ed_type_runtime_data *rt_data = engine_get_input_data("runtime_data", node); @@ -1647,54 +1655,9 @@ flow_output_sb_port_binding_handler(struct engine_node *node, void *data) struct ed_type_flow_output *fo = data; struct ovn_desired_flow_table *flow_table = &fo->flow_table; - /* XXX: now we handle port-binding changes for physical flow processing - * only, but port-binding change can have impact to logical flow - * processing, too, in below circumstances: - * - * - When a port-binding for a lport is inserted/deleted but the lflow - * using that lport doesn't change. - * - * This can happen only when the lport name is used by ACL match - * condition, which is specified by user. Even in that case, if the port - * is actually bound on the current chassis it will trigger recompute on - * that chassis since ovs interface would be updated. So the only - * situation this would have real impact is when user defines an ACL - * that includes lport that is not on current chassis, and there is a - * port-binding creation/deletion related to that lport.e.g.: an ACL is - * defined: - * - * to-lport 1000 'outport=="A" && inport=="B"' allow-related - * - * If "A" is on current chassis, but "B" is lport that hasn't been - * created yet. When a lport "B" is created and bound on another - * chassis, the ACL will not take effect on the current chassis until a - * recompute is triggered later. This case doesn't seem to be a problem - * for real world use cases because usually lport is created before - * being referenced by name in ACLs. - * - * - When is_chassis_resident() is used in lflow. In this case the - * port binding is not a regular VIF. It can be either "patch" or - * "external", with ha-chassis-group assigned. In current - * "runtime_data" handling, port-binding changes for these types always - * trigger recomputing. So it is fine even if we do not handle it here. - * (due to the ovsdb tracking support for referenced table changes, - * ha-chassis-group changes will appear as port-binding change). - * - * - When a mac-binding doesn't change but the port-binding related to - * that mac-binding is deleted. In this case the neighbor flow generated - * for the mac-binding should be deleted. This would not cause any real - * issue for now, since the port-binding related to mac-binding is - * 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 - * tricky scenarios. One approach is to maintain a mapping between lport - * names and the lflows that uses them, and reprocess the related lflows - * when related port-bindings change. + /* We handle port-binding changes for physical flow processing + * only. flow_output runtime data handler takes care of processing + * logical flows for any port binding changes. */ struct physical_ctx p_ctx; init_physical_ctx(node, rt_data, &p_ctx); @@ -1794,6 +1757,8 @@ _flow_output_resource_ref_handler(struct engine_node *node, void *data, updated = &pg_data->updated; deleted = &pg_data->deleted; break; + + case REF_TYPE_PORTBINDING: default: OVS_NOT_REACHED(); } @@ -1917,16 +1882,52 @@ flow_output_runtime_data_handler(struct engine_node *node, return false; } - if (!hmap_is_empty(&tracked_data->tracked_dp_bindings)) { - /* We are not yet handling the tracked datapath binding - * changes. Return false to trigger full recompute. */ - return false; + if (hmap_is_empty(&tracked_data->tracked_dp_bindings)) { + if (tracked_data->local_lports_changed) { + engine_set_node_state(node, EN_UPDATED); + } + return true; } - if (tracked_data->local_lports_changed) { + struct lflow_ctx_in l_ctx_in; + struct lflow_ctx_out l_ctx_out; + struct ed_type_flow_output *fo = data; + struct ed_type_runtime_data *rt_data = + engine_get_input_data("runtime_data", node); + init_lflow_ctx(node, rt_data, fo, &l_ctx_in, &l_ctx_out); + + struct physical_ctx p_ctx; + init_physical_ctx(node, rt_data, &p_ctx); + + bool handled = true; + struct tracked_binding_datapath *tdp; + HMAP_FOR_EACH (tdp, node, &tracked_data->tracked_dp_bindings) { + if (tdp->is_new) { + handled = lflow_add_flows_for_datapath(tdp->dp, &l_ctx_in, + &l_ctx_out); + if (!handled) { + break; + } + } else { + struct tracked_binding_lport *lport; + LIST_FOR_EACH (lport, list_node, &tdp->lports_head) { + if (!lflow_handle_flows_for_lport(lport->pb, &l_ctx_in, + &l_ctx_out)) { + handled = false; + break; + } + } + if (!handled) { + break; + } + } + } + + if (handled) { engine_set_node_state(node, EN_UPDATED); } - return true; + + return handled; } struct ovn_controller_exit_args { @@ -1983,6 +1984,9 @@ main(int argc, char *argv[]) = chassis_index_create(ovnsb_idl_loop.idl); struct ovsdb_idl_index *sbrec_multicast_group_by_name_datapath = mcast_group_index_create(ovnsb_idl_loop.idl); + struct ovsdb_idl_index *sbrec_logical_flow_by_logical_datapath + = ovsdb_idl_index_create1(ovnsb_idl_loop.idl, + &sbrec_logical_flow_col_logical_datapath); struct ovsdb_idl_index *sbrec_port_binding_by_name = ovsdb_idl_index_create1(ovnsb_idl_loop.idl, &sbrec_port_binding_col_logical_port); @@ -2132,6 +2136,8 @@ main(int argc, char *argv[]) engine_ovsdb_node_add_index(&en_sb_chassis, "name", sbrec_chassis_by_name); engine_ovsdb_node_add_index(&en_sb_multicast_group, "name_datapath", sbrec_multicast_group_by_name_datapath); + engine_ovsdb_node_add_index(&en_sb_logical_flow, "logical_datapath", + sbrec_logical_flow_by_logical_datapath); engine_ovsdb_node_add_index(&en_sb_port_binding, "name", sbrec_port_binding_by_name); engine_ovsdb_node_add_index(&en_sb_port_binding, "key", diff --git a/controller/physical.h b/controller/physical.h index 9ca34436a..481f03901 100644 --- a/controller/physical.h +++ b/controller/physical.h @@ -33,6 +33,7 @@ struct ovsrec_bridge; struct simap; struct sbrec_multicast_group_table; struct sbrec_port_binding_table; +struct sbrec_port_binding; struct sset; /* OVN Geneve option information. @@ -61,7 +62,7 @@ struct physical_ctx { void physical_register_ovs_idl(struct ovsdb_idl *); void physical_run(struct physical_ctx *, struct ovn_desired_flow_table *); -void physical_handle_port_binding_changes(struct physical_ctx *, +void physical_handle_port_binding_changes(struct physical_ctx *p_ctx, struct ovn_desired_flow_table *); void physical_handle_mc_group_changes(struct physical_ctx *, struct ovn_desired_flow_table *); diff --git a/tests/ovn-performance.at b/tests/ovn-performance.at index 6df42ce3a..a12757e18 100644 --- a/tests/ovn-performance.at +++ b/tests/ovn-performance.at @@ -353,8 +353,8 @@ for i in 1 2; do ) # Bind port $lp and wait for it to come up - OVN_CONTROLLER_EXPECT_HIT_COND( - [hv$i hv$j], [lflow_run], [>0 =0], + OVN_CONTROLLER_EXPECT_NO_HIT( + [hv$i hv$j], [lflow_run], [as hv$i ovs-vsctl add-port br-int $vif -- set Interface $vif external-ids:iface-id=$lp && ovn-nbctl wait-until Logical_Switch_Port $lp 'up=true' && ovn-nbctl --wait=hv sync] @@ -404,8 +404,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 From patchwork Thu Apr 30 17:00:42 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Numan Siddique X-Patchwork-Id: 1280684 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 49ChWB3mQ3z9sSM for ; Fri, 1 May 2020 03:01:58 +1000 (AEST) Received: from localhost (localhost [127.0.0.1]) by fraxinus.osuosl.org (Postfix) with ESMTP id D74AA86E1C; Thu, 30 Apr 2020 17:01:56 +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 pbQ1WH5hT-DD; Thu, 30 Apr 2020 17:01:56 +0000 (UTC) Received: from lists.linuxfoundation.org (lf-lists.osuosl.org [140.211.9.56]) by fraxinus.osuosl.org (Postfix) with ESMTP id 09A5E86DDA; Thu, 30 Apr 2020 17:01:56 +0000 (UTC) Received: from lf-lists.osuosl.org (localhost [127.0.0.1]) by lists.linuxfoundation.org (Postfix) with ESMTP id E13ADC016F; Thu, 30 Apr 2020 17:01:55 +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 A26D9C016F for ; Thu, 30 Apr 2020 17:01:54 +0000 (UTC) Received: from localhost (localhost [127.0.0.1]) by silver.osuosl.org (Postfix) with ESMTP id 7200E251E1 for ; Thu, 30 Apr 2020 17:01:54 +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 8T0lP9gkLEoP for ; Thu, 30 Apr 2020 17:01:42 +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 silver.osuosl.org (Postfix) with ESMTPS id 8798524F0B for ; Thu, 30 Apr 2020 17:00:50 +0000 (UTC) X-Originating-IP: 125.99.245.47 Received: from nummac.local (unknown [125.99.245.47]) (Authenticated sender: numans@ovn.org) by relay1-d.mail.gandi.net (Postfix) with ESMTPSA id 59299240009; Thu, 30 Apr 2020 17:00:48 +0000 (UTC) From: numans@ovn.org To: dev@openvswitch.org Date: Thu, 30 Apr 2020 22:30:42 +0530 Message-Id: <20200430170042.1975904-1-numans@ovn.org> X-Mailer: git-send-email 2.25.4 In-Reply-To: <20200430165741.1975097-1-numans@ovn.org> References: <20200430165741.1975097-1-numans@ovn.org> MIME-Version: 1.0 Subject: [ovs-dev] [PATCH v4 10/10] I-P: Handle sbrec_chassis changes 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 When ovn-controller updates the nb_cfg column of its chassis, this results in full recomputation on all the nodes. This results in wastage of CPU cycles. To address this, this patch handles sbrec_chassis changes incrementally. We don't need to handle any sbrec_chassis changes during runtime_data stage, because before engine_run() is called, encaps_run() is called which will handle any chassis/encap changes. For new chassis addition and deletion, we need to add/delete flows for the tunnel ports created/deleted. So physical_run() is called for this. For any chassis updates, we can ignore this for flow computation. This patch handles all these. Signed-off-by: Numan Siddique --- controller/ovn-controller.c | 53 ++++++++++++++++++++++++++++++------- tests/ovn.at | 21 +++++++++++++-- 2 files changed, 62 insertions(+), 12 deletions(-) diff --git a/controller/ovn-controller.c b/controller/ovn-controller.c index 58ede3a54..8c5ac2d8e 100644 --- a/controller/ovn-controller.c +++ b/controller/ovn-controller.c @@ -1379,7 +1379,8 @@ static void init_physical_ctx(struct engine_node *node, struct sbrec_chassis_table *chassis_table = (struct sbrec_chassis_table *)EN_OVSDB_GET( - engine_get_input("SB_chassis", node)); + engine_get_input("SB_chassis", + engine_get_input("physical_flow_changes", node))); struct ed_type_mff_ovn_geneve *ed_mff_ovn_geneve = engine_get_input_data("mff_ovn_geneve", node); @@ -1395,7 +1396,8 @@ static void init_physical_ctx(struct engine_node *node, const struct sbrec_chassis *chassis = NULL; struct ovsdb_idl_index *sbrec_chassis_by_name = engine_ovsdb_node_get_index( - engine_get_input("SB_chassis", node), + engine_get_input("SB_chassis", + engine_get_input("physical_flow_changes", node)), "name"); if (chassis_id) { chassis = chassis_lookup_by_name(sbrec_chassis_by_name, chassis_id); @@ -1472,7 +1474,8 @@ static void init_lflow_ctx(struct engine_node *node, const struct sbrec_chassis *chassis = NULL; struct ovsdb_idl_index *sbrec_chassis_by_name = engine_ovsdb_node_get_index( - engine_get_input("SB_chassis", node), + engine_get_input("SB_chassis", + engine_get_input("physical_flow_changes", node)), "name"); if (chassis_id) { chassis = chassis_lookup_by_name(sbrec_chassis_by_name, chassis_id); @@ -1553,8 +1556,9 @@ en_flow_output_run(struct engine_node *node, void *data) struct ovsdb_idl_index *sbrec_chassis_by_name = engine_ovsdb_node_get_index( - engine_get_input("SB_chassis", node), - "name"); + engine_get_input("SB_chassis", + engine_get_input("physical_flow_changes", node)), + "name"); const struct sbrec_chassis *chassis = NULL; if (chassis_id) { @@ -1719,8 +1723,9 @@ _flow_output_resource_ref_handler(struct engine_node *node, void *data, struct ovsdb_idl_index *sbrec_chassis_by_name = engine_ovsdb_node_get_index( - engine_get_input("SB_chassis", node), - "name"); + engine_get_input("SB_chassis", + engine_get_input("physical_flow_changes", node)), + "name"); const struct sbrec_chassis *chassis = NULL; if (chassis_id) { chassis = chassis_lookup_by_name(sbrec_chassis_by_name, chassis_id); @@ -1871,6 +1876,31 @@ physical_flow_changes_ovs_iface_handler(struct engine_node *node OVS_UNUSED, return true; } +/* Handles sbrec_chassis changes. + * If a new chassis is added or removed return false, so that + * physical flows are programmed. + * For any updates, there is no need for any flow computation. + * Encap changes will also result in sbrec_chassis changes, + * but we handle encap changes separately. + */ +static bool +physical_flow_changes_sb_chassis_handler(struct engine_node *node OVS_UNUSED, + void *data OVS_UNUSED) +{ + struct sbrec_chassis_table *chassis_table = + (struct sbrec_chassis_table *)EN_OVSDB_GET( + engine_get_input("SB_chassis", node)); + + const struct sbrec_chassis *ch; + SBREC_CHASSIS_TABLE_FOR_EACH_TRACKED (ch, chassis_table) { + if (sbrec_chassis_is_deleted(ch) || sbrec_chassis_is_new(ch)) { + return false; + } + } + + return true; +} + static bool flow_output_runtime_data_handler(struct engine_node *node, void *data OVS_UNUSED) @@ -2079,6 +2109,10 @@ main(int argc, char *argv[]) NULL); engine_add_input(&en_physical_flow_changes, &en_ovs_interface, physical_flow_changes_ovs_iface_handler); + engine_add_input(&en_physical_flow_changes, &en_sb_chassis, + physical_flow_changes_sb_chassis_handler); + engine_add_input(&en_physical_flow_changes, &en_sb_encap, + NULL); engine_add_input(&en_flow_output, &en_addr_sets, flow_output_addr_sets_handler); @@ -2093,8 +2127,6 @@ main(int argc, char *argv[]) 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_sb_chassis, NULL); - engine_add_input(&en_flow_output, &en_sb_encap, NULL); engine_add_input(&en_flow_output, &en_sb_multicast_group, flow_output_sb_multicast_group_handler); engine_add_input(&en_flow_output, &en_sb_port_binding, @@ -2121,7 +2153,8 @@ main(int argc, char *argv[]) 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); + engine_add_input(&en_runtime_data, &en_sb_chassis, + runtime_data_noop_handler); 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, diff --git a/tests/ovn.at b/tests/ovn.at index c04cd06d2..08ba219fd 100644 --- a/tests/ovn.at +++ b/tests/ovn.at @@ -11081,6 +11081,12 @@ only_broadcast_from_lrp1() { garp="fffffffffffff0000000000108060001080006040001f00000000001c0a80064000000000000c0a80064" echo $garp > expout +OVS_WAIT_UNTIL( + [$PYTHON "$ovs_srcdir/utilities/ovs-pcap.in" hv1/snoopvif-tx.pcap > rcv_text + exp_rcvd=$(cat rcv_text | grep $garp | wc -l) + echo "expected received = $exp_rcvd" + test $exp_rcvd -ge 1]) + $PYTHON "$ovs_srcdir/utilities/ovs-pcap.in" hv1/snoopvif-tx.pcap | trim_zeros | only_broadcast_from_lrp1 | uniq > hv1_snoop_tx echo "packets on hv1-snoopvif:" cat hv1_snoop_tx @@ -11109,12 +11115,17 @@ as hv1 reset_pcap_file snoopvif hv1/snoopvif as hv2 reset_pcap_file br-phys_n1 hv2/br-phys_n1 as hv3 reset_pcap_file br-phys_n1 hv3/br-phys_n1 -# Wait for packets to be received. -OVS_WAIT_UNTIL([test `wc -c < "hv1/snoopvif-tx.pcap"` -ge 100]) trim_zeros() { sed 's/\(00\)\{1,\}$//' } +# Wait for packets to be received. +OVS_WAIT_UNTIL( + [$PYTHON "$ovs_srcdir/utilities/ovs-pcap.in" hv1/snoopvif-tx.pcap > rcv_text + exp_rcvd=$(cat rcv_text | grep $garp | wc -l) + echo "expected received = $exp_rcvd" + test $exp_rcvd -ge 1]) + $PYTHON "$ovs_srcdir/utilities/ovs-pcap.in" hv1/snoopvif-tx.pcap | trim_zeros | only_broadcast_from_lrp1 | uniq > hv1_snoopvif_tx AT_CHECK([sort hv1_snoopvif_tx], [0], [expout]) $PYTHON "$ovs_srcdir/utilities/ovs-pcap.in" hv3/br-phys_n1-tx.pcap | trim_zeros | only_broadcast_from_lrp1 | uniq > hv3_br_phys_tx @@ -11160,6 +11171,12 @@ trim_zeros() { garp="fffffffffffff00000000001810007de08060001080006040001f00000000001c0a80064000000000000c0a80064" echo $garp > expout +OVS_WAIT_UNTIL( + [$PYTHON "$ovs_srcdir/utilities/ovs-pcap.in" hv1/snoopvif-tx.pcap > rcv_text + exp_rcvd=$(cat rcv_text | grep $garp | wc -l) + echo "expected received = $exp_rcvd" + test $exp_rcvd -ge 1]) + $PYTHON "$ovs_srcdir/utilities/ovs-pcap.in" hv1/snoopvif-tx.pcap | trim_zeros | only_broadcast_from_lrp1 | uniq > hv1_snoopvif_tx AT_CHECK([sort hv1_snoopvif_tx], [0], [expout]) $PYTHON "$ovs_srcdir/utilities/ovs-pcap.in" hv3/br-phys_n1-tx.pcap | trim_zeros | only_broadcast_from_lrp1 | uniq > hv3_br_phys_tx