From patchwork Wed Jan 6 16:37:37 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Dumitru Ceara X-Patchwork-Id: 1423004 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=fail (p=none dis=none) header.from=redhat.com Authentication-Results: ozlabs.org; dkim=fail reason="signature verification failed" (1024-bit key; unprotected) header.d=redhat.com header.i=@redhat.com header.a=rsa-sha256 header.s=mimecast20190719 header.b=TlEe7ozi; dkim-atps=neutral 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 4D9w612w2Qz9sVb for ; Thu, 7 Jan 2021 03:38:17 +1100 (AEDT) Received: from localhost (localhost [127.0.0.1]) by silver.osuosl.org (Postfix) with ESMTP id 99A9A27366; Wed, 6 Jan 2021 16:38:15 +0000 (UTC) X-Virus-Scanned: amavisd-new at osuosl.org Received: from silver.osuosl.org ([127.0.0.1]) by localhost (.osuosl.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id 7SvdaYATnAap; Wed, 6 Jan 2021 16:38:00 +0000 (UTC) Received: from lists.linuxfoundation.org (lf-lists.osuosl.org [140.211.9.56]) by silver.osuosl.org (Postfix) with ESMTP id DFDDF272BB; Wed, 6 Jan 2021 16:37:59 +0000 (UTC) Received: from lf-lists.osuosl.org (localhost [127.0.0.1]) by lists.linuxfoundation.org (Postfix) with ESMTP id C6E2DC0891; Wed, 6 Jan 2021 16:37:59 +0000 (UTC) X-Original-To: dev@openvswitch.org Delivered-To: ovs-dev@lists.linuxfoundation.org Received: from whitealder.osuosl.org (smtp1.osuosl.org [140.211.166.138]) by lists.linuxfoundation.org (Postfix) with ESMTP id BFF48C013A for ; Wed, 6 Jan 2021 16:37:58 +0000 (UTC) Received: from localhost (localhost [127.0.0.1]) by whitealder.osuosl.org (Postfix) with ESMTP id AF49A86BE1 for ; Wed, 6 Jan 2021 16:37:58 +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 zZF01LN+g9g8 for ; Wed, 6 Jan 2021 16:37:52 +0000 (UTC) X-Greylist: domain auto-whitelisted by SQLgrey-1.7.6 Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [216.205.24.124]) by whitealder.osuosl.org (Postfix) with ESMTPS id DC22786BCB for ; Wed, 6 Jan 2021 16:37:44 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1609951063; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=ucmV4e2Rw0TTqD4/0/6qZgfmdaSwxtmEwKLxPTJh/Sg=; b=TlEe7ozi+35Tn0woiW97Ej9SchcTZEXOPSnK05l5YYbwwA0qzYvt/gnekFiEyfmJaGfXG9 fuLgSW49+rpjyZgkARXLOAgo3yVLs+3xZ8oBYJrd+GX3Z8wdnP57eIHvraLw7DhQQ03i67 mIoezLen6qDNLB0PTFGEJLK++zuBw/M= Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-313-4xuhZ7cENkGsikUm9By23Q-1; Wed, 06 Jan 2021 11:37:41 -0500 X-MC-Unique: 4xuhZ7cENkGsikUm9By23Q-1 Received: from smtp.corp.redhat.com (int-mx06.intmail.prod.int.phx2.redhat.com [10.5.11.16]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id E68708049C4 for ; Wed, 6 Jan 2021 16:37:40 +0000 (UTC) Received: from dceara.remote.csb (ovpn-113-197.ams2.redhat.com [10.36.113.197]) by smtp.corp.redhat.com (Postfix) with ESMTP id 198FC5C1C4 for ; Wed, 6 Jan 2021 16:37:39 +0000 (UTC) From: Dumitru Ceara To: dev@openvswitch.org Date: Wed, 6 Jan 2021 17:37:37 +0100 Message-Id: <20210106163734.20347.51643.stgit@dceara.remote.csb> In-Reply-To: <20210106163634.20347.76943.stgit@dceara.remote.csb> References: <20210106163634.20347.76943.stgit@dceara.remote.csb> User-Agent: StGit/0.17.1-dirty MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.79 on 10.5.11.16 Authentication-Results: relay.mimecast.com; auth=pass smtp.auth=CUSA124A263 smtp.mailfrom=dceara@redhat.com X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Subject: [ovs-dev] [PATCH ovn 3/3] binding: Set Logical_Switch_Port.up when all OVS flows are installed. 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" Using the ofctrl-seqno generic barrier, register a new type of notifications for Port_Bindings. This allows us to delay setting the Logical_Switch_Port.up field until all OVS flows corresponding to the logical port and underlying OVS interface have been installed. This commit also marks the OVS interface as "installed by OVN" by setting a new "ovn-installed" external-id in the OVS Interface record when the port is fully wired by OVN. Signed-off-by: Dumitru Ceara --- NEWS | 6 + controller/binding.c | 174 +++++++++++++++++++++++++++++++++++++++++++ controller/binding.h | 5 + controller/ovn-controller.c | 14 +++ northd/ovn-northd.c | 4 + ovn-sb.ovsschema | 5 + ovn-sb.xml | 8 ++ utilities/ovn-sbctl.c | 4 + 8 files changed, 214 insertions(+), 6 deletions(-) diff --git a/NEWS b/NEWS index f0ac94b..058ebe4 100644 --- a/NEWS +++ b/NEWS @@ -1,6 +1,12 @@ Post-v20.12.0 ------------------------- - Support ECMP multiple nexthops for reroute router policies. + - Change the semantic of the "Logical_Switch_Port.up" field such that it is + set to "true" only when all corresponding OVS openflow operations have + been processed. This also introduces a new "OVS.Interface.external-id", + "ovn-installed". This external-id is set by ovn-controller only after all + openflow operations corresponding to the OVS interface being added have + been processed. OVN v20.12.0 - xx xxx xxxx -------------------------- diff --git a/controller/binding.c b/controller/binding.c index e632203..8582e45 100644 --- a/controller/binding.c +++ b/controller/binding.c @@ -18,6 +18,7 @@ #include "ha-chassis.h" #include "lflow.h" #include "lport.h" +#include "ofctrl-seqno.h" #include "patch.h" #include "lib/bitmap.h" @@ -34,6 +35,38 @@ VLOG_DEFINE_THIS_MODULE(binding); +/* External ID to be set in the OVS.Interface record when the OVS interface + * is ready for use, i.e., is bound to an OVN port and its corresponding + * flows have been installed. + */ +#define OVN_INSTALLED_EXT_ID "ovn-installed" + +/* Set of OVS interface IDs that have been released in the most recent + * processing iterations. This gets updated in release_lport() and is + * periodically emptied in binding_seqno_run(). + */ +static struct sset binding_iface_released_set = + SSET_INITIALIZER(&binding_iface_released_set); + +/* Set of OVS interface IDs that have been bound in the most recent + * processing iterations. This gets updated in release_lport() and is + * periodically emptied in binding_seqno_run(). + */ +static struct sset binding_iface_bound_set = + SSET_INITIALIZER(&binding_iface_bound_set); + +static void +binding_iface_released_add(const char *iface_id) +{ + sset_add(&binding_iface_released_set, iface_id); +} + +static void +binding_iface_bound_add(const char *iface_id) +{ + sset_add(&binding_iface_bound_set, iface_id); +} + #define OVN_QOS_TYPE "linux-htb" struct qos_queue { @@ -837,6 +870,7 @@ claim_lport(const struct sbrec_port_binding *pb, return false; } + binding_iface_bound_add(pb->logical_port); if (pb->chassis) { VLOG_INFO("Changing chassis for lport %s from %s to %s.", pb->logical_port, pb->chassis->name, @@ -900,7 +934,9 @@ release_lport(const struct sbrec_port_binding *pb, bool sb_readonly, sbrec_port_binding_set_virtual_parent(pb, NULL); } + sbrec_port_binding_set_up(pb, NULL, 0); update_lport_tracking(pb, tracked_datapaths); + binding_iface_released_add(pb->logical_port); VLOG_INFO("Releasing lport %s from this chassis.", pb->logical_port); return true; } @@ -2287,3 +2323,141 @@ binding_handle_port_binding_changes(struct binding_ctx_in *b_ctx_in, destroy_qos_map(&qos_map); return handled; } + +/* Registered ofctrl seqno type for port_binding flow installation. */ +static size_t binding_seq_type_pb_cfg; + +/* Binding specific seqno to be acked by ofctrl when flows for new interfaces + * have been installed. + */ +static uint32_t binding_iface_seqno = 0; + +/* Map indexed by iface-id containing the sequence numbers that when acked + * indicate that the OVS flows for the iface-id have been installed. + */ +static struct simap binding_iface_seqno_map = + SIMAP_INITIALIZER(&binding_iface_seqno_map); + +void +binding_init(void) +{ + binding_seq_type_pb_cfg = ofctrl_seqno_add_type(); +} + +/* Processes new release/bind operations OVN ports. For newly bound ports + * it creates ofctrl seqno update requests that will be acked when + * corresponding OVS flows have been installed. + * + * NOTE: Should be called only when valid SB and OVS transactions are + * available. + */ +void +binding_seqno_run(struct shash *local_bindings) +{ + const char *iface_id; + const char *iface_id_next; + + SSET_FOR_EACH_SAFE (iface_id, iface_id_next, &binding_iface_released_set) { + struct shash_node *lb_node = shash_find(local_bindings, iface_id); + + /* If the local binding still exists (i.e., the OVS interface is + * still configured locally) then remove the external id and remove + * it from the in-flight seqno map. + */ + if (lb_node) { + struct local_binding *lb = lb_node->data; + + if (lb->iface && smap_get(&lb->iface->external_ids, + OVN_INSTALLED_EXT_ID)) { + ovsrec_interface_update_external_ids_delkey( + lb->iface, OVN_INSTALLED_EXT_ID); + } + } + simap_find_and_delete(&binding_iface_seqno_map, iface_id); + sset_delete(&binding_iface_released_set, + SSET_NODE_FROM_NAME(iface_id)); + } + + bool new_ifaces = false; + uint32_t new_seqno = binding_iface_seqno + 1; + + SSET_FOR_EACH_SAFE (iface_id, iface_id_next, &binding_iface_bound_set) { + struct shash_node *lb_node = shash_find(local_bindings, iface_id); + + if (lb_node) { + /* This is a newly bound interface, make sure we reset the + * Port_Binding 'up' field and the OVS Interface 'external-id'. + */ + struct local_binding *lb = lb_node->data; + + ovs_assert(lb->pb && lb->iface); + new_ifaces = true; + + if (lb->iface && smap_get(&lb->iface->external_ids, + OVN_INSTALLED_EXT_ID)) { + ovsrec_interface_update_external_ids_delkey( + lb->iface, OVN_INSTALLED_EXT_ID); + } + if (lb->pb) { + sbrec_port_binding_set_up(lb->pb, NULL, 0); + } + simap_put(&binding_iface_seqno_map, lb->name, new_seqno); + } + sset_delete(&binding_iface_bound_set, SSET_NODE_FROM_NAME(iface_id)); + } + + /* Request a seqno update when the flows for new interfaces have been + * installed in OVS. + */ + if (new_ifaces) { + binding_iface_seqno = new_seqno; + ofctrl_seqno_update_create(binding_seq_type_pb_cfg, new_seqno); + } +} + +/* Processes ofctrl seqno ACKs for new bindings. Sets the + * 'OVN_INSTALLED_EXT_ID' external-id in the OVS interface and the + * Port_Binding.up field for all ports for which OVS flows have been + * installed. + * + * NOTE: Should be called only when valid SB and OVS transactions are + * available. + */ +void +binding_seqno_install(struct shash *local_bindings) +{ + struct ofctrl_acked_seqnos *acked_seqnos = + ofctrl_acked_seqnos_get(binding_seq_type_pb_cfg); + struct simap_node *node; + struct simap_node *node_next; + + SIMAP_FOR_EACH_SAFE (node, node_next, &binding_iface_seqno_map) { + if (!ofctrl_acked_seqnos_contains(acked_seqnos, node->data)) { + continue; + } + + struct shash_node *lb_node = shash_find(local_bindings, node->name); + struct local_binding *lb = lb_node->data; + bool up = true; + + ovsrec_interface_update_external_ids_setkey(lb->iface, + OVN_INSTALLED_EXT_ID, + "true"); + sbrec_port_binding_set_up(lb->pb, &up, 1); + + struct shash_node *child_node; + SHASH_FOR_EACH (child_node, &lb->children) { + struct local_binding *lb_child = child_node->data; + sbrec_port_binding_set_up(lb_child->pb, &up, 1); + } + simap_delete(&binding_iface_seqno_map, node); + } + + ofctrl_acked_seqnos_destroy(acked_seqnos); +} + +void +binding_seqno_flush(void) +{ + simap_clear(&binding_iface_seqno_map); +} diff --git a/controller/binding.h b/controller/binding.h index c974056..3bb1690 100644 --- a/controller/binding.h +++ b/controller/binding.h @@ -134,4 +134,9 @@ bool binding_handle_ovs_interface_changes(struct binding_ctx_in *, bool binding_handle_port_binding_changes(struct binding_ctx_in *, struct binding_ctx_out *); void binding_tracked_dp_destroy(struct hmap *tracked_datapaths); + +void binding_init(void); +void binding_seqno_run(struct shash *local_bindings); +void binding_seqno_install(struct shash *local_bindings); +void binding_seqno_flush(void); #endif /* controller/binding.h */ diff --git a/controller/ovn-controller.c b/controller/ovn-controller.c index 7a7825d..f3951b6 100644 --- a/controller/ovn-controller.c +++ b/controller/ovn-controller.c @@ -981,6 +981,7 @@ en_ofctrl_is_connected_run(struct engine_node *node, void *data) /* Flush ofctrl seqno requests when the ofctrl connection goes down. */ if (!of_data->connected) { ofctrl_seqno_flush(); + binding_seqno_flush(); } engine_set_node_state(node, EN_UPDATED); return; @@ -2404,13 +2405,14 @@ main(int argc, char *argv[]) daemonize_complete(); + /* Register ofctrl seqno types. */ + ofctrl_seq_type_nb_cfg = ofctrl_seqno_add_type(); + + binding_init(); patch_init(); pinctrl_init(); lflow_init(); - /* Register ofctrl seqno types. */ - ofctrl_seq_type_nb_cfg = ofctrl_seqno_add_type(); - /* Connect to OVS OVSDB instance. */ struct ovsdb_idl_loop ovs_idl_loop = OVSDB_IDL_LOOP_INITIALIZER( ovsdb_idl_create(ovs_remote, &ovsrec_idl_class, false, true)); @@ -2878,6 +2880,9 @@ main(int argc, char *argv[]) ovnsb_idl_loop.idl), ovnsb_cond_seqno, ovnsb_expected_cond_seqno)); + if (runtime_data && ovs_idl_txn && ovnsb_idl_txn) { + binding_seqno_run(&runtime_data->local_bindings); + } flow_output_data = engine_get_data(&en_flow_output); if (flow_output_data && ct_zones_data) { @@ -2888,6 +2893,9 @@ main(int argc, char *argv[]) engine_node_changed(&en_flow_output)); } ofctrl_seqno_run(ofctrl_get_cur_cfg()); + if (runtime_data && ovs_idl_txn && ovnsb_idl_txn) { + binding_seqno_install(&runtime_data->local_bindings); + } } } diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c index f6acdc1..a9e8edf 100644 --- a/northd/ovn-northd.c +++ b/northd/ovn-northd.c @@ -12678,7 +12678,7 @@ handle_port_binding_changes(struct northd_context *ctx, struct hmap *ports, continue; } - bool up = (sb->chassis || lsp_is_router(op->nbsp)); + bool up = ((sb->up && (*sb->up)) || lsp_is_router(op->nbsp)); if (!op->nbsp->up || *op->nbsp->up != up) { nbrec_logical_switch_port_set_up(op->nbsp, &up, 1); } @@ -13315,6 +13315,8 @@ main(int argc, char *argv[]) ovsdb_idl_add_column(ovnsb_idl_loop.idl, &sbrec_port_binding_col_virtual_parent); ovsdb_idl_add_column(ovnsb_idl_loop.idl, + &sbrec_port_binding_col_up); + ovsdb_idl_add_column(ovnsb_idl_loop.idl, &sbrec_gateway_chassis_col_chassis); ovsdb_idl_add_column(ovnsb_idl_loop.idl, &sbrec_gateway_chassis_col_name); ovsdb_idl_add_column(ovnsb_idl_loop.idl, diff --git a/ovn-sb.ovsschema b/ovn-sb.ovsschema index 5228839..b47879a 100644 --- a/ovn-sb.ovsschema +++ b/ovn-sb.ovsschema @@ -1,7 +1,7 @@ { "name": "OVN_Southbound", - "version": "20.12.0", - "cksum": "3969471120 24441", + "version": "20.12.1", + "cksum": "1448228268 24513", "tables": { "SB_Global": { "columns": { @@ -225,6 +225,7 @@ "nat_addresses": {"type": {"key": "string", "min": 0, "max": "unlimited"}}, + "up": {"type": {"key": "boolean", "min": 0, "max": 1}}, "external_ids": {"type": {"key": "string", "value": "string", "min": 0, diff --git a/ovn-sb.xml b/ovn-sb.xml index c139948..744bbfc 100644 --- a/ovn-sb.xml +++ b/ovn-sb.xml @@ -2771,6 +2771,14 @@ tcp.flags = RST;

+ +

+ This is set to true whenever all OVS flows + required by this Port_Binding have been installed. This is + populated by ovn-controller. +

+ +

A number that represents the logical port in the key (e.g. STT key or diff --git a/utilities/ovn-sbctl.c b/utilities/ovn-sbctl.c index 0a1b9ff..c38e8ec 100644 --- a/utilities/ovn-sbctl.c +++ b/utilities/ovn-sbctl.c @@ -526,6 +526,7 @@ pre_get_info(struct ctl_context *ctx) ovsdb_idl_add_column(ctx->idl, &sbrec_port_binding_col_tunnel_key); ovsdb_idl_add_column(ctx->idl, &sbrec_port_binding_col_chassis); ovsdb_idl_add_column(ctx->idl, &sbrec_port_binding_col_datapath); + ovsdb_idl_add_column(ctx->idl, &sbrec_port_binding_col_up); ovsdb_idl_add_column(ctx->idl, &sbrec_logical_flow_col_logical_datapath); ovsdb_idl_add_column(ctx->idl, &sbrec_logical_flow_col_logical_dp_group); @@ -665,6 +666,7 @@ cmd_lsp_bind(struct ctl_context *ctx) struct sbctl_chassis *sbctl_ch; struct sbctl_port_binding *sbctl_bd; char *lport_name, *ch_name; + bool up = true; /* port_binding must exist, chassis must exist! */ lport_name = ctx->argv[1]; @@ -683,6 +685,7 @@ cmd_lsp_bind(struct ctl_context *ctx) } } sbrec_port_binding_set_chassis(sbctl_bd->bd_cfg, sbctl_ch->ch_cfg); + sbrec_port_binding_set_up(sbctl_bd->bd_cfg, &up, 1); sbctl_context_invalidate_cache(ctx); } @@ -699,6 +702,7 @@ cmd_lsp_unbind(struct ctl_context *ctx) sbctl_bd = find_port_binding(sbctl_ctx, lport_name, must_exist); if (sbctl_bd) { sbrec_port_binding_set_chassis(sbctl_bd->bd_cfg, NULL); + sbrec_port_binding_set_up(sbctl_bd->bd_cfg, NULL, 0); } }