From patchwork Mon Jan 13 22:52:23 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Han Zhou X-Patchwork-Id: 1222379 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 47xTPl48VTz9sP6 for ; Tue, 14 Jan 2020 09:52:43 +1100 (AEDT) Received: from localhost (localhost [127.0.0.1]) by hemlock.osuosl.org (Postfix) with ESMTP id 97E4A87765; Mon, 13 Jan 2020 22:52:41 +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 GIrdRd+IHGsS; Mon, 13 Jan 2020 22:52:38 +0000 (UTC) Received: from lists.linuxfoundation.org (lf-lists.osuosl.org [140.211.9.56]) by hemlock.osuosl.org (Postfix) with ESMTP id C85A5865A1; Mon, 13 Jan 2020 22:52:36 +0000 (UTC) Received: from lf-lists.osuosl.org (localhost [127.0.0.1]) by lists.linuxfoundation.org (Postfix) with ESMTP id A2099C1D8D; Mon, 13 Jan 2020 22:52:36 +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 9B8DCC18DD for ; Mon, 13 Jan 2020 22:52:33 +0000 (UTC) Received: from localhost (localhost [127.0.0.1]) by whitealder.osuosl.org (Postfix) with ESMTP id 8F64F844C0 for ; Mon, 13 Jan 2020 22:52:33 +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 seoVfoUoKqwk for ; Mon, 13 Jan 2020 22:52:32 +0000 (UTC) X-Greylist: domain auto-whitelisted by SQLgrey-1.7.6 Received: from relay10.mail.gandi.net (relay10.mail.gandi.net [217.70.178.230]) by whitealder.osuosl.org (Postfix) with ESMTPS id 3066284332 for ; Mon, 13 Jan 2020 22:52:31 +0000 (UTC) Received: from localhost.localdomain.localdomain (unknown [216.113.160.77]) (Authenticated sender: hzhou@ovn.org) by relay10.mail.gandi.net (Postfix) with ESMTPSA id CE72F240003; Mon, 13 Jan 2020 22:52:27 +0000 (UTC) From: Han Zhou To: dev@openvswitch.org Date: Mon, 13 Jan 2020 14:52:23 -0800 Message-Id: <1578955945-19812-1-git-send-email-hzhou@ovn.org> X-Mailer: git-send-email 2.1.0 Cc: Han Zhou Subject: [ovs-dev] [PATCH 1/3] ovn-controller.c: Fix possible NULL pointer dereference. 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: , MIME-Version: 1.0 Errors-To: ovs-dev-bounces@openvswitch.org Sender: "dev" In function update_sb_db(), it tries to access cfg->external_ids outside of the "if (cfg)" block. This patch fixes it. Signed-off-by: Han Zhou Acked-by: Numan Siddique --- controller/ovn-controller.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/controller/ovn-controller.c b/controller/ovn-controller.c index 17744d4..3d843f7 100644 --- a/controller/ovn-controller.c +++ b/controller/ovn-controller.c @@ -431,12 +431,12 @@ static void update_sb_db(struct ovsdb_idl *ovs_idl, struct ovsdb_idl *ovnsb_idl) { const struct ovsrec_open_vswitch *cfg = ovsrec_open_vswitch_first(ovs_idl); + if (!cfg) { + return; + } /* Set remote based on user configuration. */ - const char *remote = NULL; - if (cfg) { - remote = smap_get(&cfg->external_ids, "ovn-remote"); - } + const char *remote = smap_get(&cfg->external_ids, "ovn-remote"); ovsdb_idl_set_remote(ovnsb_idl, remote, true); /* Set probe interval, based on user configuration and the remote. */ From patchwork Mon Jan 13 22:52:25 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Han Zhou X-Patchwork-Id: 1222381 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 47xTPl43THz9s29 for ; Tue, 14 Jan 2020 09:52:40 +1100 (AEDT) Received: from localhost (localhost [127.0.0.1]) by fraxinus.osuosl.org (Postfix) with ESMTP id 359B8858DD; Mon, 13 Jan 2020 22:52:38 +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 5PQVC2SOnXi3; Mon, 13 Jan 2020 22:52:36 +0000 (UTC) Received: from lists.linuxfoundation.org (lf-lists.osuosl.org [140.211.9.56]) by fraxinus.osuosl.org (Postfix) with ESMTP id 2567785B5B; Mon, 13 Jan 2020 22:52:36 +0000 (UTC) Received: from lf-lists.osuosl.org (localhost [127.0.0.1]) by lists.linuxfoundation.org (Postfix) with ESMTP id 0D1BAC1D8D; Mon, 13 Jan 2020 22:52:36 +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 8C38FC077D for ; Mon, 13 Jan 2020 22:52:33 +0000 (UTC) Received: from localhost (localhost [127.0.0.1]) by fraxinus.osuosl.org (Postfix) with ESMTP id 7A7C8858D4 for ; Mon, 13 Jan 2020 22:52:33 +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 XNqKV747e89k for ; Mon, 13 Jan 2020 22:52:32 +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 70908858DD for ; Mon, 13 Jan 2020 22:52:32 +0000 (UTC) Received: from localhost.localdomain.localdomain (unknown [216.113.160.77]) (Authenticated sender: hzhou@ovn.org) by relay10.mail.gandi.net (Postfix) with ESMTPSA id 4EA28240006; Mon, 13 Jan 2020 22:52:30 +0000 (UTC) From: Han Zhou To: dev@openvswitch.org Date: Mon, 13 Jan 2020 14:52:25 -0800 Message-Id: <1578955945-19812-3-git-send-email-hzhou@ovn.org> X-Mailer: git-send-email 2.1.0 In-Reply-To: <1578955945-19812-1-git-send-email-hzhou@ovn.org> References: <1578955945-19812-1-git-send-email-hzhou@ovn.org> Cc: Han Zhou Subject: [ovs-dev] [PATCH 3/3] ovn-controller.c: Support option "ovn-monitor-all". 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: , MIME-Version: 1.0 Errors-To: ovs-dev-bounces@openvswitch.org Sender: "dev" Set this option to true will avoid using conditional monitoring in ovn-controller. Setting it to true helps in environments where all (or most) workloads need to be reachable from each other, thus the effectiveness of conditional monitoring is low, but the overhead of conditional monitoring is high. Requested-by: Girish Moodalbail Signed-off-by: Han Zhou Acked-by: Numan Siddique --- controller/ovn-controller.8.xml | 21 ++++++++++++++++++++ controller/ovn-controller.c | 44 +++++++++++++++++++++++++++++++++++------ 2 files changed, 59 insertions(+), 6 deletions(-) diff --git a/controller/ovn-controller.8.xml b/controller/ovn-controller.8.xml index a226802..a163ff5 100644 --- a/controller/ovn-controller.8.xml +++ b/controller/ovn-controller.8.xml @@ -98,6 +98,27 @@

+
external_ids:ovn-monitor-all
+
+

+ A boolean value that tells if ovn-controller should + monitor all records of tables in ovs-database. If + set to false, it will conditionally monitor the records + that is needed in the current chassis. +

+

+ It is more optimal to set it to true in use cases when + the chassis would anyway need to monitor most of the records in + ovs-database, which would save the overhead of conditions + processing, especially for server side. Typically, set it to + true for environments that all workloads need to be + reachable from each other. +

+

+ Default value is false. +

+
+
external_ids:ovn-remote-probe-interval

diff --git a/controller/ovn-controller.c b/controller/ovn-controller.c index abb1b4c..ccd63b3 100644 --- a/controller/ovn-controller.c +++ b/controller/ovn-controller.c @@ -129,7 +129,8 @@ static void update_sb_monitors(struct ovsdb_idl *ovnsb_idl, const struct sbrec_chassis *chassis, const struct sset *local_ifaces, - struct hmap *local_datapaths) + struct hmap *local_datapaths, + bool monitor_all) { /* Monitor Port_Bindings rows for local interfaces and local datapaths. * @@ -154,6 +155,19 @@ update_sb_monitors(struct ovsdb_idl *ovnsb_idl, struct ovsdb_idl_condition ce = OVSDB_IDL_CONDITION_INIT(&ce); struct ovsdb_idl_condition ip_mcast = OVSDB_IDL_CONDITION_INIT(&ip_mcast); struct ovsdb_idl_condition igmp = OVSDB_IDL_CONDITION_INIT(&igmp); + + if (monitor_all) { + ovsdb_idl_condition_add_clause_true(&pb); + ovsdb_idl_condition_add_clause_true(&lf); + ovsdb_idl_condition_add_clause_true(&mb); + ovsdb_idl_condition_add_clause_true(&mg); + ovsdb_idl_condition_add_clause_true(&dns); + ovsdb_idl_condition_add_clause_true(&ce); + ovsdb_idl_condition_add_clause_true(&ip_mcast); + ovsdb_idl_condition_add_clause_true(&igmp); + goto out; + } + sbrec_port_binding_add_clause_type(&pb, OVSDB_F_EQ, "patch"); /* XXX: We can optimize this, if we find a way to only monitor * ports that have a Gateway_Chassis that point's to our own @@ -205,6 +219,8 @@ update_sb_monitors(struct ovsdb_idl *ovnsb_idl, uuid); } } + +out: sbrec_port_binding_set_condition(ovnsb_idl, &pb); sbrec_logical_flow_set_condition(ovnsb_idl, &lf); sbrec_mac_binding_set_condition(ovnsb_idl, &mb); @@ -428,7 +444,8 @@ get_ofctrl_probe_interval(struct ovsdb_idl *ovs_idl) /* Retrieves the pointer to the OVN Southbound database from 'ovs_idl' and * updates 'sbdb_idl' with that pointer. */ static void -update_sb_db(struct ovsdb_idl *ovs_idl, struct ovsdb_idl *ovnsb_idl) +update_sb_db(struct ovsdb_idl *ovs_idl, struct ovsdb_idl *ovnsb_idl, + bool *monitor_all_p) { const struct ovsrec_open_vswitch *cfg = ovsrec_open_vswitch_first(ovs_idl); if (!cfg) { @@ -445,6 +462,19 @@ update_sb_db(struct ovsdb_idl *ovs_idl, struct ovsdb_idl *ovnsb_idl) int interval = smap_get_int(&cfg->external_ids, "ovn-remote-probe-interval", default_interval); ovsdb_idl_set_probe_interval(ovnsb_idl, interval); + + bool monitor_all = smap_get_bool(&cfg->external_ids, "ovn-monitor-all", + false); + if (monitor_all) { + /* Always call update_sb_monitors when monitor_all is true. + * Otherwise, don't call it here, because there would be unnecessary + * extra cost. Instead, it is called after the engine execution only + * when it is necessary. */ + update_sb_monitors(ovnsb_idl, NULL, NULL, NULL, true); + } + if (monitor_all_p) { + *monitor_all_p = monitor_all; + } } static void @@ -1887,7 +1917,7 @@ main(int argc, char *argv[]) ovsdb_idl_omit(ovnsb_idl_loop.idl, &sbrec_connection_col_status); ovsdb_idl_omit(ovnsb_idl_loop.idl, &sbrec_connection_col_target); - update_sb_monitors(ovnsb_idl_loop.idl, NULL, NULL, NULL); + update_sb_monitors(ovnsb_idl_loop.idl, NULL, NULL, NULL, false); stopwatch_create(CONTROLLER_LOOP_STOPWATCH_NAME, SW_MS); @@ -2009,6 +2039,7 @@ main(int argc, char *argv[]) /* Main loop. */ exiting = false; restart = false; + bool sb_monitor_all = false; while (!exiting) { engine_init_run(); @@ -2023,7 +2054,7 @@ main(int argc, char *argv[]) ovs_cond_seqno = new_ovs_cond_seqno; } - update_sb_db(ovs_idl_loop.idl, ovnsb_idl_loop.idl); + update_sb_db(ovs_idl_loop.idl, ovnsb_idl_loop.idl, &sb_monitor_all); update_ssl_config(ovsrec_ssl_table_get(ovs_idl_loop.idl)); ofctrl_set_probe_interval(get_ofctrl_probe_interval(ovs_idl_loop.idl)); @@ -2163,7 +2194,8 @@ main(int argc, char *argv[]) if (engine_node_changed(&en_runtime_data)) { update_sb_monitors(ovnsb_idl_loop.idl, chassis, &runtime_data->local_lports, - &runtime_data->local_datapaths); + &runtime_data->local_datapaths, + sb_monitor_all); } } } @@ -2272,7 +2304,7 @@ main(int argc, char *argv[]) if (!restart) { bool done = !ovsdb_idl_has_ever_connected(ovnsb_idl_loop.idl); while (!done) { - update_sb_db(ovs_idl_loop.idl, ovnsb_idl_loop.idl); + update_sb_db(ovs_idl_loop.idl, ovnsb_idl_loop.idl, NULL); update_ssl_config(ovsrec_ssl_table_get(ovs_idl_loop.idl)); struct ovsdb_idl_txn *ovs_idl_txn