From patchwork Wed May 20 19:39:41 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Numan Siddique X-Patchwork-Id: 1294686 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 49S34T6QV1z9sT9 for ; Thu, 21 May 2020 05:40:09 +1000 (AEST) Received: from localhost (localhost [127.0.0.1]) by silver.osuosl.org (Postfix) with ESMTP id 07FEC251C1; Wed, 20 May 2020 19:40:08 +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 N1CDKOKEuthO; Wed, 20 May 2020 19:39:57 +0000 (UTC) Received: from lists.linuxfoundation.org (lf-lists.osuosl.org [140.211.9.56]) by silver.osuosl.org (Postfix) with ESMTP id C0F9724DD1; Wed, 20 May 2020 19:39:57 +0000 (UTC) Received: from lf-lists.osuosl.org (localhost [127.0.0.1]) by lists.linuxfoundation.org (Postfix) with ESMTP id AF690C07FF; Wed, 20 May 2020 19:39:57 +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 6E64AC0176 for ; Wed, 20 May 2020 19:39:56 +0000 (UTC) Received: from localhost (localhost [127.0.0.1]) by hemlock.osuosl.org (Postfix) with ESMTP id 5216D88BC9 for ; Wed, 20 May 2020 19:39:56 +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 PAW2yK8E7-Je for ; Wed, 20 May 2020 19:39:53 +0000 (UTC) X-Greylist: domain auto-whitelisted by SQLgrey-1.7.6 Received: from relay12.mail.gandi.net (relay12.mail.gandi.net [217.70.178.232]) by hemlock.osuosl.org (Postfix) with ESMTPS id F091A88C40 for ; Wed, 20 May 2020 19:39:52 +0000 (UTC) Received: from nusiddiq.home.org.home.org (unknown [125.99.233.239]) (Authenticated sender: numans@ovn.org) by relay12.mail.gandi.net (Postfix) with ESMTPSA id CC73A200002; Wed, 20 May 2020 19:39:49 +0000 (UTC) From: numans@ovn.org To: dev@openvswitch.org Date: Thu, 21 May 2020 01:09:41 +0530 Message-Id: <20200520193941.2351769-1-numans@ovn.org> X-Mailer: git-send-email 2.26.2 In-Reply-To: <20200520193918.2351632-1-numans@ovn.org> References: <20200520193918.2351632-1-numans@ovn.org> MIME-Version: 1.0 Subject: [ovs-dev] [PATCH ovn v7 1/9] 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 | 1012 ++++++++++++++++++++++++----------- controller/binding.h | 14 +- controller/ovn-controller.c | 48 +- tests/ovn.at | 36 ++ 4 files changed, 754 insertions(+), 356 deletions(-) diff --git a/controller/binding.c b/controller/binding.c index 79dc046d9..2997fcae8 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, @@ -448,219 +407,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 +482,588 @@ 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 is 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 !pb->type[0] && 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) +{ + if (!pb) { + return; + } + + 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 +1071,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 +1211,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 11fa2840d..6b759966b 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 4370b3728..84376b3d6 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 Wed May 20 19:39:51 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Numan Siddique X-Patchwork-Id: 1294695 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 49S36Y6vLTz9sT9 for ; Thu, 21 May 2020 05:41:57 +1000 (AEST) Received: from localhost (localhost [127.0.0.1]) by silver.osuosl.org (Postfix) with ESMTP id 3685924723; Wed, 20 May 2020 19:41:56 +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 SOa4+6XoPler; Wed, 20 May 2020 19:41:22 +0000 (UTC) Received: from lists.linuxfoundation.org (lf-lists.osuosl.org [140.211.9.56]) by silver.osuosl.org (Postfix) with ESMTP id 4C727251E1; Wed, 20 May 2020 19:40:38 +0000 (UTC) Received: from lf-lists.osuosl.org (localhost [127.0.0.1]) by lists.linuxfoundation.org (Postfix) with ESMTP id 23598C07FF; Wed, 20 May 2020 19:40:38 +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 A8702C0176 for ; Wed, 20 May 2020 19:40:36 +0000 (UTC) Received: from localhost (localhost [127.0.0.1]) by silver.osuosl.org (Postfix) with ESMTP id 7271F2561C for ; Wed, 20 May 2020 19:40:36 +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 KDRPKJEaobYo for ; Wed, 20 May 2020 19:40:08 +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 4D99C25067 for ; Wed, 20 May 2020 19:39:58 +0000 (UTC) Received: from nusiddiq.home.org.home.org (unknown [125.99.233.239]) (Authenticated sender: numans@ovn.org) by relay12.mail.gandi.net (Postfix) with ESMTPSA id 18E33200005; Wed, 20 May 2020 19:39:54 +0000 (UTC) From: numans@ovn.org To: dev@openvswitch.org Date: Thu, 21 May 2020 01:09:51 +0530 Message-Id: <20200520193951.2351827-1-numans@ovn.org> X-Mailer: git-send-email 2.26.2 In-Reply-To: <20200520193918.2351632-1-numans@ovn.org> References: <20200520193918.2351632-1-numans@ovn.org> MIME-Version: 1.0 Subject: [ovs-dev] [PATCH ovn v7 2/9] 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 | 906 +++++++++++++++++++++++++++++++----- controller/binding.h | 9 +- controller/ovn-controller.c | 61 ++- tests/ovn-performance.at | 13 + 4 files changed, 855 insertions(+), 134 deletions(-) diff --git a/controller/binding.c b/controller/binding.c index 2997fcae8..d5e8e4773 100644 --- a/controller/binding.c +++ b/controller/binding.c @@ -360,17 +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 *binding_rec) -{ - char buf[16]; - snprintf(buf, sizeof(buf), "%"PRId64"_%"PRId64, - binding_rec->datapath->tunnel_key, - binding_rec->tunnel_key); - sset_add(local_lport_ids, buf); -} - /* * Get the encap from the chassis for this port. The interface * may have an external_ids:encap-ip= set; if so we @@ -449,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)) { @@ -482,6 +471,28 @@ consider_localnet_port(const struct sbrec_port_binding *binding_rec, ld->localnet_port = binding_rec; } +static void +update_local_lport_ids(struct sset *local_lport_ids, + const struct sbrec_port_binding *binding_rec) +{ + char buf[16]; + snprintf(buf, sizeof(buf), "%"PRId64"_%"PRId64, + binding_rec->datapath->tunnel_key, + binding_rec->tunnel_key); + sset_add(local_lport_ids, buf); +} + +static void +remove_local_lport_ids(const struct sbrec_port_binding *binding_rec, + struct sset *local_lport_ids) +{ + char buf[16]; + snprintf(buf, sizeof(buf), "%"PRId64"_%"PRId64, + binding_rec->datapath->tunnel_key, + binding_rec->tunnel_key); + sset_find_and_delete(local_lport_ids, buf); +} + /* Local bindings. binding.c module binds the logical port (represented by * Port_Binding rows) and sets the 'chassis' column when it is sees the * OVS interface row (of type "" or "internal") with the @@ -599,6 +610,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) @@ -625,6 +644,7 @@ is_lport_container(const struct sbrec_port_binding *pb) return !pb->type[0] && pb->parent_port && pb->parent_port[0]; } +/* Corresponds to each Port_Binding.type. */ enum en_lport_type { LP_UNKNOWN, LP_VIF, @@ -670,12 +690,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, @@ -694,26 +722,48 @@ 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) { if (!pb) { - return; + return true; } - 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 @@ -738,7 +788,41 @@ can_bind_on_this_chassis(const struct sbrec_chassis *chassis_rec, || !strcmp(requested_chassis, chassis_rec->hostname); } -static void +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; + } + } + } + + 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); + } + + 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, @@ -750,7 +834,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, @@ -775,13 +862,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, @@ -801,11 +889,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, @@ -815,7 +903,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); @@ -824,37 +944,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, @@ -881,11 +992,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, @@ -895,14 +1011,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, @@ -912,13 +1027,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) @@ -927,10 +1045,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) @@ -939,7 +1057,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 @@ -958,7 +1076,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) @@ -986,18 +1104,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) @@ -1050,6 +1168,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. */ @@ -1165,9 +1285,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); } @@ -1185,95 +1305,625 @@ 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; +} + +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; + } + + for (size_t i = 0; i < peer_ld->n_peer_ports; i++) { + if (peer_ld->peer_ports[i].local == peer) { + return; } + } - if (strcmp(binding_rec->type, "")) { - changed = true; + 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_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; + } + } +} + +static bool +handle_iface_add(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; +} + +static bool +handle_iface_delete(const char *lport_name, const char *iface_name, + 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, + lport_name); + if (lbinding && lbinding->pb && + lbinding->pb->chassis == 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); + } + local_binding_delete(b_ctx_out->local_bindings, lbinding); + *changed = true; + } + + sset_find_and_delete(b_ctx_out->local_lports, lport_name); + smap_remove(b_ctx_out->local_iface_ids, iface_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. + */ + 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)) { + if (iface_id) { + /* Check if iface_id is changed. If so we need to + * release the old port binding and associate this + * inteface to new port binding. */ + if (old_iface_id && strcmp(iface_id, old_iface_id)) { + cleared_iface_id = old_iface_id; + } + } else if (old_iface_id) { + cleared_iface_id = old_iface_id; + } } else { - lbinding = local_binding_find(b_ctx_out->local_bindings, - binding_rec->parent_port); + cleared_iface_id = iface_id; + iface_id = NULL; } - if (lbinding) { - changed = true; + if (cleared_iface_id) { + handled = handle_iface_delete(cleared_iface_id, iface_rec->name, + b_ctx_in, b_ctx_out, changed); + } + + if (!handled) { break; } } - return changed; + 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; + + 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 = handle_iface_add(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 6b759966b..08b074415 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. */ @@ -1894,7 +1944,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 Wed May 20 19:39:57 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Numan Siddique X-Patchwork-Id: 1294688 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 49S3502dVJz9sTR for ; Thu, 21 May 2020 05:40:36 +1000 (AEST) Received: from localhost (localhost [127.0.0.1]) by silver.osuosl.org (Postfix) with ESMTP id C652D252CA; Wed, 20 May 2020 19:40:34 +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 iBjjs8N4sayk; Wed, 20 May 2020 19:40:28 +0000 (UTC) Received: from lists.linuxfoundation.org (lf-lists.osuosl.org [140.211.9.56]) by silver.osuosl.org (Postfix) with ESMTP id D2E3C2517C; Wed, 20 May 2020 19:40:15 +0000 (UTC) Received: from lf-lists.osuosl.org (localhost [127.0.0.1]) by lists.linuxfoundation.org (Postfix) with ESMTP id BA029C088C; Wed, 20 May 2020 19:40:15 +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 E1EE9C0176 for ; Wed, 20 May 2020 19:40:13 +0000 (UTC) Received: from localhost (localhost [127.0.0.1]) by silver.osuosl.org (Postfix) with ESMTP id CF7DD25173 for ; Wed, 20 May 2020 19:40:13 +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 OFSH5yLSndgC for ; Wed, 20 May 2020 19:40:09 +0000 (UTC) X-Greylist: domain auto-whitelisted by SQLgrey-1.7.6 Received: from relay3-d.mail.gandi.net (relay3-d.mail.gandi.net [217.70.183.195]) by silver.osuosl.org (Postfix) with ESMTPS id 4E55625175 for ; Wed, 20 May 2020 19:40:04 +0000 (UTC) X-Originating-IP: 125.99.233.239 Received: from nusiddiq.home.org.home.org (unknown [125.99.233.239]) (Authenticated sender: numans@ovn.org) by relay3-d.mail.gandi.net (Postfix) with ESMTPSA id E5A9D60005; Wed, 20 May 2020 19:39:59 +0000 (UTC) From: numans@ovn.org To: dev@openvswitch.org Date: Thu, 21 May 2020 01:09:57 +0530 Message-Id: <20200520193957.2351877-1-numans@ovn.org> X-Mailer: git-send-email 2.26.2 In-Reply-To: <20200520193918.2351632-1-numans@ovn.org> References: <20200520193918.2351632-1-numans@ovn.org> MIME-Version: 1.0 Subject: [ovs-dev] [PATCH ovn v7 3/9] 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 Signed-off-by: Numan Siddique Acked-by: Han Zhou --- 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 08b074415..9ad5be1c6 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)]; @@ -1951,7 +1973,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 Wed May 20 19:40:01 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Numan Siddique X-Patchwork-Id: 1294687 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 49S34Z5vMqz9sT9 for ; Thu, 21 May 2020 05:40:14 +1000 (AEST) Received: from localhost (localhost [127.0.0.1]) by hemlock.osuosl.org (Postfix) with ESMTP id 270DC88C6C; Wed, 20 May 2020 19:40:13 +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 XS8Fm-kuvNpe; Wed, 20 May 2020 19:40:12 +0000 (UTC) Received: from lists.linuxfoundation.org (lf-lists.osuosl.org [140.211.9.56]) by hemlock.osuosl.org (Postfix) with ESMTP id E7DB388C43; Wed, 20 May 2020 19:40:11 +0000 (UTC) Received: from lf-lists.osuosl.org (localhost [127.0.0.1]) by lists.linuxfoundation.org (Postfix) with ESMTP id CE647C07FF; Wed, 20 May 2020 19:40:11 +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 C6544C0176 for ; Wed, 20 May 2020 19:40:09 +0000 (UTC) Received: from localhost (localhost [127.0.0.1]) by whitealder.osuosl.org (Postfix) with ESMTP id C253086B48 for ; Wed, 20 May 2020 19:40:09 +0000 (UTC) X-Virus-Scanned: amavisd-new at osuosl.org Received: from whitealder.osuosl.org ([127.0.0.1]) by localhost (.osuosl.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id x4jp6k5cKRtI for ; Wed, 20 May 2020 19:40:08 +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 whitealder.osuosl.org (Postfix) with ESMTPS id 3F87A88366 for ; Wed, 20 May 2020 19:40:08 +0000 (UTC) Received: from nusiddiq.home.org.home.org (unknown [125.99.233.239]) (Authenticated sender: numans@ovn.org) by relay12.mail.gandi.net (Postfix) with ESMTPSA id 46A50200005; Wed, 20 May 2020 19:40:03 +0000 (UTC) From: numans@ovn.org To: dev@openvswitch.org Date: Thu, 21 May 2020 01:10:01 +0530 Message-Id: <20200520194001.2351927-1-numans@ovn.org> X-Mailer: git-send-email 2.26.2 In-Reply-To: <20200520193918.2351632-1-numans@ovn.org> References: <20200520193918.2351632-1-numans@ovn.org> MIME-Version: 1.0 Subject: [ovs-dev] [PATCH ovn v7 4/9] ofctrl: Replace the actions of an existing flow if actions have changed. X-BeenThere: ovs-dev@openvswitch.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: ovs-dev-bounces@openvswitch.org Sender: "dev" From: Numan Siddique If ofctrl_check_and_add_flow(F') is called where flow F' has match-actions (M, A2) and if there already exists a flow F with match-actions (M, A1) in the desired flow table, then (1) If F and F' share the same 'sb_uuid', this function doesn't update the existing flow F with new actions A2. (2) If F and F' don't share the same 'sb_uuid', this function adds F' also into the flow table with F and F' having the same hash. While installing the flows only one will be considered. This patch fixes (1) by updating the F with the new actions A2. This is required for the upcoming patch in this series which adds incremental processing for OVS interface in the flow output stage. Since we will be not be clearing the flow output data if these changes are handled incrementally, some of the existing flows will be updated with new actions. One such example would be flows in physical table OFTABLE_LOCAL_OUTPUT (table 33). This patch is required to update such flows. Otherwise we will have incomplete actions installed. We should also address (2) and it should be fixed - by logging the duplicate flow F' - and discarding F'. Scenario (2) can happen when - either ovn-northd installs 2 logical flows with same match but with different actions due to some bug in ovn-northd - CMS adds 2 ACLs with the same match and priority, but with different actions. However this patch doesn't attempt to fix (2). Acked-by: Mark Michelson Signed-off-by: Numan Siddique --- controller/ofctrl.c | 34 +++++++++++++++++++++++++++------- 1 file changed, 27 insertions(+), 7 deletions(-) diff --git a/controller/ofctrl.c b/controller/ofctrl.c index b8a9c2da8..edc22824f 100644 --- a/controller/ofctrl.c +++ b/controller/ofctrl.c @@ -642,6 +642,19 @@ ofctrl_recv(const struct ofp_header *oh, enum ofptype type) } } +static void +ofctrl_swap_flow_actions(struct ovn_flow *a, struct ovn_flow *b) +{ + struct ofpact *tmp = a->ofpacts; + size_t tmp_len = a->ofpacts_len; + + a->ofpacts = b->ofpacts; + a->ofpacts_len = b->ofpacts_len; + + b->ofpacts = tmp; + b->ofpacts_len = tmp_len; +} + /* Flow table interfaces to the rest of ovn-controller. */ /* Adds a flow to 'desired_flows' with the specified 'match' and 'actions' to @@ -667,14 +680,21 @@ ofctrl_check_and_add_flow(struct ovn_desired_flow_table *flow_table, ovn_flow_log(f, "ofctrl_add_flow"); - if (ovn_flow_lookup(&flow_table->match_flow_table, f, true)) { - if (log_duplicate_flow) { - static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 5); - if (!VLOG_DROP_DBG(&rl)) { - char *s = ovn_flow_to_string(f); - VLOG_DBG("dropping duplicate flow: %s", s); - free(s); + struct ovn_flow *existing_f = + ovn_flow_lookup(&flow_table->match_flow_table, f, true); + if (existing_f) { + if (ofpacts_equal(f->ofpacts, f->ofpacts_len, + existing_f->ofpacts, existing_f->ofpacts_len)) { + if (log_duplicate_flow) { + static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 5); + if (!VLOG_DROP_DBG(&rl)) { + char *s = ovn_flow_to_string(f); + VLOG_DBG("dropping duplicate flow: %s", s); + free(s); + } } + } else { + ofctrl_swap_flow_actions(f, existing_f); } ovn_flow_destroy(f); return; From patchwork Wed May 20 19:40:07 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Numan Siddique X-Patchwork-Id: 1294691 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 49S35l0jfrz9sT9 for ; Thu, 21 May 2020 05:41:15 +1000 (AEST) Received: from localhost (localhost [127.0.0.1]) by whitealder.osuosl.org (Postfix) with ESMTP id 68CDA88367; Wed, 20 May 2020 19:41:13 +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 vllGDKxfcnD4; Wed, 20 May 2020 19:41:05 +0000 (UTC) Received: from lists.linuxfoundation.org (lf-lists.osuosl.org [140.211.9.56]) by whitealder.osuosl.org (Postfix) with ESMTP id A9824883D3; Wed, 20 May 2020 19:40:41 +0000 (UTC) Received: from lf-lists.osuosl.org (localhost [127.0.0.1]) by lists.linuxfoundation.org (Postfix) with ESMTP id 7B3D3C07FF; Wed, 20 May 2020 19:40:41 +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 A25DFC0176 for ; Wed, 20 May 2020 19:40:37 +0000 (UTC) Received: from localhost (localhost [127.0.0.1]) by silver.osuosl.org (Postfix) with ESMTP id 92F5725067 for ; Wed, 20 May 2020 19:40:37 +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 HOmiQtLeKKf5 for ; Wed, 20 May 2020 19:40:24 +0000 (UTC) X-Greylist: domain auto-whitelisted by SQLgrey-1.7.6 Received: from relay9-d.mail.gandi.net (relay9-d.mail.gandi.net [217.70.183.199]) by silver.osuosl.org (Postfix) with ESMTPS id B8D8F25351 for ; Wed, 20 May 2020 19:40:14 +0000 (UTC) X-Originating-IP: 125.99.233.239 Received: from nusiddiq.home.org.home.org (unknown [125.99.233.239]) (Authenticated sender: numans@ovn.org) by relay9-d.mail.gandi.net (Postfix) with ESMTPSA id 84DBCFF804; Wed, 20 May 2020 19:40:10 +0000 (UTC) From: numans@ovn.org To: dev@openvswitch.org Date: Thu, 21 May 2020 01:10:07 +0530 Message-Id: <20200520194007.2351985-1-numans@ovn.org> X-Mailer: git-send-email 2.26.2 In-Reply-To: <20200520193918.2351632-1-numans@ovn.org> References: <20200520193918.2351632-1-numans@ovn.org> MIME-Version: 1.0 Subject: [ovs-dev] [PATCH ovn v7 5/9] 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 | 82 ++++++++++++++++++++++++++++++++++++- controller/physical.c | 51 +++++++++++++++++++++++ controller/physical.h | 5 ++- 5 files changed, 159 insertions(+), 26 deletions(-) diff --git a/controller/binding.c b/controller/binding.c index d5e8e4773..f00c975ce 100644 --- a/controller/binding.c +++ b/controller/binding.c @@ -504,7 +504,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 @@ -540,21 +540,6 @@ remove_local_lport_ids(const struct sbrec_port_binding *binding_rec, * 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, @@ -576,12 +561,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 9ad5be1c6..8f95fff1f 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", + engine_get_input("physical_flow_changes", node))); struct ed_type_ct_zones *ct_zones_data = - engine_get_input_data("ct_zones", node); + engine_get_input_data("ct_zones", + engine_get_input("physical_flow_changes", node)); struct simap *ct_zones = &ct_zones_data->current; p_ctx->sbrec_port_binding_by_name = sbrec_port_binding_by_name; @@ -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,70 @@ flow_output_port_groups_handler(struct engine_node *node, void *data) return _flow_output_resource_ref_handler(node, data, REF_TYPE_PORTGROUP); } +struct ed_type_physical_flow_changes { + bool need_physical_run; + bool ovs_ifaces_changed; +}; + +static bool +flow_output_physical_flow_changes_handler(struct engine_node *node, void *data) +{ + struct ed_type_physical_flow_changes *pfc = + engine_get_input_data("physical_flow_changes", node); + struct ed_type_runtime_data *rt_data = + engine_get_input_data("runtime_data", node); + + struct ed_type_flow_output *fo = data; + struct physical_ctx p_ctx; + init_physical_ctx(node, rt_data, &p_ctx); + + engine_set_node_state(node, EN_UPDATED); + if (pfc->need_physical_run) { + pfc->need_physical_run = false; + physical_run(&p_ctx, &fo->flow_table); + return true; + } + + if (pfc->ovs_ifaces_changed) { + pfc->ovs_ifaces_changed = false; + return physical_handle_ovs_iface_changes(&p_ctx, &fo->flow_table); + } + + return true; +} + +static void * +en_physical_flow_changes_init(struct engine_node *node OVS_UNUSED, + struct engine_arg *arg OVS_UNUSED) +{ + struct ed_type_physical_flow_changes *data = xzalloc(sizeof *data); + return data; +} + +static void +en_physical_flow_changes_cleanup(void *data OVS_UNUSED) +{ + +} + +static void +en_physical_flow_changes_run(struct engine_node *node, void *data OVS_UNUSED) +{ + struct ed_type_physical_flow_changes *pfc = data; + pfc->need_physical_run = true; + engine_set_node_state(node, EN_UPDATED); +} + +static bool +physical_flow_changes_ovs_iface_handler(struct engine_node *node OVS_UNUSED, + void *data OVS_UNUSED) +{ + struct ed_type_physical_flow_changes *pfc = data; + pfc->ovs_ifaces_changed = true; + engine_set_node_state(node, EN_UPDATED); + return true; +} + struct ovn_controller_exit_args { bool *exiting; bool *restart; @@ -1914,6 +1985,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"); @@ -1933,13 +2005,19 @@ main(int argc, char *argv[]) engine_add_input(&en_port_groups, &en_sb_port_group, port_groups_sb_port_group_handler); + engine_add_input(&en_physical_flow_changes, &en_ct_zones, + NULL); + engine_add_input(&en_physical_flow_changes, &en_ovs_interface, + physical_flow_changes_ovs_iface_handler); + engine_add_input(&en_flow_output, &en_addr_sets, flow_output_addr_sets_handler); engine_add_input(&en_flow_output, &en_port_groups, flow_output_port_groups_handler); engine_add_input(&en_flow_output, &en_runtime_data, NULL); - engine_add_input(&en_flow_output, &en_ct_zones, NULL); engine_add_input(&en_flow_output, &en_mff_ovn_geneve, NULL); + engine_add_input(&en_flow_output, &en_physical_flow_changes, + flow_output_physical_flow_changes_handler); engine_add_input(&en_flow_output, &en_ovs_open_vswitch, NULL); engine_add_input(&en_flow_output, &en_ovs_bridge, NULL); diff --git a/controller/physical.c b/controller/physical.c index 144aeb7bd..03eb677d1 100644 --- a/controller/physical.c +++ b/controller/physical.c @@ -1780,6 +1780,57 @@ physical_run(struct physical_ctx *p_ctx, simap_destroy(&new_tunnel_to_ofport); } +bool +physical_handle_ovs_iface_changes(struct physical_ctx *p_ctx, + struct ovn_desired_flow_table *flow_table) +{ + const struct ovsrec_interface *iface_rec; + OVSREC_INTERFACE_TABLE_FOR_EACH_TRACKED (iface_rec, p_ctx->iface_table) { + if (!strcmp(iface_rec->type, "geneve") || + !strcmp(iface_rec->type, "patch")) { + return false; + } + } + + struct ofpbuf ofpacts; + ofpbuf_init(&ofpacts, 0); + + OVSREC_INTERFACE_TABLE_FOR_EACH_TRACKED (iface_rec, p_ctx->iface_table) { + const char *iface_id = smap_get(&iface_rec->external_ids, "iface-id"); + if (!iface_id) { + continue; + } + + const struct local_binding *lb = + local_binding_find(p_ctx->local_bindings, iface_id); + + if (!lb || !lb->pb) { + continue; + } + + int64_t ofport = iface_rec->n_ofport ? *iface_rec->ofport : 0; + if (ovsrec_interface_is_deleted(iface_rec)) { + ofctrl_remove_flows(flow_table, &lb->pb->header_.uuid); + simap_find_and_delete(&localvif_to_ofport, iface_id); + } else { + if (!ovsrec_interface_is_new(iface_rec)) { + ofctrl_remove_flows(flow_table, &lb->pb->header_.uuid); + } + + simap_put(&localvif_to_ofport, iface_id, ofport); + consider_port_binding(p_ctx->sbrec_port_binding_by_name, + p_ctx->mff_ovn_geneve, p_ctx->ct_zones, + p_ctx->active_tunnels, + p_ctx->local_datapaths, + lb->pb, p_ctx->chassis, + flow_table, &ofpacts); + } + } + + ofpbuf_uninit(&ofpacts); + return true; +} + bool get_tunnel_ofport(const char *chassis_name, char *encap_ip, ofp_port_t *ofport) { diff --git a/controller/physical.h b/controller/physical.h index dadf84ea1..9ca34436a 100644 --- a/controller/physical.h +++ b/controller/physical.h @@ -49,11 +49,13 @@ struct physical_ctx { const struct ovsrec_bridge *br_int; const struct sbrec_chassis_table *chassis_table; const struct sbrec_chassis *chassis; + const struct ovsrec_interface_table *iface_table; const struct sset *active_tunnels; struct hmap *local_datapaths; struct sset *local_lports; const struct simap *ct_zones; enum mf_field_id mff_ovn_geneve; + struct shash *local_bindings; }; void physical_register_ovs_idl(struct ovsdb_idl *); @@ -63,7 +65,8 @@ void physical_handle_port_binding_changes(struct physical_ctx *, struct ovn_desired_flow_table *); void physical_handle_mc_group_changes(struct physical_ctx *, struct ovn_desired_flow_table *); - +bool physical_handle_ovs_iface_changes(struct physical_ctx *, + struct ovn_desired_flow_table *); bool get_tunnel_ofport(const char *chassis_name, char *encap_ip, ofp_port_t *ofport); #endif /* controller/physical.h */ From patchwork Wed May 20 19:40:12 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Numan Siddique X-Patchwork-Id: 1294692 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 49S35r3YL0z9sTC for ; Thu, 21 May 2020 05:41:20 +1000 (AEST) Received: from localhost (localhost [127.0.0.1]) by fraxinus.osuosl.org (Postfix) with ESMTP id A51F687126; Wed, 20 May 2020 19:40: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 9ZHFuGR-3tsh; Wed, 20 May 2020 19:40:47 +0000 (UTC) Received: from lists.linuxfoundation.org (lf-lists.osuosl.org [140.211.9.56]) by fraxinus.osuosl.org (Postfix) with ESMTP id A8784871C6; Wed, 20 May 2020 19:40:43 +0000 (UTC) Received: from lf-lists.osuosl.org (localhost [127.0.0.1]) by lists.linuxfoundation.org (Postfix) with ESMTP id 84DB7C07FF; Wed, 20 May 2020 19:40:43 +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 7BB2DC0894 for ; Wed, 20 May 2020 19:40:41 +0000 (UTC) Received: from localhost (localhost [127.0.0.1]) by fraxinus.osuosl.org (Postfix) with ESMTP id 981CC8458C for ; Wed, 20 May 2020 19:40:21 +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 5kmBvklBKGz4 for ; Wed, 20 May 2020 19:40:20 +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 F3513870A7 for ; Wed, 20 May 2020 19:40:19 +0000 (UTC) Received: from nusiddiq.home.org.home.org (unknown [125.99.233.239]) (Authenticated sender: numans@ovn.org) by relay10.mail.gandi.net (Postfix) with ESMTPSA id E3227240006; Wed, 20 May 2020 19:40:15 +0000 (UTC) From: numans@ovn.org To: dev@openvswitch.org Date: Thu, 21 May 2020 01:10:12 +0530 Message-Id: <20200520194012.2352034-1-numans@ovn.org> X-Mailer: git-send-email 2.26.2 In-Reply-To: <20200520193918.2351632-1-numans@ovn.org> References: <20200520193918.2351632-1-numans@ovn.org> MIME-Version: 1.0 Subject: [ovs-dev] [PATCH ovn v7 6/9] 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 Wed May 20 19:40: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: 1294689 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 49S35G3DmZz9sT9 for ; Thu, 21 May 2020 05:40:50 +1000 (AEST) Received: from localhost (localhost [127.0.0.1]) by fraxinus.osuosl.org (Postfix) with ESMTP id 871D887145; Wed, 20 May 2020 19:40:29 +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 zEwInx8ypXiI; Wed, 20 May 2020 19:40:28 +0000 (UTC) Received: from lists.linuxfoundation.org (lf-lists.osuosl.org [140.211.9.56]) by fraxinus.osuosl.org (Postfix) with ESMTP id F00808712B; Wed, 20 May 2020 19:40:27 +0000 (UTC) Received: from lf-lists.osuosl.org (localhost [127.0.0.1]) by lists.linuxfoundation.org (Postfix) with ESMTP id D4036C07FF; Wed, 20 May 2020 19:40:27 +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 C57DBC0176 for ; Wed, 20 May 2020 19:40:26 +0000 (UTC) Received: from localhost (localhost [127.0.0.1]) by hemlock.osuosl.org (Postfix) with ESMTP id B432088C19 for ; Wed, 20 May 2020 19:40:26 +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 U4QMM3iyQyyS for ; Wed, 20 May 2020 19:40:25 +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 hemlock.osuosl.org (Postfix) with ESMTPS id 9C42F88C6F for ; Wed, 20 May 2020 19:40:24 +0000 (UTC) X-Originating-IP: 125.99.233.239 Received: from nusiddiq.home.org.home.org (unknown [125.99.233.239]) (Authenticated sender: numans@ovn.org) by relay9-d.mail.gandi.net (Postfix) with ESMTPSA id 438ADFF805; Wed, 20 May 2020 19:40:20 +0000 (UTC) From: numans@ovn.org To: dev@openvswitch.org Date: Thu, 21 May 2020 01:10:17 +0530 Message-Id: <20200520194017.2352088-1-numans@ovn.org> X-Mailer: git-send-email 2.26.2 In-Reply-To: <20200520193918.2351632-1-numans@ovn.org> References: <20200520193918.2351632-1-numans@ovn.org> MIME-Version: 1.0 Cc: Venkata Anil Subject: [ovs-dev] [PATCH ovn v7 7/9] 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 | 159 ++++++++++++++++++++++++++++++++---- controller/binding.h | 21 +++++ controller/ovn-controller.c | 62 +++++++++++++- tests/ovn-performance.at | 28 +++---- 4 files changed, 240 insertions(+), 30 deletions(-) diff --git a/controller/binding.c b/controller/binding.c index f00c975ce..9b3d46b23 100644 --- a/controller/binding.c +++ b/controller/binding.c @@ -69,13 +69,20 @@ binding_register_ovs_idl(struct ovsdb_idl *ovs_idl) ovsdb_idl_add_column(ovs_idl, &ovsrec_qos_col_type); } +static struct tracked_binding_datapath *tracked_binding_datapath_create( + 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 add_local_datapath__(struct ovsdb_idl_index *sbrec_datapath_binding_by_key, struct ovsdb_idl_index *sbrec_port_binding_by_datapath, struct ovsdb_idl_index *sbrec_port_binding_by_name, const struct sbrec_datapath_binding *datapath, bool has_local_l3gateway, int depth, - struct hmap *local_datapaths) + struct hmap *local_datapaths, + struct hmap *updated_dp_bindings) { uint32_t dp_key = datapath->tunnel_key; struct local_datapath *ld = get_local_datapath(local_datapaths, dp_key); @@ -92,6 +99,11 @@ add_local_datapath__(struct ovsdb_idl_index *sbrec_datapath_binding_by_key, ld->localnet_port = NULL; ld->has_local_l3gateway = has_local_l3gateway; + if (updated_dp_bindings && + !tracked_binding_datapath_find(updated_dp_bindings, datapath)) { + tracked_binding_datapath_create(datapath, true, updated_dp_bindings); + } + if (depth >= 100) { static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 1); VLOG_WARN_RL(&rl, "datapaths nested too deep"); @@ -124,7 +136,8 @@ add_local_datapath__(struct ovsdb_idl_index *sbrec_datapath_binding_by_key, sbrec_port_binding_by_datapath, sbrec_port_binding_by_name, peer->datapath, false, - depth + 1, local_datapaths); + depth + 1, local_datapaths, + updated_dp_bindings); } ld->n_peer_ports++; if (ld->n_peer_ports > ld->n_allocated_peer_ports) { @@ -147,12 +160,14 @@ add_local_datapath(struct ovsdb_idl_index *sbrec_datapath_binding_by_key, struct ovsdb_idl_index *sbrec_port_binding_by_datapath, struct ovsdb_idl_index *sbrec_port_binding_by_name, const struct sbrec_datapath_binding *datapath, - bool has_local_l3gateway, struct hmap *local_datapaths) + bool has_local_l3gateway, struct hmap *local_datapaths, + struct hmap *updated_dp_bindings) { add_local_datapath__(sbrec_datapath_binding_by_key, sbrec_port_binding_by_datapath, sbrec_port_binding_by_name, - datapath, has_local_l3gateway, 0, local_datapaths); + datapath, has_local_l3gateway, 0, local_datapaths, + updated_dp_bindings); } static void @@ -623,6 +638,71 @@ is_lport_container(const struct sbrec_port_binding *pb) return !pb->type[0] && 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; + ovs_list_init(&t_dp->lports_head); + hmap_insert(tracked_datapaths, &t_dp->node, uuid_hash(&dp->header_.uuid)); + return t_dp; +} + +static struct tracked_binding_datapath * +tracked_binding_datapath_find(struct hmap *tracked_datapaths, + const struct sbrec_datapath_binding *dp) +{ + struct tracked_binding_datapath *t_dp; + size_t hash = uuid_hash(&dp->header_.uuid); + HMAP_FOR_EACH_WITH_HASH (t_dp, node, hash, tracked_datapaths) { + if (uuid_equals(&t_dp->dp->header_.uuid, &dp->header_.uuid)) { + return t_dp; + } + } + + return NULL; +} + +static void +tracked_binding_datapath_lport_add(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); + } + struct tracked_binding_lport *lport = xmalloc(sizeof *lport); + lport->pb = pb; + lport->deleted = deleted; + ovs_list_push_back(&tracked_dp->lports_head, &lport->list_node); +} + +void +binding_tracked_dp_destroy(struct hmap *tracked_datapaths) +{ + struct tracked_binding_datapath *t_dp; + HMAP_FOR_EACH_POP (t_dp, node, tracked_datapaths) { + struct tracked_binding_lport *lport, *next; + LIST_FOR_EACH_SAFE (lport, next, list_node, &t_dp->lports_head) { + ovs_list_remove(&lport->list_node); + free(lport); + } + free(t_dp); + } + + hmap_destroy(tracked_datapaths); +} + /* Corresponds to each Port_Binding.type. */ enum en_lport_type { LP_UNKNOWN, @@ -770,7 +850,8 @@ can_bind_on_this_chassis(const struct sbrec_chassis *chassis_rec, 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) { @@ -780,6 +861,10 @@ release_local_binding_children(const struct sbrec_chassis *chassis_rec, return false; } } + if (tracked_dp_bindings) { + tracked_binding_datapath_lport_add(l->pb, true, + tracked_dp_bindings); + } } return true; @@ -787,10 +872,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; } @@ -798,6 +884,10 @@ 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); + } return true; } @@ -821,7 +911,8 @@ consider_vif_lport_(const struct sbrec_port_binding *pb, add_local_datapath(b_ctx_in->sbrec_datapath_binding_by_key, b_ctx_in->sbrec_port_binding_by_datapath, b_ctx_in->sbrec_port_binding_by_name, - pb->datapath, false, b_ctx_out->local_datapaths); + pb->datapath, false, b_ctx_out->local_datapaths, + b_ctx_out->tracked_dp_bindings); update_local_lport_ids(b_ctx_out->local_lport_ids, pb); if (lbinding->iface && qos_map && b_ctx_in->ovs_idl_txn) { get_qos_params(pb, qos_map); @@ -1003,7 +1094,8 @@ consider_nonvif_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, 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); return claim_lport(pb, b_ctx_in->chassis_rec, NULL, @@ -1080,7 +1172,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); @@ -1367,7 +1460,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; } @@ -1444,6 +1538,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); +} + static bool handle_iface_add(const struct ovsrec_interface *iface_rec, const char *iface_id, @@ -1472,6 +1578,7 @@ handle_iface_add(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)) { @@ -1479,6 +1586,10 @@ handle_iface_add(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; @@ -1486,10 +1597,15 @@ handle_iface_add(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); } } @@ -1508,7 +1624,8 @@ handle_iface_delete(const char *lport_name, const char *iface_name, lbinding->pb->chassis == 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; } @@ -1556,6 +1673,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. @@ -1698,9 +1817,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; } @@ -1746,6 +1866,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); @@ -1755,11 +1878,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); } } diff --git a/controller/binding.h b/controller/binding.h index 21118ecd4..fc2a673e5 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; @@ -58,6 +61,8 @@ struct binding_ctx_out { struct sset *local_lport_ids; struct sset *egress_ifaces; struct smap *local_iface_ids; + struct hmap *tracked_dp_bindings; + bool *local_lports_changed; }; enum local_binding_type { @@ -82,6 +87,21 @@ local_binding_find(struct shash *local_bindings, const char *name) return shash_find_data(local_bindings, name); } +/* Represents a tracked binding logical port. */ +struct tracked_binding_lport { + const struct sbrec_port_binding *pb; + struct ovs_list list_node; + bool deleted; +}; + +/* Represent a tracked binding datapath. */ +struct tracked_binding_datapath { + struct hmap_node node; + const struct sbrec_datapath_binding *dp; + bool is_new; + struct ovs_list lports_head; /* List of struct tracked_binding_lport. */ +}; + 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 +116,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 8f95fff1f..dc790f0f7 100644 --- a/controller/ovn-controller.c +++ b/controller/ovn-controller.c @@ -978,6 +978,23 @@ struct ed_type_runtime_data { struct smap local_iface_ids; }; +struct ed_type_runtime_tracked_data { + struct hmap tracked_dp_bindings; + bool local_lports_changed; + bool tracked; +}; + +static void +en_runtime_clear_tracked_data(void *tracked_data) +{ + struct ed_type_runtime_tracked_data *data = tracked_data; + + binding_tracked_dp_destroy(&data->tracked_dp_bindings); + hmap_init(&data->tracked_dp_bindings); + data->local_lports_changed = false; + data->tracked = false; +} + static void * en_runtime_data_init(struct engine_node *node OVS_UNUSED, struct engine_arg *arg OVS_UNUSED) @@ -991,6 +1008,13 @@ en_runtime_data_init(struct engine_node *node OVS_UNUSED, sset_init(&data->egress_ifaces); 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 +1117,8 @@ init_binding_ctx(struct engine_node *node, b_ctx_out->egress_ifaces = &rt_data->egress_ifaces; b_ctx_out->local_bindings = &rt_data->local_bindings; b_ctx_out->local_iface_ids = &rt_data->local_iface_ids; + b_ctx_out->tracked_dp_bindings = NULL; + b_ctx_out->local_lports_changed = NULL; } static void @@ -1104,6 +1130,8 @@ en_runtime_data_run(struct engine_node *node, void *data) struct sset *local_lport_ids = &rt_data->local_lport_ids; struct sset *active_tunnels = &rt_data->active_tunnels; + en_runtime_clear_tracked_data(node->tracked_data); + static bool first_run = true; if (first_run) { /* don't cleanup since there is no data yet */ @@ -1156,9 +1184,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 +1223,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)) { @@ -1862,6 +1898,29 @@ physical_flow_changes_ovs_iface_handler(struct engine_node *node OVS_UNUSED, return true; } +static bool +flow_output_runtime_data_handler(struct engine_node *node, + void *data OVS_UNUSED) +{ + struct ed_type_runtime_tracked_data *tracked_data = + engine_get_input_tracked_data("runtime_data", node); + + if (!tracked_data || !tracked_data->tracked) { + return false; + } + + if (!hmap_is_empty(&tracked_data->tracked_dp_bindings)) { + /* We are not yet handling the tracked datapath binding + * changes. Return false to trigger full recompute. */ + return false; + } + + if (tracked_data->local_lports_changed) { + engine_set_node_state(node, EN_UPDATED); + } + return true; +} + struct ovn_controller_exit_args { bool *exiting; bool *restart; @@ -2014,7 +2073,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..6e064e64f 100644 --- a/tests/ovn-performance.at +++ b/tests/ovn-performance.at @@ -274,7 +274,7 @@ for i in 1 2; do ) # Add router port to $ls - OVN_CONTROLLER_EXPECT_HIT( + OVN_CONTROLLER_EXPECT_NO_HIT( [hv1 hv2], [lflow_run], [ovn-nbctl --wait=hv lrp-add lr1 $lrp 02:00:00:00:0$i:01 10.0.$i.1/24] ) @@ -282,15 +282,15 @@ for i in 1 2; do [hv1 hv2], [lflow_run], [ovn-nbctl --wait=hv lsp-add $ls $lsp] ) - OVN_CONTROLLER_EXPECT_HIT( + OVN_CONTROLLER_EXPECT_NO_HIT( [hv1 hv2], [lflow_run], [ovn-nbctl --wait=hv lsp-set-type $lsp router] ) - OVN_CONTROLLER_EXPECT_HIT( + OVN_CONTROLLER_EXPECT_NO_HIT( [hv1 hv2], [lflow_run], [ovn-nbctl --wait=hv lsp-set-options $lsp router-port=$lrp] ) - OVN_CONTROLLER_EXPECT_HIT( + OVN_CONTROLLER_EXPECT_NO_HIT( [hv1 hv2], [lflow_run], [ovn-nbctl --wait=hv lsp-set-addresses $lsp router] ) @@ -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 @@ -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 Wed May 20 19:40:23 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Numan Siddique X-Patchwork-Id: 1294690 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 49S35R24JZz9sT9 for ; Thu, 21 May 2020 05:40:59 +1000 (AEST) Received: from localhost (localhost [127.0.0.1]) by fraxinus.osuosl.org (Postfix) with ESMTP id A7F9B8716C; Wed, 20 May 2020 19:40:37 +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 1F8hpd0117Y0; Wed, 20 May 2020 19:40:35 +0000 (UTC) Received: from lists.linuxfoundation.org (lf-lists.osuosl.org [140.211.9.56]) by fraxinus.osuosl.org (Postfix) with ESMTP id 48FF487100; Wed, 20 May 2020 19:40:35 +0000 (UTC) Received: from lf-lists.osuosl.org (localhost [127.0.0.1]) by lists.linuxfoundation.org (Postfix) with ESMTP id 2FD6FC088C; Wed, 20 May 2020 19:40:35 +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 8FFCFC07FF for ; Wed, 20 May 2020 19:40:33 +0000 (UTC) Received: from localhost (localhost [127.0.0.1]) by hemlock.osuosl.org (Postfix) with ESMTP id 7D0D388C87 for ; Wed, 20 May 2020 19:40:33 +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 mx93qPUYeo5e for ; Wed, 20 May 2020 19:40:30 +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 hemlock.osuosl.org (Postfix) with ESMTPS id 1C88788C19 for ; Wed, 20 May 2020 19:40:29 +0000 (UTC) Received: from nusiddiq.home.org.home.org (unknown [125.99.233.239]) (Authenticated sender: numans@ovn.org) by relay10.mail.gandi.net (Postfix) with ESMTPSA id 091BD240002; Wed, 20 May 2020 19:40:26 +0000 (UTC) From: numans@ovn.org To: dev@openvswitch.org Date: Thu, 21 May 2020 01:10:23 +0530 Message-Id: <20200520194023.2352138-1-numans@ovn.org> X-Mailer: git-send-email 2.26.2 In-Reply-To: <20200520193918.2351632-1-numans@ovn.org> References: <20200520193918.2351632-1-numans@ovn.org> MIME-Version: 1.0 Cc: Venkata Anil Subject: [ovs-dev] [PATCH ovn v7 8/9] 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 | 82 +++++++++++++++++++++++- controller/lflow.h | 14 +++- controller/ovn-controller.c | 124 ++++++++++++++++++++---------------- controller/physical.h | 3 +- tests/ovn-performance.at | 4 +- 5 files changed, 168 insertions(+), 59 deletions(-) diff --git a/controller/lflow.c b/controller/lflow.c index 01214a3a6..45bf4aa4b 100644 --- a/controller/lflow.c +++ b/controller/lflow.c @@ -258,7 +258,7 @@ lflow_resource_destroy_lflow(struct lflow_resource_ref *lfrr, /* Adds the logical flows from the Logical_Flow table to flow tables. */ static void add_logical_flows(struct lflow_ctx_in *l_ctx_in, - struct lflow_ctx_out *l_ctx_out) + struct lflow_ctx_out *l_ctx_out) { const struct sbrec_logical_flow *lflow; @@ -649,6 +649,8 @@ consider_logical_flow(const struct sbrec_logical_flow *lflow, int64_t dp_id = lflow->logical_datapath->tunnel_key; char buf[16]; snprintf(buf, sizeof(buf), "%"PRId64"_%"PRId64, dp_id, port_id); + lflow_resource_add(l_ctx_out->lfrr, REF_TYPE_PORTBINDING, buf, + &lflow->header_.uuid); if (!sset_contains(l_ctx_in->local_lport_ids, buf)) { VLOG_DBG("lflow "UUID_FMT " port %s in match is not local, skip", @@ -847,3 +849,81 @@ lflow_destroy(void) expr_symtab_destroy(&symtab); shash_destroy(&symtab); } + +bool +lflow_evaluate_pb_changes(const struct sbrec_port_binding_table *pb_table) +{ + const struct sbrec_port_binding *binding; + SBREC_PORT_BINDING_TABLE_FOR_EACH_TRACKED (binding, pb_table) { + if (!strcmp(binding->type, "remote")) { + return false; + } + } + + return true; +} + +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..00c1b5c47 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,11 @@ 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 *); +bool lflow_evaluate_pb_changes(const struct sbrec_port_binding_table *); #endif /* controller/lflow.h */ diff --git a/controller/ovn-controller.c b/controller/ovn-controller.c index dc790f0f7..e53f750bf 100644 --- a/controller/ovn-controller.c +++ b/controller/ovn-controller.c @@ -1440,6 +1440,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), @@ -1487,6 +1492,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; @@ -1641,7 +1648,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); @@ -1649,56 +1657,21 @@ 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); + if (!lflow_evaluate_pb_changes(p_ctx.port_binding_table)) { + /* If this returns false, it means there is an impact on + * the logical flow processing because of some changes in + * port bindings. Return false so that recompute is triggered + * for this stage. */ + return false; + } + + /* 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); @@ -1786,6 +1759,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(); } @@ -1909,16 +1884,52 @@ flow_output_runtime_data_handler(struct engine_node *node, return false; } - if (!hmap_is_empty(&tracked_data->tracked_dp_bindings)) { - /* We are not yet handling the tracked datapath binding - * changes. Return false to trigger full recompute. */ - return false; + if (hmap_is_empty(&tracked_data->tracked_dp_bindings)) { + if (tracked_data->local_lports_changed) { + engine_set_node_state(node, EN_UPDATED); + } + return true; } - if (tracked_data->local_lports_changed) { + struct lflow_ctx_in l_ctx_in; + struct lflow_ctx_out l_ctx_out; + struct ed_type_flow_output *fo = data; + struct ed_type_runtime_data *rt_data = + engine_get_input_data("runtime_data", node); + init_lflow_ctx(node, rt_data, fo, &l_ctx_in, &l_ctx_out); + + struct physical_ctx p_ctx; + init_physical_ctx(node, rt_data, &p_ctx); + + bool handled = true; + struct tracked_binding_datapath *tdp; + HMAP_FOR_EACH (tdp, node, &tracked_data->tracked_dp_bindings) { + if (tdp->is_new) { + handled = lflow_add_flows_for_datapath(tdp->dp, &l_ctx_in, + &l_ctx_out); + if (!handled) { + break; + } + } else { + struct tracked_binding_lport *lport; + LIST_FOR_EACH (lport, list_node, &tdp->lports_head) { + if (!lflow_handle_flows_for_lport(lport->pb, &l_ctx_in, + &l_ctx_out)) { + handled = false; + break; + } + } + if (!handled) { + break; + } + } + } + + if (handled) { engine_set_node_state(node, EN_UPDATED); } - return true; + + return handled; } struct ovn_controller_exit_args { @@ -1976,6 +1987,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); @@ -2125,6 +2139,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 6e064e64f..a12757e18 100644 --- a/tests/ovn-performance.at +++ b/tests/ovn-performance.at @@ -353,8 +353,8 @@ for i in 1 2; do ) # Bind port $lp and wait for it to come up - OVN_CONTROLLER_EXPECT_HIT_COND( - [hv$i hv$j], [lflow_run], [>0 =0], + OVN_CONTROLLER_EXPECT_NO_HIT( + [hv$i hv$j], [lflow_run], [as hv$i ovs-vsctl add-port br-int $vif -- set Interface $vif external-ids:iface-id=$lp && ovn-nbctl wait-until Logical_Switch_Port $lp 'up=true' && ovn-nbctl --wait=hv sync] From patchwork Wed May 20 19:40:28 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Numan Siddique X-Patchwork-Id: 1294693 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 49S3652H2Bz9sT9 for ; Thu, 21 May 2020 05:41:33 +1000 (AEST) Received: from localhost (localhost [127.0.0.1]) by whitealder.osuosl.org (Postfix) with ESMTP id 9A13388399; Wed, 20 May 2020 19:41:31 +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 dMEJJ5sp79ZQ; Wed, 20 May 2020 19:41:26 +0000 (UTC) Received: from lists.linuxfoundation.org (lf-lists.osuosl.org [140.211.9.56]) by whitealder.osuosl.org (Postfix) with ESMTP id 6CB9986DBD; Wed, 20 May 2020 19:40:58 +0000 (UTC) Received: from lf-lists.osuosl.org (localhost [127.0.0.1]) by lists.linuxfoundation.org (Postfix) with ESMTP id 4E848C0894; Wed, 20 May 2020 19:40:58 +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 1B8EFC0176 for ; Wed, 20 May 2020 19:40:57 +0000 (UTC) Received: from localhost (localhost [127.0.0.1]) by fraxinus.osuosl.org (Postfix) with ESMTP id 54C068719C for ; Wed, 20 May 2020 19:40:40 +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 UvccuvRkkUSq for ; Wed, 20 May 2020 19:40:38 +0000 (UTC) X-Greylist: domain auto-whitelisted by SQLgrey-1.7.6 Received: from relay7-d.mail.gandi.net (relay7-d.mail.gandi.net [217.70.183.200]) by fraxinus.osuosl.org (Postfix) with ESMTPS id BE89687100 for ; Wed, 20 May 2020 19:40:37 +0000 (UTC) X-Originating-IP: 125.99.233.239 Received: from nusiddiq.home.org.home.org (unknown [125.99.233.239]) (Authenticated sender: numans@ovn.org) by relay7-d.mail.gandi.net (Postfix) with ESMTPSA id 22AEA20006; Wed, 20 May 2020 19:40:32 +0000 (UTC) From: numans@ovn.org To: dev@openvswitch.org Date: Thu, 21 May 2020 01:10:28 +0530 Message-Id: <20200520194029.2352187-1-numans@ovn.org> X-Mailer: git-send-email 2.26.2 In-Reply-To: <20200520193918.2351632-1-numans@ovn.org> References: <20200520193918.2351632-1-numans@ovn.org> MIME-Version: 1.0 Subject: [ovs-dev] [PATCH ovn v7 9/9] 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 | 53 ++++++++++++++++++++++++++++++------- 1 file changed, 43 insertions(+), 10 deletions(-) diff --git a/controller/ovn-controller.c b/controller/ovn-controller.c index e53f750bf..871f9aa27 100644 --- a/controller/ovn-controller.c +++ b/controller/ovn-controller.c @@ -1381,7 +1381,8 @@ static void init_physical_ctx(struct engine_node *node, struct sbrec_chassis_table *chassis_table = (struct sbrec_chassis_table *)EN_OVSDB_GET( - engine_get_input("SB_chassis", node)); + engine_get_input("SB_chassis", + engine_get_input("physical_flow_changes", node))); struct ed_type_mff_ovn_geneve *ed_mff_ovn_geneve = engine_get_input_data("mff_ovn_geneve", node); @@ -1397,7 +1398,8 @@ static void init_physical_ctx(struct engine_node *node, const struct sbrec_chassis *chassis = NULL; struct ovsdb_idl_index *sbrec_chassis_by_name = engine_ovsdb_node_get_index( - engine_get_input("SB_chassis", node), + engine_get_input("SB_chassis", + engine_get_input("physical_flow_changes", node)), "name"); if (chassis_id) { chassis = chassis_lookup_by_name(sbrec_chassis_by_name, chassis_id); @@ -1474,7 +1476,8 @@ static void init_lflow_ctx(struct engine_node *node, const struct sbrec_chassis *chassis = NULL; struct ovsdb_idl_index *sbrec_chassis_by_name = engine_ovsdb_node_get_index( - engine_get_input("SB_chassis", node), + engine_get_input("SB_chassis", + engine_get_input("physical_flow_changes", node)), "name"); if (chassis_id) { chassis = chassis_lookup_by_name(sbrec_chassis_by_name, chassis_id); @@ -1555,8 +1558,9 @@ en_flow_output_run(struct engine_node *node, void *data) struct ovsdb_idl_index *sbrec_chassis_by_name = engine_ovsdb_node_get_index( - engine_get_input("SB_chassis", node), - "name"); + engine_get_input("SB_chassis", + engine_get_input("physical_flow_changes", node)), + "name"); const struct sbrec_chassis *chassis = NULL; if (chassis_id) { @@ -1721,8 +1725,9 @@ _flow_output_resource_ref_handler(struct engine_node *node, void *data, struct ovsdb_idl_index *sbrec_chassis_by_name = engine_ovsdb_node_get_index( - engine_get_input("SB_chassis", node), - "name"); + engine_get_input("SB_chassis", + engine_get_input("physical_flow_changes", node)), + "name"); const struct sbrec_chassis *chassis = NULL; if (chassis_id) { chassis = chassis_lookup_by_name(sbrec_chassis_by_name, chassis_id); @@ -1873,6 +1878,31 @@ physical_flow_changes_ovs_iface_handler(struct engine_node *node OVS_UNUSED, return true; } +/* Handles sbrec_chassis changes. + * If a new chassis is added or removed return false, so that + * physical flows are programmed. + * For any updates, there is no need for any flow computation. + * Encap changes will also result in sbrec_chassis changes, + * but we handle encap changes separately. + */ +static bool +physical_flow_changes_sb_chassis_handler(struct engine_node *node OVS_UNUSED, + void *data OVS_UNUSED) +{ + struct sbrec_chassis_table *chassis_table = + (struct sbrec_chassis_table *)EN_OVSDB_GET( + engine_get_input("SB_chassis", node)); + + const struct sbrec_chassis *ch; + SBREC_CHASSIS_TABLE_FOR_EACH_TRACKED (ch, chassis_table) { + if (sbrec_chassis_is_deleted(ch) || sbrec_chassis_is_new(ch)) { + return false; + } + } + + return true; +} + static bool flow_output_runtime_data_handler(struct engine_node *node, void *data OVS_UNUSED) @@ -2082,6 +2112,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); @@ -2096,8 +2130,6 @@ main(int argc, char *argv[]) engine_add_input(&en_flow_output, &en_ovs_open_vswitch, NULL); engine_add_input(&en_flow_output, &en_ovs_bridge, NULL); - engine_add_input(&en_flow_output, &en_sb_chassis, NULL); - engine_add_input(&en_flow_output, &en_sb_encap, NULL); engine_add_input(&en_flow_output, &en_sb_multicast_group, flow_output_sb_multicast_group_handler); engine_add_input(&en_flow_output, &en_sb_port_binding, @@ -2124,7 +2156,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,