[ovs-dev] [PATCH] Revert "dp-packet: Handle multi-seg mbufs in resize__()."

Tiago Lam tiago.lam at intel.com
Fri Jul 20 17:11:56 UTC 2018


This reverts commit bc4b614. The commit tries to alleviate the call to
OVS_NOT_REACHED() in dp_packet_resize__(), for DPDK packets, by trying
to reuse the available tailroom space when no more headroom space is
available, and vice-versa. A simpler approach is to mitigate the call to
dp_packet_resize__() first, when DPDK packets are in use. Later, if
needed, this approach can be revisited.

Additionally, it also fixes the tests that were relying on the removed
functionality.

CC: Darrell Ball <dlu998 at gmail.com>
Signed-off-by: Tiago Lam <tiago.lam at intel.com>
---
Note that the above commit, bc4b614, is in dpdk_merge still, and not yet merged to master.
---
 lib/dp-packet.c         | 48 ++----------------------------------------------
 tests/test-dpdk-mbufs.c | 27 +++++++++++----------------
 2 files changed, 13 insertions(+), 62 deletions(-)

diff --git a/lib/dp-packet.c b/lib/dp-packet.c
index ae060e2..6773535 100644
--- a/lib/dp-packet.c
+++ b/lib/dp-packet.c
@@ -285,51 +285,9 @@ dp_packet_resize__(struct dp_packet *b, size_t new_headroom, size_t new_tailroom
     new_allocated = new_headroom + dp_packet_size(b) + new_tailroom;
 
     switch (b->source) {
-    /* When resizing mbufs, both a single mbuf and multi-segment mbufs (where
-     * data is not contigously held in memory), both the headroom and the
-     * tailroom available will be used to make more space for where data needs
-     * to be inserted. I.e if there's not enough headroom, data may be shifted
-     * right if there's enough tailroom.
-     * However, this is not bulletproof and in some cases the space available
-     * won't be enough - in those cases, an error should be returned and the
-     * packet dropped. */
     case DPBUF_DPDK:
-    {
-        size_t miss_len;
-
-        if (new_headroom == dp_packet_headroom(b)) {
-            /* This is a tailroom adjustment. Since there's no tailroom space
-             * left, try and shift data towards the head to free up tail space,
-             * if there's enough headroom */
-
-            miss_len = new_tailroom - dp_packet_tailroom(b);
-
-            if (miss_len <= new_headroom) {
-                dp_packet_shift(b, -miss_len);
-            } else {
-                /* XXX: Handle error case and report error to caller */
-                OVS_NOT_REACHED();
-            }
-        } else {
-            /* Otherwise, this is a headroom adjustment. Try to shift data
-             * towards the tail to free up head space, if there's enough
-             * tailroom */
-
-            miss_len = new_headroom - dp_packet_headroom(b);
-
-
-            if (miss_len <= new_tailroom) {
-                dp_packet_shift(b, miss_len);
-            } else {
-                /* XXX: Handle error case and report error to caller */
-                OVS_NOT_REACHED();
-            }
-        }
-
-        new_base = dp_packet_base(b);
+        OVS_NOT_REACHED();
 
-        break;
-    }
     case DPBUF_MALLOC:
         if (new_headroom == dp_packet_headroom(b)) {
             new_base = xrealloc(dp_packet_base(b), new_allocated);
@@ -353,9 +311,7 @@ dp_packet_resize__(struct dp_packet *b, size_t new_headroom, size_t new_tailroom
         OVS_NOT_REACHED();
     }
 
-    if (b->source != DPBUF_DPDK) {
-        dp_packet_set_allocated(b, new_allocated);
-    }
+    dp_packet_set_allocated(b, new_allocated);
     dp_packet_set_base(b, new_base);
 
     new_data = (char *) new_base + new_headroom;
diff --git a/tests/test-dpdk-mbufs.c b/tests/test-dpdk-mbufs.c
index 1c77038..8168cae 100644
--- a/tests/test-dpdk-mbufs.c
+++ b/tests/test-dpdk-mbufs.c
@@ -215,15 +215,6 @@ dpdk_pkt_put(struct dp_packet *pkt, void *p, size_t size) {
 
     dp_packet_mbuf_write(fmbuf, 0, size, p);
 
-    /* Adjust size of intermediate mbufs from current tail to end */
-    /*size_t pkt_len = size;
-    while (fmbuf && pkt_len > 0) {
-        fmbuf->data_len = MIN(pkt_len, fmbuf->buf_len - fmbuf->data_off);
-        pkt_len -= fmbuf->data_len;
-
-        fmbuf = fmbuf->next;
-    }*/
-
     dp_packet_set_size(pkt, size);
 
     return pkt;
@@ -256,17 +247,21 @@ test_dpdk_packet_insert_tailroom_and_headroom(void) {
     struct dp_packet *pkt = dpdk_mp_alloc_pkt(mp);
     ovs_assert(pkt != NULL);
 
+    /* Reserve 256B of header */
+    size_t head_len = 256;
+    dp_packet_reserve(pkt, head_len);
+
     /* Put the first 512B of "test_str" in the packet's header */
     size_t str_len = 512;
     char *p = dp_packet_put(pkt, test_str, str_len);
     ovs_assert(p != NULL);
-    /* Allocate extra 256B of header */
-    size_t head_len = 256;
+
+    /* Fill the reserved 256B of header */
     p = dp_packet_push_uninit(pkt, head_len);
     ovs_assert(p != NULL);
 
     /* Check properties and data are as expected */
-    assert_single_mbuf(pkt, 0, str_len + head_len);
+    assert_single_mbuf(pkt, RTE_PKTMBUF_HEADROOM, str_len + head_len);
 
     /* Check the data inserted in the packet is correct */
     char data[str_len + head_len];
@@ -296,8 +291,8 @@ test_dpdk_packet_insert_tailroom_and_headroom_multiple_mbufs(void) {
     char *p = dp_packet_put(pkt, test_str, tail_len);
     ovs_assert(p != NULL);
 
-    /* Allocate extra 256B of header */
-    size_t head_len = 256;
+    /* Fill the entire headroom */
+    size_t head_len = RTE_PKTMBUF_HEADROOM;
     p = dp_packet_push_uninit(pkt, head_len);
     ovs_assert(p != NULL);
     /* Copy the data to the reserved headroom */
@@ -358,8 +353,8 @@ test_dpdk_packet_insert_headroom_multiple_mbufs(void) {
     size_t str_len = MBUF_DATA_LEN + 2;
     pkt = dpdk_pkt_put(pkt, test_str, str_len);
 
-    /* Push the first 2000B of "test_str" in the packet's header */
-    size_t head_len = 2000;
+    /* Fill the entire headroom */
+    size_t head_len = RTE_PKTMBUF_HEADROOM;
     char *p = dp_packet_push_uninit(pkt, head_len);
     ovs_assert(p != NULL);
 
-- 
2.7.4



More information about the dev mailing list