From 348698095d1e1dc9904e13c86ce8492d89558a8d Mon Sep 17 00:00:00 2001 From: Mark Stapp Date: Thu, 10 Feb 2022 13:29:59 -0500 Subject: [PATCH] zebra: make netlink object hash threadsafe The recently-added hashtable of nlsock objects needs to be thread-safe: it's accessed from the main and dplane pthreads. Add a mutex for it, use wrapper apis when accessing it. Add a per-OS init/terminate api so we can do init that's not per-vrf or per-namespace. Signed-off-by: Mark Stapp --- zebra/kernel_netlink.c | 125 ++++++++++++++++++++++++++--------------- zebra/kernel_socket.c | 14 +++++ zebra/rt.h | 4 ++ zebra/zebra_router.c | 6 ++ 4 files changed, 105 insertions(+), 44 deletions(-) diff --git a/zebra/kernel_netlink.c b/zebra/kernel_netlink.c index d0c86a6bb0..33f26ab0b6 100644 --- a/zebra/kernel_netlink.c +++ b/zebra/kernel_netlink.c @@ -159,7 +159,17 @@ extern struct zebra_privs_t zserv_privs; DEFINE_MTYPE_STATIC(ZEBRA, NL_BUF, "Zebra Netlink buffers"); -struct hash *nlsock_hash; +/* Hashtable and mutex to allow lookup of nlsock structs by socket/fd value. + * We have both the main and dplane pthreads using these structs, so we have + * to protect the hash with a lock. + */ +static struct hash *nlsock_hash; +pthread_mutex_t nlsock_mutex; + +/* Lock and unlock wrappers for nlsock hash */ +#define NLSOCK_LOCK() pthread_mutex_lock(&nlsock_mutex) +#define NLSOCK_UNLOCK() pthread_mutex_unlock(&nlsock_mutex) + size_t nl_batch_tx_bufsize; char *nl_batch_tx_buf; @@ -1513,11 +1523,31 @@ void kernel_update_multi(struct dplane_ctx_q *ctx_list) struct nlsock *kernel_netlink_nlsock_lookup(int sock) { - struct nlsock lookup; + struct nlsock lookup, *retval; lookup.sock = sock; - return hash_lookup(nlsock_hash, &lookup); + NLSOCK_LOCK(); + retval = hash_lookup(nlsock_hash, &lookup); + NLSOCK_UNLOCK(); + + return retval; +} + +/* Insert nlsock entry into hash */ +static void kernel_netlink_nlsock_insert(struct nlsock *nls) +{ + NLSOCK_LOCK(); + (void)hash_get(nlsock_hash, nls, hash_alloc_intern); + NLSOCK_UNLOCK(); +} + +/* Remove nlsock entry from hash */ +static void kernel_netlink_nlsock_remove(struct nlsock *nls) +{ + NLSOCK_LOCK(); + (void)hash_release(nlsock_hash, nls); + NLSOCK_UNLOCK(); } static uint32_t kernel_netlink_nlsock_key(const void *arg) @@ -1547,11 +1577,6 @@ void kernel_init(struct zebra_ns *zns) int one, ret; #endif - if (!nlsock_hash) - nlsock_hash = hash_create_size(8, kernel_netlink_nlsock_key, - kernel_netlink_nlsock_hash_equal, - "Netlink Socket Hash"); - /* * Initialize netlink sockets * @@ -1584,7 +1609,8 @@ void kernel_init(struct zebra_ns *zns) zns->netlink.name); exit(-1); } - (void)hash_get(nlsock_hash, &zns->netlink, hash_alloc_intern); + + kernel_netlink_nlsock_insert(&zns->netlink); snprintf(zns->netlink_cmd.name, sizeof(zns->netlink_cmd.name), "netlink-cmd (NS %u)", zns->ns_id); @@ -1594,7 +1620,8 @@ void kernel_init(struct zebra_ns *zns) zns->netlink_cmd.name); exit(-1); } - (void)hash_get(nlsock_hash, &zns->netlink_cmd, hash_alloc_intern); + + kernel_netlink_nlsock_insert(&zns->netlink_cmd); /* Outbound socket for dplane programming of the host OS. */ snprintf(zns->netlink_dplane_out.name, @@ -1606,8 +1633,8 @@ void kernel_init(struct zebra_ns *zns) zns->netlink_dplane_out.name); exit(-1); } - (void)hash_get(nlsock_hash, &zns->netlink_dplane_out, - hash_alloc_intern); + + kernel_netlink_nlsock_insert(&zns->netlink_dplane_out); /* Inbound socket for OS events coming to the dplane. */ snprintf(zns->netlink_dplane_in.name, @@ -1620,7 +1647,8 @@ void kernel_init(struct zebra_ns *zns) zns->netlink_dplane_in.name); exit(-1); } - (void)hash_get(nlsock_hash, &zns->netlink_dplane_in, hash_alloc_intern); + + kernel_netlink_nlsock_insert(&zns->netlink_dplane_in); /* * SOL_NETLINK is not available on all platforms yet @@ -1706,47 +1734,56 @@ void kernel_init(struct zebra_ns *zns) rt_netlink_init(); } +/* Helper to clean up an nlsock */ +static void kernel_nlsock_fini(struct nlsock *nls) +{ + if (nls && nls->sock >= 0) { + kernel_netlink_nlsock_remove(nls); + close(nls->sock); + nls->sock = -1; + XFREE(MTYPE_NL_BUF, nls->buf); + nls->buflen = 0; + } +} + void kernel_terminate(struct zebra_ns *zns, bool complete) { thread_cancel(&zns->t_netlink); - if (zns->netlink.sock >= 0) { - hash_release(nlsock_hash, &zns->netlink); - close(zns->netlink.sock); - zns->netlink.sock = -1; - XFREE(MTYPE_NL_BUF, zns->netlink.buf); - zns->netlink.buflen = 0; - } + kernel_nlsock_fini(&zns->netlink); - if (zns->netlink_cmd.sock >= 0) { - hash_release(nlsock_hash, &zns->netlink_cmd); - close(zns->netlink_cmd.sock); - zns->netlink_cmd.sock = -1; - XFREE(MTYPE_NL_BUF, zns->netlink_cmd.buf); - zns->netlink_cmd.buflen = 0; - } + kernel_nlsock_fini(&zns->netlink_cmd); - if (zns->netlink_dplane_in.sock >= 0) { - hash_release(nlsock_hash, &zns->netlink_dplane_in); - close(zns->netlink_dplane_in.sock); - zns->netlink_dplane_in.sock = -1; - XFREE(MTYPE_NL_BUF, zns->netlink_dplane_in.buf); - zns->netlink_dplane_in.buflen = 0; - } + kernel_nlsock_fini(&zns->netlink_dplane_in); /* During zebra shutdown, we need to leave the dataplane socket * around until all work is done. */ - if (complete) { - if (zns->netlink_dplane_out.sock >= 0) { - hash_release(nlsock_hash, &zns->netlink_dplane_out); - close(zns->netlink_dplane_out.sock); - zns->netlink_dplane_out.sock = -1; - XFREE(MTYPE_NL_BUF, zns->netlink_dplane_out.buf); - zns->netlink_dplane_out.buflen = 0; - } + if (complete) + kernel_nlsock_fini(&zns->netlink_dplane_out); +} - hash_free(nlsock_hash); - } +/* + * Global init for platform-/OS-specific things + */ +void kernel_router_init(void) +{ + /* Init nlsock hash and lock */ + pthread_mutex_init(&nlsock_mutex, NULL); + nlsock_hash = hash_create_size(8, kernel_netlink_nlsock_key, + kernel_netlink_nlsock_hash_equal, + "Netlink Socket Hash"); +} + +/* + * Global deinit for platform-/OS-specific things + */ +void kernel_router_terminate(void) +{ + pthread_mutex_destroy(&nlsock_mutex); + + hash_free(nlsock_hash); + nlsock_hash = NULL; } + #endif /* HAVE_NETLINK */ diff --git a/zebra/kernel_socket.c b/zebra/kernel_socket.c index 3c9d203a0b..b9228072e5 100644 --- a/zebra/kernel_socket.c +++ b/zebra/kernel_socket.c @@ -1459,6 +1459,20 @@ void kernel_terminate(struct zebra_ns *zns, bool complete) return; } +/* + * Global init for platform-/OS-specific things + */ +void kernel_router_init(void) +{ +} + +/* + * Global deinit for platform-/OS-specific things + */ +void kernel_router_terminate(void) +{ +} + /* * Called by the dplane pthread to read incoming OS messages and dispatch them. */ diff --git a/zebra/rt.h b/zebra/rt.h index 90148d2c0d..5e626928d9 100644 --- a/zebra/rt.h +++ b/zebra/rt.h @@ -78,6 +78,10 @@ extern int kernel_interface_set_master(struct interface *master, extern int mpls_kernel_init(void); +/* Global init and deinit for platform-/OS-specific things */ +void kernel_router_init(void); +void kernel_router_terminate(void); + extern uint32_t kernel_get_speed(struct interface *ifp, int *error); extern int kernel_get_ipmr_sg_stats(struct zebra_vrf *zvrf, void *mroute); diff --git a/zebra/zebra_router.c b/zebra/zebra_router.c index 92a3b9424b..6b4a7543cd 100644 --- a/zebra/zebra_router.c +++ b/zebra/zebra_router.c @@ -262,6 +262,9 @@ void zebra_router_terminate(void) #ifdef HAVE_SCRIPTING zebra_script_destroy(); #endif + + /* OS-specific deinit */ + kernel_router_terminate(); } bool zebra_router_notify_on_ack(void) @@ -307,4 +310,7 @@ void zebra_router_init(bool asic_offload, bool notify_on_ack) #ifdef HAVE_SCRIPTING zebra_script_init(); #endif + + /* OS-specific init */ + kernel_router_init(); } -- 2.39.5