[ovs-dev] [PATCH ovn 3/5] ovn-trace: Make the "datapath" command-line argument optional.

Ben Pfaff blp at ovn.org
Thu Oct 22 21:24:20 UTC 2020


It can be inferred from the (required) input port.

Signed-off-by: Ben Pfaff <blp at ovn.org>
---
 NEWS                      |  2 +
 include/ovn/expr.h        |  1 +
 lib/expr.c                | 52 ++++++++++++++++++++++
 utilities/ovn-trace.8.xml | 14 +++---
 utilities/ovn-trace.c     | 94 +++++++++++++++++++++++++++++++--------
 5 files changed, 138 insertions(+), 25 deletions(-)

diff --git a/NEWS b/NEWS
index eaacf7340d6a..35825ac34919 100644
--- a/NEWS
+++ b/NEWS
@@ -1,5 +1,7 @@
 Post-v20.09.0
 ---------------------
+   - The "datapath" argument to ovn-trace is now optional, since the
+     datapath can be inferred from the inport (which is required).
 
 
 OVN v20.09.0 - 28 Sep 2020
diff --git a/include/ovn/expr.h b/include/ovn/expr.h
index ec8e95b2d50c..0a83ec7a80b6 100644
--- a/include/ovn/expr.h
+++ b/include/ovn/expr.h
@@ -452,6 +452,7 @@ char *expr_parse_microflow(const char *, const struct shash *symtab,
                                                unsigned int *portp),
                            const void *aux, struct flow *uflow)
     OVS_WARN_UNUSED_RESULT;
