diff mbox

[ovs-dev,2/2] ovn-trace: Include source file and line number reference in output.

Message ID 1475859724-27803-2-git-send-email-blp@ovn.org
State Accepted
Headers show

Commit Message

Ben Pfaff Oct. 7, 2016, 5:02 p.m. UTC
This should make it that much easier to track down the code that emitted
a particular flow.

Signed-off-by: Ben Pfaff <blp@ovn.org>
---
 ovn/northd/ovn-northd.c       | 40 +++++++++++++++++++++++++++++++---------
 ovn/ovn-sb.xml                |  5 +++++
 ovn/utilities/ovn-trace.8.xml |  7 ++++---
 ovn/utilities/ovn-trace.c     |  9 ++++++++-
 4 files changed, 48 insertions(+), 13 deletions(-)

Comments

Andy Zhou Oct. 7, 2016, 9:39 p.m. UTC | #1
On Fri, Oct 7, 2016 at 10:02 AM, Ben Pfaff <blp@ovn.org> wrote:

> This should make it that much easier to track down the code that emitted
> a particular flow.
>
> Signed-off-by: Ben Pfaff <blp@ovn.org>
>

 Acked-by: Andy Zhou <azhou@ovn.org>

Also tested in the OVN sandbox.
Ben Pfaff Oct. 7, 2016, 10:09 p.m. UTC | #2
On Fri, Oct 07, 2016 at 02:39:00PM -0700, Andy Zhou wrote:
> On Fri, Oct 7, 2016 at 10:02 AM, Ben Pfaff <blp@ovn.org> wrote:
> 
> > This should make it that much easier to track down the code that emitted
> > a particular flow.
> >
> > Signed-off-by: Ben Pfaff <blp@ovn.org>
> >
> 
>  Acked-by: Andy Zhou <azhou@ovn.org>
> 
> Also tested in the OVN sandbox.

Thanks, I applied both of these to master.
diff mbox

Patch

diff --git a/ovn/northd/ovn-northd.c b/ovn/northd/ovn-northd.c
index 2e79156..281dc62 100644
--- a/ovn/northd/ovn-northd.c
+++ b/ovn/northd/ovn-northd.c
@@ -1635,6 +1635,7 @@  struct ovn_lflow {
     uint16_t priority;
     char *match;
     char *actions;
+    const char *where;
 };
 
 static size_t
@@ -1658,30 +1659,36 @@  ovn_lflow_equal(const struct ovn_lflow *a, const struct ovn_lflow *b)
 
 static void
 ovn_lflow_init(struct ovn_lflow *lflow, struct ovn_datapath *od,
-              enum ovn_stage stage, uint16_t priority,
-              char *match, char *actions)
+               enum ovn_stage stage, uint16_t priority,
+               char *match, char *actions, const char *where)
 {
     lflow->od = od;
     lflow->stage = stage;
     lflow->priority = priority;
     lflow->match = match;
     lflow->actions = actions;
+    lflow->where = where;
 }
 
 /* Adds a row with the specified contents to the Logical_Flow table. */
 static void
-ovn_lflow_add(struct hmap *lflow_map, struct ovn_datapath *od,
-              enum ovn_stage stage, uint16_t priority,
-              const char *match, const char *actions)
+ovn_lflow_add_at(struct hmap *lflow_map, struct ovn_datapath *od,
+                 enum ovn_stage stage, uint16_t priority,
+                 const char *match, const char *actions, const char *where)
 {
     ovs_assert(ovn_stage_to_datapath_type(stage) == ovn_datapath_get_type(od));
 
     struct ovn_lflow *lflow = xmalloc(sizeof *lflow);
     ovn_lflow_init(lflow, od, stage, priority,
-                   xstrdup(match), xstrdup(actions));
+                   xstrdup(match), xstrdup(actions), where);
     hmap_insert(lflow_map, &lflow->hmap_node, ovn_lflow_hash(lflow));
 }
 
