Re: [lng-odp] [PATCH] example: classifier: fix add queue param init call

2015-12-11 Thread Stuart Haslam
On Fri, Dec 11, 2015 at 02:46:28PM +0530, Balasubramanian Manoharan wrote:
> Fixes crash caused by queue param not being initialized.
> 
> Signed-off-by: Balasubramanian Manoharan 

That fixed it..

Reviewed-by: Stuart Haslam 

> ---
>  example/classifier/odp_classifier.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/example/classifier/odp_classifier.c 
> b/example/classifier/odp_classifier.c
> index c8f264f..81e6bf0 100644
> --- a/example/classifier/odp_classifier.c
> +++ b/example/classifier/odp_classifier.c
> @@ -401,6 +401,7 @@ static void configure_cos_queue(odp_pktio_t pktio, 
> appl_args_t *args)
>   };
>  
>   stats->pmr = odp_pmr_create();
> + odp_queue_param_init();
>   qparam.sched.prio = i % odp_schedule_num_prio();
>   qparam.sched.sync = ODP_SCHED_SYNC_NONE;
>   qparam.sched.group = ODP_SCHED_GROUP_ALL;
> -- 
> 1.9.1
___
lng-odp mailing list
lng-odp@lists.linaro.org
https://lists.linaro.org/mailman/listinfo/lng-odp


Re: [lng-odp] [PATCH] configure.ac: add user_guides config status

2015-12-11 Thread Maxim Uvarov

Merged.

On 12/11/2015 02:17, Bill Fischofer wrote:



On Thu, Dec 10, 2015 at 3:08 PM, Mike Holmes > wrote:


Signed-off-by: Mike Holmes >


Reviewed-by: Bill Fischofer >


---
configure.ac  | 1 +
 1 file changed, 1 insertion(+)

diff --git a/configure.ac  b/configure.ac

index 00a1415..e45d90a 100644
--- a/configure.ac 
+++ b/configure.ac 
@@ -413,4 +413,5 @@ AC_MSG_RESULT([
test_perf:  ${test_perf}
test_cpp:   ${test_cpp}
test_helper:${test_helper}
+   user_guides:${user_guides}
 ])
--
2.5.0

___
lng-odp mailing list
lng-odp@lists.linaro.org 
https://lists.linaro.org/mailman/listinfo/lng-odp




___
lng-odp mailing list
lng-odp@lists.linaro.org
https://lists.linaro.org/mailman/listinfo/lng-odp


___
lng-odp mailing list
lng-odp@lists.linaro.org
https://lists.linaro.org/mailman/listinfo/lng-odp


Re: [lng-odp] [PATCH] example: classifier: fix add queue param init call

2015-12-11 Thread Maxim Uvarov

Ok, you added that init() inside that commit of api-next:

commit 8da0ee0b2a43bfdd5b0d37d2d2a10a45bbf2d7bd
Author: Balasubramanian Manoharan 
Date:   Thu Dec 3 16:16:06 2015 +0530

example: classifier: add odp_cls_cos_pool_set() api

Adds packet pool to CoS using odp_cls_cos_pool_set() api.

Signed-off-by: Balasubramanian Manoharan 
Reviewed-by: Petri Savolainen 
Reviewed-and-tested-by: Bill Fischofer 
Signed-off-by: Maxim Uvarov 

That is why it does not fail on api-next. So we can apply that change 
now to master as bugfix

and port all commit later, closer to next release.

Maxim.

On 12/11/2015 12:16, Balasubramanian Manoharan wrote:

Fixes crash caused by queue param not being initialized.

Signed-off-by: Balasubramanian Manoharan 
---
  example/classifier/odp_classifier.c | 1 +
  1 file changed, 1 insertion(+)

diff --git a/example/classifier/odp_classifier.c 
b/example/classifier/odp_classifier.c
index c8f264f..81e6bf0 100644
--- a/example/classifier/odp_classifier.c
+++ b/example/classifier/odp_classifier.c
@@ -401,6 +401,7 @@ static void configure_cos_queue(odp_pktio_t pktio, 
appl_args_t *args)
};
  
  		stats->pmr = odp_pmr_create();

+   odp_queue_param_init();
qparam.sched.prio = i % odp_schedule_num_prio();
qparam.sched.sync = ODP_SCHED_SYNC_NONE;
qparam.sched.group = ODP_SCHED_GROUP_ALL;


___
lng-odp mailing list
lng-odp@lists.linaro.org
https://lists.linaro.org/mailman/listinfo/lng-odp


Re: [lng-odp] [PATCH] example: classifier: fix add queue param init call

2015-12-11 Thread Bala Manoharan
It is fine from my side.

Regards,
Bala

On 11 December 2015 at 14:56, Maxim Uvarov  wrote:
> Ok, you added that init() inside that commit of api-next:
>
> commit 8da0ee0b2a43bfdd5b0d37d2d2a10a45bbf2d7bd
> Author: Balasubramanian Manoharan 
> Date:   Thu Dec 3 16:16:06 2015 +0530
>
> example: classifier: add odp_cls_cos_pool_set() api
>
> Adds packet pool to CoS using odp_cls_cos_pool_set() api.
>
> Signed-off-by: Balasubramanian Manoharan 
> Reviewed-by: Petri Savolainen 
> Reviewed-and-tested-by: Bill Fischofer 
> Signed-off-by: Maxim Uvarov 
>
> That is why it does not fail on api-next. So we can apply that change now to
> master as bugfix
> and port all commit later, closer to next release.
>
> Maxim.
>
> On 12/11/2015 12:16, Balasubramanian Manoharan wrote:
>>
>> Fixes crash caused by queue param not being initialized.
>>
>> Signed-off-by: Balasubramanian Manoharan 
>> ---
>>   example/classifier/odp_classifier.c | 1 +
>>   1 file changed, 1 insertion(+)
>>
>> diff --git a/example/classifier/odp_classifier.c
>> b/example/classifier/odp_classifier.c
>> index c8f264f..81e6bf0 100644
>> --- a/example/classifier/odp_classifier.c
>> +++ b/example/classifier/odp_classifier.c
>> @@ -401,6 +401,7 @@ static void configure_cos_queue(odp_pktio_t pktio,
>> appl_args_t *args)
>> };
>> stats->pmr = odp_pmr_create();
>> +   odp_queue_param_init();
>> qparam.sched.prio = i % odp_schedule_num_prio();
>> qparam.sched.sync = ODP_SCHED_SYNC_NONE;
>> qparam.sched.group = ODP_SCHED_GROUP_ALL;
>
>
> ___
> lng-odp mailing list
> lng-odp@lists.linaro.org
> https://lists.linaro.org/mailman/listinfo/lng-odp



-- 
Regards,
Bala
___
lng-odp mailing list
lng-odp@lists.linaro.org
https://lists.linaro.org/mailman/listinfo/lng-odp


Re: [lng-odp] [API-NEXT PATCH v3] api: atomic: added atomic_lock_free_u64

2015-12-11 Thread Maxim Uvarov

Merged with spell fix.

Maxim.

On 11/12/2015 11:30, Petri Savolainen wrote:

Platforms may support some uint64 operations lock-free and
others not. For example, inc_64 can be natively supported but
cas_64 (or max_64/min_64) not. User may be able to switch to
32 bit variables when a 64 bit operation uses locks. The same
atomic operation struture could be used for platform init to guide
lock vs. lock-free implementation (e.g. if cas_64 is never
called, inc_64 can be lock-free).

Signed-off-by: Petri Savolainen 
---
  include/odp/api/atomic.h| 48 +
  platform/linux-generic/Makefile.am  |  1 +
  platform/linux-generic/odp_atomic.c | 24 +++
  3 files changed, 73 insertions(+)
  create mode 100644 platform/linux-generic/odp_atomic.c

diff --git a/include/odp/api/atomic.h b/include/odp/api/atomic.h
index 316f13a..5e897c1 100644
--- a/include/odp/api/atomic.h
+++ b/include/odp/api/atomic.h
@@ -388,6 +388,54 @@ void odp_atomic_add_rls_u32(odp_atomic_u32_t *atom, 
uint32_t val);
  void odp_atomic_sub_rls_u32(odp_atomic_u32_t *atom, uint32_t val);
  
  /**

+ * Atomic operations
+ *
+ * Atomic operations listed in a bit field structure.
+ */
+typedef union odp_atomic_op_t {
+   /** Operation flags */
+   struct {
+   uint32_t init  : 1;  /**< Init atomic variable */
+   uint32_t load  : 1;  /**< Atomic load */
+   uint32_t store : 1;  /**< Atomic store */
+   uint32_t fetch_add : 1;  /**< Atomic fetch and add */
+   uint32_t add   : 1;  /**< Atomic add */
+   uint32_t fetch_sub : 1;  /**< Atomic fetch and substract */
+   uint32_t sub   : 1;  /**< Atomic substract */
+   uint32_t fetch_inc : 1;  /**< Atomic fetch and increment */
+   uint32_t inc   : 1;  /**< Atomic increment */
+   uint32_t fetch_dec : 1;  /**< Atomic fetch and decrement */
+   uint32_t dec   : 1;  /**< Atomic decrement */
+   uint32_t min   : 1;  /**< Atomic minimum */
+   uint32_t max   : 1;  /**< Atomic maximum */
+   uint32_t cas   : 1;  /**< Atomic compare and swap */
+   } op;
+
+   /** All bits of the bit field structure.
+ * Operation flag mapping is architecture specific. This field can be
+ * used to set/clear all flags, or bitwise operations over the entire
+ * structure. */
+   uint32_t all_bits;
+} odp_atomic_op_t;
+
+/**
+ * Query which atomic uint64 operations are lock-free
+ *
+ * Lock-free implementations have higher performance and scale better than
+ * implementations using locks. User can decide to use e.g. uint32 atomic
+ * variables instead of uint64 to optimize performance on platforms that
+ * implement a performance critical operation using locks.
+ *
+ * @param atomic_op  Pointer to atomic operation structure for storing
+ *   operation flags. All bits are initialized to zero during
+ *   the operation. The parameter is ignored when NULL.
+ * @retval 0 None of the operations are lock-free
+ * @retval 1 Some of the operations are lock-free
+ * @retval 2 All operations are lock-free
+ */
+int odp_atomic_lock_free_u64(odp_atomic_op_t *atomic_op);
+
+/**
   * @}
   */
  
diff --git a/platform/linux-generic/Makefile.am b/platform/linux-generic/Makefile.am

index a6b6029..0b7234e 100644
--- a/platform/linux-generic/Makefile.am
+++ b/platform/linux-generic/Makefile.am
@@ -151,6 +151,7 @@ noinst_HEADERS = \
  ${srcdir}/Makefile.inc
  
  __LIB__libodp_la_SOURCES = \

+  odp_atomic.c \
   odp_barrier.c \
   odp_buffer.c \
   odp_classification.c \
diff --git a/platform/linux-generic/odp_atomic.c 
b/platform/linux-generic/odp_atomic.c
new file mode 100644
index 000..996d09a
--- /dev/null
+++ b/platform/linux-generic/odp_atomic.c
@@ -0,0 +1,24 @@
+/* Copyright (c) 2015, Linaro Limited
+ * All rights reserved.
+ *
+ * SPDX-License-Identifier: BSD-3-Clause
+ */
+
+#include 
+
+int odp_atomic_lock_free_u64(odp_atomic_op_t *atomic_op)
+{
+#if __GCC_ATOMIC_LLONG_LOCK_FREE < 2
+   /* All operations have locks */
+   if (atomic_op)
+   atomic_op->all_bits = 0;
+
+   return 0;
+#else
+   /* All operations are lock-free */
+   if (atomic_op)
+   atomic_op->all_bits = ~((uint32_t)0);
+
+   return 2;
+#endif
+}


___
lng-odp mailing list
lng-odp@lists.linaro.org
https://lists.linaro.org/mailman/listinfo/lng-odp


Re: [lng-odp] [PATCHv6] helper : Fix UDP checksum computation

2015-12-11 Thread Ilya Maximets
On 11.12.2015 12:19, Maxim Uvarov wrote:
> On 12/11/2015 10:39, Ilya Maximets wrote:
>> Comments inlined.
>>
>> On 10.12.2015 12:59, ion.grig...@freescale.com wrote:
>>> From: Grigore Ion 
>>>
>>> This patch fixes the following problems:
>>> - checksum computation for LE platforms
>>> - checksum is computed in the CPU endianness. The returned result
>>> must be converted to the BE ordering when it is used to update
>>> the UDP checksum in a packet.
>>> - checksum computation for packets having the UDP length not a
>>> multiple of 2
>>> - fixes the UDP checksum associated validation test
>>>
>>> Signed-off-by: Grigore Ion 
>>> ---
>>>   v6:
>>>   - Make code more understandable (Ilya Maximets)
>>>   v5:
>>>   - Checksum in CPU endianness fix added (Ilya Maximets)
>>>   v4:
>>>   - Verify checksum in CPU endianness in the associated test
>>>   (Ilya Maximets)
>>>   v3:
>>>   - fix the UDP checksum computation in the associated test
>>>   (Maxim Uvarov)
>>>   v2:
>>>   - patch updated to the last master (Maxim Uvarov)
>>>   v1:
>>>   - Move variables declaration on top of block. (Maxim Uvarov)
>>>   - Check patch with checkpatch script.  (Maxim Uvarov)
>>>   - L3 header presence is tested twice. (Alexandru Badicioiu)
>>>   - Remove unnecessary check for L3 header presence. (Bill Fischofer)
>>>   - Modify check of odp_packet_l4_offset() return. (Bill Fischofer)
>>>
>>>   helper/include/odp/helper/udp.h | 81 
>>> +
>>>   helper/test/odp_chksum.c|  4 +-
>>>   2 files changed, 51 insertions(+), 34 deletions(-)
>>>
>>> diff --git a/helper/include/odp/helper/udp.h 
>>> b/helper/include/odp/helper/udp.h
>>> index 06c439b..d0599b1 100644
>>> --- a/helper/include/odp/helper/udp.h
>>> +++ b/helper/include/odp/helper/udp.h
>>> @@ -4,7 +4,6 @@
>>>* SPDX-License-Identifier: BSD-3-Clause
>>>*/
>>>   -
>>>   /**
>>>* @file
>>>*
>>> @@ -22,7 +21,6 @@ extern "C" {
>>>   #include 
>>>   #include 
>>>   -
>>>   /** @addtogroup odph_header ODPH HEADER
>>>*  @{
>>>*/
>>> @@ -39,51 +37,70 @@ typedef struct ODP_PACKED {
>>>   } odph_udphdr_t;
>>> /**
>>> + * Perform byte swap required by UDP checksum computation algorithm
>>> + */
>>> +static inline uint16_t csum_bswap(uint16_t val)
>>> +{
>>> +#if ODP_BYTE_ORDER == ODP_LITTLE_ENDIAN
>>> +val = __odp_builtin_bswap16((__odp_force uint16_t)val);
>>> +#endif
>>> +return val;
>>> +}
>> Please, don't duplicate the code.
>>
>>> +
>>> +/**
>>> + * Pad a byte value to the left or to the right as required by UDP checksum
>>> + * computation algorithm and convert the result to CPU native uint16_t.
>>> + * Left padding is performed for the IP protocol field in the UDP
>>> + * pseudo-header (RFC 768). Right padding is performed in the case of the 
>>> odd
>>> + * byte in a UDP packet having the length not a 2 multiple.
>>> + */
>>> +static inline uint16_t u8_pad_to_u16(uint8_t val, odp_bool_t left)
>>> +{
>>> +uint16_tret;
>>> +
>>> +ret = (left) ? val : val << 8;
>>> +return csum_bswap(ret);
>>> +}
>>> +
>>> +/**
>>>* UDP checksum
>>>*
>>>* This function uses odp packet to calc checksum
>>>*
>>>* @param pkt  calculate chksum for pkt
>>> - * @return  checksum value
>>> + * @return  checksum value in CPU endianness
>>>*/
>>>   static inline uint16_t odph_ipv4_udp_chksum(odp_packet_t pkt)
>>>   {
>>> -uint32_t sum = 0;
>>> -odph_udphdr_t *udph;
>>> -odph_ipv4hdr_t *iph;
>>> -uint16_t udplen;
>>> -uint8_t *buf;
>>> -
>>> -if (!odp_packet_l3_offset(pkt))
>>> -return 0;
>>> +odph_ipv4hdr_t*iph;
>>> +odph_udphdr_t*udph;
>>> +uint32_tsum;
>>> +uint16_tudplen, *buf;
>>>   -if (!odp_packet_l4_offset(pkt))
>>> +if (odp_packet_l4_offset(pkt) == ODP_PACKET_OFFSET_INVALID)
>>>   return 0;
>>> -
>>>   iph = (odph_ipv4hdr_t *)odp_packet_l3_ptr(pkt, NULL);
>>>   udph = (odph_udphdr_t *)odp_packet_l4_ptr(pkt, NULL);
>>> -udplen = odp_be_to_cpu_16(udph->length);
>>> -
>>> -/* 32-bit sum of all 16-bit words covered by UDP chksum */
>>> +/* 32-bit sum of UDP pseudo-header */
>>>   sum = (iph->src_addr & 0x) + (iph->src_addr >> 16) +
>>> -  (iph->dst_addr & 0x) + (iph->dst_addr >> 16) +
>>> -  (uint16_t)iph->proto + udplen;
>>> -for (buf = (uint8_t *)udph; udplen > 1; udplen -= 2) {
>>> -sum += ((*buf << 8) + *(buf + 1));
>>> -buf += 2;
>>> -}
>>> -
>>> -/* Fold sum to 16 bits: add carrier to result */
>>> -while (sum >> 16)
>>> -sum = (sum & 0x) + (sum >> 16);
>>> -
>>> +(iph->dst_addr & 0x) + (iph->dst_addr >> 16) +
>>> +u8_pad_to_u16(iph->proto, 1) + udph->length;
>> I meant something like:
>>
>> union {
>> uint8_t v8[2];
>> uint16_t v16;
>> } val;
>>
>> val.v8[0] = 0;
>> val.v8[1] = iph->proto;
>>
>> sum = (iph->src_addr & 

[lng-odp] [API-NEXT PATCH 1/2] api: barrier: added memory barriers

2015-12-11 Thread Petri Savolainen
Added new memory barriers. These follow C11 release /
acquire specification and replaces odp_sync_stores().
Used GCC __atomic_thread_fence to implement all three
barriers.

Signed-off-by: Petri Savolainen 
---
 include/odp/api/barrier.h | 11 -
 include/odp/api/sync.h| 82 ---
 platform/linux-generic/include/odp/sync.h | 28 +++
 3 files changed, 90 insertions(+), 31 deletions(-)

diff --git a/include/odp/api/barrier.h b/include/odp/api/barrier.h
index 8ca2647..823eae6 100644
--- a/include/odp/api/barrier.h
+++ b/include/odp/api/barrier.h
@@ -18,8 +18,15 @@
 extern "C" {
 #endif
 
-/** @defgroup odp_barrier ODP BARRIER
- *  Thread excution and memory ordering barriers.
+/**
+ * @defgroup odp_barrier ODP BARRIER
+ * Thread excution and memory ordering barriers.
+ *
+ * @details
+ *  Thread execution barrier (odp_barrier_t) 
+ *
+ * Thread execution barrier synchronizes a group of threads to wait on the
+ * barrier until the entire group has reached the barrier.
  *  @{
  */
 
diff --git a/include/odp/api/sync.h b/include/odp/api/sync.h
index 6477e74..c6f790c 100644
--- a/include/odp/api/sync.h
+++ b/include/odp/api/sync.h
@@ -8,7 +8,7 @@
 /**
  * @file
  *
- * ODP synchronisation
+ * ODP memory barriers
  */
 
 #ifndef ODP_API_SYNC_H_
@@ -18,42 +18,66 @@
 extern "C" {
 #endif
 
-/** @addtogroup odp_barrier
+/**
+ * @addtogroup odp_barrier
+ * @details
+ *  Memory barriers 
+ *
+ * Memory barriers enforce ordering of memory load and store operations
+ * specified before and after the barrier. These barriers may affect both
+ * compiler optimizations and CPU out-of-order execution. All ODP
+ * synchronization mechanisms (e.g. execution barriers, locks, queues, etc )
+ * include all necessary memory barriers, so these calls are not needed when
+ * using those. Also ODP atomic operations have memory ordered versions. These
+ * explicit barriers may be needed when thread synchronization is based on
+ * a non-ODP defined mechanism. Depending on the HW platform, heavy usage of
+ * memory barriers may cause significant performance degradation.
+ *
  *  @{
  */
 
 /**
- * Synchronise stores
+ * Memory barrier for release operations
  *
- * Ensures that all CPU store operations that precede the odp_sync_stores()
- * call are globally visible before any store operation that follows it.
+ * This memory barrier has release semantics. It synchronizes with a pairing
+ * barrier for acquire operations. The releasing and acquiring threads
+ * synchronize through shared memory. The releasing thread must call this
+ * barrier before signaling the acquiring thread. After the acquiring thread
+ * receives the signal, it must call odp_mb_acquire() before it reads the
+ * memory written by the releasing thread.
+ *
+ * This call is not needed when using ODP defined synchronization mechanisms.
+ *
+ * @see odp_mb_acquire()
  */
-static inline void odp_sync_stores(void)
-{
-#if defined __x86_64__ || defined __i386__
-
-   __asm__  __volatile__ ("sfence\n" : : : "memory");
-
-#elif defined(__arm__)
-#if __ARM_ARCH == 6
-   __asm__ __volatile__ ("mcr p15, 0, %0, c7, c10, 5" \
-   : : "r" (0) : "memory");
-#elif __ARM_ARCH >= 7 || defined __aarch64__
-
-   __asm__ __volatile__ ("dmb st" : : : "memory");
-#else
-   __asm__ __volatile__ ("" : : : "memory");
-#endif
-
-#elif defined __OCTEON__
-
-   __asm__  __volatile__ ("syncws\n" : : : "memory");
+void odp_mb_release(void);
 
-#else
-   __sync_synchronize();
-#endif
-}
+/**
+ * Memory barrier for acquire operations
+ *
+ * This memory barrier has acquire semantics. It synchronizes with a pairing
+ * barrier for release operations. The releasing and acquiring threads
+ * synchronize through shared memory. The releasing thread must call
+ * odp_mb_release() before signaling the acquiring thread. After the acquiring
+ * thread receives the signal, it must call this barrier before it reads the
+ * memory written by the releasing thread.
+ *
+ * This call is not needed when using ODP defined synchronization mechanisms.
+ *
+ * @see odp_mb_release()
+ */
+void odp_mb_acquire(void);
 
+/**
+ * Full memory barrier
+ *
+ * This is a full memory barrier. It guarantees that all load and store
+ * operations specified before it are visible to other threads before
+ * all load and store operations specified after it.
+ *
+ * This call is not needed when using ODP defined synchronization mechanisms.
+ */
+void odp_mb_full(void);
 
 /**
  * @}
diff --git a/platform/linux-generic/include/odp/sync.h 
b/platform/linux-generic/include/odp/sync.h
index bc73083..bfe67ee 100644
--- a/platform/linux-generic/include/odp/sync.h
+++ b/platform/linux-generic/include/odp/sync.h
@@ -17,6 +17,34 @@
 extern "C" {
 #endif
 
+/** @ingroup odp_barrier
+ *  @{
+ */
+
+static inline void odp_mb_release(void)
+{
+   __atomic_thread_fence(__ATOMIC_RELEASE);
+}
+
+static 

[lng-odp] [API-NEXT PATCH 2/2] api: sync: removed odp_sync_stores

2015-12-11 Thread Petri Savolainen
Removed odp_sync_stores() implementation and replaced usage with
odp_mb_full(), to minimize functional changes. Added minimal
validation test function for the new memory barriers.

Signed-off-by: Petri Savolainen 
---
 platform/linux-generic/include/odp/sync.h |  5 ---
 platform/linux-generic/odp_queue.c|  4 +--
 test/performance/odp_l2fwd.c  |  8 ++---
 test/validation/synchronizers/synchronizers.c | 51 +++
 test/validation/synchronizers/synchronizers.h |  1 +
 5 files changed, 44 insertions(+), 25 deletions(-)

diff --git a/platform/linux-generic/include/odp/sync.h 
b/platform/linux-generic/include/odp/sync.h
index bfe67ee..b2995e1 100644
--- a/platform/linux-generic/include/odp/sync.h
+++ b/platform/linux-generic/include/odp/sync.h
@@ -36,11 +36,6 @@ static inline void odp_mb_full(void)
__atomic_thread_fence(__ATOMIC_SEQ_CST);
 }
 
-static inline void odp_sync_stores(void)
-{
-   __atomic_thread_fence(__ATOMIC_RELEASE);
-}
-
 /**
  * @}
  */
diff --git a/platform/linux-generic/odp_queue.c 
b/platform/linux-generic/odp_queue.c
index 8b3671d..a2fd4ec 100644
--- a/platform/linux-generic/odp_queue.c
+++ b/platform/linux-generic/odp_queue.c
@@ -357,9 +357,9 @@ int odp_queue_context_set(odp_queue_t handle, void *context)
 {
queue_entry_t *queue;
queue = queue_to_qentry(handle);
-   odp_sync_stores();
+   odp_mb_full();
queue->s.param.context = context;
-   odp_sync_stores();
+   odp_mb_full();
return 0;
 }
 
diff --git a/test/performance/odp_l2fwd.c b/test/performance/odp_l2fwd.c
index 67a23ed..c2dfeec 100644
--- a/test/performance/odp_l2fwd.c
+++ b/test/performance/odp_l2fwd.c
@@ -237,8 +237,8 @@ static void *pktio_queue_thread(void *arg)
stats->s.packets += pkts;
}
 
-   /* Make sure that the last stats write is visible to readers */
-   odp_sync_stores();
+   /* Make sure that latest stat writes are visible to other threads */
+   odp_mb_full();
 
return NULL;
 }
@@ -365,8 +365,8 @@ static void *pktio_direct_recv_thread(void *arg)
 
}
 
