Re: [lng-odp] [API-NEXT PATCHv6 0/5] Add Packet Splice/Reference APIs

[Bill Fischofer]

Ping. I'd like to get some review feedback on this patch series so we can
make progress on this item. I believe this reflects the basics of what we
discussed at LAS16. But if the APIs need to be adjusted then what sort of
adjustment is needed?

Thanks.

On Mon, Oct 10, 2016 at 10:45 PM, Bill Fischofer 
wrote:

> This patch adds support for packet references and splices following
> discussions at LAS16 on this subject.
>
> I've changed things around from Petri's original proposal by splitting this
> into two separate APIs: odp_packet_splice() and odp_packet_ref(), where the
> latter is just a splice of a zero-length header on to a base packet. The
> various odp packet manipulation APIs have also been enhanced to behave
> sensibly when presented with a spliced packet as input. Reference counts
> are
> used to enable odp_packet_free() to not free a packet until all splices
> based
> on it are also freed.
>
> Also added are two new APIs for working with spliced packets as these seem
> necessary for completeness:
>
> - odp_packet_is_a_splice() tells whether an input packet is a splice, and
> if
> so how many spliced packets it contains
>
> - odp_packet_is_spliced() tells whether any splices have been created on
> this
> packet, and if so how many.
>
> Note that there is no odp_packet_unsplice() API. To remove a splice from a
> base packet currently requres that the splice be freed via an
> odp_packet_free() call. We should discuss and decide if such an API is
> warranted for symmetry.
>
> Changes for v6:
> - Correct splice handling in odp_packet_offset(), odp_packet_trunc_head(),
> and odp_packet_trunc_tail()
>
> Changes for v5:
> - Struct layout and pathlength optimizations
> - Correct array addressing in validation test suite
>
> Changes for v4:
> - Add negative tests to validation test suite
> - Fix implementation bugs relating to negative tests
>
> Changes for v3:
> - Bug fixes (detected by the validation tests)
> - Addition of validation tests for these new APIs
> - Diagrams and User Guide documentation for these new APIs
>
> Changes for v2:
> - Bug fixes
> - Enhance ODP packet segment APIs to behave properly with spliced packets
>
> Bill Fischofer (5):
>   api: packet: add support for packet splices and references
>   linux-generic: packet: implement splice/reference apis
>   validation: packet: add packet splice/reference tests
>   doc: images: add images for packet splice/reference documentation
>   doc: userguide: add user documentation for packet splice/reference
> APIs
>
>  doc/images/doublesplice.svg|  67 ++
>  doc/images/pktref.svg  |  49 
>  doc/images/splice.svg  |  64 ++
>  doc/users-guide/users-guide-packet.adoc| 118 ++
>  include/odp/api/spec/packet.h  | 103 +
>  .../linux-generic/include/odp_packet_internal.h|  54 -
>  platform/linux-generic/odp_packet.c| 246
> ++---
>  test/common_plat/validation/api/packet/packet.c| 176 +++
>  test/common_plat/validation/api/packet/packet.h|   1 +
>  9 files changed, 840 insertions(+), 38 deletions(-)
>  create mode 100644 doc/images/doublesplice.svg
>  create mode 100644 doc/images/pktref.svg
>  create mode 100644 doc/images/splice.svg
>
> --
> 2.7.4
>
>

[lng-odp] [Bug 2552] New: Timer pools expire one tick later than they should

[bugzilla-daemon]

https://bugs.linaro.org/show_bug.cgi?id=2552

Bug ID: 2552
   Summary: Timer pools expire one tick later than they should
   Product: OpenDataPlane - linux- generic reference
   Version: v1.11.0.0
  Hardware: All
OS: Linux
Status: UNCONFIRMED
  Severity: normal
  Priority: ---
 Component: Timers
  Assignee: lng-odp@lists.linaro.org
  Reporter: brian.bro...@linaro.org
CC: lng-odp@lists.linaro.org
  Target Milestone: ---

Patch: https://lists.linaro.org/pipermail/lng-odp/2016-October/025856.html

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

Re: [lng-odp] [PATCH] timers: fix off by one tick in timer expiration processing

[Bill Fischofer]

Since this is a bug fix, please open a Bug for it so that this can be
tracked as a defect closure.

On Thu, Oct 13, 2016 at 4:18 PM, Brian Brooks 
wrote:

> A timer pool's tick starts at t0 (zero). Once the first period has passed,
> the timer pool is scanned for any timers that have expired since t0 + 1.
>
> Current code does an atomic fetch increment on the tick, but uses the
> previous tick during timer expiration processing. What is needed is the
> previous tick + 1.
>
> The observable effect without this patch is that timers are expired one
> tick
> period (timer resolution) later than they should be.
>
> Signed-off-by: Brian Brooks 
>

Reviewed-by: Bill Fischofer 


> ---
>  platform/linux-generic/odp_timer.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/platform/linux-generic/odp_timer.c
> b/platform/linux-generic/odp_timer.c
> index becea9d..b26ac6b 100644
> --- a/platform/linux-generic/odp_timer.c
> +++ b/platform/linux-generic/odp_timer.c
> @@ -691,7 +691,7 @@ static void timer_notify(odp_timer_pool *tp)
> prev_tick = odp_atomic_fetch_inc_u64(&tp->cur_tick);
>
> /* Scan timer array, looking for timers to expire */
> -   (void)odp_timer_pool_expire(tp, prev_tick);
> +   (void)odp_timer_pool_expire(tp, prev_tick + 1);
>
> /* Else skip scan of timers. cur_tick was updated and next itimer
>  * invocation will process older expiration ticks as well */
> --
> 2.7.4
>
>

[lng-odp] [PATCH] timers: fix off by one tick in timer expiration processing

[Brian Brooks]

A timer pool's tick starts at t0 (zero). Once the first period has passed,
the timer pool is scanned for any timers that have expired since t0 + 1.

Current code does an atomic fetch increment on the tick, but uses the
previous tick during timer expiration processing. What is needed is the
previous tick + 1.

The observable effect without this patch is that timers are expired one tick
period (timer resolution) later than they should be.

Signed-off-by: Brian Brooks 
---
 platform/linux-generic/odp_timer.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/platform/linux-generic/odp_timer.c 
b/platform/linux-generic/odp_timer.c
index becea9d..b26ac6b 100644
--- a/platform/linux-generic/odp_timer.c
+++ b/platform/linux-generic/odp_timer.c
@@ -691,7 +691,7 @@ static void timer_notify(odp_timer_pool *tp)
prev_tick = odp_atomic_fetch_inc_u64(&tp->cur_tick);
 
/* Scan timer array, looking for timers to expire */
-   (void)odp_timer_pool_expire(tp, prev_tick);
+   (void)odp_timer_pool_expire(tp, prev_tick + 1);
 
/* Else skip scan of timers. cur_tick was updated and next itimer
 * invocation will process older expiration ticks as well */
-- 
2.7.4

Re: [lng-odp] [PATCHv2] test: skip pktio_perf tests on 1 and 2 cpus machines

[Mike Holmes]

On 13 October 2016 at 12:37, Maxim Uvarov  wrote:

> Make check should skip the test instead of failing it.
> Test splits RX and TX cores for packet processing. Core
> 0 bind to control thread. So running machine should have
> at least 2 worker threads which is not enough on 1 and 2
> cpus machine. CUnit uses special value 77 to mark test as
> SKIPPED and not fail on it.
>
> Signed-off-by: Maxim Uvarov 

Using .travis.yml script added to the root of odp  to get github to run on
every push you make to git hub.

before
https://travis-ci.org/mike-holmes-linaro/odp/builds/167472534#L1595
after
https://travis-ci.org/mike-holmes-linaro/odp/builds/167474316



>

