| Message ID | 20170412220602.9079-1-bill.fischofer@linaro.org |
|---|---|
| State | Superseded |
| Headers | show |
On 13 April 2017 at 00:06, Bill Fischofer <bill.fischofer@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.fischofer@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 > >
Thanks. v2 sent. On Thu, Apr 13, 2017 at 3:32 AM, Krishna Garapati <balakrishna.garapati@linaro.org> wrote: > > > On 13 April 2017 at 00:06, Bill Fischofer <bill.fischofer@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.fischofer@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 >> >
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 +/** @internal Inline function @param seg @return */ +static inline int _odp_packet_seg_to_ndx(odp_packet_seg_t seg) +{ + return _odp_typeval(seg); +} + +/** @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)); } /*
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.fischofer@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(-) -- 2.9.3