vlib: clean up the "pcap dispatch trace" debug CLI

Separate debug CLI arg parsing from the underlying action
function. Fixes a number of subtle ordering dependencies, and will
allow us to add a binary API to control the feature at some point in
the future.

Type: refactor
Ticket: VPP-1762

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

2 files changed, 183 insertions, 138 deletions

diff --git a/src/vlib/main.c b/src/vlib/main.c
index 66c0023cc94..1c6b9ba6329 100644
--- a/src/vlib/main.c
+++ b/src/vlib/main.c
@@ -2159,171 +2159,203 @@ done:
   return 0;
 }
 
-static inline clib_error_t *
-pcap_dispatch_trace_command_internal (vlib_main_t * vm,
-				      unformat_input_t * input,
-				      vlib_cli_command_t * cmd, int rx_tx)
+int
+vlib_pcap_dispatch_trace_configure (vlib_pcap_dispatch_trace_args_t * a)
 {
-  unformat_input_t _line_input, *line_input = &_line_input;
+  vlib_main_t *vm = vlib_get_main ();
   pcap_main_t *pm = &vm->dispatch_pcap_main;
-  u8 *filename = 0;
-  u32 max = 1000;
-  int enabled = 0;
-  int is_error = 0;
-  clib_error_t *error = 0;
-  u32 node_index, add;
   vlib_trace_main_t *tm;
   vlib_trace_node_t *tn;
 
-  /* Get a line of input. */
-  if (!unformat_user (input, unformat_line_input, line_input))
-    return 0;
-
-  while (unformat_check_input (line_input) != UNFORMAT_END_OF_INPUT)
+  if (a->status)
     {
-      if (unformat (line_input, "on"))
-	{
-	  if (vm->dispatch_pcap_enable == 0)
-	    {
-	      enabled = 1;
-	    }
-	  else
-	    {
-	      vlib_cli_output (vm, "pcap dispatch capture already on...");
-	      is_error = 1;
-	      break;
-	    }
-	}
-      else if (unformat (line_input, "off"))
-	{
-	  if (vm->dispatch_pcap_enable)
-	    {
-	      vlib_cli_output
-		(vm, "captured %d pkts...", pm->n_packets_captured);
-	      if (pm->n_packets_captured)
-		{
-		  pm->n_packets_to_capture = pm->n_packets_captured;
-		  error = pcap_write (pm);
-		  if (pm->file_descriptor >= 0)
-		    pcap_close (pm);
-		  if (error)
-		    clib_error_report (error);
-		  else
-		    vlib_cli_output (vm, "saved to %s...", pm->file_name);
-		}
-	      vm->dispatch_pcap_enable = 0;
-	    }
-	  else
-	    {
-	      vlib_cli_output (vm, "pcap tx capture already off...");
-	      is_error = 1;
-	      break;
-	    }
-	}
-      else if (unformat (line_input, "max %d", &max))
+      if (vm->dispatch_pcap_enable)
 	{
-	  if (vm->dispatch_pcap_enable)
+	  int i;
+	  vlib_cli_output
+	    (vm, "pcap dispatch capture enabled: %d of %d pkts...",
+	     pm->n_packets_captured, pm->n_packets_to_capture);
+	  vlib_cli_output (vm, "capture to file %s", pm->file_name);
+
+	  for (i = 0; i < vec_len (vm->dispatch_buffer_trace_nodes); i++)
 	    {
-	      vlib_cli_output
-		(vm,
-		 "can't change max value while pcap tx capture active...");
-	      is_error = 1;
-	      break;
+	      vlib_cli_output (vm,
+			       "Buffer trace of %d pkts from %U enabled...",
+			       a->buffer_traces_to_capture,
+			       format_vlib_node_name, vm,
+			       vm->dispatch_buffer_trace_nodes[i]);
 	    }
-	  pm->n_packets_to_capture = max;
 	}
       else
-	if (unformat
-	    (line_input, "file %U", unformat_vlib_tmpfile, &filename))
-	{
-	  if (vm->dispatch_pcap_enable)
-	    {
-	      vlib_cli_output
-		(vm, "can't change file while pcap tx capture active...");
-	      is_error = 1;
-	      break;
-	    }
-	}
-      else if (unformat (line_input, "status"))
-	{
-	  if (vm->dispatch_pcap_enable)
-	    {
-	      vlib_cli_output
-		(vm, "pcap dispatch capture is on: %d of %d pkts...",
-		 pm->n_packets_captured, pm->n_packets_to_capture);
-	      vlib_cli_output (vm, "Capture to file %s", pm->file_name);
-	    }
-	  else
-	    {
-	      vlib_cli_output (vm, "pcap dispatch capture is off...");
-	    }
-	  break;
-	}
-      else if (unformat (line_input, "buffer-trace %U %d",
-			 unformat_vlib_node, vm, &node_index, &add))
-	{
-	  if (vnet_trace_dummy == 0)
-	    vec_validate_aligned (vnet_trace_dummy, 2048,
-				  CLIB_CACHE_LINE_BYTES);
-	  vlib_cli_output (vm, "Buffer tracing of %d pkts from %U enabled...",
-			   add, format_vlib_node_name, vm, node_index);
+	vlib_cli_output (vm, "pcap dispatch capture disabled");
+      return 0;
+    }
 
-          /* *INDENT-OFF* */
-          foreach_vlib_main ((
-            {
-              tm = &this_vlib_main->trace_main;
-              tm->verbose = 0;  /* not sure this ever did anything... */
-              vec_validate (tm->nodes, node_index);
-              tn = tm->nodes + node_index;
-              tn->limit += add;
-              tm->trace_enable = 1;
-            }));
-          /* *INDENT-ON* */
-	}
+  /* Consistency checks */
 
-      else
-	{
-	  error = clib_error_return (0, "unknown input `%U'",
-				     format_unformat_error, line_input);
-	  is_error = 1;
-	  break;
-	}
+  /* Enable w/ capture already enabled not allowed */
+  if (vm->dispatch_pcap_enable && a->enable)
+    return -7;			/* VNET_API_ERROR_INVALID_VALUE */
+
+  /* Disable capture with capture already disabled, not interesting */
+  if (vm->dispatch_pcap_enable == 0 && a->enable == 0)
+    return -81;			/* VNET_API_ERROR_VALUE_EXIST */
+
+  /* Change number of packets to capture while capturing */
+  if (vm->dispatch_pcap_enable
+      && (pm->n_packets_to_capture != a->packets_to_capture))
+    return -8;			/* VNET_API_ERROR_INVALID_VALUE_2 */
+
+  /* Independent of enable/disable, to allow buffer trace multi nodes */
+  if (a->buffer_trace_node_index != ~0)
+    {
+      /* *INDENT-OFF* */
+      foreach_vlib_main ((
+        {
+          tm = &this_vlib_main->trace_main;
+          tm->verbose = 0;  /* not sure this ever did anything... */
+          vec_validate (tm->nodes, a->buffer_trace_node_index);
+          tn = tm->nodes + a->buffer_trace_node_index;
+          tn->limit += a->buffer_traces_to_capture;
+          tm->trace_enable = 1;
+        }));
+      /* *INDENT-ON* */
+      vec_add1 (vm->dispatch_buffer_trace_nodes, a->buffer_trace_node_index);
     }
-  unformat_free (line_input);
 
-  if (is_error == 0)
+  if (a->enable)
     {
-      /* Clean up from previous run */
+      /* Clean up from previous run, if any */
       vec_free (pm->file_name);
       vec_free (pm->pcap_data);
-
       memset (pm, 0, sizeof (*pm));
-      pm->n_packets_to_capture = max;
 
-      if (enabled)
+      vec_validate_aligned (vnet_trace_dummy, 2048, CLIB_CACHE_LINE_BYTES);
+      if (pm->lock == 0)
+	clib_spinlock_init (&(pm->lock));
+
+      if (a->filename == 0)
+	a->filename = format (0, "/tmp/dispatch.pcap%c", 0);
+
+      pm->file_name = (char *) a->filename;
+      pm->n_packets_captured = 0;
+      pm->packet_type = PCAP_PACKET_TYPE_vpp;
+      vm->dispatch_pcap_enable = 1;
+      pm->n_packets_to_capture = a->packets_to_capture;
+    }
+  else
+    {
+      vm->dispatch_pcap_enable = 0;
+      vec_reset_length (vm->dispatch_buffer_trace_nodes);
+      if (pm->n_packets_captured)
 	{
-	  if (filename == 0)
-	    filename = format (0, "/tmp/dispatch.pcap%c", 0);
-
-	  pm->file_name = (char *) filename;
-	  pm->n_packets_captured = 0;
-	  pm->packet_type = PCAP_PACKET_TYPE_vpp;
-	  if (pm->lock == 0)
-	    clib_spinlock_init (&(pm->lock));
-	  vm->dispatch_pcap_enable = 1;
-	  vlib_cli_output (vm, "pcap dispatch capture on...");
+	  clib_error_t *error;
+	  pm->n_packets_to_capture = pm->n_packets_captured;
+	  vlib_cli_output (vm, "Write %d packets to %s, and stop capture...",
+			   pm->n_packets_captured, pm->file_name);
+	  error = pcap_write (pm);
+	  if (pm->file_descriptor >= 0)
+	    pcap_close (pm);
+	  /* Report I/O errors... */
+	  if (error)
+	    {
+	      clib_error_report (error);
+	      return -11;	/* VNET_API_ERROR_SYSCALL_ERROR_1 */
+	    }
+	  return 0;
 	}
+      else
+	return -6;		/* VNET_API_ERROR_NO_SUCH_ENTRY */
     }
 
-  return error;
+  return 0;
 }
 
 static clib_error_t *
