From: Dave Barach Date: Thu, 26 Jul 2018 16:27:27 +0000 (-0400) Subject: Clean up dpdk plugin rx/tx pcap tracing X-Git-Tag: v18.10-rc1~531 X-Git-Url: https://gerrit.fd.io/r/gitweb?p=vpp.git;a=commitdiff_plain;h=f91080c01104a5999fe6c08e699b3426fea62dad 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 --- diff --git a/src/plugins/dpdk/device/cli.c b/src/plugins/dpdk/device/cli.c index 2a49771ef26..40ec32325e4 100644 --- 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 f09a69c7f45..2a3794735d2 100644 --- 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 473430a670e..e91b8792c2a 100644 --- 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 1ab1531cfa3..2c174fbedf9 100644 --- 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); } }