From: Dave Barach <dave@barachs.net>
Date: Fri, 17 Aug 2018 22:29:07 +0000 (-0400)
Subject: Clean up packet tracer, especially "clear trace"
X-Git-Tag: v18.10-rc1~370
X-Git-Url: https://gerrit.fd.io/r/gitweb?a=commitdiff_plain;h=f8b8586b699bae9e786726f2697c3e642d904c61;p=vpp.git

Clean up packet tracer, especially "clear trace"

There are multiple trace enablement schemes. It's easy to end up in
vlib_add_trace with tracing disabled insofar as the packet tracer is
concerned. When that happens, return the address of a per-system
dummy trace record.

Change-Id: I929391b8be4ed57e26e291afdc509a15f09a3160
Signed-off-by: Dave Barach <dave@barachs.net>
---

diff --git a/src/vlib/trace.c b/src/vlib/trace.c
index 6d487ae1f40..cfc3bda2cd0 100644
--- a/src/vlib/trace.c
+++ b/src/vlib/trace.c
@@ -40,6 +40,8 @@
 #include <vlib/vlib.h>
 #include <vlib/threads.h>
 
+u8 *vnet_trace_dummy;
+
 /* Helper function for nodes which only trace buffer data. */
 void
 vlib_trace_frame_buffers_only (vlib_main_t * vm,
@@ -117,18 +119,14 @@ clear_trace_buffer (void)
   /* *INDENT-OFF* */
   foreach_vlib_main (
   ({
-    void *mainheap;
-
     tm = &this_vlib_main->trace_main;
-    mainheap = clib_mem_set_heap (this_vlib_main->heap_base);
 
-    tm->trace_active_hint = 0;
+    tm->trace_enable = 0;
 
     for (i = 0; i < vec_len (tm->trace_buffer_pool); i++)
       if (! pool_is_free_index (tm->trace_buffer_pool, i))
         vec_free (tm->trace_buffer_pool[i]);
     pool_free (tm->trace_buffer_pool);
-    clib_mem_set_heap (mainheap);
   }));
   /* *INDENT-ON* */
 }
@@ -298,15 +296,11 @@ cli_show_trace_buffer (vlib_main_t * vm,
   /* *INDENT-OFF* */
   foreach_vlib_main (
   ({
-    void *mainheap;
-
     fmt = "------------------- Start of thread %d %s -------------------\n";
     s = format (s, fmt, index, vlib_worker_threads[index].name);
 
     tm = &this_vlib_main->trace_main;
 
-    mainheap = clib_mem_set_heap (this_vlib_main->heap_base);
-
     trace_apply_filter(this_vlib_main);
 
     traces = 0;
@@ -317,7 +311,6 @@ cli_show_trace_buffer (vlib_main_t * vm,
 
     if (vec_len (traces) == 0)
       {
-        clib_mem_set_heap (mainheap);
         s = format (s, "No packets in trace buffer\n");
         goto done;
       }
@@ -334,17 +327,12 @@ cli_show_trace_buffer (vlib_main_t * vm,
             goto done;
           }
 
-        clib_mem_set_heap (mainheap);
-
         s = format (s, "Packet %d\n%U\n\n", i + 1,
                          format_vlib_trace, vm, traces[i]);
-
-        mainheap = clib_mem_set_heap (this_vlib_main->heap_base);
       }
 
   done:
     vec_free (traces);
-    clib_mem_set_heap (mainheap);
 
     index++;
   }));
@@ -377,6 +365,9 @@ cli_add_trace_buffer (vlib_main_t * vm,
   if (!unformat_user (input, unformat_line_input, line_input))
     return 0;
 
+  if (vnet_trace_dummy == 0)
+    vec_validate_aligned (vnet_trace_dummy, 2048, CLIB_CACHE_LINE_BYTES);
+
   while (unformat_check_input (line_input) != (uword) UNFORMAT_END_OF_INPUT)
     {
       if (unformat (line_input, "%U %d",
@@ -395,15 +386,12 @@ cli_add_trace_buffer (vlib_main_t * vm,
   /* *INDENT-OFF* */
   foreach_vlib_main ((
     {
-      void *oldheap;
       tm = &this_vlib_main->trace_main;
-      tm->trace_active_hint = 1;
       tm->verbose = verbose;
-      oldheap =
-	clib_mem_set_heap (this_vlib_main->heap_base);
       vec_validate (tm->nodes, node_index);
       tn = tm->nodes + node_index;
-      tn->limit += add; clib_mem_set_heap (oldheap);
+      tn->limit += add;
+      tm->trace_enable = 1;
     }));
   /* *INDENT-ON* */
 
@@ -467,7 +455,6 @@ cli_filter_trace (vlib_main_t * vm,
   u32 filter_node_index;
   u32 filter_flag;
   u32 filter_count;
-  void *mainheap;
 
   if (unformat (input, "include %U %d",
 		unformat_vlib_node, vm, &filter_node_index, &filter_count))
@@ -502,11 +489,10 @@ cli_filter_trace (vlib_main_t * vm,
 
     /*
      * Clear the trace limits to stop any in-progress tracing
-     * Prevents runaway trace allocations when the filter changes (or is removed)
+     * Prevents runaway trace allocations when the filter changes
+     * (or is removed)
      */
-    mainheap = clib_mem_set_heap (this_vlib_main->heap_base);
     vec_free (tm->nodes);
-    clib_mem_set_heap (mainheap);
   }));
   /* *INDENT-ON* */
 
diff --git a/src/vlib/trace.h b/src/vlib/trace.h
index fc0fc5c8ed4..0560913c588 100644
--- a/src/vlib/trace.h
+++ b/src/vlib/trace.h
@@ -80,7 +80,7 @@ typedef struct
   u32 filter_count;
 
   /* set on trace add, cleared on clear trace */
-  u32 trace_active_hint;
+  u32 trace_enable;
 
   /* Per node trace counts. */
   vlib_trace_node_t *nodes;
diff --git a/src/vlib/trace_funcs.h b/src/vlib/trace_funcs.h
index 5280eae9904..eb06799305c 100644
--- a/src/vlib/trace_funcs.h
+++ b/src/vlib/trace_funcs.h
@@ -40,6 +40,8 @@
 #ifndef included_vlib_trace_funcs_h
 #define included_vlib_trace_funcs_h
 
+extern u8 *vnet_trace_dummy;
+
 always_inline void
 vlib_validate_trace (vlib_trace_main_t * tm, vlib_buffer_t * b)
 {
@@ -59,6 +61,14 @@ vlib_add_trace (vlib_main_t * vm,
   vlib_trace_header_t *h;
   u32 n_data_words;
 
+  ASSERT (vnet_trace_dummy);
+
+  if (PREDICT_FALSE (tm->trace_enable == 0))
+    {
+      ASSERT (vec_len (vnet_trace_dummy) >= n_data_bytes + sizeof (*h));
+      return vnet_trace_dummy;
+    }
+
   vlib_validate_trace (tm, b);
 
   n_data_bytes = round_pow2 (n_data_bytes, sizeof (h[0]));
@@ -108,6 +118,9 @@ vlib_trace_buffer (vlib_main_t * vm,
   vlib_trace_main_t *tm = &vm->trace_main;
   vlib_trace_header_t **h;
 
+  if (PREDICT_FALSE (tm->trace_enable == 0))
+    return;
+
   /*
    * Apply filter to existing traces to keep number of allocated traces low.
    * Performed each time around the main loop.