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 <mstapp@nvidia.com>

diff --git a/zebra/kernel_netlink.c b/zebra/kernel_netlink.c
index d0c86a6bb0e36ab2d77378bb8e173d95af8f5d76..33f26ab0b6e961ea15f59be575eb4c010a7eec02 100644 (file)
--- 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 3c9d203a0b034658c1592d5fedd391c2f8075365..b9228072e5ed01abc9eb2bf9d25c4a17c81b3681 100644 (file)
--- 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 90148d2c0d8489ea79ef2703509c0731dfdaa3e9..5e626928d9adffe90f59f439bc551bf79eb7dc65 100644 (file)
--- 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 92a3b9424b3778fd5325cbada85c3734f1b53d36..6b4a7543cd17c2ba48dbdb62b38baf707cb92199 100644 (file)
--- 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();
 }