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

Lam, Tiago tiago.lam at intel.com
Tue Jul 24 14:30:19 UTC 2018


Please discard this patch as it becomes obsolete with v6 of the "Support
multi-segment mbufs" series.

On 20/07/2018 18:11, Tiago Lam wrote:
> 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);
>  
> 


More information about the dev mailing list