On 05/14/2015 13:45, Mike Holmes wrote:
On 14 May 2015 at 06:31, Maxim Uvarov <maxim.uva...@linaro.org
<mailto:maxim.uva...@linaro.org>> wrote:
On 05/14/2015 01:17, Mike Holmes wrote:
On 13 May 2015 at 05:03, Maxim Uvarov <maxim.uva...@linaro.org
<mailto:maxim.uva...@linaro.org>
<mailto:maxim.uva...@linaro.org
<mailto:maxim.uva...@linaro.org>>> wrote:
On 05/12/2015 20:52, Mike Holmes wrote:
I dont think you can implement a mandatory ODP API
based on an
optional helper api.
The ring implementation should be inside the linux-generic
implementation if you want to use it, other
implementations
can copy it as they do now from linux-generic for other
features such as timers.
Helpers need to be supporting the tests, examples and
applications and not the implementations.
We have been walking a fuzzy line previously but I
think the
complexities and coupling get unwound when
linux-generic does
not rely on or compile in any helper code.
Mike
I think it depends if someone else uses odp ring from
helper or
not. Ring itself might be useful as middle buffer between
processes. Also I just added shm flag which is in our api
to have
smaller changes. I think later we can decided where it's
better to
move odp ring.
I dont agree that we can decide later. The test
infrastructure that we decided to punt on to get 1.0 done is
proving very difficult to fix now that it has established a
pattern of use that is coupled to the platform is awkward
ways, lets not do the same with helpers.
Basing an implementation on optional code is not very robust,
the makefile looks wrong with ../../ (below)
we have the same for linux.c and it works. That looks good for me.
Don't know why you don't like ../../
I object strongly to linux.c why is that built into the implementation
when it is not an ODP API and is not in any way used by the
implementation of linux-genric? It is optional support for tests, it
makes no sense - this is exactly what I am trying to undo.
Ok, now looks like I understand. Looks like we wrongly compiled linux.c
inside linux-generic implementation. linux-generic does not use any
helper/linux.c functions. So that tests have to compile that helper on
their build.
Send a quick patch for that.
, we have made helpers not optional in this way. We compile in
the now not optional ring.c and we dont test it or use it in
any way at all! that can't be correct.
We compile it and test:
./test/api_test/odp_ring_test.c
__LIB__libodp_la_SOURCES = \
>------->------->------- odp_barrier.c \
>------->------->------- odp_buffer.c \
>------->------->------- odp_classification.c \
>------->------->------- odp_cpumask.c \
>------->------->------- odp_crypto.c \
>------->------->------- odp_errno.c \
>------->------->------- odp_event.c \
>------->------->------- odp_init.c \
>------->------->------- odp_impl.c \
>------->------->------- ../../helper/linux.c \
>------->------->------- odp_packet.c \
>------->------->------- odp_packet_flags.c \
>------->------->------- odp_packet_io.c \
>------->------->------- odp_packet_socket.c \
>------->------->------- odp_pool.c \
>------->------->------- odp_queue.c \
>------->------->------- ../../helper/ring.c \
To share implementations, thus far KS2 and DPDK have their
make file include the src from linux-generic as appropriate,
so they will reuse ring & IPC from there and all re-use is
then uniformly handled.
Both KS2 and DPDK do not need odph_ring and linux-generic IPC for
IPC support. IPC there will be implemented in different way. But
if needed they can use odph_ring for something else
Sure, I have no objection to reusing the code, but I dont agree to
doing so by including it from the helper directory.
I do not see nothing bad in including them. But that include has to be
reasonable. In case of IPC it is reasonable (ring.c). In case of linux.c
it's not reasonable due to implementation even do not use that functions.
Let start by being clear what the helpers are before we start
coupling them even more tightly with IPC.
ok.
Previously stated and presumed still true is this description
"Helpers are there to help tests, examples and applications,
not implementations. Helpers are optional for applications"
we need to establish this as fact before deciding much more.
If helpers are not separate support code from the
implementation then we need to state that all implementations
should compile them all in so that customers of vendor
implementations can run the validation tests.
I think that implementation should not be limitation here. If any
implementation wish to use helper it's free to do it.
I would say that "Helpers are functions which are optional for
applications. That function is very often used in ODP applications
but they are not separate ODP API.
If they are not separate why are they not in the ODP API ? We say very
clearly that there are no optional odp apis yet here we are coding
exactly that so lets add them to the API or not.
Because they are optional and you can create application without that
functions. I.e. use systems one or create your custom function. Most of
that function can not be HW offloaded so no need
to put them to separate API.
These functions are not hided inside implementation so that apps
can use them directly. "
Maxim.
Mike
Maxim.
On 8 May 2015 at 05:57, Maxim Uvarov
<maxim.uva...@linaro.org <mailto:maxim.uva...@linaro.org>
<mailto:maxim.uva...@linaro.org
<mailto:maxim.uva...@linaro.org>>
<mailto:maxim.uva...@linaro.org
<mailto:maxim.uva...@linaro.org>
<mailto:maxim.uva...@linaro.org
<mailto:maxim.uva...@linaro.org>>>> wrote:
Signed-off-by: Maxim Uvarov
<maxim.uva...@linaro.org <mailto:maxim.uva...@linaro.org>
<mailto:maxim.uva...@linaro.org
<mailto:maxim.uva...@linaro.org>>
<mailto:maxim.uva...@linaro.org
<mailto:maxim.uva...@linaro.org>
<mailto:maxim.uva...@linaro.org
<mailto:maxim.uva...@linaro.org>>>>
---
helper/include/odp/helper/ring.h | 2 ++
helper/ring.c | 9 ++++++++-
2 files changed, 10 insertions(+), 1 deletion(-)
diff --git a/helper/include/odp/helper/ring.h
b/helper/include/odp/helper/ring.h
index 65c32ad..5e640a7 100644
--- a/helper/include/odp/helper/ring.h
+++ b/helper/include/odp/helper/ring.h
@@ -158,6 +158,8 @@ typedef struct odph_ring {
#define ODPH_RING_F_SP_ENQ 0x0001 /* The default
enqueue is
"single-producer".*/
#define ODPH_RING_F_SC_DEQ 0x0002 /* The default
dequeue is
"single-consumer".*/
+#define ODPH_RING_SHM_PROC 0x0004 /* If set -
ring is visible
from different
+ processes. Default is
thread
visible. */
#define ODPH_RING_QUOT_EXCEED (1 << 31) /* Quota
exceed for
burst ops */
#define ODPH_RING_SZ_MASK (unsigned)(0x0fffffff)
/* Ring size
mask */
diff --git a/helper/ring.c b/helper/ring.c
index a24a020..0927a6c 100644
--- a/helper/ring.c
+++ b/helper/ring.c
@@ -159,8 +159,14 @@ odph_ring_create(const char
*name,
unsigned
count, unsigned flags)
char ring_name[ODPH_RING_NAMESIZE];
odph_ring_t *r;
size_t ring_size;
+ uint32_t shm_flag;
odp_shm_t shm;
+ if (flags & ODPH_RING_SHM_PROC)
+ shm_flag = ODP_SHM_PROC;
+ else
+ shm_flag = 0;
+
/* count must be a power of 2 */
if (!ODP_VAL_IS_POWER_2(count) || (count >
ODPH_RING_SZ_MASK)) {
ODP_ERR("Requested size is
invalid, must
be power
of 2, and do not exceed the size limit %u\n",
@@ -173,7 +179,8 @@ odph_ring_create(const char *name,
unsigned
count, unsigned flags)
odp_rwlock_write_lock(&qlock);
/* reserve a memory zone for this ring.*/
- shm = odp_shm_reserve(ring_name, ring_size,
ODP_CACHE_LINE_SIZE, 0);
+ shm = odp_shm_reserve(ring_name, ring_size,
ODP_CACHE_LINE_SIZE,
+ shm_flag);
r = odp_shm_addr(shm);
--
1.9.1
_______________________________________________
lng-odp mailing list
lng-odp@lists.linaro.org <mailto:lng-odp@lists.linaro.org>
<mailto:lng-odp@lists.linaro.org
<mailto:lng-odp@lists.linaro.org>>
<mailto:lng-odp@lists.linaro.org
<mailto:lng-odp@lists.linaro.org>
<mailto:lng-odp@lists.linaro.org
<mailto:lng-odp@lists.linaro.org>>>
https://lists.linaro.org/mailman/listinfo/lng-odp
-- Mike Holmes
Technical Manager - Linaro Networking Group
Linaro.org <http://www.linaro.org/>***│ *Open source
software
for ARM SoCs
--
Mike Holmes
Technical Manager - Linaro Networking Group
Linaro.org <http://www.linaro.org/>***│ *Open source software
for ARM SoCs
--
Mike Holmes
Technical Manager - Linaro Networking Group
Linaro.org <http://www.linaro.org/>***│ *Open source software for ARM SoCs
_______________________________________________
lng-odp mailing list
lng-odp@lists.linaro.org
https://lists.linaro.org/mailman/listinfo/lng-odp
