> -----Original Message-----
> From: lng-odp-boun...@lists.linaro.org [mailto:lng-odp-
> boun...@lists.linaro.org] On Behalf Of ext Taras Kondratiuk
> Sent: Tuesday, November 04, 2014 7:28 PM
> To: Ciprian Barbu; lng-odp@lists.linaro.org
> Subject: Re: [lng-odp] [RFC PATCH] Platform specific definitions and
> inlines
> 
> On 11/04/2014 04:29 PM, Ciprian Barbu wrote:
> > Here is a first attempt to remove inlines from API files. I also moved
> some
> > defines and typedefs out of some of the API headers. There is still room
> for
> > improvement I guess, the inline implementation and defines could be
> split into
> > their own files, but I just tried to show the general idea.
> >
> > I also tried to make the documentation look good, the platform inlines
> reference
> > the documentation in the corresponding API file.
> >
> > I removed almost all of the individual API includes from C files,
> keeping only
> > odp.h, which I remember was desirable. This can still be discussed.
> >
> > ---
> >   example/ipsec/odp_ipsec.c                          |   2 -
> >   example/ipsec/odp_ipsec_cache.c                    |   2 -
> >   example/ipsec/odp_ipsec_fwd_db.c                   |   1 -
> >   example/ipsec/odp_ipsec_loop_db.c                  |   2 -
> >   example/ipsec/odp_ipsec_sa_db.c                    |   2 -
> >   example/ipsec/odp_ipsec_sp_db.c                    |   2 -
> >   example/ipsec/odp_ipsec_stream.c                   |   3 -
> >   helper/include/odph_eth.h                          |   4 -
> >   helper/include/odph_icmp.h                         |   3 -
> >   helper/include/odph_ip.h                           |   3 -
> >   helper/include/odph_ipsec.h                        |   4 -
> >   helper/include/odph_udp.h                          |   3 -
> >   platform/linux-generic/include/api/odp.h           |   6 +
> >   platform/linux-generic/include/api/odp_atomic.h    | 282 ++++-------
> >   platform/linux-generic/include/api/odp_byteorder.h | 187 ++------
> >   platform/linux-generic/include/api/odp_coremask.h  |  47 +-
> >   .../include/api/odp_platform_defines.h             | 152 ++++++
> >   .../include/api/odp_platform_inlines.h             | 527
> +++++++++++++++++++++
> >   platform/linux-generic/include/api/odp_version.h   |  44 +-
> >   platform/linux-generic/odp_barrier.c               |   3 +-
> >   platform/linux-generic/odp_buffer.c                |   2 +-
> >   platform/linux-generic/odp_buffer_pool.c           |   8 +-
> >   platform/linux-generic/odp_coremask.c              |   3 +-
> >   platform/linux-generic/odp_crypto.c                |   9 +-
> >   platform/linux-generic/odp_packet.c                |   4 +-
> >   platform/linux-generic/odp_packet_flags.c          |   2 +-
> >   platform/linux-generic/odp_packet_io.c             |  11 +-
> >   platform/linux-generic/odp_packet_socket.c         |   2 +-
> >   platform/linux-generic/odp_queue.c                 |  10 +-
> >   platform/linux-generic/odp_ring.c                  |   6 +-
> >   platform/linux-generic/odp_rwlock.c                |   3 +-
> >   platform/linux-generic/odp_schedule.c              |  11 +-
> >   platform/linux-generic/odp_thread.c                |   7 +-
> >   platform/linux-generic/odp_ticketlock.c            |   4 +-
> >   platform/linux-generic/odp_timer.c                 |   7 +-
> >   35 files changed, 849 insertions(+), 519 deletions(-)
> >   create mode 100644 platform/linux-
> generic/include/api/odp_platform_defines.h
> >   create mode 100644 platform/linux-
> generic/include/api/odp_platform_inlines.h
> 
> Having all 'inlines' in one file isn't a good idea. I assume most of
> fast path APIs will be there as wrappers to vendor's SDK.
> This approach will also face the same issues I've faced in my previous
> try.
> 
> Current ODP headers contains following parts in each file:
> 
> 1. Platform-specific
> 1.a. Handle typedefs: e.g. odp_queue_t
> 1.b. Macros, enums:   e.g. ODP_SCHED_PRIO_NORMAL
> 
> 2. Platform-independent
> 2.a. Data structures, typedefs: e.g. odp_queue_param_t
> 2.b. Function prototypes:       e.g. odp_queue_create()
> 
> Splitting them into platform-specific and platform-independent
> parts looks straight forward until one wants to implement some API as
> inline but that API needs a platform-independent typedef (2.a).
> According to current C standards 'static inline' implementation must be
> ahead of its prototype if prototype doesn't have 'static inline'. This
> is our case.
> If (2.a) and (2.b) bundled together, then there is no way for
> implementation to get only typedef (2.a), implement function and then
> include API prototype (2.b) to pull-in documentation and verification
> of the function prototype.
> 
> For now I see two ways to solve it:
> 1. ODP provides (2.a) and (2.b) as two separate include files, so
>     implementation can use them as it wish. E.g. odp_queue_struct.h and
>     odp_queue_func.h
> 2. (2.a) and (2.b) are in the same file, but there is a platform include
>     hook between them. Similarly to plat/odp_*.h approach in my last
>     series on this topic.
> 
> Unfortunately both approaches has its own pros and cons.
> 


Ciprian, your patch is not done against the tip of linux-generic. E.g. 
example/ipsec/odp_ipsec.c does not currently include other API headers than  
odp.h (and helpers).

I suggest that we keep the reference (linux-generic/include/api) simple. I'd 
remove all inlines from there and make sure that the documentation is only 
showing the normative API (not the implementation). Then every implementation 
would solve the reuse problem as it wishes.

It's much more important that reference API definition and documentation are 
clean/clear, than provide some level of (speculative) help for implementation 
reuse.

-Petri



 

_______________________________________________
lng-odp mailing list
lng-odp@lists.linaro.org
http://lists.linaro.org/mailman/listinfo/lng-odp

Reply via email to