-   /* Make sure that the last stats write is visible to readers */
-   odp_sync_stores();
+   /* Make sure that latest stat writes are visible to other threads */
+   odp_mb_full();
 
return NULL;
 }
diff --git a/test/validation/synchronizers/synchronizers.c 
b/test/validation/synchronizers/synchronizers.c
index cebe0d2..0302069 100644
--- a/test/validation/synchronizers/synchronizers.c
+++ b/test/validation/synchronizers/synchronizers.c
@@ -36,6 +36,8 @@
 static odp_atomic_u32_t a32u;
 static odp_atomic_u64_t a64u;
 
+static volatile int temp_result;
+
 typedef __volatile uint32_t volatile_u32_t;
 typedef __volatile uint64_t volatile_u64_t;
 
@@ -224,12 +226,12 @@ static uint32_t barrier_test(per_thread_mem_t 
*per_thread_mem,
odp_barrier_wait(_mem->test_barriers[cnt]);
 
global_mem->barrier_cnt1 = cnt + 1;
-   odp_sync_stores();
+   odp_mb_full();
 
if (i_am_slow_thread) {
global_mem->slow_thread_num = next_slow_thread;
global_mem->barrier_cnt2 = cnt + 1;
-   odp_sync_stores();
+   odp_mb_full();
} else {
while (global_mem->barrier_cnt2 != (cnt + 1))
thread_delay(per_thread_mem, BASE_DELAY);
@@ -501,7 +503,7 @@ static void *no_lock_functional_test(void *arg UNUSED)
 
for (cnt = 1; cnt <= iterations; cnt++) {
global_mem->global_lock_owner = thread_num;
-   odp_sync_stores();
+   odp_mb_full();
thread_delay(per_thread_mem, lock_owner_delay);
 
if (global_mem->global_lock_owner != thread_num) {
@@ -510,7 +512,7 @@ static void *no_lock_functional_test(void *arg UNUSED)
}
 
global_mem->global_lock_owner = 0;
-   odp_sync_stores();
+   odp_mb_full();
thread_delay(per_thread_mem, MIN_DELAY);
 
if (global_mem->global_lock_owner == thread_num) {
@@ -591,7 +593,7 @@ static void *spinlock_functional_test(void *arg UNUSED)
* global_lock_owner to be themselves
*/
global_mem->global_lock_owner = thread_num;
-   odp_sync_stores();
+   odp_mb_full();
thread_delay(per_thread_mem, lock_owner_delay);
if (global_mem->global_lock_owner != thread_num) {
current_errs++;
@@ -600,7 +602,7 @@ static void *spinlock_functional_test(void *arg UNUSED)
 
/* Release shared lock, and make sure we no longer have it */
global_mem->global_lock_owner = 0;
-   odp_sync_stores();
+   odp_mb_full();
  

Re: [lng-odp] [API-NEXT PATCH v2 4/5] linux-generic: netmap: add multi queue support

2015-12-11 Thread Stuart Haslam
On Wed, Nov 25, 2015 at 03:15:52PM +0200, Matias Elo wrote:
> Added multi queue support to netmap pktio.
> 
> Signed-off-by: Matias Elo 
> ---
>  platform/linux-generic/include/odp_packet_netmap.h |  34 +-
>  platform/linux-generic/pktio/netmap.c  | 426 
> ++---
>  2 files changed, 406 insertions(+), 54 deletions(-)
> 
> diff --git a/platform/linux-generic/include/odp_packet_netmap.h 
> b/platform/linux-generic/include/odp_packet_netmap.h
> index a088d23..b38363a 100644
> --- a/platform/linux-generic/include/odp_packet_netmap.h
> +++ b/platform/linux-generic/include/odp_packet_netmap.h
> @@ -7,23 +7,53 @@
>  #ifndef ODP_PACKET_NETMAP_H
>  #define ODP_PACKET_NETMAP_H
>  
> +#include 
> +#include 
>  #include 
>  #include 
> +#include 
> +#include 
>  
>  #include 
>  #include 
>  
> +#define NM_MAX_DESC 32
> +#define NM_MAX_INPUT_QUEUES 32
> +#define NM_MAX_OUTPUT_QUEUES 32
> +
> +/** Ring for mapping pktin/pktout queues to netmap descriptors */
> +struct netmap_ring_t {
> + unsigned first; /**< Index of first netmap descriptor */
> + unsigned last;  /**< Index of last netmap descriptor */
> + unsigned num;   /**< Number of netmap descriptors */
> + /** Netmap metadata for the device */
> + struct nm_desc *desc[NM_MAX_DESC];
> + unsigned cur;   /**< Index of current netmap descriptor */
> + odp_ticketlock_t lock;  /**< Queue lock */
> +};
> +
> +typedef union {
> + struct netmap_ring_t s;
> + uint8_t pad[ODP_CACHE_LINE_SIZE_ROUNDUP(sizeof(struct netmap_ring_t))];
> +} netmap_ring_t ODP_ALIGNED_CACHE;
> +
>  /** Packet socket using netmap mmaped rings for both Rx and Tx */
>  typedef struct {
>   odp_pool_t pool;/**< pool to alloc packets from */
>   size_t max_frame_len;   /**< buf_size - sizeof(pkt_hdr) */
> - struct nm_desc *rx_desc;/**< netmap meta-data for the device */
> - struct nm_desc *tx_desc;/**< netmap meta-data for the device */
>   uint32_t if_flags;  /**< interface flags */
>   int sockfd; /**< control socket */
>   unsigned char if_mac[ETH_ALEN]; /**< eth mac address */
>   char nm_name[IF_NAMESIZE + 7];  /**< netmap: */
>   odp_pktio_capability_t  capa;   /**< interface capabilities */
> + unsigned num_rx_queues; /**< number of pktin queues */
> + unsigned num_tx_queues; /**< number of pktout queues */
> + odp_bool_t lockless_rx; /**< no locking for rx */
> + odp_bool_t lockless_tx; /**< no locking for tx */
> + /** mapping of pktin queues to netmap rx descriptors */
> + netmap_ring_t rx_desc_ring[NM_MAX_INPUT_QUEUES];
> + /** mapping of pktout queues to netmap tx descriptors */
> + netmap_ring_t tx_desc_ring[NM_MAX_OUTPUT_QUEUES];
>  } pkt_netmap_t;
>  
>  #endif
> diff --git a/platform/linux-generic/pktio/netmap.c 
> b/platform/linux-generic/pktio/netmap.c
> index 313bcbe..236b48c 100644
> --- a/platform/linux-generic/pktio/netmap.c
> +++ b/platform/linux-generic/pktio/netmap.c
> @@ -75,15 +75,146 @@ done:
>   return err;
>  }
>  
> +/**
> + * Map netmap rings to pktin/pktout queues
> + *
> + * @param rings  Array of netmap descriptor rings
> + * @param num_queues Number of pktin/pktout queues
> + * @param num_rings  Number of matching netmap rings
> + */
> +static inline void map_netmap_rings(netmap_ring_t *rings,
> + unsigned num_queues, unsigned num_rings)
> +{
> + struct netmap_ring_t *desc_ring;
> + unsigned rings_per_queue;
> + unsigned remainder;
> + unsigned mapped_rings;
> + unsigned i;
> + unsigned desc_id = 0;
> +
> + rings_per_queue = num_rings / num_queues;
> + remainder = num_rings % num_queues;
> +
> + if (remainder)
> + ODP_DBG("WARNING: Netmap rings mapped unevenly to queues\n");
> +
> + for (i = 0; i < num_queues; i++) {
> + desc_ring = [i].s;
> + if (i < remainder)
> + mapped_rings = rings_per_queue + 1;
> + else
> + mapped_rings = rings_per_queue;
> +
> + desc_ring->first = desc_id;
> + desc_ring->cur = desc_id;
> + desc_ring->last = desc_ring->first + mapped_rings - 1;
> + desc_ring->num = mapped_rings;
> +
> + desc_id = desc_ring->last + 1;
> + }
> +}
> +
> +static int netmap_input_queues_config(pktio_entry_t *pktio_entry,
> +   const odp_pktio_input_queue_param_t *p)
> +{
> + struct pktio_entry *pktio = _entry->s;
> + pkt_netmap_t *pkt_nm = _entry->s.pkt_nm;
> + odp_pktio_input_mode_t mode = pktio_entry->s.param.in_mode;
> + odp_queue_t queue;
> + unsigned num_queues = p->num_queues;
> + unsigned i;
> +
> + if (mode == ODP_PKTIN_MODE_DISABLED)
> + return -1;
> +
> + if (num_queues <= 0 || 

Re: [lng-odp] ODP_CLASSIFIER and ODP_GENERATOR failing in master branch

2015-12-11 Thread Bala Manoharan
Patch posted to fix the issue in odp_classifier.
odp_generator is working but showing the following error message continuously.

===
ktio/netmap.c:139:netmap_open():nm_open(netmap:lo:1) failed
odp_timer.c:641:timer_notify():
2 ticks overrun on timer pool "timer_pool", timer resolution too high
odp_timer.c:641:timer_notify():
3 ticks overrun on timer pool "timer_pool", timer resolution too high
odp_timer.c:641:timer_notify():
1 ticks overrun on timer pool "timer_pool", timer resolution too high
odp_timer.c:641:timer_notify():
1 ticks overrun on timer pool "timer_pool", timer resolution too high
odp_timer.c:641:timer_notify():


Regards,
Bala


On 11 December 2015 at 12:12, Bala Manoharan  wrote:
> Hi Maxim,
>
> I just tested it again and I am seeing the same behaviour I explained
> in the mail above.
> In Master branch odp_classifier and odp_generator fails and both are
> working on api-next branch.
> This behaviour was seen by Stuart as well.
>
> Regards,
> Bala
>
> On 11 December 2015 at 01:41, Maxim Uvarov  wrote:
>> That patches introduce segfault. Pktio is socket_mmap:
>>
>> 8cf523f api: classification: add odp_cls_cos_pool_set() api
>> 8da0ee0 example: classifier: add odp_cls_cos_pool_set() api
>> 4ade6a3 validation: classification: add odp_cls_cos_pool_set() api
>> e695969 linux-generic: classification: implements odp_cls_cos_pool_set() api
>>
>> Maxim.
>>
>>
>> On 12/10/2015 23:04, Maxim Uvarov wrote:
>>>
>>> Bala,
>>>
>>> I have opposite result. After you patch master never fails. api-next fails
>>> and my merge master to api-next also fails.
>>> So it looks like we have something in api-next that breaks the work.
>>>
>>>
>>> queue1   |queue2   |queue3   |DefaultCos   |Total Packets
>>> queue  pool  |queue  pool  |queue  pool  |queue  pool  |
>>> 437867 437867|131602 131602|653612 653612|59811  59811 |1282892
>>> Program received signal SIGSEGV, Segmentation fault.
>>> [Switching to Thread 0x74a91700 (LWP 3363)]
>>> 0x0042652a in _odp_packet_cls_enq (pktio_entry=0x2b600040,
>>> base=0x75307042 "33", buf_len=107, pkt_ret=0x74a90c40) at
>>> pktio/pktio_common.c:29
>>> 29pool = cos->s.pool->s.pool_hdl;
>>> (gdb) bt
>>> #0  0x0042652a in _odp_packet_cls_enq (pktio_entry=0x2b600040,
>>> base=0x75307042 "33", buf_len=107, pkt_ret=0x74a90c40) at
>>> pktio/pktio_common.c:29
>>> #1  0x00410a88 in pkt_mmap_v2_rx (pktio_entry=0x2b600040,
>>> pkt_sock=0x2b600080, pkt_table=0x74a90c40, len=8,
>>> if_mac=0x2b6001a4 "") at pktio/socket_mmap.c:152
>>> #2  0x004118b3 in sock_mmap_recv (pktio_entry=0x2b600040,
>>> pkt_table=0x74a90c40, len=8) at pktio/socket_mmap.c:519
>>> #3  0x0040cee1 in odp_pktio_recv (id=0x1,
>>> pkt_table=0x74a90c40, len=8) at odp_packet_io.c:397
>>> #4  0x0040d843 in pktin_poll (entry=0x2b600040) at
>>> odp_packet_io.c:667
>>> #5  0x00417a0f in schedule (out_queue=0x74a90e10,
>>> out_ev=0x74a90dd8, max_num=1, max_deq=4) at odp_schedule.c:496
>>> #6  0x00417d90 in schedule_loop (out_queue=0x74a90e10,
>>> wait=18446744073709551615, out_ev=0x74a90dd8, max_num=1, max_deq=4) at
>>> odp_schedule.c:594
>>> #7  0x00417e67 in odp_schedule (out_queue=0x74a90e10,
>>> wait=18446744073709551615) at odp_schedule.c:626
>>> #8  0x0040321f in pktio_receive_thread (arg=0x77ff4000) at
>>> odp_classifier.c:304
>>> #9  0x0042900b in odp_run_start_routine (arg=0x6954e0) at
>>> linux.c:36
>>> #10 0x773a3182 in start_thread (arg=0x74a91700) at
>>> pthread_create.c:312
>>> #11 0x770d047d in clone () at
>>> ../sysdeps/unix/sysv/linux/x86_64/clone.S:111
>>> (gdb) p cos
>>> $1 = (cos_t *) 0x0
>>> (gdb)
>>>
>>>
>>> On 12/10/2015 18:12, Bala Manoharan wrote:

 odp_classifier runs on api-next branch but if we run on master it
 currently crashes during queue_enq function.
 I remember the crash was similar to the one we had during queue
 reorder implementation.

 Regards,
 Bala

 On 10 December 2015 at 20:38, Stuart Haslam 
 wrote:
>
> On Thu, Dec 10, 2015 at 06:01:05PM +0300, Maxim Uvarov wrote:
>>
>> How to reproduce that it is broken?
>>
> I was looking at:
>
> sudo ODP_PLATFORM=linux-generic ./test/performance/odp_l2fwd_run
>
> This shows a few packets going through initially then 0. Turns out this
> is related to the netmap pktio, this works:
>
> sudo ODP_PKTIO_DISABLE_NETMAP=y ODP_PLATFORM=linux-generic
> ./test/performance/odp_l2fwd_run
>
> Not sure if this is the same failure Bala was seeing.
>
> --
> Stuart.



>>>
>>
>
>
>
> --
> Regards,
> Bala



-- 
Regards,
Bala
___
lng-odp mailing list
lng-odp@lists.linaro.org

Re: [lng-odp] [API-NEXT PATCH v3] api: atomic: added atomic_lock_free_u64

2015-12-11 Thread Maxim Uvarov

Does your checkpatch warns on spelling? I will fix on apply.

WARNING: 'substract' may be misspelled - perhaps 'subtract'?
#43: FILE: include/odp/api/atomic.h:403:
+uint32_t fetch_sub : 1;  /**< Atomic fetch and substract */

WARNING: 'substract' may be misspelled - perhaps 'subtract'?
#44: FILE: include/odp/api/atomic.h:404:
+uint32_t sub   : 1;  /**< Atomic substract */


On 12/10/2015 20:22, Ola Liljedahl wrote:



On 26 November 2015 at 09:56, Savolainen, Petri (Nokia - FI/Espoo) 
> wrote:


ping.

> -Original Message-
> From: lng-odp [mailto:lng-odp-boun...@lists.linaro.org
] On Behalf Of EXT
> Petri Savolainen
> Sent: Thursday, November 12, 2015 10:30 AM
> To: lng-odp@lists.linaro.org 
> Subject: [lng-odp] [API-NEXT PATCH v3] api: atomic: added
> atomic_lock_free_u64
>
> Platforms may support some uint64 operations lock-free and
> others not. For example, inc_64 can be natively supported but
> cas_64 (or max_64/min_64) not. User may be able to switch to
> 32 bit variables when a 64 bit operation uses locks. The same
> atomic operation struture could be used for platform init to guide
> lock vs. lock-free implementation (e.g. if cas_64 is never
> called, inc_64 can be lock-free).
>
> Signed-off-by: Petri Savolainen >

Reviewed-by: Ola Liljedahl >


> ---
>  include/odp/api/atomic.h| 48
> +
>  platform/linux-generic/Makefile.am  |  1 +
>  platform/linux-generic/odp_atomic.c | 24 +++
>  3 files changed, 73 insertions(+)
>  create mode 100644 platform/linux-generic/odp_atomic.c
>
> diff --git a/include/odp/api/atomic.h b/include/odp/api/atomic.h
> index 316f13a..5e897c1 100644
> --- a/include/odp/api/atomic.h
> +++ b/include/odp/api/atomic.h
> @@ -388,6 +388,54 @@ void
odp_atomic_add_rls_u32(odp_atomic_u32_t *atom,
> uint32_t val);
>  void odp_atomic_sub_rls_u32(odp_atomic_u32_t *atom, uint32_t val);
>
>  /**
> + * Atomic operations
> + *
> + * Atomic operations listed in a bit field structure.
> + */
> +typedef union odp_atomic_op_t {
> + /** Operation flags */
> + struct {
> + uint32_t init  : 1;  /**< Init atomic variable */
> + uint32_t load  : 1;  /**< Atomic load */
> + uint32_t store : 1;  /**< Atomic store */
> + uint32_t fetch_add : 1;  /**< Atomic fetch and add */
> + uint32_t add   : 1;  /**< Atomic add */
> + uint32_t fetch_sub : 1;  /**< Atomic fetch and
substract */
> + uint32_t sub   : 1;  /**< Atomic substract */
> + uint32_t fetch_inc : 1;  /**< Atomic fetch and
increment */
> + uint32_t inc   : 1;  /**< Atomic increment */
> + uint32_t fetch_dec : 1;  /**< Atomic fetch and
decrement */
> + uint32_t dec   : 1;  /**< Atomic decrement */
> + uint32_t min   : 1;  /**< Atomic minimum */
> + uint32_t max   : 1;  /**< Atomic maximum */
> + uint32_t cas   : 1;  /**< Atomic compare and
swap */
> + } op;
> +
> + /** All bits of the bit field structure.
> +   * Operation flag mapping is architecture specific. This
field can
> be
> +   * used to set/clear all flags, or bitwise operations
over the
> entire
> +   * structure. */
> + uint32_t all_bits;
> +} odp_atomic_op_t;
> +
> +/**
> + * Query which atomic uint64 operations are lock-free
> + *
> + * Lock-free implementations have higher performance and scale
better
> than
> + * implementations using locks. User can decide to use e.g.
uint32 atomic
> + * variables instead of uint64 to optimize performance on
platforms that
> + * implement a performance critical operation using locks.
> + *
> + * @param atomic_op  Pointer to atomic operation structure for
storing
> + *   operation flags. All bits are initialized
to zero
> during
> + *   the operation. The parameter is ignored
when NULL.
> + * @retval 0 None of the operations are lock-free
> + * @retval 1 Some of the operations are lock-free
> + * @retval 2 All operations are lock-free
> + */
> +int odp_atomic_lock_free_u64(odp_atomic_op_t *atomic_op);
> +
> +/**
>   * @}
>   */
>
> diff --git a/platform/linux-generic/Makefile.am b/platform/linux-
> 

Re: [lng-odp] [PATCHv6] helper : Fix UDP checksum computation

2015-12-11 Thread Ola Liljedahl
Something else but related to the reliable computation of checksums.