---
>
>  v2: update description (Mike)
>
>  test/common_plat/performance/odp_pktio_perf.c | 14 ++
>  1 file changed, 10 insertions(+), 4 deletions(-)
>
> diff --git a/test/common_plat/performance/odp_pktio_perf.c
> b/test/common_plat/performance/odp_pktio_perf.c
> index f041b13..846dfaa 100644
> --- a/test/common_plat/performance/odp_pktio_perf.c
> +++ b/test/common_plat/performance/odp_pktio_perf.c
> @@ -34,6 +34,8 @@
>  #include 
>  #include 
>
> +#define TEST_SKIP 77
> +
>  #define PKT_BUF_NUM   8192
>  #define MAX_NUM_IFACES2
>  #define TEST_HDR_MAGIC0x92749451
> @@ -558,7 +560,7 @@ static int setup_txrx_masks(odp_cpumask_t *thd_mask_tx,
>gbl_args->args.cpu_count);
> if (num_workers < 2) {
> LOG_ERR("Need at least two cores\n");
> -   return -1;
> +   return TEST_SKIP;
> }
>
> if (gbl_args->args.num_tx_workers) {
> @@ -669,8 +671,9 @@ static int run_test(void)
> .warmup = 1,
> };
>
> -   if (setup_txrx_masks(&txmask, &rxmask) != 0)
> -   return -1;
> +   ret = setup_txrx_masks(&txmask, &rxmask);
> +   if (ret)
> +   return ret;
>
> printf("Starting test with params:\n");
> printf("\tTransmit workers: \t%d\n",
> odp_cpumask_count(&txmask));
> @@ -691,8 +694,11 @@ static int run_test(void)
> run_test_single(&txmask, &rxmask, &status);
> status.warmup = 0;
>
> -   while (ret > 0)
> +   while (1) {
> ret = run_test_single(&txmask, &rxmask, &status);
> +   if (ret)
> +   break;
> +   }
>
> return ret;
>  }
> --
> 2.7.1.250.gff4ea60
>
>


-- 
Mike Holmes
Program Manager - Linaro Networking Group
Linaro.org  *│ *Open source software for ARM SoCs
"Work should be fun and collaborative, the rest follows"

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

[Mike Holmes]

On 13 October 2016 at 12:39, Maxim Uvarov  wrote:

> On 10/13/16 19:33, Mike Holmes wrote:
>
>> Anders comments have not been addressed on the list, are they resloved ?
>>
>>
> In current patch set - no. Because it's subject for separate patch. But we
> need resolve version name before tagging.
>

I would be personally happier if we had waited for Petri and Anders to
agree on a course of action, the discussion is between them, that would be
normal. Possibly they agree that it can be a later patch, perhaps not.
We want to be very cautious about not addressing a reviewers comments on
the list, people comment becasue they think there is an issue and they
might be right :)

If this is in it needs a bug to track the lib numbering so that it is
addressed before the  next release.

Mike



>
> Maxim.
>
>
> On 13 October 2016 at 11:40, Maxim Uvarov > > wrote:
>>
>> Merged,
>>
>> we also need this option to be tested in CI.
>>
>> Maxim.
>>
>>
>> On 10/12/16 18:21, Bill Fischofer wrote:
>>
>> This version resolves the issue with running from an installed
>> copy of ODP.
>>
>> On Thu, Sep 29, 2016 at 6:46 PM, 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
>> > >
>>
>> Reviewed-and-tested-by: Bill Fischofer
>> mailto:bill.fischo...@linaro.org>>
>>
>>
>> ---
>> configure.ac   |
>> 26 -
>>   platform/linux-generic/.gitignore   |  2 +-
>>   platform/linux-generic/Makefile.am 
>>  |  2 +-
>>   platform/linux-generic/include/odp/api/atomic.h   |  4 +--
>>   platform/linux-generic/include/odp/api/byteorder.h |  4 +--
>>   .../include/odp/api/plat/inlines.h.in
>>  | 33
>> --
>>   .../include/odp/api/plat/static_inline.h.in
>>  | 32
>> +
>>   platform/linux-generic/include/odp/api/std_clib.h |  4 +--
>>   platform/linux-generic/include/odp/api/sync.h |  4 +--
>>   platform/linux-generic/m4/configure.m4  |  2 +-
>>   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 +-
>>   14 files changed, 66 insertions(+), 55 deletions(-)
>>   delete mode 100644
>> platform/linux-generic/include/odp/api/plat/inlines.
>> h.in 
>>   create mode 100644
>> platform/linux-generic/include/odp/api/plat/static_
>> inline.h.in 
>>
>> diff --git a/configure.ac 
>> b/configure.ac 
>> index 982aff7..f081c51 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"
>>
>>   ##
>> ##
>> ##
>> +# Enable/disable ABI compatible build
>>  

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

[Maxim Uvarov]

On 10/13/16 19:33, Mike Holmes wrote:

Anders comments have not been addressed on the list, are they resloved ?



In current patch set - no. Because it's subject for separate patch. But 
we need resolve version name before tagging.


Maxim.


On 13 October 2016 at 11:40, Maxim Uvarov > wrote:


Merged,

we also need this option to be tested in CI.

Maxim.


On 10/12/16 18:21, Bill Fischofer wrote:

This version resolves the issue with running from an installed
copy of ODP.

On Thu, Sep 29, 2016 at 6:46 PM, 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
mailto:petri.savolai...@nokia.com>>

Reviewed-and-tested-by: Bill Fischofer
mailto:bill.fischo...@linaro.org>>


---
configure.ac   |
26 -
  platform/linux-generic/.gitignore   |  2 +-
  platform/linux-generic/Makefile.am 
   |  2 +-

  platform/linux-generic/include/odp/api/atomic.h   |  4 +--
  platform/linux-generic/include/odp/api/byteorder.h |  4 +--
  .../include/odp/api/plat/inlines.h.in
 | 33
--
  .../include/odp/api/plat/static_inline.h.in
 | 32
+
  platform/linux-generic/include/odp/api/std_clib.h |  4 +--
  platform/linux-generic/include/odp/api/sync.h |  4 +--
  platform/linux-generic/m4/configure.m4  |  2 +-
  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 +-
  14 files changed, 66 insertions(+), 55 deletions(-)
  delete mode 100644
platform/linux-generic/include/odp/api/plat/inlines.
h.in 
  create mode 100644
platform/linux-generic/include/odp/api/plat/static_
inline.h.in 

diff --git a/configure.ac 
b/configure.ac 
index 982aff7..f081c51 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"

  
##
+# Enable/disable ABI compatible build
+###
###
+ODP_ABI_COMPAT=1
+abi_compat=yes
+AC_ARG_ENABLE([abi-compat],
+[  --enable-abi-compat build all targets in ABI
compatible mode
(default=yes)],
+[if test "x$enableval" = "xyes"; then
+   ODP_ABI_COMPAT=1
+   abi_compat=yes
+ else
+   ODP_ABI_COMPAT=0
+   abi_compat=no
+fi])
+AC_SUBST(ODP_ABI_COMPAT)
+
+###
###
  # Default warning setup
  

[lng-odp] [PATCHv2] test: skip pktio_perf tests on 1 and 2 cpus machines

[Maxim Uvarov]

Make check should skip the test instead of failing it.
Test splits RX and TX cores for packet processing. Core
0 bind to control thread. So running machine should have
at least 2 worker threads which is not enough on 1 and 2
cpus machine. CUnit uses special value 77 to mark test as
SKIPPED and not fail on it.

Signed-off-by: Maxim Uvarov 
---

 v2: update description (Mike)

 test/common_plat/performance/odp_pktio_perf.c | 14 ++
 1 file changed, 10 insertions(+), 4 deletions(-)

diff --git a/test/common_plat/performance/odp_pktio_perf.c 
b/test/common_plat/performance/odp_pktio_perf.c
index f041b13..846dfaa 100644
--- a/test/common_plat/performance/odp_pktio_perf.c
+++ b/test/common_plat/performance/odp_pktio_perf.c
@@ -34,6 +34,8 @@
 #include 
 #include 
 
+#define TEST_SKIP 77
+
 #define PKT_BUF_NUM   8192
 #define MAX_NUM_IFACES2
 #define TEST_HDR_MAGIC0x92749451
@@ -558,7 +560,7 @@ static int setup_txrx_masks(odp_cpumask_t *thd_mask_tx,
   gbl_args->args.cpu_count);
