vcl: remove listen_no_mq from state to flag

One less state in state machine

Type: improvement

Change-Id: Ib6445a425b1e2d5a957318a94c3f132cddd8370b
Signed-off-by: Florin Coras <[email protected]>

diff --git a/src/vcl/vcl_locked.c b/src/vcl/vcl_locked.c
index f38df8f..7ba9fab 100644 (file)
--- a/src/vcl/vcl_locked.c
+++ b/src/vcl/vcl_locked.c
@@ -743,6 +743,7 @@ vls_listener_wrk_start_listen (vcl_locked_session_t * vls, u32 wrk_index)
   if (ls->flags & VCL_SESSION_F_PENDING_LISTEN)
     return;
 
+  ls->flags &= ~VCL_SESSION_F_LISTEN_NO_MQ;
   vcl_send_session_listen (wrk, ls);
 
   vls_listener_wrk_set (vls, wrk_index, 1 /* is_active */);
@@ -759,7 +760,7 @@ vls_listener_wrk_stop_listen (vcl_locked_session_t * vls, u32 wrk_index)
   if (s->session_state != VCL_STATE_LISTEN)
     return;
   vcl_send_session_unlisten (wrk, s);
-  s->session_state = VCL_STATE_LISTEN_NO_MQ;
+  s->flags |= VCL_SESSION_F_LISTEN_NO_MQ;
   vls_listener_wrk_set (vls, wrk_index, 0 /* is_active */ );
 }
 
@@ -912,7 +913,7 @@ vls_share_session (vls_worker_t * vls_wrk, vcl_locked_session_t * vls)
 
   if (s->session_state == VCL_STATE_LISTEN)
     {
-      s->session_state = VCL_STATE_LISTEN_NO_MQ;
+      s->flags |= VCL_SESSION_F_LISTEN_NO_MQ;
       s->rx_fifo = s->tx_fifo = 0;
     }
   else if (s->rx_fifo)
