From patchwork Sat May 16 17:56:25 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Numan Siddique X-Patchwork-Id: 1291943 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 49PXz05WFpz9sTK for ; Sun, 17 May 2020 03:56:44 +1000 (AEST) Received: from localhost (localhost [127.0.0.1]) by silver.osuosl.org (Postfix) with ESMTP id A99BD203BB; Sat, 16 May 2020 17:56:42 +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 lC6jTAYrnSSx; Sat, 16 May 2020 17:56:39 +0000 (UTC) Received: from lists.linuxfoundation.org (lf-lists.osuosl.org [140.211.9.56]) by silver.osuosl.org (Postfix) with ESMTP id 5F832203A7; Sat, 16 May 2020 17:56:39 +0000 (UTC) Received: from lf-lists.osuosl.org (localhost [127.0.0.1]) by lists.linuxfoundation.org (Postfix) with ESMTP id 45D80C0865; Sat, 16 May 2020 17:56:39 +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 6A120C016F for ; Sat, 16 May 2020 17:56:37 +0000 (UTC) Received: from localhost (localhost [127.0.0.1]) by hemlock.osuosl.org (Postfix) with ESMTP id 4E47488543 for ; Sat, 16 May 2020 17:56:37 +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 k1NXQULxx4Zx for ; Sat, 16 May 2020 17:56:34 +0000 (UTC) X-Greylist: domain auto-whitelisted by SQLgrey-1.7.6 Received: from relay11.mail.gandi.net (relay11.mail.gandi.net [217.70.178.231]) by hemlock.osuosl.org (Postfix) with ESMTPS id 16E5388521 for ; Sat, 16 May 2020 17:56:33 +0000 (UTC) Received: from nusiddiq.home.org.com (unknown [115.99.184.184]) (Authenticated sender: numans@ovn.org) by relay11.mail.gandi.net (Postfix) with ESMTPSA id 22901100006; Sat, 16 May 2020 17:56:30 +0000 (UTC) From: numans@ovn.org To: dev@openvswitch.org Date: Sat, 16 May 2020 23:26:25 +0530 Message-Id: <20200516175625.4003685-1-numans@ovn.org> X-Mailer: git-send-email 2.26.2 In-Reply-To: <20200516175539.4003530-1-numans@ovn.org> References: <20200516175539.4003530-1-numans@ovn.org> MIME-Version: 1.0 Subject: [ovs-dev] [PATCH ovn v6 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. Signed-off-by: Numan Siddique --- controller/binding.c | 993 ++++++++++++++++++++++++------------ controller/binding.h | 14 +- controller/ovn-controller.c | 49 +- 3 files changed, 700 insertions(+), 356 deletions(-) diff --git a/controller/binding.c b/controller/binding.c index a5525a310..0c215d8d6 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, @@ -723,6 +469,586 @@ 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_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_VIF, + LP_PATCH, + LP_L3GATEWAY, + LP_LOCALNET, + LP_LOCALPORT, + LP_L2GATEWAY, + LP_VTEP, + LP_CHASSISREDIRECT, + LP_VIRTUAL, + LP_EXTERNAL +}; + +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 { + return LP_VTEP; + } + + OVS_NOT_REACHED(); +} + +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 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); + + struct local_binding *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) { + /* 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. + * + * Right now we don't handle OVS interface and SB Port_Binding + * changes incrementally. Once we do that, this book keeping + * is required. + * */ + const struct sbrec_port_binding *parent = + lport_lookup_by_name(b_ctx_in->sbrec_port_binding_by_name, + pb->parent_port); + + parent_lbinding = local_binding_create(pb->parent_port, NULL, parent, + 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) { + /* 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)) { + release_lport(pb); + } + return; + } + + 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; + + 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); + 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, + 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) { + const struct sbrec_port_binding *pb + = lport_lookup_by_name( + b_ctx_in->sbrec_port_binding_by_name, iface_id); + struct local_binding *lbinding = + local_binding_find(b_ctx_out->local_bindings, iface_id); + if (!lbinding) { + lbinding = local_binding_create(iface_id, iface_rec, pb, + BT_VIF); + local_binding_add(b_ctx_out->local_bindings, lbinding); + } else { + lbinding->type = BT_VIF; + lbinding->pb = pb; + } + + sset_add(b_ctx_out->local_lports, iface_id); + } + + /* Check if this is a tunnel interface. */ + if (smap_get(&iface_rec->options, "remote_ip")) { + const char *tunnel_iface + = smap_get(&iface_rec->status, "tunnel_egress_iface"); + if (tunnel_iface) { + sset_add(b_ctx_out->egress_ifaces, tunnel_iface); + } + } + } + } +} + void binding_run(struct binding_ctx_in *b_ctx_in, struct binding_ctx_out *b_ctx_out) { @@ -730,106 +1056,122 @@ 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, 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; + } + } } - 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 @@ -841,24 +1183,31 @@ binding_evaluate_port_binding_changes( * interface table will be updated, which will trigger recompute. * * - If the port is not a regular VIF, always trigger recompute. */ - if (binding_rec->chassis == chassis_rec) { + if (binding_rec->chassis == b_ctx_in->chassis_rec) { changed = true; break; } - const struct ovsrec_interface *iface_rec - = shash_find_data(&lport_to_iface, binding_rec->logical_port); - if (is_our_chassis(chassis_rec, binding_rec, active_tunnels, iface_rec, - local_lports) || - (strcmp(binding_rec->type, "") && strcmp(binding_rec->type, - "remote"))) { + if (strcmp(binding_rec->type, "")) { + changed = true; + break; + } + + struct local_binding *lbinding = NULL; + if (!binding_rec->parent_port || !binding_rec->parent_port[0]) { + lbinding = local_binding_find(b_ctx_out->local_bindings, + binding_rec->logical_port); + } else { + lbinding = local_binding_find(b_ctx_out->local_bindings, + binding_rec->parent_port); + } + + if (lbinding) { changed = true; break; } } - shash_destroy(&lport_to_iface); - sset_destroy(&egress_ifaces); return changed; } diff --git a/controller/binding.h b/controller/binding.h index d0b8331af..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 c0a97a265..3bda1065c 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,8 @@ 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); + sset_init(&rt_data->egress_ifaces); struct local_datapath *cur_node, *next_node; HMAP_FOR_EACH_SAFE (cur_node, next_node, hmap_node, &rt_data->local_datapaths) { @@ -1000,6 +1009,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 +1082,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 +1107,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 +1145,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; } From patchwork Sat May 16 17:56:32 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Numan Siddique X-Patchwork-Id: 1291945 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 49PXzD5VTFz9sTK for ; Sun, 17 May 2020 03:56:56 +1000 (AEST) Received: from localhost (localhost [127.0.0.1]) by fraxinus.osuosl.org (Postfix) with ESMTP id 597568705D; Sat, 16 May 2020 17:56:54 +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 TKz3A6Rkj_Tq; Sat, 16 May 2020 17:56:48 +0000 (UTC) Received: from lists.linuxfoundation.org (lf-lists.osuosl.org [140.211.9.56]) by fraxinus.osuosl.org (Postfix) with ESMTP id C16E686F85; Sat, 16 May 2020 17:56:48 +0000 (UTC) Received: from lf-lists.osuosl.org (localhost [127.0.0.1]) by lists.linuxfoundation.org (Postfix) with ESMTP id 98342C0893; Sat, 16 May 2020 17:56:48 +0000 (UTC) X-Original-To: dev@openvswitch.org Delivered-To: ovs-dev@lists.linuxfoundation.org Received: from silver.osuosl.org (smtp3.osuosl.org [140.211.166.136]) by lists.linuxfoundation.org (Postfix) with ESMTP id 0EAB9C016F for ; Sat, 16 May 2020 17:56:47 +0000 (UTC) Received: from localhost (localhost [127.0.0.1]) by silver.osuosl.org (Postfix) with ESMTP id EA4EE203B2 for ; Sat, 16 May 2020 17:56:46 +0000 (UTC) X-Virus-Scanned: amavisd-new at osuosl.org Received: from silver.osuosl.org ([127.0.0.1]) by localhost (.osuosl.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id rAIzukdMzKoZ for ; Sat, 16 May 2020 17:56:43 +0000 (UTC) X-Greylist: domain auto-whitelisted by SQLgrey-1.7.6 Received: from relay4-d.mail.gandi.net (relay4-d.mail.gandi.net [217.70.183.196]) by silver.osuosl.org (Postfix) with ESMTPS id 4F6E220441 for ; Sat, 16 May 2020 17:56:43 +0000 (UTC) X-Originating-IP: 115.99.184.184 Received: from nusiddiq.home.org.com (unknown [115.99.184.184]) (Authenticated sender: numans@ovn.org) by relay4-d.mail.gandi.net (Postfix) with ESMTPSA id DBFC4E0003; Sat, 16 May 2020 17:56:38 +0000 (UTC) From: numans@ovn.org To: dev@openvswitch.org Date: Sat, 16 May 2020 23:26:32 +0530 Message-Id: <20200516175632.4003740-1-numans@ovn.org> X-Mailer: git-send-email 2.26.2 In-Reply-To: <20200516175539.4003530-1-numans@ovn.org> References: <20200516175539.4003530-1-numans@ovn.org> MIME-Version: 1.0 Subject: [ovs-dev] [PATCH ovn v6 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. Signed-off-by: Numan Siddique --- controller/binding.c | 783 +++++++++++++++++++++++++++++++----- controller/binding.h | 9 +- controller/ovn-controller.c | 61 ++- tests/ovn-performance.at | 13 + tests/ovn.at | 41 ++ 5 files changed, 797 insertions(+), 110 deletions(-) diff --git a/controller/binding.c b/controller/binding.c index 0c215d8d6..5b7cde81d 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 @@ -441,10 +430,10 @@ add_localnet_egress_interface_mappings( } 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) { add_localnet_egress_interface_mappings(binding_rec, bridge_mappings, egress_ifaces); @@ -469,6 +458,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 @@ -585,6 +596,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) @@ -611,6 +630,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_VIF, LP_PATCH, @@ -652,12 +672,20 @@ get_lport_type(const struct sbrec_port_binding *pb) OVS_NOT_REACHED(); } -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, @@ -676,26 +704,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 @@ -720,7 +770,36 @@ 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(struct local_binding *lbinding, + bool sb_readonly) +{ + struct shash_node *node; + SHASH_FOR_EACH (node, &lbinding->children) { + struct local_binding *l = node->data; + if (!release_lport(l->pb, sb_readonly)) { + return false; + } + } + + return true; +} + +static bool +release_local_binding(struct local_binding *lbinding, bool sb_readonly) +{ + if (!release_local_binding_children(lbinding, sb_readonly)) { + return false; + } + + if (!release_lport(lbinding->pb, sb_readonly)) { + return false; + } + + 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, @@ -732,7 +811,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, @@ -757,13 +839,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, @@ -779,11 +862,11 @@ consider_vif_lport(const struct sbrec_port_binding *pb, if (lbinding) { 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, @@ -837,9 +920,9 @@ consider_container_lport(const struct sbrec_port_binding *pb, * it was bound earlier. */ if (is_lbinding_this_chassis(container_lbinding, b_ctx_in->chassis_rec)) { - release_lport(pb); + return release_lport(pb, !b_ctx_in->ovnsb_idl_txn); } - return; + return true; } const char *vif_chassis = smap_get(&parent_lbinding->pb->options, @@ -847,11 +930,11 @@ consider_container_lport(const struct sbrec_port_binding *pb, 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, @@ -885,14 +968,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, @@ -902,13 +984,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) @@ -917,10 +1002,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) @@ -929,7 +1014,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 @@ -948,7 +1033,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) @@ -976,18 +1061,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) @@ -1035,6 +1120,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. */ @@ -1137,9 +1224,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); } @@ -1157,60 +1244,6 @@ 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. */ -bool -binding_evaluate_port_binding_changes(struct binding_ctx_in *b_ctx_in, - struct binding_ctx_out *b_ctx_out) -{ - if (!b_ctx_in->chassis_rec) { - return true; - } - - bool changed = false; - - const struct sbrec_port_binding *binding_rec; - SBREC_PORT_BINDING_TABLE_FOR_EACH_TRACKED (binding_rec, - b_ctx_in->port_binding_table) { - /* XXX: currently OVSDB change tracking doesn't support getting old - * data when the operation is update, so if a port-binding moved from - * this chassis to another, there is no easy way to find out the - * change. To workaround this problem, we just makes sure if - * any port *related to* this chassis has any change, then trigger - * recompute. - * - * - If a regular VIF is unbound from this chassis, the local ovsdb - * interface table will be updated, which will trigger recompute. - * - * - If the port is not a regular VIF, always trigger recompute. */ - if (binding_rec->chassis == b_ctx_in->chassis_rec) { - changed = true; - break; - } - - if (strcmp(binding_rec->type, "")) { - changed = true; - break; - } - - struct local_binding *lbinding = NULL; - if (!binding_rec->parent_port || !binding_rec->parent_port[0]) { - lbinding = local_binding_find(b_ctx_out->local_bindings, - binding_rec->logical_port); - } else { - lbinding = local_binding_find(b_ctx_out->local_bindings, - binding_rec->parent_port); - } - - if (lbinding) { - changed = true; - break; - } - } - - return changed; -} - /* Returns true if the database is all cleaned up, false if more work is * required. */ bool @@ -1245,3 +1278,545 @@ binding_cleanup(struct ovsdb_idl_txn *ovnsb_idl_txn, return !any_changes; } + +static void +add_local_datapath_peer_port(const struct sbrec_port_binding *pb, + struct binding_ctx_in *b_ctx_in, + struct binding_ctx_out *b_ctx_out, + struct local_datapath *ld) +{ + 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 (!present) { + ld->n_peer_ports++; + if (ld->n_peer_ports > ld->n_allocated_peer_ports) { + ld->peer_ports = + x2nrealloc(ld->peer_ports, + &ld->n_allocated_peer_ports, + sizeof *ld->peer_ports); + } + ld->peer_ports[ld->n_peer_ports - 1].local = pb; + ld->peer_ports[ld->n_peer_ports - 1].remote = peer; + } + + struct local_datapath *peer_ld = + get_local_datapath(b_ctx_out->local_datapaths, + peer->datapath->tunnel_key); + if (!peer_ld) { + add_local_datapath__(b_ctx_in->sbrec_datapath_binding_by_key, + b_ctx_in->sbrec_port_binding_by_datapath, + b_ctx_in->sbrec_port_binding_by_name, + peer->datapath, false, + 1, b_ctx_out->local_datapaths); + return; + } + + for (size_t i = 0; i < peer_ld->n_peer_ports; i++) { + if (peer_ld->peer_ports[i].local == peer) { + return; + } + } + + peer_ld->n_peer_ports++; + if (peer_ld->n_peer_ports > peer_ld->n_allocated_peer_ports) { + peer_ld->peer_ports = + x2nrealloc(peer_ld->peer_ports, + &peer_ld->n_allocated_peer_ports, + sizeof *peer_ld->peer_ports); + } + peer_ld->peer_ports[peer_ld->n_peer_ports - 1].local = peer; + peer_ld->peer_ports[peer_ld->n_peer_ports - 1].remote = pb; +} + +static void +remove_local_datapath_peer_port(const struct sbrec_port_binding *pb, + struct local_datapath *ld, + struct hmap *local_datapaths) +{ + size_t i = 0; + for (i = 0; i < ld->n_peer_ports; i++) { + if (ld->peer_ports[i].local == pb) { + break; + } + } + + if (i == ld->n_peer_ports) { + return; + } + + const struct sbrec_port_binding *peer = ld->peer_ports[i].remote; + + /* 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, 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(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; + + struct hmap qos_map = HMAP_INITIALIZER(&qos_map); + struct hmap *qos_map_ptr = + sset_is_empty(b_ctx_out->egress_ifaces) ? NULL : &qos_map; + + const struct ovsrec_interface *iface_rec; + OVSREC_INTERFACE_TABLE_FOR_EACH_TRACKED (iface_rec, + b_ctx_in->iface_table) { + const char *iface_id = smap_get(&iface_rec->external_ids, "iface-id"); + const char *old_iface_id = smap_get(b_ctx_out->local_iface_ids, + iface_rec->name); + + if (!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 *cleared_iface_id = NULL; + int64_t ofport = iface_rec->n_ofport ? *iface_rec->ofport : 0; + if (!ovsrec_interface_is_deleted(iface_rec)) { + if (iface_id) { + /* Check if iface_id is changed. If so we need to + * release the old port binding and associate this + * inteface to new port binding. */ + if (old_iface_id && strcmp(iface_id, old_iface_id)) { + cleared_iface_id = old_iface_id; + } + } else if (old_iface_id) { + cleared_iface_id = old_iface_id; + } + } else { + cleared_iface_id = iface_id; + iface_id = NULL; + } + + if (cleared_iface_id) { + handled = handle_iface_delete(cleared_iface_id, iface_rec->name, + b_ctx_in, b_ctx_out, changed); + + if (!handled) { + break; + } + } + + 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; +} + +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) +{ + 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(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, qos_map); + } + + if (!handled) { + return false; + } + + bool now_claimed = (pb->chassis == b_ctx_in->chassis_rec); + *changed = (claimed != now_claimed); + + if (lport_type == LP_VIRTUAL || + (lport_type == LP_VIF && is_lport_container(pb)) || !(*changed)) { + return 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; + } + } + } + + 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; + struct hmap qos_map = HMAP_INITIALIZER(&qos_map); + struct hmap *qos_map_ptr = + sset_is_empty(b_ctx_out->egress_ifaces) ? NULL : &qos_map; + + *changed = false; + + const struct sbrec_port_binding *pb; + SBREC_PORT_BINDING_TABLE_FOR_EACH_TRACKED (pb, + b_ctx_in->port_binding_table) { + bool is_deleted = sbrec_port_binding_is_deleted(pb); + enum en_lport_type lport_type = get_lport_type(pb); + + if (is_deleted) { + 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; + } + } else { + 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; + } + } + + 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 3bda1065c..b1f70ac16 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; } @@ -1001,6 +1004,7 @@ en_runtime_data_cleanup(void *data) sset_destroy(&rt_data->active_tunnels); sset_destroy(&rt_data->egress_ifaces); sset_init(&rt_data->egress_ifaces); + smap_destroy(&rt_data->local_iface_ids); struct local_datapath *cur_node, *next_node; HMAP_FOR_EACH_SAFE (cur_node, next_node, hmap_node, &rt_data->local_datapaths) { @@ -1042,6 +1046,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)); @@ -1071,6 +1079,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; @@ -1084,6 +1093,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 @@ -1112,10 +1122,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); } @@ -1141,6 +1153,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) { @@ -1148,11 +1188,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], diff --git a/tests/ovn.at b/tests/ovn.at index f39fda2e4..b8784bbe5 100644 --- a/tests/ovn.at +++ b/tests/ovn.at @@ -8102,6 +8102,47 @@ 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-sbctl show + +as hv1 ovs-vsctl show + +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)]) + +# Delete vm1 port +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 Sat May 16 17:56:40 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Numan Siddique X-Patchwork-Id: 1291946 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 49PXzG35kfz9sTK for ; Sun, 17 May 2020 03:56:57 +1000 (AEST) Received: from localhost (localhost [127.0.0.1]) by hemlock.osuosl.org (Postfix) with ESMTP id 6592788CF2; Sat, 16 May 2020 17:56: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 I+cTIAIBim78; Sat, 16 May 2020 17:56:55 +0000 (UTC) Received: from lists.linuxfoundation.org (lf-lists.osuosl.org [140.211.9.56]) by hemlock.osuosl.org (Postfix) with ESMTP id E11DD88C01; Sat, 16 May 2020 17:56:53 +0000 (UTC) Received: from lf-lists.osuosl.org (localhost [127.0.0.1]) by lists.linuxfoundation.org (Postfix) with ESMTP id C0F69C0865; Sat, 16 May 2020 17:56:53 +0000 (UTC) X-Original-To: dev@openvswitch.org Delivered-To: ovs-dev@lists.linuxfoundation.org Received: from fraxinus.osuosl.org (smtp4.osuosl.org [140.211.166.137]) by lists.linuxfoundation.org (Postfix) with ESMTP id 1326EC0865 for ; Sat, 16 May 2020 17:56:52 +0000 (UTC) Received: from localhost (localhost [127.0.0.1]) by fraxinus.osuosl.org (Postfix) with ESMTP id 0960787386 for ; Sat, 16 May 2020 17:56:52 +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 gzx0mWr-uB_H for ; Sat, 16 May 2020 17:56:49 +0000 (UTC) X-Greylist: domain auto-whitelisted by SQLgrey-1.7.6 Received: from relay4-d.mail.gandi.net (relay4-d.mail.gandi.net [217.70.183.196]) by fraxinus.osuosl.org (Postfix) with ESMTPS id 3F848872BC for ; Sat, 16 May 2020 17:56:49 +0000 (UTC) X-Originating-IP: 115.99.184.184 Received: from nusiddiq.home.org.com (unknown [115.99.184.184]) (Authenticated sender: numans@ovn.org) by relay4-d.mail.gandi.net (Postfix) with ESMTPSA id A0FFCE0002; Sat, 16 May 2020 17:56:46 +0000 (UTC) From: numans@ovn.org To: dev@openvswitch.org Date: Sat, 16 May 2020 23:26:40 +0530 Message-Id: <20200516175640.4003790-1-numans@ovn.org> X-Mailer: git-send-email 2.26.2 In-Reply-To: <20200516175539.4003530-1-numans@ovn.org> References: <20200516175539.4003530-1-numans@ovn.org> MIME-Version: 1.0 Subject: [ovs-dev] [PATCH ovn v6 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. Signed-off-by: Numan Siddique --- controller/ovn-controller.c | 25 ++++++++++++++++++++++++- tests/ovn-performance.at | 6 +++--- 2 files changed, 27 insertions(+), 4 deletions(-) diff --git a/controller/ovn-controller.c b/controller/ovn-controller.c index b1f70ac16..4a251b2a9 100644 --- a/controller/ovn-controller.c +++ b/controller/ovn-controller.c @@ -1205,6 +1205,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 Sat May 16 17:56:47 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Numan Siddique X-Patchwork-Id: 1291948 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 49PXzb532lz9sTT for ; Sun, 17 May 2020 03:57:15 +1000 (AEST) Received: from localhost (localhost [127.0.0.1]) by silver.osuosl.org (Postfix) with ESMTP id 0DE64227FC; Sat, 16 May 2020 17:57:14 +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 6ZyyWK4xhl6s; Sat, 16 May 2020 17:57:04 +0000 (UTC) Received: from lists.linuxfoundation.org (lf-lists.osuosl.org [140.211.9.56]) by silver.osuosl.org (Postfix) with ESMTP id 9FBBF20532; Sat, 16 May 2020 17:57:00 +0000 (UTC) Received: from lf-lists.osuosl.org (localhost [127.0.0.1]) by lists.linuxfoundation.org (Postfix) with ESMTP id 923DCC088F; Sat, 16 May 2020 17:57:00 +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 4F8EDC016F for ; Sat, 16 May 2020 17:56:59 +0000 (UTC) Received: from localhost (localhost [127.0.0.1]) by fraxinus.osuosl.org (Postfix) with ESMTP id 48E0D873A3 for ; Sat, 16 May 2020 17:56:59 +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 ryBcznyla9-v for ; Sat, 16 May 2020 17:56:58 +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 fraxinus.osuosl.org (Postfix) with ESMTPS id 98E4C874AB for ; Sat, 16 May 2020 17:56:57 +0000 (UTC) X-Originating-IP: 115.99.184.184 Received: from nusiddiq.home.org.com (unknown [115.99.184.184]) (Authenticated sender: numans@ovn.org) by relay3-d.mail.gandi.net (Postfix) with ESMTPSA id 4CB3160002; Sat, 16 May 2020 17:56:52 +0000 (UTC) From: numans@ovn.org To: dev@openvswitch.org Date: Sat, 16 May 2020 23:26:47 +0530 Message-Id: <20200516175647.4003845-1-numans@ovn.org> X-Mailer: git-send-email 2.26.2 In-Reply-To: <20200516175539.4003530-1-numans@ovn.org> References: <20200516175539.4003530-1-numans@ovn.org> MIME-Version: 1.0 Subject: [ovs-dev] [PATCH ovn v6 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). 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 Sat May 16 17:56:54 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Numan Siddique X-Patchwork-Id: 1291951 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 49PXzy2yNsz9sTT for ; Sun, 17 May 2020 03:57:34 +1000 (AEST) Received: from localhost (localhost [127.0.0.1]) by hemlock.osuosl.org (Postfix) with ESMTP id B8F5088543; Sat, 16 May 2020 17:57:32 +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 XIOyt5Ss5sFo; Sat, 16 May 2020 17:57:29 +0000 (UTC) Received: from lists.linuxfoundation.org (lf-lists.osuosl.org [140.211.9.56]) by hemlock.osuosl.org (Postfix) with ESMTP id 4291E88D5B; Sat, 16 May 2020 17:57:29 +0000 (UTC) Received: from lf-lists.osuosl.org (localhost [127.0.0.1]) by lists.linuxfoundation.org (Postfix) with ESMTP id 11CDDC08A5; Sat, 16 May 2020 17:57:29 +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 64DF3C016F for ; Sat, 16 May 2020 17:57:27 +0000 (UTC) Received: from localhost (localhost [127.0.0.1]) by silver.osuosl.org (Postfix) with ESMTP id 5B4D720525 for ; Sat, 16 May 2020 17:57:27 +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 z6KySqO-0Uoo for ; Sat, 16 May 2020 17:57:20 +0000 (UTC) X-Greylist: domain auto-whitelisted by SQLgrey-1.7.6 Received: from relay5-d.mail.gandi.net (relay5-d.mail.gandi.net [217.70.183.197]) by silver.osuosl.org (Postfix) with ESMTPS id 0CD6A23358 for ; Sat, 16 May 2020 17:57:05 +0000 (UTC) X-Originating-IP: 115.99.184.184 Received: from nusiddiq.home.org.com (unknown [115.99.184.184]) (Authenticated sender: numans@ovn.org) by relay5-d.mail.gandi.net (Postfix) with ESMTPSA id 9C2821C0003; Sat, 16 May 2020 17:57:01 +0000 (UTC) From: numans@ovn.org To: dev@openvswitch.org Date: Sat, 16 May 2020 23:26:54 +0530 Message-Id: <20200516175654.4003897-1-numans@ovn.org> X-Mailer: git-send-email 2.26.2 In-Reply-To: <20200516175539.4003530-1-numans@ovn.org> References: <20200516175539.4003530-1-numans@ovn.org> MIME-Version: 1.0 Subject: [ovs-dev] [PATCH ovn v6 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. 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 5b7cde81d..7e8adacd9 100644 --- a/controller/binding.c +++ b/controller/binding.c @@ -491,7 +491,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 @@ -527,21 +527,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, @@ -563,12 +548,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 4a251b2a9..10da807e4 100644 --- a/controller/ovn-controller.c +++ b/controller/ovn-controller.c @@ -1370,8 +1370,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; @@ -1379,12 +1384,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, @@ -1792,6 +1799,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 Sat May 16 17:57:03 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Numan Siddique X-Patchwork-Id: 1291949 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 49PXzg0qnDz9sTK for ; Sun, 17 May 2020 03:57:19 +1000 (AEST) Received: from localhost (localhost [127.0.0.1]) by fraxinus.osuosl.org (Postfix) with ESMTP id A3E5E87300; Sat, 16 May 2020 17:57:17 +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 0EJWzaYUHs5O; Sat, 16 May 2020 17:57:16 +0000 (UTC) Received: from lists.linuxfoundation.org (lf-lists.osuosl.org [140.211.9.56]) by fraxinus.osuosl.org (Postfix) with ESMTP id CAE5F86F3B; Sat, 16 May 2020 17:57:16 +0000 (UTC) Received: from lf-lists.osuosl.org (localhost [127.0.0.1]) by lists.linuxfoundation.org (Postfix) with ESMTP id B2A36C0865; Sat, 16 May 2020 17:57:16 +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 7966AC016F for ; Sat, 16 May 2020 17:57:15 +0000 (UTC) Received: from localhost (localhost [127.0.0.1]) by fraxinus.osuosl.org (Postfix) with ESMTP id 632AE87241 for ; Sat, 16 May 2020 17:57:15 +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 aRH1R20n5BMI for ; Sat, 16 May 2020 17:57:13 +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 25506870B0 for ; Sat, 16 May 2020 17:57:12 +0000 (UTC) Received: from nusiddiq.home.org.com (unknown [115.99.184.184]) (Authenticated sender: numans@ovn.org) by relay10.mail.gandi.net (Postfix) with ESMTPSA id 1BD5B240003; Sat, 16 May 2020 17:57:08 +0000 (UTC) From: numans@ovn.org To: dev@openvswitch.org Date: Sat, 16 May 2020 23:27:03 +0530 Message-Id: <20200516175703.4003948-1-numans@ovn.org> X-Mailer: git-send-email 2.26.2 In-Reply-To: <20200516175539.4003530-1-numans@ovn.org> References: <20200516175539.4003530-1-numans@ovn.org> MIME-Version: 1.0 Subject: [ovs-dev] [PATCH ovn v6 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. 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 Sat May 16 17:57:10 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Numan Siddique X-Patchwork-Id: 1291950 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 49PXzt63lHz9sTK for ; Sun, 17 May 2020 03:57:30 +1000 (AEST) Received: from localhost (localhost [127.0.0.1]) by fraxinus.osuosl.org (Postfix) with ESMTP id 5EE328737E; Sat, 16 May 2020 17:57: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 G4Y4g2bGEeMm; Sat, 16 May 2020 17:57:27 +0000 (UTC) Received: from lists.linuxfoundation.org (lf-lists.osuosl.org [140.211.9.56]) by fraxinus.osuosl.org (Postfix) with ESMTP id B301C86F3B; Sat, 16 May 2020 17:57:27 +0000 (UTC) Received: from lf-lists.osuosl.org (localhost [127.0.0.1]) by lists.linuxfoundation.org (Postfix) with ESMTP id A6671C0893; Sat, 16 May 2020 17:57: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 56B18C016F for ; Sat, 16 May 2020 17:57:26 +0000 (UTC) Received: from localhost (localhost [127.0.0.1]) by hemlock.osuosl.org (Postfix) with ESMTP id 45B5E88BC3 for ; Sat, 16 May 2020 17:57: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 KVVuaeVrVzsu for ; Sat, 16 May 2020 17:57:23 +0000 (UTC) X-Greylist: domain auto-whitelisted by SQLgrey-1.7.6 Received: from relay5-d.mail.gandi.net (relay5-d.mail.gandi.net [217.70.183.197]) by hemlock.osuosl.org (Postfix) with ESMTPS id 473E788E5B for ; Sat, 16 May 2020 17:57:19 +0000 (UTC) X-Originating-IP: 115.99.184.184 Received: from nusiddiq.home.org.com (unknown [115.99.184.184]) (Authenticated sender: numans@ovn.org) by relay5-d.mail.gandi.net (Postfix) with ESMTPSA id 248F61C0005; Sat, 16 May 2020 17:57:15 +0000 (UTC) From: numans@ovn.org To: dev@openvswitch.org Date: Sat, 16 May 2020 23:27:10 +0530 Message-Id: <20200516175710.4003999-1-numans@ovn.org> X-Mailer: git-send-email 2.26.2 In-Reply-To: <20200516175539.4003530-1-numans@ovn.org> References: <20200516175539.4003530-1-numans@ovn.org> MIME-Version: 1.0 Cc: Venkata Anil Subject: [ovs-dev] [PATCH ovn v6 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. Co-Authored-by: Venkata Anil Signed-off-by: Venkata Anil Signed-off-by: Numan Siddique --- controller/binding.c | 158 ++++++++++++++++++++++++++++++++---- controller/binding.h | 21 +++++ controller/ovn-controller.c | 62 +++++++++++++- tests/ovn-performance.at | 28 +++---- 4 files changed, 240 insertions(+), 29 deletions(-) diff --git a/controller/binding.c b/controller/binding.c index 7e8adacd9..91542f8a1 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 @@ -609,6 +624,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_VIF, @@ -751,7 +831,8 @@ can_bind_on_this_chassis(const struct sbrec_chassis *chassis_rec, static bool release_local_binding_children(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) { @@ -759,15 +840,22 @@ release_local_binding_children(struct local_binding *lbinding, if (!release_lport(l->pb, sb_readonly)) { return false; } + if (tracked_dp_bindings) { + tracked_binding_datapath_lport_add(l->pb, true, + tracked_dp_bindings); + } } return true; } static bool -release_local_binding(struct local_binding *lbinding, bool sb_readonly) +release_local_binding(struct local_binding *lbinding, + bool sb_readonly, + struct hmap *tracked_dp_bindings) { - if (!release_local_binding_children(lbinding, sb_readonly)) { + if (!release_local_binding_children(lbinding, sb_readonly, + tracked_dp_bindings)) { return false; } @@ -775,6 +863,10 @@ release_local_binding(struct local_binding *lbinding, bool sb_readonly) return false; } + if (tracked_dp_bindings) { + tracked_binding_datapath_lport_add(lbinding->pb, true, + tracked_dp_bindings); + } return true; } @@ -798,7 +890,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); @@ -960,7 +1053,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, @@ -1037,7 +1131,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); @@ -1305,7 +1400,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; } @@ -1382,6 +1478,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, @@ -1410,12 +1518,17 @@ 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, qos_map)) { return false; } } + 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; @@ -1423,10 +1536,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); } } @@ -1444,7 +1562,8 @@ handle_iface_delete(const char *lport_name, const char *iface_name, if (lbinding && lbinding->pb && lbinding->pb->chassis == b_ctx_in->chassis_rec) { - if (!release_local_binding(lbinding, !b_ctx_in->ovnsb_idl_txn)) { + if (!release_local_binding(lbinding, !b_ctx_in->ovnsb_idl_txn, + b_ctx_out->tracked_dp_bindings)) { return false; } @@ -1496,6 +1615,7 @@ binding_handle_ovs_interface_changes(struct binding_ctx_in *b_ctx_in, struct hmap *qos_map_ptr = sset_is_empty(b_ctx_out->egress_ifaces) ? NULL : &qos_map; + *b_ctx_out->local_lports_changed = false; const struct ovsrec_interface *iface_rec; OVSREC_INTERFACE_TABLE_FOR_EACH_TRACKED (iface_rec, b_ctx_in->iface_table) { @@ -1618,8 +1738,9 @@ 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(lbinding, - !b_ctx_in->ovnsb_idl_txn)) { + !release_local_binding_children( + lbinding, !b_ctx_in->ovnsb_idl_txn, + b_ctx_out->tracked_dp_bindings)) { return false; } @@ -1661,6 +1782,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); @@ -1670,11 +1794,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 10da807e4..282081e79 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; } @@ -1094,6 +1118,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 @@ -1105,6 +1131,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 */ @@ -1157,9 +1185,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, @@ -1192,6 +1224,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)) { @@ -1863,6 +1899,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 Sat May 16 17:57:18 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Numan Siddique X-Patchwork-Id: 1291953 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 49PY0m3rN0z9sTK for ; Sun, 17 May 2020 03:58:16 +1000 (AEST) Received: from localhost (localhost [127.0.0.1]) by whitealder.osuosl.org (Postfix) with ESMTP id 0F20488A7B; Sat, 16 May 2020 17:58:15 +0000 (UTC) X-Virus-Scanned: amavisd-new at osuosl.org Received: from whitealder.osuosl.org ([127.0.0.1]) by localhost (.osuosl.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id CSXqtG7GCfZC; Sat, 16 May 2020 17:58:06 +0000 (UTC) Received: from lists.linuxfoundation.org (lf-lists.osuosl.org [140.211.9.56]) by whitealder.osuosl.org (Postfix) with ESMTP id 5F9C488C3F; Sat, 16 May 2020 17:57:32 +0000 (UTC) Received: from lf-lists.osuosl.org (localhost [127.0.0.1]) by lists.linuxfoundation.org (Postfix) with ESMTP id 3FA07C0894; Sat, 16 May 2020 17:57:32 +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 223F0C0865 for ; Sat, 16 May 2020 17:57:31 +0000 (UTC) Received: from localhost (localhost [127.0.0.1]) by hemlock.osuosl.org (Postfix) with ESMTP id 1E2A788543 for ; Sat, 16 May 2020 17:57:31 +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 rcPft7Z7Wghx for ; Sat, 16 May 2020 17:57:27 +0000 (UTC) X-Greylist: domain auto-whitelisted by SQLgrey-1.7.6 Received: from relay8-d.mail.gandi.net (relay8-d.mail.gandi.net [217.70.183.201]) by hemlock.osuosl.org (Postfix) with ESMTPS id 41E1788B39 for ; Sat, 16 May 2020 17:57:27 +0000 (UTC) X-Originating-IP: 115.99.184.184 Received: from nusiddiq.home.org.com (unknown [115.99.184.184]) (Authenticated sender: numans@ovn.org) by relay8-d.mail.gandi.net (Postfix) with ESMTPSA id 110371BF207; Sat, 16 May 2020 17:57:22 +0000 (UTC) From: numans@ovn.org To: dev@openvswitch.org Date: Sat, 16 May 2020 23:27:18 +0530 Message-Id: <20200516175718.4004050-1-numans@ovn.org> X-Mailer: git-send-email 2.26.2 In-Reply-To: <20200516175539.4003530-1-numans@ovn.org> References: <20200516175539.4003530-1-numans@ovn.org> MIME-Version: 1.0 Cc: Venkata Anil Subject: [ovs-dev] [PATCH ovn v6 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. 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 282081e79..410ef96fa 100644 --- a/controller/ovn-controller.c +++ b/controller/ovn-controller.c @@ -1441,6 +1441,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), @@ -1488,6 +1493,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; @@ -1642,7 +1649,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); @@ -1650,56 +1658,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); @@ -1787,6 +1760,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(); } @@ -1910,16 +1885,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 Sat May 16 17:57:24 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Numan Siddique X-Patchwork-Id: 1291954 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 49PY170Ydcz9sTK for ; Sun, 17 May 2020 03:58:35 +1000 (AEST) Received: from localhost (localhost [127.0.0.1]) by silver.osuosl.org (Postfix) with ESMTP id 8A897232D2; Sat, 16 May 2020 17:58:33 +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 cOavlrh+lO4Q; Sat, 16 May 2020 17:58:28 +0000 (UTC) Received: from lists.linuxfoundation.org (lf-lists.osuosl.org [140.211.9.56]) by silver.osuosl.org (Postfix) with ESMTP id 7674D233B0; Sat, 16 May 2020 17:57:40 +0000 (UTC) Received: from lf-lists.osuosl.org (localhost [127.0.0.1]) by lists.linuxfoundation.org (Postfix) with ESMTP id 5C8BBC0865; Sat, 16 May 2020 17:57:40 +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 7F0B8C0865 for ; Sat, 16 May 2020 17:57:39 +0000 (UTC) Received: from localhost (localhost [127.0.0.1]) by fraxinus.osuosl.org (Postfix) with ESMTP id 6F4F2870BD for ; Sat, 16 May 2020 17:57:39 +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 svEtdgW4HOxA for ; Sat, 16 May 2020 17:57:36 +0000 (UTC) X-Greylist: domain auto-whitelisted by SQLgrey-1.7.6 Received: from relay5-d.mail.gandi.net (relay5-d.mail.gandi.net [217.70.183.197]) by fraxinus.osuosl.org (Postfix) with ESMTPS id 1B14E873F0 for ; Sat, 16 May 2020 17:57:33 +0000 (UTC) X-Originating-IP: 115.99.184.184 Received: from nusiddiq.home.org.com (unknown [115.99.184.184]) (Authenticated sender: numans@ovn.org) by relay5-d.mail.gandi.net (Postfix) with ESMTPSA id A98671C0002; Sat, 16 May 2020 17:57:31 +0000 (UTC) From: numans@ovn.org To: dev@openvswitch.org Date: Sat, 16 May 2020 23:27:24 +0530 Message-Id: <20200516175724.4004103-1-numans@ovn.org> X-Mailer: git-send-email 2.26.2 In-Reply-To: <20200516175539.4003530-1-numans@ovn.org> References: <20200516175539.4003530-1-numans@ovn.org> MIME-Version: 1.0 Subject: [ovs-dev] [PATCH ovn v6 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. 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 410ef96fa..2bb56b6e0 100644 --- a/controller/ovn-controller.c +++ b/controller/ovn-controller.c @@ -1382,7 +1382,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); @@ -1398,7 +1399,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); @@ -1475,7 +1477,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); @@ -1556,8 +1559,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) { @@ -1722,8 +1726,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); @@ -1874,6 +1879,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,