tls: postpone ho cleanup if not fully established

If ho cleans up on first worker before owner of established session
receives connected notification, the ho session is prematurely cleaned
up.

Wait for established ctx to be allocated before freeing ho.

Type: fix

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

2 files changed, 80 insertions, 8 deletions

diff --git a/src/vnet/tls/tls.c b/src/vnet/tls/tls.c
index 32077c2a85b..1364a4fbbe7 100644
--- a/src/vnet/tls/tls.c
+++ b/src/vnet/tls/tls.c
@@ -117,6 +117,9 @@ tls_ctx_half_open_alloc (void)
   tls_main_t *tm = &tls_main;
   tls_ctx_t *ctx;
 
+  if (vec_len (tm->postponed_ho_free))
+    tls_flush_postponed_ho_cleanups ();
+
   pool_get_aligned_safe (tm->half_open_ctx_pool, ctx, CLIB_CACHE_LINE_BYTES);
 
   clib_memset (ctx, 0, sizeof (*ctx));
@@ -140,6 +143,49 @@ tls_ctx_half_open_get (u32 ctx_index)
 }
 
 void
+tls_add_postponed_ho_cleanups (u32 ho_index)
+{
+  tls_main_t *tm = &tls_main;
+  vec_add1 (tm->postponed_ho_free, ho_index);
+}
+
+static void
+tls_ctx_ho_try_free (u32 ho_index)
+{
+  tls_ctx_t *ctx;
+
+  ctx = tls_ctx_half_open_get (ho_index);
+  /* Probably tcp connected just before tcp establish timeout and
+   * worker that owns established session has not yet received
+   * @ref tls_session_connected_cb */
+  if (!(ctx->flags & TLS_CONN_F_HO_DONE))
+    {
+      ctx->tls_session_handle = SESSION_INVALID_HANDLE;
+      tls_add_postponed_ho_cleanups (ho_index);
+      return;
+    }
+  if (!ctx->no_app_session)
+    session_half_open_delete_notify (&ctx->connection);
+  tls_ctx_half_open_free (ho_index);
+}
+
+void
+tls_flush_postponed_ho_cleanups ()
+{
+  tls_main_t *tm = &tls_main;
+  u32 *ho_indexp, *tmp;
+
+  tmp = tm->postponed_ho_free;
+  tm->postponed_ho_free = tm->ho_free_list;
+  tm->ho_free_list = tmp;
+
+  vec_foreach (ho_indexp, tm->ho_free_list)
+    tls_ctx_ho_try_free (*ho_indexp);
+
+  vec_reset_length (tm->ho_free_list);
+}
+
+void
 tls_notify_app_enqueue (tls_ctx_t * ctx, session_t * app_session)
 {
   app_worker_t *app_wrk;
@@ -421,15 +467,8 @@ tls_session_reset_callback (session_t * s)
 static void
 tls_session_cleanup_ho (session_t *s)
 {
-  tls_ctx_t *ctx;
-  u32 ho_index;
-
   /* session opaque stores the opaque passed on connect */
-  ho_index = s->opaque;
-  ctx = tls_ctx_half_open_get (ho_index);
-  if (!ctx->no_app_session)
-    session_half_open_delete_notify (&ctx->connection);
-  tls_ctx_half_open_free (ho_index);
+  tls_ctx_ho_try_free (s->opaque);
 }
 
 int
@@ -551,6 +590,7 @@ tls_session_connected_cb (u32 tls_app_index, u32 ho_ctx_index,
   u32 ctx_handle;
 
   ho_ctx = tls_ctx_half_open_get (ho_ctx_index);
+  ho_ctx->flags |= TLS_CONN_F_HO_DONE;
 
   ctx_handle = tls_ctx_alloc (ho_ctx->tls_ctx_engine);
   ctx = tls_ctx_get (ctx_handle);
@@ -616,6 +656,7 @@ tls_session_connected_callback (u32 tls_app_index, u32 ho_ctx_index,
       u32 api_context;
 
       ho_ctx = tls_ctx_half_open_get (ho_ctx_index);
+      ho_ctx->flags |= TLS_CONN_F_HO_DONE;
       app_wrk = app_worker_get_if_valid (ho_ctx->parent_app_wrk_index);
       if (app_wrk)
 	{
@@ -950,6 +991,14 @@ tls_cleanup_ho (u32 ho_index)
   session_t *s;
 
   ctx = tls_ctx_half_open_get (ho_index);
+  /* Already pending cleanup */
+  if (ctx->tls_session_handle == SESSION_INVALID_HANDLE)
+    {
+      ASSERT (ctx->flags & TLS_CONN_F_HO_DONE);
+      ctx->no_app_session = 1;
+      return;
+    }
+
   s = session_get_from_handle (ctx->tls_session_handle);
   /* If no pending cleanup notification, force cleanup now. Otherwise,
    * wait for cleanup notification and set no app session on ctx */
diff --git a/src/vnet/tls/tls.h b/src/vnet/tls/tls.h
index 2938cdb4a17..0ae8123aed0 100644
--- a/src/vnet/tls/tls.h
+++ b/src/vnet/tls/tls.h
@@ -56,6 +56,22 @@ typedef struct tls_cxt_id_
 STATIC_ASSERT (sizeof (tls_ctx_id_t) <= TRANSPORT_CONN_ID_LEN,
 	       "ctx id must be less than TRANSPORT_CONN_ID_LEN");
 
+#define foreach_tls_conn_flags _ (HO_DONE, "ho done")
+
+typedef enum tls_conn_flags_bit_
+{
+#define _(sym, str) TLS_CONN_F_BIT_##sym,
+  foreach_tls_conn_flags
+#undef _
+} tls_conn_flags_bit_t;
+
+typedef enum tls_conn_flags_
+{
+#define _(sym, str) TLS_CONN_F_##sym = 1 << TLS_CONN_F_BIT_##sym,
+  foreach_tls_conn_flags
+#undef _
+} __clib_packed tls_conn_flags_t;
+
 typedef struct tls_ctx_
 {
   union
@@ -81,6 +97,7 @@ typedef struct tls_ctx_
   u8 app_closed;
   u8 no_app_session;
   u8 is_migrated;
+  tls_conn_flags_t flags;
   u8 *srv_hostname;
   u32 evt_index;
   u32 ckpair_index;
@@ -92,6 +109,8 @@ typedef struct tls_main_
   u32 app_index;
   tls_ctx_t *listener_ctx_pool;
   tls_ctx_t *half_open_ctx_pool;
+  u32 *postponed_ho_free;
+  u32 *ho_free_list;
   u8 **rx_bufs;
   u8 **tx_bufs;
 
@@ -140,6 +159,10 @@ void tls_notify_app_enqueue (tls_ctx_t * ctx, session_t * app_session);
 void tls_notify_app_io_error (tls_ctx_t *ctx);
 void tls_disconnect_transport (tls_ctx_t * ctx);
 int tls_reinit_ca_chain (crypto_engine_type_t tls_engine_id);
+
+void tls_add_postponed_ho_cleanups (u32 ho_index);
+void tls_flush_postponed_ho_cleanups ();
+
 #endif /* SRC_VNET_TLS_TLS_H_ */
 
 /*