Hi Pravin, The tests fail because the amount of data that an 'mbuf-type' ofpbuf can store is now restricted to UINT16_MAX.
In the failed tests, the amount of data added to the ofpbuf far exceeds this limit; resolving this shortcoming will require modifications to additional lib functions (in particular ofpbuf_resize__), but it's not clear at this time if this is a viable option. Thanks, Mark > -----Original Message----- > From: Kavanagh, Mark B > Sent: Monday, February 9, 2015 4:15 PM > To: Pravin Shelar > Cc: dev@openvswitch.org; Rory Sexton > Subject: RE: [ovs-dev] [PATCHv2] lib: upgrade to DPDK 1.8 > > Hi Pravin, > > I checked the patch before submission using 'make distcheck', which came back > clean - > having run 'make check' I can now see the failures that you mentioned. > > I'll address these, and your comments below in the next version. > > Thanks, > Mark > > > -----Original Message----- > > From: Pravin Shelar [mailto:pshe...@nicira.com] > > Sent: Friday, February 6, 2015 11:05 PM > > To: Kavanagh, Mark B > > Cc: dev@openvswitch.org; Rory Sexton > > Subject: Re: [ovs-dev] [PATCHv2] lib: upgrade to DPDK 1.8 > > > > 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.kavan...@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.kavan...@intel.com> > > > Signed-off-by: Mark Gray <mark.d.g...@intel.com> > > > Signed-off-by: Rory Sexton <rory.sex...@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; > > > + } > > > } > > > _______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
