VPP API: Memory trace

if you plan to put a hash into shared memory, the key sum and key
equal functions MUST be set to constants such as KEY_FUNC_STRING,
KEY_FUNC_MEM, etc. -lvppinfra is PIC, which means that the process
which set up the hash won't have the same idea where the key sum and
key compare functions live in other processes.

Change-Id: Ib3b5963a0d2fb467b91e1f16274df66ac74009e9
Signed-off-by: Ole Troan <ot@cisco.com>
Signed-off-by: Dave Barach <dave@barachs.net>
Signed-off-by: Ole Troan <ot@cisco.com>

12 files changed, 188 insertions, 35 deletions

diff --git a/src/svm/svm.c b/src/svm/svm.c
index 7d98fbe936d..d958c8378e5 100644
--- a/src/svm/svm.c
+++ b/src/svm/svm.c
@@ -339,9 +339,13 @@ svm_data_region_create (svm_map_region_args_t * a, svm_region_t * rp)
 
   if (a->flags & SVM_FLAGS_MHEAP)
     {
+      mheap_t *heap_header;
       rp->data_heap =
 	mheap_alloc_with_flags ((void *) (rp->data_base), map_size,
 				MHEAP_FLAG_DISABLE_VM);
+      heap_header = mheap_header (rp->data_heap);
+      heap_header->flags |= MHEAP_FLAG_THREAD_SAFE;
+
       rp->flags |= SVM_FLAGS_MHEAP;
     }
   return 0;
diff --git a/src/vlib/cli.c b/src/vlib/cli.c
index 48cf0426488..a85fa93906c 100644
--- a/src/vlib/cli.c
+++ b/src/vlib/cli.c
@@ -699,11 +699,14 @@ vlib_cli_output (vlib_main_t * vm, char *fmt, ...)
   vec_free (s);
 }
 
