| Message ID | 20170413114833.29648-1-bill.fischofer@linaro.org |
|---|---|
| State | Accepted |
| Commit | 8c0df898b6f90564037fba43b1e9e6224e4ee740 |
| Headers | show |
Reviewed-by: Balakrishna Garapati <balakrishna.garapati@linaro.org> /Krishna On 13 April 2017 at 13:48, 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 > > Reported-by: Krishna Garapati <balakrishna.garapati@linaro.org> > Signed-off-by: Bill Fischofer <bill.fischofer@linaro.org> > --- > v2: > - Incorporate changes suggested by Krishna > - Added Reported-by attribution > > 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..3dd643f 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 == 1 > +/** @internal Inline function @param seg @return */ > +static inline uint32_t _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(uint32_t 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..7e3c51e 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 uint8_t _odp_packet_seg_to_ndx(odp_packet_seg_t seg) > +{ > + return (uint8_t)seg; > +} > + > +static inline odp_packet_seg_t _odp_packet_seg_from_ndx(uint8_t 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)); > } > > /* > -- > 2.9.3 > >
Thanks. On Thu, Apr 13, 2017 at 7:05 AM, Krishna Garapati <balakrishna.garapati@linaro.org> wrote: > Reviewed-by: Balakrishna Garapati <balakrishna.garapati@linaro.org> > > /Krishna > > On 13 April 2017 at 13:48, 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 Maxim: Just noticed this typo (should be --enable-abi-compat=no). Can this be fixed in merge? If you'd prefer I'll send a v3. >> 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 >> >> Reported-by: Krishna Garapati <balakrishna.garapati@linaro.org> >> Signed-off-by: Bill Fischofer <bill.fischofer@linaro.org> >> --- >> v2: >> - Incorporate changes suggested by Krishna >> - Added Reported-by attribution >> >> 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..3dd643f 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 == 1 >> +/** @internal Inline function @param seg @return */ >> +static inline uint32_t _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(uint32_t 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..7e3c51e 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 uint8_t _odp_packet_seg_to_ndx(odp_packet_seg_t seg) >> +{ >> + return (uint8_t)seg; >> +} >> + >> +static inline odp_packet_seg_t _odp_packet_seg_from_ndx(uint8_t 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)); >> } >> >> /* >> -- >> 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..3dd643f 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 == 1 +/** @internal Inline function @param seg @return */ +static inline uint32_t _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(uint32_t 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..7e3c51e 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 uint8_t _odp_packet_seg_to_ndx(odp_packet_seg_t seg) +{ + return (uint8_t)seg; +} + +static inline odp_packet_seg_t _odp_packet_seg_from_ndx(uint8_t 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 Reported-by: Krishna Garapati <balakrishna.garapati@linaro.org> Signed-off-by: Bill Fischofer <bill.fischofer@linaro.org> --- v2: - Incorporate changes suggested by Krishna - Added Reported-by attribution 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