Re: [lng-odp] [API-NEXT PATCH v2 3/4] Revert "linux-generic: packet: implement reference apis"

[Savolainen, Petri (Nokia - FI/Espoo)]

> -Original Message-
> From: lng-odp [mailto:lng-odp-boun...@lists.linaro.org] On Behalf Of Maxim
> Uvarov
> Sent: Wednesday, February 15, 2017 6:19 PM
> To: lng-odp@lists.linaro.org
> Subject: Re: [lng-odp] [API-NEXT PATCH v2 3/4] Revert "linux-generic:
> packet: implement reference apis"
> 
> On 02/15/17 15:29, Petri Savolainen wrote:
> > This reverts commit 22b3986fea090986625f3255d57b64de35bbc475.
> > ---
> 
> sign-off is missing. Checkpatch should warn about that.
> 
> Maxim.

This patch is output of the revert command as-is, and has couple of check patch 
warnings due to that. I can add sign-off there if that's needed, although I 
didn't do any modifications into the commit itself.

Also 'make check' fails after this commit since the new implementation is added 
in the next commit. Anyway, I think this  results cleaner git history as the 
revert is not modified (== not squashed with the new implementation). E.g. 
odp-dpdk can directly merge the new implementation, without the need to first 
merge the reverted one.

-Petri

Re: [lng-odp] [PATCH 1/5] test: generator: compose sending packets from reference packet plus differences

[Bogdan Pricope]

Hi,

>From my point of view, at the end of review process this series (+ the
patch related to stats logging message) should be added at least in
odp-dpdk.

Bogdan

On 15 February 2017 at 18:55, Mike Holmes  wrote:
> So this is a request to add this to Monarch LTS then ?
>
> On 15 February 2017 at 03:58, Bogdan Pricope 
> wrote:
>>
>> Hi,
>>
>> There were multiple small “fixes” required to make the packet valid.
>> They should go in together and in all branches.
>>
>> Br,
>> Bogdan
>>
>> On 14 February 2017 at 16:38, Nicolas Morey-Chaisemartin
>>  wrote:
>> >
>> >
>> > Le 02/13/2017 à 01:49 PM, Bogdan Pricope a écrit :
>> >> Signed-off-by: Bogdan Pricope 
>> >> ---
>> >>  example/generator/odp_generator.c | 131
>> >> +-
>> >>  1 file changed, 114 insertions(+), 17 deletions(-)
>> >>
>> >> diff --git a/example/generator/odp_generator.c
>> >> b/example/generator/odp_generator.c
>> >> index 8062d87..d1e3ecc 100644
>> >> --- a/example/generator/odp_generator.c
>> >> +++ b/example/generator/odp_generator.c
>> >> @@ -170,21 +170,20 @@ static int scan_ip(char *buf, unsigned int
>> >> *paddr)
>> >>  }
>> >>
>> >>  /**
>> >> - * set up an udp packet
>> >> + * set up an udp packet reference
>> >>   *
>> >>   * @param pool Buffer pool to create packet in
>> >>   *
>> >>   * @return Handle of created packet
>> >>   * @retval ODP_PACKET_INVALID  Packet could not be created
>> >>   */
>> >> -static odp_packet_t pack_udp_pkt(odp_pool_t pool)
>> >> +static odp_packet_t setup_udp_pkt_ref(odp_pool_t pool)
>> >>  {
>> >>   odp_packet_t pkt;
>> >>   char *buf;
>> >>   odph_ethhdr_t *eth;
>> >>   odph_ipv4hdr_t *ip;
>> >>   odph_udphdr_t *udp;
>> >> - unsigned short seq;
>> >>
>> >>   pkt = odp_packet_alloc(pool, args->appl.payload + ODPH_UDPHDR_LEN
>> >> +
>> >>  ODPH_IPV4HDR_LEN + ODPH_ETHHDR_LEN);
>> >> @@ -200,8 +199,10 @@ static odp_packet_t pack_udp_pkt(odp_pool_t pool)
>> >>   memcpy((char *)eth->src.addr, args->appl.srcmac.addr,
>> >> ODPH_ETHADDR_LEN);
>> >>   memcpy((char *)eth->dst.addr, args->appl.dstmac.addr,
>> >> ODPH_ETHADDR_LEN);
>> >>   eth->type = odp_cpu_to_be_16(ODPH_ETHTYPE_IPV4);
>> >> +
>> >>   /* ip */
>> >>   odp_packet_l3_offset_set(pkt, ODPH_ETHHDR_LEN);
>> >> + odp_packet_has_ipv4_set(pkt, 1);
>> >>   ip = (odph_ipv4hdr_t *)(buf + ODPH_ETHHDR_LEN);
>> >>   ip->dst_addr = odp_cpu_to_be_32(args->appl.dstip);
>> >>   ip->src_addr = odp_cpu_to_be_32(args->appl.srcip);
>> >> @@ -209,12 +210,13 @@ static odp_packet_t pack_udp_pkt(odp_pool_t pool)
>> >>   ip->tot_len = odp_cpu_to_be_16(args->appl.payload +
>> >> ODPH_UDPHDR_LEN +
>> >>  ODPH_IPV4HDR_LEN);
>> >>   ip->proto = ODPH_IPPROTO_UDP;
>> >> - seq = odp_atomic_fetch_add_u64(&counters.seq, 1) % 0x;
>> >> - ip->id = odp_cpu_to_be_16(seq);
>> >> + ip->id = 0;
>> >> + ip->ttl = 64;
>> >>   ip->chksum = 0;
>> >> - odph_ipv4_csum_update(pkt);
>> >> +
>> >>   /* udp */
>> >>   odp_packet_l4_offset_set(pkt, ODPH_ETHHDR_LEN +
>> >> ODPH_IPV4HDR_LEN);
>> >> + odp_packet_has_udp_set(pkt, 1);
>> >>   udp = (odph_udphdr_t *)(buf + ODPH_ETHHDR_LEN +
>> >> ODPH_IPV4HDR_LEN);
>> >>   udp->src_port = 0;
>> >>   udp->dst_port = 0;
>> >> @@ -226,27 +228,60 @@ static odp_packet_t pack_udp_pkt(odp_pool_t pool)
>> >>  }
>> >>
>> > The calls to odp_packet_has_ipv4_set and odp_packet_has_udp_set are
>> > actually a bug fix needed to have the proper checksum computations.
>> > Without these calls, the checksum are set to 0 as all the packet parse
>> > flags are 0.
>> >
>> > It might be worth splitting this into a specific patch.
>> > I posted one for monarch without any answer to it, but it's something
>> > that should be put in all branches as it's a bug fix.
>> >
>> > Nicolas
>
>
>
>
> --
> 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] [API-NEXT PATCHv4 1/4] api: timer: add odp_timer_capability() api

[Yi He]

For this patch series:

Reviewed-and-tested-by: Yi He 

On 8 February 2017 at 10:37, Kevin Wang  wrote:

