Thanks. v2 sent.
On Thu, Apr 13, 2017 at 3:32 AM, Krishna Garapati <balakrishna.garap...@linaro.org> wrote: > > > On 13 April 2017 at 00:06, Bill Fischofer <bill.fischo...@linaro.org> wrote: >> >> When running in --enable-abi-compat=yes mode, all ODP types need to be >> of pointer width in the default ABI definition. The optimization of the >> odp_packet_seg_t type to uint8_t can only be supported when running in >> --enable-abi-compate=no mode. Change the ODP packet routines to use >> type converter routines that have varying definitions based on whether >> we're running in ABI compatibility mode and provide these variant >> definitions to enable proper ABI compatibility while still supporting an >> optimized typedef for non-ABI mode. >> >> This resolves Bug https://bugs.linaro.org/show_bug.cgi?id=2940 >> >> Signed-off-by: Bill Fischofer <bill.fischo...@linaro.org> >> --- >> include/odp/arch/default/api/abi/packet.h | 7 +++++-- >> .../include/odp/api/plat/packet_inlines.h | 21 >> ++++++++++++++++++--- >> .../include/odp/api/plat/packet_types.h | 10 ++++++++++ >> platform/linux-generic/odp_packet.c | 12 +++++++----- >> 4 files changed, 40 insertions(+), 10 deletions(-) >> >> diff --git a/include/odp/arch/default/api/abi/packet.h >> b/include/odp/arch/default/api/abi/packet.h >> index 60a41b8..4aac75b 100644 >> --- a/include/odp/arch/default/api/abi/packet.h >> +++ b/include/odp/arch/default/api/abi/packet.h >> @@ -16,15 +16,18 @@ extern "C" { >> /** @internal Dummy type for strong typing */ >> typedef struct { char dummy; /**< @internal Dummy */ } _odp_abi_packet_t; >> >> +/** @internal Dummy type for strong typing */ >> +typedef struct { char dummy; /**< *internal Dummy */ } >> _odp_abi_packet_seg_t; >> + >> /** @ingroup odp_packet >> * @{ >> */ >> >> typedef _odp_abi_packet_t *odp_packet_t; >> -typedef uint8_t odp_packet_seg_t; >> +typedef _odp_abi_packet_seg_t *odp_packet_seg_t; >> >> #define ODP_PACKET_INVALID ((odp_packet_t)0xffffffff) >> -#define ODP_PACKET_SEG_INVALID ((odp_packet_seg_t)-1) >> +#define ODP_PACKET_SEG_INVALID ((odp_packet_seg_t)0xffffffff) >> #define ODP_PACKET_OFFSET_INVALID (0x0fffffff) >> >> typedef enum { >> diff --git a/platform/linux-generic/include/odp/api/plat/packet_inlines.h >> b/platform/linux-generic/include/odp/api/plat/packet_inlines.h >> index eb36aa9..bd735c1 100644 >> --- a/platform/linux-generic/include/odp/api/plat/packet_inlines.h >> +++ b/platform/linux-generic/include/odp/api/plat/packet_inlines.h >> @@ -21,6 +21,20 @@ >> /** @internal Inline function offsets */ >> extern const _odp_packet_inline_offset_t _odp_packet_inline; >> >> +#if ODP_ABI_COMPAT > > Everywhere it explicitly says ODP_ABI_COMPAT == 1 or 0 except here. Would be > good to see it here the same way. >> >> +/** @internal Inline function @param seg @return */ >> +static inline int _odp_packet_seg_to_ndx(odp_packet_seg_t seg) > > is "_ndx" a typo instead of _idx ?. >> >> +{ >> + return _odp_typeval(seg); > > odp_typeval converts "seg" to uint32_t and we return it as int. >> >> +} >> + >> +/** @internal Inline function @param ndx @return */ >> +static inline odp_packet_seg_t _odp_packet_seg_from_ndx(int ndx) >> +{ >> + return _odp_cast_scalar(odp_packet_seg_t, ndx); >> +} >> +#endif >> + >> /** @internal Inline function @param pkt @return */ >> static inline void *_odp_packet_data(odp_packet_t pkt) >> { >> @@ -128,20 +142,21 @@ static inline odp_packet_seg_t >> _odp_packet_first_seg(odp_packet_t pkt) >> { >> (void)pkt; >> >> - return 0; >> + return _odp_packet_seg_from_ndx(0); >> } >> >> /** @internal Inline function @param pkt @return */ >> static inline odp_packet_seg_t _odp_packet_last_seg(odp_packet_t pkt) >> { >> - return _odp_packet_num_segs(pkt) - 1; >> + return _odp_packet_seg_from_ndx(_odp_packet_num_segs(pkt) - 1); >> } >> >> /** @internal Inline function @param pkt @param seg @return */ >> static inline odp_packet_seg_t _odp_packet_next_seg(odp_packet_t pkt, >> odp_packet_seg_t seg) >> { >> - if (odp_unlikely(seg >= _odp_packet_last_seg(pkt))) >> + if (odp_unlikely(_odp_packet_seg_to_ndx(seg) >= >> + >> _odp_packet_seg_to_ndx(_odp_packet_last_seg(pkt)))) >> return ODP_PACKET_SEG_INVALID; >> >> return seg + 1; >> diff --git a/platform/linux-generic/include/odp/api/plat/packet_types.h >> b/platform/linux-generic/include/odp/api/plat/packet_types.h >> index b8f665d..59e8b2f 100644 >> --- a/platform/linux-generic/include/odp/api/plat/packet_types.h >> +++ b/platform/linux-generic/include/odp/api/plat/packet_types.h >> @@ -40,6 +40,16 @@ typedef ODP_HANDLE_T(odp_packet_t); >> >> typedef uint8_t odp_packet_seg_t; >> >> +static inline int _odp_packet_seg_to_ndx(odp_packet_seg_t seg) >> +{ >> + return (int)seg; >> +} >> + >> +static inline odp_packet_seg_t _odp_packet_seg_from_ndx(int ndx) >> +{ >> + return (odp_packet_seg_t)ndx; >> +} >> + >> #define ODP_PACKET_SEG_INVALID ((odp_packet_seg_t)-1) >> >> typedef enum { >> diff --git a/platform/linux-generic/odp_packet.c >> b/platform/linux-generic/odp_packet.c >> index b8aac6b..e99e8b8 100644 >> --- a/platform/linux-generic/odp_packet.c >> +++ b/platform/linux-generic/odp_packet.c >> @@ -1185,7 +1185,7 @@ void *odp_packet_offset(odp_packet_t pkt, uint32_t >> offset, uint32_t *len, >> void *addr = packet_map(pkt_hdr, offset, len, &seg_idx); >> >> if (addr != NULL && seg != NULL) >> - *seg = seg_idx; >> + *seg = _odp_packet_seg_from_ndx(seg_idx); >> >> return addr; >> } >> @@ -1326,20 +1326,22 @@ void *odp_packet_seg_data(odp_packet_t pkt, >> odp_packet_seg_t seg) >> { >> odp_packet_hdr_t *pkt_hdr = packet_hdr(pkt); >> >> - if (odp_unlikely(seg >= pkt_hdr->buf_hdr.segcount)) >> + if (odp_unlikely(_odp_packet_seg_to_ndx(seg) >= >> + pkt_hdr->buf_hdr.segcount)) >> return NULL; >> >> - return packet_seg_data(pkt_hdr, seg); >> + return packet_seg_data(pkt_hdr, _odp_packet_seg_to_ndx(seg)); >> } >> >> uint32_t odp_packet_seg_data_len(odp_packet_t pkt, odp_packet_seg_t seg) >> { >> odp_packet_hdr_t *pkt_hdr = packet_hdr(pkt); >> >> - if (odp_unlikely(seg >= pkt_hdr->buf_hdr.segcount)) >> + if (odp_unlikely(_odp_packet_seg_to_ndx(seg) >= >> + pkt_hdr->buf_hdr.segcount)) >> return 0; >> >> - return packet_seg_len(pkt_hdr, seg); >> + return packet_seg_len(pkt_hdr, _odp_packet_seg_to_ndx(seg)); > > packet_seg_len expects seg_idx as uint32_t but the seg_to_ndx returns int > type. > > /Krishna >> >> } >> >> /* >> -- >> 2.9.3 >> >
