diff options
author | Damjan Marion <damarion@cisco.com> | 2017-11-03 12:24:37 +0100 |
---|---|---|
committer | Damjan Marion <dmarion.lists@gmail.com> | 2017-11-08 19:52:38 +0000 |
commit | 6d56fa4b0aa2e789f1bdc8bf8280d65d87f6a541 (patch) | |
tree | 1a4701e35cfeb538b814a3b6f2a21a262cf9fbd0 | |
parent | 8daa80a4adfd82a19017c2c12554a8a43dddccd7 (diff) |
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>
-rw-r--r-- | extras/libmemif/src/libmemif.h | 1 | ||||
-rw-r--r-- | extras/libmemif/src/main.c | 117 | ||||
-rw-r--r-- | extras/libmemif/src/memif_private.h | 2 | ||||
-rw-r--r-- | src/plugins/memif/cli.c | 3 | ||||
-rw-r--r-- | src/plugins/memif/device.c | 58 | ||||
-rw-r--r-- | src/plugins/memif/node.c | 47 |
6 files changed, 101 insertions, 127 deletions
diff --git a/extras/libmemif/src/libmemif.h b/extras/libmemif/src/libmemif.h index da48edd4dee..3fc06488c62 100644 --- 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 39c18f27477..2e2602e52c1 100644 --- 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 83962bcfaf7..c213ee635aa 100644 --- 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 deca27af2ef..3d38550c1ba 100644 --- 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 f7eb862eef5..012c75327f1 100644 --- 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 8190441f4b5..74b238931ac 100644 --- 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; |