@@ -1384,36 +1385,41 @@ vls_mp_checks (vcl_locked_session_t * vls, int is_add)
   switch (s->session_state)
     {
     case VCL_STATE_LISTEN:
-      if (is_add)
+      if (!(s->flags & VCL_SESSION_F_LISTEN_NO_MQ))
        {
-         vls_listener_wrk_set (vls, vls->vcl_wrk_index, 1 /* is_active */);
-         break;
+         if (is_add)
+           {
+             vls_listener_wrk_set (vls, vls->vcl_wrk_index,
+                                   1 /* is_active */);
+             break;
+           }
+         /* Although removal from epoll means listener no longer accepts new
+          * sessions, the accept queue built by vpp cannot be drained by
+          * stopping the listener. Morover, some applications, e.g., nginx,
+          * might constantly remove and add listeners to their epfds. Removing
+          * listeners in such situations causes a lot of churn in vpp as
+          * segments and segment managers need to be recreated. */
+         /* vls_listener_wrk_stop_listen (vls, vls->vcl_wrk_index); */
+       }
+      else
+       {
+         if (!is_add)
+           break;
+
+         /* Register worker as listener */
+         vls_listener_wrk_start_listen (vls, vls->vcl_wrk_index);
+
+         /* If owner worker did not attempt to accept/xpoll on the session,
+          * force a listen stop for it, since it may not be interested in
+          * accepting new sessions.
+          * This is pretty much a hack done to give app workers the illusion
+          * that it is fine to listen and not accept new sessions for a
+          * given listener. Without it, we would accumulate unhandled
+          * accepts on the passive worker message queue. */
+         owner_wrk = vls_shared_get_owner (vls);
+         if (!vls_listener_wrk_is_active (vls, owner_wrk))
+           vls_listener_wrk_stop_listen (vls, owner_wrk);
        }
-      /* Although removal from epoll means listener no longer accepts new
-       * sessions, the accept queue built by vpp cannot be drained by stopping
-       * the listener. Morover, some applications, e.g., nginx, might
-       * constantly remove and add listeners to their epfds. Removing
-       * listeners in such situations causes a lot of churn in vpp as segments
-       * and segment managers need to be recreated. */
-      /* vls_listener_wrk_stop_listen (vls, vls->vcl_wrk_index); */
-      break;
-    case VCL_STATE_LISTEN_NO_MQ:
-      if (!is_add)
-       break;
-
-      /* Register worker as listener */
-      vls_listener_wrk_start_listen (vls, vls->vcl_wrk_index);
-
-      /* If owner worker did not attempt to accept/xpoll on the session,
-       * force a listen stop for it, since it may not be interested in
-       * accepting new sessions.
-       * This is pretty much a hack done to give app workers the illusion
-       * that it is fine to listen and not accept new sessions for a
-       * given listener. Without it, we would accumulate unhandled
-       * accepts on the passive worker message queue. */
-      owner_wrk = vls_shared_get_owner (vls);
-      if (!vls_listener_wrk_is_active (vls, owner_wrk))
-       vls_listener_wrk_stop_listen (vls, owner_wrk);
       break;
     default:
       break;

diff --git a/src/vcl/vcl_private.c b/src/vcl/vcl_private.c
index d981439..b1f159f 100644 (file)
--- a/src/vcl/vcl_private.c
+++ b/src/vcl/vcl_private.c
@@ -201,11 +201,10 @@ vcl_worker_detach_sessions (vcl_worker_t *wrk)
     {
       if (s->session_state == VCL_STATE_LISTEN)
        {
-         s->session_state = VCL_STATE_LISTEN_NO_MQ;
+         s->flags |= VCL_SESSION_F_LISTEN_NO_MQ;
          continue;
        }
       if ((s->flags & VCL_SESSION_F_IS_VEP) ||
-         s->session_state == VCL_STATE_LISTEN_NO_MQ ||
          s->session_state == VCL_STATE_CLOSED)
        continue;
 
@@ -773,9 +772,6 @@ vcl_session_state_str (vcl_session_state_t state)
     case VCL_STATE_UPDATED:
       st = "STATE_UPDATED";
       break;
-    case VCL_STATE_LISTEN_NO_MQ:
-      st = "STATE_LISTEN_NO_MQ";
-      break;
     default:
       st = "UNKNOWN_STATE";
       break;

diff --git a/src/vcl/vcl_private.h b/src/vcl/vcl_private.h
index c98e1cd..c92bb58 100644 (file)
--- a/src/vcl/vcl_private.h
+++ b/src/vcl/vcl_private.h
@@ -71,7 +71,6 @@ typedef enum vcl_session_state_
   VCL_STATE_DISCONNECT,
   VCL_STATE_DETACHED,
   VCL_STATE_UPDATED,
-  VCL_STATE_LISTEN_NO_MQ,
 } vcl_session_state_t;
 
 typedef struct epoll_event vppcom_epoll_event_t;
@@ -144,6 +143,7 @@ typedef enum vcl_session_flags_
   VCL_SESSION_F_PENDING_FREE = 1 << 7,
   VCL_SESSION_F_PENDING_LISTEN = 1 << 8,
   VCL_SESSION_F_APP_CLOSING = 1 << 9,
+  VCL_SESSION_F_LISTEN_NO_MQ = 1 << 10,
 } __clib_packed vcl_session_flags_t;
 
 typedef enum vcl_worker_wait_
@@ -563,9 +563,8 @@ vcl_session_table_lookup_listener (vcl_worker_t * wrk, u64 handle)
       return 0;
     }
 
-  ASSERT (s->session_state == VCL_STATE_LISTEN
-         || s->session_state == VCL_STATE_LISTEN_NO_MQ
-         || vcl_session_is_connectable_listener (wrk, s));
+  ASSERT (s->session_state == VCL_STATE_LISTEN ||
+         vcl_session_is_connectable_listener (wrk, s));
   return s;
 }
 

diff --git a/src/vcl/vppcom.c b/src/vcl/vppcom.c
index 19d58c3..d09dd06 100644 (file)
--- a/src/vcl/vppcom.c
+++ b/src/vcl/vppcom.c
@@ -519,8 +519,7 @@ vcl_session_reset_handler (vcl_worker_t * wrk,
     }
 
   /* Caught a reset before actually accepting the session */