+void *vl_msg_push_heap (void) __attribute__ ((weak));
+void vl_msg_pop_heap (void *oldheap) __attribute__ ((weak));
+
 static clib_error_t *
 show_memory_usage (vlib_main_t * vm,
 		   unformat_input_t * input, vlib_cli_command_t * cmd)
 {
-  int verbose = 0;
+  int verbose = 0, api_segment = 0;
   clib_error_t *error;
   u32 index = 0;
 
@@ -711,6 +714,8 @@ show_memory_usage (vlib_main_t * vm,
     {
       if (unformat (input, "verbose"))
 	verbose = 1;
+      else if (unformat (input, "api-segment"))
+	api_segment = 1;
       else
 	{
 	  error = clib_error_return (0, "unknown input `%U'",
@@ -719,6 +724,23 @@ show_memory_usage (vlib_main_t * vm,
 	}
     }
 
+  if (api_segment)
+    {
+      void *oldheap = vl_msg_push_heap ();
+      u8 *s_in_svm =
+	format (0, "%U\n", format_mheap, clib_mem_get_heap (), 1);
+      vl_msg_pop_heap (oldheap);
+      u8 *s = vec_dup (s_in_svm);
+
+      oldheap = vl_msg_push_heap ();
+      vec_free (s_in_svm);
+      vl_msg_pop_heap (oldheap);
+      vlib_cli_output (vm, "API segment start:");
+      vlib_cli_output (vm, "%v", s);
+      vlib_cli_output (vm, "API segment end:");
+      vec_free (s);
+    }
+
   /* *INDENT-OFF* */
   foreach_vlib_main (
   ({
@@ -733,7 +755,7 @@ show_memory_usage (vlib_main_t * vm,
 /* *INDENT-OFF* */
 VLIB_CLI_COMMAND (show_memory_usage_command, static) = {
   .path = "show memory",
-  .short_help = "Show current memory usage",
+  .short_help = "[verbose | api-segment] Show current memory usage",
   .function = show_memory_usage,
 };
 /* *INDENT-ON* */
@@ -771,30 +793,48 @@ VLIB_CLI_COMMAND (show_cpu_command, static) = {
 };
 
 /* *INDENT-ON* */
+
 static clib_error_t *
 enable_disable_memory_trace (vlib_main_t * vm,
 			     unformat_input_t * input,
 			     vlib_cli_command_t * cmd)
 {
-  clib_error_t *error = 0;
+  unformat_input_t _line_input, *line_input = &_line_input;
   int enable;
+  int api_segment = 0;
+  void *oldheap;
+
+
+  if (!unformat_user (input, unformat_line_input, line_input))
+    return 0;
 
-  if (!unformat_user (input, unformat_vlib_enable_disable, &enable))
+  while (unformat_check_input (line_input) != UNFORMAT_END_OF_INPUT)
     {
-      error = clib_error_return (0, "expecting enable/on or disable/off");
-      goto done;
+      if (!unformat (line_input, "%U", unformat_vlib_enable_disable, &enable))
+	;
+      else if (unformat (line_input, "api-segment"))
+	api_segment = 1;
+      else
+	{
+	  unformat_free (line_input);
+	  return clib_error_return (0, "invalid input");
+	}
     }
+  unformat_free (line_input);
 
+  if (api_segment)
+    oldheap = vl_msg_push_heap ();
   clib_mem_trace (enable);
+  if (api_segment)
+    vl_msg_pop_heap (oldheap);
 
-done:
-  return error;
+  return 0;
 }
 
 /* *INDENT-OFF* */
 VLIB_CLI_COMMAND (enable_disable_memory_trace_command, static) = {
   .path = "memory-trace",
-  .short_help = "Enable/disable memory allocation trace",
+  .short_help = "on|off [api-segment] Enable/disable memory allocation trace",
   .function = enable_disable_memory_trace,
 };
 /* *INDENT-ON* */
diff --git a/src/vlibapi/api_common.h b/src/vlibapi/api_common.h
index 3d55e2acdd6..73c5a50e850 100644
--- a/src/vlibapi/api_common.h
+++ b/src/vlibapi/api_common.h
@@ -178,6 +178,8 @@ int vl_msg_api_pd_handler (void *mp, int rv);
 void vl_msg_api_set_first_available_msg_id (u16 first_avail);
 u16 vl_msg_api_get_msg_ids (const char *name, int n);
 u32 vl_msg_api_get_msg_index (u8 * name_and_crc);
+void *vl_msg_push_heap (void);
+void vl_msg_pop_heap (void *oldheap);
 
 typedef clib_error_t *(vl_msg_api_init_function_t) (u32 client_index);
 
diff --git a/src/vlibapi/api_shared.c b/src/vlibapi/api_shared.c
index 3b9c0f47b66..24ec33b1506 100644
--- a/src/vlibapi/api_shared.c
+++ b/src/vlibapi/api_shared.c
@@ -955,6 +955,23 @@ vl_msg_api_get_msg_index (u8 * name_and_crc)
   return ~0;
 }
 
+void *
+vl_msg_push_heap (void)
+{
+  api_main_t *am = &api_main;
+  pthread_mutex_lock (&am->vlib_rp->mutex);
+  return svm_push_data_heap (am->vlib_rp);
+}
+
+void
+vl_msg_pop_heap (void *oldheap)
+{
+  api_main_t *am = &api_main;
+  svm_pop_heap (oldheap);
+  pthread_mutex_unlock (&am->vlib_rp->mutex);
+}
+
+
 /*
  * fd.io coding-style-patch-verification: ON
  *
diff --git a/src/vlibmemory/memory_api.c b/src/vlibmemory/memory_api.c
index 0ee1384f22b..205bea57532 100644
--- a/src/vlibmemory/memory_api.c
+++ b/src/vlibmemory/memory_api.c
@@ -339,6 +339,7 @@ vl_api_memclnt_delete_t_handler (vl_api_memclnt_delete_t * mp)
 			  regp->vl_api_registration_pool_index);
 	  pthread_mutex_lock (&svm->mutex);
 	  oldheap = svm_push_data_heap (svm);
+	  vec_free (regp->name);
 	  /* Poison the old registration */
 	  memset (regp, 0xF1, sizeof (*regp));
 	  clib_mem_free (regp);
diff --git a/src/vlibmemory/memory_client.c b/src/vlibmemory/memory_client.c
index 9c8dcaa9d64..55ef0011cb7 100644
--- a/src/vlibmemory/memory_client.c
+++ b/src/vlibmemory/memory_client.c
@@ -105,6 +105,29 @@ vl_api_rx_thread_exit_t_handler (vl_api_rx_thread_exit_t * mp)
 }
 
 static void
+vl_api_name_and_crc_free (void)
+{
+  api_main_t *am = &api_main;
+  int i;
+  u8 **keys = 0;
+  hash_pair_t *hp;
+
+  if (!am->msg_index_by_name_and_crc)
+    return;
+
+  /* *INDENT-OFF* */
+  hash_foreach_pair (hp, am->msg_index_by_name_and_crc,
+      ({
+        vec_add1 (keys, (u8 *) hp->key);
+      }));
+  /* *INDENT-ON* */
+  for (i = 0; i < vec_len (keys); i++)
+    vec_free (keys[i]);
+  vec_free (keys);
+  hash_free (am->msg_index_by_name_and_crc);
+}
+
+static void
 vl_api_memclnt_create_reply_t_handler (vl_api_memclnt_create_reply_t * mp)
 {
   serialize_main_t _sm, *sm = &_sm;
@@ -119,21 +142,7 @@ vl_api_memclnt_create_reply_t_handler (vl_api_memclnt_create_reply_t * mp)
   am->my_registration = (vl_api_registration_t *) (uword) mp->handle;
 
   /* Clean out any previous hash table (unlikely) */
-  if (am->msg_index_by_name_and_crc)
-    {
-      int i;
-      u8 **keys = 0;
-      hash_pair_t *hp;
-      /* *INDENT-OFF* */
-      hash_foreach_pair (hp, am->msg_index_by_name_and_crc,
-      ({
-        vec_add1 (keys, (u8 *) hp->key);
-      }));
-      /* *INDENT-ON* */
-      for (i = 0; i < vec_len (keys); i++)
-	vec_free (keys[i]);
-      vec_free (keys);
-    }
+  vl_api_name_and_crc_free ();
 
   am->msg_index_by_name_and_crc = hash_create_string (0, sizeof (uword));
 
@@ -192,8 +201,8 @@ vl_client_connect (const char *name, int ctx_quota, int input_queue_size)
   oldheap = svm_push_data_heap (svm);
   vl_input_queue = svm_queue_init (input_queue_size, sizeof (uword),
 				   getpid (), 0);
-  pthread_mutex_unlock (&svm->mutex);
   svm_pop_heap (oldheap);
+  pthread_mutex_unlock (&svm->mutex);
 
   am->my_client_index = ~0;
   am->my_registration = 0;
@@ -317,6 +326,8 @@ vl_client_disconnect (void)
       vl_msg_api_handler ((void *) rp);
       break;
     }
+
+  vl_api_name_and_crc_free ();
   return 0;
 }
 
diff --git a/src/vpp-api/client/client.c b/src/vpp-api/client/client.c
index c05ea37e00b..cd0b5b9e45d 100644
--- a/src/vpp-api/client/client.c
+++ b/src/vpp-api/client/client.c
@@ -72,6 +72,35 @@ vac_callback_t vac_callback;
 u16 read_timeout = 0;
 bool rx_is_running = false;
 
+/* Set to true to enable memory tracing */
+bool mem_trace = false;
+
+__attribute__((constructor))
+static void
+vac_client_constructor (void)
+{
+  u8 *heap;
+  mheap_t *h;
+  clib_mem_init (0, 1 << 30);
+  heap = clib_mem_get_per_cpu_heap ();
+  h = mheap_header (heap);
+  /* make the main heap thread-safe */
+  h->flags |= MHEAP_FLAG_THREAD_SAFE;
+  if (mem_trace)
+    clib_mem_trace (1);
+}
+
+__attribute__((destructor))
+static void
+vac_client_destructor (void)
+{
+  if (mem_trace)
+    fformat(stderr, "TRACE: %s",
+	    format (0, "%U\n",
+		    format_mheap, clib_mem_get_heap (), 1));
+}
+
+
 static void
 init (void)
 {
@@ -90,14 +119,14 @@ static void
 cleanup (void)
 {
   vac_main_t *pm = &vac_main;
+  pthread_mutex_destroy(&pm->queue_lock);
   pthread_cond_destroy(&pm->suspend_cv);
   pthread_cond_destroy(&pm->resume_cv);
+  pthread_mutex_destroy(&pm->timeout_lock);
   pthread_cond_destroy(&pm->timeout_cv);
   pthread_cond_destroy(&pm->timeout_cancel_cv);
   pthread_cond_destroy(&pm->terminate_cv);
-  pthread_mutex_destroy(&pm->queue_lock);
-  pthread_mutex_destroy(&pm->timeout_lock);
-  memset (pm, 0, sizeof (*pm));
+  memset(pm, 0, sizeof(*pm));
 }
 
 /*
@@ -415,6 +444,7 @@ vac_read (char **p, int *l, u16 timeout)
     switch (msg_id) {
     case VL_API_RX_THREAD_EXIT:
       printf("Received thread exit\n");
+      vl_msg_api_free((void *) msg);
       return -1;
     case VL_API_MEMCLNT_RX_THREAD_SUSPEND:
       printf("Received thread suspend\n");
diff --git a/src/vpp-api/client/test.c b/src/vpp-api/client/test.c
index c1b3808a5f5..8061fe58fb6 100644
--- a/src/vpp-api/client/test.c
+++ b/src/vpp-api/client/test.c
@@ -70,20 +70,34 @@ wrap_vac_callback (unsigned char *data, int len)
   result_msg_id = ntohs(*((u16 *)data));
 }
 
-int main (int argc, char ** argv)
+static void
+test_connect ()
+{
+  static int i;
+  int rv = vac_connect("vac_client", NULL, wrap_vac_callback, 32 /* rx queue-length*/);
+  if (rv != 0) {
+    printf("Connect failed: %d\n", rv);
+    exit(rv);
+  }
+  printf(".");
+  vac_disconnect();
+  i++;
+}
+
+static void
+test_messages (void)
 {
   api_main_t * am = &api_main;
   vl_api_show_version_t message;
   vl_api_show_version_t *mp;
   int async = 1;
-  int rv = vac_connect("vac_client", NULL, wrap_vac_callback, 32 /* rx queue-length*/);
 
+  int rv = vac_connect("vac_client", NULL, wrap_vac_callback, 32 /* rx queue-length*/);
   if (rv != 0) {
     printf("Connect failed: %d\n", rv);
     exit(rv);
   }
- 
- struct timeb timer_msec;
+  struct timeb timer_msec;
   long long int timestamp_msec_start; /* timestamp in millisecond. */
   if (!ftime(&timer_msec)) {
     timestamp_msec_start = ((long long int) timer_msec.time) * 1000ll + 
@@ -135,5 +149,15 @@ int main (int argc, char ** argv)
 	 no_msgs/(timestamp_msec_end - timestamp_msec_start));
   printf("Exiting...\n");
   vac_disconnect();
+}
+
+int main (int argc, char ** argv)
+{
+  int i;
+
+  for (i = 0; i < 1000; i++) {
+    test_connect();
+  }
+  test_messages();
   exit (0);
 }
diff --git a/src/vppinfra/hash.c b/src/vppinfra/hash.c
index 79103b6d3f4..abc7c4ce4a2 100644
--- a/src/vppinfra/hash.c
+++ b/src/vppinfra/hash.c
@@ -282,6 +282,10 @@ key_sum (hash_t * h, uword key)
       sum = string_key_sum (h, key);
       break;
 
+    case KEY_FUNC_MEM:
+      sum = mem_key_sum (h, key);
+      break;
+
     default:
       sum = h->key_sum (h, key);
       break;
@@ -312,6 +316,10 @@ key_equal1 (hash_t * h, uword key1, uword key2, uword e)
       e = string_key_equal (h, key1, key2);
       break;
 
+    case KEY_FUNC_MEM:
+      e = mem_key_equal (h, key1, key2);
+      break;
+
     default:
       e = h->key_equal (h, key1, key2);
       break;
diff --git a/src/vppinfra/hash.h b/src/vppinfra/hash.h
index 21986ea774c..de155e6ac1c 100644
--- a/src/vppinfra/hash.h
+++ b/src/vppinfra/hash.h
@@ -77,6 +77,7 @@ typedef struct hash_header
 #define KEY_FUNC_POINTER_UWORD	(1)	/*< sum = *(uword *) key */
 #define KEY_FUNC_POINTER_U32	(2)	/*< sum = *(u32 *) key */
 #define KEY_FUNC_STRING         (3)	/*< sum = string_key_sum, etc. */
+#define KEY_FUNC_MEM		(4)	/*< sum = mem_key_sum */
 
   /* key comparison function */
   hash_key_equal_function_t *key_equal;
@@ -672,6 +673,20 @@ extern uword string_key_sum (hash_t * h, uword key);
 extern uword string_key_equal (hash_t * h, uword key1, uword key2);
 extern u8 *string_key_format_pair (u8 * s, va_list * args);
 
+/*
+ * Note: if you plan to put a hash into shared memory,
+ * the key sum and key equal functions MUST be set to magic constants!
+ * PIC means that whichever process sets up the hash won't have
+ * the actual key sum functions at the same place, leading to
+ * very hard-to-find bugs...
+ */
+
+#define hash_create_shmem(elts,key_bytes,value_bytes)           \
+  hash_create2((elts),(key_bytes),(value_bytes),                \
+               (hash_key_sum_function_t *) KEY_FUNC_MEM,        \
+               (hash_key_equal_function_t *)KEY_FUNC_MEM,       \
+               0, 0)
+
 #define hash_create_string(elts,value_bytes)                    \
   hash_create2((elts),0,(value_bytes),                          \
                (hash_key_sum_function_t *) KEY_FUNC_STRING,     \
diff --git a/src/vppinfra/mheap.c b/src/vppinfra/mheap.c
index 4d27d419e64..fceca95ff7d 100644
--- a/src/vppinfra/mheap.c
+++ b/src/vppinfra/mheap.c
@@ -1512,7 +1512,7 @@ mheap_get_trace (void *v, uword offset, uword size)
 
   if (!tm->trace_by_callers)
     tm->trace_by_callers =
-      hash_create_mem (0, sizeof (trace.callers), sizeof (uword));
+      hash_create_shmem (0, sizeof (trace.callers), sizeof (uword));
 
   p = hash_get_mem (tm->trace_by_callers, &trace.callers);
   if (p)
diff --git a/src/vppinfra/test_macros.c b/src/vppinfra/test_macros.c
index de8f2c49fc1..05299b38e3f 100644
--- a/src/vppinfra/test_macros.c
+++ b/src/vppinfra/test_macros.c
@@ -26,13 +26,14 @@ test_macros_main (unformat_input_t * input)
   clib_macro_init (mm);
 
   fformat (stdout, "hostname: %s\n",
-	   clib_macro_eval_dollar (mm, "hostname", 1 /* complain */ ));
+	   clib_macro_eval_dollar (mm, (i8 *) "hostname", 1 /* complain */ ));
 
   clib_macro_set_value (mm, "foo", "this is foo which contains $(bar)");
   clib_macro_set_value (mm, "bar", "bar");
 
   fformat (stdout, "evaluate: %s\n",
-	   clib_macro_eval (mm, "returns '$(foo)'", 1 /* complain */ ));
+	   clib_macro_eval (mm, (i8 *) "returns '$(foo)'",
+			    1 /* complain */ ));
 
   clib_macro_free (mm);