Message ID | 1464185585-24454-1-git-send-email-zoltan.kiss@linaro.org |
---|---|
State | New |
Headers | show |
On Wed, May 25, 2016 at 9:13 AM, Zoltan Kiss <zoltan.kiss@linaro.org> wrote: > In order to have proper packaging and dynamic linking we need a fixed ABI. > For that, each function has to be in the library file. But that hurts > performance a bit, as small accessor functions couldn't be inlined, > despite it's a viable option with static linking. > This patch lets the platform automatically decide what should be done. > ./configure generates inlines.h based on whether --enable-shared=yes was > added or not. If yes, it enables the INLINES macro. > 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 INLINES defined, the accessors appear as static inline functions in the > header, and the application can directly inline them. > > Signed-off-by: Zoltan Kiss <zoltan.kiss@linaro.org> > --- > .gitignore | 1 + > configure.ac | 7 ++ > platform/linux-dpdk/Makefile.am | 2 + > platform/linux-dpdk/include/odp/api/inlines.h.in | 27 +++++++ > platform/linux-dpdk/include/odp/api/packet.h | 60 +-------------- > .../linux-dpdk/include/odp/api/packet_inlines.h | 87 > ++++++++++++++++++++++ > platform/linux-dpdk/m4/configure.m4 | 3 +- > platform/linux-dpdk/odp_packet.c | 3 + > 8 files changed, 133 insertions(+), 57 deletions(-) > create mode 100644 platform/linux-dpdk/include/odp/api/inlines.h.in > create mode 100644 platform/linux-dpdk/include/odp/api/packet_inlines.h > > diff --git a/.gitignore b/.gitignore > index 2b1da19..4040e2b 100644 > --- a/.gitignore > +++ b/.gitignore > @@ -40,5 +40,6 @@ ltmain.sh > m4/*.m4 > missing > pkgconfig/libodp*.pc > +platform/linux-dpdk/include/odp/api/inlines.h > tags > test-driver > diff --git a/configure.ac b/configure.ac > index 99772a1..199a6e2 100644 > --- a/configure.ac > +++ b/configure.ac > @@ -141,6 +141,13 @@ AM_CONDITIONAL([test_helper], [test x$test_helper = > xyes ]) > AM_CONDITIONAL([HAVE_DOXYGEN], [test "x${DOXYGEN}" = "xdoxygen"]) > AM_CONDITIONAL([user_guide], [test "x${user_guides}" = "xyes" ]) > AM_CONDITIONAL([HAVE_MSCGEN], [test "x${MSCGEN}" = "xmscgen"]) > +if test x$enable_shared != xyes; > +then > + INLINES="INLINES" > +else > + INLINES="__NOTHING" > +fi > +AC_SUBST(INLINES) > > ########################################################################## > # Setup doxygen documentation > diff --git a/platform/linux-dpdk/Makefile.am > b/platform/linux-dpdk/Makefile.am > index eadaf63..49ad877 100644 > --- a/platform/linux-dpdk/Makefile.am > +++ b/platform/linux-dpdk/Makefile.am > @@ -47,8 +47,10 @@ odpapiinclude_HEADERS = \ > $(srcdir)/include/odp/api/hash.h \ > $(srcdir)/include/odp/api/hints.h \ > $(srcdir)/include/odp/api/init.h \ > + $(srcdir)/include/odp/api/inlines.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/inlines.h.in > b/platform/linux-dpdk/include/odp/api/inlines.h.in > new file mode 100644 > index 0000000..1e8bac8 > --- /dev/null > +++ b/platform/linux-dpdk/include/odp/api/inlines.h.in > @@ -0,0 +1,27 @@ > +/* Copyright (c) 2016, Linaro Limited > + * All rights reserved. > + * > + * SPDX-License-Identifier: BSD-3-Clause > + */ > + > +/** > + * @file > + * > + * ODP packet inline functions > + */ > + > +#ifndef ODP_PLAT_INLINES_H_ > +#define ODP_PLAT_INLINES_H_ > + > +#ifdef __cplusplus > +extern "C" { > +#endif > + > +#define @INLINES@ > Why the @ symbols here? Also, as an ODP configuration option, shouldn't this be something like ODP_INLINES to avoid potential name space issues? > + > +#ifdef __cplusplus > +} > +#endif > + > + > +#endif /* ODP_PLAT_INLINES_H_ */ > diff --git a/platform/linux-dpdk/include/odp/api/packet.h > b/platform/linux-dpdk/include/odp/api/packet.h > index 7c0db32..0fb31e3 100644 > --- a/platform/linux-dpdk/include/odp/api/packet.h > +++ b/platform/linux-dpdk/include/odp/api/packet.h > @@ -27,62 +27,10 @@ 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; > -} > - > +#include <odp/api/inlines.h> > +#ifdef INLINES > ODP_INLINES ? (see earlier comment) > +#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..569090b > --- /dev/null > +++ b/platform/linux-dpdk/include/odp/api/packet_inlines.h > @@ -0,0 +1,87 @@ > +/* 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/m4/configure.m4 > b/platform/linux-dpdk/m4/configure.m4 > index fab0f6a..7c5ccbc 100644 > --- a/platform/linux-dpdk/m4/configure.m4 > +++ b/platform/linux-dpdk/m4/configure.m4 > @@ -47,4 +47,5 @@ m4_include([platform/linux-dpdk/m4/odp_openssl.m4]) > > AC_CONFIG_FILES([platform/linux-dpdk/Makefile > platform/linux-dpdk/test/Makefile > - platform/linux-dpdk/test/pktio/Makefile]) > + platform/linux-dpdk/test/pktio/Makefile > + platform/linux-dpdk/include/odp/api/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 > Same comment as above. > +#include <odp/api/packet_inlines.h> > +#endif > > odp_packet_t _odp_packet_from_buffer(odp_buffer_t buf) > { > -- > 1.9.1 > > _______________________________________________ > lng-odp-dpdk mailing list > lng-odp-dpdk@lists.linaro.org > https://lists.linaro.org/mailman/listinfo/lng-odp-dpdk >
Hi, On 30/05/16 02:46, Bill Fischofer wrote: > > diff --git a/configure.ac <http://configure.ac> b/configure.ac > <http://configure.ac> > index 99772a1..199a6e2 100644 > --- a/configure.ac <http://configure.ac> > +++ b/configure.ac <http://configure.ac> > @@ -141,6 +141,13 @@ AM_CONDITIONAL([test_helper], [test > x$test_helper = xyes ]) > AM_CONDITIONAL([HAVE_DOXYGEN], [test "x${DOXYGEN}" = "xdoxygen"]) > AM_CONDITIONAL([user_guide], [test "x${user_guides}" = "xyes" ]) > AM_CONDITIONAL([HAVE_MSCGEN], [test "x${MSCGEN}" = "xmscgen"]) > +if test x$enable_shared != xyes; > +then > + INLINES="INLINES" > +else > + INLINES="__NOTHING" > +fi > +AC_SUBST(INLINES) > > ########################################################################## > # Setup doxygen documentation > diff --git a/platform/linux-dpdk/Makefile.am > b/platform/linux-dpdk/Makefile.am > index eadaf63..49ad877 100644 > --- a/platform/linux-dpdk/Makefile.am > +++ b/platform/linux-dpdk/Makefile.am > @@ -47,8 +47,10 @@ odpapiinclude_HEADERS = \ > $(srcdir)/include/odp/api/hash.h \ > $(srcdir)/include/odp/api/hints.h \ > $(srcdir)/include/odp/api/init.h \ > + $(srcdir)/include/odp/api/inlines.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/inlines.h.in > <http://inlines.h.in> > b/platform/linux-dpdk/include/odp/api/inlines.h.in > <http://inlines.h.in> > new file mode 100644 > index 0000000..1e8bac8 > --- /dev/null > +++ b/platform/linux-dpdk/include/odp/api/inlines.h.in > <http://inlines.h.in> > @@ -0,0 +1,27 @@ > +/* Copyright (c) 2016, Linaro Limited > + * All rights reserved. > + * > + * SPDX-License-Identifier: BSD-3-Clause > + */ > + > +/** > + * @file > + * > + * ODP packet inline functions > + */ > I just noticed, this comment is also wrong. > + > +#ifndef ODP_PLAT_INLINES_H_ > +#define ODP_PLAT_INLINES_H_ > + > +#ifdef __cplusplus > +extern "C" { > +#endif > + > +#define @INLINES@ > > > Why the @ symbols here? Also, as an ODP configuration option, > shouldn't this be something like ODP_INLINES to avoid potential name > space issues? AC_SUBST(INLINES) should replace @INLINES@ to the value determined in the configure script. That's why platform/linux-dpdk/m4/configure.m4 includes this header file as well. Yes, we can prepend it with "ODP_", but I would add an underscore to the beginning, because this supposed to be an internal configuration variable, applications are not supposed to use it directly. But they include the generated inlines.h, so when they get to packet_inlines.h, they'll know whether they should inline the functions or not. Regards, Zoli
I've merged this patch as it was required for 1.10 to work on ODP-DPDK: https://git.linaro.org/lng/odp-dpdk.git/commitdiff/968237b592a8162041a4edf0091575fd1390d647 The changes I made: - packet_flags also handled - now it called _ODP_INLINES On 30/05/16 11:03, Zoltan Kiss wrote: > Hi, > > On 30/05/16 02:46, Bill Fischofer wrote: >> >> diff --git a/configure.ac <http://configure.ac> b/configure.ac >> <http://configure.ac> >> index 99772a1..199a6e2 100644 >> --- a/configure.ac <http://configure.ac> >> +++ b/configure.ac <http://configure.ac> >> @@ -141,6 +141,13 @@ AM_CONDITIONAL([test_helper], [test >> x$test_helper = xyes ]) >> AM_CONDITIONAL([HAVE_DOXYGEN], [test "x${DOXYGEN}" = "xdoxygen"]) >> AM_CONDITIONAL([user_guide], [test "x${user_guides}" = "xyes" ]) >> AM_CONDITIONAL([HAVE_MSCGEN], [test "x${MSCGEN}" = "xmscgen"]) >> +if test x$enable_shared != xyes; >> +then >> + INLINES="INLINES" >> +else >> + INLINES="__NOTHING" >> +fi >> +AC_SUBST(INLINES) >> >> ########################################################################## >> # Setup doxygen documentation >> diff --git a/platform/linux-dpdk/Makefile.am >> b/platform/linux-dpdk/Makefile.am >> index eadaf63..49ad877 100644 >> --- a/platform/linux-dpdk/Makefile.am >> +++ b/platform/linux-dpdk/Makefile.am >> @@ -47,8 +47,10 @@ odpapiinclude_HEADERS = \ >> $(srcdir)/include/odp/api/hash.h \ >> $(srcdir)/include/odp/api/hints.h \ >> $(srcdir)/include/odp/api/init.h \ >> + $(srcdir)/include/odp/api/inlines.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/inlines.h.in >> <http://inlines.h.in> >> b/platform/linux-dpdk/include/odp/api/inlines.h.in >> <http://inlines.h.in> >> new file mode 100644 >> index 0000000..1e8bac8 >> --- /dev/null >> +++ b/platform/linux-dpdk/include/odp/api/inlines.h.in >> <http://inlines.h.in> >> @@ -0,0 +1,27 @@ >> +/* Copyright (c) 2016, Linaro Limited >> + * All rights reserved. >> + * >> + * SPDX-License-Identifier: BSD-3-Clause >> + */ >> + >> +/** >> + * @file >> + * >> + * ODP packet inline functions >> + */ >> > > I just noticed, this comment is also wrong. > >> + >> +#ifndef ODP_PLAT_INLINES_H_ >> +#define ODP_PLAT_INLINES_H_ >> + >> +#ifdef __cplusplus >> +extern "C" { >> +#endif >> + >> +#define @INLINES@ >> >> >> Why the @ symbols here? Also, as an ODP configuration option, >> shouldn't this be something like ODP_INLINES to avoid potential name >> space issues? > > AC_SUBST(INLINES) should replace @INLINES@ to the value determined in > the configure script. That's why platform/linux-dpdk/m4/configure.m4 > includes this header file as well. > Yes, we can prepend it with "ODP_", but I would add an underscore to the > beginning, because this supposed to be an internal configuration > variable, applications are not supposed to use it directly. But they > include the generated inlines.h, so when they get to packet_inlines.h, > they'll know whether they should inline the functions or not. > > Regards, > > Zoli
diff --git a/.gitignore b/.gitignore index 2b1da19..4040e2b 100644 --- a/.gitignore +++ b/.gitignore @@ -40,5 +40,6 @@ ltmain.sh m4/*.m4 missing pkgconfig/libodp*.pc +platform/linux-dpdk/include/odp/api/inlines.h tags test-driver diff --git a/configure.ac b/configure.ac index 99772a1..199a6e2 100644 --- a/configure.ac +++ b/configure.ac @@ -141,6 +141,13 @@ AM_CONDITIONAL([test_helper], [test x$test_helper = xyes ]) AM_CONDITIONAL([HAVE_DOXYGEN], [test "x${DOXYGEN}" = "xdoxygen"]) AM_CONDITIONAL([user_guide], [test "x${user_guides}" = "xyes" ]) AM_CONDITIONAL([HAVE_MSCGEN], [test "x${MSCGEN}" = "xmscgen"]) +if test x$enable_shared != xyes; +then + INLINES="INLINES" +else + INLINES="__NOTHING" +fi +AC_SUBST(INLINES) ########################################################################## # Setup doxygen documentation diff --git a/platform/linux-dpdk/Makefile.am b/platform/linux-dpdk/Makefile.am index eadaf63..49ad877 100644 --- a/platform/linux-dpdk/Makefile.am +++ b/platform/linux-dpdk/Makefile.am @@ -47,8 +47,10 @@ odpapiinclude_HEADERS = \ $(srcdir)/include/odp/api/hash.h \ $(srcdir)/include/odp/api/hints.h \ $(srcdir)/include/odp/api/init.h \ + $(srcdir)/include/odp/api/inlines.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/inlines.h.in b/platform/linux-dpdk/include/odp/api/inlines.h.in new file mode 100644 index 0000000..1e8bac8 --- /dev/null +++ b/platform/linux-dpdk/include/odp/api/inlines.h.in @@ -0,0 +1,27 @@ +/* Copyright (c) 2016, Linaro Limited + * All rights reserved. + * + * SPDX-License-Identifier: BSD-3-Clause + */ + +/** + * @file + * + * ODP packet inline functions + */ + +#ifndef ODP_PLAT_INLINES_H_ +#define ODP_PLAT_INLINES_H_ + +#ifdef __cplusplus +extern "C" { +#endif + +#define @INLINES@ + +#ifdef __cplusplus +} +#endif + + +#endif /* ODP_PLAT_INLINES_H_ */ diff --git a/platform/linux-dpdk/include/odp/api/packet.h b/platform/linux-dpdk/include/odp/api/packet.h index 7c0db32..0fb31e3 100644 --- a/platform/linux-dpdk/include/odp/api/packet.h +++ b/platform/linux-dpdk/include/odp/api/packet.h @@ -27,62 +27,10 @@ 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; -} - +#include <odp/api/inlines.h> +#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..569090b --- /dev/null +++ b/platform/linux-dpdk/include/odp/api/packet_inlines.h @@ -0,0 +1,87 @@ +/* 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/m4/configure.m4 b/platform/linux-dpdk/m4/configure.m4 index fab0f6a..7c5ccbc 100644 --- a/platform/linux-dpdk/m4/configure.m4 +++ b/platform/linux-dpdk/m4/configure.m4 @@ -47,4 +47,5 @@ m4_include([platform/linux-dpdk/m4/odp_openssl.m4]) AC_CONFIG_FILES([platform/linux-dpdk/Makefile platform/linux-dpdk/test/Makefile - platform/linux-dpdk/test/pktio/Makefile]) + platform/linux-dpdk/test/pktio/Makefile + platform/linux-dpdk/include/odp/api/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 proper packaging and dynamic linking we need a fixed ABI. For that, each function has to be in the library file. But that hurts performance a bit, as small accessor functions couldn't be inlined, despite it's a viable option with static linking. This patch lets the platform automatically decide what should be done. ./configure generates inlines.h based on whether --enable-shared=yes was added or not. If yes, it enables the INLINES macro. 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 INLINES defined, the accessors appear as static inline functions in the header, and the application can directly inline them. Signed-off-by: Zoltan Kiss <zoltan.kiss@linaro.org> --- .gitignore | 1 + configure.ac | 7 ++ platform/linux-dpdk/Makefile.am | 2 + platform/linux-dpdk/include/odp/api/inlines.h.in | 27 +++++++ platform/linux-dpdk/include/odp/api/packet.h | 60 +-------------- .../linux-dpdk/include/odp/api/packet_inlines.h | 87 ++++++++++++++++++++++ platform/linux-dpdk/m4/configure.m4 | 3 +- platform/linux-dpdk/odp_packet.c | 3 + 8 files changed, 133 insertions(+), 57 deletions(-) create mode 100644 platform/linux-dpdk/include/odp/api/inlines.h.in create mode 100644 platform/linux-dpdk/include/odp/api/packet_inlines.h