-  if (session->session_state == VCL_STATE_LISTEN ||
-      session->session_state == VCL_STATE_LISTEN_NO_MQ)
+  if (session->session_state == VCL_STATE_LISTEN)
     {
       if (!vcl_flag_accepted_session (session, reset_msg->handle,
                                      VCL_ACCEPTED_F_RESET))
@@ -712,8 +711,7 @@ vcl_session_disconnected_handler (vcl_worker_t * wrk,
     return 0;
 
   /* Caught a disconnect before actually accepting the session */
-  if (session->session_state == VCL_STATE_LISTEN ||
-      session->session_state == VCL_STATE_LISTEN_NO_MQ)
+  if (session->session_state == VCL_STATE_LISTEN)
     {
       if (!vcl_flag_accepted_session (session, msg->handle,
                                      VCL_ACCEPTED_F_CLOSED))
@@ -1085,8 +1083,7 @@ vcl_handle_mq_event (vcl_worker_t * wrk, session_event_t * e)
        *    VPP_CLOSING state instead can been marked as ACCEPTED_F_CLOSED.
        */
       if (vcl_session_has_attr (s, VCL_SESS_ATTR_NONBLOCK) &&
-         !(s->session_state == VCL_STATE_LISTEN ||
-           s->session_state == VCL_STATE_LISTEN_NO_MQ))
+         !(s->session_state == VCL_STATE_LISTEN))
        {
          s->session_state = VCL_STATE_VPP_CLOSING;
          s->flags |= VCL_SESSION_F_PENDING_DISCONNECT;
@@ -1114,8 +1111,7 @@ vcl_handle_mq_event (vcl_worker_t * wrk, session_event_t * e)
        *    DISCONNECT state instead can been marked as ACCEPTED_F_RESET.
        */
       if (vcl_session_has_attr (s, VCL_SESS_ATTR_NONBLOCK) &&
-         !(s->session_state == VCL_STATE_LISTEN ||
-           s->session_state == VCL_STATE_LISTEN_NO_MQ))
+         !(s->session_state == VCL_STATE_LISTEN))
        {
          s->flags |= VCL_SESSION_F_PENDING_DISCONNECT;
          s->session_state = VCL_STATE_DISCONNECT;
@@ -1331,6 +1327,12 @@ vppcom_session_unbind (u32 session_handle)
     }
   clib_fifo_free (session->accept_evts_fifo);
 
+  if (session->flags & VCL_SESSION_F_LISTEN_NO_MQ)
+    {
+      vcl_session_free (wrk, session);
+      return VPPCOM_OK;
+    }
+
   vcl_send_session_unlisten (wrk, session);
 
   VDBG (0, "session %u [0x%llx]: sending unbind!", session->session_index,
@@ -1425,7 +1427,7 @@ vcl_api_retry_attach (vcl_worker_t *wrk)
     {
       if (s->flags & VCL_SESSION_F_IS_VEP)
        continue;
-      if (s->session_state == VCL_STATE_LISTEN_NO_MQ)
+      if (s->session_state == VCL_STATE_LISTEN)
        vppcom_session_listen (vcl_session_handle (s), 10);
       else
        VDBG (0, "internal error: unexpected state %d", s->session_state);
@@ -1769,13 +1771,16 @@ vppcom_session_listen (uint32_t listen_sh, uint32_t q_len)
     return VPPCOM_EBADFD;
 
   listen_vpp_handle = listen_session->vpp_handle;
-  if (listen_session->session_state == VCL_STATE_LISTEN)
+  if (listen_session->session_state == VCL_STATE_LISTEN &&
+      !(listen_session->flags & VCL_SESSION_F_LISTEN_NO_MQ))
     {
       VDBG (0, "session %u [0x%llx]: already in listen state!",
            listen_sh, listen_vpp_handle);
       return VPPCOM_OK;
     }
 
+  listen_session->flags &= ~VCL_SESSION_F_LISTEN_NO_MQ;
+
   VDBG (0, "session %u: sending vpp listen request...", listen_sh);
 
   /*
@@ -1851,7 +1856,6 @@ again:
     return VPPCOM_EBADFD;
 
   if ((ls->session_state != VCL_STATE_LISTEN) &&
-      (ls->session_state != VCL_STATE_LISTEN_NO_MQ) &&
       (!vcl_session_is_connectable_listener (wrk, ls)))
     {
       VDBG (0, "ERROR: session [0x%llx]: not in listen state! state (%s)",