Re: [lng-odp] lng-odp mailman settings

[Bill Fischofer]

On Friday, May 13, 2016, Brian Brooks  wrote:

> On 05/13 10:07:44, Bill Fischofer wrote:
> > I don't think we need to impose those sort of restrictions. I personally
> > use sylpheed for patches and GMail for everything else and they have no
> > problem sorting things out.
> >
> > The ODP mailing list covers both discussions as well as patches, and HTML
> > is useful for the former. The onus should be on those who have problems
> > with this to upgrade their tools rather than sending everyone else back
> to
> > the 1980s.
> >
> > On Fri, May 13, 2016 at 9:57 AM, Mike Holmes  > wrote:
> >
> > > All,
> > >
> > > A topic that comes up occasionally that affects some mail tools like
> > > outlook is that html email on this list makes it harder for some folks
> to
> > > process patches.
> > >
> > > I use mutt specifically to avoid using a fancy email client for all my
> > > patch download/send work, it is powered by coal/steam and would work
> fine
> > > on a 1970 VAX but there you go.
> > >
> > > Anyway, does anyone object to having mailman reject HTML mail ? One up
> > > side is that I will not waste time blocking offers of cheap watches
> and fax
> > > services from the list :)
> > >
> > > On the other hand I will miss bullet lists and the color red when I am
> > > grumpy.
> > >
> > > Mike
>
> +1 for plain text
>
> I think this topic is more about simplicity and best practices rather than
> anything else.
>
> Here is an example of why HTML doesn't work well with archives:
> https://lists.linaro.org/pipermail/lng-odp/2016-May/023134.html
>
> That link displays perfectly fine for me.  What problem do you see?



> Another useful link: http://www.tux.org/lkml/#s3-9
>
___
lng-odp mailing list
lng-odp@lists.linaro.org
https://lists.linaro.org/mailman/listinfo/lng-odp

Re: [lng-odp] [PATCH 1/3] test: scheduling: simplify code structure

[Maxim Uvarov]

Merged,
Maxim.

On 05/13/16 17:05, Savolainen, Petri (Nokia - FI/Espoo) wrote:


Ping for merge. I’ll continue scheduler interface development on top 
of this.


-Petri

*From:*EXT Bill Fischofer [mailto:bill.fischo...@linaro.org]
*Sent:* Thursday, May 12, 2016 2:30 AM
*To:* Savolainen, Petri (Nokia - FI/Espoo) 
*Cc:* LNG ODP Mailman List 
*Subject:* Re: [lng-odp] [PATCH 1/3] test: scheduling: simplify code 
structure


For this series:

Reviewed-and-tested-by: Bill Fischofer >


On Wed, May 11, 2016 at 7:38 AM, Petri Savolainen 
mailto:petri.savolai...@nokia.com>> wrote:


Recorded queue and pool handles into shared data, so that
multiple lookups (and related name generation code) can be
avoided. Use single function (enqueue_events()) to enqueue
events into queues on all test cases.

Signed-off-by: Petri Savolainen mailto:petri.savolai...@nokia.com>>
---
 test/performance/odp_scheduling.c | 282
--
 1 file changed, 87 insertions(+), 195 deletions(-)

diff --git a/test/performance/odp_scheduling.c
b/test/performance/odp_scheduling.c
index 6592277..ec913ae 100644
--- a/test/performance/odp_scheduling.c
+++ b/test/performance/odp_scheduling.c
@@ -32,6 +32,7 @@
 #define MSG_POOL_SIZE (4*1024*1024) /**< Message pool size */
 #define MAX_ALLOCS35/**< Alloc burst size */
 #define QUEUES_PER_PRIO   64/**< Queue per
priority */
+#define NUM_PRIOS 2 /**< Number of tested
priorities */
 #define QUEUE_ROUNDS  (512*1024)/**< Queue test rounds */
 #define ALLOC_ROUNDS  (1024*1024)   /**< Alloc test rounds */
 #define MULTI_BUFS_MAX4 /**< Buffer burst size */
@@ -52,13 +53,13 @@ typedef struct {
int proc_mode;  /**< Process mode */
 } test_args_t;

-
 /** Test global variables */
 typedef struct {
-   odp_barrier_t barrier;/**< @private Barrier for test
synchronisation */
+   odp_barrier_t barrier;
+   odp_pool_tpool;
+   odp_queue_t  queue[NUM_PRIOS][QUEUES_PER_PRIO];
 } test_globals_t;

-
 /**
  * @internal Clear all scheduled queues. Retry to be sure that all
  * buffers have been scheduled.
@@ -78,89 +79,45 @@ static void clear_sched_queues(void)
 }

 /**
- * @internal Create a single queue from a pool of buffers
+ * @internal Enqueue events into queues
  *
- * @param thr  Thread
- * @param msg_pool  Buffer pool
- * @param prio   Queue priority
+ * @param thrThread
+ * @param prio   Queue priority
+ * @param num_queues Number of queues
+ * @param num_events Number of events
+ * @param globalsTest shared data
  *
  * @return 0 if successful
  */
-static int create_queue(int thr, odp_pool_t msg_pool, int prio)
+static int enqueue_events(int thr, int prio, int num_queues, int
num_events,
+ test_globals_t *globals)
 {
-   char name[] = "sched_XX_00";
odp_buffer_t buf;
odp_queue_t queue;
+   int i, j, k;

-   buf = odp_buffer_alloc(msg_pool);
-
-   if (!odp_buffer_is_valid(buf)) {
-   LOG_ERR("  [%i] msg_pool alloc failed\n", thr);
-   return -1;
-   }
-
-   name[6] = '0' + prio/10;
-   name[7] = '0' + prio - 10*(prio/10);
-
-   queue = odp_queue_lookup(name);
-
-   if (queue == ODP_QUEUE_INVALID) {
-   LOG_ERR("  [%i] Queue %s lookup failed.\n", thr,
name);
-   return -1;
-   }
-
-   if (odp_queue_enq(queue, odp_buffer_to_event(buf))) {
-   LOG_ERR("  [%i] Queue enqueue failed.\n", thr);
-   odp_buffer_free(buf);
-   return -1;
-   }
-
-   return 0;
-}
-
-/**
- * @internal Create multiple queues from a pool of buffers
- *
- * @param thr  Thread
- * @param msg_pool  Buffer pool
- * @param prio   Queue priority
- *
- * @return 0 if successful
- */
-static int create_queues(int thr, odp_pool_t msg_pool, int prio)
-{
-   char name[] = "sched_XX_YY";
-   odp_buffer_t buf;
-   odp_queue_t queue;
-   int i;
-
-   name[6] = '0' + prio/10;
-   name[7] = '0' + prio - 10*(prio/10);
+   if (prio == ODP_SCHED_PRIO_HIGHEST)
+   i = 0;
+   else
+   i = 1;

/* Alloc and enqueue a buffer per queue */
-   for (i = 0; i < QUEUES_PER_PRIO; i++) {
-   name[9]  = '0' + i/10;
-   name[10] = '0' + i - 10*(i/10);
-
-   queue = odp_queue_lookup(name);
+   for (j = 0;

Re: [lng-odp] lng-odp mailman settings

[Brian Brooks]

On 05/13 10:07:44, Bill Fischofer wrote:
> I don't think we need to impose those sort of restrictions. I personally
> use sylpheed for patches and GMail for everything else and they have no
> problem sorting things out.
> 
> The ODP mailing list covers both discussions as well as patches, and HTML
> is useful for the former. The onus should be on those who have problems
> with this to upgrade their tools rather than sending everyone else back to
> the 1980s.
> 
> On Fri, May 13, 2016 at 9:57 AM, Mike Holmes  wrote:
> 
> > All,
> >
> > A topic that comes up occasionally that affects some mail tools like
> > outlook is that html email on this list makes it harder for some folks to
> > process patches.
> >
> > I use mutt specifically to avoid using a fancy email client for all my
> > patch download/send work, it is powered by coal/steam and would work fine
> > on a 1970 VAX but there you go.
> >
> > Anyway, does anyone object to having mailman reject HTML mail ? One up
> > side is that I will not waste time blocking offers of cheap watches and fax
> > services from the list :)
> >
> > On the other hand I will miss bullet lists and the color red when I am
> > grumpy.
> >
> > Mike

+1 for plain text

I think this topic is more about simplicity and best practices rather than
anything else.

Here is an example of why HTML doesn't work well with archives:
https://lists.linaro.org/pipermail/lng-odp/2016-May/023134.html

Another useful link: http://www.tux.org/lkml/#s3-9
___
lng-odp mailing list
lng-odp@lists.linaro.org
https://lists.linaro.org/mailman/listinfo/lng-odp

[lng-odp] [PATCH v2] doc: process-guide: convert CONTRIBUTING to asciidoc

[Mike Holmes]

Converting to asciidoc allows a tidy page to be added to the online
documentation without cutting and pasting into wordpress.
Being Asccidoc a tiny amount of clutter is added to show code snippets
attractively when rendered that make it slightly hard to read as a raw
document.

Signed-off-by: Mike Holmes 
---
v2
  [source,] formatting, ":", improve section on .svg (Bill)

 CONTRIBUTING  | 108 ++
 doc/process-guide/.gitignore  |   1 +
 doc/process-guide/Makefile.am |  11 -
 3 files changed, 76 insertions(+), 44 deletions(-)

diff --git a/CONTRIBUTING b/CONTRIBUTING
index f2f8947..a4ed988 100644
--- a/CONTRIBUTING
+++ b/CONTRIBUTING
@@ -1,28 +1,32 @@
-Contributing to the OpenDataplane (ODP) API

-
-Table of content:
--
-1. New Development
-2. ODP patch expectations as an  open source project
-3. Common Errors in Patch and Commit Messages
-4. Documenting the code
-5. Documenting the user docs
-6. References
-
-1. New Development
---
-ODP code shall be written with the kernel coding style [1].
+:doctitle: OpenDataPlane (ODP) CONTRIBUTING
+:description: This document is intended to guide a new application developer +
+in understanding the  contributing requirements for ODP
+:imagesdir: ../images
+:toc:
+:numbered!:
+[abstract]
+
+== Abstract
+
+This document is intended to guide a new application developer in understanding
+the  contributing requirements for ODP
+
+== New Development
+
+ODP code shall be written with the kernel coding style 
https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/Documentation/CodingStyle[Kernel
 Coding Style]
+
 ODP code shall be documented using the doxygen style described in the
 "Documenting the code" section.
 Check patch script/checkpatch.pl shall be used before submitting a patch.
 
-2. ODP patch expectations as an  open source project
-
+== ODP patch expectations as an  open source project
+
 While specific to the Linux kernel development, the following reference could
-also be considered a general guide for any Open Source development [2] and is
-relevant to ODP. Many of the guidelines in this ODP document are related to the
-items in that information.
+also be considered a general guide for any Open Source development
+http://ldn.linuxfoundation.org/book/how-participate-linux-community[Participating
 in the Community]
+and is relevant to ODP. Many of the guidelines in this ODP document are related
+to the items in that information.
+
 Pay particular attention to section 5.3 that talks about patch preparation.
 The key thing to remember is to break up your changes into logical sections.
 Otherwise you run the risk of not being able to even explain the purpose of a
@@ -34,11 +38,12 @@ Signed-off-by: tag line a copy of the description follows:
 Signed-off-by: this is a developer's certification that he or she has
 the right to submit the patch for inclusion into the [project].  It is
 an agreement to the Developer's Certificate of Origin, the full text of
-which can be found in [3] Documentation/SubmittingPatches.
+which can be found in 
https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/Documentation/SubmittingPatches[Submitting
 Patches]
+
 Code without a proper signoff cannot be merged into the mainline.
 
-3. Common Errors in Patch and Commit Messages
--
+== Common Errors in Patch and Commit Messages
+
 - Avoid starting the summary line with a capital letter, unless the component
   being referred to also begins with a capital letter.
 - Don't have one huge patch, split your change into logical subparts. It's
@@ -83,8 +88,8 @@ Code without a proper signoff cannot be merged into the 
mainline.
   sources.
 - Avoid punctuation in the short log.
 
-4. Documenting the code

+== Documenting the code
+
 - Allow doxygen to use all its default behaviors to identify tagged
   information but where a doxygen tag must be specified use @
 - The first line is by default the brief summary.
@@ -93,49 +98,68 @@ Code without a proper signoff cannot be merged into the 
mainline.
 - Normal comment sections should be before the code block and start with
   /** on its own line and finish with */ on its own line. The exception
   to this rule is a case where the comment is very small, for example
-  /** macro description */
-  #define SOME_MACRO 0
+[source,doxygen]
+
+/** macro description */
+#define SOME_MACRO 0
+
 - Commenting on the end of a line for macros and struct members is allowed
-  using:
-  /**<  */ for example
-  #define SOME_MACRO 0 /**<  */
+  using: /**<  */ for example
+[source,doxygen]
+
+#define SOME_MACRO 0 /**<  */
+
 - Files should start with a files description using @file
 - Functions should specify their parameters with @param[in] and @param[out]
 - 

Re: [lng-odp] [PATCHv6 1/2] doc: userguide: add timer/timeout state diagrams

[Christophe Milard]

For the series:
Reviewed-by: Christophe Milard 

Christophe.


On 13 May 2016 at 17:01, Bill Fischofer  wrote:

> Signed-off-by: Bill Fischofer 
> Signed-off-by: Christophe Milard 

---
>  doc/images/.gitignore   |  2 ++
>  doc/images/timeout_fsm.gv   | 28 
>  doc/images/timer_fsm.gv | 20 
>  doc/users-guide/Makefile.am |  2 ++
>  4 files changed, 52 insertions(+)
>  create mode 100644 doc/images/timeout_fsm.gv
>  create mode 100644 doc/images/timer_fsm.gv
>
> diff --git a/doc/images/.gitignore b/doc/images/.gitignore
> index a19aa75..003dbe6 100644
> --- a/doc/images/.gitignore
> +++ b/doc/images/.gitignore
> @@ -1,2 +1,4 @@
>  resource_management.svg
>  pktio_fsm.svg
> +timer_fsm.svg
> +timeout_fsm.svg
> diff --git a/doc/images/timeout_fsm.gv b/doc/images/timeout_fsm.gv
> new file mode 100644
> index 000..21ecb59
> --- /dev/null
> +++ b/doc/images/timeout_fsm.gv
> @@ -0,0 +1,28 @@
> +digraph timer_state_machine {
> +   rankdir=LR;
> +   size="12,20";
> +   node [fontsize=28];
> +   edge [fontsize=28];
> +   node [shape=doublecircle]; TO_Unalloc;
> +   node [shape=circle]; TO_Alloc TO_Pending TO_Delivered;
> +   node [shape=rect]; TO_Enqueued;
> +   TO_Unalloc -> TO_Alloc [label="odp_timeout_alloc()"];
> +   TO_Alloc -> TO_Unalloc [label="odp_timeout_free()"];
> +   TO_Alloc -> TO_Pending [fontcolor=green,
> +  label="odp_timer_set_abs()"];
> +   TO_Alloc -> TO_Pending [fontcolor=green,
> +  label="odp_timer_set_rel()"];
> +   TO_Pending -> TO_Alloc [fontcolor=green,
> +  label="odp_timer_cancel()"];
> +   TO_Pending -> TO_Enqueued [fontcolor=green, label="timer expires"];
> +   TO_Enqueued -> TO_Delivered [label="odp_schedule()"];
> +   TO_Delivered -> TO_Pending [fontcolor=green,
> +  label="odp_timer_set_abs()"];
> +   TO_Delivered -> TO_Pending [fontcolor=green,
> +  label="odp_timer_set_rel()"];
> +   TO_Delivered -> TO_Delivered [label="odp_timeout_from_event()"];
> +   TO_Delivered -> TO_Delivered [label="odp_timeout_timer()"];
> +   TO_Delivered -> TO_Unalloc
> +   [label="odp_timeout_free() / odp_event_free()"];
> +
> +}
> diff --git a/doc/images/timer_fsm.gv b/doc/images/timer_fsm.gv
> new file mode 100644
> index 000..1798d31
> --- /dev/null
> +++ b/doc/images/timer_fsm.gv
> @@ -0,0 +1,20 @@
> +digraph timer_state_machine {
> +   rankdir=LR;
> +   size="12,20";
> +   node [fontsize=28];
> +   edge [fontsize=28];
> +   node [shape=doublecircle]; Timer_Unalloc;
> +   node [shape=circle]; Timer_Alloc Timer_Set Timer_Expired
> +   Timer_Unalloc -> Timer_Alloc [label="odp_timer_alloc()"];
> +   Timer_Alloc -> Timer_Unalloc [label="odp_timer_free()"];
> +   Timer_Alloc -> Timer_Set
> [fontcolor=green,label="odp_timer_set_abs()"];
> +   Timer_Alloc -> Timer_Set
> [fontcolor=green,label="odp_timer_set_rel()"];
> +   Timer_Set -> Timer_Alloc
> [fontcolor=green,label="odp_timer_cancel()"];
> +   Timer_Set -> Timer_Expired [fontcolor=green,label="timer expires"];
> +   Timer_Expired -> Timer_Unalloc [label="odp_timer_free()"];
> +   Timer_Expired -> Timer_Set [fontcolor=green,
> +  label="odp_timer_set_abs()"];
> +   Timer_Expired -> Timer_Set [fontcolor=green,
> +  label="odp_timer_set_rel()"];
> +
> +}
> diff --git a/doc/users-guide/Makefile.am b/doc/users-guide/Makefile.am
> index 74caa96..2e1195e 100644
> --- a/doc/users-guide/Makefile.am
> +++ b/doc/users-guide/Makefile.am
> @@ -30,6 +30,8 @@ IMAGES = $(top_srcdir)/doc/images/overview.svg \
>  $(top_srcdir)/doc/images/release_git.svg \
>  $(top_srcdir)/doc/images/segment.svg \
>  $(top_srcdir)/doc/images/simple_release_git.svg \
> +$(top_srcdir)/doc/images/timeout_fsm.svg \
> +$(top_srcdir)/doc/images/timer_fsm.svg \
>  $(top_srcdir)/doc/images/tm_hierarchy.svg \
>  $(top_srcdir)/doc/images/tm_node.svg
>
> --
> 2.7.4
>
>
___
lng-odp mailing list
lng-odp@lists.linaro.org
https://lists.linaro.org/mailman/listinfo/lng-odp

Re: [lng-odp] [ODP/PATCH] api: sync: remove odp_sync_stores call under octeon.

[Mike Holmes]

I think a more usual patch name would be

linux-generic: ticketlock: remove odp_sync_stores for octeon

we try to do approximate area, api, test, docs, linux-generic
implementation, followed by a tighter localisation and then a description,
when I read  "api sync" I assumed it should have gone to api-next

On 13 May 2016 at 11:04, Rizwan Ansari  wrote:

> Removes odp_sync_stores() call for OCTEON, Octeon
> build will fail, As this api has been already
> replaced by odp_mb_full().
>
> Signed-off-by: Rizwan Ansari 
> Reviewed-by: Petri Savolainen 
> ---
>  platform/linux-generic/odp_ticketlock.c | 3 ---
>  1 file changed, 3 deletions(-)
>
> diff --git a/platform/linux-generic/odp_ticketlock.c
> b/platform/linux-generic/odp_ticketlock.c
> index 7b4246f..353af9a 100644
> --- a/platform/linux-generic/odp_ticketlock.c
> +++ b/platform/linux-generic/odp_ticketlock.c
> @@ -69,9 +69,6 @@ void odp_ticketlock_unlock(odp_ticketlock_t *ticketlock)
>
> odp_atomic_store_rel_u32(&ticketlock->cur_ticket, cur + 1);
>
> -#if defined __OCTEON__
> -   odp_sync_stores(); /* SYNCW to flush write buffer */
> -#endif
>  }
>
>  int odp_ticketlock_is_locked(odp_ticketlock_t *ticketlock)
> --
> 2.5.5
>
> ___
> 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] lng-odp mailman settings

