vcl: mt detection and cleanup

Type: improvement

Signed-off-by: Florin Coras <fcoras@cisco.com>
Change-Id: I521c110fd4d7445bd585c96d4c768f16a0a7d3b8

diff --git a/src/vcl/ldp.c b/src/vcl/ldp.c
index fddf45c..a4b66fe 100644 (file)
--- a/src/vcl/ldp.c
+++ b/src/vcl/ldp.c
@@ -2438,9 +2438,10 @@ ldp_epoll_pwait_eventfd (int epfd, struct epoll_event *events,
       return -1;
     }
 
-  if (vls_mt_wrk_supported ())
-    if (PREDICT_FALSE (vppcom_worker_index () == ~0))
-      vls_register_vcl_worker ();
+  /* Make sure the vcl worker is valid. Could be that epoll fd was created on
+   * one thread but it is now used on another */
+  if (PREDICT_FALSE (vppcom_worker_index () == ~0))
+    vls_register_vcl_worker ();
 
   ldpw = ldp_worker_get_current ();
   if (epfd == ldpw->vcl_mq_epfd)

diff --git a/src/vcl/vcl_cfg.c b/src/vcl/vcl_cfg.c
index a94c874..f7e271b 100644 (file)
--- a/src/vcl/vcl_cfg.c
+++ b/src/vcl/vcl_cfg.c
@@ -498,10 +498,11 @@ vppcom_cfg_read_file (char *conf_fname)
              VCFG_DBG (0, "VCL<%d>: configured tls-engine %u (0x%x)",
                        getpid (), vcl_cfg->tls_engine, vcl_cfg->tls_engine);
            }
-         else if (unformat (line_input, "multi-thread"))
+         else if (unformat (line_input, "multi-thread-workers"))
            {
-             vcl_cfg->mt_supported = 1;
-             VCFG_DBG (0, "VCL<%d>: configured with multithread", getpid ());
+             vcl_cfg->mt_wrk_supported = 1;
+             VCFG_DBG (0, "VCL<%d>: configured with multithread workers",
+                       getpid ());
            }
          else if (unformat (line_input, "}"))
            {

diff --git a/src/vcl/vcl_locked.c b/src/vcl/vcl_locked.c
index 02da8cd..da4522a 100644 (file)
--- a/src/vcl/vcl_locked.c
+++ b/src/vcl/vcl_locked.c
@@ -169,28 +169,28 @@ vls_shared_data_pool_runlock (void)
 }
 
 static inline void
-vls_table_rlock (void)
+vls_mt_table_rlock (void)
 {
   if (vlsl->vls_mt_n_threads > 1)
     clib_rwlock_reader_lock (&vlsm->vls_table_lock);
 }
 
 static inline void
-vls_table_runlock (void)
+vls_mt_table_runlock (void)
 {
   if (vlsl->vls_mt_n_threads > 1)
     clib_rwlock_reader_unlock (&vlsm->vls_table_lock);
 }
 
 static inline void
-vls_table_wlock (void)
+vls_mt_table_wlock (void)
 {
   if (vlsl->vls_mt_n_threads > 1)
     clib_rwlock_writer_lock (&vlsm->vls_table_lock);
 }
 
 static inline void
-vls_table_wunlock (void)
+vls_mt_table_wunlock (void)
 {
   if (vlsl->vls_mt_n_threads > 1)
     clib_rwlock_writer_unlock (&vlsm->vls_table_lock);
@@ -214,6 +214,10 @@ static void
 vls_mt_add (void)
 {
   vlsl->vls_mt_n_threads += 1;
+
+  /* If multi-thread workers are supported, for each new thread register a new
+   * vcl worker with vpp. Otherwise, all threads use the same vcl worker, so
+   * update the vcl worker's thread local worker index variable */
   if (vls_mt_wrk_supported ())
     vls_register_vcl_worker ();
   else
@@ -282,7 +286,7 @@ vls_to_sh_tu (vcl_locked_session_t * vls)
 {
   vcl_session_handle_t sh;
   sh = vls_to_sh (vls);
-  vls_table_runlock ();
+  vls_mt_table_runlock ();
   return sh;
 }
 
@@ -323,7 +327,7 @@ vls_alloc (vcl_session_handle_t sh)
   vls_worker_t *wrk = vls_worker_get_current ();
   vcl_locked_session_t *vls;
 
-  vls_table_wlock ();
+  vls_mt_table_wlock ();
 
   pool_get_zero (wrk->vls_pool, vls);
   vls->session_index = vppcom_session_index (sh);
@@ -340,7 +344,7 @@ vls_alloc (vcl_session_handle_t sh)
     }
   clib_spinlock_init (&vls->lock);
 
-  vls_table_wunlock ();
+  vls_mt_table_wunlock ();
   return vls->vls_index;
 }
 
