VPP-1335 vapi crash when memclnt_keepalive received 49/13349/4
authorKlement Sekera <ksekera@cisco.com>
Wed, 4 Jul 2018 11:43:46 +0000 (13:43 +0200)
committerDamjan Marion <dmarion@me.com>
Thu, 5 Jul 2018 09:01:22 +0000 (09:01 +0000)
Change-Id: If33a7cc6c76147fd3ea9d8118370e7a508819b81
Signed-off-by: Klement Sekera <ksekera@cisco.com>
src/vlibmemory/memclnt.api
src/vpp-api/vapi/vapi.c
src/vpp-api/vapi/vapi.h
src/vpp-api/vapi/vapi.hpp
src/vpp-api/vapi/vapi_c_gen.py
src/vpp-api/vapi/vapi_common.h
test/ext/vapi_c_test.c

index 6e06a99..923df69 100644 (file)
@@ -31,18 +31,18 @@ service {
  */
 manual_print 
 define memclnt_create {
-    i32 ctx_quota;              /* requested punt context quota */
     u32 context;                /* opaque value to be returned in the reply */
+    i32 ctx_quota;              /* requested punt context quota */
     u64 input_queue;            /* client's queue */
     u8 name[64];                /* for show, find by name, whatever */
     u32 api_versions[8];        /* client-server pairs use as desired */
 };
 
 define memclnt_create_reply {
+    u32 context;                /* opaque value from the create request */
     i32 response;               /* Non-negative = success */
     u64 handle;                 /* handle by which vlib knows this client */
     u32 index;                  /* index, used e.g. by API trace replay */
-    u32 context;                /* opaque value from the create request */
     u64 message_table;          /* serialized message table in shmem */
 };
 
@@ -150,15 +150,15 @@ manual_print define trace_plugin_msg_ids
  * Create a socket client registration. 
  */
 define sockclnt_create {
-    u8 name[64];                /* for show, find by name, whatever */
     u32 context;                /* opaque value to be returned in the reply */
+    u8 name[64];                /* for show, find by name, whatever */
 };
 
 define sockclnt_create_reply {
+    u32 context;                /* opaque value from the create request */
     i32 response;               /* Non-negative = success */
     u64 handle;                /* handle by which vlib knows this client */
     u32 index;                  /* index, used e.g. by API trace replay */
-    u32 context;                /* opaque value from the create request */
 };
 
 /*
index 754c89c..1a0fdbb 100644 (file)
 #include <vlibapi/api_common.h>
 #include <vlibmemory/memory_client.h>
 
+#include <vapi/memclnt.api.vapi.h>
+
 /* we need to use control pings for some stuff and because we're forced to put
  * the code in headers, we need a way to be able to grab the ids of these
  * messages - so declare them here as extern */
 vapi_msg_id_t vapi_msg_id_control_ping = 0;
 vapi_msg_id_t vapi_msg_id_control_ping_reply = 0;
 
+DEFINE_VAPI_MSG_IDS_MEMCLNT_API_JSON;
+
 struct
 {
   size_t count;
@@ -81,6 +85,7 @@ struct vapi_ctx_s
   u16 vl_msg_id_max;
   vapi_msg_id_t *vl_msg_id_to_vapi_msg_t;
   bool connected;
+  bool handle_keepalives;
   pthread_mutex_t requests_mutex;
 };
 
@@ -238,7 +243,7 @@ vapi_lookup_vapi_msg_id_t (vapi_ctx_t ctx, u16 vl_msg_id)
     {
       return ctx->vl_msg_id_to_vapi_msg_t[vl_msg_id];
     }
-  return INVALID_MSG_ID;
+  return VAPI_INVALID_MSG_ID;
 }
 
 vapi_error_e
@@ -257,6 +262,8 @@ vapi_ctx_alloc (vapi_ctx_t * result)
     {
       goto fail;
     }
