stats: string vector and node collector improvements

Type: improvement
Change-Id: Ibdadeb4e685f45a93f45504a84709391489abb6a
Signed-off-by: Damjan Marion <damarion@cisco.com>

5 files changed, 161 insertions, 135 deletions

diff --git a/src/vlib/stats/collector.c b/src/vlib/stats/collector.c
index 9c982a9a14e..36d54a4fa71 100644
--- a/src/vlib/stats/collector.c
+++ b/src/vlib/stats/collector.c
@@ -6,63 +6,84 @@
 #include <vlib/unix/unix.h>
 #include <vlib/stats/stats.h>
 
+enum
+{
+  NODE_CLOCKS,
+  NODE_VECTORS,
+  NODE_CALLS,
+  NODE_SUSPENDS,
+  N_NODE_COUNTERS
+};
+
+struct
+{
+  u32 entry_index;
+  char *name;
+} node_counters[] = {
+  [NODE_CLOCKS] = { .name = "clocks" },
+  [NODE_VECTORS] = { .name = "vectors" },
+  [NODE_CALLS] = { .name = "calls" },
+  [NODE_SUSPENDS] = { .name = "suspends" },
+};
+
+static struct
+{
+  u8 *name;
+  u32 symlinks[N_NODE_COUNTERS];
+} *node_data = 0;
+
+static vlib_stats_string_vector_t node_names = 0;
+
 static inline void
 update_node_counters (vlib_stats_segment_t *sm)
 {
+  clib_bitmap_t *bmp = 0;
   vlib_main_t **stat_vms = 0;
   vlib_node_t ***node_dups = 0;
+  u32 n_nodes;
   int i, j;
-  static u32 no_max_nodes = 0;
 
   vlib_node_get_nodes (0 /* vm, for barrier sync */,
 		       (u32) ~0 /* all threads */, 1 /* include stats */,
 		       0 /* barrier sync */, &node_dups, &stat_vms);
 
-  u32 l = vec_len (node_dups[0]);
-  u8 *symlink_name = 0;
+  n_nodes = vec_len (node_dups[0]);
+
+  vec_validate (node_data, n_nodes - 1);
 
-  /*
-   * Extend performance nodes if necessary
-   */
-  if (l > no_max_nodes)
+  for (i = 0; i < n_nodes; i++)
+    if (vec_is_equal (node_data[i].name, node_dups[0][i]) == 0)
+      bmp = clib_bitmap_set (bmp, i, 1);
+
+  if (bmp)
     {
       u32 last_thread = vlib_get_n_threads ();
-      void *oldheap = clib_mem_set_heap (sm->heap);
       vlib_stats_segment_lock ();
+      clib_bitmap_foreach (i, bmp)
+	{
+	  vlib_node_t *n = node_dups[0][i];
+	  if (node_data[i].name)
+	    {
+	      vec_free (node_data[i].name);
+	      for (j = 0; j < ARRAY_LEN (node_data->symlinks); j++)
+		vlib_stats_remove_entry (node_data[i].symlinks[j]);
+	    }
 
-      vlib_stats_validate (STAT_COUNTER_NODE_CLOCKS, last_thread, l - 1);
-      vlib_stats_validate (STAT_COUNTER_NODE_VECTORS, last_thread, l - 1);
-      vlib_stats_validate (STAT_COUNTER_NODE_CALLS, last_thread, l - 1);
-      vlib_stats_validate (STAT_COUNTER_NODE_SUSPENDS, last_thread, l - 1);
-
-      vec_validate (sm->nodes, l - 1);
-      vlib_stats_entry_t *ep;
-      ep = &sm->directory_vector[STAT_COUNTER_NODE_NAMES];
-      ep->data = sm->nodes;
+	  node_data[i].name = vec_dup (node_dups[0][i]->name);
+	  vlib_stats_set_string_vector (&node_names, n->index, "%v", n->name);
 
-      /* Update names dictionary */
-      vlib_node_t **nodes = node_dups[0];
-      int i;
-      for (i = 0; i < vec_len (nodes); i++)
-	{
-	  vlib_node_t *n = nodes[i];
-	  u8 *s = format (0, "%v%c", n->name, 0);
-	  if (sm->nodes[n->index])
-	    vec_free (sm->nodes[n->index]);
-	  sm->nodes[n->index] = s;
-
-	  oldheap = clib_mem_set_heap (oldheap);
-#define _(E, t, name, p)                                                      \
-  vlib_stats_add_symlink (STAT_COUNTER_##E, n->index, "/nodes/%U/" #name,     \
-			  format_vlib_stats_symlink, s);
-	  foreach_stat_segment_node_counter_name
-#undef _
-	    oldheap = clib_mem_set_heap (oldheap);
+	  for (int j = 0; j < ARRAY_LEN (node_counters); j++)
+	    {
+	      vlib_stats_validate (node_counters[j].entry_index, last_thread,
+				   n_nodes - 1);
+	      node_data[i].symlinks[j] = vlib_stats_add_symlink (
+		node_counters[j].entry_index, n->index, "/nodes/%U/%s",
+		format_vlib_stats_symlink, n->name, node_counters[j].name);
+	      ASSERT (node_data[i].symlinks[j] != CLIB_U32_MAX);
+	    }
 	}
-
       vlib_stats_segment_unlock ();
-      clib_mem_set_heap (oldheap);
-      no_max_nodes = l;
+      vec_free (bmp);
     }
 
   for (j = 0; j < vec_len (node_dups); j++)
@@ -75,48 +96,23 @@ update_node_counters (vlib_stats_segment_t *sm)
 	  counter_t *c;
 	  vlib_node_t *n = nodes[i];
 
-	  if (j == 0)
-	    {
-	      if (strncmp ((char *) sm->nodes[n->index], (char *) n->name,
-			   strlen ((char *) sm->nodes[n->index])))
-		{
-		  u32 vector_index;
-		  void *oldheap = clib_mem_set_heap (sm->heap);
-		  vlib_stats_segment_lock ();
-		  u8 *s = format (0, "%v%c", n->name, 0);
-		  clib_mem_set_heap (oldheap);
-#define _(E, t, name, p)                                                      \
-  vec_reset_length (symlink_name);                                            \
-  symlink_name = format (symlink_name, "/nodes/%U/" #name,                    \
-			 format_vlib_stats_symlink, sm->nodes[n->index]);     \
-  vector_index = vlib_stats_find_entry_index ("%v", symlink_name);            \
-  ASSERT (vector_index != -1);                                                \
-  vlib_stats_rename_symlink (vector_index, "/nodes/%U/" #name,                \
-			     format_vlib_stats_symlink, s);
-		  foreach_stat_segment_node_counter_name
-#undef _
-		    vec_free (symlink_name);
-		  clib_mem_set_heap (sm->heap);
-		  vec_free (sm->nodes[n->index]);
-		  sm->nodes[n->index] = s;
-		  vlib_stats_segment_unlock ();
-		  clib_mem_set_heap (oldheap);
-		}
-	    }
-
-	  counters = sm->directory_vector[STAT_COUNTER_NODE_CLOCKS].data;
+	  counters = vlib_stats_get_entry_data_pointer (
+	    node_counters[NODE_CLOCKS].entry_index);
 	  c = counters[j];
 	  c[n->index] = n->stats_total.clocks - n->stats_last_clear.clocks;
 
-	  counters = sm->directory_vector[STAT_COUNTER_NODE_VECTORS].data;
+	  counters = vlib_stats_get_entry_data_pointer (
+	    node_counters[NODE_VECTORS].entry_index);
 	  c = counters[j];
 	  c[n->index] = n->stats_total.vectors - n->stats_last_clear.vectors;
 
-	  counters = sm->directory_vector[STAT_COUNTER_NODE_CALLS].data;
+	  counters = vlib_stats_get_entry_data_pointer (
+	    node_counters[NODE_CALLS].entry_index);
 	  c = counters[j];
 	  c[n->index] = n->stats_total.calls - n->stats_last_clear.calls;
 
-	  counters = sm->directory_vector[STAT_COUNTER_NODE_SUSPENDS].data;
+	  counters = vlib_stats_get_entry_data_pointer (
+	    node_counters[NODE_SUSPENDS].entry_index);
 	  c = counters[j];
 	  c[n->index] = n->stats_total.suspends - n->stats_last_clear.suspends;
 	}
@@ -154,6 +150,19 @@ stat_segment_collector_process (vlib_main_t *vm, vlib_node_runtime_t *rt,
 {
   vlib_stats_segment_t *sm = vlib_stats_get_segment ();
 
+  if (sm->node_counters_enabled)
+    {
+      node_names = vlib_stats_add_string_vector ("/sys/node/names");
+      ASSERT (node_names);
+
+      for (int x = 0; x < ARRAY_LEN (node_counters); x++)
+	{
+	  node_counters[x].entry_index = vlib_stats_add_counter_vector (
+	    "/sys/node/%s", node_counters[x].name);
+	  ASSERT (node_counters[x].entry_index != CLIB_U32_MAX);
+	}
+    }
+
   while (1)
     {
       do_stat_segment_updates (vm, sm);
diff --git a/src/vlib/stats/format.c b/src/vlib/stats/format.c
index 419ff6b4657..54c11aff743 100644
--- a/src/vlib/stats/format.c
+++ b/src/vlib/stats/format.c
@@ -9,16 +9,13 @@
 u8 *
 format_vlib_stats_symlink (u8 *s, va_list *args)
 {
-  char *input = va_arg (*args, char *);
-  char *modified_input = vec_dup (input);
-  int i;
-  u8 *result;
+  u8 *input = va_arg (*args, u8 *);
 
-  for (i = 0; i < strlen (modified_input); i++)
-    if (modified_input[i] == '/')
-      modified_input[i] = '_';
+  for (int i = 0; i < vec_len (input); i++)
+    if (input[i] == '/')
+      vec_add1 (s, '_');
+    else
+      vec_add1 (s, input[i]);
 
-  result = format (s, "%s", modified_input);
-  vec_free (modified_input);
-  return result;
+  return s;
 }
diff --git a/src/vlib/stats/stats.c b/src/vlib/stats/stats.c
index 61dadd74b1c..ee657089fd2 100644
--- a/src/vlib/stats/stats.c
+++ b/src/vlib/stats/stats.c
@@ -132,6 +132,7 @@ vlib_stats_remove_entry (u32 entry_index)
   vlib_stats_entry_t *e = vlib_stats_get_entry (sm, entry_index);
   counter_t **c;
   vlib_counter_t **vc;
+  void *oldheap;
   u32 i;
 
   if (entry_index >= vec_len (sm->directory_vector))
@@ -150,17 +151,21 @@ vlib_stats_remove_entry (u32 entry_index)
     case STAT_DIR_TYPE_COUNTER_VECTOR_SIMPLE:
       c = e->data;
       e->data = 0;
+      oldheap = clib_mem_set_heap (sm->heap);
       for (i = 0; i < vec_len (c); i++)
 	vec_free (c[i]);
       vec_free (c);
+      clib_mem_set_heap (oldheap);
       break;
 
     case STAT_DIR_TYPE_COUNTER_VECTOR_COMBINED:
       vc = e->data;
       e->data = 0;
+      oldheap = clib_mem_set_heap (sm->heap);
       for (i = 0; i < vec_len (vc); i++)
 	vec_free (vc[i]);
       vec_free (vc);
+      clib_mem_set_heap (oldheap);
       break;
 
     case STAT_DIR_TYPE_SCALAR_INDEX:
@@ -269,40 +274,77 @@ vlib_stats_set_timestamp (u32 entry_index, f64 value)
   sm->directory_vector[entry_index].value = value;
 }
 
-u32
+vlib_stats_string_vector_t
 vlib_stats_add_string_vector (char *fmt, ...)
 {
+  vlib_stats_segment_t *sm = vlib_stats_get_segment ();
   va_list va;
+  vlib_stats_header_t *sh;
+  vlib_stats_string_vector_t sv;
+  u32 index;
   u8 *name;
 
   va_start (va, fmt);
   name = va_format (0, fmt, &va);
   va_end (va);
-  return vlib_stats_new_entry_internal (STAT_DIR_TYPE_NAME_VECTOR, name);
+
+  index = vlib_stats_new_entry_internal (STAT_DIR_TYPE_NAME_VECTOR, name);
+  if (index == CLIB_U32_MAX)
+    return 0;
+
+  sv = vec_new_generic (vlib_stats_string_vector_t, 0,
+			sizeof (vlib_stats_header_t), 0, sm->heap);
+  sh = vec_header (sv);
+  sh->entry_index = index;
+  sm->directory_vector[index].string_vector = sv;
+  return sv;
 }
 
 void
-vlib_stats_set_string_vector (u32 entry_index, u32 vector_index, char *fmt,
-			      ...)
+vlib_stats_set_string_vector (vlib_stats_string_vector_t *svp,
+			      u32 vector_index, char *fmt, ...)
 {
   vlib_stats_segment_t *sm = vlib_stats_get_segment ();
-  vlib_stats_entry_t *e = vlib_stats_get_entry (sm, entry_index);
+  vlib_stats_header_t *sh = vec_header (*svp);
+  vlib_stats_entry_t *e = vlib_stats_get_entry (sm, sh->entry_index);
   va_list va;
-  void *oldheap;
+  u8 *s;
+
+  if (fmt[0] == 0)
+    {
+      if (vec_len (e->string_vector) <= vector_index)
+	return;
+
+      if (e->string_vector[vector_index] == 0)
+	return;
+
+      vlib_stats_segment_lock ();
+      vec_free (e->string_vector[vector_index]);
+      vlib_stats_segment_unlock ();
+      return;
+    }
 
-  oldheap = clib_mem_set_heap (sm->heap);
   vlib_stats_segment_lock ();
 
+  ASSERT (e->string_vector);
+
   vec_validate (e->string_vector, vector_index);
-  vec_reset_length (e->string_vector[vector_index]);
+  svp[0] = e->string_vector;
+
+  s = e->string_vector[vector_index];
+
+  if (s == 0)
+    s = vec_new_heap (u8 *, 0, sm->heap);
+
+  vec_reset_length (s);
 
   va_start (va, fmt);
-  e->string_vector[vector_index] =
-    va_format (e->string_vector[vector_index], fmt, &va);
+  s = va_format (s, fmt, &va);
   va_end (va);
 
+  e->string_vector[vector_index] = s;
+
   vlib_stats_segment_unlock ();
-  clib_mem_set_heap (oldheap);
 }
 
 u32
diff --git a/src/vlib/stats/stats.h b/src/vlib/stats/stats.h
index 09a9aef002d..5f5581ca62b 100644
--- a/src/vlib/stats/stats.h
+++ b/src/vlib/stats/stats.h
@@ -21,25 +21,12 @@ typedef enum
 {
   STAT_COUNTER_HEARTBEAT = 0,
   STAT_COUNTER_LAST_STATS_CLEAR,
-  STAT_COUNTER_NODE_CLOCKS,
-  STAT_COUNTER_NODE_VECTORS,
-  STAT_COUNTER_NODE_CALLS,
-  STAT_COUNTER_NODE_SUSPENDS,
-  STAT_COUNTER_NODE_NAMES,
   STAT_COUNTERS
 } stat_segment_counter_t;
 
-#define foreach_stat_segment_node_counter_name                                \
-  _ (NODE_CLOCKS, COUNTER_VECTOR_SIMPLE, clocks, "/sys/node")                 \
-  _ (NODE_VECTORS, COUNTER_VECTOR_SIMPLE, vectors, "/sys/node")               \
-  _ (NODE_CALLS, COUNTER_VECTOR_SIMPLE, calls, "/sys/node")                   \
-  _ (NODE_SUSPENDS, COUNTER_VECTOR_SIMPLE, suspends, "/sys/node")
-
 #define foreach_stat_segment_counter_name                                     \
   _ (LAST_STATS_CLEAR, SCALAR_INDEX, last_stats_clear, "/sys")                \
-  _ (HEARTBEAT, SCALAR_INDEX, heartbeat, "/sys")                              \
-  _ (NODE_NAMES, NAME_VECTOR, names, "/sys/node")                             \
-  foreach_stat_segment_node_counter_name
+  _ (HEARTBEAT, SCALAR_INDEX, heartbeat, "/sys")
 
 typedef struct
 {
@@ -76,7 +63,6 @@ typedef struct
   uword *directory_vector_by_name;
   vlib_stats_entry_t *directory_vector;
   u32 dir_vector_first_free_elt;
-  u8 **nodes;
 
   /* Update interval */
   f64 update_interval;
@@ -98,6 +84,11 @@ typedef struct
 
 typedef struct
 {
+  u32 entry_index;
+} vlib_stats_header_t;
+
+typedef struct
+{
   vlib_stats_segment_t segment;
 } vlib_stats_main_t;
 
@@ -149,8 +140,9 @@ u32 vlib_stats_add_counter_vector (char *fmt, ...);
 u32 vlib_stats_add_counter_pair_vector (char *fmt, ...);
 
 /* string vector */
-u32 vlib_stats_add_string_vector (char *fmt, ...);
-void vlib_stats_set_string_vector (u32 entry_index, u32 vector_index,
+typedef u8 **vlib_stats_string_vector_t;
+vlib_stats_string_vector_t vlib_stats_add_string_vector (char *fmt, ...);
+void vlib_stats_set_string_vector (vlib_stats_string_vector_t *sv, u32 index,
 				   char *fmt, ...);
 
 /* symlink */
diff --git a/src/vnet/interface/stats.c b/src/vnet/interface/stats.c
index 55e49eb1bc2..f58ffa32586 100644
--- a/src/vnet/interface/stats.c
+++ b/src/vnet/interface/stats.c
@@ -9,7 +9,7 @@
 #include <vnet/devices/devices.h> /* vnet_get_aggregate_rx_packets */
 #include <vnet/interface.h>
 
-static u32 if_names_stats_entry_index = ~0;
+vlib_stats_string_vector_t if_names = 0;
 static u32 **dir_entry_indices = 0;
 
 static struct
@@ -25,21 +25,15 @@ static struct
 static clib_error_t *
 statseg_sw_interface_add_del (vnet_main_t *vnm, u32 sw_if_index, u32 is_add)
 {
-  vlib_stats_segment_t *sm = vlib_stats_get_segment ();
-  vlib_stats_entry_t *e;
-  void *oldheap;
-
-  if (if_names_stats_entry_index == ~0)
+  if (if_names == 0)
     {
-      if_names_stats_entry_index = vlib_stats_add_string_vector ("/if/names");
+      if_names = vlib_stats_add_string_vector ("/if/names");
 
       for (int i = 0; i < ARRAY_LEN (if_counters); i++)
 	if_counters[i].index = vlib_stats_find_entry_index (
 	  "/%s/%s", if_counters[i].prefix, if_counters[i].name);
     }
 
-  e = sm->directory_vector + if_names_stats_entry_index;
-
   vec_validate (dir_entry_indices, sw_if_index);
 
   vlib_stats_segment_lock ();
@@ -48,40 +42,32 @@ statseg_sw_interface_add_del (vnet_main_t *vnm, u32 sw_if_index, u32 is_add)
     {
       vnet_sw_interface_t *si, *si_sup;
       vnet_hw_interface_t *hi_sup;
-      u8 *s;
+      u8 *name;
 
       si = vnet_get_sw_interface (vnm, sw_if_index);
       si_sup = vnet_get_sup_sw_interface (vnm, si->sw_if_index);
       ASSERT (si_sup->type == VNET_SW_INTERFACE_TYPE_HARDWARE);
       hi_sup = vnet_get_hw_interface (vnm, si_sup->hw_if_index);
 
-      oldheap = clib_mem_set_heap (sm->heap);
-      s = format (0, "%v", hi_sup->name);
+      name = format (0, "%v", hi_sup->name);
       if (si->type != VNET_SW_INTERFACE_TYPE_HARDWARE)
-	s = format (s, ".%d", si->sub.id);
-      s = format (s, "%c", 0);
+	name = format (name, ".%d", si->sub.id);
 
-      vec_validate (e->string_vector, sw_if_index);
+      vlib_stats_set_string_vector (&if_names, sw_if_index, "%v", name);
 
-      ASSERT (e->string_vector[sw_if_index] == 0);
-      e->string_vector[sw_if_index] = s;
-      clib_mem_set_heap (oldheap);
-
-      s = format (0, "/interfaces/%U", format_vlib_stats_symlink, s);
       for (u32 index, i = 0; i < ARRAY_LEN (if_counters); i++)
 	{
-	  index = vlib_stats_add_symlink (if_counters[i].index, sw_if_index,
-					  "%v/%s", s, if_counters[i].name);
+	  index = vlib_stats_add_symlink (
+	    if_counters[i].index, sw_if_index, "/interfaces/%U/%s",
+	    format_vlib_stats_symlink, name, if_counters[i].name);
 	  ASSERT (index != ~0);
 	  vec_add1 (dir_entry_indices[sw_if_index], index);
 	}
-      vec_free (s);
+
+      vec_free (name);
     }
   else
     {
-      oldheap = clib_mem_set_heap (sm->heap);
-      vec_free (e->string_vector[sw_if_index]);
-      clib_mem_set_heap (oldheap);
       for (u32 i = 0; i < vec_len (dir_entry_indices[sw_if_index]); i++)
 	vlib_stats_remove_entry (dir_entry_indices[sw_if_index][i]);
       vec_free (dir_entry_indices[sw_if_index]);