From patchwork Mon Nov 23 07:19:26 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Numan Siddique X-Patchwork-Id: 547392 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from archives.nicira.com (unknown [IPv6:2600:3c00::f03c:91ff:fe6e:bdf7]) by ozlabs.org (Postfix) with ESMTP id 39DBC14029E for ; Mon, 23 Nov 2015 18:19:42 +1100 (AEDT) Received: from archives.nicira.com (localhost [127.0.0.1]) by archives.nicira.com (Postfix) with ESMTP id 5AEA3102F0; Sun, 22 Nov 2015 23:19:41 -0800 (PST) X-Original-To: dev@openvswitch.org Delivered-To: dev@openvswitch.org Received: from mx3v3.cudamail.com (mx3.cudamail.com [64.34.241.5]) by archives.nicira.com (Postfix) with ESMTPS id 83E67102EC for ; Sun, 22 Nov 2015 23:19:40 -0800 (PST) Received: from bar3.cudamail.com (localhost [127.0.0.1]) by mx3v3.cudamail.com (Postfix) with ESMTPS id BA14716165B for ; Mon, 23 Nov 2015 00:19:39 -0700 (MST) X-ASG-Debug-ID: 1448263178-03dd7b46451ad600001-byXFYA Received: from mx3-pf1.cudamail.com ([192.168.14.2]) by bar3.cudamail.com with ESMTP id 2oPS8GyT6jMchRxM (version=TLSv1 cipher=DHE-RSA-AES256-SHA bits=256 verify=NO) for ; Mon, 23 Nov 2015 00:19:38 -0700 (MST) X-Barracuda-Envelope-From: nusiddiq@redhat.com X-Barracuda-RBL-Trusted-Forwarder: 192.168.14.2 Received: from unknown (HELO mx1.redhat.com) (209.132.183.28) by mx3-pf1.cudamail.com with ESMTPS (DHE-RSA-AES256-SHA encrypted); 23 Nov 2015 07:19:32 -0000 Received-SPF: pass (mx3-pf1.cudamail.com: SPF record at _spf1.redhat.com designates 209.132.183.28 as permitted sender) X-Barracuda-Apparent-Source-IP: 209.132.183.28 X-Barracuda-RBL-IP: 209.132.183.28 Received: from int-mx13.intmail.prod.int.phx2.redhat.com (int-mx13.intmail.prod.int.phx2.redhat.com [10.5.11.26]) by mx1.redhat.com (Postfix) with ESMTPS id 8D8E3C057EC8 for ; Mon, 23 Nov 2015 07:19:29 +0000 (UTC) Received: from nusiddiq.blr.redhat.com (dhcp-0-119.blr.redhat.com [10.70.1.119]) by int-mx13.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id tAN7JR35011324 for ; Mon, 23 Nov 2015 02:19:28 -0500 X-CudaMail-Envelope-Sender: nusiddiq@redhat.com From: Numan Siddique X-CudaMail-MID: CM-V1-1122000320 X-CudaMail-DTE: 112315 X-CudaMail-Originating-IP: 209.132.183.28 To: dev@openvswitch.org X-ASG-Orig-Subj: [##CM-V1-1122000320##][ovs-dev][PATCH 1/1] ovn-northd: Refactor main loop to use ovsdb_idl_loop_* functions Organization: Red Hat Message-ID: <5652BDFE.20900@redhat.com> Date: Mon, 23 Nov 2015 12:49:26 +0530 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.3.0 MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.68 on 10.5.11.26 X-GBUdb-Analysis: 0, 209.132.183.28, Ugly c=0 p=0 Source New X-MessageSniffer-Rules: 0-0-0-24888-c X-Barracuda-Connect: UNKNOWN[192.168.14.2] X-Barracuda-Start-Time: 1448263178 X-Barracuda-Encrypted: DHE-RSA-AES256-SHA X-Barracuda-URL: https://web.cudamail.com:443/cgi-mod/mark.cgi X-Virus-Scanned: by bsmtpd at cudamail.com X-Barracuda-BRTS-Status: 1 X-Barracuda-Spam-Score: 0.60 X-Barracuda-Spam-Status: No, SCORE=0.60 using per-user scores of TAG_LEVEL=3.5 QUARANTINE_LEVEL=1000.0 KILL_LEVEL=4.0 tests=BSF_SC5_MJ1963, RDNS_NONE X-Barracuda-Spam-Report: Code version 3.2, rules version 3.2.3.24643 Rule breakdown below pts rule name description ---- ---------------------- -------------------------------------------------- 0.10 RDNS_NONE Delivered to trusted network by a host with no rDNS 0.50 BSF_SC5_MJ1963 Custom Rule MJ1963 Subject: [ovs-dev] [PATCH 1/1] ovn-northd: Refactor main loop to use ovsdb_idl_loop_* functions X-BeenThere: dev@openvswitch.org X-Mailman-Version: 2.1.16 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@openvswitch.org Sender: "dev" This patch also addresses the issue reported at http://openvswitch.org/pipermail/discuss/2015-November/019445.html Suggested-by: Russell Bryant Signed-off-by: Numan Siddique --- ovn/northd/ovn-northd.c | 250 ++++++++++++++---------------------------------- 1 file changed, 70 insertions(+), 180 deletions(-) diff --git a/ovn/northd/ovn-northd.c b/ovn/northd/ovn-northd.c index 8fe0c2c..04402ed 100644 --- a/ovn/northd/ovn-northd.c +++ b/ovn/northd/ovn-northd.c @@ -1624,10 +1624,12 @@ build_lflows(struct northd_context *ctx, struct hmap *datapaths, } static void -ovnnb_db_changed(struct northd_context *ctx) +ovnnb_db_run(struct northd_context *ctx) { - VLOG_DBG("ovn-nb db contents have changed."); - + if (!ctx->ovnsb_txn) { + return; + } + VLOG_DBG("ovn-nb db contents may have changed."); struct hmap datapaths, ports; build_datapaths(ctx, &datapaths); build_ports(ctx, &datapaths, &ports); @@ -1652,8 +1654,11 @@ ovnnb_db_changed(struct northd_context *ctx) * need to set the corresponding logical port as 'up' in the northbound DB. */ static void -ovnsb_db_changed(struct northd_context *ctx) +ovnsb_db_run(struct northd_context *ctx) { + if (!ctx->ovnnb_txn) { + return; + } struct hmap lports_hmap; const struct sbrec_port_binding *sb; const struct nbrec_logical_port *nb; @@ -1800,14 +1805,7 @@ int main(int argc, char *argv[]) { extern struct vlog_module VLM_reconnect; - struct ovsdb_idl *ovnnb_idl, *ovnsb_idl; - unsigned int ovnnb_seqno, ovn_seqno; int res = EXIT_SUCCESS; - struct northd_context ctx = { - .ovnsb_txn = NULL, - }; - bool ovnnb_changes_pending = false; - bool ovn_changes_pending = false; struct unixctl_server *unixctl; int retval; bool exiting; @@ -1833,190 +1831,82 @@ main(int argc, char *argv[]) sbrec_init(); /* We want to detect all changes to the ovn-nb db. */ - ctx.ovnnb_idl = ovnnb_idl = ovsdb_idl_create(ovnnb_db, - &nbrec_idl_class, true, true); - - ctx.ovnsb_idl = ovnsb_idl = ovsdb_idl_create(ovnsb_db, - &sbrec_idl_class, false, true); - - ovsdb_idl_add_table(ovnsb_idl, &sbrec_table_logical_flow); - add_column_noalert(ovnsb_idl, &sbrec_logical_flow_col_logical_datapath); - add_column_noalert(ovnsb_idl, &sbrec_logical_flow_col_pipeline); - add_column_noalert(ovnsb_idl, &sbrec_logical_flow_col_table_id); - add_column_noalert(ovnsb_idl, &sbrec_logical_flow_col_priority); - add_column_noalert(ovnsb_idl, &sbrec_logical_flow_col_match); - add_column_noalert(ovnsb_idl, &sbrec_logical_flow_col_actions); - - ovsdb_idl_add_table(ovnsb_idl, &sbrec_table_multicast_group); - add_column_noalert(ovnsb_idl, &sbrec_multicast_group_col_datapath); - add_column_noalert(ovnsb_idl, &sbrec_multicast_group_col_tunnel_key); - add_column_noalert(ovnsb_idl, &sbrec_multicast_group_col_name); - add_column_noalert(ovnsb_idl, &sbrec_multicast_group_col_ports); - - ovsdb_idl_add_table(ovnsb_idl, &sbrec_table_datapath_binding); - add_column_noalert(ovnsb_idl, &sbrec_datapath_binding_col_tunnel_key); - add_column_noalert(ovnsb_idl, &sbrec_datapath_binding_col_external_ids); - - ovsdb_idl_add_table(ovnsb_idl, &sbrec_table_port_binding); - add_column_noalert(ovnsb_idl, &sbrec_port_binding_col_datapath); - add_column_noalert(ovnsb_idl, &sbrec_port_binding_col_logical_port); - add_column_noalert(ovnsb_idl, &sbrec_port_binding_col_tunnel_key); - add_column_noalert(ovnsb_idl, &sbrec_port_binding_col_parent_port); - add_column_noalert(ovnsb_idl, &sbrec_port_binding_col_tag); - add_column_noalert(ovnsb_idl, &sbrec_port_binding_col_type); - add_column_noalert(ovnsb_idl, &sbrec_port_binding_col_options); - add_column_noalert(ovnsb_idl, &sbrec_port_binding_col_mac); - ovsdb_idl_add_column(ovnsb_idl, &sbrec_port_binding_col_chassis); - - /* - * The loop here just runs the IDL in a loop waiting for the seqno to - * change, which indicates that the contents of the db have changed. - * - * If the contents of the ovn-nb db change, the mappings to the ovn-sb - * db must be recalculated. - * - * If the contents of the ovn-sb db change, it means the 'up' state of - * a port may have changed, as that's the only type of change ovn-northd is - * watching for. - */ - - ovnnb_seqno = ovsdb_idl_get_seqno(ovnnb_idl); - ovn_seqno = ovsdb_idl_get_seqno(ovnsb_idl); + struct ovsdb_idl_loop ovnnb_idl_loop = OVSDB_IDL_LOOP_INITIALIZER( + ovsdb_idl_create(ovnnb_db, &nbrec_idl_class, true, true)); + + struct ovsdb_idl_loop ovnsb_idl_loop = OVSDB_IDL_LOOP_INITIALIZER( + ovsdb_idl_create(ovnsb_db, &sbrec_idl_class, false, true)); + + ovsdb_idl_add_table(ovnsb_idl_loop.idl, &sbrec_table_logical_flow); + add_column_noalert(ovnsb_idl_loop.idl, + &sbrec_logical_flow_col_logical_datapath); + add_column_noalert(ovnsb_idl_loop.idl, &sbrec_logical_flow_col_pipeline); + add_column_noalert(ovnsb_idl_loop.idl, &sbrec_logical_flow_col_table_id); + add_column_noalert(ovnsb_idl_loop.idl, &sbrec_logical_flow_col_priority); + add_column_noalert(ovnsb_idl_loop.idl, &sbrec_logical_flow_col_match); + add_column_noalert(ovnsb_idl_loop.idl, &sbrec_logical_flow_col_actions); + + ovsdb_idl_add_table(ovnsb_idl_loop.idl, &sbrec_table_multicast_group); + add_column_noalert(ovnsb_idl_loop.idl, + &sbrec_multicast_group_col_datapath); + add_column_noalert(ovnsb_idl_loop.idl, + &sbrec_multicast_group_col_tunnel_key); + add_column_noalert(ovnsb_idl_loop.idl, &sbrec_multicast_group_col_name); + add_column_noalert(ovnsb_idl_loop.idl, &sbrec_multicast_group_col_ports); + + ovsdb_idl_add_table(ovnsb_idl_loop.idl, &sbrec_table_datapath_binding); + add_column_noalert(ovnsb_idl_loop.idl, + &sbrec_datapath_binding_col_tunnel_key); + add_column_noalert(ovnsb_idl_loop.idl, + &sbrec_datapath_binding_col_external_ids); + + ovsdb_idl_add_table(ovnsb_idl_loop.idl, &sbrec_table_port_binding); + add_column_noalert(ovnsb_idl_loop.idl, &sbrec_port_binding_col_datapath); + add_column_noalert(ovnsb_idl_loop.idl, + &sbrec_port_binding_col_logical_port); + add_column_noalert(ovnsb_idl_loop.idl, + &sbrec_port_binding_col_tunnel_key); + add_column_noalert(ovnsb_idl_loop.idl, + &sbrec_port_binding_col_parent_port); + add_column_noalert(ovnsb_idl_loop.idl, &sbrec_port_binding_col_tag); + add_column_noalert(ovnsb_idl_loop.idl, &sbrec_port_binding_col_type); + add_column_noalert(ovnsb_idl_loop.idl, &sbrec_port_binding_col_options); + add_column_noalert(ovnsb_idl_loop.idl, &sbrec_port_binding_col_mac); + ovsdb_idl_add_column(ovnsb_idl_loop.idl, &sbrec_port_binding_col_chassis); + + /* Main loop. */ exiting = false; while (!exiting) { - ovsdb_idl_run(ovnnb_idl); - ovsdb_idl_run(ovnsb_idl); - unixctl_server_run(unixctl); + struct northd_context ctx = { + .ovnnb_idl = ovnnb_idl_loop.idl, + .ovnnb_txn = ovsdb_idl_loop_run(&ovnnb_idl_loop), + .ovnsb_idl = ovnsb_idl_loop.idl, + .ovnsb_txn = ovsdb_idl_loop_run(&ovnsb_idl_loop), + }; - if (!ovsdb_idl_is_alive(ovnnb_idl)) { - int retval = ovsdb_idl_get_last_error(ovnnb_idl); - VLOG_ERR("%s: database connection failed (%s)", - ovnnb_db, ovs_retval_to_string(retval)); - res = EXIT_FAILURE; - break; - } - - if (!ovsdb_idl_is_alive(ovnsb_idl)) { - int retval = ovsdb_idl_get_last_error(ovnsb_idl); - VLOG_ERR("%s: database connection failed (%s)", - ovnsb_db, ovs_retval_to_string(retval)); - res = EXIT_FAILURE; - break; - } - - if (ovnnb_seqno != ovsdb_idl_get_seqno(ovnnb_idl)) { - ovnnb_seqno = ovsdb_idl_get_seqno(ovnnb_idl); - ovnnb_changes_pending = true; - } - - if (ovn_seqno != ovsdb_idl_get_seqno(ovnsb_idl)) { - ovn_seqno = ovsdb_idl_get_seqno(ovnsb_idl); - ovn_changes_pending = true; - } + ovnnb_db_run(&ctx); + ovnsb_db_run(&ctx); - /* - * If there are any pending changes, we delay recalculating the - * necessary updates until after an existing transaction finishes. - * This avoids the possibility of rapid updates causing ovn-northd to - * never be able to successfully make the corresponding updates to the - * other db. Instead, pending changes are batched up until the next - * time we get a chance to calculate the new state and apply it. - */ - - if (ovnnb_changes_pending && !ctx.ovnsb_txn) { - /* - * The OVN-nb db contents have changed, so create a transaction for - * updating the OVN-sb DB. - */ - ctx.ovnsb_txn = ovsdb_idl_txn_create(ctx.ovnsb_idl); - ovsdb_idl_txn_add_comment(ctx.ovnsb_txn, - "ovn-northd: northbound db changed"); - ovnnb_db_changed(&ctx); - ovnnb_changes_pending = false; - } - - if (ovn_changes_pending && !ctx.ovnnb_txn) { - /* - * The OVN-sb db contents have changed, so create a transaction for - * updating the northbound DB. - */ - ctx.ovnnb_txn = ovsdb_idl_txn_create(ctx.ovnnb_idl); - ovsdb_idl_txn_add_comment(ctx.ovnnb_txn, - "ovn-northd: southbound db changed"); - ovnsb_db_changed(&ctx); - ovn_changes_pending = false; - } - - if (ctx.ovnnb_txn) { - enum ovsdb_idl_txn_status txn_status; - txn_status = ovsdb_idl_txn_commit(ctx.ovnnb_txn); - switch (txn_status) { - case TXN_UNCOMMITTED: - case TXN_INCOMPLETE: - /* Come back around and try to commit this transaction again */ - break; - case TXN_ABORTED: - case TXN_TRY_AGAIN: - case TXN_NOT_LOCKED: - case TXN_ERROR: - /* Something went wrong, so try creating a new transaction. */ - ovn_changes_pending = true; - case TXN_UNCHANGED: - case TXN_SUCCESS: - ovsdb_idl_txn_destroy(ctx.ovnnb_txn); - ctx.ovnnb_txn = NULL; - } - } - - if (ctx.ovnsb_txn) { - enum ovsdb_idl_txn_status txn_status; - txn_status = ovsdb_idl_txn_commit(ctx.ovnsb_txn); - switch (txn_status) { - case TXN_UNCOMMITTED: - case TXN_INCOMPLETE: - /* Come back around and try to commit this transaction again */ - break; - case TXN_ABORTED: - case TXN_TRY_AGAIN: - case TXN_NOT_LOCKED: - case TXN_ERROR: - /* Something went wrong, so try creating a new transaction. */ - ovnnb_changes_pending = true; - case TXN_UNCHANGED: - case TXN_SUCCESS: - ovsdb_idl_txn_destroy(ctx.ovnsb_txn); - ctx.ovnsb_txn = NULL; - } + unixctl_server_run(unixctl); + unixctl_server_wait(unixctl); + if (exiting) { + poll_immediate_wake(); } + ovsdb_idl_loop_commit_and_wait(&ovnnb_idl_loop); + ovsdb_idl_loop_commit_and_wait(&ovnsb_idl_loop); - if (ovnnb_seqno == ovsdb_idl_get_seqno(ovnnb_idl) && - ovn_seqno == ovsdb_idl_get_seqno(ovnsb_idl)) { - ovsdb_idl_wait(ovnnb_idl); - ovsdb_idl_wait(ovnsb_idl); - if (ctx.ovnnb_txn) { - ovsdb_idl_txn_wait(ctx.ovnnb_txn); - } - if (ctx.ovnsb_txn) { - ovsdb_idl_txn_wait(ctx.ovnsb_txn); - } - unixctl_server_wait(unixctl); - if (exiting) { - poll_immediate_wake(); - } - poll_block(); - } + poll_block(); if (should_service_stop()) { exiting = true; } } unixctl_server_destroy(unixctl); - ovsdb_idl_destroy(ovnsb_idl); - ovsdb_idl_destroy(ovnnb_idl); + ovsdb_idl_loop_destroy(&ovnnb_idl_loop); + ovsdb_idl_loop_destroy(&ovnsb_idl_loop); service_stop(); free(default_db_); - exit(res); }