[ovs-dev] [PATCHv2] lib: upgrade to DPDK 1.8

[Pravin Shelar]

I am seeing 4 tests failed with following assertion failed which is
introduced by the patch.
"assertion v <= OFPBUF_DATA_MAX failed in ofpbuf_set_size()"

I have couple of comments inline.

On Fri, Feb 6, 2015 at 8:43 AM, Mark Kavanagh <mark.b.kavanagh at intel.com> wrote:
> DPDK v1.8.0 makes significant changes to struct rte_mbuf, including
> removal of the 'pkt' and 'data' fields. The latter, formally a
> pointer, is now calculated via an offset from the start of the
> segment buffer. These fields are referenced by OVS when accessing
> the data section of an ofpbuf.
>
> The following changes are required to add support for DPDK 1.8:
> - update affected functions to use the correct rte_mbuf fields
> - remove init function from netdev-dpdk (no longer required as
>   rte_eal_pci_probe is now invoked from eal_init)
> - split large amounts of data across multiple ofpbufs; with the
>   removal of the mbuf's 'data' pointer, and replacement with a
>   'data_off' field, it is necessary to limit the size of data
>   contained in an ofpbuf to UINT16_MAX when mbufs are used
>   (data_off and data_len are both of type uint16_t).
>   Were data not split across multiple ofpbufs, values larger
>   than UINT16_MAX for 'data_len' and 'data_off' would result
>   in wrap-around, and consequently, data corruption. Changes
>   introduced in this patch prevent this from occurring.
>
> Signed-off-by: Mark Kavanagh <mark.b.kavanagh at intel.com>
> Signed-off-by: Mark Gray <mark.d.gray at intel.com>
> Signed-off-by: Rory Sexton <rory.sexton at intel.com>
> ---
>  lib/jsonrpc.c     |   27 +++++++++++++++++++--------
>  lib/netdev-dpdk.c |   31 +++++++++----------------------
>  lib/ofpbuf.c      |    4 +++-
>  lib/ofpbuf.h      |   35 +++++++++++++++++++++++++++++------
>  lib/packet-dpif.h |    4 ++--
>  5 files changed, 62 insertions(+), 39 deletions(-)

Can you send one combined patch with documentations fixes that you had
last time.

>
...

> diff --git a/lib/ofpbuf.h b/lib/ofpbuf.h
> index 4e7038d0..ef0c319 100644
> --- a/lib/ofpbuf.h
> +++ b/lib/ofpbuf.h
> @@ -19,6 +19,11 @@
>
>  #include <stddef.h>
>  #include <stdint.h>
> +
> +#ifdef DPDK_NETDEV
> +#include <rte_common.h>
> +#endif
> +
>  #include "list.h"
>  #include "packets.h"
>  #include "util.h"
> @@ -28,6 +33,12 @@
>  extern "C" {
>  #endif
>
> +#ifdef DPDK_NETDEV
> +    #define OFPBUF_DATA_MAX UINT16_MAX
> +#else
> +    #define OFPBUF_DATA_MAX UINT32_MAX
> +#endif
> +
>  enum OVS_PACKED_ENUM ofpbuf_source {
>      OFPBUF_MALLOC,              /* Obtained via malloc(). */
>      OFPBUF_STACK,               /* Un-movable stack space or static buffer. */
> @@ -386,12 +397,23 @@ BUILD_ASSERT_DECL(offsetof(struct ofpbuf, mbuf) == 0);
>
>  static inline void * ofpbuf_data(const struct ofpbuf *b)
>  {
> -    return b->mbuf.pkt.data;
> +    return rte_pktmbuf_mtod(&(b->mbuf), void *);
>  }
>
>  static inline void ofpbuf_set_data(struct ofpbuf *b, void *d)
>  {
> -    b->mbuf.pkt.data = d;
> +    uintptr_t data_delta;
> +
> +    /* NULL 'd' value is valid */
> +    if (unlikely(d == NULL)) {
> +        b->mbuf.data_off = 0;

ofpbuf_data() need to return NULL for this case.

> +    } else {
> +        ovs_assert(d >= b->mbuf.buf_addr);
> +        /* Work out the offset between the start of segment buffer and 'd' */
> +        data_delta = RTE_PTR_DIFF(d, b->mbuf.buf_addr);
> +        ovs_assert(data_delta <= OFPBUF_DATA_MAX);
> +        b->mbuf.data_off = data_delta;
> +    }
>  }
>