Re: [lng-odp] [API-NEXT PATCH] linux-gen: scheduler: solve ordered context inversion

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

Hi,

The point is that it cannot be reviewed as stand alone, since reviwer does not 
see the reason why it's needed. Other two schedulers do not need it, since 
those do not call dequeue simultaneously from multiple threads for the same 
queue.

-Petri


From: Yi He [mailto:yi...@linaro.org] 
Sent: Thursday, December 08, 2016 9:40 AM
To: Savolainen, Petri (Nokia - FI/Espoo) 
Cc: lng-odp@lists.linaro.org; bill.fischo...@linaro.org; Elo, Matias (Nokia - 
FI/Espoo) 
Subject: Re: [lng-odp] [API-NEXT PATCH] linux-gen: scheduler: solve ordered 
context inversion

Hi, Petri

I think this patch can stand alone for the problem it explains and solves, the 
two actions (dequeue events and get context sequence) are non-atomic and 
preemptable.

This patch suggest that in queue dequeue operation, to add a 
sched->save_context() callback in an atomic manner.

So lately default scheduler and SP scheduler can also move the context saving 
code into this call back, what do you think?

Best Regards, Yi

On 8 December 2016 at 15:29, Savolainen, Petri (Nokia - FI/Espoo) 
 wrote:
This patch belongs to the iquery scheduler patch set. Without the rest of the 
code, it's impossible to tell if this change is needed or correct.

-Petri


