Re: [lng-odp] [PATCH v2] linux-gen: build: de-couple abi compatibility from shared lib

2016-09-22 Thread Bill Fischofer
I've posted patch http://patches.opendataplane.org/patch/6993/ that
performs the same function as this patch but does so by using the autoconf
structure of the base code which means that it works either directly from
the ODP repo or from an installed copy of the library.

On Thu, Sep 22, 2016 at 5:17 PM, Bill Fischofer 
wrote:

> Actually, I have to withdraw my previous review of this patch. The problem
> is that the original works as part of autoconf to create the plat/inlines.h
> file from an inlines.h.in base while this one attempts to do this by
> specifying CFLAGs that #define ODP_ABI_COMPAT.
>
> That works fine if you're compiling from the odp git repo but fails if
> you've installed a copy of ODP elsewhere. To see this:
>
> ./bootstrap
> ./configure --prefix= ...any other options
> make
> make install
>
> Then try to build an external ODP application using this installed copy.
> I've used OFP here for convenience.
>
> ./bootstrap
> ./configure --with-odp= ...any other options
> make
>
> and you get lots of error messages of the form:
>
>   CC   cli/ofp_cli_alias.lo
> In file included from /tmp/odp/include/odp/api/byteorder.h:28:0,
>  from /tmp/odp/include/odp_api.h:28,
>  from /tmp/odp/include/odp.h:21,
>  from ../include/api/ofp_log.h:17,
>  from ../include/ofpi_log.h:8,
>  from cli/ofp_cli_alias.c:12:
> /tmp/odp/include/odp/api/plat/inlines.h:20:5: warning: "ODP_ABI_COMPAT"
> is not defined [-Wundef]
>  #if ODP_ABI_COMPAT == 0
>  ^
> In file included from /tmp/odp/include/odp_api.h:28:0,
>  from /tmp/odp/include/odp.h:21,
>  from ../include/api/ofp_log.h:17,
>  from ../include/ofpi_log.h:8,
>  from cli/ofp_cli_alias.c:12:
> /tmp/odp/include/odp/api/byteorder.h:29:5: warning: "ODP_ABI_COMPAT" is
> not defined [-Wundef]
>  #if ODP_ABI_COMPAT == 0
>  ^
> In file included from /tmp/odp/include/odp/api/barrier.h:21:0,
>  from /tmp/odp/include/odp_api.h:31,
>  from /tmp/odp/include/odp.h:21,
>  from ../include/api/ofp_log.h:17,
>  from ../include/ofpi_log.h:8,
>  from cli/ofp_cli_alias.c:12:
> /tmp/odp/include/odp/api/atomic.h:28:5: warning: "ODP_ABI_COMPAT" is not
> defined [-Wundef]
>  #if ODP_ABI_COMPAT == 0
>  ^
> ...etc.
>
> This is because the CFLAGS that were set to compile ODP aren't present
> when an application using the installed ODP library is subsequently
> compiled against the library. The installed library itself must contain the
> different expansion depending on whether --enable-abi-compat=[yes|no] was
> specified, and that means this needs to be done via autoconf, not a #define.
>
>
>
> On Wed, Sep 14, 2016 at 7:10 AM, Petri Savolainen <
> petri.savolai...@nokia.com> wrote:
>
>> Building ABI compatible or shared library are two different
>> targets. A shared library may be used also without ABI
>> compatibility. A new --enable-abi-compat configuration option
>> is introduced. By default libraries are not built in ABI compat
>> mode to enable function inlining. There is a noticeable
>> performance difference when e.g. odp_atomic_xxx calls
>> are not inlined.
>>
>> Signed-off-by: Petri Savolainen 
>> ---
>>
>> v2:
>>   * ABI compat enabled by default
>>   * print static/shared/abi_compat selection in config results
>>   * added missing header file include guards
>>
>>
>>  configure.ac   | 26
>> -
>>  platform/linux-generic/.gitignore  |  1 -
>>  platform/linux-generic/include/odp/api/atomic.h|  2 +-
>>  platform/linux-generic/include/odp/api/byteorder.h |  2 +-
>>  .../linux-generic/include/odp/api/plat/inlines.h   | 30
>> 
>>  .../include/odp/api/plat/inlines.h.in  | 33
>> --
>>  platform/linux-generic/include/odp/api/std_clib.h  |  2 +-
>>  platform/linux-generic/include/odp/api/sync.h  |  2 +-
>>  platform/linux-generic/m4/configure.m4 |  3 +-
>>  platform/linux-generic/odp_atomic.c|  2 +-
>>  platform/linux-generic/odp_byteorder.c |  2 +-
>>  platform/linux-generic/odp_std_clib.c  |  2 +-
>>  platform/linux-generic/odp_sync.c  |  2 +-
>>  13 files changed, 58 insertions(+), 51 deletions(-)
>>  delete mode 100644 platform/linux-generic/.gitignore
>>  create mode 100644 platform/linux-generic/include/odp/api/plat/inlines.h
>>  delete mode 100644 platform/linux-generic/include/odp/api/plat/
>> inlines.h.in
>>
>> diff --git a/configure.ac b/configure.ac
>> index 982aff7..e05d4dd 100644
>> --- a/configure.ac
>> +++ b/configure.ac
>> @@ -176,13 +176,6 @@ AM_CONDITIONAL([test_example], [test x$test_example
>> = xyes ])
>>  AM_CONDITIONAL([HAVE_DOXYGEN], [test "x${DOXYGEN}" = "xdoxygen"])
>>  

[lng-odp] [Bug 2510] ODP build system broken

2016-09-22 Thread bugzilla-daemon
https://bugs.linaro.org/show_bug.cgi?id=2510

Bill Fischofer  changed:

   What|Removed |Added

 Status|CONFIRMED   |IN_PROGRESS

--- Comment #5 from Bill Fischofer  ---
Petri's patch has its own problems when working with an installed copy of ODP,
so I've posed a revised version of that patch at
http://patches.opendataplane.org/patch/6993/

This issue is then fixed by patch http://patches.opendataplane.org/patch/6992/

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[lng-odp] [PATCH 1/2] config: abi: add --enable-abi-compat option with default=yes

2016-09-22 Thread Bill Fischofer
Suggested-by: Petri Savolainen 
Signed-off-by: Bill Fischofer 
---
Note that this patch supersedes patch
http://patches.opendataplane.org/patch/6967/

 configure.ac | 6 +-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/configure.ac b/configure.ac
index 982aff7..58c6a16 100644
--- a/configure.ac
+++ b/configure.ac
@@ -176,7 +176,11 @@ AM_CONDITIONAL([test_example], [test x$test_example = xyes 
])
 AM_CONDITIONAL([HAVE_DOXYGEN], [test "x${DOXYGEN}" = "xdoxygen"])
 AM_CONDITIONAL([user_guide], [test "x${user_guides}" = "xyes" ])
 AM_CONDITIONAL([HAVE_MSCGEN], [test "x${MSCGEN}" = "xmscgen"])
-if test x$enable_shared != xyes;
+
+##
+# Setup for ABI compatibility
+##
+if test x$enable_abi_compat != xno;
 then
_ODP_INLINES="_ODP_INLINES"
 else
-- 
2.7.4



[lng-odp] [PATCH 2/2] config: share: use relocatable code if --enable-shared=no is specified

2016-09-22 Thread Bill Fischofer
Resolve Bug https://bugs.linaro.org/show_bug.cgi?id=2510 by adding
CFLAGS=-fPIC when --enable-shared=no is in effect to force generation of
relocatable code.

Signed-off-by: Bill Fischofer 
---
 configure.ac | 8 
 1 file changed, 8 insertions(+)

diff --git a/configure.ac b/configure.ac
index 58c6a16..8659c40 100644
--- a/configure.ac
+++ b/configure.ac
@@ -178,6 +178,14 @@ AM_CONDITIONAL([user_guide], [test "x${user_guides}" = 
"xyes" ])
 AM_CONDITIONAL([HAVE_MSCGEN], [test "x${MSCGEN}" = "xmscgen"])
 
 ##