[Bill Fischofer]

I don't think we need to impose those sort of restrictions. I personally
use sylpheed for patches and GMail for everything else and they have no
problem sorting things out.

The ODP mailing list covers both discussions as well as patches, and HTML
is useful for the former. The onus should be on those who have problems
with this to upgrade their tools rather than sending everyone else back to
the 1980s.

On Fri, May 13, 2016 at 9:57 AM, Mike Holmes  wrote:

> All,
>
> A topic that comes up occasionally that affects some mail tools like
> outlook is that html email on this list makes it harder for some folks to
> process patches.
>
> I use mutt specifically to avoid using a fancy email client for all my
> patch download/send work, it is powered by coal/steam and would work fine
> on a 1970 VAX but there you go.
>
> Anyway, does anyone object to having mailman reject HTML mail ? One up
> side is that I will not waste time blocking offers of cheap watches and fax
> services from the list :)
>
> On the other hand I will miss bullet lists and the color red when I am
> grumpy.
>
> Mike
>
> --
> 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
>
>
___
lng-odp mailing list
lng-odp@lists.linaro.org
https://lists.linaro.org/mailman/listinfo/lng-odp

[lng-odp] [ODP/PATCH] api: sync: remove odp_sync_stores call under octeon.

[Rizwan Ansari]

Removes odp_sync_stores() call for OCTEON, Octeon
build will fail, As this api has been already
replaced by odp_mb_full().

Signed-off-by: Rizwan Ansari 
Reviewed-by: Petri Savolainen 
---
 platform/linux-generic/odp_ticketlock.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/platform/linux-generic/odp_ticketlock.c 
b/platform/linux-generic/odp_ticketlock.c
index 7b4246f..353af9a 100644
--- a/platform/linux-generic/odp_ticketlock.c
+++ b/platform/linux-generic/odp_ticketlock.c
@@ -69,9 +69,6 @@ void odp_ticketlock_unlock(odp_ticketlock_t *ticketlock)
 
odp_atomic_store_rel_u32(&ticketlock->cur_ticket, cur + 1);
 
-#if defined __OCTEON__
-   odp_sync_stores(); /* SYNCW to flush write buffer */
-#endif
 }
 
 int odp_ticketlock_is_locked(odp_ticketlock_t *ticketlock)
-- 
2.5.5

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

Re: [lng-odp] [PATCH 4/4] linux-generic: packet: initialize only selected odp_packet_hdr_t members

[Mike Holmes]

On 13 May 2016 at 03:29, Elo, Matias (Nokia - FI/Espoo) <
matias@nokia.com> wrote:

>
>
>
>
> *From:* EXT Bill Fischofer [mailto:bill.fischo...@linaro.org]
> *Sent:* Thursday, May 12, 2016 10:34 PM
> *To:* Elo, Matias (Nokia - FI/Espoo) 
> *Cc:* LNG ODP Mailman List 
> *Subject:* Re: [lng-odp] [PATCH 4/4] linux-generic: packet: initialize
> only selected odp_packet_hdr_t members
>
>
>
>
>
>
>
> On Thu, May 12, 2016 at 7:36 AM, Matias Elo  wrote:
>
> Using memset to initialize struct odp_packet_hdr_t contents
> to 0 has a significant overhead. Instead, initialize only
> the required members of the struct.
>
> Signed-off-by: Matias Elo 
> ---
>  platform/linux-generic/include/odp_packet_internal.h |  8 +---
>  platform/linux-generic/odp_packet.c  | 18
> ++
>  2 files changed, 11 insertions(+), 15 deletions(-)
>
> diff --git a/platform/linux-generic/include/odp_packet_internal.h
> b/platform/linux-generic/include/odp_packet_internal.h
> index 2a12503..cdee1e1 100644
> --- a/platform/linux-generic/include/odp_packet_internal.h
> +++ b/platform/linux-generic/include/odp_packet_internal.h
> @@ -143,15 +143,17 @@ typedef struct {
> uint32_t l4_offset; /**< offset to L4 hdr (TCP, UDP, SCTP, also
> ICMP) */
> uint32_t payload_offset; /**< offset to payload */
>
> -   uint32_t l3_len; /**< Layer 3 length */
> -   uint32_t l4_len; /**< Layer 4 length */
> -
> uint32_t frame_len;
> uint32_t headroom;
> uint32_t tailroom;
>
> odp_pktio_t input;
>
> +   /* Values below are not initialized by packet_init() */
> +
> +   uint32_t l3_len; /**< Layer 3 length */
> +   uint32_t l4_len; /**< Layer 4 length */
> +
> uint32_t flow_hash;  /**< Flow hash value */
> odp_time_t timestamp;/**< Timestamp value */
>
> diff --git a/platform/linux-generic/odp_packet.c
> b/platform/linux-generic/odp_packet.c
> index 436265e..f5f224d 100644
> --- a/platform/linux-generic/odp_packet.c
> +++ b/platform/linux-generic/odp_packet.c
> @@ -50,23 +50,17 @@ void packet_parse_reset(odp_packet_hdr_t *pkt_hdr)
>  static void packet_init(pool_entry_t *pool, odp_packet_hdr_t *pkt_hdr,
> size_t size, int parse)
>  {
> -   /*
> -   * Reset parser metadata.  Note that we clear via memset to make
> -   * this routine indepenent of any additional adds to packet
> metadata.
> -   */
> -   const size_t start_offset = ODP_FIELD_SIZEOF(odp_packet_hdr_t,
> buf_hdr);
> -   uint8_t *start;
> -   size_t len;
> +   pkt_hdr->input_flags.all  = 0;
> +   pkt_hdr->output_flags.all = 0;
> +   pkt_hdr->error_flags.all  = 0;
>
> -   start = (uint8_t *)pkt_hdr + start_offset;
> -   len = sizeof(odp_packet_hdr_t) - start_offset;
> -   memset(start, 0, len);
>
>
>
> The reason for this memset is to "future proof" the header by allowing any
> new fields that are added to get initialized to a known value without
> having to update this routine. Is the impact of this single memset() all
> that large?  I can understand wanting to compress the header to possibly
> avoid another cache line reference but this optimization seems inherently
> error-prone. I'm also wondering if this might confuse tools like Coverity
> which seems to be paranoid about uninitialized variables.
>
>
>
> Hi Bill,
>
>
>
> I can understand the future proofing. However, this single memset() is
> currently the largest cycle hog when forwarding small packets. Below are
> some throughput numbers:
>
>
>
> odp_l2fwd -i p1p1 -c 1 -a 0 (netmap pktio):
>
>
>
> Niantic (1x10Gbps):7.9 vs 8.6 Mpps (64B) => 8% improvement
>
> Fortville (1x40Gbps):  7.9 vs 8.8 Mpps (64B) => 11% improvement
>
>
>
> When the performance gain is this big I would pay the price of being more
> error-prone. Is there a way to run Coverity before merging the patch?
>

Not easily until we resolve the licence/number of coverty runs per week
issue - on going :/
Coverty might be ok as long as we dont use the field for anything, and  for
11% I am happy to mark it a false positive in coverty if it is flagged
Can we mitigate some of the problems with a doxygen warning on this struct
about its behaviour ?



>
>
> -Matias
>
>
>
> -
> -   /* Set metadata items that initialize to non-zero values */
> +   pkt_hdr->l2_offset = 0;
> pkt_hdr->l3_offset = ODP_PACKET_OFFSET_INVALID;
> pkt_hdr->l4_offset = ODP_PACKET_OFFSET_INVALID;
> pkt_hdr->payload_offset = ODP_PACKET_OFFSET_INVALID;
>
> +   pkt_hdr->input = ODP_PKTIO_INVALID;
> +
> /* Disable lazy parsing on user allocated packets */
> if (!parse)
> packet_parse_disable(pkt_hdr);
> --
> 1.9.1
>
> ___
> lng-odp mailing list
> lng-odp@lists.linaro.org
> https://lists.linaro.org/mailman/listinfo/lng-odp
>
>
>
> ___
> lng-

[lng-odp] [PATCHv6 2/2] doc: userguide: add timer and timeout event section to user guide

[Bill Fischofer]

Signed-off-by: Ivan Khoronzhuk 
Signed-off-by: Bill Fischofer 
Signed-off-by: Christophe Milard 
---
 doc/users-guide/Makefile.am|   1 +
 doc/users-guide/users-guide-timer.adoc | 144 +
 doc/users-guide/users-guide.adoc   |   2 +
 3 files changed, 147 insertions(+)
 create mode 100644 doc/users-guide/users-guide-timer.adoc

diff --git a/doc/users-guide/Makefile.am b/doc/users-guide/Makefile.am
index 2e1195e..5903392 100644
--- a/doc/users-guide/Makefile.am
+++ b/doc/users-guide/Makefile.am
@@ -4,6 +4,7 @@ SRC= $(top_srcdir)/doc/users-guide/users-guide.adoc \
 $(top_srcdir)/doc/users-guide/users-guide-cls.adoc \
 $(top_srcdir)/doc/users-guide/users-guide-packet.adoc \
 $(top_srcdir)/doc/users-guide/users-guide-pktio.adoc \
+$(top_srcdir)/doc/users-guide/users-guide-timer.adoc \
 $(top_srcdir)/doc/users-guide/users-guide-tm.adoc
 TARGET = users-guide.html
 IMAGES = $(top_srcdir)/doc/images/overview.svg \
diff --git a/doc/users-guide/users-guide-timer.adoc 
b/doc/users-guide/users-guide-timer.adoc
new file mode 100644
index 000..9cd30de
--- /dev/null
+++ b/doc/users-guide/users-guide-timer.adoc
@@ -0,0 +1,144 @@
+== Timers and Timeout Events
+The ODP Timer APIs offer a set of functions that permit applications to react
+to the passage of time, and are designed to reflect the underlying hardware
+timing features found in various platforms that support ODP implementations.
+
+Timers are drawn from specialized pools called _timer pools_ that have their
+own abstract type (`odp_timer_pool_t`). Each timer pool is a logically
+independent time source with its own _resolution_ measured in nanoseconds (ns)
+and a maximum number of timers that it can support. Applications can have many
+timers active at the same time and can set them to use either relative or
+absolute time. Associated with each timer is a queue that is to receive events
+when this timer expires. This queue is created by a separate
+`odp_queue_create()` call that is passed as a parameter to `odp_timer_alloc()`.
+
+Timeouts are specialized events of type `odp_timeout_t` that are used to
+represent the expiration of timers. Timeouts are drawn from pools of type
+`ODP_POOL_TIMEOUT` that are created by the standard `odp_pool_create()` API.
+Timeout events are associated with timers when those timers are _set_ and are
+enqueued to their timer's associated queue whenever a set timer expires. So the
+effect of timer expiration is a timeout event being added to a queue and
+delivered via normal ODP event scheduling.
+
+The following diagrams show the life cycle of timers and timeout events.
+Transitions in these finite state machines are marked by the event
+triggering them. Events marked in green are common to both state machines,
+_i.e.,_ trigger both state machines.
+
+.ODP Timers lifecycle State Diagram
+image::timer_fsm.svg[align="center"]
+
+.ODP Timeout event lifecyle State Diagram
+image::timeout_fsm.svg[align="center"]
+
+Reminder:
+On a `timer expire` event, the related timeout event is enqueued to the timer
+related queue.
+
+Timers measure time in _ticks_ rather than nanoseconds because each timer pool
+may have its own time source and associated conversion ratios. It is thus more
+efficient to manipulate time in these native tick values. As a result time
+measured in nanoseconds must be converted between timer-pool specific tick
+values via the conversion functions `odp_timer_ns_to_tick()` and
+`odp_timer_tick_to_ns()` as needed.  Both of these functions take a timer pool
+as an input parameter to enable the pool-specific conversion ratios to be
+used.
+
+Associated with each timer pool is a free running tick counter that can be
+sampled at any time via the `odp_timer_current_tick()` API. Timers can be set
+to an absolute future tick value via `odp_timer_set_abs()` or to a future tick
+value relative to the current tick via `odp_timer_set_rel()`.  Implementations
+may impose minimum and maximum future values supported by a given timer pool
+and timer set operations will fail if the requested value is outside of the
+supported range.
+
+Before a set timer expires, it can be canceled via the `odp_timer_cancel()`
+API. A successful cancel has the same effect as if the timer were never set.
+An attempted cancel will fail if the timer is not set or if it has already
+expired.
+
+=== Timer Pool Management
+To facilitate implementation of the ODP timer APIs, an additional timer API is
+provided. During initialization, applications are expected to create the timer
+pools they need and then call `odp_timer_pool_start()`. ODP implementations
+may or may not fail further attempts to create timer pools after this API is
+called. For best portability, applications should not attempt to create
+further timer pools after calling `odp_timer_pool_start()`. Note that no such
+restrictions exist on timeout pools, as these are just ordinary ODP pools.
+
+Following start, a

[lng-odp] [PATCHv6 1/2] doc: userguide: add timer/timeout state diagrams

[Bill Fischofer]

Signed-off-by: Bill Fischofer 
Signed-off-by: Christophe Milard 
---
 doc/images/.gitignore   |  2 ++
 doc/images/timeout_fsm.gv   | 28 
 doc/images/timer_fsm.gv | 20 
 doc/users-guide/Makefile.am |  2 ++
 4 files changed, 52 insertions(+)
 create mode 100644 doc/images/timeout_fsm.gv
 create mode 100644 doc/images/timer_fsm.gv

diff --git a/doc/images/.gitignore b/doc/images/.gitignore
index a19aa75..003dbe6 100644
--- a/doc/images/.gitignore
+++ b/doc/images/.gitignore
@@ -1,2 +1,4 @@
 resource_management.svg
 pktio_fsm.svg
+timer_fsm.svg
+timeout_fsm.svg
diff --git a/doc/images/timeout_fsm.gv b/doc/images/timeout_fsm.gv
new file mode 100644
index 000..21ecb59
--- /dev/null
+++ b/doc/images/timeout_fsm.gv
@@ -0,0 +1,28 @@
+digraph timer_state_machine {
+   rankdir=LR;
+   size="12,20";
+   node [fontsize=28];
+   edge [fontsize=28];
+   node [shape=doublecircle]; TO_Unalloc;
+   node [shape=circle]; TO_Alloc TO_Pending TO_Delivered;
+   node [shape=rect]; TO_Enqueued;
+   TO_Unalloc -> TO_Alloc [label="odp_timeout_alloc()"];
+   TO_Alloc -> TO_Unalloc [label="odp_timeout_free()"];
+   TO_Alloc -> TO_Pending [fontcolor=green,
+  label="odp_timer_set_abs()"];
+   TO_Alloc -> TO_Pending [fontcolor=green,
+  label="odp_timer_set_rel()"];
+   TO_Pending -> TO_Alloc [fontcolor=green,
+  label="odp_timer_cancel()"];
+   TO_Pending -> TO_Enqueued [fontcolor=green, label="timer expires"];
+   TO_Enqueued -> TO_Delivered [label="odp_schedule()"];
+   TO_Delivered -> TO_Pending [fontcolor=green,
+  label="odp_timer_set_abs()"];
+   TO_Delivered -> TO_Pending [fontcolor=green,
+  label="odp_timer_set_rel()"];
+   TO_Delivered -> TO_Delivered [label="odp_timeout_from_event()"];
+   TO_Delivered -> TO_Delivered [label="odp_timeout_timer()"];
+   TO_Delivered -> TO_Unalloc
+   [label="odp_timeout_free() / odp_event_free()"];
+
+}
diff --git a/doc/images/timer_fsm.gv b/doc/images/timer_fsm.gv
new file mode 100644
index 000..1798d31
--- /dev/null
+++ b/doc/images/timer_fsm.gv
@@ -0,0 +1,20 @@
+digraph timer_state_machine {
+   rankdir=LR;
+   size="12,20";
+   node [fontsize=28];
+   edge [fontsize=28];
+   node [shape=doublecircle]; Timer_Unalloc;
+   node [shape=circle]; Timer_Alloc Timer_Set Timer_Expired
+   Timer_Unalloc -> Timer_Alloc [label="odp_timer_alloc()"];
+   Timer_Alloc -> Timer_Unalloc [label="odp_timer_free()"];
+   Timer_Alloc -> Timer_Set [fontcolor=green,label="odp_timer_set_abs()"];
+   Timer_Alloc -> Timer_Set [fontcolor=green,label="odp_timer_set_rel()"];
+   Timer_Set -> Timer_Alloc [fontcolor=green,label="odp_timer_cancel()"];
+   Timer_Set -> Timer_Expired [fontcolor=green,label="timer expires"];
+   Timer_Expired -> Timer_Unalloc [label="odp_timer_free()"];
+   Timer_Expired -> Timer_Set [fontcolor=green,
+  label="odp_timer_set_abs()"];
+   Timer_Expired -> Timer_Set [fontcolor=green,
+  label="odp_timer_set_rel()"];
+
+}
diff --git a/doc/users-guide/Makefile.am b/doc/users-guide/Makefile.am
index 74caa96..2e1195e 100644
--- a/doc/users-guide/Makefile.am
+++ b/doc/users-guide/Makefile.am
@@ -30,6 +30,8 @@ IMAGES = $(top_srcdir)/doc/images/overview.svg \
 $(top_srcdir)/doc/images/release_git.svg \
 $(top_srcdir)/doc/images/segment.svg \
 $(top_srcdir)/doc/images/simple_release_git.svg \
+$(top_srcdir)/doc/images/timeout_fsm.svg \
+$(top_srcdir)/doc/images/timer_fsm.svg \
 $(top_srcdir)/doc/images/tm_hierarchy.svg \
 $(top_srcdir)/doc/images/tm_node.svg
 
-- 
2.7.4

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

[lng-odp] lng-odp mailman settings

[Mike Holmes]

All,

A topic that comes up occasionally that affects some mail tools like
outlook is that html email on this list makes it harder for some folks to
process patches.

I use mutt specifically to avoid using a fancy email client for all my
patch download/send work, it is powered by coal/steam and would work fine
on a 1970 VAX but there you go.

Anyway, does anyone object to having mailman reject HTML mail ? One up side
is that I will not waste time blocking offers of cheap watches and fax
services from the list :)

On the other hand I will miss bullet lists and the color red when I am
grumpy.

Mike

-- 
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 v3 2/2] linux-generic: timer: use strong typing

