Re: [lng-odp] [PATCH] Scheduler atomic and ordered definitions
The specific case I was talking about is when a packet is dequeued from an ordered queue and it needs fragmentation as part of the processing before the next enqueue which is supposed to restore the order - for example IPsec red side fragmentation (fragmentation before encryption) or fragmentation before transmission on an interface having a MTU smaller than the packet size. The fragments of the same packet are ordered with respect to each other but they can be interleaved with other packets from the same queue or with fragments of another packet. As we have skip order function which essentially tells the order restoration logic to give up on the current expected packet in sequence and move to the next packet we also need a way to tell that a sequence of buffers share the same ordering information so the order restoration logic treats them as a whole rather as separate packets. Alex On 4 November 2014 17:15, Gilad Ben Yossef gil...@ezchip.com wrote: I don't think it can use the first fragment place in the order. It can use the last fragment to arrive place in the queue. Otherwise you have to stop forwarding until all fragments arrive to keep the order… no? *Gilad Ben-Yossef* Software Architect EZchip Technologies Ltd. 37 Israel Pollak Ave, Kiryat Gat 82025 ,Israel Tel: +972-4-959- ext. 576, Fax: +972-8-681-1483 Mobile: +972-52-826-0388, US Mobile: +1-973-826-0388 Email: gil...@ezchip.com, Web: http://www.ezchip.com *Ethernet always wins.* — Andy Bechtolsheim *From:* Alexandru Badicioiu [mailto:alexandru.badici...@linaro.org] *Sent:* Tuesday, November 04, 2014 4:13 PM *To:* Bill Fischofer *Cc:* Gilad Ben Yossef; Savolainen, Petri (NSN - FI/Espoo); lng-odp@lists.linaro.org *Subject:* Re: [lng-odp] [PATCH] Scheduler atomic and ordered definitions In addition to skip order function I think we need to handle the case when a packet gets fragmented (e.g. IP fragmentation). In this case the fragments should share the original sequence number and the order restoration logic should be aware when the last fragment occurs. Alex On 3 November 2014 13:45, Bill Fischofer bill.fischo...@linaro.org wrote: OK, thanks. That certainly matches the step-by-step semantics Petri proposed. We'll have to discuss what, if anything, more we need regarding ORDERED queues post-v1.0 next year. Bill On Mon, Nov 3, 2014 at 3:03 AM, Gilad Ben Yossef gil...@ezchip.com wrote: What NPS does in HW is that each buffer you dequeue actually stays on the queue (but is marked as consumed by the application) and you are given a tag for it. When I want to queue the buffer to the next, I give the HW the tag I got, not the buffer handle and what the HW does is only than fully dequeue the buffer from the original queue before queuing it into the new queue – but only after all preceding (older) buffers on the original queue have been fully dequeued, Free of a buffer causes a dequeue from the original queue as well. And I can also give release a buffer from the original queue (give the HW the tag back and say you don't have to wait for this buffer any longer). I hope this helps, Gilad *Gilad Ben-Yossef* Software Architect EZchip Technologies Ltd. 37 Israel Pollak Ave, Kiryat Gat 82025 ,Israel Tel: +972-4-959- ext. 576, Fax: +972-8-681-1483 Mobile: +972-52-826-0388, US Mobile: +1-973-826-0388 Email: gil...@ezchip.com, Web: http://www.ezchip.com *Ethernet always wins.* — Andy Bechtolsheim *From:* Bill Fischofer [mailto:bill.fischo...@linaro.org] *Sent:* Sunday, November 02, 2014 5:44 PM *To:* Gilad Ben Yossef *Cc:* Savolainen, Petri (NSN - FI/Espoo); lng-odp@lists.linaro.org *Subject:* Re: [lng-odp] [PATCH] Scheduler atomic and ordered definitions Thanks Gilad. Can you elaborate on that a bit more? I can understand how step-by-step would be potentially easier to implement, but does it also capture the majority of expected application use cases? A good confirmation that this is the correct approach for v1.0 though. On Sun, Nov 2, 2014 at 9:26 AM, Gilad Ben Yossef gil...@ezchip.com wrote: For what it's worth, as a SoC vendor rep. that has ordered queue in HW, Petri's definition is actually preferred for us even going forward J Thanks, Gilad *Gilad Ben-Yossef* Software Architect EZchip Technologies Ltd. 37 Israel Pollak Ave, Kiryat Gat 82025 ,Israel Tel: +972-4-959- ext. 576, Fax: +972-8-681-1483 Mobile: +972-52-826-0388, US Mobile: +1-973-826-0388 Email: gil...@ezchip.com, Web: http://www.ezchip.com *Ethernet always wins.* — Andy Bechtolsheim *From:* lng-odp-boun...@lists.linaro.org [mailto: lng-odp-boun...@lists.linaro.org] *On Behalf Of *Bill Fischofer *Sent:* Friday, October 31, 2014 2:59 PM *To:* Savolainen, Petri (NSN - FI/Espoo) *Cc:* lng-odp@lists.linaro.org *Subject:* Re: [lng-odp] [PATCH] Scheduler atomic and ordered
[lng-odp] [PATCH v2 1/1] cunit : add tests for crypto APIs
From: Alexandru Badicioiu alexandru.badici...@linaro.org
This patch adds a suite for sync and async inplace mode of crypto APIs.
Correctness of crypto operation output is verified with known test
vectors. Various options and functionalities like use session IV
or operation IV for ciphering are exercised for both modes.
For async mode there are options to use input packet buffer or a
separate buffer as the completion event and to set and retrieve the
context associated with an operation from the completion event.
Signed-off-by: Alexandru Badicioiu alexandru.badici...@linaro.org
---
configure.ac |1 +
test/cunit/Makefile.am|2 +
test/cunit/crypto/Makefile.am | 10 +
test/cunit/crypto/odp_crypto_test.c | 114
test/cunit/crypto/odp_crypto_test_async_inp.c | 371 +
test/cunit/crypto/odp_crypto_test_async_inp.h | 17 ++
test/cunit/crypto/odp_crypto_test_sync_inp.c | 260 +
test/cunit/crypto/odp_crypto_test_sync_inp.h | 17 ++
test/cunit/crypto/test_vectors.h | 94 +++
9 files changed, 886 insertions(+), 0 deletions(-)
create mode 100644 test/cunit/crypto/Makefile.am
create mode 100644 test/cunit/crypto/odp_crypto_test.c
create mode 100644 test/cunit/crypto/odp_crypto_test_async_inp.c
create mode 100644 test/cunit/crypto/odp_crypto_test_async_inp.h
create mode 100644 test/cunit/crypto/odp_crypto_test_sync_inp.c
create mode 100644 test/cunit/crypto/odp_crypto_test_sync_inp.h
create mode 100644 test/cunit/crypto/test_vectors.h
diff --git a/configure.ac b/configure.ac
index 1c061e9..298d50b 100644
--- a/configure.ac
+++ b/configure.ac
@@ -177,6 +177,7 @@ AC_CONFIG_FILES([Makefile
test/Makefile
test/api_test/Makefile
test/cunit/Makefile
+test/cunit/crypto/Makefile
pkgconfig/libodp.pc])
AC_SEARCH_LIBS([timer_create],[rt posix4])
diff --git a/test/cunit/Makefile.am b/test/cunit/Makefile.am
index 927a5a5..7611145 100644
--- a/test/cunit/Makefile.am
+++ b/test/cunit/Makefile.am
@@ -3,6 +3,8 @@ include $(top_srcdir)/test/Makefile.inc
AM_CFLAGS += -I$(CUNIT_PATH)/include
AM_LDFLAGS += -L$(CUNIT_PATH)/lib
+SUBDIRS = crypto
+
if ODP_CUNIT_ENABLED
TESTS = ${bin_PROGRAMS}
check_PROGRAMS = ${bin_PROGRAMS}
diff --git a/test/cunit/crypto/Makefile.am b/test/cunit/crypto/Makefile.am
new file mode 100644
index 000..0eb06df
--- /dev/null
+++ b/test/cunit/crypto/Makefile.am
@@ -0,0 +1,10 @@
+include $(top_srcdir)/test/Makefile.inc
+
+if ODP_CUNIT_ENABLED
+bin_PROGRAMS = odp_crypto
+odp_crypto_LDFLAGS = $(AM_LDFLAGS) -static -lcunit
+endif
+
+dist_odp_crypto_SOURCES = odp_crypto_test_async_inp.c \
+ odp_crypto_test_sync_inp.c \
+ odp_crypto_test.c
diff --git a/test/cunit/crypto/odp_crypto_test.c
b/test/cunit/crypto/odp_crypto_test.c
new file mode 100644
index 000..dbf245a
--- /dev/null
+++ b/test/cunit/crypto/odp_crypto_test.c
@@ -0,0 +1,114 @@
+/* Copyright (c) 2014, Linaro Limited
+ * All rights reserved.
+ *
+ * SPDX-License-Identifier:BSD-3-Clause
+ */
+
+#include odp.h
+#include CUnit/Headers/Basic.h
+#include CUnit/Headers/TestDB.h
+#include odp_crypto_test_async_inp.h
+#include odp_crypto_test_sync_inp.h
+
+#define SHM_PKT_POOL_SIZE (512*2048*2)
+#define SHM_PKT_POOL_BUF_SIZE (1024 * 32)
+
+#define SHM_COMPL_POOL_SIZE(128*1024)
+#define SHM_COMPL_POOL_BUF_SIZE 128
+
+
+/* Suites init/finalize funcs */
+/* ODP global/local initialization
+ * Packet/Completion event pool creation
+ * Crypto output queue creation
+*/
+
+static int init(void)
+{
+ odp_shm_t shm;
+ void *pool_base = NULL;
+ odp_buffer_pool_t pool;
+ odp_queue_t out_queue;
+
+ if (odp_init_global(NULL, NULL)) {
+ ODP_ERR(ODP global init failed.\n);
+ return -1;
+ }
+ odp_init_local();
+
+ shm = odp_shm_reserve(shm_packet_pool,
+ SHM_PKT_POOL_SIZE,
+ ODP_CACHE_LINE_SIZE, 0);
+
+ pool_base = odp_shm_addr(shm);
+ if (!pool_base) {
+ ODP_ERR(Packet pool allocation failed.\n);
+ return -1;
+ }
+
+ pool = odp_buffer_pool_create(packet_pool, pool_base,
+ SHM_PKT_POOL_SIZE,
+ SHM_PKT_POOL_BUF_SIZE,
+ ODP_CACHE_LINE_SIZE,
+ ODP_BUFFER_TYPE_PACKET);
+ if (pool == ODP_BUFFER_POOL_INVALID) {
+ ODP_ERR(Packet pool creation failed.\n);
+ /* TODO - cleanup */
+ return -1;
+ }
+ out_queue = odp_queue_create(crypto-out,
+ODP_QUEUE_TYPE_POLL, NULL);
+ if (out_queue == ODP_QUEUE_INVALID) {
+ ODP_ERR(Crypto
Re: [lng-odp] [RFC PATCH] Platform specific definitions and inlines
-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
Re: [lng-odp] [PATCH v2] odp_example.c: fix dead code path
Applied!
Maxim.
On 10/31/2014 01:17 AM, Mike Holmes wrote:
tot is = i which cannot leave the for loop without being positive
unless it exits entirely and never reaches the test statement.
Or QUEUE_ROUNDS is #defined to 0 rather than (512*1024)
Thus tot is always true and the else cannot execute
Signed-off-by: Mike Holmes mike.hol...@linaro.org
---
Previous patch missed a case.
Fixed indent
example/odp_example/odp_example.c | 9 ++---
1 file changed, 2 insertions(+), 7 deletions(-)
diff --git a/example/odp_example/odp_example.c
b/example/odp_example/odp_example.c
index 5f25f89..1ed4a0b 100644
--- a/example/odp_example/odp_example.c
+++ b/example/odp_example/odp_example.c
@@ -355,13 +355,8 @@ static int test_schedule_one_single(const char *str, int
thr,
odp_barrier_sync(barrier);
clear_sched_queues();
- if (tot) {
- cycles = cycles/tot;
- ns = ns/tot;
- } else {
- cycles = 0;
- ns = 0;
- }
+ cycles = cycles/tot;
+ ns = ns/tot;
printf( [%i] %s enq+deq %PRIu64 cycles, %PRIu64 ns\n,
thr, str, cycles, ns);
___
lng-odp mailing list
lng-odp@lists.linaro.org
http://lists.linaro.org/mailman/listinfo/lng-odp
Re: [lng-odp] [PATCH] odp_ring_test.c: free buffer on fail
Merged!
Maxim.
On 10/24/2014 11:46 PM, Mike Holmes wrote:
On data mismatch free the recently mallocked buffer.
Signed-off-by: Mike Holmes mike.hol...@linaro.org
---
test/api_test/odp_ring_test.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/test/api_test/odp_ring_test.c b/test/api_test/odp_ring_test.c
index 1da5845..eb1f301 100644
--- a/test/api_test/odp_ring_test.c
+++ b/test/api_test/odp_ring_test.c
@@ -286,6 +286,7 @@ static int consumer_fn(void)
if (i == 0) {
for (i = 0; i MAX_BULK; i++) {
if (src[i] != (void *)(unsigned long)i) {
+ free(src);
printf(data mismatch.. lockless ops
fail\n);
return -1;
}
___
lng-odp mailing list
lng-odp@lists.linaro.org
http://lists.linaro.org/mailman/listinfo/lng-odp
Re: [lng-odp] [RFC PATCH] Platform specific definitions and inlines
On 11/04/2014 07:35 PM, Bill Fischofer wrote:
If the ODP prototypes simply all said 'static inline' then would that
permit the implementation to choose later which were actually static inline?
I.e., if the common odp_xxx.h says
static inline void *odp_foo(...);
Then can the implementation can say;
static inline void *odp_foo(...) { };
to make the function inline and just say
void *odp_foo(...) { };
to make it an actual call?
Unfortunately GCC throws -Werror=unused-function on this in each place
where this header included, but no function implemented.
___
lng-odp mailing list
lng-odp@lists.linaro.org
http://lists.linaro.org/mailman/listinfo/lng-odp
Re: [lng-odp] [RFC PATCH] Platform specific definitions and inlines
On 11/04/2014 07:51 PM, Ciprian Barbu wrote: On Tue, Nov 4, 2014 at 7:27 PM, Taras Kondratiuk taras.kondrat...@linaro.org wrote: 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. This is solved in this patch by including odp_platform_defines.h and odp_platform_inlines.h in odp.h first before anything else. All the source files include only odp.h so the order is always preserved. It seems the issue is present there. Just try to implement odp_queue_create() as 'static inline' in odp_platform_inlines.h. I understand that there is no reason to inline odp_queue_create(), but this is just a good example of the issue. ___ lng-odp mailing list lng-odp@lists.linaro.org http://lists.linaro.org/mailman/listinfo/lng-odp
Re: [lng-odp] [PATCH] Removed odp_atomic_int_t
On 31 October 2014 19:09, Petri Savolainen petri.savolai...@linaro.org
wrote:
Integer version is not needed. Unsigned 32 and 64 bit atomics
are used instead. If signed 32/64 bits can be added later
instead - if signed 32/64 bits required then can be added later on need
basis.
on need basis.
Signed-off-by: Petri Savolainen petri.savolai...@linaro.org
---
platform/linux-generic/include/api/odp_atomic.h| 115
-
platform/linux-generic/include/api/odp_barrier.h | 4 +-
.../linux-generic/include/odp_buffer_internal.h| 2 +-
platform/linux-generic/odp_barrier.c | 6 +-
platform/linux-generic/odp_thread.c| 6 +-
test/api_test/odp_atomic_test.c| 80 ++
test/api_test/odp_atomic_test.h| 9 --
7 files changed, 16 insertions(+), 206 deletions(-)
diff --git a/platform/linux-generic/include/api/odp_atomic.h
b/platform/linux-generic/include/api/odp_atomic.h
index 213c81f..5c83b39 100644
--- a/platform/linux-generic/include/api/odp_atomic.h
+++ b/platform/linux-generic/include/api/odp_atomic.h
@@ -26,10 +26,6 @@ extern C {
* @{
*/
-/**
- * Atomic integer
- */
-typedef volatile int32_t odp_atomic_int_t;
/**
* Atomic unsigned integer 64 bits
@@ -43,117 +39,6 @@ typedef volatile uint32_t odp_atomic_u32_t;
/**
- * Initialize atomic integer
- *
- * @param ptrAn integer atomic variable
- *
- * @note The operation is not synchronized with other threads
- */
-static inline void odp_atomic_init_int(odp_atomic_int_t *ptr)
-{
- *ptr = 0;
-}
-
-/**
- * Load value of atomic integer
- *
- * @param ptrAn atomic variable
- *
- * @return atomic integer value
- *
- * @note The operation is not synchronized with other threads
- */
-static inline int odp_atomic_load_int(odp_atomic_int_t *ptr)
-{
- return *ptr;
-}
-
-/**
- * Store value to atomic integer
- *
- * @param ptrAn atomic variable
- * @param new_value Store new_value to a variable
- *
- * @note The operation is not synchronized with other threads
- */
-static inline void odp_atomic_store_int(odp_atomic_int_t *ptr, int
new_value)
-{
- *ptr = new_value;
-}
-
-/**
- * Fetch and add atomic integer
- *
- * @param ptrAn atomic variable
- * @param value A value to be added to the variable
- *
- * @return Value of the variable before the operation
- */
-static inline int odp_atomic_fetch_add_int(odp_atomic_int_t *ptr, int
value)
-{
- return __sync_fetch_and_add(ptr, value);
-}
-
-/**
- * Fetch and subtract atomic integer
- *
- * @param ptrAn atomic integer variable
- * @param value A value to be subtracted from the variable
- *
- * @return Value of the variable before the operation
- */
-static inline int odp_atomic_fetch_sub_int(odp_atomic_int_t *ptr, int
value)
-{
- return __sync_fetch_and_sub(ptr, value);
-}
-
-/**
- * Fetch and increment atomic integer by 1
- *
- * @param ptrAn atomic variable
- *
- * @return Value of the variable before the operation
- */
-static inline int odp_atomic_fetch_inc_int(odp_atomic_int_t *ptr)
-{
- return odp_atomic_fetch_add_int(ptr, 1);
-}
-
-/**
- * Increment atomic integer by 1
- *
- * @param ptrAn atomic variable
- *
- */
-static inline void odp_atomic_inc_int(odp_atomic_int_t *ptr)
-{
- odp_atomic_fetch_add_int(ptr, 1);
-}
-
-/**
- * Fetch and decrement atomic integer by 1
- *
- * @param ptrAn atomic int variable
- *
- * @return Value of the variable before the operation
- */
-static inline int odp_atomic_fetch_dec_int(odp_atomic_int_t *ptr)
-{
- return odp_atomic_fetch_sub_int(ptr, 1);
-}
-
-/**
- * Decrement atomic integer by 1
- *
- * @param ptrAn atomic variable
- *
- */
-static inline void odp_atomic_dec_int(odp_atomic_int_t *ptr)
-{
- odp_atomic_fetch_sub_int(ptr, 1);
-}
-
-/**
* Initialize atomic uint32
*
* @param ptrAn atomic variable
diff --git a/platform/linux-generic/include/api/odp_barrier.h
b/platform/linux-generic/include/api/odp_barrier.h
index 866648f..fb02a9d 100644
--- a/platform/linux-generic/include/api/odp_barrier.h
+++ b/platform/linux-generic/include/api/odp_barrier.h
@@ -31,8 +31,8 @@ extern C {
* ODP execution barrier
*/
typedef struct odp_barrier_t {
- int count; /** @private Thread count */
- odp_atomic_int_t bar;/** @private Barrier counter */
+ uint32_t count; /** @private Thread count */
+ odp_atomic_u32_t bar;/** @private Barrier counter */
} odp_barrier_t;
diff --git a/platform/linux-generic/include/odp_buffer_internal.h
b/platform/linux-generic/include/odp_buffer_internal.h
index 2002b51..0027bfc 100644
--- a/platform/linux-generic/include/odp_buffer_internal.h
+++ b/platform/linux-generic/include/odp_buffer_internal.h
@@ -88,7 +88,7 @@ typedef
Re: [lng-odp] [ODP/PATCH v2] Look ma, no barriers! C11 memory model
Hi Ola,
Below is an example of one issue I noticed.
If you would post arm counter implementation in separate
patch, I would be able to comment on all relevant places,
but with current huge patch I just give one example. Also if it
would be separate patch, we would be able to work on it in
separate thread, till we get it right. But if you would continue
to repost big whole patch with each iteration working on this
issue, it would be quite a pain for me to find out what has
changed, although I would just had one thing in mind - counter
ARM V7 implementation.
+/**
+ * Atomic add to 64-bit counter variable
+ *
+ * @param ptr Pointer to a 64-bit counter variable
+ * @param incr The value to be added to the counter variable
+ */
+static inline void odp_counter64_add(odp_counter64_t *ptr, uint64_t incr)
+{
+#if defined __arm__ /* A32/T32 ISA */
+ uint64_t old_val;
+ int status;
+ do {
+ __asm __volatile(ldrexd %0, %H0, [%2]\t\n
+adds %0, %0, %3\t\n
+adc%H0, %H3\t\n
+strexd %1, %0, %H0, [%2]
Above looks very wrong to me. Did you test that on BE
system? Please see
http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=2245f92498b216b50e744423bde17626287409d8
You should use %Q %R instead of %H.
The same issue exist other ARM V7 counter64
functions.
Thanks,
Victor
+: =r(old_val), =r(status)
+: r(ptr-v), r(incr)
+: );
+ } while (odp_unlikely(status != 0)); /* Retry until write succeeds */
+#elif defined __OCTEON__
+ __asm __volatile(saad %[inc], (%[base])
+: +m (*ptr)
+: [inc] r (incr), [base] r (ptr)
+: );
+#elif defined __x86_64__
+ /* Generates good code on x86_64 */
+ (void)__sync_fetch_and_add(ptr-v, incr);
+#else
+/* Warning odp_counter64_add() may not be efficiently implemented */
+ (void)__sync_fetch_and_add(ptr-v, incr);
+#endif
___
lng-odp mailing list
lng-odp@lists.linaro.org
http://lists.linaro.org/mailman/listinfo/lng-odp
Re: [lng-odp] [PATCH v2 1/1] linux-generic : crypto
nit:
For the subject[PATCH v2 1/1] linux-generic : crypto
I think that should be [PATCH v2] odp_crypto.c: Add stubs for missing
functions
We know it is for linux-generic because the PATCH did not have KS2, NETMAP
etc in it so no need to say linux-generic again.
When there is only one file in the series there is no need to have 1/1 or
maybe you meant this to be 1/2 ?
The more elaborate description helps define what happened in the git log
more than just saying crypto
On 5 November 2014 04:00, alexandru.badici...@linaro.org wrote:
From: Alexandru Badicioiu alexandru.badici...@linaro.org
Add missing API functions to crypto implementation.
Required by crypto unit testing.
Signed-off-by: Alexandru Badicioiu alexandru.badici...@linaro.org
---
platform/linux-generic/odp_crypto.c | 22 ++
1 files changed, 22 insertions(+), 0 deletions(-)
diff --git a/platform/linux-generic/odp_crypto.c
b/platform/linux-generic/odp_crypto.c
index 1475437..596c717 100644
--- a/platform/linux-generic/odp_crypto.c
+++ b/platform/linux-generic/odp_crypto.c
@@ -13,6 +13,7 @@
#include odp_align.h
#include odp_shared_memory.h
#include odp_crypto_internal.h
+#include odp_debug_internal.h
#include odp_hints.h
#include odph_packet.h
@@ -467,3 +468,24 @@ odp_crypto_get_ses_create_compl_session(odp_buffer_t
completion_event,
result = odp_buffer_addr(completion_event);
*session = result-session;
}
+
+void
+odp_crypto_set_operation_compl_ctx(odp_buffer_t completion_event
ODP_UNUSED,
+ void *ctx ODP_UNUSED)
+{
+ ODP_UNIMPLEMENTED();
+}
+
+void
+*odp_crypto_get_operation_compl_ctx(odp_buffer_t completion_event
ODP_UNUSED)
+{
The documentation makes no mention of NULL being a possible error case, it
just says Returns user data should the documentation be improved with
this patch submission to indicate the error case NULL with a new @retval ?
+ ODP_UNIMPLEMENTED();
+ return NULL;
+}
+
+odp_packet_t
+odp_crypto_get_operation_compl_packet(odp_buffer_t completion_event
ODP_UNUSED)
The documentation makes no mention of ODP_PACKET_INVALID being a possible
error case, it just says Returns Packet structure where data now resides
should the documentation be improved with this patch submission to
indicate the error case ODP_PACKET_INVALID with a new @retval ?
+{
+ ODP_UNIMPLEMENTED();
+ return ODP_PACKET_INVALID;
+}
--
1.7.3.4
___
lng-odp mailing list
lng-odp@lists.linaro.org
http://lists.linaro.org/mailman/listinfo/lng-odp
--
*Mike Holmes*
Linaro Sr Technical Manager
LNG - ODP
___
lng-odp mailing list
lng-odp@lists.linaro.org
http://lists.linaro.org/mailman/listinfo/lng-odp
Re: [lng-odp] [PATCH ARCH] Add API guide lines
On 29 October 2014 06:48, Ciprian Barbu ciprian.ba...@linaro.org wrote:
On Fri, Oct 24, 2014 at 5:51 PM, Mike Holmes mike.hol...@linaro.org
wrote:
Add guidelines for the ODP implimenter to follow.
Signed-off-by: Mike Holmes mike.hol...@linaro.org
---
api_guide_lines.dox | 144
1 file changed, 144 insertions(+)
create mode 100644 api_guide_lines.dox
diff --git a/api_guide_lines.dox b/api_guide_lines.dox
new file mode 100644
index 000..4944947
--- /dev/null
+++ b/api_guide_lines.dox
@@ -0,0 +1,144 @@
+/* Copyright (c) 2043, Linaro Limited
+ * All rights reserved
+ *
+ * SPDX-License-Identifier: BSD-3-Clause
+ */
+
+/**
+
+@page api_guide_lines API Guide Lines
+
+@tableofcontents
+
+@section introduction Introduction
+ODP APIs are implemented as callable C functions that often return a
typed value.
+This document describes the approach to handling return values and
error indications expected of conforming ODP implementations.
+As such it should be regarded as providing guidelines for how to create
new ODP APIs.
+
+@section functional Functional Definition
+This section defines the use of data types, calling conventions, and
return codes used by ODP APIs.
+All ODP APIs MUST follow these conventions as part of their design.
+
+@subsection naming Naming Conventions
+All ODP APIs begin with the prefix odp_ and those that describe an
action to be performed on an object follow the naming convention of object
followed by action.
Note that not all ODP APIs start with odp_, we also have
odph_packet_alloc, which is wrong to begin with in my opinion, because
the ODP Packet Management API names this API odp_packet_alloc.
I did not change this, I think the normative API is odp_
+The advantage of this approach is that an alphabetical list of APIs for
an object all sort together since they all have names of the form
odp_object_action().
+
+So for example the API call to allocate a buffer is named
odp_buffer_alloc() rather than odp_alloc_buffer().
+
+@subsection data_types Data Types and Use of typedef
+ODP is designed to allow broad variability in how APIs are implemented
on various platforms.
+To support this, most APIs operate on abstract data types that are
defined via typedef on a per-implementation basis.
+These abstract types follow the naming convention of odp_object_t.
+
+Typedefs that encapsulate C structs follow the convention:
+
+@code
+typedef struct odp_descriptive_name {
+...
+} odp_descriptive_name_t;
+@endcode
+
+The use of typedef allows implementations to choose underlying data
representations that map efficiently to platform capabilities while
providing accessor functions to provide structured access to implementation
information in a portable manner
+Similarly, the use of enum is RECOMMENDED to provide value abstraction
for API parameters while enabling the implementation to choose code points
that map well to platform native values
Final dot to end sentence here.
fixed
+
+Several native C types are used conventionally within ODP and SHOULD be
employed in API design:
+
+type | Correct use
+ |---| :-
+void | SHOULD be used for APIs that do not return a value
Full stop.
Not done because this is in a table
+void*| SHOULD be used for APIs that return a pointer intended to be
used by the caller as a pointer. For example, a routine that returns the
address of an application context area SHOULD use a void * return type
Maybe reformulate as: SHOULD be used for APIs that return a pointer
intended to be used by the caller *as such*. Just a suggestion, you
can leave it like this if you think it's clearer. Also missing full
stop.
Tidied up, no full stop as this is a table
+int | SHOULD be used for APIs that return a boolean value. The values
1 = true, 0 = false are used for this purpose. Similarly, int SHOULD also
be used for APIs that return a simple success/failure indication to the
caller. In this case the return value 0 indicates success while non-zero
(typically -1) indicates failure and errno is set to a reason code that
indicates the nature of the failure.
+
+@subsection parameters Parameter Structure and Validation
+ODP is a framework for use in the data plane.
+Data plane applications typically have extreme performance requirements
mandating very strict attention to path length considerations in the design
of all ODP APIs, with the exception of those designed to be used
infrequently such as only during initialization or termination processing.
+
+Minimizing pathlength in API design involves several considerations:
+ - The number of parameters passed to a call. In general, ODP APIs
designed for frequent use SHOULD have few parameters. Limiting parameter
count to one or two well-chosen parameters SHOULD be the goal for APIs
Re: [lng-odp] [ODP/PATCH v2] Look ma, no barriers! C11 memory model
On 5 November 2014 14:54, Mike Holmes mike.hol...@linaro.org wrote:
On 5 November 2014 15:26, Victor Kamensky victor.kamen...@linaro.org
wrote:
Hi Ola,
Please see below general comment about your approach.
+typedef enum {
+ /** Relaxed memory order, no ordering of other accesses enforced
*/
+ ODP_MEMORDER_RLX,
+ /** Acquire memory order, later accesses cannot move before
+* acquire operation */
+ ODP_MEMORDER_ACQ,
+ /** Release memory order, earlier accesses cannot move after
+* release operation */
+ ODP_MEMORDER_RLS
+} odp_memorder_t;
Why do you have 3 memory models while C11
has 6?
Are you aware about gcc __atomic built extenstion?
https://gcc.gnu.org/onlinedocs/gcc/_005f_005fatomic-Builtins.html
They follow C11 quite closely with few caveats (like run-time
vs built time). Why don't you use those, instead of implementing
them by yourself?
If we plan to be compiler agnostic do we want to add more gcc mechanisms ?
I wrote aboute that below - use tiny wrappers; there will
be similar functionality in another compiler ...
I.e look for __c11_atomic builtins
http://clang.llvm.org/docs/LanguageExtensions.html
And current and Ola's code is already doing it with __sync
atomics, those are gcc mechanisms. What is different
if __atomic gcc builtin will be used?
Implementing general purpose C11 atomics in ODP
does not make sense to me. IMHO it does not belong
here. ODP is about h/w accelerators abstraction. Where
are h/w accelerators in C11 (note in quotes) atomics
implementation?
Thanks,
Victor
What is the need to reimplement those in ODP?
I am not sure, it seems to me that Petri raised similar point.
In ODP we need to provide atomic operations that map to
possible h/w accelerations that SOCs come up with, like
Octeon atomic and ARM8.1A atomic instructions. If we just
need atomic function for general purpose memory, those
IMHO should come up from somewhere else (like compiler,
C11 implementation, etc).
Basically if linux-generic needs
some of atomic C11 style primitives it could use __atomic
builtins directly or with some tiny wrapup api, that deals
with gcc/clang differences, but those should not be exposed
as ODP apis. I think that point was raised during discussion.
BTW linux-generic atomic implementation (one that implements
ODP API) could use __atomic with relaxed option to do required
operation, but without barrier, if you are not happy with current
implementation that uses old __sync builtins.
Thanks,
Victor
___
lng-odp mailing list
lng-odp@lists.linaro.org
http://lists.linaro.org/mailman/listinfo/lng-odp
--
Mike Holmes
Linaro Sr Technical Manager
LNG - ODP
___
lng-odp mailing list
lng-odp@lists.linaro.org
http://lists.linaro.org/mailman/listinfo/lng-odp
Re: [lng-odp] linux-dpdk: need odp_init_global parameter
On 4 November 2014 20:28, Ciprian Barbu ciprian.ba...@linaro.org wrote: Hello Venky, We've been looking at some of the OVS cards we have and the subject of testing OVS on top of ODP on top of DPDK came up. Last time I worked on this I modified odp_init_dpdk with hardcoded values but we need to solve this problem in the near future. Mike pushed a patch that allows application to pass platform specific init parameters to odp_init_global_init and I think that might be enough. Do you have any Yes, we need that patch. Probably when I update odp-dpdk to odp.git we should have all of them. thoughts on this? /Ciprian ___ lng-odp mailing list lng-odp@lists.linaro.org http://lists.linaro.org/mailman/listinfo/lng-odp
