Acked-by: Mark Michelson <mmich...@redhat.com>

On 08/03/2018 01:54 PM, Ben Pfaff wrote:
This makes ovn-nbctl transparently use daemon mode if an appropriate
environment variable is set.

It also transforms ovn-nbctl.at so that it runs each ovn-nbctl test in
"direct" mode and in daemon mode.  It uses a combination of m4 macros and
shell functions to keep from expanding the generated testsuite more than
necessary.

Signed-off-by: Ben Pfaff <b...@ovn.org>
---
  NEWS                          |   6 +-
  lib/daemon.h                  |  19 +++
  lib/stream-ssl.h              |   5 +-
  ovn/utilities/ovn-nbctl.8.xml |  61 +++++++--
  ovn/utilities/ovn-nbctl.c     | 292 +++++++++++++++++++++++++++++++++++-------
  tests/ovn-nbctl.at            | 246 +++++++++++++----------------------
  6 files changed, 412 insertions(+), 217 deletions(-)

diff --git a/NEWS b/NEWS
index 27ef12d599d9..7875f6673e34 100644
--- a/NEWS
+++ b/NEWS
@@ -35,10 +35,8 @@ v2.10.0 - xx xxx xxxx
       * ACL match conditions can now match on Port_Groups as well as address
         sets that are automatically generated by Port_Groups.  ACLs can be
         applied directly to Port_Groups as well.
-     * ovn-nbctl can now run as a daemon (long-lived, background process)
-       running commands in response to JSON-RPC requests received over a UNIX
-       socket. Requests to run commands can be sent using ovs-appctl tool, same
-       as for any other OVS/OVN daemon. See ovn-nbctl(8) for details.
+     * ovn-nbctl can now run as a daemon (long-lived, background process).
+       See ovn-nbctl(8) for details.
     - DPDK:
       * New 'check-dpdk' Makefile target to run a new system testsuite.
         See Testing topic for the details.
diff --git a/lib/daemon.h b/lib/daemon.h
index bfa06408c390..f33e9df8df7a 100644
--- a/lib/daemon.h
+++ b/lib/daemon.h
@@ -84,6 +84,15 @@
              daemon_set_new_user(optarg);        \
              break;
+#define DAEMON_OPTION_CASES \
+        case OPT_DETACH:                        \
+        case OPT_NO_SELF_CONFINEMENT:           \
+        case OPT_NO_CHDIR:                      \
+        case OPT_PIDFILE:                       \
+        case OPT_OVERWRITE_PIDFILE:             \
+        case OPT_MONITOR:                       \
+        case OPT_USER_GROUP:
+
  void set_detach(void);
  void daemon_set_monitor(void);
  void set_no_chdir(void);
@@ -138,6 +147,16 @@ pid_t read_pidfile(const char *name);
          case OPT_USER_GROUP:                    \
              daemon_set_new_user(optarg);
+#define DAEMON_OPTION_CASES \
+        case OPT_DETACH:                        \
+        case OPT_NO_SELF_CONFINEMENT:           \
+        case OPT_NO_CHDIR:                      \
+        case OPT_PIDFILE:                       \
+        case OPT_PIPE_HANDLE:                   \
+        case OPT_SERVICE:                       \
+        case OPT_SERVICE_MONITOR:               \
+        case OPT_USER_GROUP:
+
  void control_handler(DWORD request);
  void set_pipe_handle(const char *pipe_handle);
  #endif /* _WIN32 */
diff --git a/lib/stream-ssl.h b/lib/stream-ssl.h
index 4bfe09b002c9..937f7c653ba9 100644
--- a/lib/stream-ssl.h
+++ b/lib/stream-ssl.h
@@ -58,6 +58,9 @@ void stream_ssl_set_ciphers(const char *arg);
                                                          \
          case OPT_SSL_CIPHERS:                           \
              stream_ssl_set_ciphers(optarg);             \
-            break;
+            break;
+
+#define STREAM_SSL_CASES \
+    case 'p': case 'c': case 'C': case OPT_SSL_PROTOCOLS: case OPT_SSL_CIPHERS:
#endif /* stream-ssl.h */
diff --git a/ovn/utilities/ovn-nbctl.8.xml b/ovn/utilities/ovn-nbctl.8.xml
index f09ef8767d2f..5e18fa7c06ee 100644
--- a/ovn/utilities/ovn-nbctl.8.xml
+++ b/ovn/utilities/ovn-nbctl.8.xml
@@ -971,15 +971,62 @@
      <h1>Daemon Mode</h1>
<p>
-      If <code>ovn-nbctl</code> is invoked with the <code>--detach</code>
-      option (see <code>Daemon Options</code>, below), it runs in the
-      background as a daemon and accepts commands from <code>ovs-appctl</code>
-      (or another JSON-RPC client) indefinitely.  The currently supported
-      commands are described below.
+      When it is invoked in the most ordinary way, <code>ovn-nbctl</code>
+      connects to an OVSDB server that hosts the northbound database, retrieves
+      a partial copy of the database that is complete enough to do its work,
+      sends a transaction request to the server, and receives and processes the
+      server's reply.  In common interactive use, this is fine, but if the
+      database is large, the step in which <code>ovn-nbctl</code> retrieves a
+      partial copy of the database can take a long time, which yields poor
+      performance overall.
      </p>
