[lng-odp] [Bug 1337] New: No means of knowing the size of the CPU mask (odp_cpumask_t)

[bugzilla-daemon]

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

Bug ID: 1337
   Summary: No means of knowing the size of the CPU mask
(odp_cpumask_t)
   Product: OpenDataPlane
   Version: 1.0
  Hardware: Other
OS: Linux
Status: UNCONFIRMED
  Severity: enhancement
  Priority: ---
 Component: General ODP API or linux-generic implimentation
  Assignee: lng-odp@lists.linaro.org
  Reporter: christophe.mil...@linaro.org

Overview: No means of knowing the size of the CPU mask (odp_cpumask_t)

The API does not provide a way to know how many CPUs (at most) an odp_cpumask_t
may contain.
the API provides odp_cpu_count() which returns the number of CPU available for
the ODP program.
This is probably not the same as the mask size. If it is, it should be clearly
stated.

-- 
You are receiving this mail because:
You are the assignee for the bug.___
lng-odp mailing list
lng-odp@lists.linaro.org
http://lists.linaro.org/mailman/listinfo/lng-odp

[lng-odp] [PATCH] validation: pktio check return code for pool_destroy

[Maxim Uvarov]

Fix CID 88056 to check return code of pool_destroy.

Signed-off-by: Maxim Uvarov 
---
 test/validation/odp_pktio.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/test/validation/odp_pktio.c b/test/validation/odp_pktio.c
index 6359452..e022c33 100644
--- a/test/validation/odp_pktio.c
+++ b/test/validation/odp_pktio.c
@@ -259,7 +259,7 @@ static odp_pktio_t create_pktio(const char *iface)
 
pool = odp_pool_lookup(pool_name);
if (pool != ODP_POOL_INVALID)
-   odp_pool_destroy(pool);
+   CU_ASSERT(odp_pool_destroy(pool) == 0);
 
pool = odp_pool_create(pool_name, ODP_SHM_NULL, ¶ms);
CU_ASSERT(pool != ODP_POOL_INVALID);
-- 
1.9.1


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

[lng-odp] proto flags set functions

[Radu-Andrei Bulie]

Hi,


Regarding the functions that set the proto flags (e.g  odp_packet_has_xx_set) 
isn't more
suitable not to pass an int value as second parameter,  in fact not to have any 
parameter at all because
howsoever inside the function we have some bit fields, and setting protos would 
mean just shifting
value 1 on different positions (depending on implementation)?
The input param value may not translate
to a correct value so inside,  the implementation will make assumption that 
anyway there is a value of 1 or 0.
What I mean to say is that the value of the second param is not relevant for 
the user (only the result - that will set a specific proto);
He does not need to know a proper value that should be passed to enable a 
specific proto

Regards,

Radu

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

[lng-odp] [Bug 1168] odp_schedule test can create more threads than ODP_CONFIG_MAX_THREADS

[bugzilla-daemon]

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

--- Comment #1 from Ciprian Barbu  ---
The bug was eventually fixed by Petri:
https://git.linaro.org/lng/odp.git/commit/70bd7de7182eaae5c920979e4d25cb512e6fffb8

-- 
You are receiving this mail because:
You are on the CC list for the bug.___
lng-odp mailing list
lng-odp@lists.linaro.org
http://lists.linaro.org/mailman/listinfo/lng-odp

Re: [lng-odp] [PATCH] validation: pktio check return code for pool_destroy

[Stuart Haslam]

On Wed, Mar 11, 2015 at 11:36:25AM +0300, Maxim Uvarov wrote:
> Fix CID 88056 to check return code of pool_destroy.
> 
> Signed-off-by: Maxim Uvarov 

Reviewed-by: Stuart Haslam 

> ---
>  test/validation/odp_pktio.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/test/validation/odp_pktio.c b/test/validation/odp_pktio.c
> index 6359452..e022c33 100644
> --- a/test/validation/odp_pktio.c
> +++ b/test/validation/odp_pktio.c
> @@ -259,7 +259,7 @@ static odp_pktio_t create_pktio(const char *iface)
>  
>   pool = odp_pool_lookup(pool_name);
>   if (pool != ODP_POOL_INVALID)
> - odp_pool_destroy(pool);
> + CU_ASSERT(odp_pool_destroy(pool) == 0);
>  
>   pool = odp_pool_create(pool_name, ODP_SHM_NULL, ¶ms);
>   CU_ASSERT(pool != ODP_POOL_INVALID);
> -- 
> 1.9.1
> 
> 
> ___
> lng-odp mailing list
> lng-odp@lists.linaro.org
> http://lists.linaro.org/mailman/listinfo/lng-odp

-- 
Stuart.

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

Re: [lng-odp] proto flags set functions

[Taras Kondratiuk]

On 03/11/2015 10:37 AM, Radu-Andrei Bulie wrote:

Hi,

Regarding the functions that set the proto flags (e.g
*odp_packet_has_xx_set*) isn’t more

suitable not to pass an int value as second parameter,  in fact not to
have any parameter at all because

howsoever inside the function we have some bit fields, and setting
protos would mean just shifting

value 1 on different positions (depending on implementation)?

The input param value may not translate

to a correct value so inside,  the implementation will make assumption
that anyway there is a value of 1 or 0.

What I mean to say is that the value of the second param is not relevant
for the user (only the result – that will set a specific proto);

He does not need to know a proper value that should be passed to enable
a specific proto


Second parameter should be of type odp_bool_t to make it clear.

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

[lng-odp] [Bug 1340] New: CID 88056: Error handling issues: odp_pktio.c

[bugzilla-daemon]

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

Bug ID: 1340
   Summary: CID 88056:  Error handling issues: odp_pktio.c
   Product: OpenDataPlane
   Version: 1.0
  Hardware: Other
OS: Linux
Status: UNCONFIRMED
  Severity: normal
  Priority: ---
 Component: Packet IO
  Assignee: maxim.uva...@linaro.org
  Reporter: mike.hol...@linaro.org
CC: lng-odp@lists.linaro.org

/test/validation/odp_pktio.c: 262 in create_pktio()
256 params.type= ODP_POOL_PACKET;
257
258 snprintf(pool_name, sizeof(pool_name), "pkt_pool_%s", iface);
259
260 pool = odp_pool_lookup(pool_name);
261 if (pool != ODP_POOL_INVALID)
>>> CID 88056:  Error handling issues  (CHECKED_RETURN)
>>> Calling "odp_pool_destroy" without checking return value (as is done 
>>> elsewhere 17 out of 19 times).
262 odp_pool_destroy(pool);
263
264 pool = odp_pool_create(pool_name, ODP_SHM_NULL, ¶ms);
265 CU_ASSERT(pool != ODP_POOL_INVALID);
266
267 pktio = odp_pktio_open(iface, pool);

-- 
You are receiving this mail because:
You are on the CC list for the bug.___
lng-odp mailing list
lng-odp@lists.linaro.org
http://lists.linaro.org/mailman/listinfo/lng-odp

[lng-odp] ODP_v1.0 @ Regarding adding some file say demo.c and demo.h in platform/linux-genic and platform/linux-genic/include

[Ayushman Kashyap]

Hi

I want to add some file in *platform/linux-generic* and
*platform/linux-genic/include*. But Not able to compile simple case. Please
see the scenario .





I have a file say:

* demo.c: *



#include 

#include 



int get_sys_page_size()

{

int page_size;

page_size = getpagesize();

printf("Page Size : %d\n",page_size);

return 0;

}



*Demo.h : *



#ifndef DEMO_H_

#define DEMO_H_

#ifdef __cplusplus

extern "C" {

#endif

int get_sys_page_size();

#ifdef __cplusplus

}

#endif

#endif



Added demo.c file in :   platform/linux-generic/

Added demo.h file in : platform/linux-generic/include/



Modified *platform/linux-generic/Makefile.am*



${top_srcdir}/platform/linux-generic/include/odp_timer_internal.h \

*++  ${top_srcdir}/platform/linux-generic/include/demo.h \*

  ${top_srcdir}/platform/linux-generic/Makefile.inc



odp_timer.c \

   odp_version.c \

   odp_weak.c \

*++   demo.c*





*./bootstrap*

*./configure*

*Make*





CC   odp_time.lo

  CC   odp_timer.lo

  CC   odp_version.lo

  CC   odp_weak.lo

  CC   demo.lo

demo.c:4:5: error: function declaration isn't a prototype
[-Werror=strict-prototypes]

 int get_sys_page_size()

 ^

demo.c: In function 'get_sys_page_size':

demo.c:4:5: error: old-style function definition
[-Werror=old-style-definition]

demo.c:7:2: error: implicit declaration of function 'getpagesize'
[-Werror=implicit-function-declaration]

  page_size = getpagesize();

  ^

demo.c:7:2: error: nested extern declaration of 'getpagesize'
[-Werror=nested-externs]

cc1: all warnings being treated as errors

make[2]: *** [demo.lo] Error 1

make[2]: Leaving directory `. /ODPV1.0-C1/platform/linux-generic'

make[1]: *** [all-recursive] Error 1

make[1]: Leaving directory `. /ODP-MJ/ODPV1.0-C1/platform'

make: *** [all-recursive] Error 1





If someone help me in this regards, it will be very helpful for me. The
same use cases, I am able to perform over ODPV0.7.



Regards

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

Re: [lng-odp] odp_cpu_count() vs odp_cpumask size

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

CPU IDs are system dependent. So e.g. odp_cpu_count() == 2 would not 
necessarily result a cpumask with cpus 0 and 1 set, but e.g. cpus 7 and 17 set. 
Typically, the mask comes from user/system level (user has decided to run the 
ODP app on cpus 7 and 17).

We may need two new functions:

/**
* @return the max CPU available to the ODP program
*/
int odp_cpu_max(void)

, but still that would not tell which ids between 0 and max are valid, so we 
need also

/**
  *  Output mask of CPUs available to the ODP program
 */
void odp_cpu_mask(odp_cpumask_t *mask)


-Petri


From: lng-odp-boun...@lists.linaro.org 
[mailto:lng-odp-boun...@lists.linaro.org] On Behalf Of ext Christophe Milard
Sent: Tuesday, March 10, 2015 4:22 PM
To: lng-odp@lists.linaro.org
Subject: [lng-odp] odp_cpu_count() vs odp_cpumask size

Hi,
I am writing validation tests for the odp_cpumask functions.
The question is: How many CPUs should the cpu mask be tested for?.
Of course, the answer is implementation dependent, but if we want the tests to 
be non platform dependent, the tests should know the max number of CPU the mask 
could contain.
In the API the function odp_cpu_count(void) is defined. This function "reports 
the number of CPUs available to this ODP program". Is this also the maximum 
number of CPU in the mask, or should the mask also have room for non "odp 
reserved cpus"?
For instance, in a system running 2 linux (control) cpus, and 5 ODP cpus, 
should the mask be tested for 5 or 7 CPUs? If 7 is the answer, how do I get 
this number from the current API...
/Christophe.
___
lng-odp mailing list
lng-odp@lists.linaro.org
http://lists.linaro.org/mailman/listinfo/lng-odp

Re: [lng-odp] odp_cpu_count() vs odp_cpumask size

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

Actually, this is equal to odp_cpu_max():

odp_cpumask_t mask;
odp_cpu_mask(&mask)
odp_cpumask_last(&mask)

So maybe odp_cpu_mask() is only new thing we need.

-Petri

From: lng-odp-boun...@lists.linaro.org 
[mailto:lng-odp-boun...@lists.linaro.org] On Behalf Of ext Savolainen, Petri 
(Nokia - FI/Espoo)
Sent: Wednesday, March 11, 2015 2:03 PM
To: ext Christophe Milard; lng-odp@lists.linaro.org
Subject: Re: [lng-odp] odp_cpu_count() vs odp_cpumask size

CPU IDs are system dependent. So e.g. odp_cpu_count() == 2 would not 
necessarily result a cpumask with cpus 0 and 1 set, but e.g. cpus 7 and 17 set. 
Typically, the mask comes from user/system level (user has decided to run the 
ODP app on cpus 7 and 17).

We may need two new functions:

/**
* @return the max CPU available to the ODP program
*/
int odp_cpu_max(void)

, but still that would not tell which ids between 0 and max are valid, so we 
need also

/**
  *  Output mask of CPUs available to the ODP program
 */
void odp_cpu_mask(odp_cpumask_t *mask)


-Petri


From: lng-odp-boun...@lists.linaro.org 
[mailto:lng-odp-boun...@lists.linaro.org] On Behalf Of ext Christophe Milard
Sent: Tuesday, March 10, 2015 4:22 PM
To: lng-odp@lists.linaro.org
Subject: [lng-odp] odp_cpu_count() vs odp_cpumask size

Hi,
I am writing validation tests for the odp_cpumask functions.
The question is: How many CPUs should the cpu mask be tested for?.
Of course, the answer is implementation dependent, but if we want the tests to 
be non platform dependent, the tests should know the max number of CPU the mask 
could contain.
In the API the function odp_cpu_count(void) is defined. This function "reports 
the number of CPUs available to this ODP program". Is this also the maximum 
number of CPU in the mask, or should the mask also have room for non "odp 
reserved cpus"?
For instance, in a system running 2 linux (control) cpus, and 5 ODP cpus, 
should the mask be tested for 5 or 7 CPUs? If 7 is the answer, how do I get 
this number from the current API...
/Christophe.
___
lng-odp mailing list
lng-odp@lists.linaro.org
http://lists.linaro.org/mailman/listinfo/lng-odp

Re: [lng-odp] [PATCH] example: odp_ipsec: fix missing definition for ifreq

[Ola Liljedahl]

On 11 March 2015 at 10:58, Ciprian Barbu  wrote:

