session svm: fix fifo migration

Allocate and attach a new pair of private fifos in the right private
slice when a session is cloned. This ensures that private fifos are not
shared between workers.

Type: fix

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

6 files changed, 66 insertions, 51 deletions

diff --git a/src/svm/fifo_segment.c b/src/svm/fifo_segment.c
index fdcf84ea8d7..d30932448c6 100644
--- a/src/svm/fifo_segment.c
+++ b/src/svm/fifo_segment.c
@@ -901,8 +901,6 @@ fifo_segment_free_fifo (fifo_segment_t * fs, svm_fifo_t * f)
 
   /* Add to free list */
   fss_fifo_free_list_push (fsh, fss, sf);
-  //  sf->next = fss->free_fifos;
-  //  fss->free_fifos = fs_sptr (fsh, sf);
 
   fss->virtual_mem -= svm_fifo_size (f);
 
@@ -935,53 +933,67 @@ fifo_segment_free_fifo (fifo_segment_t * fs, svm_fifo_t * f)
 }
 
 void
-fifo_segment_detach_fifo (fifo_segment_t * fs, svm_fifo_t * f)
+fifo_segment_detach_fifo (fifo_segment_t *fs, svm_fifo_t **f)
 {
   fifo_slice_private_t *pfss;
   fifo_segment_slice_t *fss;
-  svm_fifo_chunk_t *c;
-  u32 fl_index;
+  u32 fl_index, slice_index;
+  svm_fifo_chunk_t **c;
+  svm_fifo_t *of = *f;
 
-  ASSERT (f->refcnt == 1);
-
-  fss = fsh_slice_get (fs->h, f->shr->slice_index);
-  pfss = fs_slice_private_get (fs, f->shr->slice_index);
-  fss->virtual_mem -= svm_fifo_size (f);
-  if (f->flags & SVM_FIFO_F_LL_TRACKED)
-    pfss_fifo_del_active_list (pfss, f);
+  slice_index = of->master_thread_index;
+  fss = fsh_slice_get (fs->h, slice_index);
+  pfss = fs_slice_private_get (fs, slice_index);
+  fss->virtual_mem -= svm_fifo_size (of);
+  if (of->flags & SVM_FIFO_F_LL_TRACKED)
+    pfss_fifo_del_active_list (pfss, of);
 
-  c = fs_chunk_ptr (fs->h, f->shr->start_chunk);
-  while (c)
+  /* Update slice counts for chunks that were detached */
+  vec_foreach (c, of->chunks_at_attach)
     {
-      fl_index = fs_freelist_for_size (c->length);
+      fl_index = fs_freelist_for_size ((*c)->length);
       clib_atomic_fetch_sub_rel (&fss->num_chunks[fl_index], 1);
-      c = fs_chunk_ptr (fs->h, c->next);
     }
+  vec_free (of->chunks_at_attach);
+
+  clib_mem_bulk_free (pfss->fifos, *f);
+  *f = 0;
 }
 
 void
-fifo_segment_attach_fifo (fifo_segment_t * fs, svm_fifo_t * f,
-			  u32 slice_index)
+fifo_segment_attach_fifo (fifo_segment_t *fs, svm_fifo_t **f, u32 slice_index)
 {
   fifo_slice_private_t *pfss;
   fifo_segment_slice_t *fss;
   svm_fifo_chunk_t *c;
+  svm_fifo_t *nf, *of;
   u32 fl_index;
 
-  f->shr->slice_index = slice_index;
-  fss = fsh_slice_get (fs->h, f->shr->slice_index);
-  pfss = fs_slice_private_get (fs, f->shr->slice_index);
-  fss->virtual_mem += svm_fifo_size (f);
-  if (f->flags & SVM_FIFO_F_LL_TRACKED)
-    pfss_fifo_add_active_list (pfss, f);
+  nf = fs_fifo_alloc (fs, slice_index);
+  clib_memcpy_fast (nf, *f, sizeof (*nf));
 
-  c = fs_chunk_ptr (fs->h, f->shr->start_chunk);
+  fss = fsh_slice_get (fs->h, slice_index);
+  pfss = fs_slice_private_get (fs, slice_index);
+  fss->virtual_mem += svm_fifo_size (nf);
+  if (nf->flags & SVM_FIFO_F_LL_TRACKED)
+    pfss_fifo_add_active_list (pfss, nf);
+
+  /* Update allocated chunks for fifo segment and build list
+   * of chunks to be freed at detach */
+  of = *f;
+  of->chunks_at_attach = 0;
+
+  c = fs_chunk_ptr (fs->h, nf->shr->start_chunk);
   while (c)
     {
       fl_index = fs_freelist_for_size (c->length);
       clib_atomic_fetch_add_rel (&fss->num_chunks[fl_index], 1);
+      vec_add1 (of->chunks_at_attach, c);
       c = fs_chunk_ptr (fs->h, c->next);
     }
+
+  nf->shr->slice_index = slice_index;
+  *f = nf;
 }
 
 uword