<p>
+      To improve performance in such a case, <code>ovn-nbctl</code> offers a
+      "daemon mode," in which the user first starts <code>ovn-nbctl</code>
+      running in the background and afterward uses the daemon to execute
+      operations.  Over several <code>ovn-nbctl</code> command invocations,
+      this performs better overall because it retrieves a copy of the database
+      only once at the beginning, not once per program run.
+    </p>
+
+    <p>
+      Use the <code>--detach</code> option to start an <code>ovn-nbctl</code>
+      daemon.  With this option, <code>ovn-nbctl</code> prints the name of a
+      control socket to stdout.  The client should save this name in
+      environment variable <env>OVN_NB_DAEMON</env>.  Under the Bourne shell
+      this might be done like this:
+    </p>
+
+    <pre fixed="yes">
+      export OVN_NB_DAEMON=$(ovn-nbctl --pidfile --detach)
+    </pre>
+
+    <p>
+      When <env>OVN_NB_DAEMON</env> is set, <code>ovn-nbctl</code>
+      automatically and transparently uses the daemon to execute its commands.
+    </p>
+
+    <p>
+      When the daemon is no longer needed, kill it and unset the environment
+      variable, e.g.:
+    </p>
+
+    <pre fixed="yes">
+      kill $(cat /var/run/ovn-nbctl.pid)
+      unset OVN_NB_DAEMON
+    </pre>
+
+    <p>
+      Daemon mode is experimental.
+    </p>
+ <h2>Daemon Commands</h2>
+
+    <p>
+      Daemon mode is internally implemented using the same mechanism used by
+      <code>ovs-appctl</code>.  One may also use <code>ovs-appctl</code>
+      directly with the following commands:
      </p>
<dl>
@@ -1001,10 +1048,6 @@
        <dd>Causes <code>ovn-nbctl</code> to gracefully terminate.</dd>
      </dl>
- <p>
-      Daemon mode is considered experimental.
-    </p>
-
      <h1>Options</h1>
<dl>
diff --git a/ovn/utilities/ovn-nbctl.c b/ovn/utilities/ovn-nbctl.c
index 0659ced378ef..35a3af3d5ff3 100644
--- a/ovn/utilities/ovn-nbctl.c
+++ b/ovn/utilities/ovn-nbctl.c
@@ -24,6 +24,7 @@
  #include "db-ctl-base.h"
  #include "dirs.h"
  #include "fatal-signal.h"
+#include "jsonrpc.h"
  #include "openvswitch/json.h"
  #include "ovn/lib/acl-log.h"
  #include "ovn/lib/ovn-nb-idl.h"
@@ -91,7 +92,15 @@ static unixctl_cb_func server_cmd_run;
static void nbctl_cmd_init(void);
  OVS_NO_RETURN static void usage(void);
-static void parse_options(int argc, char *argv[], struct shash *local_options);
+static struct option *get_all_options(void);
+static bool has_option(const struct ovs_cmdl_parsed_option *, size_t n,
+                       int option);
+static void nbctl_client(const char *socket_name,
+                         const struct ovs_cmdl_parsed_option *, size_t n,
+                         int argc, char *argv[]);
+static bool will_detach(const struct ovs_cmdl_parsed_option *, size_t n);
+static void apply_options_direct(const struct ovs_cmdl_parsed_option *,
+                                 size_t n, struct shash *local_options);
  static char * OVS_WARN_UNUSED_RESULT run_prerequisites(struct ctl_command[],
                                                         size_t n_commands,
                                                         struct ovsdb_idl *);
@@ -123,10 +132,47 @@ main(int argc, char *argv[])
nbctl_cmd_init(); - /* Parse command line. */
+    /* ovn-nbctl has three operation modes:
+     *
+     *    - Direct: Executes commands by contacting ovsdb-server directly.
+     *
+     *    - Server: Runs in the background as a daemon waiting for requests
+     *      from ovn-nbctl running in client mode.
+     *
+     *    - Client: Executes commands by passing them to an ovn-nbctl running
+     *      in the server mode.
+     *
+     * At this point we don't know what mode we're running in.  The mode partly
+     * depends on the command line.  So, for now we transform the command line
+     * into a parsed form, and figure out what to do with it later.
+     */
      char *args = process_escape_args(argv);
+    struct ovs_cmdl_parsed_option *parsed_options;
+    size_t n_parsed_options;
+    char *error_s = ovs_cmdl_parse_all(argc, argv, get_all_options(),
+                                       &parsed_options, &n_parsed_options);
+    if (error_s) {
+        ctl_fatal("%s", error_s);
+    }
+
+    /* Now figure out the operation mode:
+     *
+     *    - A --detach option implies server mode.
+     *
+     *    - An OVN_NB_DAEMON environment variable implies client mode.
+     *
+     *    - Otherwise, we're in direct mode. */
+    char *socket_name = getenv("OVN_NB_DAEMON");
+    if (socket_name && socket_name[0]
+        && !will_detach(parsed_options, n_parsed_options)) {
+        nbctl_client(socket_name, parsed_options, n_parsed_options,
+                     argc, argv);
+    }
+
+    /* Parse command line. */
      shash_init(&local_options);
-    parse_options(argc, argv, &local_options);
+    apply_options_direct(parsed_options, n_parsed_options, &local_options);
+    free(parsed_options);
      argc -= optind;
      argv += optind;
@@ -256,6 +302,8 @@ enum {
      OPT_LOCAL,
      OPT_COMMANDS,
      OPT_OPTIONS,
+    OPT_LEADER_ONLY,
+    OPT_NO_LEADER_ONLY,
      OPT_BOOTSTRAP_CA_CERT,
      MAIN_LOOP_OPTION_ENUMS,
      DAEMON_OPTION_ENUMS,
@@ -347,8 +395,8 @@ append_command_options(const struct option *options, int 
opt_val)
      return o;
  }
