memif: do not mask head and tail pointers

Change-Id: Ie849ab713ff086187c18a91ab32e58207fe94033
Signed-off-by: Damjan Marion <damarion@cisco.com>
Signed-off-by: Jakub Grajciar <Jakub.Grajciar@pantheon.tech>

diff --git a/extras/libmemif/src/libmemif.h b/extras/libmemif/src/libmemif.h
index da48edd..3fc0648 100644 (file)
--- a/extras/libmemif/src/libmemif.h
+++ b/extras/libmemif/src/libmemif.h
@@ -73,6 +73,7 @@ typedef enum
   MEMIF_ERR_DISCONNECTED,      /*!< peer interface disconnected */
   MEMIF_ERR_UNKNOWN_MSG,       /*!< unknown message type */
   MEMIF_ERR_POLL_CANCEL,       /*!< memif_poll_event() was cancelled */
+  MEMIF_ERR_MAX_RING,          /*!< too large ring size */
 } memif_err_t;
 
 /**

diff --git a/extras/libmemif/src/main.c b/extras/libmemif/src/main.c
index 39c18f2..2e2602e 100644 (file)
--- a/extras/libmemif/src/main.c
+++ b/extras/libmemif/src/main.c
@@ -54,7 +54,7 @@
 /* private structs and functions */
 #include <memif_private.h>
 
-#define ERRLIST_LEN 37
+#define ERRLIST_LEN 38
 #define MAX_ERRBUF_LEN 256
 
 #if __x86_x64__
@@ -142,7 +142,9 @@ const char *memif_errlist[ERRLIST_LEN] = {  /* MEMIF_ERR_SUCCESS */
   /* MEMIF_ERR_UNKNOWN_MSG */
   "Unknown message type received on control channel. (internal error)",
   /* MEMIF_ERR_POLL_CANCEL */
-  "Memif event polling was canceled."
+  "Memif event polling was canceled.",
+  /* MEMIF_ERR_MAX_RING */
+  "Maximum log2 ring size is 15"
 };
 
 #define MEMIF_ERR_UNDEFINED "undefined error"
@@ -201,7 +203,7 @@ memif_syscall_error_handler (int err_code)
     return MEMIF_ERR_PROC_FILE_LIMIT;
   if (err_code == ENOMEM)
     return MEMIF_ERR_NOMEM;
-/* connection refused if master dows not exist
+/* connection refused if master does not exist
     this error would spam the user until master was created */
   if (err_code == ECONNREFUSED)
     return MEMIF_ERR_SUCCESS;
