From patchwork Fri Nov 22 07:47:50 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Frode Nordahl X-Patchwork-Id: 1199508 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=canonical.com 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 47KKBy12jcz9sPv for ; Sat, 23 Nov 2019 01:51:54 +1100 (AEDT) Received: from localhost (localhost [127.0.0.1]) by whitealder.osuosl.org (Postfix) with ESMTP id 1F0A1882C3; Fri, 22 Nov 2019 14:51:52 +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 iWShFluWX6w0; Fri, 22 Nov 2019 14:51:50 +0000 (UTC) Received: from lists.linuxfoundation.org (lf-lists.osuosl.org [140.211.9.56]) by whitealder.osuosl.org (Postfix) with ESMTP id 25A8188270; Fri, 22 Nov 2019 14:51:50 +0000 (UTC) Received: from lf-lists.osuosl.org (localhost [127.0.0.1]) by lists.linuxfoundation.org (Postfix) with ESMTP id EB5A3C1DE1; Fri, 22 Nov 2019 14:51:49 +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 731F6C18DA for ; Fri, 22 Nov 2019 14:51:48 +0000 (UTC) Received: from localhost (localhost [127.0.0.1]) by silver.osuosl.org (Postfix) with ESMTP id 6246C262D1 for ; Fri, 22 Nov 2019 14:51:48 +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 tcvF59xjvldH for ; Fri, 22 Nov 2019 14:51:47 +0000 (UTC) X-Greylist: from auto-whitelisted by SQLgrey-1.7.6 Received: from u.fnordahl.com (u.fnordahl.com [52.21.249.169]) by silver.osuosl.org (Postfix) with ESMTP id 891302630B for ; Fri, 22 Nov 2019 14:51:25 +0000 (UTC) X-Mailbox-Line: From 8a0b4b2fe6c4da663f2f7581485982256f3e50f3 Mon Sep 17 00:00:00 2001 Message-Id: <8a0b4b2fe6c4da663f2f7581485982256f3e50f3.1574433899.git.frode.nordahl@canonical.com> In-Reply-To: References: From: Frode Nordahl To: dev@openvswitch.org Date: Fri, 22 Nov 2019 08:47:50 +0100 Subject: [ovs-dev] [PATCH ovn v2 1/2] northd: Amend ovn-northd pause resume test X-BeenThere: ovs-dev@openvswitch.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , MIME-Version: 1.0 Errors-To: ovs-dev-bounces@openvswitch.org Sender: "dev" The current pause and resume tests rely on that ``ovn-northd`` will hold on to the lock while paused. Update the test to pause both ``ovn-northd`` instances so we may change this behaviour. Replace references to ``ovs-appctl`` with ``ovn-appctl``. Signed-off-by: Frode Nordahl --- tests/ovn-northd.at | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at index c73fd9003..1c94f2f65 100644 --- a/tests/ovn-northd.at +++ b/tests/ovn-northd.at @@ -912,7 +912,9 @@ AT_SETUP([ovn -- ovn-northd pause and resume]) AT_SKIP_IF([test $HAVE_PYTHON = no]) ovn_start -AT_CHECK([test xfalse = x`as northd ovs-appctl -t ovn-northd is-paused`]) +AT_CHECK([test xfalse = x`as northd ovn-appctl -t ovn-northd is-paused`]) +AT_CHECK([test xfalse = x`as northd-backup ovn-appctl -t ovn-northd \ +is-paused`]) ovn-nbctl ls-add sw0 @@ -927,7 +929,9 @@ OVS_WAIT_UNTIL([ # Now pause the ovn-northd as northd ovs-appctl -t ovn-northd pause -AT_CHECK([test xtrue = x`as northd ovs-appctl -t ovn-northd is-paused`]) +as northd-backup ovs-appctl -t ovn-northd pause +AT_CHECK([test xtrue = x`as northd ovn-appctl -t ovn-northd is-paused`]) +AT_CHECK([test xtrue = x`as northd-backup ovn-appctl -t ovn-northd is-paused`]) ovn-nbctl ls-add sw0 @@ -938,7 +942,10 @@ OVS_WAIT_UNTIL([ # Now resume ovn-northd as northd ovs-appctl -t ovn-northd resume -AT_CHECK([test xfalse = x`as northd ovs-appctl -t ovn-northd is-paused`]) +as northd-backup ovs-appctl -t ovn-northd resume +AT_CHECK([test xfalse = x`as northd ovn-appctl -t ovn-northd is-paused`]) +AT_CHECK([test xfalse = x`as northd-backup ovn-appctl -t ovn-northd \ +is-paused`]) OVS_WAIT_UNTIL([ ovn-sbctl lflow-list sw0 From patchwork Thu Nov 21 16:32:59 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Frode Nordahl X-Patchwork-Id: 1199509 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=canonical.com 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 47KKD4112sz9sPv for ; Sat, 23 Nov 2019 01:52:52 +1100 (AEDT) Received: from localhost (localhost [127.0.0.1]) by fraxinus.osuosl.org (Postfix) with ESMTP id 5445D869ED; Fri, 22 Nov 2019 14:52:50 +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 hmozmgoXScq6; Fri, 22 Nov 2019 14:52:48 +0000 (UTC) Received: from lists.linuxfoundation.org (lf-lists.osuosl.org [140.211.9.56]) by fraxinus.osuosl.org (Postfix) with ESMTP id 476FD85910; Fri, 22 Nov 2019 14:52:48 +0000 (UTC) Received: from lf-lists.osuosl.org (localhost [127.0.0.1]) by lists.linuxfoundation.org (Postfix) with ESMTP id 3BF8CC1DD8; Fri, 22 Nov 2019 14:52:48 +0000 (UTC) X-Original-To: dev@openvswitch.org Delivered-To: ovs-dev@lists.linuxfoundation.org Received: from silver.osuosl.org (smtp3.osuosl.org [140.211.166.136]) by lists.linuxfoundation.org (Postfix) with ESMTP id E598AC18DA for ; Fri, 22 Nov 2019 14:52:46 +0000 (UTC) Received: from localhost (localhost [127.0.0.1]) by silver.osuosl.org (Postfix) with ESMTP id DEA112625C for ; Fri, 22 Nov 2019 14:52:46 +0000 (UTC) X-Virus-Scanned: amavisd-new at osuosl.org Received: from silver.osuosl.org ([127.0.0.1]) by localhost (.osuosl.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id rqeEPYEL4O4L for ; Fri, 22 Nov 2019 14:52:46 +0000 (UTC) X-Greylist: from auto-whitelisted by SQLgrey-1.7.6 Received: from u.fnordahl.com (u.fnordahl.com [52.21.249.169]) by silver.osuosl.org (Postfix) with ESMTP id 5DAD920357 for ; Fri, 22 Nov 2019 14:52:11 +0000 (UTC) Message-Id: In-Reply-To: References: From: Frode Nordahl To: dev@openvswitch.org Date: Thu, 21 Nov 2019 17:32:59 +0100 Subject: [ovs-dev] [PATCH ovn v2 2/2] northd: Improve handling of pause and resume X-BeenThere: ovs-dev@openvswitch.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , MIME-Version: 1.0 Errors-To: ovs-dev-bounces@openvswitch.org Sender: "dev" Move paused state to ``struct northd_context`` and pass the context to paused and status command handlers. On pause release the OVSDB lock on SB DB. Re-instante the lock on resume. Status command will now provide accurate information for 'paused', 'active' and 'standby' states. Merge separate status command test into the pause and resume tests. Signed-off-by: Frode Nordahl --- northd/ovn-northd.8.xml | 9 +++-- northd/ovn-northd.c | 87 +++++++++++++++++++++++++++-------------- tests/ovn-northd.at | 24 +++++++----- 3 files changed, 79 insertions(+), 41 deletions(-) diff --git a/northd/ovn-northd.8.xml b/northd/ovn-northd.8.xml index 9734e79e6..c6d5d96b9 100644 --- a/northd/ovn-northd.8.xml +++ b/northd/ovn-northd.8.xml @@ -74,13 +74,15 @@
pause
Pauses the ovn-northd operation from processing any Northbound and - Southbound database changes. + Southbound database changes. This will also instruct ovn-northd to + drop any lock on SB DB.
resume
Resumes the ovn-northd operation to process Northbound and - Southbound database contents and generate logical flows. + Southbound database contents and generate logical flows. This will + also instruct ovn-northd to aspire for the lock on SB DB.
is-paused
@@ -91,7 +93,8 @@
status
Prints this server's status. Status will be "active" if ovn-northd has - acquired OVSDB lock on NB DB, "standby" otherwise. + acquired OVSDB lock on SB DB, "standby" if it has not or "paused" if + this instance is paused.

diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c index a943e1037..e19515d14 100644 --- a/northd/ovn-northd.c +++ b/northd/ovn-northd.c @@ -65,6 +65,7 @@ struct northd_context { struct ovsdb_idl_index *sbrec_ha_chassis_grp_by_name; struct ovsdb_idl_index *sbrec_mcast_group_by_name_dp; struct ovsdb_idl_index *sbrec_ip_mcast_by_dp; + bool paused; }; /* An IPv4 or IPv6 address */ @@ -10836,6 +10837,8 @@ add_column_noalert(struct ovsdb_idl *idl, ovsdb_idl_omit_alert(idl, column); } +#define OVN_NORTHD_SB_DB_LOCK_NAME "ovn_northd" + int main(int argc, char *argv[]) { @@ -10843,8 +10846,10 @@ main(int argc, char *argv[]) struct unixctl_server *unixctl; int retval; bool exiting; - bool paused; bool had_lock; + struct northd_context ctx; + + memset(&ctx, 0, sizeof(ctx)); fatal_ignore_sigpipe(); ovs_cmdl_proctitle_init(argc, argv); @@ -10866,11 +10871,11 @@ main(int argc, char *argv[]) exit(EXIT_FAILURE); } unixctl_command_register("exit", "", 0, 0, ovn_northd_exit, &exiting); - unixctl_command_register("pause", "", 0, 0, ovn_northd_pause, &paused); - unixctl_command_register("resume", "", 0, 0, ovn_northd_resume, &paused); + unixctl_command_register("pause", "", 0, 0, ovn_northd_pause, &ctx); + unixctl_command_register("resume", "", 0, 0, ovn_northd_resume, &ctx); unixctl_command_register("is-paused", "", 0, 0, ovn_northd_is_paused, - &paused); - unixctl_command_register("status", "", 0, 0, ovn_northd_status, &had_lock); + &ctx); + unixctl_command_register("status", "", 0, 0, ovn_northd_status, &ctx); daemonize_complete(); @@ -11075,23 +11080,21 @@ main(int argc, char *argv[]) /* Ensure that only a single ovn-northd is active in the deployment by * acquiring a lock called "ovn_northd" on the southbound database * and then only performing DB transactions if the lock is held. */ - ovsdb_idl_set_lock(ovnsb_idl_loop.idl, "ovn_northd"); + ovsdb_idl_set_lock(ovnsb_idl_loop.idl, OVN_NORTHD_SB_DB_LOCK_NAME); /* Main loop. */ exiting = false; - paused = false; + ctx.paused = false; had_lock = false; while (!exiting) { - if (!paused) { - 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), - .sbrec_ha_chassis_grp_by_name = sbrec_ha_chassis_grp_by_name, - .sbrec_mcast_group_by_name_dp = sbrec_mcast_group_by_name_dp, - .sbrec_ip_mcast_by_dp = sbrec_ip_mcast_by_dp, - }; + if (!ctx.paused) { + ctx.ovnnb_idl = ovnnb_idl_loop.idl; + ctx.ovnnb_txn = ovsdb_idl_loop_run(&ovnnb_idl_loop); + ctx.ovnsb_idl = ovnsb_idl_loop.idl; + ctx.ovnsb_txn = ovsdb_idl_loop_run(&ovnsb_idl_loop); + ctx.sbrec_ha_chassis_grp_by_name = sbrec_ha_chassis_grp_by_name; + ctx.sbrec_mcast_group_by_name_dp = sbrec_mcast_group_by_name_dp; + ctx.sbrec_ip_mcast_by_dp = sbrec_ip_mcast_by_dp; if (!had_lock && ovsdb_idl_has_lock(ovnsb_idl_loop.idl)) { VLOG_INFO("ovn-northd lock acquired. " @@ -11125,6 +11128,11 @@ main(int argc, char *argv[]) ovsdb_idl_run(ovnsb_idl_loop.idl); ovsdb_idl_wait(ovnnb_idl_loop.idl); ovsdb_idl_wait(ovnsb_idl_loop.idl); + /* + * the lock is dropped on pause, avoid incorrect message logged + * about lock lost when resumed. + */ + had_lock = false; } unixctl_server_run(unixctl); @@ -11159,30 +11167,41 @@ ovn_northd_exit(struct unixctl_conn *conn, int argc OVS_UNUSED, static void ovn_northd_pause(struct unixctl_conn *conn, int argc OVS_UNUSED, - const char *argv[] OVS_UNUSED, void *pause_) + const char *argv[] OVS_UNUSED, void *ctx_) { - bool *pause = pause_; - *pause = true; + struct northd_context *ctx = ctx_; + + if (!ctx->paused && ctx->ovnsb_idl != NULL) { + /* Drop our aspiration for the lock while paused */ + ovsdb_idl_set_lock(ctx->ovnsb_idl, NULL); + ctx->paused = true; + VLOG_INFO("This ovn-northd instance is now paused."); + } unixctl_command_reply(conn, NULL); } static void ovn_northd_resume(struct unixctl_conn *conn, int argc OVS_UNUSED, - const char *argv[] OVS_UNUSED, void *pause_) + const char *argv[] OVS_UNUSED, void *ctx_) { - bool *pause = pause_; - *pause = false; + struct northd_context *ctx = ctx_; + + if (ctx->paused && ctx->ovnsb_idl != NULL) { + /* Reinstate our aspiration for lock */ + ovsdb_idl_set_lock(ctx->ovnsb_idl, OVN_NORTHD_SB_DB_LOCK_NAME); + ctx->paused = false; + } unixctl_command_reply(conn, NULL); } static void ovn_northd_is_paused(struct unixctl_conn *conn, int argc OVS_UNUSED, - const char *argv[] OVS_UNUSED, void *paused_) + const char *argv[] OVS_UNUSED, void *ctx_) { - bool *paused = paused_; - if (*paused) { + struct northd_context *ctx = ctx_; + if (ctx->paused) { unixctl_command_reply(conn, "true"); } else { unixctl_command_reply(conn, "false"); @@ -11191,15 +11210,25 @@ ovn_northd_is_paused(struct unixctl_conn *conn, int argc OVS_UNUSED, static void ovn_northd_status(struct unixctl_conn *conn, int argc OVS_UNUSED, - const char *argv[] OVS_UNUSED, void *had_lock_) + const char *argv[] OVS_UNUSED, void *ctx_) { - bool *had_lock = had_lock_; + struct northd_context *ctx = ctx_; + char *status; + + if (ctx->paused) { + status = "paused"; + } else if (ctx->ovnsb_idl != NULL) { + status = ovsdb_idl_has_lock(ctx->ovnsb_idl) ? "active" : "standby"; + } else { + status = "unknown"; + } + /* * Use a labelled formatted output so we can add more to the status command * later without breaking any consuming scripts */ struct ds s = DS_EMPTY_INITIALIZER; - ds_put_format(&s, "Status: %s\n", *had_lock ? "active" : "standby"); + ds_put_format(&s, "Status: %s\n", status); unixctl_command_reply(conn, ds_cstr(&s)); ds_destroy(&s); } diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at index 1c94f2f65..d73a22f68 100644 --- a/tests/ovn-northd.at +++ b/tests/ovn-northd.at @@ -899,22 +899,18 @@ OVS_APP_EXIT_AND_WAIT([ovn-northd]) AT_CLEANUP -AT_SETUP([ovn -- ovn-northd status]) -AT_SKIP_IF([test $HAVE_PYTHON = no]) -ovn_start - -AT_CHECK([as northd ovn-appctl -t ovn-northd status], [0], [Status: active -]) - -AT_CLEANUP - AT_SETUP([ovn -- ovn-northd pause and resume]) AT_SKIP_IF([test $HAVE_PYTHON = no]) ovn_start AT_CHECK([test xfalse = x`as northd ovn-appctl -t ovn-northd is-paused`]) +AT_CHECK([as northd ovn-appctl -t ovn-northd status], [0], [Status: active +]) AT_CHECK([test xfalse = x`as northd-backup ovn-appctl -t ovn-northd \ is-paused`]) +AT_CHECK([as northd-backup ovn-appctl -t ovn-northd status], [0], +[Status: standby +]) ovn-nbctl ls-add sw0 @@ -931,7 +927,12 @@ OVS_WAIT_UNTIL([ as northd ovs-appctl -t ovn-northd pause as northd-backup ovs-appctl -t ovn-northd pause AT_CHECK([test xtrue = x`as northd ovn-appctl -t ovn-northd is-paused`]) +AT_CHECK([as northd ovn-appctl -t ovn-northd status], [0], [Status: paused +]) AT_CHECK([test xtrue = x`as northd-backup ovn-appctl -t ovn-northd is-paused`]) +AT_CHECK([as northd-backup ovn-appctl -t ovn-northd status], [0], +[Status: paused +]) ovn-nbctl ls-add sw0 @@ -944,8 +945,13 @@ OVS_WAIT_UNTIL([ as northd ovs-appctl -t ovn-northd resume as northd-backup ovs-appctl -t ovn-northd resume AT_CHECK([test xfalse = x`as northd ovn-appctl -t ovn-northd is-paused`]) +AT_CHECK([as northd ovn-appctl -t ovn-northd status], [0], [Status: active +]) AT_CHECK([test xfalse = x`as northd-backup ovn-appctl -t ovn-northd \ is-paused`]) +AT_CHECK([as northd-backup ovn-appctl -t ovn-northd status], [0], +[Status: standby +]) OVS_WAIT_UNTIL([ ovn-sbctl lflow-list sw0