Re: [lng-odp] [RFC/API-NEXT 1/1] api: classification: packet hashing per class of service

[Bala Manoharan]

Regards,
Bala


On 10 December 2016 at 02:06, Bill Fischofer  wrote:
> Is this supposed to be instead of queue groups or in addition to queue
> group support?

Yes. Basically the changes is that this proposal merges queue group
and queue together.

>
> On Fri, Dec 9, 2016 at 5:54 AM, Balasubramanian Manoharan
>  wrote:
>> Adds support to spread packet from a single CoS to multiple queues by
>> configuring hashing at CoS level.
>>
>> Signed-off-by: Balasubramanian Manoharan 
>> ---
>>  include/odp/api/spec/classification.h | 45 
>> ---
>>  1 file changed, 42 insertions(+), 3 deletions(-)
>>
>> diff --git a/include/odp/api/spec/classification.h 
>> b/include/odp/api/spec/classification.h
>> index 0e442c7..220b029 100644
>> --- a/include/odp/api/spec/classification.h
>> +++ b/include/odp/api/spec/classification.h
>> @@ -126,6 +126,12 @@ typedef struct odp_cls_capability_t {
>> /** Maximum number of CoS supported */
>> unsigned max_cos;
>>
>> +   /** Maximun number of queue supported per CoS */
>
> typo: queues. Having the variable being max_queue_supported is OK, but
> a better name might be cos_max_queues or max_cos_queues to be explicit
> about this being a CoS limit.
>
>> +   unsigned max_queue_supported;
>> +
>> +   /** Protocol header combination supported for Hashing */
>> +   odp_pktin_hash_proto_t hash_supported;
>> +
>> /** A Boolean to denote support of PMR range */
>> odp_bool_t pmr_range_supported;
>>  } odp_cls_capability_t;
>> @@ -164,9 +170,25 @@ typedef enum {
>>   * Used to communicate class of service creation options
>>   */
>>  typedef struct odp_cls_cos_param {
>> -   odp_queue_t queue;  /**< Queue associated with CoS */
>> -   odp_pool_t pool;/**< Pool associated with CoS */
>> -   odp_cls_drop_t drop_policy; /**< Drop policy associated with CoS 
>> */
>> +   /* Minimum number of queues to be linked to this CoS.
>> +* If the number is greater than 1 then hashing has to be
>> +* enabled */
>> +   uint32_t num_queue;
>
> If an application wishes to assign a queue itself (as is done today)
> then is this set to 0 or 1? This should be stated explicitly. For
> consistency I'd think we'd have 0 = I'm supplying my own queue, >0 =
> implementation should create num_queues.

I thought about this also but having number of queues as 0 seems
contradicting to the basic behaviour of class of service.

>
>> +   /* Denotes whether hashing is enabled for queues in this CoS
>> +* When hashing is enabled the queues are created by the 
>> implementation
>> +* and application need not configure any queue to the class of 
>> service
>> +*/
>> +   odp_bool_t enable_hashing;
>
> Why is this needed if it must be specified if num_queues >1 (and
> presumably shouldn't be specified otherwise)? This seems redundant.

IMO, I understand but having a boolean for enabling or disabling
hashing seemed cleaner since hashing is required to support more than
one queues.

>
>> +   /* Protocol header fields which are included in packet input
>> +* hash calculation */
>> +   odp_pktin_hash_proto_t hash;
>> +   /* If hashing is disabled, then application has to configure
>> +* this queue and packets are delivered to this queue */
>> +   odp_queue_t queue;
>
> Again, this seems like it would be clearer to state that this is
> supplied by the user if num_queues = 0 and ignored otherwise.
>
>> +   /* Pool associated with CoS */
>> +   odp_pool_t pool;
>> +   /* Drop policy associated with CoS */
>> +   odp_cls_drop_t drop_policy;
>>  } odp_cls_cos_param_t;
>>
>>  /**
>> @@ -209,6 +231,23 @@ int odp_cls_capability(odp_cls_capability_t 
>> *capability);
>>  odp_cos_t odp_cls_cos_create(const char *name, odp_cls_cos_param_t *param);
>>
>>  /**
>> + * Queue hash result
>> + * Returns the queue within a CoS in which a particular packet will be 
>> enqueued
>> + * based on the packet parameters and hash protocol field configured with 
>> the
>> + * class of service.
>> + *
>> + * @param  cos class of service
>> + * @param  packet  Packet handle
>> + *
>> + * @retval Returns the queue handle on which this packet will be
>> + * enqueued.
>> + * @retval ODP_QUEUE_INVALID for error case
>> + *
>> + * @note The packet has to be updated with valid header pointers L2, L3 and 
>> L4.
>> + */
>> +odp_queue_t odp_queue_hash_result(odp_cos_t cos, odp_packet_t packet);
>
> What is the use case for this API? It's not obvious why this would be needed.

This mechanism is useful when the application wants to initialise any
context by knowing what will be the queue handle for expected packet
headers. This is required since the queues are created by the
implementation during hashing and application does not know 

Re: [lng-odp] [RFC/API-NEXT 1/1] api: classification: packet hashing per class of service

[Bill Fischofer]

Is this supposed to be instead of queue groups or in addition to queue
group support?

On Fri, Dec 9, 2016 at 5:54 AM, Balasubramanian Manoharan
 wrote:
> Adds support to spread packet from a single CoS to multiple queues by
> configuring hashing at CoS level.
>
> Signed-off-by: Balasubramanian Manoharan 
> ---
>  include/odp/api/spec/classification.h | 45 
> ---
>  1 file changed, 42 insertions(+), 3 deletions(-)
>
> diff --git a/include/odp/api/spec/classification.h 
> b/include/odp/api/spec/classification.h
> index 0e442c7..220b029 100644
> --- a/include/odp/api/spec/classification.h
> +++ b/include/odp/api/spec/classification.h
> @@ -126,6 +126,12 @@ typedef struct odp_cls_capability_t {
> /** Maximum number of CoS supported */
> unsigned max_cos;
>
> +   /** Maximun number of queue supported per CoS */

typo: queues. Having the variable being max_queue_supported is OK, but
a better name might be cos_max_queues or max_cos_queues to be explicit
about this being a CoS limit.

> +   unsigned max_queue_supported;
> +
> +   /** Protocol header combination supported for Hashing */
> +   odp_pktin_hash_proto_t hash_supported;
> +
> /** A Boolean to denote support of PMR range */
> odp_bool_t pmr_range_supported;
>  } odp_cls_capability_t;
> @@ -164,9 +170,25 @@ typedef enum {
>   * Used to communicate class of service creation options
>   */
>  typedef struct odp_cls_cos_param {
> -   odp_queue_t queue;  /**< Queue associated with CoS */
> -   odp_pool_t pool;/**< Pool associated with CoS */
> -   odp_cls_drop_t drop_policy; /**< Drop policy associated with CoS 
> */
> +   /* Minimum number of queues to be linked to this CoS.
> +* If the number is greater than 1 then hashing has to be
> +* enabled */
> +   uint32_t num_queue;

If an application wishes to assign a queue itself (as is done today)
then is this set to 0 or 1? This should be stated explicitly. For
consistency I'd think we'd have 0 = I'm supplying my own queue, >0 =
implementation should create num_queues.

> +   /* Denotes whether hashing is enabled for queues in this CoS
> +* When hashing is enabled the queues are created by the 
> implementation
> +* and application need not configure any queue to the class of 
> service
> +*/
> +   odp_bool_t enable_hashing;

Why is this needed if it must be specified if num_queues >1 (and
presumably shouldn't be specified otherwise)? This seems redundant.

> +   /* Protocol header fields which are included in packet input
> +* hash calculation */
> +   odp_pktin_hash_proto_t hash;
> +   /* If hashing is disabled, then application has to configure
> +* this queue and packets are delivered to this queue */
> +   odp_queue_t queue;

Again, this seems like it would be clearer to state that this is
supplied by the user if num_queues = 0 and ignored otherwise.

> +   /* Pool associated with CoS */
> +   odp_pool_t pool;
> +   /* Drop policy associated with CoS */
> +   odp_cls_drop_t drop_policy;
>  } odp_cls_cos_param_t;
>
>  /**
> @@ -209,6 +231,23 @@ int odp_cls_capability(odp_cls_capability_t *capability);
>  odp_cos_t odp_cls_cos_create(const char *name, odp_cls_cos_param_t *param);
>
>  /**
> + * Queue hash result
> + * Returns the queue within a CoS in which a particular packet will be 
> enqueued
> + * based on the packet parameters and hash protocol field configured with the
> + * class of service.
> + *
> + * @param  cos class of service
> + * @param  packet  Packet handle
> + *
> + * @retval Returns the queue handle on which this packet will be
> + * enqueued.
> + * @retval ODP_QUEUE_INVALID for error case
> + *
> + * @note The packet has to be updated with valid header pointers L2, L3 and 
> L4.
> + */
> +odp_queue_t odp_queue_hash_result(odp_cos_t cos, odp_packet_t packet);

What is the use case for this API? It's not obvious why this would be needed.

> +
> +/**
>   * Discard a class-of-service along with all its associated resources
>   *
>   * @param[in]  cos_id  class-of-service instance.
> --
> 1.9.1
>

[lng-odp] [API-NEXT PATCH 4/4] linux-gen: pktio ipc: make it work again

[Maxim Uvarov]

Signed-off-by: Maxim Uvarov 
---
 .../linux-generic/include/odp_buffer_internal.h|  10 +-
 platform/linux-generic/include/odp_internal.h  |   1 -
 .../include/odp_packet_io_ipc_internal.h   |  27 +-
 platform/linux-generic/odp_init.c  |   5 +-
 platform/linux-generic/pktio/ipc.c | 488 +
 5 files changed, 236 insertions(+), 295 deletions(-)

diff --git a/platform/linux-generic/include/odp_buffer_internal.h 
b/platform/linux-generic/include/odp_buffer_internal.h
index 4e75908..d0eb597 100644
--- a/platform/linux-generic/include/odp_buffer_internal.h
+++ b/platform/linux-generic/include/odp_buffer_internal.h
@@ -64,6 +64,11 @@ struct odp_buffer_hdr_t {
struct {
void *hdr;
uint8_t  *data;
+#ifdef _ODP_PKTIO_IPC
+   /* ipc mapped process can not walk over pointers,
+* offset has to be used */
+   uint64_t ipc_data_offset;
+#endif
uint32_t  len;
} seg[CONFIG_PACKET_MAX_SEGS];
 
@@ -101,11 +106,6 @@ struct odp_buffer_hdr_t {
queue_entry_t   *target_qe;  /* ordered queue target */
uint64_t sync[SCHEDULE_ORDERED_LOCKS_PER_QUEUE];
};
-#ifdef _ODP_PKTIO_IPC
-   /* ipc mapped process can not walk over pointers,
-* offset has to be used */
-   uint64_t ipc_addr_offset[ODP_CONFIG_PACKET_MAX_SEGS];
-#endif
 
/* Data or next header */
uint8_t data[0];
diff --git a/platform/linux-generic/include/odp_internal.h 
b/platform/linux-generic/include/odp_internal.h
index 6a80d9d..b313b1f 100644
--- a/platform/linux-generic/include/odp_internal.h
+++ b/platform/linux-generic/include/odp_internal.h
@@ -50,7 +50,6 @@ struct odp_global_data_s {
odp_cpumask_t control_cpus;
odp_cpumask_t worker_cpus;
int num_cpus_installed;
-   int ipc_ns;
 };
 
 enum init_stage {
diff --git a/platform/linux-generic/include/odp_packet_io_ipc_internal.h 
b/platform/linux-generic/include/odp_packet_io_ipc_internal.h
index 851114d..7cd2948 100644
--- a/platform/linux-generic/include/odp_packet_io_ipc_internal.h
+++ b/platform/linux-generic/include/odp_packet_io_ipc_internal.h
@@ -26,22 +26,31 @@
  */
 struct pktio_info {
struct {
-   /* number of buffer in remote pool */
-   int shm_pool_bufs_num;
-   /* size of remote pool */
-   size_t shm_pkt_pool_size;
+   /* number of buffer*/
+   int num;
/* size of packet/segment in remote pool */
-   uint32_t shm_pkt_size;
+   uint32_t block_size;
/* offset from shared memory block start
-* to pool_mdata_addr (odp-linux pool specific) */
-   size_t mdata_offset;
+* to pool *base_addr in remote process.
+* (odp-linux pool specific) */
+   size_t base_addr_offset;
char pool_name[ODP_POOL_NAME_LEN];
+   /* 1 if master finished creation of all shared objects */
+   int init_done;
} master;
struct {
/* offset from shared memory block start
-* to pool_mdata_addr in remote process.
+* to pool *base_addr in remote process.
 * (odp-linux pool specific) */
-   size_t mdata_offset;
+   size_t base_addr_offset;
+   void *base_addr;
+   uint32_t block_size;
char pool_name[ODP_POOL_NAME_LEN];
+   /* pid of the slave process written to shm and
+* used by master to look up memory created by
+* slave
+*/
+   int pid;
+   int init_done;
} slave;
 } ODP_PACKED;
diff --git a/platform/linux-generic/odp_init.c 
b/platform/linux-generic/odp_init.c
index d40a83c..1b0d8f8 100644
--- a/platform/linux-generic/odp_init.c
+++ b/platform/linux-generic/odp_init.c
@@ -67,7 +67,7 @@ static int cleanup_files(const char *dirpath, int odp_pid)
 
 int odp_init_global(odp_instance_t *instance,
const odp_init_t *params,
-   const odp_platform_init_t *platform_params)
+   const odp_platform_init_t *platform_params ODP_UNUSED)
 {
char *hpdir;
 
@@ -75,9 +75,6 @@ int odp_init_global(odp_instance_t *instance,
odp_global_data.main_pid = getpid();
cleanup_files(_ODP_TMPDIR, odp_global_data.main_pid);
 
-   if (platform_params)
-   odp_global_data.ipc_ns = platform_params->ipc_ns;
-
enum init_stage stage = NO_INIT;
odp_global_data.log_fn = odp_override_log;
odp_global_data.abort_fn = odp_override_abort;
diff --git a/platform/linux-generic/pktio/ipc.c 
b/platform/linux-generic/pktio/ipc.c
index 0e99c6e..5e5e3b2 100644
--- a/platform/linux-generic/pktio/ipc.c
+++ 

[lng-odp] [API-NEXT PATCH 3/4] linux-gen: pktio ipc: update tests

[Maxim Uvarov]

Signed-off-by: Maxim Uvarov 
---
 test/linux-generic/pktio_ipc/ipc_common.c | 41 ++
 test/linux-generic/pktio_ipc/ipc_common.h | 15 +--
 test/linux-generic/pktio_ipc/pktio_ipc1.c | 60 --
 test/linux-generic/pktio_ipc/pktio_ipc2.c | 62 ---
 test/linux-generic/pktio_ipc/pktio_ipc_run.sh | 25 ---
 5 files changed, 127 insertions(+), 76 deletions(-)

diff --git a/test/linux-generic/pktio_ipc/ipc_common.c 
b/test/linux-generic/pktio_ipc/ipc_common.c
index 387c921..85cbc8b 100644
--- a/test/linux-generic/pktio_ipc/ipc_common.c
+++ b/test/linux-generic/pktio_ipc/ipc_common.c
@@ -8,7 +8,8 @@
 
 /** Run time in seconds */
 int run_time_sec;
-int ipc_name_space;
+/** Pid of the master process */
+int master_pid;
 
 int ipc_odp_packet_send_or_free(odp_pktio_t pktio,
odp_packet_t pkt_tbl[], int num)
@@ -33,6 +34,7 @@ int ipc_odp_packet_send_or_free(odp_pktio_t pktio,
while (sent != num) {
ret = odp_pktout_send(pktout, _tbl[sent], num - sent);
if (ret < 0) {
+   EXAMPLE_ERR("odp_pktout_send return %d\n", ret);
for (i = sent; i < num; i++)
odp_packet_free(pkt_tbl[i]);
return -1;
@@ -43,6 +45,7 @@ int ipc_odp_packet_send_or_free(odp_pktio_t pktio,
if (odp_time_cmp(end_time, odp_time_local()) < 0) {
for (i = sent; i < num; i++)
odp_packet_free(pkt_tbl[i]);
+   EXAMPLE_ERR("Send Timeout!\n");
return -1;
}
}
@@ -50,17 +53,25 @@ int ipc_odp_packet_send_or_free(odp_pktio_t pktio,
return 0;
 }
 
-odp_pktio_t create_pktio(odp_pool_t pool)
+odp_pktio_t create_pktio(odp_pool_t pool, int master_pid)
 {
odp_pktio_param_t pktio_param;
odp_pktio_t ipc_pktio;
+   char name[30];
 
odp_pktio_param_init(_param);
 
-   printf("pid: %d, create IPC pktio\n", getpid());
-   ipc_pktio = odp_pktio_open("ipc_pktio", pool, _param);
-   if (ipc_pktio == ODP_PKTIO_INVALID)
-   EXAMPLE_ABORT("Error: ipc pktio create failed.\n");
+   if (master_pid)
+   sprintf(name, TEST_IPC_PKTIO_PID_NAME, master_pid);
+   else
+   sprintf(name, TEST_IPC_PKTIO_NAME);
+
+   printf("pid: %d, create IPC pktio %s\n", getpid(), name);
+   ipc_pktio = odp_pktio_open(name, pool, _param);
+   if (ipc_pktio == ODP_PKTIO_INVALID) {
+   EXAMPLE_ERR("Error: ipc pktio %s create failed.\n", name);
+   return ODP_PKTIO_INVALID;
+   }
 
if (odp_pktin_queue_config(ipc_pktio, NULL)) {
EXAMPLE_ERR("Input queue config failed\n");
@@ -88,16 +99,16 @@ void parse_args(int argc, char *argv[])
int long_index;
static struct option longopts[] = {
{"time", required_argument, NULL, 't'},
-   {"ns", required_argument, NULL, 'n'}, /* ipc name space */
+   {"pid", required_argument, NULL, 'p'}, /* master process pid */
{"help", no_argument, NULL, 'h'}, /* return 'h' */
{NULL, 0, NULL, 0}
};
 
run_time_sec = 0; /* loop forever if time to run is 0 */
-   ipc_name_space = 0;
+   master_pid = 0;
 
while (1) {
-   opt = getopt_long(argc, argv, "+t:n:h",
+   opt = getopt_long(argc, argv, "+t:p:h",
  longopts, _index);
 
if (opt == -1)
@@ -107,24 +118,18 @@ void parse_args(int argc, char *argv[])
case 't':
run_time_sec = atoi(optarg);
break;
-   case 'n':
-   ipc_name_space = atoi(optarg);
+   case 'p':
+   master_pid = atoi(optarg);
break;
case 'h':
+   default:
usage(argv[0]);
exit(EXIT_SUCCESS);
break;
-   default:
-   break;
}
}
 
optind = 1; /* reset 'extern optind' from the getopt lib */
-
-   if (!ipc_name_space) {
-   usage(argv[0]);
-   exit(1);
-   }
 }
 
 /**
diff --git a/test/linux-generic/pktio_ipc/ipc_common.h 
b/test/linux-generic/pktio_ipc/ipc_common.h
index 99276b5..8804994 100644
--- a/test/linux-generic/pktio_ipc/ipc_common.h
+++ b/test/linux-generic/pktio_ipc/ipc_common.h
@@ -30,7 +30,7 @@
 /** @def SHM_PKT_POOL_BUF_SIZE
  * @brief Buffer size of the packet pool buffer
  */
-#define SHM_PKT_POOL_BUF_SIZE  1856
+#define SHM_PKT_POOL_BUF_SIZE  100
 
 /** @def MAX_PKT_BURST
  * @brief Maximum number of packet bursts
@@ -46,6 +46,12 @@
 
 #define TEST_ALLOC_MAGIC 

[lng-odp] [API-NEXT PATCH 2/4] linux-gen: pktio ipc: more accurate settings of pool flags

[Maxim Uvarov]

Clean up code for exporting packet pools.

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

diff --git a/platform/linux-generic/odp_pool.c 
b/platform/linux-generic/odp_pool.c
index 4be3827..cae2759 100644
--- a/platform/linux-generic/odp_pool.c
+++ b/platform/linux-generic/odp_pool.c
@@ -497,14 +497,17 @@ static int check_params(odp_pool_param_t *params)
 
 odp_pool_t odp_pool_create(const char *name, odp_pool_param_t *params)
 {
+   uint32_t shm_flags = 0;
+
+   if (check_params(params))
+   return ODP_POOL_INVALID;
+
 #ifdef _ODP_PKTIO_IPC
if (params && (params->type == ODP_POOL_PACKET))
-   return pool_create(name, params, ODP_SHM_PROC);
+   shm_flags = ODP_SHM_PROC;
 #endif
-   if (check_params(params))
-   return ODP_POOL_INVALID;
 
-   return pool_create(name, params, 0);
+   return pool_create(name, params, shm_flags);
 }
 
 int odp_pool_destroy(odp_pool_t pool_hdl)
-- 
2.7.1.250.gff4ea60

[lng-odp] [API-NEXT PATCH 1/4] linux-gen: pktio ipc: ring changes

[Maxim Uvarov]

Make rings visible by other processes.

Signed-off-by: Maxim Uvarov 
---
 platform/linux-generic/pktio/ring.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/platform/linux-generic/pktio/ring.c 
b/platform/linux-generic/pktio/ring.c
index cc84e8a..aeda04b 100644
--- a/platform/linux-generic/pktio/ring.c
+++ b/platform/linux-generic/pktio/ring.c
@@ -160,7 +160,7 @@ _ring_create(const char *name, unsigned count, unsigned 
flags)
odp_shm_t shm;
 
if (flags & _RING_SHM_PROC)
-   shm_flag = ODP_SHM_PROC;
+   shm_flag = ODP_SHM_PROC | ODP_SHM_EXPORT;
else
shm_flag = 0;
 
-- 
2.7.1.250.gff4ea60

[lng-odp] [API-NEXT PATCH 0/4] linux-gen: pktio ipc: make it work again

[Maxim Uvarov]

Make ipc pktio functional after Petris and Chistophe changes.

Maxim Uvarov (4):
  linux-gen: pktio ipc: ring changes
  linux-gen: pktio ipc: more accurate settings of pool flags
  linux-gen: pktio ipc: update tests
  linux-gen: pktio ipc: make it work again

 .../linux-generic/include/odp_buffer_internal.h|  10 +-
 platform/linux-generic/include/odp_internal.h  |   1 -
 .../include/odp_packet_io_ipc_internal.h   |  27 +-
 platform/linux-generic/odp_init.c  |   5 +-
 platform/linux-generic/odp_pool.c  |  11 +-
 platform/linux-generic/pktio/ipc.c | 488 +
 platform/linux-generic/pktio/ring.c|   2 +-
 test/linux-generic/pktio_ipc/ipc_common.c  |  41 +-
 test/linux-generic/pktio_ipc/ipc_common.h  |  15 +-
 test/linux-generic/pktio_ipc/pktio_ipc1.c  |  60 ++-
 test/linux-generic/pktio_ipc/pktio_ipc2.c  |  62 ++-
 test/linux-generic/pktio_ipc/pktio_ipc_run.sh  |  25 +-
 12 files changed, 371 insertions(+), 376 deletions(-)

-- 
2.7.1.250.gff4ea60

Re: [lng-odp] [API-NEXT PATCHv2 0/4] Add iquery scheduler

[Maxim Uvarov]

Hello Yi,

can you please add some documentation text about that scheduler. Might
be something very close to that commit description. It will be good do
have some doc to point people who ask what is propose of different
scheduler.

Thank you,
Maxim.

On 12/09/16 13:51, Yi He wrote:
> v2: rebase to Matias' ordered queue impl, fix clang compilation.
> 
> This patchset provides an alternate scheduler which can be enabled
> with --enable-schedule-iquery configuration option.
> 
> Run l2fwd and odp_scheduling performance test programs show equivalent
> performance with the improved default scheduler, and can benifit use
> cases of fewer queue count such as microservices, etc.
> 
> Supported parallel, atomic and ordered queue, with pktio polling.
> ordered queue re-uses the algorithm in default scheduler.
> pktio polling re-uses the same algorithm in default scheduler.
> 
> l2fwd performance: 2 x 10GE, 64B frame, Bi-directional
> -- DPDK pktgen --
> -- ODP-linux l2fwd (DPDK pktio) --
> 2 x E5-2650 v3 @ 2.30GHz 40 cores
> 
>  "DIRECT (REFERENCE)"  PARALLEL ATOMIC
> core default (improved, Petri) default  iquery  default  iquery
> 120%   16%  16% 16%  16%
> 238%   27%  28% 26%  24%
> 437%   16%  16% 5%   5%
> 
> * Multi-core scaling downgrade may be related to software DPDK pktgen receive 
> issue
> 
> Yi He (4):
>   linux-gen: sched: solve ordered context inversion
>   linux-gen: sched: add unsched_queue callback
>   linux-gen: add generic bitmaps and iterators
>   linux-gen: add interests query (iquery) scheduler
> 
>  platform/linux-generic/Makefile.am |3 +
>  .../linux-generic/include/odp_bitmap_internal.h|  320 
>  platform/linux-generic/include/odp_schedule_if.h   |5 +
>  platform/linux-generic/m4/odp_schedule.m4  |7 +
>  platform/linux-generic/odp_bitmap.c|  315 
>  platform/linux-generic/odp_queue.c |7 +-
>  platform/linux-generic/odp_schedule.c  |   13 +-
>  platform/linux-generic/odp_schedule_if.c   |6 +
>  platform/linux-generic/odp_schedule_iquery.c   | 1523 
> 
>  platform/linux-generic/odp_schedule_sp.c   |   13 +-
>  10 files changed, 2209 insertions(+), 3 deletions(-)
>  create mode 100644 platform/linux-generic/include/odp_bitmap_internal.h
>  create mode 100644 platform/linux-generic/odp_bitmap.c
>  create mode 100644 platform/linux-generic/odp_schedule_iquery.c
> 
> --
> 2.7.4
> 

Re: [lng-odp] [API-NEXT PATCH v3] api: ipsec: added IPSEC API

[Maxim Uvarov]

Merged,
Maxim.

On 12/09/16 12:00, Bala Manoharan wrote:
> Reviewed-by: Balasubramanian Manoharan 
> 
> 
> On 1 December 2016 at 14:47, Petri Savolainen
>  wrote:
>> Added definitions for a look-a-side IPSEC offload API. In addition to
>> IPSEC packet transformations, it also supports:
>> * inbound SA look up
>> * outbound IP fragmentation
>>
>> Signed-off-by: Petri Savolainen 
>> ---
>>
>> Changes in v3:
>> * Reword packet ordering specification
>>
>> Changes in v2:
>> * Specify that synchronous calls cannot process all packets
>>   if output.num_pkt < input.num_pkt
>> * Specify that resulting event must be freed before calling using packets
>> * Added soft/hard sec limit capability
>> * Improved packet order specification
>>
>> Changes in v1:
>> * renamed odp_ipsec_proto_t to renamed odp_ipsec_protocol_t
>> * specify that lifetime sec limit is from the SA creation
>> * added odp_ipsec_sa_context()
>> * pool for output packets is the same as packet input pool
>> * added antireplay check and protocol error codes
>> * specified which input / output packet offsets and flags are set
>> * moved sync/async mode selection to global config (odp_ipsec_config())
>> * added IPSEC capability to aid mode selection
>> * specify that also packet user area is copied from input to output packet
>>
>>
>>  include/odp/api/spec/event.h   |   2 +-
>>  include/odp/api/spec/ipsec.h   | 883 
>> +
>>  include/odp_api.h  |   1 +
>>  platform/Makefile.inc  |   1 +
>>  platform/linux-generic/Makefile.am |   2 +
>>  platform/linux-generic/include/odp/api/ipsec.h |  36 +
>>  .../include/odp/api/plat/event_types.h |   1 +
>>  .../include/odp/api/plat/ipsec_types.h |  39 +
>>  8 files changed, 964 insertions(+), 1 deletion(-)
>>  create mode 100644 include/odp/api/spec/ipsec.h
>>  create mode 100644 platform/linux-generic/include/odp/api/ipsec.h
>>  create mode 100644 platform/linux-generic/include/odp/api/plat/ipsec_types.h
>>
>> diff --git a/include/odp/api/spec/event.h b/include/odp/api/spec/event.h
>> index fdfa52d..75c0bbc 100644
>> --- a/include/odp/api/spec/event.h
>> +++ b/include/odp/api/spec/event.h
>> @@ -39,7 +39,7 @@ extern "C" {
>>   * @typedef odp_event_type_t
>>   * ODP event types:
>>   * ODP_EVENT_BUFFER, ODP_EVENT_PACKET, ODP_EVENT_TIMEOUT,
>> - * ODP_EVENT_CRYPTO_COMPL
>> + * ODP_EVENT_CRYPTO_COMPL, ODP_EVENT_IPSEC_RESULT
>>   */
>>
>>  /**
>> diff --git a/include/odp/api/spec/ipsec.h b/include/odp/api/spec/ipsec.h
>> new file mode 100644
>> index 000..86f66e6
>> --- /dev/null
>> +++ b/include/odp/api/spec/ipsec.h
>> @@ -0,0 +1,883 @@
>> +/* Copyright (c) 2016, Linaro Limited
>> + * All rights reserved.
>> + *
>> + * SPDX-License-Identifier: BSD-3-Clause
>> + */
>> +
>> +/**
>> + * @file
>> + *
>> + * ODP IPSEC API
>> + */
>> +
>> +#ifndef ODP_API_IPSEC_H_
>> +#define ODP_API_IPSEC_H_
>> +#include 
>> +
>> +#ifdef __cplusplus
>> +extern "C" {
>> +#endif
>> +
>> +#include 
>> +
>> +/** @defgroup odp_ipsec ODP IPSEC
>> + *  Operations of IPSEC API.
>> + *  @{
>> + */
>> +
>> +/**
>> + * @typedef odp_ipsec_sa_t
>> + * IPSEC Security Association (SA)
>> + */
>> +
>> + /**
>> + * @def ODP_IPSEC_SA_INVALID
>> + * Invalid IPSEC SA
>> + */
>> +
>> +/**
>> + * IPSEC operation mode
>> + */
>> +typedef enum odp_ipsec_op_mode_t {
>> +   /** Synchronous IPSEC operation
>> + *
>> + * Application uses synchronous IPSEC operations,
>> + * which output all results on function return.
>> + */
>> +   ODP_IPSEC_OP_MODE_SYNC = 0,
>> +
>> +   /** Asynchronous IPSEC operation
>> + *
>> + * Application uses asynchronous IPSEC operations,
>> + * which return results via events.
>> + */
>> +   ODP_IPSEC_OP_MODE_ASYNC
>> +
>> +} odp_ipsec_op_mode_t;
>> +
>> +/**
>> + * IPSEC capability
>> + */
>> +typedef struct odp_ipsec_capability_t {
>> +   /** Maximum number of IPSEC SAs */
>> +   uint32_t max_num_sa;
>> +
>> +   /** Synchronous IPSEC operation mode (ODP_IPSEC_OP_MODE_SYNC) support
>> +*
>> +*  0: Synchronous mode is not supported
>> +*  1: Synchronous mode is supported
>> +*  2: Synchronous mode is supported and preferred
>> +*/
>> +   uint8_t op_mode_sync;
>> +
>> +   /** Asynchronous IPSEC operation mode (ODP_IPSEC_OP_MODE_ASYNC) 
>> support
>> +*
>> +*  0: Asynchronous mode is not supported
>> +*  1: Asynchronous mode is supported
>> +*  2: Asynchronous mode is supported and preferred
>> +*/
>> +   uint8_t op_mode_async;
>> +
>> +   /** Soft expiry limit in seconds support
>> +*
>> +*  0: Limit is not supported
>> +*  1: Limit is supported
>> +*/
>> +   uint8_t 

Re: [lng-odp] contributing to the "example" directory

[Mike Holmes]

One further thought is that if it in some way benchmarks anything it is
perhaps a candidate for the test performance directory instead.

On 9 December 2016 at 09:47, Bill Fischofer 
wrote:

> This sounds like an excellent example. The goal of ODP examples is to
> illustrate programming techniques and API usage that may be of
> interest to others in either evaluating ODP and/or writing their own
> applications with it. As such, examples are expected to be reasonably
> succinct and focused, but there is no specific size or complexity
> requirement.
>
> On Fri, Dec 9, 2016 at 8:12 AM, Joe Savage  wrote:
> > Hey,
> >
> > I've been working on an implementation of IPv4 fragmentation and
> > reassembly using ODP, and was wondering whether it might be useful to
> > contribute this to the "example" directory.
> >
> > What exactly are you looking for in ODP examples, and how might I go
> > about preparing this project (or, indeed, some smaller subset of this
> > project) for contribution?
> >
> > Thanks,
> >
> > Joe
> >
>



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

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

[Bill Fischofer]

On Fri, Dec 9, 2016 at 3:27 AM, Savolainen, Petri (Nokia - FI/Espoo)
 wrote:
>
>
>> +
>> +/**
>> + * Generate repeatable random data for testing purposes
>> + *
>> + * 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
>> + * 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_test_data(uint8_t *buf, uint32_t len,
>> +  odp_random_kind_t kind, uint32_t *seed);
>>
>
> This is from rand_r() documentation:
> " The value pointed to by the seedp argument of rand_r() provides only
> a very small amount of state, so this function will be a weak pseudo-
> random generator.  Try drand48_r(3) instead."

>From the drand48_r documentation [1]:

"These functions are GNU extensions and are not portable".

So I don't think we should be using these. The whole point of
odp_random_test_data() was to provide a simple repeatable random
function for ODP testing, so I don't think we need to go beyond what
the portable rand_r() provides.

[1] http://man7.org/linux/man-pages/man3/drand48_r.3.html

>
> API should not limit the state size to be so small that e.g. 
> ODP_RANDOM_CRYPTO quality implementation is not possible.
>
> So, I think we are back in my previous proposal of implementation specific 
> state size.
>
> /** Random number generator state data */
> typedef struct odp_random_state_t {
>   // Implementation defined structure size
> uint32_t random_state[32];
> } odp_random_state_t;
>
> /**
>  * Initialize random state data with a seed value
>  *
>  * Uses some bits of seed to initialize the state data
>  */
> void odp_random_state_init(odp_random_state_t *state, uint64_t seed);
>
> /**
>  * Generate repeatable random data for testing purposes
>  *
>  * 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 state data is
>  * updated for each call to continue the sequence. A series of calls with the
>  * same initial seeded state data will generate the same sequence of random
>  * test data. Use odp_random_state_init() to initialize or restart the state.
>  * The function is thread safe as long as multiple threads do not use the same
>  * state simultaneously.
>  *
>  * 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, odp_random_state_t *state);
>
>
> For example, these functions could be used with this spec:
>
> int random_r(struct random_data *buf, int32_t *result);
> int srandom_r(unsigned int seed, struct random_data *buf);
> int initstate_r(unsigned int seed, char *statebuf,
>size_t statelen, struct random_data *buf);
> int setstate_r(char *statebuf, struct random_data *buf);
>
>
> Although, those seem to be nonstandard glibc extensions, so might not what to 
> use those in odp-linux, if strict posix API implementation is preferred.
>
>
> -Petri
>
>

Re: [lng-odp] contributing to the "example" directory

[Bill Fischofer]

This sounds like an excellent example. The goal of ODP examples is to
illustrate programming techniques and API usage that may be of
interest to others in either evaluating ODP and/or writing their own
applications with it. As such, examples are expected to be reasonably
succinct and focused, but there is no specific size or complexity
requirement.

On Fri, Dec 9, 2016 at 8:12 AM, Joe Savage  wrote:
> Hey,
>
> I've been working on an implementation of IPv4 fragmentation and
> reassembly using ODP, and was wondering whether it might be useful to
> contribute this to the "example" directory.
>
> What exactly are you looking for in ODP examples, and how might I go
> about preparing this project (or, indeed, some smaller subset of this
> project) for contribution?
>
> Thanks,
>
> Joe
>

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

[Nicolas Morey-Chaisemartin]

Le 12/09/2016 à 02:41 PM, Mike Holmes a écrit :
>
>
> On 9 December 2016 at 08:33, Nicolas Morey-Chaisemartin  > wrote:
>
>
>
> Le 12/09/2016 à 02:18 PM, Mike Holmes a écrit :
>>
>>
>> On 9 December 2016 at 04:41, Nicolas Morey-Chaisemartin 
>> > wrote:
>>
>> I'm OK with this. The only issue I have with this is that it moves 
>> back platform/OS selection back to configure.ac  while 
>> I've been trying to move it out 
>> (https://github.com/nmorey/odp/tree/dev/generic-platforms 
>> ).
>> Shouldn't each platform select the right OS ?
>> I mean linux-generic will probably always use the linux helper while 
>> the mppa implementation will use the right one for us (depending on the 
>> compilation flags).
>>
>>
>> I would actually like to make the helpers much more independent so I am 
>> in line with your thinking.
>> How about we just do exactly that and have helpers  build completely 
>> independently with its own configure.ac  ?
>
> Won't the dependence from the platform test to the helper lib be an issue?
> Splitting them up might end in a circualt dependency.
>
>
> I am hoping to find and delete any circular dependencies that exist, the 
> helper should depend on the odp api, the tests and examples should depend on 
> the helpers
>  
>
>
> Build ODP means building the tests which means building the helpers which 
> need ODP to be built (for odp_cpumask_* functions at least).
>
> I'm not sure we really need to pull them out of the ODP build system, but 
> simply keep the configure flexible enough so platforms can tweak/change the 
> settings from the platform side. 
>
> Platform could add their own options in their configure.m4 if they need 
> them, or simply select the basic helper setup and export the OS.
>
>
> Ok then I misunderstood this "Shouldn't each platform select the right OS ?" 
> - I agree it should.
>
> I added the selector "with_os" to the common configure.ac 
>  does that not allow it to be changed by platform if we 
> keep it all under one configure setup? - perhaps it can be set in a per 
> platform configure.m4 as you say, I need to fiddle with that.
Yes my bad. It works like that. configure.m4 should be able to adjust settings 
depending on the option, or overwrite the value if they know better.

>
>  
>
>
>
>
>>  
>>
>>
>> Also I think the lib should be renamed to libodphelper instead of 
>> libodphelper-linux.
>>
>>
>> agree
>>  
>>
>>
>> Any plans to get these patches in soon?
>>
>>
>> with your help asap
>>  
>>
>> Should I wait for your patch to get in master, or get them in my 
>> patch series?
>>
>>
>> Will work with you, let me make your suggested change to the lib and 
>> circle back with you on your first point.
> Glad to hear it :)
>
>
>
>
> -- 
> Mike Holmes
> Program Manager - Linaro Networking Group
> Linaro.org * **│ *Open source software for ARM SoCs
> "Work should be fun and collaborative, the rest follows"
>

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

[Mike Holmes]

On 9 December 2016 at 08:33, Nicolas Morey-Chaisemartin 
wrote:

>
>
> Le 12/09/2016 à 02:18 PM, Mike Holmes a écrit :
>
>
>
> On 9 December 2016 at 04:41, Nicolas Morey-Chaisemartin 
> wrote:
>
>> I'm OK with this. The only issue I have with this is that it moves back
>> platform/OS selection back to configure.ac while I've been trying to
>> move it out (https://github.com/nmorey/odp/tree/dev/generic-platforms).
>> Shouldn't each platform select the right OS ?
>> I mean linux-generic will probably always use the linux helper while the
>> mppa implementation will use the right one for us (depending on the
>> compilation flags).
>>
>
> I would actually like to make the helpers much more independent so I am in
> line with your thinking.
> How about we just do exactly that and have helpers  build completely
> independently with its own configure.ac ?
>
>
> Won't the dependence from the platform test to the helper lib be an issue?
> Splitting them up might end in a circualt dependency.
>

I am hoping to find and delete any circular dependencies that exist, the
helper should depend on the odp api, the tests and examples should depend
on the helpers


>
> Build ODP means building the tests which means building the helpers which
> need ODP to be built (for odp_cpumask_* functions at least).
>
> I'm not sure we really need to pull them out of the ODP build system, but
> simply keep the configure flexible enough so platforms can tweak/change the
> settings from the platform side.
>
Platform could add their own options in their configure.m4 if they need
> them, or simply select the basic helper setup and export the OS.
>

Ok then I misunderstood this "Shouldn't each platform select the right OS
?" - I agree it should.

I added the selector "with_os" to the common configure.ac does that not
allow it to be changed by platform if we keep it all under one configure
setup? - perhaps it can be set in a per platform configure.m4 as you say, I
need to fiddle with that.



>
>
>
>
>
>>
>> Also I think the lib should be renamed to libodphelper instead of
>> libodphelper-linux.
>>
>
> agree
>
>
>>
>> Any plans to get these patches in soon?
>>
>
> with your help asap
>
>
>> Should I wait for your patch to get in master, or get them in my patch
>> series?
>>
>
> Will work with you, let me make your suggested change to the lib and
> circle back with you on your first point.
>
> Glad to hear it :)
>
>


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

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

[Nicolas Morey-Chaisemartin]

Le 12/09/2016 à 02:18 PM, Mike Holmes a écrit :
>
>
> On 9 December 2016 at 04:41, Nicolas Morey-Chaisemartin  > wrote:
>
> I'm OK with this. The only issue I have with this is that it moves back 
> platform/OS selection back to configure.ac  while I've 
> been trying to move it out 
> (https://github.com/nmorey/odp/tree/dev/generic-platforms 
> ).
> Shouldn't each platform select the right OS ?
> I mean linux-generic will probably always use the linux helper while the 
> mppa implementation will use the right one for us (depending on the 
> compilation flags).
>
>
> I would actually like to make the helpers much more independent so I am in 
> line with your thinking.
> How about we just do exactly that and have helpers  build completely 
> independently with its own configure.ac  ?

Won't the dependence from the platform test to the helper lib be an issue?
Splitting them up might end in a circualt dependency.

Build ODP means building the tests which means building the helpers which need 
ODP to be built (for odp_cpumask_* functions at least).

I'm not sure we really need to pull them out of the ODP build system, but 
simply keep the configure flexible enough so platforms can tweak/change the 
settings from the platform side.
Platform could add their own options in their configure.m4 if they need them, 
or simply select the basic helper setup and export the OS.


>  
>
>
> Also I think the lib should be renamed to libodphelper instead of 
> libodphelper-linux.
>
>
> agree
>  
>
>
> Any plans to get these patches in soon?
>
>
> with your help asap
>  
>
> Should I wait for your patch to get in master, or get them in my patch 
> series?
>
>
> Will work with you, let me make your suggested change to the lib and circle 
> back with you on your first point.
Glad to hear it :)

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

[Mike Holmes]

On 9 December 2016 at 04:41, Nicolas Morey-Chaisemartin 
wrote:

> I'm OK with this. The only issue I have with this is that it moves back
> platform/OS selection back to configure.ac while I've been trying to move
> it out (https://github.com/nmorey/odp/tree/dev/generic-platforms).
> Shouldn't each platform select the right OS ?
> I mean linux-generic will probably always use the linux helper while the
> mppa implementation will use the right one for us (depending on the
> compilation flags).
>

I would actually like to make the helpers much more independent so I am in
line with your thinking.
How about we just do exactly that and have helpers  build completely
independently with its own configure.ac ?


>
> Also I think the lib should be renamed to libodphelper instead of
> libodphelper-linux.
>

agree


>
> Any plans to get these patches in soon?
>

with your help asap


> Should I wait for your patch to get in master, or get them in my patch
> series?
>

Will work with you, let me make your suggested change to the lib and circle
back with you on your first point.


>
>
> Nicolas
>
>
>
> Le 12/08/2016 à 09:47 PM, Mike Holmes a écrit :
>
> So this https://github.com/mike-holmes-linaro/odp/tree/helpers-os is what
> I was thinking
>
> It moves the Linux specifics to helper/os/linux and selects the helper os
> to be built in the configure step
> It renames the linux.h helper include to threads.h which is what it
> predominately is
> It also removes the unused Linux specific  APIs since no odp test or
> example uses them and they break having an OS agnostic helper suite.
>
> On 8 December 2016 at 13:44, Mike Holmes  wrote:
>
>>
>>
>> On 7 December 2016 at 01: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.
>>>
>>
>> I would be keen to do the same thing master, if it is OS differences then
>> that is much more in keeping with the intent.
>> I will play with a patch to move linux.c under and OS directory called
>> linux
>>
>>
>>> >
>>> >>  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
>>> >> +++ 

Re: [lng-odp] [PATCHv4] platform: linux-generic: reading cpu affinity from cpuset

[Maxim Uvarov]

Merged,
Maxim.

On 12/07/16 06:36, Yi He wrote:
> Reviewed-and-tested-by: Yi He 
> 
> 
> On 6 December 2016 at 22:15, Balakrishna Garapati <
> balakrishna.garap...@linaro.org> wrote:
> 
>> With this new proposal cpu affinity is read correctly especially
>> when using cgroups otherwise wrong cpu mask is set.
>>
>> Fixes bug: https://bugs.linaro.org/show_bug.cgi?id=2472
>>
>> Signed-off-by: Balakrishna Garapati 
>> ---
>>
>>  since v3: Renaming function call
>>
>>  platform/linux-generic/odp_cpumask.c | 73 +-
>> --
>>  1 file changed, 18 insertions(+), 55 deletions(-)
>>
>> diff --git a/platform/linux-generic/odp_cpumask.c
>> b/platform/linux-generic/odp_cpumask.c
>> index 6bf2632..64559a6 100644
>> --- a/platform/linux-generic/odp_cpumask.c
>> +++ b/platform/linux-generic/odp_cpumask.c
>> @@ -225,73 +225,36 @@ int odp_cpumask_next(const odp_cpumask_t *mask, int
>> cpu)
>>   * This function obtains system information specifying which cpus are
>>   * available at boot time.
>>   */
>> -static int get_installed_cpus(void)
>> +static int get_available_cpus(void)
>>  {
>> -   char *numptr;
>> -   char *endptr;
>> -   long int cpu_idnum;
>> -   DIR  *d;
>> -   struct dirent *dir;
>> +   int cpu_idnum;
>> +   cpu_set_t cpuset;
>> +   int ret;
>>
>> /* Clear the global cpumasks for control and worker CPUs */
>> odp_cpumask_zero(_global_data.control_cpus);
>> odp_cpumask_zero(_global_data.worker_cpus);
>>
>> -   /*
>> -* Scan the /sysfs pseudo-filesystem for CPU info directories.
>> -* There should be one subdirectory for each installed logical CPU
>> -*/
>> -   d = opendir("/sys/devices/system/cpu");
>> -   if (d) {
>> -   while ((dir = readdir(d)) != NULL) {
>> -   cpu_idnum = CPU_SETSIZE;
>> -
>> -   /*
>> -* If the current directory entry doesn't represent
>> -* a CPU info subdirectory then skip to the next
>> entry.
>> -*/
>> -   if (dir->d_type == DT_DIR) {
>> -   if (!strncmp(dir->d_name, "cpu", 3)) {
>> -   /*
>> -* Directory name starts with
>> "cpu"...
>> -* Try to extract a CPU ID number
>> -* from the remainder of the
>> dirname.
>> -*/
>> -   errno = 0;
>> -   numptr = dir->d_name;
>> -   numptr += 3;
>> -   cpu_idnum = strtol(numptr, ,
>> -  10);
>> -   if (errno || (endptr == numptr))
>> -   continue;
>> -   } else {
>> -   continue;
>> -   }
>> -   } else {
>> -   continue;
>> -   }
>> -   /*
>> -* If we get here the current directory entry
>> specifies
>> -* a CPU info subdir for the CPU indexed by
>> cpu_idnum.
>> -*/
>> +   CPU_ZERO();
>> +   ret = sched_getaffinity(0, sizeof(cpuset), );
>>
>> -   /* Track number of logical CPUs discovered */
>> -   if (odp_global_data.num_cpus_installed <
>> -   (int)(cpu_idnum + 1))
>> -   odp_global_data.num_cpus_installed =
>> -   (int)(cpu_idnum + 1);
>> +   if (ret < 0) {
>> +   ODP_ERR("Failed to get cpu affinity");
>> +   return -1;
>> +   }
>>
>> +   for (cpu_idnum = 0; cpu_idnum < CPU_SETSIZE - 1; cpu_idnum++) {
>> +   if (CPU_ISSET(cpu_idnum, )) {
>> +   odp_global_data.num_cpus_installed++;
>> /* Add the CPU to our default cpumasks */
>> odp_cpumask_set(_global_data.control_cpus,
>> -   (int)cpu_idnum);
>> +   (int)cpu_idnum);
>> odp_cpumask_set(_global_data.worker_cpus,
>> -   (int)cpu_idnum);
>> +   (int)cpu_idnum);
>> }
>> -   closedir(d);
>> -   return 0;
>> -   } else {
>> -   return -1;
>> }
>> +
>> +   return 0;
>>  }
>>
>>  /*
>> @@ -429,7 +392,7 @@ int 

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

[Maxim Uvarov]

Bala,

please review TM patch.

Thank you,
Maxim.

On 12/08/16 04:58, Forrest Shi wrote:
> 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
>>
>>

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

[Maxim Uvarov]

Merged,
Maxim.

On 12/08/16 00:05, Bill Fischofer wrote:
> 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] linux-generic: move tm system barrier to tm group

[Maxim Uvarov]

Merged,
Maxim.

On 12/08/16 05:21, Bala Manoharan wrote:
> 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] [PATCH] validation: packet: remove todos from packet tests

[Maxim Uvarov]

Merged,
Maxim.

On 12/08/16 21:24, Mike Holmes wrote:
> On 12 September 2016 at 16:48, Bill Fischofer 
> wrote:
> 
>> Resolve Bug https://bugs.linaro.org/show_bug.cgi?id=2405 by removing
>> todos in packet validation test. Add additional tests for todos that can
>> be resolved within current ODP API set and remove the rest to be deferred
>> for latest updates.
>>
>> Signed-off-by: Bill Fischofer 
>>
> 
> Reviewed-by: Mike Holmes 
> 
> 
>> ---
>>  test/common_plat/validation/api/packet/packet.c | 56
>> -
>>  1 file changed, 28 insertions(+), 28 deletions(-)
>>
>> diff --git a/test/common_plat/validation/api/packet/packet.c
>> b/test/common_plat/validation/api/packet/packet.c
>> index a4426e2..c75cde9 100644
>> --- a/test/common_plat/validation/api/packet/packet.c
>> +++ b/test/common_plat/validation/api/packet/packet.c
>> @@ -32,6 +32,28 @@ static struct udata_struct {
>> "abcdefg",
>>  };
>>
>> +static void _packet_compare_data(odp_packet_t pkt1, odp_packet_t pkt2)
>> +{
>> +   uint32_t len = odp_packet_len(pkt1);
>> +   uint32_t offset = 0;
>> +   uint32_t seglen1, seglen2, cmplen;
>> +
>> +   CU_ASSERT_FATAL(len == odp_packet_len(pkt2));
>> +
>> +   while (len > 0) {
>> +   void *pkt1map = odp_packet_offset(pkt1, offset, ,
>> NULL);
>> +   void *pkt2map = odp_packet_offset(pkt2, offset, ,
>> NULL);
>> +
>> +   CU_ASSERT_PTR_NOT_NULL_FATAL(pkt1map);
>> +   CU_ASSERT_PTR_NOT_NULL_FATAL(pkt2map);
>> +   cmplen = seglen1 < seglen2 ? seglen1 : seglen2;
>> +   CU_ASSERT(!memcmp(pkt1map, pkt2map, cmplen));
>> +
>> +   offset += cmplen;
>> +   len-= cmplen;
>> +   }
>> +}
>> +
>>  int packet_suite_init(void)
>>  {
>> odp_pool_param_t params;
>> @@ -298,7 +320,8 @@ void packet_test_event_conversion(void)
>>
>> tmp_pkt = odp_packet_from_event(ev);
>> CU_ASSERT_FATAL(tmp_pkt != ODP_PACKET_INVALID);
>> -   /** @todo: Need an API to compare packets */
>> +   CU_ASSERT(tmp_pkt == pkt);
>> +   _packet_compare_data(tmp_pkt, pkt);
>>  }
>>
>>  void packet_test_basic_metadata(void)
>> @@ -502,7 +525,7 @@ void packet_test_headroom(void)
>>
>> seg_data_len = odp_packet_seg_len(pkt);
>> CU_ASSERT(seg_data_len >= 1);
>> -   /** @todo: should be len - 1 */
>> +
>> pull_val = seg_data_len / 2;
>> push_val = room;
>>
>> @@ -625,7 +648,7 @@ void packet_test_tailroom(void)
>>
>> seg_data_len = odp_packet_seg_data_len(pkt, segment);
>> CU_ASSERT(seg_data_len >= 1);
>> -   /** @todo: should be len - 1 */
>> +
>> pull_val = seg_data_len / 2;
>> /* Leave one byte in a tailroom for odp_packet_tail() to succeed */
>> push_val = (room > 0) ? room - 1 : room;
>> @@ -681,10 +704,10 @@ void packet_test_segments(void)
>> CU_ASSERT_PTR_NOT_NULL(seg_data);
>> CU_ASSERT(odp_packet_seg_to_u64(seg) !=
>>   odp_packet_seg_to_u64(ODP_PACKET_SEG_INVALID));
>> +   CU_ASSERT(odp_memcmp(seg_data, seg_data, seg_data_len) ==
>> 0);
>>
>> data_len += seg_data_len;
>>
>> -   /** @todo: touch memory in a segment */
>> seg_index++;
>> seg = odp_packet_next_seg(pkt, seg);
>> }
>> @@ -713,10 +736,10 @@ void packet_test_segments(void)
>> CU_ASSERT(seg_data != NULL);
>> CU_ASSERT(odp_packet_seg_to_u64(seg) !=
>>   odp_packet_seg_to_u64(ODP_PACKET_SEG_INVALID));
>> +   CU_ASSERT(odp_memcmp(seg_data, seg_data, seg_data_len) ==
>> 0);
>>
>> data_len += seg_data_len;
>>
>> -   /** @todo: touch memory in a segment */
>> seg_index++;
>> seg = odp_packet_next_seg(seg_pkt, seg);
>> }
>> @@ -784,7 +807,6 @@ void packet_test_error_flags(void)
>> /**
>>  * The packet have not been classified so it doesn't have error
>> flags
>>  * properly set. Just check that functions return one of allowed
>> values.
>> -* @todo: try with known good and bad packets.
>>  */
>> err = odp_packet_has_error(pkt);
>> CU_ASSERT(err == 0 || err == 1);
>> @@ -922,28 +944,6 @@ static void _packet_compare_inflags(odp_packet_t
>> pkt1, odp_packet_t pkt2)
>> COMPARE_INFLAG(pkt1, pkt2, shaper_len_adjust);
>>  }
>>
>> -static void _packet_compare_data(odp_packet_t pkt1, odp_packet_t pkt2)
>> -{
>> -   uint32_t len = odp_packet_len(pkt1);
>> -   uint32_t offset = 0;
>> -   uint32_t seglen1, seglen2, cmplen;
>> -
>> -   CU_ASSERT_FATAL(len == odp_packet_len(pkt2));
>> -
>> -   while (len > 0) {
>> -   void *pkt1map = odp_packet_offset(pkt1, offset, ,
>> NULL);
>> -   void *pkt2map = 

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

[Maxim Uvarov]

Merged with fix.

Maxim.

On 12/08/16 18:58, Mike Holmes wrote:
> Thanks Petri
> 
> Maxim can you fix typo on commit  ?
> 
> On 8 December 2016 at 02:32, Savolainen, Petri (Nokia - FI/Espoo) <
> petri.savolai...@nokia-bell-labs.com> wrote:
> 
>>
>>
>>> -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
>>
>>
> 
> 

[lng-odp] [RFC/API-NEXT 1/1] api: classification: packet hashing per class of service

[Balasubramanian Manoharan]

Adds support to spread packet from a single CoS to multiple queues by
configuring hashing at CoS level.

Signed-off-by: Balasubramanian Manoharan 
---
 include/odp/api/spec/classification.h | 45 ---
 1 file changed, 42 insertions(+), 3 deletions(-)

diff --git a/include/odp/api/spec/classification.h 
b/include/odp/api/spec/classification.h
index 0e442c7..220b029 100644
--- a/include/odp/api/spec/classification.h
+++ b/include/odp/api/spec/classification.h
@@ -126,6 +126,12 @@ typedef struct odp_cls_capability_t {
/** Maximum number of CoS supported */
unsigned max_cos;
 
+   /** Maximun number of queue supported per CoS */
+   unsigned max_queue_supported;
+
+   /** Protocol header combination supported for Hashing */
+   odp_pktin_hash_proto_t hash_supported;
+
/** A Boolean to denote support of PMR range */
odp_bool_t pmr_range_supported;
 } odp_cls_capability_t;
@@ -164,9 +170,25 @@ typedef enum {
  * Used to communicate class of service creation options
  */
 typedef struct odp_cls_cos_param {
-   odp_queue_t queue;  /**< Queue associated with CoS */
-   odp_pool_t pool;/**< Pool associated with CoS */
-   odp_cls_drop_t drop_policy; /**< Drop policy associated with CoS */
+   /* Minimum number of queues to be linked to this CoS.
+* If the number is greater than 1 then hashing has to be
+* enabled */
+   uint32_t num_queue;
+   /* Denotes whether hashing is enabled for queues in this CoS
+* When hashing is enabled the queues are created by the implementation
+* and application need not configure any queue to the class of service
+*/
+   odp_bool_t enable_hashing;
+   /* Protocol header fields which are included in packet input
+* hash calculation */
+   odp_pktin_hash_proto_t hash;
+   /* If hashing is disabled, then application has to configure
+* this queue and packets are delivered to this queue */
+   odp_queue_t queue;
+   /* Pool associated with CoS */
+   odp_pool_t pool;
+   /* Drop policy associated with CoS */
+   odp_cls_drop_t drop_policy;
 } odp_cls_cos_param_t;
 
 /**
@@ -209,6 +231,23 @@ int odp_cls_capability(odp_cls_capability_t *capability);
 odp_cos_t odp_cls_cos_create(const char *name, odp_cls_cos_param_t *param);
 
 /**
+ * Queue hash result
+ * Returns the queue within a CoS in which a particular packet will be enqueued
+ * based on the packet parameters and hash protocol field configured with the
+ * class of service.
+ *
+ * @param  cos class of service
+ * @param  packet  Packet handle
+ *
+ * @retval Returns the queue handle on which this packet will be
+ * enqueued.
+ * @retval ODP_QUEUE_INVALID for error case
+ *
+ * @note The packet has to be updated with valid header pointers L2, L3 and L4.
+ */
+odp_queue_t odp_queue_hash_result(odp_cos_t cos, odp_packet_t packet);
+
+/**
  * Discard a class-of-service along with all its associated resources
  *
  * @param[in]  cos_id  class-of-service instance.
-- 
1.9.1

Re: [lng-odp] [API-NEXT PATCH] linux-gen: sched: fix SP scheduler hang in process mode

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

> -Original Message-
> From: Yi He [mailto:yi...@linaro.org]
> Sent: Friday, December 09, 2016 5:35 AM
> To: Savolainen, Petri (Nokia - FI/Espoo)  labs.com>; Elo, Matias (Nokia - FI/Espoo)  labs.com>; lng-odp@lists.linaro.org
> Cc: Yi He 
> Subject: [lng-odp] [API-NEXT PATCH] linux-gen: sched: fix SP scheduler
> hang in process mode
> 
> SP scheduler hangs in process mode performance test
> due to global data structure were not created in shared
> memory region.
> 
> Signed-off-by: Yi He 

This will conflict with my "linux-gen: schedule_sp: use ring as priority queue" 
patch sent yesterday. Could you rebase and send again after my patch is merged.

-Petri

Re: [lng-odp] odp_rwlock_read_trylock

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

> -Original Message-
> From: Nicolas Morey-Chaisemartin [mailto:nmo...@kalray.eu]
> Sent: Friday, December 09, 2016 12:14 PM
> To: Savolainen, Petri (Nokia - FI/Espoo)  labs.com>; LNG ODP Mailman List 
> Subject: Re: [lng-odp] odp_rwlock_read_trylock
> 
> 
> 
> Le 12/09/2016 à 10:45 AM, Savolainen, Petri (Nokia - FI/Espoo) a écrit :
> >
> >> -Original Message-
> >> From: Nicolas Morey-Chaisemartin [mailto:nmo...@kalray.eu]
> >> Sent: Thursday, December 08, 2016 5:30 PM
> >> To: Savolainen, Petri (Nokia - FI/Espoo)  >> labs.com>; LNG ODP Mailman List 
> >> Subject: Re: [lng-odp] odp_rwlock_read_trylock
> >>
> >>
> >>
> >> Le 12/08/2016 à 03:50 PM, Savolainen, Petri (Nokia - FI/Espoo) a écrit
> :
>  -Original Message-
>  From: lng-odp [mailto:lng-odp-boun...@lists.linaro.org] On Behalf Of
>  Nicolas Morey-Chaisemartin
>  Sent: Wednesday, December 07, 2016 10:12 AM
>  To: LNG ODP Mailman List 
>  Subject: [lng-odp] odp_rwlock_read_trylock
> 
>  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.
> >>> /**
> >>>  * Try to acquire read permission to a reader/writer lock.
> >>>  *
> >>>  * @param rwlock Pointer to a reader/writer lock
> >>>  *
> >>>  * @retval  0 Lock was not available for read access
> >>>  * @retval !0 Read access to lock acquired
> >>>  */
> >>> int odp_rwlock_read_trylock(odp_rwlock_t *rwlock);
> >>>
> >>>
> >>> The spec just says that "read access" was not available.
> Implementation
> >> decides when it's available and application just needs to re-try later
> on.
> >> So, this implementation is OK by the spec. Supporting multiple readers
> >> would improve parallelism, but it may not be optimal for performance if
> >> the lock is free in the common case.
> >>>
>  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)
> >>> You cannot do this on a single thread (lock the same lock twice). You
> >> must use recursive locks if the thread holding a lock will lock it
> again.
> >> The issue is odp_rwlock_read_trylock fails here, but
> odp_rwlock_read_lock
> >> would succeed.
> >> Usually rwlock are recursive on the read side as it doesn't really
> matter
> >> to the lock if the same thread owns it N times, or N threads own it
> once
> >> each.
> >> IMHO, in a non-concurrent setup, read_trylock should always succeed if
> >> read_lock would succeed.
> >> (I say non concurrent here to not acount for the case of contention on
> the
> >> lock where many retries could be needed to get the lock).
> >
> > We have recursive version of the lock (odp_rwlock_recursive_read_lock(),
> etc) and application should use that if the same thread will call lock()
> twice (or lock + try_lock).
> 
> Yes, but the test is not doing that.
> I think we have to distinguished the proper usage and what works.
> 
> The proper usage if you want to do lock + lock or lock + trylock is to use
> a recursive rw lock.
> But if you do that on a non recursive one, what is supposed to happen?
> Right now lock + lock works and lock + trylock fails which doesn't feel
> natural.

It's undefined what happens if application tries to lock the same lock twice. 
It's a bug in the test.


> 
> 
> >
> >> What bothers me is this:
> >>
> >> A thread that wants write access will have to wait until
> >>  * there are no threads that want read access.
> >>
> >> This clearly states that a writer cannot get the write lock if a reader
> >> *wants* it.
> >> I think
> >>
> >> A thread that wants write access will have to wait until
> >>  * there are no threads that own read permission on the lock.
> >>
> >>
> >> would relax this constraint and allow for write-priority rwlock.
> >
> > That's OK wording.
> >
> > -Petri
> >
> >
> I'll send a patch for that. Should it go in all branches (including
> monarch) ?
> 
> Nicolas

All API changes go through api-next. So, send both (API and validation test 
fix) to api-next and write a bug against monarch if you 

[lng-odp] [API-NEXT PATCHv2 3/4] linux-gen: add generic bitmaps and iterators

[Yi He]

Add C++ template alike bitmap to allow
instantiate bitmap data structure of any size,
and provide iterators to help walking through
the bitmap objects.

Signed-off-by: Yi He 
---
 platform/linux-generic/Makefile.am |   2 +
 .../linux-generic/include/odp_bitmap_internal.h| 320 +
 platform/linux-generic/odp_bitmap.c| 315 
 3 files changed, 637 insertions(+)
 create mode 100644 platform/linux-generic/include/odp_bitmap_internal.h
 create mode 100644 platform/linux-generic/odp_bitmap.c

diff --git a/platform/linux-generic/Makefile.am 
b/platform/linux-generic/Makefile.am
index adbe24d..a6e62ef 100644
--- a/platform/linux-generic/Makefile.am
+++ b/platform/linux-generic/Makefile.am
@@ -128,6 +128,7 @@ noinst_HEADERS = \
  ${srcdir}/include/odp_align_internal.h \
  ${srcdir}/include/odp_atomic_internal.h \
  ${srcdir}/include/odp_buffer_inlines.h \
+ ${srcdir}/include/odp_bitmap_internal.h \
  ${srcdir}/include/odp_buffer_internal.h \
  ${srcdir}/include/odp_classification_datamodel.h \
  ${srcdir}/include/odp_classification_inlines.h \
@@ -171,6 +172,7 @@ __LIB__libodp_linux_la_SOURCES = \
   _ishmphy.c \
   odp_atomic.c \
   odp_barrier.c \
+  odp_bitmap.c \
   odp_buffer.c \
   odp_byteorder.c \
   odp_classification.c \
diff --git a/platform/linux-generic/include/odp_bitmap_internal.h 
b/platform/linux-generic/include/odp_bitmap_internal.h
new file mode 100644
index 000..192c6f9
--- /dev/null
+++ b/platform/linux-generic/include/odp_bitmap_internal.h
@@ -0,0 +1,320 @@
+/* Copyright (c) 2016, Linaro Limited
+ * All rights reserved.
+ *
+ * SPDX-License-Identifier: BSD-3-Clause
+ */
+
+/**
+ * @file
+ *
+ * ODP generic bitmap types and operations.
+ */
+
+#ifndef ODP_BITMAP_INTERNAL_H_
+#define ODP_BITMAP_INTERNAL_H_
+
+#ifdef __cplusplus
+extern "C" {
+#endif
+
+#include 
+#include 
+#include 
+#include 
+
+/* Generate unique identifier for instantiated class */
+#define TOKENIZE(template, line) \
+   template ## _ ## line ## _ ## __COUNTER__
+
+/* Array size in general */
+#define ARRAY_SIZE(array) (sizeof(array) / sizeof(array[0]))
+
+#define BITS_PER_BYTE  (8)
+#define BITS_PER_LONG  __WORDSIZE
+#define BYTES_PER_LONG (BITS_PER_LONG / BITS_PER_BYTE)
+
+#define BIT_WORD(nr)   ((nr) / BITS_PER_LONG)
+#define BITS_TO_LONGS(nr) BIT_WORD(nr + BITS_PER_LONG - 1)
+
+#define BITMAP_FIRST_WORD_MASK(start) \
+   (~0UL << ((start) & (BITS_PER_LONG - 1)))
+#define BITMAP_LAST_WORD_MASK(nbits)  \
+   (~0UL >> (-(nbits) & (BITS_PER_LONG - 1)))
+
+/* WAPL bitmap base class */
+typedef struct {
+   unsigned int  nwords;
+   unsigned int  *pl;
+   unsigned long *ul;
+} wapl_bitmap_t;
+
+/*
+ * Word-Aligned Position List (WAPL) bitmap, which actually
+ * is not a compression, but with an extra list of non-empty
+ * word positions.
+ *
+ * WAPL accelerates bitwise operations and iterations by
+ * applying only to non-empty positions instead of walking
+ * through the whole bitmap.
+ *
+ * WAPL uses [1 ~ N] instead of [0 ~ N - 1] as position
+ * values and an extra 0 as end indicator for position list.
+ * This is the reason to allocate one extra room below.
+ */
+#define instantiate_wapl_bitmap(line, nbits)   \
+   struct TOKENIZE(wapl_bitmap, line) {\
+   unsigned int pl[BITS_TO_LONGS(nbits) + 1];  \
+   unsigned long ul[BITS_TO_LONGS(nbits) + 1]; \
+   }
+
+#define WAPL_BITMAP(nbits) instantiate_wapl_bitmap(__LINE__, nbits)
+
+/*
+ * Upcast any derived WAPL bitmap class to its base class
+ */
+#define __wapl_upcast(base, derived)   \
+   do {\
+   __typeof__(derived) p = derived;\
+   base.pl = p->pl;\
+   base.ul = p->ul;\
+   base.nwords = ARRAY_SIZE(p->ul) - 1;\
+   } while (0)
+
+/*
+ * WAPL base class bitmap operations
+ */
+void __wapl_bitmap_and(wapl_bitmap_t *dst,
+  wapl_bitmap_t *src, wapl_bitmap_t *and);
+
+void __wapl_bitmap_or(wapl_bitmap_t *dst, wapl_bitmap_t *or);
+
+void __wapl_bitmap_set(wapl_bitmap_t *map, unsigned int bit);
+
+void __wapl_bitmap_clear(wapl_bitmap_t *map, unsigned int bit);
+
+/*
+ * Generic WAPL bitmap operations
+ */
+#define wapl_bitmap_zero(map)  \
+   ({  \
+   __typeof__(map) p = map;\
+   memset((void *)p, 0, sizeof(__typeof__(*p)));   

[lng-odp] [API-NEXT PATCHv2 4/4] linux-gen: add interests query (iquery) scheduler

[Yi He]

Add this interests query (iquery) scheduler as an
alternate choice of ODP-linux scheduler component
for performance optimization especially in lower
queue counts use cases.

It includes a new core algorithm, but adopted the
ring-based pktio poll algorithm from default scheduler,
and still uses the old ordered queue implementation.

Signed-off-by: Yi He 
---
 platform/linux-generic/Makefile.am   |1 +
 platform/linux-generic/m4/odp_schedule.m4|7 +
 platform/linux-generic/odp_schedule_if.c |6 +
 platform/linux-generic/odp_schedule_iquery.c | 1523 ++
 4 files changed, 1537 insertions(+)
 create mode 100644 platform/linux-generic/odp_schedule_iquery.c

diff --git a/platform/linux-generic/Makefile.am 
b/platform/linux-generic/Makefile.am
index a6e62ef..cc2bf73 100644
--- a/platform/linux-generic/Makefile.am
+++ b/platform/linux-generic/Makefile.am
@@ -209,6 +209,7 @@ __LIB__libodp_linux_la_SOURCES = \
   odp_schedule.c \
   odp_schedule_if.c \
   odp_schedule_sp.c \
+  odp_schedule_iquery.c \
   odp_shared_memory.c \
   odp_sorted_list.c \
   odp_spinlock.c \
diff --git a/platform/linux-generic/m4/odp_schedule.m4 
b/platform/linux-generic/m4/odp_schedule.m4
index bc70c1f..2dcc9a7 100644
--- a/platform/linux-generic/m4/odp_schedule.m4
+++ b/platform/linux-generic/m4/odp_schedule.m4
@@ -4,3 +4,10 @@ AC_ARG_ENABLE([schedule-sp],
schedule-sp=yes
ODP_CFLAGS="$ODP_CFLAGS -DODP_SCHEDULE_SP"
 fi])
+
+AC_ARG_ENABLE([schedule-iquery],
+[  --enable-schedule-iqueryenable interests query (sparse bitmap) 
scheduler],
+[if test x$enableval = xyes; then
+   schedule-iquery=yes
+   ODP_CFLAGS="$ODP_CFLAGS -DODP_SCHEDULE_IQUERY"
+fi])
diff --git a/platform/linux-generic/odp_schedule_if.c 
b/platform/linux-generic/odp_schedule_if.c
index daf6c98..a9ede98 100644
--- a/platform/linux-generic/odp_schedule_if.c
+++ b/platform/linux-generic/odp_schedule_if.c
@@ -12,9 +12,15 @@ extern const schedule_api_t schedule_sp_api;
 extern const schedule_fn_t schedule_default_fn;
 extern const schedule_api_t schedule_default_api;

+extern const schedule_fn_t schedule_iquery_fn;
+extern const schedule_api_t schedule_iquery_api;
+
 #ifdef ODP_SCHEDULE_SP
 const schedule_fn_t *sched_fn   = _sp_fn;
 const schedule_api_t *sched_api = _sp_api;
+#elif defined(ODP_SCHEDULE_IQUERY)
+const schedule_fn_t *sched_fn   = _iquery_fn;
+const schedule_api_t *sched_api = _iquery_api;
 #else
 const schedule_fn_t  *sched_fn  = _default_fn;
 const schedule_api_t *sched_api = _default_api;
diff --git a/platform/linux-generic/odp_schedule_iquery.c 
b/platform/linux-generic/odp_schedule_iquery.c
new file mode 100644
index 000..41f7f27
--- /dev/null
+++ b/platform/linux-generic/odp_schedule_iquery.c
@@ -0,0 +1,1523 @@
+/* Copyright (c) 2016, Linaro Limited
+ * All rights reserved.
+ *
+ * SPDX-License-Identifier: BSD-3-Clause
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+/* Number of priority levels */
+#define NUM_SCHED_PRIO 8
+
+ODP_STATIC_ASSERT(ODP_SCHED_PRIO_LOWEST == (NUM_SCHED_PRIO - 1),
+ "lowest_prio_does_not_match_with_num_prios");
+
+ODP_STATIC_ASSERT((ODP_SCHED_PRIO_NORMAL > 0) &&
+ (ODP_SCHED_PRIO_NORMAL < (NUM_SCHED_PRIO - 1)),
+ "normal_prio_is_not_between_highest_and_lowest");
+
+/* Number of scheduling groups */
+#define NUM_SCHED_GRPS 256
+
+/* Start of named groups in group mask arrays */
+#define SCHED_GROUP_NAMED (ODP_SCHED_GROUP_CONTROL + 1)
+
+/* Instantiate a WAPL bitmap to be used as queue index bitmap */
+typedef WAPL_BITMAP(ODP_CONFIG_QUEUES) queue_index_bitmap_t;
+
+typedef struct {
+   odp_rwlock_t lock;
+   queue_index_bitmap_t queues; /* queues in this priority level */
+} sched_prio_t;
+
+typedef struct {
+   odp_rwlock_t lock;
+   bool allocated;
+   odp_thrmask_t threads; /* threads subscribe to this group */
+   queue_index_bitmap_t queues; /* queues in this group */
+   char name[ODP_SCHED_GROUP_NAME_LEN];
+} sched_group_t;
+
+/* Packet input poll command queues */
+#define PKTIO_CMD_QUEUES 4
+
+/* Maximum number of packet input queues per command */
+#define MAX_PKTIN 16
+
+/* Maximum number of packet IO interfaces */
+#define NUM_PKTIO ODP_CONFIG_PKTIO_ENTRIES
+
+/* Maximum number of pktio poll commands */
+#define NUM_PKTIO_CMD (MAX_PKTIN * NUM_PKTIO)
+
+/* Pktio command is free */
+#define PKTIO_CMD_FREE ((uint32_t)-1)
+
+/* Packet IO poll queue ring size. In worst case, all pktios
+ * have all pktins enabled and one poll command is created per
+ * pktin queue. The ring size must be larger than or equal to
+ * 

[lng-odp] [API-NEXT PATCHv2 2/4] linux-gen: sched: add unsched_queue callback

[Yi He]

Add this unsched_queue callback to indicate queue's
ineligible for scheduling.

Signed-off-by: Yi He 
---
 platform/linux-generic/include/odp_schedule_if.h | 2 ++
 platform/linux-generic/odp_queue.c   | 4 +++-
 platform/linux-generic/odp_schedule.c| 6 ++
 platform/linux-generic/odp_schedule_sp.c | 6 ++
 4 files changed, 17 insertions(+), 1 deletion(-)

diff --git a/platform/linux-generic/include/odp_schedule_if.h 
b/platform/linux-generic/include/odp_schedule_if.h
index c0aee42..530d157 100644
--- a/platform/linux-generic/include/odp_schedule_if.h
+++ b/platform/linux-generic/include/odp_schedule_if.h
@@ -25,6 +25,7 @@ typedef int (*schedule_init_queue_fn_t)(uint32_t queue_index,
   );
 typedef void (*schedule_destroy_queue_fn_t)(uint32_t queue_index);
 typedef int (*schedule_sched_queue_fn_t)(uint32_t queue_index);
+typedef int (*schedule_unsched_queue_fn_t)(uint32_t queue_index);
 typedef int (*schedule_ord_enq_multi_fn_t)(uint32_t queue_index,
   void *buf_hdr[], int num, int *ret);
 typedef int (*schedule_init_global_fn_t)(void);
@@ -44,6 +45,7 @@ typedef struct schedule_fn_t {
schedule_init_queue_fn_tinit_queue;
schedule_destroy_queue_fn_t destroy_queue;
schedule_sched_queue_fn_t   sched_queue;
+   schedule_unsched_queue_fn_t unsched_queue;
schedule_ord_enq_multi_fn_t ord_enq_multi;
schedule_init_global_fn_t   init_global;
schedule_term_global_fn_t   term_global;
diff --git a/platform/linux-generic/odp_queue.c 
b/platform/linux-generic/odp_queue.c
index 3b6a68b..67b5e85 100644
--- a/platform/linux-generic/odp_queue.c
+++ b/platform/linux-generic/odp_queue.c
@@ -525,8 +525,10 @@ static inline int deq_multi(queue_entry_t *queue, 
odp_buffer_hdr_t *buf_hdr[],

if (hdr == NULL) {
/* Already empty queue */
-   if (queue->s.status == QUEUE_STATUS_SCHED)
+   if (queue->s.status == QUEUE_STATUS_SCHED) {
queue->s.status = QUEUE_STATUS_NOTSCHED;
+   sched_fn->unsched_queue(queue->s.index);
+   }

UNLOCK(>s.lock);
return 0;
diff --git a/platform/linux-generic/odp_schedule.c 
b/platform/linux-generic/odp_schedule.c
index 264c58f..704dbd8 100644
--- a/platform/linux-generic/odp_schedule.c
+++ b/platform/linux-generic/odp_schedule.c
@@ -1200,6 +1200,11 @@ static int schedule_sched_queue(uint32_t queue_index)
return 0;
 }

+static int schedule_unsched_queue(uint32_t queue_index ODP_UNUSED)
+{
+   return 0;
+}
+
 static int schedule_num_grps(void)
 {
return NUM_SCHED_GRPS;
@@ -1218,6 +1223,7 @@ const schedule_fn_t schedule_default_fn = {
.init_queue = schedule_init_queue,
.destroy_queue = schedule_destroy_queue,
.sched_queue = schedule_sched_queue,
+   .unsched_queue = schedule_unsched_queue,
.ord_enq_multi = schedule_ord_enq_multi,
.init_global = schedule_init_global,
.term_global = schedule_term_global,
diff --git a/platform/linux-generic/odp_schedule_sp.c 
b/platform/linux-generic/odp_schedule_sp.c
index 8481f29..5c0b503 100644
--- a/platform/linux-generic/odp_schedule_sp.c
+++ b/platform/linux-generic/odp_schedule_sp.c
@@ -307,6 +307,11 @@ static int sched_queue(uint32_t qi)
return 0;
 }

+static int unsched_queue(uint32_t qi ODP_UNUSED)
+{
+   return 0;
+}
+
 static int ord_enq_multi(uint32_t queue_index, void *buf_hdr[], int num,
 int *ret)
 {
@@ -689,6 +694,7 @@ const schedule_fn_t schedule_sp_fn = {
.init_queue= init_queue,
.destroy_queue = destroy_queue,
.sched_queue   = sched_queue,
+   .unsched_queue = unsched_queue,
.ord_enq_multi = ord_enq_multi,
.init_global   = init_global,
.term_global   = term_global,
--
2.7.4

[lng-odp] [API-NEXT PATCHv2 1/4] linux-gen: sched: 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 
---
 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] odp_rwlock_read_trylock

[Nicolas Morey-Chaisemartin]

Le 12/09/2016 à 10:45 AM, Savolainen, Petri (Nokia - FI/Espoo) a écrit :
>
>> -Original Message-
>> From: Nicolas Morey-Chaisemartin [mailto:nmo...@kalray.eu]
>> Sent: Thursday, December 08, 2016 5:30 PM
>> To: Savolainen, Petri (Nokia - FI/Espoo) > labs.com>; LNG ODP Mailman List 
>> Subject: Re: [lng-odp] odp_rwlock_read_trylock
>>
>>
>>
>> Le 12/08/2016 à 03:50 PM, Savolainen, Petri (Nokia - FI/Espoo) a écrit :
 -Original Message-
 From: lng-odp [mailto:lng-odp-boun...@lists.linaro.org] On Behalf Of
 Nicolas Morey-Chaisemartin
 Sent: Wednesday, December 07, 2016 10:12 AM
 To: LNG ODP Mailman List 
 Subject: [lng-odp] odp_rwlock_read_trylock

 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.
>>> /**
>>>  * Try to acquire read permission to a reader/writer lock.
>>>  *
>>>  * @param rwlock Pointer to a reader/writer lock
>>>  *
>>>  * @retval  0 Lock was not available for read access
>>>  * @retval !0 Read access to lock acquired
>>>  */
>>> int odp_rwlock_read_trylock(odp_rwlock_t *rwlock);
>>>
>>>
>>> The spec just says that "read access" was not available. Implementation
>> decides when it's available and application just needs to re-try later on.
>> So, this implementation is OK by the spec. Supporting multiple readers
>> would improve parallelism, but it may not be optimal for performance if
>> the lock is free in the common case.
>>>
 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)
>>> You cannot do this on a single thread (lock the same lock twice). You
>> must use recursive locks if the thread holding a lock will lock it again.
>> The issue is odp_rwlock_read_trylock fails here, but odp_rwlock_read_lock
>> would succeed.
>> Usually rwlock are recursive on the read side as it doesn't really matter
>> to the lock if the same thread owns it N times, or N threads own it once
>> each.
>> IMHO, in a non-concurrent setup, read_trylock should always succeed if
>> read_lock would succeed.
>> (I say non concurrent here to not acount for the case of contention on the
>> lock where many retries could be needed to get the lock).
>
> We have recursive version of the lock (odp_rwlock_recursive_read_lock(), etc) 
> and application should use that if the same thread will call lock() twice (or 
> lock + try_lock).

Yes, but the test is not doing that.
I think we have to distinguished the proper usage and what works.

The proper usage if you want to do lock + lock or lock + trylock is to use a 
recursive rw lock.
But if you do that on a non recursive one, what is supposed to happen? Right 
now lock + lock works and lock + trylock fails which doesn't feel natural.


>
>> What bothers me is this:
>>
>> A thread that wants write access will have to wait until
>>  * there are no threads that want read access.
>>
>> This clearly states that a writer cannot get the write lock if a reader
>> *wants* it.
>> I think
>>
>> A thread that wants write access will have to wait until
>>  * there are no threads that own read permission on the lock.
>>
>>
>> would relax this constraint and allow for write-priority rwlock.
>
> That's OK wording.
>
> -Petri
>
>
I'll send a patch for that. Should it go in all branches (including monarch) ?

Nicolas

Re: [lng-odp] odp_rwlock_read_trylock

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

> -Original Message-
> From: Nicolas Morey-Chaisemartin [mailto:nmo...@kalray.eu]
> Sent: Thursday, December 08, 2016 5:30 PM
> To: Savolainen, Petri (Nokia - FI/Espoo)  labs.com>; LNG ODP Mailman List 
> Subject: Re: [lng-odp] odp_rwlock_read_trylock
> 
> 
> 
> Le 12/08/2016 à 03:50 PM, Savolainen, Petri (Nokia - FI/Espoo) a écrit :
> >
> >> -Original Message-
> >> From: lng-odp [mailto:lng-odp-boun...@lists.linaro.org] On Behalf Of
> >> Nicolas Morey-Chaisemartin
> >> Sent: Wednesday, December 07, 2016 10:12 AM
> >> To: LNG ODP Mailman List 
> >> Subject: [lng-odp] odp_rwlock_read_trylock
> >>
> >> 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.
> > /**
> >  * Try to acquire read permission to a reader/writer lock.
> >  *
> >  * @param rwlock Pointer to a reader/writer lock
> >  *
> >  * @retval  0 Lock was not available for read access
> >  * @retval !0 Read access to lock acquired
> >  */
> > int odp_rwlock_read_trylock(odp_rwlock_t *rwlock);
> >
> >
> > The spec just says that "read access" was not available. Implementation
> decides when it's available and application just needs to re-try later on.
> So, this implementation is OK by the spec. Supporting multiple readers
> would improve parallelism, but it may not be optimal for performance if
> the lock is free in the common case.
> >
> >
> >>
> >> 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)
> >
> > You cannot do this on a single thread (lock the same lock twice). You
> must use recursive locks if the thread holding a lock will lock it again.
> >
> 
> The issue is odp_rwlock_read_trylock fails here, but odp_rwlock_read_lock
> would succeed.
> Usually rwlock are recursive on the read side as it doesn't really matter
> to the lock if the same thread owns it N times, or N threads own it once
> each.
> IMHO, in a non-concurrent setup, read_trylock should always succeed if
> read_lock would succeed.
> (I say non concurrent here to not acount for the case of contention on the
> lock where many retries could be needed to get the lock).


We have recursive version of the lock (odp_rwlock_recursive_read_lock(), etc) 
and application should use that if the same thread will call lock() twice (or 
lock + try_lock).


> 
> >
> >> 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?
> >>
> > Do you mean this:
> >  * A reader/writer lock allows multiple simultaneous readers but only
> one
> >  * writer at a time. A thread that wants write access will have to wait
> until
> >  * there are no threads that want read access. This causes a risk for
> >  * starvation. The trylock variants can be used to avoid blocking when
> >  * the lock is not immediately available.
> >
> > It could be softened with "may have to wait" and "may cause a risk".
> Different RW lock implementations should be possible.
> >
> > -Petri
> What bothers me is this:
> 
> A thread that wants write access will have to wait until
>  * there are no threads that want read access.
> 
> This clearly states that a writer cannot get the write lock if a reader
> *wants* it.
> I think
> 
> A thread that wants write access will have to wait until
>  * there are no threads that own read permission on the lock.
> 
> 
> would relax this constraint and allow for write-priority rwlock.


That's OK wording.

-Petri

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

[Nicolas Morey-Chaisemartin]

I'm OK with this. The only issue I have with this is that it moves back 
platform/OS selection back to configure.ac while I've been trying to move it 
out (https://github.com/nmorey/odp/tree/dev/generic-platforms).
Shouldn't each platform select the right OS ?
I mean linux-generic will probably always use the linux helper while the mppa 
implementation will use the right one for us (depending on the compilation 
flags).

Also I think the lib should be renamed to libodphelper instead of 
libodphelper-linux.

Any plans to get these patches in soon?
Should I wait for your patch to get in master, or get them in my patch series?

Nicolas


Le 12/08/2016 à 09:47 PM, Mike Holmes a écrit :
> So this https://github.com/mike-holmes-linaro/odp/tree/helpers-os is what I 
> was thinking
>
> It moves the Linux specifics to helper/os/linux and selects the helper os to 
> be built in the configure step
> It renames the linux.h helper include to threads.h which is what it 
> predominately is
> It also removes the unused Linux specific  APIs since no odp test or example 
> uses them and they break having an OS agnostic helper suite.
>
> On 8 December 2016 at 13:44, Mike Holmes  > wrote:
>
>
>
> On 7 December 2016 at 01: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.
>
>
> I would be keen to do the same thing master, if it is OS differences then 
> that is much more in keeping with the intent.
> I will play with a patch to move linux.c under and OS directory called 
> linux
>  
>
> >
> >>  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
> >> +++ 

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

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

> +
> +/**
> + * Generate repeatable random data for testing purposes
> + *
> + * 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
> + * 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_test_data(uint8_t *buf, uint32_t len,
> +  odp_random_kind_t kind, uint32_t *seed);
> 

This is from rand_r() documentation:
" The value pointed to by the seedp argument of rand_r() provides only
a very small amount of state, so this function will be a weak pseudo-
random generator.  Try drand48_r(3) instead."

API should not limit the state size to be so small that e.g. ODP_RANDOM_CRYPTO 
quality implementation is not possible.

So, I think we are back in my previous proposal of implementation specific 
state size.

/** Random number generator state data */
typedef struct odp_random_state_t {
  // Implementation defined structure size
uint32_t random_state[32];
} odp_random_state_t;

/**
 * Initialize random state data with a seed value
 * 
 * Uses some bits of seed to initialize the state data
 */
void odp_random_state_init(odp_random_state_t *state, uint64_t seed);

/**
 * Generate repeatable random data for testing purposes
 *
 * 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 state data is
 * updated for each call to continue the sequence. A series of calls with the
 * same initial seeded state data will generate the same sequence of random 
 * test data. Use odp_random_state_init() to initialize or restart the state.
 * The function is thread safe as long as multiple threads do not use the same
 * state simultaneously.
 *
 * 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, odp_random_state_t *state);


For example, these functions could be used with this spec:

int random_r(struct random_data *buf, int32_t *result);
int srandom_r(unsigned int seed, struct random_data *buf);
int initstate_r(unsigned int seed, char *statebuf,
   size_t statelen, struct random_data *buf);
int setstate_r(char *statebuf, struct random_data *buf);


Although, those seem to be nonstandard glibc extensions, so might not what to 
use those in odp-linux, if strict posix API implementation is preferred.


-Petri

Re: [lng-odp] [API-NEXT PATCH v3] api: ipsec: added IPSEC API

[Bala Manoharan]

Reviewed-by: Balasubramanian Manoharan 


On 1 December 2016 at 14:47, Petri Savolainen
 wrote:
> Added definitions for a look-a-side IPSEC offload API. In addition to
> IPSEC packet transformations, it also supports:
> * inbound SA look up
> * outbound IP fragmentation
>
> Signed-off-by: Petri Savolainen 
> ---
>
> Changes in v3:
> * Reword packet ordering specification
>
> Changes in v2:
> * Specify that synchronous calls cannot process all packets
>   if output.num_pkt < input.num_pkt
> * Specify that resulting event must be freed before calling using packets
> * Added soft/hard sec limit capability
> * Improved packet order specification
>
> Changes in v1:
> * renamed odp_ipsec_proto_t to renamed odp_ipsec_protocol_t
> * specify that lifetime sec limit is from the SA creation
> * added odp_ipsec_sa_context()
> * pool for output packets is the same as packet input pool
> * added antireplay check and protocol error codes
> * specified which input / output packet offsets and flags are set
> * moved sync/async mode selection to global config (odp_ipsec_config())
> * added IPSEC capability to aid mode selection
> * specify that also packet user area is copied from input to output packet
>
>
>  include/odp/api/spec/event.h   |   2 +-
>  include/odp/api/spec/ipsec.h   | 883 
> +
>  include/odp_api.h  |   1 +
>  platform/Makefile.inc  |   1 +
>  platform/linux-generic/Makefile.am |   2 +
>  platform/linux-generic/include/odp/api/ipsec.h |  36 +
>  .../include/odp/api/plat/event_types.h |   1 +
>  .../include/odp/api/plat/ipsec_types.h |  39 +
>  8 files changed, 964 insertions(+), 1 deletion(-)
>  create mode 100644 include/odp/api/spec/ipsec.h
>  create mode 100644 platform/linux-generic/include/odp/api/ipsec.h
>  create mode 100644 platform/linux-generic/include/odp/api/plat/ipsec_types.h
>
> diff --git a/include/odp/api/spec/event.h b/include/odp/api/spec/event.h
> index fdfa52d..75c0bbc 100644
> --- a/include/odp/api/spec/event.h
> +++ b/include/odp/api/spec/event.h
> @@ -39,7 +39,7 @@ extern "C" {
>   * @typedef odp_event_type_t
>   * ODP event types:
>   * ODP_EVENT_BUFFER, ODP_EVENT_PACKET, ODP_EVENT_TIMEOUT,
> - * ODP_EVENT_CRYPTO_COMPL
> + * ODP_EVENT_CRYPTO_COMPL, ODP_EVENT_IPSEC_RESULT
>   */
>
>  /**
> diff --git a/include/odp/api/spec/ipsec.h b/include/odp/api/spec/ipsec.h
> new file mode 100644
> index 000..86f66e6
> --- /dev/null
> +++ b/include/odp/api/spec/ipsec.h
> @@ -0,0 +1,883 @@
> +/* Copyright (c) 2016, Linaro Limited
> + * All rights reserved.
> + *
> + * SPDX-License-Identifier: BSD-3-Clause
> + */
> +
> +/**
> + * @file
> + *
> + * ODP IPSEC API
> + */
> +
> +#ifndef ODP_API_IPSEC_H_
> +#define ODP_API_IPSEC_H_
> +#include 
> +
> +#ifdef __cplusplus
> +extern "C" {
> +#endif
> +
> +#include 
> +
> +/** @defgroup odp_ipsec ODP IPSEC
> + *  Operations of IPSEC API.
> + *  @{
> + */
> +
> +/**
> + * @typedef odp_ipsec_sa_t
> + * IPSEC Security Association (SA)
> + */
> +
> + /**
> + * @def ODP_IPSEC_SA_INVALID
> + * Invalid IPSEC SA
> + */
> +
> +/**
> + * IPSEC operation mode
> + */
> +typedef enum odp_ipsec_op_mode_t {
> +   /** Synchronous IPSEC operation
> + *
> + * Application uses synchronous IPSEC operations,
> + * which output all results on function return.
> + */
> +   ODP_IPSEC_OP_MODE_SYNC = 0,
> +
> +   /** Asynchronous IPSEC operation
> + *
> + * Application uses asynchronous IPSEC operations,
> + * which return results via events.
> + */
> +   ODP_IPSEC_OP_MODE_ASYNC
> +
> +} odp_ipsec_op_mode_t;
> +
> +/**
> + * IPSEC capability
> + */
> +typedef struct odp_ipsec_capability_t {
> +   /** Maximum number of IPSEC SAs */
> +   uint32_t max_num_sa;
> +
> +   /** Synchronous IPSEC operation mode (ODP_IPSEC_OP_MODE_SYNC) support
> +*
> +*  0: Synchronous mode is not supported
> +*  1: Synchronous mode is supported
> +*  2: Synchronous mode is supported and preferred
> +*/
> +   uint8_t op_mode_sync;
> +
> +   /** Asynchronous IPSEC operation mode (ODP_IPSEC_OP_MODE_ASYNC) 
> support
> +*
> +*  0: Asynchronous mode is not supported
> +*  1: Asynchronous mode is supported
> +*  2: Asynchronous mode is supported and preferred
> +*/
> +   uint8_t op_mode_async;
> +
> +   /** Soft expiry limit in seconds support
> +*
> +*  0: Limit is not supported
> +*  1: Limit is supported
> +*/
> +   uint8_t soft_limit_sec;
> +
> +   /** Hard expiry limit in seconds support
> +*
> +*  0: Limit is not supported
> +*  1: Limit is supported
> +*/
> +   uint8_t hard_limit_sec;
> +
> +