@@ -560,6 +562,11 @@ memif_create (memif_conn_handle_t * c, memif_conn_args_t * args,
 
   if (args->log2_ring_size == 0)
     args->log2_ring_size = MEMIF_DEFAULT_LOG2_RING_SIZE;
+  else if (args->log2_ring_size > MEMIF_MAX_LOG2_RING_SIZE)
+    {
+      err = MEMIF_ERR_MAX_RING;
+      goto error;
+    }
   if (args->buffer_size == 0)
     args->buffer_size = MEMIF_DEFAULT_BUFFER_SIZE;
   if (args->num_s2m_rings == 0)
@@ -1390,18 +1397,15 @@ memif_buffer_alloc (memif_conn_handle_t conn, uint16_t qid,
   memif_buffer_t *b0, *b1;
   uint8_t chain_buf = 1;
   uint16_t mask = (1 << mq->log2_ring_size) - 1;
+  uint16_t head = ring->head;
+  uint16_t tail = ring->tail;
   uint16_t s0, s1, ns;
   *count_out = 0;
   int i, err = MEMIF_ERR_SUCCESS;      /* 0 */
 
-  if (ring->tail > ring->head)
-    ns = ring->tail - ring->head;
-  else
-    ns = (1 << mq->log2_ring_size) - ring->head + ring->tail;
-
-  /* (head == tail) ? receive function will asume that no packets are available */
-  ns -= 1;
+  ns = (1 << mq->log2_ring_size) - head + tail;
 
+  /* calculate number of chain buffers */
   if (size > ring->desc[0].buffer_length)
     {
       chain_buf = size / ring->desc[s0].buffer_length;
@@ -1422,8 +1426,8 @@ memif_buffer_alloc (memif_conn_handle_t conn, uint16_t qid,
          b0 = (bufs + *count_out);
          b1 = (bufs + *count_out + 1);
 
-         b0->desc_index = s0;
-         b1->desc_index = s1;
+         b0->desc_index = head + mq->alloc_bufs;
+         b1->desc_index = head + mq->alloc_bufs + chain_buf;
          ring->desc[s0].flags = 0;
          ring->desc[s1].flags = 0;
          b0->buffer_len = ring->desc[s0].buffer_length * chain_buf;
@@ -1453,7 +1457,7 @@ memif_buffer_alloc (memif_conn_handle_t conn, uint16_t qid,
       if (chain_buf > ns)
        break;
 
-      b0->desc_index = s0;
+      b0->desc_index = head + mq->alloc_bufs;
       ring->desc[s0].flags = 0;
       b0->buffer_len = ring->desc[s0].buffer_length * chain_buf;
       b0->data = c->regions->shm + ring->desc[s0].offset;
@@ -1527,7 +1531,7 @@ memif_buffer_free (memif_conn_handle_t conn, uint16_t qid,
          if ((b1->buffer_len % ring->desc[b1->desc_index].buffer_length) !=
              0)
            chain_buf1++;
-         tail = (b1->desc_index + chain_buf1) & mask;
+         tail = b1->desc_index + chain_buf1;
          b0->data = NULL;
          b1->data = NULL;
 
@@ -1539,7 +1543,7 @@ memif_buffer_free (memif_conn_handle_t conn, uint16_t qid,
       chain_buf0 = b0->buffer_len / ring->desc[b0->desc_index].buffer_length;
       if ((b0->buffer_len % ring->desc[b0->desc_index].buffer_length) != 0)
        chain_buf0++;
-      tail = (b0->desc_index + chain_buf0) & mask;
+      tail = b0->desc_index + chain_buf0;
       b0->data = NULL;
 
       count--;
@@ -1808,10 +1812,7 @@ memif_rx_burst (memif_conn_handle_t conn, uint16_t qid,
   if (head == mq->last_head)
     return 0;
 
-  if (head > mq->last_head)
-    ns = head - mq->last_head;
-  else
-    ns = (1 << mq->log2_ring_size) - mq->last_head + head;
+  ns = head - mq->last_head;
 
   while (ns && count)
     {
@@ -1821,62 +1822,62 @@ memif_rx_burst (memif_conn_handle_t conn, uint16_t qid,
          b1 = (bufs + curr_buf + 1);
 
          b0->desc_index = mq->last_head;
-         b0->data = memif_get_buffer (conn, ring, mq->last_head);
-         b0->data_len = ring->desc[mq->last_head].length;
-         b0->buffer_len = ring->desc[mq->last_head].buffer_length;
+         b0->data = memif_get_buffer (conn, ring, mq->last_head & mask);
+         b0->data_len = ring->desc[mq->last_head & mask].length;
+         b0->buffer_len = ring->desc[mq->last_head & mask].buffer_length;
 #ifdef MEMIF_DBG_SHM
          i = 0;
          print_bytes (b0->data +
-                      ring->desc[b0->desc_index].buffer_length * i++,
-                      ring->desc[b0->desc_index].buffer_length, DBG_TX_BUF);
+                      ring->desc[b0->desc_index & mask].buffer_length * i++,
+                      ring->desc[b0->desc_index & mask].buffer_length, DBG_TX_BUF);
 #endif /* MEMIF_DBG_SHM */
          ns--;
          *rx += 1;
-         while (ring->desc[mq->last_head].flags & MEMIF_DESC_FLAG_NEXT)
+         while (ring->desc[mq->last_head & mask].flags & MEMIF_DESC_FLAG_NEXT)
            {
-             ring->desc[mq->last_head].flags &= ~MEMIF_DESC_FLAG_NEXT;
-             mq->last_head = (mq->last_head + 1) & mask;
-             b0->data_len += ring->desc[mq->last_head].length;
-             b0->buffer_len += ring->desc[mq->last_head].buffer_length;
+             ring->desc[mq->last_head & mask].flags &= ~MEMIF_DESC_FLAG_NEXT;
+             mq->last_head++;
+             b0->data_len += ring->desc[mq->last_head & mask].length;
+             b0->buffer_len += ring->desc[mq->last_head & mask].buffer_length;
 #ifdef MEMIF_DBG_SHM
              print_bytes (b0->data +
-                          ring->desc[b0->desc_index].buffer_length * i++,
-                          ring->desc[b0->desc_index].buffer_length,
+                          ring->desc[b0->desc_index & mask].buffer_length * i++,
+                          ring->desc[b0->desc_index & mask].buffer_length,
                           DBG_TX_BUF);
 #endif /* MEMIF_DBG_SHM */
              ns--;
              *rx += 1;
            }
-         mq->last_head = (mq->last_head + 1) & mask;
+         mq->last_head++;
 
          b1->desc_index = mq->last_head;
-         b1->data = memif_get_buffer (conn, ring, mq->last_head);
-         b1->data_len = ring->desc[mq->last_head].length;
-         b1->buffer_len = ring->desc[mq->last_head].buffer_length;
+         b1->data = memif_get_buffer (conn, ring, mq->last_head & mask);
+         b1->data_len = ring->desc[mq->last_head & mask].length;
+         b1->buffer_len = ring->desc[mq->last_head & mask].buffer_length;
 #ifdef MEMIF_DBG_SHM
          i = 0;
          print_bytes (b1->data +
-                      ring->desc[b1->desc_index].buffer_length * i++,
-                      ring->desc[b1->desc_index].buffer_length, DBG_TX_BUF);
+                      ring->desc[b1->desc_index & mask].buffer_length * i++,
+                      ring->desc[b1->desc_index & mask].buffer_length, DBG_TX_BUF);
 #endif /* MEMIF_DBG_SHM */
          ns--;
          *rx += 1;
-         while (ring->desc[mq->last_head].flags & MEMIF_DESC_FLAG_NEXT)
+         while (ring->desc[mq->last_head & mask].flags & MEMIF_DESC_FLAG_NEXT)
            {
-             ring->desc[mq->last_head].flags &= ~MEMIF_DESC_FLAG_NEXT;
-             mq->last_head = (mq->last_head + 1) & mask;
-             b1->data_len += ring->desc[mq->last_head].length;
-             b1->buffer_len += ring->desc[mq->last_head].buffer_length;
+             ring->desc[mq->last_head & mask].flags &= ~MEMIF_DESC_FLAG_NEXT;
+             mq->last_head++;
+             b1->data_len += ring->desc[mq->last_head & mask].length;
+             b1->buffer_len += ring->desc[mq->last_head & mask].buffer_length;
 #ifdef MEMIF_DBG_SHM
              print_bytes (b1->data +
-                          ring->desc[b1->desc_index].buffer_length * i++,
-                          ring->desc[b1->desc_index].buffer_length,
+                          ring->desc[b1->desc_index & mask].buffer_length * i++,
+                          ring->desc[b1->desc_index & mask].buffer_length,
                           DBG_TX_BUF);
 #endif /* MEMIF_DBG_SHM */
              ns--;
              *rx += 1;
            }
-         mq->last_head = (mq->last_head + 1) & mask;
+         mq->last_head++;
 
          count -= 2;
          curr_buf += 2;
@@ -1884,33 +1885,33 @@ memif_rx_burst (memif_conn_handle_t conn, uint16_t qid,
       b0 = (bufs + curr_buf);
 
       b0->desc_index = mq->last_head;
-      b0->data = memif_get_buffer (conn, ring, mq->last_head);
-      b0->data_len = ring->desc[mq->last_head].length;
-      b0->buffer_len = ring->desc[mq->last_head].buffer_length;
+      b0->data = memif_get_buffer (conn, ring, mq->last_head & mask);
+      b0->data_len = ring->desc[mq->last_head & mask].length;
+      b0->buffer_len = ring->desc[mq->last_head & mask].buffer_length;
 #ifdef MEMIF_DBG_SHM
          i = 0;
       print_bytes (b0->data +
-                  ring->desc[b0->desc_index].buffer_length * i++,
-                  ring->desc[b0->desc_index].buffer_length, DBG_TX_BUF);
+                  ring->desc[b0->desc_index & mask].buffer_length * i++,
+                  ring->desc[b0->desc_index & mask].buffer_length, DBG_TX_BUF);
 #endif /* MEMIF_DBG_SHM */
       ns--;
       *rx += 1;
 
-      while (ring->desc[mq->last_head].flags & MEMIF_DESC_FLAG_NEXT)
+      while (ring->desc[mq->last_head & mask].flags & MEMIF_DESC_FLAG_NEXT)
        {
-         ring->desc[mq->last_head].flags &= ~MEMIF_DESC_FLAG_NEXT;
-         mq->last_head = (mq->last_head + 1) & mask;
-         b0->data_len += ring->desc[mq->last_head].length;
-         b0->buffer_len += ring->desc[mq->last_head].buffer_length;
+         ring->desc[mq->last_head & mask].flags &= ~MEMIF_DESC_FLAG_NEXT;
+         mq->last_head++;
+         b0->data_len += ring->desc[mq->last_head & mask].length;
+         b0->buffer_len += ring->desc[mq->last_head & mask].buffer_length;
 #ifdef MEMIF_DBG_SHM
          print_bytes (b0->data +
-                      ring->desc[b0->desc_index].buffer_length * i++,
-                      ring->desc[b0->desc_index].buffer_length, DBG_TX_BUF);
+                      ring->desc[b0->desc_index & mask].buffer_length * i++,
+                      ring->desc[b0->desc_index & mask].buffer_length, DBG_TX_BUF);
 #endif /* MEMIF_DBG_SHM */
          ns--;
          *rx += 1;
        }
-      mq->last_head = (mq->last_head + 1) & mask;
+      mq->last_head++;
 
       count--;
       curr_buf++;

diff --git a/extras/libmemif/src/memif_private.h b/extras/libmemif/src/memif_private.h
index 83962bc..c213ee6 100644 (file)
--- a/extras/libmemif/src/memif_private.h
+++ b/extras/libmemif/src/memif_private.h
@@ -40,7 +40,7 @@
 #define MEMIF_MAX_M2S_RING             255
 #define MEMIF_MAX_S2M_RING             255
 #define MEMIF_MAX_REGION               255
-#define MEMIF_MAX_LOG2_RING_SIZE       14
+#define MEMIF_MAX_LOG2_RING_SIZE       15
 
 #define MEMIF_MAX_FDS 512
 

diff --git a/src/plugins/memif/cli.c b/src/plugins/memif/cli.c
index deca27a..3d38550 100644 (file)
--- a/src/plugins/memif/cli.c
+++ b/src/plugins/memif/cli.c
@@ -76,6 +76,9 @@ memif_create_command_fn (vlib_main_t * vm, unformat_input_t * input,
   if (!is_pow2 (ring_size))
     return clib_error_return (0, "ring size must be power of 2");
 
+  if (ring_size > 32768)
+    return clib_error_return (0, "maximum ring size is 32768");
+
   args.log2_ring_size = min_log2 (ring_size);
 
   if (rx_queues > 255 || rx_queues < 1)

diff --git a/src/plugins/memif/device.c b/src/plugins/memif/device.c
index f7eb862..012c753 100644 (file)
--- a/src/plugins/memif/device.c
+++ b/src/plugins/memif/device.c
@@ -109,26 +109,28 @@ memif_copy_buffer_to_tx_ring (vlib_main_t * vm, vlib_node_runtime_t * node,
   vlib_buffer_t *b0;
   void *mb0;
   u32 total = 0, len;
+  u16 slot = (*head) & mask;
 
-  mb0 = memif_get_buffer (mif, ring, *head);
-  ring->desc[*head].flags = 0;
+  mb0 = memif_get_buffer (mif, ring, slot);
+  ring->desc[slot].flags = 0;
   do
     {
       b0 = vlib_get_buffer (vm, bi);
       len = b0->current_length;
-      if (PREDICT_FALSE (ring->desc[*head].buffer_length < (total + len)))
+      if (PREDICT_FALSE (ring->desc[slot].buffer_length < (total + len)))
        {
          if (PREDICT_TRUE (total))
            {
-             ring->desc[*head].length = total;
+             ring->desc[slot].length = total;
              total = 0;
-             ring->desc[*head].flags |= MEMIF_DESC_FLAG_NEXT;
-             *head = (*head + 1) & mask;
-             mb0 = memif_get_buffer (mif, ring, *head);
-             ring->desc[*head].flags = 0;
+             ring->desc[slot].flags |= MEMIF_DESC_FLAG_NEXT;
+             (*head)++;
+             slot = (*head) & mask;
+             mb0 = memif_get_buffer (mif, ring, slot);
+             ring->desc[slot].flags = 0;
            }
        }
-      if (PREDICT_TRUE (ring->desc[*head].buffer_length >= (total + len)))
+      if (PREDICT_TRUE (ring->desc[slot].buffer_length >= (total + len)))
        {
          clib_memcpy (mb0 + total, vlib_buffer_get_current (b0),
                       CLIB_CACHE_LINE_BYTES);
@@ -149,8 +151,8 @@ memif_copy_buffer_to_tx_ring (vlib_main_t * vm, vlib_node_runtime_t * node,
 
   if (PREDICT_TRUE (total))
     {
-      ring->desc[*head].length = total;
-      *head = (*head + 1) & mask;
+      ring->desc[slot].length = total;
+      (*head)++;
     }
 }
 
@@ -196,34 +198,18 @@ memif_interface_tx_inline (vlib_main_t * vm, vlib_node_runtime_t * node,
   head = ring->head;
   tail = ring->tail;
 
-  if (tail > head)
-    free_slots = tail - head;
-  else
-    free_slots = ring_size - head + tail;
+  free_slots = ring_size - head + tail;
 
   while (n_left > 5 && free_slots > 1)
     {
-      if (PREDICT_TRUE (head + 5 < ring_size))
-       {
-         CLIB_PREFETCH (memif_get_buffer (mif, ring, head + 2),
-                        CLIB_CACHE_LINE_BYTES, STORE);
-         CLIB_PREFETCH (memif_get_buffer (mif, ring, head + 3),
-                        CLIB_CACHE_LINE_BYTES, STORE);
-         CLIB_PREFETCH (&ring->desc[head + 4], CLIB_CACHE_LINE_BYTES, STORE);
-         CLIB_PREFETCH (&ring->desc[head + 5], CLIB_CACHE_LINE_BYTES, STORE);
-       }
-      else
-       {
-         CLIB_PREFETCH (memif_get_buffer (mif, ring, (head + 2) % mask),
-                        CLIB_CACHE_LINE_BYTES, STORE);
-         CLIB_PREFETCH (memif_get_buffer (mif, ring, (head + 3) % mask),
-                        CLIB_CACHE_LINE_BYTES, STORE);
-         CLIB_PREFETCH (&ring->desc[(head + 4) % mask],
-                        CLIB_CACHE_LINE_BYTES, STORE);
-         CLIB_PREFETCH (&ring->desc[(head + 5) % mask],
-                        CLIB_CACHE_LINE_BYTES, STORE);
-       }
-
+      CLIB_PREFETCH (memif_get_buffer (mif, ring, (head + 2) & mask),
+                    CLIB_CACHE_LINE_BYTES, STORE);
+      CLIB_PREFETCH (memif_get_buffer (mif, ring, (head + 3) & mask),
+                    CLIB_CACHE_LINE_BYTES, STORE);
+      CLIB_PREFETCH (&ring->desc[(head + 4) & mask], CLIB_CACHE_LINE_BYTES,
+                    STORE);
+      CLIB_PREFETCH (&ring->desc[(head + 5) & mask], CLIB_CACHE_LINE_BYTES,
+                    STORE);
       memif_prefetch_buffer_and_data (vm, buffers[2]);
       memif_prefetch_buffer_and_data (vm, buffers[3]);
 

diff --git a/src/plugins/memif/node.c b/src/plugins/memif/node.c
index 8190441..74b2389 100644 (file)
--- a/src/plugins/memif/node.c
+++ b/src/plugins/memif/node.c
@@ -132,7 +132,7 @@ memif_copy_buffer_from_rx_ring (vlib_main_t * vm, memif_if_t * mif,
 
   while (*num_slots)
     {
-      data_len = ring->desc[mq->last_head].length;
+      data_len = ring->desc[mq->last_head & mask].length;
       while (data_len && (*n_free_bufs))
        {
          /* get empty buffer */
@@ -161,7 +161,7 @@ memif_copy_buffer_from_rx_ring (vlib_main_t * vm, memif_if_t * mif,
          bytes_to_copy =
            data_len > n_buffer_bytes ? n_buffer_bytes : data_len;
          b->current_data = 0;
-         mb = memif_get_buffer (mif, ring, mq->last_head);
+         mb = memif_get_buffer (mif, ring, mq->last_head & mask);
          clib_memcpy (vlib_buffer_get_current (b), mb + offset,
                       CLIB_CACHE_LINE_BYTES);
          if (bytes_to_copy > CLIB_CACHE_LINE_BYTES)
@@ -191,10 +191,10 @@ memif_copy_buffer_from_rx_ring (vlib_main_t * vm, memif_if_t * mif,
        }
       last_head = mq->last_head;
       /* Advance to next descriptor */
-      mq->last_head = (mq->last_head + 1) & mask;
+      mq->last_head++;
       offset = 0;
       (*num_slots)--;
-      if ((ring->desc[last_head].flags & MEMIF_DESC_FLAG_NEXT) == 0)
+      if ((ring->desc[last_head & mask].flags & MEMIF_DESC_FLAG_NEXT) == 0)
        break;
     }
 
@@ -269,10 +269,7 @@ memif_device_input_inline (vlib_main_t * vm, vlib_node_runtime_t * node,
   if (head == mq->last_head)
     return 0;
 
-  if (head > mq->last_head)
-    num_slots = head - mq->last_head;
-  else
-    num_slots = ring_size - mq->last_head + head;
+  num_slots = head - mq->last_head;
 
   while (num_slots)
     {
@@ -283,30 +280,16 @@ memif_device_input_inline (vlib_main_t * vm, vlib_node_runtime_t * node,
 
       while (num_slots > 11 && n_left_to_next > 2)
        {
-         if (PREDICT_TRUE (mq->last_head + 5 < ring_size))
-           {
-             CLIB_PREFETCH (memif_get_buffer (mif, ring, mq->last_head + 2),
-                            CLIB_CACHE_LINE_BYTES, LOAD);
-             CLIB_PREFETCH (memif_get_buffer (mif, ring, mq->last_head + 3),
-                            CLIB_CACHE_LINE_BYTES, LOAD);
-             CLIB_PREFETCH (&ring->desc[mq->last_head + 4],
-                            CLIB_CACHE_LINE_BYTES, LOAD);
-             CLIB_PREFETCH (&ring->desc[mq->last_head + 5],
-                            CLIB_CACHE_LINE_BYTES, LOAD);
-           }
-         else
-           {
-             CLIB_PREFETCH (memif_get_buffer
-                            (mif, ring, (mq->last_head + 2) % mask),
-                            CLIB_CACHE_LINE_BYTES, LOAD);
-             CLIB_PREFETCH (memif_get_buffer
-                            (mif, ring, (mq->last_head + 3) % mask),
-                            CLIB_CACHE_LINE_BYTES, LOAD);
-             CLIB_PREFETCH (&ring->desc[(mq->last_head + 4) % mask],
-                            CLIB_CACHE_LINE_BYTES, LOAD);
-             CLIB_PREFETCH (&ring->desc[(mq->last_head + 5) % mask],
-                            CLIB_CACHE_LINE_BYTES, LOAD);
-           }
+         CLIB_PREFETCH (memif_get_buffer
+                        (mif, ring, (mq->last_head + 2) & mask),
+                        CLIB_CACHE_LINE_BYTES, LOAD);
+         CLIB_PREFETCH (memif_get_buffer
+                        (mif, ring, (mq->last_head + 3) & mask),
+                        CLIB_CACHE_LINE_BYTES, LOAD);
+         CLIB_PREFETCH (&ring->desc[(mq->last_head + 4) & mask],
+                        CLIB_CACHE_LINE_BYTES, LOAD);
+         CLIB_PREFETCH (&ring->desc[(mq->last_head + 5) & mask],
+                        CLIB_CACHE_LINE_BYTES, LOAD);
 
          vlib_buffer_t *first_b0 = 0;
          u32 bi0 = 0, first_bi0 = 0;