Clean up dpdk plugin rx/tx pcap tracing

Needed a spinlock to protect the data vector. Cleaned up debug cli so
the output makes sense, and so that various parameters exist in one
place. Removed a nonsense memset-to-zero which led to ultra-confusing
results.

Change-Id: I91cd14ce7fe84fd2eceab86e016b5ee001993be4
Signed-off-by: Dave Barach <dbarach@cisco.com>

diff --git a/src/plugins/dpdk/device/cli.c b/src/plugins/dpdk/device/cli.c
index 2a49771..40ec323 100644 (file)
--- a/src/plugins/dpdk/device/cli.c
+++ b/src/plugins/dpdk/device/cli.c
@@ -147,7 +147,7 @@ pcap_trace_command_internal (vlib_main_t * vm,
                    clib_error_report (error);
                  else
                    vlib_cli_output (vm, "saved to %s...",
-                                    dm->pcap[rx_tx].pcap_filename);
+                                    dm->pcap[rx_tx].pcap_main.file_name);
                }
 
              dm->pcap[rx_tx].pcap_enable = 0;
@@ -211,23 +211,25 @@ pcap_trace_command_internal (vlib_main_t * vm,
            {
              vlib_cli_output
                (vm, "max is %d for any interface to file %s",
-                dm->pcap_pkts_to_capture ?
-                dm->pcap[rx_tx].pcap_pkts_to_capture
+                dm->pcap[rx_tx].pcap_main.n_packets_to_capture ?
+                dm->pcap[rx_tx].pcap_main.n_packets_to_capture
                 : PCAP_DEF_PKT_TO_CAPTURE,
-                dm->pcap_filename ?
-                dm->pcap[rx_tx].pcap_filename : (u8 *) "/tmp/vpe.pcap");
+                dm->pcap[rx_tx].pcap_main.file_name ?
+                (u8 *) dm->pcap[rx_tx].pcap_main.file_name :
+                (u8 *) "/tmp/vpe.pcap");
            }
          else
            {
              vlib_cli_output (vm, "max is %d for interface %U to file %s",
-                              dm->pcap[rx_tx].pcap_pkts_to_capture
-                              ? dm->pcap_pkts_to_capture
-                              : PCAP_DEF_PKT_TO_CAPTURE,
+                              dm->pcap[rx_tx].pcap_main.n_packets_to_capture
+                              ? dm->pcap[rx_tx].
+                              pcap_main.n_packets_to_capture :
+                              PCAP_DEF_PKT_TO_CAPTURE,
                               format_vnet_sw_if_index_name, dm->vnet_main,
                               dm->pcap_sw_if_index,
-                              dm->pcap[rx_tx].pcap_filename
-                              ? dm->pcap[rx_tx].pcap_filename : (u8 *)
-                              "/tmp/vpe.pcap");
+                              dm->pcap[rx_tx].
+                              pcap_main.file_name ? (u8 *) dm->pcap[rx_tx].
+                              pcap_main.file_name : (u8 *) "/tmp/vpe.pcap");
            }
 
          if (dm->pcap[rx_tx].pcap_enable == 0)
