On Wed, Nov 5, 2014 at 11:02 AM, Savolainen, Petri (NSN - FI/Espoo) <petri.savolai...@nsn.com> wrote: > > >> -----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 can always spend a bit more time and rebase it to latest version, I think I used an old version. But it seems we wont be actually going forward with this thread. > > 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