> -Original Message-
> From: lng-odp [mailto:lng-odp-boun...@lists.linaro.org] On Behalf Of Yi He
> Sent: Thursday, December 08, 2016 8:33 AM
> To: lng-odp@lists.linaro.org; bill.fischo...@linaro.org; Elo, Matias
> (Nokia - FI/Espoo) 
> Subject: [lng-odp] [API-NEXT PATCH] linux-gen: scheduler: solve ordered
> context inversion
>
> For ordered queue, a thread consumes events (dequeue) and
> acquires its unique sequential context in two steps, non
> atomic and preemptable.
>
> This leads to potential ordered context inversion in case
> the thread consumes prior events acquired subsequent context,
> while the thread consumes subsequent events but acquired
> prior context.
>
> This patch insert the ordered context acquisition into
> event dequeue operation to make these two steps atomic.
>
> Signed-off-by: Yi He 
> ---
> When I'm rebase iquery scheduler onto Matias' new ordered
> queue impl I found this potential ordered context inversion
> issue, it didnot happen to the default scheduler but happens
> to the iquery scheduler, so after patchset will rely on this
> patch. please help review thanks.
>
>  platform/linux-generic/include/odp_schedule_if.h | 3 +++
>  platform/linux-generic/odp_queue.c               | 3 +++
>  platform/linux-generic/odp_schedule.c            | 7 ++-
>  platform/linux-generic/odp_schedule_sp.c         | 7 ++-
>  4 files changed, 18 insertions(+), 2 deletions(-)
>
> diff --git a/platform/linux-generic/include/odp_schedule_if.h
> b/platform/linux-generic/include/odp_schedule_if.h
> index 6c2b050..c0aee42 100644
> --- a/platform/linux-generic/include/odp_schedule_if.h
> +++ b/platform/linux-generic/include/odp_schedule_if.h
> @@ -12,6 +12,7 @@ extern "C" {
>  #endif
>
>  #include 
> +#include 
>  #include 
>
>  typedef void (*schedule_pktio_start_fn_t)(int pktio_index, int
> num_in_queue,
> @@ -33,6 +34,7 @@ typedef int (*schedule_term_local_fn_t)(void);
>  typedef void (*schedule_order_lock_fn_t)(void);
>  typedef void (*schedule_order_unlock_fn_t)(void);
>  typedef unsigned (*schedule_max_ordered_locks_fn_t)(void);
> +typedef void (*schedule_save_context_fn_t)(queue_entry_t *queue);
>
>  typedef struct schedule_fn_t {
>       schedule_pktio_start_fn_t   pktio_start;
> @@ -50,6 +52,7 @@ typedef struct schedule_fn_t {
>       schedule_order_lock_fn_t    order_lock;
>       schedule_order_unlock_fn_t  order_unlock;
>       schedule_max_ordered_locks_fn_t max_ordered_locks;
> +     schedule_save_context_fn_t  save_context;
>  } schedule_fn_t;
>
>  /* Interface towards the scheduler */
> diff --git a/platform/linux-generic/odp_queue.c b/platform/linux-
> generic/odp_queue.c
> index d9cb9f3..3b6a68b 100644
> --- a/platform/linux-generic/odp_queue.c
> +++ b/platform/linux-generic/odp_queue.c
> @@ -568,6 +568,9 @@ static inline int deq_multi(queue_entry_t *queue,
> odp_buffer_hdr_t *buf_hdr[],
>       if (hdr == NULL)
>               queue->s.tail = NULL;
>
> +     if (queue->s.type == ODP_QUEUE_TYPE_SCHED)
> +             sched_fn->save_context(queue);
> +
>       UNLOCK(>s.lock);
>
>       return i;
> diff --git a/platform/linux-generic/odp_schedule.c b/platform/linux-
> generic/odp_schedule.c
> index 645630a..264c58f 100644
> --- a/platform/linux-generic/odp_schedule.c
> +++ b/platform/linux-generic/odp_schedule.c
> @@ -1205,6 +1205,10 @@ static int schedule_num_grps(void)
>       return NUM_SCHED_GRPS;
>  }
>
> +static void schedule_save_context(queue_entry_t *queue ODP_UNUSED)
> +{
> +}
> +
>  /* Fill in scheduler interface */
>  const schedule_fn_t schedule_default_fn = {
>       .pktio_start = 

Re: [lng-odp] [API-NEXT PATCH] linux-gen: scheduler: solve ordered context inversion

[Yi He]

Hi, Petri

I think this patch can stand alone for the problem it explains and solves,
the two actions (dequeue events and get context sequence) are non-atomic
and preemptable.

This patch suggest that in queue dequeue operation, to add a
sched->save_context() callback in an atomic manner.

So lately default scheduler and SP scheduler can also move the context
saving code into this call back, what do you think?

Best Regards, Yi

On 8 December 2016 at 15:29, Savolainen, Petri (Nokia - FI/Espoo) <
petri.savolai...@nokia-bell-labs.com> wrote:

> This patch belongs to the iquery scheduler patch set. Without the rest of
> the code, it's impossible to tell if this change is needed or correct.
>
> -Petri
>
>
> > -Original Message-
> > From: lng-odp [mailto:lng-odp-boun...@lists.linaro.org] On Behalf Of Yi
> He
> > Sent: Thursday, December 08, 2016 8:33 AM
> > To: lng-odp@lists.linaro.org; bill.fischo...@linaro.org; Elo, Matias
> > (Nokia - FI/Espoo) 
> > Subject: [lng-odp] [API-NEXT PATCH] linux-gen: scheduler: solve ordered
> > context inversion
> >
> > For ordered queue, a thread consumes events (dequeue) and
> > acquires its unique sequential context in two steps, non
> > atomic and preemptable.
> >
> > This leads to potential ordered context inversion in case
> > the thread consumes prior events acquired subsequent context,
> > while the thread consumes subsequent events but acquired
> > prior context.
> >
> > This patch insert the ordered context acquisition into
> > event dequeue operation to make these two steps atomic.
> >
> > Signed-off-by: Yi He 
> > ---
> > When I'm rebase iquery scheduler onto Matias' new ordered
> > queue impl I found this potential ordered context inversion
> > issue, it didnot happen to the default scheduler but happens
> > to the iquery scheduler, so after patchset will rely on this
> > patch. please help review thanks.
> >
> >  platform/linux-generic/include/odp_schedule_if.h | 3 +++
> >  platform/linux-generic/odp_queue.c   | 3 +++
> >  platform/linux-generic/odp_schedule.c| 7 ++-
> >  platform/linux-generic/odp_schedule_sp.c | 7 ++-
> >  4 files changed, 18 insertions(+), 2 deletions(-)
> >
> > diff --git a/platform/linux-generic/include/odp_schedule_if.h
> > b/platform/linux-generic/include/odp_schedule_if.h
> > index 6c2b050..c0aee42 100644
> > --- a/platform/linux-generic/include/odp_schedule_if.h
> > +++ b/platform/linux-generic/include/odp_schedule_if.h
> > @@ -12,6 +12,7 @@ extern "C" {
> >  #endif
> >
> >  #include 
> > +#include 
> >  #include 
> >
> >  typedef void (*schedule_pktio_start_fn_t)(int pktio_index, int
> > num_in_queue,
> > @@ -33,6 +34,7 @@ typedef int (*schedule_term_local_fn_t)(void);
> >  typedef void (*schedule_order_lock_fn_t)(void);
> >  typedef void (*schedule_order_unlock_fn_t)(void);
> >  typedef unsigned (*schedule_max_ordered_locks_fn_t)(void);
> > +typedef void (*schedule_save_context_fn_t)(queue_entry_t *queue);
> >
> >  typedef struct schedule_fn_t {
> >   schedule_pktio_start_fn_t   pktio_start;
> > @@ -50,6 +52,7 @@ typedef struct schedule_fn_t {
> >   schedule_order_lock_fn_torder_lock;
> >   schedule_order_unlock_fn_t  order_unlock;
> >   schedule_max_ordered_locks_fn_t max_ordered_locks;
> > + schedule_save_context_fn_t  save_context;
> >  } schedule_fn_t;
> >
> >  /* Interface towards the scheduler */
> > diff --git a/platform/linux-generic/odp_queue.c b/platform/linux-
> > generic/odp_queue.c
> > index d9cb9f3..3b6a68b 100644
> > --- a/platform/linux-generic/odp_queue.c
> > +++ b/platform/linux-generic/odp_queue.c
> > @@ -568,6 +568,9 @@ static inline int deq_multi(queue_entry_t *queue,
> > odp_buffer_hdr_t *buf_hdr[],
> >   if (hdr == NULL)
> >   queue->s.tail = NULL;
> >
> > + if (queue->s.type == ODP_QUEUE_TYPE_SCHED)
> > + sched_fn->save_context(queue);
> > +
> >   UNLOCK(>s.lock);
> >
> >   return i;
> > diff --git a/platform/linux-generic/odp_schedule.c b/platform/linux-
> > generic/odp_schedule.c
> > index 645630a..264c58f 100644
> > --- a/platform/linux-generic/odp_schedule.c
> > +++ b/platform/linux-generic/odp_schedule.c
> > @@ -1205,6 +1205,10 @@ static int schedule_num_grps(void)
> >   return NUM_SCHED_GRPS;
> >  }
> >
> > +static void schedule_save_context(queue_entry_t *queue ODP_UNUSED)
> > +{
> > +}
> > +
> >  /* Fill in scheduler interface */
> >  const schedule_fn_t schedule_default_fn = {
> >   .pktio_start = schedule_pktio_start,
> > @@ -1221,7 +1225,8 @@ const schedule_fn_t schedule_default_fn = {
> >   .term_local  = schedule_term_local,
> >   .order_lock = order_lock,
> >   .order_unlock = order_unlock,
> > - .max_ordered_locks = schedule_max_ordered_locks
> > + .max_ordered_locks = schedule_max_ordered_locks,
> > + .save_context = schedule_save_context
> >  };
> >
> >  /* Fill in scheduler API calls */
> > diff --git 

Re: [lng-odp] [PATCH 1/2] configure: if no ABI reset .so to 0

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

> -Original Message-
> From: lng-odp [mailto:lng-odp-boun...@lists.linaro.org] On Behalf Of Mike
> Holmes
> Sent: Wednesday, December 07, 2016 9:43 PM
> To: lng-odp@lists.linaro.org
> Subject: [lng-odp] [PATCH 1/2] configure: if no ABI reset .so to 0
> 
> Signed-off-by: Mike Holmes 
> ---
>  configure.ac | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/configure.ac b/configure.ac
> index 3e89b0a..543f535 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -254,6 +254,8 @@ AC_ARG_ENABLE([abi-compat],
>   ODP_ABI_COMPAT=0
>   abi_compat=no
>   enable_shared=no
> + #if there is no API compatibility the .so numbers are meaningless

Typo: API => ABI

-Petri

> + ODP_LIBSO_VERSION=0:0:0
>  fi])
>  AC_SUBST(ODP_ABI_COMPAT)
> 
> --
> 2.9.3

Re: [lng-odp] [API-NEXT PATCH] linux-gen: scheduler: solve ordered context inversion

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

This patch belongs to the iquery scheduler patch set. Without the rest of the 
code, it's impossible to tell if this change is needed or correct.

-Petri


> -Original Message-
> From: lng-odp [mailto:lng-odp-boun...@lists.linaro.org] On Behalf Of Yi He
> Sent: Thursday, December 08, 2016 8:33 AM
> To: lng-odp@lists.linaro.org; bill.fischo...@linaro.org; Elo, Matias
> (Nokia - FI/Espoo) 
> Subject: [lng-odp] [API-NEXT PATCH] linux-gen: scheduler: solve ordered
> context inversion
> 
> For ordered queue, a thread consumes events (dequeue) and
> acquires its unique sequential context in two steps, non
> atomic and preemptable.
> 
> This leads to potential ordered context inversion in case
> the thread consumes prior events acquired subsequent context,
> while the thread consumes subsequent events but acquired
> prior context.
> 
> This patch insert the ordered context acquisition into
> event dequeue operation to make these two steps atomic.
> 
> Signed-off-by: Yi He 
> ---
> When I'm rebase iquery scheduler onto Matias' new ordered
> queue impl I found this potential ordered context inversion
> issue, it didnot happen to the default scheduler but happens
> to the iquery scheduler, so after patchset will rely on this
> patch. please help review thanks.
> 
>  platform/linux-generic/include/odp_schedule_if.h | 3 +++
>  platform/linux-generic/odp_queue.c   | 3 +++
>  platform/linux-generic/odp_schedule.c| 7 ++-
>  platform/linux-generic/odp_schedule_sp.c | 7 ++-
>  4 files changed, 18 insertions(+), 2 deletions(-)
> 
> diff --git a/platform/linux-generic/include/odp_schedule_if.h
> b/platform/linux-generic/include/odp_schedule_if.h
> index 6c2b050..c0aee42 100644
> --- a/platform/linux-generic/include/odp_schedule_if.h
> +++ b/platform/linux-generic/include/odp_schedule_if.h
> @@ -12,6 +12,7 @@ extern "C" {
>  #endif
> 
>  #include 
> +#include 
>  #include 
> 
>  typedef void (*schedule_pktio_start_fn_t)(int pktio_index, int
> num_in_queue,
> @@ -33,6 +34,7 @@ typedef int (*schedule_term_local_fn_t)(void);
>  typedef void (*schedule_order_lock_fn_t)(void);
>  typedef void (*schedule_order_unlock_fn_t)(void);
>  typedef unsigned (*schedule_max_ordered_locks_fn_t)(void);
> +typedef void (*schedule_save_context_fn_t)(queue_entry_t *queue);
> 
>  typedef struct schedule_fn_t {
>   schedule_pktio_start_fn_t   pktio_start;
> @@ -50,6 +52,7 @@ typedef struct schedule_fn_t {
>   schedule_order_lock_fn_torder_lock;
>   schedule_order_unlock_fn_t  order_unlock;
>   schedule_max_ordered_locks_fn_t max_ordered_locks;
> + schedule_save_context_fn_t  save_context;
>  } schedule_fn_t;
> 
>  /* Interface towards the scheduler */
> diff --git a/platform/linux-generic/odp_queue.c b/platform/linux-
> generic/odp_queue.c
> index d9cb9f3..3b6a68b 100644
> --- a/platform/linux-generic/odp_queue.c
> +++ b/platform/linux-generic/odp_queue.c
> @@ -568,6 +568,9 @@ static inline int deq_multi(queue_entry_t *queue,
> odp_buffer_hdr_t *buf_hdr[],
>   if (hdr == NULL)
>   queue->s.tail = NULL;
> 
> + if (queue->s.type == ODP_QUEUE_TYPE_SCHED)
> + sched_fn->save_context(queue);
> +
>   UNLOCK(>s.lock);
> 
>   return i;
> diff --git a/platform/linux-generic/odp_schedule.c b/platform/linux-
> generic/odp_schedule.c
> index 645630a..264c58f 100644
> --- a/platform/linux-generic/odp_schedule.c
> +++ b/platform/linux-generic/odp_schedule.c
> @@ -1205,6 +1205,10 @@ static int schedule_num_grps(void)
>   return NUM_SCHED_GRPS;
>  }
> 
> +static void schedule_save_context(queue_entry_t *queue ODP_UNUSED)
> +{
> +}
> +
>  /* Fill in scheduler interface */
>  const schedule_fn_t schedule_default_fn = {
>   .pktio_start = schedule_pktio_start,
> @@ -1221,7 +1225,8 @@ const schedule_fn_t schedule_default_fn = {
>   .term_local  = schedule_term_local,
>   .order_lock = order_lock,
>   .order_unlock = order_unlock,
> - .max_ordered_locks = schedule_max_ordered_locks
> + .max_ordered_locks = schedule_max_ordered_locks,
> + .save_context = schedule_save_context
>  };
> 
>  /* Fill in scheduler API calls */
> diff --git a/platform/linux-generic/odp_schedule_sp.c b/platform/linux-
> generic/odp_schedule_sp.c
> index 76d1357..8481f29 100644
> --- a/platform/linux-generic/odp_schedule_sp.c
> +++ b/platform/linux-generic/odp_schedule_sp.c
> @@ -676,6 +676,10 @@ static void order_unlock(void)
>  {
>  }
> 
> +static void save_context(queue_entry_t *queue ODP_UNUSED)
> +{
> +}
> +
>  /* Fill in scheduler interface */
>  const schedule_fn_t schedule_sp_fn = {
>   .pktio_start   = pktio_start,
> @@ -692,7 +696,8 @@ const schedule_fn_t schedule_sp_fn = {
>   .term_local= term_local,
>   .order_lock =order_lock,
>   .order_unlock =  order_unlock,
> - .max_ordered_locks = max_ordered_locks
> + .max_ordered_locks = max_ordered_locks,
> + 

[lng-odp] [API-NEXT PATCH] linux-gen: scheduler: solve ordered context inversion

[Yi He]

For ordered queue, a thread consumes events (dequeue) and
acquires its unique sequential context in two steps, non
atomic and preemptable.

This leads to potential ordered context inversion in case
the thread consumes prior events acquired subsequent context,
while the thread consumes subsequent events but acquired
prior context.

This patch insert the ordered context acquisition into
event dequeue operation to make these two steps atomic.

Signed-off-by: Yi He 
---
When I'm rebase iquery scheduler onto Matias' new ordered
queue impl I found this potential ordered context inversion
issue, it didnot happen to the default scheduler but happens
to the iquery scheduler, so after patchset will rely on this
patch. please help review thanks.

 platform/linux-generic/include/odp_schedule_if.h | 3 +++
 platform/linux-generic/odp_queue.c   | 3 +++
 platform/linux-generic/odp_schedule.c| 7 ++-
 platform/linux-generic/odp_schedule_sp.c | 7 ++-
 4 files changed, 18 insertions(+), 2 deletions(-)

diff --git a/platform/linux-generic/include/odp_schedule_if.h 
b/platform/linux-generic/include/odp_schedule_if.h
index 6c2b050..c0aee42 100644
--- a/platform/linux-generic/include/odp_schedule_if.h
+++ b/platform/linux-generic/include/odp_schedule_if.h
@@ -12,6 +12,7 @@ extern "C" {
 #endif
 
 #include 
+#include 
 #include 
 
 typedef void (*schedule_pktio_start_fn_t)(int pktio_index, int num_in_queue,
@@ -33,6 +34,7 @@ typedef int (*schedule_term_local_fn_t)(void);
 typedef void (*schedule_order_lock_fn_t)(void);
 typedef void (*schedule_order_unlock_fn_t)(void);
 typedef unsigned (*schedule_max_ordered_locks_fn_t)(void);
+typedef void (*schedule_save_context_fn_t)(queue_entry_t *queue);
 
 typedef struct schedule_fn_t {
schedule_pktio_start_fn_t   pktio_start;
@@ -50,6 +52,7 @@ typedef struct schedule_fn_t {
schedule_order_lock_fn_torder_lock;
schedule_order_unlock_fn_t  order_unlock;
schedule_max_ordered_locks_fn_t max_ordered_locks;
+   schedule_save_context_fn_t  save_context;
 } schedule_fn_t;
 
 /* Interface towards the scheduler */
diff --git a/platform/linux-generic/odp_queue.c 
b/platform/linux-generic/odp_queue.c
index d9cb9f3..3b6a68b 100644
--- a/platform/linux-generic/odp_queue.c
+++ b/platform/linux-generic/odp_queue.c
@@ -568,6 +568,9 @@ static inline int deq_multi(queue_entry_t *queue, 
odp_buffer_hdr_t *buf_hdr[],
if (hdr == NULL)
queue->s.tail = NULL;
 
+   if (queue->s.type == ODP_QUEUE_TYPE_SCHED)
+   sched_fn->save_context(queue);
+
UNLOCK(>s.lock);
 
return i;
diff --git a/platform/linux-generic/odp_schedule.c 
b/platform/linux-generic/odp_schedule.c
index 645630a..264c58f 100644
--- a/platform/linux-generic/odp_schedule.c
+++ b/platform/linux-generic/odp_schedule.c
@@ -1205,6 +1205,10 @@ static int schedule_num_grps(void)
return NUM_SCHED_GRPS;
 }
 
+static void schedule_save_context(queue_entry_t *queue ODP_UNUSED)
+{
+}
+
 /* Fill in scheduler interface */
 const schedule_fn_t schedule_default_fn = {
.pktio_start = schedule_pktio_start,
@@ -1221,7 +1225,8 @@ const schedule_fn_t schedule_default_fn = {
.term_local  = schedule_term_local,
.order_lock = order_lock,
.order_unlock = order_unlock,
-   .max_ordered_locks = schedule_max_ordered_locks
+   .max_ordered_locks = schedule_max_ordered_locks,
+   .save_context = schedule_save_context
 };
 
 /* Fill in scheduler API calls */
diff --git a/platform/linux-generic/odp_schedule_sp.c 
b/platform/linux-generic/odp_schedule_sp.c
index 76d1357..8481f29 100644
--- a/platform/linux-generic/odp_schedule_sp.c
+++ b/platform/linux-generic/odp_schedule_sp.c
@@ -676,6 +676,10 @@ static void order_unlock(void)
 {
 }
 
+static void save_context(queue_entry_t *queue ODP_UNUSED)
+{
+}
+
 /* Fill in scheduler interface */
 const schedule_fn_t schedule_sp_fn = {
.pktio_start   = pktio_start,
@@ -692,7 +696,8 @@ const schedule_fn_t schedule_sp_fn = {
.term_local= term_local,
.order_lock =order_lock,
.order_unlock =  order_unlock,
-   .max_ordered_locks = max_ordered_locks
+   .max_ordered_locks = max_ordered_locks,
+   .save_context  = save_context
 };
 
 /* Fill in scheduler API calls */
-- 
2.7.4

Re: [lng-odp] [PATCH] linux-generic: move tm system barrier to tm group

[Bala Manoharan]

Reviewed-by: Balasubramanian Manoharan 

On 2 December 2016 at 13:41,   wrote:
> From: Xuelin Shi 
>
> since tm thread is handling tm group, move the thread based
> barrier to tm group. otherwise, packet cannot get into the
> second tm system in the same group.
>
> Signed-off-by: Xuelin Shi 
> ---
>  platform/linux-generic/include/odp_traffic_mngr_internal.h |  3 ++-
>  platform/linux-generic/odp_traffic_mngr.c  | 12 +++-
>  2 files changed, 9 insertions(+), 6 deletions(-)
>
> diff --git a/platform/linux-generic/include/odp_traffic_mngr_internal.h 
> b/platform/linux-generic/include/odp_traffic_mngr_internal.h
> index 858183b..9f821fe 100644
> --- a/platform/linux-generic/include/odp_traffic_mngr_internal.h
> +++ b/platform/linux-generic/include/odp_traffic_mngr_internal.h
> @@ -367,7 +367,6 @@ struct tm_system_s {
> _odp_tm_group_t odp_tm_group;
>
> odp_ticketlock_t tm_system_lock;
> -   odp_barrier_ttm_system_barrier;
> odp_barrier_ttm_system_destroy_barrier;
> odp_atomic_u64_t destroying;
> _odp_int_name_t  name_tbl_id;
> @@ -416,8 +415,10 @@ struct tm_system_group_s {
> tm_system_group_t *prev;
> tm_system_group_t *next;
>
> +   odp_barrier_t  tm_group_barrier;
> tm_system_t   *first_tm_system;
> uint32_t   num_tm_systems;
> +   uint32_t   first_enq;
> pthread_t  thread;
> pthread_attr_t attr;
>  };
> diff --git a/platform/linux-generic/odp_traffic_mngr.c 
> b/platform/linux-generic/odp_traffic_mngr.c
> index a1f990f..62e5c63 100644
> --- a/platform/linux-generic/odp_traffic_mngr.c
> +++ b/platform/linux-generic/odp_traffic_mngr.c
> @@ -1854,6 +1854,7 @@ static int tm_enqueue(tm_system_t *tm_system,
>   tm_queue_obj_t *tm_queue_obj,
>   odp_packet_t pkt)
>  {
> +   tm_system_group_t *tm_group;
> input_work_item_t work_item;
> odp_packet_color_t pkt_color;
> tm_wred_node_t *initial_tm_wred_node;
> @@ -1868,9 +1869,10 @@ static int tm_enqueue(tm_system_t *tm_system,
> if (queue_tm_reorder(_queue_obj->tm_qentry, _hdr->buf_hdr))
> return 0;
>
> -   if (tm_system->first_enq == 0) {
> -   odp_barrier_wait(_system->tm_system_barrier);
> -   tm_system->first_enq = 1;
> +   tm_group = GET_TM_GROUP(tm_system->odp_tm_group);
> +   if (tm_group->first_enq == 0) {
> +   odp_barrier_wait(_group->tm_group_barrier);
> +   tm_group->first_enq = 1;
> }
>
> pkt_color = odp_packet_color(pkt);
> @@ -2327,7 +2329,7 @@ static void *tm_system_thread(void *arg)
> input_work_queue = tm_system->input_work_queue;
>
> /* Wait here until we have seen the first enqueue operation. */
> -   odp_barrier_wait(_system->tm_system_barrier);
> +   odp_barrier_wait(_group->tm_group_barrier);
> main_loop_running = true;
>
> destroying = odp_atomic_load_u64(_system->destroying);
> @@ -2625,6 +2627,7 @@ static _odp_tm_group_t _odp_tm_group_create(const char 
> *name ODP_UNUSED)
>
> tm_group = malloc(sizeof(tm_system_group_t));
> memset(tm_group, 0, sizeof(tm_system_group_t));
> +   odp_barrier_init(_group->tm_group_barrier, 2);
>
> /* Add this group to the tm_group_list linked list. */
> if (tm_group_list == NULL) {
> @@ -2868,7 +2871,6 @@ odp_tm_t odp_tm_create(const char*name,
> tm_system->_odp_int_timer_wheel = _ODP_INT_TIMER_WHEEL_INVALID;
>
> odp_ticketlock_init(_system->tm_system_lock);
> -   odp_barrier_init(_system->tm_system_barrier, 2);
> odp_atomic_init_u64(_system->destroying, 0);
>
> tm_system->_odp_int_sorted_pool = _odp_sorted_pool_create(
> --
> 1.8.3.1
>

Re: [lng-odp] [RFC v2] example: traffic_mgmt: enhancement with multiple pktios

[Forrest Shi]

ping

On 2 December 2016 at 16:29,  wrote:

> From: Xuelin Shi 
>
> This patch made the following changes:
>   - try to create one tm background thread for each pktio
>   - receiving packets from multiple pktios other than generating packets
>   - identifying TM user by ip with individual class of service
>   - print the user service while starting the program
>   - pirnt the packets counts every 10 seconds for each user
>
> Currently only allow to create multiple tm threads for system with
> more than 24 cores. This patch also reduce the constraint to 4 cores.
>
> Signed-off-by: Xuelin Shi 
> ---
>  example/traffic_mgmt/Makefile.am  |   7 +-
>  example/traffic_mgmt/odp_traffic_mgmt.c   | 559 -
>  example/traffic_mgmt/odp_traffic_mgmt.h   |  48 ++
>  example/traffic_mgmt/odp_traffic_pktio.c  | 794
> ++
>  platform/linux-generic/odp_traffic_mngr.c |   2 +-
>  5 files changed, 1162 insertions(+), 248 deletions(-)
>  create mode 100644 example/traffic_mgmt/odp_traffic_mgmt.h
>  create mode 100644 example/traffic_mgmt/odp_traffic_pktio.c
>
> diff --git a/example/traffic_mgmt/Makefile.am b/example/traffic_mgmt/
> Makefile.am
> index c8ff797..d2c7929 100644
> --- a/example/traffic_mgmt/Makefile.am
> +++ b/example/traffic_mgmt/Makefile.am
> @@ -2,8 +2,9 @@ include $(top_srcdir)/example/Makefile.inc
>
>  bin_PROGRAMS = odp_traffic_mgmt$(EXEEXT)
>  odp_traffic_mgmt_LDFLAGS = $(AM_LDFLAGS) -static
> -odp_traffic_mgmt_CFLAGS = $(AM_CFLAGS) -I${top_srcdir}/example
> +odp_traffic_mgmt_CFLAGS = $(AM_CFLAGS) -I${top_srcdir}/example
> -I${top_srcdir}/test
>
> -noinst_HEADERS = $(top_srcdir)/example/example_debug.h
> +noinst_HEADERS = $(top_srcdir)/example/example_debug.h \
> +$(top_srcdir)/example/traffic_mgmt/odp_traffic_mgmt.h
>
> -dist_odp_traffic_mgmt_SOURCES = odp_traffic_mgmt.c
> +dist_odp_traffic_mgmt_SOURCES = odp_traffic_mgmt.c odp_traffic_pktio.c
> diff --git a/example/traffic_mgmt/odp_traffic_mgmt.c
> b/example/traffic_mgmt/odp_traffic_mgmt.c
> index c4f5356..3879f8e 100644
> --- a/example/traffic_mgmt/odp_traffic_mgmt.c
> +++ b/example/traffic_mgmt/odp_traffic_mgmt.c
> @@ -9,22 +9,31 @@
>  #define _GNU_SOURCE
>
>  #include 
> +#include 
>  #include 
> +#include 
>  #include 
>  #include 
>  #include 
>  #include 
> +#include 
>
> +#include "odp_traffic_mgmt.h"
> +
> +#define TM_USER_IP_START 0x01010101
>  #define NUM_SVC_CLASSES 4
>  #define USERS_PER_SVC_CLASS 2
> -#define APPS_PER_USER   2
> -#define TM_QUEUES_PER_APP   2
> +#define APPS_PER_USER   1
> +#define TM_QUEUES_PER_APP   1
>  #define NUM_USERS   (USERS_PER_SVC_CLASS * NUM_SVC_CLASSES)
>  #define NUM_TM_QUEUES   (NUM_USERS * APPS_PER_USER *
> TM_QUEUES_PER_APP)
>  #define TM_QUEUES_PER_USER  (TM_QUEUES_PER_APP * APPS_PER_USER)
>  #define TM_QUEUES_PER_CLASS (USERS_PER_SVC_CLASS * TM_QUEUES_PER_USER)
>  #define MAX_NODES_PER_LEVEL (NUM_USERS * APPS_PER_USER)
>
> +#define MAX_NB_PKTIO   32
> +#define MAX_NB_USERS   (NUM_USERS * APPS_PER_USER * MAX_NB_PKTIO)
> +
>  #define KBPS   1000
>  #define MBPS   100
>  #define PERCENT(percent)  (100 * percent)
> @@ -49,11 +58,6 @@ typedef struct {
> odp_tm_wred_t  wred_profiles[ODP_NUM_PACKET_COLORS];
>  } profile_set_t;
>
> -static const odp_init_t ODP_INIT_PARAMS = {
> -   .log_fn   = odp_override_log,
> -   .abort_fn = odp_override_abort
> -};
> -
>  static profile_params_set_t COMPANY_PROFILE_PARAMS = {
> .shaper_params = {
> .commit_bps = 50  * MBPS,  .commit_burst  = 100,
> @@ -161,8 +165,8 @@ static profile_params_set_t COS2_PROFILE_PARAMS = {
> },
>
> .threshold_params = {
> -   .max_pkts  = 1000,.enable_max_pkts  = TRUE,
> -   .max_bytes = 10,  .enable_max_bytes = TRUE
> +   .max_pkts  = 1000,.enable_max_pkts  = FALSE,
> +   .max_bytes = 10,  .enable_max_bytes = FALSE
> },
>
> .wred_params = {
> @@ -194,8 +198,8 @@ static profile_params_set_t COS3_PROFILE_PARAMS = {
> },
>
> .threshold_params = {
> -   .max_pkts  = 400,.enable_max_pkts  = TRUE,
> -   .max_bytes = 6,  .enable_max_bytes = TRUE
> +   .max_pkts  = 400,.enable_max_pkts  = FALSE,
> +   .max_bytes = 6,  .enable_max_bytes = FALSE
> },
>
> .wred_params = {
> @@ -219,26 +223,59 @@ static profile_params_set_t COS3_PROFILE_PARAMS = {
> }
>  };
>
> -static profile_set_t COMPANY_PROFILE_SET;
>  static profile_set_t COS_PROFILE_SETS[NUM_SVC_CLASSES];
>  static profile_set_t USER_PROFILE_SETS[NUM_SVC_CLASSES];
>  static profile_set_t APP_PROFILE_SETS[NUM_SVC_CLASSES][APPS_PER_USER];
>
> -static odp_tm_t odp_tm_test;
> -
> -static odp_pool_t odp_pool;
> -
> -static odp_tm_queue_t 

Re: [lng-odp] [PATCH] linux-generic: only enable pktout when egress kind is pktio

[Forrest Shi]

ping.

This is a simple fix.
The original code skips the support of egress_kind as ODP_TM_EGRESS_FN

On 2 December 2016 at 15:42,  wrote:

> From: Xuelin Shi 
>
> Signed-off-by: Xuelin Shi 
> ---
>  platform/linux-generic/odp_traffic_mngr.c | 5 ++---
>  1 file changed, 2 insertions(+), 3 deletions(-)
>
> diff --git a/platform/linux-generic/odp_traffic_mngr.c
> b/platform/linux-generic/odp_traffic_mngr.c
> index ffb149b..a1f990f 100644
> --- a/platform/linux-generic/odp_traffic_mngr.c
> +++ b/platform/linux-generic/odp_traffic_mngr.c
> @@ -2838,10 +2838,9 @@ odp_tm_t odp_tm_create(const char*name,
> return ODP_TM_INVALID;
> }
>
> -   if (odp_pktout_queue(egress->pktio, , 1) != 1)
> -   return ODP_TM_INVALID;
> +   if (egress->egress_kind == ODP_TM_EGRESS_PKT_IO)
> +   tm_system->pktout = pktout;
>
> -   tm_system->pktout = pktout;
> tm_system->name_tbl_id = name_tbl_id;
> max_tm_queues = requirements->max_tm_queues;
> memcpy(_system->egress, egress, sizeof(odp_tm_egress_t));
> --
> 1.8.3.1
>
>

[lng-odp] [Bug 2557] API random.h contains doxygen todo

[bugzilla-daemon]

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

--- Comment #5 from Bill Fischofer  ---
Patch v7 submitted http://patches.opendataplane.org/patch/7589/ This should be
the final revision. Waiting for final review by Petri.

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

[lng-odp] [Bug 2622] scheduler test fails in api-next

[bugzilla-daemon]

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

Bill Fischofer  changed:

   What|Removed |Added

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

--- Comment #7 from Bill Fischofer  ---
This is now fully resolved with the merge of Matias' redesigned ordered queue
logic. Commit id d16d1783b8b01c5a7bbd256567fa226775a4ba93

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

Re: [lng-odp] [PATCH] helper: remove unused variable

[Bill Fischofer]

On Fri, Dec 2, 2016 at 9:55 AM, Mike Holmes  wrote:
> Signed-off-by: Mike Holmes 

Reviewed-by: Bill Fischofer 

> ---
>  helper/linux.c | 1 -
>  1 file changed, 1 deletion(-)
>
> diff --git a/helper/linux.c b/helper/linux.c
> index bb4b22c..7bd0b07 100644
> --- a/helper/linux.c
> +++ b/helper/linux.c
> @@ -295,7 +295,6 @@ static int odph_linux_process_create(odph_odpthread_t 
> *thread_tbl,
>  int cpu,
>  const odph_odpthread_params_t 
> *thr_params)
>  {
> -   int ret;
> cpu_set_t cpu_set;
> pid_t pid;
>
> --
> 2.9.3
>

Re: [lng-odp] [PATCH 1/2] configure: if no ABI reset .so to 0

[Bill Fischofer]

For this series:

Reviewed-by: Bill Fischofer 

On Wed, Dec 7, 2016 at 1:43 PM, Mike Holmes  wrote:
> Signed-off-by: Mike Holmes 
> ---
>  configure.ac | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/configure.ac b/configure.ac
> index 3e89b0a..543f535 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -254,6 +254,8 @@ AC_ARG_ENABLE([abi-compat],
> ODP_ABI_COMPAT=0
> abi_compat=no
> enable_shared=no
> +   #if there is no API compatibility the .so numbers are meaningless
> +   ODP_LIBSO_VERSION=0:0:0
>  fi])
>  AC_SUBST(ODP_ABI_COMPAT)
>
> --
> 2.9.3
>

[lng-odp] [PATCH 2/2] configure: remove duplicate info

[Mike Holmes]

Signed-off-by: Mike Holmes 
---
 configure.ac | 1 -
 1 file changed, 1 deletion(-)

diff --git a/configure.ac b/configure.ac
index 543f535..1b358ac 100644
--- a/configure.ac
+++ b/configure.ac
@@ -344,7 +344,6 @@ AC_MSG_RESULT([
static libraries:   ${enable_static}
shared libraries:   ${enable_shared}
ABI compatible: ${abi_compat}
-   ODP_ABI_COMPAT: ${ODP_ABI_COMPAT}
cunit:  ${cunit_support}
test_vald:  ${test_vald}
test_perf:  ${test_perf}
-- 
2.9.3

[lng-odp] [PATCH 1/2] configure: if no ABI reset .so to 0

[Mike Holmes]

Signed-off-by: Mike Holmes 
---
 configure.ac | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/configure.ac b/configure.ac
index 3e89b0a..543f535 100644
--- a/configure.ac
+++ b/configure.ac
@@ -254,6 +254,8 @@ AC_ARG_ENABLE([abi-compat],
ODP_ABI_COMPAT=0
abi_compat=no
enable_shared=no
+   #if there is no API compatibility the .so numbers are meaningless
+   ODP_LIBSO_VERSION=0:0:0
 fi])
 AC_SUBST(ODP_ABI_COMPAT)
 
-- 
2.9.3

Re: [lng-odp] [PATCH] helper: remove unused variable

[Mike Holmes]

ping trivial fix that cleans up RPM generation logs

On 2 December 2016 at 10:55, Mike Holmes  wrote:

> Signed-off-by: Mike Holmes 
> ---
>  helper/linux.c | 1 -
>  1 file changed, 1 deletion(-)
>
> diff --git a/helper/linux.c b/helper/linux.c
> index bb4b22c..7bd0b07 100644
> --- a/helper/linux.c
> +++ b/helper/linux.c
> @@ -295,7 +295,6 @@ static int odph_linux_process_create(odph_odpthread_t
> *thread_tbl,
>  int cpu,
>  const odph_odpthread_params_t
> *thr_params)
>  {
> -   int ret;
> cpu_set_t cpu_set;
> pid_t pid;
>
> --
> 2.9.3
>
>


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

[lng-odp] [API-NEXT PATCHv7 1/3] api: random: add explicit controls over random data

[Bill Fischofer]

Rework the odp_random_data() API to replace the use_entropy with an
explicit odp_random_kind parameter that controls the type of random
desired. Two new APIs are also introduced:

- odp_random_max_kind() returns the maximum kind of random data available

- odp_random_repeatable_data() permits applications to generate repeatable
  random sequences for testing purposes

Signed-off-by: Bill Fischofer 
---
Changes in v7:
- Clarify hierarchy of random kinds
- Expand on intended use of odp_random_repeatable_data()

Changes in v6:
- Add odp_random_max_kind() API instead of adding this to the
  odp_crypto_capability() API.
- Rename odp_random_seeded_data() to odp_random_repeatable_data()
- Merge API defs, implementation, and validation to preserve bisectability

Changes in v5:
- Change return type from int to int32_t for random APIs

Changes in v4:
- Normalize random API signatures with other ODP APIs
- Add new odp_random_seeded_data() API for repeatable random data generation
- Add additional tests for new odp_random_seeded_data() API
- Break out crypto section of User Guide to its own sub-document
- Add User Guide docuemntation for ODP random data API.

Changes in v3:
- Address commments by Petri
- Rename ODP_RAND_NORMAL to ODP_RANDOM_BASIC to avoid confusion with the
mathematical term "normal"

 include/odp/api/spec/random.h   | 74 +++--
 platform/linux-generic/odp_crypto.c | 50 +++--
 test/common_plat/validation/api/random/random.c | 54 +-
 test/common_plat/validation/api/random/random.h |  2 +
 4 files changed, 170 insertions(+), 10 deletions(-)

diff --git a/include/odp/api/spec/random.h b/include/odp/api/spec/random.h
index 00fa15b..6d770a6 100644
--- a/include/odp/api/spec/random.h
+++ b/include/odp/api/spec/random.h
@@ -24,18 +24,84 @@ extern "C" {
  */
 
 /**
+ * Random kind selector
+ *
+ * The kind of random denotes the statistical quality of the random data
+ * returned. Basic random simply appears uniformly distributed, Cryptographic
+ * random is statistically random and suitable for use by cryptographic
+ * functions. True random is generated from a hardware entropy source rather
+ * than an algorithm and is thus completely unpredictable. These form a
+ * hierarchy where higher quality data is presumably more costly to generate
+ * than lower quality data.
+ */
+typedef enum {
+   /** Basic random, presumably pseudo-random generated by SW */
+   ODP_RANDOM_BASIC,
+   /** Cryptographic quality random */
+   ODP_RANDOM_CRYPTO,
+   /** True random, generated from a HW entropy source */
+   ODP_RANDOM_TRUE,
+} odp_random_kind_t;
+
+/**
+ * Query random max kind
+ *
+ * Implementations support the returned max kind and all kinds weaker than it.
+ *
+ * @return kind The maximum odp_random_kind_t supported by this implementation
+ */
+odp_random_kind_t odp_random_max_kind(void);
+
+/**
  * Generate random byte data
  *
+ * The intent in supporting different kinds of random data is to allow
+ * tradeoffs between performance and the quality of random data needed. The
+ * assumption is that basic random is cheap while true random is relatively
+ * expensive in terms of time to generate, with cryptographic random being
+ * something in between. Implementations that support highly efficient true
+ * random are free to use this for all requested kinds. So it is always
+ * permissible to "upgrade" a random data request, but never to "downgrade"
+ * such requests.
+ *
  * @param[out]buf   Output buffer
- * @param size  Size of output buffer
- * @param use_entropy   Use entropy
+ * @param len   Length of output buffer in bytes
+ * @param kind  Specifies the type of random data required. Request
+ *  is expected to fail if the implementation is unable to
+ *  provide the requested type.
+ *
+ * @return Number of bytes written
+ * @retval <0 on failure
+ */
+int32_t odp_random_data(uint8_t *buf, uint32_t len, odp_random_kind_t kind);
+
+/**
+ * Generate repeatable random byte data
+ *
+ * For testing purposes it is often useful to generate "random" sequences that
+ * are repeatable. This is accomplished by supplying a seed value that is used
+ * for pseudo-random data generation. The caller-provided seed value is
+ * updated for each call to continue the sequence. Restarting a series of
+ * calls with the same initial seed value will generate the same sequence of
+ * random test data.
+ *
+ * This function should be used only for testing purposes. Use
+ * odp_random_data() for production.
  *
- * @todo Define the implication of the use_entropy parameter
+ * @param[out]buf  Output buffer
+ * @param len  Length of output buffer in bytes
+ * @param kind Specifies the type of random data required. Request
+ * will fail if the implementation is unable to provide
+ * 

[lng-odp] [API-NEXT PATCHv7 3/3] doc: userguide: expand crypto documentation to cover random apis

[Bill Fischofer]

Clean up the crypto section of the User Guide and expand on the
ODP random data APIs.

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

diff --git a/doc/users-guide/users-guide-crypto.adoc 
b/doc/users-guide/users-guide-crypto.adoc
index 04b3e87..fd117e9 100644
--- a/doc/users-guide/users-guide-crypto.adoc
+++ b/doc/users-guide/users-guide-crypto.adoc
@@ -1,7 +1,8 @@
 == Cryptographic services
 
 ODP provides APIs to perform cryptographic operations required by various
-communication protocols (e.g. IPSec). ODP cryptographic APIs are session based.
+communication protocols (_e.g.,_ IPsec). ODP cryptographic APIs are session
+based.
 
 ODP provides APIs for following cryptographic services:
 
@@ -19,24 +20,26 @@ ODP supports synchronous and asynchronous crypto sessions. 
For asynchronous
 sessions, the output of crypto operation is posted in a queue defined as
 the completion queue in its session parameters.
 
-ODP crypto APIs support chained operation sessions in which hashing and 
ciphering
-both can be achieved using a single session and operation call. The order of
-cipher and hashing can be controlled by the `auth_cipher_text` session 
parameter.
+ODP crypto APIs support chained operation sessions in which hashing and
+ciphering both can be achieved using a single session and operation call. The
+order of cipher and hashing can be controlled by the `auth_cipher_text`
+session parameter.
 
 Other Session parameters include algorithms, keys, initialization vector
-(optional), encode or decode, output queue for async mode and output packet 
pool
-for allocation of an output packet if required.
+(optional), encode or decode, output queue for async mode and output packet
+pool for allocation of an output packet if required.
 
 === Crypto operations
 
 After session creation, a cryptographic operation can be applied to a packet
 using the `odp_crypto_operation()` API. Applications may indicate a preference
-for synchronous or asynchronous processing in the session's `pref_mode` 
parameter.
-However crypto operations may complete synchronously even if an asynchronous
-preference is indicated, and applications must examine the `posted` output
-parameter from `odp_crypto_operation()` to determine whether the operation has
-completed or if an `ODP_EVENT_CRYPTO_COMPL` notification is expected. In the 
case
-of an async operation, the `posted` output parameter will be set to true.
+for synchronous or asynchronous processing in the session's `pref_mode`
+parameter.  However crypto operations may complete synchronously even if an
+asynchronous preference is indicated, and applications must examine the
+`posted` output parameter from `odp_crypto_operation()` to determine whether
+the operation has completed or if an `ODP_EVENT_CRYPTO_COMPL` notification is
+expected. In the case of an async operation, the `posted` output parameter
+will be set to true.
 
 
 The operation arguments specify for each packet the areas that are to be
@@ -49,9 +52,9 @@ In case of out-of-place mode output packet is different from 
input packet as
 specified by the application, while in new buffer mode implementation allocates
 a new output buffer from the session’s output pool.
 
-The application can also specify a context associated with a given operation 
that
-will be retained during async operation and can be retrieved via the completion
-event.
+The application can also specify a context associated with a given operation
+that will be retained during async operation and can be retrieved via the
+completion event.
 
 Results of an asynchronous session will be posted as completion events to the
 session’s completion queue, which can be accessed directly or via the ODP
@@ -60,12 +63,59 @@ result. The application has the responsibility to free the 
completion event.
 
 === Random number Generation
 
-ODP provides an API `odp_random_data()` to generate random data bytes. It has
-an argument to specify whether to use system entropy source for random number
-generation or not.
+ODP provides two APIs to generate various kinds of random data bytes. Random
+data is characterized by _kind_, which specifies the "quality" of the
+randomness required. ODP support three kinds of random data:
+
+ODP_RANDOM_BASIC:: No specific requirement other than the data appear to be
+uniformly distributed. Suitable for load-balancing or other non-cryptographic
+use.
+
+ODP_RANDOM_CRYPTO:: Data suitable for cryptographic use. This is a more
+stringent requirement that the data pass tests for statistical randomness.
+
+ODP_RANDOM_TRUE:: Data generated from a hardware entropy source rather than
+any software generated pseudo-random data. May not be available on all
+platforms.
+
+The main API for accessing random data is:
+
+[source,c]
+-
+int32_t odp_random_data(uint8_t buf, uint32_t len, odp_random_kind_t kind);

[lng-odp] [API-NEXT PATCHv7 2/3] doc: userguide: move crypto documentation to its own sub-document

[Bill Fischofer]

Signed-off-by: Bill Fischofer 
---
 doc/users-guide/Makefile.am |  1 +
 doc/users-guide/users-guide-crypto.adoc | 71 
 doc/users-guide/users-guide.adoc| 72 +
 3 files changed, 73 insertions(+), 71 deletions(-)
 create mode 100644 doc/users-guide/users-guide-crypto.adoc

diff --git a/doc/users-guide/Makefile.am b/doc/users-guide/Makefile.am
index a01c717..01b4df3 100644
--- a/doc/users-guide/Makefile.am
+++ b/doc/users-guide/Makefile.am
@@ -2,6 +2,7 @@ include ../Makefile.inc
 
 SRC= $(top_srcdir)/doc/users-guide/users-guide.adoc \
 $(top_srcdir)/doc/users-guide/users-guide-cls.adoc \
+$(top_srcdir)/doc/users-guide/users-guide-crypto.adoc \
 $(top_srcdir)/doc/users-guide/users-guide-packet.adoc \
 $(top_srcdir)/doc/users-guide/users-guide-pktio.adoc \
 $(top_srcdir)/doc/users-guide/users-guide-timer.adoc \
diff --git a/doc/users-guide/users-guide-crypto.adoc 
b/doc/users-guide/users-guide-crypto.adoc
new file mode 100644
index 000..04b3e87
--- /dev/null
+++ b/doc/users-guide/users-guide-crypto.adoc
@@ -0,0 +1,71 @@
+== Cryptographic services
+
+ODP provides APIs to perform cryptographic operations required by various
+communication protocols (e.g. IPSec). ODP cryptographic APIs are session based.
+
+ODP provides APIs for following cryptographic services:
+
+* Ciphering
+* Authentication/data integrity via Keyed-Hashing (HMAC)
+* Random number generation
+* Crypto capability inquiries
+
+=== Crypto Sessions
+
+To apply a cryptographic operation to a packet a session must be created. All
+packets processed by a session share the parameters that define the session.
+
+ODP supports synchronous and asynchronous crypto sessions. For asynchronous
+sessions, the output of crypto operation is posted in a queue defined as
+the completion queue in its session parameters.
+
+ODP crypto APIs support chained operation sessions in which hashing and 
ciphering
+both can be achieved using a single session and operation call. The order of
+cipher and hashing can be controlled by the `auth_cipher_text` session 
parameter.
+
+Other Session parameters include algorithms, keys, initialization vector
+(optional), encode or decode, output queue for async mode and output packet 
pool
+for allocation of an output packet if required.
+
+=== Crypto operations
+
+After session creation, a cryptographic operation can be applied to a packet
+using the `odp_crypto_operation()` API. Applications may indicate a preference
+for synchronous or asynchronous processing in the session's `pref_mode` 
parameter.
+However crypto operations may complete synchronously even if an asynchronous
+preference is indicated, and applications must examine the `posted` output
+parameter from `odp_crypto_operation()` to determine whether the operation has
+completed or if an `ODP_EVENT_CRYPTO_COMPL` notification is expected. In the 
case
+of an async operation, the `posted` output parameter will be set to true.
+
+
+The operation arguments specify for each packet the areas that are to be
+encrypted or decrypted and authenticated. Also, there is an option of 
overriding
+the initialization vector specified in session parameters.
+
+An operation can be executed in in-place, out-of-place or new buffer mode.
+In in-place mode output packet is same as the input packet.
+In case of out-of-place mode output packet is different from input packet as
+specified by the application, while in new buffer mode implementation allocates
+a new output buffer from the session’s output pool.
+
+The application can also specify a context associated with a given operation 
that
+will be retained during async operation and can be retrieved via the completion
+event.
+
+Results of an asynchronous session will be posted as completion events to the
+session’s completion queue, which can be accessed directly or via the ODP
+scheduler. The completion event contains the status of the operation and the
+result. The application has the responsibility to free the completion event.
+
+=== Random number Generation
+
+ODP provides an API `odp_random_data()` to generate random data bytes. It has
+an argument to specify whether to use system entropy source for random number
+generation or not.
+
+=== Capability inquiries
+
+ODP provides an API interface `odp_crypto_capability()` to inquire 
implementation’s
+crypto capabilities. This interface returns a bitmask for supported algorithms
+and hardware backed algorithms.
diff --git a/doc/users-guide/users-guide.adoc b/doc/users-guide/users-guide.adoc
index 9a427fa..41c57d1 100755
--- a/doc/users-guide/users-guide.adoc
+++ b/doc/users-guide/users-guide.adoc
@@ -1018,77 +1018,7 @@ include::users-guide-pktio.adoc[]
 
 include::users-guide-timer.adoc[]
 
-== Cryptographic services
-
-ODP provides APIs to perform cryptographic operations required by various
-communication protocols (e.g. IPSec). 

Re: [lng-odp] [APO-NEXT PATCHv6 1/3] api: random: add explicit controls over random data

[Bill Fischofer]

On Wed, Dec 7, 2016 at 8:21 AM, Savolainen, Petri (Nokia - FI/Espoo)
 wrote:
>
>> >> +int32_t odp_random_repeatable_data(uint8_t *buf, uint32_t len,
>> >> +odp_random_kind_t kind, uint32_t
>> *seed);
>> >>
>> >
>> > If pseudo/deterministic is not a good term for this, then I think it's
>> better to profile it strictly for testing (all production cases are
>> covered by odp_random_data()). Also for testing the kind is not needed, we
>> just say that: "this is good only for testing, do not use for production".
>> If kind would be there, it would give two ways to achieve "cryptograptic
>> quality" randoms and that's not the intention of the call. Also a larger
>> seed enables better randomness of the output.
>>
>> Actually if you want to test the implementation of a cryptographic
>> algorithm you need repeatable cryptographic data as input. This is
>> just being consistent across the API. You would never use the
>> repeatable variant in production and I don't think that would be a
>> point of confusion. The term "cryptographic quality" simply refers to
>> the statistical properties of the data, and nothing more should be
>> read into that term.
>>
>
> OK. This is then the compromise
>
> /**
>  * Generate repeatable random data for testing purposes
>  *
>  * For testing purposes it is often useful to generate "random" sequences
>  * that are repeatable...
>  *
>  * This function should be used only for testing purposes, use 
> odp_random_data()
>  * for production.
>  */
> int32_t odp_random_test_data(uint8_t *buf, uint32_t len, odp_random_kind_t 
> kind, uint64_t *seed);
>
>
> -Petri

I'll send a v7 with these changes.

>
>
>> >
>> >
>> > /**
>> >  * Generate repeatable random data for testing purposes
>> >  *
>> >  * For testing purposes it is often useful to generate "random"
>> sequences
>> >  * that are repeatable...
>> >  *
>> >  * This function should be used only for testing purposes, use
>> odp_random_data()
>> >  * for production.
>> >  */
>> > int32_t odp_random_test_data(uint8_t *buf, uint32_t len, uint64_t
>> *seed);
>> >
>> >
>> > -Petri
>> >

Re: [lng-odp] [API-NEXT PATCH v2 1/7] api: crypto: decouple key length from algorithm enumeration

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

> -Original Message-
> From: Bill Fischofer [mailto:bill.fischo...@linaro.org]
> Sent: Wednesday, December 07, 2016 2:35 PM
> To: Savolainen, Petri (Nokia - FI/Espoo)  labs.com>
> Cc: lng-odp-forward 
> Subject: Re: [lng-odp] [API-NEXT PATCH v2 1/7] api: crypto: decouple key
> length from algorithm enumeration
> 
> On Wed, Dec 7, 2016 at 3:32 AM, Savolainen, Petri (Nokia - FI/Espoo)
>  wrote:
> >
> >> >  /**
> >> > + * Cipher algorithm capabilities
> >> > + */
> >> > +typedef struct odp_crypto_cipher_capa_t {
> >> > +   /** Key length in bytes */
> >> > +   uint32_t key_len;
> >> > +
> >> > +   /** IV length in bytes */
> >> > +   uint32_t iv_len;
> >> > +
> >> > +} odp_crypto_cipher_capa_t;
> >>
> >> This should be odp_crypto_cipher_capability_t for consistency with
> >> other odp_xxx_capability_t types.
> >
> > int odp_crypto_cipher_capability(odp_cipher_alg_t cipher,
> >
> odp_crypto_cipher_capability_t capa[], int num);
> >
> >
> > It's getting rather long (28 char function name). For example, the line
> above does not fit into 80 chars any more. Also odp_cipher_alg_t is not
> odp_cipher_algorithm_t, but an abbreviation of that.
> 
> I don't see a problem with adding a newline for readability here. The
> use of 'alg' is Ok here (though spelling this out as algorithm would
> be preferable), but we already have a set of odp_xxx_capability() APIs
> so we should be consistent with that precedent when we introduce new
> members to that "family".
> 

If everybody are OK with longer names (odp_crypto_cipher_capability_t) and 
turning crypto _params_t to _param_t, I'll do those for v3.

-Petri

Re: [lng-odp] [APO-NEXT PATCHv6 1/3] api: random: add explicit controls over random data

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

> >> +int32_t odp_random_repeatable_data(uint8_t *buf, uint32_t len,
> >> +odp_random_kind_t kind, uint32_t
> *seed);
> >>
> >
> > If pseudo/deterministic is not a good term for this, then I think it's
> better to profile it strictly for testing (all production cases are
> covered by odp_random_data()). Also for testing the kind is not needed, we
> just say that: "this is good only for testing, do not use for production".
> If kind would be there, it would give two ways to achieve "cryptograptic
> quality" randoms and that's not the intention of the call. Also a larger
> seed enables better randomness of the output.
> 
> Actually if you want to test the implementation of a cryptographic
> algorithm you need repeatable cryptographic data as input. This is
> just being consistent across the API. You would never use the
> repeatable variant in production and I don't think that would be a
> point of confusion. The term "cryptographic quality" simply refers to
> the statistical properties of the data, and nothing more should be
> read into that term.
> 

OK. This is then the compromise

/**
 * Generate repeatable random data for testing purposes
 *
 * For testing purposes it is often useful to generate "random" sequences
 * that are repeatable...
 *
 * This function should be used only for testing purposes, use odp_random_data()
 * for production.
 */
int32_t odp_random_test_data(uint8_t *buf, uint32_t len, odp_random_kind_t 
kind, uint64_t *seed);


-Petri


> >
> >
> > /**
> >  * Generate repeatable random data for testing purposes
> >  *
> >  * For testing purposes it is often useful to generate "random"
> sequences
> >  * that are repeatable...
> >  *
> >  * This function should be used only for testing purposes, use
> odp_random_data()
> >  * for production.
> >  */
> > int32_t odp_random_test_data(uint8_t *buf, uint32_t len, uint64_t
> *seed);
> >
> >
> > -Petri
> >

Re: [lng-odp] [API-NEXT PATCH v2 1/7] api: crypto: decouple key length from algorithm enumeration

[Bill Fischofer]

On Wed, Dec 7, 2016 at 3:32 AM, Savolainen, Petri (Nokia - FI/Espoo)
 wrote:
>
>> >  /**
>> > + * Cipher algorithm capabilities
>> > + */
>> > +typedef struct odp_crypto_cipher_capa_t {
>> > +   /** Key length in bytes */
>> > +   uint32_t key_len;
>> > +
>> > +   /** IV length in bytes */
>> > +   uint32_t iv_len;
>> > +
>> > +} odp_crypto_cipher_capa_t;
>>
>> This should be odp_crypto_cipher_capability_t for consistency with
>> other odp_xxx_capability_t types.
>
> int odp_crypto_cipher_capability(odp_cipher_alg_t cipher,
>odp_crypto_cipher_capability_t 
> capa[], int num);
>
>
> It's getting rather long (28 char function name). For example, the line above 
> does not fit into 80 chars any more. Also odp_cipher_alg_t is not 
> odp_cipher_algorithm_t, but an abbreviation of that.

I don't see a problem with adding a newline for readability here. The
use of 'alg' is Ok here (though spelling this out as algorithm would
be preferable), but we already have a set of odp_xxx_capability() APIs
so we should be consistent with that precedent when we introduce new
members to that "family".

>
> -Petri
>

Re: [lng-odp] [APO-NEXT PATCHv6 1/3] api: random: add explicit controls over random data

[Bill Fischofer]

On Wed, Dec 7, 2016 at 3:10 AM, Savolainen, Petri (Nokia - FI/Espoo)
 wrote:
>
>
>> -Original Message-
>> From: lng-odp [mailto:lng-odp-boun...@lists.linaro.org] On Behalf Of Bill
>> Fischofer
>> Sent: Monday, December 05, 2016 11:29 PM
>> To: lng-odp@lists.linaro.org
>> Subject: [lng-odp] [APO-NEXT PATCHv6 1/3] api: random: add explicit
>> controls over random data
>>
>> Rework the odp_random_data() API to replace the use_entropy with an
>> explicit odp_random_kind parameter that controls the type of random
>> desired. Two new APIs are also introduced:
>>
>> - odp_random_max_kind() returns the maximum kind of random data available
>>
>> - odp_random_repeatable_data() permits applications to generate repeatable
>>   random sequences for testing purposes
>>
>> Because the type signature of odp_random_data() is changed, the
>> implementation and validation tests are included here for bisectability.
>
> This does not need to be mentioned in git log, since it's our rule anyway 
> (that every commit must build and pass tests). Actually, it would be cleaner 
> to include only the minimum amount of implementation changes into the API 
> patch: the new odp_random_data() implementation and the test for that (single 
> line change).

OK

>
>
>
>> --- a/include/odp/api/spec/random.h
>> +++ b/include/odp/api/spec/random.h
>> @@ -24,18 +24,68 @@ extern "C" {
>>   */
>>
>>  /**
>> + * Random kind selector
>> + */
>> +typedef enum {
>> + /** Basic random, presumably pseudo-random generated by SW */
>> + ODP_RANDOM_BASIC,
>> + /** Cryptographic quality random */
>> + ODP_RANDOM_CRYPTO,
>> + /** True random, generated from a HW entropy source */
>> + ODP_RANDOM_TRUE,
>> +} odp_random_kind_t;
>> +
>> +/**
>> + * Query random max kind
>> + *
>
> This would be a good place to state the order of kinds: which is the "max" 
> end the enum.
>
>> + * @return kind The maximum odp_random_kind_t supported by this
>> implementation
>> + */
>> +odp_random_kind_t odp_random_max_kind(void);
>> +
>> +/**
>>   * Generate random byte data
>>   *
>> + * The intent in supporting different kinds of random data is to allow
>> + * tradeoffs between performance and the quality of random data needed.
>> The
>> + * assumption is that basic random is cheap while true random is
>> relatively
>> + * expensive in terms of time to generate, with cryptographic random
>> being
>> + * something in between. Implementations that support highly efficient
>> true
>> + * random are free to use this for all requested kinds. So it is always
>> + * permissible to "upgrade" a random data request, but never to
>> "downgrade"
>> + * such requests.
>> + *
>>   * @param[out]buf   Output buffer
>> - * @param size  Size of output buffer
>> - * @param use_entropy   Use entropy
>> + * @param len   Length of output buffer in bytes
>> + * @param kind  Specifies the type of random data required.
>> Request
>> + *  is expected to fail if the implementation is
>> unable to
>> + *  provide the requested type.
>> + *
>> + * @return Number of bytes written
>> + * @retval <0 on failure
>> + */
>> +int32_t odp_random_data(uint8_t *buf, uint32_t len, odp_random_kind_t
>> kind);
>> +
>> +/**
>> + * Generate repeatable random byte data
>> + *
>> + * For testing purposes it is often useful to generate "random" sequences
>> + * that are repeatable. This is accomplished by supplying a seed value
>> that
>> + * is used for pseudo-random data generation. The caller provides
>>   *
>> - * @todo Define the implication of the use_entropy parameter
>> + * @param[out]buf  Output buffer
>> + * @param len  Length of output buffer in bytes
>> + * @param kind Specifies the type of random data required.
>> Request
>> + * will fail if the implementation is unable to
>> provide
>> + * repeatable random of the requested type. This is
>> + * always true for true random and may be true for
>> + * cryptographic random.
>> + * @param[in,out] seed Seed value to use
>>   *
>>   * @return Number of bytes written
>>   * @retval <0 on failure
>>   */
>> -int32_t odp_random_data(uint8_t *buf, int32_t size, odp_bool_t
>> use_entropy);
>> +int32_t odp_random_repeatable_data(uint8_t *buf, uint32_t len,
>> +odp_random_kind_t kind, uint32_t *seed);
>>
>
> If pseudo/deterministic is not a good term for this, then I think it's better 
> to profile it strictly for testing (all production cases are covered by 
> odp_random_data()). Also for testing the kind is not needed, we just say 
> that: "this is good only for testing, do not use for production". If kind 
> would be there, it would give two ways to achieve "cryptograptic quality" 
> randoms and that's not the intention of the call. Also a larger seed enables 
> better randomness of the output.

Re: [lng-odp] [API-NEXT PATCHv3 0/5] driver initialisation framework

[Christophe Milard]

Ooops Clang. forgot this one will go through this. thanks...

On 7 December 2016 at 13:21, Bill Fischofer 
wrote:

> On Wed, Dec 7, 2016 at 1:19 AM, Christophe Milard
>  wrote:
> >
> > On 7 December 2016 at 01:19, Bill Fischofer 
> > wrote:
> >>
> >> After applying this series ./configure fails:
> >>
> >> ./configure
> >> 
> >> checking for GCC atomic builtins... yes
> >> checking libconfig.h usability... no
> >> checking libconfig.h presence... no
> >> checking for libconfig.h... no
> >> checking for pkg-config... /usr/bin/pkg-config
> >> checking pkg-config is at least version 0.9.0... yes
> >> checking for PKGCONFIG... no
> >> configure: error: Package requirements (libconfig >= 1.3.2) were not
> met:
> >>
> >> No package 'libconfig' found
> >>
> >> Consider adjusting the PKG_CONFIG_PATH environment variable if you
> >> installed software in a non-standard prefix.
> >>
> >> Alternatively, you may set the environment variables PKGCONFIG_CFLAGS
> >> and PKGCONFIG_LIBS to avoid the need to call pkg-config.
> >> See the pkg-config man page for more details.
> >>
> >> Is there a new DEPENDENCIES add that's needed?
> >
> >
> > Yes: patch " linux-gen: init: adding configuration file parsing" (in this
> > series)
> > needs libconfig-dev. The patch adds libconfig-dev in DEPENDENCIES too.
> > Is there anything else which needs to be done when an extra dependency is
> > added?
> > Thanks for your attention,
> >
> > Christophe
>
> Ok, thanks!  I should have found that.  After installing libconfig-dev
> ./configure now works, however with:
>
> make CC=clang I get:
>
>   ...
>   CC   drv_atomic.lo
>   CC   drv_barrier.lo
>   CC   drv_driver.lo
> In file included from drv_driver.c:11:
> In file included from ./include/odp/drv/driver.h:21:
> ../../include/odp/drv/spec/driver.h:126:3: error: redefinition of typedef
>   'odpdrv_enum_class_t' is a C11 feature [-Werror,-Wtypedef-
> redefinition]
> } odpdrv_enum_class_t;
>   ^
> ../../include/odp/drv/spec/driver.h:85:36: note: previous definition is
> here
> typedef struct odpdrv_enum_class_t odpdrv_enum_class_t;
>^
> ../../include/odp/drv/spec/driver.h:151:3: error: redefinition of typedef
>   'odpdrv_enum_t' is a C11 feature [-Werror,-Wtypedef-redefinition]
> } odpdrv_enum_t;
>   ^
> ../../include/odp/drv/spec/driver.h:86:30: note: previous definition is
> here
> typedef struct odpdrv_enum_t odpdrv_enum_t;
>  ^
> ../../include/odp/drv/spec/driver.h:182:3: error: redefinition of typedef
>   'odpdrv_enumerated_dev_t' is a C11 feature
>   [-Werror,-Wtypedef-redefinition]
> } odpdrv_enumerated_dev_t;
>   ^
> ../../include/odp/drv/spec/driver.h:87:40: note: previous definition is
> here
> typedef struct odpdrv_enumerated_dev_t odpdrv_enumerated_dev_t;
>^
> ../../include/odp/drv/spec/driver.h:253:3: error: redefinition of typedef
>   'odpdrv_driver_t' is a C11 feature [-Werror,-Wtypedef-redefinition]
> } odpdrv_driver_t;
>   ^
> ../../include/odp/drv/spec/driver.h:88:32: note: previous definition is
> here
> typedef struct odpdrv_driver_t odpdrv_driver_t;
>^
> 4 errors generated.
> Makefile:934: recipe for target 'drv_driver.lo' failed
> make[1]: *** [drv_driver.lo] Error 1
> make[1]: Leaving directory
> '/home/bill/linaro/drvregister/platform/linux-generic'
> Makefile:498: recipe for target 'all-recursive' failed
> make: *** [all-recursive] Error 1
>
>
> >
> >>
> >> On Tue, Dec 6, 2016 at 8:23 AM, Christophe Milard
> >>  wrote:
> >> > Since V2:
> >> >  -function odp_load_driver removed. replaced by config file. (Petri,
> FF)
> >> >  -configuration file "odp.conf" added. Configuration file is:
> >> > 1) as specified in env variable ODP_SYSCONFIG_FILE (which can be
> >> > "none").
> >> > 2) ./odp.conf
> >> > 3) $(prefix)/etc/odp.conf
> >> >  -test removed: will be sent in a separate patch as many questions
> >> > remains.
> >> >  -All libdl tests removed: libdl is assumed to always be on linux
> >> > (Maxim)
> >> >
> >> > Since V1:
> >> >  -enum names prefixed by ODPDRV (Yi)
> >> >  -better commit message for last patch (Christophe)
> >> >  -typo fix (Christophe)
> >> >
> >> > This patch series puts the driver initialisation framework in place:
> >> > Loadable modules (*.so) are given in the odp.conf file added here.
> >> > Once loaded, the drivers init function (declared as __constructor__)
> >> > calls the ODP odp_driver_register() intialialisation function which,
> >> > at this stage does nothing (just print an error message).
> >> > odp_driver_register() is of course part of the driver interface
> (south).
> >> >
> >> > Christophe Milard (5):
> >> >   drv: adding driver registration interface (stub)
> >> >   linux-gen: adding enum, devio and driver registration interface
> 

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

[Bill Fischofer]

I've included this topic in today's ARCH call.

On Wed, Dec 7, 2016 at 6:09 AM, Savolainen, Petri (Nokia - FI/Espoo)
 wrote:
> DDF may be part of some implementations, but it's not compulsory. So, DFF 
> must be able utilize dev API, but dev API does not depend on DDF. Dev API 
> needs to provide freedom of implementation.
>
> Weather a device is behind PCI or not (or if SFP is used or not) is far 
> beyond the API level. Application may ask to open "eth0" or 
> "eth10g_backplane", which is the name of an Ethernet port on the blade (on 
> the system). Implementation knows which driver to use for that interface 
> (from system configuration, etc), application does not need to know about 
> drivers or other implementation details of the interface.
>
> -Petri
>
>
> From: Francois Ozog [mailto:francois.o...@linaro.org]
> Sent: Wednesday, December 07, 2016 1:34 PM
> To: Savolainen, Petri (Nokia - FI/Espoo) 
> 
> Cc: Bill Fischofer ; lng-odp-forward 
> ; Christophe Milard 
> Subject: Re: [lng-odp] [API-NEXT PATCHv3 1/3] api: dev: add device apis for 
> numa support
>
> Well, we need to formalize the scope and it looks like this is part of the 
> Device and Driver Framework (DDF) that Christophe works on.
>
> I have asked Christophe to create a document complementing the patches it 
> pushed so that we have a human readable reference on what the DDF does and 
> the general mechanics of it.
>
> For instance the DDF can build a "physical" view of things through the 
> various device enumerators (from DT to DPAA2 including ACPi, PCI, virtual, 
> static...). But applications should not care about that. They may care of a 
> "functional representation" derived from that hardware aspect. For instance 
> controlling SFP aspects of a port may require controlling a specific PCI 
> device ID while the port itself is under the control of another PCI device ID.
> Additional information can be found here: 
> https://docs.google.com/a/linaro.org/presentation/d/1Ecb9-jA_7jbXtgMVImiTePzpOJsa6IldZMeFdjG8ypQ/edit?usp=sharing
>
> So until we know how (or even if) the proposed API fits in a functional 
> framework (how it is used by what - application, odp internal, drivers 
> only...) this is premature to introduce it.
>
> FF
>
>
> On 7 December 2016 at 12:13, Savolainen, Petri (Nokia - FI/Espoo) 
>  wrote:
> I know device trees and how ugly those can be ….  values are not documented 
> but you (your driver) just have to know what some list of four integers (reg 
> = <4, 78865, 6, 565>) may actually mean…
>
> Anyway, the idea here is that
> • an implementation has a way to enumerate resources to the user as 
> names
> o   a name may come from a standard or system specific tool (ifconfig), a 
> configuration file (even device trees), etc. API does not dictate how user 
> gets a resource name, or which kind of string  that name is.
> • User passes the name to the application (through command line, 
> config file, env variable). API don’t dictate how that happens.
> • Application passes the name to the implementation and gets a device 
> ID (odp_dev_t), which it uses as a parameter on other calls
>
> Resource discovery/enumeration/management/etc is left to upper layer. API is 
> only needed for passing resource identifier information down to the  
> implementation. And this is how it should be: ODP is a passive library which 
> does what it’s told to do, the active role is on upper layers (user, scripts, 
> resource management SW, etc entity).
>
> So, no need to defer the API since it’s flexible enough (to pass any kind of 
> string down to implementation). Driver, cloud, etc frameworks may then 
> specify the dev name string format, etc later on, but it would not change the 
> API if a timer is called “timer0” or “timer0:2” or “/proc/soc/0/timer/1” or 
> something else.
>
> -Petri
>
>
>
> From: Francois Ozog [mailto:francois.o...@linaro.org]
> Sent: Tuesday, December 06, 2016 5:10 PM
> To: Bill Fischofer 
> Cc: lng-odp-forward ; Savolainen, Petri (Nokia - 
> FI/Espoo) ; Christophe Milard 
> 
> Subject: Re: [lng-odp] [API-NEXT PATCHv3 1/3] api: dev: add device apis for 
> numa support
>
> For the link, I think it ate the  following it.
> Here it is without decoration:
> http://free-electrons.com/pub/conferences/2013/elce/petazzoni-device-tree-dummies/petazzoni-device-tree-dummies.pdf
>
>
>
> On 6 December 2016 at 16:05, Francois Ozog  wrote:
> OK. So let's defer the API but not the discussion.
> I think we need a "strategic architecture" call.
>
> FF
>
>
>
> On 6 December 2016 at 15:59, Bill Fischofer  wrote:
> I have no problem deferring 

Re: [lng-odp] [API-NEXT PATCHv3 0/5] driver initialisation framework

[Bill Fischofer]

On Wed, Dec 7, 2016 at 1:19 AM, Christophe Milard
 wrote:
>
> On 7 December 2016 at 01:19, Bill Fischofer 
> wrote:
>>
>> After applying this series ./configure fails:
>>
>> ./configure
>> 
>> checking for GCC atomic builtins... yes
>> checking libconfig.h usability... no
>> checking libconfig.h presence... no
>> checking for libconfig.h... no
>> checking for pkg-config... /usr/bin/pkg-config
>> checking pkg-config is at least version 0.9.0... yes
>> checking for PKGCONFIG... no
>> configure: error: Package requirements (libconfig >= 1.3.2) were not met:
>>
>> No package 'libconfig' found
>>
>> Consider adjusting the PKG_CONFIG_PATH environment variable if you
>> installed software in a non-standard prefix.
>>
>> Alternatively, you may set the environment variables PKGCONFIG_CFLAGS
>> and PKGCONFIG_LIBS to avoid the need to call pkg-config.
>> See the pkg-config man page for more details.
>>
>> Is there a new DEPENDENCIES add that's needed?
>
>
> Yes: patch " linux-gen: init: adding configuration file parsing" (in this
> series)
> needs libconfig-dev. The patch adds libconfig-dev in DEPENDENCIES too.
> Is there anything else which needs to be done when an extra dependency is
> added?
> Thanks for your attention,
>
> Christophe

Ok, thanks!  I should have found that.  After installing libconfig-dev
./configure now works, however with:

make CC=clang I get:

  ...
  CC   drv_atomic.lo
  CC   drv_barrier.lo
  CC   drv_driver.lo
In file included from drv_driver.c:11:
In file included from ./include/odp/drv/driver.h:21:
../../include/odp/drv/spec/driver.h:126:3: error: redefinition of typedef
  'odpdrv_enum_class_t' is a C11 feature [-Werror,-Wtypedef-redefinition]
} odpdrv_enum_class_t;
  ^
../../include/odp/drv/spec/driver.h:85:36: note: previous definition is here
typedef struct odpdrv_enum_class_t odpdrv_enum_class_t;
   ^
../../include/odp/drv/spec/driver.h:151:3: error: redefinition of typedef
  'odpdrv_enum_t' is a C11 feature [-Werror,-Wtypedef-redefinition]
} odpdrv_enum_t;
  ^
../../include/odp/drv/spec/driver.h:86:30: note: previous definition is here
typedef struct odpdrv_enum_t odpdrv_enum_t;
 ^
../../include/odp/drv/spec/driver.h:182:3: error: redefinition of typedef
  'odpdrv_enumerated_dev_t' is a C11 feature
  [-Werror,-Wtypedef-redefinition]
} odpdrv_enumerated_dev_t;
  ^
../../include/odp/drv/spec/driver.h:87:40: note: previous definition is here
typedef struct odpdrv_enumerated_dev_t odpdrv_enumerated_dev_t;
   ^
../../include/odp/drv/spec/driver.h:253:3: error: redefinition of typedef
  'odpdrv_driver_t' is a C11 feature [-Werror,-Wtypedef-redefinition]
} odpdrv_driver_t;
  ^
../../include/odp/drv/spec/driver.h:88:32: note: previous definition is here
typedef struct odpdrv_driver_t odpdrv_driver_t;
   ^
4 errors generated.
Makefile:934: recipe for target 'drv_driver.lo' failed
make[1]: *** [drv_driver.lo] Error 1
make[1]: Leaving directory
'/home/bill/linaro/drvregister/platform/linux-generic'
Makefile:498: recipe for target 'all-recursive' failed
make: *** [all-recursive] Error 1


>
>>
>> On Tue, Dec 6, 2016 at 8:23 AM, Christophe Milard
>>  wrote:
>> > Since V2:
>> >  -function odp_load_driver removed. replaced by config file. (Petri, FF)
>> >  -configuration file "odp.conf" added. Configuration file is:
>> > 1) as specified in env variable ODP_SYSCONFIG_FILE (which can be
>> > "none").
>> > 2) ./odp.conf
>> > 3) $(prefix)/etc/odp.conf
>> >  -test removed: will be sent in a separate patch as many questions
>> > remains.
>> >  -All libdl tests removed: libdl is assumed to always be on linux
>> > (Maxim)
>> >
>> > Since V1:
>> >  -enum names prefixed by ODPDRV (Yi)
>> >  -better commit message for last patch (Christophe)
>> >  -typo fix (Christophe)
>> >
>> > This patch series puts the driver initialisation framework in place:
>> > Loadable modules (*.so) are given in the odp.conf file added here.
>> > Once loaded, the drivers init function (declared as __constructor__)
>> > calls the ODP odp_driver_register() intialialisation function which,
>> > at this stage does nothing (just print an error message).
>> > odp_driver_register() is of course part of the driver interface (south).
>> >
>> > Christophe Milard (5):
>> >   drv: adding driver registration interface (stub)
>> >   linux-gen: adding enum, devio and driver registration interface (stub)
>> >   linux-gen: init: adding configuration file parsing
>> >   test: preventing odp.conf loading for tests
>> >   linux-gen: drv_drivers: loading modules from config file
>> >
>> >  DEPENDENCIES|   2 +-
>> >  configure.ac|   4 +-
>> >  include/odp/drv/spec/driver.h   | 311
>> > 
>> 

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

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

DDF may be part of some implementations, but it's not compulsory. So, DFF must 
be able utilize dev API, but dev API does not depend on DDF. Dev API needs to 
provide freedom of implementation.

Weather a device is behind PCI or not (or if SFP is used or not) is far beyond 
the API level. Application may ask to open "eth0" or "eth10g_backplane", which 
is the name of an Ethernet port on the blade (on the system). Implementation 
knows which driver to use for that interface (from system configuration, etc), 
application does not need to know about drivers or other implementation details 
of the interface.  

-Petri


From: Francois Ozog [mailto:francois.o...@linaro.org] 
Sent: Wednesday, December 07, 2016 1:34 PM
To: Savolainen, Petri (Nokia - FI/Espoo) 
Cc: Bill Fischofer ; lng-odp-forward 
; Christophe Milard 
Subject: Re: [lng-odp] [API-NEXT PATCHv3 1/3] api: dev: add device apis for 
numa support

Well, we need to formalize the scope and it looks like this is part of the 
Device and Driver Framework (DDF) that Christophe works on.

I have asked Christophe to create a document complementing the patches it 
pushed so that we have a human readable reference on what the DDF does and the 
general mechanics of it.

For instance the DDF can build a "physical" view of things through the various 
device enumerators (from DT to DPAA2 including ACPi, PCI, virtual, static...). 
But applications should not care about that. They may care of a "functional 
representation" derived from that hardware aspect. For instance controlling SFP 
aspects of a port may require controlling a specific PCI device ID while the 
port itself is under the control of another PCI device ID.
Additional information can be found here: 
https://docs.google.com/a/linaro.org/presentation/d/1Ecb9-jA_7jbXtgMVImiTePzpOJsa6IldZMeFdjG8ypQ/edit?usp=sharing

So until we know how (or even if) the proposed API fits in a functional 
framework (how it is used by what - application, odp internal, drivers only...) 
this is premature to introduce it.

FF


On 7 December 2016 at 12:13, Savolainen, Petri (Nokia - FI/Espoo) 
 wrote:
I know device trees and how ugly those can be ….  values are not documented but 
you (your driver) just have to know what some list of four integers (reg = <4, 
78865, 6, 565>) may actually mean…
 
Anyway, the idea here is that
• an implementation has a way to enumerate resources to the user as 
names
o   a name may come from a standard or system specific tool (ifconfig), a 
configuration file (even device trees), etc. API does not dictate how user gets 
a resource name, or which kind of string  that name is.
• User passes the name to the application (through command line, config 
file, env variable). API don’t dictate how that happens.
• Application passes the name to the implementation and gets a device 
ID (odp_dev_t), which it uses as a parameter on other calls
 
Resource discovery/enumeration/management/etc is left to upper layer. API is 
only needed for passing resource identifier information down to the  
implementation. And this is how it should be: ODP is a passive library which 
does what it’s told to do, the active role is on upper layers (user, scripts, 
resource management SW, etc entity).
 
So, no need to defer the API since it’s flexible enough (to pass any kind of 
string down to implementation). Driver, cloud, etc frameworks may then specify 
the dev name string format, etc later on, but it would not change the API if a 
timer is called “timer0” or “timer0:2” or “/proc/soc/0/timer/1” or something 
else.
 
-Petri
 
 
 
From: Francois Ozog [mailto:francois.o...@linaro.org] 
Sent: Tuesday, December 06, 2016 5:10 PM
To: Bill Fischofer 
Cc: lng-odp-forward ; Savolainen, Petri (Nokia - 
FI/Espoo) ; Christophe Milard 

Subject: Re: [lng-odp] [API-NEXT PATCHv3 1/3] api: dev: add device apis for 
numa support
 
For the link, I think it ate the  following it.
Here it is without decoration:
http://free-electrons.com/pub/conferences/2013/elce/petazzoni-device-tree-dummies/petazzoni-device-tree-dummies.pdf
 
 
 
On 6 December 2016 at 16:05, Francois Ozog  wrote:
OK. So let's defer the API but not the discussion.
I think we need a "strategic architecture" call. 
 
FF
 
 
 
On 6 December 2016 at 15:59, Bill Fischofer  wrote:
I have no problem deferring this if we want to have one of the SoC vendors 
drive a more comprehensive approach based on their capabilities which can then 
be generalized. BTW, your link doesn't resolve. Typo somewhere?
 
On Tue, Dec 6, 2016 at 8:47 AM, Francois Ozog  wrote:
Devicetree has been formed by and for SoC vendors to 

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

[Francois Ozog]

Well, we need to formalize the scope and it looks like this is part of the
Device and Driver Framework (DDF) that Christophe works on.

I have asked Christophe to create a document complementing the patches it
pushed so that we have a human readable reference on what the DDF does and
the general mechanics of it.

For instance the DDF can build a "physical" view of things through the
various device enumerators (from DT to DPAA2 including ACPi, PCI, virtual,
static...). But applications should not care about that. They may care of a
"functional representation" derived from that hardware aspect. For instance
controlling SFP aspects of a port may require controlling a specific PCI
device ID while the port itself is under the control of another PCI device
ID.
Additional information can be found here:
https://docs.google.com/a/linaro.org/presentation/d/1Ecb9-jA_7jbXtgMVImiTePzpOJsa6IldZMeFdjG8ypQ/edit?usp=sharing

So until we know how (or even if) the proposed API fits in a functional
framework (how it is used by what - application, odp internal, drivers
only...) this is premature to introduce it.

FF


On 7 December 2016 at 12:13, Savolainen, Petri (Nokia - FI/Espoo) <
petri.savolai...@nokia-bell-labs.com> wrote:

> I know device trees and how ugly those can be ….  values are not
> documented but you (your driver) just have to know what some list of four
> integers (reg = <4, 78865, 6, 565>) may actually mean…
>
>
>
> Anyway, the idea here is that
>
> · an implementation has a way to enumerate resources to the user
> as names
>
> o   a name may come from a standard or system specific tool (ifconfig), a
> configuration file (even device trees), etc. API does not dictate how user
> gets a resource name, or which kind of string  that name is.
>
> · User passes the name to the application (through command line,
> config file, env variable). API don’t dictate how that happens.
>
> · Application passes the name to the implementation and gets a
> device ID (odp_dev_t), which it uses as a parameter on other calls
>
>
>
> Resource discovery/enumeration/management/etc is left to upper layer. API
> is only needed for passing resource identifier information down to the
>  implementation. And this is how it should be: ODP is a passive library
> which does what it’s told to do, the active role is on upper layers (user,
> scripts, resource management SW, etc entity).
>
>
>
> So, no need to defer the API since it’s flexible enough (to pass any kind
> of string down to implementation). Driver, cloud, etc frameworks may then
> specify the dev name string format, etc later on, but it would not change
> the API if a timer is called “timer0” or “timer0:2” or
> “/proc/soc/0/timer/1” or something else.
>
>
>
> -Petri
>
>
>
>
>
>
>
> *From:* Francois Ozog [mailto:francois.o...@linaro.org]
> *Sent:* Tuesday, December 06, 2016 5:10 PM
> *To:* Bill Fischofer 
> *Cc:* lng-odp-forward ; Savolainen, Petri
> (Nokia - FI/Espoo) ; Christophe
> Milard 
> *Subject:* Re: [lng-odp] [API-NEXT PATCHv3 1/3] api: dev: add device apis
> for numa support
>
>
>
> For the link, I think it ate the  following it.
>
> Here it is without decoration:
>
> http://free-electrons.com/pub/conferences/2013/elce/
> petazzoni-device-tree-dummies/petazzoni-device-tree-dummies.pdf
>
>
>
>
>
>
>
> On 6 December 2016 at 16:05, Francois Ozog 
> wrote:
>
> OK. So let's defer the API but not the discussion.
>
> I think we need a "strategic architecture" call.
>
>
>
> FF
>
>
>
>
>
>
>
> On 6 December 2016 at 15:59, Bill Fischofer 
> wrote:
>
> I have no problem deferring this if we want to have one of the SoC vendors
> drive a more comprehensive approach based on their capabilities which can
> then be generalized. BTW, your link doesn't resolve. Typo somewhere?
>
>
>
> On Tue, Dec 6, 2016 at 8:47 AM, Francois Ozog 
> wrote:
>
> Devicetree has been formed by and for SoC vendors to represent what they
> have on their platforms (CPUs, Memory, interconnects, IOMMUs, ports, IRQ
> lines, firmware - http://free-electrons.com/pub/conferences/2013/elce/
> petazzoni-device-tree-dummies/petazzoni-device-tree-dummies.pdf). so
> it looks like very similar to what Petri is talking about.
>
>
>
> ACPI has also a concept of a proximity domains and the relative cost to
> use a resource from another domain.
>
>
>
> Handling NUMA requires a well thought through approach because it has
> implications at many levels in particular for device and driver framework.
>
>
>
> Bottom line, regardless if it's value, the introduction of the API seem
> too early as I haven't seen the use case that it supports and in particular
> the device and drivers framework.
>
>
>
> FF
>
>
>
>
>
>
>
>
>
>
>
>
>
> On 6 December 2016 at 14:23, Bill Fischofer 
> 

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

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

... sending again as plain text ...

I know device trees and how ugly those can be ….  values are not documented but 
you (your driver) just have to know what some list of four integers (reg = <4, 
78865, 6, 565>) may actually mean…

Anyway, the idea here is that
• an implementation has a way to enumerate resources to the user as names
o a name may come from a standard or system specific tool (ifconfig), a 
configuration file (even device trees), etc. API does not dictate how user gets 
a resource name, or which kind of string  that name is.
• User passes the name to the application (through command line, config file, 
env variable). API don’t dictate how that happens.
• Application passes the name to the implementation and gets a device ID 
(odp_dev_t), which it uses as a parameter on other calls

Resource discovery/enumeration/management/etc is left to upper layer. API is 
only needed for passing resource identifier information down to the  
implementation. And this is how it should be: ODP is a passive library which 
does what it’s told to do, the active role is on upper layers (user, scripts, 
resource management SW, etc entity).

So, no need to defer the API since it’s flexible enough (to pass any kind of 
string down to implementation). Driver, cloud, etc frameworks may then specify 
the dev name string format, etc later on, but it would not change the API if a 
timer is called “timer0” or “timer0:2” or “/proc/soc/0/timer/1” or something 
else.

-Petri



From: Francois Ozog [mailto:francois.o...@linaro.org] 
Sent: Tuesday, December 06, 2016 5:10 PM
To: Bill Fischofer 
Cc: lng-odp-forward ; Savolainen, Petri (Nokia - 
FI/Espoo) ; Christophe Milard 

Subject: Re: [lng-odp] [API-NEXT PATCHv3 1/3] api: dev: add device apis for 
numa support

For the link, I think it ate the  following it.
Here it is without decoration:
http://free-electrons.com/pub/conferences/2013/elce/petazzoni-device-tree-dummies/petazzoni-device-tree-dummies.pdf



On 6 December 2016 at 16:05, Francois Ozog  wrote:
OK. So let's defer the API but not the discussion.
I think we need a "strategic architecture" call. 

FF



On 6 December 2016 at 15:59, Bill Fischofer  wrote:
I have no problem deferring this if we want to have one of the SoC vendors 
drive a more comprehensive approach based on their capabilities which can then 
be generalized. BTW, your link doesn't resolve. Typo somewhere?

On Tue, Dec 6, 2016 at 8:47 AM, Francois Ozog  wrote:
Devicetree has been formed by and for SoC vendors to represent what they have 
on their platforms (CPUs, Memory, interconnects, IOMMUs, ports, IRQ lines, 
firmware - 
http://free-electrons.com/pub/conferences/2013/elce/petazzoni-device-tree-dummies/petazzoni-device-tree-dummies.pdf).
 so it looks like very similar to what Petri is talking about.  

ACPI has also a concept of a proximity domains and the relative cost to use a 
resource from another domain.

Handling NUMA requires a well thought through approach because it has 
implications at many levels in particular for device and driver framework.

Bottom line, regardless if it's value, the introduction of the API seem too 
early as I haven't seen the use case that it supports and in particular the 
device and drivers framework.

FF






On 6 December 2016 at 14:23, Bill Fischofer  wrote:


On Tue, Dec 6, 2016 at 5:03 AM, Francois Ozog  wrote:
Hi Bill,

could you clarify if this "device" is related to devicetree as defined by 
Linaro (https://github.com/devicetree-org/devicetree-specification) or the 
devices that are in the scope of Christophe Millard, or some other device 
concept ?

This is the Device ID proposal that Petri suggested at BKK16. It is not related 
to devtree or any other specific taxonomy as the idea was to establish an 
abstract "device" framework that would support both individual sockets or 
multi-SoC environments. In odp-linux these are basically placeholder functions 
as we don't have any cross-platform concept of NUMA. The idea is that each 
individual platform that does support NUMA should be able to map their native 
concepts into this framework to permit applications to associate an internal 
odp_dev_t handle with named devices.
 

FF



On 6 December 2016 at 04:43, Bill Fischofer  wrote:
Ping. v3 of this patch is still awaiting review.

On Mon, Oct 24, 2016 at 4:29 PM, Bill Fischofer
 wrote:
> Add the odp_dev_id() API used for NUMA support
>
> Signed-off-by: Bill Fischofer 
> ---
> Changes for v3:
> - Correct ODP_DEV_ANY to ODP_DEV_DEFAULT
>
> Changes for v2:
> - Incorporate changes suggested by Petri
>
>  include/odp/api/spec/dev.h | 89 

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

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

I know device trees and how ugly those can be ….  values are not documented but 
you (your driver) just have to know what some list of four integers (reg = <4, 
78865, 6, 565>) may actually mean…

Anyway, the idea here is that

· an implementation has a way to enumerate resources to the user as 
names

o   a name may come from a standard or system specific tool (ifconfig), a 
configuration file (even device trees), etc. API does not dictate how user gets 
a resource name, or which kind of string  that name is.

· User passes the name to the application (through command line, config 
file, env variable). API don’t dictate how that happens.

· Application passes the name to the implementation and gets a device 
ID (odp_dev_t), which it uses as a parameter on other calls

Resource discovery/enumeration/management/etc is left to upper layer. API is 
only needed for passing resource identifier information down to the  
implementation. And this is how it should be: ODP is a passive library which 
does what it’s told to do, the active role is on upper layers (user, scripts, 
resource management SW, etc entity).

So, no need to defer the API since it’s flexible enough (to pass any kind of 
string down to implementation). Driver, cloud, etc frameworks may then specify 
the dev name string format, etc later on, but it would not change the API if a 
timer is called “timer0” or “timer0:2” or “/proc/soc/0/timer/1” or something 
else.

-Petri



From: Francois Ozog [mailto:francois.o...@linaro.org]
Sent: Tuesday, December 06, 2016 5:10 PM
To: Bill Fischofer 
Cc: lng-odp-forward ; Savolainen, Petri (Nokia - 
FI/Espoo) ; Christophe Milard 

Subject: Re: [lng-odp] [API-NEXT PATCHv3 1/3] api: dev: add device apis for 
numa support

For the link, I think it ate the  following it.
Here it is without decoration:
http://free-electrons.com/pub/conferences/2013/elce/petazzoni-device-tree-dummies/petazzoni-device-tree-dummies.pdf



On 6 December 2016 at 16:05, Francois Ozog 
> wrote:
OK. So let's defer the API but not the discussion.
I think we need a "strategic architecture" call.

FF



On 6 December 2016 at 15:59, Bill Fischofer 
> wrote:
I have no problem deferring this if we want to have one of the SoC vendors 
drive a more comprehensive approach based on their capabilities which can then 
be generalized. BTW, your link doesn't resolve. Typo somewhere?

On Tue, Dec 6, 2016 at 8:47 AM, Francois Ozog 
> wrote:
Devicetree has been formed by and for SoC vendors to represent what they have 
on their platforms (CPUs, Memory, interconnects, IOMMUs, ports, IRQ lines, 
firmware - 
http://free-electrons.com/pub/conferences/2013/elce/petazzoni-device-tree-dummies/petazzoni-device-tree-dummies.pdf).
 so it looks like very similar to what Petri is talking about.

ACPI has also a concept of a proximity domains and the relative cost to use a 
resource from another domain.

Handling NUMA requires a well thought through approach because it has 
implications at many levels in particular for device and driver framework.

Bottom line, regardless if it's value, the introduction of the API seem too 
early as I haven't seen the use case that it supports and in particular the 
device and drivers framework.

FF






On 6 December 2016 at 14:23, Bill Fischofer 
> wrote:


On Tue, Dec 6, 2016 at 5:03 AM, Francois Ozog 
> wrote:
Hi Bill,

could you clarify if this "device" is related to devicetree as defined by 
Linaro (https://github.com/devicetree-org/devicetree-specification) or the 
devices that are in the scope of Christophe Millard, or some other device 
concept ?

This is the Device ID proposal that Petri suggested at BKK16. It is not related 
to devtree or any other specific taxonomy as the idea was to establish an 
abstract "device" framework that would support both individual sockets or 
multi-SoC environments. In odp-linux these are basically placeholder functions 
as we don't have any cross-platform concept of NUMA. The idea is that each 
individual platform that does support NUMA should be able to map their native 
concepts into this framework to permit applications to associate an internal 
odp_dev_t handle with named devices.


FF



On 6 December 2016 at 04:43, Bill Fischofer 
> wrote:
Ping. v3 of this patch is still awaiting review.

On Mon, Oct 24, 2016 at 4:29 PM, Bill Fischofer
> wrote:
> Add the odp_dev_id() API used for NUMA support
>
> Signed-off-by: Bill Fischofer 
> 

Re: [lng-odp] [API-NEXT PATCH v2 4/7] api: crypto: added session params init

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

> >  /**
> > + * Initialize crypto session parameters
> > + *
> > + * Initialize an odp_crypto_session_params_t to its default values for
> > + * all fields.
> > + *
> > + * @param params   Pointer to odp_crypto_session_params_t to be
> initialized
> > + */
> > +void odp_crypto_session_params_init(odp_crypto_session_params_t
> *params);
> 
> Since we're cleaning up and adding this for completeness, the
> underlying struct should be changed to odp_crypto_session_param_t and
> this API should be odp_crypto_session_param_init() for consistency
> with all other odp_xxx_param_t and odp_xxx_param_init() functions. The
> inconsistent use of params instead of param in the crypto API was an
> oversight in Monarch.
>

It would be nice if only _param_t would be used throughout all APIs, but this 
would not be the only one to change. This is the list of all _params_t:

odp_crypto_session_params_t
odp_crypto_op_params_t

odp_tm_shaper_params_t
odp_tm_sched_params_t
odp_tm_threshold_params_t
odp_tm_wred_params_t
odp_tm_node_params_t
odp_tm_queue_params_t

odph_linux_thr_params_t
odph_odpthread_params_t


It's questionable if it pays off to rename 8 types in the API, since type name 
change is non-backward compatible and the gain is rather cosmetic / 
nice-to-have. I could deprecated old type names for crypto in this patch set 
and add new ones if everybody agrees.

-Petri

Re: [lng-odp] [API-NEXT PATCH v2 1/7] api: crypto: decouple key length from algorithm enumeration

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

> >  /**
> > + * Cipher algorithm capabilities
> > + */
> > +typedef struct odp_crypto_cipher_capa_t {
> > +   /** Key length in bytes */
> > +   uint32_t key_len;
> > +
> > +   /** IV length in bytes */
> > +   uint32_t iv_len;
> > +
> > +} odp_crypto_cipher_capa_t;
> 
> This should be odp_crypto_cipher_capability_t for consistency with
> other odp_xxx_capability_t types.

int odp_crypto_cipher_capability(odp_cipher_alg_t cipher,
   odp_crypto_cipher_capability_t 
capa[], int num);


It's getting rather long (28 char function name). For example, the line above 
does not fit into 80 chars any more. Also odp_cipher_alg_t is not 
odp_cipher_algorithm_t, but an abbreviation of that.

-Petri

Re: [lng-odp] [PATCH] validation: packet: fix concat test

[Nicolas Morey-Chaisemartin]

The goal for me is to get this into monarch_lts so I posted it on master first.

If API-next should fix this, I'll post the monarch patch instead


Nicolas


Le 12/07/2016 à 10:21 AM, Savolainen, Petri (Nokia - FI/Espoo) a écrit :
> Pool re-implementation in api-next fixes a number of validation test bugs. 
> This may be fixed already there, since I first implemented pools with single 
> segment. Also concat test tried to concat a single packet into the end of 
> itself...
>
> I suggest to postpone this until api-next is merged to master, or try 
> api-next in the meanwhile.
>
> -Petri
>
>> -Original Message-
>> From: lng-odp [mailto:lng-odp-boun...@lists.linaro.org] On Behalf Of
>> Nicolas Morey-Chaisemartin
>> Sent: Wednesday, December 07, 2016 11:01 AM
>> To: lng-odp@lists.linaro.org
>> Subject: [lng-odp] [PATCH] validation: packet: fix concat test
>>
>> concat test assumes that segmentation is supported.
>> Add an extra concat_test_packet  that can fit twice into
>>  seglen if segmentation is not supported.
>>
>> Signed-off-by: Nicolas Morey-Chaisemartin 
>> ---
>>
>> The patch should be applied on monarch_lts too. apart from the path
>> change, it applies withotu conflict.
>>
>>  test/common_plat/validation/api/packet/packet.c | 23 ++--
>> ---
>>  1 file changed, 18 insertions(+), 5 deletions(-)
>>
>> diff --git a/test/common_plat/validation/api/packet/packet.c
>> b/test/common_plat/validation/api/packet/packet.c
>> index a4426e2..b8af2f8 100644
>> --- a/test/common_plat/validation/api/packet/packet.c
>> +++ b/test/common_plat/validation/api/packet/packet.c
>> @@ -15,12 +15,12 @@
>>  #define PACKET_TAILROOM_RESERVE  4
>>
>>  static odp_pool_t packet_pool, packet_pool_no_uarea,
>> packet_pool_double_uarea;
>> -static uint32_t packet_len;
>> +static uint32_t packet_len, concat_packet_len;
>>
>>  static uint32_t segmented_packet_len;
>>  static odp_bool_t segmentation_supported = true;
>>
>> -odp_packet_t test_packet, segmented_test_packet;
>> +odp_packet_t test_packet, concat_test_packet, segmented_test_packet;
>>
>>  static struct udata_struct {
>>  uint64_t u64;
>> @@ -111,6 +111,18 @@ int packet_suite_init(void)
>>  data++;
>>  }
>>
>> +if (segmentation_supported)
>> +concat_packet_len = packet_len;
>> +else
>> +concat_packet_len = packet_len / 2;
>> +
>> +concat_test_packet = odp_packet_alloc(packet_pool,
>> concat_packet_len);
>> +
>> +for (i = 0; i < concat_packet_len; i++) {
>> +odp_packet_copy_from_mem(concat_test_packet, i, 1, );
>> +data++;
>> +}
>> +
>>  udat = odp_packet_user_area(test_packet);
>>  udat_size = odp_packet_user_area_size(test_packet);
>>  if (!udat || udat_size != sizeof(struct udata_struct))
>> @@ -1152,8 +1164,9 @@ void packet_test_concatsplit(void)
>>  uint32_t pkt_len;
>>  odp_packet_t splits[4];
>>
>> -pkt = odp_packet_copy(test_packet, odp_packet_pool(test_packet));
>> -pkt_len = odp_packet_len(test_packet);
>> +pkt = odp_packet_copy(concat_test_packet,
>> +  odp_packet_pool(concat_test_packet));
>> +pkt_len = odp_packet_len(concat_test_packet);
>>  CU_ASSERT_FATAL(pkt != ODP_PACKET_INVALID);
>>
>>  CU_ASSERT(odp_packet_concat(, pkt) == 0);
>> @@ -1165,7 +1178,7 @@ void packet_test_concatsplit(void)
>>  CU_ASSERT(odp_packet_data(pkt) != odp_packet_data(pkt2));
>>  CU_ASSERT(odp_packet_len(pkt) == odp_packet_len(pkt2));
>>  _packet_compare_data(pkt, pkt2);
>> -_packet_compare_data(pkt, test_packet);
>> +_packet_compare_data(pkt, concat_test_packet);
>>
>>  odp_packet_free(pkt);
>>  odp_packet_free(pkt2);
>> --
>> 2.10.1.4.g0ffc436

Re: [lng-odp] [PATCH] validation: packet: fix concat test

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

Pool re-implementation in api-next fixes a number of validation test bugs. This 
may be fixed already there, since I first implemented pools with single 
segment. Also concat test tried to concat a single packet into the end of 
itself...

I suggest to postpone this until api-next is merged to master, or try api-next 
in the meanwhile.

-Petri

> -Original Message-
> From: lng-odp [mailto:lng-odp-boun...@lists.linaro.org] On Behalf Of
> Nicolas Morey-Chaisemartin
> Sent: Wednesday, December 07, 2016 11:01 AM
> To: lng-odp@lists.linaro.org
> Subject: [lng-odp] [PATCH] validation: packet: fix concat test
> 
> concat test assumes that segmentation is supported.
> Add an extra concat_test_packet  that can fit twice into
>  seglen if segmentation is not supported.
> 
> Signed-off-by: Nicolas Morey-Chaisemartin 
> ---
> 
> The patch should be applied on monarch_lts too. apart from the path
> change, it applies withotu conflict.
> 
>  test/common_plat/validation/api/packet/packet.c | 23 ++--
> ---
>  1 file changed, 18 insertions(+), 5 deletions(-)
> 
> diff --git a/test/common_plat/validation/api/packet/packet.c
> b/test/common_plat/validation/api/packet/packet.c
> index a4426e2..b8af2f8 100644
> --- a/test/common_plat/validation/api/packet/packet.c
> +++ b/test/common_plat/validation/api/packet/packet.c
> @@ -15,12 +15,12 @@
>  #define PACKET_TAILROOM_RESERVE  4
> 
>  static odp_pool_t packet_pool, packet_pool_no_uarea,
> packet_pool_double_uarea;
> -static uint32_t packet_len;
> +static uint32_t packet_len, concat_packet_len;
> 
>  static uint32_t segmented_packet_len;
>  static odp_bool_t segmentation_supported = true;
> 
> -odp_packet_t test_packet, segmented_test_packet;
> +odp_packet_t test_packet, concat_test_packet, segmented_test_packet;
> 
>  static struct udata_struct {
>   uint64_t u64;
> @@ -111,6 +111,18 @@ int packet_suite_init(void)
>   data++;
>   }
> 
> + if (segmentation_supported)
> + concat_packet_len = packet_len;
> + else
> + concat_packet_len = packet_len / 2;
> +
> + concat_test_packet = odp_packet_alloc(packet_pool,
> concat_packet_len);
> +
> + for (i = 0; i < concat_packet_len; i++) {
> + odp_packet_copy_from_mem(concat_test_packet, i, 1, );
> + data++;
> + }
> +
>   udat = odp_packet_user_area(test_packet);
>   udat_size = odp_packet_user_area_size(test_packet);
>   if (!udat || udat_size != sizeof(struct udata_struct))
> @@ -1152,8 +1164,9 @@ void packet_test_concatsplit(void)
>   uint32_t pkt_len;
>   odp_packet_t splits[4];
> 
> - pkt = odp_packet_copy(test_packet, odp_packet_pool(test_packet));
> - pkt_len = odp_packet_len(test_packet);
> + pkt = odp_packet_copy(concat_test_packet,
> +   odp_packet_pool(concat_test_packet));
> + pkt_len = odp_packet_len(concat_test_packet);
>   CU_ASSERT_FATAL(pkt != ODP_PACKET_INVALID);
> 
>   CU_ASSERT(odp_packet_concat(, pkt) == 0);
> @@ -1165,7 +1178,7 @@ void packet_test_concatsplit(void)
>   CU_ASSERT(odp_packet_data(pkt) != odp_packet_data(pkt2));
>   CU_ASSERT(odp_packet_len(pkt) == odp_packet_len(pkt2));
>   _packet_compare_data(pkt, pkt2);
> - _packet_compare_data(pkt, test_packet);
> + _packet_compare_data(pkt, concat_test_packet);
> 
>   odp_packet_free(pkt);
>   odp_packet_free(pkt2);
> --
> 2.10.1.4.g0ffc436

[lng-odp] [PATCH] validation: classification: increase SHM_PKT_BUF_SIZE

[Nicolas Morey-Chaisemartin]

classification_test_pmr_term_packet_len allocs a packet with 1024
 of payload + headers which is greater than the segsize used to allocate
 the pool and breaks on implementation without segmentation support

Signed-off-by: Nicolas Morey-Chaisemartin 
---

Should be aplied on monarch_lts too. Applies without conflict apart from path 
change

 test/common_plat/validation/api/classification/classification.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/test/common_plat/validation/api/classification/classification.h 
b/test/common_plat/validation/api/classification/classification.h
index d73c821..73de40a 100644
--- a/test/common_plat/validation/api/classification/classification.h
+++ b/test/common_plat/validation/api/classification/classification.h
@@ -10,7 +10,7 @@
 #include 
 
 #define SHM_PKT_NUM_BUFS32
-#define SHM_PKT_BUF_SIZE1024
+#define SHM_PKT_BUF_SIZE1088
 
 /* Config values for Default CoS */
 #define TEST_DEFAULT   1
-- 
2.10.1.4.g0ffc436

Re: [lng-odp] [APO-NEXT PATCHv6 1/3] api: random: add explicit controls over random data

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

> -Original Message-
> From: lng-odp [mailto:lng-odp-boun...@lists.linaro.org] On Behalf Of Bill
> Fischofer
> Sent: Monday, December 05, 2016 11:29 PM
> To: lng-odp@lists.linaro.org
> Subject: [lng-odp] [APO-NEXT PATCHv6 1/3] api: random: add explicit
> controls over random data
> 
> Rework the odp_random_data() API to replace the use_entropy with an
> explicit odp_random_kind parameter that controls the type of random
> desired. Two new APIs are also introduced:
> 
> - odp_random_max_kind() returns the maximum kind of random data available
> 
> - odp_random_repeatable_data() permits applications to generate repeatable
>   random sequences for testing purposes
> 
> Because the type signature of odp_random_data() is changed, the
> implementation and validation tests are included here for bisectability.

This does not need to be mentioned in git log, since it's our rule anyway (that 
every commit must build and pass tests). Actually, it would be cleaner to 
include only the minimum amount of implementation changes into the API patch: 
the new odp_random_data() implementation and the test for that (single line 
change).



> --- a/include/odp/api/spec/random.h
> +++ b/include/odp/api/spec/random.h
> @@ -24,18 +24,68 @@ extern "C" {
>   */
> 
>  /**
> + * Random kind selector
> + */
> +typedef enum {
> + /** Basic random, presumably pseudo-random generated by SW */
> + ODP_RANDOM_BASIC,
> + /** Cryptographic quality random */
> + ODP_RANDOM_CRYPTO,
> + /** True random, generated from a HW entropy source */
> + ODP_RANDOM_TRUE,
> +} odp_random_kind_t;
> +
> +/**
> + * Query random max kind
> + *

This would be a good place to state the order of kinds: which is the "max" end 
the enum.

> + * @return kind The maximum odp_random_kind_t supported by this
> implementation
> + */
> +odp_random_kind_t odp_random_max_kind(void);
> +
> +/**
>   * Generate random byte data
>   *
> + * The intent in supporting different kinds of random data is to allow
> + * tradeoffs between performance and the quality of random data needed.
> The
> + * assumption is that basic random is cheap while true random is
> relatively
> + * expensive in terms of time to generate, with cryptographic random
> being
> + * something in between. Implementations that support highly efficient
> true
> + * random are free to use this for all requested kinds. So it is always
> + * permissible to "upgrade" a random data request, but never to
> "downgrade"
> + * such requests.
> + *
>   * @param[out]buf   Output buffer
> - * @param size  Size of output buffer
> - * @param use_entropy   Use entropy
> + * @param len   Length of output buffer in bytes
> + * @param kind  Specifies the type of random data required.
> Request
> + *  is expected to fail if the implementation is
> unable to
> + *  provide the requested type.
> + *
> + * @return Number of bytes written
> + * @retval <0 on failure
> + */
> +int32_t odp_random_data(uint8_t *buf, uint32_t len, odp_random_kind_t
> kind);
> +
> +/**
> + * Generate repeatable random byte data
> + *
> + * For testing purposes it is often useful to generate "random" sequences
> + * that are repeatable. This is accomplished by supplying a seed value
> that
> + * is used for pseudo-random data generation. The caller provides
>   *
> - * @todo Define the implication of the use_entropy parameter
> + * @param[out]buf  Output buffer
> + * @param len  Length of output buffer in bytes
> + * @param kind Specifies the type of random data required.
> Request
> + * will fail if the implementation is unable to
> provide
> + * repeatable random of the requested type. This is
> + * always true for true random and may be true for
> + * cryptographic random.
> + * @param[in,out] seed Seed value to use
>   *
>   * @return Number of bytes written
>   * @retval <0 on failure
>   */
> -int32_t odp_random_data(uint8_t *buf, int32_t size, odp_bool_t
> use_entropy);
> +int32_t odp_random_repeatable_data(uint8_t *buf, uint32_t len,
> +odp_random_kind_t kind, uint32_t *seed);
> 

If pseudo/deterministic is not a good term for this, then I think it's better 
to profile it strictly for testing (all production cases are covered by 
odp_random_data()). Also for testing the kind is not needed, we just say that: 
"this is good only for testing, do not use for production". If kind would be 
there, it would give two ways to achieve "cryptograptic quality" randoms and 
that's not the intention of the call. Also a larger seed enables better 
randomness of the output.


/**
 * Generate repeatable random data for testing purposes
 *
 * For testing purposes it is often useful to generate "random" sequences
 * that are repeatable...
 *
 * This function should be used only for testing purposes, use odp_random_data()
 * for 

[lng-odp] [PATCH] validation: packet: fix concat test

[Nicolas Morey-Chaisemartin]

concat test assumes that segmentation is supported.
Add an extra concat_test_packet  that can fit twice into
 seglen if segmentation is not supported.

Signed-off-by: Nicolas Morey-Chaisemartin 
---

The patch should be applied on monarch_lts too. apart from the path change, it 
applies withotu conflict.

 test/common_plat/validation/api/packet/packet.c | 23 ++-
 1 file changed, 18 insertions(+), 5 deletions(-)

diff --git a/test/common_plat/validation/api/packet/packet.c 
b/test/common_plat/validation/api/packet/packet.c
index a4426e2..b8af2f8 100644
--- a/test/common_plat/validation/api/packet/packet.c
+++ b/test/common_plat/validation/api/packet/packet.c
@@ -15,12 +15,12 @@
 #define PACKET_TAILROOM_RESERVE  4
 
 static odp_pool_t packet_pool, packet_pool_no_uarea, packet_pool_double_uarea;
-static uint32_t packet_len;
+static uint32_t packet_len, concat_packet_len;
 
 static uint32_t segmented_packet_len;
 static odp_bool_t segmentation_supported = true;
 
-odp_packet_t test_packet, segmented_test_packet;
+odp_packet_t test_packet, concat_test_packet, segmented_test_packet;
 
 static struct udata_struct {
uint64_t u64;
@@ -111,6 +111,18 @@ int packet_suite_init(void)
data++;
}
 
+   if (segmentation_supported)
+   concat_packet_len = packet_len;
+   else
+   concat_packet_len = packet_len / 2;
+
+   concat_test_packet = odp_packet_alloc(packet_pool, concat_packet_len);
+
+   for (i = 0; i < concat_packet_len; i++) {
+   odp_packet_copy_from_mem(concat_test_packet, i, 1, );
+   data++;
+   }
+
udat = odp_packet_user_area(test_packet);
udat_size = odp_packet_user_area_size(test_packet);
if (!udat || udat_size != sizeof(struct udata_struct))
@@ -1152,8 +1164,9 @@ void packet_test_concatsplit(void)
uint32_t pkt_len;
odp_packet_t splits[4];
 
-   pkt = odp_packet_copy(test_packet, odp_packet_pool(test_packet));
-   pkt_len = odp_packet_len(test_packet);
+   pkt = odp_packet_copy(concat_test_packet,
+ odp_packet_pool(concat_test_packet));
+   pkt_len = odp_packet_len(concat_test_packet);
CU_ASSERT_FATAL(pkt != ODP_PACKET_INVALID);
 
CU_ASSERT(odp_packet_concat(, pkt) == 0);
@@ -1165,7 +1178,7 @@ void packet_test_concatsplit(void)
CU_ASSERT(odp_packet_data(pkt) != odp_packet_data(pkt2));
CU_ASSERT(odp_packet_len(pkt) == odp_packet_len(pkt2));
_packet_compare_data(pkt, pkt2);
-   _packet_compare_data(pkt, test_packet);
+   _packet_compare_data(pkt, concat_test_packet);
 
odp_packet_free(pkt);
odp_packet_free(pkt2);
-- 
2.10.1.4.g0ffc436

Re: [lng-odp] [PATCH v2 4/6] helper: use ODP_LIB_FLAVOR to generate libodp name

[Nicolas Morey-Chaisemartin]

The test code needs some of those (the no_barrier for examples).

I'm pretty sure the hashtable will need some of those because it used a 
globally shared structure.



Le 12/07/2016 à 09:15 AM, Maxim Uvarov a écrit :
> it's ok if this places in api. But do we have such places in examples or 
> helper code?
>
> On 7 December 2016 at 10:43, Nicolas Morey-Chaisemartin  > wrote:
>
> I usually force somme access to be uncached. A whole barrier would work, 
> but the performance cost is way too big.
>
>
> Just as an example:
>
> static inline void odp_atomic_init_u64(odp_atomic_u64_t *atom, uint64_t 
> val)
> {
> atom->v = val;
> #if __GCC_ATOMIC_LLONG_LOCK_FREE < 2
> __atomic_clear(>lock, __ATOMIC_RELAXED);
> #endif
> }
>
>
> becomes
>
> static inline void odp_atomic_init_u64(odp_atomic_u64_t *atom, uint64_t 
> val)
> {
> STORE_U64(atom->v, val);
> }
>
> where STORE_U64 forces an uncached write directly to memory.
>
> Nicolas
>
>
> Le 12/07/2016 à 08:09 AM, Maxim Uvarov a écrit :
>> Nicolas, what do you do to solve coherency problem? I think that maybe 
>> we are missing some api in odp, like read/write barriers.
>>
>> Maxim.
>>
>> On 7 December 2016 at 09:54, Nicolas Morey-Chaisemartin 
>> > wrote:
>>
>>
>>
>> Le 12/06/2016 à 09:37 PM, Mike Holmes a écrit :
>> > On 6 December 2016 at 12:08, Nicolas Morey-Chaisemartin
>> > > wrote:
>> >> Signed-off-by: Nicolas Morey-Chaisemartin > >
>> >> ---
>> >>  helper/Makefile.am  | 6 +++---
>> >>  helper/test/Makefile.am | 6 +++---
>> >>  2 files changed, 6 insertions(+), 6 deletions(-)
>> >>
>> >> diff --git a/helper/Makefile.am b/helper/Makefile.am
>> >> index d09d900..c75db00 100644
>> >> --- a/helper/Makefile.am
>> >> +++ b/helper/Makefile.am
>> >> @@ -1,7 +1,7 @@
>> >>  include $(top_srcdir)/platform/@with_platform@/Makefile.inc 
>> 
>> >>
>> >>  pkgconfigdir = $(libdir)/pkgconfig
>> >> -pkgconfig_DATA = $(top_builddir)/pkgconfig/libodphelper-linux.pc
>> >> +pkgconfig_DATA = 
>> $(top_builddir)/pkgconfig/libodphelper@ODP_LIB_FLAVOR@.pc
>> > Why would the helpers be tied to the implementation being built ? 
>> They
>> > should be agnostic and depend on the OS
>> I agree that they should not depend on the implementation of ODP. 
>> But as for libodp, helper might not be ABI compatible.
>> The issue is that there's no evident place in the connfigure to 
>> select which implementation of the helper should be used.
>> platform configure.m4 are the easiest place to add this which ties 
>> this to the platform.
>> But I could add an ODPH_LIB_IMPL. So several platforms could use the 
>> same helper, while keeping the possibility of using non ABI compatible 
>> versions in other platforms.
>>
>> > I think the helpers are broken or we need an alternative to 
>> linux.c in
>> > there, perhaps a odp/helpers/platform/linux/linux.c and
>> > odp/helpers/platform/mpaa/linux.c
>> I changed it in our git tree. helper/linux.c got moved to 
>> helper/os/linux/linux.c
>> And we added helper/os/nodeos/linux.c and helper/os/mos/linux.c (we 
>> have 2 OS flavors usable with ODP)
>> The platform configure.m4 is in charge of selecting the OS depending 
>> on flags & compiler selected.
>> >
>> >>  LIB   = $(top_builddir)/lib
>> >>  AM_CFLAGS  = -I$(srcdir)/include
>> >> @@ -31,7 +31,7 @@ noinst_HEADERS = \
>> >>  $(srcdir)/odph_lineartable.h \
>> >>  $(srcdir)/odph_list_internal.h
>> >>
>> >> -__LIB__libodphelper_linux_la_SOURCES = \
>> >> +__LIB__libodphelper@ODP_LIB_FLAVOR@_la_SOURCES = \
>> >> eth.c \
>> >> ip.c \
>> >> chksum.c \
>> >> @@ -39,4 +39,4 @@ __LIB__libodphelper_linux_la_SOURCES = \
>> >> hashtable.c \
>> >> lineartable.c
>> >>
>> >> -lib_LTLIBRARIES = $(LIB)/libodphelper-linux.la 
>> 
>> >> +lib_LTLIBRARIES = $(LIB)/libodphelper@ODP_LIB_FLAVOR@.la 
>> 
>> >> diff --git a/helper/test/Makefile.am b/helper/test/Makefile.am
>> >> index 545db73..02c260c 100644
>> >> --- a/helper/test/Makefile.am
>> >> +++ b/helper/test/Makefile.am
>> >> @@ -28,10 +28,10 @@ EXTRA_DIST = 

Re: [lng-odp] [PATCH v2 4/6] helper: use ODP_LIB_FLAVOR to generate libodp name

[Maxim Uvarov]

it's ok if this places in api. But do we have such places in examples or
helper code?

On 7 December 2016 at 10:43, Nicolas Morey-Chaisemartin 
wrote:

> I usually force somme access to be uncached. A whole barrier would work,
> but the performance cost is way too big.
>
>
> Just as an example:
>
> static inline void odp_atomic_init_u64(odp_atomic_u64_t *atom, uint64_t
> val)
> {
> atom->v = val;
> #if __GCC_ATOMIC_LLONG_LOCK_FREE < 2
> __atomic_clear(>lock, __ATOMIC_RELAXED);
> #endif
> }
>
>
> becomes
>
> static inline void odp_atomic_init_u64(odp_atomic_u64_t *atom, uint64_t
> val)
> {
> STORE_U64(atom->v, val);
> }
> where STORE_U64 forces an uncached write directly to memory.
>
> Nicolas
>
>
> Le 12/07/2016 à 08:09 AM, Maxim Uvarov a écrit :
>
> Nicolas, what do you do to solve coherency problem? I think that maybe we
> are missing some api in odp, like read/write barriers.
>
> Maxim.
>
> On 7 December 2016 at 09:54, Nicolas Morey-Chaisemartin 
> wrote:
>
>>
>>
>> Le 12/06/2016 à 09:37 PM, Mike Holmes a écrit :
>> > On 6 December 2016 at 12:08, Nicolas Morey-Chaisemartin
>> >  wrote:
>> >> Signed-off-by: Nicolas Morey-Chaisemartin 
>> >> ---
>> >>  helper/Makefile.am  | 6 +++---
>> >>  helper/test/Makefile.am | 6 +++---
>> >>  2 files changed, 6 insertions(+), 6 deletions(-)
>> >>
>> >> diff --git a/helper/Makefile.am b/helper/Makefile.am
>> >> index d09d900..c75db00 100644
>> >> --- a/helper/Makefile.am
>> >> +++ b/helper/Makefile.am
>> >> @@ -1,7 +1,7 @@
>> >>  include $(top_srcdir)/platform/@with_platform@/Makefile.inc
>> >>
>> >>  pkgconfigdir = $(libdir)/pkgconfig
>> >> -pkgconfig_DATA = $(top_builddir)/pkgconfig/libodphelper-linux.pc
>> >> +pkgconfig_DATA = $(top_builddir)/pkgconfig/libo
>> dphelper@ODP_LIB_FLAVOR@.pc
>> > Why would the helpers be tied to the implementation being built ? They
>> > should be agnostic and depend on the OS
>> I agree that they should not depend on the implementation of ODP. But as
>> for libodp, helper might not be ABI compatible.
>> The issue is that there's no evident place in the connfigure to select
>> which implementation of the helper should be used.
>> platform configure.m4 are the easiest place to add this which ties this
>> to the platform.
>> But I could add an ODPH_LIB_IMPL. So several platforms could use the same
>> helper, while keeping the possibility of using non ABI compatible versions
>> in other platforms.
>>
>> > I think the helpers are broken or we need an alternative to linux.c in
>> > there, perhaps a odp/helpers/platform/linux/linux.c and
>> > odp/helpers/platform/mpaa/linux.c
>> I changed it in our git tree. helper/linux.c got moved to
>> helper/os/linux/linux.c
>> And we added helper/os/nodeos/linux.c and helper/os/mos/linux.c (we have
>> 2 OS flavors usable with ODP)
>> The platform configure.m4 is in charge of selecting the OS depending on
>> flags & compiler selected.
>> >
>> >>  LIB   = $(top_builddir)/lib
>> >>  AM_CFLAGS  = -I$(srcdir)/include
>> >> @@ -31,7 +31,7 @@ noinst_HEADERS = \
>> >>  $(srcdir)/odph_lineartable.h \
>> >>  $(srcdir)/odph_list_internal.h
>> >>
>> >> -__LIB__libodphelper_linux_la_SOURCES = \
>> >> +__LIB__libodphelper@ODP_LIB_FLAVOR@_la_SOURCES = \
>> >> eth.c \
>> >> ip.c \
>> >> chksum.c \
>> >> @@ -39,4 +39,4 @@ __LIB__libodphelper_linux_la_SOURCES = \
>> >> hashtable.c \
>> >> lineartable.c
>> >>
>> >> -lib_LTLIBRARIES = $(LIB)/libodphelper-linux.la
>> >> +lib_LTLIBRARIES = $(LIB)/libodphelper@ODP_LIB_FLAVOR@.la
>> >> diff --git a/helper/test/Makefile.am b/helper/test/Makefile.am
>> >> index 545db73..02c260c 100644
>> >> --- a/helper/test/Makefile.am
>> >> +++ b/helper/test/Makefile.am
>> >> @@ -28,10 +28,10 @@ EXTRA_DIST = odpthreads_as_processes
>> odpthreads_as_pthreads
>> >>
>> >>  dist_chksum_SOURCES = chksum.c
>> >>  dist_odpthreads_SOURCES = odpthreads.c
>> >> -odpthreads_LDADD = $(LIB)/libodphelper-linux.la $(LIB)/
>> libodp-linux.la
>> >> +odpthreads_LDADD = $(LIB)/libodphelper@ODP_LIB_FLAVOR@.la
>> $(LIB)/libodp@ODP_LIB_FLAVOR@.la
>> >>  dist_thread_SOURCES = thread.c
>> >> -thread_LDADD = $(LIB)/libodphelper-linux.la $(LIB)/libodp-linux.la
>> >> +thread_LDADD = $(LIB)/libodphelper@ODP_LIB_FLAVOR@.la
>> $(LIB)/libodp@ODP_LIB_FLAVOR@.la
>> >>  dist_process_SOURCES = process.c
>> >>  dist_parse_SOURCES = parse.c
>> >> -process_LDADD = $(LIB)/libodphelper-linux.la $(LIB)/libodp-linux.la
>> >> +process_LDADD = $(LIB)/libodphelper@ODP_LIB_FLAVOR@.la
>> $(LIB)/libodp@ODP_LIB_FLAVOR@.la
>> >>  dist_table_SOURCES = table.c
>> >> --
>> >> 2.10.1.4.g0ffc436
>> >>
>> >>
>> >
>> >
>>
>>
>
>

[lng-odp] odp_rwlock_read_trylock

[Nicolas Morey-Chaisemartin]

HI,


While looking into the test/code, I noticed something weird in the 
implementation of the rwlock read trylock on monarch.

int odp_rwlock_read_trylock(odp_rwlock_t *rwlock)
{
uint32_t zero = 0;

return odp_atomic_cas_acq_u32(>cnt, , (uint32_t)1);
}


This mean the trylock only succeed if no one was using the lock. But it will 
fail if someone was owning it *even* if it is a reader.
Is this something expected? If yes, it should probably be detailed in the API.


The lock implementation I wrote allows to get the lock if a reader already owns 
it. And causes the testsuite to deadlock:

odp_rwlock_init(rw_lock);
/* CU_ASSERT(odp_rwlock_is_locked(rw_lock) == 0); */

odp_rwlock_read_lock(rw_lock); => Lock is owned in read

rc = odp_rwlock_read_trylock(rw_lock); => Locked is owned a second time in 
read (rc = 1)
CU_ASSERT(rc == 0);
rc = odp_rwlock_write_trylock(rw_lock); => Expected failure
CU_ASSERT(rc == 0);

odp_rwlock_read_unlock(rw_lock); => Lock is still owned once.


So either the API should describe more accurately what are the expected 
success/failure case, or the test and implementation changed.

On a side note, the API explicitly says that reader have the priority on 
writers on rwlock. Is this really an API requirement?
Our rwlocks are the other way around to avoid the starvation issue. Do I need 
to change them ? Or is it OK with the API?

Nicolas