-static void
-parse_options(int argc, char *argv[], struct shash *local_options)
+static struct option *
+get_all_options(void)
  {
      static const struct option global_long_options[] = {
          {"db", required_argument, NULL, OPT_DB},
@@ -356,8 +404,8 @@ parse_options(int argc, char *argv[], struct shash 
*local_options)
          {"help", no_argument, NULL, 'h'},
          {"commands", no_argument, NULL, OPT_COMMANDS},
          {"options", no_argument, NULL, OPT_OPTIONS},
-        {"leader-only", no_argument, &leader_only, true},
-        {"no-leader-only", no_argument, &leader_only, false},
+        {"leader-only", no_argument, NULL, OPT_LEADER_ONLY},
+        {"no-leader-only", no_argument, NULL, OPT_NO_LEADER_ONLY},
          {"version", no_argument, NULL, 'V'},
          MAIN_LOOP_LONG_OPTIONS,
          DAEMON_LONG_OPTIONS,
@@ -367,24 +415,57 @@ parse_options(int argc, char *argv[], struct shash 
*local_options)
          TABLE_LONG_OPTIONS,
          {NULL, 0, NULL, 0},
      };
-    const int n_global_long_options = ARRAY_SIZE(global_long_options) - 1;
-    struct option *options;
-    size_t i;
- options = append_command_options(global_long_options, OPT_LOCAL);
+    static struct option *options;
+    if (!options) {
+        options = append_command_options(global_long_options, OPT_LOCAL);
+    }
- struct ovs_cmdl_parsed_option *parsed_options;
-    size_t n_po;
-    char *error = ovs_cmdl_parse_all(argc, argv, options,
-                                     &parsed_options, &n_po);
-    if (error) {
-        ctl_fatal("%s", error);
+    return options;
+}
+
+static bool
+has_option(const struct ovs_cmdl_parsed_option *parsed_options, size_t n,
+           int option)
+{
+    for (const struct ovs_cmdl_parsed_option *po = parsed_options;
+         po < &parsed_options[n]; po++) {
+        if (po->o->val == option) {
+            return true;
+        }
+    }
+    return false;
+}
+
+static bool
+will_detach(const struct ovs_cmdl_parsed_option *parsed_options, size_t n)
+{
+    return has_option(parsed_options, n, OPT_DETACH);
+}
+
+static char * OVS_WARN_UNUSED_RESULT
+add_local_option(const char *name, const char *arg,
+                 struct shash *local_options)
+{
+    char *full_name = xasprintf("--%s", name);
+    if (shash_find(local_options, full_name)) {
+        char *error = xasprintf("'%s' option specified multiple times",
+                                full_name);
+        free(full_name);
+        return error;
      }
+    shash_add_nocopy(local_options, full_name, nullable_xstrdup(arg));
+    return NULL;
+}
+static void
+apply_options_direct(const struct ovs_cmdl_parsed_option *parsed_options,
+                     size_t n, struct shash *local_options)
+{
      for (const struct ovs_cmdl_parsed_option *po = parsed_options;
-         po < &parsed_options[n_po]; po++) {
+         po < &parsed_options[n]; po++) {
          bool handled;
-        error = handle_main_loop_option(po->o->val, po->arg, &handled);
+        char *error = handle_main_loop_option(po->o->val, po->arg, &handled);
          if (error) {
              ctl_fatal("%s", error);
          }
@@ -403,13 +484,10 @@ parse_options(int argc, char *argv[], struct shash 
*local_options)
              break;
case OPT_LOCAL:
-            if (shash_find(local_options, po->o->name)) {
-                ctl_fatal("'%s' option specified multiple times",
-                            po->o->name);
+            error = add_local_option(po->o->name, po->arg, local_options);
+            if (error) {
+                ctl_fatal("%s", error);
              }
-            shash_add_nocopy(local_options,
-                             xasprintf("--%s", po->o->name),
-                             nullable_xstrdup(po->arg));
              break;
case 'h':
@@ -421,9 +499,17 @@ parse_options(int argc, char *argv[], struct shash 
*local_options)
              /* fall through */
case OPT_OPTIONS:
-            ctl_print_options(global_long_options);
+            ctl_print_options(get_all_options());
              /* fall through */
+ case OPT_LEADER_ONLY:
+            leader_only = true;
+            break;
+
+        case OPT_NO_LEADER_ONLY:
+            leader_only = false;
+            break;
+
          case 'V':
              ovs_print_version(0, 0);
              printf("DB Schema %s\n", nbrec_get_db_version());
@@ -448,16 +534,10 @@ parse_options(int argc, char *argv[], struct shash 
*local_options)
              break;
          }
      }
-    free(parsed_options);
if (!db) {
          db = default_nb_db();
      }
-
-    for (i = n_global_long_options; options[i].name; i++) {
-        free(CONST_CAST(char *, options[i].name));
-    }
-    free(options);
  }
static void
@@ -641,6 +721,10 @@ Other options:\n\
      exit(EXIT_SUCCESS);
  }
  
+/* One should not use ctl_fatal() within commands because it will kill the
+ * daemon if we're in daemon mode.  Use ctl_error() instead and return
+ * gracefully.  */
+#define ctl_fatal dont_use_ctl_fatal_use_ctl_error_and_return
/* Find a logical router given its id. */
  static char * OVS_WARN_UNUSED_RESULT
@@ -2368,35 +2452,41 @@ nbctl_meter_add(struct ctl_context *ctx)
      const char *name = ctx->argv[1];
      NBREC_METER_FOR_EACH (meter, ctx->idl) {
          if (!strcmp(meter->name, name)) {
-            ctl_fatal("meter with name \"%s\" already exists", name);
+            ctl_error(ctx, "meter with name \"%s\" already exists", name);
+            return;
          }
      }
if (!strncmp(name, "__", 2)) {
-        ctl_fatal("meter names that begin with \"__\" are reserved");
+        ctl_error(ctx, "meter names that begin with \"__\" are reserved");
+        return;
      }
const char *action = ctx->argv[2];
      if (strcmp(action, "drop")) {
-        ctl_fatal("action must be \"drop\"");
+        ctl_error(ctx, "action must be \"drop\"");
+        return;
      }
int64_t rate;
      if (!ovs_scan(ctx->argv[3], "%"SCNd64, &rate)
          || rate < 1 || rate > UINT32_MAX) {
-        ctl_fatal("rate must be in the range 1...4294967295");
+        ctl_error(ctx, "rate must be in the range 1...4294967295");
+        return;
      }
const char *unit = ctx->argv[4];
      if (strcmp(unit, "kbps") && strcmp(unit, "pktps")) {
-        ctl_fatal("unit must be \"kbps\" or \"pktps\"");
+        ctl_error(ctx, "unit must be \"kbps\" or \"pktps\"");
+        return;
      }