@@ -380,10 +384,10 @@ static vcl_locked_session_t *
 vls_get_w_dlock (vls_handle_t vlsh)
 {
   vcl_locked_session_t *vls;
-  vls_table_rlock ();
+  vls_mt_table_rlock ();
   vls = vls_get_and_lock (vlsh);
   if (!vls)
-    vls_table_runlock ();
+    vls_mt_table_runlock ();
   return vls;
 }
 
@@ -391,17 +395,17 @@ static inline void
 vls_get_and_unlock (vls_handle_t vlsh)
 {
   vcl_locked_session_t *vls;
-  vls_table_rlock ();
+  vls_mt_table_rlock ();
   vls = vls_get (vlsh);
   vls_unlock (vls);
-  vls_table_runlock ();
+  vls_mt_table_runlock ();
 }
 
 static inline void
 vls_dunlock (vcl_locked_session_t * vls)
 {
   vls_unlock (vls);
-  vls_table_runlock ();
+  vls_mt_table_runlock ();
 }
 
 static vcl_locked_session_t *
@@ -448,9 +452,9 @@ vls_session_index_to_vlsh (uint32_t session_index)
 {
   vls_handle_t vlsh;
 
-  vls_table_rlock ();
+  vls_mt_table_rlock ();
   vlsh = vls_si_to_vlsh (session_index);
-  vls_table_runlock ();
+  vls_mt_table_runlock ();
 
   return vlsh;
 }
@@ -826,8 +830,15 @@ vls_mt_rel_locks (int locks_acq)
     vls_mt_create_unlock ();
 }
 
+static inline u8
+vls_mt_session_should_migrate (vcl_locked_session_t * vls)
+{
+  return (vls_mt_wrk_supported ()
+         && vls->worker_index != vcl_get_worker_index ());
+}
+
 static void
-vls_session_migrate (vcl_locked_session_t * vls)
+vls_mt_session_migrate (vcl_locked_session_t * vls)
 {
   u32 wrk_index = vcl_get_worker_index ();
   vcl_worker_t *wrk;
@@ -835,11 +846,12 @@ vls_session_migrate (vcl_locked_session_t * vls)
   vcl_session_t *session;
   uword *p;
 
-  if (!vls_mt_wrk_supported ())
-    return;
+  ASSERT (vls_mt_wrk_supported () && vls->worker_index != wrk_index);
 
-  if (PREDICT_TRUE (vls->worker_index == wrk_index))
-    return;
+  /*
+   * VCL session on current vcl worker already allocated. Update current
+   * owner worker and index and return
+   */
   if ((p = hash_get (vls->vcl_wrk_index_to_session_index, wrk_index)))
     {
       vls->worker_index = wrk_index;
@@ -847,7 +859,11 @@ vls_session_migrate (vcl_locked_session_t * vls)
       return;
     }
 
-  /* migrate from orignal vls */
+  /*
+   * Ask vcl worker that owns the original vcl session to clone it into
+   * current vcl worker session pool
+   */
+
   if (!(p = hash_get (vls->vcl_wrk_index_to_session_index,
                      vls->owner_vcl_wrk_index)))
     {
@@ -888,19 +904,30 @@ vls_session_migrate (vcl_locked_session_t * vls)
     }
 }
 
-#define vls_mt_guard(_vls, _op)                                \
-  int _locks_acq = 0;                                  \
-  if (PREDICT_FALSE (vcl_get_worker_index () == ~0))   \
-    vls_mt_add ();                                     \
-  if (PREDICT_FALSE (_vls && vls_mt_wrk_supported () && \
-      ((vcl_locked_session_t *)_vls)->worker_index !=  \
-      vcl_get_worker_index ()))                                \
-    vls_session_migrate (_vls);                                \
-  if (PREDICT_FALSE (vlsl->vls_mt_n_threads > 1))      \
-    vls_mt_acq_locks (_vls, _op, &_locks_acq);         \
+static inline void
+vls_mt_detect (void)
+{
+  if (PREDICT_FALSE (vcl_get_worker_index () == ~0))
+    vls_mt_add ();
+}
 
