From patchwork Fri Nov 22 16:13:21 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Dumitru Ceara X-Patchwork-Id: 1199539 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=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.b="DQMgtdr1"; dkim-atps=neutral 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 47KM1H2MZsz9sPK for ; Sat, 23 Nov 2019 03:13:39 +1100 (AEDT) Received: from localhost (localhost [127.0.0.1]) by hemlock.osuosl.org (Postfix) with ESMTP id BB0AB88D8D; Fri, 22 Nov 2019 16:13: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 iiEqhoYcSOl1; Fri, 22 Nov 2019 16:13:33 +0000 (UTC) Received: from lists.linuxfoundation.org (lf-lists.osuosl.org [140.211.9.56]) by hemlock.osuosl.org (Postfix) with ESMTP id C271088D81; Fri, 22 Nov 2019 16:13:33 +0000 (UTC) Received: from lf-lists.osuosl.org (localhost [127.0.0.1]) by lists.linuxfoundation.org (Postfix) with ESMTP id AB76DC1D74; Fri, 22 Nov 2019 16:13:33 +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 AEA0FC18DA for ; Fri, 22 Nov 2019 16:13:32 +0000 (UTC) Received: from localhost (localhost [127.0.0.1]) by silver.osuosl.org (Postfix) with ESMTP id AA488263F9 for ; Fri, 22 Nov 2019 16:13:32 +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 olz4I-qQkxbF for ; Fri, 22 Nov 2019 16:13:31 +0000 (UTC) X-Greylist: domain auto-whitelisted by SQLgrey-1.7.6 Received: from us-smtp-delivery-1.mimecast.com (us-smtp-2.mimecast.com [205.139.110.61]) by silver.osuosl.org (Postfix) with ESMTPS id D5D7A26428 for ; Fri, 22 Nov 2019 16:13:30 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1574439209; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=DFoQSfpsfIE74vs0TPsspMZku/2wLOeztTGDcMfSP/g=; b=DQMgtdr1oYcMz/+yk7Rwjm2KF0VwgyGn/HNRNzhKq84uffSOYHT+Pu/BL159t5SUUFv9+y Ysc8DVmyAHOYQSGZoDnxCorpf4OWBD+r9SebjVJG84cFLy/RqmuNUdUzSGwT00g9KiXUit 414ijV4SnrmdhoL7Jc2/2F3/IC7QTzE= 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-336-tCgoGMJoM2-_zX-I-LBxNA-1; Fri, 22 Nov 2019 11:13:28 -0500 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 508C110054E3; Fri, 22 Nov 2019 16:13:27 +0000 (UTC) Received: from dceara.remote.csb (ovpn-117-130.ams2.redhat.com [10.36.117.130]) by smtp.corp.redhat.com (Postfix) with ESMTP id 9529763742; Fri, 22 Nov 2019 16:13:25 +0000 (UTC) From: Dumitru Ceara To: dev@openvswitch.org Date: Fri, 22 Nov 2019 17:13:21 +0100 Message-Id: <20191122161318.4719.87657.stgit@dceara.remote.csb> In-Reply-To: <20191122161303.4719.78753.stgit@dceara.remote.csb> References: <20191122161303.4719.78753.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 X-MC-Unique: tCgoGMJoM2-_zX-I-LBxNA-1 X-Mimecast-Spam-Score: 0 Cc: hzhou@ovn.org Subject: [ovs-dev] [PATCH v6 ovn 1/4] ovn-controller: Refactor I-P engine_run() tracking. 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" This commit simplifies the logic of calling engine_run and engine_need_run in order to reduce the number of external variables required to track the result of the last engine execution. The engine code is also refactored a bit and the engine_run() function is split in different functions that handle computing/recomputing a node. Signed-off-by: Dumitru Ceara --- controller/ovn-controller.c | 33 ++++++------ lib/inc-proc-eng.c | 120 +++++++++++++++++++++++++++++-------------- lib/inc-proc-eng.h | 7 ++- 3 files changed, 103 insertions(+), 57 deletions(-) diff --git a/controller/ovn-controller.c b/controller/ovn-controller.c index 27cb488..c56190f 100644 --- a/controller/ovn-controller.c +++ b/controller/ovn-controller.c @@ -1942,7 +1942,6 @@ main(int argc, char *argv[]) &pending_pkt); uint64_t engine_run_id = 0; - uint64_t old_engine_run_id = 0; bool engine_run_done = true; unsigned int ovs_cond_seqno = UINT_MAX; @@ -1952,10 +1951,11 @@ main(int argc, char *argv[]) exiting = false; restart = false; while (!exiting) { + engine_run_id++; + update_sb_db(ovs_idl_loop.idl, ovnsb_idl_loop.idl); update_ssl_config(ovsrec_ssl_table_get(ovs_idl_loop.idl)); ofctrl_set_probe_interval(get_ofctrl_probe_interval(ovs_idl_loop.idl)); - old_engine_run_id = engine_run_id; struct ovsdb_idl_txn *ovs_idl_txn = ovsdb_idl_loop_run(&ovs_idl_loop); unsigned int new_ovs_cond_seqno @@ -2047,12 +2047,12 @@ main(int argc, char *argv[]) if (engine_run_done) { engine_set_abort_recompute(true); engine_run_done = engine_run(&en_flow_output, - ++engine_run_id); + engine_run_id); } } else { engine_set_abort_recompute(false); engine_run_done = true; - engine_run(&en_flow_output, ++engine_run_id); + engine_run(&en_flow_output, engine_run_id); } } stopwatch_stop(CONTROLLER_LOOP_STOPWATCH_NAME, @@ -2097,17 +2097,20 @@ main(int argc, char *argv[]) } } - if (old_engine_run_id == engine_run_id || !engine_run_done) { - if (!engine_run_done || engine_need_run(&en_flow_output)) { - VLOG_DBG("engine did not run, force recompute next time: " - "br_int %p, chassis %p", br_int, chassis); - engine_set_force_recompute(true); - poll_immediate_wake(); - } else { - VLOG_DBG("engine did not run, and it was not needed" - " either: br_int %p, chassis %p", - br_int, chassis); - } + if (engine_need_run(&en_flow_output, engine_run_id)) { + VLOG_DBG("engine did not run, force recompute next time: " + "br_int %p, chassis %p", br_int, chassis); + engine_set_force_recompute(true); + poll_immediate_wake(); + } else if (!engine_run_done) { + VLOG_DBG("engine was aborted, force recompute next time: " + "br_int %p, chassis %p", br_int, chassis); + engine_set_force_recompute(true); + poll_immediate_wake(); + } else if (!engine_has_run(&en_flow_output, engine_run_id)) { + VLOG_DBG("engine did not run, and it was not needed" + " either: br_int %p, chassis %p", + br_int, chassis); } else { engine_set_force_recompute(false); } diff --git a/lib/inc-proc-eng.c b/lib/inc-proc-eng.c index 1064a08..ff07ad9 100644 --- a/lib/inc-proc-eng.c +++ b/lib/inc-proc-eng.c @@ -129,14 +129,68 @@ engine_ovsdb_node_add_index(struct engine_node *node, const char *name, } bool -engine_run(struct engine_node *node, uint64_t run_id) +engine_has_run(struct engine_node *node, uint64_t run_id) +{ + return node->run_id == run_id; +} + +/* Do a full recompute (or at least try). If we're not allowed then + * mark the node as "aborted". + */ +static bool +engine_recompute(struct engine_node *node, bool forced, bool allowed) +{ + VLOG_DBG("node: %s, recompute (%s)", node->name, + forced ? "forced" : "triggered"); + + if (!allowed) { + VLOG_DBG("node: %s, recompute aborted", node->name); + return false; + } + + node->run(node); + VLOG_DBG("node: %s, changed: %d", node->name, node->changed); + return true; +} + +/* Return true if the node could be computed, false otherwise. */ +static bool +engine_compute(struct engine_node *node, bool recompute_allowed) +{ + for (size_t i = 0; i < node->n_inputs; i++) { + /* If the input node data changed call its change handler. */ + if (node->inputs[i].node->changed) { + VLOG_DBG("node: %s, handle change for input %s", + node->name, node->inputs[i].node->name); + + /* If the input change can't be handled incrementally, run + * the node handler. + */ + if (!node->inputs[i].change_handler(node)) { + VLOG_DBG("node: %s, can't handle change for input %s, " + "fall back to recompute", + node->name, node->inputs[i].node->name); + return engine_recompute(node, false, recompute_allowed); + } + } + } + + return true; +} + +bool engine_run(struct engine_node *node, uint64_t run_id) { if (node->run_id == run_id) { + /* The node was already updated in this run (could be input for + * multiple other nodes). Stop processing. + */ return true; } - node->run_id = run_id; + /* Initialize the node for this run. */ + node->run_id = run_id; node->changed = false; + if (!node->n_inputs) { node->run(node); VLOG_DBG("node: %s, changed: %d", node->name, node->changed); @@ -150,59 +204,45 @@ engine_run(struct engine_node *node, uint64_t run_id) } bool need_compute = false; - bool need_recompute = false; if (engine_force_recompute) { - need_recompute = true; - } else { - for (size_t i = 0; i < node->n_inputs; i++) { - if (node->inputs[i].node->changed) { - need_compute = true; - if (!node->inputs[i].change_handler) { - need_recompute = true; - break; - } - } - } + return engine_recompute(node, true, !engine_abort_recompute); } - if (need_recompute) { - VLOG_DBG("node: %s, recompute (%s)", node->name, - engine_force_recompute ? "forced" : "triggered"); - if (engine_abort_recompute) { - VLOG_DBG("node: %s, recompute aborted", node->name); - return false; - } - node->run(node); - } else if (need_compute) { - for (size_t i = 0; i < node->n_inputs; i++) { - if (node->inputs[i].node->changed) { - VLOG_DBG("node: %s, handle change for input %s", - node->name, node->inputs[i].node->name); - if (!node->inputs[i].change_handler(node)) { - VLOG_DBG("node: %s, can't handle change for input %s, " - "fall back to recompute", - node->name, node->inputs[i].node->name); - if (engine_abort_recompute) { - VLOG_DBG("node: %s, recompute aborted", node->name); - return false; - } - node->run(node); - break; - } + /* If any of the inputs updated data but there is no change_handler, then + * recompute the current node too. + */ + for (size_t i = 0; i < node->n_inputs; i++) { + if (node->inputs[i].node->changed) { + need_compute = true; + + /* Trigger a recompute if we don't have a change handler. */ + if (!node->inputs[i].change_handler) { + return engine_recompute(node, false, !engine_abort_recompute); } } } + if (need_compute) { + /* If we couldn't compute the node we either aborted or triggered + * a full recompute. In any case, stop processing. + */ + return engine_compute(node, !engine_abort_recompute); + } + VLOG_DBG("node: %s, changed: %d", node->name, node->changed); return true; } bool -engine_need_run(struct engine_node *node) +engine_need_run(struct engine_node *node, uint64_t run_id) { size_t i; + if (node->run_id == run_id) { + return false; + } + if (!node->n_inputs) { node->run(node); VLOG_DBG("input node: %s, changed: %d", node->name, node->changed); @@ -210,7 +250,7 @@ engine_need_run(struct engine_node *node) } for (i = 0; i < node->n_inputs; i++) { - if (engine_need_run(node->inputs[i].node)) { + if (engine_need_run(node->inputs[i].node, run_id)) { return true; } } diff --git a/lib/inc-proc-eng.h b/lib/inc-proc-eng.h index 3a69dc2..abd41b2 100644 --- a/lib/inc-proc-eng.h +++ b/lib/inc-proc-eng.h @@ -130,9 +130,9 @@ bool engine_run(struct engine_node *, uint64_t run_id); * terminates. */ void engine_cleanup(struct engine_node *); -/* Check if engine needs to run, i.e. any change to be processed. */ +/* Check if engine needs to run but didn't. */ bool -engine_need_run(struct engine_node *); +engine_need_run(struct engine_node *, uint64_t run_id); /* Get the input node with for */ struct engine_node * engine_get_input(const char *input_name, @@ -159,6 +159,9 @@ const struct engine_context * engine_get_context(void); void engine_set_context(const struct engine_context *); +/* Return true if the engine has run for 'node' in the 'run_id' iteration. */ +bool engine_has_run(struct engine_node *node, uint64_t run_id); + struct ed_ovsdb_index { const char *name; struct ovsdb_idl_index *index; From patchwork Fri Nov 22 16:13:36 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Dumitru Ceara X-Patchwork-Id: 1199540 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.b="YWfDQd5c"; 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 47KM1f2gWgz9sPK for ; Sat, 23 Nov 2019 03:13:57 +1100 (AEDT) Received: from localhost (localhost [127.0.0.1]) by silver.osuosl.org (Postfix) with ESMTP id 47A4D26450; Fri, 22 Nov 2019 16:13:56 +0000 (UTC) X-Virus-Scanned: amavisd-new at osuosl.org Received: from silver.osuosl.org ([127.0.0.1]) by localhost (.osuosl.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id hDVMHmnrjdpD; Fri, 22 Nov 2019 16:13:51 +0000 (UTC) Received: from lists.linuxfoundation.org (lf-lists.osuosl.org [140.211.9.56]) by silver.osuosl.org (Postfix) with ESMTP id ED6352632A; Fri, 22 Nov 2019 16:13:50 +0000 (UTC) Received: from lf-lists.osuosl.org (localhost [127.0.0.1]) by lists.linuxfoundation.org (Postfix) with ESMTP id DC70CC1DDB; Fri, 22 Nov 2019 16:13:50 +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 2FC6DC1D74 for ; Fri, 22 Nov 2019 16:13:50 +0000 (UTC) Received: from localhost (localhost [127.0.0.1]) by silver.osuosl.org (Postfix) with ESMTP id 1DFD62632A for ; Fri, 22 Nov 2019 16:13:50 +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 AgjYG-q4zmqM for ; Fri, 22 Nov 2019 16:13:45 +0000 (UTC) X-Greylist: domain auto-whitelisted by SQLgrey-1.7.6 Received: from us-smtp-1.mimecast.com (us-smtp-delivery-1.mimecast.com [205.139.110.120]) by silver.osuosl.org (Postfix) with ESMTPS id 2B2C8263F9 for ; Fri, 22 Nov 2019 16:13:45 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1574439223; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=XdDtTnwR7XddM6o7/agkNRqNQy0ktvXiGc6rmRb4gGs=; b=YWfDQd5cS3k1zZ9+ukxlxu3ACjzKRGe1HJIiLsjNxZEYgrgn0sssOcRV4Qh8NqnUOKb7xh YsvtSY8ehksBEtd0JD3T7TcGUUXBocwMLXjGiNqQvjLde3GPEn245tliajYgUYKhAkNRDw 8Og1KAC4BtlIVPJWEBPNVnxyNNFj5jo= 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-286-ljkz2CwOPka9FRS_W3J2NA-1; Fri, 22 Nov 2019 11:13:42 -0500 Received: from smtp.corp.redhat.com (int-mx01.intmail.prod.int.phx2.redhat.com [10.5.11.11]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id 9832C477; Fri, 22 Nov 2019 16:13:41 +0000 (UTC) Received: from dceara.remote.csb (ovpn-117-130.ams2.redhat.com [10.36.117.130]) by smtp.corp.redhat.com (Postfix) with ESMTP id 4A6CD60141; Fri, 22 Nov 2019 16:13:40 +0000 (UTC) From: Dumitru Ceara To: dev@openvswitch.org Date: Fri, 22 Nov 2019 17:13:36 +0100 Message-Id: <20191122161332.4719.31973.stgit@dceara.remote.csb> In-Reply-To: <20191122161303.4719.78753.stgit@dceara.remote.csb> References: <20191122161303.4719.78753.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.11 X-MC-Unique: ljkz2CwOPka9FRS_W3J2NA-1 X-Mimecast-Spam-Score: 0 Cc: hzhou@ovn.org Subject: [ovs-dev] [PATCH v6 ovn 2/4] ovn-controller: Add per node states to I-P 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" This commit transforms the 'changed' field in struct engine_node in a 'state' field. Possible node states are: - "Stale": data in the node is not up to date with the DB. - "Updated": data in the node is valid but was updated during the last run of the engine. - "Valid": data in the node is valid and didn't change during the last run of the engine. - "Aborted": during the last run, processing was aborted for this node. This commit also further refactors the I-P engine: - instead of recursively performing all the engine processing a preprocessing stage is added (engine_get_nodes()) before the main processing loop is executed in order to topologically sort nodes in the engine such that all inputs of a given node appear in the sorted array before the node itself. This simplifies a bit the code in engine_run(). - remove the need for using an engine_run_id by using the newly added states. - turn the global 'engine_abort_recompute' into an argument to be passed to engine_run(). It's relevant only in the current run context anyway as we reset it before every call to engine_run(). Signed-off-by: Dumitru Ceara --- controller/ovn-controller.c | 84 ++++++++------- lib/inc-proc-eng.c | 242 ++++++++++++++++++++++++++++++++----------- lib/inc-proc-eng.h | 74 +++++++++---- 3 files changed, 276 insertions(+), 124 deletions(-) diff --git a/controller/ovn-controller.c b/controller/ovn-controller.c index c56190f..a588531 100644 --- a/controller/ovn-controller.c +++ b/controller/ovn-controller.c @@ -758,10 +758,10 @@ en_ofctrl_is_connected_run(struct engine_node *node) (struct ed_type_ofctrl_is_connected *)node->data; if (data->connected != ofctrl_is_connected()) { data->connected = !data->connected; - node->changed = true; + engine_set_node_state(node, EN_UPDATED); return; } - node->changed = false; + engine_set_node_state(node, EN_VALID); } struct ed_type_addr_sets { @@ -811,7 +811,7 @@ en_addr_sets_run(struct engine_node *node) addr_sets_init(as_table, &as->addr_sets); as->change_tracked = false; - node->changed = true; + engine_set_node_state(node, EN_UPDATED); } static bool @@ -830,11 +830,14 @@ addr_sets_sb_address_set_handler(struct engine_node *node) addr_sets_update(as_table, &as->addr_sets, &as->new, &as->deleted, &as->updated); - node->changed = !sset_is_empty(&as->new) || !sset_is_empty(&as->deleted) - || !sset_is_empty(&as->updated); + if (!sset_is_empty(&as->new) || !sset_is_empty(&as->deleted) || + !sset_is_empty(&as->updated)) { + engine_set_node_state(node, EN_UPDATED); + } else { + engine_set_node_state(node, EN_VALID); + } as->change_tracked = true; - node->changed = true; return true; } @@ -885,7 +888,7 @@ en_port_groups_run(struct engine_node *node) port_groups_init(pg_table, &pg->port_groups); pg->change_tracked = false; - node->changed = true; + engine_set_node_state(node, EN_UPDATED); } static bool @@ -904,11 +907,14 @@ port_groups_sb_port_group_handler(struct engine_node *node) port_groups_update(pg_table, &pg->port_groups, &pg->new, &pg->deleted, &pg->updated); - node->changed = !sset_is_empty(&pg->new) || !sset_is_empty(&pg->deleted) - || !sset_is_empty(&pg->updated); + if (!sset_is_empty(&pg->new) || !sset_is_empty(&pg->deleted) || + !sset_is_empty(&pg->updated)) { + engine_set_node_state(node, EN_UPDATED); + } else { + engine_set_node_state(node, EN_VALID); + } pg->change_tracked = true; - node->changed = true; return true; } @@ -1091,7 +1097,7 @@ en_runtime_data_run(struct engine_node *node) update_ct_zones(local_lports, local_datapaths, ct_zones, ct_zone_bitmap, pending_ct_zones); - node->changed = true; + engine_set_node_state(node, EN_UPDATED); } static bool @@ -1157,10 +1163,10 @@ en_mff_ovn_geneve_run(struct engine_node *node) enum mf_field_id mff_ovn_geneve = ofctrl_get_mf_field_id(); if (data->mff_ovn_geneve != mff_ovn_geneve) { data->mff_ovn_geneve = mff_ovn_geneve; - node->changed = true; + engine_set_node_state(node, EN_UPDATED); return; } - node->changed = false; + engine_set_node_state(node, EN_VALID); } struct ed_type_flow_output { @@ -1322,7 +1328,7 @@ en_flow_output_run(struct engine_node *node) active_tunnels, flow_table); - node->changed = true; + engine_set_node_state(node, EN_UPDATED); } static bool @@ -1404,7 +1410,7 @@ flow_output_sb_logical_flow_handler(struct engine_node *node) flow_table, group_table, meter_table, lfrr, conj_id_ofs); - node->changed = true; + engine_set_node_state(node, EN_UPDATED); return handled; } @@ -1427,7 +1433,7 @@ flow_output_sb_mac_binding_handler(struct engine_node *node) lflow_handle_changed_neighbors(sbrec_port_binding_by_name, mac_binding_table, flow_table); - node->changed = true; + engine_set_node_state(node, EN_UPDATED); return true; } @@ -1531,7 +1537,7 @@ flow_output_sb_port_binding_handler(struct engine_node *node) chassis, ct_zones, local_datapaths, active_tunnels, flow_table); - node->changed = true; + engine_set_node_state(node, EN_UPDATED); return true; } @@ -1580,7 +1586,7 @@ flow_output_sb_multicast_group_handler(struct engine_node *node) mff_ovn_geneve, chassis, ct_zones, local_datapaths, flow_table); - node->changed = true; + engine_set_node_state(node, EN_UPDATED); return true; } @@ -1694,7 +1700,9 @@ _flow_output_resource_ref_handler(struct engine_node *node, conj_id_ofs, &changed)) { return false; } - node->changed = changed || node->changed; + if (changed) { + engine_set_node_state(node, EN_UPDATED); + } } SSET_FOR_EACH (ref_name, updated) { if (!lflow_handle_changed_ref(ref_type, ref_name, @@ -1707,7 +1715,9 @@ _flow_output_resource_ref_handler(struct engine_node *node, conj_id_ofs, &changed)) { return false; } - node->changed = changed || node->changed; + if (changed) { + engine_set_node_state(node, EN_UPDATED); + } } SSET_FOR_EACH (ref_name, new) { if (!lflow_handle_changed_ref(ref_type, ref_name, @@ -1720,7 +1730,9 @@ _flow_output_resource_ref_handler(struct engine_node *node, conj_id_ofs, &changed)) { return false; } - node->changed = changed || node->changed; + if (changed) { + engine_set_node_state(node, EN_UPDATED); + } } return true; @@ -1941,9 +1953,6 @@ main(int argc, char *argv[]) unixctl_command_register("inject-pkt", "MICROFLOW", 1, 1, inject_pkt, &pending_pkt); - uint64_t engine_run_id = 0; - bool engine_run_done = true; - unsigned int ovs_cond_seqno = UINT_MAX; unsigned int ovnsb_cond_seqno = UINT_MAX; @@ -1951,7 +1960,7 @@ main(int argc, char *argv[]) exiting = false; restart = false; while (!exiting) { - engine_run_id++; + engine_init_run(); update_sb_db(ovs_idl_loop.idl, ovnsb_idl_loop.idl); update_ssl_config(ovsrec_ssl_table_get(ovs_idl_loop.idl)); @@ -2044,15 +2053,11 @@ main(int argc, char *argv[]) * this round of engine_run and continue processing * acculated changes incrementally later when * ofctrl_can_put() returns true. */ - if (engine_run_done) { - engine_set_abort_recompute(true); - engine_run_done = engine_run(&en_flow_output, - engine_run_id); + if (!engine_aborted()) { + engine_run(true); } } else { - engine_set_abort_recompute(false); - engine_run_done = true; - engine_run(&en_flow_output, engine_run_id); + engine_run(false); } } stopwatch_stop(CONTROLLER_LOOP_STOPWATCH_NAME, @@ -2071,7 +2076,7 @@ main(int argc, char *argv[]) sbrec_meter_table_get(ovnsb_idl_loop.idl), get_nb_cfg(sbrec_sb_global_table_get( ovnsb_idl_loop.idl)), - en_flow_output.changed); + engine_node_changed(&en_flow_output)); pinctrl_run(ovnsb_idl_txn, sbrec_datapath_binding_by_key, sbrec_port_binding_by_datapath, @@ -2089,7 +2094,7 @@ main(int argc, char *argv[]) &ed_runtime_data.local_datapaths, &ed_runtime_data.active_tunnels); - if (en_runtime_data.changed) { + if (engine_node_changed(&en_runtime_data)) { update_sb_monitors(ovnsb_idl_loop.idl, chassis, &ed_runtime_data.local_lports, &ed_runtime_data.local_datapaths); @@ -2097,17 +2102,17 @@ main(int argc, char *argv[]) } } - if (engine_need_run(&en_flow_output, engine_run_id)) { + if (engine_need_run()) { VLOG_DBG("engine did not run, force recompute next time: " "br_int %p, chassis %p", br_int, chassis); engine_set_force_recompute(true); poll_immediate_wake(); - } else if (!engine_run_done) { + } else if (engine_aborted()) { VLOG_DBG("engine was aborted, force recompute next time: " "br_int %p, chassis %p", br_int, chassis); engine_set_force_recompute(true); poll_immediate_wake(); - } else if (!engine_has_run(&en_flow_output, engine_run_id)) { + } else if (!engine_has_run()) { VLOG_DBG("engine did not run, and it was not needed" " either: br_int %p, chassis %p", br_int, chassis); @@ -2135,8 +2140,7 @@ main(int argc, char *argv[]) } } else { VLOG_DBG("Pending_pkt conn but br_int %p or chassis " - "%p not ready. run-id: %"PRIu64, br_int, - chassis, engine_run_id); + "%p not ready.", br_int, chassis); unixctl_command_reply_error(pending_pkt.conn, "ovn-controller not ready."); } @@ -2185,7 +2189,7 @@ main(int argc, char *argv[]) } engine_set_context(NULL); - engine_cleanup(&en_flow_output); + engine_cleanup(); /* It's time to exit. Clean up the databases if we are not restarting */ if (!restart) { diff --git a/lib/inc-proc-eng.c b/lib/inc-proc-eng.c index ff07ad9..f88116f 100644 --- a/lib/inc-proc-eng.c +++ b/lib/inc-proc-eng.c @@ -31,21 +31,24 @@ VLOG_DEFINE_THIS_MODULE(inc_proc_eng); static bool engine_force_recompute = false; -static bool engine_abort_recompute = false; static const struct engine_context *engine_context; +static struct engine_node **engine_nodes; +static size_t engine_n_nodes; + +static const char *engine_node_state_name[EN_STATE_MAX] = { + [EN_STALE] = "Stale", + [EN_UPDATED] = "Updated", + [EN_VALID] = "Valid", + [EN_ABORTED] = "Aborted", +}; + void engine_set_force_recompute(bool val) { engine_force_recompute = val; } -void -engine_set_abort_recompute(bool val) -{ - engine_abort_recompute = val; -} - const struct engine_context * engine_get_context(void) { @@ -58,26 +61,69 @@ engine_set_context(const struct engine_context *ctx) engine_context = ctx; } -void -engine_init(struct engine_node *node) +/* Builds the topologically sorted 'sorted_nodes' array starting from + * 'node'. + */ +static struct engine_node ** +engine_topo_sort(struct engine_node *node, struct engine_node **sorted_nodes, + size_t *n_count, size_t *n_size) { + /* It's not so efficient to walk the array of already sorted nodes but + * we know that sorting is done only once at startup so it's ok for now. + */ + for (size_t i = 0; i < *n_count; i++) { + if (sorted_nodes[i] == node) { + return sorted_nodes; + } + } + for (size_t i = 0; i < node->n_inputs; i++) { - engine_init(node->inputs[i].node); + sorted_nodes = engine_topo_sort(node->inputs[i].node, sorted_nodes, + n_count, n_size); } - if (node->init) { - node->init(node); + if (*n_count == *n_size) { + sorted_nodes = x2nrealloc(sorted_nodes, n_size, sizeof *sorted_nodes); } + sorted_nodes[(*n_count)] = node; + (*n_count)++; + return sorted_nodes; +} + +/* Return the array of topologically sorted nodes when starting from + * 'node'. Stores the number of nodes in 'n_count'. + */ +static struct engine_node ** +engine_get_nodes(struct engine_node *node, size_t *n_count) +{ + size_t n_size = 0; + + *n_count = 0; + return engine_topo_sort(node, NULL, n_count, &n_size); } void -engine_cleanup(struct engine_node *node) +engine_init(struct engine_node *node) { - for (size_t i = 0; i < node->n_inputs; i++) { - engine_cleanup(node->inputs[i].node); + engine_nodes = engine_get_nodes(node, &engine_n_nodes); + + for (size_t i = 0; i < engine_n_nodes; i++) { + if (engine_nodes[i]->init) { + engine_nodes[i]->init(engine_nodes[i]); + } } - if (node->cleanup) { - node->cleanup(node); +} + +void +engine_cleanup(void) +{ + for (size_t i = 0; i < engine_n_nodes; i++) { + if (engine_nodes[i]->cleanup) { + engine_nodes[i]->cleanup(engine_nodes[i]); + } } + free(engine_nodes); + engine_nodes = NULL; + engine_n_nodes = 0; } struct engine_node * @@ -128,16 +174,70 @@ engine_ovsdb_node_add_index(struct engine_node *node, const char *name, ed->n_indexes ++; } +void +engine_set_node_state_at(struct engine_node *node, + enum engine_node_state state, + const char *where) +{ + if (node->state == state) { + return; + } + + VLOG_DBG("%s: node: %s, old_state %s, new_state %s", + where, node->name, + engine_node_state_name[node->state], + engine_node_state_name[state]); + + node->state = state; +} + +static bool +engine_node_valid(struct engine_node *node) +{ + return (node->state == EN_UPDATED || node->state == EN_VALID); +} + bool -engine_has_run(struct engine_node *node, uint64_t run_id) +engine_node_changed(struct engine_node *node) { - return node->run_id == run_id; + return node->state == EN_UPDATED; +} + +bool +engine_has_run(void) +{ + for (size_t i = 0; i < engine_n_nodes; i++) { + if (engine_nodes[i]->state == EN_STALE) { + return false; + } + } + return true; +} + +bool +engine_aborted(void) +{ + for (size_t i = 0; i < engine_n_nodes; i++) { + if (engine_nodes[i]->state == EN_ABORTED) { + return true; + } + } + return false; +} + +void +engine_init_run(void) +{ + VLOG_DBG("Initializing new run"); + for (size_t i = 0; i < engine_n_nodes; i++) { + engine_set_node_state(engine_nodes[i], EN_STALE); + } } /* Do a full recompute (or at least try). If we're not allowed then * mark the node as "aborted". */ -static bool +static void engine_recompute(struct engine_node *node, bool forced, bool allowed) { VLOG_DBG("node: %s, recompute (%s)", node->name, @@ -145,12 +245,12 @@ engine_recompute(struct engine_node *node, bool forced, bool allowed) if (!allowed) { VLOG_DBG("node: %s, recompute aborted", node->name); - return false; + engine_set_node_state(node, EN_ABORTED); + return; } + /* Run the node handler which might change state. */ node->run(node); - VLOG_DBG("node: %s, changed: %d", node->name, node->changed); - return true; } /* Return true if the node could be computed, false otherwise. */ @@ -159,7 +259,7 @@ engine_compute(struct engine_node *node, bool recompute_allowed) { for (size_t i = 0; i < node->n_inputs; i++) { /* If the input node data changed call its change handler. */ - if (node->inputs[i].node->changed) { + if (node->inputs[i].node->state == EN_UPDATED) { VLOG_DBG("node: %s, handle change for input %s", node->name, node->inputs[i].node->name); @@ -170,55 +270,62 @@ engine_compute(struct engine_node *node, bool recompute_allowed) VLOG_DBG("node: %s, can't handle change for input %s, " "fall back to recompute", node->name, node->inputs[i].node->name); - return engine_recompute(node, false, recompute_allowed); + engine_recompute(node, false, recompute_allowed); + if (node->state == EN_ABORTED) { + return false; + } + return true; } } } - return true; } -bool engine_run(struct engine_node *node, uint64_t run_id) +static void +engine_run_node(struct engine_node *node, bool recompute_allowed) { - if (node->run_id == run_id) { - /* The node was already updated in this run (could be input for - * multiple other nodes). Stop processing. - */ - return true; - } - - /* Initialize the node for this run. */ - node->run_id = run_id; - node->changed = false; - if (!node->n_inputs) { + /* Run the node handler which might change state. */ node->run(node); - VLOG_DBG("node: %s, changed: %d", node->name, node->changed); - return true; + return; } + bool input_stale = false; for (size_t i = 0; i < node->n_inputs; i++) { - if (!engine_run(node->inputs[i].node, run_id)) { - return false; + if (!engine_node_valid(node->inputs[i].node)) { + /* If the input node aborted computation, move to EN_ABORTED. + * This will be propagated to following nodes. + */ + if (node->inputs[i].node->state == EN_ABORTED) { + engine_set_node_state(node, EN_ABORTED); + } + + input_stale = true; } } - bool need_compute = false; + /* If at least one input is stale, don't change state. */ + if (input_stale) { + return; + } if (engine_force_recompute) { - return engine_recompute(node, true, !engine_abort_recompute); + engine_recompute(node, true, recompute_allowed); + return; } /* If any of the inputs updated data but there is no change_handler, then * recompute the current node too. */ + bool need_compute = false; for (size_t i = 0; i < node->n_inputs; i++) { - if (node->inputs[i].node->changed) { + if (node->inputs[i].node->state == EN_UPDATED) { need_compute = true; /* Trigger a recompute if we don't have a change handler. */ if (!node->inputs[i].change_handler) { - return engine_recompute(node, false, !engine_abort_recompute); + engine_recompute(node, false, recompute_allowed); + return; } } } @@ -227,33 +334,46 @@ bool engine_run(struct engine_node *node, uint64_t run_id) /* If we couldn't compute the node we either aborted or triggered * a full recompute. In any case, stop processing. */ - return engine_compute(node, !engine_abort_recompute); + if (!engine_compute(node, recompute_allowed)) { + return; + } } - VLOG_DBG("node: %s, changed: %d", node->name, node->changed); - return true; + /* If we reached this point, either the node was updated or its state is + * still valid. + */ + if (!engine_node_changed(node)) { + engine_set_node_state(node, EN_VALID); + } } -bool -engine_need_run(struct engine_node *node, uint64_t run_id) +void +engine_run(bool abort_on_recompute) { - size_t i; + for (size_t i = 0; i < engine_n_nodes; i++) { + engine_run_node(engine_nodes[i], !abort_on_recompute); + } +} - if (node->run_id == run_id) { +bool +engine_need_run(void) +{ + if (engine_has_run()) { return false; } - if (!node->n_inputs) { - node->run(node); - VLOG_DBG("input node: %s, changed: %d", node->name, node->changed); - return node->changed; - } + for (size_t i = 0; i < engine_n_nodes; i++) { + /* Check only leaf nodes for updates. */ + if (engine_nodes[i]->n_inputs) { + continue; + } - for (i = 0; i < node->n_inputs; i++) { - if (engine_need_run(node->inputs[i].node, run_id)) { + engine_nodes[i]->run(engine_nodes[i]); + VLOG_DBG("input node: %s, state: %s", engine_nodes[i]->name, + engine_node_state_name[engine_nodes[i]->state]); + if (engine_nodes[i]->state == EN_UPDATED) { return true; } } - return false; } diff --git a/lib/inc-proc-eng.h b/lib/inc-proc-eng.h index abd41b2..5315649 100644 --- a/lib/inc-proc-eng.h +++ b/lib/inc-proc-eng.h @@ -82,10 +82,21 @@ struct engine_node_input { bool (*change_handler)(struct engine_node *node); }; -struct engine_node { - /* A unique id to distinguish each iteration of the engine_run(). */ - uint64_t run_id; +enum engine_node_state { + EN_STALE, /* Data in the node is not up to date with the DB. */ + EN_UPDATED, /* Data in the node is valid but was updated during the + * last run. + */ + EN_VALID, /* Data in the node is valid and didn't change during the + * last run. + */ + EN_ABORTED, /* During the last run, processing was aborted for + * this node. + */ + EN_STATE_MAX, +}; +struct engine_node { /* A unique name for each node. */ char *name; @@ -102,8 +113,8 @@ struct engine_node { * node. */ void *data; - /* Whether the data changed in the last engine run. */ - bool changed; + /* State of the node after the last engine run. */ + enum engine_node_state state; /* Method to initialize data. It may be NULL. */ void (*init)(struct engine_node *); @@ -116,23 +127,29 @@ struct engine_node { void (*run)(struct engine_node *); }; -/* Initialize the data for the engine nodes recursively. It calls each node's +/* Initialize the data for the engine nodes. It calls each node's * init() method if not NULL. It should be called before the main loop. */ -void engine_init(struct engine_node *); +void engine_init(struct engine_node *node); -/* Execute the processing recursively, which should be called in the main - * loop. Returns true if the execution is compelte, false if it is aborted, - * which could happen when engine_abort_recompute is set. */ -bool engine_run(struct engine_node *, uint64_t run_id); +/* Initialize the engine nodes for a new run. It should be called in the + * main processing loop before every potential engine_run(). + */ +void engine_init_run(void); + +/* Execute the processing, which should be called in the main loop. + * Updates the engine node's states accordingly. If 'abort_on_recompute' is + * true, if a recompute is required by the current engine run then the engine + * aborts. + */ +void engine_run(bool abort_on_recompute); -/* Clean up the data for the engine nodes recursively. It calls each node's +/* Clean up the data for the engine nodes. It calls each node's * cleanup() method if not NULL. It should be called before the program * terminates. */ -void engine_cleanup(struct engine_node *); +void engine_cleanup(void); /* Check if engine needs to run but didn't. */ -bool -engine_need_run(struct engine_node *, uint64_t run_id); +bool engine_need_run(void); /* Get the input node with for */ struct engine_node * engine_get_input(const char *input_name, @@ -151,16 +168,26 @@ void engine_add_input(struct engine_node *node, struct engine_node *input, * iteration, and the change can't be tracked across iterations */ void engine_set_force_recompute(bool val); -/* Set the flag to cause engine execution to be aborted when there - * is any recompute to be triggered in any node. */ -void engine_set_abort_recompute(bool val); - const struct engine_context * engine_get_context(void); void engine_set_context(const struct engine_context *); -/* Return true if the engine has run for 'node' in the 'run_id' iteration. */ -bool engine_has_run(struct engine_node *node, uint64_t run_id); +void engine_set_node_state_at(struct engine_node *node, + enum engine_node_state state, + const char *where); + +/* Return true if during the last iteration the node's data was updated. */ +bool engine_node_changed(struct engine_node *node); + +/* Return true if the engine has run in the last iteration. */ +bool engine_has_run(void); + +/* Returns true if during the last engine run we had to abort processing. */ +bool engine_aborted(void); + +/* Set the state of the node and log changes. */ +#define engine_set_node_state(node, state) \ + engine_set_node_state_at(node, state, OVS_SOURCE_LOCATOR) struct ed_ovsdb_index { const char *name; @@ -187,6 +214,7 @@ void engine_ovsdb_node_add_index(struct engine_node *, const char *name, struct engine_node en_##NAME = { \ .name = NAME_STR, \ .data = &ed_##NAME, \ + .state = EN_STALE, \ .init = en_##NAME##_init, \ .run = en_##NAME##_run, \ .cleanup = en_##NAME##_cleanup, \ @@ -201,10 +229,10 @@ en_##DB_NAME##_##TBL_NAME##_run(struct engine_node *node) \ const struct DB_NAME##rec_##TBL_NAME##_table *table = \ EN_OVSDB_GET(node); \ if (DB_NAME##rec_##TBL_NAME##_table_track_get_first(table)) { \ - node->changed = true; \ + engine_set_node_state(node, EN_UPDATED); \ return; \ } \ - node->changed = false; \ + engine_set_node_state(node, EN_VALID); \ } \ static void (*en_##DB_NAME##_##TBL_NAME##_init)(struct engine_node *node) \ = NULL; \ From patchwork Fri Nov 22 16:13:50 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Dumitru Ceara X-Patchwork-Id: 1199541 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=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.b="cC8w+hX4"; dkim-atps=neutral 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 47KM1t6CC9z9sPK for ; Sat, 23 Nov 2019 03:14:10 +1100 (AEDT) Received: from localhost (localhost [127.0.0.1]) by whitealder.osuosl.org (Postfix) with ESMTP id 46F2B88371; Fri, 22 Nov 2019 16:14:09 +0000 (UTC) X-Virus-Scanned: amavisd-new at osuosl.org Received: from whitealder.osuosl.org ([127.0.0.1]) by localhost (.osuosl.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id QSH6cCkYV4Uj; Fri, 22 Nov 2019 16:14:02 +0000 (UTC) Received: from lists.linuxfoundation.org (lf-lists.osuosl.org [140.211.9.56]) by whitealder.osuosl.org (Postfix) with ESMTP id 4912C8833D; Fri, 22 Nov 2019 16:14:02 +0000 (UTC) Received: from lf-lists.osuosl.org (localhost [127.0.0.1]) by lists.linuxfoundation.org (Postfix) with ESMTP id 2BF3FC1DDB; Fri, 22 Nov 2019 16:14:02 +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 4239BC18DA for ; Fri, 22 Nov 2019 16:14:01 +0000 (UTC) Received: from localhost (localhost [127.0.0.1]) by fraxinus.osuosl.org (Postfix) with ESMTP id 2C1E1874A2 for ; Fri, 22 Nov 2019 16:14:01 +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 o5CYAjhwyCzt for ; Fri, 22 Nov 2019 16:14:00 +0000 (UTC) X-Greylist: domain auto-whitelisted by SQLgrey-1.7.6 Received: from us-smtp-delivery-1.mimecast.com (us-smtp-1.mimecast.com [207.211.31.81]) by fraxinus.osuosl.org (Postfix) with ESMTPS id E245987420 for ; Fri, 22 Nov 2019 16:13:59 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1574439238; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=wCQONzqlFO2A09lCqv5abSF0yrtayvo7q4YeAvTgxuw=; b=cC8w+hX4tXyvJw8B2Yy3jCurB/wD0e/HvQerzTKoob2OJO2WX3hsIcubickXcS1HXdrrku qBtjc1m3FEZOYdaWxDyfcHQxuf+KsawIpVJXolG6KCzux2d7S5+9y/GHp1gArAe1Ib0pOZ ETiYE6KNes7Eykg4LIj7H7llmxY8owk= 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-225-9wQtwuf8NpC0fSTxeyzUAA-1; Fri, 22 Nov 2019 11:13:55 -0500 Received: from smtp.corp.redhat.com (int-mx04.intmail.prod.int.phx2.redhat.com [10.5.11.14]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id 41ADB10054E3; Fri, 22 Nov 2019 16:13:54 +0000 (UTC) Received: from dceara.remote.csb (ovpn-117-130.ams2.redhat.com [10.36.117.130]) by smtp.corp.redhat.com (Postfix) with ESMTP id 00D765F906; Fri, 22 Nov 2019 16:13:52 +0000 (UTC) From: Dumitru Ceara To: dev@openvswitch.org Date: Fri, 22 Nov 2019 17:13:50 +0100 Message-Id: <20191122161347.4719.66842.stgit@dceara.remote.csb> In-Reply-To: <20191122161303.4719.78753.stgit@dceara.remote.csb> References: <20191122161303.4719.78753.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.14 X-MC-Unique: 9wQtwuf8NpC0fSTxeyzUAA-1 X-Mimecast-Spam-Score: 0 Cc: hzhou@ovn.org Subject: [ovs-dev] [PATCH v6 ovn 3/4] ovn-controller: Add separate I-P engine node for processing ct-zones. 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" Signed-off-by: Dumitru Ceara --- controller/ovn-controller.c | 117 +++++++++++++++++++++++++++++-------------- 1 file changed, 78 insertions(+), 39 deletions(-) diff --git a/controller/ovn-controller.c b/controller/ovn-controller.c index a588531..201ef9e 100644 --- a/controller/ovn-controller.c +++ b/controller/ovn-controller.c @@ -933,11 +933,6 @@ struct ed_type_runtime_data { * _ */ struct sset local_lport_ids; struct sset active_tunnels; - - /* connection tracking zones. */ - unsigned long ct_zone_bitmap[BITMAP_N_LONGS(MAX_CT_ZONES)]; - struct shash pending_ct_zones; - struct simap ct_zones; }; static void @@ -945,24 +940,11 @@ en_runtime_data_init(struct engine_node *node) { struct ed_type_runtime_data *data = (struct ed_type_runtime_data *)node->data; - 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)); + hmap_init(&data->local_datapaths); sset_init(&data->local_lports); sset_init(&data->local_lport_ids); sset_init(&data->active_tunnels); - shash_init(&data->pending_ct_zones); - simap_init(&data->ct_zones); - - /* Initialize connection tracking zones. */ - memset(data->ct_zone_bitmap, 0, sizeof data->ct_zone_bitmap); - bitmap_set1(data->ct_zone_bitmap, 0); /* Zone 0 is reserved. */ - restore_ct_zones(bridge_table, ovs_table, - &data->ct_zones, data->ct_zone_bitmap); } static void @@ -983,9 +965,6 @@ en_runtime_data_cleanup(struct engine_node *node) free(cur_node); } hmap_destroy(&data->local_datapaths); - - simap_destroy(&data->ct_zones); - shash_destroy(&data->pending_ct_zones); } static void @@ -997,9 +976,6 @@ en_runtime_data_run(struct engine_node *node) struct sset *local_lports = &data->local_lports; struct sset *local_lport_ids = &data->local_lport_ids; struct sset *active_tunnels = &data->active_tunnels; - unsigned long *ct_zone_bitmap = data->ct_zone_bitmap; - struct shash *pending_ct_zones = &data->pending_ct_zones; - struct simap *ct_zones = &data->ct_zones; static bool first_run = true; if (first_run) { @@ -1094,9 +1070,6 @@ en_runtime_data_run(struct engine_node *node) ovs_table, local_datapaths, local_lports, local_lport_ids); - update_ct_zones(local_lports, local_datapaths, ct_zones, - ct_zone_bitmap, pending_ct_zones); - engine_set_node_state(node, EN_UPDATED); } @@ -1138,6 +1111,55 @@ runtime_data_sb_port_binding_handler(struct engine_node *node) return !changed; } +/* Connection tracking zones. */ +struct ed_type_ct_zones { + unsigned long bitmap[BITMAP_N_LONGS(MAX_CT_ZONES)]; + struct shash pending; + struct simap current; +}; + +static void +en_ct_zones_init(struct engine_node *node) +{ + struct ed_type_ct_zones *data = node->data; + 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)); + + shash_init(&data->pending); + simap_init(&data->current); + + memset(data->bitmap, 0, sizeof data->bitmap); + bitmap_set1(data->bitmap, 0); /* Zone 0 is reserved. */ + restore_ct_zones(bridge_table, ovs_table, &data->current, data->bitmap); +} + +static void +en_ct_zones_cleanup(struct engine_node *node) +{ + struct ed_type_ct_zones *data = node->data; + + simap_destroy(&data->current); + shash_destroy(&data->pending); +} + +static void +en_ct_zones_run(struct engine_node *node) +{ + struct ed_type_ct_zones *data = node->data; + struct ed_type_runtime_data *rt_data = + (struct ed_type_runtime_data *)engine_get_input( + "runtime_data", node)->data; + + update_ct_zones(&rt_data->local_lports, &rt_data->local_datapaths, + &data->current, data->bitmap, &data->pending); + + engine_set_node_state(node, EN_UPDATED); +} + struct ed_type_mff_ovn_geneve { enum mf_field_id mff_ovn_geneve; }; @@ -1215,7 +1237,11 @@ en_flow_output_run(struct engine_node *node) struct sset *local_lports = &rt_data->local_lports; struct sset *local_lport_ids = &rt_data->local_lport_ids; struct sset *active_tunnels = &rt_data->active_tunnels; - struct simap *ct_zones = &rt_data->ct_zones; + + struct ed_type_ct_zones *ct_zones_data = + (struct ed_type_ct_zones *)engine_get_input( + "ct_zones", node)->data; + struct simap *ct_zones = &ct_zones_data->current; struct ed_type_mff_ovn_geneve *ed_mff_ovn_geneve = (struct ed_type_mff_ovn_geneve *)engine_get_input( @@ -1445,7 +1471,11 @@ flow_output_sb_port_binding_handler(struct engine_node *node) "runtime_data", node)->data; struct hmap *local_datapaths = &data->local_datapaths; struct sset *active_tunnels = &data->active_tunnels; - struct simap *ct_zones = &data->ct_zones; + + struct ed_type_ct_zones *ct_zones_data = + (struct ed_type_ct_zones *)engine_get_input( + "ct_zones", node)->data; + struct simap *ct_zones = &ct_zones_data->current; struct ed_type_mff_ovn_geneve *ed_mff_ovn_geneve = (struct ed_type_mff_ovn_geneve *)engine_get_input( @@ -1548,7 +1578,11 @@ flow_output_sb_multicast_group_handler(struct engine_node *node) (struct ed_type_runtime_data *)engine_get_input( "runtime_data", node)->data; struct hmap *local_datapaths = &data->local_datapaths; - struct simap *ct_zones = &data->ct_zones; + + struct ed_type_ct_zones *ct_zones_data = + (struct ed_type_ct_zones *)engine_get_input( + "ct_zones", node)->data; + struct simap *ct_zones = &ct_zones_data->current; struct ed_type_mff_ovn_geneve *ed_mff_ovn_geneve = (struct ed_type_mff_ovn_geneve *)engine_get_input( @@ -1857,6 +1891,7 @@ main(int argc, char *argv[]) stopwatch_create(CONTROLLER_LOOP_STOPWATCH_NAME, SW_MS); /* Define inc-proc-engine nodes. */ + struct ed_type_ct_zones ed_ct_zones; struct ed_type_runtime_data ed_runtime_data; struct ed_type_mff_ovn_geneve ed_mff_ovn_geneve; struct ed_type_ofctrl_is_connected ed_ofctrl_is_connected; @@ -1864,6 +1899,7 @@ main(int argc, char *argv[]) struct ed_type_addr_sets ed_addr_sets; struct ed_type_port_groups ed_port_groups; + ENGINE_NODE(ct_zones, "ct_zones"); ENGINE_NODE(runtime_data, "runtime_data"); ENGINE_NODE(mff_ovn_geneve, "mff_ovn_geneve"); ENGINE_NODE(ofctrl_is_connected, "ofctrl_is_connected"); @@ -1903,6 +1939,7 @@ main(int argc, char *argv[]) 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_ovs_open_vswitch, NULL); @@ -1922,6 +1959,10 @@ main(int argc, char *argv[]) engine_add_input(&en_flow_output, &en_sb_dhcpv6_options, NULL); engine_add_input(&en_flow_output, &en_sb_dns, NULL); + engine_add_input(&en_ct_zones, &en_ovs_open_vswitch, NULL); + engine_add_input(&en_ct_zones, &en_ovs_bridge, NULL); + engine_add_input(&en_ct_zones, &en_runtime_data, NULL); + engine_add_input(&en_runtime_data, &en_ofctrl_is_connected, NULL); engine_add_input(&en_runtime_data, &en_ovs_open_vswitch, NULL); @@ -1947,7 +1988,7 @@ main(int argc, char *argv[]) meter_table_list, &ed_flow_output.meter_table); unixctl_command_register("ct-zone-list", "", 0, 0, - ct_zone_list, &ed_runtime_data.ct_zones); + ct_zone_list, &ed_ct_zones.current); struct pending_pkt pending_pkt = { .conn = NULL }; unixctl_command_register("inject-pkt", "MICROFLOW", 1, 1, inject_pkt, @@ -2020,7 +2061,7 @@ main(int argc, char *argv[]) } if (br_int) { - ofctrl_run(br_int, &ed_runtime_data.pending_ct_zones); + ofctrl_run(br_int, &ed_ct_zones.pending); if (chassis) { patch_run(ovs_idl_txn, @@ -2063,8 +2104,7 @@ main(int argc, char *argv[]) stopwatch_stop(CONTROLLER_LOOP_STOPWATCH_NAME, time_msec()); if (ovs_idl_txn) { - commit_ct_zones(br_int, - &ed_runtime_data.pending_ct_zones); + commit_ct_zones(br_int, &ed_ct_zones.pending); bfd_run(ovsrec_interface_table_get(ovs_idl_loop.idl), br_int, chassis, sbrec_ha_chassis_group_table_get( @@ -2072,7 +2112,7 @@ main(int argc, char *argv[]) sbrec_sb_global_table_get(ovnsb_idl_loop.idl)); } ofctrl_put(&ed_flow_output.flow_table, - &ed_runtime_data.pending_ct_zones, + &ed_ct_zones.pending, sbrec_meter_table_get(ovnsb_idl_loop.idl), get_nb_cfg(sbrec_sb_global_table_get( ovnsb_idl_loop.idl)), @@ -2170,11 +2210,10 @@ main(int argc, char *argv[]) if (ovsdb_idl_loop_commit_and_wait(&ovs_idl_loop) == 1) { struct shash_node *iter, *iter_next; - SHASH_FOR_EACH_SAFE (iter, iter_next, - &ed_runtime_data.pending_ct_zones) { + SHASH_FOR_EACH_SAFE (iter, iter_next, &ed_ct_zones.pending) { struct ct_zone_pending_entry *ctzpe = iter->data; if (ctzpe->state == CT_ZONE_DB_SENT) { - shash_delete(&ed_runtime_data.pending_ct_zones, iter); + shash_delete(&ed_ct_zones.pending, iter); free(ctzpe); } } From patchwork Fri Nov 22 16:14:03 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Dumitru Ceara X-Patchwork-Id: 1199543 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=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.b="MmalPoIc"; dkim-atps=neutral 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 47KM241LHyz9sPK for ; Sat, 23 Nov 2019 03:14:20 +1100 (AEDT) Received: from localhost (localhost [127.0.0.1]) by fraxinus.osuosl.org (Postfix) with ESMTP id A00F787419; Fri, 22 Nov 2019 16:14:18 +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 kRxUv8QhNbVW; Fri, 22 Nov 2019 16:14:16 +0000 (UTC) Received: from lists.linuxfoundation.org (lf-lists.osuosl.org [140.211.9.56]) by fraxinus.osuosl.org (Postfix) with ESMTP id ED88D87469; Fri, 22 Nov 2019 16:14:15 +0000 (UTC) Received: from lf-lists.osuosl.org (localhost [127.0.0.1]) by lists.linuxfoundation.org (Postfix) with ESMTP id E3902C1DDB; Fri, 22 Nov 2019 16:14:15 +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 D3937C1D74 for ; Fri, 22 Nov 2019 16:14:14 +0000 (UTC) Received: from localhost (localhost [127.0.0.1]) by hemlock.osuosl.org (Postfix) with ESMTP id D02D788D7C for ; Fri, 22 Nov 2019 16:14:14 +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 Qa5MZJFGOIYu for ; Fri, 22 Nov 2019 16:14:12 +0000 (UTC) X-Greylist: domain auto-whitelisted by SQLgrey-1.7.6 Received: from us-smtp-delivery-1.mimecast.com (us-smtp-1.mimecast.com [207.211.31.81]) by hemlock.osuosl.org (Postfix) with ESMTPS id 4662888D97 for ; Fri, 22 Nov 2019 16:14:12 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1574439250; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=X0w6M4HkpfLwPLpO9gjWtYSe9AZvvbck5vGUOO/o/jc=; b=MmalPoIcIMbt1hNBcF8p1wdgebw1hr4BOQVLK5ZAD3D2H4wt4y1xnCRff/1Lu78Cm4PbIA Zwy1QLKu8gWIFghhXOkDI7+TxGuvVMNKQASPXZkFQS0TWC88oELYHjUqdCnADwthSAx4T8 Dt6OYXKJs13wxsag0Nh2U7yQPppvNR0= 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-213-GVz-4IxHMYSxOXLhacydHw-1; Fri, 22 Nov 2019 11:14:09 -0500 Received: from smtp.corp.redhat.com (int-mx08.intmail.prod.int.phx2.redhat.com [10.5.11.23]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id 4787918B5F71; Fri, 22 Nov 2019 16:14:08 +0000 (UTC) Received: from dceara.remote.csb (ovpn-117-130.ams2.redhat.com [10.36.117.130]) by smtp.corp.redhat.com (Postfix) with ESMTP id 4CAAB28DFD; Fri, 22 Nov 2019 16:14:06 +0000 (UTC) From: Dumitru Ceara To: dev@openvswitch.org Date: Fri, 22 Nov 2019 17:14:03 +0100 Message-Id: <20191122161359.4719.39730.stgit@dceara.remote.csb> In-Reply-To: <20191122161303.4719.78753.stgit@dceara.remote.csb> References: <20191122161303.4719.78753.stgit@dceara.remote.csb> User-Agent: StGit/0.17.1-dirty MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.84 on 10.5.11.23 X-MC-Unique: GVz-4IxHMYSxOXLhacydHw-1 X-Mimecast-Spam-Score: 0 Cc: hzhou@ovn.org Subject: [ovs-dev] [PATCH v6 ovn 4/4] ovn-controller: Fix use of dangling pointers in I-P 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" The incremental processing engine might stop a run before the en_runtime_data node is processed. In such cases the ed_runtime_data fields might contain pointers to already deleted SB records. For example, if a port binding corresponding to a patch port is removed from the SB database and the incremental processing engine aborts before the en_runtime_data node is processed then the corresponding local_datapath hashtable entry in ed_runtime_data is stale and will store a pointer to the already freed sbrec_port_binding record. This will cause invalid memory accesses in various places (e.g., pinctrl_run() -> prepare_ipv6_ras()). To fix the issue we introduce the concept of "internal_data" vs "data" in engine nodes. The first field, "internal_data", is data that can be accessed by the incremental engine nodes handlers (data from other nodes must be considered read-only and data from other nodes must not be accessed if the nodes haven't been refreshed in the current iteration). The second field, "data" is a pointer reset at engine_run() and if non-NULL indicates to users outside the incremental engine that the data is safe to use. This commit also adds an "is_valid()" method to engine nodes to allow users to override the default behavior of determining if data is valid in a node (e.g., for the ct-zones node the data is always safe to access). CC: Han Zhou Fixes: ca278d98a4f5 ("ovn-controller: Initial use of incremental engine - quiet mode.") Signed-off-by: Dumitru Ceara --- controller/ovn-controller.c | 230 ++++++++++++++++++++++--------------------- lib/inc-proc-eng.c | 32 +++++- lib/inc-proc-eng.h | 28 +++++ 3 files changed, 169 insertions(+), 121 deletions(-) diff --git a/controller/ovn-controller.c b/controller/ovn-controller.c index 201ef9e..f6945fb 100644 --- a/controller/ovn-controller.c +++ b/controller/ovn-controller.c @@ -741,8 +741,7 @@ struct ed_type_ofctrl_is_connected { static void en_ofctrl_is_connected_init(struct engine_node *node) { - struct ed_type_ofctrl_is_connected *data = - (struct ed_type_ofctrl_is_connected *)node->data; + struct ed_type_ofctrl_is_connected *data = node->internal_data; data->connected = false; } @@ -754,8 +753,7 @@ en_ofctrl_is_connected_cleanup(struct engine_node *node OVS_UNUSED) static void en_ofctrl_is_connected_run(struct engine_node *node) { - struct ed_type_ofctrl_is_connected *data = - (struct ed_type_ofctrl_is_connected *)node->data; + struct ed_type_ofctrl_is_connected *data = node->internal_data; if (data->connected != ofctrl_is_connected()) { data->connected = !data->connected; engine_set_node_state(node, EN_UPDATED); @@ -775,7 +773,7 @@ struct ed_type_addr_sets { static void en_addr_sets_init(struct engine_node *node) { - struct ed_type_addr_sets *as = (struct ed_type_addr_sets *)node->data; + struct ed_type_addr_sets *as = node->internal_data; shash_init(&as->addr_sets); as->change_tracked = false; sset_init(&as->new); @@ -786,7 +784,7 @@ en_addr_sets_init(struct engine_node *node) static void en_addr_sets_cleanup(struct engine_node *node) { - struct ed_type_addr_sets *as = (struct ed_type_addr_sets *)node->data; + struct ed_type_addr_sets *as = node->internal_data; expr_const_sets_destroy(&as->addr_sets); shash_destroy(&as->addr_sets); sset_destroy(&as->new); @@ -797,7 +795,7 @@ en_addr_sets_cleanup(struct engine_node *node) static void en_addr_sets_run(struct engine_node *node) { - struct ed_type_addr_sets *as = (struct ed_type_addr_sets *)node->data; + struct ed_type_addr_sets *as = node->internal_data; sset_clear(&as->new); sset_clear(&as->deleted); @@ -817,7 +815,7 @@ en_addr_sets_run(struct engine_node *node) static bool addr_sets_sb_address_set_handler(struct engine_node *node) { - struct ed_type_addr_sets *as = (struct ed_type_addr_sets *)node->data; + struct ed_type_addr_sets *as = node->internal_data; sset_clear(&as->new); sset_clear(&as->deleted); @@ -852,7 +850,7 @@ struct ed_type_port_groups{ static void en_port_groups_init(struct engine_node *node) { - struct ed_type_port_groups *pg = (struct ed_type_port_groups *)node->data; + struct ed_type_port_groups *pg = node->internal_data; shash_init(&pg->port_groups); pg->change_tracked = false; sset_init(&pg->new); @@ -863,7 +861,7 @@ en_port_groups_init(struct engine_node *node) static void en_port_groups_cleanup(struct engine_node *node) { - struct ed_type_port_groups *pg = (struct ed_type_port_groups *)node->data; + struct ed_type_port_groups *pg = node->internal_data; expr_const_sets_destroy(&pg->port_groups); shash_destroy(&pg->port_groups); sset_destroy(&pg->new); @@ -874,7 +872,7 @@ en_port_groups_cleanup(struct engine_node *node) static void en_port_groups_run(struct engine_node *node) { - struct ed_type_port_groups *pg = (struct ed_type_port_groups *)node->data; + struct ed_type_port_groups *pg = node->internal_data; sset_clear(&pg->new); sset_clear(&pg->deleted); @@ -894,7 +892,7 @@ en_port_groups_run(struct engine_node *node) static bool port_groups_sb_port_group_handler(struct engine_node *node) { - struct ed_type_port_groups *pg = (struct ed_type_port_groups *)node->data; + struct ed_type_port_groups *pg = node->internal_data; sset_clear(&pg->new); sset_clear(&pg->deleted); @@ -938,8 +936,7 @@ struct ed_type_runtime_data { static void en_runtime_data_init(struct engine_node *node) { - struct ed_type_runtime_data *data = - (struct ed_type_runtime_data *)node->data; + struct ed_type_runtime_data *data = node->internal_data; hmap_init(&data->local_datapaths); sset_init(&data->local_lports); @@ -950,8 +947,7 @@ en_runtime_data_init(struct engine_node *node) static void en_runtime_data_cleanup(struct engine_node *node) { - struct ed_type_runtime_data *data = - (struct ed_type_runtime_data *)node->data; + struct ed_type_runtime_data *data = node->internal_data; sset_destroy(&data->local_lports); sset_destroy(&data->local_lport_ids); @@ -970,8 +966,7 @@ en_runtime_data_cleanup(struct engine_node *node) static void en_runtime_data_run(struct engine_node *node) { - struct ed_type_runtime_data *data = - (struct ed_type_runtime_data *)node->data; + struct ed_type_runtime_data *data = node->internal_data; struct hmap *local_datapaths = &data->local_datapaths; struct sset *local_lports = &data->local_lports; struct sset *local_lport_ids = &data->local_lport_ids; @@ -1019,8 +1014,7 @@ en_runtime_data_run(struct engine_node *node) ovs_assert(chassis); struct ed_type_ofctrl_is_connected *ed_ofctrl_is_connected = - (struct ed_type_ofctrl_is_connected *)engine_get_input( - "ofctrl_is_connected", node)->data; + engine_get_input("ofctrl_is_connected", node)->internal_data; if (ed_ofctrl_is_connected->connected) { /* Calculate the active tunnels only if have an an active * OpenFlow connection to br-int. @@ -1076,8 +1070,7 @@ en_runtime_data_run(struct engine_node *node) static bool runtime_data_sb_port_binding_handler(struct engine_node *node) { - struct ed_type_runtime_data *data = - (struct ed_type_runtime_data *)node->data; + struct ed_type_runtime_data *data = node->internal_data; struct sset *local_lports = &data->local_lports; struct sset *active_tunnels = &data->active_tunnels; @@ -1121,7 +1114,7 @@ struct ed_type_ct_zones { static void en_ct_zones_init(struct engine_node *node) { - struct ed_type_ct_zones *data = node->data; + struct ed_type_ct_zones *data = node->internal_data; struct ovsrec_open_vswitch_table *ovs_table = (struct ovsrec_open_vswitch_table *)EN_OVSDB_GET( engine_get_input("OVS_open_vswitch", node)); @@ -1140,7 +1133,7 @@ en_ct_zones_init(struct engine_node *node) static void en_ct_zones_cleanup(struct engine_node *node) { - struct ed_type_ct_zones *data = node->data; + struct ed_type_ct_zones *data = node->internal_data; simap_destroy(&data->current); shash_destroy(&data->pending); @@ -1149,10 +1142,9 @@ en_ct_zones_cleanup(struct engine_node *node) static void en_ct_zones_run(struct engine_node *node) { - struct ed_type_ct_zones *data = node->data; + struct ed_type_ct_zones *data = node->internal_data; struct ed_type_runtime_data *rt_data = - (struct ed_type_runtime_data *)engine_get_input( - "runtime_data", node)->data; + engine_get_input("runtime_data", node)->internal_data; update_ct_zones(&rt_data->local_lports, &rt_data->local_datapaths, &data->current, data->bitmap, &data->pending); @@ -1160,6 +1152,13 @@ en_ct_zones_run(struct engine_node *node) engine_set_node_state(node, EN_UPDATED); } +/* The data in the ct_zones node is always valid (i.e., no stale pointers). */ +static bool +en_ct_zones_is_valid(struct engine_node *node OVS_UNUSED) +{ + return true; +} + struct ed_type_mff_ovn_geneve { enum mf_field_id mff_ovn_geneve; }; @@ -1167,8 +1166,7 @@ struct ed_type_mff_ovn_geneve { static void en_mff_ovn_geneve_init(struct engine_node *node) { - struct ed_type_mff_ovn_geneve *data = - (struct ed_type_mff_ovn_geneve *)node->data; + struct ed_type_mff_ovn_geneve *data = node->internal_data; data->mff_ovn_geneve = 0; } @@ -1180,8 +1178,7 @@ en_mff_ovn_geneve_cleanup(struct engine_node *node OVS_UNUSED) static void en_mff_ovn_geneve_run(struct engine_node *node) { - struct ed_type_mff_ovn_geneve *data = - (struct ed_type_mff_ovn_geneve *)node->data; + struct ed_type_mff_ovn_geneve *data = node->internal_data; enum mf_field_id mff_ovn_geneve = ofctrl_get_mf_field_id(); if (data->mff_ovn_geneve != mff_ovn_geneve) { data->mff_ovn_geneve = mff_ovn_geneve; @@ -1207,8 +1204,7 @@ struct ed_type_flow_output { static void en_flow_output_init(struct engine_node *node) { - struct ed_type_flow_output *data = - (struct ed_type_flow_output *)node->data; + struct ed_type_flow_output *data = node->internal_data; ovn_desired_flow_table_init(&data->flow_table); ovn_extend_table_init(&data->group_table); ovn_extend_table_init(&data->meter_table); @@ -1219,8 +1215,7 @@ en_flow_output_init(struct engine_node *node) static void en_flow_output_cleanup(struct engine_node *node) { - struct ed_type_flow_output *data = - (struct ed_type_flow_output *)node->data; + struct ed_type_flow_output *data = node->internal_data; ovn_desired_flow_table_destroy(&data->flow_table); ovn_extend_table_destroy(&data->group_table); ovn_extend_table_destroy(&data->meter_table); @@ -1231,21 +1226,18 @@ static void en_flow_output_run(struct engine_node *node) { struct ed_type_runtime_data *rt_data = - (struct ed_type_runtime_data *)engine_get_input( - "runtime_data", node)->data; + engine_get_input("runtime_data", node)->internal_data; struct hmap *local_datapaths = &rt_data->local_datapaths; struct sset *local_lports = &rt_data->local_lports; struct sset *local_lport_ids = &rt_data->local_lport_ids; struct sset *active_tunnels = &rt_data->active_tunnels; struct ed_type_ct_zones *ct_zones_data = - (struct ed_type_ct_zones *)engine_get_input( - "ct_zones", node)->data; + engine_get_input("ct_zones", node)->internal_data; struct simap *ct_zones = &ct_zones_data->current; struct ed_type_mff_ovn_geneve *ed_mff_ovn_geneve = - (struct ed_type_mff_ovn_geneve *)engine_get_input( - "mff_ovn_geneve", node)->data; + engine_get_input("mff_ovn_geneve", node)->internal_data; enum mf_field_id mff_ovn_geneve = ed_mff_ovn_geneve->mff_ovn_geneve; struct ovsrec_open_vswitch_table *ovs_table = @@ -1262,12 +1254,11 @@ en_flow_output_run(struct engine_node *node) engine_get_input("SB_chassis", node), "name"); struct ed_type_addr_sets *as_data = - (struct ed_type_addr_sets *)engine_get_input("addr_sets", node)->data; + engine_get_input("addr_sets", node)->internal_data; struct shash *addr_sets = &as_data->addr_sets; struct ed_type_port_groups *pg_data = - (struct ed_type_port_groups *)engine_get_input( - "port_groups", node)->data; + engine_get_input("port_groups", node)->internal_data; struct shash *port_groups = &pg_data->port_groups; const struct sbrec_chassis *chassis = NULL; @@ -1277,8 +1268,7 @@ en_flow_output_run(struct engine_node *node) ovs_assert(br_int && chassis); - struct ed_type_flow_output *fo = - (struct ed_type_flow_output *)node->data; + struct ed_type_flow_output *fo = node->internal_data; struct ovn_desired_flow_table *flow_table = &fo->flow_table; struct ovn_extend_table *group_table = &fo->group_table; struct ovn_extend_table *meter_table = &fo->meter_table; @@ -1361,18 +1351,16 @@ static bool flow_output_sb_logical_flow_handler(struct engine_node *node) { struct ed_type_runtime_data *data = - (struct ed_type_runtime_data *)engine_get_input( - "runtime_data", node)->data; + engine_get_input("runtime_data", node)->internal_data; struct hmap *local_datapaths = &data->local_datapaths; struct sset *local_lport_ids = &data->local_lport_ids; struct sset *active_tunnels = &data->active_tunnels; struct ed_type_addr_sets *as_data = - (struct ed_type_addr_sets *)engine_get_input("addr_sets", node)->data; + engine_get_input("addr_sets", node)->internal_data; struct shash *addr_sets = &as_data->addr_sets; struct ed_type_port_groups *pg_data = - (struct ed_type_port_groups *)engine_get_input( - "port_groups", node)->data; + engine_get_input("port_groups", node)->internal_data; struct shash *port_groups = &pg_data->port_groups; struct ovsrec_open_vswitch_table *ovs_table = @@ -1396,8 +1384,7 @@ flow_output_sb_logical_flow_handler(struct engine_node *node) ovs_assert(br_int && chassis); - struct ed_type_flow_output *fo = - (struct ed_type_flow_output *)node->data; + struct ed_type_flow_output *fo = node->internal_data; struct ovn_desired_flow_table *flow_table = &fo->flow_table; struct ovn_extend_table *group_table = &fo->group_table; struct ovn_extend_table *meter_table = &fo->meter_table; @@ -1452,8 +1439,7 @@ flow_output_sb_mac_binding_handler(struct engine_node *node) (struct sbrec_mac_binding_table *)EN_OVSDB_GET( engine_get_input("SB_mac_binding", node)); - struct ed_type_flow_output *fo = - (struct ed_type_flow_output *)node->data; + struct ed_type_flow_output *fo = node->internal_data; struct ovn_desired_flow_table *flow_table = &fo->flow_table; lflow_handle_changed_neighbors(sbrec_port_binding_by_name, @@ -1467,19 +1453,16 @@ static bool flow_output_sb_port_binding_handler(struct engine_node *node) { struct ed_type_runtime_data *data = - (struct ed_type_runtime_data *)engine_get_input( - "runtime_data", node)->data; + engine_get_input("runtime_data", node)->internal_data; struct hmap *local_datapaths = &data->local_datapaths; struct sset *active_tunnels = &data->active_tunnels; struct ed_type_ct_zones *ct_zones_data = - (struct ed_type_ct_zones *)engine_get_input( - "ct_zones", node)->data; + engine_get_input("ct_zones", node)->internal_data; struct simap *ct_zones = &ct_zones_data->current; struct ed_type_mff_ovn_geneve *ed_mff_ovn_geneve = - (struct ed_type_mff_ovn_geneve *)engine_get_input( - "mff_ovn_geneve", node)->data; + engine_get_input("mff_ovn_geneve", node)->internal_data; enum mf_field_id mff_ovn_geneve = ed_mff_ovn_geneve->mff_ovn_geneve; struct ovsrec_open_vswitch_table *ovs_table = @@ -1501,8 +1484,7 @@ flow_output_sb_port_binding_handler(struct engine_node *node) } ovs_assert(br_int && chassis); - struct ed_type_flow_output *fo = - (struct ed_type_flow_output *)node->data; + struct ed_type_flow_output *fo = node->internal_data; struct ovn_desired_flow_table *flow_table = &fo->flow_table; struct ovsdb_idl_index *sbrec_port_binding_by_name = @@ -1575,18 +1557,15 @@ static bool flow_output_sb_multicast_group_handler(struct engine_node *node) { struct ed_type_runtime_data *data = - (struct ed_type_runtime_data *)engine_get_input( - "runtime_data", node)->data; + engine_get_input("runtime_data", node)->internal_data; struct hmap *local_datapaths = &data->local_datapaths; struct ed_type_ct_zones *ct_zones_data = - (struct ed_type_ct_zones *)engine_get_input( - "ct_zones", node)->data; + engine_get_input("ct_zones", node)->internal_data; struct simap *ct_zones = &ct_zones_data->current; struct ed_type_mff_ovn_geneve *ed_mff_ovn_geneve = - (struct ed_type_mff_ovn_geneve *)engine_get_input( - "mff_ovn_geneve", node)->data; + engine_get_input("mff_ovn_geneve", node)->internal_data; enum mf_field_id mff_ovn_geneve = ed_mff_ovn_geneve->mff_ovn_geneve; struct ovsrec_open_vswitch_table *ovs_table = @@ -1608,8 +1587,7 @@ flow_output_sb_multicast_group_handler(struct engine_node *node) } ovs_assert(br_int && chassis); - struct ed_type_flow_output *fo = - (struct ed_type_flow_output *)node->data; + struct ed_type_flow_output *fo = node->internal_data; struct ovn_desired_flow_table *flow_table = &fo->flow_table; struct sbrec_multicast_group_table *multicast_group_table = @@ -1630,19 +1608,17 @@ _flow_output_resource_ref_handler(struct engine_node *node, enum ref_type ref_type) { struct ed_type_runtime_data *data = - (struct ed_type_runtime_data *)engine_get_input( - "runtime_data", node)->data; + engine_get_input("runtime_data", node)->internal_data; struct hmap *local_datapaths = &data->local_datapaths; struct sset *local_lport_ids = &data->local_lport_ids; struct sset *active_tunnels = &data->active_tunnels; struct ed_type_addr_sets *as_data = - (struct ed_type_addr_sets *)engine_get_input("addr_sets", node)->data; + engine_get_input("addr_sets", node)->internal_data; struct shash *addr_sets = &as_data->addr_sets; struct ed_type_port_groups *pg_data = - (struct ed_type_port_groups *)engine_get_input( - "port_groups", node)->data; + engine_get_input("port_groups", node)->internal_data; struct shash *port_groups = &pg_data->port_groups; struct ovsrec_open_vswitch_table *ovs_table = @@ -1666,7 +1642,7 @@ _flow_output_resource_ref_handler(struct engine_node *node, ovs_assert(br_int && chassis); struct ed_type_flow_output *fo = - (struct ed_type_flow_output *)node->data; + node->internal_data; struct ovn_desired_flow_table *flow_table = &fo->flow_table; struct ovn_extend_table *group_table = &fo->group_table; struct ovn_extend_table *meter_table = &fo->meter_table; @@ -1899,7 +1875,7 @@ main(int argc, char *argv[]) struct ed_type_addr_sets ed_addr_sets; struct ed_type_port_groups ed_port_groups; - ENGINE_NODE(ct_zones, "ct_zones"); + ENGINE_NODE_CUSTOM_DATA(ct_zones, "ct_zones"); ENGINE_NODE(runtime_data, "runtime_data"); ENGINE_NODE(mff_ovn_geneve, "mff_ovn_geneve"); ENGINE_NODE(ofctrl_is_connected, "ofctrl_is_connected"); @@ -2061,7 +2037,10 @@ main(int argc, char *argv[]) } if (br_int) { - ofctrl_run(br_int, &ed_ct_zones.pending); + if (en_ct_zones.data) { + struct ed_type_ct_zones *ct_zones = en_ct_zones.data; + ofctrl_run(br_int, &ct_zones->pending); + } if (chassis) { patch_run(ovs_idl_txn, @@ -2104,40 +2083,53 @@ main(int argc, char *argv[]) stopwatch_stop(CONTROLLER_LOOP_STOPWATCH_NAME, time_msec()); if (ovs_idl_txn) { - commit_ct_zones(br_int, &ed_ct_zones.pending); + if (en_ct_zones.data) { + struct ed_type_ct_zones *ct_zones = + en_ct_zones.data; + commit_ct_zones(br_int, &ct_zones->pending); + } bfd_run(ovsrec_interface_table_get(ovs_idl_loop.idl), br_int, chassis, sbrec_ha_chassis_group_table_get( ovnsb_idl_loop.idl), sbrec_sb_global_table_get(ovnsb_idl_loop.idl)); } - ofctrl_put(&ed_flow_output.flow_table, - &ed_ct_zones.pending, - sbrec_meter_table_get(ovnsb_idl_loop.idl), - get_nb_cfg(sbrec_sb_global_table_get( - ovnsb_idl_loop.idl)), - engine_node_changed(&en_flow_output)); - pinctrl_run(ovnsb_idl_txn, - sbrec_datapath_binding_by_key, - sbrec_port_binding_by_datapath, - sbrec_port_binding_by_key, - sbrec_port_binding_by_name, - sbrec_mac_binding_by_lport_ip, - sbrec_igmp_group, - sbrec_ip_multicast, - sbrec_dns_table_get(ovnsb_idl_loop.idl), - sbrec_controller_event_table_get( - ovnsb_idl_loop.idl), - sbrec_service_monitor_table_get( - ovnsb_idl_loop.idl), - br_int, chassis, - &ed_runtime_data.local_datapaths, - &ed_runtime_data.active_tunnels); - - if (engine_node_changed(&en_runtime_data)) { - update_sb_monitors(ovnsb_idl_loop.idl, chassis, - &ed_runtime_data.local_lports, - &ed_runtime_data.local_datapaths); + if (en_flow_output.data && en_ct_zones.data) { + struct ed_type_ct_zones *ct_zones = + en_ct_zones.data; + struct ed_type_flow_output *flow_output = + en_flow_output.data; + ofctrl_put(&flow_output->flow_table, + &ct_zones->pending, + sbrec_meter_table_get(ovnsb_idl_loop.idl), + get_nb_cfg(sbrec_sb_global_table_get( + ovnsb_idl_loop.idl)), + engine_node_changed(&en_flow_output)); + } + if (en_runtime_data.data) { + struct ed_type_runtime_data *rt_data = + en_runtime_data.data; + pinctrl_run(ovnsb_idl_txn, + sbrec_datapath_binding_by_key, + sbrec_port_binding_by_datapath, + sbrec_port_binding_by_key, + sbrec_port_binding_by_name, + sbrec_mac_binding_by_lport_ip, + sbrec_igmp_group, + sbrec_ip_multicast, + sbrec_dns_table_get(ovnsb_idl_loop.idl), + sbrec_controller_event_table_get( + ovnsb_idl_loop.idl), + sbrec_service_monitor_table_get( + ovnsb_idl_loop.idl), + br_int, chassis, + &rt_data->local_datapaths, + &rt_data->active_tunnels); + if (engine_node_changed(&en_runtime_data)) { + update_sb_monitors(ovnsb_idl_loop.idl, chassis, + &rt_data->local_lports, + &rt_data->local_datapaths); + } } } @@ -2169,9 +2161,14 @@ main(int argc, char *argv[]) if (pending_pkt.conn) { - if (br_int && chassis) { + if (br_int && chassis && en_addr_sets.data && + en_port_groups.data) { + struct ed_type_addr_sets *as_data = + en_addr_sets.data; + struct ed_type_port_groups *pg_data = + en_port_groups.data; char *error = ofctrl_inject_pkt(br_int, pending_pkt.flow_s, - &ed_addr_sets.addr_sets, &ed_port_groups.port_groups); + &as_data->addr_sets, &pg_data->port_groups); if (error) { unixctl_command_reply_error(pending_pkt.conn, error); free(error); @@ -2209,12 +2206,15 @@ main(int argc, char *argv[]) } if (ovsdb_idl_loop_commit_and_wait(&ovs_idl_loop) == 1) { - struct shash_node *iter, *iter_next; - SHASH_FOR_EACH_SAFE (iter, iter_next, &ed_ct_zones.pending) { - struct ct_zone_pending_entry *ctzpe = iter->data; - if (ctzpe->state == CT_ZONE_DB_SENT) { - shash_delete(&ed_ct_zones.pending, iter); - free(ctzpe); + if (en_ct_zones.data) { + struct ed_type_ct_zones *ct_zones = en_ct_zones.data; + struct shash_node *iter, *iter_next; + SHASH_FOR_EACH_SAFE (iter, iter_next, &ct_zones->pending) { + struct ct_zone_pending_entry *ctzpe = iter->data; + if (ctzpe->state == CT_ZONE_DB_SENT) { + shash_delete(&ct_zones->pending, iter); + free(ctzpe); + } } } } diff --git a/lib/inc-proc-eng.c b/lib/inc-proc-eng.c index f88116f..f080d71 100644 --- a/lib/inc-proc-eng.c +++ b/lib/inc-proc-eng.c @@ -152,7 +152,7 @@ engine_add_input(struct engine_node *node, struct engine_node *input, struct ovsdb_idl_index * engine_ovsdb_node_get_index(struct engine_node *node, const char *name) { - struct ed_type_ovsdb_table *ed = (struct ed_type_ovsdb_table *)node->data; + struct ed_type_ovsdb_table *ed = node->internal_data; for (size_t i = 0; i < ed->n_indexes; i++) { if (!strcmp(ed->indexes[i].name, name)) { return ed->indexes[i].index; @@ -166,7 +166,7 @@ void engine_ovsdb_node_add_index(struct engine_node *node, const char *name, struct ovsdb_idl_index *index) { - struct ed_type_ovsdb_table *ed = (struct ed_type_ovsdb_table *)node->data; + struct ed_type_ovsdb_table *ed = node->internal_data; ovs_assert(ed->n_indexes < ENGINE_MAX_OVSDB_INDEX); ed->indexes[ed->n_indexes].name = name; @@ -194,7 +194,14 @@ engine_set_node_state_at(struct engine_node *node, static bool engine_node_valid(struct engine_node *node) { - return (node->state == EN_UPDATED || node->state == EN_VALID); + if (node->state == EN_UPDATED || node->state == EN_VALID) { + return true; + } + + if (node->is_valid) { + return node->is_valid(node); + } + return false; } bool @@ -225,12 +232,26 @@ engine_aborted(void) return false; } +static void * +engine_get_data(struct engine_node *node) +{ + if (engine_node_valid(node)) { + return node->internal_data; + } + return NULL; +} + void engine_init_run(void) { VLOG_DBG("Initializing new run"); for (size_t i = 0; i < engine_n_nodes; i++) { engine_set_node_state(engine_nodes[i], EN_STALE); + + /* Make sure we reset the data pointer for outside users. + * For nodes that always store valid data the value will be non-NULL. + */ + engine_nodes[i]->data = engine_get_data(engine_nodes[i]); } } @@ -352,6 +373,11 @@ engine_run(bool abort_on_recompute) { for (size_t i = 0; i < engine_n_nodes; i++) { engine_run_node(engine_nodes[i], !abort_on_recompute); + + /* Make sure we reset the data pointer for outside users as the + * node's state might have changed. + */ + engine_nodes[i]->data = engine_get_data(engine_nodes[i]); } } diff --git a/lib/inc-proc-eng.h b/lib/inc-proc-eng.h index 5315649..ef15735 100644 --- a/lib/inc-proc-eng.h +++ b/lib/inc-proc-eng.h @@ -111,6 +111,12 @@ struct engine_node { * and run() function of the current node. Users should ensure that the * data is read-only in change-handlers of the nodes that depends on this * node. */ + void *internal_data; + + /* A pointer to node data accessible for users outside the processing + * engine. The value of the pointer is updated by the engine itself and + * users should ensure that the data is only read. + */ void *data; /* State of the node after the last engine run. */ @@ -125,6 +131,13 @@ struct engine_node { /* Fully processes all inputs of this node and regenerates the data * of this node */ void (*run)(struct engine_node *); + + /* Method to validate if the 'internal_data' is valid. This allows users + * to customize when 'internal_data' can be used (e.g., even if the node + * hasn't been refreshed in the last iteration, if 'internal_data' + * doesn't store pointers to DB records it's still safe to use). + */ + bool (*is_valid)(struct engine_node *); }; /* Initialize the data for the engine nodes. It calls each node's @@ -201,7 +214,7 @@ struct ed_type_ovsdb_table { }; #define EN_OVSDB_GET(NODE) \ - (((struct ed_type_ovsdb_table *)NODE->data)->table) + (((struct ed_type_ovsdb_table *)NODE->internal_data)->table) struct ovsdb_idl_index * engine_ovsdb_node_get_index(struct engine_node *, const char *name); @@ -210,16 +223,25 @@ void engine_ovsdb_node_add_index(struct engine_node *, const char *name, struct ovsdb_idl_index *); /* Macro to define an engine node. */ -#define ENGINE_NODE(NAME, NAME_STR) \ +#define ENGINE_NODE_DEF(NAME, NAME_STR) \ struct engine_node en_##NAME = { \ .name = NAME_STR, \ - .data = &ed_##NAME, \ + .internal_data = &ed_##NAME, \ + .data = NULL, \ .state = EN_STALE, \ .init = en_##NAME##_init, \ .run = en_##NAME##_run, \ .cleanup = en_##NAME##_cleanup, \ + .is_valid = en_##NAME##_is_valid, \ }; +#define ENGINE_NODE_CUSTOM_DATA(NAME, NAME_STR) \ + ENGINE_NODE_DEF(NAME, NAME_STR) + +#define ENGINE_NODE(NAME, NAME_STR) \ + static bool (*en_##NAME##_is_valid)(struct engine_node *node) = NULL; \ + ENGINE_NODE_DEF(NAME, NAME_STR) + /* Macro to define member functions of an engine node which represents * a table of OVSDB */ #define ENGINE_FUNC_OVSDB(DB_NAME, TBL_NAME) \