> Currently, user needs to decide the timer resolution before creating
> a timer pool. But sometimes it will cause timer overrun as the system
> can't support such high resolution.
> So a new API is required to expose the timer capability to the user.
>
> Signed-off-by: Kevin Wang 
> ---
>  include/odp/api/spec/timer.h | 21 +
>  1 file changed, 21 insertions(+)
>
> diff --git a/include/odp/api/spec/timer.h b/include/odp/api/spec/timer.h
> index 75f9db9..e8d85b7 100644
> --- a/include/odp/api/spec/timer.h
> +++ b/include/odp/api/spec/timer.h
> @@ -108,6 +108,27 @@ typedef struct {
>  } odp_timer_pool_param_t;
>
>  /**
> + * Timer capability
> + */
> +typedef struct {
> +   uint64_t res_ns; /**< Timeout resolution in nanoseconds */
> +} odp_timer_capability_t;
> +
> +/**
> + * Query Timer interface capabilities
> + *
> + * Outputs Timer interface capabilities on success.
> + *
> + * @param clk_src Clock source for timers
> + * @param[out] capa   Pointer to capability structure for output
> + *
> + * @retval 0 on success
> + * @retval <0 on failure
> + */
> +int odp_timer_capability(odp_timer_clk_src_t clk_src,
> +odp_timer_capability_t *capa);
> +
> +/**
>   * Create a timer pool
>   *
>   * The use of pool name is optional. Unique names are not required.
> --
> 1.9.1
>
>

[lng-odp] [Bug 2862] CID 177251 et al: Unsafe pointer dereferences in iplookuptable() helper

[bugzilla-daemon]

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

Bill Fischofer  changed:

   What|Removed |Added

 Status|IN_PROGRESS |RESOLVED
 Resolution|--- |FIXED

--- Comment #2 from Bill Fischofer  ---
Merged. Commit ID dd494a2b6a22f0516ce8890df05de826377fdae2

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

[lng-odp] [Bug 2829] CID 168965: Unused value in helper iptablelookup

[bugzilla-daemon]

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

Bill Fischofer  changed:

   What|Removed |Added

 Status|IN_PROGRESS |RESOLVED
 Resolution|--- |FIXED

--- Comment #2 from Bill Fischofer  ---
Merged. Commit ID 483af96a7a14885694496843551ed8fad708192c

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

[lng-odp] [Bug 2830] CID 168956: Storage leak in cuckoo.c

[bugzilla-daemon]

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

Bill Fischofer  changed:

   What|Removed |Added

 Status|IN_PROGRESS |RESOLVED
 Resolution|--- |FIXED

--- Comment #3 from Bill Fischofer  ---
Merged. Commit ID f59d87a65cf1d08f8715a01dceeaf370a29e51f3

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

[lng-odp] [Bug 2865] odp helper table APIs are not documented

[bugzilla-daemon]

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

Bill Fischofer  changed:

   What|Removed |Added

 Status|IN_PROGRESS |RESOLVED
 Resolution|--- |FIXED

--- Comment #6 from Bill Fischofer  ---
Merged. Commit ID 3528ca70668da492a2f1fe85c167b2024406dd8d

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

Re: [lng-odp] [PATCH v3 0/8] First ABI files

[Bill Fischofer]

On Wed, Feb 15, 2017 at 12:53 PM, Brian Brooks  wrote:
> On Wed, Feb 15, 2017 at 5:14 AM, Savolainen, Petri (Nokia - FI/Espoo)
>  wrote:
>>
>>
>> From: Yi He [mailto:yi...@linaro.org]
>> Sent: Tuesday, February 14, 2017 3:59 AM
>> To: Savolainen, Petri (Nokia - FI/Espoo) 
>> 
>> Cc: Brian Brooks ; lng-odp@lists.linaro.org
>> Subject: Re: [lng-odp] [PATCH v3 0/8] First ABI files
>>
>> Hi, Petri and Brian
>>
>> Can I understand this patch series are giving various platforms a chance to 
>> define their own data structures (like odp_event_t etc) in arch directory, 
>> instead of sharing the same definitions as previously?
>
> The way this software reads is that some data types of the API are now
> defined in these Arch+OS (pair) files instead of by the ODP
> implementation itself.  This is headed in the right direction! ..
> because the implementation should NOT be allowed to define API types.
> But, it needs to be taken even further.  One complete and universal
> set of APIs.  What is the concrete use case for allowing all of these
> API types to be defined differently for each Arch+OS pair?  I have
> previously sent a patch to the list which defines all of the API in
> one universal set of header files.

There are several things going on here, and it's important to
distinguish them to avoid confusion.

1. The ODP API provides flexibility to implementers by allowing them
to instantiate ODP's abstract data types however they wish. This is
the foundation for source-level portability across disparate platforms
and system architectures. So each platform provides the typedefs that
define each of these types.

2. The ODP ABI provides a means whereby different platforms that share
a common ISA can agree to certain conventions that ensure that
applications have binary portability across each of the ODP
implementations subscribing to that ABI specification.

But here's where the confusion arises. ODP provides a default ABI that
is a least-common-denominator "universal" ABI that only assumes that
the sizes of each of the defined typedefs are the same and that every
ODP API involves a call to a separately-linked corresponding
implementation of that API. While this satisfies the goals of having
an ABI, it does so at a non-negligible performance cost. For some
applications this may not matter, while for others this may be a
concern. To address those performance concerns, this new directory
organization is designed to permit platforms to support multiple ABIs
tailored to different performance needs.

If two or more platforms agree on not just the size of certain
typedefs, but on the organization of the internals behind those
typedefs, then they can define an ABI among themselves that permits
certain ODP APIs to be inlined in ODP applications and still retain
binary portability across platforms that subscribe to that ABI. To see
this, take a concrete example. The odp_ticketlock APIs may be used
frequently so it is a benefit if these can be expanded inline. To do
that two or more platforms sharing an ISA would need to agree on what
an odp_ticketlock_t actually is and how a the various ticketlock
operations are to be realized. Without such an agreement, an
application that had an inline expansion of the ticketlock APIs from
platform A would not be binary portable to platform B that used a
different set of ticketlock internals. But if these platforms all
agree on how these APIs are implemented then they can be expanded
inline for improved performance while still retaining binary
portability among all platforms that have made such an agreement.

The same is true for all other ODP APIs that might be frequently used.
Some of these, like ticketlocks, may be easy to find agreement on
while others that might benefit from inlining, such as
odp_packet_len(), might be more problematic since to inline these
functions platforms would have to agree on common odp_packet_t
internals that may be quite different when HW packet/buffer managers
are involved.

The point is that ABIs are an important part of the ODP cloud profile
and achieving the right balance between binary portability and
performance in cloud environments is something we can expect to see
some experimentation with over the next few years as applications gain
production experience in such environments. What we see in this
directory layout is simple the framework for supporting these
experiments along with the initial (default) ABI that will always be
available.

>
>> Best Regards, Yi
>>
>>
>> No, this adds ABI spec(s). One ABI spec cover (potentially) many 
>> implementations. If an implementation is built in ABI compat mode, it has to 
>> use definitions of the ABI spec. Otherwise (non-ABI compat mode), it can use 
>> its own definitions.
>>
>> -Petri
>>

Re: [lng-odp] [PATCH 3/5] travis: catch doxygen errors

[Maxim Uvarov]

On 02/15/17 20:56, Mike Holmes wrote:
> 
> 
> On 14 February 2017 at 15:32, Maxim Uvarov  > wrote:
> 
> make doxygen-doc does not return negative code on
> errors. Do it manually.
> 
> Signed-off-by: Maxim Uvarov  >
> ---
>  .travis.yml | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/.travis.yml b/.travis.yml
> index 9e7471c2..1ab943fe 100644
> --- a/.travis.yml
> +++ b/.travis.yml
> @@ -119,7 +119,9 @@ script:
> 
>  - ./bootstrap
>  - ./configure
> -- make doxygen-doc
> +#doxygen does not trap on warnings, check for them here.
> +- make doxygen-doc |tee doxygen.log
> +- fgrep -rvq warning ./doxygen.log
> 
> 
> Could Doxygens own error parameter be used to achieve this  transparently ?
> 
> |WARN_AS_ERROR|
> 
> If the |WARN_AS_ERROR| tag is set to |YES| then doxygen will
> immediately stop when a warning is encountered.
> 
>  

Do you have working version for this option? I tried to do it but might
be it works only with latest doxygen.


Warning: ignoring unsupported tag `WARN_AS_ERROR =' at line 10, file
./doc/Doxyfile_common
22:27 /opt/Linaro/odp3.git (master)$echo $?
0




> 
>  - make distcheck
> 
>  - ./bootstrap
> --
> 2.11.0.295.gd7dffce
> 
> 
> 
> 
> -- 
> 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 5/5] travis: check odp license agreement

[Maxim Uvarov]

On 02/15/17 20:49, Mike Holmes wrote:
> 
> 
> On 14 February 2017 at 15:32, Maxim Uvarov  > wrote:
> 
> Signed-off-by: Maxim Uvarov  >
> ---
>  .travis.yml | 5 +
>  1 file changed, 5 insertions(+)
> 
> diff --git a/.travis.yml b/.travis.yml
> index ef9c3283..d7f4c66e 100644
> --- a/.travis.yml
> +++ b/.travis.yml
> @@ -100,6 +100,9 @@ before_install:
>  - make > /dev/null
>  - sudo insmod ./netmap.ko
>  - popd
> +#   Download tool to check odp contibutor agreement
> +- git clone
> https://git.linaro.org/people/maxim.uvarov/odp-agreement.git
> 
> 
> 
> Should this just be in odp/scripts and thus self contained ? If not
> should it be in an official Linaro repo rather than a private one ?
> 
> I vote in odp/scripts becasue the .travis itself is already tailored to
> this reference implimentation of the ODP API, and this is an extension
> of that
>  

Well.. there is database of users which accepted agreement. And we can
not put that DB to main git. That is why it can not be in scripts.

The right place is github web hook integration. The same way as SAP did
for their projects. But it will take some time to understand web code
setup and do required changes. After set up it has to look like this:

https://github.com/muvarov/odp/pull/1

Maxim.


> 
> 
> +
> 
>  script:
>  - echo $TRAVIS_COMMIT_RANGE
> @@ -112,9 +115,11 @@ script:
>  if [ $? -ne 0 ]; then
>git format-patch HEAD^;
>perl ./scripts/checkpatch.pl 
> *.patch;
> +  ./odp-agreement/check_patchlist.sh *.patch;
>  fi;
>else
>  perl ./scripts/checkpatch.pl 
> *.patch;
> +./odp-agreement/check_patchlist.sh *.patch;
>fi
> 
>  - ./bootstrap
> --
> 2.11.0.295.gd7dffce
> 
> 
> 
> 
> -- 
> 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 v3 0/8] First ABI files

[Brian Brooks]

On Wed, Feb 15, 2017 at 5:14 AM, Savolainen, Petri (Nokia - FI/Espoo)
 wrote:
>
>
> From: Yi He [mailto:yi...@linaro.org]
> Sent: Tuesday, February 14, 2017 3:59 AM
> To: Savolainen, Petri (Nokia - FI/Espoo) 
> 
> Cc: Brian Brooks ; lng-odp@lists.linaro.org
> Subject: Re: [lng-odp] [PATCH v3 0/8] First ABI files
>
> Hi, Petri and Brian
>
> Can I understand this patch series are giving various platforms a chance to 
> define their own data structures (like odp_event_t etc) in arch directory, 
> instead of sharing the same definitions as previously?

The way this software reads is that some data types of the API are now
defined in these Arch+OS (pair) files instead of by the ODP
implementation itself.  This is headed in the right direction! ..
because the implementation should NOT be allowed to define API types.
But, it needs to be taken even further.  One complete and universal
set of APIs.  What is the concrete use case for allowing all of these
API types to be defined differently for each Arch+OS pair?  I have
previously sent a patch to the list which defines all of the API in
one universal set of header files.

> Best Regards, Yi
>
>
> No, this adds ABI spec(s). One ABI spec cover (potentially) many 
> implementations. If an implementation is built in ABI compat mode, it has to 
> use definitions of the ABI spec. Otherwise (non-ABI compat mode), it can use 
> its own definitions.
>
> -Petri
>

Re: [lng-odp] [PATCH 3/5] travis: catch doxygen errors

[Mike Holmes]

On 14 February 2017 at 15:32, Maxim Uvarov  wrote:

> make doxygen-doc does not return negative code on
> errors. Do it manually.
>
> Signed-off-by: Maxim Uvarov 
> ---
>  .travis.yml | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/.travis.yml b/.travis.yml
> index 9e7471c2..1ab943fe 100644
> --- a/.travis.yml
> +++ b/.travis.yml
> @@ -119,7 +119,9 @@ script:
>
>  - ./bootstrap
>  - ./configure
> -- make doxygen-doc
> +#doxygen does not trap on warnings, check for them here.
> +- make doxygen-doc |tee doxygen.log
> +- fgrep -rvq warning ./doxygen.log
>

Could Doxygens own error parameter be used to achieve this  transparently ?

WARN_AS_ERROR

If the WARN_AS_ERROR tag is set to YES then doxygen will immediately stop
when a warning is encountered.


>  - make distcheck
>
>  - ./bootstrap
> --
> 2.11.0.295.gd7dffce
>
>


-- 
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 5/5] travis: check odp license agreement

[Mike Holmes]

On 14 February 2017 at 15:32, Maxim Uvarov  wrote:

> Signed-off-by: Maxim Uvarov 
> ---
>  .travis.yml | 5 +
>  1 file changed, 5 insertions(+)
>
> diff --git a/.travis.yml b/.travis.yml
> index ef9c3283..d7f4c66e 100644
> --- a/.travis.yml
> +++ b/.travis.yml
> @@ -100,6 +100,9 @@ before_install:
>  - make > /dev/null
>  - sudo insmod ./netmap.ko
>  - popd
> +#   Download tool to check odp contibutor agreement
> +- git clone https://git.linaro.org/people/
> maxim.uvarov/odp-agreement.git


Should this just be in odp/scripts and thus self contained ? If not should
it be in an official Linaro repo rather than a private one ?

I vote in odp/scripts becasue the .travis itself is already tailored to
this reference implimentation of the ODP API, and this is an extension of
that


>
> +
>
>  script:
>  - echo $TRAVIS_COMMIT_RANGE
> @@ -112,9 +115,11 @@ script:
>  if [ $? -ne 0 ]; then
>git format-patch HEAD^;
>perl ./scripts/checkpatch.pl *.patch;
> +  ./odp-agreement/check_patchlist.sh *.patch;
>  fi;
>else
>  perl ./scripts/checkpatch.pl *.patch;
> +./odp-agreement/check_patchlist.sh *.patch;
>fi
>
>  - ./bootstrap
> --
> 2.11.0.295.gd7dffce
>
>


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

[lng-odp] [PATCH] travis: mirror cunit on github

[Maxim Uvarov]

because of cunit fails to download from sourceforge.net
it's reasonable place source to github.

Signed-off-by: Maxim Uvarov 
---
 .travis.yml | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/.travis.yml b/.travis.yml
index 9544ec8f..66248b44 100644
--- a/.travis.yml
+++ b/.travis.yml
@@ -62,7 +62,7 @@ before_install:
 
 #   Install cunit for the validation tests because distro version is too 
old and fails C99 compile
 - export CUNIT_VERSION=2.1-3
-- curl -sSOL 
http://sourceforge.net/projects/cunit/files/CUnit/${CUNIT_VERSION}/CUnit-${CUNIT_VERSION}.tar.bz2
+- curl -sSOL 
https://github.com/Linaro/libcunit/releases/download/${CUNIT_VERSION}/CUnit-${CUNIT_VERSION}.tar.bz2
 - tar -jxf *.bz2
 - cd CUnit*
 - ./bootstrap
-- 
2.11.0.295.gd7dffce

Re: [lng-odp] [PATCH 1/5] test: generator: compose sending packets from reference packet plus differences

[Mike Holmes]

So this is a request to add this to Monarch LTS then ?

On 15 February 2017 at 03:58, Bogdan Pricope 
wrote:

> Hi,
>
> There were multiple small “fixes” required to make the packet valid.
> They should go in together and in all branches.
>
> Br,
> Bogdan
>
> On 14 February 2017 at 16:38, Nicolas Morey-Chaisemartin
>  wrote:
> >
> >
> > Le 02/13/2017 à 01:49 PM, Bogdan Pricope a écrit :
> >> Signed-off-by: Bogdan Pricope 
> >> ---
> >>  example/generator/odp_generator.c | 131 ++
> +++-
> >>  1 file changed, 114 insertions(+), 17 deletions(-)
> >>
> >> diff --git a/example/generator/odp_generator.c b/example/generator/odp_
> generator.c
> >> index 8062d87..d1e3ecc 100644
> >> --- a/example/generator/odp_generator.c
> >> +++ b/example/generator/odp_generator.c
> >> @@ -170,21 +170,20 @@ static int scan_ip(char *buf, unsigned int *paddr)
> >>  }
> >>
> >>  /**
> >> - * set up an udp packet
> >> + * set up an udp packet reference
> >>   *
> >>   * @param pool Buffer pool to create packet in
> >>   *
> >>   * @return Handle of created packet
> >>   * @retval ODP_PACKET_INVALID  Packet could not be created
> >>   */
> >> -static odp_packet_t pack_udp_pkt(odp_pool_t pool)
> >> +static odp_packet_t setup_udp_pkt_ref(odp_pool_t pool)
> >>  {
> >>   odp_packet_t pkt;
> >>   char *buf;
> >>   odph_ethhdr_t *eth;
> >>   odph_ipv4hdr_t *ip;
> >>   odph_udphdr_t *udp;
> >> - unsigned short seq;
> >>
> >>   pkt = odp_packet_alloc(pool, args->appl.payload + ODPH_UDPHDR_LEN
> +
> >>  ODPH_IPV4HDR_LEN + ODPH_ETHHDR_LEN);
> >> @@ -200,8 +199,10 @@ static odp_packet_t pack_udp_pkt(odp_pool_t pool)
> >>   memcpy((char *)eth->src.addr, args->appl.srcmac.addr,
> ODPH_ETHADDR_LEN);
> >>   memcpy((char *)eth->dst.addr, args->appl.dstmac.addr,
> ODPH_ETHADDR_LEN);
> >>   eth->type = odp_cpu_to_be_16(ODPH_ETHTYPE_IPV4);
> >> +
> >>   /* ip */
> >>   odp_packet_l3_offset_set(pkt, ODPH_ETHHDR_LEN);
> >> + odp_packet_has_ipv4_set(pkt, 1);
> >>   ip = (odph_ipv4hdr_t *)(buf + ODPH_ETHHDR_LEN);
> >>   ip->dst_addr = odp_cpu_to_be_32(args->appl.dstip);
> >>   ip->src_addr = odp_cpu_to_be_32(args->appl.srcip);
> >> @@ -209,12 +210,13 @@ static odp_packet_t pack_udp_pkt(odp_pool_t pool)
> >>   ip->tot_len = odp_cpu_to_be_16(args->appl.payload +
> ODPH_UDPHDR_LEN +
> >>  ODPH_IPV4HDR_LEN);
> >>   ip->proto = ODPH_IPPROTO_UDP;
> >> - seq = odp_atomic_fetch_add_u64(&counters.seq, 1) % 0x;
> >> - ip->id = odp_cpu_to_be_16(seq);
> >> + ip->id = 0;
> >> + ip->ttl = 64;
> >>   ip->chksum = 0;
> >> - odph_ipv4_csum_update(pkt);
> >> +
> >>   /* udp */
> >>   odp_packet_l4_offset_set(pkt, ODPH_ETHHDR_LEN + ODPH_IPV4HDR_LEN);
> >> + odp_packet_has_udp_set(pkt, 1);
> >>   udp = (odph_udphdr_t *)(buf + ODPH_ETHHDR_LEN + ODPH_IPV4HDR_LEN);
> >>   udp->src_port = 0;
> >>   udp->dst_port = 0;
> >> @@ -226,27 +228,60 @@ static odp_packet_t pack_udp_pkt(odp_pool_t pool)
> >>  }
> >>
> > The calls to odp_packet_has_ipv4_set and odp_packet_has_udp_set are
> actually a bug fix needed to have the proper checksum computations.
> > Without these calls, the checksum are set to 0 as all the packet parse
> flags are 0.
> >
> > It might be worth splitting this into a specific patch.
> > I posted one for monarch without any answer to it, but it's something
> that should be put in all branches as it's a bug fix.
> >
> > Nicolas
>



