From 072f8debf21c786ab785ed623229935e0a6cddb6 Mon Sep 17 00:00:00 2001 From: Dave Barach Date: Fri, 2 Dec 2016 13:31:25 -0500 Subject: [PATCH] Variable-message-length tracing support, VPP-370 Clean up several message handlers which spuriously depended on having a vlib_main_t * pointer passed as a second argument. That definitely doesn't happen when replaying an api trace... Change-Id: Id4cf9745f770933566cb13698ee779333ee35d79 Signed-off-by: Dave Barach --- vlib-api/vlibapi/api.h | 8 +++ vlib-api/vlibapi/api_shared.c | 47 +++++++++++++-- vlib-api/vlibmemory/api.h | 8 --- vlib-api/vlibmemory/memory_vlib.c | 121 -------------------------------------- vpp/vpp-api/api.c | 29 +++++---- 5 files changed, 67 insertions(+), 146 deletions(-) diff --git a/vlib-api/vlibapi/api.h b/vlib-api/vlibapi/api.h index 10ca443b434..52452526b9b 100644 --- a/vlib-api/vlibapi/api.h +++ b/vlib-api/vlibapi/api.h @@ -208,6 +208,14 @@ typedef struct int is_mp_safe; } vl_msg_api_msg_config_t; +typedef struct msgbuf_ +{ + unix_shared_memory_queue_t *q; + u32 data_len; + u32 pad; + u8 data[0]; +} msgbuf_t; + /* api_shared.c prototypes */ int vl_msg_api_rx_trace_enabled (api_main_t * am); int vl_msg_api_tx_trace_enabled (api_main_t * am); diff --git a/vlib-api/vlibapi/api_shared.c b/vlib-api/vlibapi/api_shared.c index 2b2d81c2735..6a04fac92f4 100644 --- a/vlib-api/vlibapi/api_shared.c +++ b/vlib-api/vlibapi/api_shared.c @@ -21,6 +21,7 @@ #include #include +#include #include #include #include @@ -85,8 +86,10 @@ vl_msg_api_trace (api_main_t * am, vl_api_trace_t * tp, void *msg) u8 **this_trace; u8 **old_trace; u8 *msg_copy; + u32 length; trace_cfg_t *cfgp; u16 msg_id = ntohs (*((u16 *) msg)); + msgbuf_t *header = (msgbuf_t *) (((u8 *) msg) - offsetof (msgbuf_t, data)); cfgp = am->api_trace_cfg + msg_id; @@ -116,8 +119,10 @@ vl_msg_api_trace (api_main_t * am, vl_api_trace_t * tp, void *msg) this_trace = old_trace; } - vec_validate (msg_copy, cfgp->size - 1); - clib_memcpy (msg_copy, msg, cfgp->size); + length = clib_net_to_host_u32 (header->data_len); + + vec_validate (msg_copy, length - 1); + clib_memcpy (msg_copy, msg, length); *this_trace = msg_copy; } @@ -254,6 +259,7 @@ vl_msg_api_trace_save (api_main_t * am, vl_api_trace_which_t which, FILE * fp) */ for (i = 0; i < vec_len (tp->traces); i++) { + u32 msg_length; /*sa_ignore NO_NULL_CHK */ msg = tp->traces[i]; /* @@ -262,6 +268,13 @@ vl_msg_api_trace_save (api_main_t * am, vl_api_trace_which_t which, FILE * fp) */ if (!msg) continue; + + msg_length = clib_host_to_net_u32 (vec_len (msg)); + if (fwrite (&msg_length, 1, sizeof (msg_length), fp) + != sizeof (msg_length)) + { + return (-14); + } if (fwrite (msg, 1, vec_len (msg), fp) != vec_len (msg)) { return (-11); @@ -273,6 +286,7 @@ vl_msg_api_trace_save (api_main_t * am, vl_api_trace_which_t which, FILE * fp) /* Wrap case: write oldest -> end of buffer */ for (i = tp->curindex; i < vec_len (tp->traces); i++) { + u32 msg_length; msg = tp->traces[i]; /* * This retarded check required to pass @@ -281,6 +295,13 @@ vl_msg_api_trace_save (api_main_t * am, vl_api_trace_which_t which, FILE * fp) if (!msg) continue; + msg_length = clib_host_to_net_u32 (vec_len (msg)); + if (fwrite (&msg_length, 1, sizeof (msg_length), fp) + != sizeof (msg_length)) + { + return (-14); + } + if (fwrite (msg, 1, vec_len (msg), fp) != vec_len (msg)) { return (-12); @@ -289,6 +310,7 @@ vl_msg_api_trace_save (api_main_t * am, vl_api_trace_which_t which, FILE * fp) /* write beginning of buffer -> oldest-1 */ for (i = 0; i < tp->curindex; i++) { + u32 msg_length; /*sa_ignore NO_NULL_CHK */ msg = tp->traces[i]; /* @@ -298,6 +320,13 @@ vl_msg_api_trace_save (api_main_t * am, vl_api_trace_which_t which, FILE * fp) if (!msg) continue; + msg_length = clib_host_to_net_u32 (vec_len (msg)); + if (fwrite (&msg_length, 1, sizeof (msg_length), fp) + != sizeof (msg_length)) + { + return (-14); + } + if (fwrite (msg, 1, vec_len (msg), fp) != vec_len (msg)) { return (-13); @@ -668,7 +697,6 @@ vl_msg_api_set_handlers (int id, char *name, void *handler, void *cleanup, c->cleanup = cleanup; c->endian = endian; c->print = print; - c->size = size; c->traced = traced; c->replay = 1; c->message_bounce = 0; @@ -759,7 +787,7 @@ vl_msg_api_process_file (vlib_main_t * vm, u8 * filename, u8 *msg; u8 endian_swap_needed = 0; api_main_t *am = &api_main; - static u8 *tmpbuf; + u8 *tmpbuf = 0; u32 nitems; void **saved_print_handlers = 0; @@ -838,6 +866,9 @@ vl_msg_api_process_file (vlib_main_t * vm, u8 * filename, int size; u16 msg_id; + size = clib_host_to_net_u32 (*(u32 *) msg); + msg += sizeof (u32); + if (clib_arch_is_little_endian) msg_id = ntohs (*((u16 *) msg)); else @@ -850,7 +881,6 @@ vl_msg_api_process_file (vlib_main_t * vm, u8 * filename, munmap (hp, file_size); return; } - size = cfgp->size; msg += size; } @@ -864,6 +894,9 @@ vl_msg_api_process_file (vlib_main_t * vm, u8 * filename, if (which == DUMP) vlib_cli_output (vm, "---------- trace %d -----------\n", i); + size = clib_host_to_net_u32 (*(u32 *) msg); + msg += sizeof (u32); + if (clib_arch_is_little_endian) msg_id = ntohs (*((u16 *) msg)); else @@ -874,9 +907,9 @@ vl_msg_api_process_file (vlib_main_t * vm, u8 * filename, { vlib_cli_output (vm, "Ugh: msg id %d no trace config\n", msg_id); munmap (hp, file_size); + vec_free (tmpbuf); return; } - size = cfgp->size; /* Copy the buffer (from the read-only mmap'ed file) */ vec_validate (tmpbuf, size - 1 + sizeof (uword)); @@ -897,6 +930,7 @@ vl_msg_api_process_file (vlib_main_t * vm, u8 * filename, { vlib_cli_output (vm, "Ugh: msg id %d no endian swap\n", msg_id); munmap (hp, file_size); + vec_free (tmpbuf); return; } endian_fp = am->msg_endian_handlers[msg_id]; @@ -997,6 +1031,7 @@ vl_msg_api_process_file (vlib_main_t * vm, u8 * filename, } munmap (hp, file_size); + vec_free (tmpbuf); } u8 * diff --git a/vlib-api/vlibmemory/api.h b/vlib-api/vlibmemory/api.h index 825891cf9ed..f1f8bb736b0 100644 --- a/vlib-api/vlibmemory/api.h +++ b/vlib-api/vlibmemory/api.h @@ -88,14 +88,6 @@ typedef struct vl_shmem_hdr_ } vl_shmem_hdr_t; -/* Note that the size of the structure is 16 bytes, with 4 bytes of padding after data[0]. */ -typedef struct msgbuf_ -{ - unix_shared_memory_queue_t *q; - u32 data_len; - u8 data[0]; -} msgbuf_t; - #define VL_SHM_VERSION 2 #define VL_API_EPOCH_MASK 0xFF diff --git a/vlib-api/vlibmemory/memory_vlib.c b/vlib-api/vlibmemory/memory_vlib.c index 5f97f1611d1..1d40bcb791f 100644 --- a/vlib-api/vlibmemory/memory_vlib.c +++ b/vlib-api/vlibmemory/memory_vlib.c @@ -1042,122 +1042,12 @@ VLIB_CLI_COMMAND (cli_show_api_message_table_command, static) = { }; /* *INDENT-ON* */ -void -vl_api_trace_print_file_cmd (vlib_main_t * vm, u32 first, u32 last, - u8 * filename) -{ - FILE *fp; - static vl_api_trace_t *tp = 0; - int endian_swap = 0; - u32 i; - u16 msg_id; - static u8 *msg_buf = 0; - void (*endian_fp) (void *); - u8 *(*print_fp) (void *, void *); - int size; - api_main_t *am = &api_main; - - /* - * On-demand: allocate enough space for the largest message - */ - if (msg_buf == 0) - { - vec_validate (tp, 0); - int max_size = 0; - for (i = 0; i < vec_len (am->api_trace_cfg); i++) - { - if (am->api_trace_cfg[i].size > max_size) - max_size = am->api_trace_cfg[i].size; - } - /* round size to a multiple of the cache-line size */ - max_size = (max_size + (CLIB_CACHE_LINE_BYTES - 1)) & - (~(CLIB_CACHE_LINE_BYTES - 1)); - vec_validate (msg_buf, max_size - 1); - } - - fp = fopen ((char *) filename, "r"); - - if (fp == NULL) - { - vlib_cli_output (vm, "Couldn't open %s\n", filename); - return; - } - - /* first, fish the header record from the file */ - - if (fread (tp, sizeof (*tp), 1, fp) != 1) - { - fclose (fp); - vlib_cli_output (vm, "Header read error\n"); - return; - } - - /* Endian swap required? */ - if (clib_arch_is_big_endian != tp->endian) - { - endian_swap = 1; - } - - for (i = 0; i <= last; i++) - { - /* First 2 bytes are the message type */ - if (fread (&msg_id, sizeof (u16), 1, fp) != 1) - { - break; - } - msg_id = ntohs (msg_id); - - if (fseek (fp, -2, SEEK_CUR) < 0) - { - vlib_cli_output (vm, "fseek failed, %s", strerror (errno)); - fclose (fp); - return; - } - - /* Mild sanity check */ - if (msg_id >= vec_len (am->msg_handlers)) - { - fclose (fp); - vlib_cli_output (vm, "msg_id %d out of bounds\n", msg_id); - return; - } - - size = am->api_trace_cfg[msg_id].size; - - if (fread (msg_buf, size, 1, fp) != 1) - { - fclose (fp); - vlib_cli_output (vm, "read error on %s\n", filename); - return; - } - - if (i < first) - continue; - - if (endian_swap) - { - endian_fp = am->msg_endian_handlers[msg_id]; - (*endian_fp) (msg_buf); - } - - vlib_cli_output (vm, "[%d]: %s\n", i, am->msg_names[msg_id]); - - print_fp = (void *) am->msg_print_handlers[msg_id]; - (*print_fp) (msg_buf, vm); - vlib_cli_output (vm, "-------------\n"); - } - fclose (fp); -} - static clib_error_t * vl_api_trace_command (vlib_main_t * vm, unformat_input_t * input, vlib_cli_command_t * cli_cmd) { u32 nitems = 1024; vl_api_trace_which_t which = VL_API_TRACE_RX; - u8 *filename; - u32 first = 0; - u32 last = ~0; api_main_t *am = &api_main; while (unformat_check_input (input) != UNFORMAT_END_OF_INPUT) @@ -1194,12 +1084,6 @@ vl_api_trace_command (vlib_main_t * vm, vl_msg_api_trace_free (am, VL_API_TRACE_RX); vl_msg_api_trace_free (am, VL_API_TRACE_TX); } - else if (unformat (input, "print %s from %d to %d", &filename, - &first, &last) - || unformat (input, "print %s", &filename)) - { - goto print; - } else if (unformat (input, "debug on")) { am->msg_print_flag = 1; @@ -1214,10 +1098,6 @@ vl_api_trace_command (vlib_main_t * vm, } return 0; -print: - vl_api_trace_print_file_cmd (vm, first, last, filename); - goto out; - configure: if (vl_msg_api_trace_configure (am, which, nitems)) { @@ -1225,7 +1105,6 @@ configure: which, nitems); } -out: return 0; } diff --git a/vpp/vpp-api/api.c b/vpp/vpp-api/api.c index b778d3cad04..5b2490958ff 100644 --- a/vpp/vpp-api/api.c +++ b/vpp/vpp-api/api.c @@ -1854,8 +1854,9 @@ out: } static void -vl_api_tap_connect_t_handler (vl_api_tap_connect_t * mp, vlib_main_t * vm) +vl_api_tap_connect_t_handler (vl_api_tap_connect_t * mp) { + vlib_main_t *vm = vlib_get_main (); int rv; vl_api_tap_connect_reply_t *rmp; vnet_main_t *vnm = vnet_get_main (); @@ -1868,10 +1869,6 @@ vl_api_tap_connect_t_handler (vl_api_tap_connect_t * mp, vlib_main_t * vm) &sw_if_index, mp->renumber, ntohl (mp->custom_dev_instance)); - q = vl_api_client_index_to_input_queue (mp->client_index); - if (!q) - return; - /* Add tag if supplied */ if (rv == 0 && mp->tag[0]) { @@ -1880,6 +1877,10 @@ vl_api_tap_connect_t_handler (vl_api_tap_connect_t * mp, vlib_main_t * vm) vnet_set_sw_interface_tag (vnm, tag, sw_if_index); } + q = vl_api_client_index_to_input_queue (mp->client_index); + if (!q) + return; + rmp = vl_msg_api_alloc (sizeof (*rmp)); rmp->_vl_msg_id = ntohs (VL_API_TAP_CONNECT_REPLY); rmp->context = mp->context; @@ -1890,12 +1891,13 @@ vl_api_tap_connect_t_handler (vl_api_tap_connect_t * mp, vlib_main_t * vm) } static void -vl_api_tap_modify_t_handler (vl_api_tap_modify_t * mp, vlib_main_t * vm) +vl_api_tap_modify_t_handler (vl_api_tap_modify_t * mp) { int rv; vl_api_tap_modify_reply_t *rmp; unix_shared_memory_queue_t *q; u32 sw_if_index = (u32) ~ 0; + vlib_main_t *vm = vlib_get_main (); rv = vnet_tap_modify (vm, ntohl (mp->sw_if_index), mp->tap_name, mp->use_random_mac ? 0 : mp->mac_address, @@ -1916,8 +1918,9 @@ vl_api_tap_modify_t_handler (vl_api_tap_modify_t * mp, vlib_main_t * vm) } static void -vl_api_tap_delete_t_handler (vl_api_tap_delete_t * mp, vlib_main_t * vm) +vl_api_tap_delete_t_handler (vl_api_tap_delete_t * mp) { + vlib_main_t *vm = vlib_get_main (); int rv; vpe_api_main_t *vam = &vpe_api_main; vl_api_tap_delete_reply_t *rmp; @@ -3003,9 +3006,10 @@ static void vl_api_dhcp_client_config_t_handler static void vl_api_sw_interface_ip6nd_ra_config_t_handler - (vl_api_sw_interface_ip6nd_ra_config_t * mp, vlib_main_t * vm) + (vl_api_sw_interface_ip6nd_ra_config_t * mp) { vl_api_sw_interface_ip6nd_ra_config_reply_t *rmp; + vlib_main_t *vm = vlib_get_main (); int rv = 0; u8 is_no, suppress, managed, other, ll_option, send_unicast, cease, default_router; @@ -3037,8 +3041,9 @@ static void static void vl_api_sw_interface_ip6nd_ra_prefix_t_handler - (vl_api_sw_interface_ip6nd_ra_prefix_t * mp, vlib_main_t * vm) + (vl_api_sw_interface_ip6nd_ra_prefix_t * mp) { + vlib_main_t *vm = vlib_get_main (); vl_api_sw_interface_ip6nd_ra_prefix_reply_t *rmp; int rv = 0; u8 is_no, use_default, no_advertise, off_link, no_autoconfig, no_onlink; @@ -3065,8 +3070,9 @@ static void static void vl_api_sw_interface_ip6_enable_disable_t_handler - (vl_api_sw_interface_ip6_enable_disable_t * mp, vlib_main_t * vm) + (vl_api_sw_interface_ip6_enable_disable_t * mp) { + vlib_main_t *vm = vlib_get_main (); vl_api_sw_interface_ip6_enable_disable_reply_t *rmp; vnet_main_t *vnm = vnet_get_main (); int rv = 0; @@ -3098,8 +3104,9 @@ static void static void vl_api_sw_interface_ip6_set_link_local_address_t_handler - (vl_api_sw_interface_ip6_set_link_local_address_t * mp, vlib_main_t * vm) + (vl_api_sw_interface_ip6_set_link_local_address_t * mp) { + vlib_main_t *vm = vlib_get_main (); vl_api_sw_interface_ip6_set_link_local_address_reply_t *rmp; int rv = 0; clib_error_t *error; -- 2.16.6