[Maxim Uvarov]

Merged,
Maxim.

On 05/12/16 22:37, Bill Fischofer wrote:



On Thu, May 12, 2016 at 5:13 AM, Matias Elo > wrote:


Use strong typing for odp_timer_t and odp_timeout_t. Fix
couple exposed usage errors.

Signed-off-by: Matias Elo mailto:matias@nokia.com>>


Reviewed-and-tested-by: Bill Fischofer >


---
V2:
+ Split too long string lines (Maxim)

 example/timer/odp_timer_test.c | 10 ++
 platform/linux-generic/include/odp/api/plat/timer_types.h | 10
++
 platform/linux-generic/odp_timer.c |  6 +++---
 test/validation/timer/timer.c  |  2 +-
 4 files changed, 16 insertions(+), 12 deletions(-)

diff --git a/example/timer/odp_timer_test.c
b/example/timer/odp_timer_test.c
index f1d3be4..3880664 100644
--- a/example/timer/odp_timer_test.c
+++ b/example/timer/odp_timer_test.c
@@ -165,16 +165,18 @@ static void test_abs_timeouts(int thr,
test_globals_t *gbls)
if (!odp_timeout_fresh(tmo)) {
/* Not the expected expiration tick, timer has
 * been reset or cancelled or freed */
-   EXAMPLE_ABORT("Unexpected timeout received
(timer %" PRIx32 ", tick %" PRIu64 ")\n",
- ttp->tim, tick);
+   EXAMPLE_ABORT("Unexpected timeout received
(timer "
+ "%" PRIu64", tick %" PRIu64
")\n",
+  odp_timer_to_u64(ttp->tim), tick);
}
EXAMPLE_DBG("  [%i] timeout, tick %"PRIu64"\n",
thr, tick);

uint32_t rx_num =
odp_atomic_fetch_dec_u32(&gbls->remain);

if (!rx_num)
-   EXAMPLE_ABORT("Unexpected timeout received
(timer %x, tick %"PRIu64")\n",
- ttp->tim, tick);
+   EXAMPLE_ABORT("Unexpected timeout received
(timer "
+ "%" PRIu64 ", tick %" PRIu64
")\n",
+  odp_timer_to_u64(ttp->tim), tick);
else if (rx_num > num_workers)
continue;

diff --git
a/platform/linux-generic/include/odp/api/plat/timer_types.h
b/platform/linux-generic/include/odp/api/plat/timer_types.h
index 006683e..93ea162 100644
--- a/platform/linux-generic/include/odp/api/plat/timer_types.h
+++ b/platform/linux-generic/include/odp/api/plat/timer_types.h
@@ -18,6 +18,8 @@
 extern "C" {
 #endif

+#include 
+
 /** @addtogroup odp_timer
  *  @{
  **/
@@ -28,13 +30,13 @@ typedef struct odp_timer_pool_s *odp_timer_pool_t;

 #define ODP_TIMER_POOL_INVALID NULL

-typedef uint32_t odp_timer_t;
+typedef ODP_HANDLE_T(odp_timer_t);

-#define ODP_TIMER_INVALID ((uint32_t)~0U)
+#define ODP_TIMER_INVALID _odp_cast_scalar(odp_timer_t, 0x)

-typedef void *odp_timeout_t;
+typedef ODP_HANDLE_T(odp_timeout_t);

-#define ODP_TIMEOUT_INVALID NULL
+#define ODP_TIMEOUT_INVALID _odp_cast_scalar(odp_timeout_t, 0)

 /**
  * @}
diff --git a/platform/linux-generic/odp_timer.c
b/platform/linux-generic/odp_timer.c
index cd8365c..6b84309 100644
--- a/platform/linux-generic/odp_timer.c
+++ b/platform/linux-generic/odp_timer.c
@@ -184,7 +184,7 @@ static odp_timer_pool
*timer_pool[MAX_TIMER_POOLS];

 static inline odp_timer_pool *handle_to_tp(odp_timer_t hdl)
 {
-   uint32_t tp_idx = hdl >> INDEX_BITS;
+   uint32_t tp_idx = _odp_typeval(hdl) >> INDEX_BITS;
if (odp_likely(tp_idx < MAX_TIMER_POOLS)) {
odp_timer_pool *tp = timer_pool[tp_idx];
if (odp_likely(tp != NULL))
@@ -196,7 +196,7 @@ static inline odp_timer_pool
*handle_to_tp(odp_timer_t hdl)
 static inline uint32_t handle_to_idx(odp_timer_t hdl,
struct odp_timer_pool_s *tp)
 {
-   uint32_t idx = hdl & ((1U << INDEX_BITS) - 1U);
+   uint32_t idx = _odp_typeval(hdl) & ((1U << INDEX_BITS) - 1U);
PREFETCH(&tp->tick_buf[idx]);
if (odp_likely(idx < odp_atomic_load_u32(&tp->high_wm)))
return idx;
@@ -207,7 +207,7 @@ static inline odp_timer_t
tp_idx_to_handle(struct odp_timer_pool_s *tp,
uint32_t idx)
 {
ODP_ASSERT(idx < (1U << INDEX_BITS));
-   return (tp->tp_idx << INDEX_BITS) | idx;
+   return _odp_cast_scalar(odp_timer_t, (tp->tp_idx <<
INDEX_BITS) | idx);
 }

 /* Forward declarations */
diff --git a/test/validation/timer/timer.c
b/test/validation/timer/timer.c
index c8ef785..6b484cd 100644
--- a/test/validation/timer/timer.c
+++ b/test/validation/timer/timer.c
@@ -299,7 +299,7 @@ static void *worker_entrypoint(void *ar

[lng-odp] [Bug 2224] Coverity: Negative return

[bugzilla-daemon]

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

--- Comment #3 from Maxim Uvarov  ---
commit 01c3f0d8b945dc276f31cde88c1bf4fdcb7ad83b
Author: Maxim Uvarov 
Date:   Thu May 12 18:20:41 2016 +0300

validation: pktio: fix assert for odp_pktin_queue ret code

Fix wrong return code check due to assignment to unsigned variable.
https://bugs.linaro.org/show_bug.cgi?id=2224

Signed-off-by: Maxim Uvarov 
Reviewed-by: Bill Fischofer 

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

Re: [lng-odp] [PATCH] linux-generic:pktio mmap: fix nb_rx overflow

[Maxim Uvarov]

Merged,
Maxim.

On 05/12/16 21:44, Bill Fischofer wrote:



On Thu, May 12, 2016 at 10:24 AM, Maxim Uvarov 
mailto:maxim.uva...@linaro.org>> wrote:


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

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


Reviewed-by: Bill Fischofer >


---
 platform/linux-generic/pktio/socket_mmap.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/platform/linux-generic/pktio/socket_mmap.c
b/platform/linux-generic/pktio/socket_mmap.c
index 4170456..22a762f 100644
--- a/platform/linux-generic/pktio/socket_mmap.c
+++ b/platform/linux-generic/pktio/socket_mmap.c
@@ -154,7 +154,7 @@ static inline unsigned
pkt_mmap_v2_rx(pktio_entry_t *pktio_entry,
int pkt_len;
struct ethhdr *eth_hdr;
unsigned i = 0;
-   uint8_t nb_rx = 0;
+   unsigned nb_rx = 0;
struct ring *ring;
int ret;

--
2.7.1.250.gff4ea60

___
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

[lng-odp] [Bug 2224] Coverity: Negative return

[bugzilla-daemon]

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

Maxim Uvarov  changed:

   What|Removed |Added

 Status|CONFIRMED   |RESOLVED
 Resolution|--- |FIXED

--- Comment #4 from Maxim Uvarov  ---
commit 5b895ff75edeca76e6cb94618e4ca9c4f7990489
Author: Maxim Uvarov 
Date:   Thu May 12 18:24:28 2016 +0300

linux-generic:pktio mmap: fix nb_rx overflow

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

Signed-off-by: Maxim Uvarov 
Reviewed-by: Bill Fischofer 

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

Re: [lng-odp] [PATCH 1/3] test: scheduling: simplify code structure

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

Ping for merge. I’ll continue scheduler interface development on top of this.

-Petri

From: EXT Bill Fischofer [mailto:bill.fischo...@linaro.org]
Sent: Thursday, May 12, 2016 2:30 AM
To: Savolainen, Petri (Nokia - FI/Espoo) 
Cc: LNG ODP Mailman List 
Subject: Re: [lng-odp] [PATCH 1/3] test: scheduling: simplify code structure

For this series:

Reviewed-and-tested-by: Bill Fischofer 
mailto:bill.fischo...@linaro.org>>

On Wed, May 11, 2016 at 7:38 AM, Petri Savolainen 
mailto:petri.savolai...@nokia.com>> wrote:
Recorded queue and pool handles into shared data, so that
multiple lookups (and related name generation code) can be
avoided. Use single function (enqueue_events()) to enqueue
events into queues on all test cases.

Signed-off-by: Petri Savolainen 
mailto:petri.savolai...@nokia.com>>
---
 test/performance/odp_scheduling.c | 282 --
 1 file changed, 87 insertions(+), 195 deletions(-)

diff --git a/test/performance/odp_scheduling.c 
b/test/performance/odp_scheduling.c
index 6592277..ec913ae 100644
--- a/test/performance/odp_scheduling.c
+++ b/test/performance/odp_scheduling.c
@@ -32,6 +32,7 @@
 #define MSG_POOL_SIZE (4*1024*1024) /**< Message pool size */
 #define MAX_ALLOCS35/**< Alloc burst size */
 #define QUEUES_PER_PRIO   64/**< Queue per priority */
+#define NUM_PRIOS 2 /**< Number of tested priorities */
 #define QUEUE_ROUNDS  (512*1024)/**< Queue test rounds */
 #define ALLOC_ROUNDS  (1024*1024)   /**< Alloc test rounds */
 #define MULTI_BUFS_MAX4 /**< Buffer burst size */
@@ -52,13 +53,13 @@ typedef struct {
int proc_mode;  /**< Process mode */
 } test_args_t;

-
 /** Test global variables */
 typedef struct {
-   odp_barrier_t barrier;/**< @private Barrier for test synchronisation */
+   odp_barrier_t barrier;
+   odp_pool_tpool;
+   odp_queue_t   queue[NUM_PRIOS][QUEUES_PER_PRIO];
 } test_globals_t;

-
 /**
  * @internal Clear all scheduled queues. Retry to be sure that all
  * buffers have been scheduled.
@@ -78,89 +79,45 @@ static void clear_sched_queues(void)
 }

 /**
- * @internal Create a single queue from a pool of buffers
+ * @internal Enqueue events into queues
  *
- * @param thr  Thread
- * @param msg_pool  Buffer pool
- * @param prio   Queue priority
+ * @param thrThread
+ * @param prio   Queue priority
+ * @param num_queues Number of queues
+ * @param num_events Number of events
+ * @param globalsTest shared data
  *
  * @return 0 if successful
  */
-static int create_queue(int thr, odp_pool_t msg_pool, int prio)
+static int enqueue_events(int thr, int prio, int num_queues, int num_events,
+ test_globals_t *globals)
 {
-   char name[] = "sched_XX_00";
odp_buffer_t buf;
odp_queue_t queue;
+   int i, j, k;

-   buf = odp_buffer_alloc(msg_pool);
-
-   if (!odp_buffer_is_valid(buf)) {
-   LOG_ERR("  [%i] msg_pool alloc failed\n", thr);
-   return -1;
-   }
-
-   name[6] = '0' + prio/10;
-   name[7] = '0' + prio - 10*(prio/10);
-
-   queue = odp_queue_lookup(name);
-
-   if (queue == ODP_QUEUE_INVALID) {
-   LOG_ERR("  [%i] Queue %s lookup failed.\n", thr, name);
-   return -1;
-   }
-
-   if (odp_queue_enq(queue, odp_buffer_to_event(buf))) {
-   LOG_ERR("  [%i] Queue enqueue failed.\n", thr);
-   odp_buffer_free(buf);
-   return -1;
-   }
-
-   return 0;
-}
-
-/**
- * @internal Create multiple queues from a pool of buffers
- *
- * @param thr  Thread
- * @param msg_pool  Buffer pool
- * @param prio   Queue priority
- *
- * @return 0 if successful
- */
-static int create_queues(int thr, odp_pool_t msg_pool, int prio)
-{
-   char name[] = "sched_XX_YY";
-   odp_buffer_t buf;
-   odp_queue_t queue;
-   int i;
-
-   name[6] = '0' + prio/10;
-   name[7] = '0' + prio - 10*(prio/10);
+   if (prio == ODP_SCHED_PRIO_HIGHEST)
+   i = 0;
+   else
+   i = 1;

/* Alloc and enqueue a buffer per queue */
-   for (i = 0; i < QUEUES_PER_PRIO; i++) {
-   name[9]  = '0' + i/10;
-   name[10] = '0' + i - 10*(i/10);
-
-   queue = odp_queue_lookup(name);
+   for (j = 0; j < num_queues; j++) {
+   queue = globals->queue[i][j];

-   if (queue == ODP_QUEUE_INVALID) {
-   LOG_ERR("  [%i] Queue %s lookup failed.\n", thr,
-   name);
-   return -1;
-   }
+   for (k = 0; k < num_events; k++) {
+   buf = odp_buffer_alloc(globals->pool);

-   buf = odp_buffer_alloc(msg_pool);
-
-   if (!odp_buffer_is_valid(buf)) {
-   LOG_ERR("  [%i] msg_pool alloc failed\n", thr);
-

Re: [lng-odp] [PATCH] validation: pktio: fix assert for odp_pktin_queue ret code

[Maxim Uvarov]

Merged,
Maxim.

On 05/12/16 21:43, Bill Fischofer wrote:



On Thu, May 12, 2016 at 10:20 AM, Maxim Uvarov 
mailto:maxim.uva...@linaro.org>> wrote:


Fix wrong return code check due to assignment to unsigned variable.
https://bugs.linaro.org/show_bug.cgi?id=2224

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


Reviewed-by: Bill Fischofer >


---
 test/validation/pktio/pktio.c | 7 ---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/test/validation/pktio/pktio.c
b/test/validation/pktio/pktio.c
index a51e0b2..7b7c0a5 100644
--- a/test/validation/pktio/pktio.c
+++ b/test/validation/pktio/pktio.c
@@ -493,9 +493,9 @@ static int recv_packets_tmo(odp_pktio_t pktio,
odp_packet_t pkt_tbl[],
odp_pktin_queue_t pktin[MAX_QUEUES];
odp_time_t ts1, ts2;
int num_rx = 0;
+   int num_q;
int i;
int n;
-   unsigned num_q;
unsigned from_val;
unsigned *from = NULL;

@@ -513,7 +513,8 @@ static int recv_packets_tmo(odp_pktio_t pktio,
odp_packet_t pkt_tbl[],
n = odp_pktin_recv_tmo(pktin[0], pkt_tmp,
num - num_rx,
   tmo);
else
-   n = odp_pktin_recv_mq_tmo(pktin, num_q,
from, pkt_tmp,
+   n = odp_pktin_recv_mq_tmo(pktin,
(unsigned)num_q,
+ from, pkt_tmp,
  num - num_rx, tmo);
ts2 = odp_time_global();

@@ -526,7 +527,7 @@ static int recv_packets_tmo(odp_pktio_t pktio,
odp_packet_t pkt_tbl[],
odp_packet_free(pkt_tmp[i]);
}
if (mode == RECV_MQ_TMO)
-   CU_ASSERT(from_val < num_q);
+   CU_ASSERT(from_val < (unsigned)num_q);
} while (num_rx < num);

if (tmo == ODP_PKTIN_WAIT)
--
2.7.1.250.gff4ea60

___
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] validation: timer: fix handle unused tmo

[Maxim Uvarov]

merged,
Maxim.

On 05/12/16 23:14, Bill Fischofer wrote:



On Thu, May 12, 2016 at 2:35 PM, Maxim Uvarov > wrote:


Current validation test can produce bug:
timer.c:250:handle_tmo():Wrong tick: expected 18446744073709551615
actual 149
FAILED
1. timer.c:246  - CU_FAIL("Wrong status (stale) for fresh
timeout")
2. timer.c:251  - CU_FAIL("odp_timeout_tick() wrong tick")
Which caused with wrong destroy sequence:
timer_free() calls timer_cancel(tp, idx, TMO_UNUSED).
which is: ((uint64_t)0x) or 18446744073709551615
and test calls handle_tmo() for event with TMO_UNUSED.
To fix reoder destroy sequence
odp_timer_cancel()->sleep()->handle_tmo()->
->odp_timer_free().

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


Reviewed-by: Bill Fischofer >


---
 test/validation/timer/timer.c | 8 ++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/test/validation/timer/timer.c
b/test/validation/timer/timer.c
index aad11aa..a396405 100644
--- a/test/validation/timer/timer.c
+++ b/test/validation/timer/timer.c
@@ -397,8 +397,6 @@ static void *worker_entrypoint(void *arg
TEST_UNUSED)
/* Cancel too late, timer already expired and
 * timeout enqueued */
nstale++;
-   if (odp_timer_free(tt[i].tim) != ODP_EVENT_INVALID)
-   CU_FAIL("odp_timer_free");
}

LOG_DBG("Thread %u: %" PRIu32 " timers set\n", thr, nset);
@@ -429,6 +427,12 @@ static void *worker_entrypoint(void *arg
TEST_UNUSED)
break;
}
}
+
+   for (i = 0; i < allocated; i++) {
+   if (odp_timer_free(tt[i].tim) != ODP_EVENT_INVALID)
+   CU_FAIL("odp_timer_free");
+   }
+
/* Check if there any more (unexpected) events */
odp_event_t ev = odp_queue_deq(queue);
if (ev != ODP_EVENT_INVALID)
--
2.7.1.250.gff4ea60

___
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] validation: time: remove print and add verbose faults

[Maxim Uvarov]

Merged,
Maxim.

On 05/12/16 22:05, Bill Fischofer wrote:



On Thu, May 12, 2016 at 6:22 AM, Maxim Uvarov > wrote:


Having system call inside loop couse unpredictable delay
as the result wrong time diff calculation. Just removing
print make hole test execution shorter on 0.1 seconds
according to cunit stats. Also make verbose errors in
places where we out of limit in cunit. This should be
very helpful to understand DELAY_TOLERANCE value suitable
for all platforms.

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


Reviewed-by: Bill Fischofer >


---
 test/validation/time/time.c | 35 +++
 1 file changed, 27 insertions(+), 8 deletions(-)

diff --git a/test/validation/time/time.c b/test/validation/time/time.c
index da456ea..f7f3d14 100644
--- a/test/validation/time/time.c
+++ b/test/validation/time/time.c
@@ -331,7 +331,6 @@ static void time_test_wait_until(time_cb time,
time_from_ns_cb time_from_ns)
for (i = 0; i < WAIT_SECONDS; i++) {
wait = odp_time_sum(wait, second);
odp_time_wait_until(wait);
-   printf("%d..", i + 1);
}
end_time = time();

@@ -341,8 +340,19 @@ static void time_test_wait_until(time_cb
time, time_from_ns_cb time_from_ns)
upper_limit = time_from_ns(WAIT_SECONDS * ODP_TIME_SEC_IN_NS +
   DELAY_TOLERANCE);

-   CU_ASSERT(odp_time_cmp(wait, lower_limit) >= 0);
-   CU_ASSERT(odp_time_cmp(wait, upper_limit) <= 0);
+   if (odp_time_cmp(wait, lower_limit) < 0) {
+   fprintf(stderr, "Exceed lower limit: "
+   "wait is %" PRIu64 ", lower_limit %"
PRIu64 "\n",
+   odp_time_to_ns(wait),
odp_time_to_ns(lower_limit));
+   CU_FAIL("Exceed lower limit\n");
+   }
+
+   if (odp_time_cmp(wait, upper_limit) > 0) {
+   fprintf(stderr, "Exceed upper limit: "
+   "wait is %" PRIu64 ", upper_limit %"
PRIu64 "\n",
+   odp_time_to_ns(wait),
odp_time_to_ns(lower_limit));
+   CU_FAIL("Exceed upper limit\n");
+   }
 }

 void time_test_local_wait_until(void)
@@ -362,10 +372,8 @@ void time_test_wait_ns(void)
odp_time_t start_time, end_time, diff;

start_time = odp_time_local();
-   for (i = 0; i < WAIT_SECONDS; i++) {
+   for (i = 0; i < WAIT_SECONDS; i++)
odp_time_wait_ns(ODP_TIME_SEC_IN_NS);
-   printf("%d..", i + 1);
-   }
end_time = odp_time_local();

diff = odp_time_diff(end_time, start_time);
@@ -375,8 +383,19 @@ void time_test_wait_ns(void)
upper_limit = odp_time_local_from_ns(WAIT_SECONDS *
ODP_TIME_SEC_IN_NS +
 DELAY_TOLERANCE);

-   CU_ASSERT(odp_time_cmp(diff, lower_limit) >= 0);
-   CU_ASSERT(odp_time_cmp(diff, upper_limit) <= 0);
+   if (odp_time_cmp(diff, lower_limit) < 0) {
+   fprintf(stderr, "Exceed lower limit: "
+   "diff is %" PRIu64 ", lower_limit %"
PRIu64 "\n",
+   odp_time_to_ns(diff),
odp_time_to_ns(lower_limit));
+   CU_FAIL("Exceed lower limit\n");
+   }
+
+   if (odp_time_cmp(diff, upper_limit) > 0) {
+   fprintf(stderr, "Exceed upper limit: "
+   "diff is %" PRIu64 ", upper_limit %"
PRIu64 "\n",
+   odp_time_to_ns(diff),
odp_time_to_ns(lower_limit));
+   CU_FAIL("Exceed upper limit\n");
+   }
 }

 static void time_test_to_u64(time_cb time)
--
2.7.1.250.gff4ea60

___
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 1/2] linux-generic: includes: typo correction

[Maxim Uvarov]

Merged,
Maxim.

On 05/12/16 20:59, Zoltan Kiss wrote:

Reviewed-by: Zoltan Kiss 

On 12/05/16 17:29, Bill Fischofer wrote:

Correct typo in spelling of _LARGEFILE64_SOURCE. This also removes an
extraneous non-ascii character from the source file.

Suggested-by: Zoltan Kiss 
Signed-off-by: Bill Fischofer 
---
  platform/linux-generic/include/odp_posix_extensions.h | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/platform/linux-generic/include/odp_posix_extensions.h 
b/platform/linux-generic/include/odp_posix_extensions.h

index 4b6e335..2c468b4 100644
--- a/platform/linux-generic/include/odp_posix_extensions.h
+++ b/platform/linux-generic/include/odp_posix_extensions.h
@@ -17,7 +17,7 @@
   * Enable POSIX and GNU extensions
   *
   * This macro defines:
- *   o  _BSD_SOURCE, _SVID_SOURCE, _ATFILE_SOURCE, 
_LARGE‐FILE64_SOURCE,

+ *   o  _BSD_SOURCE, _SVID_SOURCE, _ATFILE_SOURCE, _LARGEFILE64_SOURCE,
   *  _ISOC99_SOURCE, _XOPEN_SOURCE_EXTENDED, _POSIX_SOURCE
   *   o  _POSIX_C_SOURCE with the value:
   ** 200809L since  glibc v2.10 (== POSIX.1-2008 base 
specification)



___
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

[lng-odp] [Bug 2221] IPC maintainability issues identified by coverity

[bugzilla-daemon]

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

Maxim Uvarov  changed:

   What|Removed |Added

 Resolution|--- |FIXED
 Status|CONFIRMED   |RESOLVED

--- Comment #1 from Maxim Uvarov  ---
commit cc53ba882a26ec34d75681d9870d476e4b70e75b
Author: Maxim Uvarov 
Date:   Thu May 12 21:53:57 2016 +0300

linux-generic: pktio: ipc: remove not needed assignment

Not used assigned, should just removed.
https://bugs.linaro.org/show_bug.cgi?id=2221

Signed-off-by: Maxim Uvarov 
Reviewed-by: Bill Fischofer 

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

Re: [lng-odp] [PATCH] linux-generic: pktio: ipc: remove not needed assignment

[Maxim Uvarov]

Merged,
Maxim.

On 05/13/16 02:41, Bill Fischofer wrote:



On Thu, May 12, 2016 at 1:56 PM, Maxim Uvarov > wrote:


Not used assigned, should just removed.
https://bugs.linaro.org/show_bug.cgi?id=2221

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


Reviewed-by: Bill Fischofer >


---
 platform/linux-generic/pktio/ipc.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/platform/linux-generic/pktio/ipc.c
b/platform/linux-generic/pktio/ipc.c
index b16c611..12cc286 100644
--- a/platform/linux-generic/pktio/ipc.c
+++ b/platform/linux-generic/pktio/ipc.c
@@ -627,8 +627,6 @@ static int ipc_pktio_send(pktio_entry_t
*pktio_entry,
pkt_table_mapped[i] = pkt;
}

-   rbuf_p = (void *)&pkt;
-
/* buf_hdr.addr can not be used directly in remote
process,
 * convert it to offset
 */
--
2.7.1.250.gff4ea60

___
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] [PATCHv2] validation: pktio: initialize pkt_seq

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

Reviewed-and-tested-by: Matias Elo 

> -Original Message-
> From: lng-odp [mailto:lng-odp-boun...@lists.linaro.org] On Behalf Of EXT Maxim
> Uvarov
> Sent: Friday, May 13, 2016 3:45 PM
> To: lng-odp@lists.linaro.org
> Subject: [lng-odp] [PATCHv2] validation: pktio: initialize pkt_seq
> 
> Initialize to zero pkt_seq array first used
> in recv_packets_tmo().
> https://bugs.linaro.org/show_bug.cgi?id=
> 
> Signed-off-by: Maxim Uvarov 
> ---
>  test/validation/pktio/pktio.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/test/validation/pktio/pktio.c b/test/validation/pktio/pktio.c
> index a51e0b2..d035a26 100644
> --- a/test/validation/pktio/pktio.c
> +++ b/test/validation/pktio/pktio.c
> @@ -919,9 +919,11 @@ static void test_recv_tmo(recv_tmo_mode_e mode)
>   ret = odp_pktout_queue(pktio_tx, &pktout_queue, 1);
>   CU_ASSERT_FATAL(ret > 0);
> 
> + memset(pkt_seq, 0, sizeof(pkt_seq));
> +
>   /* No packets sent yet, so should wait */
>   ns = 100 * ODP_TIME_MSEC_IN_NS;
> - ret = recv_packets_tmo(pktio_rx, pkt_tbl, pkt_seq, 1, mode,
> + ret = recv_packets_tmo(pktio_rx, &pkt_tbl[0], &pkt_seq[0], 1, mode,
>  odp_pktin_wait_time(ns), ns);
>   CU_ASSERT(ret == 0);
> 
> --
> 2.7.1.250.gff4ea60
> 
> ___
> 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

[lng-odp] [PATCHv2] validation: pktio: initialize pkt_seq

[Maxim Uvarov]

Initialize to zero pkt_seq array first used
in recv_packets_tmo().
https://bugs.linaro.org/show_bug.cgi?id=

Signed-off-by: Maxim Uvarov 
---
 test/validation/pktio/pktio.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/test/validation/pktio/pktio.c b/test/validation/pktio/pktio.c
index a51e0b2..d035a26 100644
--- a/test/validation/pktio/pktio.c
+++ b/test/validation/pktio/pktio.c
@@ -919,9 +919,11 @@ static void test_recv_tmo(recv_tmo_mode_e mode)
ret = odp_pktout_queue(pktio_tx, &pktout_queue, 1);
CU_ASSERT_FATAL(ret > 0);
 
+   memset(pkt_seq, 0, sizeof(pkt_seq));
+
/* No packets sent yet, so should wait */
ns = 100 * ODP_TIME_MSEC_IN_NS;
-   ret = recv_packets_tmo(pktio_rx, pkt_tbl, pkt_seq, 1, mode,
+   ret = recv_packets_tmo(pktio_rx, &pkt_tbl[0], &pkt_seq[0], 1, mode,
   odp_pktin_wait_time(ns), ns);
CU_ASSERT(ret == 0);
 
-- 
2.7.1.250.gff4ea60

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

Re: [lng-odp] [PATCH] validation: pktio: initialize pkt_seq

[Maxim Uvarov]

On 05/13/16 11:58, Elo, Matias (Nokia - FI/Espoo) wrote:

-Original Message-
From: lng-odp [mailto:lng-odp-boun...@lists.linaro.org] On Behalf Of EXT Maxim
Uvarov
Sent: Thursday, May 12, 2016 6:51 PM
To: lng-odp@lists.linaro.org
Subject: [lng-odp] [PATCH] validation: pktio: initialize pkt_seq

recv_packets_tmo() has if for not initialized
pkt_seq array, while init is done in create_packets(),
so we need just reorder that lines.
https://bugs.linaro.org/show_bug.cgi?id=

Signed-off-by: Maxim Uvarov 
---
  test/validation/pktio/pktio.c | 10 +-
  1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/test/validation/pktio/pktio.c b/test/validation/pktio/pktio.c
index a51e0b2..90adc02 100644
--- a/test/validation/pktio/pktio.c
+++ b/test/validation/pktio/pktio.c
@@ -919,16 +919,16 @@ static void test_recv_tmo(recv_tmo_mode_e mode)
ret = odp_pktout_queue(pktio_tx, &pktout_queue, 1);
CU_ASSERT_FATAL(ret > 0);

+   ret = create_packets(pkt_tbl, pkt_seq, test_pkt_count, pktio_tx,
+pktio_rx);
+   CU_ASSERT_FATAL(ret == test_pkt_count);
+

I think it would clearer to simply initialize pkt_seq to 0 and leave 
create_packets() where it is now and the packets are actually used.

The pktio_pkt_seq() helper used inside recv_packets_tmo() checks also received 
packets' magic numbers, so the sequence number can be 0.

-Matias
yes, you are right.  I did  memset(0), initially but that thought it 
will be good to reorder lines. But did not catch this reference.

Will send v2.

Maxim.

/* No packets sent yet, so should wait */
ns = 100 * ODP_TIME_MSEC_IN_NS;
-   ret = recv_packets_tmo(pktio_rx, pkt_tbl, pkt_seq, 1, mode,
+   ret = recv_packets_tmo(pktio_rx, &pkt_tbl[0], &pkt_seq[0], 1, mode,
   odp_pktin_wait_time(ns), ns);
CU_ASSERT(ret == 0);

-   ret = create_packets(pkt_tbl, pkt_seq, test_pkt_count, pktio_tx,
-pktio_rx);
-   CU_ASSERT_FATAL(ret == test_pkt_count);
-
ret = odp_pktout_send(pktout_queue, pkt_tbl, test_pkt_count);
CU_ASSERT_FATAL(ret == test_pkt_count);

--
2.7.1.250.gff4ea60

___
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

[lng-odp] [Bug 2227] classification_main: Conditional jump or move depends on uninitialised value

[bugzilla-daemon]

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

--- Comment #1 from Bala Manoharan  ---
v1 submitted: https://patches.linaro.org/patch/67762/

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

[lng-odp] [PATCH] fix: classification: uninitialized pmr param value

[Balasubramanian Manoharan]

Fix memory leaked caused by uninitialized pmr param struct

Fixes: https://bugs.linaro.org/show_bug.cgi?id=2227
 
Signed-off-by: Balasubramanian Manoharan 
---
 test/validation/classification/odp_classification_test_pmr.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/test/validation/classification/odp_classification_test_pmr.c 
b/test/validation/classification/odp_classification_test_pmr.c
index 344503a..7c7d07e 100644
--- a/test/validation/classification/odp_classification_test_pmr.c
+++ b/test/validation/classification/odp_classification_test_pmr.c
@@ -250,6 +250,7 @@ void classification_test_pmr_term_tcp_sport(void)
cos = odp_cls_cos_create(cosname, &cls_param);
CU_ASSERT_FATAL(cos != ODP_COS_INVALID);
 
+   odp_cls_pmr_param_init(&pmr_param);
pmr_param.term = ODP_PMR_TCP_SPORT;
pmr_param.match.value = &val;
pmr_param.match.mask = &mask;
@@ -474,6 +475,7 @@ void classification_test_pmr_term_udp_sport(void)
cos = odp_cls_cos_create(cosname, &cls_param);
CU_ASSERT_FATAL(cos != ODP_COS_INVALID);
 
+   odp_cls_pmr_param_init(&pmr_param);
pmr_param.term = ODP_PMR_UDP_SPORT;
pmr_param.match.value = &val;
pmr_param.match.mask = &mask;
@@ -690,6 +692,7 @@ void classification_test_pmr_term_dmac(void)
cos = odp_cls_cos_create(cosname, &cls_param);
CU_ASSERT_FATAL(cos != ODP_COS_INVALID);
 
+   odp_cls_pmr_param_init(&pmr_param);
pmr_param.term = ODP_PMR_DMAC;
pmr_param.match.value = &val;
pmr_param.match.mask = &mask;
-- 
1.9.1

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

Re: [lng-odp] [PATCHv2 1/2] doc: userguide: add timer/timeout state diagram

[Bill Fischofer]

On Fri, May 13, 2016 at 4:50 AM, Christophe Milard <
christophe.mil...@linaro.org> wrote:

>
>
> On 12 May 2016 at 19:44, Bill Fischofer  wrote:
>
>>
>>
>> On Thu, May 12, 2016 at 8:24 AM, Christophe Milard <
>> christophe.mil...@linaro.org> wrote:
>>
>>> Hi Bill,
>>> Just sent you a v4 containing my proposal regarding the FSMs and the
>>> text around it.
>>> I still have a few comments:
>>>
>>> First, your FSM mixed events and actions on the transition arrow. I
>>> could not get graphviz to have 2 colors on the same transition (one for
>>> events, one for actions), so I have restricted them to events. The only
>>> action worth mentioning is the event queuing action which is clearly
>>> described by your text (and a reminder in mine). The square node could be a
>>> way to represent it, but I don't like mixing event and action
>>> undistinguished. As long as it is clear and consistent (it seems I have a
>>> need for that these days...), I won't bother you.
>>>
>>
>> I think it's fine this way.  The expiration/schedule call adds detail
>> because a timeout isn't actually delivered until the scheduler returns it
>> to a thread, but I think this is clear enough here.
>>
>>
>
> On V5:
> Maybe the error originated from me (?), but I see that the TO_Delivered
> node is double ringed: Double ringed node would show the start state, so I
> think it shouldn't be double ringed. If the intention was to show the final
> state (though there isn't really one here: a TO event can be reused I
> assume), then the Timer_Expired state should be double ringed as well. But
> as mentionned, as I cannot clearly see any final state here, I would simply
> remove the double ring from the TO_delivered state.
>

Yes, I noticed that too after I sent it.  I'll remove it in v6.



>
>>> Then a few questions:
>>>
>>> 1)My Time-out FSM does not show what happens to a time-out event when an
>>> odp_timer_free() is called while the timeout is in delivered state. I
>>> assume the time-out remains in delivered state until a
>>> odp_event_free/timeout_free() is called manually, but I am not sure (having
>>> 2 different FSM actally clarify this point). You are welcome to update my
>>> FSMs if we should keep them.
>>>
>>
>> Nothing happens to the event since the association between timer and
>> timeout ends as soon as the timer expires. At that point the timer is in
>> the "Timer_Expired" state and the Timeout moves to the "Timeout_Delivered"
>> state (Timeout_Pending in my previous diagram). So they're no longer
>> coupled and operations on one no longer affect the other.
>>
>
> In that case there should be a transition TO_delivered->TO_delivered
> labeled odp_timer_free()
>

I don't think that is necessary. The arrows show relevant/permitted state
transitions. Otherwise we'd have most other ODP APIs doing the same. You
stay in TO_Delivered until one of the APIs shown on the arrows is called.


>
>
>>
>>
>>>
>>> 2)Your text says "Following start, applications may allocate, set,
>>> cancel, and free, timers...". I guess the comma after "timer" is a typo.
>>>
>>
>> Yes, thanks for the correction.
>>
>>
>>>
>>> 3)The last sentence in your text is: "However, upon receiving a timeout
>>> event the application can use the odp_timeout_fresh() API to inquire
>>> whether the timeout event is fresh or stale". What is the definition of a
>>> stale timer?
>>>
>>
>> This was a back reference to the earlier sentence discussing
>> odp_timer_cancel(): "An attempted cancel will fail if the timer is not set
>> or if it has already
>> expired."  However, I recall this was an area that Ola and Petri debated
>> extensively when these were first defined. There is an implicit imprecision
>> in any efficient timer system since independent threads can be setting,
>> resetting, and cancelling timers while they are "in flight". Generally when
>> a timeout event is received it's the responsibility of the application to
>> determine whether that event is meaningful or not based on other
>> information it has (e.g., it's a TCP delayed ACK timer, but we've just
>> received a packet on that connection so I should ignore this event, etc.).
>>  odp_timer_fresh() is a hook that allows the implementation to communicate
>> staleness information if it knows something that the application might not
>> otherwise know, but I expect most applications will just ignore this as a
>> reported "fresh" timer may still not be meaningful from the application's
>> perspective for reasons it knows.
>>
>
> Not sure I understand that. I'll need explanations, and I guess the doc as
> well.
>

I'll try to rework and expand on that a bit but the bottom line is I don't
think that API is particularly well defined or useful.


>
>>
>>>
>>> 4) And I kept the best for the end... :-)
>>> What about the validity of the pointer returned by
>>> odp_timeout_user_ptr() when the timer was set by odp thread A on a queue
>>> belonging to ODP thread B (B != A). Interresting question isn't it? :-).
>>>
>>>
>> The user

[lng-odp] [PATCH] linux-generic: dpdk: use memset() to initialize dev_info

[Matias Elo]

Clang build fails due to a missing field in initializer
error. This is a known clang bug
(https://llvm.org/bugs/show_bug.cgi?id=21689). Circumvent
this by using memset() instead.

Signed-off-by: Matias Elo 
---
 platform/linux-generic/pktio/dpdk.c | 12 +---
 1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/platform/linux-generic/pktio/dpdk.c 
b/platform/linux-generic/pktio/dpdk.c
index e6af8fb..831dc26 100644
--- a/platform/linux-generic/pktio/dpdk.c
+++ b/platform/linux-generic/pktio/dpdk.c
@@ -177,11 +177,13 @@ static int dpdk_netdev_is_valid(const char *s)
 
 static uint32_t dpdk_vdev_mtu_get(uint8_t port_id)
 {
-   struct rte_eth_dev_info dev_info = {0};
+   struct rte_eth_dev_info dev_info;
struct ifreq ifr;
int sockfd;
uint32_t mtu;
 
+   memset(&dev_info, 0, sizeof(struct rte_eth_dev_info));
+
rte_eth_dev_info_get(port_id, &dev_info);
if_indextoname(dev_info.if_index, ifr.ifr_name);
 
@@ -219,11 +221,13 @@ static uint32_t dpdk_mtu_get(pktio_entry_t *pktio_entry)
 
 static int dpdk_vdev_promisc_mode_get(uint8_t port_id)
 {
-   struct rte_eth_dev_info dev_info = {0};
+   struct rte_eth_dev_info dev_info;
struct ifreq ifr;
int sockfd;
int mode;
 
+   memset(&dev_info, 0, sizeof(struct rte_eth_dev_info));
+
rte_eth_dev_info_get(port_id, &dev_info);
if_indextoname(dev_info.if_index, ifr.ifr_name);
 
@@ -240,11 +244,13 @@ static int dpdk_vdev_promisc_mode_get(uint8_t port_id)
 
 static int dpdk_vdev_promisc_mode_set(uint8_t port_id, int enable)
 {
-   struct rte_eth_dev_info dev_info = {0};
+   struct rte_eth_dev_info dev_info;
struct ifreq ifr;
int sockfd;
int mode;
 
+   memset(&dev_info, 0, sizeof(struct rte_eth_dev_info));
+
rte_eth_dev_info_get(port_id, &dev_info);
if_indextoname(dev_info.if_index, ifr.ifr_name);
 
-- 
1.9.1

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

Re: [lng-odp] process thread and mixed mode

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

From: Christophe Milard [mailto:christophe.mil...@linaro.org]
Sent: Friday, May 13, 2016 11:16 AM
To: Petri Savolainen ; Brian Brooks 
; Bill Fischofer ; Mike 
Holmes ; LNG ODP Mailman List 
; Anders Roxell 
Cc: Tobias Lindquist 
Subject: process thread and mixed mode

Hi,

Let me start in a bit unusual way :-). I do not like my "running things in 
process mode" patch series  that much...
But I do think it should be merged. With the mixed mode.

I think mixed mode is good addition, but would need more options: e.g. 
--odph-num-proc  --odph-num-pthread-per-proc  and thus can be added 
later. Pure proc mode is enough to get things started.

The main goal of the patch series is to push the work regarding a proper and 
consistent definition of some ODP low level concepts and objects. The patch 
series makes things fail, hence pushing for defining and coding things better, 
I hope.  Removing "provocative" things like the mixed mode from it is just 
defeating the purpose of this series: I'll be happy to remove it (and plenty of 
other things) when we know which well documented definition/rule makes this 
code not relevant.
As long as we keep saying that odp threads could be indifferently  linux 
processes or pthreads, I'll be thinking that this series should work, inclusive 
any kind of mixed mode.

However, processes and threads are different things in linux, and I believe 
this distinction should be kept within ODP: Trying to make processes behave as 
pthreads for some ODP objects (such as addresses for shmem objects) does not 
sound like a good idea to me. Yes, this can maybe be done with linux -and even 
possibly without preallocating the memory-, but still, linux creates 2 distinct 
virtual address spaces on fork() so why would ODP put any exception to that?.
...And If we do want to create such exceptions (which I don't like), shouln't 
ODP define its own concept of process and thread as it obviously wouldn't match 
100% the definition of the OS?


A lot of application internal state data may be directly managed with OS. But I 
think we cannot depend 100% on OS shared data (== remove shm API all together), 
since HW/implementation could have multiple ways optimize where/how the memory 
is allocated/mapped (chip internal vs. external memory, huge page mapped vs. 
normal page mapped, IO-MMU mapped, etc) … or when passing pointers to ODP API, 
the API spec may require that the memory is allocated from SHM (application 
would not know if the API function is implemented as HW or SW).


I do nevertheless definitively agree with Petri when saying that that ODP 
applications should be given a way to "share pointers" between odpthreads. 
Taking the "process only" behavior forcing every concurrent execution tasks to 
use tricky offset calculation to find their data is too nasty and inefficient.

I think this will boil down to 2 choices:
1) Take these object definitions from the OS and accept the OS rules.
2) Define our own ODP objects (and possibly defines our own rules)

1)Taking the OS definitions:
Choice 1) is consistent as long as we accept the OS rules. Taking that choice 
would mean that any address remain valid within a process only: shmem pointers 
will not be able to be shared between different processes: I do not see that as 
a big problem as the applications can use pthreads if it needs to share the 
same VA, or can fork() at a conveniant time. Moreover, existing applications 
which should be ported to ODP are probably OK as they would already be using 
pthreads/processes in the correct way.

A more tricky aspect of choice 1) is the development of ODP itself: Operations 
involving many "ODPthreads" will have to cope with both processes and threads 
possibly without a clear view of what is done by the application. ( I guess in 
linux if getpid()==gettid() when calling odp_init_local() one can assume safely 
that the app forked...?).
I guess it is doable: assuming all odpthreads are descendant of the 
instanciation process, ODP can initially reserve shared memory in 
odp_init_global() for its own purpose. It has a price: within ODP (and anything 
external to the app itself, such as drivers) a common VA cannot be assumed. 
ODP/drivers will have be prepared for an app fork at any time and hence do its 
internal stuff as if pointers could not be shared (except within the initially 
allocated memory). This will cost ODP performance.

