vlib: convert frame_index into real pointers

The fast path almost always has to deal with the real
pointers. Deriving the frame pointer from a frame_index requires a
load of the 32bit frame_index from memory, another 64bit load of the
heap base pointer and some calculations.

Lets store the full pointer instead and do a single 64bit load only.

This helps avoiding problems when the heap is grown and frames are
allocated below vm->heap_aligned_base.

Type: refactor
Change-Id: Ifa6e6e984aafe1e2755bff80f0a4dfcddee3623c
Signed-off-by: Andreas Schultz <andreas.schultz@travelping.com>
Signed-off-by: Dave Barach <dave@barachs.net>

3 files changed, 44 insertions, 82 deletions

diff --git a/src/vlib/main.c b/src/vlib/main.c
index 055ea93cec1..0786465e0e4 100644
--- a/src/vlib/main.c
+++ b/src/vlib/main.c
@@ -117,7 +117,7 @@ get_frame_size_info (vlib_node_main_t * nm,
 #endif
 }
 
-static u32
+static vlib_frame_t *
 vlib_frame_alloc_to_node (vlib_main_t * vm, u32 to_node_index,
 			  u32 frame_flags)
 {
@@ -125,7 +125,7 @@ vlib_frame_alloc_to_node (vlib_main_t * vm, u32 to_node_index,
   vlib_frame_size_t *fs;
   vlib_node_t *to_node;
   vlib_frame_t *f;
-  u32 fi, l, n, scalar_size, vector_size;
+  u32 l, n, scalar_size, vector_size;
 
   to_node = vlib_get_node (vm, to_node_index);
 
@@ -134,17 +134,15 @@ vlib_frame_alloc_to_node (vlib_main_t * vm, u32 to_node_index,
 
   fs = get_frame_size_info (nm, scalar_size, vector_size);
   n = vlib_frame_bytes (scalar_size, vector_size);
-  if ((l = vec_len (fs->free_frame_indices)) > 0)
+  if ((l = vec_len (fs->free_frames)) > 0)
     {
       /* Allocate from end of free list. */
-      fi = fs->free_frame_indices[l - 1];
-      f = vlib_get_frame_no_check (vm, fi);
-      _vec_len (fs->free_frame_indices) = l - 1;
+      f = fs->free_frames[l - 1];
+      _vec_len (fs->free_frames) = l - 1;
     }
   else
     {
       f = clib_mem_alloc_aligned_no_fail (n, VLIB_FRAME_ALIGN);
-      fi = vlib_frame_index_no_check (vm, f);
     }
 
   /* Poison frame when debugging. */
@@ -167,12 +165,12 @@ vlib_frame_alloc_to_node (vlib_main_t * vm, u32 to_node_index,
 
   fs->n_alloc_frames += 1;
 
-  return fi;
+  return f;
 }
 
 /* Allocate a frame for from FROM_NODE to TO_NODE via TO_NEXT_INDEX.
    Returns frame index. */
-static u32
+static vlib_frame_t *
 vlib_frame_alloc (vlib_main_t * vm, vlib_node_runtime_t * from_node_runtime,
 		  u32 to_next_index)
 {
@@ -188,10 +186,10 @@ vlib_frame_alloc (vlib_main_t * vm, vlib_node_runtime_t * from_node_runtime,
 vlib_frame_t *
 vlib_get_frame_to_node (vlib_main_t * vm, u32 to_node_index)
 {
-  u32 fi = vlib_frame_alloc_to_node (vm, to_node_index,
-				     /* frame_flags */
-				     VLIB_FRAME_FREE_AFTER_DISPATCH);
-  return vlib_get_frame (vm, fi);
+  vlib_frame_t *f = vlib_frame_alloc_to_node (vm, to_node_index,
+					      /* frame_flags */
+					      VLIB_FRAME_FREE_AFTER_DISPATCH);
+  return vlib_get_frame (vm, f);
 }
 
 void
@@ -208,7 +206,7 @@ vlib_put_frame_to_node (vlib_main_t * vm, u32 to_node_index, vlib_frame_t * f)
   vec_add2 (vm->node_main.pending_frames, p, 1);
 
   f->frame_flags |= VLIB_FRAME_PENDING;
-  p->frame_index = vlib_frame_index (vm, f);
+  p->frame = vlib_get_frame (vm, f);
   p->node_runtime_index = to_node->runtime_index;
   p->next_frame_index = VLIB_PENDING_FRAME_NO_NEXT_FRAME;
 }
@@ -220,28 +218,24 @@ vlib_frame_free (vlib_main_t * vm, vlib_node_runtime_t * r, vlib_frame_t * f)
   vlib_node_main_t *nm = &vm->node_main;
   vlib_node_t *node;
   vlib_frame_size_t *fs;
-  u32 frame_index;
 
   ASSERT (f->frame_flags & VLIB_FRAME_IS_ALLOCATED);
 
   node = vlib_get_node (vm, r->node_index);
   fs = get_frame_size_info (nm, node->scalar_size, node->vector_size);
 
-  frame_index = vlib_frame_index (vm, f);
-
   ASSERT (f->frame_flags & VLIB_FRAME_IS_ALLOCATED);
 
   /* No next frames may point to freed frame. */
   if (CLIB_DEBUG > 0)
     {
       vlib_next_frame_t *nf;
-      vec_foreach (nf, vm->node_main.next_frames)
-	ASSERT (nf->frame_index != frame_index);
+      vec_foreach (nf, vm->node_main.next_frames) ASSERT (nf->frame != f);
     }
 
   f->frame_flags &= ~(VLIB_FRAME_IS_ALLOCATED | VLIB_FRAME_NO_APPEND);
 
-  vec_add1 (fs->free_frame_indices, frame_index);
+  vec_add1 (fs->free_frames, f);
   ASSERT (fs->n_alloc_frames > 0);
   fs->n_alloc_frames -= 1;
 }
@@ -257,7 +251,7 @@ show_frame_stats (vlib_main_t * vm,
   vec_foreach (fs, nm->frame_sizes)
   {
     u32 n_alloc = fs->n_alloc_frames;
-    u32 n_free = vec_len (fs->free_frame_indices);
+    u32 n_free = vec_len (fs->free_frames);
 
     if (n_alloc + n_free > 0)
       vlib_cli_output (vm, "%=6d%=12d%=12d",
@@ -321,11 +315,11 @@ vlib_next_frame_change_ownership (vlib_main_t * vm,
       if (next_frame->flags & VLIB_FRAME_PENDING)
 	{
 	  vlib_pending_frame_t *p;
-	  if (next_frame->frame_index != ~0)
+	  if (next_frame->frame != NULL)
 	    {
 	      vec_foreach (p, nm->pending_frames)
 	      {
-		if (p->frame_index == next_frame->frame_index)
+		if (p->frame == next_frame->frame)
 		  {
 		    p->next_frame_index =
 		      next_frame - vm->node_main.next_frames;
@@ -377,11 +371,11 @@ vlib_get_next_frame_internal (vlib_main_t * vm,
   /* ??? Don't need valid flag: can use frame_index == ~0 */
   if (PREDICT_FALSE (!(nf->flags & VLIB_FRAME_IS_ALLOCATED)))
     {
-      nf->frame_index = vlib_frame_alloc (vm, node, next_index);
+      nf->frame = vlib_frame_alloc (vm, node, next_index);
       nf->flags |= VLIB_FRAME_IS_ALLOCATED;
     }
 
-  f = vlib_get_frame (vm, nf->frame_index);
+  f = nf->frame;
 
   /* Has frame been removed from pending vector (e.g. finished dispatching)?
      If so we can reuse frame. */
@@ -403,13 +397,12 @@ vlib_get_next_frame_internal (vlib_main_t * vm,
          two redundant frames from node -> next node. */
       if (!(nf->flags & VLIB_FRAME_NO_FREE_AFTER_DISPATCH))
 	{
-	  vlib_frame_t *f_old = vlib_get_frame (vm, nf->frame_index);
+	  vlib_frame_t *f_old = vlib_get_frame (vm, nf->frame);
 	  f_old->frame_flags |= VLIB_FRAME_FREE_AFTER_DISPATCH;
 	}
 
       /* Allocate new frame to replace full one. */
-      nf->frame_index = vlib_frame_alloc (vm, node, next_index);
-      f = vlib_get_frame (vm, nf->frame_index);
+      f = nf->frame = vlib_frame_alloc (vm, node, next_index);
       n_used = f->n_vectors;
     }
 
@@ -438,7 +431,7 @@ vlib_put_next_frame_validate (vlib_main_t * vm,
   u32 n_before, n_after;
 
   nf = vlib_node_runtime_get_next_frame (vm, rt, next_index);
-  f = vlib_get_frame (vm, nf->frame_index);
+  f = vlib_get_frame (vm, nf->frame);
 
   ASSERT (n_vectors_left <= VLIB_FRAME_SIZE);
   n_after = VLIB_FRAME_SIZE - n_vectors_left;
@@ -475,7 +468,7 @@ vlib_put_next_frame (vlib_main_t * vm,
     vlib_put_next_frame_validate (vm, r, next_index, n_vectors_left);
 
   nf = vlib_node_runtime_get_next_frame (vm, r, next_index);
-  f = vlib_get_frame (vm, nf->frame_index);
+  f = vlib_get_frame (vm, nf->frame);
 
   /* Make sure that magic number is still there.  Otherwise, caller
      has overrun frame meta data. */
@@ -511,7 +504,7 @@ vlib_put_next_frame (vlib_main_t * vm,
 
 	  vec_add2 (nm->pending_frames, p, 1);
 
-	  p->frame_index = nf->frame_index;
+	  p->frame = nf->frame;
 	  p->node_runtime_index = nf->node_runtime_index;
 	  p->next_frame_index = nf - nm->next_frames;
 	  nf->flags |= VLIB_FRAME_PENDING;
@@ -525,7 +518,7 @@ vlib_put_next_frame (vlib_main_t * vm,
 	   */
 	  if (0 && r->thread_index != next_runtime->thread_index)
 	    {
-	      nf->frame_index = ~0;
+	      nf->frame = NULL;
 	      nf->flags &= ~(VLIB_FRAME_PENDING | VLIB_FRAME_IS_ALLOCATED);
 	    }
 	}
@@ -1335,7 +1328,7 @@ dispatch_pending_node (vlib_main_t * vm, uword pending_frame_index,
   vlib_frame_t *f;
   vlib_next_frame_t *nf, nf_dummy;
   vlib_node_runtime_t *n;
-  u32 restore_frame_index;
+  vlib_frame_t *restore_frame;
   vlib_pending_frame_t *p;
 
   /* See comment below about dangling references to nm->pending_frames */
@@ -1344,13 +1337,13 @@ dispatch_pending_node (vlib_main_t * vm, uword pending_frame_index,
   n = vec_elt_at_index (nm->nodes_by_type[VLIB_NODE_TYPE_INTERNAL],
 			p->node_runtime_index);
 
-  f = vlib_get_frame (vm, p->frame_index);
+  f = vlib_get_frame (vm, p->frame);
   if (p->next_frame_index == VLIB_PENDING_FRAME_NO_NEXT_FRAME)
     {
       /* No next frame: so use dummy on stack. */
       nf = &nf_dummy;
       nf->flags = f->frame_flags & VLIB_NODE_FLAG_TRACE;
-      nf->frame_index = ~p->frame_index;
+      nf->frame = NULL;
     }
   else
     nf = vec_elt_at_index (nm->next_frames, p->next_frame_index);
@@ -1359,13 +1352,13 @@ dispatch_pending_node (vlib_main_t * vm, uword pending_frame_index,
 
   /* Force allocation of new frame while current frame is being
      dispatched. */
-  restore_frame_index = ~0;
-  if (nf->frame_index == p->frame_index)
+  restore_frame = NULL;
+  if (nf->frame == p->frame)
     {
-      nf->frame_index = ~0;
+      nf->frame = NULL;
       nf->flags &= ~VLIB_FRAME_IS_ALLOCATED;
       if (!(n->flags & VLIB_NODE_FLAG_FRAME_NO_FREE_AFTER_DISPATCH))
-	restore_frame_index = p->frame_index;
+	restore_frame = p->frame;
     }
 
   /* Frame must be pending. */
@@ -1387,7 +1380,7 @@ dispatch_pending_node (vlib_main_t * vm, uword pending_frame_index,
   f->frame_flags &= ~(VLIB_FRAME_PENDING | VLIB_FRAME_NO_APPEND);
 
   /* Frame is ready to be used again, so restore it. */
-  if (restore_frame_index != ~0)
+  if (restore_frame != NULL)
     {
       /*
        * We musn't restore a frame that is flagged to be freed. This
@@ -1415,10 +1408,10 @@ dispatch_pending_node (vlib_main_t * vm, uword pending_frame_index,
       nf = vec_elt_at_index (nm->next_frames, p->next_frame_index);
       nf->flags |= VLIB_FRAME_IS_ALLOCATED;
 
-      if (~0 == nf->frame_index)
+      if (NULL == nf->frame)
 	{
 	  /* no new frame has been assigned to this node, use the saved one */
-	  nf->frame_index = restore_frame_index;
+	  nf->frame = restore_frame;
 	  f->n_vectors = 0;
 	}
       else
@@ -1552,7 +1545,7 @@ dispatch_process (vlib_main_t * vm,
       n_vectors = 0;
       pool_get (nm->suspended_process_frames, pf);
       pf->node_runtime_index = node->runtime_index;
-      pf->frame_index = f ? vlib_frame_index (vm, f) : ~0;
+      pf->frame = f;
       pf->next_frame_index = ~0;
 
       p->n_suspends += 1;
@@ -1620,7 +1613,7 @@ dispatch_suspended_process (vlib_main_t * vm,
 
   node_runtime = &p->node_runtime;
   node = vlib_get_node (vm, node_runtime->node_index);
-  f = pf->frame_index != ~0 ? vlib_get_frame (vm, pf->frame_index) : 0;
+  f = pf->frame;
 
   vlib_elog_main_loop_event (vm, node_runtime->node_index, t,
 			     f ? f->n_vectors : 0, /* is_after */ 0);
diff --git a/src/vlib/node.h b/src/vlib/node.h
index 4264a1ba1a5..bec0ed2d2c0 100644
--- a/src/vlib/node.h
+++ b/src/vlib/node.h
@@ -400,8 +400,8 @@ typedef struct vlib_frame_t
 
 typedef struct
 {
-  /* Frame index. */
-  u32 frame_index;
+  /* Frame pointer. */
+  vlib_frame_t *frame;
 
   /* Node runtime for this next. */
   u32 node_runtime_index;
@@ -440,7 +440,6 @@ always_inline void
 vlib_next_frame_init (vlib_next_frame_t * nf)
 {
   clib_memset (nf, 0, sizeof (nf[0]));
-  nf->frame_index = ~0;
   nf->node_runtime_index = ~0;
 }
 
@@ -451,7 +450,7 @@ typedef struct
   u32 node_runtime_index;
 
   /* Frame index (in the heap). */
-  u32 frame_index;
+  vlib_frame_t *frame;
 
   /* Start of next frames for this node. */
   u32 next_frame_index;
@@ -537,8 +536,8 @@ typedef struct
   /* Number of allocated frames for this scalar/vector size. */
   u32 n_alloc_frames;
 
-  /* Vector of free frame indices for this scalar/vector size. */
-  u32 *free_frame_indices;
+  /* Vector of free frames for this scalar/vector size. */
+  vlib_frame_t **free_frames;
 } vlib_frame_size_t;
 
 typedef struct
diff --git a/src/vlib/node_funcs.h b/src/vlib/node_funcs.h
index c9ff93d6124..d6d04fb16c2 100644
--- a/src/vlib/node_funcs.h
+++ b/src/vlib/node_funcs.h
@@ -212,32 +212,10 @@ vlib_get_process_from_node (vlib_main_t * vm, vlib_node_t * node)
   return vec_elt (nm->processes, node->runtime_index);
 }
 
-/* Fetches frame with given handle. */
 always_inline vlib_frame_t *
-vlib_get_frame_no_check (vlib_main_t * vm, uword frame_index)
+vlib_get_frame (vlib_main_t * vm, vlib_frame_t * f)
 {
-  vlib_frame_t *f;
-  f = vm->heap_aligned_base + (frame_index * VLIB_FRAME_ALIGN);
-  return f;
-}
-
-always_inline u32
-vlib_frame_index_no_check (vlib_main_t * vm, vlib_frame_t * f)
-{
-  uword i;
-
-  ASSERT (((uword) f & (VLIB_FRAME_ALIGN - 1)) == 0);
-
-  i = ((u8 *) f - (u8 *) vm->heap_aligned_base);
-  ASSERT ((i / VLIB_FRAME_ALIGN) <= 0xFFFFFFFFULL);
-
-  return i / VLIB_FRAME_ALIGN;
-}
-
-always_inline vlib_frame_t *
-vlib_get_frame (vlib_main_t * vm, uword frame_index)
-{
-  vlib_frame_t *f = vlib_get_frame_no_check (vm, frame_index);
+  ASSERT (f != NULL);
   ASSERT (f->frame_flags & VLIB_FRAME_IS_ALLOCATED);
   return f;
 }
@@ -248,14 +226,6 @@ vlib_frame_no_append (vlib_frame_t * f)
   f->frame_flags |= VLIB_FRAME_NO_APPEND;
 }
 
-always_inline u32
-vlib_frame_index (vlib_main_t * vm, vlib_frame_t * f)
-{
-  uword i = vlib_frame_index_no_check (vm, f);
-  ASSERT (vlib_get_frame (vm, i) == f);
-  return i;
-}
-
 /* Byte alignment for vector arguments. */
 #define VLIB_FRAME_VECTOR_ALIGN (1 << 4)