VPP-1335 vapi crash when memclnt_keepalive received

Change-Id: If33a7cc6c76147fd3ea9d8118370e7a508819b81
Signed-off-by: Klement Sekera <ksekera@cisco.com>
This commit is contained in:
Klement Sekera
2018-07-04 13:43:46 +02:00
committed by Damjan Marion
parent ef8db36797
commit dab732a18c
7 changed files with 66 additions and 22 deletions

View 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 */
};
/*

View File

@ -31,12 +31,16 @@
#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);

View 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

View 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;
}

View File

@ -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);' %

View 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
}

View 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);
}