From aa9002a5f5ef55e4c99c99601b4aa5a164d08bbf Mon Sep 17 00:00:00 2001 From: Mark Stapp Date: Tue, 7 Apr 2020 09:50:53 -0400 Subject: [PATCH] zebra: add lock and busy counter for zclients Add a mutex used to manage the list of zclients. Add a busy counter to the zapi client session, so that we can use a client session from another pthread. Signed-off-by: Mark Stapp --- zebra/zserv.c | 183 ++++++++++++++++++++++++++++++++++++++++++++------ zebra/zserv.h | 31 ++++++++- 2 files changed, 192 insertions(+), 22 deletions(-) diff --git a/zebra/zserv.c b/zebra/zserv.c index 9d6ae2ec2e..474c69a29f 100644 --- a/zebra/zserv.c +++ b/zebra/zserv.c @@ -71,6 +71,14 @@ extern struct zebra_privs_t zserv_privs; /* The listener socket for clients connecting to us */ static int zsock; +/* The lock that protects access to zapi client objects */ +static pthread_mutex_t client_mutex; + +static struct zserv *find_client_internal(uint8_t proto, + unsigned short instance, + uint32_t session_id); + + /* * Client thread events. * @@ -629,26 +637,47 @@ static void zserv_client_free(struct zserv *client) void zserv_close_client(struct zserv *client) { - /* synchronously stop and join pthread */ - frr_pthread_stop(client->pthread, NULL); + bool free_p = true; - if (IS_ZEBRA_DEBUG_EVENT) - zlog_debug("Closing client '%s'", - zebra_route_string(client->proto)); + if (client->pthread) { + /* synchronously stop and join pthread */ + frr_pthread_stop(client->pthread, NULL); - thread_cancel_event(zrouter.master, client); - THREAD_OFF(client->t_cleanup); - THREAD_OFF(client->t_process); + if (IS_ZEBRA_DEBUG_EVENT) + zlog_debug("Closing client '%s'", + zebra_route_string(client->proto)); - /* destroy pthread */ - frr_pthread_destroy(client->pthread); - client->pthread = NULL; + thread_cancel_event(zrouter.master, client); + THREAD_OFF(client->t_cleanup); + THREAD_OFF(client->t_process); - /* remove from client list */ - listnode_delete(zrouter.client_list, client); + /* destroy pthread */ + frr_pthread_destroy(client->pthread); + client->pthread = NULL; + } + + /* + * Final check in case the client struct is in use in another + * pthread: if not in-use, continue and free the client + */ + frr_with_mutex(&client_mutex) { + if (client->busy_count <= 0) { + /* remove from client list */ + listnode_delete(zrouter.client_list, client); + } else { + /* + * The client session object may be in use, although + * the associated pthread is gone. Defer final + * cleanup. + */ + client->is_closed = true; + free_p = false; + } + } /* delete client */ - zserv_client_free(client); + if (free_p) + zserv_client_free(client); } /* @@ -708,7 +737,9 @@ static struct zserv *zserv_client_create(int sock) client->ridinfo = vrf_bitmap_init(); /* Add this client to linked list. */ - listnode_add(zrouter.client_list, client); + frr_with_mutex(&client_mutex) { + listnode_add(zrouter.client_list, client); + } struct frr_pthread_attr zclient_pthr_attrs = { .start = frr_pthread_attr_default.start, @@ -730,6 +761,81 @@ static struct zserv *zserv_client_create(int sock) return client; } +/* + * Retrieve a client object by the complete tuple of + * {protocol, instance, session}. This version supports use + * from a different pthread: the object will be returned marked + * in-use. The caller *must* release the client object with the + * release_client() api, to ensure that the in-use marker is cleared properly. + */ +struct zserv *zserv_acquire_client(uint8_t proto, unsigned short instance, + uint32_t session_id) +{ + struct zserv *client = NULL; + + frr_with_mutex(&client_mutex) { + client = find_client_internal(proto, instance, session_id); + if (client) { + /* Don't return a dead/closed client object */ + if (client->is_closed) + client = NULL; + else + client->busy_count++; + } + } + + return client; +} + +/* + * Release a client object that was acquired with the acquire_client() api. + * After this has been called, the caller must not use the client pointer - + * it may be freed if the client has closed. + */ +void zserv_release_client(struct zserv *client) +{ + bool cleanup_p = false; + const char *proto_str; + uint16_t instance; + + /* Capture some info for debugging */ + proto_str = zebra_route_string(client->proto); + instance = client->instance; + + /* + * Once we've decremented the client object's refcount, it's possible + * for it to be deleted as soon as we release the lock, so we won't + * touch the object again. + */ + frr_with_mutex(&client_mutex) { + client->busy_count--; + + if (client->busy_count <= 0) { + /* + * No more users of the client object. If the client + * session is closed, schedule cleanup on the zebra + * main pthread. + */ + if (client->is_closed) { + thread_add_event(zrouter.master, + zserv_handle_client_fail, + client, 0, &client->t_cleanup); + + cleanup_p = true; + } + } + } + + /* + * Cleanup must take place on the zebra main pthread, so we've + * scheduled an event. + */ + if (IS_ZEBRA_DEBUG_EVENT) + zlog_debug("%s: %s clean-up for client '%s'[%u]", + __func__, (cleanup_p ? "scheduled" : "NO"), + proto_str, instance); +} + /* * Accept socket connection. */ @@ -773,6 +879,9 @@ void zserv_close(void) */ close(zsock); zsock = -1; + + /* Free client list's mutex */ + pthread_mutex_destroy(&client_mutex); } void zserv_start(char *path) @@ -1077,24 +1186,55 @@ static void zebra_show_client_brief(struct vty *vty, struct zserv *client) client->v6_route_del_cnt); } -struct zserv *zserv_find_client_session(uint8_t proto, unsigned short instance, - uint32_t session_id) +/* + * Common logic that searches the client list for a zapi client; this + * MUST be called holding the client list mutex. + */ +static struct zserv *find_client_internal(uint8_t proto, + unsigned short instance, + uint32_t session_id) { struct listnode *node, *nnode; - struct zserv *client; + struct zserv *client = NULL; for (ALL_LIST_ELEMENTS(zrouter.client_list, node, nnode, client)) { if (client->proto == proto && client->instance == instance && client->session_id == session_id) - return client; + break; } - return NULL; + return client; } +/* + * Public api that searches for a client session; this version is + * used from the zebra main pthread. + */ struct zserv *zserv_find_client(uint8_t proto, unsigned short instance) { - return zserv_find_client_session(proto, instance, 0); + struct zserv *client; + + frr_with_mutex(&client_mutex) { + client = find_client_internal(proto, instance, 0); + } + + return client; +} + +/* + * Retrieve a client by its protocol, instance number, and session id. + */ +struct zserv *zserv_find_client_session(uint8_t proto, unsigned short instance, + uint32_t session_id) +{ + struct zserv *client; + + frr_with_mutex(&client_mutex) { + client = find_client_internal(proto, instance, session_id); + } + + return client; + } /* This command is for debugging purpose. */ @@ -1161,6 +1301,7 @@ void zserv_init(void) /* Misc init. */ zsock = -1; + pthread_mutex_init(&client_mutex, NULL); install_element(ENABLE_NODE, &show_zebra_client_cmd); install_element(ENABLE_NODE, &show_zebra_client_summary_cmd); diff --git a/zebra/zserv.h b/zebra/zserv.h index 9d442899f1..b943c246c6 100644 --- a/zebra/zserv.h +++ b/zebra/zserv.h @@ -96,6 +96,14 @@ struct zserv { /* Client file descriptor. */ int sock; + /* Attributes used to permit access to zapi clients from + * other pthreads: the client has a busy counter, and a + * 'closed' flag. These attributes are managed using a + * lock, via the acquire_client() and release_client() apis. + */ + int busy_count; + bool is_closed; + /* Input/output buffer to the client. */ pthread_mutex_t ibuf_mtx; struct stream_fifo *ibuf_fifo; @@ -116,7 +124,7 @@ struct zserv { /* Event for message processing, for the main pthread */ struct thread *t_process; - /* Threads for the main pthread */ + /* Event for the main pthread */ struct thread *t_cleanup; /* This client's redistribute flag. */ @@ -305,6 +313,27 @@ extern struct zserv *zserv_find_client(uint8_t proto, unsigned short instance); struct zserv *zserv_find_client_session(uint8_t proto, unsigned short instance, uint32_t session_id); +/* + * Retrieve a client object by the complete tuple of + * {protocol, instance, session}. This version supports use + * from a different pthread: the object will be returned marked + * in-use. The caller *must* release the client object with the + * release_client() api, to ensure that the in-use marker is cleared properly. + * + * Returns: + * The Zebra API client. + */ +extern struct zserv *zserv_acquire_client(uint8_t proto, + unsigned short instance, + uint32_t session_id); + +/* + * Release a client object that was acquired with the acquire_client() api. + * After this has been called, the pointer must not be used - it may be freed + * in another pthread if the client has closed. + */ +extern void zserv_release_client(struct zserv *client); + /* * Close a client. *