| Message ID | 1405623854-29976-1-git-send-email-venkatesh.vivekanandan@linaro.org |
|---|---|
| State | Superseded |
| Headers | show |
On 2014-07-18 00:34, venkatesh.vivekanandan@linaro.org wrote: > From: Venkatesh Vivekanandan <venkatesh.vivekanandan@linaro.org> > > - Added new file platform/linux-dpdk/include/odp_debug.h > - ODP_ASSERT complains of array getting a negative value. Root cause is, > struct rte_mbuf { > . > . > } __rte_cache_aligned; <-- this alignment causes the error. > > Since dpdk code can't be changed, added new file odp_debug.h as > a temporary fix. Further analysis will be done and fixed cleanly > later. > > Signed-off-by: Venkatesh Vivekanandan <venkatesh.vivekanandan@linaro.org> > --- > platform/linux-dpdk/include/odp_debug.h | 70 +++++++++++++++++++++++++++++++++ > 1 file changed, 70 insertions(+) > create mode 100644 platform/linux-dpdk/include/odp_debug.h > > diff --git a/platform/linux-dpdk/include/odp_debug.h b/platform/linux-dpdk/include/odp_debug.h > new file mode 100644 > index 0000000..e37ca8d > --- /dev/null > +++ b/platform/linux-dpdk/include/odp_debug.h > @@ -0,0 +1,70 @@ > +/* Copyright (c) 2013, Linaro Limited > + * All rights reserved. > + * > + * SPDX-License-Identifier: BSD-3-Clause > + */ > +/** > + * @file > + * > + * ODP debug > + */ > + > +#ifndef ODP_DEBUG_H_ > +#define ODP_DEBUG_H_ > + > +#include <stdio.h> > + > +#ifdef __cplusplus > +extern "C" { > +#endif > + > +#ifdef __GNUC__ > + > +/** > + * Indicate deprecated variables, functions or types > + */ > +#define ODP_DEPRECATED __attribute__((__deprecated__)) > + > +/** > + * Intentionally unused variables ot functions > + */ > +#define ODP_UNUSED __attribute__((__unused__)) > + > +#else > + > +#define ODP_DEPRECATED > +#define ODP_UNUSED > + > +#endif > + > +/** > + * Compile time assertion-macro - fail compilation if cond is false. > + */ > +#define ODP_ASSERT(cond, msg) typedef char msg[(cond) ? 1 : 0] This was the only thing you changed... and shouldn't we change this in the file include/odp_debug.h? Cheers, Anders > + > +/** > + * Compile time assertion-macro - fail compilation if cond is false. > + * @note This macro has zero runtime overhead > + */ > +#define ODP_STATIC_ASSERT(cond, msg) _static_assert(cond, msg) > + > +/** > + * Debug printing macro, which prints output when DEBUG flag is set. > + */ > +#define ODP_DBG(fmt, ...) \ > + do { if (ODP_DEBUG_PRINT == 1) \ > + printf(fmt, ##__VA_ARGS__); \ > + } while (0) > + > +/** > + * Print output to stderr (file, line and function). > + */ > +#define ODP_ERR(fmt, ...) \ > + fprintf(stderr, "%s:%d:%s(): " fmt, __FILE__, \ > + __LINE__, __func__, ##__VA_ARGS__) > + > +#ifdef __cplusplus > +} > +#endif > + > +#endif > -- > 1.8.1.2 > > > _______________________________________________ > lng-odp mailing list > lng-odp@lists.linaro.org > http://lists.linaro.org/mailman/listinfo/lng-odp
On 18 July 2014 02:17, Anders Roxell <anders.roxell@linaro.org> wrote: > On 2014-07-18 00:34, venkatesh.vivekanandan@linaro.org wrote: > > From: Venkatesh Vivekanandan <venkatesh.vivekanandan@linaro.org> > > > > - Added new file platform/linux-dpdk/include/odp_debug.h > > - ODP_ASSERT complains of array getting a negative value. Root cause is, > > struct rte_mbuf { > > . > > . > > } __rte_cache_aligned; <-- this alignment causes the error. > > > > Since dpdk code can't be changed, added new file odp_debug.h as > > a temporary fix. Further analysis will be done and fixed cleanly > > later. > > > > Signed-off-by: Venkatesh Vivekanandan <venkatesh.vivekanandan@linaro.org > > > > --- > > platform/linux-dpdk/include/odp_debug.h | 70 > +++++++++++++++++++++++++++++++++ > > 1 file changed, 70 insertions(+) > > create mode 100644 platform/linux-dpdk/include/odp_debug.h > > > > diff --git a/platform/linux-dpdk/include/odp_debug.h > b/platform/linux-dpdk/include/odp_debug.h > > new file mode 100644 > > index 0000000..e37ca8d > > --- /dev/null > > +++ b/platform/linux-dpdk/include/odp_debug.h > > @@ -0,0 +1,70 @@ > > +/* Copyright (c) 2013, Linaro Limited > > + * All rights reserved. > > + * > > + * SPDX-License-Identifier: BSD-3-Clause > > + */ > > +/** > > + * @file > > + * > > + * ODP debug > > + */ > > + > > +#ifndef ODP_DEBUG_H_ > > +#define ODP_DEBUG_H_ > > + > > +#include <stdio.h> > > + > > +#ifdef __cplusplus > > +extern "C" { > > +#endif > > + > > +#ifdef __GNUC__ > > + > > +/** > > + * Indicate deprecated variables, functions or types > > + */ > > +#define ODP_DEPRECATED __attribute__((__deprecated__)) > > + > > +/** > > + * Intentionally unused variables ot functions > > + */ > > +#define ODP_UNUSED __attribute__((__unused__)) > > + > > +#else > > + > > +#define ODP_DEPRECATED > > +#define ODP_UNUSED > > + > > +#endif > > + > > +/** > > + * Compile time assertion-macro - fail compilation if cond is false. > > + */ > > +#define ODP_ASSERT(cond, msg) typedef char msg[(cond) ? 1 : 0] > > This was the only thing you changed... and shouldn't we change this in > the file include/odp_debug.h? > That is not the right thing to do. Somehow sizeof and offsetof is not in sync giving different values, bug can be anywhere and the intention of this patch is not to hold initial patch and other demos that are dependent on this patch till then. > > Cheers, > Anders > > > + > > +/** > > + * Compile time assertion-macro - fail compilation if cond is false. > > + * @note This macro has zero runtime overhead > > + */ > > +#define ODP_STATIC_ASSERT(cond, msg) _static_assert(cond, msg) > > + > > +/** > > + * Debug printing macro, which prints output when DEBUG flag is set. > > + */ > > +#define ODP_DBG(fmt, ...) \ > > + do { if (ODP_DEBUG_PRINT == 1) \ > > + printf(fmt, ##__VA_ARGS__); \ > > + } while (0) > > + > > +/** > > + * Print output to stderr (file, line and function). > > + */ > > +#define ODP_ERR(fmt, ...) \ > > + fprintf(stderr, "%s:%d:%s(): " fmt, __FILE__, \ > > + __LINE__, __func__, ##__VA_ARGS__) > > + > > +#ifdef __cplusplus > > +} > > +#endif > > + > > +#endif > > -- > > 1.8.1.2 > > > > > > _______________________________________________ > > lng-odp mailing list > > lng-odp@lists.linaro.org > > http://lists.linaro.org/mailman/listinfo/lng-odp > > -- > Anders Roxell > anders.roxell@linaro.org > M: +46 709 71 42 85 | IRC: roxell >
We need to create an ODP bug to discover if it is our issue or possibly one in dpdk https://bugs.linaro.org/enter_bug.cgi?product=OpenDataPlane If we do that and immediately take up the bug it would allow us to move forward which frees initial demo work. On 18 July 2014 01:21, Venkatesh Vivekanandan < venkatesh.vivekanandan@linaro.org> wrote: > > > > On 18 July 2014 02:17, Anders Roxell <anders.roxell@linaro.org> wrote: > >> On 2014-07-18 00:34, venkatesh.vivekanandan@linaro.org wrote: >> > From: Venkatesh Vivekanandan <venkatesh.vivekanandan@linaro.org> >> > >> > - Added new file platform/linux-dpdk/include/odp_debug.h >> > - ODP_ASSERT complains of array getting a negative value. Root cause is, >> > struct rte_mbuf { >> > . >> > . >> > } __rte_cache_aligned; <-- this alignment causes the error. >> > >> > Since dpdk code can't be changed, added new file odp_debug.h as >> > a temporary fix. Further analysis will be done and fixed cleanly >> > later. >> > >> > Signed-off-by: Venkatesh Vivekanandan < >> venkatesh.vivekanandan@linaro.org> >> > --- >> > platform/linux-dpdk/include/odp_debug.h | 70 >> +++++++++++++++++++++++++++++++++ >> > 1 file changed, 70 insertions(+) >> > create mode 100644 platform/linux-dpdk/include/odp_debug.h >> > >> > diff --git a/platform/linux-dpdk/include/odp_debug.h >> b/platform/linux-dpdk/include/odp_debug.h >> > new file mode 100644 >> > index 0000000..e37ca8d >> > --- /dev/null >> > +++ b/platform/linux-dpdk/include/odp_debug.h >> > @@ -0,0 +1,70 @@ >> > +/* Copyright (c) 2013, Linaro Limited >> > + * All rights reserved. >> > + * >> > + * SPDX-License-Identifier: BSD-3-Clause >> > + */ >> > +/** >> > + * @file >> > + * >> > + * ODP debug >> > + */ >> > + >> > +#ifndef ODP_DEBUG_H_ >> > +#define ODP_DEBUG_H_ >> > + >> > +#include <stdio.h> >> > + >> > +#ifdef __cplusplus >> > +extern "C" { >> > +#endif >> > + >> > +#ifdef __GNUC__ >> > + >> > +/** >> > + * Indicate deprecated variables, functions or types >> > + */ >> > +#define ODP_DEPRECATED __attribute__((__deprecated__)) >> > + >> > +/** >> > + * Intentionally unused variables ot functions >> > + */ >> > +#define ODP_UNUSED __attribute__((__unused__)) >> > + >> > +#else >> > + >> > +#define ODP_DEPRECATED >> > +#define ODP_UNUSED >> > + >> > +#endif >> > + >> > +/** >> > + * Compile time assertion-macro - fail compilation if cond is false. >> > + */ >> > +#define ODP_ASSERT(cond, msg) typedef char msg[(cond) ? 1 : 0] >> >> This was the only thing you changed... and shouldn't we change this in >> the file include/odp_debug.h? >> > > That is not the right thing to do. Somehow sizeof and offsetof is not in > sync giving different values, bug can be anywhere and the intention of this > patch is not to hold initial patch and other demos that are dependent on > this patch till then. > > >> >> Cheers, >> Anders >> >> > + >> > +/** >> > + * Compile time assertion-macro - fail compilation if cond is false. >> > + * @note This macro has zero runtime overhead >> > + */ >> > +#define ODP_STATIC_ASSERT(cond, msg) _static_assert(cond, msg) >> > + >> > +/** >> > + * Debug printing macro, which prints output when DEBUG flag is set. >> > + */ >> > +#define ODP_DBG(fmt, ...) \ >> > + do { if (ODP_DEBUG_PRINT == 1) \ >> > + printf(fmt, ##__VA_ARGS__); \ >> > + } while (0) >> > + >> > +/** >> > + * Print output to stderr (file, line and function). >> > + */ >> > +#define ODP_ERR(fmt, ...) \ >> > + fprintf(stderr, "%s:%d:%s(): " fmt, __FILE__, \ >> > + __LINE__, __func__, ##__VA_ARGS__) >> > + >> > +#ifdef __cplusplus >> > +} >> > +#endif >> > + >> > +#endif >> > -- >> > 1.8.1.2 >> > >> > >> > _______________________________________________ >> > lng-odp mailing list >> > lng-odp@lists.linaro.org >> > http://lists.linaro.org/mailman/listinfo/lng-odp >> >> -- >> Anders Roxell >> anders.roxell@linaro.org >> M: +46 709 71 42 85 | IRC: roxell >> > > > _______________________________________________ > lng-odp mailing list > lng-odp@lists.linaro.org > http://lists.linaro.org/mailman/listinfo/lng-odp > >
On 22 July 2014 19:43, Mike Holmes <mike.holmes@linaro.org> wrote: > We need to create an ODP bug to discover if it is our issue or possibly > one in dpdk > https://bugs.linaro.org/enter_bug.cgi?product=OpenDataPlane > > If we do that and immediately take up the bug it would allow us to move > forward which frees initial demo work. > On further analysis it is figured out that, if there is a member structure that is aligned, then the sizeof will be more than offsetof. In this case, the check should be sizeof(odp_timer_hdr_t) >= offsetof(odp_timer_hdr_t, buf_data) at platform/linux-generic/odp_timer_internal.h, but since petri is on vacation it would be better to discuss with him before modifying it, as he introduced this check. For now, we will keep this patch pushed to the ODP upstream and keep a bug raised in parallel to track it. Opened LNG-575 <https://cards.linaro.org/browse/LNG-575> to track this as a bug. > > > On 18 July 2014 01:21, Venkatesh Vivekanandan < > venkatesh.vivekanandan@linaro.org> wrote: > >> >> >> >> On 18 July 2014 02:17, Anders Roxell <anders.roxell@linaro.org> wrote: >> >>> On 2014-07-18 00:34, venkatesh.vivekanandan@linaro.org wrote: >>> > From: Venkatesh Vivekanandan <venkatesh.vivekanandan@linaro.org> >>> > >>> > - Added new file platform/linux-dpdk/include/odp_debug.h >>> > - ODP_ASSERT complains of array getting a negative value. Root cause >>> is, >>> > struct rte_mbuf { >>> > . >>> > . >>> > } __rte_cache_aligned; <-- this alignment causes the error. >>> > >>> > Since dpdk code can't be changed, added new file odp_debug.h as >>> > a temporary fix. Further analysis will be done and fixed cleanly >>> > later. >>> > >>> > Signed-off-by: Venkatesh Vivekanandan < >>> venkatesh.vivekanandan@linaro.org> >>> > --- >>> > platform/linux-dpdk/include/odp_debug.h | 70 >>> +++++++++++++++++++++++++++++++++ >>> > 1 file changed, 70 insertions(+) >>> > create mode 100644 platform/linux-dpdk/include/odp_debug.h >>> > >>> > diff --git a/platform/linux-dpdk/include/odp_debug.h >>> b/platform/linux-dpdk/include/odp_debug.h >>> > new file mode 100644 >>> > index 0000000..e37ca8d >>> > --- /dev/null >>> > +++ b/platform/linux-dpdk/include/odp_debug.h >>> > @@ -0,0 +1,70 @@ >>> > +/* Copyright (c) 2013, Linaro Limited >>> > + * All rights reserved. >>> > + * >>> > + * SPDX-License-Identifier: BSD-3-Clause >>> > + */ >>> > +/** >>> > + * @file >>> > + * >>> > + * ODP debug >>> > + */ >>> > + >>> > +#ifndef ODP_DEBUG_H_ >>> > +#define ODP_DEBUG_H_ >>> > + >>> > +#include <stdio.h> >>> > + >>> > +#ifdef __cplusplus >>> > +extern "C" { >>> > +#endif >>> > + >>> > +#ifdef __GNUC__ >>> > + >>> > +/** >>> > + * Indicate deprecated variables, functions or types >>> > + */ >>> > +#define ODP_DEPRECATED __attribute__((__deprecated__)) >>> > + >>> > +/** >>> > + * Intentionally unused variables ot functions >>> > + */ >>> > +#define ODP_UNUSED __attribute__((__unused__)) >>> > + >>> > +#else >>> > + >>> > +#define ODP_DEPRECATED >>> > +#define ODP_UNUSED >>> > + >>> > +#endif >>> > + >>> > +/** >>> > + * Compile time assertion-macro - fail compilation if cond is false. >>> > + */ >>> > +#define ODP_ASSERT(cond, msg) typedef char msg[(cond) ? 1 : 0] >>> >>> This was the only thing you changed... and shouldn't we change this in >>> the file include/odp_debug.h? >>> >> >> That is not the right thing to do. Somehow sizeof and offsetof is not in >> sync giving different values, bug can be anywhere and the intention of this >> patch is not to hold initial patch and other demos that are dependent on >> this patch till then. >> >> >>> >>> Cheers, >>> Anders >>> >>> > + >>> > +/** >>> > + * Compile time assertion-macro - fail compilation if cond is false. >>> > + * @note This macro has zero runtime overhead >>> > + */ >>> > +#define ODP_STATIC_ASSERT(cond, msg) _static_assert(cond, msg) >>> > + >>> > +/** >>> > + * Debug printing macro, which prints output when DEBUG flag is set. >>> > + */ >>> > +#define ODP_DBG(fmt, ...) \ >>> > + do { if (ODP_DEBUG_PRINT == 1) \ >>> > + printf(fmt, ##__VA_ARGS__); \ >>> > + } while (0) >>> > + >>> > +/** >>> > + * Print output to stderr (file, line and function). >>> > + */ >>> > +#define ODP_ERR(fmt, ...) \ >>> > + fprintf(stderr, "%s:%d:%s(): " fmt, __FILE__, \ >>> > + __LINE__, __func__, ##__VA_ARGS__) >>> > + >>> > +#ifdef __cplusplus >>> > +} >>> > +#endif >>> > + >>> > +#endif >>> > -- >>> > 1.8.1.2 >>> > >>> > >>> > _______________________________________________ >>> > lng-odp mailing list >>> > lng-odp@lists.linaro.org >>> > http://lists.linaro.org/mailman/listinfo/lng-odp >>> >>> -- >>> Anders Roxell >>> anders.roxell@linaro.org >>> M: +46 709 71 42 85 | IRC: roxell >>> >> >> >> _______________________________________________ >> lng-odp mailing list >> lng-odp@lists.linaro.org >> http://lists.linaro.org/mailman/listinfo/lng-odp >> >> > > > -- > *Mike Holmes* > Linaro Technical Manager / Lead > LNG - ODP >
diff --git a/platform/linux-dpdk/include/odp_debug.h b/platform/linux-dpdk/include/odp_debug.h new file mode 100644 index 0000000..e37ca8d --- /dev/null +++ b/platform/linux-dpdk/include/odp_debug.h @@ -0,0 +1,70 @@ +/* Copyright (c) 2013, Linaro Limited + * All rights reserved. + * + * SPDX-License-Identifier: BSD-3-Clause + */ +/** + * @file + * + * ODP debug + */ + +#ifndef ODP_DEBUG_H_ +#define ODP_DEBUG_H_ + +#include <stdio.h> + +#ifdef __cplusplus +extern "C" { +#endif + +#ifdef __GNUC__ + +/** + * Indicate deprecated variables, functions or types + */ +#define ODP_DEPRECATED __attribute__((__deprecated__)) + +/** + * Intentionally unused variables ot functions + */ +#define ODP_UNUSED __attribute__((__unused__)) + +#else + +#define ODP_DEPRECATED +#define ODP_UNUSED + +#endif + +/** + * Compile time assertion-macro - fail compilation if cond is false. + */ +#define ODP_ASSERT(cond, msg) typedef char msg[(cond) ? 1 : 0] + +/** + * Compile time assertion-macro - fail compilation if cond is false. + * @note This macro has zero runtime overhead + */ +#define ODP_STATIC_ASSERT(cond, msg) _static_assert(cond, msg) + +/** + * Debug printing macro, which prints output when DEBUG flag is set. + */ +#define ODP_DBG(fmt, ...) \ + do { if (ODP_DEBUG_PRINT == 1) \ + printf(fmt, ##__VA_ARGS__); \ + } while (0) + +/** + * Print output to stderr (file, line and function). + */ +#define ODP_ERR(fmt, ...) \ + fprintf(stderr, "%s:%d:%s(): " fmt, __FILE__, \ + __LINE__, __func__, ##__VA_ARGS__) + +#ifdef __cplusplus +} +#endif + +#endif