Taking choice 1 probably also mean that shmen should no longer be part of ODP: 
Allocating memory is as much an OS thing as forking a process... why would 
shmem objects be ODP objects while ODP thread would not be?


Memory is a HW resource (such as e.g. a packet interface), and may have 
different performance/properties depending on SoCs (cpu local memory, chip 
internal memory, memory block carved out from a shared cache, DDR not shared 
with other NUMA nodes, DDR shared with other NUMA nodes, persistent memory, …). 
By default, operating system would give you access to the com

Re: [lng-odp] [PATCH] CHANGELOG: Add details for docs and arch changes

[Mike Holmes]

On 12 May 2016 at 20:13, Bill Fischofer  wrote:

> I don't see how this gets built into a output html file anywhere. Is there
> a prereq patch that's not yet been merged?
>

It does not, the HTML is external, this is the raw data for the human part
of the report that also includes the the ABI and API diff output with the
bug list.


>
> On Tue, May 10, 2016 at 8:46 PM, Mike Holmes 
> wrote:
>
>> Signed-off-by: Mike Holmes 
>> ---
>>  CHANGELOG | 12 
>>  1 file changed, 12 insertions(+)
>>
>> diff --git a/CHANGELOG b/CHANGELOG
>> index d9c66ed..b9d12f5 100644
>> --- a/CHANGELOG
>> +++ b/CHANGELOG
>> @@ -1,3 +1,15 @@
>> +== OpenDataPlane (1.10.1.0)
>> +
>> +=== New Features
>> + Documentation
>> +* The helpers library now generates its own doxygen html guide
>> independently of
>> +  the API.
>> +
>> + Architecture specific differences
>> +* Now an OS must be specifically supported or it will fail in configure.
>> Those
>> +  platforms that do not need any special code can now reference the
>> default
>> +  implementation.
>> +
>>  == OpenDataPlane (1.10.0.0)
>>
>>  === New Features
>> --
>> 2.7.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 PATCH v3 1/2] helper: table: add impl of cuckoo hash table