int64_t burst = 0;
      if (ctx->argc > 5) {
          if (!ovs_scan(ctx->argv[5], "%"SCNd64, &burst)
              || burst < 0 || burst > UINT32_MAX) {
-            ctl_fatal("burst must be in the range 0...4294967295");
+            ctl_error(ctx, "burst must be in the range 0...4294967295");
+            return;
          }
      }
@@ -4965,6 +5055,10 @@ nbctl_cmd_init(void)
      ctl_init(&nbrec_idl_class, nbrec_table_classes, tables, NULL, nbctl_exit);
      ctl_register_commands(nbctl_commands);
  }
+
+/* Server implementation. */
+
+#undef ctl_fatal
static const struct option *
  find_option_by_value(const struct option *options, int value)
@@ -5021,14 +5115,10 @@ server_parse_options(int argc, char *argv[], struct 
shash *local_options,
switch (c) {
          case OPT_LOCAL:
-            if (shash_find(local_options, options[idx].name)) {
-                error = xasprintf("'%s' option specified multiple times",
-                                  options[idx].name);
+            error = add_local_option(options[idx].name, optarg, local_options);
+            if (error) {
                  goto out;
              }
-            shash_add_nocopy(local_options,
-                             xasprintf("--%s", options[idx].name),
-                             nullable_xstrdup(optarg));
              break;
VLOG_OPTION_HANDLERS
@@ -5177,7 +5267,7 @@ static void
  server_cmd_init(struct ovsdb_idl *idl, bool *exiting)
  {
      unixctl_command_register("exit", "", 0, 0, server_cmd_exit, exiting);
-    unixctl_command_register("run", "", 1, INT_MAX, server_cmd_run, idl);
+    unixctl_command_register("run", "", 0, INT_MAX, server_cmd_run, idl);
  }
static void
@@ -5192,6 +5282,8 @@ server_loop(struct ovsdb_idl *idl)
          ctl_fatal("failed to create unixctl server (%s)",
                    ovs_retval_to_string(error));
      }
+    puts(unixctl_server_get_path(server));
+    fflush(stdout);
      server_cmd_init(idl, &exiting);
for (;;) {
@@ -5217,3 +5309,113 @@ server_loop(struct ovsdb_idl *idl)
unixctl_server_destroy(server);
  }
+
+static void
+nbctl_client(const char *socket_name,
+             const struct ovs_cmdl_parsed_option *parsed_options, size_t n,
+             int argc, char *argv[])
+{
+    struct svec args = SVEC_EMPTY_INITIALIZER;
+
+    for (const struct ovs_cmdl_parsed_option *po = parsed_options;
+         po < &parsed_options[n]; po++) {
+        optarg = po->arg;
+        switch (po->o->val) {
+        case OPT_DB:
+            VLOG_WARN("not using ovn-nbctl daemon because of %s option",
+                      po->o->name);
+            svec_destroy(&args);
+            return;
+
+        case OPT_NO_SYSLOG:
+            vlog_set_levels(&this_module, VLF_SYSLOG, VLL_WARN);
+            break;
+
+        case 'h':
+            usage();
+            exit(EXIT_SUCCESS);
+
+        case OPT_COMMANDS:
+            ctl_print_commands();
+            /* fall through */
+
+        case OPT_OPTIONS:
+            ctl_print_options(get_all_options());
+            /* fall through */
+
+        case OPT_LEADER_ONLY:
+        case OPT_NO_LEADER_ONLY:
+        case OPT_BOOTSTRAP_CA_CERT:
+        STREAM_SSL_CASES
+        DAEMON_OPTION_CASES
+            VLOG_INFO("using ovn-nbctl daemon, ignoring %s option",
+                      po->o->name);
+            break;
+
+        case 'V':
+            ovs_print_version(0, 0);
+            printf("DB Schema %s\n", nbrec_get_db_version());
+            exit(EXIT_SUCCESS);
+
+        case 't':
+            timeout = strtoul(po->arg, NULL, 10);
+            if (timeout < 0) {
+                ctl_fatal("value %s on -t or --timeout is invalid", po->arg);
+            }
+            break;
+
+        VLOG_OPTION_HANDLERS
+        TABLE_OPTION_HANDLERS(&table_style)
+
+        case OPT_LOCAL:
+        default:
+            if (po->arg) {
+                svec_add_nocopy(&args,
+                                xasprintf("--%s=%s", po->o->name, po->arg));
+            } else {
+                svec_add_nocopy(&args, xasprintf("--%s", po->o->name));
+            }
+            break;
+        }
+    }
+    svec_add(&args, "--");
+    for (int i = optind; i < argc; i++) {
+        svec_add(&args, argv[i]);
+    }
+
+    if (timeout) {
+        time_alarm(timeout);
+    }
+
+    struct jsonrpc *client;
+    int error = unixctl_client_create(socket_name, &client);
+    if (error) {
+        ctl_fatal("%s: could not connect to ovn-nb daemon (%s); "
+                  "unset OVN_NB_DAEMON to avoid using daemon",
+                  socket_name, ovs_strerror(error));
+    }
+
+    char *cmd_result;
+    char *cmd_error;
+    error = unixctl_client_transact(client, "run",
+                                    args.n, args.names,
+                                    &cmd_result, &cmd_error);
+    if (error) {
+        ctl_fatal("%s: transaction error (%s)",
+                  socket_name, ovs_strerror(error));
+    }
+    svec_destroy(&args);
+
+    int exit_status;
+    if (cmd_error) {
+        exit_status = EXIT_FAILURE;
+        fprintf(stderr, "%s: %s", program_name, cmd_error);
+    } else {
+        exit_status = EXIT_SUCCESS;
+        fputs(cmd_result, stdout);
+    }
+    free(cmd_result);
+    free(cmd_error);
+    jsonrpc_close(client);
+    exit(exit_status);
+}
diff --git a/tests/ovn-nbctl.at b/tests/ovn-nbctl.at
index 88e37191ee0a..725665b76d3f 100644
--- a/tests/ovn-nbctl.at
+++ b/tests/ovn-nbctl.at
@@ -1,30 +1,59 @@
  AT_BANNER([ovn-nbctl])
+OVS_START_SHELL_HELPERS
  # OVN_NBCTL_TEST_START
  m4_define([OVN_NBCTL_TEST_START],
-  [dnl Create ovn-nb database.
-   AT_KEYWORDS([ovn])
+  [AT_KEYWORDS([ovn])
+   AT_CAPTURE_FILE([ovsdb-server.log])
+   ovn_nbctl_test_start $1])
+ovn_nbctl_test_start() {
+   dnl Create ovn-nb database.
     AT_CHECK([ovsdb-tool create ovn-nb.db 
$abs_top_srcdir/ovn/ovn-nb.ovsschema])
dnl Start ovsdb-server.
     AT_CHECK([ovsdb-server --detach --no-chdir --pidfile --log-file 
--remote=punix:$OVS_RUNDIR/ovnnb_db.sock ovn-nb.db], [0], [], [stderr])
     on_exit "kill `cat ovsdb-server.pid`"
+   AS_CASE([$1],
+     [daemon],
+       [export OVN_NB_DAEMON=$(ovn-nbctl --pidfile --detach --log-file 
-vsocket_util:off)
+        on_exit "kill `cat ovn-nbctl.pid`"],
+     [direct], [],
+     [*], [AT_FAIL_IF(:)])
     AT_CHECK([ovn-nbctl init])
     AT_CHECK([[sed < stderr '
  /vlog|INFO|opened log file/d
  /ovsdb_server|INFO|ovsdb-server (Open vSwitch)/d']])
-   AT_CAPTURE_FILE([ovsdb-server.log])
-])
+}
# OVN_NBCTL_TEST_STOP
-m4_define([OVN_NBCTL_TEST_STOP],
-  [AT_CHECK([check_logs "$1"])
-   OVS_APP_EXIT_AND_WAIT([ovsdb-server])])
-
-
-AT_SETUP([ovn-nbctl - basic switch commands])
-OVN_NBCTL_TEST_START
-
+m4_define([OVN_NBCTL_TEST_STOP], [ovn_nbctl_test_stop])
+ovn_nbctl_test_stop() {
+   AT_CHECK([check_logs "$1"])
+   OVS_APP_EXIT_AND_WAIT([ovsdb-server])
+}
+OVS_END_SHELL_HELPERS
+
+# OVN_NBCTL_TEST(NAME, TITLE, COMMANDS)
+m4_define([OVN_NBCTL_TEST],
+   [OVS_START_SHELL_HELPERS
+    $1() {
+      $3
+    }
+    OVS_END_SHELL_HELPERS
+
+    AT_SETUP([ovn-nbctl - $2 - direct])
+    OVN_NBCTL_TEST_START direct
+    $1
+    OVN_NBCTL_TEST_STOP
+    AT_CLEANUP
+
+    AT_SETUP([ovn-nbctl - $2 - daemon])
+    OVN_NBCTL_TEST_START daemon
+    $1
+    OVN_NBCTL_TEST_STOP
+    AT_CLEANUP])
+
+OVN_NBCTL_TEST([ovn_nbctl_basic_switch], [basic switch commands], [
  AT_CHECK([ovn-nbctl ls-add ls0])
  AT_CHECK([ovn-nbctl ls-list | uuidfilt], [0], [dnl
  <0> (ls0)
@@ -73,16 +102,11 @@ AT_CHECK([ovn-nbctl --add-duplicate ls-add], [1], [],
  ])
  AT_CHECK([ovn-nbctl --may-exist ls-add], [1], [],
    [ovn-nbctl: --may-exist requires specifying a name
-])
-
-OVN_NBCTL_TEST_STOP
-AT_CLEANUP
+])])
dnl --------------------------------------------------------------------- -AT_SETUP([ovn-nbctl - basic logical switch port commands])
-OVN_NBCTL_TEST_START
-
+OVN_NBCTL_TEST([ovn_nbctl_basic_lsp], [basic logical switch port commands], [
  AT_CHECK([ovn-nbctl ls-add ls0])
  AT_CHECK([ovn-nbctl lsp-add ls0 lp0])
  AT_CHECK([ovn-nbctl lsp-add ls0 lp0], [1], [],
@@ -125,16 +149,11 @@ AT_CHECK([ovn-nbctl --may-exist lsp-add ls0 lp2 lp3 10], 
[1], [],
  AT_CHECK([ovn-nbctl clear Logical_Switch_Port lp2 tag_request])
  AT_CHECK([ovn-nbctl --may-exist lsp-add ls0 lp2 lp3 5], [1], [],
    [ovn-nbctl: lp2: port already exists but has no tag_request
-])
-
-OVN_NBCTL_TEST_STOP
-AT_CLEANUP
+])])
dnl --------------------------------------------------------------------- -AT_SETUP([ovn-nbctl - lport addresses])
-OVN_NBCTL_TEST_START
-
+OVN_NBCTL_TEST([ovn_nbctl_lport_addresses], [lport addresses], [
  AT_CHECK([ovn-nbctl ls-add ls0])
  AT_CHECK([ovn-nbctl lsp-add ls0 lp0])
  AT_CHECK([ovn-nbctl lsp-get-addresses lp0], [0], [dnl
@@ -148,16 +167,11 @@ unknown
AT_CHECK([ovn-nbctl lsp-set-addresses lp0])
  AT_CHECK([ovn-nbctl lsp-get-addresses lp0], [0], [dnl
-])
-
-OVN_NBCTL_TEST_STOP
-AT_CLEANUP
+])])
dnl --------------------------------------------------------------------- -AT_SETUP([ovn-nbctl - port security])
-OVN_NBCTL_TEST_START
-
+OVN_NBCTL_TEST([ovn_nbctl_port_security], [port security], [
  AT_CHECK([ovn-nbctl ls-add ls0])
  AT_CHECK([ovn-nbctl lsp-add ls0 lp0])
  AT_CHECK([ovn-nbctl lsp-get-addresses lp0], [0], [dnl
@@ -171,15 +185,13 @@ aa:bb:cc:dd:ee:ff
AT_CHECK([ovn-nbctl lsp-set-port-security lp0])
  AT_CHECK([ovn-nbctl lsp-get-port-security lp0], [0], [dnl
-])
-
-OVN_NBCTL_TEST_STOP
-AT_CLEANUP
+])])
dnl --------------------------------------------------------------------- -m4_define([OVN_NBCTL_TEST_ACL],
-  [AT_CHECK([ovn-nbctl $2 --log acl-add $1 from-lport 600 udp drop])
+OVN_NBCTL_TEST([ovn_nbctl_acls], [ACLs], [
+ovn_nbctl_test_acl() {
+   AT_CHECK([ovn-nbctl $2 --log acl-add $1 from-lport 600 udp drop])
     AT_CHECK([ovn-nbctl $2 --log --name=test --severity=info acl-add $1 
to-lport 500 udp drop])
     AT_CHECK([ovn-nbctl $2 acl-add $1 from-lport 400 tcp drop])
     AT_CHECK([ovn-nbctl $2 acl-add $1 to-lport 300 tcp drop])
@@ -222,17 +234,14 @@ from-lport   200 (ip) drop
  from-lport   600 (udp) drop
  from-lport   200 (ip) drop
  ])
-])
-
-AT_SETUP([ovn-nbctl - ACLs])
-OVN_NBCTL_TEST_START
+}
AT_CHECK([ovn-nbctl ls-add ls0])
-OVN_NBCTL_TEST_ACL([ls0])
+ovn_nbctl_test_acl ls0
  AT_CHECK([ovn-nbctl ls-add ls1])
-OVN_NBCTL_TEST_ACL([ls1], [--type=switch])
+ovn_nbctl_test_acl ls1 --type=switch
  AT_CHECK([ovn-nbctl create port_group name=pg0], [0], [ignore])
-OVN_NBCTL_TEST_ACL([pg0], [--type=port-group])
+ovn_nbctl_test_acl pg0 --type=port-group
dnl Test when port group doesn't exist
  AT_CHECK([ovn-nbctl --type=port-group acl-add pg1 to-lport 100 ip drop], [1], 
[], [dnl
@@ -243,16 +252,11 @@ dnl Test when same name exists in logical switches and 
portgroups
  AT_CHECK([ovn-nbctl create port_group name=ls0], [0], [ignore])
  AT_CHECK([ovn-nbctl acl-add ls0 to-lport 100 ip drop], [1], [], [stderr])
  AT_CHECK([grep 'exists in both' stderr], [0], [ignore])
-AT_CHECK([ovn-nbctl --type=port-group acl-add ls0 to-lport 100 ip drop], [0], 
[ignore])
-
-OVN_NBCTL_TEST_STOP
-AT_CLEANUP
+AT_CHECK([ovn-nbctl --type=port-group acl-add ls0 to-lport 100 ip drop], [0], 
[ignore])])
dnl --------------------------------------------------------------------- -AT_SETUP([ovn-nbctl - QoS])
-OVN_NBCTL_TEST_START
-
+OVN_NBCTL_TEST([ovn_nbctl_qos], [QoS], [
  AT_CHECK([ovn-nbctl ls-add ls0])
  AT_CHECK([ovn-nbctl qos-add ls0 from-lport 600 tcp dscp=63])
  AT_CHECK([ovn-nbctl qos-add ls0 from-lport 500 udp rate=100 burst=1000])
@@ -317,16 +321,11 @@ AT_CHECK([ovn-nbctl qos-add ls0 from-lport 600 ip 
dscpa=-1], [1], [],
AT_CHECK([ovn-nbctl qos-add ls0 from-lport 600 ip burst=123], [1], [],
  [ovn-nbctl: Either "rate" and/or "dscp" must be specified
-])
-
-OVN_NBCTL_TEST_STOP
-AT_CLEANUP
+])])
dnl --------------------------------------------------------------------- -AT_SETUP([ovn-nbctl - Meters])
-OVN_NBCTL_TEST_START
-
+OVN_NBCTL_TEST([ovn_nbctl_meters], [meters], [
  AT_CHECK([ovn-nbctl meter-add meter1 drop 10 kbps])
  AT_CHECK([ovn-nbctl meter-add meter2 drop 3 kbps 2])
  AT_CHECK([ovn-nbctl meter-add meter3 drop 100 kbps 200])
@@ -384,15 +383,11 @@ meter4: bands:
  dnl Delete all meters.
  AT_CHECK([ovn-nbctl meter-del])
  AT_CHECK([ovn-nbctl meter-list], [0], [dnl
-])
-
-OVN_NBCTL_TEST_STOP
-AT_CLEANUP
+])])
dnl --------------------------------------------------------------------- -AT_SETUP([ovn-nbctl - NATs])
-OVN_NBCTL_TEST_START
+OVN_NBCTL_TEST([ovn_nbctl_nats], [NATs], [
  AT_CHECK([ovn-nbctl lr-add lr0])
  AT_CHECK([ovn-nbctl lr-nat-add lr0 snatt 30.0.0.2 192.168.1.2], [1], [],
  [ovn-nbctl: snatt: type must be one of "dnat", "snat" and "dnat_and_snat".
@@ -537,15 +532,11 @@ snat             30.0.0.1           192.168.1.0/24
  AT_CHECK([ovn-nbctl lr-nat-del lr0])
  AT_CHECK([ovn-nbctl lr-nat-list lr0], [0], [])
  AT_CHECK([ovn-nbctl lr-nat-del lr0])
-AT_CHECK([ovn-nbctl lr-nat-del lr0 dnat])
-OVN_NBCTL_TEST_STOP
-AT_CLEANUP
+AT_CHECK([ovn-nbctl lr-nat-del lr0 dnat])])
dnl --------------------------------------------------------------------- -AT_SETUP([ovn-nbctl - LBs])
-OVN_NBCTL_TEST_START
-
+OVN_NBCTL_TEST([ovn_nbctl_lbs], [LBs], [
  dnl Add two LBs.
  AT_CHECK([ovn-nbctl -vsocket_util:off lb-add lb0 30.0.0.10:80a 
192.168.10.10:80,192.168.10.20:80 tcp], [1], [],
  [ovn-nbctl: 30.0.0.10:80a: should be an IP address (or an IP address and a 
port number with : as a separator).
@@ -761,16 +752,11 @@ AT_CHECK([ovn-nbctl lr-lb-add lr0 lb0])
  AT_CHECK([ovn-nbctl lr-lb-add lr0 lb1])
  AT_CHECK([ovn-nbctl lr-lb-add lr0 lb3])
  AT_CHECK([ovn-nbctl lr-lb-del lr0])
-AT_CHECK([ovn-nbctl lr-lb-list lr0 | uuidfilt], [0], [])
+AT_CHECK([ovn-nbctl lr-lb-list lr0 | uuidfilt], [0], [])])
-OVN_NBCTL_TEST_STOP
-AT_CLEANUP
-dnl
  dnl ---------------------------------------------------------------------
-AT_SETUP([ovn-nbctl - LBs IPv6])
-OVN_NBCTL_TEST_START
-
+OVN_NBCTL_TEST([ovn_nbctl_lbs_ipv6], [LBs IPv6], [
  dnl A bunch of commands that should fail
  AT_CHECK([ovn-nbctl -vsocket_util:off lb-add lb0 [[ae0f::10]]:80a 
[[fd0f::10]]:80,[[fd0f::20]]:80 tcp], [1], [],
  [ovn-nbctl: [[ae0f::10]]:80a: should be an IP address (or an IP address and a 
port number with : as a separator).
@@ -1025,15 +1011,11 @@ AT_CHECK([ovn-nbctl lr-lb-add lr0 lb0])
  AT_CHECK([ovn-nbctl lr-lb-add lr0 lb1])
  AT_CHECK([ovn-nbctl lr-lb-add lr0 lb3])
  AT_CHECK([ovn-nbctl lr-lb-del lr0])
-AT_CHECK([ovn-nbctl lr-lb-list lr0 | uuidfilt], [0], [])
+AT_CHECK([ovn-nbctl lr-lb-list lr0 | uuidfilt], [0], [])])
-OVN_NBCTL_TEST_STOP
-AT_CLEANUP
  dnl ---------------------------------------------------------------------
-AT_SETUP([ovn-nbctl - basic logical router commands])
-OVN_NBCTL_TEST_START
-
+OVN_NBCTL_TEST([ovn_nbctl_basic_lr], [basic logical router commands], [
  AT_CHECK([ovn-nbctl lr-add lr0])
  AT_CHECK([ovn-nbctl lr-list | uuidfilt], [0], [dnl
  <0> (lr0)
@@ -1082,16 +1064,11 @@ AT_CHECK([ovn-nbctl --add-duplicate lr-add], [1], [],
  ])
  AT_CHECK([ovn-nbctl --may-exist lr-add], [1], [],
    [ovn-nbctl: --may-exist requires specifying a name
-])
-
-OVN_NBCTL_TEST_STOP
-AT_CLEANUP
+])])
dnl --------------------------------------------------------------------- -AT_SETUP([ovn-nbctl - basic logical router port commands])
-OVN_NBCTL_TEST_START
-
+OVN_NBCTL_TEST([ovn_nbctl_basic_lrp], [basic logical router port commands], [
  AT_CHECK([ovn-nbctl lr-add lr0])
  AT_CHECK([ovn-nbctl lrp-add lr0 lrp0 00:00:00:01:02 192.168.1.1/24], [1], [],
    [ovn-nbctl: lrp0: invalid mac address 00:00:00:01:02
@@ -1157,15 +1134,11 @@ AT_CHECK([ovn-nbctl --may-exist lrp-add lr0 lrp1 
00:00:00:01:02:03 192.168.1.1/2
    [ovn-nbctl: lrp1: port already exists with different network
  ])
-AT_CHECK([ovn-nbctl --may-exist lrp-add lr0 lrp1 00:00:00:01:02:03 10.0.0.1/24 192.168.1.1/24 peer=lrp1-peer])
-
-OVN_NBCTL_TEST_STOP
-AT_CLEANUP
+AT_CHECK([ovn-nbctl --may-exist lrp-add lr0 lrp1 00:00:00:01:02:03 10.0.0.1/24 
192.168.1.1/24 peer=lrp1-peer])])
dnl --------------------------------------------------------------------- -AT_SETUP([ovn-nbctl - logical router port gateway chassis])
-OVN_NBCTL_TEST_START
+OVN_NBCTL_TEST([ovn_nbctl_lrp_gw_chassi], [logical router port gateway 
chassis], [
  AT_CHECK([ovn-nbctl lr-add lr0])
  AT_CHECK([ovn-nbctl lrp-add lr0 lrp0 00:00:00:01:02:03 192.168.1.1/24])
  AT_CHECK([ovn-nbctl lrp-get-gateway-chassis lrp0], [0], [])
@@ -1221,16 +1194,11 @@ AT_CHECK([ovn-nbctl lrp-get-gateway-chassis lrp0], [0], 
[dnl
  lrp0-chassis2    10
  lrp0-chassis3     5
  lrp0-chassis1     1
-])
-
-OVN_NBCTL_TEST_STOP
-AT_CLEANUP
+])])
dnl --------------------------------------------------------------------- -AT_SETUP([ovn-nbctl - logical router port enable and disable])
-OVN_NBCTL_TEST_START
-
+OVN_NBCTL_TEST([ovn_nbctl_lrp_enable], [logical router port enable and 
disable], [
  AT_CHECK([ovn-nbctl lr-add lr0])
  AT_CHECK([ovn-nbctl lrp-add lr0 lrp0 00:00:00:01:02:03 192.168.1.1/24])
  AT_CHECK([ovn-nbctl lrp-get-enabled lrp0], [0], [enabled
@@ -1246,16 +1214,11 @@ AT_CHECK([ovn-nbctl lrp-get-enabled lrp0], [0], [enabled
AT_CHECK([ovn-nbctl lrp-set-enabled lrp0 xyzzy], [1], [],
    [ovn-nbctl: xyzzy: state must be "enabled" or "disabled"
-])
-
-OVN_NBCTL_TEST_STOP
-AT_CLEANUP
+])])
dnl --------------------------------------------------------------------- -AT_SETUP([ovn-nbctl - routes])
-OVN_NBCTL_TEST_START
-
+OVN_NBCTL_TEST([ovn_nbctl_routes], [routes], [
  AT_CHECK([ovn-nbctl lr-add lr0])
dnl Check IPv4 routes
@@ -1364,16 +1327,11 @@ IPv6 Routes
              2001:db8::/64        2001:db8:0:f102::1 dst-ip lp0
            2001:db8:1::/64        2001:db8:0:f103::1 dst-ip
                       ::/0        2001:db8:0:f101::1 dst-ip
-])
-
-OVN_NBCTL_TEST_STOP
-AT_CLEANUP
+])])
dnl --------------------------------------------------------------------- -AT_SETUP([ovn-nbctl - lsp types])
-OVN_NBCTL_TEST_START
-
+OVN_NBCTL_TEST([ovn_nbctl_lsp_types], [lsp types], [
  AT_CHECK([ovn-nbctl ls-add ls0])
  AT_CHECK([ovn-nbctl lsp-add ls0 lp0])
@@ -1437,30 +1395,20 @@ dnl Empty string should work too
  AT_CHECK([ovn-nbctl lsp-set-type lp0 ""])
  AT_CHECK([ovn-nbctl lsp-get-type lp0], [0], [dnl
-])
-
-OVN_NBCTL_TEST_STOP
-AT_CLEANUP
+])])
dnl --------------------------------------------------------------------- -AT_SETUP([ovn-nbctl - connection])
-OVN_NBCTL_TEST_START
-
+OVN_NBCTL_TEST([ovn_nbctl_connection], [connection], [
  AT_CHECK([ovn-nbctl --inactivity-probe=30000 set-connection 
ptcp:6641:127.0.0.1 punix:$OVS_RUNDIR/ovnnb_db.sock])
  AT_CHECK([ovn-nbctl list connection | grep inactivity_probe], [0], [dnl
  inactivity_probe    : 30000
  inactivity_probe    : 30000
-])
-
-OVN_NBCTL_TEST_STOP
-AT_CLEANUP
+])])
dnl --------------------------------------------------------------------- -AT_SETUP([ovn-nbctl - dry run mode])
-OVN_NBCTL_TEST_START
-
+OVN_NBCTL_TEST([ovn_nbctl_dry_run_mode], [dry run mode], [
  dnl Check that dry run has no permanent effect.
  AT_CHECK([ovn-nbctl --dry-run ls-add ls0 -- ls-list | uuidfilt], [0], [dnl
  <0> (ls0)
@@ -1472,16 +1420,11 @@ dnl Check that dry-run mode is not sticky.
  AT_CHECK([ovn-nbctl ls-add ls0])
  AT_CHECK([ovn-nbctl ls-list | uuidfilt], [0], [dnl
  <0> (ls0)
-])
-
-OVN_NBCTL_TEST_STOP
-AT_CLEANUP
+])])
dnl --------------------------------------------------------------------- -AT_SETUP([ovn-nbctl - oneline output])
-OVN_NBCTL_TEST_START
-
+OVN_NBCTL_TEST([ovn_nbctl_oneline_output], [oneline output], [
  AT_CHECK([ovn-nbctl ls-add ls0 -- ls-add ls1])
dnl Expect one line for one command.
@@ -1493,16 +1436,11 @@ dnl Expect lines for two commands.
  AT_CHECK([ovn-nbctl --oneline ls-list -- ls-list | uuidfilt], [0], [dnl
  <0> (ls0)\n<1> (ls1)
  <0> (ls0)\n<1> (ls1)
-])
-
-OVN_NBCTL_TEST_STOP
-AT_CLEANUP
+])])
dnl --------------------------------------------------------------------- -AT_SETUP([ovn-nbctl - commands parser error paths])
-OVN_NBCTL_TEST_START
-
+OVN_NBCTL_TEST([ovn_nbctl_error_paths], [commands parser error paths], [
  dnl FIXME: Duplicate options are allowed when passed with global options.
  dnl        For example: ovn-nbctl --if-exists --if-exists list Logical_Switch
@@ -1569,21 +1507,13 @@ AT_CHECK([ovn-nbctl show foo --bar], [1], [], [stderr])
  AT_CHECK([grep 'command takes at most .* arguments' stderr], [0], [ignore])
AT_CHECK([ovn-nbctl -- show foo --bar], [1], [], [stderr])
-AT_CHECK([grep 'command takes at most .* arguments' stderr], [0], [ignore])
-
-OVN_NBCTL_TEST_STOP
-AT_CLEANUP
+AT_CHECK([grep 'command takes at most .* arguments' stderr], [0], [ignore])])
dnl --------------------------------------------------------------------- -AT_SETUP([ovn-nbctl - Port Groups])
-OVN_NBCTL_TEST_START
-
+OVN_NBCTL_TEST([ovn_nbctl_port_groups], [port groups], [
  dnl Check that port group can be looked up by name
  AT_CHECK([ovn-nbctl create Port_Group name=pg0], [0], [ignore])
  AT_CHECK([ovn-nbctl get Port_Group pg0 name], [0], [dnl
  "pg0"
-])
-
-OVN_NBCTL_TEST_STOP
-AT_CLEANUP
+])])


_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to