If you look how odp_chksum() is used e.g. in the packet generator example
(pack_icmp_pkt function), there is a lot of pointer casting and writes to
the resulting IPv4 and ICMP structures. Then we call odp_chksum() (which is
likely inlined) which casts the base pointer again and reads from it. Since
pointers to different types cannot alias in C (they cannot point to the
same memory), the compiler is free to rearrange memory accesses and indeed
read from memory (in odp_chksum) before the data has been written. I think
I saw this cause problems earlier. What saves us now is probably the call
to gettimeofday() before calling odp_chksum() because the compiler will not
reorder memory accesses over unknown function calls (the memcpy after might
get inlined so I don't see it as a barrier). If the call to gettimeofday()
would be moved after the computation (or removed), I believe we could have
problems.

The proper solution is to use a union of all relevant data structures and
perform all memory accesses through this union. From a compiler perspective
we are only using one pointer to one type so it will know that all memory
accesses are related and it can understand the dependencies. But we can't
trust the caller of odp_chksum() to do that. Instead odp_chksum() should
include a compiler memory barrier which ensures that the compiler will not
reorder memory accesses (in either direction) over this barrier. It is the
inlining of odp_chksum() that enables the problem, if odp_chksum() was not
inlined, the compiler would not be able to reorder memory accesses over the
function call.

static inline uint16sum_t odp_chksum(void *buffer, int len)

{

uint16_t *buf = buffer;

__asm __volatile("" ::: "memory");
/* Now 'buf' can be safely dereferenced */

An alternative could be to define a macro for these type-unsafe casts that
include the compiler barrier. I am not sure how such a macro would look
like.

On 11 December 2015 at 10:47, Ilya Maximets  wrote:

> On 11.12.2015 12:19, Maxim Uvarov wrote:
> > On 12/11/2015 10:39, Ilya Maximets wrote:
> >> Comments inlined.
> >>
> >> On 10.12.2015 12:59, ion.grig...@freescale.com wrote:
> >>> From: Grigore Ion 
> >>>
> >>> This patch fixes the following problems:
> >>> - checksum computation for LE platforms
> >>> - checksum is computed in the CPU endianness. The returned result
> >>> must be converted to the BE ordering when it is used to update
> >>> the UDP checksum in a packet.
> >>> - checksum computation for packets having the UDP length not a
> >>> multiple of 2
> >>> - fixes the UDP checksum associated validation test
> >>>
> >>> Signed-off-by: Grigore Ion 
> >>> ---
> >>>   v6:
> >>>   - Make code more understandable (Ilya Maximets)
> >>>   v5:
> >>>   - Checksum in CPU endianness fix added (Ilya Maximets)
> >>>   v4:
> >>>   - Verify checksum in CPU endianness in the associated test
> >>>   (Ilya Maximets)
> >>>   v3:
> >>>   - fix the UDP checksum computation in the associated test
> >>>   (Maxim Uvarov)
> >>>   v2:
> >>>   - patch updated to the last master (Maxim Uvarov)
> >>>   v1:
> >>>   - Move variables declaration on top of block. (Maxim Uvarov)
> >>>   - Check patch with checkpatch script.  (Maxim Uvarov)
> >>>   - L3 header presence is tested twice. (Alexandru Badicioiu)
> >>>   - Remove unnecessary check for L3 header presence. (Bill Fischofer)
> >>>   - Modify check of odp_packet_l4_offset() return. (Bill Fischofer)
> >>>
> >>>   helper/include/odp/helper/udp.h | 81
> +
> >>>   helper/test/odp_chksum.c|  4 +-
> >>>   2 files changed, 51 insertions(+), 34 deletions(-)
> >>>
> >>> diff --git a/helper/include/odp/helper/udp.h
> b/helper/include/odp/helper/udp.h
> >>> index 06c439b..d0599b1 100644
> >>> --- a/helper/include/odp/helper/udp.h
> >>> +++ b/helper/include/odp/helper/udp.h
> >>> @@ -4,7 +4,6 @@
> >>>* SPDX-License-Identifier: BSD-3-Clause
> >>>*/
> >>>   -
> >>>   /**
> >>>* @file
> >>>*
> >>> @@ -22,7 +21,6 @@ extern "C" {
> >>>   #include 
> >>>   #include 
> >>>   -
> >>>   /** @addtogroup odph_header ODPH HEADER
> >>>*  @{
> >>>*/
> >>> @@ -39,51 +37,70 @@ typedef struct ODP_PACKED {
> >>>   } odph_udphdr_t;
> >>> /**
> >>> + * Perform byte swap required by UDP checksum computation algorithm
> >>> + */
> >>> +static inline uint16_t csum_bswap(uint16_t val)
> >>> +{
> >>> +#if ODP_BYTE_ORDER == ODP_LITTLE_ENDIAN
> >>> +val = __odp_builtin_bswap16((__odp_force uint16_t)val);
> >>> +#endif
> >>> +return val;
> >>> +}
> >> Please, don't duplicate the code.
> >>
> >>> +
> >>> +/**
> >>> + * Pad a byte value to the left or to the right as required by UDP
> checksum
> >>> + * computation algorithm and convert the result to CPU native
> uint16_t.
> >>> + * Left padding is performed for the IP protocol 

Re: [lng-odp] [API-NEXT PATCHv2] linux-generic: time: remove posix bleed through on odp_time_t

2015-12-11 Thread Maxim Uvarov

On 12/11/2015 16:30, Bill Fischofer wrote:



On Fri, Dec 11, 2015 at 7:27 AM, Maxim Uvarov > wrote:


On 12/11/2015 16:22, Bill Fischofer wrote:

The linux-generic implementation of odp_time_t makes use of POSIX
APIs that are sensitive to the _POSIX_C_SOURCE level. Use an
indirection
mechanism so that these dependencies do not "bleed through"
the ODP API.
This means that ODP applications can be independent of
_POSIX_C_SOURCE
level.

Signed-off-by: Bill Fischofer >
---
  .../linux-generic/include/odp/plat/time_types.h |  9 ---
  platform/linux-generic/odp_time.c  | 30
+++---
  2 files changed, 20 insertions(+), 19 deletions(-)

diff --git
a/platform/linux-generic/include/odp/plat/time_types.h
b/platform/linux-generic/include/odp/plat/time_types.h
index e5765ec..e999beb 100644
--- a/platform/linux-generic/include/odp/plat/time_types.h
+++ b/platform/linux-generic/include/odp/plat/time_types.h
@@ -21,11 +21,12 @@ extern "C" {
   *  @{
   **/
  -typedef struct timespec odp_time_t;
+typedef struct {
+   int64_t tv_sec;

time_t is not reachable?

Not sure I understand this comment.  Can you elaborate?


I think this patch is correct.  First argument might be time_t, but 
looks like it's the same size as everywhere.


Will hold patch for some small time to see of somebody have any objections.

Maxim.


+   int64_t tv_nsec;
+} odp_time_t;
  -odp_time_t odp_time_null(void);
-
-#define ODP_TIME_NULL  odp_time_null()
+#define ODP_TIME_NULL ((odp_time_t){0, 0})
/**
   * @}
diff --git a/platform/linux-generic/odp_time.c
b/platform/linux-generic/odp_time.c
index 1c7c214..9b64b37 100644
--- a/platform/linux-generic/odp_time.c
+++ b/platform/linux-generic/odp_time.c
@@ -11,7 +11,12 @@
  #include 
  #include 
  -static struct timespec start_time;
+typedef union {
+   odp_time_t  ex;
+   struct timespec in;
+} _odp_time_t;
+
+static odp_time_t start_time;
static inline
  uint64_t time_to_ns(odp_time_t time)
@@ -27,7 +32,7 @@ uint64_t time_to_ns(odp_time_t time)
  static inline
  odp_time_t time_diff(odp_time_t t2, odp_time_t t1)
  {
-   struct timespec time;
+   odp_time_t time;
time.tv_sec = t2.tv_sec - t1.tv_sec;
time.tv_nsec = t2.tv_nsec - t1.tv_nsec;
@@ -43,13 +48,13 @@ odp_time_t time_diff(odp_time_t t2,
odp_time_t t1)
  odp_time_t odp_time_local(void)
  {
int ret;
-   struct timespec time;
+   _odp_time_t time;
  - ret = clock_gettime(CLOCK_MONOTONIC_RAW, );
+   ret = clock_gettime(CLOCK_MONOTONIC_RAW, 
);
if (odp_unlikely(ret != 0))
ODP_ABORT("clock_gettime failed\n");
  - return time_diff(time, start_time);
+   return time_diff(time.ex, start_time);
  }
odp_time_t odp_time_diff(odp_time_t t2, odp_time_t t1)
@@ -64,7 +69,7 @@ uint64_t odp_time_to_ns(odp_time_t time)
odp_time_t odp_time_local_from_ns(uint64_t ns)
  {
-   struct timespec time;
+   odp_time_t time;
time.tv_sec = ns / ODP_TIME_SEC_IN_NS;
time.tv_nsec = ns - time.tv_sec * ODP_TIME_SEC_IN_NS;
@@ -85,7 +90,7 @@ int odp_time_cmp(odp_time_t t2, odp_time_t t1)
odp_time_t odp_time_sum(odp_time_t t1, odp_time_t t2)
  {
-   struct timespec time;
+   odp_time_t time;
time.tv_sec = t2.tv_sec + t1.tv_sec;
time.tv_nsec = t2.tv_nsec + t1.tv_nsec;
@@ -113,18 +118,13 @@ uint64_t odp_time_to_u64(odp_time_t time)
return time_to_ns(time) / resolution;
  }
  -odp_time_t odp_time_null(void)
-{
-   return (struct timespec) {0, 0};
-}
-
  int odp_time_global_init(void)
  {
int ret;
-   struct timespec time;
+   _odp_time_t time;
  - ret = clock_gettime(CLOCK_MONOTONIC_RAW, );
-   start_time = ret ? odp_time_null() : time;
+   ret = clock_gettime(CLOCK_MONOTONIC_RAW, 
);
+   start_time = ret ? ODP_TIME_NULL : time.ex;
return ret;
  }


___
lng-odp 

Re: [lng-odp] [PATCHv6] helper : Fix UDP checksum computation

2015-12-11 Thread Ion Grigore
Comments inlined

-Original Message-
From: Maxim Uvarov [mailto:maxim.uva...@linaro.org] 
Sent: Friday, December 11, 2015 11:20 AM
To: Ilya Maximets ; Grigore Ion-B17953 
; lng-odp@lists.linaro.org
Subject: Re: [PATCHv6] helper : Fix UDP checksum computation

On 12/11/2015 10:39, Ilya Maximets wrote:
> Comments inlined.
>
> On 10.12.2015 12:59, ion.grig...@freescale.com wrote:
>> From: Grigore Ion 
>>
>> This patch fixes the following problems:
>> - checksum computation for LE platforms
>> - checksum is computed in the CPU endianness. The returned result 
>> must be converted to the BE ordering when it is used to update the 
>> UDP checksum in a packet.
>> - checksum computation for packets having the UDP length not a 
>> multiple of 2
>> - fixes the UDP checksum associated validation test
>>
>> Signed-off-by: Grigore Ion 
>> ---
>>   v6:
>>   - Make code more understandable (Ilya Maximets)
>>   v5:
>>   - Checksum in CPU endianness fix added (Ilya Maximets)
>>   v4:
>>   - Verify checksum in CPU endianness in the associated test
>>   (Ilya Maximets)
>>   v3:
>>   - fix the UDP checksum computation in the associated test
>>   (Maxim Uvarov)
>>   v2:
>>   - patch updated to the last master (Maxim Uvarov)
>>   v1:
>>   - Move variables declaration on top of block. (Maxim Uvarov)
>>   - Check patch with checkpatch script.  (Maxim Uvarov)
>>   - L3 header presence is tested twice. (Alexandru Badicioiu)
>>   - Remove unnecessary check for L3 header presence. (Bill Fischofer)
>>   - Modify check of odp_packet_l4_offset() return. (Bill Fischofer)
>>
>>   helper/include/odp/helper/udp.h | 81 
>> +
>>   helper/test/odp_chksum.c|  4 +-
>>   2 files changed, 51 insertions(+), 34 deletions(-)
>>
>> diff --git a/helper/include/odp/helper/udp.h 
>> b/helper/include/odp/helper/udp.h index 06c439b..d0599b1 100644
>> --- a/helper/include/odp/helper/udp.h
>> +++ b/helper/include/odp/helper/udp.h
>> @@ -4,7 +4,6 @@
>>* SPDX-License-Identifier: BSD-3-Clause
>>*/
>>   
>> -
>>   /**
>>* @file
>>*
>> @@ -22,7 +21,6 @@ extern "C" {
>>   #include 
>>   #include 
>>   
>> -
>>   /** @addtogroup odph_header ODPH HEADER
>>*  @{
>>*/
>> @@ -39,51 +37,70 @@ typedef struct ODP_PACKED {
>>   } odph_udphdr_t;
>>   
>>   /**
>> + * Perform byte swap required by UDP checksum computation algorithm  
>> +*/ static inline uint16_t csum_bswap(uint16_t val) { #if 
>> +ODP_BYTE_ORDER == ODP_LITTLE_ENDIAN
>> +val = __odp_builtin_bswap16((__odp_force uint16_t)val); #endif
>> +return val;
>> +}
> Please, don't duplicate the code.
>
>> +
>> +/**
>> + * Pad a byte value to the left or to the right as required by UDP 
>> +checksum
>> + * computation algorithm and convert the result to CPU native uint16_t.
>> + * Left padding is performed for the IP protocol field in the UDP
>> + * pseudo-header (RFC 768). Right padding is performed in the case 
>> +of the odd
>> + * byte in a UDP packet having the length not a 2 multiple.
>> + */
>> +static inline uint16_t u8_pad_to_u16(uint8_t val, odp_bool_t left) {
>> +uint16_tret;
>> +
>> +ret = (left) ? val : val << 8;
>> +return csum_bswap(ret);
>> +}
>> +
>> +/**
>>* UDP checksum
>>*
>>* This function uses odp packet to calc checksum
>>*
>>* @param pkt  calculate chksum for pkt
>> - * @return  checksum value
>> + * @return  checksum value in CPU endianness
>>*/
>>   static inline uint16_t odph_ipv4_udp_chksum(odp_packet_t pkt)
>>   {
>> -uint32_t sum = 0;
>> -odph_udphdr_t *udph;
>> -odph_ipv4hdr_t *iph;
>> -uint16_t udplen;
>> -uint8_t *buf;
>> -
>> -if (!odp_packet_l3_offset(pkt))
>> -return 0;
>> +odph_ipv4hdr_t  *iph;
>> +odph_udphdr_t   *udph;
>> +uint32_tsum;
>> +uint16_tudplen, *buf;
>>   
>> -if (!odp_packet_l4_offset(pkt))
>> +if (odp_packet_l4_offset(pkt) == ODP_PACKET_OFFSET_INVALID)
>>  return 0;
>> -
>>  iph = (odph_ipv4hdr_t *)odp_packet_l3_ptr(pkt, NULL);
>>  udph = (odph_udphdr_t *)odp_packet_l4_ptr(pkt, NULL);
>> -udplen = odp_be_to_cpu_16(udph->length);
>> -
>> -/* 32-bit sum of all 16-bit words covered by UDP chksum */
>> +/* 32-bit sum of UDP pseudo-header */
>>  sum = (iph->src_addr & 0x) + (iph->src_addr >> 16) +
>> -  (iph->dst_addr & 0x) + (iph->dst_addr >> 16) +
>> -  (uint16_t)iph->proto + udplen;
>> -for (buf = (uint8_t *)udph; udplen > 1; udplen -= 2) {
>> -sum += ((*buf << 8) + *(buf + 1));
>> -buf += 2;
>> -}
>> -
>> -/* Fold sum to 16 bits: add carrier to result */
>> -while (sum >> 16)
>> -sum = (sum & 0x) + (sum >> 16);
>> -
>> +(iph->dst_addr & 0x) + (iph->dst_addr >> 16) +
>> +u8_pad_to_u16(iph->proto, 1) + 

Re: [lng-odp] [API-NEXT PATCHv2] linux-generic: time: remove posix bleed through on odp_time_t

2015-12-11 Thread Bill Fischofer
On Fri, Dec 11, 2015 at 7:27 AM, Maxim Uvarov 
wrote:

> On 12/11/2015 16:22, Bill Fischofer wrote:
>
>> The linux-generic implementation of odp_time_t makes use of POSIX
>> APIs that are sensitive to the _POSIX_C_SOURCE level. Use an indirection
>> mechanism so that these dependencies do not "bleed through" the ODP API.
>> This means that ODP applications can be independent of _POSIX_C_SOURCE
>> level.
>>
>> Signed-off-by: Bill Fischofer 
>> ---
>>   .../linux-generic/include/odp/plat/time_types.h|  9 ---
>>   platform/linux-generic/odp_time.c  | 30
>> +++---
>>   2 files changed, 20 insertions(+), 19 deletions(-)
>>
>> diff --git a/platform/linux-generic/include/odp/plat/time_types.h
>> b/platform/linux-generic/include/odp/plat/time_types.h
>> index e5765ec..e999beb 100644
>> --- a/platform/linux-generic/include/odp/plat/time_types.h
>> +++ b/platform/linux-generic/include/odp/plat/time_types.h
>> @@ -21,11 +21,12 @@ extern "C" {
>>*  @{
>>**/
>>   -typedef struct timespec odp_time_t;
>> +typedef struct {
>> +   int64_t tv_sec;
>>
> time_t is not reachable?
>
> Not sure I understand this comment.  Can you elaborate?


> +   int64_t tv_nsec;
>> +} odp_time_t;
>>   -odp_time_t odp_time_null(void);
>> -
>> -#define ODP_TIME_NULL  odp_time_null()
>> +#define ODP_TIME_NULL ((odp_time_t){0, 0})
>> /**
>>* @}
>> diff --git a/platform/linux-generic/odp_time.c
>> b/platform/linux-generic/odp_time.c
>> index 1c7c214..9b64b37 100644
>> --- a/platform/linux-generic/odp_time.c
>> +++ b/platform/linux-generic/odp_time.c
>> @@ -11,7 +11,12 @@
>>   #include 
>>   #include 
>>   -static struct timespec start_time;
>> +typedef union {
>> +   odp_time_t  ex;
>> +   struct timespec in;
>> +} _odp_time_t;
>> +
>> +static odp_time_t start_time;
>> static inline
>>   uint64_t time_to_ns(odp_time_t time)
>> @@ -27,7 +32,7 @@ uint64_t time_to_ns(odp_time_t time)
>>   static inline
>>   odp_time_t time_diff(odp_time_t t2, odp_time_t t1)
>>   {
>> -   struct timespec time;
>> +   odp_time_t time;
>> time.tv_sec = t2.tv_sec - t1.tv_sec;
>> time.tv_nsec = t2.tv_nsec - t1.tv_nsec;
>> @@ -43,13 +48,13 @@ odp_time_t time_diff(odp_time_t t2, odp_time_t t1)
>>   odp_time_t odp_time_local(void)
>>   {
>> int ret;
>> -   struct timespec time;
>> +   _odp_time_t time;
>>   - ret = clock_gettime(CLOCK_MONOTONIC_RAW, );
>> +   ret = clock_gettime(CLOCK_MONOTONIC_RAW, );
>> if (odp_unlikely(ret != 0))
>> ODP_ABORT("clock_gettime failed\n");
>>   - return time_diff(time, start_time);
>> +   return time_diff(time.ex, start_time);
>>   }
>> odp_time_t odp_time_diff(odp_time_t t2, odp_time_t t1)
>> @@ -64,7 +69,7 @@ uint64_t odp_time_to_ns(odp_time_t time)
>> odp_time_t odp_time_local_from_ns(uint64_t ns)
>>   {
>> -   struct timespec time;
>> +   odp_time_t time;
>> time.tv_sec = ns / ODP_TIME_SEC_IN_NS;
>> time.tv_nsec = ns - time.tv_sec * ODP_TIME_SEC_IN_NS;
>> @@ -85,7 +90,7 @@ int odp_time_cmp(odp_time_t t2, odp_time_t t1)
>> odp_time_t odp_time_sum(odp_time_t t1, odp_time_t t2)
>>   {
>> -   struct timespec time;
>> +   odp_time_t time;
>> time.tv_sec = t2.tv_sec + t1.tv_sec;
>> time.tv_nsec = t2.tv_nsec + t1.tv_nsec;
>> @@ -113,18 +118,13 @@ uint64_t odp_time_to_u64(odp_time_t time)
>> return time_to_ns(time) / resolution;
>>   }
>>   -odp_time_t odp_time_null(void)
>> -{
>> -   return (struct timespec) {0, 0};
>> -}
>> -
>>   int odp_time_global_init(void)
>>   {
>> int ret;
>> -   struct timespec time;
>> +   _odp_time_t time;
>>   - ret = clock_gettime(CLOCK_MONOTONIC_RAW, );
>> -   start_time = ret ? odp_time_null() : time;
>> +   ret = clock_gettime(CLOCK_MONOTONIC_RAW, );
>> +   start_time = ret ? ODP_TIME_NULL : time.ex;
>> return ret;
>>   }
>>
>
> ___
> lng-odp mailing list
> lng-odp@lists.linaro.org
> https://lists.linaro.org/mailman/listinfo/lng-odp
>
___
lng-odp mailing list
lng-odp@lists.linaro.org
https://lists.linaro.org/mailman/listinfo/lng-odp


Re: [lng-odp] [API-NEXT PATCH v2 4/5] linux-generic: netmap: add multi queue support

2015-12-11 Thread Elo, Matias (Nokia - FI/Espoo)
> -Original Message-
> From: EXT Stuart Haslam [mailto:stuart.has...@linaro.org]
> Sent: Friday, December 11, 2015 2:25 PM
> To: Elo, Matias (Nokia - FI/Espoo) 
> Cc: lng-odp@lists.linaro.org
> Subject: Re: [lng-odp] [API-NEXT PATCH v2 4/5] linux-generic: netmap: add 
> multi
> queue support
> 
> On Wed, Nov 25, 2015 at 03:15:52PM +0200, Matias Elo wrote:
> > Added multi queue support to netmap pktio.
> >
> > Signed-off-by: Matias Elo 
> > ---
> >  platform/linux-generic/include/odp_packet_netmap.h |  34 +-
> >  platform/linux-generic/pktio/netmap.c  | 426 
> > ++---
> >  2 files changed, 406 insertions(+), 54 deletions(-)
> >
> > diff --git a/platform/linux-generic/include/odp_packet_netmap.h
> b/platform/linux-generic/include/odp_packet_netmap.h
> > index a088d23..b38363a 100644
> > --- a/platform/linux-generic/include/odp_packet_netmap.h
> > +++ b/platform/linux-generic/include/odp_packet_netmap.h
> > @@ -7,23 +7,53 @@
> >  #ifndef ODP_PACKET_NETMAP_H
> >  #define ODP_PACKET_NETMAP_H
> >
> > +#include 
> > +#include 
> >  #include 
> >  #include 
> > +#include 
> > +#include 
> >
> >  #include 
> >  #include 
> >
> > +#define NM_MAX_DESC 32
> > +#define NM_MAX_INPUT_QUEUES 32
> > +#define NM_MAX_OUTPUT_QUEUES 32
> > +
> > +/** Ring for mapping pktin/pktout queues to netmap descriptors */
> > +struct netmap_ring_t {
> > +   unsigned first; /**< Index of first netmap descriptor */
> > +   unsigned last;  /**< Index of last netmap descriptor */
> > +   unsigned num;   /**< Number of netmap descriptors */
> > +   /** Netmap metadata for the device */
> > +   struct nm_desc *desc[NM_MAX_DESC];
> > +   unsigned cur;   /**< Index of current netmap descriptor */
> > +   odp_ticketlock_t lock;  /**< Queue lock */
> > +};
> > +
> > +typedef union {
> > +   struct netmap_ring_t s;
> > +   uint8_t pad[ODP_CACHE_LINE_SIZE_ROUNDUP(sizeof(struct
> netmap_ring_t))];
> > +} netmap_ring_t ODP_ALIGNED_CACHE;
> > +
> >  /** Packet socket using netmap mmaped rings for both Rx and Tx */
> >  typedef struct {
> > odp_pool_t pool;/**< pool to alloc packets from */
> > size_t max_frame_len;   /**< buf_size - sizeof(pkt_hdr) */
> > -   struct nm_desc *rx_desc;/**< netmap meta-data for the device */
> > -   struct nm_desc *tx_desc;/**< netmap meta-data for the device */
> > uint32_t if_flags;  /**< interface flags */
> > int sockfd; /**< control socket */
> > unsigned char if_mac[ETH_ALEN]; /**< eth mac address */
> > char nm_name[IF_NAMESIZE + 7];  /**< netmap: */
> > odp_pktio_capability_t  capa;   /**< interface capabilities */
> > +   unsigned num_rx_queues; /**< number of pktin queues */
> > +   unsigned num_tx_queues; /**< number of pktout queues */
> > +   odp_bool_t lockless_rx; /**< no locking for rx */
> > +   odp_bool_t lockless_tx; /**< no locking for tx */
> > +   /** mapping of pktin queues to netmap rx descriptors */
> > +   netmap_ring_t rx_desc_ring[NM_MAX_INPUT_QUEUES];
> > +   /** mapping of pktout queues to netmap tx descriptors */
> > +   netmap_ring_t tx_desc_ring[NM_MAX_OUTPUT_QUEUES];
> >  } pkt_netmap_t;
> >
> >  #endif
> > diff --git a/platform/linux-generic/pktio/netmap.c b/platform/linux-
> generic/pktio/netmap.c
> > index 313bcbe..236b48c 100644
> > --- a/platform/linux-generic/pktio/netmap.c
> > +++ b/platform/linux-generic/pktio/netmap.c
> > @@ -75,15 +75,146 @@ done:
> > return err;
> >  }
> >
> > +/**
> > + * Map netmap rings to pktin/pktout queues
> > + *
> > + * @param rings  Array of netmap descriptor rings
> > + * @param num_queues Number of pktin/pktout queues
> > + * @param num_rings  Number of matching netmap rings
> > + */
> > +static inline void map_netmap_rings(netmap_ring_t *rings,
> > +   unsigned num_queues, unsigned num_rings)
> > +{
> > +   struct netmap_ring_t *desc_ring;
> > +   unsigned rings_per_queue;
> > +   unsigned remainder;
> > +   unsigned mapped_rings;
> > +   unsigned i;
> > +   unsigned desc_id = 0;
> > +
> > +   rings_per_queue = num_rings / num_queues;
> > +   remainder = num_rings % num_queues;
> > +
> > +   if (remainder)
> > +   ODP_DBG("WARNING: Netmap rings mapped unevenly to
> queues\n");
> > +
> > +   for (i = 0; i < num_queues; i++) {
> > +   desc_ring = [i].s;
> > +   if (i < remainder)
> > +   mapped_rings = rings_per_queue + 1;
> > +   else
> > +   mapped_rings = rings_per_queue;
> > +
> > +   desc_ring->first = desc_id;
> > +   desc_ring->cur = desc_id;
> > +   desc_ring->last = desc_ring->first + mapped_rings - 1;
> > +   desc_ring->num = mapped_rings;
> > +
> > +   desc_id = desc_ring->last + 1;
> > +   }
> > +}
> > +
> > +static int netmap_input_queues_config(pktio_entry_t *pktio_entry,

Re: [lng-odp] [PATCH] api: pktio link status

2015-12-11 Thread Alexandru Badicioiu
Wouldn't be the application code more clear if the function is renamed to
_link_up()?
Return values clearly suggest this. The application has to remember that 1
means link up and 0 means link down.

Alex


On 11 December 2015 at 14:44, Maxim Uvarov  wrote:

> Signed-off-by: Maxim Uvarov 
> ---
>  v1 was link_state(), now it's link_status().
>
>  include/odp/api/packet_io.h | 11 +++
>  1 file changed, 11 insertions(+)
>
> diff --git a/include/odp/api/packet_io.h b/include/odp/api/packet_io.h
> index 443841e..8999b2c 100644
> --- a/include/odp/api/packet_io.h
> +++ b/include/odp/api/packet_io.h
> @@ -673,6 +673,17 @@ void
> odp_pktio_output_queue_param_init(odp_pktio_output_queue_param_t *param);
>  void odp_pktio_print(odp_pktio_t pktio);
>
>  /**
> + * Determine pktio link is up or down for a packet IO interface.
> + *
> + * @param pktio Packet IO handle.
> + *
> + * @retval  1 link is up
> + * @retval  0 link is down
> + * @retval <0 on failure
> +*/
> +int odp_pktio_link_status(odp_pktio_t pktio);
> +
> +/**
>   * @}
>   */
>
> --
> 1.9.1
>
> ___
> lng-odp mailing list
> lng-odp@lists.linaro.org
> https://lists.linaro.org/mailman/listinfo/lng-odp
>
___
lng-odp mailing list
lng-odp@lists.linaro.org
https://lists.linaro.org/mailman/listinfo/lng-odp


Re: [lng-odp] [API-NEXT PATCHv2] linux-generic: time: remove posix bleed through on odp_time_t

2015-12-11 Thread Maxim Uvarov

On 12/11/2015 16:22, Bill Fischofer wrote:

The linux-generic implementation of odp_time_t makes use of POSIX
APIs that are sensitive to the _POSIX_C_SOURCE level. Use an indirection
mechanism so that these dependencies do not "bleed through" the ODP API.
This means that ODP applications can be independent of _POSIX_C_SOURCE
level.

Signed-off-by: Bill Fischofer 
---
  .../linux-generic/include/odp/plat/time_types.h|  9 ---
  platform/linux-generic/odp_time.c  | 30 +++---
  2 files changed, 20 insertions(+), 19 deletions(-)

diff --git a/platform/linux-generic/include/odp/plat/time_types.h 
b/platform/linux-generic/include/odp/plat/time_types.h
index e5765ec..e999beb 100644
--- a/platform/linux-generic/include/odp/plat/time_types.h
+++ b/platform/linux-generic/include/odp/plat/time_types.h
@@ -21,11 +21,12 @@ extern "C" {
   *  @{
   **/
  
-typedef struct timespec odp_time_t;

+typedef struct {
+   int64_t tv_sec;

time_t is not reachable?

+   int64_t tv_nsec;
+} odp_time_t;
  
-odp_time_t odp_time_null(void);

-
-#define ODP_TIME_NULL  odp_time_null()
+#define ODP_TIME_NULL ((odp_time_t){0, 0})
  
  /**

   * @}
diff --git a/platform/linux-generic/odp_time.c 
b/platform/linux-generic/odp_time.c
index 1c7c214..9b64b37 100644
--- a/platform/linux-generic/odp_time.c
+++ b/platform/linux-generic/odp_time.c
@@ -11,7 +11,12 @@
  #include 
  #include 
  
-static struct timespec start_time;

+typedef union {
+   odp_time_t  ex;
+   struct timespec in;
+} _odp_time_t;
+
+static odp_time_t start_time;
  
  static inline

  uint64_t time_to_ns(odp_time_t time)
@@ -27,7 +32,7 @@ uint64_t time_to_ns(odp_time_t time)
  static inline
  odp_time_t time_diff(odp_time_t t2, odp_time_t t1)
  {
-   struct timespec time;
+   odp_time_t time;
  
  	time.tv_sec = t2.tv_sec - t1.tv_sec;

time.tv_nsec = t2.tv_nsec - t1.tv_nsec;
@@ -43,13 +48,13 @@ odp_time_t time_diff(odp_time_t t2, odp_time_t t1)
  odp_time_t odp_time_local(void)
  {
int ret;
-   struct timespec time;
+   _odp_time_t time;
  
-	ret = clock_gettime(CLOCK_MONOTONIC_RAW, );

+   ret = clock_gettime(CLOCK_MONOTONIC_RAW, );
if (odp_unlikely(ret != 0))
ODP_ABORT("clock_gettime failed\n");
  
-	return time_diff(time, start_time);

+   return time_diff(time.ex, start_time);
  }
  
  odp_time_t odp_time_diff(odp_time_t t2, odp_time_t t1)

@@ -64,7 +69,7 @@ uint64_t odp_time_to_ns(odp_time_t time)
  
  odp_time_t odp_time_local_from_ns(uint64_t ns)

  {
-   struct timespec time;
+   odp_time_t time;
  
  	time.tv_sec = ns / ODP_TIME_SEC_IN_NS;

time.tv_nsec = ns - time.tv_sec * ODP_TIME_SEC_IN_NS;
@@ -85,7 +90,7 @@ int odp_time_cmp(odp_time_t t2, odp_time_t t1)
  
  odp_time_t odp_time_sum(odp_time_t t1, odp_time_t t2)

  {
-   struct timespec time;
+   odp_time_t time;
  
  	time.tv_sec = t2.tv_sec + t1.tv_sec;

time.tv_nsec = t2.tv_nsec + t1.tv_nsec;
@@ -113,18 +118,13 @@ uint64_t odp_time_to_u64(odp_time_t time)
return time_to_ns(time) / resolution;
  }
  
-odp_time_t odp_time_null(void)

-{
-   return (struct timespec) {0, 0};
-}
-
  int odp_time_global_init(void)
  {
int ret;
-   struct timespec time;
+   _odp_time_t time;
  
-	ret = clock_gettime(CLOCK_MONOTONIC_RAW, );

-   start_time = ret ? odp_time_null() : time;
+   ret = clock_gettime(CLOCK_MONOTONIC_RAW, );
+   start_time = ret ? ODP_TIME_NULL : time.ex;
  
  	return ret;

  }


___
lng-odp mailing list
lng-odp@lists.linaro.org
https://lists.linaro.org/mailman/listinfo/lng-odp


Re: [lng-odp] [PATCH] example: classifier: fix add queue param init call

2015-12-11 Thread Maxim Uvarov

Merged,
Maxim.

On 12/11/2015 13:03, Stuart Haslam wrote:

On Fri, Dec 11, 2015 at 02:46:28PM +0530, Balasubramanian Manoharan wrote:

Fixes crash caused by queue param not being initialized.

Signed-off-by: Balasubramanian Manoharan 

That fixed it..

Reviewed-by: Stuart Haslam 


---
  example/classifier/odp_classifier.c | 1 +
  1 file changed, 1 insertion(+)

diff --git a/example/classifier/odp_classifier.c 
b/example/classifier/odp_classifier.c
index c8f264f..81e6bf0 100644
--- a/example/classifier/odp_classifier.c
+++ b/example/classifier/odp_classifier.c
@@ -401,6 +401,7 @@ static void configure_cos_queue(odp_pktio_t pktio, 
appl_args_t *args)
};
  
  		stats->pmr = odp_pmr_create();

+   odp_queue_param_init();
qparam.sched.prio = i % odp_schedule_num_prio();
qparam.sched.sync = ODP_SCHED_SYNC_NONE;
qparam.sched.group = ODP_SCHED_GROUP_ALL;
--
1.9.1

___
lng-odp mailing list
lng-odp@lists.linaro.org
https://lists.linaro.org/mailman/listinfo/lng-odp


___
lng-odp mailing list
lng-odp@lists.linaro.org
https://lists.linaro.org/mailman/listinfo/lng-odp


Re: [lng-odp] [PATCHv6] helper : Fix UDP checksum computation

2015-12-11 Thread Ola Liljedahl
On 11 December 2015 at 15:07, Ion Grigore  wrote:

> Comments inlined
>
> -Original Message-
> From: Maxim Uvarov [mailto:maxim.uva...@linaro.org]
> Sent: Friday, December 11, 2015 11:20 AM
> To: Ilya Maximets ; Grigore Ion-B17953 <
> ion.grig...@freescale.com>; lng-odp@lists.linaro.org
> Subject: Re: [PATCHv6] helper : Fix UDP checksum computation
>
> On 12/11/2015 10:39, Ilya Maximets wrote:
> > Comments inlined.
> >
> > On 10.12.2015 12:59, ion.grig...@freescale.com wrote:
> >> From: Grigore Ion 
> >>
> >> This patch fixes the following problems:
> >> - checksum computation for LE platforms
> >> - checksum is computed in the CPU endianness. The returned result
> >> must be converted to the BE ordering when it is used to update the
> >> UDP checksum in a packet.
> >> - checksum computation for packets having the UDP length not a
> >> multiple of 2
> >> - fixes the UDP checksum associated validation test
> >>
> >> Signed-off-by: Grigore Ion 
> >> ---
> >>   v6:
> >>   - Make code more understandable (Ilya Maximets)
> >>   v5:
> >>   - Checksum in CPU endianness fix added (Ilya Maximets)
> >>   v4:
> >>   - Verify checksum in CPU endianness in the associated test
> >>   (Ilya Maximets)
> >>   v3:
> >>   - fix the UDP checksum computation in the associated test
> >>   (Maxim Uvarov)
> >>   v2:
> >>   - patch updated to the last master (Maxim Uvarov)
> >>   v1:
> >>   - Move variables declaration on top of block. (Maxim Uvarov)
> >>   - Check patch with checkpatch script.  (Maxim Uvarov)
> >>   - L3 header presence is tested twice. (Alexandru Badicioiu)
> >>   - Remove unnecessary check for L3 header presence. (Bill Fischofer)
> >>   - Modify check of odp_packet_l4_offset() return. (Bill Fischofer)
> >>
> >>   helper/include/odp/helper/udp.h | 81
> +
> >>   helper/test/odp_chksum.c|  4 +-
> >>   2 files changed, 51 insertions(+), 34 deletions(-)
> >>
> >> diff --git a/helper/include/odp/helper/udp.h
> >> b/helper/include/odp/helper/udp.h index 06c439b..d0599b1 100644
> >> --- a/helper/include/odp/helper/udp.h
> >> +++ b/helper/include/odp/helper/udp.h
> >> @@ -4,7 +4,6 @@
> >>* SPDX-License-Identifier: BSD-3-Clause
> >>*/
> >>
> >> -
> >>   /**
> >>* @file
> >>*
> >> @@ -22,7 +21,6 @@ extern "C" {
> >>   #include 
> >>   #include 
> >>
> >> -
> >>   /** @addtogroup odph_header ODPH HEADER
> >>*  @{
> >>*/
> >> @@ -39,51 +37,70 @@ typedef struct ODP_PACKED {
> >>   } odph_udphdr_t;
> >>
> >>   /**
> >> + * Perform byte swap required by UDP checksum computation algorithm
> >> +*/ static inline uint16_t csum_bswap(uint16_t val) { #if
> >> +ODP_BYTE_ORDER == ODP_LITTLE_ENDIAN
> >> +val = __odp_builtin_bswap16((__odp_force uint16_t)val); #endif
> >> +return val;
> >> +}
> > Please, don't duplicate the code.
> >
> >> +
> >> +/**
> >> + * Pad a byte value to the left or to the right as required by UDP
> >> +checksum
> >> + * computation algorithm and convert the result to CPU native uint16_t.
> >> + * Left padding is performed for the IP protocol field in the UDP
> >> + * pseudo-header (RFC 768). Right padding is performed in the case
> >> +of the odd
> >> + * byte in a UDP packet having the length not a 2 multiple.
> >> + */
> >> +static inline uint16_t u8_pad_to_u16(uint8_t val, odp_bool_t left) {
> >> +uint16_tret;
> >> +
> >> +ret = (left) ? val : val << 8;
> >> +return csum_bswap(ret);
> >> +}
> >> +
> >> +/**
> >>* UDP checksum
> >>*
> >>* This function uses odp packet to calc checksum
> >>*
> >>* @param pkt  calculate chksum for pkt
> >> - * @return  checksum value
> >> + * @return  checksum value in CPU endianness
> >>*/
> >>   static inline uint16_t odph_ipv4_udp_chksum(odp_packet_t pkt)
> >>   {
> >> -uint32_t sum = 0;
> >> -odph_udphdr_t *udph;
> >> -odph_ipv4hdr_t *iph;
> >> -uint16_t udplen;
> >> -uint8_t *buf;
> >> -
> >> -if (!odp_packet_l3_offset(pkt))
> >> -return 0;
> >> +odph_ipv4hdr_t  *iph;
> >> +odph_udphdr_t   *udph;
> >> +uint32_tsum;
> >> +uint16_tudplen, *buf;
> >>
> >> -if (!odp_packet_l4_offset(pkt))
> >> +if (odp_packet_l4_offset(pkt) == ODP_PACKET_OFFSET_INVALID)
> >>  return 0;
> >> -
> >>  iph = (odph_ipv4hdr_t *)odp_packet_l3_ptr(pkt, NULL);
> >>  udph = (odph_udphdr_t *)odp_packet_l4_ptr(pkt, NULL);
> >> -udplen = odp_be_to_cpu_16(udph->length);
> >> -
> >> -/* 32-bit sum of all 16-bit words covered by UDP chksum */
> >> +/* 32-bit sum of UDP pseudo-header */
> >>  sum = (iph->src_addr & 0x) + (iph->src_addr >> 16) +
> >> -  (iph->dst_addr & 0x) + (iph->dst_addr >> 16) +
> >> -  (uint16_t)iph->proto + udplen;
> >> -for (buf = (uint8_t *)udph; udplen > 1; udplen -= 2) {
> >> -sum += ((*buf << 8) 

[lng-odp] [API-NEXT PATCHv2] linux-generic: time: remove posix bleed through on odp_time_t

2015-12-11 Thread Bill Fischofer
The linux-generic implementation of odp_time_t makes use of POSIX
APIs that are sensitive to the _POSIX_C_SOURCE level. Use an indirection
mechanism so that these dependencies do not "bleed through" the ODP API.
This means that ODP applications can be independent of _POSIX_C_SOURCE
level.

Signed-off-by: Bill Fischofer 
---
 .../linux-generic/include/odp/plat/time_types.h|  9 ---
 platform/linux-generic/odp_time.c  | 30 +++---
 2 files changed, 20 insertions(+), 19 deletions(-)

diff --git a/platform/linux-generic/include/odp/plat/time_types.h 
b/platform/linux-generic/include/odp/plat/time_types.h
index e5765ec..e999beb 100644
--- a/platform/linux-generic/include/odp/plat/time_types.h
+++ b/platform/linux-generic/include/odp/plat/time_types.h
@@ -21,11 +21,12 @@ extern "C" {
  *  @{
  **/
 
-typedef struct timespec odp_time_t;
+typedef struct {
+   int64_t tv_sec;
+   int64_t tv_nsec;
+} odp_time_t;
 
-odp_time_t odp_time_null(void);
-
-#define ODP_TIME_NULL  odp_time_null()
+#define ODP_TIME_NULL ((odp_time_t){0, 0})
 
 /**
  * @}
diff --git a/platform/linux-generic/odp_time.c 
b/platform/linux-generic/odp_time.c
index 1c7c214..9b64b37 100644
--- a/platform/linux-generic/odp_time.c
+++ b/platform/linux-generic/odp_time.c
@@ -11,7 +11,12 @@
 #include 
 #include 
 
-static struct timespec start_time;
+typedef union {
+   odp_time_t  ex;
+   struct timespec in;
+} _odp_time_t;
+
+static odp_time_t start_time;
 
 static inline
 uint64_t time_to_ns(odp_time_t time)
@@ -27,7 +32,7 @@ uint64_t time_to_ns(odp_time_t time)
 static inline
 odp_time_t time_diff(odp_time_t t2, odp_time_t t1)
 {
-   struct timespec time;
+   odp_time_t time;
 
time.tv_sec = t2.tv_sec - t1.tv_sec;
time.tv_nsec = t2.tv_nsec - t1.tv_nsec;
@@ -43,13 +48,13 @@ odp_time_t time_diff(odp_time_t t2, odp_time_t t1)
 odp_time_t odp_time_local(void)
 {
int ret;
-   struct timespec time;
+   _odp_time_t time;
 
-   ret = clock_gettime(CLOCK_MONOTONIC_RAW, );
+   ret = clock_gettime(CLOCK_MONOTONIC_RAW, );
if (odp_unlikely(ret != 0))
ODP_ABORT("clock_gettime failed\n");
 
-   return time_diff(time, start_time);
+   return time_diff(time.ex, start_time);
 }
 
 odp_time_t odp_time_diff(odp_time_t t2, odp_time_t t1)
@@ -64,7 +69,7 @@ uint64_t odp_time_to_ns(odp_time_t time)
 
 odp_time_t odp_time_local_from_ns(uint64_t ns)
 {
-   struct timespec time;
+   odp_time_t time;
 
time.tv_sec = ns / ODP_TIME_SEC_IN_NS;
time.tv_nsec = ns - time.tv_sec * ODP_TIME_SEC_IN_NS;
@@ -85,7 +90,7 @@ int odp_time_cmp(odp_time_t t2, odp_time_t t1)
 
 odp_time_t odp_time_sum(odp_time_t t1, odp_time_t t2)
 {
-   struct timespec time;
+   odp_time_t time;
 
time.tv_sec = t2.tv_sec + t1.tv_sec;
time.tv_nsec = t2.tv_nsec + t1.tv_nsec;
@@ -113,18 +118,13 @@ uint64_t odp_time_to_u64(odp_time_t time)
return time_to_ns(time) / resolution;
 }
 
-odp_time_t odp_time_null(void)
-{
-   return (struct timespec) {0, 0};
-}
-
 int odp_time_global_init(void)
 {
int ret;
-   struct timespec time;
+   _odp_time_t time;
 
-   ret = clock_gettime(CLOCK_MONOTONIC_RAW, );
-   start_time = ret ? odp_time_null() : time;
+   ret = clock_gettime(CLOCK_MONOTONIC_RAW, );
+   start_time = ret ? ODP_TIME_NULL : time.ex;
 
return ret;
 }
-- 
2.1.4

___
lng-odp mailing list
lng-odp@lists.linaro.org
https://lists.linaro.org/mailman/listinfo/lng-odp


Re: [lng-odp] [PATCH] api: pktio link status

2015-12-11 Thread Savolainen, Petri (Nokia - FI/Espoo)
Mail subject should be: [API-NEXT PATCH] api: pktio: added link status

Otherwise OK. Could you update the git log entry before merging: "api: pktio: 
added link status"

Reviewed-by: Petri Savolainen 


> -Original Message-
> From: lng-odp [mailto:lng-odp-boun...@lists.linaro.org] On Behalf Of EXT
> Maxim Uvarov
> Sent: Friday, December 11, 2015 2:45 PM
> To: lng-odp@lists.linaro.org
> Subject: [lng-odp] [PATCH] api: pktio link status
> 
> Signed-off-by: Maxim Uvarov 
> ---
>  v1 was link_state(), now it's link_status().
> 
>  include/odp/api/packet_io.h | 11 +++
>  1 file changed, 11 insertions(+)
> 
> diff --git a/include/odp/api/packet_io.h b/include/odp/api/packet_io.h
> index 443841e..8999b2c 100644
> --- a/include/odp/api/packet_io.h
> +++ b/include/odp/api/packet_io.h
> @@ -673,6 +673,17 @@ void
> odp_pktio_output_queue_param_init(odp_pktio_output_queue_param_t *param);
>  void odp_pktio_print(odp_pktio_t pktio);
> 
>  /**
> + * Determine pktio link is up or down for a packet IO interface.
> + *
> + * @param pktio Packet IO handle.
> + *
> + * @retval  1 link is up
> + * @retval  0 link is down
> + * @retval <0 on failure
> +*/
> +int odp_pktio_link_status(odp_pktio_t pktio);
> +
> +/**
>   * @}
>   */
> 
> --
> 1.9.1
> 
> ___
> lng-odp mailing list
> lng-odp@lists.linaro.org
> https://lists.linaro.org/mailman/listinfo/lng-odp
___
lng-odp mailing list
lng-odp@lists.linaro.org
https://lists.linaro.org/mailman/listinfo/lng-odp


[lng-odp] Deprecation of APIs

2015-12-11 Thread Bill Fischofer
I wanted to bring up a point that is going to become increasingly important
in 2016 and beyond. To date as we've been polishing up the ODP API we've
been somewhat abrupt in removing deprecated APIs. Now that we have key
initiatives like OFP using ODP, that needs to change.

ODP has had the ODP_DEPRECATED attribute (in include/odp/api/hints.h) for
some time, however we've not been using it.

I'd like to propose that this change starting in 2016.  Whenever an API is
changed (as part of an API-NEXT patch) we should do the following:

1. The change should maintain the existing API, which should now be marked
ODP_DEPRECATED.

2. The syntax and semantics of the deprecated API should be unchanged.

3. Marking the old API deprecated should be a separate patch in the series
for orthogonality.

4. Validation tests of the old API may be removed or retained as the
submitter's option

5. The new API definition, implementation, and validation tests should be
provided as part of the patch.

6. The user documentation should be updated to note that the old API is
deprecated and to fully document the new API and its usage.

The old and new APIs should then be maintained for a period of time to be
determined by consensus.  When it is clear that sufficient time has passed
to enable migration then a separate patch can be submitted to remove the
old API definition, implementation, validation tests (if still present),
and documentation.

I'd like to discuss this during next week's ODP arch meetings and public
call to ensure that everyone is in agreement here and that we can
transition to this model in 2016.

Thanks.
___
lng-odp mailing list
lng-odp@lists.linaro.org
https://lists.linaro.org/mailman/listinfo/lng-odp


Re: [lng-odp] [API-NEXT PATCHv2] linux-generic: time: remove posix bleed through on odp_time_t

2015-12-11 Thread Bill Fischofer
No, that's what changed in _POSIX_C_SOURCE 200809L.  If you wade through
all the various levels of indirection in /usr/include/time.h in all cases
it uses two signed 64-bit ints.  It just calls them either long or long
long depending on the environment.

On Fri, Dec 11, 2015 at 10:33 AM, Savolainen, Petri (Nokia - FI/Espoo) <
petri.savolai...@nokia.com> wrote:

>
> To me it seems that "ODP timespec" and Linux timespec have still different
> field sizes when 'long' is 32 bits. Does this work in the 32 bit build?
> Struct fields in the union do not point to the same locations when field
> sizes do not match.
>
> -Petri
> --
> Lähettäjä: EXT Maxim Uvarov 
> Lähetetty: ‎11.‎12.‎2015 15:27
> Vastaanottaja: lng-odp@lists.linaro.org
> Aihe: Re: [lng-odp] [API-NEXT PATCHv2] linux-generic: time: remove posix
> bleed through on odp_time_t
>
> On 12/11/2015 16:22, Bill Fischofer wrote:
> > The linux-generic implementation of odp_time_t makes use of POSIX
> > APIs that are sensitive to the _POSIX_C_SOURCE level. Use an indirection
> > mechanism so that these dependencies do not "bleed through" the ODP API.
> > This means that ODP applications can be independent of _POSIX_C_SOURCE
> > level.
> >
> > Signed-off-by: Bill Fischofer 
> > ---
> >   .../linux-generic/include/odp/plat/time_types.h|  9 ---
> >   platform/linux-generic/odp_time.c  | 30
> +++---
> >   2 files changed, 20 insertions(+), 19 deletions(-)
> >
> > diff --git a/platform/linux-generic/include/odp/plat/time_types.h
> b/platform/linux-generic/include/odp/plat/time_types.h
> > index e5765ec..e999beb 100644
> > --- a/platform/linux-generic/include/odp/plat/time_types.h
> > +++ b/platform/linux-generic/include/odp/plat/time_types.h
> > @@ -21,11 +21,12 @@ extern "C" {
> >*  @{
> >**/
> >
> > -typedef struct timespec odp_time_t;
> > +typedef struct {
> > + int64_t tv_sec;
> time_t is not reachable?
> > + int64_t tv_nsec;
> > +} odp_time_t;
> >
> > -odp_time_t odp_time_null(void);
> > -
> > -#define ODP_TIME_NULLodp_time_null()
> > +#define ODP_TIME_NULL ((odp_time_t){0, 0})
> >
> >   /**
> >* @}
> > diff --git a/platform/linux-generic/odp_time.c
> b/platform/linux-generic/odp_time.c
> > index 1c7c214..9b64b37 100644
> > --- a/platform/linux-generic/odp_time.c
> > +++ b/platform/linux-generic/odp_time.c
> > @@ -11,7 +11,12 @@
> >   #include 
> >   #include 
> >
> > -static struct timespec start_time;
> > +typedef union {
> > + odp_time_t  ex;
> > + struct timespec in;
> > +} _odp_time_t;
> > +
> > +static odp_time_t start_time;
> >
> >   static inline
> >   uint64_t time_to_ns(odp_time_t time)
> > @@ -27,7 +32,7 @@ uint64_t time_to_ns(odp_time_t time)
> >   static inline
> >   odp_time_t time_diff(odp_time_t t2, odp_time_t t1)
> >   {
> > - struct timespec time;
> > + odp_time_t time;
> >
> >time.tv_sec = t2.tv_sec - t1.tv_sec;
> >time.tv_nsec = t2.tv_nsec - t1.tv_nsec;
> > @@ -43,13 +48,13 @@ odp_time_t time_diff(odp_time_t t2, odp_time_t t1)
> >   odp_time_t odp_time_local(void)
> >   {
> >int ret;
> > - struct timespec time;
> > + _odp_time_t time;
> >
> > - ret = clock_gettime(CLOCK_MONOTONIC_RAW, );
> > + ret = clock_gettime(CLOCK_MONOTONIC_RAW, );
> >if (odp_unlikely(ret != 0))
> >ODP_ABORT("clock_gettime failed\n");
> >
> > - return time_diff(time, start_time);
> > + return time_diff(time.ex, start_time);
> >   }
> >
> >   odp_time_t odp_time_diff(odp_time_t t2, odp_time_t t1)
> > @@ -64,7 +69,7 @@ uint64_t odp_time_to_ns(odp_time_t time)
> >
> >   odp_time_t odp_time_local_from_ns(uint64_t ns)
> >   {
> > - struct timespec time;
> > + odp_time_t time;
> >
> >time.tv_sec = ns / ODP_TIME_SEC_IN_NS;
> >time.tv_nsec = ns - time.tv_sec * ODP_TIME_SEC_IN_NS;
> > @@ -85,7 +90,7 @@ int odp_time_cmp(odp_time_t t2, odp_time_t t1)
> >
> >   odp_time_t odp_time_sum(odp_time_t t1, odp_time_t t2)
> >   {
> > - struct timespec time;
> > + odp_time_t time;
> >
> >time.tv_sec = t2.tv_sec + t1.tv_sec;
> >time.tv_nsec = t2.tv_nsec + t1.tv_nsec;
> > @@ -113,18 +118,13 @@ uint64_t odp_time_to_u64(odp_time_t time)
> >return time_to_ns(time) / resolution;
> >   }
> >
> > -odp_time_t odp_time_null(void)
> > -{
> > - return (struct timespec) {0, 0};
> > -}
> > -
> >   int odp_time_global_init(void)
> >   {
> >int ret;
> > - struct timespec time;
> > + _odp_time_t time;
> >
> > - ret = clock_gettime(CLOCK_MONOTONIC_RAW, );
> > - start_time = ret ? odp_time_null() : time;
> > + ret = clock_gettime(CLOCK_MONOTONIC_RAW, );
> > + start_time = ret ? ODP_TIME_NULL : time.ex;
> >
> >return ret;
> >   }
>
> ___
> lng-odp mailing list
> lng-odp@lists.linaro.org
> 

Re: [lng-odp] [PATCH] test/performance: pktio: perform an initial warmup run

2015-12-11 Thread Stuart Haslam
ping - needs review.

On 28 October 2015 at 18:45, Stuart Haslam  wrote:
> The results from the initial test run are often worse than would
> normally be expected due to there being no warm up phase. As a
> simple way to warm up run first test stage twice, ignoring the
> results of the first run.
>
> Signed-off-by: Stuart Haslam 
> ---
>  test/performance/odp_pktio_perf.c | 12 ++--
>  1 file changed, 10 insertions(+), 2 deletions(-)
>
> diff --git a/test/performance/odp_pktio_perf.c 
> b/test/performance/odp_pktio_perf.c
> index efd26dc..38d684a 100644
> --- a/test/performance/odp_pktio_perf.c
> +++ b/test/performance/odp_pktio_perf.c
> @@ -134,6 +134,7 @@ typedef struct {
> uint64_t pps_curr; /* Current attempted PPS */
> uint64_t pps_pass; /* Highest passing PPS */
> uint64_t pps_fail; /* Lowest failing PPS */
> +   int  warmup;   /* Warmup stage - ignore results */
>  } test_status_t;
>
>  /* Thread specific arguments */
> @@ -647,7 +648,10 @@ static int run_test_single(odp_cpumask_t *thd_mask_tx,
> /* wait for receivers */
> odph_linux_pthread_join(_tbl[0], num_rx_workers);
>
> -   return process_results(expected_tx_cnt, status);
> +   if (!status->warmup)
> +   return process_results(expected_tx_cnt, status);
> +
> +   return 1;
>  }
>
>  static int run_test(void)
> @@ -659,6 +663,7 @@ static int run_test(void)
> .pps_curr = gbl_args->args.pps,
> .pps_pass = 0,
> .pps_fail = 0,
> +   .warmup = 1,
> };
>
> if (setup_txrx_masks(, ) != 0)
> @@ -679,6 +684,10 @@ static int run_test(void)
> printf("%s ", gbl_args->args.ifaces[i]);
> printf("\n");
>
> +   /* first time just run the test but throw away the results */
> +   run_test_single(, , );
> +   status.warmup = 0;
> +
> while (ret > 0)
> ret = run_test_single(, , );
>
> @@ -957,7 +966,6 @@ static void parse_args(int argc, char *argv[], 
> test_args_t *args)
> LOG_ABORT("Failed to alloc iface storage\n");
>
> strcpy(args->if_str, optarg);
> -
> for (token = strtok(args->if_str, ",");
>  token != NULL && args->num_ifaces < 
> MAX_NUM_IFACES;
>  token = strtok(NULL, ","))
> --
> 2.1.1
>
___
lng-odp mailing list
lng-odp@lists.linaro.org
https://lists.linaro.org/mailman/listinfo/lng-odp


[lng-odp] [PATCHv2] validation: pktio: test batch receive

2015-12-11 Thread Stuart Haslam
Modify the tests that currently transmit packets in batches to also
receive packets in batches. This adds coverage of odp_queue_deq_multi()
and odp_schedule_multi() specifically against a packet input queue,
as this doesn't get tested anywhere else in the validation suite.

Signed-off-by: Stuart Haslam 
---
Changes in v2 - rebased and picked up new time API

 test/validation/pktio/pktio.c | 139 +-
 1 file changed, 68 insertions(+), 71 deletions(-)

diff --git a/test/validation/pktio/pktio.c b/test/validation/pktio/pktio.c
index 52e5414..1eca694 100644
--- a/test/validation/pktio/pktio.c
+++ b/test/validation/pktio/pktio.c
@@ -137,7 +137,7 @@ static uint32_t pktio_pkt_seq(odp_packet_t pkt)
pkt_tail_t tail;
 
if (pkt == ODP_PACKET_INVALID)
-   return -1;
+   return TEST_SEQ_INVALID;
 
off = odp_packet_l4_offset(pkt);
if (off ==  ODP_PACKET_OFFSET_INVALID)
@@ -332,68 +332,76 @@ static int destroy_inq(odp_pktio_t pktio)
return odp_queue_destroy(inq);
 }
 
-static odp_event_t queue_deq_wait_time(odp_queue_t queue, uint64_t ns)
+static int get_packets(pktio_info_t *pktio_rx, odp_packet_t pkt_tbl[], int num)
 {
-   odp_time_t start, now, diff;
-   odp_event_t ev;
+   odp_event_t evt_tbl[num];
+   int num_evts = 0;
+   int num_pkts = 0;
+   int i;
 
-   start = odp_time_local();
+   if (pktio_rx->in_mode == ODP_PKTIN_MODE_RECV)
+   return odp_pktio_recv(pktio_rx->id, pkt_tbl, num);
 
-   do {
-   ev = odp_queue_deq(queue);
-   if (ev != ODP_EVENT_INVALID)
-   return ev;
-   now = odp_time_local();
-   diff = odp_time_diff(now, start);
-   } while (odp_time_to_ns(diff) < ns);
+   if (num > 1) {
+   if (pktio_rx->in_mode == ODP_PKTIN_MODE_POLL)
+   num_evts = odp_queue_deq_multi(pktio_rx->inq, evt_tbl,
+  num);
+   else
+   num_evts = odp_schedule_multi(NULL, ODP_SCHED_NO_WAIT,
+ evt_tbl, num);
+   } else {
+   odp_event_t evt_tmp;
+
+   if (pktio_rx->in_mode == ODP_PKTIN_MODE_POLL)
+   evt_tmp = odp_queue_deq(pktio_rx->inq);
+   else
+   evt_tmp = odp_schedule(NULL, ODP_SCHED_NO_WAIT);
+
+   if (evt_tmp != ODP_EVENT_INVALID)
+   evt_tbl[num_evts++] = evt_tmp;
+   }
+
+   /* convert events to packets, discarding any non-packet events */
+   for (i = 0; i < num_evts; ++i) {
+   if (odp_event_type(evt_tbl[i]) == ODP_EVENT_PACKET)
+   pkt_tbl[num_pkts++] = odp_packet_from_event(evt_tbl[i]);
+   else
+   odp_event_free(evt_tbl[i]);
+   }
 
-   return ODP_EVENT_INVALID;
+   return num_pkts;
 }
 
-static odp_packet_t wait_for_packet(pktio_info_t *pktio_rx,
-   uint32_t seq, uint64_t ns)
+static int wait_for_packets(pktio_info_t *pktio_rx, odp_packet_t pkt_tbl[],
+   uint32_t seq_tbl[], int num, uint64_t ns)
 {
odp_time_t start, now, diff;
-   odp_event_t ev;
-   odp_packet_t pkt;
-   uint64_t wait;
+   int num_rx = 0;
+   int i;
+   odp_packet_t pkt_tmp[num];
 
start = odp_time_local();
-   wait = odp_schedule_wait_time(ns);
 
do {
-   pkt = ODP_PACKET_INVALID;
-
-   if (pktio_rx->in_mode == ODP_PKTIN_MODE_RECV) {
-   odp_pktio_recv(pktio_rx->id, , 1);
-   } else {
-   if (pktio_rx->in_mode == ODP_PKTIN_MODE_POLL)
-   ev = queue_deq_wait_time(pktio_rx->inq, ns);
-   else
-   ev = odp_schedule(NULL, wait);
-
-   if (ev != ODP_EVENT_INVALID) {
-   if (odp_event_type(ev) == ODP_EVENT_PACKET)
-   pkt = odp_packet_from_event(ev);
-   else
-   odp_event_free(ev);
-   }
-   }
+   int n = get_packets(pktio_rx, pkt_tmp, num);
 
-   if (pkt != ODP_PACKET_INVALID) {
-   if (pktio_pkt_seq(pkt) == seq)
-   return pkt;
+   if (n < 0)
+   break;
 
-   odp_packet_free(pkt);
+   for (i = 0; i < n; ++i) {
+   if (pktio_pkt_seq(pkt_tmp[i]) == seq_tbl[num_rx]) {
+   pkt_tbl[num_rx++] = pkt_tmp[i];
+   num--;
+   } else {
+   

Re: [lng-odp] [PATCHv6] helper : Fix UDP checksum computation

2015-12-11 Thread Ilya Maximets
On 11.12.2015 18:28, Ion Grigore wrote:
> 
> Comments inlined
> 
> -Original Message-
> From: Ilya Maximets [mailto:i.maxim...@samsung.com] 
> Sent: Friday, December 11, 2015 5:24 PM
> To: Grigore Ion-B17953 ; Maxim Uvarov 
> ; lng-odp@lists.linaro.org
> Subject: Re: [PATCHv6] helper : Fix UDP checksum computation
> 
> On 11.12.2015 17:07, Ion Grigore wrote:
>> Comments inlined
>>
>> -Original Message-
>> From: Maxim Uvarov [mailto:maxim.uva...@linaro.org]
>> Sent: Friday, December 11, 2015 11:20 AM
>> To: Ilya Maximets ; Grigore Ion-B17953 
>> ; lng-odp@lists.linaro.org
>> Subject: Re: [PATCHv6] helper : Fix UDP checksum computation
>>
>> On 12/11/2015 10:39, Ilya Maximets wrote:
>>> Comments inlined.
>>>
>>> On 10.12.2015 12:59, ion.grig...@freescale.com wrote:
 From: Grigore Ion 

 This patch fixes the following problems:
 - checksum computation for LE platforms
 - checksum is computed in the CPU endianness. The returned result 
 must be converted to the BE ordering when it is used to update the 
 UDP checksum in a packet.
 - checksum computation for packets having the UDP length not a 
 multiple of 2
 - fixes the UDP checksum associated validation test

 Signed-off-by: Grigore Ion 
 ---
   v6:
   - Make code more understandable (Ilya Maximets)
   v5:
   - Checksum in CPU endianness fix added (Ilya Maximets)
   v4:
   - Verify checksum in CPU endianness in the associated test
   (Ilya Maximets)
   v3:
   - fix the UDP checksum computation in the associated test
   (Maxim Uvarov)
   v2:
   - patch updated to the last master (Maxim Uvarov)
   v1:
   - Move variables declaration on top of block. (Maxim Uvarov)
   - Check patch with checkpatch script.  (Maxim Uvarov)
   - L3 header presence is tested twice. (Alexandru Badicioiu)
   - Remove unnecessary check for L3 header presence. (Bill Fischofer)
   - Modify check of odp_packet_l4_offset() return. (Bill Fischofer)

   helper/include/odp/helper/udp.h | 81 
 +
   helper/test/odp_chksum.c|  4 +-
   2 files changed, 51 insertions(+), 34 deletions(-)

 diff --git a/helper/include/odp/helper/udp.h 
 b/helper/include/odp/helper/udp.h index 06c439b..d0599b1 100644
 --- a/helper/include/odp/helper/udp.h
 +++ b/helper/include/odp/helper/udp.h
 @@ -4,7 +4,6 @@
* SPDX-License-Identifier: BSD-3-Clause
*/
   
 -
   /**
* @file
*
 @@ -22,7 +21,6 @@ extern "C" {
   #include 
   #include 
   
 -
   /** @addtogroup odph_header ODPH HEADER
*  @{
*/
 @@ -39,51 +37,70 @@ typedef struct ODP_PACKED {
   } odph_udphdr_t;
   
   /**
 + * Perform byte swap required by UDP checksum computation algorithm 
 +*/ static inline uint16_t csum_bswap(uint16_t val) { #if 
 +ODP_BYTE_ORDER == ODP_LITTLE_ENDIAN
 +  val = __odp_builtin_bswap16((__odp_force uint16_t)val); #endif
 +  return val;
 +}
>>> Please, don't duplicate the code.
>>>
 +
 +/**
 + * Pad a byte value to the left or to the right as required by UDP 
 +checksum
 + * computation algorithm and convert the result to CPU native uint16_t.
 + * Left padding is performed for the IP protocol field in the UDP
 + * pseudo-header (RFC 768). Right padding is performed in the case 
 +of the odd
 + * byte in a UDP packet having the length not a 2 multiple.
 + */
 +static inline uint16_t u8_pad_to_u16(uint8_t val, odp_bool_t left) {
 +  uint16_tret;
 +
 +  ret = (left) ? val : val << 8;
 +  return csum_bswap(ret);
 +}
 +
 +/**
* UDP checksum
*
* This function uses odp packet to calc checksum
*
* @param pkt  calculate chksum for pkt
 - * @return  checksum value
 + * @return  checksum value in CPU endianness
*/
   static inline uint16_t odph_ipv4_udp_chksum(odp_packet_t pkt)
   {
 -  uint32_t sum = 0;
 -  odph_udphdr_t *udph;
 -  odph_ipv4hdr_t *iph;
 -  uint16_t udplen;
 -  uint8_t *buf;
 -
 -  if (!odp_packet_l3_offset(pkt))
 -  return 0;
 +  odph_ipv4hdr_t  *iph;
 +  odph_udphdr_t   *udph;
 +  uint32_tsum;
 +  uint16_tudplen, *buf;
   
 -  if (!odp_packet_l4_offset(pkt))
 +  if (odp_packet_l4_offset(pkt) == ODP_PACKET_OFFSET_INVALID)
return 0;
 -
iph = (odph_ipv4hdr_t *)odp_packet_l3_ptr(pkt, NULL);
udph = (odph_udphdr_t *)odp_packet_l4_ptr(pkt, NULL);
 -  udplen = odp_be_to_cpu_16(udph->length);
 -
 -  /* 32-bit sum of all 16-bit words covered by UDP chksum */

Re: [lng-odp] [API-NEXT PATCHv2] linux-generic: time: remove posix bleed through on odp_time_t

2015-12-11 Thread Savolainen, Petri (Nokia - FI/Espoo)

To me it seems that "ODP timespec" and Linux timespec have still different 
field sizes when 'long' is 32 bits. Does this work in the 32 bit build? Struct 
fields in the union do not point to the same locations when field sizes do not 
match.

-Petri

Lähettäjä: EXT Maxim Uvarov
Lähetetty: ‎11.‎12.‎2015 15:27
Vastaanottaja: lng-odp@lists.linaro.org
Aihe: Re: [lng-odp] [API-NEXT PATCHv2] linux-generic: time: remove posix bleed 
through on odp_time_t

On 12/11/2015 16:22, Bill Fischofer wrote:
> The linux-generic implementation of odp_time_t makes use of POSIX
> APIs that are sensitive to the _POSIX_C_SOURCE level. Use an indirection
> mechanism so that these dependencies do not "bleed through" the ODP API.
> This means that ODP applications can be independent of _POSIX_C_SOURCE
> level.
>
> Signed-off-by: Bill Fischofer 
> ---
>   .../linux-generic/include/odp/plat/time_types.h|  9 ---
>   platform/linux-generic/odp_time.c  | 30 
> +++---
>   2 files changed, 20 insertions(+), 19 deletions(-)
>
> diff --git a/platform/linux-generic/include/odp/plat/time_types.h 
> b/platform/linux-generic/include/odp/plat/time_types.h
> index e5765ec..e999beb 100644
> --- a/platform/linux-generic/include/odp/plat/time_types.h
> +++ b/platform/linux-generic/include/odp/plat/time_types.h
> @@ -21,11 +21,12 @@ extern "C" {
>*  @{
>**/
>
> -typedef struct timespec odp_time_t;
> +typedef struct {
> + int64_t tv_sec;
time_t is not reachable?
> + int64_t tv_nsec;
> +} odp_time_t;
>
> -odp_time_t odp_time_null(void);
> -
> -#define ODP_TIME_NULLodp_time_null()
> +#define ODP_TIME_NULL ((odp_time_t){0, 0})
>
>   /**
>* @}
> diff --git a/platform/linux-generic/odp_time.c 
> b/platform/linux-generic/odp_time.c
> index 1c7c214..9b64b37 100644
> --- a/platform/linux-generic/odp_time.c
> +++ b/platform/linux-generic/odp_time.c
> @@ -11,7 +11,12 @@
>   #include 
>   #include 
>
> -static struct timespec start_time;
> +typedef union {
> + odp_time_t  ex;
> + struct timespec in;
> +} _odp_time_t;
> +
> +static odp_time_t start_time;
>
>   static inline
>   uint64_t time_to_ns(odp_time_t time)
> @@ -27,7 +32,7 @@ uint64_t time_to_ns(odp_time_t time)
>   static inline
>   odp_time_t time_diff(odp_time_t t2, odp_time_t t1)
>   {
> - struct timespec time;
> + odp_time_t time;
>
>time.tv_sec = t2.tv_sec - t1.tv_sec;
>time.tv_nsec = t2.tv_nsec - t1.tv_nsec;
> @@ -43,13 +48,13 @@ odp_time_t time_diff(odp_time_t t2, odp_time_t t1)
>   odp_time_t odp_time_local(void)
>   {
>int ret;
> - struct timespec time;
> + _odp_time_t time;
>
> - ret = clock_gettime(CLOCK_MONOTONIC_RAW, );
> + ret = clock_gettime(CLOCK_MONOTONIC_RAW, );
>if (odp_unlikely(ret != 0))
>ODP_ABORT("clock_gettime failed\n");
>
> - return time_diff(time, start_time);
> + return time_diff(time.ex, start_time);
>   }
>
>   odp_time_t odp_time_diff(odp_time_t t2, odp_time_t t1)
> @@ -64,7 +69,7 @@ uint64_t odp_time_to_ns(odp_time_t time)
>
>   odp_time_t odp_time_local_from_ns(uint64_t ns)
>   {
> - struct timespec time;
> + odp_time_t time;
>
>time.tv_sec = ns / ODP_TIME_SEC_IN_NS;
>time.tv_nsec = ns - time.tv_sec * ODP_TIME_SEC_IN_NS;
> @@ -85,7 +90,7 @@ int odp_time_cmp(odp_time_t t2, odp_time_t t1)
>
>   odp_time_t odp_time_sum(odp_time_t t1, odp_time_t t2)
>   {
> - struct timespec time;
> + odp_time_t time;
>
>time.tv_sec = t2.tv_sec + t1.tv_sec;
>time.tv_nsec = t2.tv_nsec + t1.tv_nsec;
> @@ -113,18 +118,13 @@ uint64_t odp_time_to_u64(odp_time_t time)
>return time_to_ns(time) / resolution;
>   }
>
> -odp_time_t odp_time_null(void)
> -{
> - return (struct timespec) {0, 0};
> -}
> -
>   int odp_time_global_init(void)
>   {
>int ret;
> - struct timespec time;
> + _odp_time_t time;
>
> - ret = clock_gettime(CLOCK_MONOTONIC_RAW, );
> - start_time = ret ? odp_time_null() : time;
> + ret = clock_gettime(CLOCK_MONOTONIC_RAW, );
> + start_time = ret ? ODP_TIME_NULL : time.ex;
>
>return ret;
>   }

___
lng-odp mailing list
lng-odp@lists.linaro.org
https://lists.linaro.org/mailman/listinfo/lng-odp
___
lng-odp mailing list
lng-odp@lists.linaro.org
https://lists.linaro.org/mailman/listinfo/lng-odp


Re: [lng-odp] [PATCHv5 2/4] validation: pktio: ability to wait for external network

2015-12-11 Thread Stuart Haslam
On Thu, Dec 10, 2015 at 06:32:02PM +0300, Ilya Maximets wrote:
> 'ODP_WAIT_FOR_NETWORK' environment variable may be used
> to wait some time right after pktio_open().
> 
> Signed-off-by: Ilya Maximets 
> ---
> 
> 'ODP_WAIT_FOR_NETWORK' also may be used to remove sleep(1)
> from netmap pktio in the future, beacause IMO waiting
> for external network is a work for application and not
> for pktio itself.

I agree doing that sort of thing in the pktio itself is a bit nasty [1],
but doing it in the application in this way also isn't ideal as it
requires pktio specific knowledge (to know to set ODP_WAIT_FOR_NETWORK
for tap pktios). Having an unconditional delay wouldn't be great either.

The intention has been to have the applications check the link status,
once the odp_pktio_link_status() API is available, but presumably that
wouldn't help in this case because both links are up but not connected
together.

Another way around the issue would be to modify the test to have an
initial "probe" period, so after opening the interfaces it sends
probe packets to check if the interfaces are connected before actually
starting the test, with a timeout after a couple of seconds if no
packets make it through. This still wouldn't handle netmap's quirk
though, as in that case after opening the interfaces both links do come
up and the can pass traffic for a short time, then one of the links
bounces briefly (at least when using an external loopback cable).

Anyway, although I think we may still need some work here, I don't think
it's worth holding up this series for, so I'm OK with this going in as
it is.

1. https://lists.linaro.org/pipermail/lng-odp/2015-October/016320.html

> 
>  test/validation/pktio/pktio.c | 26 +-
>  1 file changed, 25 insertions(+), 1 deletion(-)
> 
> diff --git a/test/validation/pktio/pktio.c b/test/validation/pktio/pktio.c
> index 53633dd..28bfd30 100644
> --- a/test/validation/pktio/pktio.c
> +++ b/test/validation/pktio/pktio.c
> @@ -29,6 +29,11 @@ static const char *iface_name[MAX_NUM_IFACES];
>  /** number of interfaces being used (1=loopback, 2=pair) */
>  static int num_ifaces;
>  
> +/** while testing real-world interfaces additional time may be
> +needed for external network to enable link to pktio
> +interface that just become up.*/
> +static bool wait_for_network;
> +
>  /** local container for pktio attributes */
>  typedef struct {
>   const char *name;
> @@ -252,6 +257,17 @@ static int default_pool_create(void)
>   return 0;
>  }
>  
> +static void spin_wait(uint64_t ns)
> +{
> + odp_time_t start, now, diff;
> +
> + start = odp_time_local();
> + do {
> + now = odp_time_local();
> + diff = odp_time_diff(now, start);
> + } while (odp_time_to_ns(diff) < ns);
> +}
> +
>  static odp_pktio_t create_pktio(int iface_idx, odp_pktio_input_mode_t imode,
>   odp_pktio_output_mode_t omode)
>  {
> @@ -271,6 +287,9 @@ static odp_pktio_t create_pktio(int iface_idx, 
> odp_pktio_input_mode_t imode,
>   CU_ASSERT(odp_pktio_to_u64(pktio) !=
> odp_pktio_to_u64(ODP_PKTIO_INVALID));
>  
> + if (wait_for_network)
> + spin_wait(ODP_TIME_SEC_IN_NS / 4);
> +
>   return pktio;
>  }
>  
> @@ -1105,11 +1124,16 @@ static int create_pool(const char *iface, int num)
>  
>  static int pktio_suite_init(void)
>  {
> + int i;
> +
>   odp_atomic_init_u32(_seq, 0);
> +
> + if (getenv("ODP_WAIT_FOR_NETWORK"))
> + wait_for_network = true;
> +
>   iface_name[0] = getenv("ODP_PKTIO_IF0");
>   iface_name[1] = getenv("ODP_PKTIO_IF1");
>   num_ifaces = 1;
> - int i;
>  
>   if (!iface_name[0]) {
>   printf("No interfaces specified, using default \"loop\".\n");
> -- 
> 2.1.4
> 
___
lng-odp mailing list
lng-odp@lists.linaro.org
https://lists.linaro.org/mailman/listinfo/lng-odp


Re: [lng-odp] [PATCHv5 0/4] TAP pktio

2015-12-11 Thread Stuart Haslam
On Thu, Dec 10, 2015 at 06:32:00PM +0300, Ilya Maximets wrote:
> Creates a new pktio type that allows for creating and
> sending/receiving packets through TAP interface.
> Detailed description in commit-message of patch
> "[PATCHv5 3/4] linux-generic: pktio: add tap pktio type".
> 
> Changelog:
> 
> Version 5:
>   * nothing changed. New patch added to add ability
> to wait some time right after pktio_open() to
> be sure that tap interface switched to enabled state
> inside the bridge. Fixes occasional first test failures.
> ODP_WAIT_FOR_NETWORK used to run tests.
>   * rebased on current master
> 
> Version 4:
>   * changed error handling part in tap_pktio_send. (Stuart Haslam)
> 
> Version 3:
>   * return 77 (TEST_SKIPPED) if user is not root. (Stuart Haslam)
> 
> Version 2:
>   * Validation tests added
>   * Pktio tests fixed to work with real-world
> interfaces.
>   * MAC of pktio now is not a kernel interface's MAC
>   * Interfaces are UP after pktio_open()
>   * Fixed getting mtu, getting/setting promisc mode
>   * Misclenious fixes
> 
> Ilya Maximets (4):
>   validation: pktio: initialize mac addresses for all packets
>   validation: pktio: ability to wait for external network
>   linux-generic: pktio: add tap pktio type
>   linux-generic: pktio: add test for tap pktio

For the series:

Reviewed-and-Tested-by: Stuart Haslam 

> 
>  platform/linux-generic/Makefile.am |   2 +
>  .../linux-generic/include/odp_packet_io_internal.h |   3 +
>  platform/linux-generic/include/odp_packet_tap.h|  21 ++
>  platform/linux-generic/pktio/io_ops.c  |   1 +
>  platform/linux-generic/pktio/tap.c | 327 
> +
>  platform/linux-generic/test/Makefile.am|   1 +
>  platform/linux-generic/test/pktio/Makefile.am  |   3 +-
>  platform/linux-generic/test/pktio/pktio_run_tap| 115 
>  test/validation/pktio/pktio.c  |  72 -
>  9 files changed, 536 insertions(+), 9 deletions(-)
>  create mode 100644 platform/linux-generic/include/odp_packet_tap.h
>  create mode 100644 platform/linux-generic/pktio/tap.c
>  create mode 100755 platform/linux-generic/test/pktio/pktio_run_tap
> 
> -- 
> 2.1.4
> 
___
lng-odp mailing list
lng-odp@lists.linaro.org
https://lists.linaro.org/mailman/listinfo/lng-odp


Re: [lng-odp] [PATCHv3] doc: userguide: add application programming section

2015-12-11 Thread Mike Holmes
The checkpatch warning is not present with the CI like version of Ubuntu
[1], I see that on 15.04 it is not a checkpatch issue.

[1] https://hub.docker.com/r/roxell/check-odp-ubuntu/



On 11 December 2015 at 10:18, Mike Holmes  wrote:

>
>
> On 11 December 2015 at 10:08, Bill Fischofer 
> wrote:
>
>> The problem is that this is documentation text (pseudocode) not code and
>> yet some tools insist on trying to treat it as if it's a code patch.
>> Checkpatch should not apply to documentation this way.
>>
>
> In this case then we need to turn all the other spaces to tabs becasue now
> we have a mix.
> I want to keep it consistent one way or the other or reviewing will be
> difficult depending on the viewers tab setting.
>
> +...thread init processing
> +
> +while (1) {
> +odp_event_t ev;   <-- only this one is newly tabbed
> +   odp_queue_t which_q;
> +   ev = odp_schedule(_q, );
> +   ...process the event
> +}
> +
>
>
> I honestly think it is just easier to not use tabs in the docs.
>
>
>
>>
>> On Fri, Dec 11, 2015 at 9:04 AM, Mike Holmes 
>> wrote:
>>
>>> Using patch:
>>> lng-odp_PATCHv3_doc_userguide_add_application_programming_section.mbox
>>>   Trying to apply patch
>>>   Patch applied
>>> Applying: doc: userguide: add application programming section
>>> /home/mike/git/check-odp/build/odp-apply/.git/rebase-apply/patch:446:
>>> indent with spaces.
>>> odp_event_t ev;
>>> /home/mike/git/check-odp/build/odp-apply/.git/rebase-apply/patch:613:
>>> indent with spaces.
>>> odp_init_local();
>>> /home/mike/git/check-odp/build/odp-apply/.git/rebase-apply/patch:616:
>>> indent with spaces.
>>> while (1) {
>>> /home/mike/git/check-odp/build/odp-apply/.git/rebase-apply/patch:617:
>>> indent with spaces.
>>>ev = odp_schedule(_q, ODP_SCHED_WAIT);
>>> warning: 4 lines add whitespace errors.
>>> 0001-doc-userguide-add-application-programming-section.patch: unified
>>> diff output, UTF-8 Unicode text
>>>
>>> When I look at all these with git show I see a tab got in there rather
>>> than a space which is inconsistent with the rest of the text.
>>>
>>> Do you have the .gitconfig whitespace =
>>> trailing-space,space-before-tab,indent-with-non-tab,error-all if not you
>>> may not be seeing some things.
>>>
>>>
>>>
>>>
>>>
>>> On 8 December 2015 at 17:36, Bill Fischofer 
>>> wrote:
>>>
 Complete the reformatting to standard asciidoc style, expand the
 ODP Application Programming section, and include a reorganized and
 expanded discussion of ODP queues.

 Signed-off-by: Bill Fischofer 
 ---
  doc/users-guide/users-guide.adoc | 451
 +++
  1 file changed, 359 insertions(+), 92 deletions(-)

 diff --git a/doc/users-guide/users-guide.adoc
 b/doc/users-guide/users-guide.adoc
 index cf77fa0..d660fb8 100644
 --- a/doc/users-guide/users-guide.adoc
 +++ b/doc/users-guide/users-guide.adoc
 @@ -8,16 +8,19 @@ OpenDataPlane (ODP)  Users-Guide
  Abstract
  
  This document is intended to guide a new ODP application developer.
 -Further details about ODP may be found at the 
 http://opendataplane.org[ODP]
 home page.
 +Further details about ODP may be found at the http://opendataplane.org
 [ODP]
 +home page.

  .Overview of a system running ODP applications
  image::../images/overview.png[align="center"]

 -ODP is an API specification that allows many implementations to
 provide platform independence, automatic hardware acceleration and CPU
 scaling to high performance networking  applications.
 -This document describes how to write an application that can
 successfully take advantage of the API.
 +ODP is an API specification that allows many implementations to provide
 +platform independence, automatic hardware acceleration and CPU scaling
 to
 +high performance networking  applications. This document describes how
 to
 +write an application that can successfully take advantage of the API.

  :numbered:
 -== Introduction ==
 +== Introduction
  .OpenDataPlane Components
  image::../images/odp_components.png[align="center"]

 @@ -42,7 +45,7 @@ ODP API specification--that is the responsibility of
 each ODP implementation.
  * Application-centric.  Covers functional needs of data plane
 applications.
  * Ensures portability by specifying the functional behavior of ODP.
  * Defined jointly and openly by application writers and platform
 implementers.
 -* Archiected to be implementable on a wide range of platforms
 efficiently
 +* Architected to be implementable on a wide range of platforms
 efficiently
  * Sponsored, governed, and maintained by the Linaro Networking Group
 (LNG)

Re: [lng-odp] [PATCHv3] doc: userguide: add application programming section

2015-12-11 Thread Mike Holmes
On 11 December 2015 at 10:08, Bill Fischofer 
wrote:

> The problem is that this is documentation text (pseudocode) not code and
> yet some tools insist on trying to treat it as if it's a code patch.
> Checkpatch should not apply to documentation this way.
>

In this case then we need to turn all the other spaces to tabs becasue now
we have a mix.
I want to keep it consistent one way or the other or reviewing will be
difficult depending on the viewers tab setting.

+...thread init processing
+
+while (1) {
+odp_event_t ev;   <-- only this one is newly tabbed
+   odp_queue_t which_q;
+   ev = odp_schedule(_q, );
+   ...process the event
+}
+


I honestly think it is just easier to not use tabs in the docs.



>
> On Fri, Dec 11, 2015 at 9:04 AM, Mike Holmes 
> wrote:
>
>> Using patch:
>> lng-odp_PATCHv3_doc_userguide_add_application_programming_section.mbox
>>   Trying to apply patch
>>   Patch applied
>> Applying: doc: userguide: add application programming section
>> /home/mike/git/check-odp/build/odp-apply/.git/rebase-apply/patch:446:
>> indent with spaces.
>> odp_event_t ev;
>> /home/mike/git/check-odp/build/odp-apply/.git/rebase-apply/patch:613:
>> indent with spaces.
>> odp_init_local();
>> /home/mike/git/check-odp/build/odp-apply/.git/rebase-apply/patch:616:
>> indent with spaces.
>> while (1) {
>> /home/mike/git/check-odp/build/odp-apply/.git/rebase-apply/patch:617:
>> indent with spaces.
>>ev = odp_schedule(_q, ODP_SCHED_WAIT);
>> warning: 4 lines add whitespace errors.
>> 0001-doc-userguide-add-application-programming-section.patch: unified
>> diff output, UTF-8 Unicode text
>>
>> When I look at all these with git show I see a tab got in there rather
>> than a space which is inconsistent with the rest of the text.
>>
>> Do you have the .gitconfig whitespace =
>> trailing-space,space-before-tab,indent-with-non-tab,error-all if not you
>> may not be seeing some things.
>>
>>
>>
>>
>>
>> On 8 December 2015 at 17:36, Bill Fischofer 
>> wrote:
>>
>>> Complete the reformatting to standard asciidoc style, expand the
>>> ODP Application Programming section, and include a reorganized and
>>> expanded discussion of ODP queues.
>>>
>>> Signed-off-by: Bill Fischofer 
>>> ---
>>>  doc/users-guide/users-guide.adoc | 451
>>> +++
>>>  1 file changed, 359 insertions(+), 92 deletions(-)
>>>
>>> diff --git a/doc/users-guide/users-guide.adoc
>>> b/doc/users-guide/users-guide.adoc
>>> index cf77fa0..d660fb8 100644
>>> --- a/doc/users-guide/users-guide.adoc
>>> +++ b/doc/users-guide/users-guide.adoc
>>> @@ -8,16 +8,19 @@ OpenDataPlane (ODP)  Users-Guide
>>>  Abstract
>>>  
>>>  This document is intended to guide a new ODP application developer.
>>> -Further details about ODP may be found at the http://opendataplane.org[ODP]
>>> home page.
>>> +Further details about ODP may be found at the http://opendataplane.org
>>> [ODP]
>>> +home page.
>>>
>>>  .Overview of a system running ODP applications
>>>  image::../images/overview.png[align="center"]
>>>
>>> -ODP is an API specification that allows many implementations to provide
>>> platform independence, automatic hardware acceleration and CPU scaling to
>>> high performance networking  applications.
>>> -This document describes how to write an application that can
>>> successfully take advantage of the API.
>>> +ODP is an API specification that allows many implementations to provide
>>> +platform independence, automatic hardware acceleration and CPU scaling
>>> to
>>> +high performance networking  applications. This document describes how
>>> to
>>> +write an application that can successfully take advantage of the API.
>>>
>>>  :numbered:
>>> -== Introduction ==
>>> +== Introduction
>>>  .OpenDataPlane Components
>>>  image::../images/odp_components.png[align="center"]
>>>
>>> @@ -42,7 +45,7 @@ ODP API specification--that is the responsibility of
>>> each ODP implementation.
>>>  * Application-centric.  Covers functional needs of data plane
>>> applications.
>>>  * Ensures portability by specifying the functional behavior of ODP.
>>>  * Defined jointly and openly by application writers and platform
>>> implementers.
>>> -* Archiected to be implementable on a wide range of platforms
>>> efficiently
>>> +* Architected to be implementable on a wide range of platforms
>>> efficiently
>>>  * Sponsored, governed, and maintained by the Linaro Networking Group
>>> (LNG)
>>>
>>>  .ODP Implementations
>>> @@ -68,7 +71,7 @@ where the application will run on a target platform
>>> chosen by someone else.
>>>  * One size does not fit all--supporting multiple implementations allows
>>> ODP
>>>  to adapt to widely differing internals among platforms.
>>>  * Anyone can create an ODP implementation tailored to their platform
>>> -* Distribution and mainteinance of each implementation is 

Re: [lng-odp] [PATCHv6] helper : Fix UDP checksum computation

2015-12-11 Thread Ilya Maximets
On 11.12.2015 18:47, Ion Grigore wrote:
> A more precise question. Did you check on a LE platform ?

Yes. On my LE i7.

> - /* Fold sum to 16 bits: add carrier to result */
> - while (sum >> 16)
> - sum = (sum & 0x) + (sum >> 16);
> -
> + (iph->dst_addr & 0x) + (iph->dst_addr >> 16) +
> + u8_pad_to_u16(iph->proto, 1) + udph->length;
 I meant something like:

 union {
uint8_t v8[2];
uint16_t v16;
 } val;

 val.v8[0] = 0;
 val.v8[1] = iph->proto;

>>>
>>> The val.val16 must be swapped on LE platforms. 
>>
>> NO.
>>
>> Did you checked this ?
> 
> And you?
> 
>>
>>>
>>>
 sum = (iph->src_addr & 0x) + (iph->src_addr >> 16) +
(iph->dst_addr & 0x) + (iph->dst_addr >> 16) +
val.v16 + udph->length;

> + udplen = odp_be_to_cpu_16(udph->length);
> + buf = (uint16_t *)((void *)udph);
> + /* 32-bit sum of UDP header (checksum field cleared) and UDP data */
> + for ( ; udplen > 1; udplen -= 2)
> + sum += *buf++;
> + /* Length is not a multiple of 2 bytes */
> + if (udplen)
> + sum += u8_pad_to_u16(*buf, 0);
 val.v8[0] = *buf;
 val.v8[1] = 0;

 sum += val.v16;

 Isn't it more understandable, then artificial byte swapping?

>>>
>>> The val.val16 must be swapped on LE platforms.
>>
>> NO.
>>
>> Did you checked this ?
> 
> And you?
> 
> P.S. Just to be honest, I checked this a few minutes ago.
>  Helper test below works.
> 
> Yes on BE/LE platforms, but not with the change you proposed.


So why you talking that val.val16 must be swapped if you do not understand
how it works and you have not tested it?
___
lng-odp mailing list
lng-odp@lists.linaro.org
https://lists.linaro.org/mailman/listinfo/lng-odp


Re: [lng-odp] [PATCHv6] helper : Fix UDP checksum computation

2015-12-11 Thread Ion Grigore
Comments inlined

-Original Message-
From: Ilya Maximets [mailto:i.maxim...@samsung.com] 
Sent: Friday, December 11, 2015 5:40 PM
To: Grigore Ion-B17953 ; Maxim Uvarov 
; lng-odp@lists.linaro.org
Subject: Re: [PATCHv6] helper : Fix UDP checksum computation

On 11.12.2015 18:28, Ion Grigore wrote:
> 
> Comments inlined
> 
> -Original Message-
> From: Ilya Maximets [mailto:i.maxim...@samsung.com]
> Sent: Friday, December 11, 2015 5:24 PM
> To: Grigore Ion-B17953 ; Maxim Uvarov 
> ; lng-odp@lists.linaro.org
> Subject: Re: [PATCHv6] helper : Fix UDP checksum computation
> 
> On 11.12.2015 17:07, Ion Grigore wrote:
>> Comments inlined
>>
>> -Original Message-
>> From: Maxim Uvarov [mailto:maxim.uva...@linaro.org]
>> Sent: Friday, December 11, 2015 11:20 AM
>> To: Ilya Maximets ; Grigore Ion-B17953 
>> ; lng-odp@lists.linaro.org
>> Subject: Re: [PATCHv6] helper : Fix UDP checksum computation
>>
>> On 12/11/2015 10:39, Ilya Maximets wrote:
>>> Comments inlined.
>>>
>>> On 10.12.2015 12:59, ion.grig...@freescale.com wrote:
 From: Grigore Ion 

 This patch fixes the following problems:
 - checksum computation for LE platforms
 - checksum is computed in the CPU endianness. The returned result 
 must be converted to the BE ordering when it is used to update the 
 UDP checksum in a packet.
 - checksum computation for packets having the UDP length not a 
 multiple of 2
 - fixes the UDP checksum associated validation test

 Signed-off-by: Grigore Ion 
 ---
   v6:
   - Make code more understandable (Ilya Maximets)
   v5:
   - Checksum in CPU endianness fix added (Ilya Maximets)
   v4:
   - Verify checksum in CPU endianness in the associated test
   (Ilya Maximets)
   v3:
   - fix the UDP checksum computation in the associated test
   (Maxim Uvarov)
   v2:
   - patch updated to the last master (Maxim Uvarov)
   v1:
   - Move variables declaration on top of block. (Maxim Uvarov)
   - Check patch with checkpatch script.  (Maxim Uvarov)
   - L3 header presence is tested twice. (Alexandru Badicioiu)
   - Remove unnecessary check for L3 header presence. (Bill Fischofer)
   - Modify check of odp_packet_l4_offset() return. (Bill Fischofer)

   helper/include/odp/helper/udp.h | 81 
 +
   helper/test/odp_chksum.c|  4 +-
   2 files changed, 51 insertions(+), 34 deletions(-)

 diff --git a/helper/include/odp/helper/udp.h 
 b/helper/include/odp/helper/udp.h index 06c439b..d0599b1 100644
 --- a/helper/include/odp/helper/udp.h
 +++ b/helper/include/odp/helper/udp.h
 @@ -4,7 +4,6 @@
* SPDX-License-Identifier: BSD-3-Clause
*/
   
 -
   /**
* @file
*
 @@ -22,7 +21,6 @@ extern "C" {
   #include 
   #include 
   
 -
   /** @addtogroup odph_header ODPH HEADER
*  @{
*/
 @@ -39,51 +37,70 @@ typedef struct ODP_PACKED {
   } odph_udphdr_t;
   
   /**
 + * Perform byte swap required by UDP checksum computation 
 +algorithm */ static inline uint16_t csum_bswap(uint16_t val) { #if 
 +ODP_BYTE_ORDER == ODP_LITTLE_ENDIAN
 +  val = __odp_builtin_bswap16((__odp_force uint16_t)val); #endif
 +  return val;
 +}
>>> Please, don't duplicate the code.
>>>
 +
 +/**
 + * Pad a byte value to the left or to the right as required by UDP 
 +checksum
 + * computation algorithm and convert the result to CPU native uint16_t.
 + * Left padding is performed for the IP protocol field in the UDP
 + * pseudo-header (RFC 768). Right padding is performed in the case 
 +of the odd
 + * byte in a UDP packet having the length not a 2 multiple.
 + */
 +static inline uint16_t u8_pad_to_u16(uint8_t val, odp_bool_t left) {
 +  uint16_tret;
 +
 +  ret = (left) ? val : val << 8;
 +  return csum_bswap(ret);
 +}
 +
 +/**
* UDP checksum
*
* This function uses odp packet to calc checksum
*
* @param pkt  calculate chksum for pkt
 - * @return  checksum value
 + * @return  checksum value in CPU endianness
*/
   static inline uint16_t odph_ipv4_udp_chksum(odp_packet_t pkt)
   {
 -  uint32_t sum = 0;
 -  odph_udphdr_t *udph;
 -  odph_ipv4hdr_t *iph;
 -  uint16_t udplen;
 -  uint8_t *buf;
 -
 -  if (!odp_packet_l3_offset(pkt))
 -  return 0;
 +  odph_ipv4hdr_t  *iph;
 +  odph_udphdr_t   *udph;
 +  uint32_tsum;
 +  uint16_tudplen, *buf;
   
 -  if (!odp_packet_l4_offset(pkt))
 +  if 

Re: [lng-odp] [PATCHv6] helper : Fix UDP checksum computation

2015-12-11 Thread Ion Grigore
A more precise question. Did you check on a LE platform ?

-Original Message-
From: Grigore Ion-B17953 
Sent: Friday, December 11, 2015 5:44 PM
To: 'Ilya Maximets' ; Maxim Uvarov 
; lng-odp@lists.linaro.org
Subject: RE: [PATCHv6] helper : Fix UDP checksum computation

Comments inlined

-Original Message-
From: Ilya Maximets [mailto:i.maxim...@samsung.com]
Sent: Friday, December 11, 2015 5:40 PM
To: Grigore Ion-B17953 ; Maxim Uvarov 
; lng-odp@lists.linaro.org
Subject: Re: [PATCHv6] helper : Fix UDP checksum computation

On 11.12.2015 18:28, Ion Grigore wrote:
> 
> Comments inlined
> 
> -Original Message-
> From: Ilya Maximets [mailto:i.maxim...@samsung.com]
> Sent: Friday, December 11, 2015 5:24 PM
> To: Grigore Ion-B17953 ; Maxim Uvarov 
> ; lng-odp@lists.linaro.org
> Subject: Re: [PATCHv6] helper : Fix UDP checksum computation
> 
> On 11.12.2015 17:07, Ion Grigore wrote:
>> Comments inlined
>>
>> -Original Message-
>> From: Maxim Uvarov [mailto:maxim.uva...@linaro.org]
>> Sent: Friday, December 11, 2015 11:20 AM
>> To: Ilya Maximets ; Grigore Ion-B17953 
>> ; lng-odp@lists.linaro.org
>> Subject: Re: [PATCHv6] helper : Fix UDP checksum computation
>>
>> On 12/11/2015 10:39, Ilya Maximets wrote:
>>> Comments inlined.
>>>
>>> On 10.12.2015 12:59, ion.grig...@freescale.com wrote:
 From: Grigore Ion 

 This patch fixes the following problems:
 - checksum computation for LE platforms
 - checksum is computed in the CPU endianness. The returned result 
 must be converted to the BE ordering when it is used to update the 
 UDP checksum in a packet.
 - checksum computation for packets having the UDP length not a 
 multiple of 2
 - fixes the UDP checksum associated validation test

 Signed-off-by: Grigore Ion 
 ---
   v6:
   - Make code more understandable (Ilya Maximets)
   v5:
   - Checksum in CPU endianness fix added (Ilya Maximets)
   v4:
   - Verify checksum in CPU endianness in the associated test
   (Ilya Maximets)
   v3:
   - fix the UDP checksum computation in the associated test
   (Maxim Uvarov)
   v2:
   - patch updated to the last master (Maxim Uvarov)
   v1:
   - Move variables declaration on top of block. (Maxim Uvarov)
   - Check patch with checkpatch script.  (Maxim Uvarov)
   - L3 header presence is tested twice. (Alexandru Badicioiu)
   - Remove unnecessary check for L3 header presence. (Bill Fischofer)
   - Modify check of odp_packet_l4_offset() return. (Bill Fischofer)

   helper/include/odp/helper/udp.h | 81 
 +
   helper/test/odp_chksum.c|  4 +-
   2 files changed, 51 insertions(+), 34 deletions(-)

 diff --git a/helper/include/odp/helper/udp.h 
 b/helper/include/odp/helper/udp.h index 06c439b..d0599b1 100644
 --- a/helper/include/odp/helper/udp.h
 +++ b/helper/include/odp/helper/udp.h
 @@ -4,7 +4,6 @@
* SPDX-License-Identifier: BSD-3-Clause
*/
   
 -
   /**
* @file
*
 @@ -22,7 +21,6 @@ extern "C" {
   #include 
   #include 
   
 -
   /** @addtogroup odph_header ODPH HEADER
*  @{
*/
 @@ -39,51 +37,70 @@ typedef struct ODP_PACKED {
   } odph_udphdr_t;
   
   /**
 + * Perform byte swap required by UDP checksum computation 
 +algorithm */ static inline uint16_t csum_bswap(uint16_t val) { #if 
 +ODP_BYTE_ORDER == ODP_LITTLE_ENDIAN
 +  val = __odp_builtin_bswap16((__odp_force uint16_t)val); #endif
 +  return val;
 +}
>>> Please, don't duplicate the code.
>>>
 +
 +/**
 + * Pad a byte value to the left or to the right as required by UDP 
 +checksum
 + * computation algorithm and convert the result to CPU native uint16_t.
 + * Left padding is performed for the IP protocol field in the UDP
 + * pseudo-header (RFC 768). Right padding is performed in the case 
 +of the odd
 + * byte in a UDP packet having the length not a 2 multiple.
 + */
 +static inline uint16_t u8_pad_to_u16(uint8_t val, odp_bool_t left) {
 +  uint16_tret;
 +
 +  ret = (left) ? val : val << 8;
 +  return csum_bswap(ret);
 +}
 +
 +/**
* UDP checksum
*
* This function uses odp packet to calc checksum
*
* @param pkt  calculate chksum for pkt
 - * @return  checksum value
 + * @return  checksum value in CPU endianness
*/
   static inline uint16_t odph_ipv4_udp_chksum(odp_packet_t pkt)
   {
 -  uint32_t sum = 0;
 -  odph_udphdr_t *udph;
 -  odph_ipv4hdr_t *iph;

Re: [lng-odp] [PATCHv6] helper : Fix UDP checksum computation

2015-12-11 Thread Ilya Maximets
On 11.12.2015 17:07, Ion Grigore wrote:
> Comments inlined
> 
> -Original Message-
> From: Maxim Uvarov [mailto:maxim.uva...@linaro.org] 
> Sent: Friday, December 11, 2015 11:20 AM
> To: Ilya Maximets ; Grigore Ion-B17953 
> ; lng-odp@lists.linaro.org
> Subject: Re: [PATCHv6] helper : Fix UDP checksum computation
> 
> On 12/11/2015 10:39, Ilya Maximets wrote:
>> Comments inlined.
>>
>> On 10.12.2015 12:59, ion.grig...@freescale.com wrote:
>>> From: Grigore Ion 
>>>
>>> This patch fixes the following problems:
>>> - checksum computation for LE platforms
>>> - checksum is computed in the CPU endianness. The returned result 
>>> must be converted to the BE ordering when it is used to update the 
>>> UDP checksum in a packet.
>>> - checksum computation for packets having the UDP length not a 
>>> multiple of 2
>>> - fixes the UDP checksum associated validation test
>>>
>>> Signed-off-by: Grigore Ion 
>>> ---
>>>   v6:
>>>   - Make code more understandable (Ilya Maximets)
>>>   v5:
>>>   - Checksum in CPU endianness fix added (Ilya Maximets)
>>>   v4:
>>>   - Verify checksum in CPU endianness in the associated test
>>>   (Ilya Maximets)
>>>   v3:
>>>   - fix the UDP checksum computation in the associated test
>>>   (Maxim Uvarov)
>>>   v2:
>>>   - patch updated to the last master (Maxim Uvarov)
>>>   v1:
>>>   - Move variables declaration on top of block. (Maxim Uvarov)
>>>   - Check patch with checkpatch script.  (Maxim Uvarov)
>>>   - L3 header presence is tested twice. (Alexandru Badicioiu)
>>>   - Remove unnecessary check for L3 header presence. (Bill Fischofer)
>>>   - Modify check of odp_packet_l4_offset() return. (Bill Fischofer)
>>>
>>>   helper/include/odp/helper/udp.h | 81 
>>> +
>>>   helper/test/odp_chksum.c|  4 +-
>>>   2 files changed, 51 insertions(+), 34 deletions(-)
>>>
>>> diff --git a/helper/include/odp/helper/udp.h 
>>> b/helper/include/odp/helper/udp.h index 06c439b..d0599b1 100644
>>> --- a/helper/include/odp/helper/udp.h
>>> +++ b/helper/include/odp/helper/udp.h
>>> @@ -4,7 +4,6 @@
>>>* SPDX-License-Identifier: BSD-3-Clause
>>>*/
>>>   
>>> -
>>>   /**
>>>* @file
>>>*
>>> @@ -22,7 +21,6 @@ extern "C" {
>>>   #include 
>>>   #include 
>>>   
>>> -
>>>   /** @addtogroup odph_header ODPH HEADER
>>>*  @{
>>>*/
>>> @@ -39,51 +37,70 @@ typedef struct ODP_PACKED {
>>>   } odph_udphdr_t;
>>>   
>>>   /**
>>> + * Perform byte swap required by UDP checksum computation algorithm  
>>> +*/ static inline uint16_t csum_bswap(uint16_t val) { #if 
>>> +ODP_BYTE_ORDER == ODP_LITTLE_ENDIAN
>>> +   val = __odp_builtin_bswap16((__odp_force uint16_t)val); #endif
>>> +   return val;
>>> +}
>> Please, don't duplicate the code.
>>
>>> +
>>> +/**
>>> + * Pad a byte value to the left or to the right as required by UDP 
>>> +checksum
>>> + * computation algorithm and convert the result to CPU native uint16_t.
>>> + * Left padding is performed for the IP protocol field in the UDP
>>> + * pseudo-header (RFC 768). Right padding is performed in the case 
>>> +of the odd
>>> + * byte in a UDP packet having the length not a 2 multiple.
>>> + */
>>> +static inline uint16_t u8_pad_to_u16(uint8_t val, odp_bool_t left) {
>>> +   uint16_tret;
>>> +
>>> +   ret = (left) ? val : val << 8;
>>> +   return csum_bswap(ret);
>>> +}
>>> +
>>> +/**
>>>* UDP checksum
>>>*
>>>* This function uses odp packet to calc checksum
>>>*
>>>* @param pkt  calculate chksum for pkt
>>> - * @return  checksum value
>>> + * @return  checksum value in CPU endianness
>>>*/
>>>   static inline uint16_t odph_ipv4_udp_chksum(odp_packet_t pkt)
>>>   {
>>> -   uint32_t sum = 0;
>>> -   odph_udphdr_t *udph;
>>> -   odph_ipv4hdr_t *iph;
>>> -   uint16_t udplen;
>>> -   uint8_t *buf;
>>> -
>>> -   if (!odp_packet_l3_offset(pkt))
>>> -   return 0;
>>> +   odph_ipv4hdr_t  *iph;
>>> +   odph_udphdr_t   *udph;
>>> +   uint32_tsum;
>>> +   uint16_tudplen, *buf;
>>>   
>>> -   if (!odp_packet_l4_offset(pkt))
>>> +   if (odp_packet_l4_offset(pkt) == ODP_PACKET_OFFSET_INVALID)
>>> return 0;
>>> -
>>> iph = (odph_ipv4hdr_t *)odp_packet_l3_ptr(pkt, NULL);
>>> udph = (odph_udphdr_t *)odp_packet_l4_ptr(pkt, NULL);
>>> -   udplen = odp_be_to_cpu_16(udph->length);
>>> -
>>> -   /* 32-bit sum of all 16-bit words covered by UDP chksum */
>>> +   /* 32-bit sum of UDP pseudo-header */
>>> sum = (iph->src_addr & 0x) + (iph->src_addr >> 16) +
>>> - (iph->dst_addr & 0x) + (iph->dst_addr >> 16) +
>>> - (uint16_t)iph->proto + udplen;
>>> -   for (buf = (uint8_t *)udph; udplen > 1; udplen -= 2) {
>>> -   sum += ((*buf << 8) + *(buf + 1));
>>> -   buf += 2;
>>> -   }
>>> -
>>> -   /* Fold sum to 16 bits: add carrier to result */
>>> -   while (sum >> 16)
>>> -   sum = (sum 

Re: [lng-odp] [PATCHv6] helper : Fix UDP checksum computation

2015-12-11 Thread Ion Grigore

Comments inlined

-Original Message-
From: Ilya Maximets [mailto:i.maxim...@samsung.com] 
Sent: Friday, December 11, 2015 5:24 PM
To: Grigore Ion-B17953 ; Maxim Uvarov 
; lng-odp@lists.linaro.org
Subject: Re: [PATCHv6] helper : Fix UDP checksum computation

On 11.12.2015 17:07, Ion Grigore wrote:
> Comments inlined
> 
> -Original Message-
> From: Maxim Uvarov [mailto:maxim.uva...@linaro.org]
> Sent: Friday, December 11, 2015 11:20 AM
> To: Ilya Maximets ; Grigore Ion-B17953 
> ; lng-odp@lists.linaro.org
> Subject: Re: [PATCHv6] helper : Fix UDP checksum computation
> 
> On 12/11/2015 10:39, Ilya Maximets wrote:
>> Comments inlined.
>>
>> On 10.12.2015 12:59, ion.grig...@freescale.com wrote:
>>> From: Grigore Ion 
>>>
>>> This patch fixes the following problems:
>>> - checksum computation for LE platforms
>>> - checksum is computed in the CPU endianness. The returned result 
>>> must be converted to the BE ordering when it is used to update the 
>>> UDP checksum in a packet.
>>> - checksum computation for packets having the UDP length not a 
>>> multiple of 2
>>> - fixes the UDP checksum associated validation test
>>>
>>> Signed-off-by: Grigore Ion 
>>> ---
>>>   v6:
>>>   - Make code more understandable (Ilya Maximets)
>>>   v5:
>>>   - Checksum in CPU endianness fix added (Ilya Maximets)
>>>   v4:
>>>   - Verify checksum in CPU endianness in the associated test
>>>   (Ilya Maximets)
>>>   v3:
>>>   - fix the UDP checksum computation in the associated test
>>>   (Maxim Uvarov)
>>>   v2:
>>>   - patch updated to the last master (Maxim Uvarov)
>>>   v1:
>>>   - Move variables declaration on top of block. (Maxim Uvarov)
>>>   - Check patch with checkpatch script.  (Maxim Uvarov)
>>>   - L3 header presence is tested twice. (Alexandru Badicioiu)
>>>   - Remove unnecessary check for L3 header presence. (Bill Fischofer)
>>>   - Modify check of odp_packet_l4_offset() return. (Bill Fischofer)
>>>
>>>   helper/include/odp/helper/udp.h | 81 
>>> +
>>>   helper/test/odp_chksum.c|  4 +-
>>>   2 files changed, 51 insertions(+), 34 deletions(-)
>>>
>>> diff --git a/helper/include/odp/helper/udp.h 
>>> b/helper/include/odp/helper/udp.h index 06c439b..d0599b1 100644
>>> --- a/helper/include/odp/helper/udp.h
>>> +++ b/helper/include/odp/helper/udp.h
>>> @@ -4,7 +4,6 @@
>>>* SPDX-License-Identifier: BSD-3-Clause
>>>*/
>>>   
>>> -
>>>   /**
>>>* @file
>>>*
>>> @@ -22,7 +21,6 @@ extern "C" {
>>>   #include 
>>>   #include 
>>>   
>>> -
>>>   /** @addtogroup odph_header ODPH HEADER
>>>*  @{
>>>*/
>>> @@ -39,51 +37,70 @@ typedef struct ODP_PACKED {
>>>   } odph_udphdr_t;
>>>   
>>>   /**
>>> + * Perform byte swap required by UDP checksum computation algorithm 
>>> +*/ static inline uint16_t csum_bswap(uint16_t val) { #if 
>>> +ODP_BYTE_ORDER == ODP_LITTLE_ENDIAN
>>> +   val = __odp_builtin_bswap16((__odp_force uint16_t)val); #endif
>>> +   return val;
>>> +}
>> Please, don't duplicate the code.
>>
>>> +
>>> +/**
>>> + * Pad a byte value to the left or to the right as required by UDP 
>>> +checksum
>>> + * computation algorithm and convert the result to CPU native uint16_t.
>>> + * Left padding is performed for the IP protocol field in the UDP
>>> + * pseudo-header (RFC 768). Right padding is performed in the case 
>>> +of the odd
>>> + * byte in a UDP packet having the length not a 2 multiple.
>>> + */
>>> +static inline uint16_t u8_pad_to_u16(uint8_t val, odp_bool_t left) {
>>> +   uint16_tret;
>>> +
>>> +   ret = (left) ? val : val << 8;
>>> +   return csum_bswap(ret);
>>> +}
>>> +
>>> +/**
>>>* UDP checksum
>>>*
>>>* This function uses odp packet to calc checksum
>>>*
>>>* @param pkt  calculate chksum for pkt
>>> - * @return  checksum value
>>> + * @return  checksum value in CPU endianness
>>>*/
>>>   static inline uint16_t odph_ipv4_udp_chksum(odp_packet_t pkt)
>>>   {
>>> -   uint32_t sum = 0;
>>> -   odph_udphdr_t *udph;
>>> -   odph_ipv4hdr_t *iph;
>>> -   uint16_t udplen;
>>> -   uint8_t *buf;
>>> -
>>> -   if (!odp_packet_l3_offset(pkt))
>>> -   return 0;
>>> +   odph_ipv4hdr_t  *iph;
>>> +   odph_udphdr_t   *udph;
>>> +   uint32_tsum;
>>> +   uint16_tudplen, *buf;
>>>   
>>> -   if (!odp_packet_l4_offset(pkt))
>>> +   if (odp_packet_l4_offset(pkt) == ODP_PACKET_OFFSET_INVALID)
>>> return 0;
>>> -
>>> iph = (odph_ipv4hdr_t *)odp_packet_l3_ptr(pkt, NULL);
>>> udph = (odph_udphdr_t *)odp_packet_l4_ptr(pkt, NULL);
>>> -   udplen = odp_be_to_cpu_16(udph->length);
>>> -
>>> -   /* 32-bit sum of all 16-bit words covered by UDP chksum */
>>> +   /* 32-bit sum of UDP pseudo-header */
>>> sum = (iph->src_addr & 0x) + (iph->src_addr >> 16) +
>>> - (iph->dst_addr & 0x) + (iph->dst_addr >> 

Re: [lng-odp] [PATCHv6] helper : Fix UDP checksum computation

2015-12-11 Thread Ion Grigore
Ok. I'll check your proposal on BE/LE.

-Original Message-
From: Ilya Maximets [mailto:i.maxim...@samsung.com] 
Sent: Friday, December 11, 2015 5:54 PM
To: Grigore Ion-B17953 ; Maxim Uvarov 
; lng-odp@lists.linaro.org
Subject: Re: [PATCHv6] helper : Fix UDP checksum computation

On 11.12.2015 18:47, Ion Grigore wrote:
> A more precise question. Did you check on a LE platform ?

Yes. On my LE i7.

> - /* Fold sum to 16 bits: add carrier to result */
> - while (sum >> 16)
> - sum = (sum & 0x) + (sum >> 16);
> -
> + (iph->dst_addr & 0x) + (iph->dst_addr >> 16) +
> + u8_pad_to_u16(iph->proto, 1) + udph->length;
 I meant something like:

 union {
uint8_t v8[2];
uint16_t v16;
 } val;

 val.v8[0] = 0;
 val.v8[1] = iph->proto;

>>>
>>> The val.val16 must be swapped on LE platforms. 
>>
>> NO.
>>
>> Did you checked this ?
> 
> And you?
> 
>>
>>>
>>>
 sum = (iph->src_addr & 0x) + (iph->src_addr >> 16) +
(iph->dst_addr & 0x) + (iph->dst_addr >> 16) +
val.v16 + udph->length;

> + udplen = odp_be_to_cpu_16(udph->length);
> + buf = (uint16_t *)((void *)udph);
> + /* 32-bit sum of UDP header (checksum field cleared) and UDP data */
> + for ( ; udplen > 1; udplen -= 2)
> + sum += *buf++;
> + /* Length is not a multiple of 2 bytes */
> + if (udplen)
> + sum += u8_pad_to_u16(*buf, 0);
 val.v8[0] = *buf;
 val.v8[1] = 0;

 sum += val.v16;

 Isn't it more understandable, then artificial byte swapping?

>>>
>>> The val.val16 must be swapped on LE platforms.
>>
>> NO.
>>
>> Did you checked this ?
> 
> And you?
> 
> P.S. Just to be honest, I checked this a few minutes ago.
>  Helper test below works.
> 
> Yes on BE/LE platforms, but not with the change you proposed.


So why you talking that val.val16 must be swapped if you do not understand how 
it works and you have not tested it?
___
lng-odp mailing list
lng-odp@lists.linaro.org
https://lists.linaro.org/mailman/listinfo/lng-odp


Re: [lng-odp] [API-NEXT/PATCH] fix: classification: add null check for pool assigned to CoS

2015-12-11 Thread Maxim Uvarov


Reviewed-and-Tested-by: Maxim Uvarov 

On 12/11/2015 11:34, Balasubramanian Manoharan wrote:

Fixes a crash caused by pool not assigned to CoS.

Signed-off-by: Balasubramanian Manoharan 
---
  platform/linux-generic/pktio/pktio_common.c | 5 +++--
  1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/platform/linux-generic/pktio/pktio_common.c 
b/platform/linux-generic/pktio/pktio_common.c
index 1fb10a0..1ed3260 100644
--- a/platform/linux-generic/pktio/pktio_common.c
+++ b/platform/linux-generic/pktio/pktio_common.c
@@ -26,12 +26,13 @@ int _odp_packet_cls_enq(pktio_entry_t *pktio_entry,
  
  	_odp_cls_parse(_hdr, base);

cos = pktio_select_cos(pktio_entry, (uint8_t *)base, _hdr);
-   pool = cos->s.pool->s.pool_hdl;
  
  	/* if No CoS found then drop the packet */

-   if (cos == NULL || cos->s.queue == NULL)
+   if (cos == NULL || cos->s.queue == NULL || cos->s.pool == NULL)
return 0;
  
+	pool = cos->s.pool->s.pool_hdl;

+
pkt = odp_packet_alloc(pool, buf_len);
if (odp_unlikely(pkt == ODP_PACKET_INVALID))
return 0;


___
lng-odp mailing list
lng-odp@lists.linaro.org
https://lists.linaro.org/mailman/listinfo/lng-odp


[lng-odp] [PATCH] example: classifier: fix add queue param init call

2015-12-11 Thread Balasubramanian Manoharan
Fixes crash caused by queue param not being initialized.

Signed-off-by: Balasubramanian Manoharan 
---
 example/classifier/odp_classifier.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/example/classifier/odp_classifier.c 
b/example/classifier/odp_classifier.c
index c8f264f..81e6bf0 100644
--- a/example/classifier/odp_classifier.c
+++ b/example/classifier/odp_classifier.c
@@ -401,6 +401,7 @@ static void configure_cos_queue(odp_pktio_t pktio, 
appl_args_t *args)
};
 
stats->pmr = odp_pmr_create();
+   odp_queue_param_init();
qparam.sched.prio = i % odp_schedule_num_prio();
qparam.sched.sync = ODP_SCHED_SYNC_NONE;
qparam.sched.group = ODP_SCHED_GROUP_ALL;
-- 
1.9.1

___
lng-odp mailing list
lng-odp@lists.linaro.org
https://lists.linaro.org/mailman/listinfo/lng-odp


Re: [lng-odp] [PATCHv6] helper : Fix UDP checksum computation

2015-12-11 Thread Maxim Uvarov

On 12/11/2015 10:39, Ilya Maximets wrote:

Comments inlined.

On 10.12.2015 12:59, ion.grig...@freescale.com wrote:

From: Grigore Ion 

This patch fixes the following problems:
- checksum computation for LE platforms
- checksum is computed in the CPU endianness. The returned result
must be converted to the BE ordering when it is used to update
the UDP checksum in a packet.
- checksum computation for packets having the UDP length not a
multiple of 2
- fixes the UDP checksum associated validation test

Signed-off-by: Grigore Ion 
---
  v6:
  - Make code more understandable (Ilya Maximets)
  v5:
  - Checksum in CPU endianness fix added (Ilya Maximets)
  v4:
  - Verify checksum in CPU endianness in the associated test
  (Ilya Maximets)
  v3:
  - fix the UDP checksum computation in the associated test
  (Maxim Uvarov)
  v2:
  - patch updated to the last master (Maxim Uvarov)
  v1:
  - Move variables declaration on top of block. (Maxim Uvarov)
  - Check patch with checkpatch script.  (Maxim Uvarov)
  - L3 header presence is tested twice. (Alexandru Badicioiu)
  - Remove unnecessary check for L3 header presence. (Bill Fischofer)
  - Modify check of odp_packet_l4_offset() return. (Bill Fischofer)

  helper/include/odp/helper/udp.h | 81 +
  helper/test/odp_chksum.c|  4 +-
  2 files changed, 51 insertions(+), 34 deletions(-)

diff --git a/helper/include/odp/helper/udp.h b/helper/include/odp/helper/udp.h
index 06c439b..d0599b1 100644
--- a/helper/include/odp/helper/udp.h
+++ b/helper/include/odp/helper/udp.h
@@ -4,7 +4,6 @@
   * SPDX-License-Identifier: BSD-3-Clause
   */
  
-

  /**
   * @file
   *
@@ -22,7 +21,6 @@ extern "C" {
  #include 
  #include 
  
-

  /** @addtogroup odph_header ODPH HEADER
   *  @{
   */
@@ -39,51 +37,70 @@ typedef struct ODP_PACKED {
  } odph_udphdr_t;
  
  /**

+ * Perform byte swap required by UDP checksum computation algorithm
+ */
+static inline uint16_t csum_bswap(uint16_t val)
+{
+#if ODP_BYTE_ORDER == ODP_LITTLE_ENDIAN
+   val = __odp_builtin_bswap16((__odp_force uint16_t)val);
+#endif
+   return val;
+}

Please, don't duplicate the code.


+
+/**
+ * Pad a byte value to the left or to the right as required by UDP checksum
+ * computation algorithm and convert the result to CPU native uint16_t.
+ * Left padding is performed for the IP protocol field in the UDP
+ * pseudo-header (RFC 768). Right padding is performed in the case of the odd
+ * byte in a UDP packet having the length not a 2 multiple.
+ */
+static inline uint16_t u8_pad_to_u16(uint8_t val, odp_bool_t left)
+{
+   uint16_tret;
+
+   ret = (left) ? val : val << 8;
+   return csum_bswap(ret);
+}
+
+/**
   * UDP checksum
   *
   * This function uses odp packet to calc checksum
   *
   * @param pkt  calculate chksum for pkt
- * @return  checksum value
+ * @return  checksum value in CPU endianness
   */
  static inline uint16_t odph_ipv4_udp_chksum(odp_packet_t pkt)
  {
-   uint32_t sum = 0;
-   odph_udphdr_t *udph;
-   odph_ipv4hdr_t *iph;
-   uint16_t udplen;
-   uint8_t *buf;
-
-   if (!odp_packet_l3_offset(pkt))
-   return 0;
+   odph_ipv4hdr_t  *iph;
+   odph_udphdr_t   *udph;
+   uint32_tsum;
+   uint16_tudplen, *buf;
  
-	if (!odp_packet_l4_offset(pkt))

+   if (odp_packet_l4_offset(pkt) == ODP_PACKET_OFFSET_INVALID)
return 0;
-
iph = (odph_ipv4hdr_t *)odp_packet_l3_ptr(pkt, NULL);
udph = (odph_udphdr_t *)odp_packet_l4_ptr(pkt, NULL);
-   udplen = odp_be_to_cpu_16(udph->length);
-
-   /* 32-bit sum of all 16-bit words covered by UDP chksum */
+   /* 32-bit sum of UDP pseudo-header */
sum = (iph->src_addr & 0x) + (iph->src_addr >> 16) +
- (iph->dst_addr & 0x) + (iph->dst_addr >> 16) +
- (uint16_t)iph->proto + udplen;
-   for (buf = (uint8_t *)udph; udplen > 1; udplen -= 2) {
-   sum += ((*buf << 8) + *(buf + 1));
-   buf += 2;
-   }
-
-   /* Fold sum to 16 bits: add carrier to result */
-   while (sum >> 16)
-   sum = (sum & 0x) + (sum >> 16);
-
+   (iph->dst_addr & 0x) + (iph->dst_addr >> 16) +
+   u8_pad_to_u16(iph->proto, 1) + udph->length;

I meant something like:

union {
uint8_t v8[2];
uint16_t v16;
} val;

val.v8[0] = 0;
val.v8[1] = iph->proto;

sum = (iph->src_addr & 0x) + (iph->src_addr >> 16) +
   (iph->dst_addr & 0x) + (iph->dst_addr >> 16) +
   val.v16 + udph->length;


+   udplen = odp_be_to_cpu_16(udph->length);
+   buf = (uint16_t *)((void *)udph);
+   /* 32-bit sum of UDP header (checksum field cleared) and UDP data */
+   for ( ; udplen > 1; udplen -= 2)
+   sum += *buf++;
+   /* Length is not a multiple of 2 bytes */
+   if (udplen)
+   

[lng-odp] [API-NEXT/PATCHv2] pktio: linux-generic: add null check for pool assigned to CoS

2015-12-11 Thread Balasubramanian Manoharan
Fixes a crash caused by pool not assigned to CoS.

Signed-off-by: Balasubramanian Manoharan 
Reviewed-and-Tested-by: Maxim Uvarov 
---
v2: patch subject updated to ODP format

 platform/linux-generic/pktio/pktio_common.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/platform/linux-generic/pktio/pktio_common.c 
b/platform/linux-generic/pktio/pktio_common.c
index 1fb10a0..1ed3260 100644
--- a/platform/linux-generic/pktio/pktio_common.c
+++ b/platform/linux-generic/pktio/pktio_common.c
@@ -26,12 +26,13 @@ int _odp_packet_cls_enq(pktio_entry_t *pktio_entry,
 
_odp_cls_parse(_hdr, base);
cos = pktio_select_cos(pktio_entry, (uint8_t *)base, _hdr);
-   pool = cos->s.pool->s.pool_hdl;
 
/* if No CoS found then drop the packet */
-   if (cos == NULL || cos->s.queue == NULL)
+   if (cos == NULL || cos->s.queue == NULL || cos->s.pool == NULL)
return 0;
 
+   pool = cos->s.pool->s.pool_hdl;
+
pkt = odp_packet_alloc(pool, buf_len);
if (odp_unlikely(pkt == ODP_PACKET_INVALID))
return 0;
-- 
1.9.1

___
lng-odp mailing list
lng-odp@lists.linaro.org
https://lists.linaro.org/mailman/listinfo/lng-odp


[lng-odp] [PATCH] doc/users-guide: add helpers section

2015-12-11 Thread Mike Holmes
Signed-off-by: Mike Holmes 
---
 doc/users-guide/users-guide.adoc | 161 +++
 1 file changed, 161 insertions(+)

diff --git a/doc/users-guide/users-guide.adoc b/doc/users-guide/users-guide.adoc
index cf77fa0..d2e1a16 100644
--- a/doc/users-guide/users-guide.adoc
+++ b/doc/users-guide/users-guide.adoc
@@ -431,6 +431,167 @@ Applications only include the 'include/odp.h file which 
includes the 'platform/<
 The doxygen documentation defining the behavior of the ODP API is all 
contained in the public API files, and the actual definitions for an 
implementation will be found in the per platform directories.
 Per-platform data that might normally be a #define can be recovered via the 
appropriate access function if the #define is not directly visible to the 
application.
 
+== Helpers
+Many small helper functions and definitions are needed to enable ODP
+applications to be hardware optimized but not tied to a particular hardware or
+execution environment. These are typically implemented with inline functions,
+preprocessor macros, or compiler built­in features. Thus API definitions are
+normally inline when possible.
+
+=== Core enumeration
+Application or middleware need to handle physical and/or logical core IDs, core
+counts and core masks quite often. Core enumeration has to remain consistent
+even when core deployment may change during application execution (e.g., due to
+adaptation to changing traffic profile, etc).
+
+* +odp_cpumask_from_str()+
+* +odp_cpumask_to_str()+
+* +odp_cpumask_zero()+
+* +odp_cpumask_set()+
+* +odp_cpumask_setall()+
+* +odp_cpumask_clr()+
+* +odp_cpumask_isset()+
+* +odp_cpumask_count()+
+* +odp_cpumask_and()+
+* +odp_cpumask_or()+
+* +odp_cpumask_xor()+
+* +odp_cpumask_equal()+
+* +odp_cpumask_copy()+
+* +odp_cpumask_first()+
+* +odp_cpumask_last()+
+* +odp_cpumask_next()+
+* +odp_cpumask_default_worker()+
+* +odp_cpumask_default_control()+
+
+=== Memory alignments
+For optimal performance and scalability (e.g., to avoid false sharing and cache
+line aliasing), some application data structures need to be aligned to cache
+(cache line) and/or memory subsystem (page, DRAM burst) alignments.  NUMA
+systems also support location­awareness and potentially different cache line
+sizes on a per­memory basis. Static memory allocation Serves application needs
+for portable definitions for global and core/thread local data.
+
+* +ODP_ALIGNED+
+* +ODP_PACKED+
+* +ODP_OFFSETOF+
+* +ODP_FIELD_SIZEOF+
+* +ODP_CACHE_LINE_SIZE+
+* +ODP_PAGE_SIZE+
+* +ODP_ALIGNED_CACHE+
+* +ODP_ALIGNED_PAGE+
+
+=== Compiler hints
+The compiler and linker can do better optimizations if code includes hints on
+expected application  behavior.  Examples of these are classification of
+branches with likely/unlikely hints, or marking  code with hot (optimize for
+speed) or cold (optimize for size) tags.
+
+* +odp_likely()+
+* +odp_unlikely()+
+* +odp_prefetch()+
+* +odp_prefetch_store()+
+
+=== Atomic operations
+Modern ISAs offers various atomic instructions to access/manipulate data
+concurrently from multiple cores. Well scalable multicore software is possible
+only through correct usage (and combination) of hardware acceleration and
+atomic instructions. Applications use atomic operations to update global
+statistics, sequence counters, quotas, etc., and to build concurrent data
+structures.
+
+* +odp_atomic_init_u64()+
+* +odp_atomic_load_u64()+
+* +odp_atomic_store_u64()+
+* +odp_atomic_fetch_add_u64()+
+* +odp_atomic_add_u64()+
+* +odp_atomic_fetch_sub_u64()+
+* +odp_atomic_sub_u64()+
+* +odp_atomic_fetch_inc_u64()+
+* +odp_atomic_inc_u64()+
+* +odp_atomic_fetch_dec_u64()+
+* +odp_atomic_dec_u64()+
+
+=== Memory synchronization barriers
+Application (or middleware) needs a portable way to synchronize data
+modifications into main memory before messaging other cores or hardware
+acceleration about the changes. The nature of the synchronization needs are
+cache coherence protocol specific.
+
+* +odp_barrier_t()+
+* +odp_rwlock_t()+
+* +odp_ticketlock_t()+
+* +odp_barrier_init()+
+* +odp_barrier_wait()+
+* +odp_rwlock_init()+
+* +odp_rwlock_read_lock()+
+* +odp_rwlock_read_unlock()+
+* +odp_rwlock_write_lock()+
+* +odp_rwlock_write_unlock()+
+* +odp_sync_stores()+
+* +odp_ticketlock_init()+
+* +odp_ticketlock_lock()+
+* +odp_ticketlock_trylock()+
+* +odp_ticketlock_unlock()+
+* +odp_ticketlock_is_locked()+
+
+=== Execution barriers and spinlocks
+Although software locking should be avoided (especially in fast path code), at
+times there is no practical way to synchronize cores other than using execution
+barriers or spinlocks. For example, the application initialization phase
+typically is not performance critical and may be much simpler with synchronous
+interfaces and locking.
+
+* +odp_spinlock_t()+
+* +odp_spinlock_init()+
+* +odp_spinlock_lock()+
+* +odp_spinlock_trylock()+
+* +odp_spinlock_unlock()+
+* +odp_spinlock_is_locked()+
+
+=== Profiling and 

[lng-odp] [PATCHv4] doc: userguide: add application programming section

2015-12-11 Thread Bill Fischofer
Complete the reformatting to standard asciidoc style, expand the
ODP Application Programming section, and include a reorganized and
expanded discussion of ODP queues.

Signed-off-by: Bill Fischofer 
---
 doc/users-guide/users-guide.adoc | 450 +++
 1 file changed, 358 insertions(+), 92 deletions(-)

diff --git a/doc/users-guide/users-guide.adoc b/doc/users-guide/users-guide.adoc
index cf77fa0..2e30f3a 100644
--- a/doc/users-guide/users-guide.adoc
+++ b/doc/users-guide/users-guide.adoc
@@ -8,16 +8,19 @@ OpenDataPlane (ODP)  Users-Guide
 Abstract
 
 This document is intended to guide a new ODP application developer.
-Further details about ODP may be found at the http://opendataplane.org[ODP] 
home page.
+Further details about ODP may be found at the http://opendataplane.org[ODP]
+home page.
 
 .Overview of a system running ODP applications
 image::../images/overview.png[align="center"]
 
-ODP is an API specification that allows many implementations to provide 
platform independence, automatic hardware acceleration and CPU scaling to high 
performance networking  applications.
-This document describes how to write an application that can successfully take 
advantage of the API.
+ODP is an API specification that allows many implementations to provide
+platform independence, automatic hardware acceleration and CPU scaling to
+high performance networking  applications. This document describes how to
+write an application that can successfully take advantage of the API.
 
 :numbered:
-== Introduction ==
+== Introduction
 .OpenDataPlane Components
 image::../images/odp_components.png[align="center"]
 
@@ -42,7 +45,7 @@ ODP API specification--that is the responsibility of each ODP 
implementation.
 * Application-centric.  Covers functional needs of data plane applications.
 * Ensures portability by specifying the functional behavior of ODP.
 * Defined jointly and openly by application writers and platform implementers.
-* Archiected to be implementable on a wide range of platforms efficiently
+* Architected to be implementable on a wide range of platforms efficiently
 * Sponsored, governed, and maintained by the Linaro Networking Group (LNG)
 
 .ODP Implementations
@@ -68,7 +71,7 @@ where the application will run on a target platform chosen by 
someone else.
 * One size does not fit all--supporting multiple implementations allows ODP
 to adapt to widely differing internals among platforms.
 * Anyone can create an ODP implementation tailored to their platform
-* Distribution and mainteinance of each implementation is as owner wishes
+* Distribution and maintenance of each implementation is as owner wishes
   - Open source or closed source as business needs determine
   - Have independent release cycles and service streams
 * Allows HW and SW innovation in how ODP APIs are implemented on each platform.
@@ -100,7 +103,7 @@ drivers supported by DPDK.
 they are derived from a reference implementation.
 
 .ODP Validation Test Suite
-Third, to enure consistency between different ODP implementations, ODP
+Third, to ensure consistency between different ODP implementations, ODP
 consists of a validation suite that verifies that any given implementation of
 ODP faithfully provides the specified functional behavior of each ODP API.
 As a separate open source component, the validation suite may be used by
@@ -115,16 +118,16 @@ ODP API specification.
 * Key to ensuring application portability across all ODP implementations
 * Tests that ODP implementations conform to the specified functional behavior
 of ODP APIs.
-* Can be run at any time by users and vendors to validat implementations
-od ODP.
+* Can be run at any time by users and vendors to validate implementations
+of ODP.
 
-=== ODP API Specification Versioning ===
+=== ODP API Specification Versioning
 As an evolving standard, the ODP API specification is released under an
 incrementing version number, and corresponding implementations of ODP, as well
 as the validation suite that verifies API conformance, are linked to this
-version number. ODP versions are specified using a stanard three-level
+version number. ODP versions are specified using a standard three-level
 number (major.minor.fixlevel) that are incremented according to the degree of
-change the level represents. Increments to the fixlevel represent clarification
+change the level represents. Increments to the fix level represent 
clarification
 of the specification or other minor changes that do not affect either the
 syntax or semantics of the specification. Such changes in the API specification
 are expected to be rare. Increments to the minor level
@@ -136,26 +139,26 @@ the major level represent significant structural changes 
that most likely
 require some level of application source code change, again as documented in
 the release notes for that version.
 
-=== ODP Implementation Versioning ===
+=== ODP Implementation Versioning
 ODP 

[lng-odp] [PATCH] api: pktio link status

2015-12-11 Thread Maxim Uvarov
Signed-off-by: Maxim Uvarov 
---
 v1 was link_state(), now it's link_status().

 include/odp/api/packet_io.h | 11 +++
 1 file changed, 11 insertions(+)

diff --git a/include/odp/api/packet_io.h b/include/odp/api/packet_io.h
index 443841e..8999b2c 100644
--- a/include/odp/api/packet_io.h
+++ b/include/odp/api/packet_io.h
@@ -673,6 +673,17 @@ void 
odp_pktio_output_queue_param_init(odp_pktio_output_queue_param_t *param);
 void odp_pktio_print(odp_pktio_t pktio);
 
 /**
+ * Determine pktio link is up or down for a packet IO interface.
+ *
+ * @param pktio Packet IO handle.
+ *
+ * @retval  1 link is up
+ * @retval  0 link is down
+ * @retval <0 on failure
+*/
+int odp_pktio_link_status(odp_pktio_t pktio);
+
+/**
  * @}
  */
 
-- 
1.9.1

___
lng-odp mailing list
lng-odp@lists.linaro.org
https://lists.linaro.org/mailman/listinfo/lng-odp


Re: [lng-odp] [PATCHv4] doc: userguide: add application programming section

2015-12-11 Thread Mike Holmes
On 11 December 2015 at 12:23, Bill Fischofer 
wrote:

> Complete the reformatting to standard asciidoc style, expand the
> ODP Application Programming section, and include a reorganized and
> expanded discussion of ODP queues.
>
> Signed-off-by: Bill Fischofer 
>

Reviewed-by: Mike Holmes 

Mike



> ---
>  doc/users-guide/users-guide.adoc | 450
> +++
>  1 file changed, 358 insertions(+), 92 deletions(-)
>
> diff --git a/doc/users-guide/users-guide.adoc
> b/doc/users-guide/users-guide.adoc
> index cf77fa0..2e30f3a 100644
> --- a/doc/users-guide/users-guide.adoc
> +++ b/doc/users-guide/users-guide.adoc
> @@ -8,16 +8,19 @@ OpenDataPlane (ODP)  Users-Guide
>  Abstract
>  
>  This document is intended to guide a new ODP application developer.
> -Further details about ODP may be found at the http://opendataplane.org[ODP]
> home page.
> +Further details about ODP may be found at the http://opendataplane.org
> [ODP]
> +home page.
>
>  .Overview of a system running ODP applications
>  image::../images/overview.png[align="center"]
>
> -ODP is an API specification that allows many implementations to provide
> platform independence, automatic hardware acceleration and CPU scaling to
> high performance networking  applications.
> -This document describes how to write an application that can successfully
> take advantage of the API.
> +ODP is an API specification that allows many implementations to provide
> +platform independence, automatic hardware acceleration and CPU scaling to
> +high performance networking  applications. This document describes how to
> +write an application that can successfully take advantage of the API.
>
>  :numbered:
> -== Introduction ==
> +== Introduction
>  .OpenDataPlane Components
>  image::../images/odp_components.png[align="center"]
>
> @@ -42,7 +45,7 @@ ODP API specification--that is the responsibility of
> each ODP implementation.
>  * Application-centric.  Covers functional needs of data plane
> applications.
>  * Ensures portability by specifying the functional behavior of ODP.
>  * Defined jointly and openly by application writers and platform
> implementers.
> -* Archiected to be implementable on a wide range of platforms efficiently
> +* Architected to be implementable on a wide range of platforms efficiently
>  * Sponsored, governed, and maintained by the Linaro Networking Group (LNG)
>
>  .ODP Implementations
> @@ -68,7 +71,7 @@ where the application will run on a target platform
> chosen by someone else.
>  * One size does not fit all--supporting multiple implementations allows
> ODP
>  to adapt to widely differing internals among platforms.
>  * Anyone can create an ODP implementation tailored to their platform
> -* Distribution and mainteinance of each implementation is as owner wishes
> +* Distribution and maintenance of each implementation is as owner wishes
>- Open source or closed source as business needs determine
>- Have independent release cycles and service streams
>  * Allows HW and SW innovation in how ODP APIs are implemented on each
> platform.
> @@ -100,7 +103,7 @@ drivers supported by DPDK.
>  they are derived from a reference implementation.
>
>  .ODP Validation Test Suite
> -Third, to enure consistency between different ODP implementations, ODP
> +Third, to ensure consistency between different ODP implementations, ODP
>  consists of a validation suite that verifies that any given
> implementation of
>  ODP faithfully provides the specified functional behavior of each ODP API.
>  As a separate open source component, the validation suite may be used by
> @@ -115,16 +118,16 @@ ODP API specification.
>  * Key to ensuring application portability across all ODP implementations
>  * Tests that ODP implementations conform to the specified functional
> behavior
>  of ODP APIs.
> -* Can be run at any time by users and vendors to validat implementations
> -od ODP.
> +* Can be run at any time by users and vendors to validate implementations
> +of ODP.
>
> -=== ODP API Specification Versioning ===
> +=== ODP API Specification Versioning
>  As an evolving standard, the ODP API specification is released under an
>  incrementing version number, and corresponding implementations of ODP, as
> well
>  as the validation suite that verifies API conformance, are linked to this
> -version number. ODP versions are specified using a stanard three-level
> +version number. ODP versions are specified using a standard three-level
>  number (major.minor.fixlevel) that are incremented according to the
> degree of
> -change the level represents. Increments to the fixlevel represent
> clarification
> +change the level represents. Increments to the fix level represent
> clarification
>  of the specification or other minor changes that do not affect either the
>  syntax or semantics of the specification. Such changes in the API
> specification
>  are expected to 

[lng-odp] [API-NEXT/PATCH] fix: classification: add null check for pool assigned to CoS

2015-12-11 Thread Balasubramanian Manoharan
Fixes a crash caused by pool not assigned to CoS.

Signed-off-by: Balasubramanian Manoharan 
---
 platform/linux-generic/pktio/pktio_common.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/platform/linux-generic/pktio/pktio_common.c 
b/platform/linux-generic/pktio/pktio_common.c
index 1fb10a0..1ed3260 100644
--- a/platform/linux-generic/pktio/pktio_common.c
+++ b/platform/linux-generic/pktio/pktio_common.c
@@ -26,12 +26,13 @@ int _odp_packet_cls_enq(pktio_entry_t *pktio_entry,
 
_odp_cls_parse(_hdr, base);
cos = pktio_select_cos(pktio_entry, (uint8_t *)base, _hdr);
-   pool = cos->s.pool->s.pool_hdl;
 
/* if No CoS found then drop the packet */
-   if (cos == NULL || cos->s.queue == NULL)
+   if (cos == NULL || cos->s.queue == NULL || cos->s.pool == NULL)
return 0;
 
+   pool = cos->s.pool->s.pool_hdl;
+
pkt = odp_packet_alloc(pool, buf_len);
if (odp_unlikely(pkt == ODP_PACKET_INVALID))
return 0;
-- 
1.9.1

___
lng-odp mailing list
lng-odp@lists.linaro.org
https://lists.linaro.org/mailman/listinfo/lng-odp