svm session: document unsupported fifo deq combinations 87/28687/3
authorFlorin Coras <fcoras@cisco.com>
Fri, 4 Sep 2020 15:57:27 +0000 (08:57 -0700)
committerDave Barach <openvpp@barachs.net>
Tue, 8 Sep 2020 16:14:08 +0000 (16:14 +0000)
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 <fcoras@cisco.com>
Change-Id: Ic40d020b3f0391fcf022ea3c906b86121744144f

src/plugins/unittest/svm_fifo_test.c
src/svm/svm_fifo.c
src/svm/svm_fifo.h
src/vnet/session/application_local.c

index ccaffa1..f5d4818 100644 (file)
@@ -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);
index 8e3bb0a..fda9481 100644 (file)
@@ -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));
index e08b3e9..f7503c8 100644 (file)
@@ -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
index cea22e3..8143500 100644 (file)
@@ -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)))
     {