Thanks.

On Thu, Apr 13, 2017 at 7:05 AM, Krishna Garapati
<balakrishna.garap...@linaro.org> wrote:
> Reviewed-by: Balakrishna Garapati <balakrishna.garap...@linaro.org>
>
> /Krishna
>
> On 13 April 2017 at 13:48, 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

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.garap...@linaro.org>
>> Signed-off-by: Bill Fischofer <bill.fischo...@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
>>
>

Reply via email to