| Message ID | 1461348713-28411-1-git-send-email-zoltan.kiss@linaro.org |
|---|---|
| State | New |
| Headers | show |
Originally I planned to have something like "-DINLINES=0/1", but that would force the application to define this macro in any case. Creating a config.h would be a way to overcome this maybe. On 22/04/16 19:11, Zoltan Kiss wrote: > ########################################################################## > +# Enable/disable static inlines > +########################################################################## > +AC_ARG_ENABLE([inlines], > + [ --enable-inlines allow inlines], > + [if test "x$enableval" = "xyes"; then > + ODP_CFLAGS="$ODP_CFLAGS -DINLINES" > + fi]) > +
--enable-inlines seems reasonable but is perhaps only one component in what's needed to enable abi mode operation. So do we need/want an umbrella --enable-abi option that turns on all sub-options (whatever they may be) to achieve ABI mode? On Mon, Apr 25, 2016 at 6:39 AM, Zoltan Kiss <zoltan.kiss@linaro.org> wrote: > Originally I planned to have something like "-DINLINES=0/1", but that > would force the application to define this macro in any case. > Creating a config.h would be a way to overcome this maybe. > > > > > On 22/04/16 19:11, Zoltan Kiss wrote: > >> >> ########################################################################## >> +# Enable/disable static inlines >> >> +########################################################################## >> +AC_ARG_ENABLE([inlines], >> + [ --enable-inlines allow inlines], >> + [if test "x$enableval" = "xyes"; then >> + ODP_CFLAGS="$ODP_CFLAGS -DINLINES" >> + fi]) >> + >> >
Yes, we can have a more general name, but I tend towards using something along the lines of --enable-abi-breakage On 25/04/16 12:59, Bill Fischofer wrote: > --enable-inlines seems reasonable but is perhaps only one component in > what's needed to enable abi mode operation. So do we need/want an > umbrella --enable-abi option that turns on all sub-options (whatever > they may be) to achieve ABI mode? > > On Mon, Apr 25, 2016 at 6:39 AM, Zoltan Kiss <zoltan.kiss@linaro.org > <mailto:zoltan.kiss@linaro.org>> wrote: > > Originally I planned to have something like "-DINLINES=0/1", but > that would force the application to define this macro in any case. > Creating a config.h would be a way to overcome this maybe. > > > > > On 22/04/16 19:11, Zoltan Kiss wrote: > > > ########################################################################## > +# Enable/disable static inlines > +########################################################################## > +AC_ARG_ENABLE([inlines], > + [ --enable-inlines allow inlines], > + [if test "x$enableval" = "xyes"; then > + ODP_CFLAGS="$ODP_CFLAGS -DINLINES" > + fi]) > + > >
On 25 April 2016 at 09:50, Zoltan Kiss <zoltan.kiss@linaro.org> wrote: > Yes, we can have a more general name, but I tend towards using something > along the lines of --enable-abi-breakage I dont like that, enable a negative? It needs to describe the intent and not a side effect. Assuming the default is performance - basically today's case, then we need to just enable abi-compliance or visa versa is enable-performance optimisations > > > On 25/04/16 12:59, Bill Fischofer wrote: > >> --enable-inlines seems reasonable but is perhaps only one component in >> what's needed to enable abi mode operation. So do we need/want an >> umbrella --enable-abi option that turns on all sub-options (whatever >> they may be) to achieve ABI mode? >> >> On Mon, Apr 25, 2016 at 6:39 AM, Zoltan Kiss <zoltan.kiss@linaro.org >> <mailto:zoltan.kiss@linaro.org>> wrote: >> >> Originally I planned to have something like "-DINLINES=0/1", but >> that would force the application to define this macro in any case. >> Creating a config.h would be a way to overcome this maybe. >> >> >> >> >> On 22/04/16 19:11, Zoltan Kiss wrote: >> >> >> >> ########################################################################## >> +# Enable/disable static inlines >> >> +########################################################################## >> +AC_ARG_ENABLE([inlines], >> + [ --enable-inlines allow inlines], >> + [if test "x$enableval" = "xyes"; then >> + ODP_CFLAGS="$ODP_CFLAGS -DINLINES" >> + fi]) >> + >> >> >> -- Mike Holmes Technical Manager - Linaro Networking Group Linaro.org <http://www.linaro.org/> *│ *Open source software for ARM SoCs "Work should be fun and collaborative, the rest follows"
Another option we just discussed with Mike: how about we use the --enable-dynamic configure option instead of this? If people want dynamic libraries, they want ABI compatibility, if not, they don't. I think the two thing are quite directly related to each other. In that case the question is how do you use that setting in your source files ... On 22/04/16 19:11, Zoltan Kiss wrote: > In order to have a fixed ABI, we need each function to be in the library > file. But that hurts performance a bit, as small accessor functions > couldn't be inlined. To have the ability to use both, this patch > creates a new configure option, which enables the use of inline > functions for users who want better performance. > The accessors are moved to a packet_inlines.h file, by default it is > included from odp_packet.c, and they appear as fully fledged functions. > If --enable-inlines=yes is added to ./configure, they won't be compiled > into the .so file, but the application has to add "#define INLINES" before > including odp.h > > Signed-off-by: Zoltan Kiss <zoltan.kiss@linaro.org> > --- > configure.ac | 9 +++ > platform/linux-dpdk/Makefile.am | 1 + > platform/linux-dpdk/include/odp/api/packet.h | 59 +-------------- > .../linux-dpdk/include/odp/api/packet_inlines.h | 86 ++++++++++++++++++++++ > platform/linux-dpdk/odp_packet.c | 3 + > 5 files changed, 102 insertions(+), 56 deletions(-) > create mode 100644 platform/linux-dpdk/include/odp/api/packet_inlines.h > > diff --git a/configure.ac b/configure.ac > index 99772a1..6b8fb76 100644 > --- a/configure.ac > +++ b/configure.ac > @@ -173,6 +173,15 @@ AC_ARG_ENABLE([debug], > ODP_CFLAGS="$ODP_CFLAGS -DODP_DEBUG=$ODP_DEBUG" > > ########################################################################## > +# Enable/disable static inlines > +########################################################################## > +AC_ARG_ENABLE([inlines], > + [ --enable-inlines allow inlines], > + [if test "x$enableval" = "xyes"; then > + ODP_CFLAGS="$ODP_CFLAGS -DINLINES" > + fi]) > + > +########################################################################## > # Save and set temporary compilation flags > ########################################################################## > OLD_LDFLAGS=$LDFLAGS > diff --git a/platform/linux-dpdk/Makefile.am b/platform/linux-dpdk/Makefile.am > index eadaf63..b8cba23 100644 > --- a/platform/linux-dpdk/Makefile.am > +++ b/platform/linux-dpdk/Makefile.am > @@ -49,6 +49,7 @@ odpapiinclude_HEADERS = \ > $(srcdir)/include/odp/api/init.h \ > $(srcdir)/include/odp/api/packet_flags.h \ > $(srcdir)/include/odp/api/packet.h \ > + $(srcdir)/include/odp/api/packet_inlines.h \ > $(srcdir)/include/odp/api/packet_io.h \ > $(srcdir)/include/odp/api/pool.h \ > $(srcdir)/include/odp/api/queue.h \ > diff --git a/platform/linux-dpdk/include/odp/api/packet.h b/platform/linux-dpdk/include/odp/api/packet.h > index 7c0db32..e2ba78a 100644 > --- a/platform/linux-dpdk/include/odp/api/packet.h > +++ b/platform/linux-dpdk/include/odp/api/packet.h > @@ -27,62 +27,9 @@ extern "C" { > /** @ingroup odp_packet > * @{ > */ > - > -extern const unsigned int buf_addr_offset; > -extern const unsigned int data_off_offset; > -extern const unsigned int pkt_len_offset; > -extern const unsigned int seg_len_offset; > -extern const unsigned int udata_len_offset; > -extern const unsigned int udata_offset; > -extern const unsigned int rss_offset; > -extern const unsigned int ol_flags_offset; > -extern const uint64_t rss_flag; > - > -/* > - * NOTE: These functions are inlined because they are on a performance hot path. > - * As we can't force the application to directly include DPDK headers we have to > - * export these fields through constants calculated compile time in > - * odp_packet.c, where we can see the DPDK definitions. > - * > - */ > -static inline uint32_t odp_packet_len(odp_packet_t pkt) > -{ > - return *(uint32_t *)(void *)((char *)pkt + pkt_len_offset); > -} > - > -static inline uint32_t odp_packet_seg_len(odp_packet_t pkt) > -{ > - return *(uint16_t *)(void *)((char *)pkt + seg_len_offset); > -} > - > -static inline void *odp_packet_user_area(odp_packet_t pkt) > -{ > - return (void *)((char *)pkt + udata_offset); > -} > - > -static inline uint32_t odp_packet_user_area_size(odp_packet_t pkt) > -{ > - return *(uint32_t *)(void *)((char *)pkt + udata_len_offset); > -} > - > -static inline void *odp_packet_data(odp_packet_t pkt) > -{ > - char** buf_addr = (char **)(void *)((char *)pkt + buf_addr_offset); > - uint16_t data_off = *(uint16_t *)(void *)((char *)pkt + data_off_offset); > - return (void *)(*buf_addr + data_off); > -} > - > -static inline uint32_t odp_packet_flow_hash(odp_packet_t pkt) > -{ > - return *(uint32_t *)(void *)((char *)pkt + rss_offset); > -} > - > -static inline void odp_packet_flow_hash_set(odp_packet_t pkt, uint32_t flow_hash) > -{ > - *(uint32_t *)(void *)((char *)pkt + rss_offset) = flow_hash; > - *(uint64_t *)(void *)((char *)pkt + ol_flags_offset) |= rss_flag; > -} > - > +#ifdef INLINES > +#include <odp/api/packet_inlines.h> > +#endif > /** > * @} > */ > diff --git a/platform/linux-dpdk/include/odp/api/packet_inlines.h b/platform/linux-dpdk/include/odp/api/packet_inlines.h > new file mode 100644 > index 0000000..3d3ca81 > --- /dev/null > +++ b/platform/linux-dpdk/include/odp/api/packet_inlines.h > @@ -0,0 +1,86 @@ > +/* Copyright (c) 2016, Linaro Limited > + * All rights reserved. > + * > + * SPDX-License-Identifier: BSD-3-Clause > + */ > + > +/** > + * @file > + * > + * ODP packet inline functions > + */ > + > +#ifndef ODP_PLAT_PACKET_INLINES_H_ > +#define ODP_PLAT_PACKET_INLINES_H_ > + > +#ifdef __cplusplus > +extern "C" { > +#endif > + > +#ifdef INLINES > +#define _STATIC static inline > + > +extern const unsigned int buf_addr_offset; > +extern const unsigned int data_off_offset; > +extern const unsigned int pkt_len_offset; > +extern const unsigned int seg_len_offset; > +extern const unsigned int udata_len_offset; > +extern const unsigned int udata_offset; > +extern const unsigned int rss_offset; > +extern const unsigned int ol_flags_offset; > +extern const uint64_t rss_flag; > + > +#else > +#define _STATIC > +#endif > + > +/* > + * NOTE: These functions are inlined because they are on a performance hot path. > + * As we can't force the application to directly include DPDK headers we have to > + * export these fields through constants calculated compile time in > + * odp_packet.c, where we can see the DPDK definitions. > + * > + */ > +_STATIC uint32_t odp_packet_len(odp_packet_t pkt) > +{ > + return *(uint32_t *)(void *)((char *)pkt + pkt_len_offset); > +} > + > +_STATIC uint32_t odp_packet_seg_len(odp_packet_t pkt) > +{ > + return *(uint16_t *)(void *)((char *)pkt + seg_len_offset); > +} > + > +_STATIC void *odp_packet_user_area(odp_packet_t pkt) > +{ > + return (void *)((char *)pkt + udata_offset); > +} > + > +_STATIC uint32_t odp_packet_user_area_size(odp_packet_t pkt) > +{ > + return *(uint32_t *)(void *)((char *)pkt + udata_len_offset); > +} > + > +_STATIC void *odp_packet_data(odp_packet_t pkt) > +{ > + char** buf_addr = (char **)(void *)((char *)pkt + buf_addr_offset); > + uint16_t data_off = *(uint16_t *)(void *)((char *)pkt + data_off_offset); > + return (void *)(*buf_addr + data_off); > +} > + > +_STATIC uint32_t odp_packet_flow_hash(odp_packet_t pkt) > +{ > + return *(uint32_t *)(void *)((char *)pkt + rss_offset); > +} > + > +_STATIC void odp_packet_flow_hash_set(odp_packet_t pkt, uint32_t flow_hash) > +{ > + *(uint32_t *)(void *)((char *)pkt + rss_offset) = flow_hash; > + *(uint64_t *)(void *)((char *)pkt + ol_flags_offset) |= rss_flag; > +} > + > +#ifdef __cplusplus > +} > +#endif > + > +#endif /* ODP_PLAT_PACKET_INLINES_H_ */ > diff --git a/platform/linux-dpdk/odp_packet.c b/platform/linux-dpdk/odp_packet.c > index e031b09..456ced6 100644 > --- a/platform/linux-dpdk/odp_packet.c > +++ b/platform/linux-dpdk/odp_packet.c > @@ -60,6 +60,9 @@ _ODP_STATIC_ASSERT(sizeof(dummy.hash.rss) == sizeof(uint32_t), > _ODP_STATIC_ASSERT(sizeof(dummy.ol_flags) == sizeof(uint64_t), > "ol_flags should be uint64_t"); > > +#ifndef INLINES > +#include <odp/api/packet_inlines.h> > +#endif > > odp_packet_t _odp_packet_from_buffer(odp_buffer_t buf) > { >
ODP applications compile against the ODP library, which is either built stand-alone or (more typically) installed into /usr/include. Depending on how the library is built (Performance vs. ABI) , that include library either will or will not contain inlines. So there shouldn't be the need for anything special in the application source. Whether the application is ABI mode or not should depend only on the "flavor" of the ODP library it compiles against. On Tue, Apr 26, 2016 at 1:22 PM, Zoltan Kiss <zoltan.kiss@linaro.org> wrote: > Another option we just discussed with Mike: how about we use the > --enable-dynamic configure option instead of this? If people want dynamic > libraries, they want ABI compatibility, if not, they don't. I think the two > thing are quite directly related to each other. > In that case the question is how do you use that setting in your source > files ... > > On 22/04/16 19:11, Zoltan Kiss wrote: > >> In order to have a fixed ABI, we need each function to be in the library >> file. But that hurts performance a bit, as small accessor functions >> couldn't be inlined. To have the ability to use both, this patch >> creates a new configure option, which enables the use of inline >> functions for users who want better performance. >> The accessors are moved to a packet_inlines.h file, by default it is >> included from odp_packet.c, and they appear as fully fledged functions. >> If --enable-inlines=yes is added to ./configure, they won't be compiled >> into the .so file, but the application has to add "#define INLINES" before >> including odp.h >> >> Signed-off-by: Zoltan Kiss <zoltan.kiss@linaro.org> >> --- >> configure.ac | 9 +++ >> platform/linux-dpdk/Makefile.am | 1 + >> platform/linux-dpdk/include/odp/api/packet.h | 59 +-------------- >> .../linux-dpdk/include/odp/api/packet_inlines.h | 86 >> ++++++++++++++++++++++ >> platform/linux-dpdk/odp_packet.c | 3 + >> 5 files changed, 102 insertions(+), 56 deletions(-) >> create mode 100644 platform/linux-dpdk/include/odp/api/packet_inlines.h >> >> diff --git a/configure.ac b/configure.ac >> index 99772a1..6b8fb76 100644 >> --- a/configure.ac >> +++ b/configure.ac >> @@ -173,6 +173,15 @@ AC_ARG_ENABLE([debug], >> ODP_CFLAGS="$ODP_CFLAGS -DODP_DEBUG=$ODP_DEBUG" >> >> >> ########################################################################## >> +# Enable/disable static inlines >> >> +########################################################################## >> +AC_ARG_ENABLE([inlines], >> + [ --enable-inlines allow inlines], >> + [if test "x$enableval" = "xyes"; then >> + ODP_CFLAGS="$ODP_CFLAGS -DINLINES" >> + fi]) >> + >> >> +########################################################################## >> # Save and set temporary compilation flags >> >> ########################################################################## >> OLD_LDFLAGS=$LDFLAGS >> diff --git a/platform/linux-dpdk/Makefile.am >> b/platform/linux-dpdk/Makefile.am >> index eadaf63..b8cba23 100644 >> --- a/platform/linux-dpdk/Makefile.am >> +++ b/platform/linux-dpdk/Makefile.am >> @@ -49,6 +49,7 @@ odpapiinclude_HEADERS = \ >> $(srcdir)/include/odp/api/init.h \ >> $(srcdir)/include/odp/api/packet_flags.h \ >> $(srcdir)/include/odp/api/packet.h \ >> + $(srcdir)/include/odp/api/packet_inlines.h \ >> $(srcdir)/include/odp/api/packet_io.h \ >> $(srcdir)/include/odp/api/pool.h \ >> $(srcdir)/include/odp/api/queue.h \ >> diff --git a/platform/linux-dpdk/include/odp/api/packet.h >> b/platform/linux-dpdk/include/odp/api/packet.h >> index 7c0db32..e2ba78a 100644 >> --- a/platform/linux-dpdk/include/odp/api/packet.h >> +++ b/platform/linux-dpdk/include/odp/api/packet.h >> @@ -27,62 +27,9 @@ extern "C" { >> /** @ingroup odp_packet >> * @{ >> */ >> - >> -extern const unsigned int buf_addr_offset; >> -extern const unsigned int data_off_offset; >> -extern const unsigned int pkt_len_offset; >> -extern const unsigned int seg_len_offset; >> -extern const unsigned int udata_len_offset; >> -extern const unsigned int udata_offset; >> -extern const unsigned int rss_offset; >> -extern const unsigned int ol_flags_offset; >> -extern const uint64_t rss_flag; >> - >> -/* >> - * NOTE: These functions are inlined because they are on a performance >> hot path. >> - * As we can't force the application to directly include DPDK headers we >> have to >> - * export these fields through constants calculated compile time in >> - * odp_packet.c, where we can see the DPDK definitions. >> - * >> - */ >> -static inline uint32_t odp_packet_len(odp_packet_t pkt) >> -{ >> - return *(uint32_t *)(void *)((char *)pkt + pkt_len_offset); >> -} >> - >> -static inline uint32_t odp_packet_seg_len(odp_packet_t pkt) >> -{ >> - return *(uint16_t *)(void *)((char *)pkt + seg_len_offset); >> -} >> - >> -static inline void *odp_packet_user_area(odp_packet_t pkt) >> -{ >> - return (void *)((char *)pkt + udata_offset); >> -} >> - >> -static inline uint32_t odp_packet_user_area_size(odp_packet_t pkt) >> -{ >> - return *(uint32_t *)(void *)((char *)pkt + udata_len_offset); >> -} >> - >> -static inline void *odp_packet_data(odp_packet_t pkt) >> -{ >> - char** buf_addr = (char **)(void *)((char *)pkt + >> buf_addr_offset); >> - uint16_t data_off = *(uint16_t *)(void *)((char *)pkt + >> data_off_offset); >> - return (void *)(*buf_addr + data_off); >> -} >> - >> -static inline uint32_t odp_packet_flow_hash(odp_packet_t pkt) >> -{ >> - return *(uint32_t *)(void *)((char *)pkt + rss_offset); >> -} >> - >> -static inline void odp_packet_flow_hash_set(odp_packet_t pkt, uint32_t >> flow_hash) >> -{ >> - *(uint32_t *)(void *)((char *)pkt + rss_offset) = flow_hash; >> - *(uint64_t *)(void *)((char *)pkt + ol_flags_offset) |= rss_flag; >> -} >> - >> +#ifdef INLINES >> +#include <odp/api/packet_inlines.h> >> +#endif >> /** >> * @} >> */ >> diff --git a/platform/linux-dpdk/include/odp/api/packet_inlines.h >> b/platform/linux-dpdk/include/odp/api/packet_inlines.h >> new file mode 100644 >> index 0000000..3d3ca81 >> --- /dev/null >> +++ b/platform/linux-dpdk/include/odp/api/packet_inlines.h >> @@ -0,0 +1,86 @@ >> +/* Copyright (c) 2016, Linaro Limited >> + * All rights reserved. >> + * >> + * SPDX-License-Identifier: BSD-3-Clause >> + */ >> + >> +/** >> + * @file >> + * >> + * ODP packet inline functions >> + */ >> + >> +#ifndef ODP_PLAT_PACKET_INLINES_H_ >> +#define ODP_PLAT_PACKET_INLINES_H_ >> + >> +#ifdef __cplusplus >> +extern "C" { >> +#endif >> + >> +#ifdef INLINES >> +#define _STATIC static inline >> + >> +extern const unsigned int buf_addr_offset; >> +extern const unsigned int data_off_offset; >> +extern const unsigned int pkt_len_offset; >> +extern const unsigned int seg_len_offset; >> +extern const unsigned int udata_len_offset; >> +extern const unsigned int udata_offset; >> +extern const unsigned int rss_offset; >> +extern const unsigned int ol_flags_offset; >> +extern const uint64_t rss_flag; >> + >> +#else >> +#define _STATIC >> +#endif >> + >> +/* >> + * NOTE: These functions are inlined because they are on a performance >> hot path. >> + * As we can't force the application to directly include DPDK headers we >> have to >> + * export these fields through constants calculated compile time in >> + * odp_packet.c, where we can see the DPDK definitions. >> + * >> + */ >> +_STATIC uint32_t odp_packet_len(odp_packet_t pkt) >> +{ >> + return *(uint32_t *)(void *)((char *)pkt + pkt_len_offset); >> +} >> + >> +_STATIC uint32_t odp_packet_seg_len(odp_packet_t pkt) >> +{ >> + return *(uint16_t *)(void *)((char *)pkt + seg_len_offset); >> +} >> + >> +_STATIC void *odp_packet_user_area(odp_packet_t pkt) >> +{ >> + return (void *)((char *)pkt + udata_offset); >> +} >> + >> +_STATIC uint32_t odp_packet_user_area_size(odp_packet_t pkt) >> +{ >> + return *(uint32_t *)(void *)((char *)pkt + udata_len_offset); >> +} >> + >> +_STATIC void *odp_packet_data(odp_packet_t pkt) >> +{ >> + char** buf_addr = (char **)(void *)((char *)pkt + >> buf_addr_offset); >> + uint16_t data_off = *(uint16_t *)(void *)((char *)pkt + >> data_off_offset); >> + return (void *)(*buf_addr + data_off); >> +} >> + >> +_STATIC uint32_t odp_packet_flow_hash(odp_packet_t pkt) >> +{ >> + return *(uint32_t *)(void *)((char *)pkt + rss_offset); >> +} >> + >> +_STATIC void odp_packet_flow_hash_set(odp_packet_t pkt, uint32_t >> flow_hash) >> +{ >> + *(uint32_t *)(void *)((char *)pkt + rss_offset) = flow_hash; >> + *(uint64_t *)(void *)((char *)pkt + ol_flags_offset) |= rss_flag; >> +} >> + >> +#ifdef __cplusplus >> +} >> +#endif >> + >> +#endif /* ODP_PLAT_PACKET_INLINES_H_ */ >> diff --git a/platform/linux-dpdk/odp_packet.c >> b/platform/linux-dpdk/odp_packet.c >> index e031b09..456ced6 100644 >> --- a/platform/linux-dpdk/odp_packet.c >> +++ b/platform/linux-dpdk/odp_packet.c >> @@ -60,6 +60,9 @@ _ODP_STATIC_ASSERT(sizeof(dummy.hash.rss) == >> sizeof(uint32_t), >> _ODP_STATIC_ASSERT(sizeof(dummy.ol_flags) == sizeof(uint64_t), >> "ol_flags should be uint64_t"); >> >> +#ifndef INLINES >> +#include <odp/api/packet_inlines.h> >> +#endif >> >> odp_packet_t _odp_packet_from_buffer(odp_buffer_t buf) >> { >> >>
diff --git a/configure.ac b/configure.ac index 99772a1..6b8fb76 100644 --- a/configure.ac +++ b/configure.ac @@ -173,6 +173,15 @@ AC_ARG_ENABLE([debug], ODP_CFLAGS="$ODP_CFLAGS -DODP_DEBUG=$ODP_DEBUG" ########################################################################## +# Enable/disable static inlines +########################################################################## +AC_ARG_ENABLE([inlines], + [ --enable-inlines allow inlines], + [if test "x$enableval" = "xyes"; then + ODP_CFLAGS="$ODP_CFLAGS -DINLINES" + fi]) + +########################################################################## # Save and set temporary compilation flags ########################################################################## OLD_LDFLAGS=$LDFLAGS diff --git a/platform/linux-dpdk/Makefile.am b/platform/linux-dpdk/Makefile.am index eadaf63..b8cba23 100644 --- a/platform/linux-dpdk/Makefile.am +++ b/platform/linux-dpdk/Makefile.am @@ -49,6 +49,7 @@ odpapiinclude_HEADERS = \ $(srcdir)/include/odp/api/init.h \ $(srcdir)/include/odp/api/packet_flags.h \ $(srcdir)/include/odp/api/packet.h \ + $(srcdir)/include/odp/api/packet_inlines.h \ $(srcdir)/include/odp/api/packet_io.h \ $(srcdir)/include/odp/api/pool.h \ $(srcdir)/include/odp/api/queue.h \ diff --git a/platform/linux-dpdk/include/odp/api/packet.h b/platform/linux-dpdk/include/odp/api/packet.h index 7c0db32..e2ba78a 100644 --- a/platform/linux-dpdk/include/odp/api/packet.h +++ b/platform/linux-dpdk/include/odp/api/packet.h @@ -27,62 +27,9 @@ extern "C" { /** @ingroup odp_packet * @{ */ - -extern const unsigned int buf_addr_offset; -extern const unsigned int data_off_offset; -extern const unsigned int pkt_len_offset; -extern const unsigned int seg_len_offset; -extern const unsigned int udata_len_offset; -extern const unsigned int udata_offset; -extern const unsigned int rss_offset; -extern const unsigned int ol_flags_offset; -extern const uint64_t rss_flag; - -/* - * NOTE: These functions are inlined because they are on a performance hot path. - * As we can't force the application to directly include DPDK headers we have to - * export these fields through constants calculated compile time in - * odp_packet.c, where we can see the DPDK definitions. - * - */ -static inline uint32_t odp_packet_len(odp_packet_t pkt) -{ - return *(uint32_t *)(void *)((char *)pkt + pkt_len_offset); -} - -static inline uint32_t odp_packet_seg_len(odp_packet_t pkt) -{ - return *(uint16_t *)(void *)((char *)pkt + seg_len_offset); -} - -static inline void *odp_packet_user_area(odp_packet_t pkt) -{ - return (void *)((char *)pkt + udata_offset); -} - -static inline uint32_t odp_packet_user_area_size(odp_packet_t pkt) -{ - return *(uint32_t *)(void *)((char *)pkt + udata_len_offset); -} - -static inline void *odp_packet_data(odp_packet_t pkt) -{ - char** buf_addr = (char **)(void *)((char *)pkt + buf_addr_offset); - uint16_t data_off = *(uint16_t *)(void *)((char *)pkt + data_off_offset); - return (void *)(*buf_addr + data_off); -} - -static inline uint32_t odp_packet_flow_hash(odp_packet_t pkt) -{ - return *(uint32_t *)(void *)((char *)pkt + rss_offset); -} - -static inline void odp_packet_flow_hash_set(odp_packet_t pkt, uint32_t flow_hash) -{ - *(uint32_t *)(void *)((char *)pkt + rss_offset) = flow_hash; - *(uint64_t *)(void *)((char *)pkt + ol_flags_offset) |= rss_flag; -} - +#ifdef INLINES +#include <odp/api/packet_inlines.h> +#endif /** * @} */ diff --git a/platform/linux-dpdk/include/odp/api/packet_inlines.h b/platform/linux-dpdk/include/odp/api/packet_inlines.h new file mode 100644 index 0000000..3d3ca81 --- /dev/null +++ b/platform/linux-dpdk/include/odp/api/packet_inlines.h @@ -0,0 +1,86 @@ +/* Copyright (c) 2016, Linaro Limited + * All rights reserved. + * + * SPDX-License-Identifier: BSD-3-Clause + */ + +/** + * @file + * + * ODP packet inline functions + */ + +#ifndef ODP_PLAT_PACKET_INLINES_H_ +#define ODP_PLAT_PACKET_INLINES_H_ + +#ifdef __cplusplus +extern "C" { +#endif + +#ifdef INLINES +#define _STATIC static inline + +extern const unsigned int buf_addr_offset; +extern const unsigned int data_off_offset; +extern const unsigned int pkt_len_offset; +extern const unsigned int seg_len_offset; +extern const unsigned int udata_len_offset; +extern const unsigned int udata_offset; +extern const unsigned int rss_offset; +extern const unsigned int ol_flags_offset; +extern const uint64_t rss_flag; + +#else +#define _STATIC +#endif + +/* + * NOTE: These functions are inlined because they are on a performance hot path. + * As we can't force the application to directly include DPDK headers we have to + * export these fields through constants calculated compile time in + * odp_packet.c, where we can see the DPDK definitions. + * + */ +_STATIC uint32_t odp_packet_len(odp_packet_t pkt) +{ + return *(uint32_t *)(void *)((char *)pkt + pkt_len_offset); +} + +_STATIC uint32_t odp_packet_seg_len(odp_packet_t pkt) +{ + return *(uint16_t *)(void *)((char *)pkt + seg_len_offset); +} + +_STATIC void *odp_packet_user_area(odp_packet_t pkt) +{ + return (void *)((char *)pkt + udata_offset); +} + +_STATIC uint32_t odp_packet_user_area_size(odp_packet_t pkt) +{ + return *(uint32_t *)(void *)((char *)pkt + udata_len_offset); +} + +_STATIC void *odp_packet_data(odp_packet_t pkt) +{ + char** buf_addr = (char **)(void *)((char *)pkt + buf_addr_offset); + uint16_t data_off = *(uint16_t *)(void *)((char *)pkt + data_off_offset); + return (void *)(*buf_addr + data_off); +} + +_STATIC uint32_t odp_packet_flow_hash(odp_packet_t pkt) +{ + return *(uint32_t *)(void *)((char *)pkt + rss_offset); +} + +_STATIC void odp_packet_flow_hash_set(odp_packet_t pkt, uint32_t flow_hash) +{ + *(uint32_t *)(void *)((char *)pkt + rss_offset) = flow_hash; + *(uint64_t *)(void *)((char *)pkt + ol_flags_offset) |= rss_flag; +} + +#ifdef __cplusplus +} +#endif + +#endif /* ODP_PLAT_PACKET_INLINES_H_ */ diff --git a/platform/linux-dpdk/odp_packet.c b/platform/linux-dpdk/odp_packet.c index e031b09..456ced6 100644 --- a/platform/linux-dpdk/odp_packet.c +++ b/platform/linux-dpdk/odp_packet.c @@ -60,6 +60,9 @@ _ODP_STATIC_ASSERT(sizeof(dummy.hash.rss) == sizeof(uint32_t), _ODP_STATIC_ASSERT(sizeof(dummy.ol_flags) == sizeof(uint64_t), "ol_flags should be uint64_t"); +#ifndef INLINES +#include <odp/api/packet_inlines.h> +#endif odp_packet_t _odp_packet_from_buffer(odp_buffer_t buf) {
In order to have a fixed ABI, we need each function to be in the library file. But that hurts performance a bit, as small accessor functions couldn't be inlined. To have the ability to use both, this patch creates a new configure option, which enables the use of inline functions for users who want better performance. The accessors are moved to a packet_inlines.h file, by default it is included from odp_packet.c, and they appear as fully fledged functions. If --enable-inlines=yes is added to ./configure, they won't be compiled into the .so file, but the application has to add "#define INLINES" before including odp.h Signed-off-by: Zoltan Kiss <zoltan.kiss@linaro.org> --- configure.ac | 9 +++ platform/linux-dpdk/Makefile.am | 1 + platform/linux-dpdk/include/odp/api/packet.h | 59 +-------------- .../linux-dpdk/include/odp/api/packet_inlines.h | 86 ++++++++++++++++++++++ platform/linux-dpdk/odp_packet.c | 3 + 5 files changed, 102 insertions(+), 56 deletions(-) create mode 100644 platform/linux-dpdk/include/odp/api/packet_inlines.h