+char *expr_find_inport(const struct expr *, char **errorp);
 
 bool expr_evaluate(const struct expr *, const struct flow *uflow,
                    bool (*lookup_port)(const void *aux, const char *port_name,
diff --git a/lib/expr.c b/lib/expr.c
index acb1f3a042b7..4566d9110ee7 100644
--- a/lib/expr.c
+++ b/lib/expr.c
@@ -3538,3 +3538,55 @@ expr_parse_microflow(const char *s, const struct shash *symtab,
     }
     return error;
 }
+
+static void
+expr_find_inports(const struct expr *e, struct sset *inports)
+{
+    const struct expr *sub;
+
+    switch (e->type) {
+    case EXPR_T_CMP:
+        if (!strcmp(e->cmp.symbol->name, "inport")
+            && !e->cmp.symbol->width
+            && e->cmp.relop == EXPR_R_EQ) {
+            sset_add(inports, e->cmp.string);
+        }
+        break;
+
+    case EXPR_T_AND:
+    case EXPR_T_OR:
+        LIST_FOR_EACH (sub, node, &e->andor) {
+            expr_find_inports(sub, inports);
+        }
+        break;
+
+    case EXPR_T_BOOLEAN:
+    case EXPR_T_CONDITION:
+        /* Nothing to do. */
+        break;
+    }
+}
+
+/* Traverses 'e' looking for a match against inport.  If found, returns a copy
+ * of its name.  If no matches or more than one (different) match is found,
+ * returns NULL and stores an error message in '*errorp'.  The caller must free
+ * both the error message and the port name. */
+char *
+expr_find_inport(const struct expr *e, char **errorp)
+{
+    struct sset inports = SSET_INITIALIZER(&inports);
+    expr_find_inports(e, &inports);
+
+    char *retval = NULL;
+    if (sset_count(&inports) == 1) {
+        retval = sset_pop(&inports);
+        *errorp = NULL;
+    } else if (sset_is_empty(&inports)) {
+        *errorp = xstrdup("flow match expression does not match on inport");
+    } else {
+        *errorp = xstrdup("flow match expression matches on multiple inports");
+    }
+
+    sset_destroy(&inports);
+    return retval;
+}
diff --git a/utilities/ovn-trace.8.xml b/utilities/ovn-trace.8.xml
index 23a41affb7d9..09c86fc3616e 100644
--- a/utilities/ovn-trace.8.xml
+++ b/utilities/ovn-trace.8.xml
@@ -4,7 +4,7 @@
   <p>ovn-trace -- Open Virtual Network logical network tracing utility</p>
 
   <h1>Synopsis</h1>
-  <p><code>ovn-trace</code> [<var>options</var>] <var>datapath</var> <var>microflow</var></p>
+  <p><code>ovn-trace</code> [<var>options</var>] <var>[datapath]</var> <var>microflow</var></p>
   <p><code>ovn-trace</code> [<var>options</var>] <code>--detach</code></p>
   
   <h1>Description</h1>
@@ -40,18 +40,20 @@
   </p>
 
   <p>
-    The simplest way to use <code>ovn-trace</code> is to provide
-    <var>datapath</var> and <var>microflow</var> arguments on the command
+    The simplest way to use <code>ovn-trace</code> is to provide the
+    <var>microflow</var> (and optional <var>datapath</var>) arguments on the command
     line.  In this case, it simulates the behavior of a single packet and
     exits.  For an alternate usage model, see <code>Daemon Mode</code> below.
   </p>
 
   <p>
-    The <var>datapath</var> argument specifies the name of a logical
+    The optional <var>datapath</var> argument specifies the name of a logical
     datapath.  Acceptable names are the <code>name</code> from the northbound
     <code>Logical_Switch</code> or <code>Logical_Router</code> table, the
     UUID of a record from one of those tables, or the UUID of a record from
-    the southbound <code>Datapath_Binding</code> table.
+    the southbound <code>Datapath_Binding</code> table.  (The <code>datapath</code>
+    is optional because <code>ovn-trace</code> can figure it out from the
+    <var>inport</var> that the <var>microflow</var> matches.)
   </p>
 
   <p>
@@ -289,7 +291,7 @@
   </p>
 
   <dl>
-    <dt><code>trace</code> [<var>options</var>] <var>datapath</var> <var>microflow</var></dt>
+    <dt><code>trace</code> [<var>options</var>] [<var>datapath</var>] <var>microflow</var></dt>
     <dd>
       Traces <var>microflow</var> through <var>datapath</var> and replies with
       the results of the trace.  Accepts the <var>options</var> described under
diff --git a/utilities/ovn-trace.c b/utilities/ovn-trace.c
index db5bb301e807..29bf7a20845c 100644
--- a/utilities/ovn-trace.c
+++ b/utilities/ovn-trace.c
@@ -115,8 +115,8 @@ main(int argc, char *argv[])
                       "(use --help for help)");
         }
     } else {
-        if (argc != 2) {
-            ovs_fatal(0, "exactly two non-option arguments are required "
+        if (argc != 1 && argc != 2) {
+            ovs_fatal(0, "one or two non-option arguments are required "
                       "(use --help for help)");
         }
     }
@@ -134,8 +134,8 @@ main(int argc, char *argv[])
             ovs_fatal(error, "failed to create unixctl server");
         }
         unixctl_command_register("exit", "", 0, 0, ovntrace_exit, &exiting);
-        unixctl_command_register("trace", "[OPTIONS] DATAPATH MICROFLOW",
-                                 2, INT_MAX, ovntrace_trace, NULL);
+        unixctl_command_register("trace", "[OPTIONS] [DATAPATH] MICROFLOW",
+                                 1, INT_MAX, ovntrace_trace, NULL);
     }
     ovnsb_idl = ovsdb_idl_create(db, &sbrec_idl_class, true, false);
 
