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 <dave@barachs.net>

diff --git a/vlib-api/vlibapi/api.h b/vlib-api/vlibapi/api.h
index 10ca443..5245252 100644 (file)
--- 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 2b2d81c..6a04fac 100644 (file)
--- a/vlib-api/vlibapi/api_shared.c
+++ b/vlib-api/vlibapi/api_shared.c
@@ -21,6 +21,7 @@
 
 #include <stdio.h>
 #include <stdlib.h>
+#include <stddef.h>
 #include <string.h>
 #include <sys/types.h>
 #include <sys/mman.h>
@@ -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 825891c..f1f8bb7 100644 (file)
--- 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 5f97f16..1d40bcb 100644 (file)
--- 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 b778d3c..5b24909 100644 (file)
--- 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;