session: connects seg manager is always first

By convention, connects segment manager will be first. Therefore it will
be the one with the first segment wherein lies the app's message queue.

Saves us the trouble of allocating it on first connect, if app started
by listening, and we no longer need to track if it's assignable to a
listener or if it can be removed.

Type: improvement

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

diff --git a/src/plugins/hs_apps/http_server.c b/src/plugins/hs_apps/http_server.c
index 34892b6..a46e0a4 100644 (file)
--- a/src/plugins/hs_apps/http_server.c
+++ b/src/plugins/hs_apps/http_server.c
@@ -671,8 +671,7 @@ http_server_session_connected_callback (u32 app_index, u32 api_context,
 static int
 http_server_add_segment_callback (u32 client_index, u64 segment_handle)
 {
-  clib_warning ("called...");
-  return -1;
+  return 0;
 }
 
 static void
@@ -732,6 +731,7 @@ http_server_attach ()
   a->session_cb_vft = &http_server_session_cb_vft;
   a->options = options;
   a->options[APP_OPTIONS_SEGMENT_SIZE] = segment_size;
+  a->options[APP_OPTIONS_ADD_SEGMENT_SIZE] = segment_size;
   a->options[APP_OPTIONS_RX_FIFO_SIZE] =
     hsm->fifo_size ? hsm->fifo_size : 8 << 10;
   a->options[APP_OPTIONS_TX_FIFO_SIZE] =

diff --git a/src/plugins/hs_apps/proxy.c b/src/plugins/hs_apps/proxy.c
index 1a49a0f..18d6edd 100644 (file)
--- a/src/plugins/hs_apps/proxy.c
+++ b/src/plugins/hs_apps/proxy.c
@@ -282,8 +282,7 @@ proxy_connected_callback (u32 app_index, u32 api_context,
 static int
 proxy_add_segment_callback (u32 client_index, u64 segment_handle)
 {
-  clib_warning ("called...");
-  return -1;
+  return 0;
 }
 
 static int

diff --git a/src/plugins/http_static/static_server.c b/src/plugins/http_static/static_server.c
index 23860b0..c715dfa 100644 (file)
--- a/src/plugins/http_static/static_server.c
+++ b/src/plugins/http_static/static_server.c
@@ -1095,8 +1095,7 @@ http_static_server_session_connected_callback (u32 app_index, u32 api_context,
 static int
 http_static_server_add_segment_callback (u32 client_index, u64 segment_handle)
 {
-  clib_warning ("called...");
-  return -1;
+  return 0;
 }
 
 static void
@@ -1157,6 +1156,7 @@ http_static_server_attach ()
   a->session_cb_vft = &http_static_server_session_cb_vft;
   a->options = options;
   a->options[APP_OPTIONS_SEGMENT_SIZE] = segment_size;
+  a->options[APP_OPTIONS_ADD_SEGMENT_SIZE] = segment_size;
   a->options[APP_OPTIONS_RX_FIFO_SIZE] =
     hsm->fifo_size ? hsm->fifo_size : 8 << 10;
   a->options[APP_OPTIONS_TX_FIFO_SIZE] =

diff --git a/src/plugins/quic/quic.c b/src/plugins/quic/quic.c
index 11c3ac2..5a57947 100644 (file)
--- a/src/plugins/quic/quic.c
+++ b/src/plugins/quic/quic.c
@@ -1451,9 +1451,6 @@ quic_start_listen (u32 quic_listen_session_index, transport_endpoint_t * tep)
 
   ccfg = &sep->ext_cfg->crypto;
   app_wrk = app_worker_get (sep->app_wrk_index);
-  /* We need to call this because we call app_worker_init_connected in
-   * quic_accept_stream, which assumes the connect segment manager exists */
-  app_worker_alloc_connects_segment_manager (app_wrk);
   app = application_get (app_wrk->app_index);
   QUIC_DBG (2, "Called quic_start_listen for app %d", app_wrk->app_index);
 

diff --git a/src/plugins/unittest/session_test.c b/src/plugins/unittest/session_test.c
index cd99b0c..ad9c976 100644 (file)
--- a/src/plugins/unittest/session_test.c
+++ b/src/plugins/unittest/session_test.c
@@ -337,6 +337,9 @@ session_test_endpoint_cfg (vlib_main_t * vm, unformat_input_t * input)
 
   attach_args.name = format (0, "session_test_server");
   attach_args.namespace_id = appns_id;
+  /* Allow server to allocate another segment for listens. Needed
+   * because by default we do not allow segment additions */
+  attach_args.options[APP_OPTIONS_ADD_SEGMENT_SIZE] = 32 << 20;
   attach_args.options[APP_OPTIONS_NAMESPACE_SECRET] = placeholder_secret;
   error = vnet_application_attach (&attach_args);
   SESSION_TEST ((error == 0), "server app attached: %U", format_clib_error,
@@ -1839,7 +1842,7 @@ session_test_mq_speed (vlib_main_t * vm, unformat_input_t * input)
       SESSION_TEST (prod_fd != -1, "mq producer eventd valid %u", prod_fd);
     }
 
-  sm = app_worker_get_or_alloc_connect_segment_manager (app_wrk);
+  sm = app_worker_get_connect_segment_manager (app_wrk);
   segment_manager_alloc_session_fifos (sm, 0, &rx_fifo, &tx_fifo);
   s.rx_fifo = rx_fifo;
   s.tx_fifo = tx_fifo;

diff --git a/src/vnet/session/application.c b/src/vnet/session/application.c
index 2f6bfa9..8abec77 100644 (file)
--- a/src/vnet/session/application.c
+++ b/src/vnet/session/application.c
@@ -1008,7 +1008,7 @@ application_alloc_worker_and_init (application_t * app, app_worker_t ** wrk)
   /*
    * Setup app worker
    */
-  app_wrk->first_segment_manager = segment_manager_index (sm);
+  app_wrk->connects_seg_manager = segment_manager_index (sm);
   app_wrk->listeners_table = hash_create (0, sizeof (u64));
   app_wrk->event_queue = segment_manager_event_queue (sm);
   app_wrk->app_is_builtin = application_is_builtin (app);
@@ -1041,7 +1041,7 @@ vnet_app_worker_add_del (vnet_app_worker_add_del_args_t * a)
       app_wrk->api_client_index = a->api_client_index;
       application_api_table_add (app->app_index, a->api_client_index);
 
-      sm = segment_manager_get (app_wrk->first_segment_manager);
+      sm = segment_manager_get (app_wrk->connects_seg_manager);
       fs = segment_manager_get_segment_w_lock (sm, 0);
       a->segment = &fs->ssvm;
       a->segment_handle = segment_manager_segment_handle (sm, fs);
@@ -1157,7 +1157,7 @@ vnet_application_attach (vnet_app_attach_args_t * a)
 
   a->app_evt_q = app_wrk->event_queue;
   app_wrk->api_client_index = a->api_client_index;
-  sm = segment_manager_get (app_wrk->first_segment_manager);
+  sm = segment_manager_get (app_wrk->connects_seg_manager);
   fs = segment_manager_get_segment_w_lock (sm, 0);
 
   if (application_is_proxy (app))
@@ -1165,7 +1165,7 @@ vnet_application_attach (vnet_app_attach_args_t * a)
       application_setup_proxy (app);
       /* The segment manager pool is reallocated because a new listener
        * is added. Re-grab segment manager to avoid dangling reference */
-      sm = segment_manager_get (app_wrk->first_segment_manager);
+      sm = segment_manager_get (app_wrk->connects_seg_manager);
     }
 
   ASSERT (vec_len (fs->ssvm.name) <= 128);

diff --git a/src/vnet/session/application.h b/src/vnet/session/application.h
index 0bfd4d1..a8ddfec 100644 (file)
--- a/src/vnet/session/application.h
+++ b/src/vnet/session/application.h
@@ -45,20 +45,17 @@ typedef struct app_worker_
   /** Application listens for events on this svm queue */
   svm_msg_q_t *event_queue;
 
-  /** Segment manager used for outgoing connects issued by the app */
+  /**
+   * Segment manager used for outgoing connects issued by the app. By
+   * convention this is the first segment manager allocated by the worker
+   * so it's also the one that holds the first segment with the app's
+   * message queue in it.
+   */
   u32 connects_seg_manager;
 
   /** Lookup tables for listeners. Value is segment manager index */
   uword *listeners_table;
 
-  /**
-   * First segment manager has in the the first segment the application's
-   * event fifo. Depending on what the app does, it may be either used for
-   * a listener or for connects.
-   */
-  u32 first_segment_manager;
-  u8 first_segment_manager_in_use;
-
   /** API index for the worker. Needed for multi-process apps */
   u32 api_client_index;
 
@@ -339,9 +336,6 @@ int app_worker_session_fifo_tuning (app_worker_t * app_wrk, session_t * s,
 segment_manager_t *app_worker_get_listen_segment_manager (app_worker_t *,
                                                          session_t *);
 segment_manager_t *app_worker_get_connect_segment_manager (app_worker_t *);
-segment_manager_t
-  * app_worker_get_or_alloc_connect_segment_manager (app_worker_t *);
-int app_worker_alloc_connects_segment_manager (app_worker_t * app);
 int app_worker_add_segment_notify (app_worker_t * app_wrk,
                                   u64 segment_handle);
 int app_worker_del_segment_notify (app_worker_t * app_wrk,

diff --git a/src/vnet/session/application_worker.c b/src/vnet/session/application_worker.c
index 8c3be22..7e03171 100644 (file)
--- a/src/vnet/session/application_worker.c
+++ b/src/vnet/session/application_worker.c
@@ -32,7 +32,6 @@ app_worker_alloc (application_t * app)
   app_wrk->app_index = app->app_index;
   app_wrk->wrk_map_index = ~0;
   app_wrk->connects_seg_manager = APP_INVALID_SEGMENT_MANAGER_INDEX;
-  app_wrk->first_segment_manager = APP_INVALID_SEGMENT_MANAGER_INDEX;
   clib_spinlock_init (&app_wrk->detached_seg_managers_lock);
   APP_DBG ("New app %v worker %u", app->name, app_wrk->wrk_index);
   return app_wrk;
@@ -143,17 +142,6 @@ app_worker_free (app_worker_t * app_wrk)
   vec_free (app_wrk->detached_seg_managers);
   clib_spinlock_free (&app_wrk->detached_seg_managers_lock);
 
-  /* If first segment manager is used by a listener that recently
-   * stopped listening, mark it as detached */
-  if (app_wrk->first_segment_manager != app_wrk->connects_seg_manager
-      && (sm = segment_manager_get_if_valid (app_wrk->first_segment_manager))
-      && !segment_manager_app_detached (sm))
-    {
-      sm->first_is_protected = 0;
-      sm->app_wrk_index = SEGMENT_MANAGER_INVALID_APP_INDEX;
-      segment_manager_init_free (sm);
-    }
-
   if (CLIB_DEBUG)
     clib_memset (app_wrk, 0xfe, sizeof (*app_wrk));
   pool_put (app_workers, app_wrk);
@@ -172,19 +160,9 @@ app_worker_get_app (u32 wrk_index)
 static segment_manager_t *
 app_worker_alloc_segment_manager (app_worker_t * app_wrk)
 {
-  segment_manager_t *sm = 0;
+  segment_manager_t *sm;
 
-  /* If the first segment manager is not in use, don't allocate a new one */
-  if (app_wrk->first_segment_manager != APP_INVALID_SEGMENT_MANAGER_INDEX
-      && app_wrk->first_segment_manager_in_use == 0)
-    {
-      sm = segment_manager_get (app_wrk->first_segment_manager);
-      app_wrk->first_segment_manager_in_use = 1;
-    }
-  else
-    {
-      sm = segment_manager_alloc ();
-    }
+  sm = segment_manager_alloc ();
   sm->app_wrk_index = app_wrk->wrk_index;
   segment_manager_init (sm);
   return sm;
@@ -309,20 +287,21 @@ app_worker_stop_listen_session (app_worker_t * app_wrk, session_t * ls)
 
   /* Try to cleanup segment manager */
   sm = segment_manager_get (*sm_indexp);
-  if (sm && segment_manager_has_fifos (sm))
-    {
-      /* Delete sessions in CREATED state */
-      vec_add1 (states, SESSION_STATE_CREATED);
-      segment_manager_del_sessions_filter (sm, states);
-      vec_free (states);
-    }
-  if (sm && app_wrk->first_segment_manager != *sm_indexp)
+  if (sm)
     {
       segment_manager_app_detach (sm);
       if (!segment_manager_has_fifos (sm))
-       segment_manager_free (sm);
+       {
+         /* Empty segment manager, cleanup it up */
+         segment_manager_free (sm);
+       }
       else
        {
+         /* Delete sessions in CREATED state */
+         vec_add1 (states, SESSION_STATE_CREATED);
+         segment_manager_del_sessions_filter (sm, states);
+         vec_free (states);
+
          /* Track segment manager in case app detaches and all the
           * outstanding sessions need to be closed */
          app_worker_add_detached_sm (app_wrk, *sm_indexp);
@@ -534,7 +513,7 @@ app_worker_own_session (app_worker_t * app_wrk, session_t * s)
   s->rx_fifo = 0;
   s->tx_fifo = 0;
 
-  sm = app_worker_get_or_alloc_connect_segment_manager (app_wrk);
+  sm = app_worker_get_connect_segment_manager (app_wrk);
   if (app_worker_alloc_session_fifos (sm, s))
     return -1;
 
@@ -555,10 +534,6 @@ app_worker_connect_session (app_worker_t * app_wrk, session_endpoint_t * sep,
 {
   int rv;
 
-  /* Make sure we have a segment manager for connects */
-  if (app_worker_alloc_connects_segment_manager (app_wrk))
-    return SESSION_E_ALLOC;
-
   if ((rv = session_open (app_wrk->wrk_index, sep, api_context)))
     return rv;
 
@@ -574,21 +549,6 @@ app_worker_session_fifo_tuning (app_worker_t * app_wrk, session_t * s,
   return app->cb_fns.fifo_tuning_callback (s, f, act, len);
 }
 
-int
-app_worker_alloc_connects_segment_manager (app_worker_t * app_wrk)
-{
-  segment_manager_t *sm;
-
-  if (app_wrk->connects_seg_manager == APP_INVALID_SEGMENT_MANAGER_INDEX)
-    {
-      sm = app_worker_alloc_segment_manager (app_wrk);
-      if (sm == 0)
-       return -1;
-      app_wrk->connects_seg_manager = segment_manager_index (sm);
-    }
-  return 0;
-}
-
 segment_manager_t *
 app_worker_get_connect_segment_manager (app_worker_t * app)
 {
@@ -596,14 +556,6 @@ app_worker_get_connect_segment_manager (app_worker_t * app)
   return segment_manager_get (app->connects_seg_manager);
 }
 
-segment_manager_t *
-app_worker_get_or_alloc_connect_segment_manager (app_worker_t * app_wrk)
-{
-  if (app_wrk->connects_seg_manager == (u32) ~ 0)
-    app_worker_alloc_connects_segment_manager (app_wrk);
-  return segment_manager_get (app_wrk->connects_seg_manager);
-}
-
 segment_manager_t *
 app_worker_get_listen_segment_manager (app_worker_t * app,
                                       session_t * listener)

diff --git a/src/vnet/tls/tls.c b/src/vnet/tls/tls.c
index 57dcc7f..e19f8e6 100644 (file)
--- a/src/vnet/tls/tls.c
+++ b/src/vnet/tls/tls.c
@@ -711,7 +711,6 @@ tls_connect (transport_endpoint_cfg_t * tep)
     }
   tls_ctx_half_open_reader_unlock ();
 
-  app_worker_alloc_connects_segment_manager (app_wrk);
   ctx->tls_ctx_engine = engine_type;
 
   clib_memcpy_fast (&cargs->sep, sep, sizeof (session_endpoint_t));
@@ -1138,7 +1137,6 @@ dtls_connect (transport_endpoint_cfg_t *tep)
       vec_terminate_c_string (ctx->srv_hostname);
     }
 
-  app_worker_alloc_connects_segment_manager (app_wrk);
   ctx->tls_ctx_engine = engine_type;
 
   clib_memcpy_fast (&cargs->sep, sep, sizeof (session_endpoint_t));