-#define vls_mt_unguard()                               \
-  if (PREDICT_FALSE (_locks_acq))                      \
+#define vls_mt_guard(_vls, _op)                                                \
+  int _locks_acq = 0;                                                  \
+  if (vls_mt_wrk_supported ())                                         \
+    {                                                                  \
+      if (PREDICT_FALSE (_vls                                          \
+           && ((vcl_locked_session_t *)_vls)->worker_index !=          \
+               vcl_get_worker_index ()))                               \
+         vls_mt_session_migrate (_vls);                                \
+    }                                                                  \
+  else                                                                 \
+    {                                                                  \
+      if (PREDICT_FALSE (vlsl->vls_mt_n_threads > 1))                  \
+        vls_mt_acq_locks (_vls, _op, &_locks_acq);                     \
+    }                                                                  \
+
+#define vls_mt_unguard()                                               \
+  if (PREDICT_FALSE (_locks_acq))                                      \
     vls_mt_rel_locks (_locks_acq)
 
 int
@@ -909,6 +936,7 @@ vls_write (vls_handle_t vlsh, void *buf, size_t nbytes)
   vcl_locked_session_t *vls;
   int rv;
 
+  vls_mt_detect ();
   if (!(vls = vls_get_w_dlock (vlsh)))
     return VPPCOM_EBADFD;
 
@@ -925,6 +953,7 @@ vls_write_msg (vls_handle_t vlsh, void *buf, size_t nbytes)
   vcl_locked_session_t *vls;
   int rv;
 
+  vls_mt_detect ();
   if (!(vls = vls_get_w_dlock (vlsh)))
     return VPPCOM_EBADFD;
   vls_mt_guard (vls, VLS_MT_OP_WRITE);