[Maxim Uvarov]

Hello He,

this patch can be accepted after removing define RETURN_IF_TRUE and 
define RETURN_IF_ERROR.


Thanks,
Maxim.

On 28.04.2016 10:38, HePeng wrote:

Hi,

   We appreciate more comments on the patch. Thanks.
在 2016年4月27日，下午7:35，Raj Murali > 写道：


Agree with Bill.
The original copyright statement should be retained and we need to 
add our's over and above it.


Raj Murali


Raj Murali
Director - Linaro Networking Group
Skype: rajmurali_s  │ M: +91 98450 70135
Linaro.org ***│ *Open source software for ARM 
SoCs



On 27 April 2016 at 16:53, Bill Fischofer > wrote:


I believe the rule is you retain the original copyright from the
base code but can add your own copyright for any
additions/changes. The point is not to erase history but preserve
acknowledgement and the record of the code's BSD provenance.

See http://www.openbsd.org/policy.html for some discussion of
this topic.

On Wed, Apr 27, 2016 at 4:06 AM, Maxim Uvarov
mailto:maxim.uva...@linaro.org>> wrote:

On 04/27/16 11:12, Ru Jia wrote:

Signed-off-by: Ru Jia mailto:ji...@ict.ac.cn>>
---
  helper/Makefile.am|   6 +-
  helper/cuckootable.c  | 746