if (num_workers < 2) {
LOG_ERR("Need at least two cores\n");
-   return -1;
+   return TEST_SKIP;
}
 
if (gbl_args->args.num_tx_workers) {
@@ -669,8 +671,9 @@ static int run_test(void)
.warmup = 1,
};
 
-   if (setup_txrx_masks(&txmask, &rxmask) != 0)
-   return -1;
+   ret = setup_txrx_masks(&txmask, &rxmask);
+   if (ret)
+   return ret;
 
printf("Starting test with params:\n");
printf("\tTransmit workers: \t%d\n", odp_cpumask_count(&txmask));
@@ -691,8 +694,11 @@ static int run_test(void)
run_test_single(&txmask, &rxmask, &status);
status.warmup = 0;
 
-   while (ret > 0)
+   while (1) {
ret = run_test_single(&txmask, &rxmask, &status);
+   if (ret)
+   break;
+   }
 
return ret;
 }
-- 
2.7.1.250.gff4ea60

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

[Mike Holmes]

Anders comments have not been addressed on the list, are they resloved ?

On 13 October 2016 at 11:40, Maxim Uvarov  wrote:

> Merged,
>
> we also need this option to be tested in CI.
>
> Maxim.
>
>
> On 10/12/16 18:21, Bill Fischofer wrote:
>
>> This version resolves the issue with running from an installed copy of
>> ODP.
>>
>> On Thu, Sep 29, 2016 at 6:46 PM, 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 
>>>
>>> Reviewed-and-tested-by: Bill Fischofer 
>>
>>
>> ---
>>>   configure.ac   | 26
>>> -
>>>   platform/linux-generic/.gitignore  |  2 +-
>>>   platform/linux-generic/Makefile.am |  2 +-
>>>   platform/linux-generic/include/odp/api/atomic.h|  4 +--
>>>   platform/linux-generic/include/odp/api/byteorder.h |  4 +--
>>>   .../include/odp/api/plat/inlines.h.in  | 33
>>> --
>>>   .../include/odp/api/plat/static_inline.h.in| 32
>>> +
>>>   platform/linux-generic/include/odp/api/std_clib.h  |  4 +--
>>>   platform/linux-generic/include/odp/api/sync.h  |  4 +--
>>>   platform/linux-generic/m4/configure.m4 |  2 +-
>>>   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 +-
>>>   14 files changed, 66 insertions(+), 55 deletions(-)
>>>   delete mode 100644 platform/linux-generic/include
>>> /odp/api/plat/inlines.
>>> h.in
>>>   create mode 100644 platform/linux-generic/include/odp/api/plat/static_
>>> inline.h.in
>>>
>>> diff --git a/configure.ac b/configure.ac
>>> index 982aff7..f081c51 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"
>>>
>>>   
>>> ##
>>> +# Enable/disable ABI compatible build
>>> +###
>>> ###
>>> +ODP_ABI_COMPAT=1
>>> +abi_compat=yes
>>> +AC_ARG_ENABLE([abi-compat],
>>> +[  --enable-abi-compat build all targets in ABI compatible mode
>>> (default=yes)],
>>> +[if test "x$enableval" = "xyes"; then
>>> +   ODP_ABI_COMPAT=1
>>> +   abi_compat=yes
>>> + else
>>> +   ODP_ABI_COMPAT=0
>>> +   abi_compat=no
>>> +fi])
>>> +AC_SUBST(ODP_ABI_COMPAT)
>>> +
>>> +###
>>> ###
>>>   # Default warning setup
>>>   
>>> ##
>>>   ODP_CFLAGS="$ODP_CFLAGS -W -Wall -Werror -Wstrict-prototypes
>>> -Wmissing-prototypes"
>>> @@ -307,6 +316,9 @@ AC_MSG_RESULT([
>>>  am_ldflags: ${AM_LDFLAGS}
>>>  libs:   ${LIBS}
>>>  defs:   ${DEFS}
>>> +   static libraries:   ${enable_static}
>>> +   shared libraries:   ${enable_shared}
>>> +   ABI compatible: ${abi_compat}
>>>  cunit:  ${cunit_support}
>>>  test_vald:  ${test_vald}
>>>  test_perf:  ${test_perf}
>>> diff --git a/platform/linux-generic/.gitignore
>>> b/platform/linux-generic/.
>>> gitignore
>>> index ec6ca37..909756a 100644
>>> --- a/platform/linux-generic/.gitignore
>>> +++ b/platform/linux-generic/.gitignore
>>> @@ -1 +1 @@
>>> -include/odp/api/plat/inlines.h
>>> +include/odp/api/plat/static_inline.h
>>> diff --git a/platform/linux-generic/Makefile.am
>>> b/platform/linux-generic/
>>> Makefile.am
>>> index 900ac08..0ec13d4 100644
>>> --- a/platform/linux-generic/Makefile.am
>>> +++ b/platform/linux-generic/Makefile.am
>>> @@ -61,7 +61,7 @@ odpapiinclude_HEADERS = \
>>>
>>>   odpapiplatincludedir= $

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

[Maxim Uvarov]

Merged,

we also need this option to be tested in CI.

Maxim.

On 10/12/16 18:21, Bill Fischofer wrote:

This version resolves the issue with running from an installed copy of ODP.

On Thu, Sep 29, 2016 at 6:46 PM, 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 


Reviewed-and-tested-by: Bill Fischofer 



---
  configure.ac   | 26 -
  platform/linux-generic/.gitignore  |  2 +-
  platform/linux-generic/Makefile.am |  2 +-
  platform/linux-generic/include/odp/api/atomic.h|  4 +--
  platform/linux-generic/include/odp/api/byteorder.h |  4 +--
  .../include/odp/api/plat/inlines.h.in  | 33
--
  .../include/odp/api/plat/static_inline.h.in| 32
+
  platform/linux-generic/include/odp/api/std_clib.h  |  4 +--
  platform/linux-generic/include/odp/api/sync.h  |  4 +--
  platform/linux-generic/m4/configure.m4 |  2 +-
  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 +-
  14 files changed, 66 insertions(+), 55 deletions(-)
  delete mode 100644 platform/linux-generic/include/odp/api/plat/inlines.
h.in
  create mode 100644 platform/linux-generic/include/odp/api/plat/static_
inline.h.in

diff --git a/configure.ac b/configure.ac
index 982aff7..f081c51 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"

  
##
+# Enable/disable ABI compatible build
+###
###
+ODP_ABI_COMPAT=1
+abi_compat=yes
+AC_ARG_ENABLE([abi-compat],
+[  --enable-abi-compat build all targets in ABI compatible mode
(default=yes)],
+[if test "x$enableval" = "xyes"; then
+   ODP_ABI_COMPAT=1
+   abi_compat=yes
+ else
+   ODP_ABI_COMPAT=0
+   abi_compat=no
+fi])
+AC_SUBST(ODP_ABI_COMPAT)
+
+###
###
  # Default warning setup
  
##
  ODP_CFLAGS="$ODP_CFLAGS -W -Wall -Werror -Wstrict-prototypes
-Wmissing-prototypes"
@@ -307,6 +316,9 @@ AC_MSG_RESULT([
 am_ldflags: ${AM_LDFLAGS}
 libs:   ${LIBS}
 defs:   ${DEFS}
+   static libraries:   ${enable_static}
+   shared libraries:   ${enable_shared}
+   ABI compatible: ${abi_compat}
 cunit:  ${cunit_support}
 test_vald:  ${test_vald}
 test_perf:  ${test_perf}
diff --git a/platform/linux-generic/.gitignore b/platform/linux-generic/.
gitignore
index ec6ca37..909756a 100644
--- a/platform/linux-generic/.gitignore
+++ b/platform/linux-generic/.gitignore
@@ -1 +1 @@
-include/odp/api/plat/inlines.h
+include/odp/api/plat/static_inline.h
diff --git a/platform/linux-generic/Makefile.am b/platform/linux-generic/
Makefile.am
index 900ac08..0ec13d4 100644
--- a/platform/linux-generic/Makefile.am
+++ b/platform/linux-generic/Makefile.am
@@ -61,7 +61,7 @@ odpapiinclude_HEADERS = \

  odpapiplatincludedir= $(includedir)/odp/api/plat
  odpapiplatinclude_HEADERS = \
- $(builddir)/include/odp/api/plat/inlines.h \
+ $(builddir)/include/odp/api/plat/static_inline.h \
   $(srcdir)/include/odp/api/plat/atomic_inlines.h \
   $(srcdir)/include/odp/api/plat/atomic_types.h \
   $(srcdir)/include/odp/api/plat/barrier_types.h \
diff --git a/platform/linux-generic/include/odp/api/atomic.h
b/platform/linux-generic/include/odp/api/atomic.h
index c18e68b..7886cb4 100644
--- a/platform/linux-generic/include/odp/api/atomic.h
+++ b/platform/

Re: [lng-odp] [API-NEXT PATCHv2 1/2] api: dev: add device apis for numa support

[Christophe Milard]

On 13 October 2016 at 14:06, Bill Fischofer  wrote:
>
>
> On Thu, Oct 13, 2016 at 6:53 AM, Christophe Milard
>  wrote:
>>
>> On 13 October 2016 at 13:44, Bill Fischofer 
>> wrote:
>> >
>> >
>> > On Thu, Oct 13, 2016 at 6:33 AM, Christophe Milard
>> >  wrote:
>> >>
>> >> On 13 October 2016 at 13:20, Bill Fischofer 
>> >> wrote:
>> >> >
>> >> >
>> >> > On Thu, Oct 13, 2016 at 2:03 AM, Christophe Milard
>> >> >  wrote:
>> >> >>
>> >> >> On 13 October 2016 at 02:44, Bill Fischofer
>> >> >> 
>> >> >> wrote:
>> >> >> > Add the odp_dev_id() API used for NUMA support
>> >> >> >
>> >> >>
>> >> >> I am a bit confused here: what is a device? a numa_id or other
>> >> >> things
>> >> >> as well? In this patch series everything that relates to numa is
>> >> >> called "device". Shouldn't be called numa_dev when it is a numa
>> >> >> device?
>> >> >> If devices are numa dev only, they should be called numa_dev. If a
>> >> >> device can be anything else (which you general approach seems to
>> >> >> imply), how are they different from handles?
>> >> >>
>> >> >> Not sure I understand where these patches lead to...
>> >> >
>> >> >
>> >> > These patches are just implementing the APIs proposed by Petri during
>> >> > the
>> >> > ODP Design Summit at LAS16.  We can consider them RFCs for now if
>> >> > that's
>> >> > preferable. A dev_id in ODP is supposed to be the same as a socket_id
>> >> > in
>> >> > DPDK, but not necessarily tied to the CPU socket config. The intent
>> >> > is
>> >> > simply to have a placeholder where hierarchical NUMA-type identifiers
>> >> > can be
>> >> > obtained and then used as part of resource (pools, etc.) creation.
>> >> > This
>> >> > is
>> >> > inherently system dependent, which is why the odp-linux versions are
>> >> > mostly
>> >> > placeholders, and why I put the implementations under the arch
>> >> > directory.
>> >> >
>> >>
>> >> But still: is this for numa only (in which case I would expect a
>> >> clearer name) or are these devices meant to be used by other things
>> >> (which ones?). And how does this differ from the handles we already
>> >> have for other objects?
>> >
>> >
>> > It's not necessarily NUMA since the intent is to be able to cover
>> > multi-SoC
>> > configurations as well. A dev_id differs from a handle because it's a
>> > qualifier. So, for example, an odp_pool_t is the ODP handle for a pool,
>> > however the pool may have a couple of dev_id qualifiers that are used as
>> > part of it's creation (Petri identified pool_id and dram_id as two).
>> > Similarly a crypto_session uses a dev_id to identify a specific crypto
>> > resource bound to that session. For example a system might have four
>> > crypto
>> > engines that have different "distance" depending on where the thread is
>> > running and the dev_id would distinguish those.
>>
>> hmmm interesting... not very clear to me what would be the difference
>> between a handle and a dev. If handles can be references to anything,
>> I don't really see why we wouldn't keep using handles for these
>> things. And I am not sure either having a name as "dev" to cover
>> everything makes it clear.
>> Thanks for answering anyway :-) maybe it will become clearer in the
>> future.
>>
> I'll add this to Monday's ARCH call so Petri can explain it in more detail
> :)

Thanks!

Christophe
>
>
>>
>> >
>> >>
>> >>
>> >> Christophe
>> >> >>
>> >> >>
>> >> >> Christophe.
>> >> >>
>> >> >> > Signed-off-by: Bill Fischofer 
>> >> >> > ---
>> >> >> >  include/odp/api/spec/dev.h | 89
>> >> >> > ++
>> >> >> >  1 file changed, 89 insertions(+)
>> >> >> >  create mode 100644 include/odp/api/spec/dev.h
>> >> >> >
>> >> >> > diff --git a/include/odp/api/spec/dev.h
>> >> >> > b/include/odp/api/spec/dev.h
>> >> >> > new file mode 100644
>> >> >> > index 000..1f7ed8b
>> >> >> > --- /dev/null
>> >> >> > +++ b/include/odp/api/spec/dev.h
>> >> >> > @@ -0,0 +1,89 @@
>> >> >> > +/* Copyright (c) 2016, Linaro Limited
>> >> >> > + * All rights reserved.
>> >> >> > + *
>> >> >> > + * SPDX-License-Identifier: BSD-3-Clause
>> >> >> > + */
>> >> >> > +
>> >> >> > +/**
>> >> >> > + * @file
>> >> >> > + *
>> >> >> > + * ODP device
>> >> >> > + */
>> >> >> > +
>> >> >> > +#ifndef ODP_API_DEV_H_
>> >> >> > +#define ODP_API_DEV_H_
>> >> >> > +#include 
>> >> >> > +
>> >> >> > +#ifdef __cplusplus
>> >> >> > +extern "C" {
>> >> >> > +#endif
>> >> >> > +
>> >> >> > +#include 
>> >> >> > +
>> >> >> > +/** @defgroup odp_dev ODP DEVICE
>> >> >> > + *  Operations on devices
>> >> >> > + *  @{
>> >> >> > + */
>> >> >> > +
>> >> >> > +/**
>> >> >> > + * @typedef odp_dev_t
>> >> >> > + * ODP Device
>> >> >> > + */
>> >> >> > +
>> >> >> > +/**
>> >> >> > + * @def ODP_DEV_NAME_LEN
>> >> >> > + * Maximum device name length in chars
>> >> >> > + */
>> >> >> > +
>> >> >> > +/**
>> >> >> > + * @def ODP_DEV_ANY
>> >> >> > + * Any device
>> >> >> > + */
>> >> >> > +
>> >> >> > +/**
>> >> >> > + * @def ODP_DEV_INVALID
>> >> >> > + * Invalid

Re: [lng-odp] [API-NEXT PATCHv2 1/2] api: dev: add device apis for numa support

[Bill Fischofer]

On Thu, Oct 13, 2016 at 6:53 AM, Christophe Milard <
christophe.mil...@linaro.org> wrote:

> On 13 October 2016 at 13:44, Bill Fischofer 
> wrote:
> >
> >
> > On Thu, Oct 13, 2016 at 6:33 AM, Christophe Milard
> >  wrote:
> >>
> >> On 13 October 2016 at 13:20, Bill Fischofer 
> >> wrote:
> >> >
> >> >
> >> > On Thu, Oct 13, 2016 at 2:03 AM, Christophe Milard
> >> >  wrote:
> >> >>
> >> >> On 13 October 2016 at 02:44, Bill Fischofer <
> bill.fischo...@linaro.org>
> >> >> wrote:
> >> >> > Add the odp_dev_id() API used for NUMA support
> >> >> >
> >> >>
> >> >> I am a bit confused here: what is a device? a numa_id or other things
> >> >> as well? In this patch series everything that relates to numa is
> >> >> called "device". Shouldn't be called numa_dev when it is a numa
> >> >> device?
> >> >> If devices are numa dev only, they should be called numa_dev. If a
> >> >> device can be anything else (which you general approach seems to
> >> >> imply), how are they different from handles?
> >> >>
> >> >> Not sure I understand where these patches lead to...
> >> >
> >> >
> >> > These patches are just implementing the APIs proposed by Petri during
> >> > the
> >> > ODP Design Summit at LAS16.  We can consider them RFCs for now if
> that's
> >> > preferable. A dev_id in ODP is supposed to be the same as a socket_id
> in
> >> > DPDK, but not necessarily tied to the CPU socket config. The intent is
> >> > simply to have a placeholder where hierarchical NUMA-type identifiers
> >> > can be
> >> > obtained and then used as part of resource (pools, etc.) creation.
> This
> >> > is
> >> > inherently system dependent, which is why the odp-linux versions are
> >> > mostly
> >> > placeholders, and why I put the implementations under the arch
> >> > directory.
> >> >
> >>
> >> But still: is this for numa only (in which case I would expect a
> >> clearer name) or are these devices meant to be used by other things
> >> (which ones?). And how does this differ from the handles we already
> >> have for other objects?
> >
> >
> > It's not necessarily NUMA since the intent is to be able to cover
> multi-SoC
> > configurations as well. A dev_id differs from a handle because it's a
> > qualifier. So, for example, an odp_pool_t is the ODP handle for a pool,
> > however the pool may have a couple of dev_id qualifiers that are used as
> > part of it's creation (Petri identified pool_id and dram_id as two).
> > Similarly a crypto_session uses a dev_id to identify a specific crypto
> > resource bound to that session. For example a system might have four
> crypto
> > engines that have different "distance" depending on where the thread is
> > running and the dev_id would distinguish those.
>
> hmmm interesting... not very clear to me what would be the difference
> between a handle and a dev. If handles can be references to anything,
> I don't really see why we wouldn't keep using handles for these
> things. And I am not sure either having a name as "dev" to cover
> everything makes it clear.
> Thanks for answering anyway :-) maybe it will become clearer in the future.
>
> I'll add this to Monday's ARCH call so Petri can explain it in more detail
:)



> >
> >>
> >>
> >> Christophe
> >> >>
> >> >>
> >> >> Christophe.
> >> >>
> >> >> > Signed-off-by: Bill Fischofer 
> >> >> > ---
> >> >> >  include/odp/api/spec/dev.h | 89
> >> >> > ++
> >> >> >  1 file changed, 89 insertions(+)
> >> >> >  create mode 100644 include/odp/api/spec/dev.h
> >> >> >
> >> >> > diff --git a/include/odp/api/spec/dev.h
> b/include/odp/api/spec/dev.h
> >> >> > new file mode 100644
> >> >> > index 000..1f7ed8b
> >> >> > --- /dev/null
> >> >> > +++ b/include/odp/api/spec/dev.h
> >> >> > @@ -0,0 +1,89 @@
> >> >> > +/* Copyright (c) 2016, Linaro Limited
> >> >> > + * All rights reserved.
> >> >> > + *
> >> >> > + * SPDX-License-Identifier: BSD-3-Clause
> >> >> > + */
> >> >> > +
> >> >> > +/**
> >> >> > + * @file
> >> >> > + *
> >> >> > + * ODP device
> >> >> > + */
> >> >> > +
> >> >> > +#ifndef ODP_API_DEV_H_
> >> >> > +#define ODP_API_DEV_H_
> >> >> > +#include 
> >> >> > +
> >> >> > +#ifdef __cplusplus
> >> >> > +extern "C" {
> >> >> > +#endif
> >> >> > +
> >> >> > +#include 
> >> >> > +
> >> >> > +/** @defgroup odp_dev ODP DEVICE
> >> >> > + *  Operations on devices
> >> >> > + *  @{
> >> >> > + */
> >> >> > +
> >> >> > +/**
> >> >> > + * @typedef odp_dev_t
> >> >> > + * ODP Device
> >> >> > + */
> >> >> > +
> >> >> > +/**
> >> >> > + * @def ODP_DEV_NAME_LEN
> >> >> > + * Maximum device name length in chars
> >> >> > + */
> >> >> > +
> >> >> > +/**
> >> >> > + * @def ODP_DEV_ANY
> >> >> > + * Any device
> >> >> > + */
> >> >> > +
> >> >> > +/**
> >> >> > + * @def ODP_DEV_INVALID
> >> >> > + * Invalid device
> >> >> > + */
> >> >> > +
> >> >> > +/**
> >> >> > + * Get Device ID by Name
> >> >> > + *
> >> >> > + * Get an implementation-defined device identifier from a device
> >> >> > name.
> >> >> > Device
> >> >> > + * n

Re: [lng-odp] [API-NEXT PATCHv2 1/2] api: dev: add device apis for numa support

[Christophe Milard]

On 13 October 2016 at 13:44, Bill Fischofer  wrote:
>
>
> On Thu, Oct 13, 2016 at 6:33 AM, Christophe Milard
>  wrote:
>>
>> On 13 October 2016 at 13:20, Bill Fischofer 
>> wrote:
>> >
>> >
>> > On Thu, Oct 13, 2016 at 2:03 AM, Christophe Milard
>> >  wrote:
>> >>
>> >> On 13 October 2016 at 02:44, Bill Fischofer 
>> >> wrote:
>> >> > Add the odp_dev_id() API used for NUMA support
>> >> >
>> >>
>> >> I am a bit confused here: what is a device? a numa_id or other things
>> >> as well? In this patch series everything that relates to numa is
>> >> called "device". Shouldn't be called numa_dev when it is a numa
>> >> device?
>> >> If devices are numa dev only, they should be called numa_dev. If a
>> >> device can be anything else (which you general approach seems to
>> >> imply), how are they different from handles?
>> >>
>> >> Not sure I understand where these patches lead to...
>> >
>> >
>> > These patches are just implementing the APIs proposed by Petri during
>> > the
>> > ODP Design Summit at LAS16.  We can consider them RFCs for now if that's
>> > preferable. A dev_id in ODP is supposed to be the same as a socket_id in
>> > DPDK, but not necessarily tied to the CPU socket config. The intent is
>> > simply to have a placeholder where hierarchical NUMA-type identifiers
>> > can be
>> > obtained and then used as part of resource (pools, etc.) creation. This
>> > is
>> > inherently system dependent, which is why the odp-linux versions are
>> > mostly
>> > placeholders, and why I put the implementations under the arch
>> > directory.
>> >
>>
>> But still: is this for numa only (in which case I would expect a
>> clearer name) or are these devices meant to be used by other things
>> (which ones?). And how does this differ from the handles we already
>> have for other objects?
>
>
> It's not necessarily NUMA since the intent is to be able to cover multi-SoC
> configurations as well. A dev_id differs from a handle because it's a
> qualifier. So, for example, an odp_pool_t is the ODP handle for a pool,
> however the pool may have a couple of dev_id qualifiers that are used as
> part of it's creation (Petri identified pool_id and dram_id as two).
> Similarly a crypto_session uses a dev_id to identify a specific crypto
> resource bound to that session. For example a system might have four crypto
> engines that have different "distance" depending on where the thread is
> running and the dev_id would distinguish those.

hmmm interesting... not very clear to me what would be the difference
between a handle and a dev. If handles can be references to anything,
I don't really see why we wouldn't keep using handles for these
things. And I am not sure either having a name as "dev" to cover
everything makes it clear.
Thanks for answering anyway :-) maybe it will become clearer in the future.

>
>>
>>
>> Christophe
>> >>
>> >>
>> >> Christophe.
>> >>
>> >> > Signed-off-by: Bill Fischofer 
>> >> > ---
>> >> >  include/odp/api/spec/dev.h | 89
>> >> > ++
>> >> >  1 file changed, 89 insertions(+)
>> >> >  create mode 100644 include/odp/api/spec/dev.h
>> >> >
>> >> > diff --git a/include/odp/api/spec/dev.h b/include/odp/api/spec/dev.h
>> >> > new file mode 100644
>> >> > index 000..1f7ed8b
>> >> > --- /dev/null
>> >> > +++ b/include/odp/api/spec/dev.h
>> >> > @@ -0,0 +1,89 @@
>> >> > +/* Copyright (c) 2016, Linaro Limited
>> >> > + * All rights reserved.
>> >> > + *
>> >> > + * SPDX-License-Identifier: BSD-3-Clause
>> >> > + */
>> >> > +
>> >> > +/**
>> >> > + * @file
>> >> > + *
>> >> > + * ODP device
>> >> > + */
>> >> > +
>> >> > +#ifndef ODP_API_DEV_H_
>> >> > +#define ODP_API_DEV_H_
>> >> > +#include 
>> >> > +
>> >> > +#ifdef __cplusplus
>> >> > +extern "C" {
>> >> > +#endif
>> >> > +
>> >> > +#include 
>> >> > +
>> >> > +/** @defgroup odp_dev ODP DEVICE
>> >> > + *  Operations on devices
>> >> > + *  @{
>> >> > + */
>> >> > +
>> >> > +/**
>> >> > + * @typedef odp_dev_t
>> >> > + * ODP Device
>> >> > + */
>> >> > +
>> >> > +/**
>> >> > + * @def ODP_DEV_NAME_LEN
>> >> > + * Maximum device name length in chars
>> >> > + */
>> >> > +
>> >> > +/**
>> >> > + * @def ODP_DEV_ANY
>> >> > + * Any device
>> >> > + */
>> >> > +
>> >> > +/**
>> >> > + * @def ODP_DEV_INVALID
>> >> > + * Invalid device
>> >> > + */
>> >> > +
>> >> > +/**
>> >> > + * Get Device ID by Name
>> >> > + *
>> >> > + * Get an implementation-defined device identifier from a device
>> >> > name.
>> >> > Device
>> >> > + * names are supplied as parameter info (command line, file, etc.)
>> >> > to
>> >> > the
>> >> > + * application. This routine translates this symbolic name into an
>> >> > internal
>> >> > + * identifier that can be used to define a device connection
>> >> > hierarchy
>> >> > for
>> >> > + * NUMA or other purposes.
>> >> > + *
>> >> > + * The reserved id ODP_DEV_ANY may be used as a "don't care"
>> >> > placeholder
>> >> > + * wherever a device id is required.
>> >> > + *
>> >> > + * @param name N

Re: [lng-odp] [API-NEXT PATCHv2 1/2] api: dev: add device apis for numa support

[Bill Fischofer]

On Thu, Oct 13, 2016 at 6:33 AM, Christophe Milard <
christophe.mil...@linaro.org> wrote:

> On 13 October 2016 at 13:20, Bill Fischofer 
> wrote:
> >
> >
> > On Thu, Oct 13, 2016 at 2:03 AM, Christophe Milard
> >  wrote:
> >>
> >> On 13 October 2016 at 02:44, Bill Fischofer 
> >> wrote:
> >> > Add the odp_dev_id() API used for NUMA support
> >> >
> >>
> >> I am a bit confused here: what is a device? a numa_id or other things
> >> as well? In this patch series everything that relates to numa is
> >> called "device". Shouldn't be called numa_dev when it is a numa
> >> device?
> >> If devices are numa dev only, they should be called numa_dev. If a
> >> device can be anything else (which you general approach seems to
> >> imply), how are they different from handles?
> >>
> >> Not sure I understand where these patches lead to...
> >
> >
> > These patches are just implementing the APIs proposed by Petri during the
> > ODP Design Summit at LAS16.  We can consider them RFCs for now if that's
> > preferable. A dev_id in ODP is supposed to be the same as a socket_id in
> > DPDK, but not necessarily tied to the CPU socket config. The intent is
> > simply to have a placeholder where hierarchical NUMA-type identifiers
> can be
> > obtained and then used as part of resource (pools, etc.) creation. This
> is
> > inherently system dependent, which is why the odp-linux versions are
> mostly
> > placeholders, and why I put the implementations under the arch directory.
> >
>
> But still: is this for numa only (in which case I would expect a
> clearer name) or are these devices meant to be used by other things
> (which ones?). And how does this differ from the handles we already
> have for other objects?
>

It's not necessarily NUMA since the intent is to be able to cover multi-SoC
configurations as well. A dev_id differs from a handle because it's a
qualifier. So, for example, an odp_pool_t is the ODP handle for a pool,
however the pool may have a couple of dev_id qualifiers that are used as
part of it's creation (Petri identified pool_id and dram_id as two).
Similarly a crypto_session uses a dev_id to identify a specific crypto
resource bound to that session. For example a system might have four crypto
engines that have different "distance" depending on where the thread is
running and the dev_id would distinguish those.


>
> Christophe
> >>
> >>
> >> Christophe.
> >>
> >> > Signed-off-by: Bill Fischofer 
> >> > ---
> >> >  include/odp/api/spec/dev.h | 89
> >> > ++
> >> >  1 file changed, 89 insertions(+)
> >> >  create mode 100644 include/odp/api/spec/dev.h
> >> >
> >> > diff --git a/include/odp/api/spec/dev.h b/include/odp/api/spec/dev.h
> >> > new file mode 100644
> >> > index 000..1f7ed8b
> >> > --- /dev/null
> >> > +++ b/include/odp/api/spec/dev.h
> >> > @@ -0,0 +1,89 @@
> >> > +/* Copyright (c) 2016, Linaro Limited
> >> > + * All rights reserved.
> >> > + *
> >> > + * SPDX-License-Identifier: BSD-3-Clause
> >> > + */
> >> > +
> >> > +/**
> >> > + * @file
> >> > + *
> >> > + * ODP device
> >> > + */
> >> > +
> >> > +#ifndef ODP_API_DEV_H_
> >> > +#define ODP_API_DEV_H_
> >> > +#include 
> >> > +
> >> > +#ifdef __cplusplus
> >> > +extern "C" {
> >> > +#endif
> >> > +
> >> > +#include 
> >> > +
> >> > +/** @defgroup odp_dev ODP DEVICE
> >> > + *  Operations on devices
> >> > + *  @{
> >> > + */
> >> > +
> >> > +/**
> >> > + * @typedef odp_dev_t
> >> > + * ODP Device
> >> > + */
> >> > +
> >> > +/**
> >> > + * @def ODP_DEV_NAME_LEN
> >> > + * Maximum device name length in chars
> >> > + */
> >> > +
> >> > +/**
> >> > + * @def ODP_DEV_ANY
> >> > + * Any device
> >> > + */
> >> > +
> >> > +/**
> >> > + * @def ODP_DEV_INVALID
> >> > + * Invalid device
> >> > + */
> >> > +
> >> > +/**
> >> > + * Get Device ID by Name
> >> > + *
> >> > + * Get an implementation-defined device identifier from a device
> name.
> >> > Device
> >> > + * names are supplied as parameter info (command line, file, etc.) to
> >> > the
> >> > + * application. This routine translates this symbolic name into an
> >> > internal
> >> > + * identifier that can be used to define a device connection
> hierarchy
> >> > for
> >> > + * NUMA or other purposes.
> >> > + *
> >> > + * The reserved id ODP_DEV_ANY may be used as a "don't care"
> >> > placeholder
> >> > + * wherever a device id is required.
> >> > + *
> >> > + * @param name Name of the device
> >> > + *
> >> > + * @return Device ID
> >> > + * @retval ODP_DEV_INVALID Device is unknown
> >> > + */
> >> > +odp_dev_t odp_dev_id(const char *name);
> >> > +
> >> > +/**
> >> > + * Get printable value for an odp_dev_t
> >> > + *
> >> > + * @param hdl  odp_dev_t handle to be printed
> >> > + * @return uint64_t value that can be used to print/display this
> >> > + * handle
> >> > + *
> >> > + * @note This routine is intended to be used for diagnostic purposes
> >> > + * to enable applications to generate a printable value that
> represents
> >> 

[lng-odp] clarification of pktout checksum offload feature

[Maciej Czekaj]

Guys,

I was going to implement checksum offload for OFP project based on 
Monarch checksum offload capability and I found out that there is no 
example for using that API. Also, documentation seams to leave some room 
for various interpretations, so I would like to clarify that and post a 
patch to documentation, too.



This is an exempt from pktio.h from Monarch LTS:


/**
 * Packet output configuration options bit field
 *
 * Packet output configuration options listed in a bit field structure. 
Packet
 * output checksum insertion may be enabled or disabled. When it is 
enabled,
 * implementation will calculate and insert checksum into every 
outgoing packet
 * by default. Application may use a packet metadata flag to disable 
checksum

 * insertion per packet bases. For correct operation, packet metadata must
 * provide valid offsets for the appropriate protocols. For example, UDP
 * checksum calculation needs both L3 and L4 offsets (to access IP and UDP
 * headers). When application (e.g. a switch) does not modify L3/L4 
data and
 * thus checksum does not need to be updated, output checksum insertion 
should

 * be disabled for optimal performance.



From my contact with varoius NICs, including Octeon PKO & VNIC from 
ThunderX, offloading H/W needs at least:


For L4 offload:
L4 packet type: TCP/UDP/SCTP
L4 header offset
L3 header offset
L3 type may or may not be required but it is good to define it for 
consistency


For L3 checksum:
L3 packet type: IPv4
L3 header offset

There is also a second thing: how to disable checksum calculation 
per-packet?
If packet has no type in metadata, then obviously checksum will not be 
computed. I think that would be the recommended method for now, even if 
ODP community plans to  extend odp_packet API in the future to cover 
that case.


Maybe that is implicit that packet types should be set along header 
offsets, but it is good to state that clearly and provide some usage 
example, e.g. in examples/generator. I can send a patch for both doc and 
generator but I would like to make sure we are on the same page.



Regards
Maciej

Re: [lng-odp] [API-NEXT PATCHv2 1/2] api: dev: add device apis for numa support

[Christophe Milard]

On 13 October 2016 at 13:20, Bill Fischofer  wrote:
>
>
> On Thu, Oct 13, 2016 at 2:03 AM, Christophe Milard
>  wrote:
>>
>> On 13 October 2016 at 02:44, Bill Fischofer 
>> wrote:
>> > Add the odp_dev_id() API used for NUMA support
>> >
>>
>> I am a bit confused here: what is a device? a numa_id or other things
>> as well? In this patch series everything that relates to numa is
>> called "device". Shouldn't be called numa_dev when it is a numa
>> device?
>> If devices are numa dev only, they should be called numa_dev. If a
>> device can be anything else (which you general approach seems to
>> imply), how are they different from handles?
>>
>> Not sure I understand where these patches lead to...
>
>
> These patches are just implementing the APIs proposed by Petri during the
> ODP Design Summit at LAS16.  We can consider them RFCs for now if that's
> preferable. A dev_id in ODP is supposed to be the same as a socket_id in
> DPDK, but not necessarily tied to the CPU socket config. The intent is
> simply to have a placeholder where hierarchical NUMA-type identifiers can be
> obtained and then used as part of resource (pools, etc.) creation. This is
> inherently system dependent, which is why the odp-linux versions are mostly
> placeholders, and why I put the implementations under the arch directory.
>

But still: is this for numa only (in which case I would expect a
clearer name) or are these devices meant to be used by other things
(which ones?). And how does this differ from the handles we already
have for other objects?

Christophe
>>
>>
>> Christophe.
>>
>> > Signed-off-by: Bill Fischofer 
>> > ---
>> >  include/odp/api/spec/dev.h | 89
>> > ++
>> >  1 file changed, 89 insertions(+)
>> >  create mode 100644 include/odp/api/spec/dev.h
>> >
>> > diff --git a/include/odp/api/spec/dev.h b/include/odp/api/spec/dev.h
>> > new file mode 100644
>> > index 000..1f7ed8b
>> > --- /dev/null
>> > +++ b/include/odp/api/spec/dev.h
>> > @@ -0,0 +1,89 @@
>> > +/* Copyright (c) 2016, Linaro Limited
>> > + * All rights reserved.
>> > + *
>> > + * SPDX-License-Identifier: BSD-3-Clause
>> > + */
>> > +
>> > +/**
>> > + * @file
>> > + *
>> > + * ODP device
>> > + */
>> > +
>> > +#ifndef ODP_API_DEV_H_
>> > +#define ODP_API_DEV_H_
>> > +#include 
>> > +
>> > +#ifdef __cplusplus
>> > +extern "C" {
>> > +#endif
>> > +
>> > +#include 
>> > +
>> > +/** @defgroup odp_dev ODP DEVICE
>> > + *  Operations on devices
>> > + *  @{
>> > + */
>> > +
>> > +/**
>> > + * @typedef odp_dev_t
>> > + * ODP Device
>> > + */
>> > +
>> > +/**
>> > + * @def ODP_DEV_NAME_LEN
>> > + * Maximum device name length in chars
>> > + */
>> > +
>> > +/**
>> > + * @def ODP_DEV_ANY
>> > + * Any device
>> > + */
>> > +
>> > +/**
>> > + * @def ODP_DEV_INVALID
>> > + * Invalid device
>> > + */
>> > +
>> > +/**
>> > + * Get Device ID by Name
>> > + *
>> > + * Get an implementation-defined device identifier from a device name.
>> > Device
>> > + * names are supplied as parameter info (command line, file, etc.) to
>> > the
>> > + * application. This routine translates this symbolic name into an
>> > internal
>> > + * identifier that can be used to define a device connection hierarchy
>> > for
>> > + * NUMA or other purposes.
>> > + *
>> > + * The reserved id ODP_DEV_ANY may be used as a "don't care"
>> > placeholder
>> > + * wherever a device id is required.
>> > + *
>> > + * @param name Name of the device
>> > + *
>> > + * @return Device ID
>> > + * @retval ODP_DEV_INVALID Device is unknown
>> > + */
>> > +odp_dev_t odp_dev_id(const char *name);
>> > +
>> > +/**
>> > + * Get printable value for an odp_dev_t
>> > + *
>> > + * @param hdl  odp_dev_t handle to be printed
>> > + * @return uint64_t value that can be used to print/display this
>> > + * handle
>> > + *
>> > + * @note This routine is intended to be used for diagnostic purposes
>> > + * to enable applications to generate a printable value that represents
>> > + * an odp_dev_t handle.
>> > + */
>> > +uint64_t odp_dev_to_u64(odp_dev_t hdl);
>> > +
>> > +/**
>> > + * @}
>> > + */
>> > +
>> > +#ifdef __cplusplus
>> > +}
>> > +#endif
>> > +
>> > +#include 
>> > +#endif
>> > --
>> > 2.7.4
>> >
>
>

Re: [lng-odp] [API-NEXT PATCHv2 1/2] api: dev: add device apis for numa support

[Bill Fischofer]

On Thu, Oct 13, 2016 at 2:03 AM, Christophe Milard <
christophe.mil...@linaro.org> wrote:

> On 13 October 2016 at 02:44, Bill Fischofer 
> wrote:
> > Add the odp_dev_id() API used for NUMA support
> >
>
> I am a bit confused here: what is a device? a numa_id or other things
> as well? In this patch series everything that relates to numa is
> called "device". Shouldn't be called numa_dev when it is a numa
> device?
> If devices are numa dev only, they should be called numa_dev. If a
> device can be anything else (which you general approach seems to
> imply), how are they different from handles?
>
> Not sure I understand where these patches lead to...
>

These patches are just implementing the APIs proposed by Petri during the
ODP Design Summit at LAS16.  We can consider them RFCs for now if that's
preferable. A dev_id in ODP is supposed to be the same as a socket_id in
DPDK, but not necessarily tied to the CPU socket config. The intent is
simply to have a placeholder where hierarchical NUMA-type identifiers can
be obtained and then used as part of resource (pools, etc.) creation. This
is inherently system dependent, which is why the odp-linux versions are
mostly placeholders, and why I put the implementations under the arch
directory.


>
> Christophe.
>
> > Signed-off-by: Bill Fischofer 
> > ---
> >  include/odp/api/spec/dev.h | 89 ++
> 
> >  1 file changed, 89 insertions(+)
> >  create mode 100644 include/odp/api/spec/dev.h
> >
> > diff --git a/include/odp/api/spec/dev.h b/include/odp/api/spec/dev.h
> > new file mode 100644
> > index 000..1f7ed8b
> > --- /dev/null
> > +++ b/include/odp/api/spec/dev.h
> > @@ -0,0 +1,89 @@
> > +/* Copyright (c) 2016, Linaro Limited
> > + * All rights reserved.
> > + *
> > + * SPDX-License-Identifier: BSD-3-Clause
> > + */
> > +
> > +/**
> > + * @file
> > + *
> > + * ODP device
> > + */
> > +
> > +#ifndef ODP_API_DEV_H_
> > +#define ODP_API_DEV_H_
> > +#include 
> > +
> > +#ifdef __cplusplus
> > +extern "C" {
> > +#endif
> > +
> > +#include 
> > +
> > +/** @defgroup odp_dev ODP DEVICE
> > + *  Operations on devices
> > + *  @{
> > + */
> > +
> > +/**
> > + * @typedef odp_dev_t
> > + * ODP Device
> > + */
> > +
> > +/**
> > + * @def ODP_DEV_NAME_LEN
> > + * Maximum device name length in chars
> > + */
> > +
> > +/**
> > + * @def ODP_DEV_ANY
> > + * Any device
> > + */
> > +
> > +/**
> > + * @def ODP_DEV_INVALID
> > + * Invalid device
> > + */
> > +
> > +/**
> > + * Get Device ID by Name
> > + *
> > + * Get an implementation-defined device identifier from a device name.
> Device
> > + * names are supplied as parameter info (command line, file, etc.) to
> the
> > + * application. This routine translates this symbolic name into an
> internal
> > + * identifier that can be used to define a device connection hierarchy
> for
> > + * NUMA or other purposes.
> > + *
> > + * The reserved id ODP_DEV_ANY may be used as a "don't care" placeholder
> > + * wherever a device id is required.
> > + *
> > + * @param name Name of the device
> > + *
> > + * @return Device ID
> > + * @retval ODP_DEV_INVALID Device is unknown
> > + */
> > +odp_dev_t odp_dev_id(const char *name);
> > +
> > +/**
> > + * Get printable value for an odp_dev_t
> > + *
> > + * @param hdl  odp_dev_t handle to be printed
> > + * @return uint64_t value that can be used to print/display this
> > + * handle
> > + *
> > + * @note This routine is intended to be used for diagnostic purposes
> > + * to enable applications to generate a printable value that represents
> > + * an odp_dev_t handle.
> > + */
> > +uint64_t odp_dev_to_u64(odp_dev_t hdl);
> > +
> > +/**
> > + * @}
> > + */
> > +
> > +#ifdef __cplusplus
> > +}
> > +#endif
> > +
> > +#include 
> > +#endif
> > --
> > 2.7.4
> >
>

[lng-odp] linker --whole-archive, --start-group options and noinst_RELOCATABLES

[Christophe Milard]

Hi,

I am working for ODP (OpenDataPlane: http://opendataplane.org) and we
are using autotools (including libtool)
We want to have connectable plug-ins to implements some functions
(actually user-space NIC drivers). These plug-ins would be either
linked dynamicaly (using dlopen()) or staticaly, at build time, that
is, some can be selected at build time, and some other at run time.
All these drivers will register themselves using a driver_register()
function provided by ODP "core" , when booting, i.e. each driver will
call driver_register() from a  __constructor__ or .init function
present in each driver. From there a set of function pointers will be
exchanged to achieve the wished functionality.

As many drivers can co-exist, it is not possible for the ODP "core" to
call any known driver symbol (there would be multiple definition of
this symbol when many drivers are included at the same time)

Focusing on the build-time drivers, this means that no obvious
reference will be seen by the linker from ODP core to the drivers's
"lib".

I  have searched a while for a solution to pass the --whole-archive
option to the linker but as autotools reorder the linker options, that
did not work well.
Same problem with the --start-group option...

Maybe you understand that I eventually reached the following threads:
https://lists.gnu.org/archive/html/automake/2006-04/msg00101.html

The situation described in the following thread:
http://lists.gnu.org/archive/html/automake/2006-03/msg00042.html
Part II (motivation), section 3 is pretty much my situation...

But the thread status is not clear to me...It just dies... I could not
find what that lead to...
Did the patches reached autotools in some way?
Is there any noinst_RELOCATABLES usable? Or if not, how should such a
situation be handled?

Thanks,

Christophe MILARD

Re: [lng-odp] [API-NEXT PATCHv2 1/2] api: dev: add device apis for numa support

[Christophe Milard]

On 13 October 2016 at 02:44, Bill Fischofer  wrote:
> Add the odp_dev_id() API used for NUMA support
>

I am a bit confused here: what is a device? a numa_id or other things
as well? In this patch series everything that relates to numa is
called "device". Shouldn't be called numa_dev when it is a numa
device?
If devices are numa dev only, they should be called numa_dev. If a
device can be anything else (which you general approach seems to
imply), how are they different from handles?

Not sure I understand where these patches lead to...

Christophe.

> Signed-off-by: Bill Fischofer 
> ---
>  include/odp/api/spec/dev.h | 89 
> ++
>  1 file changed, 89 insertions(+)
>  create mode 100644 include/odp/api/spec/dev.h
>
> diff --git a/include/odp/api/spec/dev.h b/include/odp/api/spec/dev.h
> new file mode 100644
> index 000..1f7ed8b
> --- /dev/null
> +++ b/include/odp/api/spec/dev.h
> @@ -0,0 +1,89 @@
> +/* Copyright (c) 2016, Linaro Limited
> + * All rights reserved.
> + *
> + * SPDX-License-Identifier: BSD-3-Clause
> + */
> +
> +/**
> + * @file
> + *
> + * ODP device
> + */
> +
> +#ifndef ODP_API_DEV_H_
> +#define ODP_API_DEV_H_
> +#include 
> +
> +#ifdef __cplusplus
> +extern "C" {
> +#endif
> +
> +#include 
> +
> +/** @defgroup odp_dev ODP DEVICE
> + *  Operations on devices
> + *  @{
> + */
> +
> +/**
> + * @typedef odp_dev_t
> + * ODP Device
> + */
> +
> +/**
> + * @def ODP_DEV_NAME_LEN
> + * Maximum device name length in chars
> + */
> +
> +/**
> + * @def ODP_DEV_ANY
> + * Any device
> + */
> +
> +/**
> + * @def ODP_DEV_INVALID
> + * Invalid device
> + */
> +
> +/**
> + * Get Device ID by Name
> + *
> + * Get an implementation-defined device identifier from a device name. Device
> + * names are supplied as parameter info (command line, file, etc.) to the
> + * application. This routine translates this symbolic name into an internal
> + * identifier that can be used to define a device connection hierarchy for
> + * NUMA or other purposes.
> + *
> + * The reserved id ODP_DEV_ANY may be used as a "don't care" placeholder
> + * wherever a device id is required.
> + *
> + * @param name Name of the device
> + *
> + * @return Device ID
> + * @retval ODP_DEV_INVALID Device is unknown
> + */
> +odp_dev_t odp_dev_id(const char *name);
> +
> +/**
> + * Get printable value for an odp_dev_t
> + *
> + * @param hdl  odp_dev_t handle to be printed
> + * @return uint64_t value that can be used to print/display this
> + * handle
> + *
> + * @note This routine is intended to be used for diagnostic purposes
> + * to enable applications to generate a printable value that represents
> + * an odp_dev_t handle.
> + */
> +uint64_t odp_dev_to_u64(odp_dev_t hdl);
> +
> +/**
> + * @}
> + */
> +
> +#ifdef __cplusplus
> +}
> +#endif
> +
> +#include 
> +#endif
> --
> 2.7.4
>