From b97641c79f4aaf0069268c550f263167ddea2b34 Mon Sep 17 00:00:00 2001 From: Dave Barach Date: Mon, 9 Sep 2019 16:38:17 -0400 Subject: [PATCH] misc: clean up "pcap [rx|tx] 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-1770 Signed-off-by: Dave Barach Change-Id: Id0dbeda06dad20e756c941c691e2088ce3c50ec7 --- extras/pcapcli/setup.pcapcli | 34 +++++ src/vlib/main.c | 2 +- src/vnet/interface.h | 12 ++ src/vnet/interface_cli.c | 317 ++++++++++++++++++++----------------------- 4 files changed, 196 insertions(+), 169 deletions(-) create mode 100644 extras/pcapcli/setup.pcapcli diff --git a/extras/pcapcli/setup.pcapcli b/extras/pcapcli/setup.pcapcli new file mode 100644 index 00000000000..66f3caebf63 --- /dev/null +++ b/extras/pcapcli/setup.pcapcli @@ -0,0 +1,34 @@ +set term pag off +loop create +loop create +set int ip address loop0 192.168.1.1/24 +set int state loop0 up + +set int ip address loop1 192.168.2.1/24 +set int state loop1 up + +packet-generator new { + name pg0 + limit 1 + size 300-300 + interface loop0 + node ethernet-input + data { IP4: 1.2.3 -> 4.5.6 + UDP: 192.168.1.10 -> 192.168.2.10 + UDP: 1234 -> 2345 + incrementing 286 + } +} + +packet-generator new { + name pg1 + limit 1 + size 300-300 + interface loop1 + node ethernet-input + data { IP4: 1.2.3 -> 4.5.6 + UDP: 192.168.2.10 -> 192.168.1.10 + UDP: 1234 -> 2345 + incrementing 286 + } +} diff --git a/src/vlib/main.c b/src/vlib/main.c index 1c6b9ba6329..2df935a5e17 100644 --- a/src/vlib/main.c +++ b/src/vlib/main.c @@ -2202,7 +2202,7 @@ vlib_pcap_dispatch_trace_configure (vlib_pcap_dispatch_trace_args_t * a) return -81; /* VNET_API_ERROR_VALUE_EXIST */ /* Change number of packets to capture while capturing */ - if (vm->dispatch_pcap_enable + if (vm->dispatch_pcap_enable && a->enable && (pm->n_packets_to_capture != a->packets_to_capture)) return -8; /* VNET_API_ERROR_INVALID_VALUE_2 */ diff --git a/src/vnet/interface.h b/src/vnet/interface.h index e6418f9c273..42aada453c1 100644 --- a/src/vnet/interface.h +++ b/src/vnet/interface.h @@ -893,6 +893,18 @@ void vnet_register_format_buffer_opaque_helper void vnet_register_format_buffer_opaque2_helper (vnet_buffer_opquae_formatter_t fn); +typedef struct +{ + u8 *filename; + int enable; + int status; + u32 packets_to_capture; + vlib_rx_or_tx_t rxtx; + u32 sw_if_index; +} vnet_pcap_dispatch_trace_args_t; + +int vnet_pcap_dispatch_trace_configure (vnet_pcap_dispatch_trace_args_t *); + #endif /* included_vnet_interface_h */ /* diff --git a/src/vnet/interface_cli.c b/src/vnet/interface_cli.c index 8db26391573..95e1684be8b 100644 --- a/src/vnet/interface_cli.c +++ b/src/vnet/interface_cli.c @@ -1695,19 +1695,108 @@ VLIB_CLI_COMMAND (cmd_set_if_rx_placement,static) = { }; /* *INDENT-ON* */ -static inline clib_error_t * +int +vnet_pcap_dispatch_trace_configure (vnet_pcap_dispatch_trace_args_t * a) +{ + vlib_main_t *vm = vlib_get_main (); + vlib_rx_or_tx_t rxtx = a->rxtx; + vnet_pcap_t *pp = &vm->pcap[rxtx]; + pcap_main_t *pm = &pp->pcap_main; + + if (a->status) + { + if (pp->pcap_enable == 0) + { + vlib_cli_output + (vm, "pcap %s dispatch capture enabled: %d of %d pkts...", + (rxtx == VLIB_RX) ? "rx" : "tx", + 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 %s dispatch capture disabled", + (rxtx == VLIB_RX) ? "rx" : "tx"); + return 0; + } + + /* Consistency checks */ + + /* Enable w/ capture already enabled not allowed */ + if (pp->pcap_enable && a->enable) + return VNET_API_ERROR_INVALID_VALUE; + + /* Disable capture with capture already disabled, not interesting */ + if (pp->pcap_enable == 0 && a->enable == 0) + return VNET_API_ERROR_VALUE_EXIST; + + /* Change number of packets to capture while capturing */ + if (pp->pcap_enable && a->enable + && (pm->n_packets_to_capture != a->packets_to_capture)) + return VNET_API_ERROR_INVALID_VALUE_2; + + if (a->enable) + { + /* Clean up from previous run, if any */ + vec_free (pm->file_name); + vec_free (pm->pcap_data); + memset (pm, 0, sizeof (*pm)); + + 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/%s.pcap%c", + (rxtx == VLIB_RX) ? "rx" : "tx", 0); + + pm->file_name = (char *) a->filename; + pm->n_packets_captured = 0; + pm->packet_type = PCAP_PACKET_TYPE_ethernet; + pm->n_packets_to_capture = a->packets_to_capture; + pp->pcap_sw_if_index = a->sw_if_index; + pp->pcap_enable = 1; + } + else + { + pp->pcap_enable = 0; + if (pm->n_packets_captured) + { + 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 VNET_API_ERROR_SYSCALL_ERROR_1; + } + return 0; + } + else + return VNET_API_ERROR_NO_SUCH_ENTRY; + } + + return 0; +} + +static clib_error_t * pcap_trace_command_internal (vlib_main_t * vm, unformat_input_t * input, - vlib_cli_command_t * cmd, int rx_tx) + vlib_cli_command_t * cmd, vlib_rx_or_tx_t rxtx) { unformat_input_t _line_input, *line_input = &_line_input; - u8 *filename; - u8 *chroot_filename = 0; - u32 max = 0; - int enabled = 0; - int errorFlag = 0; - clib_error_t *error = 0; + vnet_pcap_dispatch_trace_args_t _a, *a = &_a; vnet_main_t *vnm = vnet_get_main (); + u8 *filename = 0; + u32 max = 1000; + int rv; + int enable = 0; + int status = 0; + u32 sw_if_index = 0; /* Get a line of input. */ if (!unformat_user (input, unformat_line_input, line_input)) @@ -1715,181 +1804,73 @@ pcap_trace_command_internal (vlib_main_t * vm, while (unformat_check_input (line_input) != UNFORMAT_END_OF_INPUT) { - if (unformat (line_input, "on")) - { - if (vm->pcap[rx_tx].pcap_enable == 0) - { - enabled = 1; - vm->pcap[rx_tx].pcap_main.n_packets_to_capture = - PCAP_DEF_PKT_TO_CAPTURE; - } - else - { - vlib_cli_output (vm, "pcap %s capture already on...", - (rx_tx == VLIB_RX) ? "rx" : "tx"); - errorFlag = 1; - break; - } - } - else if (unformat (line_input, "off")) - { - if (vm->pcap[rx_tx].pcap_enable) - { - vlib_cli_output - (vm, "captured %d pkts...", - vm->pcap[rx_tx].pcap_main.n_packets_captured); - if (vm->pcap[rx_tx].pcap_main.n_packets_captured) - { - vm->pcap[rx_tx].pcap_main.n_packets_to_capture = - vm->pcap[rx_tx].pcap_main.n_packets_captured; - error = pcap_write (&vm->pcap[rx_tx].pcap_main); - if (vm->pcap[rx_tx].pcap_main.file_descriptor >= 0) - pcap_close (&vm->pcap[rx_tx].pcap_main); - if (error) - clib_error_report (error); - else - vlib_cli_output (vm, "saved to %s...", - vm->pcap[rx_tx].pcap_main.file_name); - } - - vm->pcap[rx_tx].pcap_enable = 0; - } - else - { - vlib_cli_output (vm, "pcap %s capture already off...", - (rx_tx == VLIB_RX) ? "rx" : "tx"); - errorFlag = 1; - break; - } - } + 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)) - { - if (vm->pcap[rx_tx].pcap_enable) - { - vlib_cli_output - (vm, - "can't change max value while pcap tx capture active..."); - errorFlag = 1; - break; - } - vm->pcap[rx_tx].pcap_main.n_packets_to_capture = 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, "intfc %U", - unformat_vnet_sw_interface, vnm, - &vm->pcap[rx_tx].pcap_sw_if_index)) + unformat_vnet_sw_interface, vnm, &sw_if_index)) ; - else if (unformat (line_input, "intfc any")) - { - vm->pcap[rx_tx].pcap_sw_if_index = 0; - } - else if (unformat (line_input, "file %s", &filename)) - { - if (vm->pcap[rx_tx].pcap_enable) - { - vlib_cli_output - (vm, "can't change file while pcap tx capture active..."); - errorFlag = 1; - break; - } - - /* Brain-police user path input */ - if (strstr ((char *) filename, "..") - || index ((char *) filename, '/')) - { - vlib_cli_output (vm, "illegal characters in filename '%s'", - filename); - vlib_cli_output (vm, "Hint: .. and / are not allowed."); - vec_free (filename); - errorFlag = 1; - break; - } - - chroot_filename = format (0, "/tmp/%s%c", filename, 0); - vec_free (filename); - } - else if (unformat (line_input, "status")) - { - if (vm->pcap[rx_tx].pcap_sw_if_index == 0) - { - vlib_cli_output - (vm, "max is %d for any interface to file %s", - vm->pcap[rx_tx].pcap_main.n_packets_to_capture, - vm->pcap[rx_tx].pcap_main.file_name ? - (u8 *) vm->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", - vm->pcap[rx_tx].pcap_main.n_packets_to_capture, - format_vnet_sw_if_index_name, vnm, - vm->pcap[rx_tx].pcap_sw_if_index, - vm->pcap[rx_tx]. - pcap_main.file_name ? (u8 *) vm->pcap[rx_tx]. - pcap_main.file_name : (u8 *) "/tmp/vpe.pcap"); - } - - if (vm->pcap[rx_tx].pcap_enable == 0) - { - vlib_cli_output (vm, "pcap %s capture is off...", - (rx_tx == VLIB_RX) ? "rx" : "tx"); - } - else - { - vlib_cli_output (vm, "pcap %s capture is on: %d of %d pkts...", - (rx_tx == VLIB_RX) ? "rx" : "tx", - vm->pcap[rx_tx].pcap_main.n_packets_captured, - vm->pcap[rx_tx]. - pcap_main.n_packets_to_capture); - } - break; - } - + sw_if_index = 0; else { - error = clib_error_return (0, "unknown input `%U'", - format_unformat_error, line_input); - errorFlag = 1; - break; + 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->rxtx = rxtx; + a->sw_if_index = sw_if_index; + + rv = vnet_pcap_dispatch_trace_configure (a); - if (errorFlag == 0) + switch (rv) { - /* Since no error, save configured values. */ - if (chroot_filename) - { - if (vm->pcap[rx_tx].pcap_main.file_name) - vec_free (vm->pcap[rx_tx].pcap_main.file_name); - vec_add1 (chroot_filename, 0); - vm->pcap[rx_tx].pcap_main.file_name = (char *) chroot_filename; - } + case 0: + break; - if (max) - vm->pcap[rx_tx].pcap_main.n_packets_to_capture = max; + case VNET_API_ERROR_INVALID_VALUE: + return clib_error_return (0, "dispatch trace already enabled..."); - if (enabled) - { - if (vm->pcap[rx_tx].pcap_main.file_name == 0) - vm->pcap[rx_tx].pcap_main.file_name - = (char *) format (0, "/tmp/vpe.pcap%c", 0); - - vm->pcap[rx_tx].pcap_main.n_packets_captured = 0; - vm->pcap[rx_tx].pcap_main.packet_type = PCAP_PACKET_TYPE_ethernet; - if (vm->pcap[rx_tx].pcap_main.lock == 0) - clib_spinlock_init (&(vm->pcap[rx_tx].pcap_main.lock)); - vm->pcap[rx_tx].pcap_enable = 1; - vlib_cli_output (vm, "pcap %s capture on...", - rx_tx == VLIB_RX ? "rx" : "tx"); - } - } - else if (chroot_filename) - vec_free (chroot_filename); + case VNET_API_ERROR_VALUE_EXIST: + return clib_error_return (0, "dispatch trace already disabled..."); - return error; + case VNET_API_ERROR_INVALID_VALUE_2: + return clib_error_return + (0, "can't change number of records to capture while tracing..."); + + case VNET_API_ERROR_SYSCALL_ERROR_1: + return clib_error_return (0, "I/O writing trace capture..."); + + case VNET_API_ERROR_NO_SUCH_ENTRY: + return clib_error_return (0, "No packets captured..."); + + default: + vlib_cli_output (vm, "WARNING: trace configure returned %d", rv); + break; + } + return 0; } static clib_error_t * -- 2.16.6