> On Tue, Mar 10, 2015 at 7:19 PM, Ola Liljedahl 
> wrote:
> > On 10 March 2015 at 17:11, Ciprian Barbu 
> wrote:
> >> On Tue, Mar 10, 2015 at 6:06 PM, Ciprian Barbu <
> ciprian.ba...@linaro.org> wrote:
> >>> On Tue, Mar 10, 2015 at 4:31 PM, Maxim Uvarov 
> wrote:
>  On 03/10/15 17:16, Ciprian Barbu wrote:
> >
> > On Tue, Mar 10, 2015 at 1:33 PM, Maxim Uvarov <
> maxim.uva...@linaro.org>
> > wrote:
> >>
> >> Please also specify your env.  I can not reproduce it with
> >> ./cross-compile-test.sh
> >
> > I added some info in the bug entry. Were you able to reproduce like
> that?
> 
> 
>  I see that in one includes in net/if.h that structure is under ifdef
>  __USE_MISC, in other includes there is no such ifdef.
>  Looks like you have different version of headers. For linux/if.h
> there is no
>  ifdef for both cases. I think you patch is good to go,
>  tested it on my toolchains (compilation only).
> >>>
> >>> This might be a problem with Ubuntu 13.10, I tested on an Ubuntu 14.04
> >>> and it works.
> >>>
> >>> The whole problem comes from the ioctl command that requires struct
> >>> ifreq. From this man page (http://linux.die.net/man/7/netdevice) it
> >>> looks like it should be enough to include  and
> >>> . I also found that including linux/if.h is usually done by
> >>> code for kernel, so that might actually not be a good idea.
> >>>
> >>> Strange though, adding  doesn't fix compiling on my
> >>> environment. Does anyone else run Ubuntu 13.10? maybe I screwed my
> >>> headers somehow installing some packages ...
> >>
> >> I also found this:
> >>
> http://stackoverflow.com/questions/10433982/why-does-c99-complain-about-storage-sizes
> >> which says the problem is in fact with -std=c99. Strange though how it
> >> only behaves bad on my Ubuntu 13.10. Would still be good if someone
> >> else checks on their env ...
> > The way I interpret this is that when you specify a specific C
> > standard (e.g. C99), by default you will only get access to those
> > library features that are included in that standard. If you need to go
> > outside of the standard, you need to ask for it specifically. E.g.
> > define _BSD_SOURCE in this case.
> >
> > But this should be independent of Ubuntu releases. I can imagine
> > different libc (glibc) versions may do this differently though so this
> > could be that cause of differences in behavior between 13.10 and e.g.
> > 14.04 (which I use).
>
> Ola, we used to make use of _BSD_SOURCE to get the extra features, but
> the use of this macro has been deprecated since glib 2.20:
> http://man7.org/linux/man-pages/man7/feature_test_macros.7.html.
> Instead of defining _BSD_SOURCE the user must rely on _DEFAULT_SOURCE,
> which exists since 2.19.
>
> Looking at my glib version I get:
> GNU C Library (Ubuntu EGLIBC 2.17-93ubuntu4) stable release version
> 2.17, by Roland McGrath et al.
>
> So the problem in my case is that I have an old glibc version and the
> way we compile ODP does not cope with that. Other users will be
> affected. Either we do something about it (I haven't found a way to
> check the glibc version at compile time yet) or document that ODP will
> not support glibc older than 2.19 (where _DEFAULT_SOURCE exists).
>
Or we add some code to automake/configure to detect which (g)libc version
you are using and then use the appropriate define (_BSD_SOURCE for <= 2.17,
_DEFAULT_SOURCE for >= 2.19, don't know about 2.18).

Wouldn't this be the "right" way of solving problems like this? This might
not be the only time we have such a version problem.

-- Ola


> >
> > olli@macmini:~$ gcc --version
> > gcc (Ubuntu 4.8.2-19ubuntu1) 4.8.2
> > Copyright (C) 2013 Free Software Foundation, Inc.
> > This is free software; see the source for copying conditions.  There is
> NO
> > warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR
> PURPOSE.
> > olli@macmini:~$ dpkg -S /usr/include/net/if.h
> > libc6-dev:amd64, libc6-dev:i386: /usr/include/net/if.h
> > olli@macmini:~$ dpkg -s libc6-dev | grep Version
> > Version: 2.19-0ubuntu6.6
> >
> >
> >>
> >> /Ciprian
> >>
> >>>
> 
>  Maxim.
> 
> 
> 
> >> Maxim.
> >>
> >>
> >> On 03/10/15 14:14, Maxim Uvarov wrote:
> >>>
> >>> Please add patch description and put bug link in the bottom of it.
> Like
> >>> other git commits do.
> >>>
> >>> Thanks,
> >>> Maxim.
> >>>
> >>> On 03/10/15 12:47, Ciprian Barbu wrote:
> 
>  Signed-off-by: Ciprian Barbu 
>  ---
>  fix for https://bugs.linaro.org/show_bug.cgi?id=1330
> 
> example/ipsec/odp_ipsec.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
> 
>  diff --git a/example/ipsec/odp_ipsec.c b/example/ipsec/odp_ipsec.c
>  index 98160ba..286b9f0 100644
>  --- a/example/ipsec/odp_ipsec.c
>  +++ b/example/ipsec/odp_

[lng-odp] [PATCH 2/3] linux-geneirc: timer: remove ifdefs inside function from timer_cancel

[Maxim Uvarov]

Using ifdefs inside function make it hard to read. No functional changes.

Signed-off-by: Maxim Uvarov 
---
 platform/linux-generic/odp_timer.c | 19 ---
 1 file changed, 16 insertions(+), 3 deletions(-)

diff --git a/platform/linux-generic/odp_timer.c 
b/platform/linux-generic/odp_timer.c
index 8b04bd2..33c2758 100644
--- a/platform/linux-generic/odp_timer.c
+++ b/platform/linux-generic/odp_timer.c
@@ -467,15 +467,16 @@ static bool timer_reset(uint32_t idx,
return success;
 }
 
+
+#ifdef ODP_ATOMIC_U128
 static odp_buffer_t timer_cancel(odp_timer_pool *tp,
uint32_t idx,
uint64_t new_state)
 {
tick_buf_t *tb = &tp->tick_buf[idx];
odp_buffer_t old_buf;
-
-#ifdef ODP_ATOMIC_U128
tick_buf_t new, old;
+
/* Update the timer state (e.g. cancel the current timeout) */
new.exp_tck.v = new_state;
/* Swap out the old buffer */
@@ -485,7 +486,18 @@ static odp_buffer_t timer_cancel(odp_timer_pool *tp,
 (_uint128_t *)&new, (_uint128_t *)&old,
 _ODP_MEMMODEL_RLX);
old_buf = old.tmo_buf;
+
+   /* Return the old buffer */
+   return old_buf;
+}
 #else
+static odp_buffer_t timer_cancel(odp_timer_pool *tp,
+   uint32_t idx,
+   uint64_t new_state)
+{
+   tick_buf_t *tb = &tp->tick_buf[idx];
+   odp_buffer_t old_buf;
+
/* Take a related lock */
while (_odp_atomic_flag_tas(IDX2LOCK(idx)))
/* While lock is taken, spin using relaxed loads */
@@ -501,10 +513,11 @@ static odp_buffer_t timer_cancel(odp_timer_pool *tp,
 
/* Release the lock */
_odp_atomic_flag_clear(IDX2LOCK(idx));
-#endif
+
/* Return the old buffer */
return old_buf;
 }
+#endif
 
 static int post_timeout_to_queue(odp_queue_t queue, odp_buffer_t tmo_buf,
 uint64_t exp_tck)
-- 
1.9.1


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

[lng-odp] [PATCH 1/3] linux-generic: timer simplify timer_expire

[Maxim Uvarov]

Current timer_expire() function implementation for linux-generic is
too long and complex. This patch makes it simple to read. No functional
changes.

Signed-off-by: Maxim Uvarov 
---
 platform/linux-generic/odp_timer.c | 102 ++---
 1 file changed, 61 insertions(+), 41 deletions(-)

diff --git a/platform/linux-generic/odp_timer.c 
b/platform/linux-generic/odp_timer.c
index 6b48d2e..8b04bd2 100644
--- a/platform/linux-generic/odp_timer.c
+++ b/platform/linux-generic/odp_timer.c
@@ -506,20 +506,48 @@ static odp_buffer_t timer_cancel(odp_timer_pool *tp,
return old_buf;
 }
 
+static int post_timeout_to_queue(odp_queue_t queue, odp_buffer_t tmo_buf,
+uint64_t exp_tck)
+{
+   odp_timeout_hdr_t *tmo_hdr;
+   int rc;
+
+   if (odp_unlikely(tmo_buf == ODP_BUFFER_INVALID))
+   return 0;
+
+   /* Fill in expiration tick if timeout event */
+   if (_odp_buffer_type(tmo_buf) == ODP_EVENT_TIMEOUT) {
+   /* Convert from buffer to timeout hdr */
+   tmo_hdr = timeout_hdr_from_buf(tmo_buf);
+   tmo_hdr->expiration = exp_tck;
+   /* timer and user_ptr fields filled in when timer was set */
+   }
+   /* Else ignore events of other types.
+* Post the timeout to the destination queue. */
+   rc = odp_queue_enq(queue, odp_buffer_to_event(tmo_buf));
+   if (odp_unlikely(rc != 0))
+   ODP_ABORT("Failed to enqueue timeout buffer (%d)\n",
+   rc);
+   return 1;
+}
+
+
+#ifdef ODP_ATOMIC_U128
 static unsigned timer_expire(odp_timer_pool *tp, uint32_t idx, uint64_t tick)
 {
odp_timer *tim = &tp->timers[idx];
tick_buf_t *tb = &tp->tick_buf[idx];
odp_buffer_t tmo_buf = ODP_BUFFER_INVALID;
uint64_t exp_tck;
-#ifdef ODP_ATOMIC_U128
+   int rc;
+   tick_buf_t new, old;
+
/* Atomic re-read for correctness */
exp_tck = _odp_atomic_u64_load_mm(&tb->exp_tck, _ODP_MEMMODEL_RLX);
/* Re-check exp_tck */
if (odp_likely(exp_tck <= tick)) {
/* Attempt to grab timeout buffer, replace with inactive timer
 * and invalid buffer */
-   tick_buf_t new, old;
old.exp_tck.v = exp_tck;
old.tmo_buf = tb->tmo_buf;
TB_SET_PAD(old);
@@ -529,65 +557,57 @@ static unsigned timer_expire(odp_timer_pool *tp, uint32_t 
idx, uint64_t tick)
new.exp_tck.v = exp_tck | TMO_INACTIVE;
new.tmo_buf = ODP_BUFFER_INVALID;
TB_SET_PAD(new);
-   int succ = _odp_atomic_u128_cmp_xchg_mm(
+   rc = _odp_atomic_u128_cmp_xchg_mm(
(_odp_atomic_u128_t *)tb,
(_uint128_t *)&old, (_uint128_t *)&new,
_ODP_MEMMODEL_RLS, _ODP_MEMMODEL_RLX);
-   if (succ)
+   if (rc)
tmo_buf = old.tmo_buf;
/* Else CAS failed, something changed => skip timer
 * this tick, it will be checked again next tick */
+   } else {
+   /* Else false positive, ignore */
}
-   /* Else false positive, ignore */
+
+   return  post_timeout_to_queue(tim->queue, tmo_buf, exp_tck);
+}
 #else
+static unsigned timer_expire(odp_timer_pool *tp, uint32_t idx, uint64_t tick)
+{
+   odp_timer *tim = &tp->timers[idx];
+   tick_buf_t *tb = &tp->tick_buf[idx];
+   odp_buffer_t tmo_buf = ODP_BUFFER_INVALID;
+   uint64_t exp_tck;
+
/* Take a related lock */
while (_odp_atomic_flag_tas(IDX2LOCK(idx)))
/* While lock is taken, spin using relaxed loads */
while (_odp_atomic_flag_load(IDX2LOCK(idx)))
odp_spin();
+
/* Proper check for timer expired */
exp_tck = tb->exp_tck.v;
-   if (odp_likely(exp_tck <= tick)) {
-   /* Verify that there is a timeout buffer */
-   if (odp_likely(tb->tmo_buf != ODP_BUFFER_INVALID)) {
-   /* Grab timeout buffer, replace with inactive timer
-* and invalid buffer */
-   tmo_buf = tb->tmo_buf;
-   tb->tmo_buf = ODP_BUFFER_INVALID;
-   /* Set the inactive/expired bit keeping the expiration
-* tick so that we can check against the expiration
-* tick of the timeout when it is received */
-   tb->exp_tck.v |= TMO_INACTIVE;
-   }
+
+   /* Check that tick expired and verify that there is a timeout buffer */
+   if (odp_likely((exp_tck <= tick) && (tb->tmo_buf != 
ODP_BUFFER_INVALID))) {
+   /* Grab timeout buffer, replace with inactive timer
+* and invalid buffer */
+   tmo_buf = tb->tmo_buf;
+   tb->tmo_buf

[lng-odp] [PATCH 0/3] simplify linux-generic timer code

[Maxim Uvarov]

This patchset is propose to simplify current linux-genric timer code.
There are no functional changes, just removing ifdef from function body.
After that changes function looks much more simple, at least now I can
understand what do they do.

Maxim.

Maxim Uvarov (3):
  linux-generic: timer simplify timer_expire
  linux-geneirc: timer: remove ifdefs inside function from timer_cancel
  linux-generic: timer:  remove atomic ifdef from timer_reset() function
body

 platform/linux-generic/odp_timer.c | 195 -
 1 file changed, 130 insertions(+), 65 deletions(-)

-- 
1.9.1


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

[lng-odp] [PATCH 3/3] linux-generic: timer: remove atomic ifdef from timer_reset() function body

[Maxim Uvarov]

No functional changes, just make code more readable.

Signed-off-by: Maxim Uvarov 
---
 platform/linux-generic/odp_timer.c | 76 +++---
 1 file changed, 54 insertions(+), 22 deletions(-)

diff --git a/platform/linux-generic/odp_timer.c 
b/platform/linux-generic/odp_timer.c
index 33c2758..36480ed 100644
--- a/platform/linux-generic/odp_timer.c
+++ b/platform/linux-generic/odp_timer.c
@@ -334,6 +334,7 @@ static inline odp_buffer_t timer_free(odp_timer_pool *tp, 
uint32_t idx)
  * expire/reset/cancel timer
  */
 
+#ifdef ODP_ATOMIC_U128
 static bool timer_reset(uint32_t idx,
uint64_t abs_tck,
odp_buffer_t *tmo_buf,
@@ -343,8 +344,8 @@ static bool timer_reset(uint32_t idx,
tick_buf_t *tb = &tp->tick_buf[idx];
 
if (tmo_buf == NULL || *tmo_buf == ODP_BUFFER_INVALID) {
-#ifdef ODP_ATOMIC_U128
tick_buf_t new, old;
+
do {
/* Relaxed and non-atomic read of current values */
old.exp_tck.v = tb->exp_tck.v;
@@ -370,11 +371,52 @@ static bool timer_reset(uint32_t idx,
(_uint128_t *)&new,
_ODP_MEMMODEL_RLS,
_ODP_MEMMODEL_RLX));
-#else
+   } else {
+   /* We have a new timeout buffer which replaces any old one */
+   /* Fill in header fields if timeout event */
+   if (_odp_buffer_type(*tmo_buf) == ODP_EVENT_TIMEOUT) {
+   /* Convert from buffer to timeout hdr */
+   odp_timeout_hdr_t *tmo_hdr =
+   timeout_hdr_from_buf(*tmo_buf);
+   tmo_hdr->timer = tp_idx_to_handle(tp, idx);
+   tmo_hdr->user_ptr = tp->timers[idx].user_ptr;
+   /* expiration field filled in when timer expires */
+   }
+   /* Else ignore buffers of other types */
+   odp_buffer_t old_buf = ODP_BUFFER_INVALID;
+   tick_buf_t new, old;
+   new.exp_tck.v = abs_tck;
+   new.tmo_buf = *tmo_buf;
+   TB_SET_PAD(new);
+   /* We are releasing the new timeout buffer to some other
+* thread */
+   _odp_atomic_u128_xchg_mm((_odp_atomic_u128_t *)tb,
+(_uint128_t *)&new,
+(_uint128_t *)&old,
+_ODP_MEMMODEL_ACQ_RLS);
+   old_buf = old.tmo_buf;
+   /* Return old timeout buffer */
+   *tmo_buf = old_buf;
+   }
+   return success;
+}
+
+#else /*ODP_ATOMIC_U128*/
+
+static bool timer_reset(uint32_t idx,
+   uint64_t abs_tck,
+   odp_buffer_t *tmo_buf,
+   odp_timer_pool *tp)
+{
+   bool success = true;
+   tick_buf_t *tb = &tp->tick_buf[idx];
+
+   if (tmo_buf == NULL || *tmo_buf == ODP_BUFFER_INVALID) {
 #ifdef __ARM_ARCH
/* Since barriers are not good for C-A15, we take an
 * alternative approach using relaxed memory model */
uint64_t old;
+
/* Swap in new expiration tick, get back old tick which
 * will indicate active/inactive timer state */
old = _odp_atomic_u64_xchg_mm(&tb->exp_tck, abs_tck,
@@ -398,7 +440,7 @@ static bool timer_reset(uint32_t idx,
_ODP_MEMMODEL_RLX);
success = false;
}
-#else
+#else /*__ARM_ARCH*/
/* Take a related lock */
while (_odp_atomic_flag_tas(IDX2LOCK(idx)))
/* While lock is taken, spin using relaxed loads */
@@ -417,9 +459,10 @@ static bool timer_reset(uint32_t idx,
 
/* Release the lock */
_odp_atomic_flag_clear(IDX2LOCK(idx));
-#endif
-#endif
+#endif /*__ARM_ARCH*/
} else {
+   odp_buffer_t old_buf;
+
/* We have a new timeout buffer which replaces any old one */
/* Fill in header fields if timeout event */
if (_odp_buffer_type(*tmo_buf) == ODP_EVENT_TIMEOUT) {
@@ -429,22 +472,10 @@ static bool timer_reset(uint32_t idx,
tmo_hdr->timer = tp_idx_to_handle(tp, idx);
tmo_hdr->user_ptr = tp->timers[idx].user_ptr;
/* expiration field filled in when timer expires */
+   } else {
+   /* Else ignore buffers of other types */
}
-   /* Else ignore buffers of other types */
-   odp_buffer_t old_buf = ODP_BUFFER_INVALID;
-#ifdef ODP_ATOMIC_U128
-   tick_buf_t new, old;
-   new.exp_tck.v = abs_tck;
-   new.t

Re: [lng-odp] [PATCH] linux-generic: odp_timer: set user_ptr for cancelled timeout

[Maxim Uvarov]

On 03/10/15 18:46, Ola Liljedahl wrote:

On 10 March 2015 at 16:11, Maxim Uvarov  wrote:

On 03/10/15 18:08, Ola Liljedahl wrote:

On 10 March 2015 at 15:59, Maxim Uvarov  wrote:

On 03/10/15 17:43, Ola Liljedahl wrote:

Ensure that the timeout user_ptr and timer fields are set when the
corresponding timer is immediately cancelled.
https://bugs.linaro.org/show_bug.cgi?id=1313

Signed-off-by: Ola Liljedahl 
---
(This document/code contribution attached is provided under the terms of
agreement LES-LTM-21309)

Passes odp_timer validation with the new odp_timer_cancel() test from
Petri.

platform/linux-generic/odp_timer.c | 18 ++
1 file changed, 14 insertions(+), 4 deletions(-)

diff --git a/platform/linux-generic/odp_timer.c
b/platform/linux-generic/odp_timer.c
index 61a02b6..6b48d2e 100644
--- a/platform/linux-generic/odp_timer.c
+++ b/platform/linux-generic/odp_timer.c
@@ -421,6 +421,16 @@ static bool timer_reset(uint32_t idx,
#endif
  } else {
  /* We have a new timeout buffer which replaces any old
one
*/
+   /* Fill in header fields if timeout event */
+   if (_odp_buffer_type(*tmo_buf) == ODP_EVENT_TIMEOUT) {
+   /* Convert from buffer to timeout hdr */
+   odp_timeout_hdr_t *tmo_hdr =
+   timeout_hdr_from_buf(*tmo_buf);

as we discussed earlier  this 2 definitions of tmo_hdr can be one on top
of
timer_expire().
int succ  can be int ret and also defined on top.

Yes they can but that would be a different coding style. It is OK to
declare variables at beginning of a block, even Linux does it (see
Petri's example). This has been OK since the beginning of time (the
start of the Epoch, Jan. 1st 1970).


My point here is if you use one variable 2 times in one function, than most
probably you need define it only once.
Do you agree?

No.

There is no intrinsic value in having as few variable declarations as
possible. If the scope becomes larger, this could be confusing to the
programmers that try to understand the code. A larger scope means that
the variables are valid longer (from a compiler perspective) even if
the variables are not really valid from a runtime perspective.

If I only need to use a variable in a certain block (because the
corresponding value is only alive there), I want to declare it in that
block and have it go out of scope when the block ends. Possibly
another block in the same function needs a variable of the same type
for some short-lived purpose. Then I will declare that variable in
that block and not try to combine in with an unrelated variable in
another block just because they are of the same type and used for
similar purposes.


If you need to do this that means that this function is too long, and 
probably we need

rework it to make more clear to everybody.

Current timer_reset() is actual 4 functions depending on provided defines.

Where ODP_ATOMIC_U128 is defined? How is it related to _ARM_ARCH?

I did some simplifying of that timer functions. Please take a look.

Thanks,
Maxim.



Maxim.


Maxim.


+   tmo_hdr->timer = tp_idx_to_handle(tp, idx);
+   tmo_hdr->user_ptr = tp->timers[idx].user_ptr;
+   /* expiration field filled in when timer expires
*/
+   }
+   /* Else ignore buffers of other types */
  odp_buffer_t old_buf = ODP_BUFFER_INVALID;
#ifdef ODP_ATOMIC_U128
  tick_buf_t new, old;
@@ -556,16 +566,16 @@ static unsigned timer_expire(odp_timer_pool *tp,
uint32_t idx, uint64_t tick)
  _odp_atomic_flag_clear(IDX2LOCK(idx));
#endif
  if (odp_likely(tmo_buf != ODP_BUFFER_INVALID)) {
-   /* Fill in metadata fields in system timeout buffer */
+   /* Fill in expiration tick if timeout event */
  if (_odp_buffer_type(tmo_buf) == ODP_EVENT_TIMEOUT) {
  /* Convert from buffer to timeout hdr */
  odp_timeout_hdr_t *tmo_hdr =
  timeout_hdr_from_buf(tmo_buf);
-   tmo_hdr->timer = tp_idx_to_handle(tp, idx);
  tmo_hdr->expiration = exp_tck;
-   tmo_hdr->user_ptr = tim->user_ptr;
+   /* timer and user_ptr fields filled in when
timer
+* was set */
  }
-   /* Else ignore buffers of other types */
+   /* Else ignore events of other types */
  /* Post the timeout to the destination queue */
  int rc = odp_queue_enq(tim->queue,
 odp_buffer_to_event(tmo_buf));



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





___
lng-odp mailing list
lng-odp

Re: [lng-odp] [PATCH] example: odp_ipsec: fix missing definition for ifreq

[Ciprian Barbu]

On Wed, Mar 11, 2015 at 2:12 PM, Ola Liljedahl  wrote:
> On 11 March 2015 at 10:58, Ciprian Barbu  wrote:
>>
>> On Tue, Mar 10, 2015 at 7:19 PM, Ola Liljedahl 
>> wrote:
>> > On 10 March 2015 at 17:11, Ciprian Barbu 
>> > wrote:
>> >> On Tue, Mar 10, 2015 at 6:06 PM, Ciprian Barbu
>> >>  wrote:
>> >>> On Tue, Mar 10, 2015 at 4:31 PM, Maxim Uvarov
>> >>>  wrote:
>>  On 03/10/15 17:16, Ciprian Barbu wrote:
>> >
>> > On Tue, Mar 10, 2015 at 1:33 PM, Maxim Uvarov
>> > 
>> > wrote:
>> >>
>> >> Please also specify your env.  I can not reproduce it with
>> >> ./cross-compile-test.sh
>> >
>> > I added some info in the bug entry. Were you able to reproduce like
>> > that?
>> 
>> 
>>  I see that in one includes in net/if.h that structure is under ifdef
>>  __USE_MISC, in other includes there is no such ifdef.
>>  Looks like you have different version of headers. For linux/if.h
>>  there is no
>>  ifdef for both cases. I think you patch is good to go,
>>  tested it on my toolchains (compilation only).
>> >>>
>> >>> This might be a problem with Ubuntu 13.10, I tested on an Ubuntu 14.04
>> >>> and it works.
>> >>>
>> >>> The whole problem comes from the ioctl command that requires struct
>> >>> ifreq. From this man page (http://linux.die.net/man/7/netdevice) it
>> >>> looks like it should be enough to include  and
>> >>> . I also found that including linux/if.h is usually done by
>> >>> code for kernel, so that might actually not be a good idea.
>> >>>
>> >>> Strange though, adding  doesn't fix compiling on my
>> >>> environment. Does anyone else run Ubuntu 13.10? maybe I screwed my
>> >>> headers somehow installing some packages ...
>> >>
>> >> I also found this:
>> >>
>> >> http://stackoverflow.com/questions/10433982/why-does-c99-complain-about-storage-sizes
>> >> which says the problem is in fact with -std=c99. Strange though how it
>> >> only behaves bad on my Ubuntu 13.10. Would still be good if someone
>> >> else checks on their env ...
>> > The way I interpret this is that when you specify a specific C
>> > standard (e.g. C99), by default you will only get access to those
>> > library features that are included in that standard. If you need to go
>> > outside of the standard, you need to ask for it specifically. E.g.
>> > define _BSD_SOURCE in this case.
>> >
>> > But this should be independent of Ubuntu releases. I can imagine
>> > different libc (glibc) versions may do this differently though so this
>> > could be that cause of differences in behavior between 13.10 and e.g.
>> > 14.04 (which I use).
>>
>> Ola, we used to make use of _BSD_SOURCE to get the extra features, but
>> the use of this macro has been deprecated since glib 2.20:
>> http://man7.org/linux/man-pages/man7/feature_test_macros.7.html.
>> Instead of defining _BSD_SOURCE the user must rely on _DEFAULT_SOURCE,
>> which exists since 2.19.
>>
>> Looking at my glib version I get:
>> GNU C Library (Ubuntu EGLIBC 2.17-93ubuntu4) stable release version
>> 2.17, by Roland McGrath et al.
>>
>> So the problem in my case is that I have an old glibc version and the
>> way we compile ODP does not cope with that. Other users will be
>> affected. Either we do something about it (I haven't found a way to
>> check the glibc version at compile time yet) or document that ODP will
>> not support glibc older than 2.19 (where _DEFAULT_SOURCE exists).
>
> Or we add some code to automake/configure to detect which (g)libc version
> you are using and then use the appropriate define (_BSD_SOURCE for <= 2.17,
> _DEFAULT_SOURCE for >= 2.19, don't know about 2.18).
>
> Wouldn't this be the "right" way of solving problems like this? This might
> not be the only time we have such a version problem.

Sounds good. I think autotools should have the support to get the
version of packages.

Anders, do you have more insights on that?

>
> -- Ola
>
>>
>> >
>> > olli@macmini:~$ gcc --version
>> > gcc (Ubuntu 4.8.2-19ubuntu1) 4.8.2
>> > Copyright (C) 2013 Free Software Foundation, Inc.
>> > This is free software; see the source for copying conditions.  There is
>> > NO
>> > warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR
>> > PURPOSE.
>> > olli@macmini:~$ dpkg -S /usr/include/net/if.h
>> > libc6-dev:amd64, libc6-dev:i386: /usr/include/net/if.h
>> > olli@macmini:~$ dpkg -s libc6-dev | grep Version
>> > Version: 2.19-0ubuntu6.6
>> >
>> >
>> >>
>> >> /Ciprian
>> >>
>> >>>
>> 
>>  Maxim.
>> 
>> 
>> 
>> >> Maxim.
>> >>
>> >>
>> >> On 03/10/15 14:14, Maxim Uvarov wrote:
>> >>>
>> >>> Please add patch description and put bug link in the bottom of it.
>> >>> Like
>> >>> other git commits do.
>> >>>
>> >>> Thanks,
>> >>> Maxim.
>> >>>
>> >>> On 03/10/15 12:47, Ciprian Barbu wrote:
>> 
>>  Signed-off-by: Ciprian Barbu 
>>  ---
>>  fix for https://bugs.linaro.org/show_bug.cgi?id=1330
>> 
>> >

Re: [lng-odp] [KEYSTONE2 PATCH 06/15] linux-ks2: init: fix a minimal library build

[Maxim Uvarov]

On 03/10/15 18:31, Taras Kondratiuk wrote:

  int odp_init_global(odp_init_t *params ODP_UNUSED,
  odp_platform_init_t *platform_params ODP_UNUSED)
  {
+   odp_proc.log_fn = odp_override_log;
+   odp_proc.abort_fn = odp_override_abort;
+
+   if (params != NULL) {
+   if (params->log_fn != NULL)
+   odp_proc.log_fn = params->log_fn;
+   if (params->abort_fn != NULL)
+   odp_proc.abort_fn = params->abort_fn;
+   }
+
odp_sy


remove ODP_UNUSED from params if you use them.

Maxim.

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

[lng-odp] [PATCH] validation: schedule: code clean up

[Petri Savolainen]

Cleaned up usage of buf count, thread args and globals.

Signed-off-by: Petri Savolainen 
---
 test/validation/odp_schedule.c | 102 -
 1 file changed, 49 insertions(+), 53 deletions(-)

diff --git a/test/validation/odp_schedule.c b/test/validation/odp_schedule.c
index 3035db8..a9369c5 100644
--- a/test/validation/odp_schedule.c
+++ b/test/validation/odp_schedule.c
@@ -38,14 +38,14 @@
 typedef struct {
int cpu_count;
odp_barrier_t barrier;
-   odp_schedule_prio_t current_prio;
-   int prio_buf_count;
-   odp_ticketlock_t count_lock;
+   int buf_count;
+   odp_ticketlock_t lock;
odp_spinlock_t atomic_lock;
 } test_globals_t;
 
-typedef struct ODP_PACKED {
-   pthrd_arg thrdarg;
+typedef struct {
+   pthrd_arg cu_thr;
+   test_globals_t *globals;
odp_schedule_sync_t sync;
int num_queues;
int num_prio;
@@ -84,22 +84,13 @@ static void *schedule_common_(void *arg)
 {
thread_args_t *args = (thread_args_t *)arg;
odp_schedule_sync_t sync;
-   int num_queues, num_prio, num_bufs, num_cpus;
-   odp_shm_t shm;
+   int num_cpus;
test_globals_t *globals;
 
+   globals = args->globals;
sync = args->sync;
-   num_queues = args->num_queues;
-   num_prio = args->num_prio;
-   num_bufs = args->num_bufs;
num_cpus = args->num_cpus;
 
-   shm = odp_shm_lookup(GLOBALS_SHM_NAME);
-   CU_ASSERT_FATAL(shm != ODP_SHM_INVALID);
-   globals = odp_shm_addr(shm);
-   CU_ASSERT_FATAL(globals != NULL);
-
-
if (num_cpus == globals->cpu_count)
odp_barrier_wait(&globals->barrier);
 
@@ -110,13 +101,12 @@ static void *schedule_common_(void *arg)
int num = 0;
int locked;
 
-   odp_ticketlock_lock(&globals->count_lock);
-   if (globals->prio_buf_count ==
-   num_bufs * num_queues * num_prio) {
-   odp_ticketlock_unlock(&globals->count_lock);
+   odp_ticketlock_lock(&globals->lock);
+   if (globals->buf_count == 0) {
+   odp_ticketlock_unlock(&globals->lock);
break;
}
-   odp_ticketlock_unlock(&globals->count_lock);
+   odp_ticketlock_unlock(&globals->lock);
 
if (args->enable_schd_multi) {
odp_event_t events[BURST_BUF_SIZE];
@@ -155,13 +145,18 @@ static void *schedule_common_(void *arg)
}
}
 
-   odp_ticketlock_lock(&globals->count_lock);
-   globals->prio_buf_count += num;
-
if (sync == ODP_SCHED_SYNC_ATOMIC)
odp_schedule_release_atomic();
 
-   odp_ticketlock_unlock(&globals->count_lock);
+   odp_ticketlock_lock(&globals->lock);
+   globals->buf_count -= num;
+
+   if (globals->buf_count < 0) {
+   odp_ticketlock_unlock(&globals->lock);
+   CU_FAIL_FATAL("Buffer counting failed");
+   }
+
+   odp_ticketlock_unlock(&globals->lock);
}
 
return NULL;
@@ -173,8 +168,11 @@ static void fill_queues(thread_args_t *args)
int num_queues, num_prio;
odp_pool_t pool;
int i, j, k;
+   int buf_count = 0;
+   test_globals_t *globals;
char name[32];
 
+   globals = args->globals;
sync = args->sync;
num_queues = args->num_queues;
num_prio = args->num_prio;
@@ -214,9 +212,12 @@ static void fill_queues(thread_args_t *args)
CU_ASSERT(buf != ODP_BUFFER_INVALID);
ev = odp_buffer_to_event(buf);
CU_ASSERT(odp_queue_enq(queue, ev) == 0);
+   buf_count++;
}
}
}
+
+   globals->buf_count = buf_count;
 }
 
 static void schedule_common(odp_schedule_sync_t sync, int num_queues,
@@ -231,9 +232,7 @@ static void schedule_common(odp_schedule_sync_t sync, int 
num_queues,
globals = odp_shm_addr(shm);
CU_ASSERT_FATAL(globals != NULL);
 
-   globals->current_prio = ODP_SCHED_PRIO_HIGHEST;
-   globals->prio_buf_count = 0;
-
+   args.globals = globals;
args.sync = sync;
args.num_queues = num_queues;
args.num_prio = num_prio;
@@ -253,7 +252,7 @@ static void parallel_execute(odp_schedule_sync_t sync, int 
num_queues,
 {
odp_shm_t shm;
test_globals_t *globals;
-   thread_args_t *thr_args;
+   thread_args_t *args;
 
shm = odp_shm_lookup(GLOBALS_SHM_NAME);
CU_ASSERT_FATAL(shm != ODP_SHM_INVALID);
@@ -262,32 +261,29 @@ static void parallel_execute(odp_schedule_sync_t sync, 
int num_queues,
 
shm = odp_shm_lookup(SHM_THR_ARGS_NAME);
CU_ASSERT_FATAL(shm != ODP

[lng-odp] [PATCH] example: odp_ipsec: remove USE_MAC_ADDR_HACK

[Ciprian Barbu]

Remove the old hack and switch to using the provided API
This also fixes https://bugs.linaro.org/show_bug.cgi?id=1330

Signed-off-by: Ciprian Barbu 
---
 example/ipsec/odp_ipsec.c | 58 ++-
 1 file changed, 2 insertions(+), 56 deletions(-)

diff --git a/example/ipsec/odp_ipsec.c b/example/ipsec/odp_ipsec.c
index 98160ba..82ed0cb 100644
--- a/example/ipsec/odp_ipsec.c
+++ b/example/ipsec/odp_ipsec.c
@@ -211,56 +211,6 @@ void free_pkt_ctx(pkt_ctx_t *ctx)
 }
 
 /**
- * Use socket I/O workaround to query interface MAC address
- *
- * @todo Remove all references to USE_MAC_ADDR_HACK once
- *   https://bugs.linaro.org/show_bug.cgi?id=627 is resolved
- */
-#define USE_MAC_ADDR_HACK 1
-
-#if USE_MAC_ADDR_HACK
-
-/**
- * Query MAC address associated with an interface (temporary workaround
- * till API is created)
- *
- * @param intfString name of the interface
- * @param src_mac MAC address used by the interface
- *
- * @return 0 if successful else -1
- */
-static
-int query_mac_address(char *intf, uint8_t *src_mac)
-{
-   int sd;
-   struct ifreq ifr;
-
-   /* Get a socket descriptor */
-   sd = socket(PF_PACKET, SOCK_RAW, htons(ETH_P_ALL));
-   if (sd < 0) {
-   EXAMPLE_ERR("Error: socket() failed for %s\n", intf);
-   return -1;
-   }
-
-   /* Use ioctl() to look up interface name and get its MAC address */
-   memset(&ifr, 0, sizeof(ifr));
-   snprintf(ifr.ifr_name, sizeof(ifr.ifr_name), "%s", intf);
-   if (ioctl(sd, SIOCGIFHWADDR, &ifr) < 0) {
-   close(sd);
-   EXAMPLE_ERR("Error: ioctl() failed for %s\n", intf);
-   return -1;
-   }
-   memcpy(src_mac, ifr.ifr_hwaddr.sa_data, ODPH_ETHADDR_LEN);
-
-   /* Fini */
-   close(sd);
-
-   return 0;
-}
-
-#endif
-
-/**
  * Example supports either polling queues or using odp_schedule
  *
  * Specify "CFLAGS=-DIPSEC_POLL_QUEUES" during configure to enable polling
@@ -590,12 +540,8 @@ void initialize_intf(char *intf)
}
 
/* Read the source MAC address for this interface */
-#if USE_MAC_ADDR_HACK
-   ret = query_mac_address(intf, src_mac);
-#else
-   ret = odp_pktio_get_mac_addr(pktio, src_mac);
-#endif
-   if (ret) {
+   ret = odp_pktio_mac_addr(pktio, src_mac, sizeof(src_mac));
+   if (ret <= 0) {
EXAMPLE_ERR("Error: failed during MAC address get for %s\n",
intf);
exit(EXIT_FAILURE);
-- 
1.8.3.2


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

Re: [lng-odp] [PATCH] example: odp_ipsec: fix missing definition for ifreq

[Bala Manoharan]

On 11 March 2015 at 18:31, Ciprian Barbu  wrote:
> So after a little chat we had, Maxim suggested I remove the ioctl code
> that needs struct ifreq. There is a USE_MAC_ADDR_HACK define in
> odp_ipsec that is now set to 1, removing all that code should fix the
> problem.
>
> The problem was described here: https://bugs.linaro.org/show_bug.cgi?id=627
>
> Robbie and Bala, can you confirm that we can go ahead with this? Is
> there any reason to keep that workaround?

Hi,

This hack was used before the API odp_pktio_mac_addr() was finalised.
Since this API has been finalised now and the same has been added in
odp_packet_io.h file. IMO this hack can be removed.

Regards,
Bala
>
> Thanks,
> /Ciprian
>
> On Wed, Mar 11, 2015 at 2:45 PM, Ciprian Barbu  
> wrote:
>> On Wed, Mar 11, 2015 at 2:12 PM, Ola Liljedahl  
>> wrote:
>>> On 11 March 2015 at 10:58, Ciprian Barbu  wrote:

 On Tue, Mar 10, 2015 at 7:19 PM, Ola Liljedahl 
 wrote:
 > On 10 March 2015 at 17:11, Ciprian Barbu 
 > wrote:
 >> On Tue, Mar 10, 2015 at 6:06 PM, Ciprian Barbu
 >>  wrote:
 >>> On Tue, Mar 10, 2015 at 4:31 PM, Maxim Uvarov
 >>>  wrote:
  On 03/10/15 17:16, Ciprian Barbu wrote:
 >
 > On Tue, Mar 10, 2015 at 1:33 PM, Maxim Uvarov
 > 
 > wrote:
 >>
 >> Please also specify your env.  I can not reproduce it with
 >> ./cross-compile-test.sh
 >
 > I added some info in the bug entry. Were you able to reproduce like
 > that?
 
 
  I see that in one includes in net/if.h that structure is under ifdef
  __USE_MISC, in other includes there is no such ifdef.
  Looks like you have different version of headers. For linux/if.h
  there is no
  ifdef for both cases. I think you patch is good to go,
  tested it on my toolchains (compilation only).
 >>>
 >>> This might be a problem with Ubuntu 13.10, I tested on an Ubuntu 14.04
 >>> and it works.
 >>>
 >>> The whole problem comes from the ioctl command that requires struct
 >>> ifreq. From this man page (http://linux.die.net/man/7/netdevice) it
 >>> looks like it should be enough to include  and
 >>> . I also found that including linux/if.h is usually done by
 >>> code for kernel, so that might actually not be a good idea.
 >>>
 >>> Strange though, adding  doesn't fix compiling on my
 >>> environment. Does anyone else run Ubuntu 13.10? maybe I screwed my
 >>> headers somehow installing some packages ...
 >>
 >> I also found this:
 >>
 >> http://stackoverflow.com/questions/10433982/why-does-c99-complain-about-storage-sizes
 >> which says the problem is in fact with -std=c99. Strange though how it
 >> only behaves bad on my Ubuntu 13.10. Would still be good if someone
 >> else checks on their env ...
 > The way I interpret this is that when you specify a specific C
 > standard (e.g. C99), by default you will only get access to those
 > library features that are included in that standard. If you need to go
 > outside of the standard, you need to ask for it specifically. E.g.
 > define _BSD_SOURCE in this case.
 >
 > But this should be independent of Ubuntu releases. I can imagine
 > different libc (glibc) versions may do this differently though so this
 > could be that cause of differences in behavior between 13.10 and e.g.
 > 14.04 (which I use).

 Ola, we used to make use of _BSD_SOURCE to get the extra features, but
 the use of this macro has been deprecated since glib 2.20:
 http://man7.org/linux/man-pages/man7/feature_test_macros.7.html.
 Instead of defining _BSD_SOURCE the user must rely on _DEFAULT_SOURCE,
 which exists since 2.19.

 Looking at my glib version I get:
 GNU C Library (Ubuntu EGLIBC 2.17-93ubuntu4) stable release version
 2.17, by Roland McGrath et al.

 So the problem in my case is that I have an old glibc version and the
 way we compile ODP does not cope with that. Other users will be
 affected. Either we do something about it (I haven't found a way to
 check the glibc version at compile time yet) or document that ODP will
 not support glibc older than 2.19 (where _DEFAULT_SOURCE exists).
>>>
>>> Or we add some code to automake/configure to detect which (g)libc version
>>> you are using and then use the appropriate define (_BSD_SOURCE for <= 2.17,
>>> _DEFAULT_SOURCE for >= 2.19, don't know about 2.18).
>>>
>>> Wouldn't this be the "right" way of solving problems like this? This might
>>> not be the only time we have such a version problem.
>>
>> Sounds good. I think autotools should have the support to get the
>> version of packages.
>>
>> Anders, do you have more insights on that?
>>
>>>
>>> -- Ola
>>>

 >
 > olli@macmini:~$ gcc --version
 > gcc (Ubuntu 4.8.2-19ubuntu1) 4.8.2
 > Copyright (C) 2013 Free So

Re: [lng-odp] [PATCH] linux-generic: odp_timer: set user_ptr for cancelled timeout

[Ola Liljedahl]

On 11 March 2015 at 13:16, Maxim Uvarov  wrote:

> On 03/10/15 18:46, Ola Liljedahl wrote:
>
>> On 10 March 2015 at 16:11, Maxim Uvarov  wrote:
>>
>>> On 03/10/15 18:08, Ola Liljedahl wrote:
>>>
 On 10 March 2015 at 15:59, Maxim Uvarov 
 wrote:

> On 03/10/15 17:43, Ola Liljedahl wrote:
>
>> Ensure that the timeout user_ptr and timer fields are set when the
>> corresponding timer is immediately cancelled.
>> https://bugs.linaro.org/show_bug.cgi?id=1313
>>
>> Signed-off-by: Ola Liljedahl 
>> ---
>> (This document/code contribution attached is provided under the terms
>> of
>> agreement LES-LTM-21309)
>>
>> Passes odp_timer validation with the new odp_timer_cancel() test from
>> Petri.
>>
>> platform/linux-generic/odp_timer.c | 18 ++
>> 1 file changed, 14 insertions(+), 4 deletions(-)
>>
>> diff --git a/platform/linux-generic/odp_timer.c
>> b/platform/linux-generic/odp_timer.c
>> index 61a02b6..6b48d2e 100644
>> --- a/platform/linux-generic/odp_timer.c
>> +++ b/platform/linux-generic/odp_timer.c
>> @@ -421,6 +421,16 @@ static bool timer_reset(uint32_t idx,
>> #endif
>>   } else {
>>   /* We have a new timeout buffer which replaces any
>> old
>> one
>> */
>> +   /* Fill in header fields if timeout event */
>> +   if (_odp_buffer_type(*tmo_buf) == ODP_EVENT_TIMEOUT) {
>> +   /* Convert from buffer to timeout hdr */
>> +   odp_timeout_hdr_t *tmo_hdr =
>> +   timeout_hdr_from_buf(*tmo_buf);
>>
> as we discussed earlier  this 2 definitions of tmo_hdr can be one on
> top
> of
> timer_expire().
> int succ  can be int ret and also defined on top.
>
 Yes they can but that would be a different coding style. It is OK to
 declare variables at beginning of a block, even Linux does it (see
 Petri's example). This has been OK since the beginning of time (the
 start of the Epoch, Jan. 1st 1970).

>>>
>>> My point here is if you use one variable 2 times in one function, than
>>> most
>>> probably you need define it only once.
>>> Do you agree?
>>>
>> No.
>>
>> There is no intrinsic value in having as few variable declarations as
>> possible. If the scope becomes larger, this could be confusing to the
>> programmers that try to understand the code. A larger scope means that
>> the variables are valid longer (from a compiler perspective) even if
>> the variables are not really valid from a runtime perspective.
>>
>> If I only need to use a variable in a certain block (because the
>> corresponding value is only alive there), I want to declare it in that
>> block and have it go out of scope when the block ends. Possibly
>> another block in the same function needs a variable of the same type
>> for some short-lived purpose. Then I will declare that variable in
>> that block and not try to combine in with an unrelated variable in
>> another block just because they are of the same type and used for
>> similar purposes.
>>
>
> If you need to do this that means that this function is too long, and
> probably we need
> rework it to make more clear to everybody.

I describe situations where it is better to have shorter-lived (shorter
than the whole function) variable declarations and you just parrot "your
function is too long". Variables in a function have different regions of
aliveness and it make sense for the programmer to be explicit about this so
that the compiler can warn if you are trying to use a variable when it does
not have a valid value (think how useful it could be if you could make a
pointer variable go out of scope after you have called free(), perhaps have
you heard of those "use after free" bugs?).
http://cwe.mitre.org/data/definitions/416.html

Whether a functions is longer (or shorter) than some subjective optimum is
a different question.

Me thinks you just haven't programmed enough to appreciate such language
features. But why should your inexperience limit the rest of us from
writing better code?




> Current timer_reset() is actual 4 functions depending on provided defines.
>
> Where ODP_ATOMIC_U128 is defined? How is it related to _ARM_ARCH?
>
https://patches.linaro.org/42855/
Not merged yet.

_ARM_ARCH and ODP_ATOMIC_U128 are not related to each other.
x86-64 supports 128-bit atomic operations (e.g. CMPXHG16) when compiled
with -mcx16. A compiler flag is required since some early x86-64
implementations did not support CMPXHG16.
ARMv8/AArch64 does support 128-bit atomic operations (load/store exclusive
pair) but the compiler (e.g. GCC) does not generate those instructions yet.

Because some targets don't support 128-bit SWAP and CAS is why I added some
limited support for lock-less operations (e.g. reset reusing an existing
timeout event, you only need to reset the expiratio

Re: [lng-odp] [PATCH] linux-generic: odp_timer: set user_ptr for cancelled timeout

[Mike Holmes]

Can we take this off line, it is straying from a technical discussion.

On 11 March 2015 at 09:29, Ola Liljedahl  wrote:

> On 11 March 2015 at 13:16, Maxim Uvarov  wrote:
>
>> On 03/10/15 18:46, Ola Liljedahl wrote:
>>
>>> On 10 March 2015 at 16:11, Maxim Uvarov  wrote:
>>>
 On 03/10/15 18:08, Ola Liljedahl wrote:

> On 10 March 2015 at 15:59, Maxim Uvarov 
> wrote:
>
>> On 03/10/15 17:43, Ola Liljedahl wrote:
>>
>>> Ensure that the timeout user_ptr and timer fields are set when the
>>> corresponding timer is immediately cancelled.
>>> https://bugs.linaro.org/show_bug.cgi?id=1313
>>>
>>> Signed-off-by: Ola Liljedahl 
>>> ---
>>> (This document/code contribution attached is provided under the
>>> terms of
>>> agreement LES-LTM-21309)
>>>
>>> Passes odp_timer validation with the new odp_timer_cancel() test from
>>> Petri.
>>>
>>> platform/linux-generic/odp_timer.c | 18 ++
>>> 1 file changed, 14 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/platform/linux-generic/odp_timer.c
>>> b/platform/linux-generic/odp_timer.c
>>> index 61a02b6..6b48d2e 100644
>>> --- a/platform/linux-generic/odp_timer.c
>>> +++ b/platform/linux-generic/odp_timer.c
>>> @@ -421,6 +421,16 @@ static bool timer_reset(uint32_t idx,
>>> #endif
>>>   } else {
>>>   /* We have a new timeout buffer which replaces any
>>> old
>>> one
>>> */
>>> +   /* Fill in header fields if timeout event */
>>> +   if (_odp_buffer_type(*tmo_buf) == ODP_EVENT_TIMEOUT)
>>> {
>>> +   /* Convert from buffer to timeout hdr */
>>> +   odp_timeout_hdr_t *tmo_hdr =
>>> +   timeout_hdr_from_buf(*tmo_buf);
>>>
>> as we discussed earlier  this 2 definitions of tmo_hdr can be one on
>> top
>> of
>> timer_expire().
>> int succ  can be int ret and also defined on top.
>>
> Yes they can but that would be a different coding style. It is OK to
> declare variables at beginning of a block, even Linux does it (see
> Petri's example). This has been OK since the beginning of time (the
> start of the Epoch, Jan. 1st 1970).
>

 My point here is if you use one variable 2 times in one function, than
 most
 probably you need define it only once.
 Do you agree?

>>> No.
>>>
>>> There is no intrinsic value in having as few variable declarations as
>>> possible. If the scope becomes larger, this could be confusing to the
>>> programmers that try to understand the code. A larger scope means that
>>> the variables are valid longer (from a compiler perspective) even if
>>> the variables are not really valid from a runtime perspective.
>>>
>>> If I only need to use a variable in a certain block (because the
>>> corresponding value is only alive there), I want to declare it in that
>>> block and have it go out of scope when the block ends. Possibly
>>> another block in the same function needs a variable of the same type
>>> for some short-lived purpose. Then I will declare that variable in
>>> that block and not try to combine in with an unrelated variable in
>>> another block just because they are of the same type and used for
>>> similar purposes.
>>>
>>
>> If you need to do this that means that this function is too long, and
>> probably we need
>> rework it to make more clear to everybody.
>
> I describe situations where it is better to have shorter-lived (shorter
> than the whole function) variable declarations and you just parrot "your
> function is too long". Variables in a function have different regions of
> aliveness and it make sense for the programmer to be explicit about this so
> that the compiler can warn if you are trying to use a variable when it does
> not have a valid value (think how useful it could be if you could make a
> pointer variable go out of scope after you have called free(), perhaps have
> you heard of those "use after free" bugs?).
> http://cwe.mitre.org/data/definitions/416.html
>
> Whether a functions is longer (or shorter) than some subjective optimum is
> a different question.
>
> Me thinks you just haven't programmed enough to appreciate such language
> features. But why should your inexperience limit the rest of us from
> writing better code?
>
>
>
>
>> Current timer_reset() is actual 4 functions depending on provided defines.
>>
>> Where ODP_ATOMIC_U128 is defined? How is it related to _ARM_ARCH?
>>
> https://patches.linaro.org/42855/
> Not merged yet.
>
> _ARM_ARCH and ODP_ATOMIC_U128 are not related to each other.
> x86-64 supports 128-bit atomic operations (e.g. CMPXHG16) when compiled
> with -mcx16. A compiler flag is required since some early x86-64
> implementations did not support CMPXHG16.
> ARMv8/AArch64 does support 128-bit atomic operations (load/store exclusive
> pair)

Re: [lng-odp] [PATCH] linux-generic: odp_timer: set user_ptr for cancelled timeout

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

> -Original Message-
> From: lng-odp-boun...@lists.linaro.org [mailto:lng-odp-
> boun...@lists.linaro.org] On Behalf Of ext Ola Liljedahl
> Sent: Tuesday, March 10, 2015 4:44 PM
> To: lng-odp@lists.linaro.org
> Subject: [lng-odp] [PATCH] linux-generic: odp_timer: set user_ptr for
> cancelled timeout
> 
> Ensure that the timeout user_ptr and timer fields are set when the
> corresponding timer is immediately cancelled.
> https://bugs.linaro.org/show_bug.cgi?id=1313
> 
> Signed-off-by: Ola Liljedahl 
> ---
> (This document/code contribution attached is provided under the terms of
> agreement LES-LTM-21309)
> 
> Passes odp_timer validation with the new odp_timer_cancel() test from
> Petri.
> 
>  platform/linux-generic/odp_timer.c | 18 ++
>  1 file changed, 14 insertions(+), 4 deletions(-)
> 
> diff --git a/platform/linux-generic/odp_timer.c b/platform/linux-
> generic/odp_timer.c
> index 61a02b6..6b48d2e 100644
> --- a/platform/linux-generic/odp_timer.c
> +++ b/platform/linux-generic/odp_timer.c
> @@ -421,6 +421,16 @@ static bool timer_reset(uint32_t idx,
>  #endif
>   } else {
>   /* We have a new timeout buffer which replaces any old one */
> + /* Fill in header fields if timeout event */

Typo: "... if timeout event" => "... in timeout event"

> + if (_odp_buffer_type(*tmo_buf) == ODP_EVENT_TIMEOUT) {
> + /* Convert from buffer to timeout hdr */
> + odp_timeout_hdr_t *tmo_hdr =
> + timeout_hdr_from_buf(*tmo_buf);
> + tmo_hdr->timer = tp_idx_to_handle(tp, idx);
> + tmo_hdr->user_ptr = tp->timers[idx].user_ptr;
> + /* expiration field filled in when timer expires */
> + }
> + /* Else ignore buffers of other types */
>   odp_buffer_t old_buf = ODP_BUFFER_INVALID;
>  #ifdef ODP_ATOMIC_U128
>   tick_buf_t new, old;
> @@ -556,16 +566,16 @@ static unsigned timer_expire(odp_timer_pool *tp,
> uint32_t idx, uint64_t tick)
>   _odp_atomic_flag_clear(IDX2LOCK(idx));
>  #endif
>   if (odp_likely(tmo_buf != ODP_BUFFER_INVALID)) {
> - /* Fill in metadata fields in system timeout buffer */
> + /* Fill in expiration tick if timeout event */

Same typo.

-Petri

>   if (_odp_buffer_type(tmo_buf) == ODP_EVENT_TIMEOUT) {
>   /* Convert from buffer to timeout hdr */
>   odp_timeout_hdr_t *tmo_hdr =
>   timeout_hdr_from_buf(tmo_buf);
> - tmo_hdr->timer = tp_idx_to_handle(tp, idx);
>   tmo_hdr->expiration = exp_tck;
> - tmo_hdr->user_ptr = tim->user_ptr;
> + /* timer and user_ptr fields filled in when timer
> +  * was set */
>   }
> - /* Else ignore buffers of other types */
> + /* Else ignore events of other types */
>   /* Post the timeout to the destination queue */
>   int rc = odp_queue_enq(tim->queue,
>  odp_buffer_to_event(tmo_buf));
> --
> 1.9.1
> 
> 
> ___
> lng-odp mailing list
> lng-odp@lists.linaro.org
> http://lists.linaro.org/mailman/listinfo/lng-odp

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

[lng-odp] [PATCH 1/3] example: odp_l2fwd: replace strtok_r with strtok and fix leaks

[Ciprian Barbu]

The odp_l2fwd example leaks some strings allocated during parse_args.
https://bugs.linaro.org/show_bug.cgi?id=1117
CID 56899:  Resource leak  (RESOURCE_LEAK)

Signed-off-by: Ciprian Barbu 
---
 example/l2fwd/odp_l2fwd.c | 29 +++--
 1 file changed, 15 insertions(+), 14 deletions(-)

diff --git a/example/l2fwd/odp_l2fwd.c b/example/l2fwd/odp_l2fwd.c
index d062a72..a5f85c0 100644
--- a/example/l2fwd/odp_l2fwd.c
+++ b/example/l2fwd/odp_l2fwd.c
@@ -73,6 +73,7 @@ typedef struct {
int if_count;   /**< Number of interfaces to be used */
char **if_names;/**< Array of pointers to interface names */
int mode;   /**< Packet IO mode */
+   char *if_str;   /**< Storage for interface names */
 } appl_args_t;
 
 /**
@@ -415,6 +416,8 @@ int main(int argc, char *argv[])
/* Master thread waits for other threads to exit */
odph_linux_pthread_join(thread_tbl, num_workers);
 
+   free(gbl_args->appl.if_names);
+   free(gbl_args->appl.if_str);
printf("Exit\n\n");
 
return 0;
@@ -462,7 +465,7 @@ static void parse_args(int argc, char *argv[], appl_args_t 
*appl_args)
 {
int opt;
int long_index;
-   char *names, *str, *token, *save;
+   char *token;
size_t len;
int i;
static struct option longopts[] = {
@@ -495,19 +498,19 @@ static void parse_args(int argc, char *argv[], 
appl_args_t *appl_args)
}
len += 1;   /* add room for '\0' */
 
-   names = malloc(len);
-   if (names == NULL) {
+   appl_args->if_str = malloc(len);
+   if (appl_args->if_str == NULL) {
usage(argv[0]);
exit(EXIT_FAILURE);
}
 
/* count the number of tokens separated by ',' */
-   strcpy(names, optarg);
-   for (str = names, i = 0;; str = NULL, i++) {
-   token = strtok_r(str, ",", &save);
-   if (token == NULL)
-   break;
-   }
+   strcpy(appl_args->if_str, optarg);
+   for (token = strtok(appl_args->if_str, ","), i = 0;
+token != NULL;
+token = strtok(NULL, ","), i++)
+   ;
+
appl_args->if_count = i;
 
if (appl_args->if_count == 0) {
@@ -520,11 +523,9 @@ static void parse_args(int argc, char *argv[], appl_args_t 
*appl_args)
calloc(appl_args->if_count, sizeof(char *));
 
/* store the if names (reset names string) */
-   strcpy(names, optarg);
-   for (str = names, i = 0;; str = NULL, i++) {
-   token = strtok_r(str, ",", &save);
-   if (token == NULL)
-   break;
+   strcpy(appl_args->if_str, optarg);
+   for (token = strtok(appl_args->if_str, ","), i = 0;
+token != NULL; token = strtok(NULL, ","), i++) {
appl_args->if_names[i] = token;
}
break;
-- 
1.8.3.2


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

[lng-odp] [PATCH 2/3] example: odp_ipsec: replace strtok_r with strtok and fix leaks

[Ciprian Barbu]

The odp_ipsec example leaks some strings allocated during parse_args.
https://bugs.linaro.org/show_bug.cgi?id=1117
CID 56899:  Resource leak  (RESOURCE_LEAK)

Signed-off-by: Ciprian Barbu 
---
 example/ipsec/odp_ipsec.c | 30 ++
 1 file changed, 14 insertions(+), 16 deletions(-)

diff --git a/example/ipsec/odp_ipsec.c b/example/ipsec/odp_ipsec.c
index 98160ba..d14670d 100644
--- a/example/ipsec/odp_ipsec.c
+++ b/example/ipsec/odp_ipsec.c
@@ -57,6 +57,7 @@ typedef struct {
char **if_names;/**< Array of pointers to interface names */
crypto_api_mode_e mode; /**< Crypto API preferred mode */
odp_pool_t pool;/**< Buffer pool for packet IO */
+   char *if_str;   /**< Storage for interface names */
 } appl_args_t;
 
 /**
@@ -1328,6 +1329,8 @@ main(int argc, char *argv[])
odph_linux_pthread_join(thread_tbl, num_workers);
}
 
+   free(args->appl.if_names);
+   free(args->appl.if_str);
printf("Exit\n\n");
 
return 0;
@@ -1344,10 +1347,7 @@ static void parse_args(int argc, char *argv[], 
appl_args_t *appl_args)
 {
int opt;
int long_index;
-   char *names;
-   char *str;
char *token;
-   char *save;
size_t len;
int rc = 0;
int i;
@@ -1389,19 +1389,19 @@ static void parse_args(int argc, char *argv[], 
appl_args_t *appl_args)
}
len += 1;   /* add room for '\0' */
 
-   names = malloc(len);
-   if (NULL == names) {
+   appl_args->if_str = malloc(len);
+   if (appl_args->if_str == NULL) {
usage(argv[0]);
exit(EXIT_FAILURE);
}
 
/* count the number of tokens separated by ',' */
-   strcpy(names, optarg);
-   for (str = names, i = 0;; str = NULL, i++) {
-   token = strtok_r(str, ",", &save);
-   if (NULL == token)
-   break;
-   }
+   strcpy(appl_args->if_str, optarg);
+   for (token = strtok(appl_args->if_str, ","), i = 0;
+token != NULL;
+token = strtok(NULL, ","), i++)
+   ;
+
appl_args->if_count = i;
 
if (0 == appl_args->if_count) {
@@ -1414,11 +1414,9 @@ static void parse_args(int argc, char *argv[], 
appl_args_t *appl_args)
calloc(appl_args->if_count, sizeof(char *));
 
/* store the if names (reset names string) */
-   strcpy(names, optarg);
-   for (str = names, i = 0;; str = NULL, i++) {
-   token = strtok_r(str, ",", &save);
-   if (NULL == token)
-   break;
+   strcpy(appl_args->if_str, optarg);
+   for (token = strtok(appl_args->if_str, ","), i = 0;
+token != NULL; token = strtok(NULL, ","), i++) {
appl_args->if_names[i] = token;
}
break;
-- 
1.8.3.2


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

[lng-odp] [PATCH 0/3] Fix the leaks in examples (bug 1117)

[Ciprian Barbu]

This series continues the patch I sent some while ago to replace strtok with
strtok_r in odp_pktio:
https://git.linaro.org/lng/odp.git/commit/273955e6db6bb220f2736d3709e4237c50d04772

The bug that this series fixes only refers to resource leaks, but the initial
strtok_r fix should have been done for all examples, there is no need to use the
reentrant version of strtok because it is only used from one thread, once,
during parsing of args.

There was a suggestion at some point to make the parsing of if_names common for
all examples, I don't think it's easy to factor it out, it would introduce more
code than necessary and we're only talking about a few lines of code.

Ciprian Barbu (3):
  example: odp_l2fwd: replace strtok_r with strtok and fix leaks
  example: odp_ipsec: replace strtok_r with strtok and fix leaks
  example: generator: replace strtok_r with strtok and fix leaks

 example/generator/odp_generator.c | 30 --
 example/ipsec/odp_ipsec.c | 30 ++
 example/l2fwd/odp_l2fwd.c | 29 +++--
 3 files changed, 45 insertions(+), 44 deletions(-)

-- 
1.8.3.2


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

[lng-odp] [Bug 1117] CID 56899: Resource leak in examples

[bugzilla-daemon]

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

--- Comment #2 from Ciprian Barbu  ---
Patches v1 sent:
https://patches.linaro.org/45656/
https://patches.linaro.org/45657/
https://patches.linaro.org/45658/

-- 
You are receiving this mail because:
You are on the CC list for the bug.___
lng-odp mailing list
lng-odp@lists.linaro.org
http://lists.linaro.org/mailman/listinfo/lng-odp

[lng-odp] [Bug 1337] No means of knowing the size of the CPU mask (odp_cpumask_t)

[bugzilla-daemon]

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

Mike Holmes  changed:

   What|Removed |Added

 CC||mike.hol...@linaro.org

--- Comment #1 from Mike Holmes  ---
Need to improve CPU mask documentation when fixing this so that is is clearer
what a mask relates to.

-- 
You are receiving this mail because:
You are the assignee for the bug.___
lng-odp mailing list
lng-odp@lists.linaro.org
http://lists.linaro.org/mailman/listinfo/lng-odp

Re: [lng-odp] [PATCH] validation: pktio: prevent race when using veth pair

[Stuart Haslam]

On Thu, Mar 05, 2015 at 12:12:31PM +, Stuart Haslam wrote:
> There's a potential race condition whereby the test case could start
> running before the virtual ethernet interfaces are fully brought up.
> So replace the the arbitrary delay with a check for the interface's
> operational state before kicking off the test.
> 

Can someone review this please?

> Signed-off-by: Stuart Haslam 
> ---
>  test/validation/odp_pktio_run | 32 ++--
>  1 file changed, 30 insertions(+), 2 deletions(-)
> 
> diff --git a/test/validation/odp_pktio_run b/test/validation/odp_pktio_run
> index 08288e6..b051be1 100755
> --- a/test/validation/odp_pktio_run
> +++ b/test/validation/odp_pktio_run
> @@ -34,6 +34,28 @@ IF1=pktio-p1
>  # exit codes expected by automake for skipped tests
>  TEST_SKIPPED=77
>  
> +# wait for a network interface's operational state to be "up"
> +wait_for_iface_up()
> +{
> + iface=$1
> + cnt=0
> +
> + while [ $cnt -lt 50 ]; do
> + read operstate < /sys/class/net/$iface/operstate
> +
> + if [ $? != 0 ]; then
> + break
> + elif [ "$operstate" = "up" ]; then
> + return 0
> + fi
> +
> + sleep 0.1
> + cnt=$[$cnt+1]
> + done
> +
> + return 1
> +}
> +
>  setup_env1()
>  {
>   ip link show $IF0 2> /dev/null
> @@ -59,8 +81,14 @@ setup_env1()
>   ip link set $IF0 up
>   ip link set $IF1 up
>  
> - # network needs a little time to come up
> - sleep 1
> + # check that the interface has come up before starting the test
> + for iface in $IF0 $IF1; do
> + wait_for_iface_up $iface
> + if [ $? != 0 ]; then
> + echo "pktio: interface $iface failed to come up"
> + exit 1
> + fi
> + done
>  }
>  
>  cleanup_env1()
> -- 
> 2.1.1
> 

-- 
Stuart.

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

Re: [lng-odp] [PATCHv3] linux-generic: only print errors to stderr

[Stuart Haslam]

On Fri, Mar 06, 2015 at 01:24:52PM +, Stuart Haslam wrote:
> The default logger prints all log levels to stderr. To make things
> easier when debugging failures change it to print only errors to stderr
> and everything else to stdout.

ping - this is ready to be merged.

> 
> Signed-off-by: Stuart Haslam 
> Reviewed-by: Taras Kondratiuk 
> ---
> v2: Fixed indentation (forgot to checkpatch v1)
> v3: Removed ODP_UNUSED for level as it's now used
> 
>  platform/linux-generic/odp_weak.c | 15 +--
>  1 file changed, 13 insertions(+), 2 deletions(-)
> 
> diff --git a/platform/linux-generic/odp_weak.c 
> b/platform/linux-generic/odp_weak.c
> index 7fa5955..f4336f0 100644
> --- a/platform/linux-generic/odp_weak.c
> +++ b/platform/linux-generic/odp_weak.c
> @@ -10,13 +10,24 @@
>  #include 
>  
>  ODP_WEAK_SYMBOL ODP_PRINTF_FORMAT(2, 3)
> -int odp_override_log(odp_log_level_e level ODP_UNUSED, const char *fmt, ...)
> +int odp_override_log(odp_log_level_e level, const char *fmt, ...)
>  {
>   va_list args;
>   int r;
> + FILE *logfd;
> +
> + switch (level) {
> + case ODP_LOG_ERR:
> + case ODP_LOG_UNIMPLEMENTED:
> + case ODP_LOG_ABORT:
> + logfd = stderr;
> + break;
> + default:
> + logfd = stdout;
> + }
>  
>   va_start(args, fmt);
> - r = vfprintf(stderr, fmt, args);
> + r = vfprintf(logfd, fmt, args);
>   va_end(args);
>  
>   return r;
> -- 
> 2.1.1
> 

-- 
Stuart.

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

Re: [lng-odp] [PATCH] linux-generic: odp_timer: set user_ptr for cancelled timeout

[Ola Liljedahl]

On 11 March 2015 at 14:49, Savolainen, Petri (Nokia - FI/Espoo) <
petri.savolai...@nokia.com> wrote:

>
>
> > -Original Message-
> > From: lng-odp-boun...@lists.linaro.org [mailto:lng-odp-
> > boun...@lists.linaro.org] On Behalf Of ext Ola Liljedahl
> > Sent: Tuesday, March 10, 2015 4:44 PM
> > To: lng-odp@lists.linaro.org
> > Subject: [lng-odp] [PATCH] linux-generic: odp_timer: set user_ptr for
> > cancelled timeout
> >
> > Ensure that the timeout user_ptr and timer fields are set when the
> > corresponding timer is immediately cancelled.
> > https://bugs.linaro.org/show_bug.cgi?id=1313
> >
> > Signed-off-by: Ola Liljedahl 
> > ---
> > (This document/code contribution attached is provided under the terms of
> > agreement LES-LTM-21309)
> >
> > Passes odp_timer validation with the new odp_timer_cancel() test from
> > Petri.
> >
> >  platform/linux-generic/odp_timer.c | 18 ++
> >  1 file changed, 14 insertions(+), 4 deletions(-)
> >
> > diff --git a/platform/linux-generic/odp_timer.c b/platform/linux-
> > generic/odp_timer.c
> > index 61a02b6..6b48d2e 100644
> > --- a/platform/linux-generic/odp_timer.c
> > +++ b/platform/linux-generic/odp_timer.c
> > @@ -421,6 +421,16 @@ static bool timer_reset(uint32_t idx,
> >  #endif
> >   } else {
> >   /* We have a new timeout buffer which replaces any old one
> */
> > + /* Fill in header fields if timeout event */
>
> Typo: "... if timeout event" => "... in timeout event"
>
Will fix.

BTW should the timer implementation still be using buffers as the base
class for timeouts? I think there is some more event-ification work to do
here.


> > + if (_odp_buffer_type(*tmo_buf) == ODP_EVENT_TIMEOUT) {
> > + /* Convert from buffer to timeout hdr */
> > + odp_timeout_hdr_t *tmo_hdr =
> > + timeout_hdr_from_buf(*tmo_buf);
> > + tmo_hdr->timer = tp_idx_to_handle(tp, idx);
> > + tmo_hdr->user_ptr = tp->timers[idx].user_ptr;
> > + /* expiration field filled in when timer expires */
> > + }
> > + /* Else ignore buffers of other types */
> >   odp_buffer_t old_buf = ODP_BUFFER_INVALID;
> >  #ifdef ODP_ATOMIC_U128
> >   tick_buf_t new, old;
> > @@ -556,16 +566,16 @@ static unsigned timer_expire(odp_timer_pool *tp,
> > uint32_t idx, uint64_t tick)
> >   _odp_atomic_flag_clear(IDX2LOCK(idx));
> >  #endif
> >   if (odp_likely(tmo_buf != ODP_BUFFER_INVALID)) {
> > - /* Fill in metadata fields in system timeout buffer */
> > + /* Fill in expiration tick if timeout event */
>
> Same typo.
>
Will fix.


>
> -Petri
>
> >   if (_odp_buffer_type(tmo_buf) == ODP_EVENT_TIMEOUT) {
> >   /* Convert from buffer to timeout hdr */
> >   odp_timeout_hdr_t *tmo_hdr =
> >   timeout_hdr_from_buf(tmo_buf);
> > - tmo_hdr->timer = tp_idx_to_handle(tp, idx);
> >   tmo_hdr->expiration = exp_tck;
> > - tmo_hdr->user_ptr = tim->user_ptr;
> > + /* timer and user_ptr fields filled in when timer
> > +  * was set */
> >   }
> > - /* Else ignore buffers of other types */
> > + /* Else ignore events of other types */
> >   /* Post the timeout to the destination queue */
> >   int rc = odp_queue_enq(tim->queue,
> >  odp_buffer_to_event(tmo_buf));
> > --
> > 1.9.1
> >
> >
> > ___
> > lng-odp mailing list
> > lng-odp@lists.linaro.org
> > http://lists.linaro.org/mailman/listinfo/lng-odp
>
___
lng-odp mailing list
lng-odp@lists.linaro.org
http://lists.linaro.org/mailman/listinfo/lng-odp

Re: [lng-odp] [PATCH] linux-generic: odp_timer: set user_ptr for cancelled timeout

[Ola Liljedahl]

On 11 March 2015 at 15:26, Ola Liljedahl  wrote:

> On 11 March 2015 at 14:49, Savolainen, Petri (Nokia - FI/Espoo) <
> petri.savolai...@nokia.com> wrote:
>
>>
>>
>> > -Original Message-
>> > From: lng-odp-boun...@lists.linaro.org [mailto:lng-odp-
>> > boun...@lists.linaro.org] On Behalf Of ext Ola Liljedahl
>> > Sent: Tuesday, March 10, 2015 4:44 PM
>> > To: lng-odp@lists.linaro.org
>> > Subject: [lng-odp] [PATCH] linux-generic: odp_timer: set user_ptr for
>> > cancelled timeout
>> >
>> > Ensure that the timeout user_ptr and timer fields are set when the
>> > corresponding timer is immediately cancelled.
>> > https://bugs.linaro.org/show_bug.cgi?id=1313
>> >
>> > Signed-off-by: Ola Liljedahl 
>> > ---
>> > (This document/code contribution attached is provided under the terms of
>> > agreement LES-LTM-21309)
>> >
>> > Passes odp_timer validation with the new odp_timer_cancel() test from
>> > Petri.
>> >
>> >  platform/linux-generic/odp_timer.c | 18 ++
>> >  1 file changed, 14 insertions(+), 4 deletions(-)
>> >
>> > diff --git a/platform/linux-generic/odp_timer.c b/platform/linux-
>> > generic/odp_timer.c
>> > index 61a02b6..6b48d2e 100644
>> > --- a/platform/linux-generic/odp_timer.c
>> > +++ b/platform/linux-generic/odp_timer.c
>> > @@ -421,6 +421,16 @@ static bool timer_reset(uint32_t idx,
>> >  #endif
>> >   } else {
>> >   /* We have a new timeout buffer which replaces any old
>> one */
>> > + /* Fill in header fields if timeout event */
>>
>> Typo: "... if timeout event" => "... in timeout event"
>>
> Will fix.
>
Now looking at what I wrote I actually wrote what I meant. But I should
probably have phrased it like this: "If timeout event then fill in headers"
or "When timeout event fill in headers". I hope this would be clearer.
The specified event does not have to be a timeout event.



>
> BTW should the timer implementation still be using buffers as the base
> class for timeouts? I think there is some more event-ification work to do
> here.
>
>
>> > + if (_odp_buffer_type(*tmo_buf) == ODP_EVENT_TIMEOUT) {
>> > + /* Convert from buffer to timeout hdr */
>> > + odp_timeout_hdr_t *tmo_hdr =
>> > + timeout_hdr_from_buf(*tmo_buf);
>> > + tmo_hdr->timer = tp_idx_to_handle(tp, idx);
>> > + tmo_hdr->user_ptr = tp->timers[idx].user_ptr;
>> > + /* expiration field filled in when timer expires
>> */
>> > + }
>> > + /* Else ignore buffers of other types */
>> >   odp_buffer_t old_buf = ODP_BUFFER_INVALID;
>> >  #ifdef ODP_ATOMIC_U128
>> >   tick_buf_t new, old;
>> > @@ -556,16 +566,16 @@ static unsigned timer_expire(odp_timer_pool *tp,
>> > uint32_t idx, uint64_t tick)
>> >   _odp_atomic_flag_clear(IDX2LOCK(idx));
>> >  #endif
>> >   if (odp_likely(tmo_buf != ODP_BUFFER_INVALID)) {
>> > - /* Fill in metadata fields in system timeout buffer */
>> > + /* Fill in expiration tick if timeout event */
>>
>> Same typo.
>>
> Will fix.
>
>
>>
>> -Petri
>>
>> >   if (_odp_buffer_type(tmo_buf) == ODP_EVENT_TIMEOUT) {
>> >   /* Convert from buffer to timeout hdr */
>> >   odp_timeout_hdr_t *tmo_hdr =
>> >   timeout_hdr_from_buf(tmo_buf);
>> > - tmo_hdr->timer = tp_idx_to_handle(tp, idx);
>> >   tmo_hdr->expiration = exp_tck;
>> > - tmo_hdr->user_ptr = tim->user_ptr;
>> > + /* timer and user_ptr fields filled in when timer
>> > +  * was set */
>> >   }
>> > - /* Else ignore buffers of other types */
>> > + /* Else ignore events of other types */
>> >   /* Post the timeout to the destination queue */
>> >   int rc = odp_queue_enq(tim->queue,
>> >  odp_buffer_to_event(tmo_buf));
>> > --
>> > 1.9.1
>> >
>> >
>> > ___
>> > lng-odp mailing list
>> > lng-odp@lists.linaro.org
>> > http://lists.linaro.org/mailman/listinfo/lng-odp
>>
>
>
___
lng-odp mailing list
lng-odp@lists.linaro.org
http://lists.linaro.org/mailman/listinfo/lng-odp

Re: [lng-odp] odp_cpu_count() vs odp_cpumask size

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

Continuation of cpumask discussion on the call… For creating arbitrary masks 
(e.g. fill in a mask with CPU ids of another ODP program in the system, or 
testing all possible mask bits), we may need two more calls:

/**
 * @return Maximum number of CPUs a mask can hold
 */
int odp_cpumask_max_cpus(void)

/**
 * Set all CPUs in the mask
 *
 * After the call, the mask has odp_cpumask_max_cpus() CPUs set.
  * @note CPU numbering may not be contiguous.
  */
void odp_cpumask_setall(odp_cpumask_t *mask)


-Petri



From: lng-odp-boun...@lists.linaro.org 
[mailto:lng-odp-boun...@lists.linaro.org] On Behalf Of ext Savolainen, Petri 
(Nokia - FI/Espoo)
Sent: Wednesday, March 11, 2015 2:09 PM
To: ext Christophe Milard; lng-odp@lists.linaro.org
Subject: Re: [lng-odp] odp_cpu_count() vs odp_cpumask size

Actually, this is equal to odp_cpu_max():

odp_cpumask_t mask;
odp_cpu_mask(&mask)
odp_cpumask_last(&mask)

So maybe odp_cpu_mask() is only new thing we need.

-Petri

From: lng-odp-boun...@lists.linaro.org 
[mailto:lng-odp-boun...@lists.linaro.org] On Behalf Of ext Savolainen, Petri 
(Nokia - FI/Espoo)
Sent: Wednesday, March 11, 2015 2:03 PM
To: ext Christophe Milard; 
lng-odp@lists.linaro.org
Subject: Re: [lng-odp] odp_cpu_count() vs odp_cpumask size

CPU IDs are system dependent. So e.g. odp_cpu_count() == 2 would not 
necessarily result a cpumask with cpus 0 and 1 set, but e.g. cpus 7 and 17 set. 
Typically, the mask comes from user/system level (user has decided to run the 
ODP app on cpus 7 and 17).

We may need two new functions:

/**
* @return the max CPU available to the ODP program
*/
int odp_cpu_max(void)

, but still that would not tell which ids between 0 and max are valid, so we 
need also

/**
  *  Output mask of CPUs available to the ODP program
 */
void odp_cpu_mask(odp_cpumask_t *mask)


-Petri


From: lng-odp-boun...@lists.linaro.org 
[mailto:lng-odp-boun...@lists.linaro.org] On Behalf Of ext Christophe Milard
Sent: Tuesday, March 10, 2015 4:22 PM
To: lng-odp@lists.linaro.org
Subject: [lng-odp] odp_cpu_count() vs odp_cpumask size

Hi,
I am writing validation tests for the odp_cpumask functions.
The question is: How many CPUs should the cpu mask be tested for?.
Of course, the answer is implementation dependent, but if we want the tests to 
be non platform dependent, the tests should know the max number of CPU the mask 
could contain.
In the API the function odp_cpu_count(void) is defined. This function "reports 
the number of CPUs available to this ODP program". Is this also the maximum 
number of CPU in the mask, or should the mask also have room for non "odp 
reserved cpus"?
For instance, in a system running 2 linux (control) cpus, and 5 ODP cpus, 
should the mask be tested for 5 or 7 CPUs? If 7 is the answer, how do I get 
this number from the current API...
/Christophe.
___
lng-odp mailing list
lng-odp@lists.linaro.org
http://lists.linaro.org/mailman/listinfo/lng-odp

Re: [lng-odp] odp_cpu_count() vs odp_cpumask size

[Taras Kondratiuk]

On 03/11/2015 04:46 PM, Savolainen, Petri (Nokia - FI/Espoo) wrote:

Continuation of cpumask discussion on the call… For creating arbitrary
masks (e.g. fill in a mask with CPU ids of another ODP program in the
system, or testing all possible mask bits), we may need two more calls:

/**

* @return Maximum number of CPUs a mask can hold

*/

int odp_cpumask_max_cpus(void)

/**

  * Set all CPUs in the mask

  *

  * After the call, the mask has odp_cpumask_max_cpus() CPUs set.

   * @note CPU numbering may not be contiguous.

*/

void odp_cpumask_setall(odp_cpumask_t *mask)

-Petri

*From:*lng-odp-boun...@lists.linaro.org
[mailto:lng-odp-boun...@lists.linaro.org] *On Behalf Of *ext Savolainen,
Petri (Nokia - FI/Espoo)
*Sent:* Wednesday, March 11, 2015 2:09 PM
*To:* ext Christophe Milard; lng-odp@lists.linaro.org
*Subject:* Re: [lng-odp] odp_cpu_count() vs odp_cpumask size

Actually, this is equal to odp_cpu_max():

odp_cpumask_t mask;

odp_cpu_mask(&mask)

odp_cpumask_last(&mask)

So maybe odp_cpu_mask() is only new thing we need.


Hi Petri

It is not clear to me why do we need odp_cpu_* and odp_cpumask_* API.

They are used now to create threads and set CPU affinity. But this is
done by ODP helpers, because it is out of ODP scope. Shouldn't 'cpu' and
'cpumask' be a part of helpers too?

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

[lng-odp] [PATCHv2] linux-generic: odp_timer: set user_ptr for cancelled timeout

[Ola Liljedahl]

Ensure that the timeout user_ptr and timer fields are set when the
corresponding timer is immediately cancelled.
https://bugs.linaro.org/show_bug.cgi?id=1313

Signed-off-by: Ola Liljedahl 
---
(This document/code contribution attached is provided under the terms of
agreement LES-LTM-21309)

Passes odp_timer validation with the new odp_timer_cancel() test from Petri.

v2:
Updated some comments in odp_timer.c to make the meaning clearer.

 platform/linux-generic/odp_timer.c | 18 ++
 1 file changed, 14 insertions(+), 4 deletions(-)

diff --git a/platform/linux-generic/odp_timer.c 
b/platform/linux-generic/odp_timer.c
index 61a02b6..b7cb04f 100644
--- a/platform/linux-generic/odp_timer.c
+++ b/platform/linux-generic/odp_timer.c
@@ -421,6 +421,16 @@ static bool timer_reset(uint32_t idx,
 #endif
} else {
/* We have a new timeout buffer which replaces any old one */
+   /* Fill in some (constant) header fields for timeout events */
+   if (_odp_buffer_type(*tmo_buf) == ODP_EVENT_TIMEOUT) {
+   /* Convert from buffer to timeout hdr */
+   odp_timeout_hdr_t *tmo_hdr =
+   timeout_hdr_from_buf(*tmo_buf);
+   tmo_hdr->timer = tp_idx_to_handle(tp, idx);
+   tmo_hdr->user_ptr = tp->timers[idx].user_ptr;
+   /* expiration field filled in when timer expires */
+   }
+   /* Else ignore buffers of other types */
odp_buffer_t old_buf = ODP_BUFFER_INVALID;
 #ifdef ODP_ATOMIC_U128
tick_buf_t new, old;
@@ -556,16 +566,16 @@ static unsigned timer_expire(odp_timer_pool *tp, uint32_t 
idx, uint64_t tick)
_odp_atomic_flag_clear(IDX2LOCK(idx));
 #endif
if (odp_likely(tmo_buf != ODP_BUFFER_INVALID)) {
-   /* Fill in metadata fields in system timeout buffer */
+   /* Fill in expiration tick for timeout events */
if (_odp_buffer_type(tmo_buf) == ODP_EVENT_TIMEOUT) {
/* Convert from buffer to timeout hdr */
odp_timeout_hdr_t *tmo_hdr =
timeout_hdr_from_buf(tmo_buf);
-   tmo_hdr->timer = tp_idx_to_handle(tp, idx);
tmo_hdr->expiration = exp_tck;
-   tmo_hdr->user_ptr = tim->user_ptr;
+   /* timer and user_ptr fields filled in when timer
+* was set */
}
-   /* Else ignore buffers of other types */
+   /* Else ignore events of other types */
/* Post the timeout to the destination queue */
int rc = odp_queue_enq(tim->queue,
   odp_buffer_to_event(tmo_buf));
-- 
1.9.1


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

[lng-odp] [Bug 1330] commit 71fcc8a _DEFAULT_SOURCE breaks odp_ipsec example

[bugzilla-daemon]

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

--- Comment #2 from Ciprian Barbu  ---
Patch sent:
https://patches.linaro.org/45655/

-- 
You are receiving this mail because:
You are the assignee for the bug.___
lng-odp mailing list
lng-odp@lists.linaro.org
http://lists.linaro.org/mailman/listinfo/lng-odp

Re: [lng-odp] ODP_v1.0 @ Regarding adding some file say demo.c and demo.h in platform/linux-genic and platform/linux-genic/include

[Bill Fischofer]

I'm not sure what you're trying to do.  You don't need to add files into
the ODP directory structure in order to compile them.  Are you just trying
to compile an ODP application?

On Wed, Mar 11, 2015 at 6:39 AM, Ayushman Kashyap 
wrote:

> Hi
>
> I want to add some file in *platform/linux-generic* and
> *platform/linux-genic/include*. But Not able to compile simple case.
> Please see the scenario .
>
>
>
>
>
> I have a file say:
>
> * demo.c: *
>
>
>
> #include 
>
> #include 
>
>
>
> int get_sys_page_size()
>
> {
>
> int page_size;
>
> page_size = getpagesize();
>
> printf("Page Size : %d\n",page_size);
>
> return 0;
>
> }
>
>
>
> *Demo.h : *
>
>
>
> #ifndef DEMO_H_
>
> #define DEMO_H_
>
> #ifdef __cplusplus
>
> extern "C" {
>
> #endif
>
> int get_sys_page_size();
>
> #ifdef __cplusplus
>
> }
>
> #endif
>
> #endif
>
>
>
> Added demo.c file in :   platform/linux-generic/
>
> Added demo.h file in : platform/linux-generic/include/
>
>
>
> Modified *platform/linux-generic/Makefile.am*
>
>
>
> ${top_srcdir}/platform/linux-generic/include/odp_timer_internal.h \
>
> *++  ${top_srcdir}/platform/linux-generic/include/demo.h \*
>
>   ${top_srcdir}/platform/linux-generic/Makefile.inc
>
>
>
> odp_timer.c \
>
>odp_version.c \
>
>odp_weak.c \
>
> *++   demo.c*
>
>
>
>
>
> *./bootstrap*
>
> *./configure*
>
> *Make*
>
>
>
>
>
> CC   odp_time.lo
>
>   CC   odp_timer.lo
>
>   CC   odp_version.lo
>
>   CC   odp_weak.lo
>
>   CC   demo.lo
>
> demo.c:4:5: error: function declaration isn't a prototype
> [-Werror=strict-prototypes]
>
>  int get_sys_page_size()
>
>  ^
>
> demo.c: In function 'get_sys_page_size':
>
> demo.c:4:5: error: old-style function definition
> [-Werror=old-style-definition]
>
> demo.c:7:2: error: implicit declaration of function 'getpagesize'
> [-Werror=implicit-function-declaration]
>
>   page_size = getpagesize();
>
>   ^
>
> demo.c:7:2: error: nested extern declaration of 'getpagesize'
> [-Werror=nested-externs]
>
> cc1: all warnings being treated as errors
>
> make[2]: *** [demo.lo] Error 1
>
> make[2]: Leaving directory `. /ODPV1.0-C1/platform/linux-generic'
>
> make[1]: *** [all-recursive] Error 1
>
> make[1]: Leaving directory `. /ODP-MJ/ODPV1.0-C1/platform'
>
> make: *** [all-recursive] Error 1
>
>
>
>
>
> If someone help me in this regards, it will be very helpful for me. The
> same use cases, I am able to perform over ODPV0.7.
>
>
>
> Regards
>
> Ayushman
>
>
>
>
>
> ___
> lng-odp mailing list
> lng-odp@lists.linaro.org
> http://lists.linaro.org/mailman/listinfo/lng-odp
>
>
___
lng-odp mailing list
lng-odp@lists.linaro.org
http://lists.linaro.org/mailman/listinfo/lng-odp

Re: [lng-odp] [PATCH 0/3] Fix the leaks in examples (bug 1117)

[Mike Holmes]

On 11 March 2015 at 10:32, Ola Liljedahl  wrote:

> These examples won't leak memory continuously so why is this a problem?
> Are we catering to e.g. bare metal environments which may not clean up
> after applications have terminated?
>

I think the tests should be clean so they can be reused in bare metal
environment without too much trouble, but also I agree we only test on
linux-generic.
However on  linux generic tools such as clang-scan, Coverity, Valgrind etc
are run to catch mistakes, that is a lot easier if the code is inherently
clean in those tools to begin with.

In some cases we need to mark false positives, in others add some clean up
- this is one of those clean up items from Coverity
https://bugs.linaro.org/show_bug.cgi?id=1117

Mike




>
>
> On 11 March 2015 at 14:52, Ciprian Barbu  wrote:
>
>> This series continues the patch I sent some while ago to replace strtok
>> with
>> strtok_r in odp_pktio:
>>
>> https://git.linaro.org/lng/odp.git/commit/273955e6db6bb220f2736d3709e4237c50d04772
>>
>> The bug that this series fixes only refers to resource leaks, but the
>> initial
>> strtok_r fix should have been done for all examples, there is no need to
>> use the
>> reentrant version of strtok because it is only used from one thread, once,
>> during parsing of args.
>>
>> There was a suggestion at some point to make the parsing of if_names
>> common for
>> all examples, I don't think it's easy to factor it out, it would
>> introduce more
>> code than necessary and we're only talking about a few lines of code.
>>
>> Ciprian Barbu (3):
>>   example: odp_l2fwd: replace strtok_r with strtok and fix leaks
>>   example: odp_ipsec: replace strtok_r with strtok and fix leaks
>>   example: generator: replace strtok_r with strtok and fix leaks
>>
>>  example/generator/odp_generator.c | 30 --
>>  example/ipsec/odp_ipsec.c | 30 ++
>>  example/l2fwd/odp_l2fwd.c | 29 +++--
>>  3 files changed, 45 insertions(+), 44 deletions(-)
>>
>> --
>> 1.8.3.2
>>
>>
>> ___
>> lng-odp mailing list
>> lng-odp@lists.linaro.org
>> http://lists.linaro.org/mailman/listinfo/lng-odp
>>
>
>
> ___
> lng-odp mailing list
> lng-odp@lists.linaro.org
> http://lists.linaro.org/mailman/listinfo/lng-odp
>
>


-- 
Mike Holmes
Technical Manager - Linaro Networking Group
Linaro.org  *│ *Open source software for ARM SoCs
___
lng-odp mailing list
lng-odp@lists.linaro.org
http://lists.linaro.org/mailman/listinfo/lng-odp

Re: [lng-odp] [PATCHv2] linux-generic: odp_timer: set user_ptr for cancelled timeout

[Bill Fischofer]

I originally was going to point out the 'typo' but my first reaction was
that if should be of rather than in.  However, on reflection it seemed that
if was what was intended.  However, I agree that this revised wording is
clearer.


On Wed, Mar 11, 2015 at 10:36 AM, Ola Liljedahl 
wrote:

> Ensure that the timeout user_ptr and timer fields are set when the
> corresponding timer is immediately cancelled.
> https://bugs.linaro.org/show_bug.cgi?id=1313
>
> Signed-off-by: Ola Liljedahl 
>

Reviewed-by: Bill Fischofer 


> ---
> (This document/code contribution attached is provided under the terms of
> agreement LES-LTM-21309)
>
> Passes odp_timer validation with the new odp_timer_cancel() test from
> Petri.
>
> v2:
> Updated some comments in odp_timer.c to make the meaning clearer.
>
>  platform/linux-generic/odp_timer.c | 18 ++
>  1 file changed, 14 insertions(+), 4 deletions(-)
>
> diff --git a/platform/linux-generic/odp_timer.c
> b/platform/linux-generic/odp_timer.c
> index 61a02b6..b7cb04f 100644
> --- a/platform/linux-generic/odp_timer.c
> +++ b/platform/linux-generic/odp_timer.c
> @@ -421,6 +421,16 @@ static bool timer_reset(uint32_t idx,
>  #endif
> } else {
> /* We have a new timeout buffer which replaces any old one
> */
> +   /* Fill in some (constant) header fields for timeout
> events */
> +   if (_odp_buffer_type(*tmo_buf) == ODP_EVENT_TIMEOUT) {
> +   /* Convert from buffer to timeout hdr */
> +   odp_timeout_hdr_t *tmo_hdr =
> +   timeout_hdr_from_buf(*tmo_buf);
> +   tmo_hdr->timer = tp_idx_to_handle(tp, idx);
> +   tmo_hdr->user_ptr = tp->timers[idx].user_ptr;
> +   /* expiration field filled in when timer expires */
> +   }
> +   /* Else ignore buffers of other types */
> odp_buffer_t old_buf = ODP_BUFFER_INVALID;
>  #ifdef ODP_ATOMIC_U128
> tick_buf_t new, old;
> @@ -556,16 +566,16 @@ static unsigned timer_expire(odp_timer_pool *tp,
> uint32_t idx, uint64_t tick)
> _odp_atomic_flag_clear(IDX2LOCK(idx));
>  #endif
> if (odp_likely(tmo_buf != ODP_BUFFER_INVALID)) {
> -   /* Fill in metadata fields in system timeout buffer */
> +   /* Fill in expiration tick for timeout events */
> if (_odp_buffer_type(tmo_buf) == ODP_EVENT_TIMEOUT) {
> /* Convert from buffer to timeout hdr */
> odp_timeout_hdr_t *tmo_hdr =
> timeout_hdr_from_buf(tmo_buf);
> -   tmo_hdr->timer = tp_idx_to_handle(tp, idx);
> tmo_hdr->expiration = exp_tck;
> -   tmo_hdr->user_ptr = tim->user_ptr;
> +   /* timer and user_ptr fields filled in when timer
> +* was set */
> }
> -   /* Else ignore buffers of other types */
> +   /* Else ignore events of other types */
> /* Post the timeout to the destination queue */
> int rc = odp_queue_enq(tim->queue,
>odp_buffer_to_event(tmo_buf));
> --
> 1.9.1
>
>
> ___
> lng-odp mailing list
> lng-odp@lists.linaro.org
> http://lists.linaro.org/mailman/listinfo/lng-odp
>
___
lng-odp mailing list
lng-odp@lists.linaro.org
http://lists.linaro.org/mailman/listinfo/lng-odp

Re: [lng-odp] [PATCH] validation: schedule: code clean up

[Bill Fischofer]

On Wed, Mar 11, 2015 at 7:59 AM, Petri Savolainen <
petri.savolai...@nokia.com> wrote:

> Cleaned up usage of buf count, thread args and globals.
>
> Signed-off-by: Petri Savolainen 
>

Reviewed-and-tested-by: Bill Fischofer 


> ---
>  test/validation/odp_schedule.c | 102
> -
>  1 file changed, 49 insertions(+), 53 deletions(-)
>
> diff --git a/test/validation/odp_schedule.c
> b/test/validation/odp_schedule.c
> index 3035db8..a9369c5 100644
> --- a/test/validation/odp_schedule.c
> +++ b/test/validation/odp_schedule.c
> @@ -38,14 +38,14 @@
>  typedef struct {
> int cpu_count;
> odp_barrier_t barrier;
> -   odp_schedule_prio_t current_prio;
> -   int prio_buf_count;
> -   odp_ticketlock_t count_lock;
> +   int buf_count;
> +   odp_ticketlock_t lock;
> odp_spinlock_t atomic_lock;
>  } test_globals_t;
>
> -typedef struct ODP_PACKED {
> -   pthrd_arg thrdarg;
> +typedef struct {
> +   pthrd_arg cu_thr;
> +   test_globals_t *globals;
> odp_schedule_sync_t sync;
> int num_queues;
> int num_prio;
> @@ -84,22 +84,13 @@ static void *schedule_common_(void *arg)
>  {
> thread_args_t *args = (thread_args_t *)arg;
> odp_schedule_sync_t sync;
> -   int num_queues, num_prio, num_bufs, num_cpus;
> -   odp_shm_t shm;
> +   int num_cpus;
> test_globals_t *globals;
>
> +   globals = args->globals;
> sync = args->sync;
> -   num_queues = args->num_queues;
> -   num_prio = args->num_prio;
> -   num_bufs = args->num_bufs;
> num_cpus = args->num_cpus;
>
> -   shm = odp_shm_lookup(GLOBALS_SHM_NAME);
> -   CU_ASSERT_FATAL(shm != ODP_SHM_INVALID);
> -   globals = odp_shm_addr(shm);
> -   CU_ASSERT_FATAL(globals != NULL);
> -
> -
> if (num_cpus == globals->cpu_count)
> odp_barrier_wait(&globals->barrier);
>
> @@ -110,13 +101,12 @@ static void *schedule_common_(void *arg)
> int num = 0;
> int locked;
>
> -   odp_ticketlock_lock(&globals->count_lock);
> -   if (globals->prio_buf_count ==
> -   num_bufs * num_queues * num_prio) {
> -   odp_ticketlock_unlock(&globals->count_lock);
> +   odp_ticketlock_lock(&globals->lock);
> +   if (globals->buf_count == 0) {
> +   odp_ticketlock_unlock(&globals->lock);
> break;
> }
> -   odp_ticketlock_unlock(&globals->count_lock);
> +   odp_ticketlock_unlock(&globals->lock);
>
> if (args->enable_schd_multi) {
> odp_event_t events[BURST_BUF_SIZE];
> @@ -155,13 +145,18 @@ static void *schedule_common_(void *arg)
> }
> }
>
> -   odp_ticketlock_lock(&globals->count_lock);
> -   globals->prio_buf_count += num;
> -
> if (sync == ODP_SCHED_SYNC_ATOMIC)
> odp_schedule_release_atomic();
>
> -   odp_ticketlock_unlock(&globals->count_lock);
> +   odp_ticketlock_lock(&globals->lock);
> +   globals->buf_count -= num;
> +
> +   if (globals->buf_count < 0) {
> +   odp_ticketlock_unlock(&globals->lock);
> +   CU_FAIL_FATAL("Buffer counting failed");
> +   }
> +
> +   odp_ticketlock_unlock(&globals->lock);
> }
>
> return NULL;
> @@ -173,8 +168,11 @@ static void fill_queues(thread_args_t *args)
> int num_queues, num_prio;
> odp_pool_t pool;
> int i, j, k;
> +   int buf_count = 0;
> +   test_globals_t *globals;
> char name[32];
>
> +   globals = args->globals;
> sync = args->sync;
> num_queues = args->num_queues;
> num_prio = args->num_prio;
> @@ -214,9 +212,12 @@ static void fill_queues(thread_args_t *args)
> CU_ASSERT(buf != ODP_BUFFER_INVALID);
> ev = odp_buffer_to_event(buf);
> CU_ASSERT(odp_queue_enq(queue, ev) == 0);
> +   buf_count++;
> }
> }
> }
> +
> +   globals->buf_count = buf_count;
>  }
>
>  static void schedule_common(odp_schedule_sync_t sync, int num_queues,
> @@ -231,9 +232,7 @@ static void schedule_common(odp_schedule_sync_t sync,
> int num_queues,
> globals = odp_shm_addr(shm);
> CU_ASSERT_FATAL(globals != NULL);
>
> -   globals->current_prio = ODP_SCHED_PRIO_HIGHEST;
> -   globals->prio_buf_count = 0;
> -
> +   args.globals = globals;
> args.sync = sync;
> args.num_queues = num_queues;
> args.num_prio = num_prio;
> @@ -253,7 +252,7 @@ static void parallel_execute(odp_schedule_sync_t sync,
> int num_queues,
>  {
> odp

[lng-odp] [PATCH ARCH] remove api_guide_lines

[Mike Holmes]

This docuemtnation has been moved to the API document

Signed-off-by: Mike Holmes 
---
 api_guide_lines.dox | 178 
 1 file changed, 178 deletions(-)
 delete mode 100644 api_guide_lines.dox

diff --git a/api_guide_lines.dox b/api_guide_lines.dox
deleted file mode 100644
index 4cfe088..000
--- a/api_guide_lines.dox
+++ /dev/null
@@ -1,178 +0,0 @@
-/* Copyright (c) 2014, Linaro Limited
-
- * All rights reserved
- *
- * SPDX-License-Identifier: BSD-3-Clause
- */
-
-/**
-
-@page api_guide_lines  API Guide Lines
-
-@tableofcontents
-
-@section introduction Introduction
-ODP APIs are implemented as callable C functions that often return a typed 
value.
-This document describes the approach to handling return values and error 
indications expected of conforming ODP implementations.
-As such it should be regarded as providing guidelines for how to create new 
ODP APIs.
-
-@section functional Functional Definition
-This section defines the use of data types, calling conventions, and return 
codes used by ODP APIs.
-All ODP APIs MUST follow these conventions as part of their design.
-
-@subsection naming Naming Conventions
-All ODP APIs begin with the prefix odp_ and those that describe an action to 
be performed on an object follow the naming convention of object followed by 
action.
-The advantage of this approach is that an alphabetical list of APIs for an 
object all sort together since they all have names of the form 
odp_object_action().
-
-So for example the API call to allocate a buffer is named odp_buffer_alloc() 
rather than odp_alloc_buffer().
-
-@subsection data_types Data Types and Use of typedef
-ODP is designed to allow broad variability in how APIs are implemented on 
various platforms.
-To support this, most APIs operate on abstract data types that are defined via 
typedef on a per-implementation basis.
-These abstract types follow the naming convention of odp_object_t.
-
-Typedefs that encapsulate C structs follow the convention:
-
-@code
-typedef struct odp__s {
-...
-} odp__t;
-@endcode
-
-The use of typedef allows implementations to choose underlying data 
representations that map efficiently to platform capabilities while providing 
accessor functions to provide structured access to implementation information 
in a portable manner
-Similarly, the use of enum is RECOMMENDED to provide value abstraction for API 
parameters while enabling the implementation to choose code points that map 
well to platform native values.
-
-Several native C types are used conventionally within ODP and SHOULD be 
employed in API design:
-
-type | Correct use
- |---| :-
-void | SHOULD be used for APIs that do not return a value
-void*| SHOULD be used for APIs that return a pointer intended to be used by 
the caller. For example, a routine that returns the address of an application 
context area SHOULD use a void * return type
-odp_bool_t  | SHOULD be used for APIs that return a @ref boolean value.
-int  | SHOULD be used for success and failure indications, with 0 indicating a 
success. Errno may be set
-
-@subsection parameters Parameter Structure and Validation
-ODP is a framework for use in the data plane.
-Data plane applications typically have extreme performance requirements 
mandating very strict attention to path length considerations in the design of 
all ODP APIs, with the exception of those designed to be used infrequently such 
as only during initialization or termination processing.
-
-Minimizing pathlength in API design involves several considerations:
- - The number of parameters passed to a call.
-   In general, ODP APIs designed for frequent use SHOULD have few parameters.
-   Limiting parameter count to one or two well-chosen parameters SHOULD be the 
goal for APIs designed for frequent use.
-   If a call requires more complex parameter data then it is RECOMMENDED that 
instead of multiple parameters a single pointer to a struct that can be 
statically templated and modified by the caller be used.
- - The use of macros and inlining.
-   ODP APIs MAY be implemented as preprocessor macros and/or inline functions.
-   This is especially true for accessor functions that are designed to provide 
getters/setters for object meta data.
- - Limiting parameter validation and error-checking processing.
-   While useful for development and debugging, providing “bullet-proof” APIs 
that perform extensive parameter validation and error checking is often 
inappropriate.
-   While validations that can be performed statically at compile time or at 
little to no runtime cost SHOULD be considered, APIs MAY choose to leave 
behavior as undefined when presented with invalid parameters in the interest of 
runtime efficiency.
-
-One of the reasons for using abstract types is to avoid having implementation 
knowledge “bleed through” the API, leading to possible parameter errors.
-When one API returns an opaque token to an application it is reasonable to 
exp

Re: [lng-odp] [PATCH] doc: move api guidelines to API doc

[Bill Fischofer]

On Wed, Mar 11, 2015 at 1:24 PM, Mike Holmes  wrote:

> This documentation was moved from the architecture doc to this API doc.
>
> Signed-off-by: Mike Holmes 
>

Reviewed-by: Bill Fischofer 


> ---
>  doc/api_guide_lines.dox | 178
> 
>  1 file changed, 178 insertions(+)
>  create mode 100644 doc/api_guide_lines.dox
>
> diff --git a/doc/api_guide_lines.dox b/doc/api_guide_lines.dox
> new file mode 100644
> index 000..4cfe088
> --- /dev/null
> +++ b/doc/api_guide_lines.dox
> @@ -0,0 +1,178 @@
> +/* Copyright (c) 2014, Linaro Limited
> +
> + * All rights reserved
> + *
> + * SPDX-License-Identifier: BSD-3-Clause
> + */
> +
> +/**
> +
> +@page api_guide_lines  API Guide Lines
> +
> +@tableofcontents
> +
> +@section introduction Introduction
> +ODP APIs are implemented as callable C functions that often return a
> typed value.
> +This document describes the approach to handling return values and error
> indications expected of conforming ODP implementations.
> +As such it should be regarded as providing guidelines for how to create
> new ODP APIs.
> +
> +@section functional Functional Definition
> +This section defines the use of data types, calling conventions, and
> return codes used by ODP APIs.
> +All ODP APIs MUST follow these conventions as part of their design.
> +
> +@subsection naming Naming Conventions
> +All ODP APIs begin with the prefix odp_ and those that describe an action
> to be performed on an object follow the naming convention of object
> followed by action.
> +The advantage of this approach is that an alphabetical list of APIs for
> an object all sort together since they all have names of the form
> odp_object_action().
> +
> +So for example the API call to allocate a buffer is named
> odp_buffer_alloc() rather than odp_alloc_buffer().
> +
> +@subsection data_types Data Types and Use of typedef
> +ODP is designed to allow broad variability in how APIs are implemented on
> various platforms.
> +To support this, most APIs operate on abstract data types that are
> defined via typedef on a per-implementation basis.
> +These abstract types follow the naming convention of odp_object_t.
> +
> +Typedefs that encapsulate C structs follow the convention:
> +
> +@code
> +typedef struct odp__s {
> +...
> +} odp__t;
> +@endcode
> +
> +The use of typedef allows implementations to choose underlying data
> representations that map efficiently to platform capabilities while
> providing accessor functions to provide structured access to implementation
> information in a portable manner
> +Similarly, the use of enum is RECOMMENDED to provide value abstraction
> for API parameters while enabling the implementation to choose code points
> that map well to platform native values.
> +
> +Several native C types are used conventionally within ODP and SHOULD be
> employed in API design:
> +
> +type | Correct use
> + |---| :-
> +void | SHOULD be used for APIs that do not return a value
> +void*| SHOULD be used for APIs that return a pointer intended to be used
> by the caller. For example, a routine that returns the address of an
> application context area SHOULD use a void * return type
> +odp_bool_t  | SHOULD be used for APIs that return a @ref boolean value.
> +int  | SHOULD be used for success and failure indications, with 0
> indicating a success. Errno may be set
> +
> +@subsection parameters Parameter Structure and Validation
> +ODP is a framework for use in the data plane.
> +Data plane applications typically have extreme performance requirements
> mandating very strict attention to path length considerations in the design
> of all ODP APIs, with the exception of those designed to be used
> infrequently such as only during initialization or termination processing.
> +
> +Minimizing pathlength in API design involves several considerations:
> + - The number of parameters passed to a call.
> +   In general, ODP APIs designed for frequent use SHOULD have few
> parameters.
> +   Limiting parameter count to one or two well-chosen parameters SHOULD
> be the goal for APIs designed for frequent use.
> +   If a call requires more complex parameter data then it is RECOMMENDED
> that instead of multiple parameters a single pointer to a struct that can
> be statically templated and modified by the caller be used.
> + - The use of macros and inlining.
> +   ODP APIs MAY be implemented as preprocessor macros and/or inline
> functions.
> +   This is especially true for accessor functions that are designed to
> provide getters/setters for object meta data.
> + - Limiting parameter validation and error-checking processing.
> +   While useful for development and debugging, providing “bullet-proof”
> APIs that perform extensive parameter validation and error checking is
> often inappropriate.
> +   While validations that can be performed statically at compile time or
> at little to no runtime cost SHOULD be considered, APIs MAY choose to leave
> behavior as undefi

Re: [lng-odp] [PATCH ARCH] remove api_guide_lines

[Bill Fischofer]

On Wed, Mar 11, 2015 at 1:30 PM, Mike Holmes  wrote:

> This docuemtnation has been moved to the API document
>
> Signed-off-by: Mike Holmes 
>

Reviewed-by: Bill Fischofer 


> ---
>  api_guide_lines.dox | 178
> 
>  1 file changed, 178 deletions(-)
>  delete mode 100644 api_guide_lines.dox
>
> diff --git a/api_guide_lines.dox b/api_guide_lines.dox
> deleted file mode 100644
> index 4cfe088..000
> --- a/api_guide_lines.dox
> +++ /dev/null
> @@ -1,178 +0,0 @@
> -/* Copyright (c) 2014, Linaro Limited
> -
> - * All rights reserved
> - *
> - * SPDX-License-Identifier: BSD-3-Clause
> - */
> -
> -/**
> -
> -@page api_guide_lines  API Guide Lines
> -
> -@tableofcontents
> -
> -@section introduction Introduction
> -ODP APIs are implemented as callable C functions that often return a
> typed value.
> -This document describes the approach to handling return values and error
> indications expected of conforming ODP implementations.
> -As such it should be regarded as providing guidelines for how to create
> new ODP APIs.
> -
> -@section functional Functional Definition
> -This section defines the use of data types, calling conventions, and
> return codes used by ODP APIs.
> -All ODP APIs MUST follow these conventions as part of their design.
> -
> -@subsection naming Naming Conventions
> -All ODP APIs begin with the prefix odp_ and those that describe an action
> to be performed on an object follow the naming convention of object
> followed by action.
> -The advantage of this approach is that an alphabetical list of APIs for
> an object all sort together since they all have names of the form
> odp_object_action().
> -
> -So for example the API call to allocate a buffer is named
> odp_buffer_alloc() rather than odp_alloc_buffer().
> -
> -@subsection data_types Data Types and Use of typedef
> -ODP is designed to allow broad variability in how APIs are implemented on
> various platforms.
> -To support this, most APIs operate on abstract data types that are
> defined via typedef on a per-implementation basis.
> -These abstract types follow the naming convention of odp_object_t.
> -
> -Typedefs that encapsulate C structs follow the convention:
> -
> -@code
> -typedef struct odp__s {
> -...
> -} odp__t;
> -@endcode
> -
> -The use of typedef allows implementations to choose underlying data
> representations that map efficiently to platform capabilities while
> providing accessor functions to provide structured access to implementation
> information in a portable manner
> -Similarly, the use of enum is RECOMMENDED to provide value abstraction
> for API parameters while enabling the implementation to choose code points
> that map well to platform native values.
> -
> -Several native C types are used conventionally within ODP and SHOULD be
> employed in API design:
> -
> -type | Correct use
> - |---| :-
> -void | SHOULD be used for APIs that do not return a value
> -void*| SHOULD be used for APIs that return a pointer intended to be used
> by the caller. For example, a routine that returns the address of an
> application context area SHOULD use a void * return type
> -odp_bool_t  | SHOULD be used for APIs that return a @ref boolean value.
> -int  | SHOULD be used for success and failure indications, with 0
> indicating a success. Errno may be set
> -
> -@subsection parameters Parameter Structure and Validation
> -ODP is a framework for use in the data plane.
> -Data plane applications typically have extreme performance requirements
> mandating very strict attention to path length considerations in the design
> of all ODP APIs, with the exception of those designed to be used
> infrequently such as only during initialization or termination processing.
> -
> -Minimizing pathlength in API design involves several considerations:
> - - The number of parameters passed to a call.
> -   In general, ODP APIs designed for frequent use SHOULD have few
> parameters.
> -   Limiting parameter count to one or two well-chosen parameters SHOULD
> be the goal for APIs designed for frequent use.
> -   If a call requires more complex parameter data then it is RECOMMENDED
> that instead of multiple parameters a single pointer to a struct that can
> be statically templated and modified by the caller be used.
> - - The use of macros and inlining.
> -   ODP APIs MAY be implemented as preprocessor macros and/or inline
> functions.
> -   This is especially true for accessor functions that are designed to
> provide getters/setters for object meta data.
> - - Limiting parameter validation and error-checking processing.
> -   While useful for development and debugging, providing “bullet-proof”
> APIs that perform extensive parameter validation and error checking is
> often inappropriate.
> -   While validations that can be performed statically at compile time or
> at little to no runtime cost SHOULD be considered, APIs MAY choose to leave
> behavior as undefined when presented with invalid

[lng-odp] [Bug 1284] test/validation/odp_schedule fails to terminate cleanly

[bugzilla-daemon]

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

Mike Holmes  changed:

   What|Removed |Added

   Assignee|petri.savolai...@linaro.org |ciprian.ba...@linaro.org

-- 
You are receiving this mail because:
You are on the CC list for the bug.___
lng-odp mailing list
lng-odp@lists.linaro.org
http://lists.linaro.org/mailman/listinfo/lng-odp

Re: [lng-odp] [PATCH] validation: pktio: prevent race when using veth pair

[Mike Holmes]

I simultaneously ran two independent  instances of odp with "make check"
and neither had an issue, one had this patch the other did not.

At the same time I used the ./build.sh script from the cleanup branch of
https://git.linaro.org/people/anders.roxell/check-odp.git and it was
pointed at a patched repo
It failed consistently as before
..

PASS: odp_errno
FAIL: odp_pktio_run
make[5]: Entering directory
'/home/mike/git/check-odp/build/odp/testdir/test/validation'
make[5]: Nothing to be done for 'all'.
make[5]: Leaving directory
'/home/mike/git/check-odp/build/odp/testdir/test/validation'

Testsuite summary for OpenDataPlane 1.0.0

# TOTAL: 18
# PASS:  17
# SKIP:  0
# XFAIL: 0
# FAIL:  1
# XPASS: 0
# ERROR: 0


I don't know why this should be because I ran my local instances with the
script
for i in {1..10}; do make clean check -e; done
and the build.sh uses
make check -e - but obviously it does this after building from scratch
downloading the repo etc each time so it does fewer iterations

So I think the new approach is a better idea, and it does not appear to
hurt, but it does not help the original use case of the scrip tin CI.


On 11 March 2015 at 10:22, Stuart Haslam  wrote:

> On Thu, Mar 05, 2015 at 12:12:31PM +, Stuart Haslam wrote:
> > There's a potential race condition whereby the test case could start
> > running before the virtual ethernet interfaces are fully brought up.
> > So replace the the arbitrary delay with a check for the interface's
> > operational state before kicking off the test.
> >
>
> Can someone review this please?
>
> > Signed-off-by: Stuart Haslam 
> > ---
> >  test/validation/odp_pktio_run | 32 ++--
> >  1 file changed, 30 insertions(+), 2 deletions(-)
> >
> > diff --git a/test/validation/odp_pktio_run
> b/test/validation/odp_pktio_run
> > index 08288e6..b051be1 100755
> > --- a/test/validation/odp_pktio_run
> > +++ b/test/validation/odp_pktio_run
> > @@ -34,6 +34,28 @@ IF1=pktio-p1
> >  # exit codes expected by automake for skipped tests
> >  TEST_SKIPPED=77
> >
> > +# wait for a network interface's operational state to be "up"
> > +wait_for_iface_up()
> > +{
> > + iface=$1
> > + cnt=0
> > +
> > + while [ $cnt -lt 50 ]; do
> > + read operstate < /sys/class/net/$iface/operstate
> > +
> > + if [ $? != 0 ]; then
> > + break
> > + elif [ "$operstate" = "up" ]; then
> > + return 0
> > + fi
> > +
> > + sleep 0.1
> > + cnt=$[$cnt+1]
> > + done
> > +
> > + return 1
> > +}
> > +
> >  setup_env1()
> >  {
> >   ip link show $IF0 2> /dev/null
> > @@ -59,8 +81,14 @@ setup_env1()
> >   ip link set $IF0 up
> >   ip link set $IF1 up
> >
> > - # network needs a little time to come up
> > - sleep 1
> > + # check that the interface has come up before starting the test
> > + for iface in $IF0 $IF1; do
> > + wait_for_iface_up $iface
> > + if [ $? != 0 ]; then
> > + echo "pktio: interface $iface failed to come up"
> > + exit 1
> > + fi
> > + done
> >  }
> >
> >  cleanup_env1()
> > --
> > 2.1.1
> >
>
> --
> Stuart.
>
> ___
> lng-odp mailing list
> lng-odp@lists.linaro.org
> http://lists.linaro.org/mailman/listinfo/lng-odp
>



-- 
Mike Holmes
Technical Manager - Linaro Networking Group
Linaro.org  *│ *Open source software for ARM SoCs
___
lng-odp mailing list
lng-odp@lists.linaro.org
http://lists.linaro.org/mailman/listinfo/lng-odp

Re: [lng-odp] [PATCH] validation: pktio: prevent race when using veth pair

[Mike Holmes]

I never run it as root usually, but I did just now to check.
Again build.sh had errors and running the for loop directly did not


On 11 March 2015 at 17:43, Maxim Uvarov  wrote:

> On 03/11/15 23:47, Mike Holmes wrote:
>
>> for i in {1..10}; do make clean check -e; done
>>
> Mike do you run that under root or not? I.e. which pktio in test odp loop
> or linux veth?
>
> Maxim.
>
>
> ___
> lng-odp mailing list
> lng-odp@lists.linaro.org
> http://lists.linaro.org/mailman/listinfo/lng-odp
>



-- 
Mike Holmes
Technical Manager - Linaro Networking Group
Linaro.org  *│ *Open source software for ARM SoCs
___
lng-odp mailing list
lng-odp@lists.linaro.org
http://lists.linaro.org/mailman/listinfo/lng-odp

[lng-odp] [PATCH] doc: move api guidelines to API doc

[Mike Holmes]

This documentation was moved from the architecture doc to this API doc.

Signed-off-by: Mike Holmes 
---
 doc/api_guide_lines.dox | 178 
 1 file changed, 178 insertions(+)
 create mode 100644 doc/api_guide_lines.dox

diff --git a/doc/api_guide_lines.dox b/doc/api_guide_lines.dox
new file mode 100644
index 000..4cfe088
--- /dev/null
+++ b/doc/api_guide_lines.dox
@@ -0,0 +1,178 @@
+/* Copyright (c) 2014, Linaro Limited
+
+ * All rights reserved
+ *
+ * SPDX-License-Identifier: BSD-3-Clause
+ */
+
+/**
+
+@page api_guide_lines  API Guide Lines
+
+@tableofcontents
+
+@section introduction Introduction
+ODP APIs are implemented as callable C functions that often return a typed 
value.
+This document describes the approach to handling return values and error 
indications expected of conforming ODP implementations.
+As such it should be regarded as providing guidelines for how to create new 
ODP APIs.
+
+@section functional Functional Definition
+This section defines the use of data types, calling conventions, and return 
codes used by ODP APIs.
+All ODP APIs MUST follow these conventions as part of their design.
+
+@subsection naming Naming Conventions
+All ODP APIs begin with the prefix odp_ and those that describe an action to 
be performed on an object follow the naming convention of object followed by 
action.
+The advantage of this approach is that an alphabetical list of APIs for an 
object all sort together since they all have names of the form 
odp_object_action().
+
+So for example the API call to allocate a buffer is named odp_buffer_alloc() 
rather than odp_alloc_buffer().
+
+@subsection data_types Data Types and Use of typedef
+ODP is designed to allow broad variability in how APIs are implemented on 
various platforms.
+To support this, most APIs operate on abstract data types that are defined via 
typedef on a per-implementation basis.
+These abstract types follow the naming convention of odp_object_t.
+
+Typedefs that encapsulate C structs follow the convention:
+
+@code
+typedef struct odp__s {
+...
+} odp__t;
+@endcode
+
+The use of typedef allows implementations to choose underlying data 
representations that map efficiently to platform capabilities while providing 
accessor functions to provide structured access to implementation information 
in a portable manner
+Similarly, the use of enum is RECOMMENDED to provide value abstraction for API 
parameters while enabling the implementation to choose code points that map 
well to platform native values.
+
+Several native C types are used conventionally within ODP and SHOULD be 
employed in API design:
+
+type | Correct use
+ |---| :-
+void | SHOULD be used for APIs that do not return a value
+void*| SHOULD be used for APIs that return a pointer intended to be used by 
the caller. For example, a routine that returns the address of an application 
context area SHOULD use a void * return type
+odp_bool_t  | SHOULD be used for APIs that return a @ref boolean value.
+int  | SHOULD be used for success and failure indications, with 0 indicating a 
success. Errno may be set
+
+@subsection parameters Parameter Structure and Validation
+ODP is a framework for use in the data plane.
+Data plane applications typically have extreme performance requirements 
mandating very strict attention to path length considerations in the design of 
all ODP APIs, with the exception of those designed to be used infrequently such 
as only during initialization or termination processing.
+
+Minimizing pathlength in API design involves several considerations:
+ - The number of parameters passed to a call.
+   In general, ODP APIs designed for frequent use SHOULD have few parameters.
+   Limiting parameter count to one or two well-chosen parameters SHOULD be the 
goal for APIs designed for frequent use.
+   If a call requires more complex parameter data then it is RECOMMENDED that 
instead of multiple parameters a single pointer to a struct that can be 
statically templated and modified by the caller be used.
+ - The use of macros and inlining.
+   ODP APIs MAY be implemented as preprocessor macros and/or inline functions.
+   This is especially true for accessor functions that are designed to provide 
getters/setters for object meta data.
+ - Limiting parameter validation and error-checking processing.
+   While useful for development and debugging, providing “bullet-proof” APIs 
that perform extensive parameter validation and error checking is often 
inappropriate.
+   While validations that can be performed statically at compile time or at 
little to no runtime cost SHOULD be considered, APIs MAY choose to leave 
behavior as undefined when presented with invalid parameters in the interest of 
runtime efficiency.
+
+One of the reasons for using abstract types is to avoid having implementation 
knowledge “bleed through” the API, leading to possible parameter errors.
+When one API returns an opaque token to an appli

Re: [lng-odp] [PATCH] doc: move api guidelines to API doc

[Mike Holmes]

Please ignore this

On 11 March 2015 at 19:25, Mike Holmes  wrote:

> This documentation was moved from the architecture doc to this API doc.
>
> Signed-off-by: Mike Holmes 
> ---
>  doc/api_guide_lines.dox | 178
> 
>  1 file changed, 178 insertions(+)
>  create mode 100644 doc/api_guide_lines.dox
>
> diff --git a/doc/api_guide_lines.dox b/doc/api_guide_lines.dox
> new file mode 100644
> index 000..4cfe088
> --- /dev/null
> +++ b/doc/api_guide_lines.dox
> @@ -0,0 +1,178 @@
> +/* Copyright (c) 2014, Linaro Limited
> +
> + * All rights reserved
> + *
> + * SPDX-License-Identifier: BSD-3-Clause
> + */
> +
> +/**
> +
> +@page api_guide_lines  API Guide Lines
> +
> +@tableofcontents
> +
> +@section introduction Introduction
> +ODP APIs are implemented as callable C functions that often return a
> typed value.
> +This document describes the approach to handling return values and error
> indications expected of conforming ODP implementations.
> +As such it should be regarded as providing guidelines for how to create
> new ODP APIs.
> +
> +@section functional Functional Definition
> +This section defines the use of data types, calling conventions, and
> return codes used by ODP APIs.
> +All ODP APIs MUST follow these conventions as part of their design.
> +
> +@subsection naming Naming Conventions
> +All ODP APIs begin with the prefix odp_ and those that describe an action
> to be performed on an object follow the naming convention of object
> followed by action.
> +The advantage of this approach is that an alphabetical list of APIs for
> an object all sort together since they all have names of the form
> odp_object_action().
> +
> +So for example the API call to allocate a buffer is named
> odp_buffer_alloc() rather than odp_alloc_buffer().
> +
> +@subsection data_types Data Types and Use of typedef
> +ODP is designed to allow broad variability in how APIs are implemented on
> various platforms.
> +To support this, most APIs operate on abstract data types that are
> defined via typedef on a per-implementation basis.
> +These abstract types follow the naming convention of odp_object_t.
> +
> +Typedefs that encapsulate C structs follow the convention:
> +
> +@code
> +typedef struct odp__s {
> +...
> +} odp__t;
> +@endcode
> +
> +The use of typedef allows implementations to choose underlying data
> representations that map efficiently to platform capabilities while
> providing accessor functions to provide structured access to implementation
> information in a portable manner
> +Similarly, the use of enum is RECOMMENDED to provide value abstraction
> for API parameters while enabling the implementation to choose code points
> that map well to platform native values.
> +
> +Several native C types are used conventionally within ODP and SHOULD be
> employed in API design:
> +
> +type | Correct use
> + |---| :-
> +void | SHOULD be used for APIs that do not return a value
> +void*| SHOULD be used for APIs that return a pointer intended to be used
> by the caller. For example, a routine that returns the address of an
> application context area SHOULD use a void * return type
> +odp_bool_t  | SHOULD be used for APIs that return a @ref boolean value.
> +int  | SHOULD be used for success and failure indications, with 0
> indicating a success. Errno may be set
> +
> +@subsection parameters Parameter Structure and Validation
> +ODP is a framework for use in the data plane.
> +Data plane applications typically have extreme performance requirements
> mandating very strict attention to path length considerations in the design
> of all ODP APIs, with the exception of those designed to be used
> infrequently such as only during initialization or termination processing.
> +
> +Minimizing pathlength in API design involves several considerations:
> + - The number of parameters passed to a call.
> +   In general, ODP APIs designed for frequent use SHOULD have few
> parameters.
> +   Limiting parameter count to one or two well-chosen parameters SHOULD
> be the goal for APIs designed for frequent use.
> +   If a call requires more complex parameter data then it is RECOMMENDED
> that instead of multiple parameters a single pointer to a struct that can
> be statically templated and modified by the caller be used.
> + - The use of macros and inlining.
> +   ODP APIs MAY be implemented as preprocessor macros and/or inline
> functions.
> +   This is especially true for accessor functions that are designed to
> provide getters/setters for object meta data.
> + - Limiting parameter validation and error-checking processing.
> +   While useful for development and debugging, providing “bullet-proof”
> APIs that perform extensive parameter validation and error checking is
> often inappropriate.
> +   While validations that can be performed statically at compile time or
> at little to no runtime cost SHOULD be considered, APIs MAY choose to leave
> behavior as undefined when presented w

[lng-odp] [PATCH] CONTRIBUTING: add clarification on code style

[Mike Holmes]

Clarify the locations that variables may be declared

Signed-off-by: Mike Holmes 
---
 CONTRIBUTING | 15 +++
 1 file changed, 15 insertions(+)

diff --git a/CONTRIBUTING b/CONTRIBUTING
index 75fb711..ca1ce3f 100644
--- a/CONTRIBUTING
+++ b/CONTRIBUTING
@@ -16,6 +16,21 @@ ODP code shall be documented using the doxygen style 
described in the
 "Documenting the code" section.
 Check patch script/checkpatch.pl shall be used before submitting a patch.
 
+Supplemental notes:-
+
+ - Variables shall be declared at the begining of scope, for example :-
+
+ int start_of_global_scope;
+
+ main () {
+   int start_of_function_scope;
+   ...
+   if (foo == bar) {
+ int start_of_block_scope;
+ ...
+   }
+ }
+
 2. ODP patch expectations as an  open source project
 
 While specific to the Linux kernel development, the following reference could
-- 
2.1.0


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

Re: [lng-odp] [PATCH v2 0/3] test cleanup

[Mike Holmes]

Ping

On 5 March 2015 at 17:22, Mike Holmes  wrote:

> v2
> Inadvertently send v1 which was != local commits
>
> Mike Holmes (3):
>   test: debug: Add unused attribute MACRO
>   validation: timer: use unused attribute
>   test: debug: replace example with test
>
>  test/test_debug.h   | 11 ---
>  test/validation/odp_timer.c |  3 +--
>  2 files changed, 9 insertions(+), 5 deletions(-)
>
> --
> 2.1.0
>
>


-- 
Mike Holmes
Technical Manager - Linaro Networking Group
Linaro.org  *│ *Open source software for ARM SoCs
___
lng-odp mailing list
lng-odp@lists.linaro.org
http://lists.linaro.org/mailman/listinfo/lng-odp