++
  helper/odph_cuckootable.h |  82 +
  3 files changed, 832 insertions(+), 2 deletions(-)
  create mode 100644 helper/cuckootable.c
  create mode 100644 helper/odph_cuckootable.h

diff --git a/helper/Makefile.am b/helper/Makefile.am
index 8a86eb7..1763b99 100644
--- a/helper/Makefile.am
+++ b/helper/Makefile.am
@@ -27,13 +27,15 @@ noinst_HEADERS = \
 $(srcdir)/odph_debug.h \
 $(srcdir)/odph_hashtable.h \
 $(srcdir)/odph_lineartable.h \
- $(srcdir)/odph_list_internal.h
+ $(srcdir)/odph_list_internal.h \
+ $(srcdir)/odph_cuckootable.h
__LIB__libodphelper_linux_la_SOURCES = \
eth.c \
ip.c \
linux.c \
hashtable.c \
-lineartable.c
+lineartable.c \
+cuckootable.c
lib_LTLIBRARIES = $(LIB)/libodphelper-linux.la

diff --git a/helper/cuckootable.c b/helper/cuckootable.c
new file mode 100644
index 000..64bd894
--- /dev/null
+++ b/helper/cuckootable.c
@@ -0,0 +1,746 @@
+/* Copyright (c) 2015, Linaro Limited
+ * All rights reserved.
+ *
+ * SPDX-License-Identifier:  BSD-3-Clause
+ */
+
+/*-
+ *   BSD LICENSE
+ *
+ *   Copyright(c) 2010-2015 Intel Corporation. All
rights reserved.
+ *   All rights reserved.


Looks like this code was totally rewritten now to ODP. And I
probably we should
check does it make sense to keep Intels copyright here or
not. Any ideas?

Maxim.


+ *
+ *   Redistribution and use in source and binary forms,
with or without
+ *   modification, are permitted provided that the
following conditions
+ *   are met:
+ *
+ * * Redistributions of source code must retain the
above copyright
+ *   notice, this list of conditions and the
following disclaimer.
+ * * Redistributions in binary form must reproduce
the above copyright
+ *   notice, this list of conditions and the
following disclaimer in
+ *   the documentation and/or other materials
provided with the
+ *   distribution.
+ * * Neither the name of Intel Corporation nor the
names of its
+ *   contributors may be used to endorse or promote
products derived
+ *   from this software without specific prior
written permission.
+ *
+ *   THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS
AND CONTRIBUTORS
+ *   "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES,
INCLUDING, BUT NOT
+ *   LIMITED TO, THE IMPLIED WARRANTIES OF
MERCHANTABILITY AND FITNESS FOR
+ *   A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT
SHALL THE COPYRIGHT
+ *   OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT,
INDIRECT, INCIDENTAL,
+ *   SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES
(INCLUDING, BUT NOT
+ *   LIMITED TO, PROCUREMENT OF SUBSTITUTE G

Re: [lng-odp] [PATCH 2/4] linux-generic: packet: remove vlan_s_tag and vlan_c_tag members from odp_packet_hdr_t

[Bala Manoharan]

On 13 May 2016 at 13:46, Elo, Matias (Nokia - FI/Espoo) <
matias@nokia.com> wrote:

> Hi Bala,
>
>
>
> As this code is used in the fast path I’d rather just remove it for now.
> When someone is actually implementing this use-case he/she can choose how
> to do it without causing performance degradation.
>
>
>
> Before this patch set the size of odp_packet_hdr_t was 360 bytes (!).
> Simply by removing unused odp_packet_hdr_t members and optimizing the
> packet_init() function I was able to increase maximum packet throughput by
> almost 1 Mpps (odp_l2fwd, 64B packets, netmap/dpdk pktio).
>

Sure. If the performance boost is by this much makes sense to remove this
now.

Regards,
Bala

> -Matias
>
>
>
>
>
> *From:* EXT Bala Manoharan [mailto:bala.manoha...@linaro.org]
> *Sent:* Friday, May 13, 2016 9:39 AM
> *To:* Elo, Matias (Nokia - FI/Espoo) 
> *Cc:* LNG ODP Mailman List 
> *Subject:* Re: [lng-odp] [PATCH 2/4] linux-generic: packet: remove
> vlan_s_tag and vlan_c_tag members from odp_packet_hdr_t
>
>
>
> There is a use-case in classification, where the packet can be classified
> based on outer/inner vlan tag and for that specific use-case it is better
> to store the vlan value in packet header field.
>
>
>
> Regards,
> Bala
>
>
>
> On 12 May 2016 at 18:06, Matias Elo  wrote:
>
> There is no way to read vlan tags in the ODP API, so don't
> save them.
>
> Signed-off-by: Matias Elo 
> ---
>  platform/linux-generic/include/odp_packet_internal.h | 4 
>  platform/linux-generic/odp_packet.c  | 4 
>  2 files changed, 8 deletions(-)
>
> diff --git a/platform/linux-generic/include/odp_packet_internal.h
> b/platform/linux-generic/include/odp_packet_internal.h
> index 508adf8..2a12503 100644
> --- a/platform/linux-generic/include/odp_packet_internal.h
> +++ b/platform/linux-generic/include/odp_packet_internal.h
> @@ -143,8 +143,6 @@ typedef struct {
> uint32_t l4_offset; /**< offset to L4 hdr (TCP, UDP, SCTP, also
> ICMP) */
> uint32_t payload_offset; /**< offset to payload */
>
> -   uint32_t vlan_s_tag; /**< Parsed 1st VLAN header (S-TAG) */
> -   uint32_t vlan_c_tag; /**< Parsed 2nd VLAN header (C-TAG) */
> uint32_t l3_len; /**< Layer 3 length */
> uint32_t l4_len; /**< Layer 4 length */
>
> @@ -184,8 +182,6 @@ static inline void
> copy_packet_parser_metadata(odp_packet_hdr_t *src_hdr,
> dst_hdr->l4_offset  = src_hdr->l4_offset;
> dst_hdr->payload_offset = src_hdr->payload_offset;
>
> -   dst_hdr->vlan_s_tag = src_hdr->vlan_s_tag;
> -   dst_hdr->vlan_c_tag = src_hdr->vlan_c_tag;
> dst_hdr->l3_len = src_hdr->l3_len;
> dst_hdr->l4_len = src_hdr->l4_len;
>  }
> diff --git a/platform/linux-generic/odp_packet.c
> b/platform/linux-generic/odp_packet.c
> index 8681c08..436265e 100644
> --- a/platform/linux-generic/odp_packet.c
> +++ b/platform/linux-generic/odp_packet.c
> @@ -42,8 +42,6 @@ void packet_parse_reset(odp_packet_hdr_t *pkt_hdr)
> pkt_hdr->l3_offset= ODP_PACKET_OFFSET_INVALID;
> pkt_hdr->l4_offset= ODP_PACKET_OFFSET_INVALID;
> pkt_hdr->payload_offset   = ODP_PACKET_OFFSET_INVALID;
> -   pkt_hdr->vlan_s_tag   = 0;
> -   pkt_hdr->vlan_c_tag   = 0;
>  }
>
>  /**
> @@ -1223,7 +1221,6 @@ int _odp_parse_common(odp_packet_hdr_t *pkt_hdr,
> const uint8_t *ptr)
>
> vlan = (const odph_vlanhdr_t *)parseptr;
> ethtype = odp_be_to_cpu_16(vlan->type);
> -   pkt_hdr->vlan_s_tag = odp_be_to_cpu_16(vlan->tci);
> offset += sizeof(odph_vlanhdr_t);
> parseptr += sizeof(odph_vlanhdr_t);
> }
> @@ -1232,7 +1229,6 @@ int _odp_parse_common(odp_packet_hdr_t *pkt_hdr,
> const uint8_t *ptr)
> pkt_hdr->input_flags.vlan = 1;
> vlan = (const odph_vlanhdr_t *)parseptr;
> ethtype = odp_be_to_cpu_16(vlan->type);
> -   pkt_hdr->vlan_c_tag = odp_be_to_cpu_16(vlan->tci);
> offset += sizeof(odph_vlanhdr_t);
> parseptr += sizeof(odph_vlanhdr_t);
> }
> --
> 1.9.1
>
> ___
> 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] [PATCHv2 1/2] doc: userguide: add timer/timeout state diagram

[Christophe Milard]

On 12 May 2016 at 19:44, Bill Fischofer  wrote:

>
>
> On Thu, May 12, 2016 at 8:24 AM, Christophe Milard <
> christophe.mil...@linaro.org> wrote:
>
>> Hi Bill,
>> Just sent you a v4 containing my proposal regarding the FSMs and the text
>> around it.
>> I still have a few comments:
>>
>> First, your FSM mixed events and actions on the transition arrow. I could
>> not get graphviz to have 2 colors on the same transition (one for events,
>> one for actions), so I have restricted them to events. The only action
>> worth mentioning is the event queuing action which is clearly described by
>> your text (and a reminder in mine). The square node could be a way to
>> represent it, but I don't like mixing event and action undistinguished. As
>> long as it is clear and consistent (it seems I have a need for that these
>> days...), I won't bother you.
>>
>
> I think it's fine this way.  The expiration/schedule call adds detail
> because a timeout isn't actually delivered until the scheduler returns it
> to a thread, but I think this is clear enough here.
>
>

On V5:
Maybe the error originated from me (?), but I see that the TO_Delivered
node is double ringed: Double ringed node would show the start state, so I
think it shouldn't be double ringed. If the intention was to show the final
state (though there isn't really one here: a TO event can be reused I
assume), then the Timer_Expired state should be double ringed as well. But
as mentionned, as I cannot clearly see any final state here, I would simply
remove the double ring from the TO_delivered state.

>
>> Then a few questions:
>>
>> 1)My Time-out FSM does not show what happens to a time-out event when an
>> odp_timer_free() is called while the timeout is in delivered state. I
>> assume the time-out remains in delivered state until a
>> odp_event_free/timeout_free() is called manually, but I am not sure (having
>> 2 different FSM actally clarify this point). You are welcome to update my
>> FSMs if we should keep them.
>>
>
> Nothing happens to the event since the association between timer and
> timeout ends as soon as the timer expires. At that point the timer is in
> the "Timer_Expired" state and the Timeout moves to the "Timeout_Delivered"
> state (Timeout_Pending in my previous diagram). So they're no longer
> coupled and operations on one no longer affect the other.
>

In that case there should be a transition TO_delivered->TO_delivered
labeled odp_timer_free()


>
>
>>
>> 2)Your text says "Following start, applications may allocate, set,
>> cancel, and free, timers...". I guess the comma after "timer" is a typo.
>>
>
> Yes, thanks for the correction.
>
>
>>
>> 3)The last sentence in your text is: "However, upon receiving a timeout
>> event the application can use the odp_timeout_fresh() API to inquire
>> whether the timeout event is fresh or stale". What is the definition of a
>> stale timer?
>>
>
> This was a back reference to the earlier sentence discussing
> odp_timer_cancel(): "An attempted cancel will fail if the timer is not set
> or if it has already
> expired."  However, I recall this was an area that Ola and Petri debated
> extensively when these were first defined. There is an implicit imprecision
> in any efficient timer system since independent threads can be setting,
> resetting, and cancelling timers while they are "in flight". Generally when
> a timeout event is received it's the responsibility of the application to
> determine whether that event is meaningful or not based on other
> information it has (e.g., it's a TCP delayed ACK timer, but we've just
> received a packet on that connection so I should ignore this event, etc.).
>  odp_timer_fresh() is a hook that allows the implementation to communicate
> staleness information if it knows something that the application might not
> otherwise know, but I expect most applications will just ignore this as a
> reported "fresh" timer may still not be meaningful from the application's
> perspective for reasons it knows.
>

Not sure I understand that. I'll need explanations, and I guess the doc as
well.

>
>
>>
>> 4) And I kept the best for the end... :-)
>> What about the validity of the pointer returned by odp_timeout_user_ptr()
>> when the timer was set by odp thread A on a queue belonging to ODP thread B
>> (B != A). Interresting question isn't it? :-).
>>
>>
> The user ptr should really be revised for Tiger Moth to include a length
> (just as we did for queues) so that it can be part of scheduler
> prefetching. But as you know Monarch assumes everything is running in the
> same address space.  Make a note of this (and similar context pointers) as
> this is something we need to cover in Tiger Moth process discussions.
>

OK. Pity you are right here as I saw this as a good opportunity to trigger
some more opinions regarding the the process/thread debate... But you are
right: the aim is Monarch, and monarch just goes thread, so I cannot stop
you there :-)

I suggest we take a small HO as soon as pos

[lng-odp] [Bug 2228] ringtest: Conditional jump or move depends on uninitialised value

[bugzilla-daemon]

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

Yi He  changed:

   What|Removed |Added

 CC||yi...@linaro.org

--- Comment #1 from Yi He  ---
As suggested in log:
ringtest.c:453, rarg.stress_type was used without initialization.
 printf("starting stess test type : %d..\n", rarg.stress_type);

I plan to solve it while together with ODP-444, while converting the test cases
all to cunit framework.

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

Re: [lng-odp] [PATCH] validation: pktio: initialize pkt_seq

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

> -Original Message-
> From: lng-odp [mailto:lng-odp-boun...@lists.linaro.org] On Behalf Of EXT Maxim
> Uvarov
> Sent: Thursday, May 12, 2016 6:51 PM
> To: lng-odp@lists.linaro.org
> Subject: [lng-odp] [PATCH] validation: pktio: initialize pkt_seq
> 
> recv_packets_tmo() has if for not initialized
> pkt_seq array, while init is done in create_packets(),
> so we need just reorder that lines.
> https://bugs.linaro.org/show_bug.cgi?id=
> 
> Signed-off-by: Maxim Uvarov 
> ---
>  test/validation/pktio/pktio.c | 10 +-
>  1 file changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/test/validation/pktio/pktio.c b/test/validation/pktio/pktio.c
> index a51e0b2..90adc02 100644
> --- a/test/validation/pktio/pktio.c
> +++ b/test/validation/pktio/pktio.c
> @@ -919,16 +919,16 @@ static void test_recv_tmo(recv_tmo_mode_e mode)
>   ret = odp_pktout_queue(pktio_tx, &pktout_queue, 1);
>   CU_ASSERT_FATAL(ret > 0);
> 
> + ret = create_packets(pkt_tbl, pkt_seq, test_pkt_count, pktio_tx,
> +  pktio_rx);
> + CU_ASSERT_FATAL(ret == test_pkt_count);
> +

I think it would clearer to simply initialize pkt_seq to 0 and leave 
create_packets() where it is now and the packets are actually used.

The pktio_pkt_seq() helper used inside recv_packets_tmo() checks also received 
packets' magic numbers, so the sequence number can be 0.

-Matias

>   /* No packets sent yet, so should wait */
>   ns = 100 * ODP_TIME_MSEC_IN_NS;
> - ret = recv_packets_tmo(pktio_rx, pkt_tbl, pkt_seq, 1, mode,
> + ret = recv_packets_tmo(pktio_rx, &pkt_tbl[0], &pkt_seq[0], 1, mode,
>  odp_pktin_wait_time(ns), ns);
>   CU_ASSERT(ret == 0);
> 
> - ret = create_packets(pkt_tbl, pkt_seq, test_pkt_count, pktio_tx,
> -  pktio_rx);
> - CU_ASSERT_FATAL(ret == test_pkt_count);
> -
>   ret = odp_pktout_send(pktout_queue, pkt_tbl, test_pkt_count);
>   CU_ASSERT_FATAL(ret == test_pkt_count);
> 
> --
> 2.7.1.250.gff4ea60
> 
> ___
> 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 2/4] linux-generic: packet: remove vlan_s_tag and vlan_c_tag members from odp_packet_hdr_t

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

