From 97d39e3e054ee681335197205e94fbf9a97a40e4 Mon Sep 17 00:00:00 2001 From: Florin Coras Date: Fri, 4 Sep 2020 08:57:27 -0700 Subject: [PATCH] svm session: document unsupported fifo deq combinations Type: fix - Document that ooo dequeues with ooo lookups cannot be done in combination with in order dequeues. - Added assert to capture this scenario and de-initialized rbtrees for cut-through tx fifo Signed-off-by: Florin Coras Change-Id: Ic40d020b3f0391fcf022ea3c906b86121744144f --- src/plugins/unittest/svm_fifo_test.c | 26 +++++++++++++++++++++----- src/svm/svm_fifo.c | 4 ++++ src/svm/svm_fifo.h | 6 ++++-- src/vnet/session/application_local.c | 4 ++++ 4 files changed, 33 insertions(+), 7 deletions(-) diff --git a/src/plugins/unittest/svm_fifo_test.c b/src/plugins/unittest/svm_fifo_test.c index ccaffa1f236..f5d4818bfe0 100644 --- a/src/plugins/unittest/svm_fifo_test.c +++ b/src/plugins/unittest/svm_fifo_test.c @@ -1596,9 +1596,12 @@ sfifo_test_fifo_grow (vlib_main_t * vm, unformat_input_t * input) SFIFO_TEST (svm_fifo_is_sane (f), "fifo should be sane"); /* - * Dequeue just a part of data + * Dequeue just a part of data. Because we're tracking ooo data, we can't + * call dequeue. Therefore, first peek and then dequeue drop */ - rv = svm_fifo_dequeue (f, fifo_inc, data_buf); + rv = svm_fifo_peek (f, 0, fifo_inc, data_buf); + SFIFO_TEST (rv == fifo_inc, "should dequeue all data"); + rv = svm_fifo_dequeue_drop (f, fifo_inc); SFIFO_TEST (rv == fifo_inc, "should dequeue all data"); rv = svm_fifo_n_chunks (f); SFIFO_TEST (rv == 1, "should have %u chunks has %u", 1, rv); @@ -1624,10 +1627,11 @@ sfifo_test_fifo_grow (vlib_main_t * vm, unformat_input_t * input) SFIFO_TEST (c->length == 4096, "end chunk length should be %u", 4096); /* - * Dequeue all + * Dequeue all. Don't call dequeue see above */ - rv = svm_fifo_dequeue (f, fifo_size, data_buf + fifo_inc); + rv = svm_fifo_peek (f, 0, fifo_size, data_buf + fifo_inc); SFIFO_TEST (rv == fifo_size, "should dequeue all data"); + SFIFO_TEST (svm_fifo_is_sane (f), "fifo should be sane"); rv = compare_data (data_buf, test_data, 0, vec_len (test_data), (u32 *) & i); @@ -1635,6 +1639,10 @@ sfifo_test_fifo_grow (vlib_main_t * vm, unformat_input_t * input) vlib_cli_output (vm, "[%d] dequeued %u expected %u", i, data_buf[i], test_data[i]); SFIFO_TEST ((rv == 0), "dequeued compared to original returned %d", rv); + + rv = svm_fifo_dequeue_drop (f, fifo_size); + SFIFO_TEST (rv == fifo_size, "should dequeue all data"); + SFIFO_TEST (svm_fifo_is_sane (f), "fifo should be sane"); /* fifo does not end on chunk boundary because of the - 100 */ SFIFO_TEST (f->head_chunk != 0, "should have head chunk"); @@ -1711,7 +1719,10 @@ sfifo_test_fifo_grow (vlib_main_t * vm, unformat_input_t * input) /* * Dequeue all */ - rv = svm_fifo_dequeue (f, fifo_size, data_buf); + + /* Because we're tracking ooo data, we can't call dequeue. Therefore, + * first peek and then dequeue drop */ + rv = svm_fifo_peek (f, 0, fifo_size, data_buf); SFIFO_TEST (rv == fifo_size, "should dequeue all data"); rv = compare_data (data_buf, test_data, 0, vec_len (test_data), @@ -1721,6 +1732,11 @@ sfifo_test_fifo_grow (vlib_main_t * vm, unformat_input_t * input) test_data[i]); SFIFO_TEST (svm_fifo_is_sane (f), "fifo should be sane"); SFIFO_TEST ((rv == 0), "dequeued compared to original returned %d", rv); + + + rv = svm_fifo_dequeue_drop (f, fifo_size); + SFIFO_TEST ((rv == fifo_size), "all bytes should be dropped %u", rv); + SFIFO_TEST (svm_fifo_is_sane (f), "fifo should be sane"); SFIFO_TEST (f->ooo_deq == 0, "should have no ooo deq chunk"); rv = svm_fifo_n_chunks (f); SFIFO_TEST (rv == 1, "should have %u chunks has %u", 1, rv); diff --git a/src/svm/svm_fifo.c b/src/svm/svm_fifo.c index 8e3bb0a7c8c..fda9481e721 100644 --- a/src/svm/svm_fifo.c +++ b/src/svm/svm_fifo.c @@ -1024,6 +1024,10 @@ svm_fifo_dequeue (svm_fifo_t * f, u32 len, u8 * dst) svm_fifo_copy_from_chunk (f, f->head_chunk, head, dst, len, &f->head_chunk); head = head + len; + /* In order dequeues are not supported in combination with ooo peeking. + * Use svm_fifo_dequeue_drop instead. */ + ASSERT (rb_tree_n_nodes (&f->ooo_deq_lookup) <= 1); + if (f_pos_geq (head, f_chunk_end (f->start_chunk))) fsh_collect_chunks (f->fs_hdr, f->slice_index, f_unlink_chunks (f, head, 0)); diff --git a/src/svm/svm_fifo.h b/src/svm/svm_fifo.h index e08b3e9dfa5..f7503c8e900 100644 --- a/src/svm/svm_fifo.h +++ b/src/svm/svm_fifo.h @@ -295,7 +295,7 @@ void svm_fifo_enqueue_nocopy (svm_fifo_t * f, u32 len); * Overwrite fifo head with new data * * This should be typically used by dgram transport protocols that need - * to update the dgram header after dequeueing a chunk of data. It assumes + * to update the dgram header after dequeuing a chunk of data. It assumes * that the dgram header is at most spread over two chunks. * * @param f fifo @@ -307,7 +307,9 @@ void svm_fifo_overwrite_head (svm_fifo_t * f, u8 * src, u32 len); * Dequeue data from fifo * * Data is dequeued to consumer provided buffer and head is atomically - * updated. + * updated. This should not be used in combination with ooo lookups. If + * ooo peeking of data is needed in combination with dequeuing use @ref + * svm_fifo_dequeue_drop. * * @param f fifo * @param len length of data to dequeue diff --git a/src/vnet/session/application_local.c b/src/vnet/session/application_local.c index cea22e3ba6e..814350018bb 100644 --- a/src/vnet/session/application_local.c +++ b/src/vnet/session/application_local.c @@ -224,6 +224,10 @@ ct_init_local_session (app_worker_t * client_wrk, app_worker_t * server_wrk, ls->rx_fifo->segment_index = seg_index; ls->tx_fifo->segment_index = seg_index; + /* Disable ooo lookups on the cut-through fifos. TODO remove once init of + * chunk lookup rbtrees is delegated to transports */ + svm_fifo_free_chunk_lookup (ls->tx_fifo); + segment_handle = segment_manager_segment_handle (sm, seg); if ((rv = app_worker_add_segment_notify (server_wrk, segment_handle))) { -- 2.16.6