-- 
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] [API-NEXT PATCH v2 3/4] Revert "linux-generic: packet: implement reference apis"

[Maxim Uvarov]

On 02/15/17 15:29, Petri Savolainen wrote:
> This reverts commit 22b3986fea090986625f3255d57b64de35bbc475.
> ---

sign-off is missing. Checkpatch should warn about that.

Maxim.

>  .../linux-generic/include/odp_packet_internal.h|  85 +---
>  platform/linux-generic/odp_packet.c| 536 
> -
>  2 files changed, 107 insertions(+), 514 deletions(-)
> 
> diff --git a/platform/linux-generic/include/odp_packet_internal.h 
> b/platform/linux-generic/include/odp_packet_internal.h
> index 42df5ac..e3ada5c 100644
> --- a/platform/linux-generic/include/odp_packet_internal.h
> +++ b/platform/linux-generic/include/odp_packet_internal.h
> @@ -19,7 +19,6 @@ extern "C" {
>  
>  #include 
>  #include 
> -#include 
>  #include 
>  #include 
>  #include 
> @@ -169,7 +168,7 @@ typedef struct {
>   * packet_init(). Because of this any new fields added must be reviewed for
>   * initialization requirements.
>   */
> -typedef struct odp_packet_hdr_t {
> +typedef struct {
>   /* common buffer header */
>   odp_buffer_hdr_t buf_hdr;
>  
> @@ -185,13 +184,6 @@ typedef struct odp_packet_hdr_t {
>   uint32_t headroom;
>   uint32_t tailroom;
>  
> - /* Fields used to support packet references */
> - uint32_t unshared_len;
> - struct odp_packet_hdr_t *ref_hdr;
> - uint32_t ref_offset;
> - uint32_t ref_len;
> - odp_atomic_u32_t ref_count;
> -
>   /*
>* Members below are not initialized by packet_init()
>*/
> @@ -220,55 +212,6 @@ static inline odp_packet_hdr_t 
> *odp_packet_hdr(odp_packet_t pkt)
>   return (odp_packet_hdr_t *)buf_hdl_to_hdr((odp_buffer_t)pkt);
>  }
>  
> -static inline odp_packet_hdr_t *odp_packet_last_hdr(odp_packet_t pkt,
> - uint32_t *offset)
> -{
> - odp_packet_hdr_t *pkt_hdr = odp_packet_hdr(pkt);
> - odp_packet_hdr_t *prev_hdr = pkt_hdr;
> - uint32_t ref_offset = 0;
> -
> - while (pkt_hdr->ref_hdr) {
> - ref_offset = pkt_hdr->ref_offset;
> - prev_hdr   = pkt_hdr;
> - pkt_hdr= pkt_hdr->ref_hdr;
> - }
> -
> - if (offset) {
> - if (prev_hdr != pkt_hdr)
> - ref_offset += pkt_hdr->frame_len - prev_hdr->ref_len;
> - *offset = ref_offset;
> - }
> -
> - return pkt_hdr;
> -}
> -
> -static inline odp_packet_hdr_t *odp_packet_prev_hdr(odp_packet_hdr_t 
> *pkt_hdr,
> - odp_packet_hdr_t *cur_hdr,
> - uint32_t *offset)
> -{
> - uint32_t ref_offset = 0;
> - odp_packet_hdr_t *prev_hdr = pkt_hdr;
> -
> - while (pkt_hdr->ref_hdr != cur_hdr) {
> - ref_offset = pkt_hdr->ref_offset;
> - prev_hdr   = pkt_hdr;
> - pkt_hdr= pkt_hdr->ref_hdr;
> - }
> -
> - if (offset) {
> - if (prev_hdr != pkt_hdr)
> - ref_offset += pkt_hdr->frame_len - prev_hdr->ref_len;
> - *offset = ref_offset;
> - }
> -
> - return pkt_hdr;
> -}
> -
> -static inline odp_packet_t _odp_packet_hdl(odp_packet_hdr_t *pkt_hdr)
> -{
> - return (odp_packet_t)odp_hdr_to_buf(&pkt_hdr->buf_hdr);
> -}
> -
>  static inline void copy_packet_parser_metadata(odp_packet_hdr_t *src_hdr,
>  odp_packet_hdr_t *dst_hdr)
>  {
> @@ -291,41 +234,17 @@ static inline void pull_tail(odp_packet_hdr_t *pkt_hdr, 
> uint32_t len)
>  
>   pkt_hdr->tailroom  += len;
>   pkt_hdr->frame_len -= len;
> - pkt_hdr->unshared_len -= len;
>   pkt_hdr->buf_hdr.seg[last].len -= len;
>  }
>  
>  static inline uint32_t packet_len(odp_packet_hdr_t *pkt_hdr)
>  {
> - uint32_t pkt_len = 0;
> - uint32_t offset  = 0;
> -
> - do {
> - pkt_len += pkt_hdr->frame_len - offset;
> - offset   = pkt_hdr->ref_offset;
> - if (pkt_hdr->ref_hdr)
> - offset += (pkt_hdr->ref_hdr->frame_len -
> -pkt_hdr->ref_len);
> - pkt_hdr  = pkt_hdr->ref_hdr;
> - } while (pkt_hdr);
> -
> - return pkt_len;
> -}
> -
> -static inline uint32_t packet_ref_count(odp_packet_hdr_t *pkt_hdr)
> -{
> - return odp_atomic_load_u32(&pkt_hdr->ref_count);
> -}
> -
> -static inline void packet_ref_count_set(odp_packet_hdr_t *pkt_hdr, uint32_t 
> n)
> -{
> - odp_atomic_init_u32(&pkt_hdr->ref_count, n);
> + return pkt_hdr->frame_len;
>  }
>  
>  static inline void packet_set_len(odp_packet_hdr_t *pkt_hdr, uint32_t len)
>  {
>   pkt_hdr->frame_len = len;
> - pkt_hdr->unshared_len = len;
>  }
>  
>  static inline int packet_parse_l2_not_done(packet_parser_t *prs)
> diff --git a/platform/linux-generic/odp_packet.c 
> b/platform/linux-generic/odp_packet.c
> index 3aadc4d..024f694 100644
> --- a/platform/linux-generic/odp_packet.c
> +++ b/platform/linux-generic/odp_packet.c
> @@ -33,24 +33,13 @@ static inli

Re: [lng-odp] Merging crypto validation suite fixes to master

[Maxim Uvarov]

cherry-picked that patches from next.

Maxim.

On 15 February 2017 at 18:34, Bill Fischofer 
wrote:

> Looks like Krishna gave it his review so it should be good to merge.
> Maxim is out today but I suspect he'll get to it either tonight or
> tomorrow.
>
> On Wed, Feb 15, 2017 at 9:32 AM, Elo, Matias (Nokia - FI/Espoo)
>  wrote:
> > This patch set (http://patches.opendataplane.org/patch/7828/) should be
> merged to the master as it includes no API changes. These patches are
> needed by odp-dpdk to enable testing HW accelerated crypto.
> >
> > Regards,
> > Matias
> >
>

[lng-odp] [PATCH 1/2] linux-gen: packet: remove unnecessary packet reparsing

[Matias Elo]

Previously the highest already parsed layer was unnecessarily reparsed on
the following packet_parse_common() calls.

Signed-off-by: Matias Elo 
---
 platform/linux-generic/odp_packet.c | 22 +++---
 1 file changed, 15 insertions(+), 7 deletions(-)

diff --git a/platform/linux-generic/odp_packet.c 
b/platform/linux-generic/odp_packet.c
index 024f694..a6cf4cd 100644
--- a/platform/linux-generic/odp_packet.c
+++ b/platform/linux-generic/odp_packet.c
@@ -2022,12 +2022,15 @@ int packet_parse_common(packet_parser_t *prs, const 
uint8_t *ptr,
case LAYER_NONE:
/* Fall through */
 
-   case LAYER_L2:
+   case LAYER_L1:
{
const _odp_ethhdr_t *eth;
uint16_t macaddr0, macaddr2, macaddr4;
const _odp_vlanhdr_t *vlan;
 
+   if (layer <= LAYER_L1)
+   return prs->error_flags.all != 0;
+
offset = sizeof(_odp_ethhdr_t);
if (packet_parse_l2_not_done(prs))
packet_parse_l2(prs, frame_len);
@@ -2091,13 +2094,14 @@ int packet_parse_common(packet_parser_t *prs, const 
uint8_t *ptr,
 
prs->l3_offset = offset;
prs->parsed_layers = LAYER_L2;
-   if (layer == LAYER_L2)
-   return prs->error_flags.all != 0;
}
/* Fall through */
 
-   case LAYER_L3:
+   case LAYER_L2:
{
+   if (layer <= LAYER_L2)
+   return prs->error_flags.all != 0;
+
offset = prs->l3_offset;
parseptr = (const uint8_t *)(ptr + offset);
/* Set l3_offset+flag only for known ethtypes */
@@ -2131,13 +2135,14 @@ int packet_parse_common(packet_parser_t *prs, const 
uint8_t *ptr,
/* Set l4_offset+flag only for known ip_proto */
prs->l4_offset = offset;
prs->parsed_layers = LAYER_L3;
-   if (layer == LAYER_L3)
-   return prs->error_flags.all != 0;
}
/* Fall through */
 
-   case LAYER_L4:
+   case LAYER_L3:
{
+   if (layer <= LAYER_L3)
+   return prs->error_flags.all != 0;
+
offset = prs->l4_offset;
parseptr = (const uint8_t *)(ptr + offset);
prs->input_flags.l4 = 1;
@@ -2186,6 +2191,9 @@ int packet_parse_common(packet_parser_t *prs, const 
uint8_t *ptr,
break;
}
 
+   case LAYER_L4:
+   break;
+
case LAYER_ALL:
break;
 
-- 
2.7.4

[lng-odp] [PATCH 2/2] linux-gen: packet: reset packet parse level after offset change

[Matias Elo]

Reset packet parse level after odp_packet_*_offset_set() call. Otherwise
the following odp_packet_has_error() would fail as the upper layer offsets
are not valid anymore.

Signed-off-by: Matias Elo 
---
 platform/linux-generic/odp_packet.c | 11 +++
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/platform/linux-generic/odp_packet.c 
b/platform/linux-generic/odp_packet.c
index a6cf4cd..045ef61 100644
--- a/platform/linux-generic/odp_packet.c
+++ b/platform/linux-generic/odp_packet.c
@@ -1258,7 +1258,10 @@ int odp_packet_l2_offset_set(odp_packet_t pkt, uint32_t 
offset)
return -1;
 
packet_hdr_has_l2_set(pkt_hdr, 1);
+
pkt_hdr->p.l2_offset = offset;
+   if (pkt_hdr->p.parsed_layers < LAYER_ALL)
+   pkt_hdr->p.parsed_layers = LAYER_L1;
return 0;
 }
 
@@ -1287,9 +1290,9 @@ int odp_packet_l3_offset_set(odp_packet_t pkt, uint32_t 
offset)
if (offset >= pkt_hdr->frame_len)
return -1;
 
-   if (pkt_hdr->p.parsed_layers < LAYER_L3)
-   packet_parse_layer(pkt_hdr, LAYER_L3);
pkt_hdr->p.l3_offset = offset;
+   if (pkt_hdr->p.parsed_layers < LAYER_ALL)
+   pkt_hdr->p.parsed_layers = LAYER_L2;
return 0;
 }
 
@@ -1318,9 +1321,9 @@ int odp_packet_l4_offset_set(odp_packet_t pkt, uint32_t 
offset)
if (offset >= pkt_hdr->frame_len)
return -1;
 
-   if (pkt_hdr->p.parsed_layers < LAYER_L4)
-   packet_parse_layer(pkt_hdr, LAYER_L4);
pkt_hdr->p.l4_offset = offset;
+   if (pkt_hdr->p.parsed_layers < LAYER_ALL)
+   pkt_hdr->p.parsed_layers = LAYER_L3;
return 0;
 }
 
-- 
2.7.4

[lng-odp] [Linaro/odp] 981b77: validation: crypto: check auth options support bef...

[GitHub]

  Branch: refs/heads/master
  Home:   https://github.com/Linaro/odp
  Commit: 981b778400a726121400dd4535f3cc2006d4ac67
  
https://github.com/Linaro/odp/commit/981b778400a726121400dd4535f3cc2006d4ac67
  Author: Matias Elo 
  Date:   2017-02-15 (Wed, 15 Feb 2017)

  Changed paths:
M test/common_plat/validation/api/crypto/odp_crypto_test_inp.c

  Log Message:
  ---
  validation: crypto: check auth options support before running tests

Skip running test if authentication options are not supported.

Signed-off-by: Matias Elo 
Reviewed-by: Nikhil Agarwal 
Reviewed-by: Balakrishna Garapati 
Signed-off-by: Maxim Uvarov 

[lng-odp] [Linaro/odp] 8acb1d: validation: crypto: check cipher options support b...

[GitHub]

  Branch: refs/heads/master
  Home:   https://github.com/Linaro/odp
  Commit: 8acb1d0a0c97d6902ad1b9f975facfc9c88f47b5
  
https://github.com/Linaro/odp/commit/8acb1d0a0c97d6902ad1b9f975facfc9c88f47b5
  Author: Matias Elo 
  Date:   2017-02-15 (Wed, 15 Feb 2017)

  Changed paths:
M test/common_plat/validation/api/crypto/odp_crypto_test_inp.c

  Log Message:
  ---
  validation: crypto: check cipher options support before running tests

Skip running test if cipher options are not supported.

Signed-off-by: Matias Elo 
Reviewed-by: Nikhil Agarwal 
Reviewed-by: Balakrishna Garapati 
Signed-off-by: Maxim Uvarov 

[lng-odp] [Linaro/odp] af2c8f: validation: crypto: check cipher/auth algorithm su...

[GitHub]

  Branch: refs/heads/master
  Home:   https://github.com/Linaro/odp
  Commit: af2c8ff31853d005aaa13247e3a19e5be7138c97
  
https://github.com/Linaro/odp/commit/af2c8ff31853d005aaa13247e3a19e5be7138c97
  Author: Matias Elo 
  Date:   2017-02-15 (Wed, 15 Feb 2017)

  Changed paths:
M test/common_plat/validation/api/crypto/crypto.c
M test/common_plat/validation/api/crypto/odp_crypto_test_inp.c
M test/common_plat/validation/api/crypto/odp_crypto_test_inp.h

  Log Message:
  ---
  validation: crypto: check cipher/auth algorithm support before use

Skip test if selected cipher/authentication algorithms are not supported.

Signed-off-by: Matias Elo 
Reviewed-by: Nikhil Agarwal 
Reviewed-by: Balakrishna Garapati 
Signed-off-by: Maxim Uvarov 

[lng-odp] [Linaro/odp] 3f9e9b: validation: crypto: fix hw cipher/auth algorithm c...

[GitHub]

  Branch: refs/heads/master
  Home:   https://github.com/Linaro/odp
  Commit: 3f9e9b3f17627e35843eef7e3846a75298259b69
  
https://github.com/Linaro/odp/commit/3f9e9b3f17627e35843eef7e3846a75298259b69
  Author: Matias Elo 
  Date:   2017-02-15 (Wed, 15 Feb 2017)

  Changed paths:
M test/common_plat/validation/api/crypto/odp_crypto_test_inp.c

  Log Message:
  ---
  validation: crypto: fix hw cipher/auth algorithm check

Some algorithms may be implemented using hardware and some using software.
All supported algorithms should be set in capacity.auths /capacity.ciphers
independent of implementation types.

Signed-off-by: Matias Elo 
Reviewed-by: Nikhil Agarwal 
Reviewed-by: Balakrishna Garapati 
Signed-off-by: Maxim Uvarov 

[lng-odp] Merging crypto validation suite fixes to master

[Elo, Matias (Nokia - FI/Espoo)]

This patch set (http://patches.opendataplane.org/patch/7828/) should be merged 
to the master as it includes no API changes. These patches are needed by 
odp-dpdk to enable testing HW accelerated crypto.

Regards,
Matias

Re: [lng-odp] Merging crypto validation suite fixes to master

[Bill Fischofer]

Looks like Krishna gave it his review so it should be good to merge.
Maxim is out today but I suspect he'll get to it either tonight or
tomorrow.

On Wed, Feb 15, 2017 at 9:32 AM, Elo, Matias (Nokia - FI/Espoo)
 wrote:
> This patch set (http://patches.opendataplane.org/patch/7828/) should be 
> merged to the master as it includes no API changes. These patches are needed 
> by odp-dpdk to enable testing HW accelerated crypto.
>
> Regards,
> Matias
>

Re: [lng-odp] measuring time and/or cycles

[Francois Ozog]

what is the conclusion of today's architectural call on the time API?
Is it that we said that we keep odp_cpu_cycle? If yes, we need to be clear
on what it is and what it is not.
The difference of two measures:
- is NOT the the count of cpu cycles actively executing instructions.
- Is it the number of clock cycles applied to based on current frequency
(power management)? if yes, I wonder what it brings as opposed to full PMU
profiling and just odp_time_local profiling.
- the number of clock cycles regardless of cpu frequency (in that case it
is functionally equivalent to odp_time_local)

(see additional answers inline)

FF

On 15 February 2017 at 08:34, Brian Brooks  wrote:

> On 02/14 14:17:04, Bill Fischofer wrote:
> > I thought Petri's arguments this morning were articulate.
>
> Agreed. That is part of why I put such things into a shared document.
>
> > From a
> > performance tuning standpoint cycle count provides a time-independent
> > means of measuring algorithmic efficiency since the number of cycles
> > is simply a function of the ISA and compiler optimization level rather
> > than the specific CPU frequency or power management mode it is in.
>
> [FF] not exactly true as number of cycles to execute one instruction on a
CPU thread (on Intel/Power and on some coming ARM processors) depends on
the activity of the other CPU thread: they both share execution ports.
PCMPESTRI is an instruction that do parallel pattern matching on a 16byte
"string". This instruction takes 2 cycles to execute but has a latency of 7
cycles (you can't launch another instruction PCMPESTRI before 7 cycles). As
both threads share execution ports, if thread A issues a PCMPESTRI, thread
B will have to wait 7 cycles before issuing the instruction. If we
measure CPU_CLK_UNHALTED, this gives the number of cycles the CPU has been
doing things or waiting (for instance):
- RAT_STALLS would give how many of those cycles were consumed WAITING for
resources to be free
- SQ_FULL_STALL_CYCLES would give how many of those cycles are consumed
waiting for the uncore (L3, HomeAgent, Memory Controller...).


Yes, the number of CPU cycles it takes to execute a snippet of software
> (perhaps a LOT of times, and then the average is calculated) is dependent
> on such factors such as how the compiler translates C code into assembly
> code for a target ISA and how that assembly code is executed on a specific
> processor that implements some version of the ISA.
>
> And if the question is "how many cycles does it take?" and the answer is X
> cycles, then there is a direct relationship between cycles and wall clock
> time iff the core has been running at a fixed frequency.
>
> But... Moore's law won't save us here.
>
> This is just one level of abstraction when it comes to profiling and
> optimizing software. If you are at this level you likely need a lot more
> information from the PMU.


[FF] very true. hence no use for this single counter.


> So, the proposal in the document is to measure
> time as low-overhead as possible so that network protocol implementations
> can make use of it... and, if needed.. can be used to do basic profiling at
> a higher layer to answer such questions as "where is time being spent in
> this algorithm / data structure implementation?"
>
> Answering that question is a prerequisite to diving deeper into utilizing
> PMU data.
>
> Besides a fast timestamp for network protocol implementation use, the rest
> is profiling related. I would not expect profiling stuff to exist in a
> data plane abstraction layer, and I would expect the fast timestamp stuff
> to be a part of the Time component. Underneith the hood, it utilizes the
> CPU
> timer as specified by the ISA, and if not available, a fallback to POSIX
> clock_gettime. Benchmark data is included in the doc.
>
> > On Tue, Feb 14, 2017 at 2:15 PM, Maxim Uvarov 
> wrote:
> > > if cpu goes for some small time to turbo mode then what you will
> measure
> > > with timer?
>
> That is why CPU ISAs provide a timer that increments at a constant rate
> (fixed frequency). It is not bound to the core's clock frequency. If the
> core's frequency adjusts dynamically you can no longer use cycle counter
> to measure time. If the core's frequency is fixed, it's still not
> guaranteed
> that all cores in the chip run at the same fixed frequency.
>
> It is possible to overclock an x86 CPU, but the Invariant TSC will remain
> at the max frequency. So, on this x86 chip the processor's frequency could
> be
> below or above the Invariant TSC's frequency.
>
> > > btw, is cpu timers have only single propose -  to measure functions
> time?
>
> Yes, it is all about wall clock time. It can be used to measure how much
> time
> some software takes to execute. It can also be used in other ways such as
> to
> check whether it is time to do some work (e.g. send a packet).
>
> > > Maybe we need some tracing functionality for that case?
>
> What would you like to trace?
>
> > > Not just api for current counter

Re: [lng-odp] [API-NEXT PATCH v2 4/4] linux-gen: packet: implement references as copy

[Savolainen, Petri (Nokia - FI/Espoo)]

> -Original Message-
> From: Bill Fischofer [mailto:bill.fischo...@linaro.org]
> Sent: Wednesday, February 15, 2017 4:29 PM
> To: Petri Savolainen 
> Cc: lng-odp-forward 
> Subject: Re: [lng-odp] [API-NEXT PATCH v2 4/4] linux-gen: packet:
> implement references as copy
> 
> I'd still like to see this posted/used for odp-dpdk to give it basic
> functional parity with odp-linux in this area.

I think it's better to upgrade odp-dpdk in order from odp-linux master. There's 
again quite big gap between odp-linux and odp-dpdk. So, first this into 
api-next and master, and then from there to odp-dpdk.

-Petri

Re: [lng-odp] [API-NEXT PATCH v2 0/4] Packet references as copy

[Bill Fischofer]

For Parts 1 and 2 only:

Reviewed-and-tested-by: Bill Fischofer 



On Wed, Feb 15, 2017 at 6:29 AM, Petri Savolainen
 wrote:
> Change packet references API specification to enable implementation as packet
> copy. Remove validation test cases which test for out-of-range inputs.
> Generally, results are undefined for bad or out-of-range inputs. Update
> validation tests with the API change. Greatly simplify packet reference
> implementation by reverting the previous one and adding a packet copy based
> implementation. When the first reference implementation is simple,
> performance and simplicity of the current (single reference) code base is
> mainteined. Performance of multi-reference packets can be upgraded gradually
> as long as performance and code simplicity are not sacrificed.
>
> v2:
>  - highlight in API that zero copy is default behaviour
>  - simplified odp_packet_ref_static() implementation
>
>
> Petri Savolainen (4):
>   api: packet: references may be implemented as copy
>   validation: packet: remove non compatible tests
>   Revert "linux-generic: packet: implement reference apis"
>   linux-gen: packet: implement references as copy
>
>  include/odp/api/spec/packet.h  |  21 +-
>  .../linux-generic/include/odp_packet_internal.h|  85 +--
>  platform/linux-generic/odp_packet.c| 610 
> ++---
>  test/common_plat/validation/api/packet/packet.c| 241 +---
>  4 files changed, 356 insertions(+), 601 deletions(-)
>
> --
> 2.8.1
>

Re: [lng-odp] [API-NEXT PATCH v2 4/4] linux-gen: packet: implement references as copy

[Bill Fischofer]

I'd still like to see this posted/used for odp-dpdk to give it basic
functional parity with odp-linux in this area.

On Wed, Feb 15, 2017 at 6:29 AM, Petri Savolainen
 wrote:
> Implement packet references API as packet copy. This is the
> simplest way to support the API, as other packet functions
> are not affected at all.
>
> Signed-off-by: Petri Savolainen 
> ---
>  platform/linux-generic/odp_packet.c | 74 
> +
>  1 file changed, 74 insertions(+)
>
> diff --git a/platform/linux-generic/odp_packet.c 
> b/platform/linux-generic/odp_packet.c
> index 024f694..9eccb57 100644
> --- a/platform/linux-generic/odp_packet.c
> +++ b/platform/linux-generic/odp_packet.c
> @@ -2221,3 +2221,77 @@ uint64_t odp_packet_seg_to_u64(odp_packet_seg_t hdl)
>  {
> return _odp_pri(hdl);
>  }
> +
> +odp_packet_t odp_packet_ref_static(odp_packet_t pkt)
> +{
> +   return odp_packet_copy(pkt, odp_packet_pool(pkt));
> +}
> +
> +odp_packet_t odp_packet_ref(odp_packet_t pkt, uint32_t offset)
> +{
> +   odp_packet_t new;
> +   int ret;
> +
> +   new = odp_packet_copy(pkt, odp_packet_pool(pkt));
> +
> +   if (new == ODP_PACKET_INVALID) {
> +   ODP_ERR("copy failed\n");
> +   return ODP_PACKET_INVALID;
> +   }
> +
> +   ret = odp_packet_trunc_head(&new, offset, NULL, NULL);
> +
> +   if (ret < 0) {
> +   ODP_ERR("trunk_head failed\n");
> +   odp_packet_free(new);
> +   return ODP_PACKET_INVALID;
> +   }
> +
> +   return new;
> +}
> +
> +odp_packet_t odp_packet_ref_pkt(odp_packet_t pkt, uint32_t offset,
> +   odp_packet_t hdr)
> +{
> +   odp_packet_t new;
> +   int ret;
> +
> +   new = odp_packet_copy(pkt, odp_packet_pool(pkt));
> +
> +   if (new == ODP_PACKET_INVALID) {
> +   ODP_ERR("copy failed\n");
> +   return ODP_PACKET_INVALID;
> +   }
> +
> +   if (offset) {
> +   ret = odp_packet_trunc_head(&new, offset, NULL, NULL);
> +
> +   if (ret < 0) {
> +   ODP_ERR("trunk_head failed\n");
> +   odp_packet_free(new);
> +   return ODP_PACKET_INVALID;
> +   }
> +   }
> +
> +   ret = odp_packet_concat(&hdr, new);
> +
> +   if (ret < 0) {
> +   ODP_ERR("concat failed\n");
> +   odp_packet_free(new);
> +   return ODP_PACKET_INVALID;
> +   }
> +
> +   return hdr;
> +}
> +
> +int odp_packet_has_ref(odp_packet_t pkt)
> +{
> +   (void)pkt;
> +
> +   return 0;
> +}
> +
> +uint32_t odp_packet_unshared_len(odp_packet_t pkt)
> +{
> +   return odp_packet_len(pkt);
> +}
> --
> 2.8.1
>

[lng-odp] [PATCH v2] validation: A wrapper script to run test isolated.

[Ravineet Singh]

The wrapper script; odp_run_app_isolated.sh can be used to run ODP testcases
in a isolated environment. Background noise can also be generated.

Signed-off-by: Ravineet Singh 

v2: moved task-isolation dir to odp/scripts, requested my Maxim Uvarov
---
 scripts/task-isolation/README  |  22 ++
 scripts/task-isolation/isolate-cpu.sh  | 282 +
 scripts/task-isolation/isolate-task.sh | 158 
 .../performance/odp_run_app_isolated.sh| 108 
 4 files changed, 570 insertions(+)
 create mode 100644 scripts/task-isolation/README
 create mode 100755 scripts/task-isolation/isolate-cpu.sh
 create mode 100755 scripts/task-isolation/isolate-task.sh
 create mode 100755 test/linux-generic/performance/odp_run_app_isolated.sh

diff --git a/scripts/task-isolation/README b/scripts/task-isolation/README
new file mode 100644
index 000..cb02056
--- /dev/null
+++ b/scripts/task-isolation/README
@@ -0,0 +1,22 @@
+Helper scripts to check and set CPU isolation and execution of application in
+isolated CPU(s)
+
+Files:
+isolate-cpu.sh   isolates desired CPUs
+isolate-task.sh  uses isolate-cpu.sh to isolate CPUs before running
+ the desired task. It also provides the possibility
+ to trace kernel disturbance on the isolated CPUs.
+
+isolate-cpu.sh checks the kernel configuration and the kernel cmdline to
+ determine if one or several CPUs are isolated, i.e. it checks for;
+ CONFIG_NO_HZ_FULL_ALL=y", and CONFIG_RCU_NOCB_CPU_ALL=y" in the kernel config
+ and rcu_nocbs,nohz_full in the kernel cmdline.
+ If the desired CPU(s) are not inte the above configuration, it warns but 
continues
+ to isolate the CPU(s) as much as possibe. The isolation is accomplished by
+ - Redirecting all IRQ's away from desired isolated cores
+ - Using cset (cpuset) to move all running processes and kernel threads
+   away from desired isolated cores
+
+isolate-task.sh uses isolate-cpu.sh to isolate desired CPU(s). In addition
+ it starts the supplied application on isolated CPU(s) and optionally traces
+ the isolated CPU(s) for kernel interaction.
diff --git a/scripts/task-isolation/isolate-cpu.sh 
b/scripts/task-isolation/isolate-cpu.sh
new file mode 100755
index 000..bd7e41e
--- /dev/null
+++ b/scripts/task-isolation/isolate-cpu.sh
@@ -0,0 +1,282 @@
+#!/bin/bash
+#
+# Copyright (c) 2017, Linaro Limited
+# All rights reserved.
+#
+# SPDX-License-Identifier: BSD-3-Clause
+#
+# Script that passes command line arguments to odp_scheduling after, 
optionally, isolating CPU
+#
+# This script isolates desired CPUS, i.e.
+# - Checks kernel cmdline and kernel config to determine
+#   if the environment is optimised for isolated task execution;
+# - Moves CPU interrupts, kernel threads, tasks etc. away from the targeted 
CPU.
+# *Note* CPU 0 cannot be isolated, i.e minumum 2 CPU's are required.
+
+
+print_usage() {
+echo "$0 [-h] [-a] [-c ] [-l] [-r] [-d]"
+echo
+echo " Isolate CPU(s) from other tasks, kernel threads and IRQs"
+echo " Args:"
+echo "  -h   Print this message"
+echo "  -a   Isolate all CPUs (except CPU 0)"
+echo "  -c   List of CPUs to be isolated."
+echo "  -l   Show isolation proporties"
+echo "  -r   Reset isolation"
+echo "  -d   Show debug printouts"
+echo ""
+echo " Examples:"
+echo "  Isolate all CPU(s) (except 0) "
+echo "  $0 -a"
+echo
+echo "  Isolate CPUs 1-3 "
+echo "  $0 -c 1-3"
+echo
+echo "  Isolate CPUs 1 and 4 "
+echo "  $0 -c 1,4 "
+}
+
+dlog() {
+[ $DEBUG ] && echo "$*"
+}
+
+warn() {
+printf "Warning: $*\n" >&2
+}
+
+die() {
+printf "Error: $*\n" >&2
+exit 1
+}
+
+get_cpu_array() {
+[ $1 ] || die "$FUNCNAME internal error!"
+
+local cpus=""
+IFS=. a=$1; IFS=, a=$a;
+for str in $a; do
+if [[ $str == *[\-]* ]]; then
+str=$(echo $str| sed 's/-/../g')
+str=$(eval echo {$str})
+fi
+
+if [ "$cpus" != "" ]; then
+cpus="$cpus $str"
+else
+cpus=$str
+fi
+done
+
+echo $cpus
+}
+
+##
+# Check kernel config and kernel cmdline for rcu callbacs and no_hz
+# *Note* isolcpu= kernel cmdline option isolates CPUs from SMP balancing
+#If needed, this can be done via cpusets/user/cpuset.sched_load_balance
+##
+check_kernel_config() {
+
+eval $(cat /proc/cmdline | grep -o 'nohz_full=[^ ]*')
+
+local configs="/proc/config.gz /boot/config-$(uname -r) /boot/config "
+dlog "Looking for Kernel configs; $configs "
+for config in $configs; do
+if [ -e $config ]; then
+dlog "Kenel configuration found:$config"
+break
+fi
+done
+
+local all_except_0="1-$(($(getconf _NPROCESSORS_ONLN) - 1))"
+if [ $config ]; then
+nohz_full=$(zgrep "CONFIG_NO_HZ_FULL_ALL=y" $config  2>/dev/null) && 
nohz_full=$all_except_0
+ 

[lng-odp] [API-NEXT PATCH v2 3/4] Revert "linux-generic: packet: implement reference apis"

[Petri Savolainen]

This reverts commit 22b3986fea090986625f3255d57b64de35bbc475.
---
 .../linux-generic/include/odp_packet_internal.h|  85 +---
 platform/linux-generic/odp_packet.c| 536 -
 2 files changed, 107 insertions(+), 514 deletions(-)

diff --git a/platform/linux-generic/include/odp_packet_internal.h 
b/platform/linux-generic/include/odp_packet_internal.h
index 42df5ac..e3ada5c 100644
--- a/platform/linux-generic/include/odp_packet_internal.h
+++ b/platform/linux-generic/include/odp_packet_internal.h
@@ -19,7 +19,6 @@ extern "C" {
 
 #include 
 #include 
-#include 
 #include 
 #include 
 #include 
@@ -169,7 +168,7 @@ typedef struct {
  * packet_init(). Because of this any new fields added must be reviewed for
  * initialization requirements.
  */
-typedef struct odp_packet_hdr_t {
+typedef struct {
/* common buffer header */
odp_buffer_hdr_t buf_hdr;
 
@@ -185,13 +184,6 @@ typedef struct odp_packet_hdr_t {
uint32_t headroom;
uint32_t tailroom;
 
-   /* Fields used to support packet references */
-   uint32_t unshared_len;
-   struct odp_packet_hdr_t *ref_hdr;
-   uint32_t ref_offset;
-   uint32_t ref_len;
-   odp_atomic_u32_t ref_count;
-
/*
 * Members below are not initialized by packet_init()
 */
@@ -220,55 +212,6 @@ static inline odp_packet_hdr_t 
*odp_packet_hdr(odp_packet_t pkt)
return (odp_packet_hdr_t *)buf_hdl_to_hdr((odp_buffer_t)pkt);
 }
 
-static inline odp_packet_hdr_t *odp_packet_last_hdr(odp_packet_t pkt,
-   uint32_t *offset)
-{
-   odp_packet_hdr_t *pkt_hdr = odp_packet_hdr(pkt);
-   odp_packet_hdr_t *prev_hdr = pkt_hdr;
-   uint32_t ref_offset = 0;
-
-   while (pkt_hdr->ref_hdr) {
-   ref_offset = pkt_hdr->ref_offset;
-   prev_hdr   = pkt_hdr;
-   pkt_hdr= pkt_hdr->ref_hdr;
-   }
-
-   if (offset) {
-   if (prev_hdr != pkt_hdr)
-   ref_offset += pkt_hdr->frame_len - prev_hdr->ref_len;
-   *offset = ref_offset;
-   }
-
-   return pkt_hdr;
-}
-
-static inline odp_packet_hdr_t *odp_packet_prev_hdr(odp_packet_hdr_t *pkt_hdr,
-   odp_packet_hdr_t *cur_hdr,
-   uint32_t *offset)
-{
-   uint32_t ref_offset = 0;
-   odp_packet_hdr_t *prev_hdr = pkt_hdr;
-
-   while (pkt_hdr->ref_hdr != cur_hdr) {
-   ref_offset = pkt_hdr->ref_offset;
-   prev_hdr   = pkt_hdr;
-   pkt_hdr= pkt_hdr->ref_hdr;
-   }
-
-   if (offset) {
-   if (prev_hdr != pkt_hdr)
-   ref_offset += pkt_hdr->frame_len - prev_hdr->ref_len;
-   *offset = ref_offset;
-   }
-
-   return pkt_hdr;
-}
-
-static inline odp_packet_t _odp_packet_hdl(odp_packet_hdr_t *pkt_hdr)
-{
-   return (odp_packet_t)odp_hdr_to_buf(&pkt_hdr->buf_hdr);
-}
-
 static inline void copy_packet_parser_metadata(odp_packet_hdr_t *src_hdr,
   odp_packet_hdr_t *dst_hdr)
 {
@@ -291,41 +234,17 @@ static inline void pull_tail(odp_packet_hdr_t *pkt_hdr, 
uint32_t len)
 
pkt_hdr->tailroom  += len;
pkt_hdr->frame_len -= len;
-   pkt_hdr->unshared_len -= len;
pkt_hdr->buf_hdr.seg[last].len -= len;
 }
 
 static inline uint32_t packet_len(odp_packet_hdr_t *pkt_hdr)
 {
-   uint32_t pkt_len = 0;
-   uint32_t offset  = 0;
-
-   do {
-   pkt_len += pkt_hdr->frame_len - offset;
-   offset   = pkt_hdr->ref_offset;
-   if (pkt_hdr->ref_hdr)
-   offset += (pkt_hdr->ref_hdr->frame_len -
-  pkt_hdr->ref_len);
-   pkt_hdr  = pkt_hdr->ref_hdr;
-   } while (pkt_hdr);
-
-   return pkt_len;
-}
-
-static inline uint32_t packet_ref_count(odp_packet_hdr_t *pkt_hdr)
-{
-   return odp_atomic_load_u32(&pkt_hdr->ref_count);
-}
-
-static inline void packet_ref_count_set(odp_packet_hdr_t *pkt_hdr, uint32_t n)
-{
-   odp_atomic_init_u32(&pkt_hdr->ref_count, n);
+   return pkt_hdr->frame_len;
 }
 
 static inline void packet_set_len(odp_packet_hdr_t *pkt_hdr, uint32_t len)
 {
pkt_hdr->frame_len = len;
-   pkt_hdr->unshared_len = len;
 }
 
 static inline int packet_parse_l2_not_done(packet_parser_t *prs)
diff --git a/platform/linux-generic/odp_packet.c 
b/platform/linux-generic/odp_packet.c
index 3aadc4d..024f694 100644
--- a/platform/linux-generic/odp_packet.c
+++ b/platform/linux-generic/odp_packet.c
@@ -33,24 +33,13 @@ static inline odp_buffer_t buffer_handle(odp_packet_hdr_t 
*pkt_hdr)
return pkt_hdr->buf_hdr.handle.handle;
 }
 
-static inline uint32_t packet_ref_inc(odp_packet_hdr_t *pkt_hdr)
-{
-   return odp_atomic_fetch_inc_u32(&pkt_hdr->ref_count);
-}
-
-static inline uint32_t packet_ref_dec

[lng-odp] [API-NEXT PATCH v2 2/4] validation: packet: remove non compatible tests

[Petri Savolainen]

Tests for bad inputs are not compatible to the spec.
Out-of-range values cause undefined results and must not
be tested in validation suite.

Remove reference checks that do not comply anymore to
the new odp_packet_has_ref() specification.

Signed-off-by: Petri Savolainen 
---
 test/common_plat/validation/api/packet/packet.c | 241 
 1 file changed, 161 insertions(+), 80 deletions(-)

diff --git a/test/common_plat/validation/api/packet/packet.c 
b/test/common_plat/validation/api/packet/packet.c
index f64c599..900c426 100644
--- a/test/common_plat/validation/api/packet/packet.c
+++ b/test/common_plat/validation/api/packet/packet.c
@@ -1877,46 +1877,70 @@ void packet_test_extend_ref(void)
/* Verify ref lengths */
CU_ASSERT(ref != ODP_PACKET_INVALID);
CU_ASSERT(odp_packet_len(ref) == max_len - 200);
-   CU_ASSERT(odp_packet_unshared_len(ref) == 0);
+   if (odp_packet_has_ref(ref) == 1) {
+   CU_ASSERT(odp_packet_unshared_len(ref) == 0);
 
-   /* And ref's affect on max_pkt */
-   CU_ASSERT(odp_packet_has_ref(max_pkt) == 1);
-   CU_ASSERT(odp_packet_unshared_len(max_pkt) == 100);
+   /* And ref's affect on max_pkt */
+   CU_ASSERT(odp_packet_has_ref(max_pkt) == 1);
+   CU_ASSERT(odp_packet_unshared_len(max_pkt) == 100);
+   } else {
+   CU_ASSERT(odp_packet_unshared_len(ref) == odp_packet_len(ref));
+   CU_ASSERT(odp_packet_unshared_len(max_pkt) ==
+ odp_packet_len(max_pkt));
+   }
 
/* Now extend max_pkt and verify effect */
CU_ASSERT(odp_packet_extend_head(&max_pkt, 10, NULL, NULL) >= 0);
CU_ASSERT(odp_packet_len(max_pkt) == max_len - 90);
-   CU_ASSERT(odp_packet_unshared_len(max_pkt) == 110);
-   CU_ASSERT(odp_packet_has_ref(max_pkt) == 1);
+
+   if (odp_packet_has_ref(max_pkt) == 1) {
+   /* CU_ASSERT needs braces */
+   CU_ASSERT(odp_packet_unshared_len(max_pkt) == 110);
+   }
 
/* Extend on max_pkt should not affect ref */
-   CU_ASSERT(odp_packet_has_ref(ref) == 1);
CU_ASSERT(odp_packet_len(ref) == max_len - 200);
-   CU_ASSERT(odp_packet_unshared_len(ref) == 0);
+
+   if (odp_packet_has_ref(ref) == 1) {
+   /* CU_ASSERT needs braces */
+   CU_ASSERT(odp_packet_unshared_len(ref) == 0);
+   }
 
/* Now extend ref and verify effect*/
CU_ASSERT(odp_packet_extend_head(&ref, 20, NULL, NULL) >= 0);
CU_ASSERT(odp_packet_len(ref) == max_len - 180);
-   CU_ASSERT(odp_packet_unshared_len(ref) == 20);
+
+   if (odp_packet_has_ref(ref) == 1) {
+   /* CU_ASSERT needs braces */
+   CU_ASSERT(odp_packet_unshared_len(ref) == 20);
+   }
 
/* Extend on ref should not affect max_pkt */
CU_ASSERT(odp_packet_len(max_pkt) == max_len - 90);
-   CU_ASSERT(odp_packet_unshared_len(max_pkt) == 110);
-   CU_ASSERT(odp_packet_has_ref(max_pkt) == 1);
+
+   if (odp_packet_has_ref(max_pkt) == 1) {
+   /* CU_ASSERT needs braces */
+   CU_ASSERT(odp_packet_unshared_len(max_pkt) == 110);
+   }
 
/* Trunc max_pkt of all unshared len */
-   CU_ASSERT(odp_packet_trunc_head(&max_pkt,
-   odp_packet_unshared_len(max_pkt),
-   NULL, NULL) >= 0);
+   CU_ASSERT(odp_packet_trunc_head(&max_pkt, 110, NULL, NULL) >= 0);
 
/* Verify effect on max_pkt */
CU_ASSERT(odp_packet_len(max_pkt) == max_len - 200);
-   CU_ASSERT(odp_packet_unshared_len(max_pkt) == 0);
-   CU_ASSERT(odp_packet_has_ref(max_pkt) == 1);
+
+   if (odp_packet_has_ref(max_pkt) == 1) {
+   /* CU_ASSERT needs braces */
+   CU_ASSERT(odp_packet_unshared_len(max_pkt) == 0);
+   }
 
/* Verify that ref is unchanged */
CU_ASSERT(odp_packet_len(ref) == max_len - 180);
-   CU_ASSERT(odp_packet_unshared_len(ref) == 20);
+
+   if (odp_packet_has_ref(ref) == 1) {
+   /* CU_ASSERT needs braces */
+   CU_ASSERT(odp_packet_unshared_len(ref) == 20);
+   }
 
/* Free ref and verify that max_pkt is back to being unreferenced */
odp_packet_free(ref);
@@ -2089,37 +2113,42 @@ void packet_test_ref(void)
  odp_packet_unshared_len(hdr_pkt[i]));
}
 
-   /* Attempt an invalid ref */
-   refhdr_pkt[0] = odp_packet_ref_pkt(base_pkt, pkt_len, hdr_pkt[0]);
-   CU_ASSERT(refhdr_pkt[0] == ODP_PACKET_INVALID);
-   CU_ASSERT(odp_packet_has_ref(hdr_pkt[0]) == 0);
-   CU_ASSERT(odp_packet_has_ref(base_pkt) == 0);
-
-   /* We can't ref to ourselves */
-   refhdr_pkt[0] = odp_packet_ref_pkt(hdr_pkt[0], 0, hdr_pkt[0]);
-   CU_ASSERT(refhdr_pkt[0] == ODP_PACKET_INVALID);
-   CU_ASSERT(odp_packet_has_ref(hdr_pkt[0]) == 0);
-
-   /* Now create a co

[lng-odp] [API-NEXT PATCH v2 0/4] Packet references as copy

[Petri Savolainen]

Change packet references API specification to enable implementation as packet 
copy. Remove validation test cases which test for out-of-range inputs. 
Generally, results are undefined for bad or out-of-range inputs. Update 
validation tests with the API change. Greatly simplify packet reference 
implementation by reverting the previous one and adding a packet copy based 
implementation. When the first reference implementation is simple, 
performance and simplicity of the current (single reference) code base is 
mainteined. Performance of multi-reference packets can be upgraded gradually 
as long as performance and code simplicity are not sacrificed.

v2:
 - highlight in API that zero copy is default behaviour
 - simplified odp_packet_ref_static() implementation


Petri Savolainen (4):
  api: packet: references may be implemented as copy
  validation: packet: remove non compatible tests
  Revert "linux-generic: packet: implement reference apis"
  linux-gen: packet: implement references as copy

 include/odp/api/spec/packet.h  |  21 +-
 .../linux-generic/include/odp_packet_internal.h|  85 +--
 platform/linux-generic/odp_packet.c| 610 ++---
 test/common_plat/validation/api/packet/packet.c| 241 +---
 4 files changed, 356 insertions(+), 601 deletions(-)

-- 
2.8.1

[lng-odp] [API-NEXT PATCH v2 1/4] api: packet: references may be implemented as copy

[Petri Savolainen]

Some implementations may implement new references as packet copy.
odp_packet_has_ref() will return 0 for copies, since those are
unique packets.

Signed-off-by: Petri Savolainen 
---
 include/odp/api/spec/packet.h | 21 ++---
 1 file changed, 14 insertions(+), 7 deletions(-)

diff --git a/include/odp/api/spec/packet.h b/include/odp/api/spec/packet.h
index b6450c1..92c35ae 100644
--- a/include/odp/api/spec/packet.h
+++ b/include/odp/api/spec/packet.h
@@ -892,9 +892,6 @@ odp_packet_t odp_packet_ref_static(odp_packet_t pkt);
  * dynamic references must not be mixed. Results are undefined if these
  * restrictions are not observed.
  *
- * odp_packet_unshared_len() may be used to determine the number of bytes
- * starting at offset zero that are unique to a packet handle.
- *
  * The packet handle 'pkt' may itself by a (dynamic) reference to a packet.
  *
  * If the caller does not intend to modify either the packet or the new
@@ -952,8 +949,9 @@ odp_packet_t odp_packet_ref_pkt(odp_packet_t pkt, uint32_t 
offset,
  * When a packet has multiple references, packet data is divided into two
  * parts: unshared and shared. The unshared part always precedes the shared
  * part. This call returns number of bytes in the unshared part.  When a
- * packet has only a single reference, all packet data is unshared and
- * unshared length equals the packet length (odp_packet_len()).
+ * packet has only a single reference (see odp_packet_has_ref()), all packet
+ * data is unshared and unshared length equals the packet length
+ * (odp_packet_len()).
  *
  * Application may modify only the unshared part, the rest of the packet data
  * must be treated as read only.
@@ -967,8 +965,17 @@ uint32_t odp_packet_unshared_len(odp_packet_t pkt);
 /**
  * Test if packet has multiple references
  *
- * A packet that has multiple references shares data and possibly metadata
- * with other packets. Shared part must be treated as read only.
+ * A packet that has multiple references share data with other packets. In case
+ * of a static reference it also shares metadata. Shared parts must be treated
+ * as read only.
+ *
+ * New references are created with odp_packet_ref_static(), odp_packet_ref() 
and
+ * odp_packet_ref_pkt() calls. The intent of multiple references is to avoid
+ * packet copies, however some implementations may do a packet copy for some of
+ * the calls. If a copy is done, the new reference is actually a new, unique
+ * packet and this function returns '0' for it. When a real reference is
+ * created (instead of a copy), this function returns '1' for both packets
+ * (the original packet and the new reference).
  *
  * @param pkt Packet handle
  *
-- 
2.8.1

[lng-odp] [API-NEXT PATCH v2 4/4] linux-gen: packet: implement references as copy

[Petri Savolainen]

Implement packet references API as packet copy. This is the
simplest way to support the API, as other packet functions
are not affected at all.

Signed-off-by: Petri Savolainen 
---
 platform/linux-generic/odp_packet.c | 74 +
 1 file changed, 74 insertions(+)

diff --git a/platform/linux-generic/odp_packet.c 
b/platform/linux-generic/odp_packet.c
index 024f694..9eccb57 100644
--- a/platform/linux-generic/odp_packet.c
+++ b/platform/linux-generic/odp_packet.c
@@ -2221,3 +2221,77 @@ uint64_t odp_packet_seg_to_u64(odp_packet_seg_t hdl)
 {
return _odp_pri(hdl);
 }
+
+odp_packet_t odp_packet_ref_static(odp_packet_t pkt)
+{
+   return odp_packet_copy(pkt, odp_packet_pool(pkt));
+}
+
+odp_packet_t odp_packet_ref(odp_packet_t pkt, uint32_t offset)
+{
+   odp_packet_t new;
+   int ret;
+
+   new = odp_packet_copy(pkt, odp_packet_pool(pkt));
+
+   if (new == ODP_PACKET_INVALID) {
+   ODP_ERR("copy failed\n");
+   return ODP_PACKET_INVALID;
+   }
+
+   ret = odp_packet_trunc_head(&new, offset, NULL, NULL);
+
+   if (ret < 0) {
+   ODP_ERR("trunk_head failed\n");
+   odp_packet_free(new);
+   return ODP_PACKET_INVALID;
+   }
+
+   return new;
+}
+
+odp_packet_t odp_packet_ref_pkt(odp_packet_t pkt, uint32_t offset,
+   odp_packet_t hdr)
+{
+   odp_packet_t new;
+   int ret;
+
+   new = odp_packet_copy(pkt, odp_packet_pool(pkt));
+
+   if (new == ODP_PACKET_INVALID) {
+   ODP_ERR("copy failed\n");
+   return ODP_PACKET_INVALID;
+   }
+
+   if (offset) {
+   ret = odp_packet_trunc_head(&new, offset, NULL, NULL);
+
+   if (ret < 0) {
+   ODP_ERR("trunk_head failed\n");
+   odp_packet_free(new);
+   return ODP_PACKET_INVALID;
+   }
+   }
+
+   ret = odp_packet_concat(&hdr, new);
+
+   if (ret < 0) {
+   ODP_ERR("concat failed\n");
+   odp_packet_free(new);
+   return ODP_PACKET_INVALID;
+   }
+
+   return hdr;
+}
+
+int odp_packet_has_ref(odp_packet_t pkt)
+{
+   (void)pkt;
+
+   return 0;
+}
+
+uint32_t odp_packet_unshared_len(odp_packet_t pkt)
+{
+   return odp_packet_len(pkt);
+}
-- 
2.8.1

Re: [lng-odp] measuring time and/or cycles

[Savolainen, Petri (Nokia - FI/Espoo)]

 
> That is why CPU ISAs provide a timer that increments at a constant rate
> (fixed frequency). It is not bound to the core's clock frequency. If the
> core's frequency adjusts dynamically you can no longer use cycle counter
> to measure time. If the core's frequency is fixed, it's still not
> guaranteed
> that all cores in the chip run at the same fixed frequency.

In practice all modern CPUs have per CPU cycle counter. That measures CPU clock 
cycles (on that CPU). Frequency or frequency variation does not matter. If a 
function takes in average 10 cycles it takes 10 cycles, regardless if the first 
2 cycles did run at 1 GHz and last 8 at 2 GHz. If you measure *time*, you do 
not know if a better result was due to frequency scaling or code change (less 
instructions, better cache hit rate, etc).



> 
> It is possible to overclock an x86 CPU, but the Invariant TSC will remain
> at the max frequency. So, on this x86 chip the processor's frequency could
> be
> below or above the Invariant TSC's frequency.

This is implementation issue, not an API issue. In practice, invariant TSC 
gives good enough/correct results in busy loops of code. If you need to be 
exact on CPU cycle count: either do not use Invariant TSC (but real CPU cycle 
counter register whatever that is then) or return 0 (could not read cpu cycle 
count).

As said most CPUs have it (for perf, etc tools to work), implementation just 
have to get access to the right counter. 


-Petri

Re: [lng-odp] [PATCH v3 0/8] First ABI files

[Savolainen, Petri (Nokia - FI/Espoo)]

From: Yi He [mailto:yi...@linaro.org] 
Sent: Tuesday, February 14, 2017 3:59 AM
To: Savolainen, Petri (Nokia - FI/Espoo) 
Cc: Brian Brooks ; lng-odp@lists.linaro.org
Subject: Re: [lng-odp] [PATCH v3 0/8] First ABI files

Hi, Petri and Brian

Can I understand this patch series are giving various platforms a chance to 
define their own data structures (like odp_event_t etc) in arch directory, 
instead of sharing the same definitions as previously?

Best Regards, Yi


No, this adds ABI spec(s). One ABI spec cover (potentially) many 
implementations. If an implementation is built in ABI compat mode, it has to 
use definitions of the ABI spec. Otherwise (non-ABI compat mode), it can use 
its own definitions.

-Petri

Re: [lng-odp] [PATCH 1/2] helper: linux: renamed threads_extn to linux helpers

[Savolainen, Petri (Nokia - FI/Espoo)]

> -Original Message-
> From: Christophe Milard [mailto:christophe.mil...@linaro.org]
> Sent: Wednesday, February 15, 2017 11:40 AM
> To: Savolainen, Petri (Nokia - FI/Espoo)  labs.com>
> Cc: Mike Holmes ; lng-odp  o...@lists.linaro.org>
> Subject: Re: [lng-odp] [PATCH 1/2] helper: linux: renamed threads_extn to
> linux helpers
> 
> On 15 February 2017 at 09:34, Savolainen, Petri (Nokia - FI/Espoo)
>  wrote:
> >
> >
> >> -Original Message-
> >> From: Christophe Milard [mailto:christophe.mil...@linaro.org]
> >> Sent: Monday, February 13, 2017 5:41 PM
> >> To: Savolainen, Petri (Nokia - FI/Espoo)  >> labs.com>
> >> Cc: Mike Holmes ; lng-odp  >> o...@lists.linaro.org>
> >> Subject: Re: [lng-odp] [PATCH 1/2] helper: linux: renamed threads_extn
> to
> >> linux helpers
> >>
> >> This is getting very confusing now: why helper/?  do we have
> >> odp/?: shouldn't  these be symetric?
> >
> >
> > Application (like OFP today) which wants to use e.g. Linux pthread
> helpers include:
> >
> > #include 
> > #include 
> >
> > odph_linux_pthread_create()
> > ...
> > odph_linux_pthread_join()
> >
> >
> > It explicitly uses Linux pthreads through a helper function. There's no
> need to provide these helpers and functions on a non-Linux implementation
> (because there is no Linux).
> >
> > Linux helpers are totally different thing than ODP implementation
> internal directory structure.
> 
> Different: Yes. But related:
> We are back, sadly, to my first question: "what is a platform".
> To that, you, Petri, answered: "platform == implementation". And I agree.

Linux helpers use Linux API and ODP API calls. Those are not aware how an 
implementation is split into files and folders. So, what is platform discussion 
does not belong into this thread. This patch fixes the build issue under and 
renames --enable-helper-thread-extn to --enable-helper-linux, which is more 
logical and explicit about linux helpers.


> 
> Now, can we agree that the above platform definition implies:
> different OS => different platform? Or are you saying that a OSE ODP
> and linux ODP are the same implementation???.

Each vendor decide their folder structure. If they re-use ours you can either 
find e.g.

odp/platform/nxp-ls, which includes code for all supported OSes, or

odp/platform/odp-nxp-ls-linux
odp/platform/odp-nxp-ls-ose
odp/platform/odp-nxp-ls-vxworks

I really don't care as long as everything builds and tests run for each OS. I'd 
prefer platform per vendor + SoC family, so that in the *unlikely case* that 
everything is installed into the same directory you would see

odp/platform/linux-generic
odp/platform/odp-dpdk
odp/platform/odp-nxp-ls
odp/platform/odp-cavium-thunder
...

But, as said every vendor organize their repo as they like.



> 
> If we can agree that, for ODP implementation, different OSes means
> different platforms, do you feel comfortable calling these variation
> differently for the helpers?

Repo/directory structure is own by vendor/implementation/platform. Helpers 
depend on APIs. There's no direct link. Helpers are sorted based on the API(s) 
they depend on.


> 
> What if some helper function requires to be different for a given OS?
> Say for instance, some company wants to implement ODP threads using
> libdill or libmill? (on linux). Or some helper function would depend
> on memory available on a given HW?

odp/helper/linux/pthread.h
odp/helper/linux/dill.h
odp/helper/linux/mill.h
odp/helper/ose/interrupt.h
odp/helper/ose/xyz.h


Helper code uses ODP API exactly the same way as application does: it uses 
capability APIs if it's concerned about some capability. Helper code is part of 
application code.



> 
> Yes, ODP implementation and helpers are 2 differents things, but they
> are facing the same issue: They try to implement a given interface
> (the ODP API  for ODP and the helper API for the helpers) on different
> HW, OS, ... I'd say on different platform. I cannot see why helpers
> variants would only be limited to OSes.

No helpers do not have different implementation for different ODP 
implementation.

Even the "abstract threads" implement (today) abstraction over linux pthread 
and process, but not e.g. OSE threads. You'd need to add some #ifdef __OSE 
there to enable OSE support. Maybe first add odp/helper/ose/thread.c and then 
#ifdef abtract thread to call those functions if application (and helpers) are 
built against OSE (instead of Linux).



> 
> If we can agree on the above, we should agree that a common solution
> to tackle the same problem would be nice.
> On the ODP implementation side, we agreed that a platform was just a
> way to diverge for the main delivery.
> I still think it makes sense to have the same approach for the helpers:
> If we deliver ODP implementation "linux-gen" and "ODP-dpdk" we should
> deliver 2 variant of helper for these as well (even if they happen to
> be the same).

No, since helper code should be always the same.

Pthread system calls in Linux work the sam

Re: [lng-odp] [PATCH 1/2] helper: linux: renamed threads_extn to linux helpers

[Christophe Milard]

On 15 February 2017 at 09:34, Savolainen, Petri (Nokia - FI/Espoo)
 wrote:
>
>
>> -Original Message-
>> From: Christophe Milard [mailto:christophe.mil...@linaro.org]
>> Sent: Monday, February 13, 2017 5:41 PM
>> To: Savolainen, Petri (Nokia - FI/Espoo) > labs.com>
>> Cc: Mike Holmes ; lng-odp > o...@lists.linaro.org>
>> Subject: Re: [lng-odp] [PATCH 1/2] helper: linux: renamed threads_extn to
>> linux helpers
>>
>> This is getting very confusing now: why helper/?  do we have
>> odp/?: shouldn't  these be symetric?
>
>
> Application (like OFP today) which wants to use e.g. Linux pthread helpers 
> include:
>
> #include 
> #include 
>
> odph_linux_pthread_create()
> ...
> odph_linux_pthread_join()
>
>
> It explicitly uses Linux pthreads through a helper function. There's no need 
> to provide these helpers and functions on a non-Linux implementation (because 
> there is no Linux).
>
> Linux helpers are totally different thing than ODP implementation internal 
> directory structure.

Different: Yes. But related:
We are back, sadly, to my first question: "what is a platform".
To that, you, Petri, answered: "platform == implementation". And I agree.

Now, can we agree that the above platform definition implies:
different OS => different platform? Or are you saying that a OSE ODP
and linux ODP are the same implementation???.

If we can agree that, for ODP implementation, different OSes means
different platforms, do you feel comfortable calling these variation
differently for the helpers?

What if some helper function requires to be different for a given OS?
Say for instance, some company wants to implement ODP threads using
libdill or libmill? (on linux). Or some helper function would depend
on memory available on a given HW?

Yes, ODP implementation and helpers are 2 differents things, but they
are facing the same issue: They try to implement a given interface
(the ODP API  for ODP and the helper API for the helpers) on different
HW, OS, ... I'd say on different platform. I cannot see why helpers
variants would only be limited to OSes.

If we can agree on the above, we should agree that a common solution
to tackle the same problem would be nice.
On the ODP implementation side, we agreed that a platform was just a
way to diverge for the main delivery.
I still think it makes sense to have the same approach for the helpers:
If we deliver ODP implementation "linux-gen" and "ODP-dpdk" we should
deliver 2 variant of helper for these as well (even if they happen to
be the same).

A constructor willing to have his own ODP (whatever the reason) would
have the framework to diverge: a divergence is a new platform. Both
for helpers and ODP
The interesting question is how do we write the code so that these
divergence would be easy to do and so that maximum code can be reuse.
That is where the next challenge is.  But first we need to agree on
what a platform is and how we handle helpers.

Christophe.

>
>
>>
>> My view is getting clearer and clearer:
>> On the test side, was have made tests for {OS, HW}= {linux/PC} and
>> given the possibility to diverge from these (for any  reason, e.g.
>> difference of HW or OS) with the use of "platform":
>> If a test defined on the common validation test is inappropriate for a
>> ODP developer (whatever the reason), that developer can overwrite this
>> test (or part of it, such as its setup) with an appropriate one on the
>> platform side.
>> Kallray, AFAIK, if using the platform trick for both OS and HW divergence.
>
> If the system under test is not a Linux system. You cannot run linux helper 
> code there and thus you just skip the linux helper tests.
>
>
>>
>> Why not the same approach on the helper side? We give helpers for
>> {linux/PC} and provide a way to redefine these function for those who
>> need to,  on the platform side (e.g. using a weak symbol on the common
>> helpers and letting those who need define the same function as
>> "strong" on the helper platform side?... or any other better trick).
>
> Why a vendor which do not support Linux would rewrite e.g. 
> odph_linux_pthread_create() to emulate Linux pthreads? If the system is not 
> Linux, there is no point to run Linux system calls there and application 
> would not call odph_linux_pthread_create() there.
>
> Remember, helper code is application code. Application would not call Linux 
> system calls in an non-Linux system. So, no need for a non-Linux based ODP 
> implementation to emulate any Linux system calls.
>
>
>>
>> Then, any implementer knows where to place their hacks: on the
>> "platform side". Maybe not best, but at least consistent and clear.
>>
>> That would just give helpers, with different platform (for any reason)
>> implementations.
>>
>> The linux-specific helper would of course not fit on the common helper
>> part as they are linux specific and cannot be implementation on
>> different OS (platform). Once again, I don't understand why we needed
>> to do wrappers around well known Linux system c

Re: [lng-odp] [PATCH 1/5] test: generator: compose sending packets from reference packet plus differences

[Bogdan Pricope]

Hi,

There were multiple small “fixes” required to make the packet valid.
They should go in together and in all branches.

Br,
Bogdan

On 14 February 2017 at 16:38, Nicolas Morey-Chaisemartin
 wrote:
>
>
> Le 02/13/2017 à 01:49 PM, Bogdan Pricope a écrit :
>> Signed-off-by: Bogdan Pricope 
>> ---
>>  example/generator/odp_generator.c | 131 
>> +-
>>  1 file changed, 114 insertions(+), 17 deletions(-)
>>
>> diff --git a/example/generator/odp_generator.c 
>> b/example/generator/odp_generator.c
>> index 8062d87..d1e3ecc 100644
>> --- a/example/generator/odp_generator.c
>> +++ b/example/generator/odp_generator.c
>> @@ -170,21 +170,20 @@ static int scan_ip(char *buf, unsigned int *paddr)
>>  }
>>
>>  /**
>> - * set up an udp packet
>> + * set up an udp packet reference
>>   *
>>   * @param pool Buffer pool to create packet in
>>   *
>>   * @return Handle of created packet
>>   * @retval ODP_PACKET_INVALID  Packet could not be created
>>   */
>> -static odp_packet_t pack_udp_pkt(odp_pool_t pool)
>> +static odp_packet_t setup_udp_pkt_ref(odp_pool_t pool)
>>  {
>>   odp_packet_t pkt;
>>   char *buf;
>>   odph_ethhdr_t *eth;
>>   odph_ipv4hdr_t *ip;
>>   odph_udphdr_t *udp;
>> - unsigned short seq;
>>
>>   pkt = odp_packet_alloc(pool, args->appl.payload + ODPH_UDPHDR_LEN +
>>  ODPH_IPV4HDR_LEN + ODPH_ETHHDR_LEN);
>> @@ -200,8 +199,10 @@ static odp_packet_t pack_udp_pkt(odp_pool_t pool)
>>   memcpy((char *)eth->src.addr, args->appl.srcmac.addr, 
>> ODPH_ETHADDR_LEN);
>>   memcpy((char *)eth->dst.addr, args->appl.dstmac.addr, 
>> ODPH_ETHADDR_LEN);
>>   eth->type = odp_cpu_to_be_16(ODPH_ETHTYPE_IPV4);
>> +
>>   /* ip */
>>   odp_packet_l3_offset_set(pkt, ODPH_ETHHDR_LEN);
>> + odp_packet_has_ipv4_set(pkt, 1);
>>   ip = (odph_ipv4hdr_t *)(buf + ODPH_ETHHDR_LEN);
>>   ip->dst_addr = odp_cpu_to_be_32(args->appl.dstip);
>>   ip->src_addr = odp_cpu_to_be_32(args->appl.srcip);
>> @@ -209,12 +210,13 @@ static odp_packet_t pack_udp_pkt(odp_pool_t pool)
>>   ip->tot_len = odp_cpu_to_be_16(args->appl.payload + ODPH_UDPHDR_LEN +
>>  ODPH_IPV4HDR_LEN);
>>   ip->proto = ODPH_IPPROTO_UDP;
>> - seq = odp_atomic_fetch_add_u64(&counters.seq, 1) % 0x;
>> - ip->id = odp_cpu_to_be_16(seq);
>> + ip->id = 0;
>> + ip->ttl = 64;
>>   ip->chksum = 0;
>> - odph_ipv4_csum_update(pkt);
>> +
>>   /* udp */
>>   odp_packet_l4_offset_set(pkt, ODPH_ETHHDR_LEN + ODPH_IPV4HDR_LEN);
>> + odp_packet_has_udp_set(pkt, 1);
>>   udp = (odph_udphdr_t *)(buf + ODPH_ETHHDR_LEN + ODPH_IPV4HDR_LEN);
>>   udp->src_port = 0;
>>   udp->dst_port = 0;
>> @@ -226,27 +228,60 @@ static odp_packet_t pack_udp_pkt(odp_pool_t pool)
>>  }
>>
> The calls to odp_packet_has_ipv4_set and odp_packet_has_udp_set are actually 
> a bug fix needed to have the proper checksum computations.
> Without these calls, the checksum are set to 0 as all the packet parse flags 
> are 0.
>
> It might be worth splitting this into a specific patch.
> I posted one for monarch without any answer to it, but it's something that 
> should be put in all branches as it's a bug fix.
>
> Nicolas

Re: [lng-odp] [PATCH 1/2] helper: linux: renamed threads_extn to linux helpers

[Savolainen, Petri (Nokia - FI/Espoo)]

> -Original Message-
> From: Christophe Milard [mailto:christophe.mil...@linaro.org]
> Sent: Monday, February 13, 2017 5:41 PM
> To: Savolainen, Petri (Nokia - FI/Espoo)  labs.com>
> Cc: Mike Holmes ; lng-odp  o...@lists.linaro.org>
> Subject: Re: [lng-odp] [PATCH 1/2] helper: linux: renamed threads_extn to
> linux helpers
> 
> This is getting very confusing now: why helper/?  do we have
> odp/?: shouldn't  these be symetric?


Application (like OFP today) which wants to use e.g. Linux pthread helpers 
include:

#include 
#include 

odph_linux_pthread_create()
...
odph_linux_pthread_join()


It explicitly uses Linux pthreads through a helper function. There's no need to 
provide these helpers and functions on a non-Linux implementation (because 
there is no Linux).

Linux helpers are totally different thing than ODP implementation internal 
directory structure.


> 
> My view is getting clearer and clearer:
> On the test side, was have made tests for {OS, HW}= {linux/PC} and
> given the possibility to diverge from these (for any  reason, e.g.
> difference of HW or OS) with the use of "platform":
> If a test defined on the common validation test is inappropriate for a
> ODP developer (whatever the reason), that developer can overwrite this
> test (or part of it, such as its setup) with an appropriate one on the
> platform side.
> Kallray, AFAIK, if using the platform trick for both OS and HW divergence.

If the system under test is not a Linux system. You cannot run linux helper 
code there and thus you just skip the linux helper tests.


> 
> Why not the same approach on the helper side? We give helpers for
> {linux/PC} and provide a way to redefine these function for those who
> need to,  on the platform side (e.g. using a weak symbol on the common
> helpers and letting those who need define the same function as
> "strong" on the helper platform side?... or any other better trick).

Why a vendor which do not support Linux would rewrite e.g. 
odph_linux_pthread_create() to emulate Linux pthreads? If the system is not 
Linux, there is no point to run Linux system calls there and application would 
not call odph_linux_pthread_create() there.

Remember, helper code is application code. Application would not call Linux 
system calls in an non-Linux system. So, no need for a non-Linux based ODP 
implementation to emulate any Linux system calls.


> 
> Then, any implementer knows where to place their hacks: on the
> "platform side". Maybe not best, but at least consistent and clear.
> 
> That would just give helpers, with different platform (for any reason)
> implementations.
> 
> The linux-specific helper would of course not fit on the common helper
> part as they are linux specific and cannot be implementation on
> different OS (platform). Once again, I don't understand why we needed
> to do wrappers around well known Linux system calls: IMHO, helpers
> should contains functions that we expect to be delivered on any
> implementation (OS/HW) helpers, so these linux wrappers don't fit
> there.

Linux helpers can be used in Linux based systems - may be 90 - 95% of all data 
plane systems today support or can support Linux.

Non-Linux systems do not need to run Linux applications (helpers).


> If we really want to keep that (I'd vote against), let's create
> another directory name for what it is: "linux-wrappers"... and
> separate them from function we'd expect to see everywhere (the rest of
> the helpers, even if different platform dependent implementations for
> those might exist)

This patch uses odp/helper/linux/xx.h which is pretty explicit about the 
dependency to Linux. Anything under odp/helper/linux is Linux dependent and 
would not build/run on non-Linux systems. Clear?


-Petri





> 
> Christophe
> 
> On 13 February 2017 at 16:09, Savolainen, Petri (Nokia - FI/Espoo)
>  wrote:
> >
> >
> > From: Mike Holmes [mailto:mike.hol...@linaro.org]
> > Sent: Monday, February 13, 2017 5:02 PM
> > To: Savolainen, Petri (Nokia - FI/Espoo)  labs.com>
> > Cc: lng-odp 
> > Subject: Re: [lng-odp] [PATCH 1/2] helper: linux: renamed threads_extn
> to linux helpers
> >
> >
> >
> > On 13 February 2017 at 09:41, Savolainen, Petri (Nokia - FI/Espoo)
>  wrote:
> >
> >
> > From: Mike Holmes [mailto:mailto:mike.hol...@linaro.org]
> > Sent: Friday, February 10, 2017 5:02 PM
> > To: Petri Savolainen 
> > Cc: lng-odp 
> > Subject: Re: [lng-odp] [PATCH 1/2] helper: linux: renamed threads_extn
> to linux helpers
> >
> >
> >
> > On 3 February 2017 at 06:23, Petri Savolainen
>  wrote:
> > There's no platform specific helpers. Helpers may depend on
> > Linux and make it easier to do common series of Linux system
> > calls. These kind of helpers are grouped into helper/linux
> > directory.
> >
> > Use --enable-helper-linux configuration option to enable
> > support for Li

Re: [lng-odp] Ask a few questions about odp

[Christophe Milard]

On 15 February 2017 at 04:32, ice  wrote:
> HI：
>
> I am from a network security company in China,  We intend to use odp to
> complete a bottom forwarding engine and data channel.
> What I am concerned about before is that in the case of multithreading, the
> crash of the thread will hang all threads of the engine.
>
> I have a few questions want to ask you：
>
> 1. I see that there are several functions in the helper/linux.c, such as:
> odph_linux_process_fork() and odph_linux_process_fork_n(),
> But as you said. the process support is not really being pushed and is
> faultly. And all the code used in the example is a multithreaded
> architecture.
> So odp support for multi-process architecture is not perfect, and is not
> fully tested?

ODP is first an open  API definition. Anyone can do an ODP
implementation on its the hardware of his choice. Today, companies
like cavium, Kallray, among others, develop their own product
according to the ODP specification. The creation of a "concurrent
execution environment" (we call them -sadly- "ODP threads", even if
I'd personally prefer the choice of "ODP tasks" as they would be
processes or threads in most cases) is not, as of today, part of ODP:
It is the OS business. But a given ODP implementation may say it needs
"ODP threads" to be of a specific nature (i.e.: they have to be
pthreads, or they have to be processes, or anything else).
Most implementation I have heard of wants ODP threads to be linux pthreads.

Linaro also provides two specific ODP API implementations for linux,
which we call "odp-linux-generic" and "odp-dpdk". Both work works with
ODPthreads being pthreads. Besides the "linux-generic" implementation
claims to support ODP threads being linux processes (and even a mix of
pthreads and processes). I am not sure about what odp-dpdk claims to
support. But I know for a fact that some functionality both will break
if linux processes are used. I have fixed the issue for some part, but
some other problems remains. There has been relatively low interrest
for these issues, som far.
If you need process support, make your voice heard! write to the list,
join the public calls, contribute, or join Linaro!

The "odph_linux_process_fork*" familly of functions are just wrappers
around linux fork. nothing else. The equivalent set of functions
exists for pthreads.There is no requirements to use them, even when
using ODP.
In my humble opinion, the presence of these function in the helpers is
confusing... and is being discussed. If you feel more confident using
pthread_create() and fork() directely, you should keep doing so.
The odph_odpthreads_create() set of function, however, may be of more
interest: They abstract the ODP thread concept: they will create
whatever the ODP implementation wants as an ODPthread. For
odp-linux-generic, either pthreads or processes, depending on a flag
(as linux-generic claims to support both): This may be of interest for
those who wish to develop applications which would work regardless of
the choices which are made for an ODP thread implementation: Using
these functions will help writing a more mortable ODP app (which would
work on any ODP implementations even  if the latter made different
choices regarding the nature of an "ODP thread".)

> 2. If we use a multithreaded architecture, if one of the threads crashes,
> how does the stability of other threads guarantee?

pthreads are pthreads. No difference with ODP. They are plain linux
things: they share memory, file descriptors... So statbility is what
you would expect with them regardless of ODP.

> 3. If I want to transmit buffer from appA to appB and appC, Can I use odp's
> schedule module to implement it? Or I can only use shm ipc to achieve it?
> If I can use odp schedule to achieve, then how the performance?

If appA, appB, appC are ODPthreads belonging to the same ODP instance,
yes, you can transfer buffers between them. Support for multi
instances (many ODP in the same machine) varies from ODP
implementation to ODP implementation, and communication between
different ODP instances is more limited today (but is being worked
on).
Performance will depends on the chosen ODP implementation and HW, of course.

> 4. If I use odp to transfer data between a variety of app, the performance
> will be how?

not sure how this question differs from the previous one (3)

>
> Do you know the open event machine? What do you think of it and odp
> contrast?

I don't know much about the open event machine: My understanding was
that is was targetting embeded systems only (mostly bare-metal). If
this assumption is true, ODP is more general as it targets both
embedded systems and the VM world. But my knowledge of the open event
machine is too weak to comment more :-)
Maybe some other on the list have comments, there.

Regards,

Christophe.

>
> thank you!
>
>
>
>
>
>
> At 2017-02-14 22:12:06, "Christophe Milard" 
> wrote:
>>Hi,
>>
>>I am not sure about who you are, what you are trying to do, and how
>>you