+# Generate PIC if sharing not supported
+##
+if test $enable_shared != xyes;
+then
+   CFLAGS="$CFLAGS -fPIC"
+fi
+
+##
 # Setup for ABI compatibility
 ##
 if test x$enable_abi_compat != xno;
-- 
2.7.4



Re: [lng-odp] [PATCH 1/1] validation: classification: fix TCP/UDP checksum update

2016-09-22 Thread Bala Manoharan
Regards,
Bala


On 21 September 2016 at 19:46, Brian Brooks  wrote:
> On 09/09 19:49:09, Balasubramanian Manoharan wrote:
>> Fixes https://bugs.linaro.org/show_bug.cgi?id=2512
>>
>> Signed-off-by: Balasubramanian Manoharan 
>> ---
>>  .../validation/api/classification/odp_classification_common.c  | 10 
>> +-
>>  1 file changed, 9 insertions(+), 1 deletion(-)
>>
>> diff --git 
>> a/test/common_plat/validation/api/classification/odp_classification_common.c 
>> b/test/common_plat/validation/api/classification/odp_classification_common.c
>> index 7a42ac7..93ac0c0 100644
>> --- 
>> a/test/common_plat/validation/api/classification/odp_classification_common.c
>> +++ 
>> b/test/common_plat/validation/api/classification/odp_classification_common.c
>> @@ -11,6 +11,7 @@
>>  #include 
>>  #include 
>>  #include 
>> +#include "test_debug.h"
>>
>>  typedef struct cls_test_packet {
>>   odp_u32be_t magic;
>> @@ -291,6 +292,8 @@ odp_packet_t create_packet_len(odp_pool_t pool, bool 
>> vlan,
>>   parse_ipv4_string(CLS_DEFAULT_SADDR, , );
>>   ip->src_addr = odp_cpu_to_be_32(addr);
>>   ip->ver_ihl = ODPH_IPV4 << 4 | ODPH_IPV4HDR_IHL_MIN;
>> + odp_packet_has_ipv4_set(pkt, 1);
>> +
>>   if (flag_udp)
>>   ip->tot_len = odp_cpu_to_be_16(ODPH_UDPHDR_LEN + payload_len +
>>  ODPH_IPV4HDR_LEN);
>> @@ -318,14 +321,19 @@ odp_packet_t create_packet_len(odp_pool_t pool, bool 
>> vlan,
>>   udp->dst_port = odp_cpu_to_be_16(CLS_DEFAULT_DPORT);
>>   udp->length = odp_cpu_to_be_16(payload_len + ODPH_UDPHDR_LEN);
>>   udp->chksum = 0;
>> + odp_packet_has_udp_set(pkt, 1);
>> + if (odph_udp_tcp_chksum(pkt, ODPH_CHKSUM_GENERATE, NULL) != 0)
>> + LOG_ERR("odph_udp_tcp_chksum failed\n");
>>   } else {
>>   odp_packet_l4_offset_set(pkt, offset);
>>   tcp = (odph_tcphdr_t *)odp_packet_l4_ptr(pkt, NULL);
>>   tcp->src_port = odp_cpu_to_be_16(CLS_DEFAULT_SPORT);
>>   tcp->dst_port = odp_cpu_to_be_16(CLS_DEFAULT_DPORT);
>>   tcp->hl = ODPH_TCPHDR_LEN / 4;
>> - /* TODO: checksum field has to be updated */
>>   tcp->cksm = 0;
>> + odp_packet_has_tcp_set(pkt, 1);
>> + if (odph_udp_tcp_chksum(pkt, ODPH_CHKSUM_GENERATE, NULL) != 0)
>> + LOG_ERR("odph_udp_tcp_chksum failed\n");
>
> Should pkt be free'd here and return ODP_PACKET_INVALID, or will this be 
> caught
> later on in the test?

Yes. We have to free the packet and mark the test case as failure.
Will update the same in next version.

Regards,
Bala
>
>>   }
>>
>>   /* set pkt sequence number */
>> --
>> 1.9.1
>>


Re: [lng-odp] Query regarding odph_ipv4_csum_update

2016-09-22 Thread Bala Manoharan
Regards,
Bala


On 22 September 2016 at 02:43, Sunil Kumar Kori  wrote:
> Hello Bala,
>
>
>
> I was looking into the issue related to IPv4 checksum update. Then After
> some browsing, I found that below patch.
>
> http://comments.gmane.org/gmane.network.opendataplane/6584
>
>
>
> Within this patch, ip->chksum = 0; is done wherever applicable.
>
>
>
> So got a doubt that is this instruction can be added within the API
> odph_ipv4_csum_update.

Yes. It can be added inside the function.
>
>
>
> Also have expectation doubt of API odph_ipv4_csum_update. Is this API is
> expected to calculate fresh checksum always ?
>
> Actually in few cases there may be requirement of incremental checksum only.
> So are these operations are expected from this API ?

Currently this API does not support incremental checksum but you can
add an additional API to handle that feature.
IMO, Since these APIs are part of helper functions adding a new API
might be easier than having multiple features in a single API.

Regards,
Bala
>
>
>
> Please provide your input.
>
>
>
> Ragards
>
> Sunil Kumar
>
>


Re: [lng-odp] [PATCH v2] linux-gen: build: de-couple abi compatibility from shared lib

2016-09-22 Thread Bill Fischofer
Actually, I have to withdraw my previous review of this patch. The problem
is that the original works as part of autoconf to create the plat/inlines.h
file from an inlines.h.in base while this one attempts to do this by
specifying CFLAGs that #define ODP_ABI_COMPAT.

That works fine if you're compiling from the odp git repo but fails if
you've installed a copy of ODP elsewhere. To see this:

./bootstrap
./configure --prefix= ...any other options
make
make install

Then try to build an external ODP application using this installed copy.
I've used OFP here for convenience.

./bootstrap
./configure --with-odp= ...any other options
make

and you get lots of error messages of the form:

  CC   cli/ofp_cli_alias.lo
In file included from /tmp/odp/include/odp/api/byteorder.h:28:0,
 from /tmp/odp/include/odp_api.h:28,
 from /tmp/odp/include/odp.h:21,
 from ../include/api/ofp_log.h:17,
 from ../include/ofpi_log.h:8,
 from cli/ofp_cli_alias.c:12:
/tmp/odp/include/odp/api/plat/inlines.h:20:5: warning: "ODP_ABI_COMPAT" is
not defined [-Wundef]
 #if ODP_ABI_COMPAT == 0
 ^
In file included from /tmp/odp/include/odp_api.h:28:0,
 from /tmp/odp/include/odp.h:21,
 from ../include/api/ofp_log.h:17,
 from ../include/ofpi_log.h:8,
 from cli/ofp_cli_alias.c:12:
/tmp/odp/include/odp/api/byteorder.h:29:5: warning: "ODP_ABI_COMPAT" is not
defined [-Wundef]
 #if ODP_ABI_COMPAT == 0
 ^
In file included from /tmp/odp/include/odp/api/barrier.h:21:0,
 from /tmp/odp/include/odp_api.h:31,
 from /tmp/odp/include/odp.h:21,
 from ../include/api/ofp_log.h:17,
 from ../include/ofpi_log.h:8,
 from cli/ofp_cli_alias.c:12:
/tmp/odp/include/odp/api/atomic.h:28:5: warning: "ODP_ABI_COMPAT" is not
defined [-Wundef]
 #if ODP_ABI_COMPAT == 0
 ^
...etc.

This is because the CFLAGS that were set to compile ODP aren't present when
an application using the installed ODP library is subsequently compiled
against the library. The installed library itself must contain the
different expansion depending on whether --enable-abi-compat=[yes|no] was
specified, and that means this needs to be done via autoconf, not a #define.



On Wed, Sep 14, 2016 at 7:10 AM, Petri Savolainen <
petri.savolai...@nokia.com> wrote:

> Building ABI compatible or shared library are two different
> targets. A shared library may be used also without ABI
> compatibility. A new --enable-abi-compat configuration option
> is introduced. By default libraries are not built in ABI compat
> mode to enable function inlining. There is a noticeable
> performance difference when e.g. odp_atomic_xxx calls
> are not inlined.
>
> Signed-off-by: Petri Savolainen 
> ---
>
> v2:
>   * ABI compat enabled by default
>   * print static/shared/abi_compat selection in config results
>   * added missing header file include guards
>
>
>  configure.ac   | 26 -
>  platform/linux-generic/.gitignore  |  1 -
>  platform/linux-generic/include/odp/api/atomic.h|  2 +-
>  platform/linux-generic/include/odp/api/byteorder.h |  2 +-
>  .../linux-generic/include/odp/api/plat/inlines.h   | 30
> 
>  .../include/odp/api/plat/inlines.h.in  | 33
> --
>  platform/linux-generic/include/odp/api/std_clib.h  |  2 +-
>  platform/linux-generic/include/odp/api/sync.h  |  2 +-
>  platform/linux-generic/m4/configure.m4 |  3 +-
>  platform/linux-generic/odp_atomic.c|  2 +-
>  platform/linux-generic/odp_byteorder.c |  2 +-
>  platform/linux-generic/odp_std_clib.c  |  2 +-
>  platform/linux-generic/odp_sync.c  |  2 +-
>  13 files changed, 58 insertions(+), 51 deletions(-)
>  delete mode 100644 platform/linux-generic/.gitignore
>  create mode 100644 platform/linux-generic/include/odp/api/plat/inlines.h
>  delete mode 100644 platform/linux-generic/include/odp/api/plat/inlines.
> h.in
>
> diff --git a/configure.ac b/configure.ac
> index 982aff7..e05d4dd 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -176,13 +176,6 @@ AM_CONDITIONAL([test_example], [test x$test_example =
> xyes ])
>  AM_CONDITIONAL([HAVE_DOXYGEN], [test "x${DOXYGEN}" = "xdoxygen"])
>  AM_CONDITIONAL([user_guide], [test "x${user_guides}" = "xyes" ])
>  AM_CONDITIONAL([HAVE_MSCGEN], [test "x${MSCGEN}" = "xmscgen"])
> -if test x$enable_shared != xyes;
> -then
> -   _ODP_INLINES="_ODP_INLINES"
> -else
> -   _ODP_INLINES="_ODP_NO_INLINES"
> -fi
> -AC_SUBST(_ODP_INLINES)
>
>  
> ##
>  # Setup doxygen documentation
> @@ -225,6 +218,22 @@ AC_ARG_ENABLE([debug],
>  ODP_CFLAGS="$ODP_CFLAGS -DODP_DEBUG=$ODP_DEBUG"
>
>  

Re: [lng-odp] [PATCH 1/6] linux-gen: queue: reuse enq_ and deq_multi

2016-09-22 Thread Bill Fischofer
I posted patch http://patches.opendataplane.org/patch/6991/ to address the
doxygen issues with this patch.

On Thu, Sep 22, 2016 at 12:00 PM, Maxim Uvarov 
wrote:

> that was merged but doxygen needs refining:
>
> /opt/Linaro/odp3.git/platform/linux-generic/include/odp/api/plat/ticketlock_inlines.h:21:
> warning: Member _odp_ticketlock_lock(odp_ticketlock_t *ticketlock)
> (function) of file ticketlock_inlines.h is not documented.
> /opt/Linaro/odp3.git/platform/linux-generic/include/odp/api/plat/ticketlock_inlines.h:36:
> warning: Member _odp_ticketlock_trylock(odp_ticketlock_t *tklock)
> (function) of file ticketlock_inlines.h is not documented.
> /opt/Linaro/odp3.git/platform/linux-generic/include/odp/api/plat/ticketlock_inlines.h:64:
> warning: Member _odp_ticketlock_unlock(odp_ticketlock_t *ticketlock)
> (function) of file ticketlock_inlines.h is not documented.
> /opt/Linaro/odp3.git/platform/linux-generic/include/odp/api/plat/ticketlock_inlines.h:76:
> warning: Member _odp_ticketlock_is_locked(odp_ticketlock_t *ticketlock)
> (function) of file ticketlock_inlines.h is not documented.
>
>
>
> On 09/20/16 06:35, Bill Fischofer wrote:
>
>> For this series:
>>
>> Reviewed-and-tested-by: Bill Fischofer 
>>
>> On Thu, Sep 15, 2016 at 8:39 AM, Petri Savolainen <
>> petri.savolai...@nokia.com> wrote:
>>
>> Reuse multi enqueue and dequeue implementations for single
>>> enq/deq operations. This enables implementation to
>>> concentrate on optimizing the multi operations. Single
>>> operations do not suffer a major performance decrease since
>>> compiler likely optimizes the inlined code for single
>>> operations (num is fixed to 1).
>>>
>>> Signed-off-by: Petri Savolainen 
>>> ---
>>>   platform/linux-generic/include/odp_schedule_if.h |   3 -
>>>   platform/linux-generic/odp_queue.c   | 134
>>> +++
>>>   platform/linux-generic/odp_schedule.c|   1 -
>>>   platform/linux-generic/odp_schedule_ordered.c|  20 
>>>   platform/linux-generic/odp_schedule_sp.c |  12 --
>>>   5 files changed, 41 insertions(+), 129 deletions(-)
>>>
>>> diff --git a/platform/linux-generic/include/odp_schedule_if.h
>>> b/platform/linux-generic/include/odp_schedule_if.h
>>> index 13cdfb3..df73e70 100644
>>> --- a/platform/linux-generic/include/odp_schedule_if.h
>>> +++ b/platform/linux-generic/include/odp_schedule_if.h
>>> @@ -30,8 +30,6 @@ typedef int (*schedule_init_queue_fn_t)(uint32_t
>>> queue_index,
>>> );
>>>   typedef void (*schedule_destroy_queue_fn_t)(uint32_t queue_index);
>>>   typedef int (*schedule_sched_queue_fn_t)(uint32_t queue_index);
>>> -typedef int (*schedule_ord_enq_fn_t)(uint32_t queue_index, void
>>> *buf_hdr,
>>> -int sustain, int *ret);
>>>   typedef int (*schedule_ord_enq_multi_fn_t)(uint32_t queue_index,
>>> void *buf_hdr[], int num,
>>> int sustain, int *ret);
>>> @@ -48,7 +46,6 @@ typedef struct schedule_fn_t {
>>>  schedule_init_queue_fn_tinit_queue;
>>>  schedule_destroy_queue_fn_t destroy_queue;
>>>  schedule_sched_queue_fn_t   sched_queue;
>>> -   schedule_ord_enq_fn_t   ord_enq;
>>>  schedule_ord_enq_multi_fn_t ord_enq_multi;
>>>  schedule_init_global_fn_t   init_global;
>>>  schedule_term_global_fn_t   term_global;
>>> diff --git a/platform/linux-generic/odp_queue.c
>>> b/platform/linux-generic/odp_queue.c
>>> index bec1e51..80d99e8 100644
>>> --- a/platform/linux-generic/odp_queue.c
>>> +++ b/platform/linux-generic/odp_queue.c
>>> @@ -65,19 +65,6 @@ static inline int queue_is_ordered(queue_entry_t *qe)
>>>  return qe->s.param.sched.sync == ODP_SCHED_SYNC_ORDERED;
>>>   }
>>>
>>> -static inline void queue_add(queue_entry_t *queue,
>>> -odp_buffer_hdr_t *buf_hdr)
>>> -{
>>> -   buf_hdr->next = NULL;
>>> -
>>> -   if (queue->s.head)
>>> -   queue->s.tail->next = buf_hdr;
>>> -   else
>>> -   queue->s.head = buf_hdr;
>>> -
>>> -   queue->s.tail = buf_hdr;
>>> -}
>>> -
>>>   queue_entry_t *get_qentry(uint32_t queue_id)
>>>   {
>>>  return _tbl->queue[queue_id];
>>> @@ -396,37 +383,8 @@ odp_queue_t odp_queue_lookup(const char *name)
>>>  return ODP_QUEUE_INVALID;
>>>   }
>>>
>>> -int queue_enq(queue_entry_t *queue, odp_buffer_hdr_t *buf_hdr, int
>>> sustain)
>>> -{
>>> -   int ret;
>>> -
>>> -   if (sched_fn->ord_enq(queue->s.index, buf_hdr, sustain, ))
>>> -   return ret;
>>> -
>>> -   LOCK(>s.lock);
>>> -
>>> -   if (odp_unlikely(queue->s.status < QUEUE_STATUS_READY)) {
>>> -   UNLOCK(>s.lock);
>>> -   ODP_ERR("Bad queue status\n");
>>> -   return -1;
>>> -   }
>>> -
>>> -   

[lng-odp] [PATCH] linux-generic: ticketlock: add missing doxygen for ticketlock_inlines.h

2016-09-22 Thread Bill Fischofer
Add the missing internal doxygen documentation for the ticketlock_inlines
functions used to accelerate odp-linux even when building with
--enable-abi-compat=yes

Signed-off-by: Bill Fischofer 
---
 .../include/odp/api/plat/ticketlock_inlines.h  | 26 ++
 1 file changed, 26 insertions(+)

diff --git a/platform/linux-generic/include/odp/api/plat/ticketlock_inlines.h 
b/platform/linux-generic/include/odp/api/plat/ticketlock_inlines.h
index 957d22e..87432a7 100644
--- a/platform/linux-generic/include/odp/api/plat/ticketlock_inlines.h
+++ b/platform/linux-generic/include/odp/api/plat/ticketlock_inlines.h
@@ -18,6 +18,11 @@
 #include 
 #include 
 
+/** @internal
+ * Acquire ticket lock.
+ *
+ * @param ticketlock Pointer to a ticket lock
+ */
 static inline void _odp_ticketlock_lock(odp_ticketlock_t *ticketlock)
 {
uint32_t ticket;
@@ -33,6 +38,14 @@ static inline void _odp_ticketlock_lock(odp_ticketlock_t 
*ticketlock)
odp_cpu_pause();
 }
 
+/** @internal
+ * Try to acquire ticket lock.
+ *
+ * @param tklock Pointer to a ticket lock
+ *
+ * @retval 1 lock acquired
+ * @retval 0 lock not acquired
+ */
 static inline int _odp_ticketlock_trylock(odp_ticketlock_t *tklock)
 {
/* We read 'next_ticket' and 'cur_ticket' non-atomically which should
@@ -61,6 +74,11 @@ static inline int _odp_ticketlock_trylock(odp_ticketlock_t 
*tklock)
return 0;
 }
 
+/** @internal
+ * Release ticket lock
+ *
+ * @param ticketlock Pointer to a ticket lock
+ */
 static inline void _odp_ticketlock_unlock(odp_ticketlock_t *ticketlock)
 {
/* Release the lock by incrementing 'cur_ticket'. As we are the
@@ -73,6 +91,14 @@ static inline void _odp_ticketlock_unlock(odp_ticketlock_t 
*ticketlock)
odp_atomic_store_rel_u32(>cur_ticket, cur + 1);
 }
 
+/** @internal
+ * Check if ticket lock is locked
+ *
+ * @param ticketlock Pointer to a ticket lock
+ *
+ * @retval 1 the lock is busy (locked)
+ * @retval 0 the lock is available (unlocked)
+ */
 static inline int _odp_ticketlock_is_locked(odp_ticketlock_t *ticketlock)
 {
/* Compare 'cur_ticket' with 'next_ticket'. Ideally we should read
-- 
2.7.4



Re: [lng-odp] [PATCH v2 1/2] test: perf: add new scheduling latency test

2016-09-22 Thread Maxim Uvarov

Merged,
Maxim.

On 09/22/16 01:11, Brian Brooks wrote:

For series:

Reviewed-and-tested-by: Brian Brooks 

On 09/14 11:53:06, Matias Elo wrote:

Add new scheduling latency benchmark application. The application
measures delays (avg, min, max) for high and low priority events.

The test has a configurable number of TRAFFIC events and few SAMPLE events
(one common or one per priority). The scheduling latency is only measured
from the SAMPLE events to minimize measurement overhead.

The application's command line arguments enable configuring:
- Number of processing threads
- Number of high/low priority queues
- Number of high/low priority events
- Use separate SAMPLE events for each priority
- Scheduled queue type (PARALLEL, ATOMIC, ORDERED)

Signed-off-by: Matias Elo 
---

V2:
- Remove unnecessary 'num_workers' initialization (Maxim)

  test/common_plat/performance/.gitignore  |   1 +
  test/common_plat/performance/Makefile.am |   4 +
  test/common_plat/performance/odp_sched_latency.c | 767 +++
  3 files changed, 772 insertions(+)
  create mode 100644 test/common_plat/performance/odp_sched_latency.c

diff --git a/test/common_plat/performance/.gitignore 
b/test/common_plat/performance/.gitignore
index edcc832..1527d25 100644
--- a/test/common_plat/performance/.gitignore
+++ b/test/common_plat/performance/.gitignore
@@ -4,4 +4,5 @@ odp_atomic
  odp_crypto
  odp_l2fwd
  odp_pktio_perf
+odp_sched_latency
  odp_scheduling
diff --git a/test/common_plat/performance/Makefile.am 
b/test/common_plat/performance/Makefile.am
index d23bb3e..f5dd8dd 100644
--- a/test/common_plat/performance/Makefile.am
+++ b/test/common_plat/performance/Makefile.am
@@ -5,6 +5,7 @@ TESTS_ENVIRONMENT += TEST_DIR=${builddir}
  EXECUTABLES = odp_crypto$(EXEEXT) odp_pktio_perf$(EXEEXT)
  
  COMPILE_ONLY = odp_l2fwd$(EXEEXT) \

+  odp_sched_latency$(EXEEXT) \
   odp_scheduling$(EXEEXT)
  
  TESTSCRIPTS = odp_l2fwd_run.sh \

@@ -20,6 +21,8 @@ bin_PROGRAMS = $(EXECUTABLES) $(COMPILE_ONLY)
  
  odp_crypto_LDFLAGS = $(AM_LDFLAGS) -static

  odp_crypto_CFLAGS = $(AM_CFLAGS) -I${top_srcdir}/test
+odp_sched_latency_LDFLAGS = $(AM_LDFLAGS) -static
+odp_sched_latency_CFLAGS = $(AM_CFLAGS) -I${top_srcdir}/test
  odp_scheduling_LDFLAGS = $(AM_LDFLAGS) -static
  odp_scheduling_CFLAGS = $(AM_CFLAGS) -I${top_srcdir}/test
  
@@ -27,6 +30,7 @@ noinst_HEADERS = \

  $(top_srcdir)/test/test_debug.h
  
  dist_odp_crypto_SOURCES = odp_crypto.c

+dist_odp_sched_latency_SOURCES = odp_sched_latency.c
  dist_odp_scheduling_SOURCES = odp_scheduling.c
  dist_odp_pktio_perf_SOURCES = odp_pktio_perf.c
  
diff --git a/test/common_plat/performance/odp_sched_latency.c b/test/common_plat/performance/odp_sched_latency.c

new file mode 100644
index 000..063fb21
--- /dev/null
+++ b/test/common_plat/performance/odp_sched_latency.c
@@ -0,0 +1,767 @@
+/* Copyright (c) 2016, Linaro Limited
+ * All rights reserved.
+ *
+ * SPDX-License-Identifier: BSD-3-Clause
+ */
+
+/**
+ * @file
+ *
+ * @example odp_sched_latency.c  ODP scheduling latency benchmark application
+ */
+
+#include 
+#include 
+#include 
+
+#include 
+
+/* ODP main header */
+#include 
+
+/* ODP helper for Linux apps */
+#include 
+
+/* GNU lib C */
+#include 
+
+#define MAX_WORKERS  64/**< Maximum number of worker threads */
+#define MAX_QUEUES   4096  /**< Maximum number of queues */
+#define EVENT_POOL_SIZE  (1024 * 1024) /**< Event pool size */
+#define TEST_ROUNDS (4 * 1024 * 1024)  /**< Test rounds for each thread */
+#define MAIN_THREAD   1 /**< Thread ID performing maintenance tasks */
+
+/* Default values for command line arguments */
+#define SAMPLE_EVENT_PER_PRIO0 /**< Allocate a separate sample event for
+each priority */
+#define HI_PRIO_EVENTS   0 /**< Number of high priority events */
+#define LO_PRIO_EVENTS  32 /**< Number of low priority events */
+#define HI_PRIO_QUEUES  16 /**< Number of high priority queues */
+#define LO_PRIO_QUEUES  64 /**< Number of low priority queues */
+
+#define EVENTS_PER_HI_PRIO_QUEUE 0  /**< Alloc HI_PRIO_QUEUES x HI_PRIO_EVENTS
+events */
+#define EVENTS_PER_LO_PRIO_QUEUE 1  /**< Alloc LO_PRIO_QUEUES x LO_PRIO_EVENTS
+events */
+ODP_STATIC_ASSERT(HI_PRIO_QUEUES <= MAX_QUEUES, "Too many HI priority queues");
+ODP_STATIC_ASSERT(LO_PRIO_QUEUES <= MAX_QUEUES, "Too many LO priority queues");
+
+#define CACHE_ALIGN_ROUNDUP(x)\
+   ((ODP_CACHE_LINE_SIZE) * \
+(((x) + ODP_CACHE_LINE_SIZE - 1) / (ODP_CACHE_LINE_SIZE)))
+
+/* Test priorities */
+#define NUM_PRIOS 2 /**< Number of tested priorities */
+#define HI_PRIO  0
+#define LO_PRIO  1
+
+/** Test event types */
+typedef enum {
+   WARM_UP, /**< Warm up event */
+   

Re: [lng-odp] [PATCH 1/6] linux-gen: queue: reuse enq_ and deq_multi

2016-09-22 Thread Maxim Uvarov

that was merged but doxygen needs refining:

/opt/Linaro/odp3.git/platform/linux-generic/include/odp/api/plat/ticketlock_inlines.h:21: 
warning: Member _odp_ticketlock_lock(odp_ticketlock_t *ticketlock) 
(function) of file ticketlock_inlines.h is not documented.
/opt/Linaro/odp3.git/platform/linux-generic/include/odp/api/plat/ticketlock_inlines.h:36: 
warning: Member _odp_ticketlock_trylock(odp_ticketlock_t *tklock) 
(function) of file ticketlock_inlines.h is not documented.
/opt/Linaro/odp3.git/platform/linux-generic/include/odp/api/plat/ticketlock_inlines.h:64: 
warning: Member _odp_ticketlock_unlock(odp_ticketlock_t *ticketlock) 
(function) of file ticketlock_inlines.h is not documented.
/opt/Linaro/odp3.git/platform/linux-generic/include/odp/api/plat/ticketlock_inlines.h:76: 
warning: Member _odp_ticketlock_is_locked(odp_ticketlock_t *ticketlock) 
(function) of file ticketlock_inlines.h is not documented.



On 09/20/16 06:35, Bill Fischofer wrote:

For this series:

Reviewed-and-tested-by: Bill Fischofer 

On Thu, Sep 15, 2016 at 8:39 AM, Petri Savolainen <
petri.savolai...@nokia.com> wrote:


Reuse multi enqueue and dequeue implementations for single
enq/deq operations. This enables implementation to
concentrate on optimizing the multi operations. Single
operations do not suffer a major performance decrease since
compiler likely optimizes the inlined code for single
operations (num is fixed to 1).

Signed-off-by: Petri Savolainen 
---
  platform/linux-generic/include/odp_schedule_if.h |   3 -
  platform/linux-generic/odp_queue.c   | 134
+++
  platform/linux-generic/odp_schedule.c|   1 -
  platform/linux-generic/odp_schedule_ordered.c|  20 
  platform/linux-generic/odp_schedule_sp.c |  12 --
  5 files changed, 41 insertions(+), 129 deletions(-)

diff --git a/platform/linux-generic/include/odp_schedule_if.h
b/platform/linux-generic/include/odp_schedule_if.h
index 13cdfb3..df73e70 100644
--- a/platform/linux-generic/include/odp_schedule_if.h
+++ b/platform/linux-generic/include/odp_schedule_if.h
@@ -30,8 +30,6 @@ typedef int (*schedule_init_queue_fn_t)(uint32_t
queue_index,
);
  typedef void (*schedule_destroy_queue_fn_t)(uint32_t queue_index);
  typedef int (*schedule_sched_queue_fn_t)(uint32_t queue_index);
-typedef int (*schedule_ord_enq_fn_t)(uint32_t queue_index, void *buf_hdr,
-int sustain, int *ret);
  typedef int (*schedule_ord_enq_multi_fn_t)(uint32_t queue_index,
void *buf_hdr[], int num,
int sustain, int *ret);
@@ -48,7 +46,6 @@ typedef struct schedule_fn_t {
 schedule_init_queue_fn_tinit_queue;
 schedule_destroy_queue_fn_t destroy_queue;
 schedule_sched_queue_fn_t   sched_queue;
-   schedule_ord_enq_fn_t   ord_enq;
 schedule_ord_enq_multi_fn_t ord_enq_multi;
 schedule_init_global_fn_t   init_global;
 schedule_term_global_fn_t   term_global;
diff --git a/platform/linux-generic/odp_queue.c
b/platform/linux-generic/odp_queue.c
index bec1e51..80d99e8 100644
--- a/platform/linux-generic/odp_queue.c
+++ b/platform/linux-generic/odp_queue.c
@@ -65,19 +65,6 @@ static inline int queue_is_ordered(queue_entry_t *qe)
 return qe->s.param.sched.sync == ODP_SCHED_SYNC_ORDERED;
  }

-static inline void queue_add(queue_entry_t *queue,
-odp_buffer_hdr_t *buf_hdr)
-{
-   buf_hdr->next = NULL;
-
-   if (queue->s.head)
-   queue->s.tail->next = buf_hdr;
-   else
-   queue->s.head = buf_hdr;
-
-   queue->s.tail = buf_hdr;
-}
-
  queue_entry_t *get_qentry(uint32_t queue_id)
  {
 return _tbl->queue[queue_id];
@@ -396,37 +383,8 @@ odp_queue_t odp_queue_lookup(const char *name)
 return ODP_QUEUE_INVALID;
  }

-int queue_enq(queue_entry_t *queue, odp_buffer_hdr_t *buf_hdr, int
sustain)
-{
-   int ret;
-
-   if (sched_fn->ord_enq(queue->s.index, buf_hdr, sustain, ))
-   return ret;
-
-   LOCK(>s.lock);
-
-   if (odp_unlikely(queue->s.status < QUEUE_STATUS_READY)) {
-   UNLOCK(>s.lock);
-   ODP_ERR("Bad queue status\n");
-   return -1;
-   }
-
-   queue_add(queue, buf_hdr);
-
-   if (queue->s.status == QUEUE_STATUS_NOTSCHED) {
-   queue->s.status = QUEUE_STATUS_SCHED;
-   UNLOCK(>s.lock);
-   if (sched_fn->sched_queue(queue->s.index))
-   ODP_ABORT("schedule_queue failed\n");
-   return 0;
-   }
-
-   UNLOCK(>s.lock);
-   return 0;
-}
-
-int queue_enq_multi(queue_entry_t *queue, odp_buffer_hdr_t *buf_hdr[],
-   int num, int sustain)
+static inline int enq_multi(queue_entry_t *queue, odp_buffer_hdr_t
*buf_hdr[],
+   

Re: [lng-odp] [PATCH 1/6] linux-gen: queue: reuse enq_ and deq_multi

2016-09-22 Thread Maxim Uvarov

Merged,
Maxim.

On 09/22/16 10:28, Savolainen, Petri (Nokia - FI/Espoo) wrote:

Which config ?

$ ./configure --with-platform=linux-generic
--prefix=/opt/Linaro/check-odp-v3.git/new-build --enable-test-perf
--enable-test-perf-proc --enable-test-helper --enable-test-example
--enable-test-vald --with-testdir=yes


Which pktio ?

socket mmap

pktio: setting up test interfaces pktiop0p1, pktiop1p0, pktiop2p3,
pktiop3p2.
   PKTIO: initialized loop interface.
   PKTIO: initialized pcap interface.
   PKTIO: initialized socket mmap, use export
ODP_PKTIO_DISABLE_SOCKET_MMAP=1 to disable.
   PKTIO: initialized socket mmsg,use export
ODP_PKTIO_DISABLE_SOCKET_MMSG=1 to disable.


   Which test case ?

test/common_plat/performance/odp_l2fwd_run

For validation I run apply-and-build.sh script with following parameters:
GIT_BRANCH=master PATCH_DIR=/opt/Linaro/odp3.git IGNORE_BASE=0 CLEANUP=0
ENABLE_DPDK_PKTIO=0 ENABLE_IPC_PKTIO=0 DISTCHECK=1 ./apply-and-build.sh

Maxim.



I was able reproduce it on TIP OF THE CURRENT MASTER. It's likely caused by the 
last commit, which changes generator default settings...

diff --git a/example/generator/odp_generator.c 
b/example/generator/odp_generator.c
index b0053b9..48d7f5f 100644
--- a/example/generator/odp_generator.c
+++ b/example/generator/odp_generator.c
@@ -946,6 +946,7 @@ static void parse_args(int argc, char *argv[], appl_args_t 
*appl_args)
 appl_args->number = -1;
 appl_args->payload = 56;
 appl_args->timeout = -1;
+   appl_args->interval = DEFAULT_PKT_INTERVAL;

 opterr = 0; /* do not issue errors on helper options */


-Petri








Re: [lng-odp] [PATCH] example: generator: actually use specified default

2016-09-22 Thread Maxim Uvarov

Reverted this patch due to have issues with performance/odp_l2fwd

Maxim.

On 09/14/16 05:50, Bill Fischofer wrote:

On Tue, Sep 13, 2016 at 12:35 PM, Mike Holmes 
wrote:


The help states default is 1000ms. 0 for flood mode, however the
default was incorrectly set to zero.

Signed-off-by: Mike Holmes 


Reviewed-by: Bill Fischofer 



---
  example/generator/odp_generator.c | 1 +
  1 file changed, 1 insertion(+)

diff --git a/example/generator/odp_generator.c b/example/generator/odp_
generator.c
index b0053b9..48d7f5f 100644
--- a/example/generator/odp_generator.c
+++ b/example/generator/odp_generator.c
@@ -946,6 +946,7 @@ static void parse_args(int argc, char *argv[],
appl_args_t *appl_args)
 appl_args->number = -1;
 appl_args->payload = 56;
 appl_args->timeout = -1;
+   appl_args->interval = DEFAULT_PKT_INTERVAL;

 opterr = 0; /* do not issue errors on helper options */

--
2.7.4






[lng-odp] [Bug 2510] ODP build system broken

2016-09-22 Thread bugzilla-daemon
https://bugs.linaro.org/show_bug.cgi?id=2510

Bill Fischofer  changed:

   What|Removed |Added

   Severity|critical|normal

--- Comment #4 from Bill Fischofer  ---
Sorry for the delay. Yes, this is a problem but the circumvention is
straightforward:

When building ODP with --enable-shared=no just add CFLAGS=-fPIC to the
./configure:

./bootstrap
./configure --prefix= --enable-shared=no CFLAGS=-fPIC
make
make install

This enables applications to link normally.

I'll post a patch that adds -fPIC by default if --enable-shared=no is
specified, but I'd like to do that on top of Petri's
--enable-abi-compat=[yes|no] change that's still awaiting merge.

-- 
You are receiving this mail because:
You are on the CC list for the bug.

Re: [lng-odp] [PATCH v2 3/5] example: l3fwd: make packet error check optional

2016-09-22 Thread Elo, Matias (Nokia - FI/Espoo)

On 22 Sep 2016, at 12.02, Forrest Shi 
> wrote:

Hi Matias,

see comments inline.

thanks,
Forrest


On 16 September 2016 at 15:13, Matias Elo 
> wrote:
Make packet error check optional as it forces full packet parse.

Signed-off-by: Matias Elo >
---
 example/l3fwd/odp_l3fwd.c | 24 ++--
 1 file changed, 18 insertions(+), 6 deletions(-)

diff --git a/example/l3fwd/odp_l3fwd.c b/example/l3fwd/odp_l3fwd.c
index 95c3d85..f767fb4 100644
--- a/example/l3fwd/odp_l3fwd.c
+++ b/example/l3fwd/odp_l3fwd.c
@@ -80,6 +80,7 @@ typedef struct {
uint32_t duration; /* seconds to run */
uint8_t hash_mode; /* 1:hash, 0:lpm */
uint8_t dest_mac_changed[MAX_NB_PKTIO]; /* 1: dest mac from cmdline */
+   int error_check; /* Check packets for errors */
 } app_args_t;

 struct {
@@ -241,10 +242,10 @@ static int l3fwd_pkt_lpm(odp_packet_t pkt, int sif)
 }

 /**
- * Drop packets which input parsing marked as containing errors.
+ * Drop unsupported packets and packets containing errors.
  *
- * Frees packets with error and modifies pkt_tbl[] to only contain packets with
- * no detected errors.
+ * Frees packets with errors or unsupported protocol and modifies pkt_tbl[] to
+ * only contain valid packets.
  *
  * @param pkt_tbl  Array of packets
  * @param num  Number of packets in pkt_tbl[]
@@ -256,12 +257,16 @@ static inline int drop_err_pkts(odp_packet_t pkt_tbl[], 
unsigned num)
odp_packet_t pkt;
unsigned dropped = 0;
unsigned i, j;
+   int err;

for (i = 0, j = 0; i < num; ++i) {
pkt = pkt_tbl[i];
+   err = 0;

-   if (odp_unlikely(odp_packet_has_error(pkt) ||
-!odp_packet_has_ipv4(pkt))) {
+   if (global.cmd_args.error_check)
+   err = odp_packet_has_error(pkt);
+
+   if (odp_unlikely(err || !odp_packet_has_ipv4(pkt))) {
odp_packet_free(pkt);
dropped++;
} else if (odp_unlikely(i != j++)) {
@@ -475,6 +480,8 @@ static void print_usage(char *progname)
   "  -q, --queue  Configure rx queue(s) for port\n"
   "optional, format: [(port, queue, thread),...]\n"
   "for example: -q '(0, 0, 1),(1,0,2)'\n"
+  "  -e, --error_check 0: Don't check packet errors (default)\n"
+  "1: Check packet errors\n"
   "  -h, --help   Display help and exit.\n\n"
   "\n", NO_PATH(progname), NO_PATH(progname)
);
@@ -495,12 +502,13 @@ static void parse_cmdline_args(int argc, char *argv[], 
app_args_t *args)
{"duration", optional_argument, NULL, 'd'}, /* return 'd' */
{"thread", optional_argument, NULL, 't'},   /* return 't' */
{"queue", optional_argument, NULL, 'q'},/* return 'q' */
+   {"error_check", required_argument, NULL, 'e'},

should it be optional_argument?

Actually the modified line is correct, but the four lines above it are not. The 
‘has_arg’ field defines whether the option takes an argument. When the options 
(s, d, t, q) are handled below it is assumed that an argument is available. I 
can fix this in V2.

-Matias



{"help", no_argument, NULL, 'h'},   /* return 'h' */
{NULL, 0, NULL, 0}
};

while (1) {
-   opt = getopt_long(argc, argv, "+s:t:d:i:r:q:h",
+   opt = getopt_long(argc, argv, "+s:t:d:i:r:q:e:h",
  longopts, _index);

if (opt == -1)
@@ -585,6 +593,10 @@ static void parse_cmdline_args(int argc, char *argv[], 
app_args_t *args)
args->route_str[route_index++] = local;
break;

+   case 'e':
+   args->error_check = atoi(optarg);
+   break;
+
case 'h':
print_usage(argv[0]);
exit(EXIT_SUCCESS);
--
2.7.4



Re: [lng-odp] [PATCH v2 1/5] example: l3fwd: add missing gitignores

2016-09-22 Thread Elo, Matias (Nokia - FI/Espoo)
> 22.09.2016 11:51, Forrest Shi пишет:
> > Hi Matias,
> >
> > On what condition, *.log and *.trs will be generated? Other examples have
> > no this.
> >
> > Thanks,
> > Forrest
> 'make check' creates this files. But if other examples do not have this
> in .gitignore then probably
> they exist in top level directory.

Yep, 'make check' creates some log files (odp_l3fwd_run.sh.log, 
odp_l3fwd_run.sh.trs, test-suite.log). At least the l2fwd_simple example has 
the same values in .gitignore.

-Matias

> 
> Maxim.
> 
> 
> > On 16 September 2016 at 15:13, Matias Elo  wrote:
> >
> >> Signed-off-by: Matias Elo 
> >> ---
> >>   example/l3fwd/.gitignore | 2 ++
> >>   1 file changed, 2 insertions(+)
> >>
> >> diff --git a/example/l3fwd/.gitignore b/example/l3fwd/.gitignore
> >> index 74e501f..3411830 100644
> >> --- a/example/l3fwd/.gitignore
> >> +++ b/example/l3fwd/.gitignore
> >> @@ -1 +1,3 @@
> >>   odp_l3fwd
> >> +*.log
> >> +*.trs
> >> --
> >> 2.7.4
> >>
> >>



Re: [lng-odp] [PATCH v2 1/5] example: l3fwd: add missing gitignores

2016-09-22 Thread Maxim Uvarov

22.09.2016 11:51, Forrest Shi пишет:

Hi Matias,

On what condition, *.log and *.trs will be generated? Other examples have
no this.

Thanks,
Forrest
'make check' creates this files. But if other examples do not have this 
in .gitignore then probably

they exist in top level directory.

Maxim.



On 16 September 2016 at 15:13, Matias Elo  wrote:


Signed-off-by: Matias Elo 
---
  example/l3fwd/.gitignore | 2 ++
  1 file changed, 2 insertions(+)

diff --git a/example/l3fwd/.gitignore b/example/l3fwd/.gitignore
index 74e501f..3411830 100644
--- a/example/l3fwd/.gitignore
+++ b/example/l3fwd/.gitignore
@@ -1 +1,3 @@
  odp_l3fwd
+*.log
+*.trs
--
2.7.4






Re: [lng-odp] [PATCH v2 3/5] example: l3fwd: make packet error check optional

2016-09-22 Thread Forrest Shi
Hi Matias,

see comments inline.

thanks,
Forrest


On 16 September 2016 at 15:13, Matias Elo  wrote:

> Make packet error check optional as it forces full packet parse.
>
> Signed-off-by: Matias Elo 
> ---
>  example/l3fwd/odp_l3fwd.c | 24 ++--
>  1 file changed, 18 insertions(+), 6 deletions(-)
>
> diff --git a/example/l3fwd/odp_l3fwd.c b/example/l3fwd/odp_l3fwd.c
> index 95c3d85..f767fb4 100644
> --- a/example/l3fwd/odp_l3fwd.c
> +++ b/example/l3fwd/odp_l3fwd.c
> @@ -80,6 +80,7 @@ typedef struct {
> uint32_t duration; /* seconds to run */
> uint8_t hash_mode; /* 1:hash, 0:lpm */
> uint8_t dest_mac_changed[MAX_NB_PKTIO]; /* 1: dest mac from
> cmdline */
> +   int error_check; /* Check packets for errors */
>  } app_args_t;
>
>  struct {
> @@ -241,10 +242,10 @@ static int l3fwd_pkt_lpm(odp_packet_t pkt, int sif)
>  }
>
>  /**
> - * Drop packets which input parsing marked as containing errors.
> + * Drop unsupported packets and packets containing errors.
>   *
> - * Frees packets with error and modifies pkt_tbl[] to only contain
> packets with
> - * no detected errors.
> + * Frees packets with errors or unsupported protocol and modifies
> pkt_tbl[] to
> + * only contain valid packets.
>   *
>   * @param pkt_tbl  Array of packets
>   * @param num  Number of packets in pkt_tbl[]
> @@ -256,12 +257,16 @@ static inline int drop_err_pkts(odp_packet_t
> pkt_tbl[], unsigned num)
> odp_packet_t pkt;
> unsigned dropped = 0;
> unsigned i, j;
> +   int err;
>
> for (i = 0, j = 0; i < num; ++i) {
> pkt = pkt_tbl[i];
> +   err = 0;
>
> -   if (odp_unlikely(odp_packet_has_error(pkt) ||
> -!odp_packet_has_ipv4(pkt))) {
> +   if (global.cmd_args.error_check)
> +   err = odp_packet_has_error(pkt);
> +
> +   if (odp_unlikely(err || !odp_packet_has_ipv4(pkt))) {
> odp_packet_free(pkt);
> dropped++;
> } else if (odp_unlikely(i != j++)) {
> @@ -475,6 +480,8 @@ static void print_usage(char *progname)
>"  -q, --queue  Configure rx queue(s) for port\n"
>"optional, format: [(port, queue, thread),...]\n"
>"for example: -q '(0, 0, 1),(1,0,2)'\n"
> +  "  -e, --error_check 0: Don't check packet errors
> (default)\n"
> +  "1: Check packet errors\n"
>"  -h, --help   Display help and exit.\n\n"
>"\n", NO_PATH(progname), NO_PATH(progname)
> );
> @@ -495,12 +502,13 @@ static void parse_cmdline_args(int argc, char
> *argv[], app_args_t *args)
> {"duration", optional_argument, NULL, 'd'}, /* return
> 'd' */
> {"thread", optional_argument, NULL, 't'},   /* return
> 't' */
> {"queue", optional_argument, NULL, 'q'},/* return
> 'q' */
> +   {"error_check", required_argument, NULL, 'e'},
>

should it be optional_argument?


> {"help", no_argument, NULL, 'h'},   /* return
> 'h' */
> {NULL, 0, NULL, 0}
> };
>
> while (1) {
> -   opt = getopt_long(argc, argv, "+s:t:d:i:r:q:h",
> +   opt = getopt_long(argc, argv, "+s:t:d:i:r:q:e:h",
>   longopts, _index);
>
> if (opt == -1)
> @@ -585,6 +593,10 @@ static void parse_cmdline_args(int argc, char
> *argv[], app_args_t *args)
> args->route_str[route_index++] = local;
> break;
>
> +   case 'e':
> +   args->error_check = atoi(optarg);
> +   break;
> +
> case 'h':
> print_usage(argv[0]);
> exit(EXIT_SUCCESS);
> --
> 2.7.4
>
>


Re: [lng-odp] [PATCH v2 1/5] example: l3fwd: add missing gitignores

2016-09-22 Thread Forrest Shi
Hi Matias,

On what condition, *.log and *.trs will be generated? Other examples have
no this.

Thanks,
Forrest

On 16 September 2016 at 15:13, Matias Elo  wrote:

> Signed-off-by: Matias Elo 
> ---
>  example/l3fwd/.gitignore | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/example/l3fwd/.gitignore b/example/l3fwd/.gitignore
> index 74e501f..3411830 100644
> --- a/example/l3fwd/.gitignore
> +++ b/example/l3fwd/.gitignore
> @@ -1 +1,3 @@
>  odp_l3fwd
> +*.log
> +*.trs
> --
> 2.7.4
>
>


Re: [lng-odp] RFC: packet interface to drivers

2016-09-22 Thread Christophe Milard
On 21 September 2016 at 17:55, Bill Fischofer  wrote:
>
>
>
> On Tue, Sep 20, 2016 at 10:16 AM, Christophe Milard 
>  wrote:
>>
>>
>>
>> On 20 September 2016 at 16:01, Bill Fischofer  
>> wrote:
>>>
>>>
>>>
>>> On Tue, Sep 20, 2016 at 8:30 AM, Christophe Milard 
>>>  wrote:

 Hi,

 I am here trying to make a summary of what is needed by the driver 
 interface
 regarding odp packet handling. Will serve as the base for the discussions
 at connect. Please read and comment... possibly at connect...

 /Christophe

 From the driver perspective, the situation is rather simple: what we need 
 is:

 /* definition of a packet segment descriptor:
  * A packet segment is just an area, continuous in virtual address space,
  * and continuous in the physical address space -at least when no iommu is
  * used, e.g for virtio-. Probably we want to have physical continuity in
  * all cases (to avoid handling different cases to start with), but that
  * would not take advantage of the remapping that can be done by iommus,
  * so it can come with a little performance penalty for iommu cases.
>>>
>>>
>>> I thought we had discussed and agreed that ODP would assume it is running 
>>> on a platform with IOMMU capability? Are there any non-IOMMU platforms of 
>>> interest that we need to support? If not, then I see no need to make this 
>>> provision. In ODP we already have an odp_packet_seg_t type that represents 
>>> a portion of an odp_packet_t that can be contiguously addressed.
>>
>>
>> yes. we did. but then the focus changed to virtio. there is no iommu there...
>
>
> I thought virtio is independent of the underlying HW. If we assume the 
> underlying HW has an IOMMU, then virtio should see the benefits of that, no?


I wish you were right, but this is not my understanding: The iommu is
a physical device that sits between the IO HW (the pci bus assuming
pci) and the physical memory. The virtio communication between a
switch in a host system and odp running in a guest only involves
memory. The physical iommu is not usable there, as I understand. It
seems there are ongoing initiative to emulate iommu in qemu/kvm so
that guests could use the iommu/dma interface on all drivers, but I
doesn't seem mature yet.
I'd be happy to hear what virtualization gurus say on this topic, I
have to admit.
But since our focus moved from physical pci NIC to host to guest
access with virtio, my understandling is that we'll have to cope with
guest physical memory. sadly.
>
>
>>
>>
>>>
>>>

  * Segments are shared among all odp threads (including linux processes),
>>>
>>>
>>> Might be more precise to simply say "segments are accessible to all odp 
>>> threads". Sharing implies simultaneous access, along with some notion of 
>>> coherence, which is something that probably isn't needed.
>>
>>
>> Tell me if I am wrong, but the default in ODP is that a queue access can be 
>> shared between different ODP thread (there is a flag  to garantee 
>> 1thread<->1queue access -and hence to have performance benefit-), but as it 
>> is now, nothing
>
>
> Yes, queues store events and can be shared among threads, but remember that 
> what's on a queue is an odp_event_t, *not* an address. Events of most 
> interest to drivers are packets, which are of type odp_packet_t and are 
> obtained via the odp_packet_from_event() API. An odp_packet_t cannot be used 
> to access memory since it is not an address but an opaque type. If the driver 
> (or anyone else) needs to address the contents of an odp_packet_t, it calls a 
> data access function like odp_packet_data(), odp_packet_offset(), 
> odp_packet_seg_data(), etc., which returns a void * that can be used for 
> memory access. How this is done is implementation-defined, but the intended 
> user of the returned address is solely the calling thread.


I don't really agree with that: Not that I see any error in what you
said, but all this is valid on the north API: what we decide to show
to a driver on the south API does not need to be the same: the
abstract notion of event is probably of no interest for a driver: even
the odp_packet_t don't need to be the same as the odpdrv_packet_t (or
whatever we call it, if there is a need for it): If these 2 objects
most likely refer to the same physical memory (at least on system
where packets are on main CPU memory), many packet manipulation
methods available on the north API interface are of no interest on the
south interface. Likewise, many methods on the south interface (such
as those splitting the packet in its physically contiguous segments)
have to reasons to be known on the north interface.
Is there any reason to abstract the packet type and then use a method
to get an address when ALL drivers are known to be willing to get this
address?
My point is: the north application 

Re: [lng-odp] Suspected SPAM - [PATCH 1/3] test: l2fwd: lookup table for sched mode

2016-09-22 Thread Savolainen, Petri (Nokia - FI/Espoo)
Ping. This series optimizes l2fwd schedule mode performance by enabling 
lockless packet output (when possible) and corrects a bug in queue/plain mode 
forwarding.



> -Original Message-
> From: lng-odp [mailto:lng-odp-boun...@lists.linaro.org] On Behalf Of Petri
> Savolainen
> Sent: Thursday, September 15, 2016 12:51 PM
> To: lng-odp@lists.linaro.org
> Subject: Suspected SPAM - [lng-odp] [PATCH 1/3] test: l2fwd: lookup table
> for sched mode
> 
> Replaced linear destination port search in scheduler mode
> with a lookup table. Table index is API provided pktio_index
> and data is application port index.
> 
> Signed-off-by: Petri Savolainen 
> ---
>  test/common_plat/performance/odp_l2fwd.c | 68 +++
> -
>  1 file changed, 41 insertions(+), 27 deletions(-)


Re: [lng-odp] [PATCH 1/6] linux-gen: queue: reuse enq_ and deq_multi

2016-09-22 Thread Savolainen, Petri (Nokia - FI/Espoo)
> > Which config ?
> 
>$ ./configure --with-platform=linux-generic
> --prefix=/opt/Linaro/check-odp-v3.git/new-build --enable-test-perf
> --enable-test-perf-proc --enable-test-helper --enable-test-example
> --enable-test-vald --with-testdir=yes
> 
> > Which pktio ?
> 
> socket mmap
> 
> pktio: setting up test interfaces pktiop0p1, pktiop1p0, pktiop2p3,
> pktiop3p2.
>   PKTIO: initialized loop interface.
>   PKTIO: initialized pcap interface.
>   PKTIO: initialized socket mmap, use export
> ODP_PKTIO_DISABLE_SOCKET_MMAP=1 to disable.
>   PKTIO: initialized socket mmsg,use export
> ODP_PKTIO_DISABLE_SOCKET_MMSG=1 to disable.
> 
> >   Which test case ?
> test/common_plat/performance/odp_l2fwd_run
> 
> For validation I run apply-and-build.sh script with following parameters:
> GIT_BRANCH=master PATCH_DIR=/opt/Linaro/odp3.git IGNORE_BASE=0 CLEANUP=0
> ENABLE_DPDK_PKTIO=0 ENABLE_IPC_PKTIO=0 DISTCHECK=1 ./apply-and-build.sh
> 
> Maxim.
> 


I was able reproduce it on TIP OF THE CURRENT MASTER. It's likely caused by the 
last commit, which changes generator default settings...

diff --git a/example/generator/odp_generator.c 
b/example/generator/odp_generator.c
index b0053b9..48d7f5f 100644
--- a/example/generator/odp_generator.c
+++ b/example/generator/odp_generator.c
@@ -946,6 +946,7 @@ static void parse_args(int argc, char *argv[], appl_args_t 
*appl_args)
appl_args->number = -1;
appl_args->payload = 56;
appl_args->timeout = -1;
+   appl_args->interval = DEFAULT_PKT_INTERVAL;
 
opterr = 0; /* do not issue errors on helper options */


-Petri