From dab732a18c39d13af1770b55d7cef2359ea66412 Mon Sep 17 00:00:00 2001 From: Klement Sekera Date: Wed, 4 Jul 2018 13:43:46 +0200 Subject: [PATCH] VPP-1335 vapi crash when memclnt_keepalive received Change-Id: If33a7cc6c76147fd3ea9d8118370e7a508819b81 Signed-off-by: Klement Sekera --- src/vlibmemory/memclnt.api | 8 +++---- src/vpp-api/vapi/vapi.c | 52 +++++++++++++++++++++++++++++++++++++----- src/vpp-api/vapi/vapi.h | 4 +++- src/vpp-api/vapi/vapi.hpp | 10 ++++---- src/vpp-api/vapi/vapi_c_gen.py | 4 ++-- src/vpp-api/vapi/vapi_common.h | 2 +- test/ext/vapi_c_test.c | 8 +++---- 7 files changed, 66 insertions(+), 22 deletions(-) diff --git a/src/vlibmemory/memclnt.api b/src/vlibmemory/memclnt.api index 6e06a99b9e6..923df69fd9a 100644 --- a/src/vlibmemory/memclnt.api +++ b/src/vlibmemory/memclnt.api @@ -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 */ }; /* diff --git a/src/vpp-api/vapi/vapi.c b/src/vpp-api/vapi/vapi.c index 754c89cf561..1a0fdbb627a 100644 --- a/src/vpp-api/vapi/vapi.c +++ b/src/vpp-api/vapi/vapi.c @@ -31,12 +31,16 @@ #include #include +#include + /* 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); diff --git a/src/vpp-api/vapi/vapi.h b/src/vpp-api/vapi/vapi.h index fc48e7d402c..2a401c21189 100644 --- a/src/vpp-api/vapi/vapi.h +++ b/src/vpp-api/vapi/vapi.h @@ -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 diff --git a/src/vpp-api/vapi/vapi.hpp b/src/vpp-api/vapi/vapi.hpp index 28357db420c..a1e33a93fd4 100644 --- a/src/vpp-api/vapi/vapi.hpp +++ b/src/vpp-api/vapi/vapi.hpp @@ -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; } diff --git a/src/vpp-api/vapi/vapi_c_gen.py b/src/vpp-api/vapi/vapi_c_gen.py index b85f1e9d248..5a185398e89 100755 --- a/src/vpp-api/vapi/vapi_c_gen.py +++ b/src/vpp-api/vapi/vapi_c_gen.py @@ -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);' % diff --git a/src/vpp-api/vapi/vapi_common.h b/src/vpp-api/vapi/vapi_common.h index 764a0b71f50..7157f0a8e0d 100644 --- a/src/vpp-api/vapi/vapi_common.h +++ b/src/vpp-api/vapi/vapi_common.h @@ -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 } diff --git a/test/ext/vapi_c_test.c b/test/ext/vapi_c_test.c index 52a939f72a5..2e5b9e6472c 100644 --- a/test/ext/vapi_c_test.c +++ b/test/ext/vapi_c_test.c @@ -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); } -- 2.16.6