diff --git a/src/svm/fifo_segment.h b/src/svm/fifo_segment.h
index 8266690d4ed..268b48b39f6 100644
--- a/src/svm/fifo_segment.h
+++ b/src/svm/fifo_segment.h
@@ -139,8 +139,8 @@ svm_fifo_t *fifo_segment_alloc_fifo_w_offset (fifo_segment_t *fs,
  */
 void fifo_segment_free_fifo (fifo_segment_t * fs, svm_fifo_t * f);
 
-void fifo_segment_detach_fifo (fifo_segment_t * fs, svm_fifo_t * f);
-void fifo_segment_attach_fifo (fifo_segment_t * fs, svm_fifo_t * f,
+void fifo_segment_detach_fifo (fifo_segment_t *fs, svm_fifo_t **f);
+void fifo_segment_attach_fifo (fifo_segment_t *fs, svm_fifo_t **f,
 			       u32 slice_index);
 uword fifo_segment_fifo_offset (svm_fifo_t *f);
 
diff --git a/src/svm/fifo_types.h b/src/svm/fifo_types.h
index 592a7007c7d..03adbd33c2c 100644
--- a/src/svm/fifo_types.h
+++ b/src/svm/fifo_types.h
@@ -109,6 +109,8 @@ typedef struct _svm_fifo
   struct _svm_fifo *next; /**< prev in active chain */
   struct _svm_fifo *prev; /**< prev in active chain */
 
+  svm_fifo_chunk_t **chunks_at_attach; /**< chunks to be accounted at detach */
+
 #if SVM_FIFO_TRACE
   svm_fifo_trace_elem_t *trace;
 #endif
diff --git a/src/vnet/session/segment_manager.c b/src/vnet/session/segment_manager.c
index 2d20d9cc1e4..61108326abc 100644
--- a/src/vnet/session/segment_manager.c
+++ b/src/vnet/session/segment_manager.c
@@ -800,27 +800,27 @@ segment_manager_dealloc_fifos (svm_fifo_t * rx_fifo, svm_fifo_t * tx_fifo)
 }
 
 void
-segment_manager_detach_fifo (segment_manager_t * sm, svm_fifo_t * f)
+segment_manager_detach_fifo (segment_manager_t *sm, svm_fifo_t **f)
 {
   fifo_segment_t *fs;
 
-  fs = segment_manager_get_segment_w_lock (sm, f->segment_index);
+  fs = segment_manager_get_segment_w_lock (sm, (*f)->segment_index);
   fifo_segment_detach_fifo (fs, f);
   segment_manager_segment_reader_unlock (sm);
 }
 
 void
-segment_manager_attach_fifo (segment_manager_t * sm, svm_fifo_t * f,
-			     session_t * s)
+segment_manager_attach_fifo (segment_manager_t *sm, svm_fifo_t **f,
+			     session_t *s)
 {
   fifo_segment_t *fs;
 
-  fs = segment_manager_get_segment_w_lock (sm, f->segment_index);
+  fs = segment_manager_get_segment_w_lock (sm, (*f)->segment_index);
   fifo_segment_attach_fifo (fs, f, s->thread_index);
   segment_manager_segment_reader_unlock (sm);
 
-  f->shr->master_session_index = s->session_index;
-  f->master_thread_index = s->thread_index;
+  (*f)->shr->master_session_index = s->session_index;
+  (*f)->master_thread_index = s->thread_index;
 }
 
 u32
diff --git a/src/vnet/session/segment_manager.h b/src/vnet/session/segment_manager.h
index 454fef87967..7c3434b70d3 100644
--- a/src/vnet/session/segment_manager.h
+++ b/src/vnet/session/segment_manager.h
@@ -131,9 +131,9 @@ int segment_manager_try_alloc_fifos (fifo_segment_t * fs,
 				     svm_fifo_t ** tx_fifo);
 void segment_manager_dealloc_fifos (svm_fifo_t * rx_fifo,
 				    svm_fifo_t * tx_fifo);
-void segment_manager_detach_fifo (segment_manager_t * sm, svm_fifo_t * f);
-void segment_manager_attach_fifo (segment_manager_t * sm, svm_fifo_t * f,
-				  session_t * s);
+void segment_manager_detach_fifo (segment_manager_t *sm, svm_fifo_t **f);
+void segment_manager_attach_fifo (segment_manager_t *sm, svm_fifo_t **f,
+				  session_t *s);
 
 void segment_manager_set_watermarks (segment_manager_t * sm,
 				     u8 high_watermark, u8 low_watermark);
diff --git a/src/vnet/session/session.c b/src/vnet/session/session.c
index 90f2fdeba0c..d1f21dafdd6 100644
--- a/src/vnet/session/session.c
+++ b/src/vnet/session/session.c
@@ -854,23 +854,12 @@ static void
 session_switch_pool_reply (void *arg)
 {
   u32 session_index = pointer_to_uword (arg);
-  segment_manager_t *sm;
-  app_worker_t *app_wrk;
   session_t *s;
 
   s = session_get_if_valid (session_index, vlib_get_thread_index ());
   if (!s)
     return;
 
-  app_wrk = app_worker_get_if_valid (s->app_wrk_index);
-  if (!app_wrk)
-    return;
-
-  /* Attach fifos to the right session and segment slice */
-  sm = app_worker_get_connect_segment_manager (app_wrk);
-  segment_manager_attach_fifo (sm, s->rx_fifo, s);
-  segment_manager_attach_fifo (sm, s->tx_fifo, s);
-
   /* Notify app that it has data on the new session */
   session_enqueue_notify (s);
 }
@@ -910,8 +899,8 @@ session_switch_pool (void *cb_args)
     {
       /* Cleanup fifo segment slice state for fifos */
       sm = app_worker_get_connect_segment_manager (app_wrk);
-      segment_manager_detach_fifo (sm, s->rx_fifo);
-      segment_manager_detach_fifo (sm, s->tx_fifo);
+      segment_manager_detach_fifo (sm, &s->rx_fifo);
+      segment_manager_detach_fifo (sm, &s->tx_fifo);
 
       /* Notify app, using old session, about the migration event */
       app_worker_migrate_notify (app_wrk, s, new_sh);
@@ -935,6 +924,8 @@ session_dgram_connect_notify (transport_connection_t * tc,
 {
   session_t *new_s;
   session_switch_pool_args_t *rpc_args;
+  segment_manager_t *sm;
+  app_worker_t *app_wrk;
 
   /*
    * Clone half-open session to the right thread.
@@ -944,7 +935,17 @@ session_dgram_connect_notify (transport_connection_t * tc,
   new_s->session_state = SESSION_STATE_READY;
   new_s->flags |= SESSION_F_IS_MIGRATING;
 
-  session_lookup_add_connection (tc, session_handle (new_s));
+  if (!(tc->flags & TRANSPORT_CONNECTION_F_NO_LOOKUP))
+    session_lookup_add_connection (tc, session_handle (new_s));
+
+  app_wrk = app_worker_get_if_valid (new_s->app_wrk_index);
+  if (app_wrk)
+    {
+      /* New set of fifos attached to the same shared memory */
+      sm = app_worker_get_connect_segment_manager (app_wrk);
+      segment_manager_attach_fifo (sm, &new_s->rx_fifo, new_s);
+      segment_manager_attach_fifo (sm, &new_s->tx_fifo, new_s);
+    }
 
   /*
    * Ask thread owning the old session to clean it up and make us the tx