@@ -941,6 +970,7 @@ vls_sendto (vls_handle_t vlsh, void *buf, int buflen, int flags,
   vcl_locked_session_t *vls;
   int rv;
 
+  vls_mt_detect ();
   if (!(vls = vls_get_w_dlock (vlsh)))
     return VPPCOM_EBADFD;
   vls_mt_guard (vls, VLS_MT_OP_WRITE);
@@ -956,6 +986,7 @@ vls_read (vls_handle_t vlsh, void *buf, size_t nbytes)
   vcl_locked_session_t *vls;
   int rv;
 
+  vls_mt_detect ();
   if (!(vls = vls_get_w_dlock (vlsh)))
     return VPPCOM_EBADFD;
   vls_mt_guard (vls, VLS_MT_OP_READ);
@@ -972,6 +1003,7 @@ vls_recvfrom (vls_handle_t vlsh, void *buffer, uint32_t buflen, int flags,
   vcl_locked_session_t *vls;
   int rv;
 
+  vls_mt_detect ();
   if (!(vls = vls_get_w_dlock (vlsh)))
     return VPPCOM_EBADFD;
   vls_mt_guard (vls, VLS_MT_OP_READ);
@@ -988,12 +1020,11 @@ vls_attr (vls_handle_t vlsh, uint32_t op, void *buffer, uint32_t * buflen)
   vcl_locked_session_t *vls;
   int rv;
 
-  if (PREDICT_FALSE (vcl_get_worker_index () == ~0))
-    vls_mt_add ();
-
+  vls_mt_detect ();
   if (!(vls = vls_get_w_dlock (vlsh)))
     return VPPCOM_EBADFD;
-  vls_session_migrate (vls);
+  if (vls_mt_session_should_migrate (vls))
+    vls_mt_session_migrate (vls);
   rv = vppcom_session_attr (vls_to_sh_tu (vls), op, buffer, buflen);
   vls_get_and_unlock (vlsh);
   return rv;
@@ -1005,6 +1036,7 @@ vls_bind (vls_handle_t vlsh, vppcom_endpt_t * ep)
   vcl_locked_session_t *vls;
   int rv;
 
+  vls_mt_detect ();
   if (!(vls = vls_get_w_dlock (vlsh)))
     return VPPCOM_EBADFD;
   rv = vppcom_session_bind (vls_to_sh_tu (vls), ep);
@@ -1018,6 +1050,7 @@ vls_listen (vls_handle_t vlsh, int q_len)
   vcl_locked_session_t *vls;
   int rv;
 
+  vls_mt_detect ();
   if (!(vls = vls_get_w_dlock (vlsh)))
     return VPPCOM_EBADFD;
   vls_mt_guard (vls, VLS_MT_OP_XPOLL);
@@ -1033,6 +1066,7 @@ vls_connect (vls_handle_t vlsh, vppcom_endpt_t * server_ep)
   vcl_locked_session_t *vls;
   int rv;
 
+  vls_mt_detect ();
   if (!(vls = vls_get_w_dlock (vlsh)))
     return VPPCOM_EBADFD;
   vls_mt_guard (vls, VLS_MT_OP_XPOLL);
@@ -1093,6 +1127,7 @@ vls_accept (vls_handle_t listener_vlsh, vppcom_endpt_t * ep, int flags)
   vcl_locked_session_t *vls;
   int sh;
 
+  vls_mt_detect ();
   if (!(vls = vls_get_w_dlock (listener_vlsh)))
     return VPPCOM_EBADFD;
   if (vcl_n_workers () > 1)
@@ -1115,6 +1150,7 @@ vls_create (uint8_t proto, uint8_t is_nonblocking)
   vcl_session_handle_t sh;
   vls_handle_t vlsh;
 
+  vls_mt_detect ();
   vls_mt_guard (0, VLS_MT_OP_SPOOL);
   sh = vppcom_session_create (proto, is_nonblocking);
   vls_mt_unguard ();
@@ -1129,21 +1165,20 @@ vls_create (uint8_t proto, uint8_t is_nonblocking)
 }
 
 static void
-vls_migrate_session_close (vcl_locked_session_t * vls)
+vls_mt_session_cleanup (vcl_locked_session_t * vls)
 {
-  u32 session_index, wrk_index;
+  u32 session_index, wrk_index, current_vcl_wrk;
   vcl_worker_t *wrk = vcl_worker_get_current ();
 
-  if (!vls_mt_wrk_supported ())
-    return;
+  ASSERT (vls_mt_wrk_supported ());
+
+  current_vcl_wrk = vcl_get_worker_index ();
 
   /* *INDENT-OFF* */
   hash_foreach (wrk_index, session_index, vls->vcl_wrk_index_to_session_index,
     ({
-      if (vcl_get_worker_index () != wrk_index)
-       {
-         vls_send_session_cleanup_rpc (wrk, wrk_index, session_index);
-       }
+      if (current_vcl_wrk != wrk_index)
+       vls_send_session_cleanup_rpc (wrk, wrk_index, session_index);
     }));
   /* *INDENT-ON* */
   hash_free (vls->vcl_wrk_index_to_session_index);
@@ -1155,12 +1190,13 @@ vls_close (vls_handle_t vlsh)
   vcl_locked_session_t *vls;
   int rv;
 
-  vls_table_wlock ();
+  vls_mt_detect ();
+  vls_mt_table_wlock ();
 
   vls = vls_get_and_lock (vlsh);
   if (!vls)
     {
-      vls_table_wunlock ();
+      vls_mt_table_wunlock ();
       return VPPCOM_EBADFD;
     }
 
@@ -1171,12 +1207,13 @@ vls_close (vls_handle_t vlsh)
   else
     rv = vppcom_session_close (vls_to_sh (vls));
 
-  vls_migrate_session_close (vls);
+  if (vls_mt_wrk_supported ())
+    vls_mt_session_cleanup (vls);
 
   vls_free (vls);
   vls_mt_unguard ();
 
-  vls_table_wunlock ();
+  vls_mt_table_wunlock ();
 
   return rv;
 }
@@ -1187,8 +1224,7 @@ vls_epoll_create (void)
   vcl_session_handle_t sh;
   vls_handle_t vlsh;
 
-  if (PREDICT_FALSE (vcl_get_worker_index () == ~0))
-    vls_mt_add ();
+  vls_mt_detect ();
 
   sh = vppcom_epoll_create ();
   if (sh == INVALID_SESSION_ID)
@@ -1225,26 +1261,30 @@ vls_epoll_ctl (vls_handle_t ep_vlsh, int op, vls_handle_t vlsh,
   vcl_session_handle_t ep_sh, sh;
   int rv;
 
-  vls_table_rlock ();
+  vls_mt_detect ();
+  vls_mt_table_rlock ();
   ep_vls = vls_get_and_lock (ep_vlsh);
   vls = vls_get_and_lock (vlsh);
-  vls_session_migrate (ep_vls);
+
+  if (vls_mt_session_should_migrate (ep_vls))
+    vls_mt_session_migrate (ep_vls);
+
   ep_sh = vls_to_sh (ep_vls);
   sh = vls_to_sh (vls);
 
   if (PREDICT_FALSE (!vlsl->epoll_mp_check))
     vls_epoll_ctl_mp_checks (vls, op);
 
-  vls_table_runlock ();
+  vls_mt_table_runlock ();
 
   rv = vppcom_epoll_ctl (ep_sh, op, sh, event);
 
-  vls_table_rlock ();
+  vls_mt_table_rlock ();
   ep_vls = vls_get (ep_vlsh);
   vls = vls_get (vlsh);
   vls_unlock (vls);
   vls_unlock (ep_vls);
-  vls_table_runlock ();
+  vls_mt_table_runlock ();
   return rv;
 }
 
@@ -1255,6 +1295,7 @@ vls_epoll_wait (vls_handle_t ep_vlsh, struct epoll_event *events,
   vcl_locked_session_t *vls;
   int rv;
 
+  vls_mt_detect ();
   if (!(vls = vls_get_w_dlock (ep_vlsh)))
     return VPPCOM_EBADFD;
   vls_mt_guard (0, VLS_MT_OP_XPOLL);
@@ -1303,6 +1344,7 @@ vls_select (int n_bits, vcl_si_set * read_map, vcl_si_set * write_map,
 {
   int rv;
 
+  vls_mt_detect ();
   vls_mt_guard (0, VLS_MT_OP_XPOLL);
   if (PREDICT_FALSE (!vlsl->select_mp_check))
     vls_select_mp_checks (read_map);
@@ -1595,8 +1637,7 @@ vls_send_clone_and_share_rpc (vcl_worker_t * wrk,
   wrk->rpc_done = 0;
   ret = vcl_send_worker_rpc (dst_wrk_index, rpc, sizeof (data));
 
-  VDBG (1,
-       "send session clone of worker (session): %u (%u) -> %u (%u), ret=%d",
+  VDBG (1, "send session clone to wrk (session): %u (%u) -> %u (%u), ret=%d",
        dst_wrk_index, msg->session_index, msg->origin_vcl_wrk,
        msg->origin_session_index, ret);
   while (!ret && !wrk->rpc_done)
@@ -1621,8 +1662,7 @@ vls_send_session_cleanup_rpc (vcl_worker_t * wrk,
   wrk->rpc_done = 0;
   ret = vcl_send_worker_rpc (dst_wrk_index, rpc, sizeof (data));
 
-  VDBG (1,
-       "send session cleanup of worker (session): %u (%u) from %u (), ret=%d",
+  VDBG (1, "send session cleanup to wrk (session): %u (%u) from %u, ret=%d",
        dst_wrk_index, msg->session_index, msg->origin_vcl_wrk, ret);
   while (!ret && !wrk->rpc_done)
     ;
@@ -1661,7 +1701,7 @@ vls_use_eventfd (void)
 unsigned char
 vls_mt_wrk_supported (void)
 {
-  return vcm->cfg.mt_supported;
+  return vcm->cfg.mt_wrk_supported;
 }
 
 int

diff --git a/src/vcl/vcl_private.h b/src/vcl/vcl_private.h
index b873ad9..231d7eb 100644 (file)
--- a/src/vcl/vcl_private.h
+++ b/src/vcl/vcl_private.h
@@ -218,7 +218,7 @@ typedef struct vppcom_cfg_t_
   u8 *vpp_api_socket_name;
   u8 *vpp_api_chroot;
   u32 tls_engine;
-  u8 mt_supported;
+  u8 mt_wrk_supported;
 } vppcom_cfg_t;
 
 void vppcom_cfg (vppcom_cfg_t * vcl_cfg);