+/* Adds a row with the specified contents to the Logical_Flow table. */
+#define ovn_lflow_add(LFLOW_MAP, OD, STAGE, PRIORITY, MATCH, ACTIONS) \
+    ovn_lflow_add_at(LFLOW_MAP, OD, STAGE, PRIORITY, MATCH, ACTIONS,  \
+                     OVS_SOURCE_LOCATOR)
+
 static struct ovn_lflow *
 ovn_lflow_find(struct hmap *lflows, struct ovn_datapath *od,
                enum ovn_stage stage, uint16_t priority,
@@ -1689,7 +1696,8 @@  ovn_lflow_find(struct hmap *lflows, struct ovn_datapath *od,
 {
     struct ovn_lflow target;
     ovn_lflow_init(&target, od, stage, priority,
-                   CONST_CAST(char *, match), CONST_CAST(char *, actions));
+                   CONST_CAST(char *, match), CONST_CAST(char *, actions),
+                   NULL);
 
     struct ovn_lflow *lflow;
     HMAP_FOR_EACH_WITH_HASH (lflow, hmap_node, ovn_lflow_hash(&target),
@@ -4337,8 +4345,22 @@  build_lflows(struct northd_context *ctx, struct hmap *datapaths,
         sbrec_logical_flow_set_match(sbflow, lflow->match);
         sbrec_logical_flow_set_actions(sbflow, lflow->actions);
 
-        const struct smap ids = SMAP_CONST1(&ids, "stage-name",
-                                            ovn_stage_to_str(lflow->stage));
+        /* Trim the source locator lflow->where, which looks something like
+         * "ovn/northd/ovn-northd.c:1234", down to just the part following the
+         * last slash, e.g. "ovn-northd.c:1234". */
+        const char *slash = strrchr(lflow->where, '/');
+#if _WIN32
+        const char *backslash = strrchr(lflow->where, '\\');
+        if (!slash || backslash > slash) {
+            slash = backslash;
+        }
+#endif
+        const char *where = slash ? slash + 1 : lflow->where;
+
+        const struct smap ids = SMAP_CONST2(
+            &ids,
+            "stage-name", ovn_stage_to_str(lflow->stage),
+            "source", where);
         sbrec_logical_flow_set_external_ids(sbflow, &ids);
 
         ovn_lflow_destroy(&lflows, lflow);
diff --git a/ovn/ovn-sb.xml b/ovn/ovn-sb.xml
index 61b0f52..b484f59 100644
--- a/ovn/ovn-sb.xml
+++ b/ovn/ovn-sb.xml
@@ -1455,6 +1455,11 @@  tcp.flags = RST;
       Human-readable name for this flow's stage in the pipeline.
     </column>
 
+    <column name="external_ids" key="source">
+      Source file and line number of the code that added this flow to the
+      pipeline.
+    </column>
+
     <group title="Common Columns">
       The overall purpose of these columns is described under <code>Common
       Columns</code> at the beginning of this document.
diff --git a/ovn/utilities/ovn-trace.8.xml b/ovn/utilities/ovn-trace.8.xml
index 747e130..92dcb17 100644
--- a/ovn/utilities/ovn-trace.8.xml
+++ b/ovn/utilities/ovn-trace.8.xml
@@ -114,9 +114,10 @@ 
   <p>
     The detailed form of output is also the default form.  This form groups
     output into sections headed up by the ingress or egress pipeline being
-    traversed.  Each pipeline lists each table that was visited (by number
-    and name), the match expression and priority of the logical flow that was
-    matched, and the actions that were executed.
+    traversed.  Each pipeline lists each table that was visited (by number and
+    name), the <code>ovn-northd</code> source file and line number of the code
+    that added the flow, the match expression and priority of the logical flow
+    that was matched, and the actions that were executed.
   </p>
 
   <p>
diff --git a/ovn/utilities/ovn-trace.c b/ovn/utilities/ovn-trace.c
index a35909f..7863f70 100644
--- a/ovn/utilities/ovn-trace.c
+++ b/ovn/utilities/ovn-trace.c
@@ -300,6 +300,7 @@  struct ovntrace_flow {
     enum ovntrace_pipeline pipeline;
     int table_id;
     char *stage_name;
+    char *source;
     int priority;
     char *match_s;
     struct expr *match;
@@ -638,6 +639,8 @@  read_flows(void)
         flow->table_id = sblf->table_id;
         flow->stage_name = nullable_xstrdup(smap_get(&sblf->external_ids,
                                                      "stage-name"));
+        flow->source = nullable_xstrdup(smap_get(&sblf->external_ids,
+                                                 "source"));
         flow->priority = sblf->priority;
         flow->match_s = xstrdup(sblf->match);
         flow->match = match;
@@ -1334,8 +1337,12 @@  trace__(const struct ovntrace_datapath *dp, struct flow *uflow,
     struct ds s = DS_EMPTY_INITIALIZER;
     ds_put_format(&s, "%2d. ", table_id);
     if (f) {
-        if (f->stage_name) {
+        if (f->stage_name && f->source) {
+            ds_put_format(&s, "%s (%s): ", f->stage_name, f->source);
+        } else if (f->stage_name) {
             ds_put_format(&s, "%s: ", f->stage_name);
+        } else if (f->source) {
+            ds_put_format(&s, "(%s): ", f->source);
         }
         ds_put_format(&s, "%s, priority %d", f->match_s, f->priority);
     } else {