Hi Bala,

As this code is used in the fast path I’d rather just remove it for now. When 
someone is actually implementing this use-case he/she can choose how to do it 
without causing performance degradation.

Before this patch set the size of odp_packet_hdr_t was 360 bytes (!). Simply by 
removing unused odp_packet_hdr_t members and optimizing the packet_init() 
function I was able to increase maximum packet throughput by almost 1 Mpps 
(odp_l2fwd, 64B packets, netmap/dpdk pktio).

-Matias


From: EXT Bala Manoharan [mailto:bala.manoha...@linaro.org]
Sent: Friday, May 13, 2016 9:39 AM
To: Elo, Matias (Nokia - FI/Espoo) 
Cc: LNG ODP Mailman List 
Subject: Re: [lng-odp] [PATCH 2/4] linux-generic: packet: remove vlan_s_tag and 
vlan_c_tag members from odp_packet_hdr_t

There is a use-case in classification, where the packet can be classified based 
on outer/inner vlan tag and for that specific use-case it is better to store 
the vlan value in packet header field.

Regards,
Bala

On 12 May 2016 at 18:06, Matias Elo 
mailto:matias@nokia.com>> wrote:
There is no way to read vlan tags in the ODP API, so don't
save them.

Signed-off-by: Matias Elo mailto:matias@nokia.com>>
---
 platform/linux-generic/include/odp_packet_internal.h | 4 
 platform/linux-generic/odp_packet.c  | 4 
 2 files changed, 8 deletions(-)