+  memset (ctx->vapi_msg_id_t_to_vl_msg_id, ~0,
+         __vapi_metadata.count * sizeof (*ctx->vapi_msg_id_t_to_vl_msg_id));
   ctx->event_cbs = calloc (__vapi_metadata.count, sizeof (*ctx->event_cbs));
   if (!ctx->event_cbs)
     {
@@ -292,7 +299,8 @@ vapi_error_e
 vapi_connect (vapi_ctx_t ctx, const char *name,
              const char *chroot_prefix,
              int max_outstanding_requests,
-             int response_queue_size, vapi_mode_e mode)
+             int response_queue_size, vapi_mode_e mode,
+             bool handle_keepalives)
 {
   if (response_queue_size <= 0 || max_outstanding_requests <= 0)
     {
@@ -337,7 +345,7 @@ vapi_connect (vapi_ctx_t ctx, const char *name,
       u8 scratch[m->name_with_crc_len + 1];
       memcpy (scratch, m->name_with_crc, m->name_with_crc_len + 1);
       u32 id = vl_msg_api_get_msg_index (scratch);
-      if (INVALID_MSG_ID != id)
+      if (VAPI_INVALID_MSG_ID != id)
        {
          if (id > UINT16_MAX)
            {
@@ -386,6 +394,14 @@ vapi_connect (vapi_ctx_t ctx, const char *name,
     }
   ctx->mode = mode;
   ctx->connected = true;
+  if (vapi_is_msg_available (ctx, vapi_msg_id_memclnt_keepalive))
+    {
+      ctx->handle_keepalives = handle_keepalives;
+    }
+  else
+    {
+      ctx->handle_keepalives = false;
+    }
   return VAPI_OK;
 fail:
   vl_client_disconnect ();
@@ -519,6 +535,7 @@ vapi_recv (vapi_ctx_t ctx, void **msg, size_t * msg_size,
     }
 
   svm_queue_t *q = am->vl_input_queue;
+again:
   VAPI_DBG ("doing shm queue sub");
 
   int tmp = svm_queue_sub (q, (u8 *) & data, cond, time);
@@ -557,6 +574,30 @@ vapi_recv (vapi_ctx_t ctx, void **msg, size_t * msg_size,
          VAPI_DBG ("recv msg@%p:%u[UNKNOWN]", *msg, msgid);
        }
 #endif
+      if (ctx->handle_keepalives)
+       {
+         unsigned msgid = be16toh (*(u16 *) * msg);
+         if (msgid ==
+             vapi_lookup_vl_msg_id (ctx, vapi_msg_id_memclnt_keepalive))
+           {
+             vapi_msg_memclnt_keepalive_reply *reply = NULL;
+             do
+               {
+                 reply = vapi_msg_alloc (ctx, sizeof (*reply));
+               }
+             while (!reply);
+             reply->header.context = vapi_get_client_index (ctx);
+             reply->header._vl_msg_id =
+               vapi_lookup_vl_msg_id (ctx,
+                                      vapi_msg_id_memclnt_keepalive_reply);
+             reply->payload.retval = 0;
+             vapi_msg_memclnt_keepalive_reply_hton (reply);
+             while (VAPI_EAGAIN == vapi_send (ctx, reply));
+             vapi_msg_free (ctx, *msg);
+             VAPI_DBG ("autohandled memclnt_keepalive");
+             goto again;
+           }
+       }
     }
   else
     {
@@ -601,8 +642,7 @@ vapi_dispatch_response (vapi_ctx_t ctx, vapi_msg_id_t id,
        {
          VAPI_ERR ("No response to req with context=%u",
                    (unsigned) ctx->requests[tmp].context);
-         ctx->requests[ctx->requests_start].callback (ctx,
-                                                      ctx->requests
+         ctx->requests[ctx->requests_start].callback (ctx, ctx->requests
                                                       [ctx->
                                                        requests_start].callback_ctx,
                                                       VAPI_ENORESP, true,
@@ -717,7 +757,7 @@ vapi_dispatch_one (vapi_ctx_t ctx)
       vapi_msg_free (ctx, msg);
       return VAPI_EINVAL;
     }
-  if (INVALID_MSG_ID == (unsigned) ctx->vl_msg_id_to_vapi_msg_t[vpp_id])
+  if (VAPI_INVALID_MSG_ID == (unsigned) ctx->vl_msg_id_to_vapi_msg_t[vpp_id])
     {
       VAPI_ERR ("Unknown msg ID received, id `%u' marked as not supported",
                (unsigned) vpp_id);
index fc48e7d..2a401c2 100644 (file)
@@ -96,13 +96,15 @@ extern "C"
  * @param max_outstanding_requests max number of outstanding requests queued
  * @param response_queue_size size of the response queue
  * @param mode mode of operation - blocking or nonblocking
+ * @param handle_keepalives - if true, automatically handle memclnt_keepalive
  *
  * @return VAPI_OK on success, other error code on error
  */
   vapi_error_e vapi_connect (vapi_ctx_t ctx, const char *name,
                             const char *chroot_prefix,
                             int max_outstanding_requests,
-                            int response_queue_size, vapi_mode_e mode);
+                            int response_queue_size, vapi_mode_e mode,
+                            bool handle_keepalives);
 
 /**
  * @brief disconnect from vpp
index 28357db..a1e33a9 100644 (file)
@@ -195,15 +195,17 @@ public:
    * @param name application name
    * @param chroot_prefix shared memory prefix
    * @param max_queued_request max number of outstanding requests queued
+   * @param handle_keepalives handle memclnt_keepalive automatically
    *
    * @return VAPI_OK on success, other error code on error
    */
   vapi_error_e connect (const char *name, const char *chroot_prefix,
-                        int max_outstanding_requests, int response_queue_size)
+                        int max_outstanding_requests, int response_queue_size,
+                        bool handle_keepalives = true)
   {
     return vapi_connect (vapi_ctx, name, chroot_prefix,
                          max_outstanding_requests, response_queue_size,
-                         VAPI_MODE_BLOCKING);
+                         VAPI_MODE_BLOCKING, handle_keepalives);
   }
 
   /**
@@ -579,14 +581,14 @@ private:
 
   static void set_msg_id (vapi_msg_id_t id)
   {
-    assert ((INVALID_MSG_ID == *msg_id_holder ()) ||
+    assert ((VAPI_INVALID_MSG_ID == *msg_id_holder ()) ||
             (id == *msg_id_holder ()));
     *msg_id_holder () = id;
   }
 
   static vapi_msg_id_t *msg_id_holder ()
   {
-    static vapi_msg_id_t my_id{INVALID_MSG_ID};
+    static vapi_msg_id_t my_id{VAPI_INVALID_MSG_ID};
     return &my_id;
   }
 
index b85f1e9..5a18539 100755 (executable)
@@ -500,11 +500,11 @@ class CMessage (Message):
             '    offsetof(%s, context),' % self.header.get_c_name()
             if has_context else '    0,',
             ('    offsetof(%s, payload),' % self.get_c_name())
-            if self.has_payload() else '    INVALID_MSG_ID,',
+            if self.has_payload() else '    VAPI_INVALID_MSG_ID,',
             '    sizeof(%s),' % self.get_c_name(),
             '    (generic_swap_fn_t)%s,' % self.get_swap_to_be_func_name(),
             '    (generic_swap_fn_t)%s,' % self.get_swap_to_host_func_name(),
-            '    INVALID_MSG_ID,',
+            '    VAPI_INVALID_MSG_ID,',
             '  };',
             '',
             '  %s = vapi_register_msg(&%s);' %
index 764a0b7..7157f0a 100644 (file)
@@ -54,7 +54,7 @@ typedef enum
 
 typedef unsigned int vapi_msg_id_t;
 
-#define INVALID_MSG_ID ((vapi_msg_id_t)(~0))
+#define VAPI_INVALID_MSG_ID ((vapi_msg_id_t)(~0))
 
 #ifdef __cplusplus
 }
index 52a939f..2e5b9e6 100644 (file)
@@ -60,7 +60,7 @@ START_TEST (test_invalid_values)
   rv = vapi_send (ctx, sv);
   ck_assert_int_eq (VAPI_EINVAL, rv);
   rv = vapi_connect (ctx, app_name, api_prefix, max_outstanding_requests,
-                    response_queue_size, VAPI_MODE_BLOCKING);
+                    response_queue_size, VAPI_MODE_BLOCKING, true);
   ck_assert_int_eq (VAPI_OK, rv);
   rv = vapi_send (ctx, NULL);
   ck_assert_int_eq (VAPI_EINVAL, rv);
@@ -483,7 +483,7 @@ START_TEST (test_connect)
   vapi_error_e rv = vapi_ctx_alloc (&ctx);
   ck_assert_int_eq (VAPI_OK, rv);
   rv = vapi_connect (ctx, app_name, api_prefix, max_outstanding_requests,
-                    response_queue_size, VAPI_MODE_BLOCKING);
+                    response_queue_size, VAPI_MODE_BLOCKING, true);
   ck_assert_int_eq (VAPI_OK, rv);
   rv = vapi_disconnect (ctx);
   ck_assert_int_eq (VAPI_OK, rv);
@@ -500,7 +500,7 @@ setup_blocking (void)
   vapi_error_e rv = vapi_ctx_alloc (&ctx);
   ck_assert_int_eq (VAPI_OK, rv);
   rv = vapi_connect (ctx, app_name, api_prefix, max_outstanding_requests,
-                    response_queue_size, VAPI_MODE_BLOCKING);
+                    response_queue_size, VAPI_MODE_BLOCKING, true);
   ck_assert_int_eq (VAPI_OK, rv);
 }
 
@@ -510,7 +510,7 @@ setup_nonblocking (void)
   vapi_error_e rv = vapi_ctx_alloc (&ctx);
   ck_assert_int_eq (VAPI_OK, rv);
   rv = vapi_connect (ctx, app_name, api_prefix, max_outstanding_requests,
-                    response_queue_size, VAPI_MODE_NONBLOCKING);
+                    response_queue_size, VAPI_MODE_NONBLOCKING, true);
   ck_assert_int_eq (VAPI_OK, rv);
 }