Re: [lng-odp] [PATCH] linux-gen: pktio: fix variable overwrite in pktin_recv_buf

[Maxim Uvarov]

uh... good fix, thanks :)

On 06/09/16 08:24, Matias Elo wrote:

Fix variable overwrite bug in pktin_recv_buf().

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

diff --git a/platform/linux-generic/odp_packet_io.c 
b/platform/linux-generic/odp_packet_io.c
index 6de39b6..416a361 100644
--- a/platform/linux-generic/odp_packet_io.c
+++ b/platform/linux-generic/odp_packet_io.c
@@ -540,12 +540,12 @@ static inline int pktin_recv_buf(odp_pktin_queue_t queue,
odp_buffer_hdr_t *buf_hdr;
odp_buffer_t buf;
int i;
-   int ret;
+   int pkts;
int num_rx = 0;
  
-	ret = odp_pktin_recv(queue, packets, num);

+   pkts = odp_pktin_recv(queue, packets, num);
  
-	for (i = 0; i < ret; i++) {

+   for (i = 0; i < pkts; i++) {
pkt = packets[i];
pkt_hdr = odp_packet_hdr(pkt);
buf = _odp_packet_to_buffer(pkt);
@@ -553,6 +553,7 @@ static inline int pktin_recv_buf(odp_pktin_queue_t queue,
  
  		if (pkt_hdr->input_flags.dst_queue) {

queue_entry_t *dst_queue;
+   int ret;
  
  			dst_queue = queue_to_qentry(pkt_hdr->dst_queue);

ret = queue_enq(dst_queue, buf_hdr, 0);


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

Re: [lng-odp] [PATCH] validation: tm: avoid resouce/memory leaks in corner cases

[Maxim Uvarov]

On 06/06/16 23:40, Bill Fischofer wrote:

Resolve Bug https://bugs.linaro.org/show_bug.cgi?id=2306 by cleaning up
TM queues and nodes in error paths.

Signed-off-by: Bill Fischofer 
---
  test/validation/traffic_mngr/traffic_mngr.c | 13 +
  1 file changed, 13 insertions(+)

diff --git a/test/validation/traffic_mngr/traffic_mngr.c 
b/test/validation/traffic_mngr/traffic_mngr.c
index a0cde43..208671b 100644
--- a/test/validation/traffic_mngr/traffic_mngr.c
+++ b/test/validation/traffic_mngr/traffic_mngr.c
@@ -1325,12 +1325,19 @@ static int create_tm_queue(odp_tm_t odp_tm,
rc = odp_tm_queue_connect(tm_queue, tm_node);
if (rc != 0) {
LOG_ERR("odp_tm_queue_connect() failed\n");
+   odp_tm_queue_destroy(tm_queue);
return -1;
}
  
  	return 0;

  }
  
+static int destroy_tm_queue(odp_tm_queue_t tm_queue)

+{
+   odp_tm_queue_disconnect(tm_queue);
+   return odp_tm_queue_destroy(tm_queue);
+}
+
  static tm_node_desc_t *create_tm_node(odp_tm_todp_tm,
  uint32_tlevel,
  uint32_tnum_levels,
@@ -1392,6 +1399,7 @@ static tm_node_desc_t *create_tm_node(odp_tm_t
odp_tm,
if (rc != 0) {
LOG_ERR("odp_tm_node_connect() failed @ level=%u\n",
level);
+   odp_tm_node_destroy(tm_node);
return NULL;
}
  
@@ -1425,6 +1433,11 @@ static tm_node_desc_t *create_tm_node(odp_tm_todp_tm,

if (rc != 0) {
LOG_ERR("create_tm_queue() failed @ level=%u\n",
level);
+   while (priority > 0)
+   destroy_tm_queue
+   (queue_desc->tm_queues[--priority]);

if you do not check return code function has to be void, not it.

Interesting that checkpatch does not warn about splitting function name 
and ( on 2 lines.
But if we did not have that it practice before it's better to not use 
it. To simplify reading I would move

destroy path out of current loop, that also gave you more space for code.

Maxim.



+   free(queue_desc);
+   free(node_desc);
return NULL;
}
}


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

[lng-odp] [PATCH] linux-gen: pktio: fix variable overwrite in pktin_recv_buf

[Matias Elo]

Fix variable overwrite bug in pktin_recv_buf().

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

diff --git a/platform/linux-generic/odp_packet_io.c 
b/platform/linux-generic/odp_packet_io.c
index 6de39b6..416a361 100644
--- a/platform/linux-generic/odp_packet_io.c
+++ b/platform/linux-generic/odp_packet_io.c
@@ -540,12 +540,12 @@ static inline int pktin_recv_buf(odp_pktin_queue_t queue,
odp_buffer_hdr_t *buf_hdr;
odp_buffer_t buf;
int i;
-   int ret;
+   int pkts;
int num_rx = 0;
 
-   ret = odp_pktin_recv(queue, packets, num);
+   pkts = odp_pktin_recv(queue, packets, num);
 
-   for (i = 0; i < ret; i++) {
+   for (i = 0; i < pkts; i++) {
pkt = packets[i];
pkt_hdr = odp_packet_hdr(pkt);
buf = _odp_packet_to_buffer(pkt);
@@ -553,6 +553,7 @@ static inline int pktin_recv_buf(odp_pktin_queue_t queue,
 
if (pkt_hdr->input_flags.dst_queue) {
queue_entry_t *dst_queue;
+   int ret;
 
dst_queue = queue_to_qentry(pkt_hdr->dst_queue);
ret = queue_enq(dst_queue, buf_hdr, 0);
-- 
1.9.1

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

Re: [lng-odp] thread/shmem discussion summary V4

[Bill Fischofer]

On Wed, Jun 8, 2016 at 10:54 AM, Yi He  wrote:

> Hi, Christophe and team
>
> For the shmem part:
>
> S18: yes
> S19, S20: I agree with Bill's comments are very accurate and formalized.
>
> S21: to cover the case shm addresses(pointers) passed within data structure
>
> especially think of cases:
> 3) Context pointer getters (odp_queue_context(),
> odp_packet_user_ptr(), odp_timeout_user_ptr(),
> odp_tm_node_context(), odp_tm_queue_context(), and
> I agree with the solution, only feel it may meet difficulties
> in runtime:
>
> According to man page:
> Using shmat() with shmaddr equal to NULL is the preferred,
> portable way of attaching a shared memory segment. Be aware
> that the shared memory segment attached in this way may be
> attached at *different addresses in different processes*.
> Therefore, *any pointers maintained within the shared memory *
> * must be made relative* (typically to the starting address of
> the segment), rather than absolute.
> I found an alternate sweetheart only available in Windows,
> called based pointers (C++)
> https://msdn.microsoft.com/en-us/library/57a97k4e.aspx
>
> Maybe we can spent some time to look for a counterpart in
> std C world, that will be perfect.
>

For students of programming history, or those old enough to remember, the
concept of BASED storage originated in the (now ancient) programming
language PL/I. Pointers to BASED storage were stored as offsets and the
compiler automatically handled the relative addressing. They are very
convenient for this sort of purpose.


>
> S23: agree with Bill's comments covered the cases.
>
> Thanks and best regards, Yi
>
> On 8 June 2016 at 17:04, Christophe Milard 
> wrote:
>
>> OK. good that you agree (and please update the shared doc so it
>> becomes the central point of information).
>> There is something I like though, in your willingness to decouple the
>> function and the pinning...
>> Even if I am not sure this can be enforced by ODP at all time (as
>> already stated), there is definitively a point in helping the
>> application that with to do so. So please keep an eye on that!
>>
>> Your opinion on S19 S20 and S21 would be very welcome as well... This
>> is the main hurting point
>>
>> Christophe
>>
>> On 8 June 2016 at 09:41, Yi He  wrote:
>> > Hi, thanks Christophe and happy to discuss with and learn from team in
>> > yesterday's ARCH call :)
>> >
>> > The question which triggers this kind of thinking is: how to use ODP as
>> a
>> > framework to produce re-usable building blocks to compose "Network
>> Function
>> > Instance" in runtime, since until runtime it will prepare resources for
>> > function to settle down, thus comes the thought of seperating function
>> > implementation and launching.
>> >
>> > I agree your point it seems upper layer consideration, I'll have some
>> time
>> > to gain deeper understanding and knowledge on how upstair playings,
>> thus I
>> > agree to the current S11, S12, S13, S14, S15, S16, S17 approach and we
>> can
>> > revisit if really realized upper layer programming practise/model
>> affects
>> > ODP's design in this aspect.
>> >
>> > Best Regards, Yi
>> >
>> > On 7 June 2016 at 16:47, Christophe Milard <
>> christophe.mil...@linaro.org>
>> > wrote:
>> >>
>> >> On 7 June 2016 at 10:22, Yi He  wrote:
>> >> > Hi, team
>> >> >
>> >> > I send my thoughts on the ODP thread part:
>> >> >
>> >> > S1, S2, S3, S4
>> >> > Yi: Yes
>> >> > This set of statements defines ODP thread concept as a higher
>> >> > level abstraction of underlying concurrent execution context.
>> >> >
>> >> > S5, S6, S7, S8, S9, S10:
>> >> > Yi: Yes
>> >> > This set of statements add several constraints upon ODP
>> >> > instance concept.
>> >> >
>> >> > S11, S12, S13, S14, S15, S16, S17:
>> >> > Yi: Not very much
>> >> >
>> >> > Currently ODP application still mixes the Function Implementation
>> >> > and its Deployment, by this I mean in ODP application code, there
>> >> > is code to implement Function, there is also code to deploy the
>> >> > Function onto platform resources (CPU cores).
>> >> >
>> >> > I'd like to propose a programming practice/model as an attempt to
>> >> > decouple these two things, ODP application code only focuses on
>> >> > implementing Function, and leave it to a deployment script or
>> >> > launcher to take care the deployment with different resource spec
>> >> > (and sufficiency check also).
>> >>
>> >> Well, that goes straight against Barry's will that apps should pin
>> >> their tasks  on cpus...
>> >> Please join the public call today: if Barry is there you can discuss
>> >> this: I am happy to hear other voices in this discussion
>> >>
>> >> >
>> >> > /* Use Case: a upper layer orchestrator, say Daylight is deploying
>> >> > * an ODP application (can be a VNF instance in my opinion) onto
>> >> > * some platform:
>> >> > */
>> >> >
>> >> > /* The application can accept command line options */
>> >> > --list-launchable
>> >> > list all algorithm functions that need to be arranged
>> >

Re: [lng-odp] [PATCHv2] example: timer simple

[Bill Fischofer]

On Wed, Jun 8, 2016 at 2:27 PM, Maxim Uvarov 
wrote:

> Example for simple timer usage.
> https://bugs.linaro.org/show_bug.cgi?id=2090
>
> Signed-off-by: Maxim Uvarov 
>

Reviewed-and-tested-by: Bill Fischofer 


> ---
>  v2: use better name for pool and more return code checks on destroy
> (Bills comments).
>
>  example/timer/.gitignore |   1 +
>  example/timer/Makefile.am|  12 ++-
>  example/timer/odp_timer_simple.c | 157
> +++
>  3 files changed, 167 insertions(+), 3 deletions(-)
>  create mode 100644 example/timer/odp_timer_simple.c
>
> diff --git a/example/timer/.gitignore b/example/timer/.gitignore
> index eb59265..f53a0df 100644
> --- a/example/timer/.gitignore
> +++ b/example/timer/.gitignore
> @@ -1 +1,2 @@
>  odp_timer_test
> +odp_timer_simple
> diff --git a/example/timer/Makefile.am b/example/timer/Makefile.am
> index fcb67a9..1c733d3 100644
> --- a/example/timer/Makefile.am
> +++ b/example/timer/Makefile.am
> @@ -1,10 +1,16 @@
>  include $(top_srcdir)/example/Makefile.inc
>
> -bin_PROGRAMS = odp_timer_test$(EXEEXT)
> +bin_PROGRAMS = odp_timer_test$(EXEEXT) \
> +   odp_timer_simple$(EXEEXT)
>  odp_timer_test_LDFLAGS = $(AM_LDFLAGS) -static
>  odp_timer_test_CFLAGS = $(AM_CFLAGS) -I${top_srcdir}/example
> +dist_odp_timer_test_SOURCES = odp_timer_test.c
> +
> +odp_timer_simple_LDFLAGS = $(AM_LDFLAGS) -static
> +odp_timer_simple_CFLAGS = $(AM_CFLAGS) -I${top_srcdir}/example
> +dist_odp_timer_simple_SOURCES = odp_timer_simple.c
> +
> +TESTS = odp_timer_simple
>
>  noinst_HEADERS = \
>   $(top_srcdir)/example/example_debug.h
> -
> -dist_odp_timer_test_SOURCES = odp_timer_test.c
> diff --git a/example/timer/odp_timer_simple.c
> b/example/timer/odp_timer_simple.c
> new file mode 100644
> index 000..f33add9
> --- /dev/null
> +++ b/example/timer/odp_timer_simple.c
> @@ -0,0 +1,157 @@
> +/* Copyright (c) 2016, Linaro Limited
> + * All rights reserved.
> + *
> + * SPDX-License-Identifier: BSD-3-Clause
> + */
> +/**
> + * @file
> + *
> + * @example odp_timer_simple.c  ODP simple example to schedule timer
> + * action for 1 second.
> + */
> +
> +#include 
> +#include 
> +#include 
> +
> +/* ODP main header */
> +#include 
> +
> +int main(int argc ODP_UNUSED, char *argv[] ODP_UNUSED)
> +{
> +   odp_instance_t instance;
> +   odp_pool_param_t params;
> +   odp_timer_pool_param_t tparams;
> +   odp_pool_t timeout_pool;
> +   odp_timer_pool_t timer_pool;
> +   odp_queue_param_t qparam;
> +   odp_queue_t queue;
> +   odp_event_t ev = ODP_EVENT_INVALID;
> +   odp_timer_t tim;
> +   uint64_t sched_tmo;
> +   int i, rc;
> +   uint64_t period;
> +   uint64_t tick;
> +   odp_timeout_t tmo;
> +   int ret = 0;
> +
> +   /*
> +* Init ODP app
> +*/
> +   if (odp_init_global(&instance, NULL, NULL))
> +   goto err_global;
> +
> +   if (odp_init_local(instance, ODP_THREAD_CONTROL))
> +   goto err_local;
> +
> +   /*
> +* Create pool for timeouts
> +*/
> +   odp_pool_param_init(¶ms);
> +   params.tmo.num   = 10;
> +   params.type  = ODP_POOL_TIMEOUT;
> +
> +   timeout_pool = odp_pool_create("timeout_pool", ¶ms);
> +   if (timeout_pool == ODP_POOL_INVALID) {
> +   ret += 1;
> +   goto err_tp;
> +   }
> +
> +   /*
> +* Create pool of timeouts
> +*/
> +   tparams.res_ns = 10 * ODP_TIME_USEC_IN_NS;
> +   tparams.min_tmo = 10 * ODP_TIME_USEC_IN_NS;
> +   tparams.max_tmo = 1 * ODP_TIME_SEC_IN_NS;
> +   tparams.num_timers = 1; /* One timer per worker */
> +   tparams.priv = 0; /* Shared */
> +   tparams.clk_src = ODP_CLOCK_CPU;
> +   timer_pool = odp_timer_pool_create("timer_pool", &tparams);
> +   if (timer_pool == ODP_TIMER_POOL_INVALID) {
> +   ret += 1;
> +   goto err;
> +   }
> +
> +   /*
> +* Create a queue for timer test
> +*/
> +   odp_queue_param_init(&qparam);
> +   qparam.type= ODP_QUEUE_TYPE_SCHED;
> +   qparam.sched.prio  = ODP_SCHED_PRIO_DEFAULT;
> +   qparam.sched.sync  = ODP_SCHED_SYNC_PARALLEL;
> +   qparam.sched.group = ODP_SCHED_GROUP_ALL;
> +
> +   queue = odp_queue_create("timer_queue", &qparam);
> +   if (queue == ODP_QUEUE_INVALID) {
> +   ret += 1;
> +   goto err;
> +   }
> +
> +   tim = odp_timer_alloc(timer_pool, queue, NULL);
> +   if (tim == ODP_TIMER_INVALID) {
> +   EXAMPLE_ERR("Failed to allocate timer\n");
> +   ret += 1;
> +   goto err;
> +   }
> +
> +   tmo = odp_timeout_alloc(timeout_pool);
> +   if (tmo == ODP_TIMEOUT_INVALID) {
> +   EXAMPLE_ERR("Failed to allocate timeout\n");
> +   return -1;
> +   }
> +
> +   ev = odp_timeout_to_event(tmo);
> +
> +

Re: [lng-odp] [PATCH 1/2] linux-generic: packet: copy user area as part of odp_packet_copy()

[Bill Fischofer]

On Wed, Jun 8, 2016 at 8:23 AM, Zoltan Kiss  wrote:

> Hi,
>
>
> On 07/06/16 15:53, Bill Fischofer wrote:
>
>> Resolve Bug https://bugs.linaro.org/show_bug.cgi?id=2310 by copying the
>> user area as part of odp_packet_copy(). The copy fails if the user area
>> size of the destination pool is not large enough to hold the source packet
>> user area.
>>
>> Signed-off-by: Bill Fischofer 
>> ---
>>   platform/linux-generic/include/odp_packet_internal.h |  2 +-
>>   platform/linux-generic/odp_packet.c  | 18
>> --
>>   2 files changed, 5 insertions(+), 15 deletions(-)
>>
>> diff --git a/platform/linux-generic/include/odp_packet_internal.h
>> b/platform/linux-generic/include/odp_packet_internal.h
>> index d5ace12..5c74d97 100644
>> --- a/platform/linux-generic/include/odp_packet_internal.h
>> +++ b/platform/linux-generic/include/odp_packet_internal.h
>> @@ -294,7 +294,7 @@ static inline int
>> packet_parse_not_complete(odp_packet_hdr_t *pkt_hdr)
>>   }
>>
>>   /* Forward declarations */
>> -void _odp_packet_copy_md_to_packet(odp_packet_t srcpkt, odp_packet_t
>> dstpkt);
>> +int _odp_packet_copy_md_to_packet(odp_packet_t srcpkt, odp_packet_t
>> dstpkt);
>>
>>   odp_packet_t packet_alloc(odp_pool_t pool_hdl, uint32_t len, int parse);
>>
>> diff --git a/platform/linux-generic/odp_packet.c
>> b/platform/linux-generic/odp_packet.c
>> index 2868736..f92c257 100644
>> --- a/platform/linux-generic/odp_packet.c
>> +++ b/platform/linux-generic/odp_packet.c
>> @@ -756,22 +756,11 @@ odp_packet_t odp_packet_copy(odp_packet_t pkt,
>> odp_pool_t pool)
>>   {
>> odp_packet_hdr_t *srchdr = odp_packet_hdr(pkt);
>> uint32_t pktlen = srchdr->frame_len;
>> -   uint32_t meta_offset = ODP_FIELD_SIZEOF(odp_packet_hdr_t,
>> buf_hdr);
>> odp_packet_t newpkt = odp_packet_alloc(pool, pktlen);
>>
>> if (newpkt != ODP_PACKET_INVALID) {
>> -   odp_packet_hdr_t *newhdr = odp_packet_hdr(newpkt);
>> -   uint8_t *newstart, *srcstart;
>> -
>> -   /* Must copy metadata first, followed by packet data */
>> -   newstart = (uint8_t *)newhdr + meta_offset;
>> -   srcstart = (uint8_t *)srchdr + meta_offset;
>> -
>> -   memcpy(newstart, srcstart,
>> -  sizeof(odp_packet_hdr_t) - meta_offset);
>> -
>> -   if (odp_packet_copy_from_pkt(newpkt, 0, pkt, 0,
>> -pktlen) != 0) {
>> +   if (_odp_packet_copy_md_to_packet(pkt, newpkt) ||
>> +   odp_packet_copy_from_pkt(newpkt, 0, pkt, 0, pktlen)) {
>> odp_packet_free(newpkt);
>> newpkt = ODP_PACKET_INVALID;
>> }
>> @@ -966,7 +955,7 @@ int odp_packet_is_valid(odp_packet_t pkt)
>>*
>>*/
>>
>> -void _odp_packet_copy_md_to_packet(odp_packet_t srcpkt, odp_packet_t
>> dstpkt)
>> +int _odp_packet_copy_md_to_packet(odp_packet_t srcpkt, odp_packet_t
>> dstpkt)
>>
>
> There are other callers of this function, we should either check the
> return value there or make a comment at least about why it's not possible
> for an error to happen.
>
>   {
>> odp_packet_hdr_t *srchdr = odp_packet_hdr(srcpkt);
>> odp_packet_hdr_t *dsthdr = odp_packet_hdr(dstpkt);
>> @@ -986,6 +975,7 @@ void _odp_packet_copy_md_to_packet(odp_packet_t
>> srcpkt, odp_packet_t dstpkt)
>> odp_atomic_load_u32(
>> &srchdr->buf_hdr.ref_count));
>> copy_packet_parser_metadata(srchdr, dsthdr);
>> +   return dsthdr->buf_hdr.uarea_size < srchdr->buf_hdr.uarea_size;
>>
>
> I think we should do this check before we do any copy
>

v2 posted with comments explaining this. I've retained this functionality
because the copy is still needed and that's how it was performed before.
It's up to the caller to determine whether user area truncation is an issue
in that context. Currently that's only odp_packet_copy().


>   }
>>
>>   /**
>>
>>
___
lng-odp mailing list
lng-odp@lists.linaro.org
https://lists.linaro.org/mailman/listinfo/lng-odp

[lng-odp] [PATCHv2 2/2] validation: packet: ensure user area is copied correctly

[Bill Fischofer]

As part of resolution of Bug https://bugs.linaro.org/show_bug.cgi?id=2310
make sure that odp_packet_copy() handles user area copies correctly. The
copy should fail if the target pool's user area size is not large enough
to contain the source user area.

Signed-off-by: Bill Fischofer 
---
 test/validation/packet/packet.c | 69 +++--
 1 file changed, 67 insertions(+), 2 deletions(-)

diff --git a/test/validation/packet/packet.c b/test/validation/packet/packet.c
index f71b658..a4426e2 100644
--- a/test/validation/packet/packet.c
+++ b/test/validation/packet/packet.c
@@ -14,7 +14,7 @@
 /* Reserve some tailroom for tests */
 #define PACKET_TAILROOM_RESERVE  4
 
-static odp_pool_t packet_pool;
+static odp_pool_t packet_pool, packet_pool_no_uarea, packet_pool_double_uarea;
 static uint32_t packet_len;
 
 static uint32_t segmented_packet_len;
@@ -65,6 +65,24 @@ int packet_suite_init(void)
if (packet_pool == ODP_POOL_INVALID)
return -1;
 
+   params.pkt.uarea_size = 0;
+   packet_pool_no_uarea = odp_pool_create("packet_pool_no_uarea",
+  ¶ms);
+   if (packet_pool_no_uarea == ODP_POOL_INVALID) {
+   odp_pool_destroy(packet_pool);
+   return -1;
+   }
+
+   params.pkt.uarea_size = 2 * sizeof(struct udata_struct);
+   packet_pool_double_uarea = odp_pool_create("packet_pool_double_uarea",
+  ¶ms);
+
+   if (packet_pool_double_uarea == ODP_POOL_INVALID) {
+   odp_pool_destroy(packet_pool_no_uarea);
+   odp_pool_destroy(packet_pool);
+   return -1;
+   }
+
test_packet = odp_packet_alloc(packet_pool, packet_len);
 
for (i = 0; i < packet_len; i++) {
@@ -113,8 +131,12 @@ int packet_suite_term(void)
 {
odp_packet_free(test_packet);
odp_packet_free(segmented_test_packet);
-   if (odp_pool_destroy(packet_pool) != 0)
+
+   if (odp_pool_destroy(packet_pool_double_uarea) != 0 ||
+   odp_pool_destroy(packet_pool_no_uarea) != 0 ||
+   odp_pool_destroy(packet_pool) != 0)
return -1;
+
return 0;
 }
 
@@ -922,6 +944,20 @@ static void _packet_compare_data(odp_packet_t pkt1, 
odp_packet_t pkt2)
}
 }
 
+static void _packet_compare_udata(odp_packet_t pkt1, odp_packet_t pkt2)
+{
+   uint32_t usize1 = odp_packet_user_area_size(pkt1);
+   uint32_t usize2 = odp_packet_user_area_size(pkt2);
+
+   void *uaddr1 = odp_packet_user_area(pkt1);
+   void *uaddr2 = odp_packet_user_area(pkt2);
+
+   uint32_t cmplen = usize1 <= usize2 ? usize1 : usize2;
+
+   if (cmplen)
+   CU_ASSERT(!memcmp(uaddr1, uaddr2, cmplen));
+}
+
 static void _packet_compare_offset(odp_packet_t pkt1, uint32_t off1,
   odp_packet_t pkt2, uint32_t off2,
   uint32_t len)
@@ -957,6 +993,11 @@ void packet_test_copy(void)
uint32_t i, plen, seg_len, src_offset, dst_offset;
void *pkt_data;
 
+   pkt = odp_packet_copy(test_packet, packet_pool_no_uarea);
+   CU_ASSERT(pkt == ODP_PACKET_INVALID);
+   if (pkt != ODP_PACKET_INVALID)
+   odp_packet_free(pkt);
+
pkt = odp_packet_copy(test_packet, odp_packet_pool(test_packet));
CU_ASSERT_FATAL(pkt != ODP_PACKET_INVALID);
_packet_compare_data(pkt, test_packet);
@@ -971,6 +1012,30 @@ void packet_test_copy(void)
 
_packet_compare_inflags(pkt, pkt_copy);
_packet_compare_data(pkt, pkt_copy);
+   CU_ASSERT(odp_packet_user_area_size(pkt) ==
+ odp_packet_user_area_size(test_packet));
+   _packet_compare_udata(pkt, pkt_copy);
+   odp_packet_free(pkt_copy);
+   odp_packet_free(pkt);
+
+   pkt = odp_packet_copy(test_packet, packet_pool_double_uarea);
+   CU_ASSERT_FATAL(pkt != ODP_PACKET_INVALID);
+   _packet_compare_data(pkt, test_packet);
+   pool = odp_packet_pool(pkt);
+   CU_ASSERT_FATAL(pool != ODP_POOL_INVALID);
+   pkt_copy = odp_packet_copy(pkt, pool);
+   CU_ASSERT_FATAL(pkt_copy != ODP_PACKET_INVALID);
+
+   CU_ASSERT(pkt != pkt_copy);
+   CU_ASSERT(odp_packet_data(pkt) != odp_packet_data(pkt_copy));
+   CU_ASSERT(odp_packet_len(pkt) == odp_packet_len(pkt_copy));
+
+   _packet_compare_inflags(pkt, pkt_copy);
+   _packet_compare_data(pkt, pkt_copy);
+   CU_ASSERT(odp_packet_user_area_size(pkt) ==
+ 2 * odp_packet_user_area_size(test_packet));
+   _packet_compare_udata(pkt, pkt_copy);
+   _packet_compare_udata(pkt, test_packet);
odp_packet_free(pkt_copy);
 
/* Now test copy_part */
-- 
2.7.4

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

[lng-odp] [PATCHv2 1/2] linux-generic: packet: copy user area as part of odp_packet_copy()

[Bill Fischofer]

Resolve Bug https://bugs.linaro.org/show_bug.cgi?id=2310 by copying the
user area as part of odp_packet_copy(). The copy fails if the user area
size of the destination pool is not large enough to hold the source packet
user area.

Reported-by: Zoltan Kiss 
Signed-off-by: Bill Fischofer 
---
v2: Zoltan review comments.  Include additional metadata like ts, color, that
were added more recently.

 .../linux-generic/include/odp_packet_internal.h|  5 +-
 platform/linux-generic/odp_packet.c| 23 --
 test/validation/packet/packet.c| 53 +-
 3 files changed, 44 insertions(+), 37 deletions(-)

diff --git a/platform/linux-generic/include/odp_packet_internal.h 
b/platform/linux-generic/include/odp_packet_internal.h
index d5ace12..ab54227 100644
--- a/platform/linux-generic/include/odp_packet_internal.h
+++ b/platform/linux-generic/include/odp_packet_internal.h
@@ -192,6 +192,9 @@ static inline void 
copy_packet_parser_metadata(odp_packet_hdr_t *src_hdr,
dst_hdr->l4_len = src_hdr->l4_len;
 
dst_hdr->dst_queue  = src_hdr->dst_queue;
+   dst_hdr->flow_hash  = src_hdr->flow_hash;
+   dst_hdr->timestamp  = src_hdr->timestamp;
+   dst_hdr->op_result  = src_hdr->op_result;
 }
 
 static inline void *packet_map(odp_packet_hdr_t *pkt_hdr,
@@ -294,7 +297,7 @@ static inline int 
packet_parse_not_complete(odp_packet_hdr_t *pkt_hdr)
 }
 
 /* Forward declarations */
-void _odp_packet_copy_md_to_packet(odp_packet_t srcpkt, odp_packet_t dstpkt);
+int _odp_packet_copy_md_to_packet(odp_packet_t srcpkt, odp_packet_t dstpkt);
 
 odp_packet_t packet_alloc(odp_pool_t pool_hdl, uint32_t len, int parse);
 
diff --git a/platform/linux-generic/odp_packet.c 
b/platform/linux-generic/odp_packet.c
index 2868736..40549a8 100644
--- a/platform/linux-generic/odp_packet.c
+++ b/platform/linux-generic/odp_packet.c
@@ -756,22 +756,11 @@ odp_packet_t odp_packet_copy(odp_packet_t pkt, odp_pool_t 
pool)
 {
odp_packet_hdr_t *srchdr = odp_packet_hdr(pkt);
uint32_t pktlen = srchdr->frame_len;
-   uint32_t meta_offset = ODP_FIELD_SIZEOF(odp_packet_hdr_t, buf_hdr);
odp_packet_t newpkt = odp_packet_alloc(pool, pktlen);
 
if (newpkt != ODP_PACKET_INVALID) {
-   odp_packet_hdr_t *newhdr = odp_packet_hdr(newpkt);
-   uint8_t *newstart, *srcstart;
-
-   /* Must copy metadata first, followed by packet data */
-   newstart = (uint8_t *)newhdr + meta_offset;
-   srcstart = (uint8_t *)srchdr + meta_offset;
-
-   memcpy(newstart, srcstart,
-  sizeof(odp_packet_hdr_t) - meta_offset);
-
-   if (odp_packet_copy_from_pkt(newpkt, 0, pkt, 0,
-pktlen) != 0) {
+   if (_odp_packet_copy_md_to_packet(pkt, newpkt) ||
+   odp_packet_copy_from_pkt(newpkt, 0, pkt, 0, pktlen)) {
odp_packet_free(newpkt);
newpkt = ODP_PACKET_INVALID;
}
@@ -966,7 +955,7 @@ int odp_packet_is_valid(odp_packet_t pkt)
  *
  */
 
-void _odp_packet_copy_md_to_packet(odp_packet_t srcpkt, odp_packet_t dstpkt)
+int _odp_packet_copy_md_to_packet(odp_packet_t srcpkt, odp_packet_t dstpkt)
 {
odp_packet_hdr_t *srchdr = odp_packet_hdr(srcpkt);
odp_packet_hdr_t *dsthdr = odp_packet_hdr(dstpkt);
@@ -986,6 +975,12 @@ void _odp_packet_copy_md_to_packet(odp_packet_t srcpkt, 
odp_packet_t dstpkt)
odp_atomic_load_u32(
&srchdr->buf_hdr.ref_count));
copy_packet_parser_metadata(srchdr, dsthdr);
+
+   /* Metadata copied, but return indication of whether the packet
+* user area was truncated in the process. Note this can only
+* happen when copying between different pools.
+*/
+   return dsthdr->buf_hdr.uarea_size < srchdr->buf_hdr.uarea_size;
 }
 
 /**
diff --git a/test/validation/packet/packet.c b/test/validation/packet/packet.c
index 6770339..f71b658 100644
--- a/test/validation/packet/packet.c
+++ b/test/validation/packet/packet.c
@@ -863,32 +863,41 @@ free_packet:
odp_packet_free(pkt);
 }
 
-#define COMPARE_INFLAG(p1, p2, flag) \
+#define COMPARE_HAS_INFLAG(p1, p2, flag) \
CU_ASSERT(odp_packet_has_##flag(p1) == odp_packet_has_##flag(p2))
 
+#define COMPARE_INFLAG(p1, p2, flag) \
+   CU_ASSERT(odp_packet_##flag(p1) == odp_packet_##flag(p2))
+
 static void _packet_compare_inflags(odp_packet_t pkt1, odp_packet_t pkt2)
 {
-   COMPARE_INFLAG(pkt1, pkt2, l2);
-   COMPARE_INFLAG(pkt1, pkt2, l3);
-   COMPARE_INFLAG(pkt1, pkt2, l4);
-   COMPARE_INFLAG(pkt1, pkt2, eth);
-   COMPARE_INFLAG(pkt1, pkt2, eth_bcast);
-   COMPARE_INFLAG(pkt1, pkt2, eth_mcast);
-   COMPARE_INFLAG(pkt1, pkt2, jumbo);
-   COMPARE_INFLAG(pkt1, pkt2, vlan);
-   COMPARE_INFLAG(pkt1, pkt2, vlan_qinq);
-   COMPARE_INFLAG(p

[lng-odp] [PATCHv2] example: timer simple

[Maxim Uvarov]

Example for simple timer usage.
https://bugs.linaro.org/show_bug.cgi?id=2090

Signed-off-by: Maxim Uvarov 
---
 v2: use better name for pool and more return code checks on destroy (Bills 
comments).

 example/timer/.gitignore |   1 +
 example/timer/Makefile.am|  12 ++-
 example/timer/odp_timer_simple.c | 157 +++
 3 files changed, 167 insertions(+), 3 deletions(-)
 create mode 100644 example/timer/odp_timer_simple.c

diff --git a/example/timer/.gitignore b/example/timer/.gitignore
index eb59265..f53a0df 100644
--- a/example/timer/.gitignore
+++ b/example/timer/.gitignore
@@ -1 +1,2 @@
 odp_timer_test
+odp_timer_simple
diff --git a/example/timer/Makefile.am b/example/timer/Makefile.am
index fcb67a9..1c733d3 100644
--- a/example/timer/Makefile.am
+++ b/example/timer/Makefile.am
@@ -1,10 +1,16 @@
 include $(top_srcdir)/example/Makefile.inc
 
-bin_PROGRAMS = odp_timer_test$(EXEEXT)
+bin_PROGRAMS = odp_timer_test$(EXEEXT) \
+   odp_timer_simple$(EXEEXT)
 odp_timer_test_LDFLAGS = $(AM_LDFLAGS) -static
 odp_timer_test_CFLAGS = $(AM_CFLAGS) -I${top_srcdir}/example
+dist_odp_timer_test_SOURCES = odp_timer_test.c
+
+odp_timer_simple_LDFLAGS = $(AM_LDFLAGS) -static
+odp_timer_simple_CFLAGS = $(AM_CFLAGS) -I${top_srcdir}/example
+dist_odp_timer_simple_SOURCES = odp_timer_simple.c
+
+TESTS = odp_timer_simple
 
 noinst_HEADERS = \
  $(top_srcdir)/example/example_debug.h
-
-dist_odp_timer_test_SOURCES = odp_timer_test.c
diff --git a/example/timer/odp_timer_simple.c b/example/timer/odp_timer_simple.c
new file mode 100644
index 000..f33add9
--- /dev/null
+++ b/example/timer/odp_timer_simple.c
@@ -0,0 +1,157 @@
+/* Copyright (c) 2016, Linaro Limited
+ * All rights reserved.
+ *
+ * SPDX-License-Identifier: BSD-3-Clause
+ */
+/**
+ * @file
+ *
+ * @example odp_timer_simple.c  ODP simple example to schedule timer
+ * action for 1 second.
+ */
+
+#include 
+#include 
+#include 
+
+/* ODP main header */
+#include 
+
+int main(int argc ODP_UNUSED, char *argv[] ODP_UNUSED)
+{
+   odp_instance_t instance;
+   odp_pool_param_t params;
+   odp_timer_pool_param_t tparams;
+   odp_pool_t timeout_pool;
+   odp_timer_pool_t timer_pool;
+   odp_queue_param_t qparam;
+   odp_queue_t queue;
+   odp_event_t ev = ODP_EVENT_INVALID;
+   odp_timer_t tim;
+   uint64_t sched_tmo;
+   int i, rc;
+   uint64_t period;
+   uint64_t tick;
+   odp_timeout_t tmo;
+   int ret = 0;
+
+   /*
+* Init ODP app
+*/
+   if (odp_init_global(&instance, NULL, NULL))
+   goto err_global;
+
+   if (odp_init_local(instance, ODP_THREAD_CONTROL))
+   goto err_local;
+
+   /*
+* Create pool for timeouts
+*/
+   odp_pool_param_init(¶ms);
+   params.tmo.num   = 10;
+   params.type  = ODP_POOL_TIMEOUT;
+
+   timeout_pool = odp_pool_create("timeout_pool", ¶ms);
+   if (timeout_pool == ODP_POOL_INVALID) {
+   ret += 1;
+   goto err_tp;
+   }
+
+   /*
+* Create pool of timeouts
+*/
+   tparams.res_ns = 10 * ODP_TIME_USEC_IN_NS;
+   tparams.min_tmo = 10 * ODP_TIME_USEC_IN_NS;
+   tparams.max_tmo = 1 * ODP_TIME_SEC_IN_NS;
+   tparams.num_timers = 1; /* One timer per worker */
+   tparams.priv = 0; /* Shared */
+   tparams.clk_src = ODP_CLOCK_CPU;
+   timer_pool = odp_timer_pool_create("timer_pool", &tparams);
+   if (timer_pool == ODP_TIMER_POOL_INVALID) {
+   ret += 1;
+   goto err;
+   }
+
+   /*
+* Create a queue for timer test
+*/
+   odp_queue_param_init(&qparam);
+   qparam.type= ODP_QUEUE_TYPE_SCHED;
+   qparam.sched.prio  = ODP_SCHED_PRIO_DEFAULT;
+   qparam.sched.sync  = ODP_SCHED_SYNC_PARALLEL;
+   qparam.sched.group = ODP_SCHED_GROUP_ALL;
+
+   queue = odp_queue_create("timer_queue", &qparam);
+   if (queue == ODP_QUEUE_INVALID) {
+   ret += 1;
+   goto err;
+   }
+
+   tim = odp_timer_alloc(timer_pool, queue, NULL);
+   if (tim == ODP_TIMER_INVALID) {
+   EXAMPLE_ERR("Failed to allocate timer\n");
+   ret += 1;
+   goto err;
+   }
+
+   tmo = odp_timeout_alloc(timeout_pool);
+   if (tmo == ODP_TIMEOUT_INVALID) {
+   EXAMPLE_ERR("Failed to allocate timeout\n");
+   return -1;
+   }
+
+   ev = odp_timeout_to_event(tmo);
+
+   /* Calculate period for timer in uint64_t value, in current case
+* we will schedule timer for 1 second */
+   period = odp_timer_ns_to_tick(timer_pool, 1 * ODP_TIME_SEC_IN_NS);
+
+   /* Wait time to return from odp_schedule() if there are no
+* events
+*/
+   sched_tmo = odp_schedule_wait_time(2 * ODP_TIME_SEC_IN_NS);
+
+   for (i = 0; i < 5; i++) {
+  

Re: [lng-odp] [PATCHv2] configure: split up libodphelper SO-version from libodp

[Maxim Uvarov]

On 06/08/16 21:46, Mike Holmes wrote:



On 8 June 2016 at 14:33, Maxim Uvarov > wrote:


it will be more complicated to follow dependencies if there will
be separate version. I think for now it's better to stay with
existence version numbers.


Can you explain the complication, I thought this makes things much 
simpler.


As Yi decouples the helpers I dont think we can avoid this,  the 
helpers will have an evolution that is is no way correlated to a 
specific implementation, it can be packaged, installed and will then 
be used by any system using Linux in the case of the current helpers 
implementation.


From my point it's easy to have same version for both libodp and 
libodphelper if they are in the same git repo. It's more easy to 
maintain it and jump in code to specific version if you need debug 
something. If helpers will go to
separate repo then there is reason for helpers version and in terms of 
maintains it's separate project.



Maxim.






Maxim.

On 8 June 2016 at 19:15, Anders Roxell mailto:anders.rox...@linaro.org>> wrote:

The libodphelper and libopd need to track their evolving ABI
independently and so there needs to be two separate version
numbers.

Signed-off-by: Anders Roxell mailto:anders.rox...@linaro.org>>
---
configure.ac   | 3 +++
 helper/Makefile.am | 2 +-
 2 files changed, 4 insertions(+), 1 deletion(-)

diff --git a/configure.ac  b/configure.ac

index a12f984..85dd3b1 100644
--- a/configure.ac 
+++ b/configure.ac 
@@ -15,6 +15,9 @@ AM_SILENT_RULES([yes])
 ODP_LIBSO_VERSION=110:0:0
 AC_SUBST(ODP_LIBSO_VERSION)

+ODPHELPER_LIBSO_VERSION=110:0:0
+AC_SUBST(ODPHELPER_LIBSO_VERSION)
+
 # Checks for programs.
 AC_PROG_CC
 AM_PROG_CC_C_O
diff --git a/helper/Makefile.am b/helper/Makefile.am
index aa58e8c..a82a11a 100644
--- a/helper/Makefile.am
+++ b/helper/Makefile.am
@@ -8,7 +8,7 @@ AM_CFLAGS  = -I$(srcdir)/include
 AM_CFLAGS += -I$(top_srcdir)/platform/@with_platform@/include
 AM_CFLAGS += -I$(top_srcdir)/include

-AM_LDFLAGS += -version-number '$(ODP_LIBSO_VERSION)'
+AM_LDFLAGS += -version-number '$(ODPHELPER_LIBSO_VERSION)'

 helperincludedir = $(includedir)/odp/helper/
 helperinclude_HEADERS = \
--
2.1.4

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





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



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

Re: [lng-odp] [API-NEXT PATCHv1 0/4] implement mechanism to allow inline and no inline use of ODP API

[Zoltan Kiss]

Hi,

ODP-DPDK already has a simpler solution for this problem, the first 
patch was posted here as well:


https://git.linaro.org/lng/odp-dpdk.git/commitdiff/968237b592a8162041a4edf0091575fd1390d647

It can get simpler as it doesn't need a separate config option, but it 
automatically turns on when --enable-shared=yes (which is the default). 
When you have shared libraries, you definitely need this, as whoever 
links to it doesn't know about your inlines. But if you only compile 
static libraries, you'll need its headers anyway, so you can have inline 
functions as well.


Regards,

Zoltan

On 06/06/16 09:44, huanggaoyang wrote:

ODP needs to be portable across similar architectures with differing ODP 
implementations.
Inline functions will inhibit the compatibility.

In this patch, I add some Macros to determain those inline functions are 
exposed in .h files as before
or put together in a .c file as normal functions without "static inline" 
property.

configure by "--enable-compatibility-mode=yes" will enable the compatibility 
mode.

configure without it:
   ODP_BINARY_COMPATIBLE == 0
   in .h files:
  ODP_ENABLE_INLINE == 1
  ODP_STATIC is static
  ODP_INLINE is inline
   in odp_compatible.c & helper/compatible.c
  codes are disabled since ODP_BINARY_COMPATIBLE == 0
All the codes and APIs are just as before

configure with --enable-compatibility-mode=yes:
   ODP_BINARY_COMPATIBLE == 1
   in .h files:
  ODP_ENABLE_INLINE == 0
  all the former inline functions are disabled.
   in odp_compatible.c & helper/compatible.c
  ODP_STATIC is nothing
  ODP_INLINE is nothing
  ODP_ENABLE_INLINE == 1
  codes are enabled, and the inline functions in .h files are enabled without 
"static inline"

"CFLAGS of helper should inherit from ODP main target" is already proposed by a 
patch of mine last week.
I also include it here because the compile of helper needs it

I find this line of code:
#define ODP_STATIC static
will cause a check-patch warning: "storage class should be at the beginning of the 
declaration".
Is it a false positive? Would someone can help me resolve it?

huanggaoyang (4):
   linux-generic:configure:add configure option
 --enable-compatibility-mode
   linux-generic:compatibility:implement mechanism to allow inline and no
 inline use of ODP API
   helper: CFLAGS of helper should inherit from ODP main target
   helper: implement mechanism to allow inline and no inline use of ODP
 Helper API

  configure.ac   |  13 ++
  helper/Makefile.am |   5 +-
  helper/compatible.c|  24 
  helper/hashtable.c |   7 +-
  helper/include/odp/helper/chksum.h |  71 +-
  helper/include/odp/helper/ip.h |  11 +-
  helper/include/odp/helper/table.h  |   1 -
  helper/lineartable.c   |   8 +-
  helper/odph_hashtable.h|   1 -
  helper/odph_lineartable.h  |   9 +-
  helper/test/table.c|   2 -
  platform/linux-generic/Makefile.am |   1 +
  platform/linux-generic/include/odp/api/atomic.h| 155 -
  platform/linux-generic/include/odp/api/byteorder.h |  27 ++--
  .../linux-generic/include/odp/api/compatible.h |  62 +
  .../include/odp/api/plat/buffer_types.h|   6 +-
  .../include/odp/api/plat/classification_types.h|   9 +-
  .../include/odp/api/plat/crypto_types.h|  10 +-
  .../include/odp/api/plat/event_types.h |   6 +-
  .../include/odp/api/plat/packet_io_types.h |   6 +-
  .../include/odp/api/plat/packet_types.h|   8 +-
  .../include/odp/api/plat/pool_types.h  |   6 +-
  .../include/odp/api/plat/queue_types.h |   6 +-
  .../include/odp/api/plat/shared_memory_types.h |   6 +-
  .../include/odp/api/plat/traffic_mngr_types.h  |   1 -
  platform/linux-generic/include/odp/api/std_clib.h  |   9 +-
  platform/linux-generic/include/odp/api/sync.h  |   7 +-
  platform/linux-generic/odp_compatible.c|  46 ++
  28 files changed, 361 insertions(+), 162 deletions(-)
  create mode 100644 helper/compatible.c
  create mode 100644 platform/linux-generic/include/odp/api/compatible.h
  create mode 100644 platform/linux-generic/odp_compatible.c


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

Re: [lng-odp] [PATCHv2] configure: split up libodphelper SO-version from libodp

[Mike Holmes]

On 8 June 2016 at 14:33, Maxim Uvarov  wrote:

> it will be more complicated to follow dependencies if there will be
> separate version. I think for now it's better to stay with existence
> version numbers.
>

Can you explain the complication, I thought this makes things much simpler.

As Yi decouples the helpers I dont think we can avoid this,  the helpers
will have an evolution that is is no way correlated to a specific
implementation, it can be packaged, installed and will then be used by any
system using Linux in the case of the current helpers implementation.



>
> Maxim.
>
> On 8 June 2016 at 19:15, Anders Roxell  wrote:
>
>> The libodphelper and libopd need to track their evolving ABI
>> independently and so there needs to be two separate version numbers.
>>
>> Signed-off-by: Anders Roxell 
>> ---
>>  configure.ac   | 3 +++
>>  helper/Makefile.am | 2 +-
>>  2 files changed, 4 insertions(+), 1 deletion(-)
>>
>> diff --git a/configure.ac b/configure.ac
>> index a12f984..85dd3b1 100644
>> --- a/configure.ac
>> +++ b/configure.ac
>> @@ -15,6 +15,9 @@ AM_SILENT_RULES([yes])
>>  ODP_LIBSO_VERSION=110:0:0
>>  AC_SUBST(ODP_LIBSO_VERSION)
>>
>> +ODPHELPER_LIBSO_VERSION=110:0:0
>> +AC_SUBST(ODPHELPER_LIBSO_VERSION)
>> +
>>  # Checks for programs.
>>  AC_PROG_CC
>>  AM_PROG_CC_C_O
>> diff --git a/helper/Makefile.am b/helper/Makefile.am
>> index aa58e8c..a82a11a 100644
>> --- a/helper/Makefile.am
>> +++ b/helper/Makefile.am
>> @@ -8,7 +8,7 @@ AM_CFLAGS  = -I$(srcdir)/include
>>  AM_CFLAGS += -I$(top_srcdir)/platform/@with_platform@/include
>>  AM_CFLAGS += -I$(top_srcdir)/include
>>
>> -AM_LDFLAGS += -version-number '$(ODP_LIBSO_VERSION)'
>> +AM_LDFLAGS += -version-number '$(ODPHELPER_LIBSO_VERSION)'
>>
>>  helperincludedir = $(includedir)/odp/helper/
>>  helperinclude_HEADERS = \
>> --
>> 2.1.4
>>
>> ___
>> lng-odp mailing list
>> lng-odp@lists.linaro.org
>> https://lists.linaro.org/mailman/listinfo/lng-odp
>>
>
>


-- 
Mike Holmes
Technical Manager - Linaro Networking Group
Linaro.org  *│ *Open source software for ARM SoCs
"Work should be fun and collaborative, the rest follows"
___
lng-odp mailing list
lng-odp@lists.linaro.org
https://lists.linaro.org/mailman/listinfo/lng-odp

Re: [lng-odp] [PATCHv2] configure: split up libodphelper SO-version from libodp

[Maxim Uvarov]

it will be more complicated to follow dependencies if there will be
separate version. I think for now it's better to stay with existence
version numbers.

Maxim.

On 8 June 2016 at 19:15, Anders Roxell  wrote:

> The libodphelper and libopd need to track their evolving ABI
> independently and so there needs to be two separate version numbers.
>
> Signed-off-by: Anders Roxell 
> ---
>  configure.ac   | 3 +++
>  helper/Makefile.am | 2 +-
>  2 files changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/configure.ac b/configure.ac
> index a12f984..85dd3b1 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -15,6 +15,9 @@ AM_SILENT_RULES([yes])
>  ODP_LIBSO_VERSION=110:0:0
>  AC_SUBST(ODP_LIBSO_VERSION)
>
> +ODPHELPER_LIBSO_VERSION=110:0:0
> +AC_SUBST(ODPHELPER_LIBSO_VERSION)
> +
>  # Checks for programs.
>  AC_PROG_CC
>  AM_PROG_CC_C_O
> diff --git a/helper/Makefile.am b/helper/Makefile.am
> index aa58e8c..a82a11a 100644
> --- a/helper/Makefile.am
> +++ b/helper/Makefile.am
> @@ -8,7 +8,7 @@ AM_CFLAGS  = -I$(srcdir)/include
>  AM_CFLAGS += -I$(top_srcdir)/platform/@with_platform@/include
>  AM_CFLAGS += -I$(top_srcdir)/include
>
> -AM_LDFLAGS += -version-number '$(ODP_LIBSO_VERSION)'
> +AM_LDFLAGS += -version-number '$(ODPHELPER_LIBSO_VERSION)'
>
>  helperincludedir = $(includedir)/odp/helper/
>  helperinclude_HEADERS = \
> --
> 2.1.4
>
> ___
> lng-odp mailing list
> lng-odp@lists.linaro.org
> https://lists.linaro.org/mailman/listinfo/lng-odp
>
___
lng-odp mailing list
lng-odp@lists.linaro.org
https://lists.linaro.org/mailman/listinfo/lng-odp

Re: [lng-odp] [PATCH] example: timer simple

[Maxim Uvarov]

On 06/08/16 14:22, Bill Fischofer wrote:



On Wed, Jun 8, 2016 at 1:57 AM, Maxim Uvarov > wrote:


On 06/08/16 06:07, Bill Fischofer wrote:



On Tue, Jun 7, 2016 at 3:46 PM, Maxim Uvarov
mailto:maxim.uva...@linaro.org>
>> wrote:

Example for simple timer usage.
https://bugs.linaro.org/show_bug.cgi?id=2090

Signed-off-by: Maxim Uvarov mailto:maxim.uva...@linaro.org>
>>

---
 example/timer/.gitignore |   1 +
 example/timer/Makefile.am|  12 ++-
 example/timer/odp_timer_simple.c | 167
+++
 3 files changed, 177 insertions(+), 3 deletions(-)
 create mode 100644 example/timer/odp_timer_simple.c

diff --git a/example/timer/.gitignore
b/example/timer/.gitignore
index eb59265..f53a0df 100644
--- a/example/timer/.gitignore
+++ b/example/timer/.gitignore
@@ -1 +1,2 @@
 odp_timer_test
+odp_timer_simple
diff --git a/example/timer/Makefile.am
b/example/timer/Makefile.am
index fcb67a9..1c733d3 100644
--- a/example/timer/Makefile.am
+++ b/example/timer/Makefile.am
@@ -1,10 +1,16 @@
 include $(top_srcdir)/example/Makefile.inc

-bin_PROGRAMS = odp_timer_test$(EXEEXT)
+bin_PROGRAMS = odp_timer_test$(EXEEXT) \
+   odp_timer_simple$(EXEEXT)
 odp_timer_test_LDFLAGS = $(AM_LDFLAGS) -static
 odp_timer_test_CFLAGS = $(AM_CFLAGS) -I${top_srcdir}/example
+dist_odp_timer_test_SOURCES = odp_timer_test.c
+
+odp_timer_simple_LDFLAGS = $(AM_LDFLAGS) -static
+odp_timer_simple_CFLAGS = $(AM_CFLAGS)
-I${top_srcdir}/example
+dist_odp_timer_simple_SOURCES = odp_timer_simple.c
+
+TESTS = odp_timer_simple

 noinst_HEADERS = \
$(top_srcdir)/example/example_debug.h
-
-dist_odp_timer_test_SOURCES = odp_timer_test.c
diff --git a/example/timer/odp_timer_simple.c
b/example/timer/odp_timer_simple.c
new file mode 100644
index 000..d4917c3
--- /dev/null
+++ b/example/timer/odp_timer_simple.c
@@ -0,0 +1,167 @@
+/* Copyright (c) 2016, Linaro Limited
+ * All rights reserved.
+ *
+ * SPDX-License-Identifier: BSD-3-Clause
+ */
+/**
+ * @file
+ *
+ * @example odp_timer_simple.c  ODP simple example to
schedule timer
+ * action for 1 second.
+ */
+
+#include 
+#include 
+#include 
+
+/* ODP main header */
+#include 
+
+int main(int argc ODP_UNUSED, char *argv[] ODP_UNUSED)
+{
+   odp_instance_t instance;
+   odp_pool_param_t params;
+   odp_timer_pool_param_t tparams;
+   odp_pool_t tmo_pool;
+   odp_timer_pool_t timer_pool;
+   odp_queue_param_t qparam;
+   odp_queue_t queue;
+   odp_event_t ev = ODP_EVENT_INVALID;
+   odp_timer_t tim;
+   uint64_t sched_tmo;
+   int i, rc;
+   uint64_t period;
+   uint64_t tick;
+   odp_timeout_t tmo;
+   int ret = 0;
+
+   /*
+* Init ODP app
+*/
+   if (odp_init_global(&instance, NULL, NULL))
+   goto err_global;
+
+   if (odp_init_local(instance, ODP_THREAD_CONTROL))
+   goto err_local;
+
+   /*
+* Create pool for timeouts
+*/
+   odp_pool_param_init(¶ms);
+   params.tmo.num   = 10;
+   params.type  = ODP_POOL_TIMEOUT;
+
+   tmo_pool = odp_pool_create("msg_pool", ¶ms);


Is "msg_pool" the best choice of name for a timeout pool? 
"timeout_pool" would seem to make more sense here.



agree, will change.


+   if (tmo_pool == ODP_POOL_INVALID) {
+   ret += 1;
+   goto err_tp;
+   }
+
+   /*
+* Create pool of timeouts
   

[lng-odp] [PATCHv2] configure: split up libodphelper SO-version from libodp

[Anders Roxell]

The libodphelper and libopd need to track their evolving ABI
independently and so there needs to be two separate version numbers.

Signed-off-by: Anders Roxell 
---
 configure.ac   | 3 +++
 helper/Makefile.am | 2 +-
 2 files changed, 4 insertions(+), 1 deletion(-)

diff --git a/configure.ac b/configure.ac
index a12f984..85dd3b1 100644
--- a/configure.ac
+++ b/configure.ac
@@ -15,6 +15,9 @@ AM_SILENT_RULES([yes])
 ODP_LIBSO_VERSION=110:0:0
 AC_SUBST(ODP_LIBSO_VERSION)
 
+ODPHELPER_LIBSO_VERSION=110:0:0
+AC_SUBST(ODPHELPER_LIBSO_VERSION)
+
 # Checks for programs.
 AC_PROG_CC
 AM_PROG_CC_C_O
diff --git a/helper/Makefile.am b/helper/Makefile.am
index aa58e8c..a82a11a 100644
--- a/helper/Makefile.am
+++ b/helper/Makefile.am
@@ -8,7 +8,7 @@ AM_CFLAGS  = -I$(srcdir)/include
 AM_CFLAGS += -I$(top_srcdir)/platform/@with_platform@/include
 AM_CFLAGS += -I$(top_srcdir)/include
 
-AM_LDFLAGS += -version-number '$(ODP_LIBSO_VERSION)'
+AM_LDFLAGS += -version-number '$(ODPHELPER_LIBSO_VERSION)'
 
 helperincludedir = $(includedir)/odp/helper/
 helperinclude_HEADERS = \
-- 
2.1.4

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

Re: [lng-odp] thread/shmem discussion summary V4

[Yi He]

Hi, Christophe and team

For the shmem part:

S18: yes
S19, S20: I agree with Bill's comments are very accurate and formalized.

S21: to cover the case shm addresses(pointers) passed within data structure

especially think of cases:
3) Context pointer getters (odp_queue_context(),
odp_packet_user_ptr(), odp_timeout_user_ptr(),
odp_tm_node_context(), odp_tm_queue_context(), and
I agree with the solution, only feel it may meet difficulties
in runtime:

According to man page:
Using shmat() with shmaddr equal to NULL is the preferred,
portable way of attaching a shared memory segment. Be aware
that the shared memory segment attached in this way may be
attached at *different addresses in different processes*.
Therefore, *any pointers maintained within the shared memory *
* must be made relative* (typically to the starting address of
the segment), rather than absolute.
I found an alternate sweetheart only available in Windows,
called based pointers (C++)
https://msdn.microsoft.com/en-us/library/57a97k4e.aspx

Maybe we can spent some time to look for a counterpart in
std C world, that will be perfect.

S23: agree with Bill's comments covered the cases.

Thanks and best regards, Yi

On 8 June 2016 at 17:04, Christophe Milard 
wrote:

> OK. good that you agree (and please update the shared doc so it
> becomes the central point of information).
> There is something I like though, in your willingness to decouple the
> function and the pinning...
> Even if I am not sure this can be enforced by ODP at all time (as
> already stated), there is definitively a point in helping the
> application that with to do so. So please keep an eye on that!
>
> Your opinion on S19 S20 and S21 would be very welcome as well... This
> is the main hurting point
>
> Christophe
>
> On 8 June 2016 at 09:41, Yi He  wrote:
> > Hi, thanks Christophe and happy to discuss with and learn from team in
> > yesterday's ARCH call :)
> >
> > The question which triggers this kind of thinking is: how to use ODP as a
> > framework to produce re-usable building blocks to compose "Network
> Function
> > Instance" in runtime, since until runtime it will prepare resources for
> > function to settle down, thus comes the thought of seperating function
> > implementation and launching.
> >
> > I agree your point it seems upper layer consideration, I'll have some
> time
> > to gain deeper understanding and knowledge on how upstair playings, thus
> I
> > agree to the current S11, S12, S13, S14, S15, S16, S17 approach and we
> can
> > revisit if really realized upper layer programming practise/model affects
> > ODP's design in this aspect.
> >
> > Best Regards, Yi
> >
> > On 7 June 2016 at 16:47, Christophe Milard  >
> > wrote:
> >>
> >> On 7 June 2016 at 10:22, Yi He  wrote:
> >> > Hi, team
> >> >
> >> > I send my thoughts on the ODP thread part:
> >> >
> >> > S1, S2, S3, S4
> >> > Yi: Yes
> >> > This set of statements defines ODP thread concept as a higher
> >> > level abstraction of underlying concurrent execution context.
> >> >
> >> > S5, S6, S7, S8, S9, S10:
> >> > Yi: Yes
> >> > This set of statements add several constraints upon ODP
> >> > instance concept.
> >> >
> >> > S11, S12, S13, S14, S15, S16, S17:
> >> > Yi: Not very much
> >> >
> >> > Currently ODP application still mixes the Function Implementation
> >> > and its Deployment, by this I mean in ODP application code, there
> >> > is code to implement Function, there is also code to deploy the
> >> > Function onto platform resources (CPU cores).
> >> >
> >> > I'd like to propose a programming practice/model as an attempt to
> >> > decouple these two things, ODP application code only focuses on
> >> > implementing Function, and leave it to a deployment script or
> >> > launcher to take care the deployment with different resource spec
> >> > (and sufficiency check also).
> >>
> >> Well, that goes straight against Barry's will that apps should pin
> >> their tasks  on cpus...
> >> Please join the public call today: if Barry is there you can discuss
> >> this: I am happy to hear other voices in this discussion
> >>
> >> >
> >> > /* Use Case: a upper layer orchestrator, say Daylight is deploying
> >> > * an ODP application (can be a VNF instance in my opinion) onto
> >> > * some platform:
> >> > */
> >> >
> >> > /* The application can accept command line options */
> >> > --list-launchable
> >> > list all algorithm functions that need to be arranged
> >> > into an execution unit.
> >> >
> >> > --preparing-threads
> >> > control<1,2,3>@cpu<1>, worker<1,2,3,4>@cpu<2,3,4,5>
> >> > I'm not sure if the control/worker distinguish is needed
> >> > anymore, DPDK has similar concept lcore.
> >> >
> >> >   --wait-launching-signal
> >> >   main process can pause after preparing above threads but
> >> > before launching algorithm functions, here orchestrator can
> >> > further fine-tuning the threads by scripting (cgroups, etc).
> >> > CPU, disk/net IO, memory quotas, interrupt bindings, etc.
> >> >
> >> > 

Re: [lng-odp] [PATCH] configure: split up libodphelper SO-version from libodp

[Mike Holmes]

On 7 June 2016 at 16:29, Anders Roxell  wrote:

> They

can be updated separately so no need to bump both versions at the
> same time, specially when there is no change in one of them.
>


I think it is clearer to say that the libodphelper and  libodp lib need to
track their evolving  abi independently and so there needs to be two
separate version numbers.


>
> Signed-off-by: Anders Roxell 
> ---
>  configure.ac   | 3 +++
>  helper/Makefile.am | 2 +-
>  2 files changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/configure.ac b/configure.ac
> index a12f984..85dd3b1 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -15,6 +15,9 @@ AM_SILENT_RULES([yes])
>  ODP_LIBSO_VERSION=110:0:0
>  AC_SUBST(ODP_LIBSO_VERSION)
>
> +ODPHELPER_LIBSO_VERSION=110:0:0
> +AC_SUBST(ODPHELPER_LIBSO_VERSION)
> +
>  # Checks for programs.
>  AC_PROG_CC
>  AM_PROG_CC_C_O
> diff --git a/helper/Makefile.am b/helper/Makefile.am
> index aa58e8c..a82a11a 100644
> --- a/helper/Makefile.am
> +++ b/helper/Makefile.am
> @@ -8,7 +8,7 @@ AM_CFLAGS  = -I$(srcdir)/include
>  AM_CFLAGS += -I$(top_srcdir)/platform/@with_platform@/include
>  AM_CFLAGS += -I$(top_srcdir)/include
>
> -AM_LDFLAGS += -version-number '$(ODP_LIBSO_VERSION)'
> +AM_LDFLAGS += -version-number '$(ODPHELPER_LIBSO_VERSION)'
>
>  helperincludedir = $(includedir)/odp/helper/
>  helperinclude_HEADERS = \
> --
> 2.1.4
>
> ___
> lng-odp mailing list
> lng-odp@lists.linaro.org
> https://lists.linaro.org/mailman/listinfo/lng-odp
>



-- 
Mike Holmes
Technical Manager - Linaro Networking Group
Linaro.org  *│ *Open source software for ARM SoCs
"Work should be fun and collaborative, the rest follows"
___
lng-odp mailing list
lng-odp@lists.linaro.org
https://lists.linaro.org/mailman/listinfo/lng-odp

Re: [lng-odp] [PATCH v2 4/4] linux-gen: pktio: don't allocate new packets in classifier

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

There is a bug in the pktin_recv_buf() function. I’ll submit a proper patch 
tomorrow, but the following patch fixes the issue.

platform/linux-generic/odp_packet_io.c | 7 ---
1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/platform/linux-generic/odp_packet_io.c 
b/platform/linux-generic/odp_packet_io.c
index 6de39b6..416a361 100644
--- a/platform/linux-generic/odp_packet_io.c
+++ b/platform/linux-generic/odp_packet_io.c
@@ -540,12 +540,12 @@ static inline int pktin_recv_buf(odp_pktin_queue_t queue,
   odp_buffer_hdr_t *buf_hdr;
   odp_buffer_t buf;
   int i;
-  int ret;
+ int pkts;
   int num_rx = 0;
-  ret = odp_pktin_recv(queue, packets, num);
+ pkts = odp_pktin_recv(queue, packets, num);
-  for (i = 0; i < ret; i++) {
+ for (i = 0; i < pkts; i++) {
   pkt = packets[i];
   pkt_hdr = odp_packet_hdr(pkt);
   buf = _odp_packet_to_buffer(pkt);
@@ -553,6 +553,7 @@ static inline int pktin_recv_buf(odp_pktin_queue_t queue,
if (pkt_hdr->input_flags.dst_queue) {
   queue_entry_t *dst_queue;
+ int ret;
dst_queue = 
queue_to_qentry(pkt_hdr->dst_queue);
   ret = queue_enq(dst_queue, 
buf_hdr, 0);
--

-Matias

From: Elo, Matias (Nokia - FI/Espoo)
Sent: Wednesday, June 08, 2016 10:12 AM
To: 'Oriol Arcas' ; Bala Manoharan 

Cc: LNG ODP Mailman List 
Subject: RE: [lng-odp] [PATCH v2 4/4] linux-gen: pktio: don't allocate new 
packets in classifier

Hi Oriol,

Sounds like there is a bug in the raw socket implementation. I’ll take a look.

As a side note,  there is currently no classifier performance test as far as I 
can see. Performance test would be useful to prevent issues like this in the 
future. Additionally, classifier validation tests are only run with the 
loopback interface type.

-Matias

From: Oriol Arcas [mailto:or...@starflownetworks.com]
Sent: Tuesday, June 07, 2016 4:15 PM
To: Bala Manoharan mailto:bala.manoha...@linaro.org>>
Cc: Elo, Matias (Nokia - FI/Espoo) 
mailto:matias@nokia.com>>; LNG ODP Mailman List 
mailto:lng-odp@lists.linaro.org>>
Subject: Re: [lng-odp] [PATCH v2 4/4] linux-gen: pktio: don't allocate new 
packets in classifier

Hello all,
Sorry for joining the convesation late too. At Starflow we detected a 
regression at this exact commit. We use the RAW socket interface (socket_mmap 
PKTIO) and now the performance has dropped to kB/s. There is connectivity, but 
the behavior is erratic and the performance very low.
Since it is a deep modification, we have not tracked yet the exact problem, but 
it may be related to the previous comments.

--
Oriol Arcas
Software Engineer
Starflow Networks

On Tue, Jun 7, 2016 at 2:55 PM, Bala Manoharan 
mailto:bala.manoha...@linaro.org>> wrote:
Hi,

Just realised that I haven't sent these comment. Sorry for the late feedback.
Comments inline...

Regards,
Bala


On 2 June 2016 at 13:15, Matias Elo 
mailto:matias@nokia.com>> wrote:
> Don't allocate new packets inside of the internal
> classifier helpers _odp_packet_cls_enq() and
> _odp_packet_classifier(). Instead, save destination queue to
> the parsed packet header and return correct packet pool.
> This enables zero-copy packet classification.

The _odp_packet_cls_enq functions is a performance path where the
packet has not yet been formed cases like receiving the packet from
different pktios i.e socket, netmap, etc and the packet gets created
for the first time from the pool by the classification engine based on
the destination CoS.
The function _odp_packet_classifier is called in the case where the
packet has been pre-allocated and is being classified this is not an
actual performance path and is only called by the loopback interface.
Also in this scenario a new packet gets allocated only if the final
pool is different from the pool in which the packet was initially
allocated.

By combining these different functionality in the function
cls_classify_packet(), when the function gets called for pre-allocated
packets you have passed the address of a packet by calling
odp_packet_data() there is an issue here since if the packet is
segmented then the cls_classify_packet() function can not handle the
scenario, whereas if you pass the entire packet then it is possible
for the function to handle the same.

>
> Added new internal pktio helper pktin_recv_buf(), which
> enqueues packets to classifier queues if necessary.
>
> All pktio types use now a common cls_classify_packet()
> helper function.
>
> Signed-off-by: Matias Elo mailto:matias@nokia.com>>
> Reviewed-and-tested-by: Bill Fischofer 
> mailto:bill.fischo...@linaro.org>>
> ---
> V2:
> - Fixed possible hang in pkt_mmap_v2_

Re: [lng-odp] [PATCH 2/2] validation: packet: ensure user area is copied correctly

[Zoltan Kiss]

Reviewed-by: Zoltan Kiss 

On 07/06/16 15:53, Bill Fischofer wrote:

As part of resolution of Bug https://bugs.linaro.org/show_bug.cgi?id=2310
make sure that odp_packet_copy() handles user area copies correctly. The
copy should fail if the target pool's user area size is not large enough
to contain the source user area.

Signed-off-by: Bill Fischofer 
---
  test/validation/packet/packet.c | 69 +++--
  1 file changed, 67 insertions(+), 2 deletions(-)

diff --git a/test/validation/packet/packet.c b/test/validation/packet/packet.c
index 6770339..095f760 100644
--- a/test/validation/packet/packet.c
+++ b/test/validation/packet/packet.c
@@ -14,7 +14,7 @@
  /* Reserve some tailroom for tests */
  #define PACKET_TAILROOM_RESERVE  4

-static odp_pool_t packet_pool;
+static odp_pool_t packet_pool, packet_pool_no_uarea, packet_pool_double_uarea;
  static uint32_t packet_len;

  static uint32_t segmented_packet_len;
@@ -65,6 +65,24 @@ int packet_suite_init(void)
if (packet_pool == ODP_POOL_INVALID)
return -1;

+   params.pkt.uarea_size = 0;
+   packet_pool_no_uarea = odp_pool_create("packet_pool_no_uarea",
+  ¶ms);
+   if (packet_pool_no_uarea == ODP_POOL_INVALID) {
+   odp_pool_destroy(packet_pool);
+   return -1;
+   }
+
+   params.pkt.uarea_size = 2 * sizeof(struct udata_struct);
+   packet_pool_double_uarea = odp_pool_create("packet_pool_double_uarea",
+  ¶ms);
+
+   if (packet_pool_double_uarea == ODP_POOL_INVALID) {
+   odp_pool_destroy(packet_pool_no_uarea);
+   odp_pool_destroy(packet_pool);
+   return -1;
+   }
+
test_packet = odp_packet_alloc(packet_pool, packet_len);

for (i = 0; i < packet_len; i++) {
@@ -113,8 +131,12 @@ int packet_suite_term(void)
  {
odp_packet_free(test_packet);
odp_packet_free(segmented_test_packet);
-   if (odp_pool_destroy(packet_pool) != 0)
+
+   if (odp_pool_destroy(packet_pool_double_uarea) != 0 ||
+   odp_pool_destroy(packet_pool_no_uarea) != 0 ||
+   odp_pool_destroy(packet_pool) != 0)
return -1;
+
return 0;
  }

@@ -913,6 +935,20 @@ static void _packet_compare_data(odp_packet_t pkt1, 
odp_packet_t pkt2)
}
  }

+static void _packet_compare_udata(odp_packet_t pkt1, odp_packet_t pkt2)
+{
+   uint32_t usize1 = odp_packet_user_area_size(pkt1);
+   uint32_t usize2 = odp_packet_user_area_size(pkt2);
+
+   void *uaddr1 = odp_packet_user_area(pkt1);
+   void *uaddr2 = odp_packet_user_area(pkt2);
+
+   uint32_t cmplen = usize1 <= usize2 ? usize1 : usize2;
+
+   if (cmplen)
+   CU_ASSERT(!memcmp(uaddr1, uaddr2, cmplen));
+}
+
  static void _packet_compare_offset(odp_packet_t pkt1, uint32_t off1,
   odp_packet_t pkt2, uint32_t off2,
   uint32_t len)
@@ -948,6 +984,11 @@ void packet_test_copy(void)
uint32_t i, plen, seg_len, src_offset, dst_offset;
void *pkt_data;

+   pkt = odp_packet_copy(test_packet, packet_pool_no_uarea);
+   CU_ASSERT(pkt == ODP_PACKET_INVALID);
+   if (pkt != ODP_PACKET_INVALID)
+   odp_packet_free(pkt);
+
pkt = odp_packet_copy(test_packet, odp_packet_pool(test_packet));
CU_ASSERT_FATAL(pkt != ODP_PACKET_INVALID);
_packet_compare_data(pkt, test_packet);
@@ -962,6 +1003,30 @@ void packet_test_copy(void)

_packet_compare_inflags(pkt, pkt_copy);
_packet_compare_data(pkt, pkt_copy);
+   CU_ASSERT(odp_packet_user_area_size(pkt) ==
+ odp_packet_user_area_size(test_packet));
+   _packet_compare_udata(pkt, pkt_copy);
+   odp_packet_free(pkt_copy);
+   odp_packet_free(pkt);
+
+   pkt = odp_packet_copy(test_packet, packet_pool_double_uarea);
+   CU_ASSERT_FATAL(pkt != ODP_PACKET_INVALID);
+   _packet_compare_data(pkt, test_packet);
+   pool = odp_packet_pool(pkt);
+   CU_ASSERT_FATAL(pool != ODP_POOL_INVALID);
+   pkt_copy = odp_packet_copy(pkt, pool);
+   CU_ASSERT_FATAL(pkt_copy != ODP_PACKET_INVALID);
+
+   CU_ASSERT(pkt != pkt_copy);
+   CU_ASSERT(odp_packet_data(pkt) != odp_packet_data(pkt_copy));
+   CU_ASSERT(odp_packet_len(pkt) == odp_packet_len(pkt_copy));
+
+   _packet_compare_inflags(pkt, pkt_copy);
+   _packet_compare_data(pkt, pkt_copy);
+   CU_ASSERT(odp_packet_user_area_size(pkt) ==
+ 2 * odp_packet_user_area_size(test_packet));
+   _packet_compare_udata(pkt, pkt_copy);
+   _packet_compare_udata(pkt, test_packet);
odp_packet_free(pkt_copy);

/* Now test copy_part */


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

Re: [lng-odp] [PATCH 1/2] linux-generic: packet: copy user area as part of odp_packet_copy()

[Zoltan Kiss]

Hi,

On 07/06/16 15:53, Bill Fischofer wrote:

Resolve Bug https://bugs.linaro.org/show_bug.cgi?id=2310 by copying the
user area as part of odp_packet_copy(). The copy fails if the user area
size of the destination pool is not large enough to hold the source packet
user area.

Signed-off-by: Bill Fischofer 
---
  platform/linux-generic/include/odp_packet_internal.h |  2 +-
  platform/linux-generic/odp_packet.c  | 18 --
  2 files changed, 5 insertions(+), 15 deletions(-)

diff --git a/platform/linux-generic/include/odp_packet_internal.h 
b/platform/linux-generic/include/odp_packet_internal.h
index d5ace12..5c74d97 100644
--- a/platform/linux-generic/include/odp_packet_internal.h
+++ b/platform/linux-generic/include/odp_packet_internal.h
@@ -294,7 +294,7 @@ static inline int 
packet_parse_not_complete(odp_packet_hdr_t *pkt_hdr)
  }

  /* Forward declarations */
-void _odp_packet_copy_md_to_packet(odp_packet_t srcpkt, odp_packet_t dstpkt);
+int _odp_packet_copy_md_to_packet(odp_packet_t srcpkt, odp_packet_t dstpkt);

  odp_packet_t packet_alloc(odp_pool_t pool_hdl, uint32_t len, int parse);

diff --git a/platform/linux-generic/odp_packet.c 
b/platform/linux-generic/odp_packet.c
index 2868736..f92c257 100644
--- a/platform/linux-generic/odp_packet.c
+++ b/platform/linux-generic/odp_packet.c
@@ -756,22 +756,11 @@ odp_packet_t odp_packet_copy(odp_packet_t pkt, odp_pool_t 
pool)
  {
odp_packet_hdr_t *srchdr = odp_packet_hdr(pkt);
uint32_t pktlen = srchdr->frame_len;
-   uint32_t meta_offset = ODP_FIELD_SIZEOF(odp_packet_hdr_t, buf_hdr);
odp_packet_t newpkt = odp_packet_alloc(pool, pktlen);

if (newpkt != ODP_PACKET_INVALID) {
-   odp_packet_hdr_t *newhdr = odp_packet_hdr(newpkt);
-   uint8_t *newstart, *srcstart;
-
-   /* Must copy metadata first, followed by packet data */
-   newstart = (uint8_t *)newhdr + meta_offset;
-   srcstart = (uint8_t *)srchdr + meta_offset;
-
-   memcpy(newstart, srcstart,
-  sizeof(odp_packet_hdr_t) - meta_offset);
-
-   if (odp_packet_copy_from_pkt(newpkt, 0, pkt, 0,
-pktlen) != 0) {
+   if (_odp_packet_copy_md_to_packet(pkt, newpkt) ||
+   odp_packet_copy_from_pkt(newpkt, 0, pkt, 0, pktlen)) {
odp_packet_free(newpkt);
newpkt = ODP_PACKET_INVALID;
}
@@ -966,7 +955,7 @@ int odp_packet_is_valid(odp_packet_t pkt)
   *
   */

-void _odp_packet_copy_md_to_packet(odp_packet_t srcpkt, odp_packet_t dstpkt)
+int _odp_packet_copy_md_to_packet(odp_packet_t srcpkt, odp_packet_t dstpkt)


There are other callers of this function, we should either check the 
return value there or make a comment at least about why it's not 
possible for an error to happen.



  {
odp_packet_hdr_t *srchdr = odp_packet_hdr(srcpkt);
odp_packet_hdr_t *dsthdr = odp_packet_hdr(dstpkt);
@@ -986,6 +975,7 @@ void _odp_packet_copy_md_to_packet(odp_packet_t srcpkt, 
odp_packet_t dstpkt)
odp_atomic_load_u32(
&srchdr->buf_hdr.ref_count));
copy_packet_parser_metadata(srchdr, dsthdr);
+   return dsthdr->buf_hdr.uarea_size < srchdr->buf_hdr.uarea_size;


I think we should do this check before we do any copy


  }

  /**


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

Re: [lng-odp] thread/shmem discussion summary V4

[Christophe Milard]

Thanks Jerin,

I have merged your comments into the doc: there were many points you
did not comments (not sure whether the yes below a group applied to
the group of statement). You can review the doc to make sure I did not
corrupt your feelings :-)

once again, Thanks

Christophe.

On 8 June 2016 at 14:11, Jerin Jacob  wrote:
> On Fri, Jun 03, 2016 at 11:15:43AM +0200, Christophe Milard wrote:
>> since V3: Update following Bill's comments
>> since V2: Update following Barry and Bill's comments
>> since V1: Update following arch call 31 may 2016
>>
>> This is a tentative to sum up the discussions around the thread/process
>> that have been happening these last weeks.
>> Sorry for the formalism of this mail, but it seems we need accuracy
>> here...
>>
>> This summary is organized as follows:
>>
>> It is a set of statements, each of them expecting a separate answer
>> from you. When no specific ODP version is specified, the statement
>> regards the"ultimate" goal (i.e what we want eventually to achieve).
>> Each statement is prefixed with:
>>   - a statement number for further reference (e.g. S1)
>>   - a status word (one of 'agreed' or 'open', or 'closed').
>> Agreed statements expect a yes/no answers: 'yes' meaning that you
>> acknowledge that this is your understanding of the agreement and will
>> not nack an implementation based on this statement. You can comment
>> after a yes, but your comment will not block any implementation based
>> on the agreed statement. A 'no' implies that the statement does not
>> reflect your understanding of the agreement, or you refuse the
>> proposal.
>> Any 'no' received on an 'agreed' statement will push it back as 'open'.
>> Open statements are fully open for further discussion.
>>
>> S1  -agreed: an ODP thread is an OS/platform concurrent execution
>> environment object (as opposed to an ODP objects). No more specific
>> definition is given by the ODP API itself.
>>
>> Barry: YES
>> Bill: Yes
>
> Jerin: Yes
>
>>
>> ---
>>
>> S2  -agreed: Each ODP implementation must tell what is allowed to be
>> used as ODP thread for that specific implementation: a linux-based
>> implementation, for instance, will have to state whether odp threads
>> can be linux pthread, linux processes, or both, or any other type of
>> concurrent execution environment. ODP implementations can put any
>> restriction they wish on what an ODP thread is allowed to be. This
>> should be documented in the ODP implementation documentation.
>>
>> Barry: YES
>> Bill: Yes
>
> Jerin: Yes, Apart from Linux process or thread based implementation, an
> implementation may select to have bare-metal execution environment(i.e
> without any OS)
>
>>
>> ---
>>
>> S3  -agreed: in the linux generic ODP implementation a odpthread will be
>> either:
>> * a linux process descendant (or same as) the odp instantiation
>> process.
>> * a pthread 'member' of a linux process descendant (or same
>> as) the odp instantiation process.
>>
>> Barry: YES
>> Bill: Yes
>>
>> ---
>>
>> S4  -agreed: For monarch, the linux generic ODP implementation only
>> supports odp thread as pthread member of the instantiation process.
>>
>> Barry: YES
>> Bill: Yes
>>
>> ---
>>
>> S5  -agreed: whether multiple instances of ODP can be run on the same
>> machine is left as a implementation decision. The ODP implementation
>> document should state what is supported and any restriction is allowed.
>>
>> Barry: YES
>> Bill: Yes
>
> Jerin: Yes
>
>>
>> ---
>>
>> S6  -agreed: The l-g odp implementation will support multiple odp
>> instances whose instantiation processes are different and not
>> ancestor/descendant of each others. Different instances of ODP will,
>> of course, be restricted in sharing common OS ressources (The total
>> amount of memory available for each ODP instances may decrease as the
>> number of instances increases, the access to network interfaces will
>> probably be granted to the first instance grabbing the interface and
>> denied to others... some other rule may apply when sharing other
>> common ODP ressources.)
>>
>> Bill: Yes
>>
>> ---
>>
>> S7  -agreed: the l-g odp implementation will not support multiple ODP
>> instances initiated from the same linux process (calling
>> odp_init_global() multiple times).
>> As an illustration, This means that a single process P is not allowed
>> to execute the following calls (in any order)
>> instance1 = odp_init_global()
>> instance2 = odp_init_global()
>> pthread_create (and, in that thread, run odp_local_init(instance1) )
>> pthread_create (and, in that thread, run odp_local_init(instance2) )
>>
>> Bill: Yes
>>
>> ---
>>
>> S8  -agreed: the l-g odp implementation will not support multiple ODP
>> instances initiated from related linux processes (descendant/ancestor
>> of each other), hence enabling ODP 'sub-instance'? As a

Re: [lng-odp] thread/shmem discussion summary V4

[Jerin Jacob]

On Fri, Jun 03, 2016 at 11:15:43AM +0200, Christophe Milard wrote:
> since V3: Update following Bill's comments
> since V2: Update following Barry and Bill's comments
> since V1: Update following arch call 31 may 2016
> 
> This is a tentative to sum up the discussions around the thread/process
> that have been happening these last weeks.
> Sorry for the formalism of this mail, but it seems we need accuracy
> here...
> 
> This summary is organized as follows:
> 
> It is a set of statements, each of them expecting a separate answer
> from you. When no specific ODP version is specified, the statement
> regards the"ultimate" goal (i.e what we want eventually to achieve).
> Each statement is prefixed with:
>   - a statement number for further reference (e.g. S1)
>   - a status word (one of 'agreed' or 'open', or 'closed').
> Agreed statements expect a yes/no answers: 'yes' meaning that you
> acknowledge that this is your understanding of the agreement and will
> not nack an implementation based on this statement. You can comment
> after a yes, but your comment will not block any implementation based
> on the agreed statement. A 'no' implies that the statement does not
> reflect your understanding of the agreement, or you refuse the
> proposal.
> Any 'no' received on an 'agreed' statement will push it back as 'open'.
> Open statements are fully open for further discussion.
> 
> S1  -agreed: an ODP thread is an OS/platform concurrent execution
> environment object (as opposed to an ODP objects). No more specific
> definition is given by the ODP API itself.
> 
> Barry: YES
> Bill: Yes

Jerin: Yes

> 
> ---
> 
> S2  -agreed: Each ODP implementation must tell what is allowed to be
> used as ODP thread for that specific implementation: a linux-based
> implementation, for instance, will have to state whether odp threads
> can be linux pthread, linux processes, or both, or any other type of
> concurrent execution environment. ODP implementations can put any
> restriction they wish on what an ODP thread is allowed to be. This
> should be documented in the ODP implementation documentation.
> 
> Barry: YES
> Bill: Yes

Jerin: Yes, Apart from Linux process or thread based implementation, an
implementation may select to have bare-metal execution environment(i.e
without any OS)

> 
> ---
> 
> S3  -agreed: in the linux generic ODP implementation a odpthread will be
> either:
> * a linux process descendant (or same as) the odp instantiation
> process.
> * a pthread 'member' of a linux process descendant (or same
> as) the odp instantiation process.
> 
> Barry: YES
> Bill: Yes
> 
> ---
> 
> S4  -agreed: For monarch, the linux generic ODP implementation only
> supports odp thread as pthread member of the instantiation process.
> 
> Barry: YES
> Bill: Yes
> 
> ---
> 
> S5  -agreed: whether multiple instances of ODP can be run on the same
> machine is left as a implementation decision. The ODP implementation
> document should state what is supported and any restriction is allowed.
> 
> Barry: YES
> Bill: Yes

Jerin: Yes

> 
> ---
> 
> S6  -agreed: The l-g odp implementation will support multiple odp
> instances whose instantiation processes are different and not
> ancestor/descendant of each others. Different instances of ODP will,
> of course, be restricted in sharing common OS ressources (The total
> amount of memory available for each ODP instances may decrease as the
> number of instances increases, the access to network interfaces will
> probably be granted to the first instance grabbing the interface and
> denied to others... some other rule may apply when sharing other
> common ODP ressources.)
> 
> Bill: Yes
> 
> ---
> 
> S7  -agreed: the l-g odp implementation will not support multiple ODP
> instances initiated from the same linux process (calling
> odp_init_global() multiple times).
> As an illustration, This means that a single process P is not allowed
> to execute the following calls (in any order)
> instance1 = odp_init_global()
> instance2 = odp_init_global()
> pthread_create (and, in that thread, run odp_local_init(instance1) )
> pthread_create (and, in that thread, run odp_local_init(instance2) )
> 
> Bill: Yes
> 
> ---
> 
> S8  -agreed: the l-g odp implementation will not support multiple ODP
> instances initiated from related linux processes (descendant/ancestor
> of each other), hence enabling ODP 'sub-instance'? As an illustration,
> this means that the following is not supported:
> instance1 = odp_init_global()
> pthread_create (and, in that thread, run odp_local_init(instance1) )
> if (fork()==0) {
> instance2 = odp_init_global()
> pthread_create (and, in that thread, run odp_local_init(instance2) )
> }
> 
> Bill: Yes
> 
> 
> 
> S9  -agreed: the odp instance passed as parameter to odp_local_init()
> must alwa

[lng-odp] [RFCv2 API-NEXT] Adding API for Multi crypto operation support.

[Nikhil Agarwal]

Signed-off-by: Nikhil Agarwal 
---
 include/odp/api/spec/crypto.h | 29 +++--
 1 file changed, 27 insertions(+), 2 deletions(-)

diff --git a/include/odp/api/spec/crypto.h b/include/odp/api/spec/crypto.h
index d8123e9..7b4424d 100644
--- a/include/odp/api/spec/crypto.h
+++ b/include/odp/api/spec/crypto.h
@@ -214,7 +214,6 @@ typedef struct odp_crypto_session_params {
  * @todo Clarify who zero's ICV and how this relates to "hash_result_offset"
  */
 typedef struct odp_crypto_op_params {
-   odp_crypto_session_t session;   /**< Session handle from creation */
void *ctx;  /**< User context */
odp_packet_t pkt;   /**< Input packet buffer */
odp_packet_t out_pkt;   /**< Output packet buffer */
@@ -406,6 +405,7 @@ void odp_crypto_compl_free(odp_crypto_compl_t 
completion_event);
  * If "posted" returns TRUE the result will be delivered via the completion
  * queue specified when the session was created.
  *
+ * @param session   Session handle
  * @param paramsOperation parameters
  * @param postedPointer to return posted, TRUE for async operation
  * @param resultResults of operation (when posted returns FALSE)
@@ -413,10 +413,35 @@ void odp_crypto_compl_free(odp_crypto_compl_t 
completion_event);
  * @retval 0 on success
  * @retval <0 on failure
  */
-int odp_crypto_operation(odp_crypto_op_params_t *params,
+int odp_crypto_operation(odp_crypto_session_t session,
+odp_crypto_op_params_t *params,
 odp_bool_t *posted,
 odp_crypto_op_result_t *result);
 
+
+/**
+ * Crypto packet burst operation
+ *
+ * Performs the cryptographic operations specified during session creation
+ * on the list of packets. Burst operation can only be performed 
asynchronously,
+ * and the result will be delivered via the completion queue specified when the
+ * session was created.
+ *
+ * @param session   Session handle
+ * @param paramsArray of Operation parameters
+ * @param postedPointer to return posted, TRUE for async operation
+ * @param resultResults of operation (when posted returns FALSE)
+ * @param num   Number of operations reuested
+ *
+ * @return Number of operations actually enqueued/completed (0 ... num)
+ * @retval <0 on failure
+ */
+int odp_crypto_operation_multi(odp_crypto_session_t session,
+  odp_crypto_op_params_t *params[],
+  odp_bool_t *posted,
+  odp_crypto_op_result_t *result[],
+  int num);
+
 /**
  * Crypto per packet operation query result from completion event
  *
-- 
2.8.2

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

Re: [lng-odp] [PATCH] example: timer simple

[Bill Fischofer]

On Wed, Jun 8, 2016 at 1:57 AM, Maxim Uvarov 
wrote:

> On 06/08/16 06:07, Bill Fischofer wrote:
>
>>
>>
>> On Tue, Jun 7, 2016 at 3:46 PM, Maxim Uvarov > > wrote:
>>
>> Example for simple timer usage.
>> https://bugs.linaro.org/show_bug.cgi?id=2090
>>
>> Signed-off-by: Maxim Uvarov > >
>>
>> ---
>>  example/timer/.gitignore |   1 +
>>  example/timer/Makefile.am|  12 ++-
>>  example/timer/odp_timer_simple.c | 167
>> +++
>>  3 files changed, 177 insertions(+), 3 deletions(-)
>>  create mode 100644 example/timer/odp_timer_simple.c
>>
>> diff --git a/example/timer/.gitignore b/example/timer/.gitignore
>> index eb59265..f53a0df 100644
>> --- a/example/timer/.gitignore
>> +++ b/example/timer/.gitignore
>> @@ -1 +1,2 @@
>>  odp_timer_test
>> +odp_timer_simple
>> diff --git a/example/timer/Makefile.am b/example/timer/Makefile.am
>> index fcb67a9..1c733d3 100644
>> --- a/example/timer/Makefile.am
>> +++ b/example/timer/Makefile.am
>> @@ -1,10 +1,16 @@
>>  include $(top_srcdir)/example/Makefile.inc
>>
>> -bin_PROGRAMS = odp_timer_test$(EXEEXT)
>> +bin_PROGRAMS = odp_timer_test$(EXEEXT) \
>> +   odp_timer_simple$(EXEEXT)
>>  odp_timer_test_LDFLAGS = $(AM_LDFLAGS) -static
>>  odp_timer_test_CFLAGS = $(AM_CFLAGS) -I${top_srcdir}/example
>> +dist_odp_timer_test_SOURCES = odp_timer_test.c
>> +
>> +odp_timer_simple_LDFLAGS = $(AM_LDFLAGS) -static
>> +odp_timer_simple_CFLAGS = $(AM_CFLAGS) -I${top_srcdir}/example
>> +dist_odp_timer_simple_SOURCES = odp_timer_simple.c
>> +
>> +TESTS = odp_timer_simple
>>
>>  noinst_HEADERS = \
>>   $(top_srcdir)/example/example_debug.h
>> -
>> -dist_odp_timer_test_SOURCES = odp_timer_test.c
>> diff --git a/example/timer/odp_timer_simple.c
>> b/example/timer/odp_timer_simple.c
>> new file mode 100644
>> index 000..d4917c3
>> --- /dev/null
>> +++ b/example/timer/odp_timer_simple.c
>> @@ -0,0 +1,167 @@
>> +/* Copyright (c) 2016, Linaro Limited
>> + * All rights reserved.
>> + *
>> + * SPDX-License-Identifier: BSD-3-Clause
>> + */
>> +/**
>> + * @file
>> + *
>> + * @example odp_timer_simple.c  ODP simple example to schedule timer
>> + * action for 1 second.
>> + */
>> +
>> +#include 
>> +#include 
>> +#include 
>> +
>> +/* ODP main header */
>> +#include 
>> +
>> +int main(int argc ODP_UNUSED, char *argv[] ODP_UNUSED)
>> +{
>> +   odp_instance_t instance;
>> +   odp_pool_param_t params;
>> +   odp_timer_pool_param_t tparams;
>> +   odp_pool_t tmo_pool;
>> +   odp_timer_pool_t timer_pool;
>> +   odp_queue_param_t qparam;
>> +   odp_queue_t queue;
>> +   odp_event_t ev = ODP_EVENT_INVALID;
>> +   odp_timer_t tim;
>> +   uint64_t sched_tmo;
>> +   int i, rc;
>> +   uint64_t period;
>> +   uint64_t tick;
>> +   odp_timeout_t tmo;
>> +   int ret = 0;
>> +
>> +   /*
>> +* Init ODP app
>> +*/
>> +   if (odp_init_global(&instance, NULL, NULL))
>> +   goto err_global;
>> +
>> +   if (odp_init_local(instance, ODP_THREAD_CONTROL))
>> +   goto err_local;
>> +
>> +   /*
>> +* Create pool for timeouts
>> +*/
>> +   odp_pool_param_init(¶ms);
>> +   params.tmo.num   = 10;
>> +   params.type  = ODP_POOL_TIMEOUT;
>> +
>> +   tmo_pool = odp_pool_create("msg_pool", ¶ms);
>>
>>
>> Is "msg_pool" the best choice of name for a timeout pool?  "timeout_pool"
>> would seem to make more sense here.
>>
>
> agree, will change.
>
>
> +   if (tmo_pool == ODP_POOL_INVALID) {
>> +   ret += 1;
>> +   goto err_tp;
>> +   }
>> +
>> +   /*
>> +* Create pool of timeouts
>> +*/
>> +   tparams.res_ns = 10 * ODP_TIME_USEC_IN_NS;
>> +   tparams.min_tmo = 10 * ODP_TIME_USEC_IN_NS;
>> +   tparams.max_tmo = 1 * ODP_TIME_SEC_IN_NS;
>> +   tparams.num_timers = 1; /* One timer per worker */
>> +   tparams.priv = 0; /* Shared */
>> +   tparams.clk_src = ODP_CLOCK_CPU;
>> +   timer_pool = odp_timer_pool_create("timer_pool", &tparams);
>> +   if (timer_pool == ODP_TIMER_POOL_INVALID) {
>> +   ret += 1;
>> +   goto err;
>> +   }
>> +
>> +   /*
>> +* Create a queue for timer test
>> +*/
>> +   odp_queue_param_init(&qparam);
>> +   qparam.type= ODP_QUEUE_TYPE_SCHED;
>> +   q

Re: [lng-odp] packet copy questions

[Zoltan Kiss]

And what about this first question:

Should _odp_packet_copy_md_to_packet() copy timestamp and op_result?

On 07/06/16 13:03, Bill Fischofer wrote:

Sounds like this should be considered a bug. I'll open one to track it.

On Tue, Jun 7, 2016 at 2:18 AM, Savolainen, Petri (Nokia - FI/Espoo)
mailto:petri.savolai...@nokia.com>> wrote:

/**
  * Full copy of a packet
  *
  * Create a new copy of the packet. The new packet is exact copy of
the source
  * packet (incl. data and metadata). The pool must have been
created with
  * ODP_POOL_PACKET type.
  *

All metadata should be copied into the new packet. User controls
user_area sizes in dst and src pools and should take care that dst
pool has always at least as much user area as the src pool (when
packets will be copied between those pools). This is no different
from the actual packet data, everything will be copied as long as
the dst pool can store as large packets as the src pool.

The function just fails if copy cannot be performed (all packet data
or metadata cannot be stored into dst pool).

-Petri



From: Bill Fischofer [mailto:bill.fischo...@linaro.org
]
Sent: Monday, June 06, 2016 10:53 PM
To: Zoltan Kiss mailto:zoltan.k...@linaro.org>>
Cc: lng-odp mailto:lng-odp@lists.linaro.org>>; Savolainen, Petri (Nokia -
FI/Espoo) mailto:petri.savolai...@nokia.com>>
Subject: Re: packet copy questions

The user area is explicitly reserved for use by the application. As
a result any copying or other manipulation of it is the
application's responsibility. The exception to this is ODP APIs that
implicitly make a new copy of a source packet (e.g., in response to
odp_packet_add_data() reallocating the packet) since the
reallocation is transparent to the application.

For the specific case of odp_packet_copy(), the reason why there is
no implied copy of the user area is that odp_packet_copy() allows
copies to be made in another pool and each pool may independently
specify the size of any user area associated with packets allocated
from it. So it's up to the application to decide what is appropriate
in this case.

On Mon, Jun 6, 2016 at 2:35 PM, Zoltan Kiss mailto:zoltan.k...@linaro.org>> wrote:
Hi,

During the ODP-DPDK 1.10 upgrade I found these two things:

- Should _odp_packet_copy_md_to_packet() copy timestamp and op_result?
- odp_packet_copy() doesn't copy the user area. I think it would be
better if it just calls _odp_packet_copy_md_to_packet()

Regards,

Zoltan



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

Re: [lng-odp] thread/shmem discussion summary V4

[Christophe Milard]

OK. good that you agree (and please update the shared doc so it
becomes the central point of information).
There is something I like though, in your willingness to decouple the
function and the pinning...
Even if I am not sure this can be enforced by ODP at all time (as
already stated), there is definitively a point in helping the
application that with to do so. So please keep an eye on that!

Your opinion on S19 S20 and S21 would be very welcome as well... This
is the main hurting point

Christophe

On 8 June 2016 at 09:41, Yi He  wrote:
> Hi, thanks Christophe and happy to discuss with and learn from team in
> yesterday's ARCH call :)
>
> The question which triggers this kind of thinking is: how to use ODP as a
> framework to produce re-usable building blocks to compose "Network Function
> Instance" in runtime, since until runtime it will prepare resources for
> function to settle down, thus comes the thought of seperating function
> implementation and launching.
>
> I agree your point it seems upper layer consideration, I'll have some time
> to gain deeper understanding and knowledge on how upstair playings, thus I
> agree to the current S11, S12, S13, S14, S15, S16, S17 approach and we can
> revisit if really realized upper layer programming practise/model affects
> ODP's design in this aspect.
>
> Best Regards, Yi
>
> On 7 June 2016 at 16:47, Christophe Milard 
> wrote:
>>
>> On 7 June 2016 at 10:22, Yi He  wrote:
>> > Hi, team
>> >
>> > I send my thoughts on the ODP thread part:
>> >
>> > S1, S2, S3, S4
>> > Yi: Yes
>> > This set of statements defines ODP thread concept as a higher
>> > level abstraction of underlying concurrent execution context.
>> >
>> > S5, S6, S7, S8, S9, S10:
>> > Yi: Yes
>> > This set of statements add several constraints upon ODP
>> > instance concept.
>> >
>> > S11, S12, S13, S14, S15, S16, S17:
>> > Yi: Not very much
>> >
>> > Currently ODP application still mixes the Function Implementation
>> > and its Deployment, by this I mean in ODP application code, there
>> > is code to implement Function, there is also code to deploy the
>> > Function onto platform resources (CPU cores).
>> >
>> > I'd like to propose a programming practice/model as an attempt to
>> > decouple these two things, ODP application code only focuses on
>> > implementing Function, and leave it to a deployment script or
>> > launcher to take care the deployment with different resource spec
>> > (and sufficiency check also).
>>
>> Well, that goes straight against Barry's will that apps should pin
>> their tasks  on cpus...
>> Please join the public call today: if Barry is there you can discuss
>> this: I am happy to hear other voices in this discussion
>>
>> >
>> > /* Use Case: a upper layer orchestrator, say Daylight is deploying
>> > * an ODP application (can be a VNF instance in my opinion) onto
>> > * some platform:
>> > */
>> >
>> > /* The application can accept command line options */
>> > --list-launchable
>> > list all algorithm functions that need to be arranged
>> > into an execution unit.
>> >
>> > --preparing-threads
>> > control<1,2,3>@cpu<1>, worker<1,2,3,4>@cpu<2,3,4,5>
>> > I'm not sure if the control/worker distinguish is needed
>> > anymore, DPDK has similar concept lcore.
>> >
>> >   --wait-launching-signal
>> >   main process can pause after preparing above threads but
>> > before launching algorithm functions, here orchestrator can
>> > further fine-tuning the threads by scripting (cgroups, etc).
>> > CPU, disk/net IO, memory quotas, interrupt bindings, etc.
>> >
>> > --launching
>> >   main@control<1>,algorithm_one@control<2>,
>> > algorithm_two@worker<1,2,3,4>...
>> >
>> > In source code, the only thing ODP library and application need to do
>> > related to deployment is to declare launchable algorithm functions by
>> > adding them into special text section:
>> >
>> > int main(...) __attribute__((section(".launchable")));
>> > int algorithm_one(void *) __attribute__((section(".launchable")));
>>
>> interresting ... Does it need to be part of ODP, though? Cannot the
>> ODP api provide means of pinning, and the apps that wish it can
>> provide the options you mentioned to do it: i.e. the application that
>> wants would pin according to its command line interface and the
>> application that can't/does not want would pin manually...
>> I mean one could also imagine cases where the app needs to pin: if
>> specific HW exists connected to some cpus, or if serialisation exists
>> (e.g. something like: pin tasks 1,2,and 3 to cpu 1,2,3, and then, when
>> this work is done (e.g. after a barrier joining task 1,2,3) pin taks
>> 4,5,6 on cpu 1,2,3 again.) There could be theoriticaly complex such
>> dependency graphs where all cpus cannot be pinned from the beginning,
>> and where your approach seem too restrictive.
>>
>> But I am glad to hear your voice on the arch call :-)
>>
>> Christophe.
>> >
>> > Best Regards, Yi
>> >
>> >
>> > On 4 June 2016 at 05:56, Bill Fisch

Re: [lng-odp] [PATCH v2 4/4] linux-gen: pktio: don't allocate new packets in classifier

[Bala Manoharan]

Regards,
Bala


On 8 June 2016 at 12:24, Elo, Matias (Nokia - FI/Espoo)
 wrote:
>
>
>> On 2 June 2016 at 13:15, Matias Elo  wrote:
>> > Don't allocate new packets inside of the internal
>> > classifier helpers _odp_packet_cls_enq() and
>> > _odp_packet_classifier(). Instead, save destination queue to
>> > the parsed packet header and return correct packet pool.
>> > This enables zero-copy packet classification.
>>
>> The _odp_packet_cls_enq functions is a performance path where the
>> packet has not yet been formed cases like receiving the packet from
>> different pktios i.e socket, netmap, etc and the packet gets created
>> for the first time from the pool by the classification engine based on
>> the destination CoS.
>
> The enable zero-copy pktio packets cannot be created inside of the classify 
> function.

Not sure what is the issue here. In the previous implementation the
packets are received as byte array from socket receive function and
the classification engine in run on this byte stream to find the pool
where the packet needs to be allocated and then the packet is created
from the pool based on the CoS.
>
>> The function _odp_packet_classifier is called in the case where the
>> packet has been pre-allocated and is being classified this is not an
>> actual performance path and is only called by the loopback interface.
>> Also in this scenario a new packet gets allocated only if the final
>> pool is different from the pool in which the packet was initially
>> allocated.
>>
>> By combining these different functionality in the function
>> cls_classify_packet(), when the function gets called for pre-allocated
>> packets you have passed the address of a packet by calling
>> odp_packet_data() there is an issue here since if the packet is
>> segmented then the cls_classify_packet() function can not handle the
>> scenario, whereas if you pass the entire packet then it is possible
>> for the function to handle the same.
>
> This is clearly an issue. I'll create a fix for this.

IMO Since the path where the packet is received from sockets and
netmap etc is a performance code, any modularization should not affect
this path.

>
>>
>> >
>> > Added new internal pktio helper pktin_recv_buf(), which
>> > enqueues packets to classifier queues if necessary.
>> >
>> > All pktio types use now a common cls_classify_packet()
>> > helper function.
>> >
>> > Signed-off-by: Matias Elo 
>> > Reviewed-and-tested-by: Bill Fischofer 
>> > ---
>> > V2:
>> > - Fixed possible hang in pkt_mmap_v2_rx() receive loop (Maxim)
>> >
>> >  .../include/odp_classification_internal.h  |  21 +---
>> >  .../linux-generic/include/odp_packet_internal.h|   5 +
>> >  .../linux-generic/include/odp_packet_io_internal.h |   3 -
>> >  platform/linux-generic/odp_classification.c| 112 
>> > ++---
>> >  platform/linux-generic/odp_packet.c|   5 -
>> >  platform/linux-generic/odp_packet_io.c |  77 --
>> >  platform/linux-generic/pktio/dpdk.c|  55 +-
>> >  platform/linux-generic/pktio/loop.c|  86 
>> >  platform/linux-generic/pktio/netmap.c  |  45 -
>> >  platform/linux-generic/pktio/pktio_common.c|  67 
>> >  platform/linux-generic/pktio/socket.c  |  29 --
>> >  platform/linux-generic/pktio/socket_mmap.c |  59 ++-
>> >  12 files changed, 253 insertions(+), 311 deletions(-)
>> >
>> > diff --git a/platform/linux-generic/include/odp_classification_internal.h
>> b/platform/linux-generic/include/odp_classification_internal.h
>> > index 3a88462..d6d6904 100644
>> > --- a/platform/linux-generic/include/odp_classification_internal.h
>> > +++ b/platform/linux-generic/include/odp_classification_internal.h
>> > @@ -30,21 +30,6 @@ extern "C" {
>> >
>> >  /**
>> >  @internal
>> > -Select a CoS for the given Packet based on pktio
>> > -
>> > -This function will call all the PMRs associated with a pktio for
>> > -a given packet and will return the matched COS object.
>> > -This function will check PMR, L2 and L3 QoS COS object associated
>> > -with the PKTIO interface.
>> > -
>> > -Returns the default cos if the packet does not match any PMR
>> > -Returns the error_cos if the packet has an error
>> > -**/
>> > -cos_t *pktio_select_cos(pktio_entry_t *pktio, const uint8_t *pkt_addr,
>> > -   odp_packet_hdr_t *pkt_hdr);
>> > -
>> > -/**
>> > -@internal
>> >  match_qos_cos
>> >
>> >  Select a CoS for the given Packet based on QoS values
>> > @@ -60,10 +45,10 @@ Packet Classifier
>> >
>> >  Start function for Packet Classifier
>> >  This function calls Classifier module internal functions for a given 
>> > packet and
>> > -enqueues the packet to specific Queue based on PMR and CoS selected.
>> > -The packet is allocated from the pool associated with the CoS
>> > +selects destination queue and packet pool based on selected PMR and CoS.
>> >  **/
>> > -int 

Re: [lng-odp] thread/shmem discussion summary V4

[Yi He]

Hi, thanks Christophe and happy to discuss with and learn from team in
yesterday's ARCH call :)

The question which triggers this kind of thinking is: how to use ODP as a
framework to produce re-usable building blocks to compose "Network Function
Instance" in runtime, since until runtime it will prepare resources for
function to settle down, thus comes the thought of seperating function
implementation and launching.

I agree your point it seems upper layer consideration, I'll have some time
to gain deeper understanding and knowledge on how upstair playings, thus I
agree to the current S11, S12, S13, S14, S15, S16, S17 approach and we can
revisit if really realized upper layer programming practise/model affects
ODP's design in this aspect.

Best Regards, Yi

On 7 June 2016 at 16:47, Christophe Milard 
wrote:

> On 7 June 2016 at 10:22, Yi He  wrote:
> > Hi, team
> >
> > I send my thoughts on the ODP thread part:
> >
> > S1, S2, S3, S4
> > Yi: Yes
> > This set of statements defines ODP thread concept as a higher
> > level abstraction of underlying concurrent execution context.
> >
> > S5, S6, S7, S8, S9, S10:
> > Yi: Yes
> > This set of statements add several constraints upon ODP
> > instance concept.
> >
> > S11, S12, S13, S14, S15, S16, S17:
> > Yi: Not very much
> >
> > Currently ODP application still mixes the Function Implementation
> > and its Deployment, by this I mean in ODP application code, there
> > is code to implement Function, there is also code to deploy the
> > Function onto platform resources (CPU cores).
> >
> > I'd like to propose a programming practice/model as an attempt to
> > decouple these two things, ODP application code only focuses on
> > implementing Function, and leave it to a deployment script or
> > launcher to take care the deployment with different resource spec
> > (and sufficiency check also).
>
> Well, that goes straight against Barry's will that apps should pin
> their tasks  on cpus...
> Please join the public call today: if Barry is there you can discuss
> this: I am happy to hear other voices in this discussion
>
> >
> > /* Use Case: a upper layer orchestrator, say Daylight is deploying
> > * an ODP application (can be a VNF instance in my opinion) onto
> > * some platform:
> > */
> >
> > /* The application can accept command line options */
> > --list-launchable
> > list all algorithm functions that need to be arranged
> > into an execution unit.
> >
> > --preparing-threads
> > control<1,2,3>@cpu<1>, worker<1,2,3,4>@cpu<2,3,4,5>
> > I'm not sure if the control/worker distinguish is needed
> > anymore, DPDK has similar concept lcore.
> >
> >   --wait-launching-signal
> >   main process can pause after preparing above threads but
> > before launching algorithm functions, here orchestrator can
> > further fine-tuning the threads by scripting (cgroups, etc).
> > CPU, disk/net IO, memory quotas, interrupt bindings, etc.
> >
> > --launching
> >   main@control<1>,algorithm_one@control<2>,
> > algorithm_two@worker<1,2,3,4>...
> >
> > In source code, the only thing ODP library and application need to do
> > related to deployment is to declare launchable algorithm functions by
> > adding them into special text section:
> >
> > int main(...) __attribute__((section(".launchable")));
> > int algorithm_one(void *) __attribute__((section(".launchable")));
>
> interresting ... Does it need to be part of ODP, though? Cannot the
> ODP api provide means of pinning, and the apps that wish it can
> provide the options you mentioned to do it: i.e. the application that
> wants would pin according to its command line interface and the
> application that can't/does not want would pin manually...
> I mean one could also imagine cases where the app needs to pin: if
> specific HW exists connected to some cpus, or if serialisation exists
> (e.g. something like: pin tasks 1,2,and 3 to cpu 1,2,3, and then, when
> this work is done (e.g. after a barrier joining task 1,2,3) pin taks
> 4,5,6 on cpu 1,2,3 again.) There could be theoriticaly complex such
> dependency graphs where all cpus cannot be pinned from the beginning,
> and where your approach seem too restrictive.
>
> But I am glad to hear your voice on the arch call :-)
>
> Christophe.
> >
> > Best Regards, Yi
> >
> >
> > On 4 June 2016 at 05:56, Bill Fischofer 
> wrote:
> >>
> >> I realized I forgot to respond to s23.  Corrected here.
> >>
> >> On Fri, Jun 3, 2016 at 4:15 AM, Christophe Milard <
> >> christophe.mil...@linaro.org> wrote:
> >>
> >> > since V3: Update following Bill's comments
> >> > since V2: Update following Barry and Bill's comments
> >> > since V1: Update following arch call 31 may 2016
> >> >
> >> > This is a tentative to sum up the discussions around the
> thread/process
> >> > that have been happening these last weeks.
> >> > Sorry for the formalism of this mail, but it seems we need accuracy
> >> > here...
> >> >
> >> > This summary is organized as follows:
> >> >
> >> > It is a set of statemen

[lng-odp] ML test

[Nicolas Morey-Chaisemartin]

Hi,


Can ayone using the ML on gmail can see this?

IT tweaked some things in our mail server configuration and not sure if we 
still ned up in Junk or not :)


Thanks


Nicolas

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

Re: [lng-odp] [PATCH v2 4/4] linux-gen: pktio: don't allocate new packets in classifier

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

Hi Oriol,

Sounds like there is a bug in the raw socket implementation. I’ll take a look.

As a side note,  there is currently no classifier performance test as far as I 
can see. Performance test would be useful to prevent issues like this in the 
future. Additionally, classifier validation tests are only run with the 
loopback interface type.

-Matias

From: Oriol Arcas [mailto:or...@starflownetworks.com]
Sent: Tuesday, June 07, 2016 4:15 PM
To: Bala Manoharan 
Cc: Elo, Matias (Nokia - FI/Espoo) ; LNG ODP Mailman List 

Subject: Re: [lng-odp] [PATCH v2 4/4] linux-gen: pktio: don't allocate new 
packets in classifier

Hello all,
Sorry for joining the convesation late too. At Starflow we detected a 
regression at this exact commit. We use the RAW socket interface (socket_mmap 
PKTIO) and now the performance has dropped to kB/s. There is connectivity, but 
the behavior is erratic and the performance very low.
Since it is a deep modification, we have not tracked yet the exact problem, but 
it may be related to the previous comments.

--
Oriol Arcas
Software Engineer
Starflow Networks

On Tue, Jun 7, 2016 at 2:55 PM, Bala Manoharan 
mailto:bala.manoha...@linaro.org>> wrote:
Hi,

Just realised that I haven't sent these comment. Sorry for the late feedback.
Comments inline...

Regards,
Bala


On 2 June 2016 at 13:15, Matias Elo 
mailto:matias@nokia.com>> wrote:
> Don't allocate new packets inside of the internal
> classifier helpers _odp_packet_cls_enq() and
> _odp_packet_classifier(). Instead, save destination queue to
> the parsed packet header and return correct packet pool.
> This enables zero-copy packet classification.

The _odp_packet_cls_enq functions is a performance path where the
packet has not yet been formed cases like receiving the packet from
different pktios i.e socket, netmap, etc and the packet gets created
for the first time from the pool by the classification engine based on
the destination CoS.
The function _odp_packet_classifier is called in the case where the
packet has been pre-allocated and is being classified this is not an
actual performance path and is only called by the loopback interface.
Also in this scenario a new packet gets allocated only if the final
pool is different from the pool in which the packet was initially
allocated.

By combining these different functionality in the function
cls_classify_packet(), when the function gets called for pre-allocated
packets you have passed the address of a packet by calling
odp_packet_data() there is an issue here since if the packet is
segmented then the cls_classify_packet() function can not handle the
scenario, whereas if you pass the entire packet then it is possible
for the function to handle the same.

>
> Added new internal pktio helper pktin_recv_buf(), which
> enqueues packets to classifier queues if necessary.
>
> All pktio types use now a common cls_classify_packet()
> helper function.
>
> Signed-off-by: Matias Elo mailto:matias@nokia.com>>
> Reviewed-and-tested-by: Bill Fischofer 
> mailto:bill.fischo...@linaro.org>>
> ---
> V2:
> - Fixed possible hang in pkt_mmap_v2_rx() receive loop (Maxim)
>
>  .../include/odp_classification_internal.h  |  21 +---
>  .../linux-generic/include/odp_packet_internal.h|   5 +
>  .../linux-generic/include/odp_packet_io_internal.h |   3 -
>  platform/linux-generic/odp_classification.c| 112 
> ++---
>  platform/linux-generic/odp_packet.c|   5 -
>  platform/linux-generic/odp_packet_io.c |  77 --
>  platform/linux-generic/pktio/dpdk.c|  55 +-
>  platform/linux-generic/pktio/loop.c|  86 
>  platform/linux-generic/pktio/netmap.c  |  45 -
>  platform/linux-generic/pktio/pktio_common.c|  67 
>  platform/linux-generic/pktio/socket.c  |  29 --
>  platform/linux-generic/pktio/socket_mmap.c |  59 ++-
>  12 files changed, 253 insertions(+), 311 deletions(-)
>
> diff --git a/platform/linux-generic/include/odp_classification_internal.h 
> b/platform/linux-generic/include/odp_classification_internal.h
> index 3a88462..d6d6904 100644
> --- a/platform/linux-generic/include/odp_classification_internal.h
> +++ b/platform/linux-generic/include/odp_classification_internal.h
> @@ -30,21 +30,6 @@ extern "C" {
>
>  /**
>  @internal
> -Select a CoS for the given Packet based on pktio
> -
> -This function will call all the PMRs associated with a pktio for
> -a given packet and will return the matched COS object.
> -This function will check PMR, L2 and L3 QoS COS object associated
> -with the PKTIO interface.
> -
> -Returns the default cos if the packet does not match any PMR
> -Returns the error_cos if the packet has an error
> -**/
> -cos_t *pktio_select_cos(pktio_entry_t *pktio, const uint8_t *pkt_addr,
> -   odp_packet_hdr_t *pkt_hdr);
> -
> -/**
> -@internal
>  match_qos_cos
>
>  Select a Co