diff --git a/platform/linux-generic/include/odp_packet_internal.h 
b/platform/linux-generic/include/odp_packet_internal.h
index 508adf8..2a12503 100644
--- a/platform/linux-generic/include/odp_packet_internal.h
+++ b/platform/linux-generic/include/odp_packet_internal.h
@@ -143,8 +143,6 @@ typedef struct {
uint32_t l4_offset; /**< offset to L4 hdr (TCP, UDP, SCTP, also ICMP) */
uint32_t payload_offset; /**< offset to payload */

-   uint32_t vlan_s_tag; /**< Parsed 1st VLAN header (S-TAG) */
-   uint32_t vlan_c_tag; /**< Parsed 2nd VLAN header (C-TAG) */
uint32_t l3_len; /**< Layer 3 length */
uint32_t l4_len; /**< Layer 4 length */

@@ -184,8 +182,6 @@ static inline void 
copy_packet_parser_metadata(odp_packet_hdr_t *src_hdr,
dst_hdr->l4_offset  = src_hdr->l4_offset;
dst_hdr->payload_offset = src_hdr->payload_offset;

-   dst_hdr->vlan_s_tag = src_hdr->vlan_s_tag;
-   dst_hdr->vlan_c_tag = src_hdr->vlan_c_tag;
dst_hdr->l3_len = src_hdr->l3_len;
dst_hdr->l4_len = src_hdr->l4_len;
 }
diff --git a/platform/linux-generic/odp_packet.c 
b/platform/linux-generic/odp_packet.c
index 8681c08..436265e 100644
--- a/platform/linux-generic/odp_packet.c
+++ b/platform/linux-generic/odp_packet.c
@@ -42,8 +42,6 @@ void packet_parse_reset(odp_packet_hdr_t *pkt_hdr)
pkt_hdr->l3_offset= ODP_PACKET_OFFSET_INVALID;
pkt_hdr->l4_offset= ODP_PACKET_OFFSET_INVALID;
pkt_hdr->payload_offset   = ODP_PACKET_OFFSET_INVALID;
-   pkt_hdr->vlan_s_tag   = 0;
-   pkt_hdr->vlan_c_tag   = 0;
 }

 /**
@@ -1223,7 +1221,6 @@ int _odp_parse_common(odp_packet_hdr_t *pkt_hdr, const 
uint8_t *ptr)

vlan = (const odph_vlanhdr_t *)parseptr;
ethtype = odp_be_to_cpu_16(vlan->type);
-   pkt_hdr->vlan_s_tag = odp_be_to_cpu_16(vlan->tci);
offset += sizeof(odph_vlanhdr_t);
parseptr += sizeof(odph_vlanhdr_t);
}
@@ -1232,7 +1229,6 @@ int _odp_parse_common(odp_packet_hdr_t *pkt_hdr, const 
uint8_t *ptr)
pkt_hdr->input_flags.vlan = 1;
vlan = (const odph_vlanhdr_t *)parseptr;
ethtype = odp_be_to_cpu_16(vlan->type);
-   pkt_hdr->vlan_c_tag = odp_be_to_cpu_16(vlan->tci);
offset += sizeof(odph_vlanhdr_t);
parseptr += sizeof(odph_vlanhdr_t);
}
--
1.9.1

___
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

[lng-odp] process thread and mixed mode

[Christophe Milard]

Hi,

Let me start in a bit unusual way :-). I do not like my "running things in
process mode" patch series  that much...
But I do think it should be merged. With the mixed mode.

The main goal of the patch series is to push the work regarding a proper
and consistent definition of some ODP low level concepts and objects. The
patch series makes things fail, hence pushing for defining and coding
things better, I hope.  Removing "provocative" things like the mixed mode
from it is just defeating the purpose of this series: I'll be happy to
remove it (and plenty of other things) when we know which well documented
definition/rule makes this code not relevant.
As long as we keep saying that odp threads could be indifferently  linux
processes or pthreads, I'll be thinking that this series should work,
inclusive any kind of mixed mode.

However, processes and threads are different things in linux, and I believe
this distinction should be kept within ODP: Trying to make processes behave
as pthreads for some ODP objects (such as addresses for shmem objects) does
not sound like a good idea to me. Yes, this can maybe be done with linux
-and even possibly without preallocating the memory-, but still, linux
creates 2 distinct virtual address spaces on fork() so why would ODP put
any exception to that?.
...And If we do want to create such exceptions (which I don't like),
shouln't ODP define its own concept of process and thread as it obviously
wouldn't match 100% the definition of the OS?

I do nevertheless definitively agree with Petri when saying that that ODP
applications should be given a way to "share pointers" between odpthreads.
Taking the "process only" behavior forcing every concurrent execution tasks
to use tricky offset calculation to find their data is too nasty and
inefficient.

I think this will boil down to 2 choices:
1) Take these object definitions from the OS and accept the OS rules.
2) Define our own ODP objects (and possibly defines our own rules)

1)Taking the OS definitions:
Choice 1) is consistent as long as we accept the OS rules. Taking that
choice would mean that any address remain valid within a process only:
shmem pointers will not be able to be shared between different processes: I
do not see that as a big problem as the applications can use pthreads if it
needs to share the same VA, or can fork() at a conveniant time. Moreover,
existing applications which should be ported to ODP are probably OK as they
would already be using pthreads/processes in the correct way.

A more tricky aspect of choice 1) is the development of ODP itself:
Operations involving many "ODPthreads" will have to cope with both
processes and threads possibly without a clear view of what is done by the
application. ( I guess in linux if getpid()==gettid() when calling
odp_init_local() one can assume safely that the app forked...?).
I guess it is doable: assuming all odpthreads are descendant of the
instanciation process, ODP can initially reserve shared memory in
odp_init_global() for its own purpose. It has a price: within ODP (and
anything external to the app itself, such as drivers) a common VA cannot be
assumed. ODP/drivers will have be prepared for an app fork at any time and
hence do its internal stuff as if pointers could not be shared (except
within the initially allocated memory). This will cost ODP performance.

Taking choice 1 probably also mean that shmen should no longer be part of
ODP: Allocating memory is as much an OS thing as forking a process... why
would shmem objects be ODP objects while ODP thread would not be?

2) Defining new ODP objects
I guess in this case, we'd have to create both ODP threads and ODP
processes objects and related usual methods. The advantage if this approach
is that ODP has more control of what is being done. It also allows for a
ODP definition of things (such as the above mentioned exception), if we
really want to...
It makes applications more portable (if ODP abstracts some of the common OS
functionality, the apps using these ODP api becomes OS agnostic).
As we already have done for shmem, ODP methods for odp threads and
processes could be provided to retrieve underlying OS stuff (such as PID...)

The problem of having to assume non common VA within ODP remains.

... and of course where should we stop wrapping the OS...?

3) There is a third alternative as well: stating that ODP is thread only
:-). No process, no problem?

Thank you for reading that far :-). waiting for your comments...

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

Re: [lng-odp] [PATCHv6 12/38] helper: parsing the complete set of options

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

From: EXT Christophe Milard [mailto:christophe.mil...@linaro.org]
Sent: Thursday, May 12, 2016 6:07 PM
To: Savolainen, Petri (Nokia - FI/Espoo) 
Cc: lng-odp@lists.linaro.org
Subject: Re: [lng-odp] [PATCHv6 12/38] helper: parsing the complete set of 
options



On 12 May 2016 at 12:54, Savolainen, Petri (Nokia - FI/Espoo) 
mailto:petri.savolai...@nokia.com>> wrote:
Those that add new definitions into helper/linux.h and depend on each other, so 
that the entire proposed spec is easy to see today and later on from git 
history. Since those are new calls, it should be easy to introduce those in 
their final form. Patches (e.g. 1 and 12) should not expose intermediate, 
unnecessary steps done during development of a new feature, but the new feature 
in its final form.

At least 1 and 12, since those introduce and modify the same function. Maybe 11 
if odph_merge_getopt_options() is always needed with odph_parse_options(). Is 
merge function defined for the user or is it internal to the helper?

Does not make sense to me: as explained in the cover letter, patch 1 to 10 is 
introducing the ability to parse options in helpers. This is applied to the 
tests (patch 6 to 10) as none of the tests have their own options (as of today)

Patch  11... enables the caller helper to have its own options. This is then 
applied to exemple/perf... which already had their own option, and hence could 
not be handled as the tests were.

The thing is that you are not forced to introduce the parse function in two 
parts (first with one prototype/spec and then with another), right?  Because 
it’s a new function you can bring it to every example/test in the same, final 
form (regardless if app has its own options or not). I’m not interested to see 
(== review) the development process, but the final result of the development 
(== the final helper specification).



Maybe  4 and 5, because the first introduces types that the other depends on.

That could be done if you wish

Maybe 5 and 12 (at least 12 should be before 5), if odph_parse_options() must 
be always called before odph_odpthreads_create().

same problem as with patch 1 and 12: Patch 1 to 10 is one  group. 11 to 37 
another. I hoped the cover-letter made that clear.

It should not be a large rebase effort to merge 1 and 12, and then 
squash/rebase those patches that first modify test for patch 1 and then the 
same examples for patch 12. For example, patch 2 would directly contain

return odph_parse_options(argc, argv, NULL, NULL);

instead of

return odph_parse_options(argc, argv);

-Petri






Christophe.



-Petri


From: lng-odp 
[mailto:lng-odp-boun...@lists.linaro.org]
 On Behalf Of Christophe Milard
Sent: Thursday, May 12, 2016 12:05 PM
To: Savolainen, Petri (Nokia - FI/Espoo) 
mailto:petri.savolai...@nokia.com>>
Cc: lng-odp@lists.linaro.org
Subject: Re: [lng-odp] [PATCHv6 12/38] helper: parsing the complete set of 
options

Not sure which patches you want to squash here: "helper: adding a function to 
merge getopt parameters" and "parsing the complete set of options" ?
Aren't these 2 different steps? They were at least tested separately...
Or did I misunderstood?

Christophe.

On 12 May 2016 at 10:21, Savolainen, Petri (Nokia - FI/Espoo) 
mailto:petri.savolai...@nokia.com>> wrote:
It would be really good to squash these patches introducing (and then modifying 
the previously introduced) new helper function prototypes into a single patch. 
This way it's hard to see the complete picture what functions were actually 
added into helper/linux.h


-Petri


> -Original Message-
> From: lng-odp 
> [mailto:lng-odp-boun...@lists.linaro.org]
>  On Behalf Of
> Christophe Milard
> Sent: Wednesday, May 11, 2016 7:42 PM
> To: brian.bro...@linaro.org; 
> mike.hol...@linaro.org; lng-
> o...@lists.linaro.org
> Subject: [lng-odp] [PATCHv6 12/38] helper: parsing the complete set of
> options
>
> The odph_parse_options() function is given the ability to receive
> getopt command line description parameter from it caller, hence allowing
> the caller to have some command line parameter(s).
> The caller shall first call odph_parse_options() with its own parameter
> description as parameter.
> odph_parse_options() is then checking the complete set of options,
> issuing error message for unknown options (those being neither a caller's
> valid command line option or a helper valid command line option),
> and collecting the sementic of helper options.
> Then the caller shall parse the sementic of its own options,
> with the opterr variable set to zero (hence ignoring helper options).
>
> Signed-off-by: Christophe Milard 
> mailto:christophe.mil...@linaro.org>>
> ---
>  helper/include/odp/helper/linux.h | 16 +++-
>  helper/linux.c| 24 ++

Re: [lng-odp] [PATCHv6 05/38] helpers: linux: creating functions to handle odpthreads

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

> -Original Message-
> From: lng-odp [mailto:lng-odp-boun...@lists.linaro.org] On Behalf Of Brian
> Brooks
> Sent: Thursday, May 12, 2016 6:02 PM
> To: Christophe Milard 
> Cc: lng-odp@lists.linaro.org; Savolainen, Petri (Nokia - FI/Espoo)
> 
> Subject: Re: [lng-odp] [PATCHv6 05/38] helpers: linux: creating functions
> to handle odpthreads
> 
> On 05/12 13:57:52, Christophe Milard wrote:
> > On 12 May 2016 at 13:33, Savolainen, Petri (Nokia - FI/Espoo) <
> > petri.savolai...@nokia.com> wrote:
> >
> > >
> > > *From:* lng-odp [mailto:lng-odp-boun...@lists.linaro.org] *On Behalf
> Of *Christophe
> > > Milard
> > > *Sent:* Thursday, May 12, 2016 2:14 PM
> > > *To:* Savolainen, Petri (Nokia - FI/Espoo)
> 
> > > *Cc:* lng-odp@lists.linaro.org
> > > *Subject:* Re: [lng-odp] [PATCHv6 05/38] helpers: linux: creating
> > > functions to handle odpthreads
> > >
> > > On 12 May 2016 at 12:28, Savolainen, Petri (Nokia - FI/Espoo) <
> > > petri.savolai...@nokia.com> wrote:
> > >
> > >
> > > > +int odph_odpthreads_create(odph_odpthread_t *thread_tbl,
> > > > +const odp_cpumask_t *mask,
> > > > +const odph_odpthread_params_t *thr_params)
> > > > +{
> > > > + int i;
> > > > + int num;
> > > > + int cpu_count;
> > > > + int cpu;
> > > > +
> > > > + num = odp_cpumask_count(mask);
> > > > +
> > > > + memset(thread_tbl, 0, num * sizeof(odph_odpthread_t));
> > > > +
> > > > + cpu_count = odp_cpu_count();
> > > > +
> > > > + if (num < 1 || num > cpu_count) {
> > > > + ODPH_ERR("Invalid number of odpthreads:%d"
> > > > +  " (%d cores available)\n",
> > > > +  num, cpu_count);
> > > > + return -1;
> > > > + }
> > > > +
> > > > + cpu = odp_cpumask_first(mask);
> > > > + for (i = 0; i < num; i++) {
> > > > + /*
> > > > +  * Thread mode by default, or if both thread and proc
> mode
> > > > +  * are required each second odpthread is a linux
> thread.
> > > > +  */
> > >
> > > This is a weird logic. All threads should be one type (when you have
> > > boolean cmd line params for thread type). Pthread should be the
> default, so
> > > all threads are pthreads if user don't give any param. It's an error
> if
> > > user gives both.
> > >
> > >
> > >
> > > This follows a requirement from you that the linux ODP implementation
> > > should support a mix of both ODP threads implemented as pthread and
> process
> > > at the same time. I felt that a "--odph_mixed" option is not more
> clear
> > > than specifying both the things we want, i.e. processes and thread.
> > >
> > > Any better suggestion?
> > >
> > >
> > >
> > > You’d need more detailed information (than Boolean) for that advanced
> use
> > > case. At this point, all pthreads or all processes is sufficient for
> > > odp-linux. I have not required implementation of thread/process mix,
> but
> > > required that ODP API does not prevent different threading models.
> > >
> > >
> > >
> > > Create all threads by default, all processes if helper_options.proc is
> > > true and report error is both are true.
> > >
> > >
> > >
> > > if (helper_options.proc && helper_options.thrd)
> > >
> > >   return -1;
> > >
> > >
> > >
> > > if (helper_options.proc) {
> > >
> > >   odph_linux_process_create();
> > >
> > > } else {
> > >
> > >   odph_linux_thread_create();
> > >
> > > }
> > >
> > > Now, I am getting very confused... I thought we agreed that support
> for
> > > both process mode and thread mode at the same time was needed. I
> actually
> > > remember asking that specific question at some arch call and  the
> answer
> > > being positive: And it made sense!: If ODP threads are OS objects
> created
> > > by the application directly towards the OS, how would you expect ODP
> to
> > > prevent an app to mix pthreads and fork?. If we cannot prevent it,
> don't we
> > > have to support it?
> > >
> > >
> > >
> > > Shall I really remove support for the mixed mode from the patch
> series?
> > >
> > >
> > >
> > >
> > >
> > > There is no need for _*helper*_ to setup every second thread as
> process
> > > and every second as pthread. If an application needs mixed mode, it
> can set
> > > it up without a helper. We just need basic stuff working first (all
> > > processes). More flexible setup would need more params: “create 1
> process
> > > with 3 additional pthread, and a another 2 processes with 2 additional
> > > pthreads each, and …”
> > >
> >
> > But wasn't the point of this patch series to be able to run
> > test/examples/perf test ... in all suported modes?
> >
> 
> I admit I found it bizarre at first too, but it shows how tricky the
> multi-
> process use case can be for such a library.
> 
> This patch series simply enables existing binaries, which make use of the
> ODP APIs, to run in either multi-threaded, multi-process, or a mixture of
> both
> environme

Re: [lng-odp] [PATCH 4/4] linux-generic: packet: initialize only selected odp_packet_hdr_t members

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

From: EXT Bill Fischofer [mailto:bill.fischo...@linaro.org]
Sent: Thursday, May 12, 2016 10:34 PM
To: Elo, Matias (Nokia - FI/Espoo) 
Cc: LNG ODP Mailman List 
Subject: Re: [lng-odp] [PATCH 4/4] linux-generic: packet: initialize only 
selected odp_packet_hdr_t members



On Thu, May 12, 2016 at 7:36 AM, Matias Elo 
mailto:matias@nokia.com>> wrote:
Using memset to initialize struct odp_packet_hdr_t contents
to 0 has a significant overhead. Instead, initialize only
the required members of the struct.

Signed-off-by: Matias Elo mailto:matias@nokia.com>>
---
 platform/linux-generic/include/odp_packet_internal.h |  8 +---
 platform/linux-generic/odp_packet.c  | 18 ++
 2 files changed, 11 insertions(+), 15 deletions(-)

diff --git a/platform/linux-generic/include/odp_packet_internal.h 
b/platform/linux-generic/include/odp_packet_internal.h
index 2a12503..cdee1e1 100644
--- a/platform/linux-generic/include/odp_packet_internal.h
+++ b/platform/linux-generic/include/odp_packet_internal.h
@@ -143,15 +143,17 @@ typedef struct {
uint32_t l4_offset; /**< offset to L4 hdr (TCP, UDP, SCTP, also ICMP) */
uint32_t payload_offset; /**< offset to payload */

-   uint32_t l3_len; /**< Layer 3 length */
-   uint32_t l4_len; /**< Layer 4 length */
-
uint32_t frame_len;
uint32_t headroom;
uint32_t tailroom;

odp_pktio_t input;

+   /* Values below are not initialized by packet_init() */
+
+   uint32_t l3_len; /**< Layer 3 length */
+   uint32_t l4_len; /**< Layer 4 length */
+
uint32_t flow_hash;  /**< Flow hash value */
odp_time_t timestamp;/**< Timestamp value */

diff --git a/platform/linux-generic/odp_packet.c 
b/platform/linux-generic/odp_packet.c
index 436265e..f5f224d 100644
--- a/platform/linux-generic/odp_packet.c
+++ b/platform/linux-generic/odp_packet.c
@@ -50,23 +50,17 @@ void packet_parse_reset(odp_packet_hdr_t *pkt_hdr)
 static void packet_init(pool_entry_t *pool, odp_packet_hdr_t *pkt_hdr,
size_t size, int parse)
 {
-   /*
-   * Reset parser metadata.  Note that we clear via memset to make
-   * this routine indepenent of any additional adds to packet metadata.
-   */
-   const size_t start_offset = ODP_FIELD_SIZEOF(odp_packet_hdr_t, buf_hdr);
-   uint8_t *start;
-   size_t len;
+   pkt_hdr->input_flags.all  = 0;
+   pkt_hdr->output_flags.all = 0;
+   pkt_hdr->error_flags.all  = 0;

-   start = (uint8_t *)pkt_hdr + start_offset;
-   len = sizeof(odp_packet_hdr_t) - start_offset;
-   memset(start, 0, len);

The reason for this memset is to "future proof" the header by allowing any new 
fields that are added to get initialized to a known value without having to 
update this routine. Is the impact of this single memset() all that large?  I 
can understand wanting to compress the header to possibly avoid another cache 
line reference but this optimization seems inherently error-prone. I'm also 
wondering if this might confuse tools like Coverity which seems to be paranoid 
about uninitialized variables.

Hi Bill,

I can understand the future proofing. However, this single memset() is 
currently the largest cycle hog when forwarding small packets. Below are some 
throughput numbers:

odp_l2fwd -i p1p1 -c 1 -a 0 (netmap pktio):

Niantic (1x10Gbps):7.9 vs 8.6 Mpps (64B) => 8% improvement
Fortville (1x40Gbps):  7.9 vs 8.8 Mpps (64B) => 11% improvement

When the performance gain is this big I would pay the price of being more 
error-prone. Is there a way to run Coverity before merging the patch?

-Matias

-
-   /* Set metadata items that initialize to non-zero values */
+   pkt_hdr->l2_offset = 0;
pkt_hdr->l3_offset = ODP_PACKET_OFFSET_INVALID;
pkt_hdr->l4_offset = ODP_PACKET_OFFSET_INVALID;
pkt_hdr->payload_offset = ODP_PACKET_OFFSET_INVALID;

+   pkt_hdr->input = ODP_PKTIO_INVALID;
+
/* Disable lazy parsing on user allocated packets */
if (!parse)
packet_parse_disable(pkt_hdr);
--
1.9.1

___
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