-pcap_dispatch_trace_command_fn (vlib_main_t * vm,
-				unformat_input_t * input,
-				vlib_cli_command_t * cmd)
+dispatch_trace_command_fn (vlib_main_t * vm,
+			   unformat_input_t * input, vlib_cli_command_t * cmd)
 {
-  return pcap_dispatch_trace_command_internal (vm, input, cmd, VLIB_RX);
+  unformat_input_t _line_input, *line_input = &_line_input;
+  vlib_pcap_dispatch_trace_args_t _a, *a = &_a;
+  u8 *filename = 0;
+  u32 max = 1000;
+  int rv;
+  int enable = 0;
+  int status = 0;
+  u32 node_index = ~0, buffer_traces_to_capture = 100;
+
+  /* Get a line of input. */
+  if (!unformat_user (input, unformat_line_input, line_input))
+    return 0;
+
+  while (unformat_check_input (line_input) != UNFORMAT_END_OF_INPUT)
+    {
+      if (unformat (line_input, "on %=", &enable, 1))
+	;
+      else if (unformat (line_input, "enable %=", &enable, 1))
+	;
+      else if (unformat (line_input, "off %=", &enable, 0))
+	;
+      else if (unformat (line_input, "disable %=", &enable, 0))
+	;
+      else if (unformat (line_input, "max %d", &max))
+	;
+      else if (unformat (line_input, "packets-to-capture %d", &max))
+	;
+      else if (unformat (line_input, "file %U", unformat_vlib_tmpfile,
+			 &filename))
+	;
+      else if (unformat (line_input, "status %=", &status, 1))
+	;
+      else if (unformat (line_input, "buffer-trace %U %d",
+			 unformat_vlib_node, vm, &node_index,
+			 &buffer_traces_to_capture))
+	;
+      else
+	{
+	  return clib_error_return (0, "unknown input `%U'",
+				    format_unformat_error, line_input);
+	}
+    }
+
+  unformat_free (line_input);
+
+  /* no need for memset (a, 0, sizeof (*a)), set all fields here. */
+  a->filename = filename;
+  a->enable = enable;
+  a->status = status;
+  a->packets_to_capture = max;
+  a->buffer_trace_node_index = node_index;
+  a->buffer_traces_to_capture = buffer_traces_to_capture;
+
+  rv = vlib_pcap_dispatch_trace_configure (a);
+
+  switch (rv)
+    {
+    case 0:
+      break;
+
+    case -7:
+      return clib_error_return (0, "dispatch trace already enabled...");
+
+    case -81:
+      return clib_error_return (0, "dispatch trace already disabled...");
+
+    case -8:
+      return clib_error_return
+	(0, "can't change number of records to capture while tracing...");
+
+    case -11:
+      return clib_error_return (0, "I/O writing trace capture...");
+
+    case -6:
+      return clib_error_return (0, "No packets captured...");
+
+    default:
+      vlib_cli_output (vm, "WARNING: trace configure returned %d", rv);
+      break;
+    }
+  return 0;
 }
 
 /*?
@@ -2383,7 +2415,7 @@ VLIB_CLI_COMMAND (pcap_dispatch_trace_command, static) = {
     .short_help =
     "pcap dispatch trace [on|off] [max <nn>] [file <name>] [status]\n"
     "              [buffer-trace <input-node-name> <nn>]",
-    .function = pcap_dispatch_trace_command_fn,
+    .function = dispatch_trace_command_fn,
 };
 /* *INDENT-ON* */
 
diff --git a/src/vlib/main.h b/src/vlib/main.h
index dd28cb85e22..05687a8c987 100644
--- a/src/vlib/main.h
+++ b/src/vlib/main.h
@@ -148,6 +148,7 @@ typedef struct vlib_main_t
   /* Pcap dispatch trace main */
   pcap_main_t dispatch_pcap_main;
   uword dispatch_pcap_enable;
+  u32 *dispatch_buffer_trace_nodes;
   u8 *pcap_buffer;
 
   /* pcap rx / tx tracing */
@@ -403,6 +404,18 @@ extern void vlib_node_sync_stats (vlib_main_t * vm, vlib_node_t * n);
 #define VLIB_PCAP_MAJOR_VERSION 1
 #define VLIB_PCAP_MINOR_VERSION 0
 
+typedef struct
+{
+  u8 *filename;
+  int enable;
+  int status;
+  u32 packets_to_capture;
+  u32 buffer_trace_node_index;
+  u32 buffer_traces_to_capture;
+} vlib_pcap_dispatch_trace_args_t;
+
+int vlib_pcap_dispatch_trace_configure (vlib_pcap_dispatch_trace_args_t *);
+
 #endif /* included_vlib_main_h */
 
 /*