From patchwork Thu May 28 11:04:06 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Numan Siddique X-Patchwork-Id: 1299655 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 49XlFy4CX4z9sRY for ; Thu, 28 May 2020 21:04:38 +1000 (AEST) Received: from localhost (localhost [127.0.0.1]) by whitealder.osuosl.org (Postfix) with ESMTP id 655B880CF6; Thu, 28 May 2020 11:04:36 +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 WC-9YSJp825f; Thu, 28 May 2020 11:04:27 +0000 (UTC) Received: from lists.linuxfoundation.org (lf-lists.osuosl.org [140.211.9.56]) by whitealder.osuosl.org (Postfix) with ESMTP id DD62881388; Thu, 28 May 2020 11:04:27 +0000 (UTC) Received: from lf-lists.osuosl.org (localhost [127.0.0.1]) by lists.linuxfoundation.org (Postfix) with ESMTP id C1FDAC0890; Thu, 28 May 2020 11:04:27 +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 D85ADC016F for ; Thu, 28 May 2020 11:04:25 +0000 (UTC) Received: from localhost (localhost [127.0.0.1]) by whitealder.osuosl.org (Postfix) with ESMTP id BEE2C80CF6 for ; Thu, 28 May 2020 11:04:25 +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 8X7ujGrATGHI for ; Thu, 28 May 2020 11:04:19 +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 whitealder.osuosl.org (Postfix) with ESMTPS id CBFEE82F92 for ; Thu, 28 May 2020 11:04:18 +0000 (UTC) X-Originating-IP: 27.7.91.57 Received: from nusiddiq.home.org.com (unknown [27.7.91.57]) (Authenticated sender: numans@ovn.org) by relay7-d.mail.gandi.net (Postfix) with ESMTPSA id 22F6E20002; Thu, 28 May 2020 11:04:13 +0000 (UTC) From: numans@ovn.org To: dev@openvswitch.org Date: Thu, 28 May 2020 16:34:06 +0530 Message-Id: <20200528110406.1057952-1-numans@ovn.org> X-Mailer: git-send-email 2.26.2 In-Reply-To: <20200528110344.1057873-1-numans@ovn.org> References: <20200528110344.1057873-1-numans@ovn.org> MIME-Version: 1.0 Subject: [ovs-dev] [PATCH ovn v9 1/8] 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. This patch also refactors the port binding code by adding a port binding function for each port binding type. Acked-by: Mark Michelson Signed-off-by: Numan Siddique --- controller/binding.c | 1020 ++++++++++++++++++++++++----------- controller/binding.h | 14 +- controller/ovn-controller.c | 48 +- tests/ovn.at | 36 ++ 4 files changed, 756 insertions(+), 362 deletions(-) diff --git a/controller/binding.c b/controller/binding.c index 79dc046d9..6a13d1a0e 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, @@ -403,13 +362,12 @@ destroy_qos_map(struct hmap *qos_map) static void update_local_lport_ids(struct sset *local_lport_ids, - const struct sbrec_port_binding *binding_rec) + const struct sbrec_port_binding *pb) { - char buf[16]; - snprintf(buf, sizeof(buf), "%"PRId64"_%"PRId64, - binding_rec->datapath->tunnel_key, - binding_rec->tunnel_key); - sset_add(local_lport_ids, buf); + char buf[16]; + snprintf(buf, sizeof(buf), "%"PRId64"_%"PRId64, + pb->datapath->tunnel_key, pb->tunnel_key); + sset_add(local_lport_ids, buf); } /* @@ -448,219 +406,6 @@ sbrec_get_port_encap(const struct sbrec_chassis *chassis_rec, return best_encap; } -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))) { - /* 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")) { - const char *chassis_id = smap_get(&binding_rec->options, - "l2gateway-chassis"); - our_chassis = chassis_id && !strcmp(chassis_id, chassis_rec->name); - } else if (!strcmp(binding_rec->type, "chassisredirect") || - !strcmp(binding_rec->type, "external")) { - our_chassis = ha_chassis_group_contains(binding_rec->ha_chassis_group, - chassis_rec) && - ha_chassis_group_is_active(binding_rec->ha_chassis_group, - active_tunnels, chassis_rec); - } else if (!strcmp(binding_rec->type, "l3gateway")) { - const char *chassis_id = smap_get(&binding_rec->options, - "l3gateway-chassis"); - our_chassis = chassis_id && !strcmp(chassis_id, chassis_rec->name); - } - - 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, - const struct ovsrec_interface *iface_rec, - struct binding_ctx_in *b_ctx_in, - struct binding_ctx_out *b_ctx_out, - struct hmap *qos_map, - 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, @@ -736,6 +481,585 @@ consider_localnet_port(const struct sbrec_port_binding *binding_rec, ld->localnet_port = binding_rec; } +/* Local bindings. binding.c module binds the logical port (represented by + * Port_Binding rows) and sets the 'chassis' column when it sees the + * OVS interface row (of type "" or "internal") with the + * external_ids:iface-id= set. + * + * This module also manages the other port_bindings. + * + * To better manage the local bindings with the associated OVS interfaces, + * 'struct local_binding' is used. A shash of these local bindings is + * maintained with the 'external_ids:iface-id' as the key to the shash. + * + * struct local_binding has 3 main fields: + * - type + * - OVS interface row object + * - Port_Binding row object + * + * An instance of 'struct local_binding' can be one of 3 types. + * + * BT_VIF: Represent a local binding for an OVS interface of + * type "" or "internal" with the external_ids:iface-id + * set. + * + * This can be a + * * probable local binding - external_ids:iface-id is + * set, but the corresponding Port_Binding row is not + * created or is not visible to the local ovn-controller + * instance. + * + * * a local binding - external_ids:iface-id is set and + * which is already bound to the corresponding Port_Binding + * row. + * + * It maintains a list of children + * (of type BT_CONTAINER/BT_VIRTUAL) if any. + * + * BT_CONTAINER: Represents a local binding which has a parent of type + * BT_VIF. Its Port_Binding row's 'parent' column is set to + * its parent's Port_Binding. It shares the OVS interface row + * with the parent. + * + * BT_VIRTUAL: Represents a local binding which has a parent of type BT_VIF. + * Its Port_Binding type is "virtual" and it shares the OVS + * interface row with the parent. + * Port_Binding of type "virtual" is claimed by pinctrl module + * when it sees the ARP packet from the parent's VIF. + * + */ +enum local_binding_type { + BT_VIF, + BT_CONTAINER, + BT_VIRTUAL +}; + +struct local_binding { + char *name; + enum local_binding_type type; + const struct ovsrec_interface *iface; + const struct sbrec_port_binding *pb; + + /* shash of 'struct local_binding' representing children. */ + struct shash 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; + shash_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) +{ + local_bindings_destroy(&lbinding->children); + + free(lbinding->name); + free(lbinding); +} + +void +local_bindings_init(struct shash *local_bindings) +{ + shash_init(local_bindings); +} + +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; + local_binding_destroy(lbinding); + shash_delete(local_bindings, node); + } + + shash_destroy(local_bindings); +} + +static void +local_binding_add_child(struct local_binding *lbinding, + struct local_binding *child) +{ + local_binding_add(&lbinding->children, child); +} + +static struct local_binding * +local_binding_find_child(struct local_binding *lbinding, + const char *child_name) +{ + return local_binding_find(&lbinding->children, child_name); +} + +static bool +is_lport_vif(const struct sbrec_port_binding *pb) +{ + return !pb->type[0]; +} + +static bool +is_lport_container(const struct sbrec_port_binding *pb) +{ + return is_lport_vif(pb) && pb->parent_port && pb->parent_port[0]; +} + +enum en_lport_type { + LP_UNKNOWN, + LP_VIF, + LP_PATCH, + LP_L3GATEWAY, + LP_LOCALNET, + LP_LOCALPORT, + LP_L2GATEWAY, + LP_VTEP, + LP_CHASSISREDIRECT, + LP_VIRTUAL, + LP_EXTERNAL, + LP_REMOTE +}; + +static enum en_lport_type +get_lport_type(const struct sbrec_port_binding *pb) +{ + if (is_lport_vif(pb)) { + return LP_VIF; + } else if (!strcmp(pb->type, "patch")) { + return LP_PATCH; + } else if (!strcmp(pb->type, "chassisredirect")) { + return LP_CHASSISREDIRECT; + } else if (!strcmp(pb->type, "l3gateway")) { + return LP_L3GATEWAY; + } else if (!strcmp(pb->type, "localnet")) { + return LP_LOCALNET; + } else if (!strcmp(pb->type, "localport")) { + return LP_LOCALPORT; + } else if (!strcmp(pb->type, "l2gateway")) { + return LP_L2GATEWAY; + } else if (!strcmp(pb->type, "virtual")) { + return LP_VIRTUAL; + } else if (!strcmp(pb->type, "external")) { + return LP_EXTERNAL; + } else if (!strcmp(pb->type, "remote")) { + return LP_REMOTE; + } else if (!strcmp(pb->type, "vtep")) { + return LP_VTEP; + } + + return LP_UNKNOWN; +} + +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 bool +is_lbinding_set(struct local_binding *lbinding) +{ + return lbinding && lbinding->pb && lbinding->iface; +} + +static bool +is_lbinding_this_chassis(struct local_binding *lbinding, + const struct sbrec_chassis *chassis) +{ + return lbinding && lbinding->pb && lbinding->pb->chassis == chassis; +} + +static bool +can_bind_on_this_chassis(const struct sbrec_chassis *chassis_rec, + const char *requested_chassis) +{ + return !requested_chassis || !requested_chassis[0] + || !strcmp(requested_chassis, chassis_rec->name) + || !strcmp(requested_chassis, chassis_rec->hostname); +} + +static void +consider_vif_lport_(const struct sbrec_port_binding *pb, + bool can_bind, const char *vif_chassis, + struct binding_ctx_in *b_ctx_in, + struct binding_ctx_out *b_ctx_out, + struct local_binding *lbinding, + struct hmap *qos_map) +{ + bool lbinding_set = is_lbinding_set(lbinding); + if (lbinding_set) { + if (can_bind) { + /* We can claim the lport. */ + claim_lport(pb, b_ctx_in->chassis_rec, lbinding->iface); + + 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 { + /* We could, but can't claim the lport. */ + 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) { + /* Release the lport if there is no lbinding. */ + if (!lbinding_set || !can_bind) { + release_lport(pb); + } + } + +} + +static void +consider_vif_lport(const struct sbrec_port_binding *pb, + struct binding_ctx_in *b_ctx_in, + struct binding_ctx_out *b_ctx_out, + struct local_binding *lbinding, + struct hmap *qos_map) +{ + const char *vif_chassis = smap_get(&pb->options, "requested-chassis"); + bool can_bind = can_bind_on_this_chassis(b_ctx_in->chassis_rec, + vif_chassis); + + if (!lbinding) { + lbinding = local_binding_find(b_ctx_out->local_bindings, + pb->logical_port); + } + + if (lbinding) { + lbinding->pb = pb; + } + + consider_vif_lport_(pb, can_bind, vif_chassis, b_ctx_in, + b_ctx_out, lbinding, qos_map); +} + +static void +consider_container_lport(const struct sbrec_port_binding *pb, + struct binding_ctx_in *b_ctx_in, + struct binding_ctx_out *b_ctx_out, + struct hmap *qos_map) +{ + struct local_binding *parent_lbinding; + parent_lbinding = local_binding_find(b_ctx_out->local_bindings, + pb->parent_port); + + if (parent_lbinding && !parent_lbinding->pb) { + parent_lbinding->pb = lport_lookup_by_name( + b_ctx_in->sbrec_port_binding_by_name, pb->parent_port); + + if (parent_lbinding->pb) { + /* Its possible that the parent lport is not considered yet. + * So call consider_vif_lport() to process it first. */ + consider_vif_lport(parent_lbinding->pb, b_ctx_in, b_ctx_out, + parent_lbinding, qos_map); + } + } + + if (!parent_lbinding || !parent_lbinding->pb) { + /* Call release_lport, to release the container lport, if + * it was bound earlier. */ + if (pb->chassis == b_ctx_in->chassis_rec) { + release_lport(pb); + } + return; + } + + struct local_binding *container_lbinding = + local_binding_find_child(parent_lbinding, pb->logical_port); + ovs_assert(!container_lbinding); + + container_lbinding = local_binding_create(pb->logical_port, + parent_lbinding->iface, + pb, BT_CONTAINER); + local_binding_add_child(parent_lbinding, container_lbinding); + + const char *vif_chassis = smap_get(&parent_lbinding->pb->options, + "requested-chassis"); + bool can_bind = can_bind_on_this_chassis(b_ctx_in->chassis_rec, + vif_chassis); + + consider_vif_lport_(pb, can_bind, vif_chassis, b_ctx_in, b_ctx_out, + container_lbinding, qos_map); +} + +static void +consider_virtual_lport(const struct sbrec_port_binding *pb, + struct binding_ctx_in *b_ctx_in, + struct binding_ctx_out *b_ctx_out, + struct hmap *qos_map) +{ + struct local_binding * parent_lbinding = + pb->virtual_parent ? local_binding_find(b_ctx_out->local_bindings, + pb->virtual_parent) + : NULL; + + if (parent_lbinding && !parent_lbinding->pb) { + parent_lbinding->pb = lport_lookup_by_name( + b_ctx_in->sbrec_port_binding_by_name, pb->virtual_parent); + + if (parent_lbinding->pb) { + /* Its possible that the parent lport is not considered yet. + * So call consider_vif_lport() to process it first. */ + consider_vif_lport(parent_lbinding->pb, b_ctx_in, b_ctx_out, + parent_lbinding, qos_map); + } + } + + struct local_binding *virtual_lbinding = NULL; + if (is_lbinding_this_chassis(parent_lbinding, b_ctx_in->chassis_rec)) { + virtual_lbinding = + local_binding_find_child(parent_lbinding, pb->logical_port); + ovs_assert(!virtual_lbinding); + virtual_lbinding = local_binding_create(pb->logical_port, + parent_lbinding->iface, + pb, BT_VIRTUAL); + local_binding_add_child(parent_lbinding, virtual_lbinding); + } + + return consider_vif_lport_(pb, true, NULL, b_ctx_in, b_ctx_out, + virtual_lbinding, qos_map); +} + +/* Considers either claiming the lport or releasing the lport + * for non VIF lports. + */ +static void +consider_nonvif_lport_(const struct sbrec_port_binding *pb, + bool our_chassis, + bool has_local_l3gateway, + struct binding_ctx_in *b_ctx_in, + struct binding_ctx_out *b_ctx_out) +{ + ovs_assert(b_ctx_in->ovnsb_idl_txn); + 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, has_local_l3gateway, + b_ctx_out->local_datapaths); + + update_local_lport_ids(b_ctx_out->local_lport_ids, pb); + claim_lport(pb, b_ctx_in->chassis_rec, NULL); + } else if (pb->chassis == b_ctx_in->chassis_rec) { + release_lport(pb); + } +} + +static void +consider_l2gw_lport(const struct sbrec_port_binding *pb, + struct binding_ctx_in *b_ctx_in, + struct binding_ctx_out *b_ctx_out) +{ + const char *chassis_id = smap_get(&pb->options, "l2gateway-chassis"); + bool our_chassis = chassis_id && !strcmp(chassis_id, + b_ctx_in->chassis_rec->name); + + consider_nonvif_lport_(pb, our_chassis, false, b_ctx_in, b_ctx_out); +} + +static void +consider_l3gw_lport(const struct sbrec_port_binding *pb, + struct binding_ctx_in *b_ctx_in, + struct binding_ctx_out *b_ctx_out) +{ + const char *chassis_id = smap_get(&pb->options, "l3gateway-chassis"); + bool our_chassis = chassis_id && !strcmp(chassis_id, + b_ctx_in->chassis_rec->name); + + consider_nonvif_lport_(pb, our_chassis, true, b_ctx_in, b_ctx_out); +} + +static void +consider_localnet_lport(const struct sbrec_port_binding *pb, + struct binding_ctx_in *b_ctx_in, + struct binding_ctx_out *b_ctx_out, + struct hmap *qos_map) +{ + /* 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); + } + + update_local_lport_ids(b_ctx_out->local_lport_ids, pb); +} + +static void +consider_ha_lport(const struct sbrec_port_binding *pb, + struct binding_ctx_in *b_ctx_in, + struct binding_ctx_out *b_ctx_out) +{ + bool our_chassis = false; + bool is_ha_chassis = ha_chassis_group_contains(pb->ha_chassis_group, + b_ctx_in->chassis_rec); + our_chassis = is_ha_chassis && + ha_chassis_group_is_active(pb->ha_chassis_group, + b_ctx_in->active_tunnels, + b_ctx_in->chassis_rec); + + if (is_ha_chassis && !our_chassis) { + /* If the chassis_rec is part of ha_chassis_group associated with + * the port_binding 'pb', we need to add to the local_datapath + * in even if its not active. + * + * If the chassis is active, consider_nonvif_lport_() takes care + * of adding the datapath of this 'pb' to 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, + pb->datapath, false, + b_ctx_out->local_datapaths); + } + + consider_nonvif_lport_(pb, our_chassis, false, b_ctx_in, b_ctx_out); +} + +static void +consider_cr_lport(const struct sbrec_port_binding *pb, + struct binding_ctx_in *b_ctx_in, + struct binding_ctx_out *b_ctx_out) +{ + consider_ha_lport(pb, b_ctx_in, b_ctx_out); +} + +static void +consider_external_lport(const struct sbrec_port_binding *pb, + struct binding_ctx_in *b_ctx_in, + struct binding_ctx_out *b_ctx_out) +{ + return consider_ha_lport(pb, b_ctx_in, b_ctx_out); +} + +/* + * Builds local_bindings from the OVS interfaces. + */ +static void +build_local_bindings(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) { + struct local_binding *lbinding = + local_binding_find(b_ctx_out->local_bindings, iface_id); + if (!lbinding) { + lbinding = local_binding_create(iface_id, iface_rec, NULL, + BT_VIF); + local_binding_add(b_ctx_out->local_bindings, lbinding); + } else { + static struct vlog_rate_limit rl = + VLOG_RATE_LIMIT_INIT(1, 5); + VLOG_WARN_RL( + &rl, + "Invalid configuration: iface-id is configured on " + "interfaces : [%s] and [%s]. Ignoring the " + "configuration on interface [%s]", + lbinding->iface->name, iface_rec->name, + iface_rec->name); + ovs_assert(lbinding->type == BT_VIF); + } + + 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); + } + } + } + } +} + void binding_run(struct binding_ctx_in *b_ctx_in, struct binding_ctx_out *b_ctx_out) { @@ -743,106 +1067,135 @@ binding_run(struct binding_ctx_in *b_ctx_in, struct binding_ctx_out *b_ctx_out) return; } - 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(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; + + struct ovs_list localnet_lports = OVS_LIST_INITIALIZER(&localnet_lports); + + struct localnet_lport { + struct ovs_list list_node; + const struct sbrec_port_binding *pb; + }; /* 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, + const struct sbrec_port_binding *pb; + SBREC_PORT_BINDING_TABLE_FOR_EACH (pb, 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, - iface_rec, b_ctx_in, b_ctx_out, - sset_is_empty(&egress_ifaces) ? NULL : - &qos_map, - vpbs, &n_vpbs, &n_allocated_vpbs); - } - - /* 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]); + enum en_lport_type lport_type = get_lport_type(pb); + + switch (lport_type) { + case LP_PATCH: + case LP_LOCALPORT: + case LP_VTEP: + update_local_lport_ids(b_ctx_out->local_lport_ids, pb); + break; + + case LP_VIF: + if (is_lport_container(pb)) { + consider_container_lport(pb, b_ctx_in, b_ctx_out, qos_map_ptr); + } else { + consider_vif_lport(pb, b_ctx_in, b_ctx_out, NULL, qos_map_ptr); + } + break; + + case LP_VIRTUAL: + consider_virtual_lport(pb, b_ctx_in, b_ctx_out, qos_map_ptr); + break; + + case LP_L2GATEWAY: + consider_l2gw_lport(pb, b_ctx_in, b_ctx_out); + break; + + case LP_L3GATEWAY: + consider_l3gw_lport(pb, b_ctx_in, b_ctx_out); + break; + + case LP_CHASSISREDIRECT: + consider_cr_lport(pb, b_ctx_in, b_ctx_out); + break; + + case LP_EXTERNAL: + consider_external_lport(pb, b_ctx_in, b_ctx_out); + break; + + case LP_LOCALNET: { + consider_localnet_lport(pb, b_ctx_in, b_ctx_out, qos_map_ptr); + struct localnet_lport *lnet_lport = xmalloc(sizeof *lnet_lport); + lnet_lport->pb = pb; + ovs_list_push_back(&localnet_lports, &lnet_lport->list_node); + break; + } + + case LP_REMOTE: + /* Nothing to be done for REMOTE type. */ + break; + + case LP_UNKNOWN: { + static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 5); + VLOG_WARN_RL(&rl, + "Unknown port binding type [%s] for port binding " + "[%s]. Does ovn-controller needs update ?", + pb->type, pb->logical_port); + break; + } + } } - free(vpbs); 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 + /* Run through each localnet lport list 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, - 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); - } + struct localnet_lport *lnet_lport; + LIST_FOR_EACH_POP (lnet_lport, list_node, &localnet_lports) { + consider_localnet_port(lnet_lport->pb, &bridge_mappings, + b_ctx_out->egress_ifaces, + b_ctx_out->local_datapaths); + free(lnet_lport); } + 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); destroy_qos_map(&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 @@ -854,24 +1207,35 @@ 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, "remote")) { + continue; + } + + 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..9affc9a96 100644 --- a/controller/binding.h +++ b/controller/binding.h @@ -31,6 +31,8 @@ struct ovsrec_open_vswitch_table; struct sbrec_chassis; struct sbrec_port_binding_table; struct sset; +struct sbrec_port_binding; +struct shash; struct binding_ctx_in { struct ovsdb_idl_txn *ovnsb_idl_txn; @@ -51,8 +53,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 +64,9 @@ 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_init(struct shash *local_bindings); +void local_bindings_destroy(struct shash *local_bindings); #endif /* controller/binding.h */ diff --git a/controller/ovn-controller.c b/controller/ovn-controller.c index 794d4aa2f..386a64059 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_binding" 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); + local_bindings_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_destroy(&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 @@ -1095,12 +1106,16 @@ en_runtime_data_run(struct engine_node *node, void *data) free(cur_node); } hmap_clear(local_datapaths); + local_bindings_destroy(&rt_data->local_bindings); 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); + local_bindings_init(&rt_data->local_bindings); } struct binding_ctx_in b_ctx_in; @@ -1129,35 +1144,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/tests/ovn.at b/tests/ovn.at index 15b40ca1e..8743e0efb 100644 --- a/tests/ovn.at +++ b/tests/ovn.at @@ -8606,6 +8606,42 @@ as hv1 ovs-appctl netdev-dummy/receive vm1 $packet # expected packet at VM1 OVN_CHECK_PACKETS([hv1/vm1-tx.pcap], [expected1]) +# Test binding of parent and container ports. +ovn-nbctl lsp-set-options vm1 requested-chassis=foo + +OVS_WAIT_UNTIL([test xdown = x$(ovn-nbctl lsp-get-up vm1)]) +OVS_WAIT_UNTIL([test xdown = x$(ovn-nbctl lsp-get-up foo1)]) +OVS_WAIT_UNTIL([test xdown = x$(ovn-nbctl lsp-get-up bar1)]) + +ovn-nbctl clear logical_switch_port vm1 options +OVS_WAIT_UNTIL([test xup = x$(ovn-nbctl lsp-get-up vm1)]) +OVS_WAIT_UNTIL([test xup = x$(ovn-nbctl lsp-get-up foo1)]) +OVS_WAIT_UNTIL([test xup = x$(ovn-nbctl lsp-get-up bar1)]) + +as hv1 ovs-vsctl set interface vm1 external_ids:iface-id=foo +OVS_WAIT_UNTIL([test xdown = x$(ovn-nbctl lsp-get-up vm1)]) +OVS_WAIT_UNTIL([test xdown = x$(ovn-nbctl lsp-get-up foo1)]) +OVS_WAIT_UNTIL([test xdown = x$(ovn-nbctl lsp-get-up bar1)]) + +as hv1 ovs-vsctl set interface vm1 external_ids:iface-id=vm1 +OVS_WAIT_UNTIL([test xup = x$(ovn-nbctl lsp-get-up vm1)]) +OVS_WAIT_UNTIL([test xup = x$(ovn-nbctl lsp-get-up foo1)]) +OVS_WAIT_UNTIL([test xup = x$(ovn-nbctl lsp-get-up bar1)]) + +ovn-nbctl lsp-del vm1 +OVS_WAIT_UNTIL([test xdown = x$(ovn-nbctl lsp-get-up foo1)]) +OVS_WAIT_UNTIL([test xdown = x$(ovn-nbctl lsp-get-up bar1)]) + +ovn-nbctl lsp-add mgmt vm1 +OVS_WAIT_UNTIL([test xup = x$(ovn-nbctl lsp-get-up vm1)]) +OVS_WAIT_UNTIL([test xup = x$(ovn-nbctl lsp-get-up foo1)]) +OVS_WAIT_UNTIL([test xup = x$(ovn-nbctl lsp-get-up bar1)]) + +as hv1 ovs-vsctl del-port vm1 +OVS_WAIT_UNTIL([test xdown = x$(ovn-nbctl lsp-get-up vm1)]) +OVS_WAIT_UNTIL([test xdown = x$(ovn-nbctl lsp-get-up foo1)]) +OVS_WAIT_UNTIL([test xdown = x$(ovn-nbctl lsp-get-up bar1)]) + OVN_CLEANUP([hv1],[hv2]) AT_CLEANUP From patchwork Thu May 28 11:04:16 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Numan Siddique X-Patchwork-Id: 1299658 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 49XlGW3559z9sRY for ; Thu, 28 May 2020 21:05:07 +1000 (AEST) Received: from localhost (localhost [127.0.0.1]) by whitealder.osuosl.org (Postfix) with ESMTP id C961D882B1; Thu, 28 May 2020 11:05:05 +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 W+jjOvvsih-q; Thu, 28 May 2020 11:04:50 +0000 (UTC) Received: from lists.linuxfoundation.org (lf-lists.osuosl.org [140.211.9.56]) by whitealder.osuosl.org (Postfix) with ESMTP id 8F1A382F92; Thu, 28 May 2020 11:04:50 +0000 (UTC) Received: from lf-lists.osuosl.org (localhost [127.0.0.1]) by lists.linuxfoundation.org (Postfix) with ESMTP id 65A01C08A2; Thu, 28 May 2020 11:04:50 +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 8F369C088D for ; Thu, 28 May 2020 11:04:48 +0000 (UTC) Received: from localhost (localhost [127.0.0.1]) by fraxinus.osuosl.org (Postfix) with ESMTP id 6ECEA8712C for ; Thu, 28 May 2020 11:04:48 +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 3WUhIFZe0A-4 for ; Thu, 28 May 2020 11:04:37 +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 07784870DA for ; Thu, 28 May 2020 11:04:33 +0000 (UTC) X-Originating-IP: 27.7.91.57 Received: from nusiddiq.home.org.com (unknown [27.7.91.57]) (Authenticated sender: numans@ovn.org) by relay1-d.mail.gandi.net (Postfix) with ESMTPSA id F35EB240007; Thu, 28 May 2020 11:04:28 +0000 (UTC) From: numans@ovn.org To: dev@openvswitch.org Date: Thu, 28 May 2020 16:34:16 +0530 Message-Id: <20200528110416.1058007-1-numans@ovn.org> X-Mailer: git-send-email 2.26.2 In-Reply-To: <20200528110344.1057873-1-numans@ovn.org> References: <20200528110344.1057873-1-numans@ovn.org> MIME-Version: 1.0 Subject: [ovs-dev] [PATCH ovn v9 2/8] 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. Acked-by: Mark Michelson Signed-off-by: Numan Siddique --- controller/binding.c | 1002 ++++++++++++++++++++++++++++++----- controller/binding.h | 9 +- controller/ovn-controller.c | 61 ++- tests/ovn-performance.at | 13 + 4 files changed, 953 insertions(+), 132 deletions(-) diff --git a/controller/binding.c b/controller/binding.c index 6a13d1a0e..74e0e0710 100644 --- a/controller/binding.c +++ b/controller/binding.c @@ -360,16 +360,6 @@ destroy_qos_map(struct hmap *qos_map) hmap_destroy(qos_map); } -static void -update_local_lport_ids(struct sset *local_lport_ids, - const struct sbrec_port_binding *pb) -{ - char buf[16]; - snprintf(buf, sizeof(buf), "%"PRId64"_%"PRId64, - pb->datapath->tunnel_key, pb->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 @@ -448,10 +438,10 @@ is_network_plugged(const struct sbrec_port_binding *binding_rec, } static void -consider_localnet_port(const struct sbrec_port_binding *binding_rec, - struct shash *bridge_mappings, - struct sset *egress_ifaces, - struct hmap *local_datapaths) +update_ld_localnet_port(const struct sbrec_port_binding *binding_rec, + struct shash *bridge_mappings, + struct sset *egress_ifaces, + struct hmap *local_datapaths) { /* Ignore localnet ports for unplugged networks. */ if (!is_network_plugged(binding_rec, bridge_mappings)) { @@ -481,6 +471,27 @@ 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 *pb) +{ + char buf[16]; + snprintf(buf, sizeof(buf), "%"PRId64"_%"PRId64, + pb->datapath->tunnel_key, pb->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); +} + /* Local bindings. binding.c module binds the logical port (represented by * Port_Binding rows) and sets the 'chassis' column when it sees the * OVS interface row (of type "" or "internal") with the @@ -520,6 +531,10 @@ consider_localnet_port(const struct sbrec_port_binding *binding_rec, * BT_VIF. Its Port_Binding row's 'parent' column is set to * its parent's Port_Binding. It shares the OVS interface row * with the parent. + * Each ovn-controller when it sees a container Port_Binding, + * it creates 'struct local_binding' for the parent + * Port_Binding and for its even if the OVS interface row for + * the parent is not present. * * BT_VIRTUAL: Represents a local binding which has a parent of type BT_VIF. * Its Port_Binding type is "virtual" and it shares the OVS @@ -527,6 +542,17 @@ consider_localnet_port(const struct sbrec_port_binding *binding_rec, * Port_Binding of type "virtual" is claimed by pinctrl module * when it sees the ARP packet from the parent's VIF. * + * + * An object of 'struct local_binding' is created: + * - For each interface that has iface-id configured with the type - BT_VIF. + * + * - For each container Port Binding (of type BT_CONTAINER) and its + * parent Port_Binding (of type BT_VIF), no matter if + * they are bound to this chassis i.e even if OVS interface row for the + * parent is not present. + * + * - For each 'virtual' Port Binding (of type BT_VIRTUAL) provided its parent + * is bound to this chassis. */ enum local_binding_type { BT_VIF, @@ -598,6 +624,14 @@ local_bindings_destroy(struct shash *local_bindings) shash_destroy(local_bindings); } +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); +} + static void local_binding_add_child(struct local_binding *lbinding, struct local_binding *child) @@ -624,6 +658,7 @@ is_lport_container(const struct sbrec_port_binding *pb) return is_lport_vif(pb) && pb->parent_port && pb->parent_port[0]; } +/* Corresponds to each Port_Binding.type. */ enum en_lport_type { LP_UNKNOWN, LP_VIF, @@ -669,12 +704,20 @@ get_lport_type(const struct sbrec_port_binding *pb) return LP_UNKNOWN; } -static void +/* Returns false if lport is not claimed due to 'sb_readonly'. + * Returns true otherwise. + */ +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 sb_readonly) { if (pb->chassis != chassis_rec) { + if (sb_readonly) { + return false; + } + if (pb->chassis) { VLOG_INFO("Changing chassis for lport %s from %s to %s.", pb->logical_port, pb->chassis->name, @@ -693,22 +736,44 @@ 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 (sb_readonly) { + return false; + } sbrec_port_binding_set_encap(pb, encap_rec); } + + return true; } -static void -release_lport(const struct sbrec_port_binding *pb) +/* Returns false if lport is not released due to 'sb_readonly'. + * Returns true otherwise. + */ +static bool +release_lport(const struct sbrec_port_binding *pb, bool sb_readonly) { - VLOG_INFO("Releasing lport %s from this chassis.", pb->logical_port); if (pb->encap) { + if (sb_readonly) { + return false; + } sbrec_port_binding_set_encap(pb, NULL); } - sbrec_port_binding_set_chassis(pb, NULL); + + if (pb->chassis) { + if (sb_readonly) { + return false; + } + sbrec_port_binding_set_chassis(pb, NULL); + } if (pb->virtual_parent) { + if (sb_readonly) { + return false; + } sbrec_port_binding_set_virtual_parent(pb, NULL); } + + VLOG_INFO("Releasing lport %s from this chassis.", pb->logical_port); + return true; } static bool @@ -733,7 +798,63 @@ can_bind_on_this_chassis(const struct sbrec_chassis *chassis_rec, || !strcmp(requested_chassis, chassis_rec->hostname); } -static void +/* Returns 'true' if the 'lbinding' has children of type BT_CONTAINER, + * 'false' otherwise. */ +static bool +is_lbinding_container_parent(struct local_binding *lbinding) +{ + struct shash_node *node; + SHASH_FOR_EACH (node, &lbinding->children) { + struct local_binding *l = node->data; + if (l->type == BT_CONTAINER) { + return true; + } + } + + return false; +} + +static bool +release_local_binding_children(const struct sbrec_chassis *chassis_rec, + struct local_binding *lbinding, + bool sb_readonly) +{ + struct shash_node *node; + SHASH_FOR_EACH (node, &lbinding->children) { + struct local_binding *l = node->data; + if (is_lbinding_this_chassis(l, chassis_rec)) { + if (!release_lport(l->pb, sb_readonly)) { + return false; + } + } + + /* Clear the local bindings' 'pb' and 'iface'. */ + l->pb = NULL; + l->iface = NULL; + } + + return true; +} + +static bool +release_local_binding(const struct sbrec_chassis *chassis_rec, + struct local_binding *lbinding, bool sb_readonly) +{ + if (!release_local_binding_children(chassis_rec, lbinding, + sb_readonly)) { + return false; + } + + if (is_lbinding_this_chassis(lbinding, chassis_rec)) { + return release_lport(lbinding->pb, sb_readonly); + } + + lbinding->pb = NULL; + lbinding->iface = NULL; + return true; +} + +static bool consider_vif_lport_(const struct sbrec_port_binding *pb, bool can_bind, const char *vif_chassis, struct binding_ctx_in *b_ctx_in, @@ -745,7 +866,10 @@ consider_vif_lport_(const struct sbrec_port_binding *pb, if (lbinding_set) { if (can_bind) { /* We can claim the lport. */ - 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; + } add_local_datapath(b_ctx_in->sbrec_datapath_binding_by_key, b_ctx_in->sbrec_port_binding_by_datapath, @@ -771,13 +895,14 @@ consider_vif_lport_(const struct sbrec_port_binding *pb, if (pb->chassis == b_ctx_in->chassis_rec) { /* Release the lport if there is no lbinding. */ if (!lbinding_set || !can_bind) { - release_lport(pb); + return release_lport(pb, !b_ctx_in->ovnsb_idl_txn); } } + return true; } -static void +static bool consider_vif_lport(const struct sbrec_port_binding *pb, struct binding_ctx_in *b_ctx_in, struct binding_ctx_out *b_ctx_out, @@ -797,11 +922,11 @@ consider_vif_lport(const struct sbrec_port_binding *pb, lbinding->pb = pb; } - consider_vif_lport_(pb, can_bind, vif_chassis, b_ctx_in, - b_ctx_out, lbinding, qos_map); + return consider_vif_lport_(pb, can_bind, vif_chassis, b_ctx_in, + b_ctx_out, lbinding, qos_map); } -static void +static bool consider_container_lport(const struct sbrec_port_binding *pb, struct binding_ctx_in *b_ctx_in, struct binding_ctx_out *b_ctx_out, @@ -811,7 +936,39 @@ consider_container_lport(const struct sbrec_port_binding *pb, parent_lbinding = local_binding_find(b_ctx_out->local_bindings, pb->parent_port); - if (parent_lbinding && !parent_lbinding->pb) { + if (!parent_lbinding) { + /* There is no local_binding for parent port. Create it + * without OVS interface row. This is the only exception + * for creating the 'struct local_binding' object without + * corresponding OVS interface row. + * + * This is required for the following reasons: + * - If a logical port P1 is created and then + * few container ports - C1, C2, .. are created first by CMS. + * - And later when OVS interface row is created for P1, then + * we want the these container ports also be claimed by the + * chassis. + * */ + parent_lbinding = local_binding_create(pb->parent_port, NULL, NULL, + BT_VIF); + local_binding_add(b_ctx_out->local_bindings, parent_lbinding); + } + + struct local_binding *container_lbinding = + local_binding_find_child(parent_lbinding, pb->logical_port); + + if (!container_lbinding) { + container_lbinding = local_binding_create(pb->logical_port, + parent_lbinding->iface, + pb, BT_CONTAINER); + local_binding_add_child(parent_lbinding, container_lbinding); + } else { + ovs_assert(container_lbinding->type == BT_CONTAINER); + container_lbinding->pb = pb; + container_lbinding->iface = parent_lbinding->iface; + } + + if (!parent_lbinding->pb) { parent_lbinding->pb = lport_lookup_by_name( b_ctx_in->sbrec_port_binding_by_name, pb->parent_port); @@ -820,37 +977,28 @@ consider_container_lport(const struct sbrec_port_binding *pb, * So call consider_vif_lport() to process it first. */ consider_vif_lport(parent_lbinding->pb, b_ctx_in, b_ctx_out, parent_lbinding, qos_map); - } - } + } else { + /* The parent lport doesn't exist. Call release_lport() to + * release the container lport, if it was bound earlier. */ + if (is_lbinding_this_chassis(container_lbinding, + b_ctx_in->chassis_rec)) { + return release_lport(pb, !b_ctx_in->ovnsb_idl_txn); + } - if (!parent_lbinding || !parent_lbinding->pb) { - /* Call release_lport, to release the container lport, if - * it was bound earlier. */ - if (pb->chassis == b_ctx_in->chassis_rec) { - release_lport(pb); + return true; } - return; } - struct local_binding *container_lbinding = - local_binding_find_child(parent_lbinding, pb->logical_port); - ovs_assert(!container_lbinding); - - container_lbinding = local_binding_create(pb->logical_port, - parent_lbinding->iface, - pb, BT_CONTAINER); - local_binding_add_child(parent_lbinding, container_lbinding); - const char *vif_chassis = smap_get(&parent_lbinding->pb->options, "requested-chassis"); bool can_bind = can_bind_on_this_chassis(b_ctx_in->chassis_rec, vif_chassis); - consider_vif_lport_(pb, can_bind, vif_chassis, b_ctx_in, b_ctx_out, - container_lbinding, qos_map); + return consider_vif_lport_(pb, can_bind, vif_chassis, b_ctx_in, b_ctx_out, + container_lbinding, qos_map); } -static void +static bool consider_virtual_lport(const struct sbrec_port_binding *pb, struct binding_ctx_in *b_ctx_in, struct binding_ctx_out *b_ctx_out, @@ -877,11 +1025,16 @@ consider_virtual_lport(const struct sbrec_port_binding *pb, if (is_lbinding_this_chassis(parent_lbinding, b_ctx_in->chassis_rec)) { virtual_lbinding = local_binding_find_child(parent_lbinding, pb->logical_port); - ovs_assert(!virtual_lbinding); - virtual_lbinding = local_binding_create(pb->logical_port, - parent_lbinding->iface, - pb, BT_VIRTUAL); - local_binding_add_child(parent_lbinding, virtual_lbinding); + if (!virtual_lbinding) { + virtual_lbinding = local_binding_create(pb->logical_port, + parent_lbinding->iface, + pb, BT_VIRTUAL); + local_binding_add_child(parent_lbinding, virtual_lbinding); + } else { + ovs_assert(virtual_lbinding->type == BT_VIRTUAL); + virtual_lbinding->pb = pb; + virtual_lbinding->iface = parent_lbinding->iface; + } } return consider_vif_lport_(pb, true, NULL, b_ctx_in, b_ctx_out, @@ -891,14 +1044,13 @@ consider_virtual_lport(const struct sbrec_port_binding *pb, /* Considers either claiming the lport or releasing the lport * for non VIF lports. */ -static void +static bool consider_nonvif_lport_(const struct sbrec_port_binding *pb, bool our_chassis, bool has_local_l3gateway, struct binding_ctx_in *b_ctx_in, struct binding_ctx_out *b_ctx_out) { - ovs_assert(b_ctx_in->ovnsb_idl_txn); if (our_chassis) { sset_add(b_ctx_out->local_lports, pb->logical_port); add_local_datapath(b_ctx_in->sbrec_datapath_binding_by_key, @@ -908,13 +1060,16 @@ consider_nonvif_lport_(const struct sbrec_port_binding *pb, b_ctx_out->local_datapaths); update_local_lport_ids(b_ctx_out->local_lport_ids, pb); - claim_lport(pb, b_ctx_in->chassis_rec, NULL); + return 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); + return release_lport(pb, !b_ctx_in->ovnsb_idl_txn); } + + return true; } -static void +static bool consider_l2gw_lport(const struct sbrec_port_binding *pb, struct binding_ctx_in *b_ctx_in, struct binding_ctx_out *b_ctx_out) @@ -923,10 +1078,10 @@ consider_l2gw_lport(const struct sbrec_port_binding *pb, bool our_chassis = chassis_id && !strcmp(chassis_id, b_ctx_in->chassis_rec->name); - consider_nonvif_lport_(pb, our_chassis, false, b_ctx_in, b_ctx_out); + return consider_nonvif_lport_(pb, our_chassis, false, b_ctx_in, b_ctx_out); } -static void +static bool consider_l3gw_lport(const struct sbrec_port_binding *pb, struct binding_ctx_in *b_ctx_in, struct binding_ctx_out *b_ctx_out) @@ -935,7 +1090,7 @@ consider_l3gw_lport(const struct sbrec_port_binding *pb, bool our_chassis = chassis_id && !strcmp(chassis_id, b_ctx_in->chassis_rec->name); - consider_nonvif_lport_(pb, our_chassis, true, b_ctx_in, b_ctx_out); + return consider_nonvif_lport_(pb, our_chassis, true, b_ctx_in, b_ctx_out); } static void @@ -954,7 +1109,7 @@ consider_localnet_lport(const struct sbrec_port_binding *pb, update_local_lport_ids(b_ctx_out->local_lport_ids, pb); } -static void +static bool consider_ha_lport(const struct sbrec_port_binding *pb, struct binding_ctx_in *b_ctx_in, struct binding_ctx_out *b_ctx_out) @@ -982,18 +1137,18 @@ consider_ha_lport(const struct sbrec_port_binding *pb, b_ctx_out->local_datapaths); } - consider_nonvif_lport_(pb, our_chassis, false, b_ctx_in, b_ctx_out); + return consider_nonvif_lport_(pb, our_chassis, false, b_ctx_in, b_ctx_out); } -static void +static bool consider_cr_lport(const struct sbrec_port_binding *pb, struct binding_ctx_in *b_ctx_in, struct binding_ctx_out *b_ctx_out) { - consider_ha_lport(pb, b_ctx_in, b_ctx_out); + return consider_ha_lport(pb, b_ctx_in, b_ctx_out); } -static void +static bool consider_external_lport(const struct sbrec_port_binding *pb, struct binding_ctx_in *b_ctx_in, struct binding_ctx_out *b_ctx_out) @@ -1046,6 +1201,8 @@ build_local_bindings(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); } /* Check if this is a tunnel interface. */ @@ -1161,9 +1318,9 @@ binding_run(struct binding_ctx_in *b_ctx_in, struct binding_ctx_out *b_ctx_out) * corresponding local datapath accordingly. */ struct localnet_lport *lnet_lport; LIST_FOR_EACH_POP (lnet_lport, list_node, &localnet_lports) { - consider_localnet_port(lnet_lport->pb, &bridge_mappings, - b_ctx_out->egress_ifaces, - b_ctx_out->local_datapaths); + update_ld_localnet_port(lnet_lport->pb, &bridge_mappings, + b_ctx_out->egress_ifaces, + b_ctx_out->local_datapaths); free(lnet_lport); } @@ -1181,95 +1338,688 @@ binding_run(struct binding_ctx_in *b_ctx_in, struct binding_ctx_out *b_ctx_out) destroy_qos_map(&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. */ +/* Returns true if the database is all cleaned up, false if more work is + * required. */ bool -binding_evaluate_port_binding_changes(struct binding_ctx_in *b_ctx_in, - struct binding_ctx_out *b_ctx_out) +binding_cleanup(struct ovsdb_idl_txn *ovnsb_idl_txn, + const struct sbrec_port_binding_table *port_binding_table, + const struct sbrec_chassis *chassis_rec) { - if (!b_ctx_in->chassis_rec) { + if (!ovnsb_idl_txn) { + return false; + } + if (!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; + bool any_changes = false; + SBREC_PORT_BINDING_TABLE_FOR_EACH (binding_rec, port_binding_table) { + if (binding_rec->chassis == chassis_rec) { + if (binding_rec->encap) { + sbrec_port_binding_set_encap(binding_rec, NULL); + } + sbrec_port_binding_set_chassis(binding_rec, NULL); + any_changes = true; + } + } + + if (any_changes) { + ovsdb_idl_txn_add_comment( + ovnsb_idl_txn, + "ovn-controller: removing all port bindings for '%s'", + chassis_rec->name); + } + + return !any_changes; +} + +/* This function adds the local datapath of the 'peer' of + * lport 'pb' to the local datapaths if it is not yet added. + */ +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) +{ + 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; + } + + bool present = false; + for (size_t i = 0; i < ld->n_peer_ports; i++) { + if (ld->peer_ports[i].local == pb) { + present = true; break; } + } - if (!strcmp(binding_rec->type, "remote")) { - continue; + 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; + } - if (strcmp(binding_rec->type, "")) { - changed = true; + 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; - 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); + /* Possible improvement: We can shrint the allocated peer ports + * if (ld->n_peer_ports < ld->n_allocated_peer_ports / 2). + */ + 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 the peer port from the peer datapath. The peer + * datapath also tries to remove its peer lport, but that would + * be no-op. */ + remove_local_datapath_peer_port(peer, peer_ld, local_datapaths); + } +} + +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; + } + } +} + +/* Considers the ovs iface 'iface_rec' for claiming. + * This function should be called if the external_ids:iface-id + * and 'ofport' are set for the 'iface_rec'. + * + * If the local_binding for this 'iface_rec' already exists and its + * already claimed, then this function will be no-op. + */ +static bool +consider_iface_claim(const struct ovsrec_interface *iface_rec, + const char *iface_id, + struct binding_ctx_in *b_ctx_in, + struct binding_ctx_out *b_ctx_out, + struct hmap *qos_map) +{ + sset_add(b_ctx_out->local_lports, iface_id); + smap_replace(b_ctx_out->local_iface_ids, iface_rec->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, NULL, BT_VIF); + local_binding_add(b_ctx_out->local_bindings, lbinding); + } else { + lbinding->iface = iface_rec; + } + + if (!lbinding->pb || strcmp(lbinding->name, lbinding->pb->logical_port)) { + lbinding->pb = lport_lookup_by_name( + b_ctx_in->sbrec_port_binding_by_name, lbinding->name); + if (lbinding->pb && !strcmp(lbinding->pb->type, "virtual")) { + lbinding->pb = NULL; + } + } + + if (lbinding->pb) { + if (!consider_vif_lport(lbinding->pb, b_ctx_in, b_ctx_out, + lbinding, qos_map)) { + return false; + } + } + + /* Update the child local_binding's iface (if any children) and try to + * claim the container lbindings. */ + struct shash_node *node; + SHASH_FOR_EACH (node, &lbinding->children) { + struct local_binding *child = node->data; + child->iface = iface_rec; + if (child->type == BT_CONTAINER) { + if (!consider_container_lport(child->pb, b_ctx_in, b_ctx_out, + qos_map)) { + return false; + } + } + } + + return true; +} + +/* Considers the ovs interface 'iface_rec' for + * releasing from this chassis if local_binding for this + * 'iface_rec' (with 'iface_id' as key) already exists and + * it is claimed by the chassis. + * + * The 'iface_id' could be cleared from the 'iface_rec' + * and hence it is passed separately. + * + * This fuction should be called if + * - OVS interface 'iface_rec' is deleted. + * - OVS interface 'iface_rec' external_ids:iface-id is updated + * (with the old value being 'iface_id'.) + * - OVS interface ofport is reset to 0. + * */ +static bool +consider_iface_release(const struct ovsrec_interface *iface_rec, + const char *iface_id, + struct binding_ctx_in *b_ctx_in, + struct binding_ctx_out *b_ctx_out, bool *changed) +{ + struct local_binding *lbinding; + lbinding = local_binding_find(b_ctx_out->local_bindings, + iface_id); + if (is_lbinding_this_chassis(lbinding, b_ctx_in->chassis_rec)) { + + if (!release_local_binding(b_ctx_in->chassis_rec, lbinding, + !b_ctx_in->ovnsb_idl_txn)) { + return false; + } + + 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); + } + + /* Check if the lbinding has children of type PB_CONTAINER. + * If so, don't delete the local_binding. */ + if (!is_lbinding_container_parent(lbinding)) { + local_binding_delete(b_ctx_out->local_bindings, lbinding); + } + *changed = true; + } + + sset_find_and_delete(b_ctx_out->local_lports, iface_id); + smap_remove(b_ctx_out->local_iface_ids, iface_rec->name); + + return true; +} + +static bool +is_iface_vif(const struct ovsrec_interface *iface_rec) +{ + if (iface_rec->type && iface_rec->type[0] && + strcmp(iface_rec->type, "internal")) { + return false; + } + + return true; +} + +/* 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; + + /* Run the tracked interfaces loop twice. One to handle deleted + * changes. And another to handle add/update changes. + * This will ensure correctness. + * * + * We consider an OVS interface for release if one of the following + * happens: + * 1. OVS interface is deleted. + * 2. external_ids:iface-id is cleared in which case we need to + * release the port binding corresponding to the previously set + * 'old-iface-id' (which is stored in the smap + * 'b_ctx_out->local_iface_ids'). + * 3. external_ids:iface-id is updated with a different value + * in which case we need to release the port binding corresponding + * to the previously set 'old-iface-id' (which is stored in the smap + * 'b_ctx_out->local_iface_ids'). + * 4. ofport of the OVS interface is 0. + * + */ + const struct ovsrec_interface *iface_rec; + OVSREC_INTERFACE_TABLE_FOR_EACH_TRACKED (iface_rec, + b_ctx_in->iface_table) { + if (!is_iface_vif(iface_rec)) { + /* Right now are not handling ovs_interface changes of + * other types. This can be enhanced to handle of + * types - patch and tunnel. */ + handled = false; + break; + } + + 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); + const char *cleared_iface_id = NULL; + if (!ovsrec_interface_is_deleted(iface_rec)) { + int64_t ofport = iface_rec->n_ofport ? *iface_rec->ofport : 0; + 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 (!ofport) { + /* If ofport is 0, we need to release the iface if already + * claimed. */ + cleared_iface_id = iface_id; + } + } else if (old_iface_id) { + cleared_iface_id = old_iface_id; + } } else { - lbinding = local_binding_find(b_ctx_out->local_bindings, - binding_rec->parent_port); + cleared_iface_id = iface_id; } - if (lbinding) { - changed = true; + if (cleared_iface_id) { + handled = consider_iface_release(iface_rec, cleared_iface_id, + b_ctx_in, b_ctx_out, changed); + } + + if (!handled) { break; } } - return changed; + if (!handled) { + /* This can happen if any non vif OVS interface is in the tracked + * list or if consider_iface_release() returned false. + * There is no need to process further. */ + return 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; + + /* + * We consider an OVS interface for claiming if the following + * 2 conditions are met: + * 1. external_ids:iface-id is set. + * 2. ofport of the OVS interface is > 0. + * + * So when an update of an OVS interface happens we see if these + * conditions are still true. If so consider this interface for + * claiming. This would be no-op if the update of the OVS interface + * didn't change the above two fields. + */ + OVSREC_INTERFACE_TABLE_FOR_EACH_TRACKED (iface_rec, + b_ctx_in->iface_table) { + /* Loop to handle create and update changes only. */ + if (ovsrec_interface_is_deleted(iface_rec)) { + continue; + } + + const char *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) { + *changed = true; + handled = consider_iface_claim(iface_rec, iface_id, b_ctx_in, + b_ctx_out, qos_map_ptr); + if (!handled) { + break; + } + } + } + + if (handled && 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); + } + } + + destroy_qos_map(&qos_map); + return handled; } -/* Returns true if the database is all cleaned up, false if more work is - * required. */ -bool -binding_cleanup(struct ovsdb_idl_txn *ovnsb_idl_txn, - const struct sbrec_port_binding_table *port_binding_table, - const struct sbrec_chassis *chassis_rec) +static void +handle_deleted_lport(const struct sbrec_port_binding *pb, + struct binding_ctx_in *b_ctx_in, + struct binding_ctx_out *b_ctx_out) { - if (!ovnsb_idl_txn) { + 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); + } +} + +static struct local_binding * +get_lbinding_for_lport(const struct sbrec_port_binding *pb, + enum en_lport_type lport_type, + struct binding_ctx_out *b_ctx_out) +{ + ovs_assert(lport_type == LP_VIF || lport_type == LP_VIRTUAL); + + if (lport_type == LP_VIF && !is_lport_container(pb)) { + return local_binding_find(b_ctx_out->local_bindings, pb->logical_port); + } + + struct local_binding *parent_lbinding = NULL; + + if (lport_type == LP_VIRTUAL) { + parent_lbinding = local_binding_find(b_ctx_out->local_bindings, + pb->virtual_parent); + } else { + parent_lbinding = local_binding_find(b_ctx_out->local_bindings, + pb->parent_port); + } + + return parent_lbinding + ? local_binding_find(&parent_lbinding->children, pb->logical_port) + : NULL; +} + +static bool +handle_deleted_vif_lport(const struct sbrec_port_binding *pb, + enum en_lport_type lport_type, + struct binding_ctx_in *b_ctx_in, + struct binding_ctx_out *b_ctx_out, + bool *changed) +{ + struct local_binding *lbinding = + get_lbinding_for_lport(pb, lport_type, b_ctx_out); + + if (lbinding) { + lbinding->pb = NULL; + /* The port_binding 'pb' is deleted. So there is no need to + * clear the 'chassis' column of 'pb'. But we need to do + * for the local_binding's children. */ + if (lbinding->type == BT_VIF && + !release_local_binding_children(b_ctx_in->chassis_rec, + lbinding, + !b_ctx_in->ovnsb_idl_txn)) { + return false; + } + + *changed = true; + } + + handle_deleted_lport(pb, b_ctx_in, b_ctx_out); + return true; +} + +static bool +handle_updated_vif_lport(const struct sbrec_port_binding *pb, + enum en_lport_type lport_type, + struct binding_ctx_in *b_ctx_in, + struct binding_ctx_out *b_ctx_out, + struct hmap *qos_map, + bool *changed) +{ + bool claimed = (pb->chassis == b_ctx_in->chassis_rec); + bool handled = true; + + if (lport_type == LP_VIRTUAL) { + handled = consider_virtual_lport(pb, b_ctx_in, b_ctx_out, qos_map); + } else if (lport_type == LP_VIF && is_lport_container(pb)) { + handled = consider_container_lport(pb, b_ctx_in, b_ctx_out, qos_map); + } else { + handled = consider_vif_lport(pb, b_ctx_in, b_ctx_out, NULL, qos_map); + } + + if (!handled) { return false; } - if (!chassis_rec) { + + bool now_claimed = (pb->chassis == b_ctx_in->chassis_rec); + bool changed_ = (claimed != now_claimed); + + if (changed_) { + *changed = true; + } + + if (lport_type == LP_VIRTUAL || + (lport_type == LP_VIF && is_lport_container(pb)) || !changed_) { return true; } - const struct sbrec_port_binding *binding_rec; - bool any_changes = false; - SBREC_PORT_BINDING_TABLE_FOR_EACH (binding_rec, port_binding_table) { - if (binding_rec->chassis == chassis_rec) { - if (binding_rec->encap) - sbrec_port_binding_set_encap(binding_rec, NULL); - sbrec_port_binding_set_chassis(binding_rec, NULL); - any_changes = true; + struct local_binding *lbinding = + local_binding_find(b_ctx_out->local_bindings, pb->logical_port); + + ovs_assert(lbinding); + + struct shash_node *node; + SHASH_FOR_EACH (node, &lbinding->children) { + struct local_binding *child = node->data; + if (child->type == BT_CONTAINER) { + handled = consider_container_lport(child->pb, b_ctx_in, b_ctx_out, + qos_map); + if (!handled) { + return false; + } } } - if (any_changes) { - ovsdb_idl_txn_add_comment( - ovnsb_idl_txn, - "ovn-controller: removing all port bindings for '%s'", - chassis_rec->name); + return true; +} + +/* 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; + *changed = false; + + const struct sbrec_port_binding *pb; + + /* Run the tracked port binding loop twice. One to handle deleted + * changes. And another to handle add/update changes. + * This will ensure correctness. + */ + SBREC_PORT_BINDING_TABLE_FOR_EACH_TRACKED (pb, + b_ctx_in->port_binding_table) { + if (!sbrec_port_binding_is_deleted(pb)) { + continue; + } + + enum en_lport_type lport_type = get_lport_type(pb); + if (lport_type == LP_VIF || lport_type == LP_VIRTUAL) { + handled = handle_deleted_vif_lport(pb, lport_type, b_ctx_in, + b_ctx_out, changed); + } else { + handle_deleted_lport(pb, b_ctx_in, b_ctx_out); + } + + if (!handled) { + break; + } } - return !any_changes; + if (!handled) { + return 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; + + SBREC_PORT_BINDING_TABLE_FOR_EACH_TRACKED (pb, + b_ctx_in->port_binding_table) { + /* Loop to handle create and update changes only. */ + if (sbrec_port_binding_is_deleted(pb)) { + continue; + } + + enum en_lport_type lport_type = get_lport_type(pb); + + struct local_datapath *ld = + get_local_datapath(b_ctx_out->local_datapaths, + pb->datapath->tunnel_key); + + switch (lport_type) { + case LP_VIF: + case LP_VIRTUAL: + handled = handle_updated_vif_lport(pb, lport_type, b_ctx_in, + b_ctx_out, qos_map_ptr, + changed); + break; + + case LP_PATCH: + case LP_LOCALPORT: + case LP_VTEP: + *changed = true; + update_local_lport_ids(b_ctx_out->local_lport_ids, pb); + if (lport_type == LP_PATCH) { + /* Add the peer datapath to the local datapaths if it's + * not present yet. + */ + if (ld) { + add_local_datapath_peer_port(pb, b_ctx_in, b_ctx_out, ld); + } + } + break; + + case LP_L2GATEWAY: + *changed = true; + handled = consider_l2gw_lport(pb, b_ctx_in, b_ctx_out); + break; + + case LP_L3GATEWAY: + *changed = true; + handled = consider_l3gw_lport(pb, b_ctx_in, b_ctx_out); + break; + + case LP_CHASSISREDIRECT: + *changed = true; + handled = consider_cr_lport(pb, b_ctx_in, b_ctx_out); + break; + + case LP_EXTERNAL: + *changed = true; + handled = consider_external_lport(pb, b_ctx_in, b_ctx_out); + break; + + case LP_LOCALNET: { + *changed = true; + consider_localnet_lport(pb, b_ctx_in, b_ctx_out, qos_map_ptr); + + struct shash bridge_mappings = + SHASH_INITIALIZER(&bridge_mappings); + add_ovs_bridge_mappings(b_ctx_in->ovs_table, + b_ctx_in->bridge_table, + &bridge_mappings); + update_ld_localnet_port(pb, &bridge_mappings, + b_ctx_out->egress_ifaces, + b_ctx_out->local_datapaths); + shash_destroy(&bridge_mappings); + break; + } + + case LP_REMOTE: + case LP_UNKNOWN: + break; + } + + if (!handled) { + break; + } + } + + if (handled && 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); + } + } + + destroy_qos_map(&qos_map); + return handled; } diff --git a/controller/binding.h b/controller/binding.h index 9affc9a96..f7ace6faf 100644 --- a/controller/binding.h +++ b/controller/binding.h @@ -57,6 +57,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 *); @@ -64,9 +65,13 @@ 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_init(struct shash *local_bindings); void local_bindings_destroy(struct shash *local_bindings); +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 386a64059..9ce9684e9 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 * @@ -987,6 +989,7 @@ en_runtime_data_init(struct engine_node *node OVS_UNUSED, sset_init(&data->local_lport_ids); sset_init(&data->active_tunnels); sset_init(&data->egress_ifaces); + smap_init(&data->local_iface_ids); local_bindings_init(&data->local_bindings); 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_destroy(&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 @@ -1111,10 +1121,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); local_bindings_init(&rt_data->local_bindings); } @@ -1140,6 +1152,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) { @@ -1147,11 +1187,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. */ @@ -1895,7 +1945,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 May 28 11:04:31 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Numan Siddique X-Patchwork-Id: 1299656 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 49XlG937xTz9sSJ for ; Thu, 28 May 2020 21:04:48 +1000 (AEST) Received: from localhost (localhost [127.0.0.1]) by hemlock.osuosl.org (Postfix) with ESMTP id 9E8FA88C3C; Thu, 28 May 2020 11:04:46 +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 RQAh+IAPR1W6; Thu, 28 May 2020 11:04:45 +0000 (UTC) Received: from lists.linuxfoundation.org (lf-lists.osuosl.org [140.211.9.56]) by hemlock.osuosl.org (Postfix) with ESMTP id 4C14D88B4B; Thu, 28 May 2020 11:04:45 +0000 (UTC) Received: from lf-lists.osuosl.org (localhost [127.0.0.1]) by lists.linuxfoundation.org (Postfix) with ESMTP id 3EBB7C088D; Thu, 28 May 2020 11:04:45 +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 6EE28C088D for ; Thu, 28 May 2020 11:04:43 +0000 (UTC) Received: from localhost (localhost [127.0.0.1]) by fraxinus.osuosl.org (Postfix) with ESMTP id 604CA8711A for ; Thu, 28 May 2020 11:04:43 +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 1xaDHOHt2lGl for ; Thu, 28 May 2020 11:04:41 +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 08A15870F7 for ; Thu, 28 May 2020 11:04:40 +0000 (UTC) Received: from nusiddiq.home.org.com (unknown [27.7.91.57]) (Authenticated sender: numans@ovn.org) by relay10.mail.gandi.net (Postfix) with ESMTPSA id 3A63A240002; Thu, 28 May 2020 11:04:35 +0000 (UTC) From: numans@ovn.org To: dev@openvswitch.org Date: Thu, 28 May 2020 16:34:31 +0530 Message-Id: <20200528110431.1058063-1-numans@ovn.org> X-Mailer: git-send-email 2.26.2 In-Reply-To: <20200528110344.1057873-1-numans@ovn.org> References: <20200528110344.1057873-1-numans@ovn.org> MIME-Version: 1.0 Cc: Han Zhou Subject: [ovs-dev] [PATCH ovn v9 3/8] 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. Acked-by: Mark Michelson Acked-by: Han Zhou 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 9ce9684e9..89a911b36 100644 --- a/controller/ovn-controller.c +++ b/controller/ovn-controller.c @@ -1204,6 +1204,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)]; @@ -1952,7 +1974,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 May 28 11:04:38 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Numan Siddique X-Patchwork-Id: 1299657 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 49XlGM4hkFz9sSJ for ; Thu, 28 May 2020 21:04:59 +1000 (AEST) Received: from localhost (localhost [127.0.0.1]) by hemlock.osuosl.org (Postfix) with ESMTP id 2F7C688C17; Thu, 28 May 2020 11:04: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 r1hGYiRicqdE; Thu, 28 May 2020 11:04:55 +0000 (UTC) Received: from lists.linuxfoundation.org (lf-lists.osuosl.org [140.211.9.56]) by hemlock.osuosl.org (Postfix) with ESMTP id 3070188C62; Thu, 28 May 2020 11:04:53 +0000 (UTC) Received: from lf-lists.osuosl.org (localhost [127.0.0.1]) by lists.linuxfoundation.org (Postfix) with ESMTP id 0C229C088D; Thu, 28 May 2020 11:04:53 +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 B70DFC016F for ; Thu, 28 May 2020 11:04:49 +0000 (UTC) Received: from localhost (localhost [127.0.0.1]) by fraxinus.osuosl.org (Postfix) with ESMTP id 96D2B87126 for ; Thu, 28 May 2020 11:04:49 +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 UfRii3yxMJDC for ; Thu, 28 May 2020 11:04:48 +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 fraxinus.osuosl.org (Postfix) with ESMTPS id 4DBA88716A for ; Thu, 28 May 2020 11:04:47 +0000 (UTC) X-Originating-IP: 27.7.91.57 Received: from nusiddiq.home.org.com (unknown [27.7.91.57]) (Authenticated sender: numans@ovn.org) by relay9-d.mail.gandi.net (Postfix) with ESMTPSA id D0702FF808; Thu, 28 May 2020 11:04:42 +0000 (UTC) From: numans@ovn.org To: dev@openvswitch.org Date: Thu, 28 May 2020 16:34:38 +0530 Message-Id: <20200528110438.1058114-1-numans@ovn.org> X-Mailer: git-send-email 2.26.2 In-Reply-To: <20200528110344.1057873-1-numans@ovn.org> References: <20200528110344.1057873-1-numans@ovn.org> MIME-Version: 1.0 Subject: [ovs-dev] [PATCH ovn v9 4/8] 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. Acked-by: Mark Michelson 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 May 28 11:04:44 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Numan Siddique X-Patchwork-Id: 1299659 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 49XlGx0lgmz9sSJ for ; Thu, 28 May 2020 21:05:29 +1000 (AEST) Received: from localhost (localhost [127.0.0.1]) by fraxinus.osuosl.org (Postfix) with ESMTP id 659CF8531D; Thu, 28 May 2020 11:05:27 +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 R7P8kSSqIhZY; Thu, 28 May 2020 11:05:23 +0000 (UTC) Received: from lists.linuxfoundation.org (lf-lists.osuosl.org [140.211.9.56]) by fraxinus.osuosl.org (Postfix) with ESMTP id 777FE855B1; Thu, 28 May 2020 11:05:23 +0000 (UTC) Received: from lf-lists.osuosl.org (localhost [127.0.0.1]) by lists.linuxfoundation.org (Postfix) with ESMTP id 38E46C089E; Thu, 28 May 2020 11:05:23 +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 05577C016F for ; Thu, 28 May 2020 11:05:21 +0000 (UTC) Received: from localhost (localhost [127.0.0.1]) by silver.osuosl.org (Postfix) with ESMTP id ED75925570 for ; Thu, 28 May 2020 11:05:20 +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 Sm-ovV0MaZ5H for ; Thu, 28 May 2020 11:05:12 +0000 (UTC) X-Greylist: domain auto-whitelisted by SQLgrey-1.7.6 Received: from relay11.mail.gandi.net (relay11.mail.gandi.net [217.70.178.231]) by silver.osuosl.org (Postfix) with ESMTPS id B9F0F204E2 for ; Thu, 28 May 2020 11:04:58 +0000 (UTC) Received: from nusiddiq.home.org.com (unknown [27.7.91.57]) (Authenticated sender: numans@ovn.org) by relay11.mail.gandi.net (Postfix) with ESMTPSA id 891BB10000C; Thu, 28 May 2020 11:04:54 +0000 (UTC) From: numans@ovn.org To: dev@openvswitch.org Date: Thu, 28 May 2020 16:34:44 +0530 Message-Id: <20200528110444.1058163-1-numans@ovn.org> X-Mailer: git-send-email 2.26.2 In-Reply-To: <20200528110344.1057873-1-numans@ovn.org> References: <20200528110344.1057873-1-numans@ovn.org> MIME-Version: 1.0 Subject: [ovs-dev] [PATCH ovn v9 5/8] 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. Acked-by: Mark Michelson Signed-off-by: Numan Siddique --- controller/binding.c | 23 +----- controller/binding.h | 24 +++++- controller/ovn-controller.c | 146 +++++++++++++++++++++++++++++++++++- controller/physical.c | 51 +++++++++++++ controller/physical.h | 5 +- 5 files changed, 224 insertions(+), 25 deletions(-) diff --git a/controller/binding.c b/controller/binding.c index 74e0e0710..63c7203bb 100644 --- a/controller/binding.c +++ b/controller/binding.c @@ -503,7 +503,7 @@ remove_local_lport_ids(const struct sbrec_port_binding *binding_rec, * 'struct local_binding' is used. A shash of these local bindings is * maintained with the 'external_ids:iface-id' as the key to the shash. * - * struct local_binding has 3 main fields: + * struct local_binding (defined in binding.h) has 3 main fields: * - type * - OVS interface row object * - Port_Binding row object @@ -554,21 +554,6 @@ remove_local_lport_ids(const struct sbrec_port_binding *binding_rec, * - For each 'virtual' Port Binding (of type BT_VIRTUAL) provided its parent * is bound to this chassis. */ -enum local_binding_type { - BT_VIF, - BT_CONTAINER, - BT_VIRTUAL -}; - -struct local_binding { - char *name; - enum local_binding_type type; - const struct ovsrec_interface *iface; - const struct sbrec_port_binding *pb; - - /* shash of 'struct local_binding' representing children. */ - struct shash children; -}; static struct local_binding * local_binding_create(const char *name, const struct ovsrec_interface *iface, @@ -590,12 +575,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/binding.h b/controller/binding.h index f7ace6faf..21118ecd4 100644 --- a/controller/binding.h +++ b/controller/binding.h @@ -18,6 +18,7 @@ #define OVN_BINDING_H 1 #include +#include "openvswitch/shash.h" struct hmap; struct ovsdb_idl; @@ -32,7 +33,6 @@ struct sbrec_chassis; struct sbrec_port_binding_table; struct sset; struct sbrec_port_binding; -struct shash; struct binding_ctx_in { struct ovsdb_idl_txn *ovnsb_idl_txn; @@ -60,6 +60,28 @@ struct binding_ctx_out { struct smap *local_iface_ids; }; +enum local_binding_type { + BT_VIF, + BT_CONTAINER, + BT_VIRTUAL +}; + +struct local_binding { + char *name; + enum local_binding_type type; + const struct ovsrec_interface *iface; + const struct sbrec_port_binding *pb; + + /* shash of 'struct local_binding' representing children. */ + struct shash children; +}; + +static inline struct local_binding * +local_binding_find(struct shash *local_bindings, const char *name) +{ + return shash_find_data(local_bindings, name); +} + void binding_register_ovs_idl(struct ovsdb_idl *); void binding_run(struct binding_ctx_in *, struct binding_ctx_out *); bool binding_cleanup(struct ovsdb_idl_txn *ovnsb_idl_txn, diff --git a/controller/ovn-controller.c b/controller/ovn-controller.c index 89a911b36..6381e3856 100644 --- a/controller/ovn-controller.c +++ b/controller/ovn-controller.c @@ -1369,8 +1369,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", node)); + struct ed_type_ct_zones *ct_zones_data = engine_get_input_data("ct_zones", node); + struct simap *ct_zones = &ct_zones_data->current; p_ctx->sbrec_port_binding_by_name = sbrec_port_binding_by_name; @@ -1378,12 +1383,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, @@ -1791,6 +1798,127 @@ flow_output_port_groups_handler(struct engine_node *node, void *data) return _flow_output_resource_ref_handler(node, data, REF_TYPE_PORTGROUP); } +/* Engine node en_physical_flow_changes indicates whether + * there is a need to + * - recompute only physical flows or + * - we can incrementally process the physical flows. + * + * en_physical_flow_changes is an input to flow_output engine node. + * If the engine node 'en_physical_flow_changes' gets updated during + * engine run, it means the handler for this - + * flow_output_physical_flow_changes_handler() will either + * - recompute the physical flows by calling 'physical_run() or + * - incrementlly process some of the changes for physical flow + * calculation. Right now we handle OVS interfaces changes + * for physical flow computation. + * + * When ever a port binding happens, the follow up + * activity is the zone id allocation for that port binding. + * With this intermediate engine node, we avoid full recomputation. + * Instead we do physical flow computation (either full recomputation + * by calling physical_run() or handling the changes incrementally. + * + * Hence this is an intermediate engine node to indicate the + * flow_output engine to recomputes/compute the physical flows. + * + * TODO 1. Ideally this engine node should recompute/compute the physica + * flows instead of relegating it to the flow_output node. + * But this requires splitting the flow_output node to + * logical_flow_output and physical_flow_output. + * + * TODO 2. We can further optimise the en_ct_zone changes to + * compute the phsyical flows for changed zone ids. + * + * TODO 3: physical.c has a global simap -localvif_to_ofport which stores the + * local OVS interfaces and the ofport numbers. Ideally this should be + * part of the engine data. + */ +struct ed_type_pfc_tracked_data { + bool recompute_physical_flows; + bool ovs_ifaces_changed; +}; + +static void +en_physical_flow_changes_clear_data(void *tracked_data) +{ + struct ed_type_pfc_tracked_data *data = tracked_data; + data->recompute_physical_flows = false; + data->ovs_ifaces_changed = false; +} + +static void * +en_physical_flow_changes_init(struct engine_node *node, + struct engine_arg *arg OVS_UNUSED) +{ + struct ed_type_pfc_tracked_data *tdata = xzalloc(sizeof *tdata); + node->tracked_data = tdata; + node->clear_tracked_data = en_physical_flow_changes_clear_data; + return NULL; +} + +static void +en_physical_flow_changes_cleanup(void *data OVS_UNUSED) +{ +} + +/* Indicate to the flow_output engine that we need to recompute physical + * flows. */ +static void +en_physical_flow_changes_run(struct engine_node *node, void *data OVS_UNUSED) +{ + struct ed_type_pfc_tracked_data *pfc_tdata = node->tracked_data; + pfc_tdata->recompute_physical_flows = true; + engine_set_node_state(node, EN_UPDATED); +} + +/* There are OVS interface changes. Indicate to the flow_output engine + * to handle these OVS interface changes for physical flow computations. */ +static bool +physical_flow_changes_ovs_iface_handler(struct engine_node *node, + void *data OVS_UNUSED) +{ + struct ed_type_pfc_tracked_data *pfc_tdata = node->tracked_data; + pfc_tdata->ovs_ifaces_changed = true; + engine_set_node_state(node, EN_UPDATED); + return true; +} + +static bool +flow_output_physical_flow_changes_handler(struct engine_node *node, void *data) +{ + 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); + struct ed_type_pfc_tracked_data *pfc_tdata = + engine_get_input_tracked_data("physical_flow_changes", node); + + if (pfc_tdata->recompute_physical_flows) { + /* This indicates that we need to recompute the physical flows. */ + physical_run(&p_ctx, &fo->flow_table); + return true; + } + + if (pfc_tdata->ovs_ifaces_changed) { + /* There are OVS interface changes. Try to handle them + * incrementally. */ + return physical_handle_ovs_iface_changes(&p_ctx, &fo->flow_table); + } + + return true; +} + +static bool +flow_output_noop_handler(struct engine_node *node OVS_UNUSED, + void *data OVS_UNUSED) +{ + return true; +} + struct ovn_controller_exit_args { bool *exiting; bool *restart; @@ -1915,6 +2043,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"); @@ -1934,13 +2063,28 @@ main(int argc, char *argv[]) engine_add_input(&en_port_groups, &en_sb_port_group, port_groups_sb_port_group_handler); + /* Engine node physical_flow_changes indicates whether + * we can recompute only physical flows or we can + * incrementally process the physical flows. + */ + 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); + + /* We need this input nodes for only data. Hence the noop handler. */ + engine_add_input(&en_flow_output, &en_ct_zones, flow_output_noop_handler); + engine_add_input(&en_flow_output, &en_ovs_interface, + flow_output_noop_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/physical.c b/controller/physical.c index f06313b9d..240ec158e 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 May 28 11:04:56 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Numan Siddique X-Patchwork-Id: 1299664 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 49XlHP4fWrz9sRY for ; Thu, 28 May 2020 21:05:53 +1000 (AEST) Received: from localhost (localhost [127.0.0.1]) by hemlock.osuosl.org (Postfix) with ESMTP id DE51288C40; Thu, 28 May 2020 11:05:51 +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 wvZ48XWIox-w; Thu, 28 May 2020 11:05:48 +0000 (UTC) Received: from lists.linuxfoundation.org (lf-lists.osuosl.org [140.211.9.56]) by hemlock.osuosl.org (Postfix) with ESMTP id B373988C36; Thu, 28 May 2020 11:05:48 +0000 (UTC) Received: from lf-lists.osuosl.org (localhost [127.0.0.1]) by lists.linuxfoundation.org (Postfix) with ESMTP id A6163C088D; Thu, 28 May 2020 11:05:48 +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 05CC9C0890 for ; Thu, 28 May 2020 11:05:47 +0000 (UTC) Received: from localhost (localhost [127.0.0.1]) by silver.osuosl.org (Postfix) with ESMTP id DC3942202E for ; Thu, 28 May 2020 11:05:46 +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 wrApIVlCpETf for ; Thu, 28 May 2020 11:05:28 +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 silver.osuosl.org (Postfix) with ESMTPS id 5FB6D254EB for ; Thu, 28 May 2020 11:05:06 +0000 (UTC) Received: from nusiddiq.home.org.com (unknown [27.7.91.57]) (Authenticated sender: numans@ovn.org) by relay10.mail.gandi.net (Postfix) with ESMTPSA id BF7E2240008; Thu, 28 May 2020 11:05:02 +0000 (UTC) From: numans@ovn.org To: dev@openvswitch.org Date: Thu, 28 May 2020 16:34:56 +0530 Message-Id: <20200528110456.1058214-1-numans@ovn.org> X-Mailer: git-send-email 2.26.2 In-Reply-To: <20200528110344.1057873-1-numans@ovn.org> References: <20200528110344.1057873-1-numans@ovn.org> MIME-Version: 1.0 Cc: Venkata Anil Subject: [ovs-dev] [PATCH ovn v9 6/8] ovn-controller: 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. Acked-by: Mark Michelson Co-Authored-by: Venkata Anil Signed-off-by: Venkata Anil Signed-off-by: Numan Siddique --- controller/binding.c | 252 ++++++++++++++++++++++++++++++------ controller/binding.h | 37 ++++++ controller/ovn-controller.c | 125 +++++++++++++++++- tests/ovn-performance.at | 20 +-- 4 files changed, 385 insertions(+), 49 deletions(-) diff --git a/controller/binding.c b/controller/binding.c index 63c7203bb..601da65a9 100644 --- a/controller/binding.c +++ b/controller/binding.c @@ -69,13 +69,23 @@ 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( + const struct sbrec_datapath_binding *, + bool is_new, struct hmap *tracked_dps); +static struct tracked_binding_datapath *tracked_binding_datapath_find( + struct hmap *, const struct sbrec_datapath_binding *); +static void tracked_binding_datapath_lport_add( + const struct sbrec_port_binding *, bool deleted, + struct hmap *tracked_datapaths); + 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 *tracked_datapaths) { uint32_t dp_key = datapath->tunnel_key; struct local_datapath *ld = get_local_datapath(local_datapaths, dp_key); @@ -92,6 +102,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 (tracked_datapaths && + !tracked_binding_datapath_find(tracked_datapaths, datapath)) { + tracked_binding_datapath_create(datapath, true, tracked_datapaths); + } + 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 +139,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, + tracked_datapaths); } ld->n_peer_ports++; if (ld->n_peer_ports > ld->n_allocated_peer_ports) { @@ -147,12 +163,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 *tracked_datapaths) { 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, + tracked_datapaths); } static void @@ -473,23 +491,45 @@ update_ld_localnet_port(const struct sbrec_port_binding *binding_rec, static void update_local_lport_ids(struct sset *local_lport_ids, - const struct sbrec_port_binding *pb) + const struct sbrec_port_binding *pb, + struct hmap *tracked_datapaths) { char buf[16]; snprintf(buf, sizeof(buf), "%"PRId64"_%"PRId64, pb->datapath->tunnel_key, pb->tunnel_key); - sset_add(local_lport_ids, buf); + bool added = sset_add(local_lport_ids, buf); + if (added && tracked_datapaths) { + /* Add the 'pb' to the tracked_datapaths. */ + tracked_binding_datapath_lport_add(pb, false, tracked_datapaths); + } } static void -remove_local_lport_ids(const struct sbrec_port_binding *binding_rec, - struct sset *local_lport_ids) +remove_local_lport_ids(const struct sbrec_port_binding *pb, + struct sset *local_lport_ids, + struct hmap *tracked_datapaths) { 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); + pb->datapath->tunnel_key, pb->tunnel_key); + bool deleted = sset_find_and_delete(local_lport_ids, buf); + if (deleted && tracked_datapaths) { + /* Add the 'pb' to the tracked_datapaths. */ + tracked_binding_datapath_lport_add(pb, true, tracked_datapaths); + } +} + +static bool +add_lport_to_local_lports(struct sset *local_lports, const char *lport_name) +{ + return !!sset_add(local_lports, lport_name); +} + +static bool +remove_lport_from_local_lports(struct sset *local_lports, + const char *lport_name) +{ + return sset_find_and_delete(local_lports, lport_name); } /* Local bindings. binding.c module binds the logical port (represented by @@ -637,6 +677,77 @@ is_lport_container(const struct sbrec_port_binding *pb) return is_lport_vif(pb) && pb->parent_port && pb->parent_port[0]; } +static struct tracked_binding_datapath * +tracked_binding_datapath_create(const struct sbrec_datapath_binding *dp, + bool is_new, + struct hmap *tracked_datapaths) +{ + struct tracked_binding_datapath *t_dp = xzalloc(sizeof *t_dp); + t_dp->dp = dp; + t_dp->is_new = is_new; + shash_init(&t_dp->lports); + 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(const struct sbrec_port_binding *pb, + bool deleted, + struct hmap *tracked_datapaths) +{ + 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(pb->datapath, false, + tracked_datapaths); + } + + /* Check if the lport is already present or not. + * If it is already present, then just update the 'pb' and 'deleted' + * fields. */ + struct tracked_binding_lport *lport = + shash_find_data(&tracked_dp->lports, pb->logical_port); + + if (!lport) { + lport = xmalloc(sizeof *lport); + shash_add(&tracked_dp->lports, pb->logical_port, lport); + } + + lport->pb = pb; + lport->deleted = deleted; +} + +void +binding_tracked_dp_destroy(struct hmap *tracked_datapaths) +{ + struct tracked_binding_datapath *t_dp; + HMAP_FOR_EACH_POP (t_dp, node, tracked_datapaths) { + shash_destroy_free_data(&t_dp->lports); + free(t_dp); + } + + hmap_destroy(tracked_datapaths); +} + /* Corresponds to each Port_Binding.type. */ enum en_lport_type { LP_UNKNOWN, @@ -796,7 +907,8 @@ is_lbinding_container_parent(struct local_binding *lbinding) static bool release_local_binding_children(const struct sbrec_chassis *chassis_rec, struct local_binding *lbinding, - bool sb_readonly) + bool sb_readonly, + struct hmap *tracked_dp_bindings) { struct shash_node *node; SHASH_FOR_EACH (node, &lbinding->children) { @@ -807,6 +919,11 @@ release_local_binding_children(const struct sbrec_chassis *chassis_rec, } } + if (tracked_dp_bindings) { + tracked_binding_datapath_lport_add(l->pb, true, + tracked_dp_bindings); + } + /* Clear the local bindings' 'pb' and 'iface'. */ l->pb = NULL; l->iface = NULL; @@ -817,10 +934,11 @@ release_local_binding_children(const struct sbrec_chassis *chassis_rec, static bool release_local_binding(const struct sbrec_chassis *chassis_rec, - struct local_binding *lbinding, bool sb_readonly) + struct local_binding *lbinding, bool sb_readonly, + struct hmap *tracked_dp_bindings) { if (!release_local_binding_children(chassis_rec, lbinding, - sb_readonly)) { + sb_readonly, tracked_dp_bindings)) { return false; } @@ -828,6 +946,11 @@ release_local_binding(const struct sbrec_chassis *chassis_rec, return release_lport(lbinding->pb, sb_readonly); } + if (tracked_dp_bindings) { + tracked_binding_datapath_lport_add(lbinding->pb, true, + tracked_dp_bindings); + } + lbinding->pb = NULL; lbinding->iface = NULL; return true; @@ -854,8 +977,10 @@ consider_vif_lport_(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); - update_local_lport_ids(b_ctx_out->local_lport_ids, pb); + b_ctx_out->local_datapaths, + b_ctx_out->tracked_dp_bindings); + update_local_lport_ids(b_ctx_out->local_lport_ids, pb, + b_ctx_out->tracked_dp_bindings); if (lbinding->iface && qos_map && b_ctx_in->ovs_idl_txn) { get_qos_params(pb, qos_map); } @@ -1031,14 +1156,16 @@ consider_nonvif_lport_(const struct sbrec_port_binding *pb, struct binding_ctx_out *b_ctx_out) { if (our_chassis) { - sset_add(b_ctx_out->local_lports, pb->logical_port); + add_lport_to_local_lports(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, has_local_l3gateway, - b_ctx_out->local_datapaths); + b_ctx_out->local_datapaths, + b_ctx_out->tracked_dp_bindings); - update_local_lport_ids(b_ctx_out->local_lport_ids, pb); + update_local_lport_ids(b_ctx_out->local_lport_ids, pb, + b_ctx_out->tracked_dp_bindings); return claim_lport(pb, b_ctx_in->chassis_rec, NULL, !b_ctx_in->ovnsb_idl_txn); } else if (pb->chassis == b_ctx_in->chassis_rec) { @@ -1078,14 +1205,15 @@ consider_localnet_lport(const struct sbrec_port_binding *pb, struct binding_ctx_out *b_ctx_out, struct hmap *qos_map) { - /* Add all localnet ports to local_lports so that we allocate ct zones + /* Add all localnet ports to local_ifaces so that we allocate ct zones * for them. */ - sset_add(b_ctx_out->local_lports, pb->logical_port); + add_lport_to_local_lports(b_ctx_out->local_lports, pb->logical_port); if (qos_map && b_ctx_in->ovs_idl_txn) { get_qos_params(pb, qos_map); } - update_local_lport_ids(b_ctx_out->local_lport_ids, pb); + update_local_lport_ids(b_ctx_out->local_lport_ids, pb, + b_ctx_out->tracked_dp_bindings); } static bool @@ -1113,7 +1241,8 @@ consider_ha_lport(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); } return consider_nonvif_lport_(pb, our_chassis, false, b_ctx_in, b_ctx_out); @@ -1179,7 +1308,7 @@ build_local_bindings(struct binding_ctx_in *b_ctx_in, ovs_assert(lbinding->type == BT_VIF); } - sset_add(b_ctx_out->local_lports, iface_id); + add_lport_to_local_lports(b_ctx_out->local_lports, iface_id); smap_replace(b_ctx_out->local_iface_ids, iface_rec->name, iface_id); } @@ -1235,7 +1364,7 @@ binding_run(struct binding_ctx_in *b_ctx_in, struct binding_ctx_out *b_ctx_out) case LP_PATCH: case LP_LOCALPORT: case LP_VTEP: - update_local_lport_ids(b_ctx_out->local_lport_ids, pb); + update_local_lport_ids(b_ctx_out->local_lport_ids, pb, NULL); break; case LP_VIF: @@ -1403,7 +1532,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; } @@ -1465,7 +1595,8 @@ remove_pb_from_local_datapath(const struct sbrec_port_binding *pb, struct binding_ctx_out *b_ctx_out, struct local_datapath *ld) { - remove_local_lport_ids(pb, b_ctx_out->local_lport_ids); + remove_local_lport_ids(pb, b_ctx_out->local_lport_ids, + b_ctx_out->tracked_dp_bindings); if (!strcmp(pb->type, "patch") || !strcmp(pb->type, "l3gateway")) { remove_local_datapath_peer_port(pb, ld, b_ctx_out->local_datapaths); @@ -1483,6 +1614,18 @@ remove_pb_from_local_datapath(const struct sbrec_port_binding *pb, } } +static void +update_lport_tracking(const struct sbrec_port_binding *pb, + bool old_claim, bool new_claim, + struct hmap *tracked_dp_bindings) +{ + if (!tracked_dp_bindings || !pb || (old_claim == new_claim)) { + return; + } + + tracked_binding_datapath_lport_add(pb, old_claim, tracked_dp_bindings); +} + /* Considers the ovs iface 'iface_rec' for claiming. * This function should be called if the external_ids:iface-id * and 'ofport' are set for the 'iface_rec'. @@ -1495,9 +1638,13 @@ consider_iface_claim(const struct ovsrec_interface *iface_rec, const char *iface_id, struct binding_ctx_in *b_ctx_in, struct binding_ctx_out *b_ctx_out, - struct hmap *qos_map) + struct hmap *qos_map, + bool *local_lports_changed) { - sset_add(b_ctx_out->local_lports, iface_id); + if (add_lport_to_local_lports(b_ctx_out->local_lports, iface_id)) { + *local_lports_changed = true; + } + smap_replace(b_ctx_out->local_iface_ids, iface_rec->name, iface_id); struct local_binding *lbinding = @@ -1518,6 +1665,7 @@ consider_iface_claim(const struct ovsrec_interface *iface_rec, } } + bool claimed = is_lbinding_this_chassis(lbinding, b_ctx_in->chassis_rec); if (lbinding->pb) { if (!consider_vif_lport(lbinding->pb, b_ctx_in, b_ctx_out, lbinding, qos_map)) { @@ -1525,6 +1673,10 @@ consider_iface_claim(const struct ovsrec_interface *iface_rec, } } + bool now_claimed = + is_lbinding_this_chassis(lbinding, b_ctx_in->chassis_rec); + update_lport_tracking(lbinding->pb, claimed, now_claimed, + b_ctx_out->tracked_dp_bindings); /* Update the child local_binding's iface (if any children) and try to * claim the container lbindings. */ struct shash_node *node; @@ -1532,10 +1684,15 @@ consider_iface_claim(const struct ovsrec_interface *iface_rec, struct local_binding *child = node->data; child->iface = iface_rec; if (child->type == BT_CONTAINER) { + claimed = is_lbinding_this_chassis(child, b_ctx_in->chassis_rec); if (!consider_container_lport(child->pb, b_ctx_in, b_ctx_out, qos_map)) { return false; } + now_claimed = + is_lbinding_this_chassis(child, b_ctx_in->chassis_rec); + update_lport_tracking(child->pb, claimed, now_claimed, + b_ctx_out->tracked_dp_bindings); } } @@ -1560,7 +1717,8 @@ static bool consider_iface_release(const struct ovsrec_interface *iface_rec, const char *iface_id, struct binding_ctx_in *b_ctx_in, - struct binding_ctx_out *b_ctx_out, bool *changed) + struct binding_ctx_out *b_ctx_out, bool *changed, + bool *local_lports_changed) { struct local_binding *lbinding; lbinding = local_binding_find(b_ctx_out->local_bindings, @@ -1568,7 +1726,8 @@ consider_iface_release(const struct ovsrec_interface *iface_rec, if (is_lbinding_this_chassis(lbinding, b_ctx_in->chassis_rec)) { if (!release_local_binding(b_ctx_in->chassis_rec, lbinding, - !b_ctx_in->ovnsb_idl_txn)) { + !b_ctx_in->ovnsb_idl_txn, + b_ctx_out->tracked_dp_bindings)) { return false; } @@ -1589,7 +1748,9 @@ consider_iface_release(const struct ovsrec_interface *iface_rec, *changed = true; } - sset_find_and_delete(b_ctx_out->local_lports, iface_id); + if (remove_lport_from_local_lports(b_ctx_out->local_lports, iface_id)) { + *local_lports_changed = true; + } smap_remove(b_ctx_out->local_iface_ids, iface_rec->name); return true; @@ -1621,6 +1782,8 @@ binding_handle_ovs_interface_changes(struct binding_ctx_in *b_ctx_in, bool handled = true; *changed = false; + *b_ctx_out->local_lports_changed = false; + /* Run the tracked interfaces loop twice. One to handle deleted * changes. And another to handle add/update changes. * This will ensure correctness. @@ -1676,7 +1839,8 @@ binding_handle_ovs_interface_changes(struct binding_ctx_in *b_ctx_in, if (cleared_iface_id) { handled = consider_iface_release(iface_rec, cleared_iface_id, - b_ctx_in, b_ctx_out, changed); + b_ctx_in, b_ctx_out, changed, + b_ctx_out->local_lports_changed); } if (!handled) { @@ -1718,7 +1882,8 @@ binding_handle_ovs_interface_changes(struct binding_ctx_in *b_ctx_in, if (iface_id && ofport > 0) { *changed = true; handled = consider_iface_claim(iface_rec, iface_id, b_ctx_in, - b_ctx_out, qos_map_ptr); + b_ctx_out, qos_map_ptr, + b_ctx_out->local_lports_changed); if (!handled) { break; } @@ -1795,9 +1960,10 @@ handle_deleted_vif_lport(const struct sbrec_port_binding *pb, * clear the 'chassis' column of 'pb'. But we need to do * for the local_binding's children. */ if (lbinding->type == BT_VIF && - !release_local_binding_children(b_ctx_in->chassis_rec, - lbinding, - !b_ctx_in->ovnsb_idl_txn)) { + !release_local_binding_children( + b_ctx_in->chassis_rec, lbinding, + !b_ctx_in->ovnsb_idl_txn, + b_ctx_out->tracked_dp_bindings)) { return false; } @@ -1843,6 +2009,9 @@ handle_updated_vif_lport(const struct sbrec_port_binding *pb, return true; } + update_lport_tracking(pb, claimed, now_claimed, + b_ctx_out->tracked_dp_bindings); + struct local_binding *lbinding = local_binding_find(b_ctx_out->local_bindings, pb->logical_port); @@ -1852,11 +2021,17 @@ handle_updated_vif_lport(const struct sbrec_port_binding *pb, SHASH_FOR_EACH (node, &lbinding->children) { struct local_binding *child = node->data; if (child->type == BT_CONTAINER) { + claimed = is_lbinding_this_chassis(child, b_ctx_in->chassis_rec); handled = consider_container_lport(child->pb, b_ctx_in, b_ctx_out, qos_map); if (!handled) { return false; } + + now_claimed = is_lbinding_this_chassis(child, + b_ctx_in->chassis_rec); + update_lport_tracking(child->pb, claimed, now_claimed, + b_ctx_out->tracked_dp_bindings); } } @@ -1932,7 +2107,8 @@ binding_handle_port_binding_changes(struct binding_ctx_in *b_ctx_in, case LP_LOCALPORT: case LP_VTEP: *changed = true; - update_local_lport_ids(b_ctx_out->local_lport_ids, pb); + update_local_lport_ids(b_ctx_out->local_lport_ids, pb, + b_ctx_out->tracked_dp_bindings); if (lport_type == LP_PATCH) { /* Add the peer datapath to the local datapaths if it's * not present yet. diff --git a/controller/binding.h b/controller/binding.h index 21118ecd4..2974ebb30 100644 --- a/controller/binding.h +++ b/controller/binding.h @@ -19,6 +19,9 @@ #include #include "openvswitch/shash.h" +#include "openvswitch/hmap.h" +#include "openvswitch/uuid.h" +#include "openvswitch/list.h" struct hmap; struct ovsdb_idl; @@ -54,10 +57,28 @@ struct binding_ctx_in { struct binding_ctx_out { struct hmap *local_datapaths; struct shash *local_bindings; + struct sset *local_lports; + + /* If the sset local_lports is modified, this is set to true by + * the callee. */ + bool *local_lports_changed; + + /* sset of local lport ids in the format + * _. */ struct sset *local_lport_ids; + struct sset *egress_ifaces; + + /* smap of OVS interface name as key and + * OVS interface external_ids:iface-id as value. */ struct smap *local_iface_ids; + + /* hmap of 'struct tracked_binding_datapath' which the + * callee (binding_handle_ovs_interface_changes and + * binding_handle_port_binding_changes) fills in for + * the changed datapaths and port bindings. */ + struct hmap *tracked_dp_bindings; }; enum local_binding_type { @@ -82,6 +103,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 shash lports; /* shash of struct tracked_binding_lport. */ +}; + void binding_register_ovs_idl(struct ovsdb_idl *); void binding_run(struct binding_ctx_in *, struct binding_ctx_out *); bool binding_cleanup(struct ovsdb_idl_txn *ovnsb_idl_txn, @@ -96,4 +132,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 6381e3856..745163783 100644 --- a/controller/ovn-controller.c +++ b/controller/ovn-controller.c @@ -974,10 +974,89 @@ struct ed_type_runtime_data { struct sset local_lport_ids; struct sset active_tunnels; + /* runtime data engine private data. */ struct sset egress_ifaces; struct smap local_iface_ids; }; +/* + * This structure tracks the changes done to the runtime_data engine + * by the runtime_data engine handlers. Since this engine is an input + * to the flow_output engine, the flow output runtime data handler + * will make use of this tracked data. + * + * ------------------------------------------------------------------------ + * | | This is a hmap of | + * | | 'struct tracked_binding_datapath' defined in | + * | | binding.h. Runtime data handlers for OVS | + * | | Interface and Port Binding changes store the | + * | @tracked_dp_bindings | changed datapaths (datapaths added/removed from | + * | | local_datapaths) and changed port bindings | + * | | (added/updated/deleted in 'local_bindings'). | + * | | So any changes to the runtime data - | + * | | local_datapaths and local_bindings is captured | + * | | here. | + * ------------------------------------------------------------------------ + * | | This is a bool which represents if the runtime | + * | | data 'local_lports' changed by the runtime data | + * | | handlers for OVS Interface and Port Binding | + * | | changes. If 'local_lports' is updated and also | + * | | results in any port binding updates, it is | + * |@local_lports_changed | captured in the @tracked_dp_bindings. So there | + * | | is no need to capture the changes in the | + * | | local_lports. If @local_lports_changed is true | + * | | but without anydata in the @tracked_dp_bindings,| + * | | it means we needto only update the SB monitor | + * | | clauses and there isno need for any flow | + * | | (re)computations. | + * ------------------------------------------------------------------------ + * | | This represents if the data was tracked or not | + * | | by the runtime data handlers during the engine | + * | @tracked | run. If the runtime data recompute is | + * | | triggered, it means there is no tracked data. | + * ------------------------------------------------------------------------ + * + * + * The changes to the following runtime_data variables are not tracked. + * + * --------------------------------------------------------------------- + * | local_datapaths | The changes to these runtime data is captured in | + * | local_bindings | the @tracked_dp_bindings indirectly and hence it | + * | local_lport_ids | is not tracked explicitly. | + * --------------------------------------------------------------------- + * | local_iface_ids | This is used internally within the runtime data | + * | egress_ifaces | engine (used only in binding.c) and hence there | + * | | there is no need to track. | + * --------------------------------------------------------------------- + * | | Active tunnels is built in the | + * | | bfd_calculate_active_tunnels() for the tunnel | + * | | OVS interfaces. Any changes to non VIF OVS | + * | | interfaces results in triggering the full | + * | active_tunnels | recompute of runtime data engine and hence there | + * | | the tracked data doesn't track it. When we | + * | | support handling changes to non VIF OVS | + * | | interfaces we need to track the changes to the | + * | | active tunnels. | + * --------------------------------------------------------------------- + * + */ +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 +1070,13 @@ en_runtime_data_init(struct engine_node *node OVS_UNUSED, sset_init(&data->egress_ifaces); smap_init(&data->local_iface_ids); local_bindings_init(&data->local_bindings); + + 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 +1179,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 +1192,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 */ @@ -1156,9 +1246,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, @@ -1191,6 +1285,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)) { @@ -1912,6 +2010,30 @@ flow_output_physical_flow_changes_handler(struct engine_node *node, void *data) 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; +} + static bool flow_output_noop_handler(struct engine_node *node OVS_UNUSED, void *data OVS_UNUSED) @@ -2076,7 +2198,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/tests/ovn-performance.at b/tests/ovn-performance.at index 5cc1960b6..08acd3bae 100644 --- a/tests/ovn-performance.at +++ b/tests/ovn-performance.at @@ -286,11 +286,11 @@ for i in 1 2; do [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 May 28 11:05:05 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Numan Siddique X-Patchwork-Id: 1299660 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 49XlH850Sfz9sSJ for ; Thu, 28 May 2020 21:05:40 +1000 (AEST) Received: from localhost (localhost [127.0.0.1]) by whitealder.osuosl.org (Postfix) with ESMTP id 2359380E69; Thu, 28 May 2020 11:05:39 +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 jpRxEOivJiu3; Thu, 28 May 2020 11:05:30 +0000 (UTC) Received: from lists.linuxfoundation.org (lf-lists.osuosl.org [140.211.9.56]) by whitealder.osuosl.org (Postfix) with ESMTP id 3F51887E34; Thu, 28 May 2020 11:05:22 +0000 (UTC) Received: from lf-lists.osuosl.org (localhost [127.0.0.1]) by lists.linuxfoundation.org (Postfix) with ESMTP id 221EDC088D; Thu, 28 May 2020 11:05:22 +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 AC844C016F for ; Thu, 28 May 2020 11:05:20 +0000 (UTC) Received: from localhost (localhost [127.0.0.1]) by hemlock.osuosl.org (Postfix) with ESMTP id 9C98888C56 for ; Thu, 28 May 2020 11:05:20 +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 v3Zaysup5zKJ for ; Thu, 28 May 2020 11:05:19 +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 B508488C43 for ; Thu, 28 May 2020 11:05:18 +0000 (UTC) X-Originating-IP: 27.7.91.57 Received: from nusiddiq.home.org.com (unknown [27.7.91.57]) (Authenticated sender: numans@ovn.org) by relay7-d.mail.gandi.net (Postfix) with ESMTPSA id 53DF020007; Thu, 28 May 2020 11:05:15 +0000 (UTC) From: numans@ovn.org To: dev@openvswitch.org Date: Thu, 28 May 2020 16:35:05 +0530 Message-Id: <20200528110505.1058281-1-numans@ovn.org> X-Mailer: git-send-email 2.26.2 In-Reply-To: <20200528110344.1057873-1-numans@ovn.org> References: <20200528110344.1057873-1-numans@ovn.org> MIME-Version: 1.0 Cc: Venkata Anil Subject: [ovs-dev] [PATCH ovn v9 7/8] ovn-controller: 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. Acked-by: Mark Michelson Co-Authored-by: Numan Siddique Signed-off-by: Venkata Anil Signed-off-by: Numan Siddique --- controller/lflow.c | 69 ++++++++++++++++++++- controller/lflow.h | 13 +++- controller/ovn-controller.c | 117 +++++++++++++++++++----------------- controller/physical.h | 3 +- tests/ovn-performance.at | 12 ++-- 5 files changed, 151 insertions(+), 63 deletions(-) diff --git a/controller/lflow.c b/controller/lflow.c index 01214a3a6..8fd205c9c 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", @@ -847,3 +849,68 @@ lflow_destroy(void) expr_symtab_destroy(&symtab); shash_destroy(&symtab); } + +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 f02f709d7..c8ed5b686 100644 --- a/controller/lflow.h +++ b/controller/lflow.h @@ -48,6 +48,9 @@ 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 sbrec_datapath_binding; +struct sbrec_port_binding; struct simap; struct sset; struct uuid; @@ -72,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 @@ -117,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; @@ -157,4 +162,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 745163783..2a177ef9b 100644 --- a/controller/ovn-controller.c +++ b/controller/ovn-controller.c @@ -1502,6 +1502,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), @@ -1549,6 +1554,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; @@ -1703,7 +1710,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); @@ -1711,56 +1719,13 @@ 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. - * - * 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. - */ struct physical_ctx p_ctx; init_physical_ctx(node, rt_data, &p_ctx); + /* 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. + */ physical_handle_port_binding_changes(&p_ctx, flow_table); engine_set_node_state(node, EN_UPDATED); @@ -1848,6 +1813,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(); } @@ -2021,17 +1988,54 @@ 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; + } + + 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 shash_node *shash_node; + SHASH_FOR_EACH (shash_node, &tdp->lports) { + struct tracked_binding_lport *lport = shash_node->data; + if (!lflow_handle_flows_for_lport(lport->pb, &l_ctx_in, + &l_ctx_out)) { + handled = false; + break; + } + } + + if (!handled) { + break; + } + } } - if (tracked_data->local_lports_changed) { + if (handled) { engine_set_node_state(node, EN_UPDATED); } - return true; + return handled; } static bool @@ -2096,6 +2100,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); @@ -2255,6 +2262,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 08acd3bae..a12757e18 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,7 +282,7 @@ 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] ) @@ -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 May 28 11:05:17 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Numan Siddique X-Patchwork-Id: 1299669 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 49XlJq2ZYMz9sRY for ; Thu, 28 May 2020 21:07:06 +1000 (AEST) Received: from localhost (localhost [127.0.0.1]) by silver.osuosl.org (Postfix) with ESMTP id 1B610253E7; Thu, 28 May 2020 11:07:05 +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 WZ1KIEoVl2MD; Thu, 28 May 2020 11:06:59 +0000 (UTC) Received: from lists.linuxfoundation.org (lf-lists.osuosl.org [140.211.9.56]) by silver.osuosl.org (Postfix) with ESMTP id 59506258C2; Thu, 28 May 2020 11:06:17 +0000 (UTC) Received: from lf-lists.osuosl.org (localhost [127.0.0.1]) by lists.linuxfoundation.org (Postfix) with ESMTP id 3EB77C088D; Thu, 28 May 2020 11:06:17 +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 C0BFAC016F for ; Thu, 28 May 2020 11:06:15 +0000 (UTC) Received: from localhost (localhost [127.0.0.1]) by silver.osuosl.org (Postfix) with ESMTP id B30212580F for ; Thu, 28 May 2020 11:06:15 +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 zIDVS04EnJpK for ; Thu, 28 May 2020 11:06:10 +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 silver.osuosl.org (Postfix) with ESMTPS id E74EC2078C for ; Thu, 28 May 2020 11:05:27 +0000 (UTC) Received: from nusiddiq.home.org.com (unknown [27.7.91.57]) (Authenticated sender: numans@ovn.org) by relay12.mail.gandi.net (Postfix) with ESMTPSA id 6493B200007; Thu, 28 May 2020 11:05:22 +0000 (UTC) From: numans@ovn.org To: dev@openvswitch.org Date: Thu, 28 May 2020 16:35:17 +0530 Message-Id: <20200528110517.1058340-1-numans@ovn.org> X-Mailer: git-send-email 2.26.2 In-Reply-To: <20200528110344.1057873-1-numans@ovn.org> References: <20200528110344.1057873-1-numans@ovn.org> MIME-Version: 1.0 Subject: [ovs-dev] [PATCH ovn v9 8/8] ovn-controller: Handle sbrec_chassis changes incrementally. 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. Acked-by: Mark Michelson Signed-off-by: Numan Siddique --- controller/ovn-controller.c | 50 ++++++++++++++++++++++++++++++------- 1 file changed, 41 insertions(+), 9 deletions(-) diff --git a/controller/ovn-controller.c b/controller/ovn-controller.c index 2a177ef9b..56744a39e 100644 --- a/controller/ovn-controller.c +++ b/controller/ovn-controller.c @@ -1461,6 +1461,7 @@ static void init_physical_ctx(struct engine_node *node, engine_ovsdb_node_get_index( engine_get_input("SB_chassis", node), "name"); + if (chassis_id) { chassis = chassis_lookup_by_name(sbrec_chassis_by_name, chassis_id); } @@ -1536,8 +1537,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), - "name"); + engine_get_input("SB_chassis", node), + "name"); if (chassis_id) { chassis = chassis_lookup_by_name(sbrec_chassis_by_name, chassis_id); } @@ -1617,8 +1618,8 @@ 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", node), + "name"); const struct sbrec_chassis *chassis = NULL; if (chassis_id) { @@ -1775,8 +1776,8 @@ _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", node), + "name"); const struct sbrec_chassis *chassis = NULL; if (chassis_id) { chassis = chassis_lookup_by_name(sbrec_chassis_by_name, chassis_id); @@ -1948,6 +1949,31 @@ physical_flow_changes_ovs_iface_handler(struct engine_node *node, 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_physical_flow_changes_handler(struct engine_node *node, void *data) { @@ -2200,6 +2226,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); @@ -2218,9 +2248,10 @@ 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, + flow_output_noop_handler); + 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, @@ -2247,7 +2278,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,