@@ -157,7 +157,9 @@ main(int argc, char *argv[])
 
             daemonize_complete();
             if (!get_detach()) {
-                char *output = trace(argv[0], argv[1]);
+                const char *dp_s = argc > 1 ? argv[0] : NULL;
+                const char *flow_s = argv[argc - 1];
+                char *output = trace(dp_s, flow_s);
                 fputs(output, stdout);
                 free(output);
                 return 0;
@@ -360,7 +362,7 @@ usage(void)
 {
     printf("\
 %s: OVN trace utility\n\
-usage: %s [OPTIONS] DATAPATH MICROFLOW\n\
+usage: %s [OPTIONS] [DATAPATH] MICROFLOW\n\
        %s [OPTIONS] --detach\n\
 \n\
 Output format options:\n\
@@ -2559,24 +2561,76 @@ trace__(const struct ovntrace_datapath *dp, struct flow *uflow,
     }
 }
 
-static char *
-trace(const char *dp_s, const char *flow_s)
+static char * OVS_WARN_UNUSED_RESULT
+trace_parse_error(char *error)
 {
-    const struct ovntrace_datapath *dp = ovntrace_datapath_find_by_name(dp_s);
-    if (!dp) {
-        return xasprintf("unknown datapath \"%s\"\n", dp_s);
+    char *s = xasprintf("error parsing flow: %s\n", error);
+    free(error);
+    return s;
+}
+
+static char * OVS_WARN_UNUSED_RESULT
+trace_parse(const char *dp_s, const char *flow_s,
+            const struct ovntrace_datapath **dpp, struct flow *uflow)
+{
+    *dpp = NULL;
+    if (dp_s) {
+        /* Use the specified datapath. */
+        *dpp = ovntrace_datapath_find_by_name(dp_s);
+        if (!dpp) {
+            return xasprintf("unknown datapath \"%s\"\n", dp_s);
+        }
+    } else {
+        /* Figure out the datapath from the flow, based on its inport.
+         *
+         * First make sure that the expression parses. */
+        char *error;
+        struct expr *e = expr_parse_string(flow_s, &symtab, &address_sets,
+                                           &port_groups, NULL, NULL, 0,
+                                           &error);
+        if (!e) {
+            return trace_parse_error(error);
+        }
+
+        /* Extract the name of the inport. */
+        char *port_name = expr_find_inport(e, &error);
+        expr_destroy(e);
+        if (!port_name) {
+            return trace_parse_error(error);
+        }
+
+        /* Find the port by name. */
+        const struct ovntrace_port *port = shash_find_data(&ports, port_name);
+        if (!port) {
+            char *s = xasprintf("unknown port \"%s\"\n", port_name);
+            free(port_name);
+            return s;
+        }
+
+        /* Use the port's datapath. */
+        *dpp = port->dp;
+        free(port_name);
     }
 
-    struct flow uflow;
     char *error = expr_parse_microflow(flow_s, &symtab, &address_sets,
                                        &port_groups, ovntrace_lookup_port,
-                                       dp, &uflow);
+                                       *dpp, uflow);
     if (error) {
-        char *s = xasprintf("error parsing flow: %s\n", error);
-        free(error);
-        return s;
+        return trace_parse_error(error);
     }
 
+    return NULL;
+}
+
+static char *
+trace(const char *dp_s, const char *flow_s)
+{
+    const struct ovntrace_datapath *dp;
+    struct flow uflow;
+    char *error = trace_parse(dp_s, flow_s, &dp, &uflow);
+    if (error) {
+        return error;
+    }
     uint32_t in_key = uflow.regs[MFF_LOG_INPORT - MFF_REG0];
     if (!in_key) {
         VLOG_WARN("microflow does not specify ingress port");
@@ -2671,13 +2725,15 @@ ovntrace_trace(struct unixctl_conn *conn, int argc,
         detailed = true;
     }
 
-    if (argc != 3) {
+    if (argc != 2 && argc != 3) {
         unixctl_command_reply_error(
-            conn, "exactly 2 non-option arguments are required");
+            conn, "one or two non-option arguments are required");
         return;
     }
 
-    char *output = trace(argv[1], argv[2]);
+    const char *dp_s = argc > 2 ? argv[1] : NULL;
+    const char *flow_s = argv[argc - 1];
+    char *output = trace(dp_s, flow_s);
     unixctl_command_reply(conn, output);
     free(output);
 }
-- 
2.26.2



More information about the dev mailing list