@@ -262,34 +264,28 @@ pcap_trace_command_internal (vlib_main_t * vm,
       /* Since no error, save configured values. */
       if (chroot_filename)
        {
-         if (dm->pcap[rx_tx].pcap_filename)
-           vec_free (dm->pcap[rx_tx].pcap_filename);
+         if (dm->pcap[rx_tx].pcap_main.file_name)
+           vec_free (dm->pcap[rx_tx].pcap_main.file_name);
          vec_add1 (chroot_filename, 0);
-         dm->pcap[rx_tx].pcap_filename = chroot_filename;
+         dm->pcap[rx_tx].pcap_main.file_name = (char *) chroot_filename;
        }
 
       if (max)
-       dm->pcap[rx_tx].pcap_pkts_to_capture = max;
-
+       dm->pcap[rx_tx].pcap_main.n_packets_to_capture = max;
 
       if (enabled)
        {
-         if (dm->pcap[rx_tx].pcap_filename == 0)
-           dm->pcap[rx_tx].pcap_filename = format (0, "/tmp/vpe.pcap%c", 0);
-
-         memset (&dm->pcap[rx_tx].pcap_main, 0,
-                 sizeof (dm->pcap[rx_tx].pcap_main));
-         dm->pcap[rx_tx].pcap_main.file_name =
-           (char *) dm->pcap[rx_tx].pcap_filename;
-         dm->pcap[rx_tx].pcap_main.n_packets_to_capture
-           = PCAP_DEF_PKT_TO_CAPTURE;
-         if (dm->pcap[rx_tx].pcap_pkts_to_capture)
-           dm->pcap[rx_tx].pcap_main.n_packets_to_capture
-             = dm->pcap[rx_tx].pcap_pkts_to_capture;
+         if (dm->pcap[rx_tx].pcap_main.file_name == 0)
+           dm->pcap[rx_tx].pcap_main.file_name
+             = (char *) format (0, "/tmp/vpe.pcap%c", 0);
 
+         dm->pcap[rx_tx].pcap_main.n_packets_captured = 0;
          dm->pcap[rx_tx].pcap_main.packet_type = PCAP_PACKET_TYPE_ethernet;
+         if (dm->pcap[rx_tx].pcap_main.lock == 0)
+           clib_spinlock_init (&(dm->pcap[rx_tx].pcap_main.lock));
          dm->pcap[rx_tx].pcap_enable = 1;
-         vlib_cli_output (vm, "pcap tx capture on...");
+         vlib_cli_output (vm, "pcap %s capture on...",
+                          rx_tx == VLIB_RX ? "rx" : "tx");
        }
     }
   else if (chroot_filename)

diff --git a/src/plugins/dpdk/device/dpdk.h b/src/plugins/dpdk/device/dpdk.h
index f09a69c..2a37947 100644 (file)
--- a/src/plugins/dpdk/device/dpdk.h
+++ b/src/plugins/dpdk/device/dpdk.h
@@ -391,10 +391,8 @@ typedef struct
 typedef struct
 {
   int pcap_enable;
-  pcap_main_t pcap_main;
-  u8 *pcap_filename;
   u32 pcap_sw_if_index;
-  u32 pcap_pkts_to_capture;
+  pcap_main_t pcap_main;
 } dpdk_pcap_t;
 
 typedef struct

diff --git a/src/vnet/unix/pcap.c b/src/vnet/unix/pcap.c
index 473430a..e91b879 100644 (file)
--- a/src/vnet/unix/pcap.c
+++ b/src/vnet/unix/pcap.c
@@ -110,6 +110,7 @@ pcap_write (pcap_main_t * pm)
       pm->flags |= PCAP_MAIN_INIT_DONE;
       pm->n_packets_captured = 0;
       pm->n_pcap_data_written = 0;
+      clib_spinlock_init (&pm->lock);
 
       /* Write file header. */
       memset (&fh, 0, sizeof (fh));

diff --git a/src/vnet/unix/pcap.h b/src/vnet/unix/pcap.h
index 1ab1531..2c174fb 100644 (file)
--- a/src/vnet/unix/pcap.h
+++ b/src/vnet/unix/pcap.h
@@ -123,6 +123,9 @@ typedef struct
  */
 typedef struct
 {
+  /** spinlock to protect e.g. pcap_data */
+  clib_spinlock_t lock;
+
   /** File name of pcap output. */
   char *file_name;
 
@@ -213,6 +216,7 @@ pcap_add_buffer (pcap_main_t * pm,
 
   if (PREDICT_TRUE (pm->n_packets_captured < pm->n_packets_to_capture))
     {
+      clib_spinlock_lock_if_init (&pm->lock);
       d = pcap_add_packet (pm, time_now, n_left, n);
       while (1)
        {
@@ -225,6 +229,7 @@ pcap_add_buffer (pcap_main_t * pm,
          ASSERT (b->flags & VLIB_BUFFER_NEXT_PRESENT);
          b = vlib_get_buffer (vm, b->next_buffer);
        }
+      clib_spinlock_unlock_if_init (&pm->lock);
     }
 }