Re: [lng-odp] [API-NEXT PATCH] linux-generic: packet: fix buggy compiler error

2016-12-21 Thread Maxim Uvarov
On 21 December 2016 at 23:45, Bill Fischofer 
wrote:

> On Wed, Dec 21, 2016 at 9:26 AM, Maxim Uvarov 
> wrote:
> > On debian jessie gcc unable to compile current code
> > with error:
> > odp_packet.c: In function 'odp_packet_trunc_head':
> > odp_packet.c:314:46: error: array subscript is above array bounds
> [-Werror=array-bounds]
> > to->buf_hdr.seg[i].len = from->buf_hdr.seg[num + i].len;
> >
> > The problem is that it breaks compilation only on .len line:
> > for (i = 0; i < num; i++) {
> > to->buf_hdr.seg[i].hdr = from->buf_hdr.seg[num + i].hdr;
> > to->buf_hdr.seg[i].data = from->buf_hdr.seg[num + i].data;
> > to->buf_hdr.seg[i].len = from->buf_hdr.seg[num + i].len;
> > }
> >
> > If that line is commented out compilation passes. If lines are reordered
> > than compilation also passes. Because there is no warning on .hdr and
> .data
> > it looks like compiler error. Additional check for preconfigured value
> > is workaround to that situation.
>
> If reordering solves the problem then why is that not the preferred
> solution since that doesn't add any pathlength? These statements are
> independent so it shouldn't matter. I would put a comment in
> referencing the bug to give future maintainers a caution about this.
>
>
Sorry, I wanted to write that reorder also does not solve the problems.

If that line is commented out compilation passes. If lines are reordered
than compilation also fails. Because there is no warning on .hdr and .data
it looks like compiler error. Additional check for preconfigured value
is workaround to that situation.

Maxim.



> >
> > Signed-off-by: Maxim Uvarov 
> > ---
> >  platform/linux-generic/odp_packet.c | 2 ++
> >  1 file changed, 2 insertions(+)
> >
> > diff --git a/platform/linux-generic/odp_packet.c
> b/platform/linux-generic/odp_packet.c
> > index 0d3fd05..f3e0a12 100644
> > --- a/platform/linux-generic/odp_packet.c
> > +++ b/platform/linux-generic/odp_packet.c
> > @@ -309,6 +309,8 @@ static inline void copy_num_segs(odp_packet_hdr_t
> *to, odp_packet_hdr_t *from,
> > int i;
> >
> > for (i = 0; i < num; i++) {
> > +   if (odp_unlikely((num + i) >= CONFIG_PACKET_MAX_SEGS))
> > +   ODP_ABORT("packet segmenation error\n");
> > to->buf_hdr.seg[i].hdr  = from->buf_hdr.seg[num + i].hdr;
> > to->buf_hdr.seg[i].data = from->buf_hdr.seg[num +
> i].data;
> > to->buf_hdr.seg[i].len  = from->buf_hdr.seg[num + i].len;
> > --
> > 2.7.1.250.gff4ea60
> >
>


Re: [lng-odp] [PATCHv2, 1/1] validation: pktio: fix invalid mac addr

2016-12-21 Thread Bala Manoharan
Regards,
Bala


On 8 December 2016 at 21:03, Josep Puigdemont
 wrote:
> On Thu, Nov 10, 2016 at 07:58:39PM +0530, Bala Manoharan wrote:
>> Fixes https://bugs.linaro.org/show_bug.cgi?id=2496
>>
>> Signed-off-by: Balasubramanian Manoharan 
>> ---
>> v2: Incorporate review comments
>>  test/common_plat/validation/api/pktio/pktio.c | 24 +---
>>  1 file changed, 21 insertions(+), 3 deletions(-)
>>
>> --
>> 1.9.1
>> Signed-off-by: Balasubramanian Manoharan 
>>
>> diff --git a/test/common_plat/validation/api/pktio/pktio.c 
>> b/test/common_plat/validation/api/pktio/pktio.c
>> index a6a18c3..7115def 100644
>> --- a/test/common_plat/validation/api/pktio/pktio.c
>> +++ b/test/common_plat/validation/api/pktio/pktio.c
>> @@ -31,6 +31,8 @@
>>  #define PKTIN_TS_MAX_RES   100
>>  #define PKTIN_TS_CMP_RES   1
>>
>> +#define PKTIO_SRC_MAC{1, 2, 3, 4, 5, 6}
>> +#define PKTIO_DST_MAC{1, 2, 3, 4, 5, 6}
>>  #undef DEBUG_STATS
>>
>>  /** interface names used for testing */
>> @@ -241,16 +243,32 @@ static uint32_t pktio_init_packet(odp_packet_t pkt)
>>   odph_udphdr_t *udp;
>>   char *buf;
>>   uint16_t seq;
>> - uint8_t mac[ODP_PKTIO_MACADDR_MAXSIZE] = {0};
>> + uint8_t src_mac[ODP_PKTIO_MACADDR_MAXSIZE] = PKTIO_SRC_MAC;
>> + uint8_t dst_mac[ODP_PKTIO_MACADDR_MAXSIZE] = PKTIO_DST_MAC;
>> + uint8_t src_mac_be[ODP_PKTIO_MACADDR_MAXSIZE];
>> + uint8_t dst_mac_be[ODP_PKTIO_MACADDR_MAXSIZE];
>
> we don't need big endian versions of the MAC address, it's a string of
> bytes, so it has no endianess.
>
>>   int pkt_len = odp_packet_len(pkt);
>> + int i;
>> +
>> + #if ODP_BYTE_ORDER == ODP_LITTLE_ENDIAN
>> + for (i = 0; i < ODP_PKTIO_MACADDR_MAXSIZE; i++) {
>> + src_mac_be[i] = src_mac[i];
>> + dst_mac_be[i] = dst_mac[i];
>> + }
>> + #else
>> + for (i = 0; i < ODP_PKTIO_MACADDR_MAXSIZE; i++) {
>> + src_mac_be[i] = src_mac[ODP_PKTIO_MACADDR_MAXSIZE - i];
>> + dst_mac_be[i] = dst_mac[ODP_PKTIO_MACADDR_MAXSIZE - i];
>> + }
>> + #endif
>
> this is not needed.
> For other than ODP_LITTLE_ENDIAN this just reverses the MAC address, but I
> guess it wouldn't matter for the test.

This will have an issue since we have a mac addr based test case to be
added for PMR and it will fail if the address is not reversed.

Regards,
Bala
>
>>
>>   buf = odp_packet_data(pkt);
>>
>>   /* Ethernet */
>>   odp_packet_l2_offset_set(pkt, 0);
>>   eth = (odph_ethhdr_t *)buf;
>> - memcpy(eth->src.addr, mac, ODPH_ETHADDR_LEN);
>> - memcpy(eth->dst.addr, mac, ODPH_ETHADDR_LEN);
>> + memcpy(eth->src.addr, _mac_be, ODPH_ETHADDR_LEN);
>> + memcpy(eth->dst.addr, _mac_be, ODPH_ETHADDR_LEN);
>
> use src_mac and dst_mac instead.
>
> Sorry for the late reply, I missed this earlier.
>
> /Josep
>>   eth->type = odp_cpu_to_be_16(ODPH_ETHTYPE_IPV4);
>>
>>   /* IP */


Re: [lng-odp] [API-NEXT PATCH] linux-gen: schedule_sp: use ring as priority queue

2016-12-21 Thread Bill Fischofer
Yi He had previously given his review for this patch. The subsequent
side-discussion don't constitute a NAK so this should be good to
merge.

On Mon, Dec 19, 2016 at 4:37 AM, Savolainen, Petri (Nokia - FI/Espoo)
 wrote:
> Ping.
>
>> -Original Message-
>> From: lng-odp [mailto:lng-odp-boun...@lists.linaro.org] On Behalf Of Petri
>> Savolainen
>> Sent: Thursday, December 08, 2016 1:04 PM
>> To: lng-odp@lists.linaro.org
>> Subject: Suspected SPAM - [lng-odp] [API-NEXT PATCH] linux-gen:
>> schedule_sp: use ring as priority queue
>>
>> Improve scalability by replacing lock protected linked list
>> with a ring. Schedule group supported was updated also, since
>> ring does not support peek of the head item.
>>
>> Signed-off-by: Petri Savolainen 
>> ---
>>  platform/linux-generic/odp_schedule_sp.c | 271 +++---
>> -
>>  1 file changed, 199 insertions(+), 72 deletions(-)
>>
>


Re: [lng-odp] [API-NEXT PATCH] linux-generic: packet: fix buggy compiler error

2016-12-21 Thread Bill Fischofer
On Wed, Dec 21, 2016 at 9:26 AM, Maxim Uvarov  wrote:
> On debian jessie gcc unable to compile current code
> with error:
> odp_packet.c: In function 'odp_packet_trunc_head':
> odp_packet.c:314:46: error: array subscript is above array bounds 
> [-Werror=array-bounds]
> to->buf_hdr.seg[i].len = from->buf_hdr.seg[num + i].len;
>
> The problem is that it breaks compilation only on .len line:
> for (i = 0; i < num; i++) {
> to->buf_hdr.seg[i].hdr = from->buf_hdr.seg[num + i].hdr;
> to->buf_hdr.seg[i].data = from->buf_hdr.seg[num + i].data;
> to->buf_hdr.seg[i].len = from->buf_hdr.seg[num + i].len;
> }
>
> If that line is commented out compilation passes. If lines are reordered
> than compilation also passes. Because there is no warning on .hdr and .data
> it looks like compiler error. Additional check for preconfigured value
> is workaround to that situation.

If reordering solves the problem then why is that not the preferred
solution since that doesn't add any pathlength? These statements are
independent so it shouldn't matter. I would put a comment in
referencing the bug to give future maintainers a caution about this.

>
> Signed-off-by: Maxim Uvarov 
> ---
>  platform/linux-generic/odp_packet.c | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/platform/linux-generic/odp_packet.c 
> b/platform/linux-generic/odp_packet.c
> index 0d3fd05..f3e0a12 100644
> --- a/platform/linux-generic/odp_packet.c
> +++ b/platform/linux-generic/odp_packet.c
> @@ -309,6 +309,8 @@ static inline void copy_num_segs(odp_packet_hdr_t *to, 
> odp_packet_hdr_t *from,
> int i;
>
> for (i = 0; i < num; i++) {
> +   if (odp_unlikely((num + i) >= CONFIG_PACKET_MAX_SEGS))
> +   ODP_ABORT("packet segmenation error\n");
> to->buf_hdr.seg[i].hdr  = from->buf_hdr.seg[num + i].hdr;
> to->buf_hdr.seg[i].data = from->buf_hdr.seg[num + i].data;
> to->buf_hdr.seg[i].len  = from->buf_hdr.seg[num + i].len;
> --
> 2.7.1.250.gff4ea60
>


[lng-odp] [Bug 2789] increasing CONFIG_PACKET_MAX_SEGS leads to segfault

2016-12-21 Thread bugzilla-daemon
https://bugs.linaro.org/show_bug.cgi?id=2789

Bill Fischofer  changed:

   What|Removed |Added

 Ever confirmed|0   |1
   Priority|--- |High
 Status|UNCONFIRMED |IN_PROGRESS
   Severity|enhancement |critical

--- Comment #1 from Bill Fischofer  ---
Patch http://patches.opendataplane.org/patch/7696/ posted to address this
issue. This should be considered a MUSTFIX item for v1.13.0.0

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

Re: [lng-odp] [API-NEXT PATCH] linux-generic: packet: fix buggy compiler error

2016-12-21 Thread Bill Fischofer
I posted a patch to fix Bug 2789 but it's orthogonal to this issue. We
should still see if we can come up with a compile-time-only
circumvention.

On Wed, Dec 21, 2016 at 12:30 PM, Maxim Uvarov  wrote:
> On 12/21/16 19:07, Bill Fischofer wrote:
>> On Wed, Dec 21, 2016 at 9:58 AM, Mike Holmes  wrote:
>>> On 21 December 2016 at 10:49, Maxim Uvarov  wrote:
>>>
 On 12/21/16 18:31, Mike Holmes wrote:
>
>
> On 21 December 2016 at 10:26, Maxim Uvarov  > wrote:
>
> On debian jessie gcc unable to compile current code
> with error:
> odp_packet.c: In function 'odp_packet_trunc_head':
> odp_packet.c:314:46: error: array subscript is above array bounds
> [-Werror=array-bounds]
> to->buf_hdr.seg[i].len = from->buf_hdr.seg[num + i].len;
>
> The problem is that it breaks compilation only on .len line:
> for (i = 0; i < num; i++) {
> to->buf_hdr.seg[i].hdr = from->buf_hdr.seg[num + i].hdr;
> to->buf_hdr.seg[i].data = from->buf_hdr.seg[num + i].data;
> to->buf_hdr.seg[i].len = from->buf_hdr.seg[num + i].len;
> }
>
> If that line is commented out compilation passes. If lines are
 reordered
> than compilation also passes. Because there is no warning on .hdr
> and .data
> it looks like compiler error. Additional check for preconfigured
 value
> is workaround to that situation.
>
> Signed-off-by: Maxim Uvarov  >
> ---
>  platform/linux-generic/odp_packet.c | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/platform/linux-generic/odp_packet.c
> b/platform/linux-generic/odp_packet.c
> index 0d3fd05..f3e0a12 100644
> --- a/platform/linux-generic/odp_packet.c
> +++ b/platform/linux-generic/odp_packet.c
> @@ -309,6 +309,8 @@ static inline void
> copy_num_segs(odp_packet_hdr_t *to, odp_packet_hdr_t *from,
> int i;
>
> for (i = 0; i < num; i++) {
> +   if (odp_unlikely((num + i) >=
 CONFIG_PACKET_MAX_SEGS))
> +   ODP_ABORT("packet segmenation error\n");
>
>
> Does this  impact performance, if so should it be optionally compiled in
> for that GCC version ?
>


 CONFIG_PACKET_MAX_SEG is currently 2. And according to our tests it will
 add one if check for odp_packet_trunc_head() and odp_packet_trunc_tail()
 calls. So overhead is one if instruction.
>>
>> I agree that this is a bit of a kludge. I'm currently debugging Bug
>> 2789 where things fail if CONFIG_PACKET_MAX_SEG > 2. I'd like to see
>> if the fix for that doesn't result in a cleaner implementation in this
>> area that may not expose this issue.
>>
>
> btw, compilation fails only with -O2 or -O3 and -O0 compiles.
>
> Maxim.
>
>


 It fails on Jessie, which is:
 # gcc -v
 Using built-in specs.
 COLLECT_GCC=gcc
 COLLECT_LTO_WRAPPER=/usr/lib/gcc/x86_64-linux-gnu/4.9/lto-wrapper
 Target: x86_64-linux-gnu
 Configured with: ../src/configure -v --with-pkgversion='Debian 4.9.2-10'
 --with-bugurl=file:///usr/share/doc/gcc-4.9/README.Bugs
 --enable-languages=c,c++,java,go,d,fortran,objc,obj-c++ --prefix=/usr
 --program-suffix=-4.9 --enable-shared --enable-linker-build-id
 --libexecdir=/usr/lib --without-included-gettext --enable-threads=posix
 --with-gxx-include-dir=/usr/include/c++/4.9 --libdir=/usr/lib
 --enable-nls --with-sysroot=/ --enable-clocale=gnu
 --enable-libstdcxx-debug --enable-libstdcxx-time=yes
 --enable-gnu-unique-object --disable-vtable-verify --enable-plugin
 --with-system-zlib --disable-browser-plugin --enable-java-awt=gtk
 --enable-gtk-cairo
 --with-java-home=/usr/lib/jvm/java-1.5.0-gcj-4.9-amd64/jre
 --enable-java-home
 --with-jvm-root-dir=/usr/lib/jvm/java-1.5.0-gcj-4.9-amd64
 --with-jvm-jar-dir=/usr/lib/jvm-exports/java-1.5.0-gcj-4.9-amd64
 --with-arch-directory=amd64
 --with-ecj-jar=/usr/share/java/eclipse-ecj.jar --enable-objc-gc
 --enable-multiarch --with-arch-32=i586 --with-abi=m64
 --with-multilib-list=m32,m64,mx32 --enable-multilib --with-tune=generic
 --enable-checking=release --build=x86_64-linux-gnu
 --host=x86_64-linux-gnu --target=x86_64-linux-gnu
 Thread model: posix
 gcc version 4.9.2 (Debian 4.9.2-10)


 And passes on my Ubuntu which is (4.8 or 4.9):
 gcc-4.9 -v
 Using built-in specs.
 COLLECT_GCC=gcc-4.9
 COLLECT_LTO_WRAPPER=/usr/lib/gcc/x86_64-linux-gnu/4.9/lto-wrapper
 Target: x86_64-linux-gnu
 Configured with: ../src/configure -v --with-pkgversion='Ubuntu
 

[lng-odp] [API-NEXT PATCH] linux-generic: packet: set correct tailroom on trim of multi-seg pkts

2016-12-21 Thread Bill Fischofer
Fix Bug https://bugs.linaro.org/show_bug.cgi?id=2789 by calculating
tailroom correctly when trimming multi-segment packets. Also handle
head/tail push/pull operations that result in a zero-length packet
correctly.

Signed-off-by: Bill Fischofer 
---
 platform/linux-generic/odp_packet.c | 18 --
 1 file changed, 12 insertions(+), 6 deletions(-)

diff --git a/platform/linux-generic/odp_packet.c 
b/platform/linux-generic/odp_packet.c
index 0d3fd05..ab352f2 100644
--- a/platform/linux-generic/odp_packet.c
+++ b/platform/linux-generic/odp_packet.c
@@ -413,8 +413,8 @@ static inline odp_packet_hdr_t 
*free_segments(odp_packet_hdr_t *pkt_hdr,
 * of the metadata. */
pkt_hdr->buf_hdr.segcount = n;
pkt_hdr->frame_len -= free_len;
-   pkt_hdr->tailroom = pkt_hdr->buf_hdr.buf_end -
-   (uint8_t *)packet_tail(pkt_hdr);
+   pkt_hdr->tailroom =
+   pkt_hdr->buf_hdr.size - packet_last_seg_len(pkt_hdr);
 
pull_tail(pkt_hdr, pull_len);
}
@@ -719,9 +719,12 @@ int odp_packet_trunc_head(odp_packet_t *pkt, uint32_t len,
if (len > pkt_hdr->frame_len)
return -1;
 
-   if (len < seg_len) {
+   /* Only the first segment may be zero length */
+   if (CONFIG_PACKET_MAX_SEGS == 1 ||
+   len < seg_len ||
+   pkt_hdr->buf_hdr.segcount == 1) {
pull_head(pkt_hdr, len);
-   } else if (CONFIG_PACKET_MAX_SEGS != 1) {
+   } else {
int num = 0;
uint32_t pull_len = 0;
 
@@ -811,9 +814,12 @@ int odp_packet_trunc_tail(odp_packet_t *pkt, uint32_t len,
last= packet_last_seg(pkt_hdr);
seg_len = packet_seg_len(pkt_hdr, last);
 
-   if (len < seg_len) {
+   /* Only the first segment may be zero length */
+   if (CONFIG_PACKET_MAX_SEGS == 1 ||
+   len < seg_len ||
+   pkt_hdr->buf_hdr.segcount == 1) {
pull_tail(pkt_hdr, len);
-   } else if (CONFIG_PACKET_MAX_SEGS != 1) {
+   } else {
int num = 0;
uint32_t pull_len = 0;
 
-- 
2.7.4



Re: [lng-odp] [API-NEXT PATCH] linux-generic: packet: fix buggy compiler error

2016-12-21 Thread Maxim Uvarov
On 12/21/16 19:07, Bill Fischofer wrote:
> On Wed, Dec 21, 2016 at 9:58 AM, Mike Holmes  wrote:
>> On 21 December 2016 at 10:49, Maxim Uvarov  wrote:
>>
>>> On 12/21/16 18:31, Mike Holmes wrote:


 On 21 December 2016 at 10:26, Maxim Uvarov > wrote:

 On debian jessie gcc unable to compile current code
 with error:
 odp_packet.c: In function 'odp_packet_trunc_head':
 odp_packet.c:314:46: error: array subscript is above array bounds
 [-Werror=array-bounds]
 to->buf_hdr.seg[i].len = from->buf_hdr.seg[num + i].len;

 The problem is that it breaks compilation only on .len line:
 for (i = 0; i < num; i++) {
 to->buf_hdr.seg[i].hdr = from->buf_hdr.seg[num + i].hdr;
 to->buf_hdr.seg[i].data = from->buf_hdr.seg[num + i].data;
 to->buf_hdr.seg[i].len = from->buf_hdr.seg[num + i].len;
 }

 If that line is commented out compilation passes. If lines are
>>> reordered
 than compilation also passes. Because there is no warning on .hdr
 and .data
 it looks like compiler error. Additional check for preconfigured
>>> value
 is workaround to that situation.

 Signed-off-by: Maxim Uvarov >
 ---
  platform/linux-generic/odp_packet.c | 2 ++
  1 file changed, 2 insertions(+)

 diff --git a/platform/linux-generic/odp_packet.c
 b/platform/linux-generic/odp_packet.c
 index 0d3fd05..f3e0a12 100644
 --- a/platform/linux-generic/odp_packet.c
 +++ b/platform/linux-generic/odp_packet.c
 @@ -309,6 +309,8 @@ static inline void
 copy_num_segs(odp_packet_hdr_t *to, odp_packet_hdr_t *from,
 int i;

 for (i = 0; i < num; i++) {
 +   if (odp_unlikely((num + i) >=
>>> CONFIG_PACKET_MAX_SEGS))
 +   ODP_ABORT("packet segmenation error\n");


 Does this  impact performance, if so should it be optionally compiled in
 for that GCC version ?

>>>
>>>
>>> CONFIG_PACKET_MAX_SEG is currently 2. And according to our tests it will
>>> add one if check for odp_packet_trunc_head() and odp_packet_trunc_tail()
>>> calls. So overhead is one if instruction.
> 
> I agree that this is a bit of a kludge. I'm currently debugging Bug
> 2789 where things fail if CONFIG_PACKET_MAX_SEG > 2. I'd like to see
> if the fix for that doesn't result in a cleaner implementation in this
> area that may not expose this issue.
> 

btw, compilation fails only with -O2 or -O3 and -O0 compiles.

Maxim.


>>>
>>>
>>> It fails on Jessie, which is:
>>> # gcc -v
>>> Using built-in specs.
>>> COLLECT_GCC=gcc
>>> COLLECT_LTO_WRAPPER=/usr/lib/gcc/x86_64-linux-gnu/4.9/lto-wrapper
>>> Target: x86_64-linux-gnu
>>> Configured with: ../src/configure -v --with-pkgversion='Debian 4.9.2-10'
>>> --with-bugurl=file:///usr/share/doc/gcc-4.9/README.Bugs
>>> --enable-languages=c,c++,java,go,d,fortran,objc,obj-c++ --prefix=/usr
>>> --program-suffix=-4.9 --enable-shared --enable-linker-build-id
>>> --libexecdir=/usr/lib --without-included-gettext --enable-threads=posix
>>> --with-gxx-include-dir=/usr/include/c++/4.9 --libdir=/usr/lib
>>> --enable-nls --with-sysroot=/ --enable-clocale=gnu
>>> --enable-libstdcxx-debug --enable-libstdcxx-time=yes
>>> --enable-gnu-unique-object --disable-vtable-verify --enable-plugin
>>> --with-system-zlib --disable-browser-plugin --enable-java-awt=gtk
>>> --enable-gtk-cairo
>>> --with-java-home=/usr/lib/jvm/java-1.5.0-gcj-4.9-amd64/jre
>>> --enable-java-home
>>> --with-jvm-root-dir=/usr/lib/jvm/java-1.5.0-gcj-4.9-amd64
>>> --with-jvm-jar-dir=/usr/lib/jvm-exports/java-1.5.0-gcj-4.9-amd64
>>> --with-arch-directory=amd64
>>> --with-ecj-jar=/usr/share/java/eclipse-ecj.jar --enable-objc-gc
>>> --enable-multiarch --with-arch-32=i586 --with-abi=m64
>>> --with-multilib-list=m32,m64,mx32 --enable-multilib --with-tune=generic
>>> --enable-checking=release --build=x86_64-linux-gnu
>>> --host=x86_64-linux-gnu --target=x86_64-linux-gnu
>>> Thread model: posix
>>> gcc version 4.9.2 (Debian 4.9.2-10)
>>>
>>>
>>> And passes on my Ubuntu which is (4.8 or 4.9):
>>> gcc-4.9 -v
>>> Using built-in specs.
>>> COLLECT_GCC=gcc-4.9
>>> COLLECT_LTO_WRAPPER=/usr/lib/gcc/x86_64-linux-gnu/4.9/lto-wrapper
>>> Target: x86_64-linux-gnu
>>> Configured with: ../src/configure -v --with-pkgversion='Ubuntu
>>> 4.9.3-0ubuntu4'
>>> --with-bugurl=file:///usr/share/doc/gccgo-4.9/README.Bugs
>>> --enable-languages=c,c++,go --prefix=/usr --program-suffix=-4.9
>>> --enable-shared --enable-linker-build-id --libexecdir=/usr/lib
>>> --without-included-gettext --enable-threads=posix
>>> --with-gxx-include-dir=/usr/include/c++/4.9 --libdir=/usr/lib
>>> --enable-nls 

Re: [lng-odp] [API-NEXT PATCH] linux-generic: packet: fix buggy compiler error

2016-12-21 Thread Bill Fischofer
On Wed, Dec 21, 2016 at 9:58 AM, Mike Holmes  wrote:
> On 21 December 2016 at 10:49, Maxim Uvarov  wrote:
>
>> On 12/21/16 18:31, Mike Holmes wrote:
>> >
>> >
>> > On 21 December 2016 at 10:26, Maxim Uvarov > > > wrote:
>> >
>> > On debian jessie gcc unable to compile current code
>> > with error:
>> > odp_packet.c: In function 'odp_packet_trunc_head':
>> > odp_packet.c:314:46: error: array subscript is above array bounds
>> > [-Werror=array-bounds]
>> > to->buf_hdr.seg[i].len = from->buf_hdr.seg[num + i].len;
>> >
>> > The problem is that it breaks compilation only on .len line:
>> > for (i = 0; i < num; i++) {
>> > to->buf_hdr.seg[i].hdr = from->buf_hdr.seg[num + i].hdr;
>> > to->buf_hdr.seg[i].data = from->buf_hdr.seg[num + i].data;
>> > to->buf_hdr.seg[i].len = from->buf_hdr.seg[num + i].len;
>> > }
>> >
>> > If that line is commented out compilation passes. If lines are
>> reordered
>> > than compilation also passes. Because there is no warning on .hdr
>> > and .data
>> > it looks like compiler error. Additional check for preconfigured
>> value
>> > is workaround to that situation.
>> >
>> > Signed-off-by: Maxim Uvarov > > >
>> > ---
>> >  platform/linux-generic/odp_packet.c | 2 ++
>> >  1 file changed, 2 insertions(+)
>> >
>> > diff --git a/platform/linux-generic/odp_packet.c
>> > b/platform/linux-generic/odp_packet.c
>> > index 0d3fd05..f3e0a12 100644
>> > --- a/platform/linux-generic/odp_packet.c
>> > +++ b/platform/linux-generic/odp_packet.c
>> > @@ -309,6 +309,8 @@ static inline void
>> > copy_num_segs(odp_packet_hdr_t *to, odp_packet_hdr_t *from,
>> > int i;
>> >
>> > for (i = 0; i < num; i++) {
>> > +   if (odp_unlikely((num + i) >=
>> CONFIG_PACKET_MAX_SEGS))
>> > +   ODP_ABORT("packet segmenation error\n");
>> >
>> >
>> > Does this  impact performance, if so should it be optionally compiled in
>> > for that GCC version ?
>> >
>>
>>
>> CONFIG_PACKET_MAX_SEG is currently 2. And according to our tests it will
>> add one if check for odp_packet_trunc_head() and odp_packet_trunc_tail()
>> calls. So overhead is one if instruction.

I agree that this is a bit of a kludge. I'm currently debugging Bug
2789 where things fail if CONFIG_PACKET_MAX_SEG > 2. I'd like to see
if the fix for that doesn't result in a cleaner implementation in this
area that may not expose this issue.

>>
>>
>> It fails on Jessie, which is:
>> # gcc -v
>> Using built-in specs.
>> COLLECT_GCC=gcc
>> COLLECT_LTO_WRAPPER=/usr/lib/gcc/x86_64-linux-gnu/4.9/lto-wrapper
>> Target: x86_64-linux-gnu
>> Configured with: ../src/configure -v --with-pkgversion='Debian 4.9.2-10'
>> --with-bugurl=file:///usr/share/doc/gcc-4.9/README.Bugs
>> --enable-languages=c,c++,java,go,d,fortran,objc,obj-c++ --prefix=/usr
>> --program-suffix=-4.9 --enable-shared --enable-linker-build-id
>> --libexecdir=/usr/lib --without-included-gettext --enable-threads=posix
>> --with-gxx-include-dir=/usr/include/c++/4.9 --libdir=/usr/lib
>> --enable-nls --with-sysroot=/ --enable-clocale=gnu
>> --enable-libstdcxx-debug --enable-libstdcxx-time=yes
>> --enable-gnu-unique-object --disable-vtable-verify --enable-plugin
>> --with-system-zlib --disable-browser-plugin --enable-java-awt=gtk
>> --enable-gtk-cairo
>> --with-java-home=/usr/lib/jvm/java-1.5.0-gcj-4.9-amd64/jre
>> --enable-java-home
>> --with-jvm-root-dir=/usr/lib/jvm/java-1.5.0-gcj-4.9-amd64
>> --with-jvm-jar-dir=/usr/lib/jvm-exports/java-1.5.0-gcj-4.9-amd64
>> --with-arch-directory=amd64
>> --with-ecj-jar=/usr/share/java/eclipse-ecj.jar --enable-objc-gc
>> --enable-multiarch --with-arch-32=i586 --with-abi=m64
>> --with-multilib-list=m32,m64,mx32 --enable-multilib --with-tune=generic
>> --enable-checking=release --build=x86_64-linux-gnu
>> --host=x86_64-linux-gnu --target=x86_64-linux-gnu
>> Thread model: posix
>> gcc version 4.9.2 (Debian 4.9.2-10)
>>
>>
>> And passes on my Ubuntu which is (4.8 or 4.9):
>> gcc-4.9 -v
>> Using built-in specs.
>> COLLECT_GCC=gcc-4.9
>> COLLECT_LTO_WRAPPER=/usr/lib/gcc/x86_64-linux-gnu/4.9/lto-wrapper
>> Target: x86_64-linux-gnu
>> Configured with: ../src/configure -v --with-pkgversion='Ubuntu
>> 4.9.3-0ubuntu4'
>> --with-bugurl=file:///usr/share/doc/gccgo-4.9/README.Bugs
>> --enable-languages=c,c++,go --prefix=/usr --program-suffix=-4.9
>> --enable-shared --enable-linker-build-id --libexecdir=/usr/lib
>> --without-included-gettext --enable-threads=posix
>> --with-gxx-include-dir=/usr/include/c++/4.9 --libdir=/usr/lib
>> --enable-nls --with-sysroot=/ --enable-clocale=gnu
>> --enable-libstdcxx-time=yes --enable-gnu-unique-object
>> --disable-vtable-verify --disable-libquadmath --enable-plugin
>> --with-system-zlib 

Re: [lng-odp] [API-NEXT PATCH] linux-generic: packet: fix buggy compiler error

2016-12-21 Thread Mike Holmes
On 21 December 2016 at 10:49, Maxim Uvarov  wrote:

> On 12/21/16 18:31, Mike Holmes wrote:
> >
> >
> > On 21 December 2016 at 10:26, Maxim Uvarov  > > wrote:
> >
> > On debian jessie gcc unable to compile current code
> > with error:
> > odp_packet.c: In function 'odp_packet_trunc_head':
> > odp_packet.c:314:46: error: array subscript is above array bounds
> > [-Werror=array-bounds]
> > to->buf_hdr.seg[i].len = from->buf_hdr.seg[num + i].len;
> >
> > The problem is that it breaks compilation only on .len line:
> > for (i = 0; i < num; i++) {
> > to->buf_hdr.seg[i].hdr = from->buf_hdr.seg[num + i].hdr;
> > to->buf_hdr.seg[i].data = from->buf_hdr.seg[num + i].data;
> > to->buf_hdr.seg[i].len = from->buf_hdr.seg[num + i].len;
> > }
> >
> > If that line is commented out compilation passes. If lines are
> reordered
> > than compilation also passes. Because there is no warning on .hdr
> > and .data
> > it looks like compiler error. Additional check for preconfigured
> value
> > is workaround to that situation.
> >
> > Signed-off-by: Maxim Uvarov  > >
> > ---
> >  platform/linux-generic/odp_packet.c | 2 ++
> >  1 file changed, 2 insertions(+)
> >
> > diff --git a/platform/linux-generic/odp_packet.c
> > b/platform/linux-generic/odp_packet.c
> > index 0d3fd05..f3e0a12 100644
> > --- a/platform/linux-generic/odp_packet.c
> > +++ b/platform/linux-generic/odp_packet.c
> > @@ -309,6 +309,8 @@ static inline void
> > copy_num_segs(odp_packet_hdr_t *to, odp_packet_hdr_t *from,
> > int i;
> >
> > for (i = 0; i < num; i++) {
> > +   if (odp_unlikely((num + i) >=
> CONFIG_PACKET_MAX_SEGS))
> > +   ODP_ABORT("packet segmenation error\n");
> >
> >
> > Does this  impact performance, if so should it be optionally compiled in
> > for that GCC version ?
> >
>
>
> CONFIG_PACKET_MAX_SEG is currently 2. And according to our tests it will
> add one if check for odp_packet_trunc_head() and odp_packet_trunc_tail()
> calls. So overhead is one if instruction.
>
>
> It fails on Jessie, which is:
> # gcc -v
> Using built-in specs.
> COLLECT_GCC=gcc
> COLLECT_LTO_WRAPPER=/usr/lib/gcc/x86_64-linux-gnu/4.9/lto-wrapper
> Target: x86_64-linux-gnu
> Configured with: ../src/configure -v --with-pkgversion='Debian 4.9.2-10'
> --with-bugurl=file:///usr/share/doc/gcc-4.9/README.Bugs
> --enable-languages=c,c++,java,go,d,fortran,objc,obj-c++ --prefix=/usr
> --program-suffix=-4.9 --enable-shared --enable-linker-build-id
> --libexecdir=/usr/lib --without-included-gettext --enable-threads=posix
> --with-gxx-include-dir=/usr/include/c++/4.9 --libdir=/usr/lib
> --enable-nls --with-sysroot=/ --enable-clocale=gnu
> --enable-libstdcxx-debug --enable-libstdcxx-time=yes
> --enable-gnu-unique-object --disable-vtable-verify --enable-plugin
> --with-system-zlib --disable-browser-plugin --enable-java-awt=gtk
> --enable-gtk-cairo
> --with-java-home=/usr/lib/jvm/java-1.5.0-gcj-4.9-amd64/jre
> --enable-java-home
> --with-jvm-root-dir=/usr/lib/jvm/java-1.5.0-gcj-4.9-amd64
> --with-jvm-jar-dir=/usr/lib/jvm-exports/java-1.5.0-gcj-4.9-amd64
> --with-arch-directory=amd64
> --with-ecj-jar=/usr/share/java/eclipse-ecj.jar --enable-objc-gc
> --enable-multiarch --with-arch-32=i586 --with-abi=m64
> --with-multilib-list=m32,m64,mx32 --enable-multilib --with-tune=generic
> --enable-checking=release --build=x86_64-linux-gnu
> --host=x86_64-linux-gnu --target=x86_64-linux-gnu
> Thread model: posix
> gcc version 4.9.2 (Debian 4.9.2-10)
>
>
> And passes on my Ubuntu which is (4.8 or 4.9):
> gcc-4.9 -v
> Using built-in specs.
> COLLECT_GCC=gcc-4.9
> COLLECT_LTO_WRAPPER=/usr/lib/gcc/x86_64-linux-gnu/4.9/lto-wrapper
> Target: x86_64-linux-gnu
> Configured with: ../src/configure -v --with-pkgversion='Ubuntu
> 4.9.3-0ubuntu4'
> --with-bugurl=file:///usr/share/doc/gccgo-4.9/README.Bugs
> --enable-languages=c,c++,go --prefix=/usr --program-suffix=-4.9
> --enable-shared --enable-linker-build-id --libexecdir=/usr/lib
> --without-included-gettext --enable-threads=posix
> --with-gxx-include-dir=/usr/include/c++/4.9 --libdir=/usr/lib
> --enable-nls --with-sysroot=/ --enable-clocale=gnu
> --enable-libstdcxx-time=yes --enable-gnu-unique-object
> --disable-vtable-verify --disable-libquadmath --enable-plugin
> --with-system-zlib --enable-multiarch --disable-werror
> --with-arch-32=i686 --with-abi=m64 --with-multilib-list=m32,m64,mx32
> --enable-multilib --with-tune=generic --enable-checking=release
> --build=x86_64-linux-gnu --host=x86_64-linux-gnu --target=x86_64-linux-gnu
> Thread model: posix
> gcc version 4.9.3 (Ubuntu 4.9.3-0ubuntu4)
>
>
> And for arm it looks like it failed with some 5.x gcc version. So it
> might be not easy to define exactly which 

Re: [lng-odp] [PATCH] linux-gen: use odp_queue_t for the internal queue interface

2016-12-21 Thread Kevin Wang
Hi Mike,
Yes.I noticed that and this patch is rejected by Bill and Petri for some 
performance concern. I will use the Linaro account in the next patch after fix 
their concern
Thanks for your remind!
 Kevin
Get Outlook for iOS


From: Mike Holmes 
Sent: Wednesday, December 21, 2016 11:26:42 PM
To: Kevin Wang
Cc: lng-odp; Kevin Wang
Subject: Re: [lng-odp] [PATCH] linux-gen: use odp_queue_t for the internal 
queue interface

Hi Kevin

This came from your ARM address, does it need to be via your Linaro address for 
legal reasons ?

Mike

On 18 December 2016 at 20:58, Kevin Wang 
> wrote:
From: "kevin.wang" >

To make the queue component to be modulized, the other
components aren't allowed to access the detailed data
structure of queue. The only way to access queue is
from the internal queue interface by using odp_queue_t
as a handler

Signed-off-by: kevin.wang >
---
 .../include/odp_classification_datamodel.h |  2 +-
 .../linux-generic/include/odp_packet_io_queue.h| 16 ++---
 .../linux-generic/include/odp_queue_internal.h | 71 
 .../include/odp_traffic_mngr_internal.h|  2 +-
 platform/linux-generic/odp_classification.c| 18 ++---
 platform/linux-generic/odp_packet_io.c | 77 ++
 platform/linux-generic/odp_queue.c | 24 +++
 platform/linux-generic/odp_schedule_ordered.c  | 22 ---
 platform/linux-generic/odp_traffic_mngr.c  | 15 +++--
 platform/linux-generic/pktio/loop.c|  8 +--
 10 files changed, 143 insertions(+), 112 deletions(-)

diff --git a/platform/linux-generic/include/odp_classification_datamodel.h 
b/platform/linux-generic/include/odp_classification_datamodel.h
index dc2190d..fa437d0 100644
--- a/platform/linux-generic/include/odp_classification_datamodel.h
+++ b/platform/linux-generic/include/odp_classification_datamodel.h
@@ -76,7 +76,7 @@ typedef struct pmr_term_value {
 Class Of Service
 */
 struct cos_s {
-   queue_entry_t *queue;   /* Associated Queue */
+   odp_queue_t queue;  /* Associated Queue */
pool_entry_t *pool; /* Associated Buffer pool */
union pmr_u *pmr[ODP_PMR_PER_COS_MAX];  /* Chained PMR */
union cos_u *linked_cos[ODP_PMR_PER_COS_MAX]; /* Chained CoS with PMR*/
diff --git a/platform/linux-generic/include/odp_packet_io_queue.h 
b/platform/linux-generic/include/odp_packet_io_queue.h
index 13b79f3..a14cf00 100644
--- a/platform/linux-generic/include/odp_packet_io_queue.h
+++ b/platform/linux-generic/include/odp_packet_io_queue.h
@@ -28,20 +28,20 @@ extern "C" {
 ODP_STATIC_ASSERT(ODP_PKTIN_QUEUE_MAX_BURST >= QUEUE_MULTI_MAX,
  "ODP_PKTIN_DEQ_MULTI_MAX_ERROR");

-int pktin_enqueue(queue_entry_t *queue, odp_buffer_hdr_t *buf_hdr, int 
sustain);
-odp_buffer_hdr_t *pktin_dequeue(queue_entry_t *queue);
+int pktin_enqueue(odp_queue_t queue, odp_buffer_hdr_t *buf_hdr, int sustain);
+odp_buffer_hdr_t *pktin_dequeue(odp_queue_t queue);

-int pktin_enq_multi(queue_entry_t *queue, odp_buffer_hdr_t *buf_hdr[], int num,
+int pktin_enq_multi(odp_queue_t queue, odp_buffer_hdr_t *buf_hdr[], int num,
int sustain);
-int pktin_deq_multi(queue_entry_t *queue, odp_buffer_hdr_t *buf_hdr[], int 
num);
+int pktin_deq_multi(odp_queue_t queue, odp_buffer_hdr_t *buf_hdr[], int num);


-int pktout_enqueue(queue_entry_t *queue, odp_buffer_hdr_t *buf_hdr);
-odp_buffer_hdr_t *pktout_dequeue(queue_entry_t *queue);
+int pktout_enqueue(odp_queue_t queue, odp_buffer_hdr_t *buf_hdr);
+odp_buffer_hdr_t *pktout_dequeue(odp_queue_t queue);

-int pktout_enq_multi(queue_entry_t *queue, odp_buffer_hdr_t *buf_hdr[],
+int pktout_enq_multi(odp_queue_t queue, odp_buffer_hdr_t *buf_hdr[],
 int num);
-int pktout_deq_multi(queue_entry_t *queue, odp_buffer_hdr_t *buf_hdr[],
+int pktout_deq_multi(odp_queue_t queue, odp_buffer_hdr_t *buf_hdr[],
 int num);

 #ifdef __cplusplus
diff --git a/platform/linux-generic/include/odp_queue_internal.h 
b/platform/linux-generic/include/odp_queue_internal.h
index e223d9f..ccd4e15 100644
--- a/platform/linux-generic/include/odp_queue_internal.h
+++ b/platform/linux-generic/include/odp_queue_internal.h
@@ -41,12 +41,12 @@ extern "C" {
 /* forward declaration */
 union queue_entry_u;

-typedef int (*enq_func_t)(union queue_entry_u *, odp_buffer_hdr_t *, int);
-typedefodp_buffer_hdr_t *(*deq_func_t)(union queue_entry_u *);
+typedef int (*enq_func_t)(odp_queue_t queue, odp_buffer_hdr_t *, int);
+typedefodp_buffer_hdr_t *(*deq_func_t)(odp_queue_t queue);

-typedef int (*enq_multi_func_t)(union queue_entry_u *,
+typedef int (*enq_multi_func_t)(odp_queue_t queue,

Re: [lng-odp] [API-NEXT PATCH] linux-generic: packet: fix buggy compiler error

2016-12-21 Thread Maxim Uvarov
On 12/21/16 18:31, Mike Holmes wrote:
> 
> 
> On 21 December 2016 at 10:26, Maxim Uvarov  > wrote:
> 
> On debian jessie gcc unable to compile current code
> with error:
> odp_packet.c: In function 'odp_packet_trunc_head':
> odp_packet.c:314:46: error: array subscript is above array bounds
> [-Werror=array-bounds]
> to->buf_hdr.seg[i].len = from->buf_hdr.seg[num + i].len;
> 
> The problem is that it breaks compilation only on .len line:
> for (i = 0; i < num; i++) {
> to->buf_hdr.seg[i].hdr = from->buf_hdr.seg[num + i].hdr;
> to->buf_hdr.seg[i].data = from->buf_hdr.seg[num + i].data;
> to->buf_hdr.seg[i].len = from->buf_hdr.seg[num + i].len;
> }
> 
> If that line is commented out compilation passes. If lines are reordered
> than compilation also passes. Because there is no warning on .hdr
> and .data
> it looks like compiler error. Additional check for preconfigured value
> is workaround to that situation.
> 
> Signed-off-by: Maxim Uvarov  >
> ---
>  platform/linux-generic/odp_packet.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/platform/linux-generic/odp_packet.c
> b/platform/linux-generic/odp_packet.c
> index 0d3fd05..f3e0a12 100644
> --- a/platform/linux-generic/odp_packet.c
> +++ b/platform/linux-generic/odp_packet.c
> @@ -309,6 +309,8 @@ static inline void
> copy_num_segs(odp_packet_hdr_t *to, odp_packet_hdr_t *from,
> int i;
> 
> for (i = 0; i < num; i++) {
> +   if (odp_unlikely((num + i) >= CONFIG_PACKET_MAX_SEGS))
> +   ODP_ABORT("packet segmenation error\n");
> 
> 
> Does this  impact performance, if so should it be optionally compiled in
> for that GCC version ?
> 


CONFIG_PACKET_MAX_SEG is currently 2. And according to our tests it will
add one if check for odp_packet_trunc_head() and odp_packet_trunc_tail()
calls. So overhead is one if instruction.


It fails on Jessie, which is:
# gcc -v
Using built-in specs.
COLLECT_GCC=gcc
COLLECT_LTO_WRAPPER=/usr/lib/gcc/x86_64-linux-gnu/4.9/lto-wrapper
Target: x86_64-linux-gnu
Configured with: ../src/configure -v --with-pkgversion='Debian 4.9.2-10'
--with-bugurl=file:///usr/share/doc/gcc-4.9/README.Bugs
--enable-languages=c,c++,java,go,d,fortran,objc,obj-c++ --prefix=/usr
--program-suffix=-4.9 --enable-shared --enable-linker-build-id
--libexecdir=/usr/lib --without-included-gettext --enable-threads=posix
--with-gxx-include-dir=/usr/include/c++/4.9 --libdir=/usr/lib
--enable-nls --with-sysroot=/ --enable-clocale=gnu
--enable-libstdcxx-debug --enable-libstdcxx-time=yes
--enable-gnu-unique-object --disable-vtable-verify --enable-plugin
--with-system-zlib --disable-browser-plugin --enable-java-awt=gtk
--enable-gtk-cairo
--with-java-home=/usr/lib/jvm/java-1.5.0-gcj-4.9-amd64/jre
--enable-java-home
--with-jvm-root-dir=/usr/lib/jvm/java-1.5.0-gcj-4.9-amd64
--with-jvm-jar-dir=/usr/lib/jvm-exports/java-1.5.0-gcj-4.9-amd64
--with-arch-directory=amd64
--with-ecj-jar=/usr/share/java/eclipse-ecj.jar --enable-objc-gc
--enable-multiarch --with-arch-32=i586 --with-abi=m64
--with-multilib-list=m32,m64,mx32 --enable-multilib --with-tune=generic
--enable-checking=release --build=x86_64-linux-gnu
--host=x86_64-linux-gnu --target=x86_64-linux-gnu
Thread model: posix
gcc version 4.9.2 (Debian 4.9.2-10)


And passes on my Ubuntu which is (4.8 or 4.9):
gcc-4.9 -v
Using built-in specs.
COLLECT_GCC=gcc-4.9
COLLECT_LTO_WRAPPER=/usr/lib/gcc/x86_64-linux-gnu/4.9/lto-wrapper
Target: x86_64-linux-gnu
Configured with: ../src/configure -v --with-pkgversion='Ubuntu
4.9.3-0ubuntu4'
--with-bugurl=file:///usr/share/doc/gccgo-4.9/README.Bugs
--enable-languages=c,c++,go --prefix=/usr --program-suffix=-4.9
--enable-shared --enable-linker-build-id --libexecdir=/usr/lib
--without-included-gettext --enable-threads=posix
--with-gxx-include-dir=/usr/include/c++/4.9 --libdir=/usr/lib
--enable-nls --with-sysroot=/ --enable-clocale=gnu
--enable-libstdcxx-time=yes --enable-gnu-unique-object
--disable-vtable-verify --disable-libquadmath --enable-plugin
--with-system-zlib --enable-multiarch --disable-werror
--with-arch-32=i686 --with-abi=m64 --with-multilib-list=m32,m64,mx32
--enable-multilib --with-tune=generic --enable-checking=release
--build=x86_64-linux-gnu --host=x86_64-linux-gnu --target=x86_64-linux-gnu
Thread model: posix
gcc version 4.9.3 (Ubuntu 4.9.3-0ubuntu4)


And for arm it looks like it failed with some 5.x gcc version. So it
might be not easy to define exactly which version is it.





>  
> 
> to->buf_hdr.seg[i].hdr  = from->buf_hdr.seg[num +
> i].hdr;
> to->buf_hdr.seg[i].data = from->buf_hdr.seg[num +
> i].data;
> to->buf_hdr.seg[i].len  = from->buf_hdr.seg[num +
> i].len;
> --
> 

Re: [lng-odp] [API-NEXT PATCH] linux-generic: packet: fix buggy compiler error

2016-12-21 Thread Mike Holmes
On 21 December 2016 at 10:26, Maxim Uvarov  wrote:

> On debian jessie gcc unable to compile current code
> with error:
> odp_packet.c: In function 'odp_packet_trunc_head':
> odp_packet.c:314:46: error: array subscript is above array bounds
> [-Werror=array-bounds]
> to->buf_hdr.seg[i].len = from->buf_hdr.seg[num + i].len;
>
> The problem is that it breaks compilation only on .len line:
> for (i = 0; i < num; i++) {
> to->buf_hdr.seg[i].hdr = from->buf_hdr.seg[num + i].hdr;
> to->buf_hdr.seg[i].data = from->buf_hdr.seg[num + i].data;
> to->buf_hdr.seg[i].len = from->buf_hdr.seg[num + i].len;
> }
>
> If that line is commented out compilation passes. If lines are reordered
> than compilation also passes. Because there is no warning on .hdr and .data
> it looks like compiler error. Additional check for preconfigured value
> is workaround to that situation.
>
> Signed-off-by: Maxim Uvarov 
> ---
>  platform/linux-generic/odp_packet.c | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/platform/linux-generic/odp_packet.c
> b/platform/linux-generic/odp_packet.c
> index 0d3fd05..f3e0a12 100644
> --- a/platform/linux-generic/odp_packet.c
> +++ b/platform/linux-generic/odp_packet.c
> @@ -309,6 +309,8 @@ static inline void copy_num_segs(odp_packet_hdr_t *to,
> odp_packet_hdr_t *from,
> int i;
>
> for (i = 0; i < num; i++) {
> +   if (odp_unlikely((num + i) >= CONFIG_PACKET_MAX_SEGS))
> +   ODP_ABORT("packet segmenation error\n");
>

Does this  impact performance, if so should it be optionally compiled in
for that GCC version ?



> to->buf_hdr.seg[i].hdr  = from->buf_hdr.seg[num + i].hdr;
> to->buf_hdr.seg[i].data = from->buf_hdr.seg[num + i].data;
> to->buf_hdr.seg[i].len  = from->buf_hdr.seg[num + i].len;
> --
> 2.7.1.250.gff4ea60
>
>


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


Re: [lng-odp] [PATCH] linux-gen: use odp_queue_t for the internal queue interface

2016-12-21 Thread Mike Holmes
Hi Kevin

This came from your ARM address, does it need to be via your Linaro address
for legal reasons ?

Mike

On 18 December 2016 at 20:58, Kevin Wang  wrote:

> From: "kevin.wang" 
>
> To make the queue component to be modulized, the other
> components aren't allowed to access the detailed data
> structure of queue. The only way to access queue is
> from the internal queue interface by using odp_queue_t
> as a handler
>
> Signed-off-by: kevin.wang 
> ---
>  .../include/odp_classification_datamodel.h |  2 +-
>  .../linux-generic/include/odp_packet_io_queue.h| 16 ++---
>  .../linux-generic/include/odp_queue_internal.h | 71
> 
>  .../include/odp_traffic_mngr_internal.h|  2 +-
>  platform/linux-generic/odp_classification.c| 18 ++---
>  platform/linux-generic/odp_packet_io.c | 77
> ++
>  platform/linux-generic/odp_queue.c | 24 +++
>  platform/linux-generic/odp_schedule_ordered.c  | 22 ---
>  platform/linux-generic/odp_traffic_mngr.c  | 15 +++--
>  platform/linux-generic/pktio/loop.c|  8 +--
>  10 files changed, 143 insertions(+), 112 deletions(-)
>
> diff --git a/platform/linux-generic/include/odp_classification_datamodel.h
> b/platform/linux-generic/include/odp_classification_datamodel.h
> index dc2190d..fa437d0 100644
> --- a/platform/linux-generic/include/odp_classification_datamodel.h
> +++ b/platform/linux-generic/include/odp_classification_datamodel.h
> @@ -76,7 +76,7 @@ typedef struct pmr_term_value {
>  Class Of Service
>  */
>  struct cos_s {
> -   queue_entry_t *queue;   /* Associated Queue */
> +   odp_queue_t queue;  /* Associated Queue */
> pool_entry_t *pool; /* Associated Buffer pool */
> union pmr_u *pmr[ODP_PMR_PER_COS_MAX];  /* Chained PMR */
> union cos_u *linked_cos[ODP_PMR_PER_COS_MAX]; /* Chained CoS with
> PMR*/
> diff --git a/platform/linux-generic/include/odp_packet_io_queue.h
> b/platform/linux-generic/include/odp_packet_io_queue.h
> index 13b79f3..a14cf00 100644
> --- a/platform/linux-generic/include/odp_packet_io_queue.h
> +++ b/platform/linux-generic/include/odp_packet_io_queue.h
> @@ -28,20 +28,20 @@ extern "C" {
>  ODP_STATIC_ASSERT(ODP_PKTIN_QUEUE_MAX_BURST >= QUEUE_MULTI_MAX,
>   "ODP_PKTIN_DEQ_MULTI_MAX_ERROR");
>
> -int pktin_enqueue(queue_entry_t *queue, odp_buffer_hdr_t *buf_hdr, int
> sustain);
> -odp_buffer_hdr_t *pktin_dequeue(queue_entry_t *queue);
> +int pktin_enqueue(odp_queue_t queue, odp_buffer_hdr_t *buf_hdr, int
> sustain);
> +odp_buffer_hdr_t *pktin_dequeue(odp_queue_t queue);
>
> -int pktin_enq_multi(queue_entry_t *queue, odp_buffer_hdr_t *buf_hdr[],
> int num,
> +int pktin_enq_multi(odp_queue_t queue, odp_buffer_hdr_t *buf_hdr[], int
> num,
> int sustain);
> -int pktin_deq_multi(queue_entry_t *queue, odp_buffer_hdr_t *buf_hdr[],
> int num);
> +int pktin_deq_multi(odp_queue_t queue, odp_buffer_hdr_t *buf_hdr[], int
> num);
>
>
> -int pktout_enqueue(queue_entry_t *queue, odp_buffer_hdr_t *buf_hdr);
> -odp_buffer_hdr_t *pktout_dequeue(queue_entry_t *queue);
> +int pktout_enqueue(odp_queue_t queue, odp_buffer_hdr_t *buf_hdr);
> +odp_buffer_hdr_t *pktout_dequeue(odp_queue_t queue);
>
> -int pktout_enq_multi(queue_entry_t *queue, odp_buffer_hdr_t *buf_hdr[],
> +int pktout_enq_multi(odp_queue_t queue, odp_buffer_hdr_t *buf_hdr[],
>  int num);
> -int pktout_deq_multi(queue_entry_t *queue, odp_buffer_hdr_t *buf_hdr[],
> +int pktout_deq_multi(odp_queue_t queue, odp_buffer_hdr_t *buf_hdr[],
>  int num);
>
>  #ifdef __cplusplus
> diff --git a/platform/linux-generic/include/odp_queue_internal.h
> b/platform/linux-generic/include/odp_queue_internal.h
> index e223d9f..ccd4e15 100644
> --- a/platform/linux-generic/include/odp_queue_internal.h
> +++ b/platform/linux-generic/include/odp_queue_internal.h
> @@ -41,12 +41,12 @@ extern "C" {
>  /* forward declaration */
>  union queue_entry_u;
>
> -typedef int (*enq_func_t)(union queue_entry_u *, odp_buffer_hdr_t *, int);
> -typedefodp_buffer_hdr_t *(*deq_func_t)(union queue_entry_u *);
> +typedef int (*enq_func_t)(odp_queue_t queue, odp_buffer_hdr_t *, int);
> +typedefodp_buffer_hdr_t *(*deq_func_t)(odp_queue_t queue);
>
> -typedef int (*enq_multi_func_t)(union queue_entry_u *,
> +typedef int (*enq_multi_func_t)(odp_queue_t queue,
> odp_buffer_hdr_t **, int, int);
> -typedefint (*deq_multi_func_t)(union queue_entry_u *,
> +typedefint (*deq_multi_func_t)(odp_queue_t queue,
> odp_buffer_hdr_t **, int);
>
>  struct queue_entry_s {
> @@ -84,23 +84,23 @@ union queue_entry_u {
>
>  queue_entry_t *get_qentry(uint32_t queue_id);
>
> -int queue_enq(queue_entry_t *queue, odp_buffer_hdr_t *buf_hdr, int
> 

[lng-odp] [API-NEXT PATCH] linux-generic: packet: fix buggy compiler error

2016-12-21 Thread Maxim Uvarov
On debian jessie gcc unable to compile current code
with error:
odp_packet.c: In function 'odp_packet_trunc_head':
odp_packet.c:314:46: error: array subscript is above array bounds 
[-Werror=array-bounds]
to->buf_hdr.seg[i].len = from->buf_hdr.seg[num + i].len;

The problem is that it breaks compilation only on .len line:
for (i = 0; i < num; i++) {
to->buf_hdr.seg[i].hdr = from->buf_hdr.seg[num + i].hdr;
to->buf_hdr.seg[i].data = from->buf_hdr.seg[num + i].data;
to->buf_hdr.seg[i].len = from->buf_hdr.seg[num + i].len;
}

If that line is commented out compilation passes. If lines are reordered
than compilation also passes. Because there is no warning on .hdr and .data
it looks like compiler error. Additional check for preconfigured value
is workaround to that situation.

Signed-off-by: Maxim Uvarov 
---
 platform/linux-generic/odp_packet.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/platform/linux-generic/odp_packet.c 
b/platform/linux-generic/odp_packet.c
index 0d3fd05..f3e0a12 100644
--- a/platform/linux-generic/odp_packet.c
+++ b/platform/linux-generic/odp_packet.c
@@ -309,6 +309,8 @@ static inline void copy_num_segs(odp_packet_hdr_t *to, 
odp_packet_hdr_t *from,
int i;
 
for (i = 0; i < num; i++) {
+   if (odp_unlikely((num + i) >= CONFIG_PACKET_MAX_SEGS))
+   ODP_ABORT("packet segmenation error\n");
to->buf_hdr.seg[i].hdr  = from->buf_hdr.seg[num + i].hdr;
to->buf_hdr.seg[i].data = from->buf_hdr.seg[num + i].data;
to->buf_hdr.seg[i].len  = from->buf_hdr.seg[num + i].len;
-- 
2.7.1.250.gff4ea60



[lng-odp] [Bug 2790] New: CID 173382: Memory - illegal accesses /test/common_plat/performance/odp_l2fwd.c

2016-12-21 Thread bugzilla-daemon
https://bugs.linaro.org/show_bug.cgi?id=2790

Bug ID: 2790
   Summary: CID 173382: Memory - illegal accesses
/test/common_plat/performance/odp_l2fwd.c
   Product: OpenDataPlane - linux- generic reference
   Version: v1.12.0.0
  Hardware: Other
OS: Linux
Status: UNCONFIRMED
  Severity: enhancement
  Priority: ---
 Component: Buffers & Packets
  Assignee: bill.fischo...@linaro.org
  Reporter: mike.hol...@linaro.org
CC: lng-odp@lists.linaro.org
  Target Milestone: ---

*** CID 173382:  Memory - illegal accesses  (NEGATIVE_RETURNS)
/test/common_plat/performance/odp_l2fwd.c: 356 in run_worker_sched_mode()
350 pkts -= rx_drops;
351 }
352 }
353
354 /* packets from the same queue are from the same
interface */
355 src_idx = odp_packet_input_index(pkt_tbl[0]);
>>> CID 173382:  Memory - illegal accesses  (NEGATIVE_RETURNS)
>>> Using variable "src_idx" as an index to array 
>>> "gbl_args->dst_port_from_idx".
356 dst_idx = gbl_args->dst_port_from_idx[src_idx];
357 fill_eth_addrs(pkt_tbl, pkts, dst_idx);
358
359 if (odp_unlikely(use_event_queue))
360 sent = event_queue_send(tx_queue[dst_idx],
pkt_tbl,
361 pkts);

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

[lng-odp] [Bug 2789] New: increasing CONFIG_PACKET_MAX_SEGS leads to segfault

2016-12-21 Thread bugzilla-daemon
https://bugs.linaro.org/show_bug.cgi?id=2789

Bug ID: 2789
   Summary: increasing CONFIG_PACKET_MAX_SEGS leads to segfault
   Product: OpenDataPlane - linux- generic reference
   Version: api-next
  Hardware: Other
OS: Linux
Status: UNCONFIRMED
  Severity: enhancement
  Priority: ---
 Component: Buffers & Packets
  Assignee: bill.fischo...@linaro.org
  Reporter: maxim.uva...@linaro.org
CC: lng-odp@lists.linaro.org
  Target Milestone: ---

use case:
1. increase  CONFIG_PACKET_MAX_SEGS 
2. recompile and run test:

./test/common_plat/validation/api/packet/packet_main

Program received signal SIGSEGV, Segmentation fault.
__memcpy_sse2_unaligned () at
../sysdeps/x86_64/multiarch/memcpy-sse2-unaligned.S:35
35  ../sysdeps/x86_64/multiarch/memcpy-sse2-unaligned.S: No such file or
directory.
(gdb) up
#1  0x004168cf in odp_packet_copy_from_pkt (dst=0x1a00, dst_offset=0,
src=0x1600, src_offset=10157, len=10158) at odp_packet.c:1357
1357memcpy(dst_map, src_map, cpylen);
(gdb) l
1352cpylen = len > minseg ? minseg : len;
1353
1354if (overlap)
1355memmove(dst_map, src_map, cpylen);
1356else
1357memcpy(dst_map, src_map, cpylen);
1358

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

Re: [lng-odp] [PATCH] test: tm: queue id can be not updated

2016-12-21 Thread Mike Holmes
On 21 December 2016 at 08:38, Maxim Uvarov  wrote:

> get_unique_id() function on error do not update it's second and
> third arguments (unique_id and is_ipv4). gcc -O3 finds that
> and traps compilation.
>
> Signed-off-by: Maxim Uvarov 
>

Tested-and-reviewed-by: Mike Holmes 

Fixes
traffic_mngr.c: In function ‘dump_rcvd_pkts.constprop’:
traffic_mngr.c:948:18: error: ‘is_ipv4’ may be used uninitialized in this
function [-Werror=maybe-uninitialized]
  uint8_t is_ipv4;
  ^~~
traffic_mngr.c:959:3: error: ‘unique_id’ may be used uninitialized in this
function [-Werror=maybe-uninitialized]
   printf("rcv_idx=%" PRIu32 " odp_pkt=0x%" PRIX64 " "
   ^~~
  "xmt_idx=%" PRId32 " pkt_class=%u is_ipv4=%u "
  ~~
  "unique_id=0x%X (rc=%d)\n",
  ~~~
  rcv_idx, odp_packet_to_u64(rcv_pkt), xmt_idx,
  ~
  rcv_pkt_desc->pkt_class, is_ipv4, unique_id, rc);
  
cc1: all warnings being treated as errors




> ---
>  test/common_plat/validation/api/traffic_mngr/traffic_mngr.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/test/common_plat/validation/api/traffic_mngr/traffic_mngr.c
> b/test/common_plat/validation/api/traffic_mngr/traffic_mngr.c
> index fcc7187..0271758 100644
> --- a/test/common_plat/validation/api/traffic_mngr/traffic_mngr.c
> +++ b/test/common_plat/validation/api/traffic_mngr/traffic_mngr.c
> @@ -944,8 +944,8 @@ static void dump_rcvd_pkts(uint32_t first_rcv_idx,
> uint32_t last_rcv_idx)
> odp_packet_trcv_pkt;
> uint32_trcv_idx;
> int32_t xmt_idx;
> -   uint16_tunique_id;
> -   uint8_t is_ipv4;
> +   uint16_tunique_id = 0;
> +   uint8_t is_ipv4 = 0;
> int rc;
>
> for (rcv_idx = first_rcv_idx; rcv_idx <= last_rcv_idx; rcv_idx++) {
> --
> 2.7.1.250.gff4ea60
>
>


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


[lng-odp] Change in syssw/odp[odp-devel]: odp: add zero-copy support for netmap

2016-12-21 Thread Alan Winkowski (Code Review)
Alan Winkowski has uploaded a new change for review.

  http://vgitil04.il.marvell.com:8080/34865

Change subject: odp: add zero-copy support for netmap
..

odp: add zero-copy support for netmap

Change-Id: I820bd4fa858fbe0e6bfa883648e092332164eb8f
Signed-off-by: Alan Winkowski 
---
M platform/linux-generic/include/odp_buffer_inlines.h
M platform/linux-generic/include/odp_buffer_internal.h
M platform/linux-generic/include/odp_packet_internal.h
M platform/linux-generic/include/odp_packet_netmap.h
M platform/linux-generic/odp_pool.c
M platform/linux-generic/pktio/netmap.c
6 files changed, 253 insertions(+), 2 deletions(-)


  git pull ssh://vgitil04.il.marvell.com:29418/syssw/odp refs/changes/65/34865/1
-- 
To view, visit http://vgitil04.il.marvell.com:8080/34865
To unsubscribe, visit http://vgitil04.il.marvell.com:8080/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: I820bd4fa858fbe0e6bfa883648e092332164eb8f
Gerrit-PatchSet: 1
Gerrit-Project: syssw/odp
Gerrit-Branch: odp-devel
Gerrit-Owner: Alan Winkowski 


[lng-odp] Change in syssw/odp[odp-devel]: odp: add support for busy-wait use ioctl instead of poll Sig...

2016-12-21 Thread Alan Winkowski (Code Review)
Alan Winkowski has uploaded a new change for review.

  http://vgitil04.il.marvell.com:8080/34864

Change subject: odp: add support for busy-wait use ioctl instead of poll 
Signed-off-by: Alan Winkowski 
..

odp: add support for busy-wait use ioctl instead of poll
Signed-off-by: Alan Winkowski 

Change-Id: Ied7773e6cb97b376bcb4892b6c43e176c45a7e8f
---
M platform/linux-generic/pktio/netmap.c
1 file changed, 26 insertions(+), 0 deletions(-)


  git pull ssh://vgitil04.il.marvell.com:29418/syssw/odp refs/changes/64/34864/1
-- 
To view, visit http://vgitil04.il.marvell.com:8080/34864
To unsubscribe, visit http://vgitil04.il.marvell.com:8080/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: Ied7773e6cb97b376bcb4892b6c43e176c45a7e8f
Gerrit-PatchSet: 1
Gerrit-Project: syssw/odp
Gerrit-Branch: odp-devel
Gerrit-Owner: Alan Winkowski 


[lng-odp] Change in syssw/odp[odp-devel]: odp: netmap_slot_t: add support for mvpp2x-data_offs field

2016-12-21 Thread Alan Winkowski (Code Review)
Alan Winkowski has uploaded a new change for review.

  http://vgitil04.il.marvell.com:8080/34863

Change subject: odp: netmap_slot_t: add support for mvpp2x-data_offs field
..

odp: netmap_slot_t: add support for mvpp2x-data_offs field

Change-Id: Ib2d86d7251fb948705f0c5ef32a9340efc5fe0be
Signed-off-by: Alan Winkowski 
---
M platform/linux-generic/include/odp_packet_netmap.h
M platform/linux-generic/pktio/netmap.c
2 files changed, 10 insertions(+), 5 deletions(-)


  git pull ssh://vgitil04.il.marvell.com:29418/syssw/odp refs/changes/63/34863/1
-- 
To view, visit http://vgitil04.il.marvell.com:8080/34863
To unsubscribe, visit http://vgitil04.il.marvell.com:8080/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: Ib2d86d7251fb948705f0c5ef32a9340efc5fe0be
Gerrit-PatchSet: 1
Gerrit-Project: syssw/odp
Gerrit-Branch: odp-devel
Gerrit-Owner: Alan Winkowski 


[lng-odp] Change in syssw/odp[odp-devel]: odp: odp_packet_print: add dump for payload

2016-12-21 Thread Alan Winkowski (Code Review)
Alan Winkowski has uploaded a new change for review.

  http://vgitil04.il.marvell.com:8080/34861

Change subject: odp: odp_packet_print: add dump for payload
..

odp: odp_packet_print: add dump for payload

Change-Id: Ib73a21e85e3b90c8f0344ad9f230f47642a7
Signed-off-by: Alan Winkowski 
---
M platform/linux-generic/odp_packet.c
1 file changed, 24 insertions(+), 0 deletions(-)


  git pull ssh://vgitil04.il.marvell.com:29418/syssw/odp refs/changes/61/34861/1
-- 
To view, visit http://vgitil04.il.marvell.com:8080/34861
To unsubscribe, visit http://vgitil04.il.marvell.com:8080/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: Ib73a21e85e3b90c8f0344ad9f230f47642a7
Gerrit-PatchSet: 1
Gerrit-Project: syssw/odp
Gerrit-Branch: odp-devel
Gerrit-Owner: Alan Winkowski 


[lng-odp] Change in syssw/odp[odp-devel]: odp: increase burst size to 256

2016-12-21 Thread Alan Winkowski (Code Review)
Alan Winkowski has uploaded a new change for review.

  http://vgitil04.il.marvell.com:8080/34859

Change subject: odp: increase burst size to 256
..

odp: increase burst size to 256

Change-Id: Ie484b15a2c94b9dfd1d506555b8bee5a47dcce55
Signed-off-by: Alan Winkowski 
---
M platform/linux-generic/include/odp_config_internal.h
1 file changed, 1 insertion(+), 1 deletion(-)


  git pull ssh://vgitil04.il.marvell.com:29418/syssw/odp refs/changes/59/34859/1
-- 
To view, visit http://vgitil04.il.marvell.com:8080/34859
To unsubscribe, visit http://vgitil04.il.marvell.com:8080/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: Ie484b15a2c94b9dfd1d506555b8bee5a47dcce55
Gerrit-PatchSet: 1
Gerrit-Project: syssw/odp
Gerrit-Branch: odp-devel
Gerrit-Owner: Alan Winkowski 


Re: [lng-odp] [API-NEXT PATCHv2 1/5] api: packet: add support for packet references

2016-12-21 Thread Bill Fischofer
On Wed, Dec 21, 2016 at 6:42 AM, Bala Manoharan
 wrote:
> Regards,
> Bala
>
>
> On 20 December 2016 at 22:35, Bill Fischofer  
> wrote:
>> On Tue, Dec 20, 2016 at 8:26 AM, Bala Manoharan
>>  wrote:
>>> On 20 December 2016 at 18:26, Bill Fischofer  
>>> wrote:
 On Tue, Dec 20, 2016 at 5:59 AM, Bala Manoharan
  wrote:
> Regards,
> Bala
>
>
> On 19 December 2016 at 20:23, Bill Fischofer  
> wrote:
>> On Mon, Dec 19, 2016 at 4:06 AM, Bala Manoharan
>>  wrote:
>>> Comments inline
>>>
>>>
>>> On 15 November 2016 at 20:14, Bill Fischofer 
>>>  wrote:
 Introduce three new APIs that support efficient sharing of portions of
 packets.

 odp_packet_ref_static() creates an alias for a base packet

 odp_packet_ref() creates a reference to a base packet

 odp_packet_ref_pkt() creates a reference to a base packet from a 
 supplied
 header packet

 In addition, three other APIs simplify working with references

 odp_packet_is_ref() says whether a packet is a reference

 odp_packet_has_ref() says whether a packet has had a reference made to 
 it

 odp_packet_unshared_len() gives the length of the prefix bytes that are
 unique to this reference. These are the only bytes of the packet that 
 may
 be modified safely.

 Signed-off-by: Bill Fischofer 
 ---
  include/odp/api/spec/packet.h | 168 
 ++
  1 file changed, 168 insertions(+)

 diff --git a/include/odp/api/spec/packet.h 
 b/include/odp/api/spec/packet.h
 index faf62e2..137024f 100644
 --- a/include/odp/api/spec/packet.h
 +++ b/include/odp/api/spec/packet.h
 @@ -256,6 +256,23 @@ uint32_t odp_packet_seg_len(odp_packet_t pkt);
  uint32_t odp_packet_len(odp_packet_t pkt);

  /**
 + * Packet unshared data len
 + *
 + * Returns the sum of data lengths over all unshared packet segments. 
 These
 + * are the only bytes that should be modified by the caller. The rest 
 of the
 + * packet should be treated as read-only.
 + *
 + * This routine will return 0 if odp_packet_has_ref() != 0 and will 
 be the
 + * same as odp_packet_len() if odp_packet_has_ref() == 0 and
 + * odp_packet_is_ref() == 0.
 + *
 + * @param pkt Packet handle
 + *
 + * @return Packet unshared data length
 + */
 +uint32_t odp_packet_unshared_len(odp_packet_t pkt);
 +
 +/**
   * Packet headroom length
   *
   * Returns the current packet level headroom length.
 @@ -847,6 +864,157 @@ int odp_packet_split(odp_packet_t *pkt, uint32_t 
 len, odp_packet_t *tail);

  /*
   *
 + * References
 + * 
 + *
 + */
 +
 +/**
 + * Create a static reference to a packet
 + *
 + * A static reference is used to obtain an additional handle for 
 referring to
 + * a packet so that the storage behind it is not freed until all 
 references to
 + * the packet are freed. This can be used, for example, to support 
 efficient
 + * retransmission processing.
 + *
 + * The intent of a static reference is that both the base packet and 
 the
 + * returned reference will be treated as read-only after this call. 
 Results
 + * are unpredictable if this restriction is not observed.
 + *
 + * Static references have restrictions but may have performance 
 advantages on
 + * some platforms if the caller does not intend to modify the 
 reference
 + * packet. If modification is needed (e.g., to prefix a unique header 
 onto the
 + * packet) then odp_packet_ref() or odp_packet_ref_pkt() should be 
 used.
 + *
 + * @param pktHandle of the base packet for which a static 
 reference is
 + *   to be created.
 + *
 + * @returnHandle of the static reference packet
 + * @retval ODP_PACKET_INVALID Operation failed. Base packet remains 
 unchanged.
 + */
 +odp_packet_t odp_packet_ref_static(odp_packet_t pkt);
>>>
>>> It is better to change the API signature to return the updated handle

[lng-odp] [PATCH] test: tm: queue id can be not updated

2016-12-21 Thread Maxim Uvarov
get_unique_id() function on error do not update it's second and
third arguments (unique_id and is_ipv4). gcc -O3 finds that
and traps compilation.

Signed-off-by: Maxim Uvarov 
---
 test/common_plat/validation/api/traffic_mngr/traffic_mngr.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/test/common_plat/validation/api/traffic_mngr/traffic_mngr.c 
b/test/common_plat/validation/api/traffic_mngr/traffic_mngr.c
index fcc7187..0271758 100644
--- a/test/common_plat/validation/api/traffic_mngr/traffic_mngr.c
+++ b/test/common_plat/validation/api/traffic_mngr/traffic_mngr.c
@@ -944,8 +944,8 @@ static void dump_rcvd_pkts(uint32_t first_rcv_idx, uint32_t 
last_rcv_idx)
odp_packet_trcv_pkt;
uint32_trcv_idx;
int32_t xmt_idx;
-   uint16_tunique_id;
-   uint8_t is_ipv4;
+   uint16_tunique_id = 0;
+   uint8_t is_ipv4 = 0;
int rc;
 
for (rcv_idx = first_rcv_idx; rcv_idx <= last_rcv_idx; rcv_idx++) {
-- 
2.7.1.250.gff4ea60



Re: [lng-odp] Merge API-NEXT to NEXT

2016-12-21 Thread Savolainen, Petri (Nokia - FI/Espoo)
I'd like to get this to api-next and after that to next/master.

linux-gen: schedule_sp: use ringas  priority queue

-Petri


> -Original Message-
> From: lng-odp [mailto:lng-odp-boun...@lists.linaro.org] On Behalf Of Mike
> Holmes
> Sent: Tuesday, December 20, 2016 6:57 PM
> To: lng-odp 
> Subject: Re: [lng-odp] Merge API-NEXT to NEXT
> 
> This is the ABI change
> http://people.linaro.org/~mike.holmes/diff-abi/libodp-compat_report.html
> 
> On 20 December 2016 at 11:03, Mike Holmes  wrote:
> 
> > All
> >
> > next has been updated with the proposed 12.x tag, please respond to this
> > thread if you have  any issues with it.
> >
> > Mike
> >
> > --
> > Mike Holmes
> > Program Manager - Linaro Networking Group
> > Linaro.org  *│ *Open source software for ARM
> SoCs
> > "Work should be fun and collaborative, the rest follows"
> >
> >
> >
> 
> 
> --
> Mike Holmes
> Program Manager - Linaro Networking Group
> Linaro.org  *│ *Open source software for ARM SoCs
> "Work should be fun and collaborative, the rest follows"


[lng-odp] [API-NEXT PATCHv2] linux-gen: _ishm: fixing typos

2016-12-21 Thread Christophe Milard
Fixing a set of iritating typos. just in comments.

Signed-off-by: Christophe Milard 
---

Since V1:
another typo found by Bill

 platform/linux-generic/_ishm.c | 24 
 1 file changed, 12 insertions(+), 12 deletions(-)

diff --git a/platform/linux-generic/_ishm.c b/platform/linux-generic/_ishm.c
index 449e357..a49c8e4 100644
--- a/platform/linux-generic/_ishm.c
+++ b/platform/linux-generic/_ishm.c
@@ -75,7 +75,7 @@
 /*
  * Maximum number of internal shared memory blocks.
  *
- * This the the number of separate ISHM areas that can be reserved concurrently
+ * This is the number of separate ISHM areas that can be reserved concurrently
  * (Note that freeing such blocks may take time, or possibly never happen
  * if some of the block ownwers never procsync() after free). This number
  * should take that into account)
@@ -236,7 +236,7 @@ static void procsync(void);
  * Take a piece of the preallocated virtual space to fit "size" bytes.
  * (best fit). Size must be rounded up to an integer number of pages size.
  * Possibly split the fragment to keep track of remaining space.
- * Returns the allocated fragment (best_fragmnt) and the corresponding address.
+ * Returns the allocated fragment (best_fragment) and the corresponding 
address.
  * External caller must ensure mutex before the call!
  */
 static void *alloc_fragment(uintptr_t size, int block_index, intptr_t align,
@@ -282,11 +282,11 @@ static void *alloc_fragment(uintptr_t size, int 
block_index, intptr_t align,
 
/*
 * if there is room between previous fragment and new one, (due to
-* alignement requirement) then fragment (split) the space between
+* alignment requirement) then fragment (split) the space between
 * the end of the previous fragment and the beginning of the new one:
 */
if (border - (uintptr_t)(*best_fragmnt)->start > 0) {
-   /* frangment space, i.e. take a new fragment descriptor... */
+   /* fragment space, i.e. take a new fragment descriptor... */
rem_fragmnt = ishm_ftbl->unused_fragmnts;
if (!rem_fragmnt) {
ODP_ERR("unable to get shmem fragment descriptor!\n.");
@@ -316,7 +316,7 @@ static void *alloc_fragment(uintptr_t size, int 
block_index, intptr_t align,
if (remainder == 0)
return (*best_fragmnt)->start;
 
-   /* otherwise, frangment space, i.e. take a new fragment descriptor... */
+   /* otherwise, fragment space, i.e. take a new fragment descriptor... */
rem_fragmnt = ishm_ftbl->unused_fragmnts;
if (!rem_fragmnt) {
ODP_ERR("unable to get shmem fragment descriptor!\n.");
@@ -503,7 +503,7 @@ static void delete_file(ishm_block_t *block)
  * performs the mapping, possibly allocating a fragment of the pre-reserved
  * VA space if the _ODP_ISHM_SINGLE_VA flag was given.
  * Sets fd, and returns the mapping address.
- * This funstion will also set the _ODP_ISHM_SINGLE_VA flag if the alignment
+ * This function will also set the _ODP_ISHM_SINGLE_VA flag if the alignment
  * requires it
  * Mutex must be assured by the caller.
  */
@@ -724,7 +724,7 @@ static void procsync(void)
 
last = ishm_proctable->nb_entries;
while (i < last) {
-   /* if the procecess sequence number doesn't match the main
+   /* if the process sequence number doesn't match the main
 * table seq number, this entry is obsolete
 */
block = _tbl->block[ishm_proctable->entry[i].block_index];
@@ -1033,7 +1033,7 @@ static int block_free(int block_index)
 }
 
 /*
- * Free and unmap internal shared memory, intentified by its block number:
+ * Free and unmap internal shared memory, identified by its block number:
  * return -1 on error. 0 if OK.
  */
 int _odp_ishm_free_by_index(int block_index)
@@ -1049,7 +1049,7 @@ int _odp_ishm_free_by_index(int block_index)
 }
 
 /*
- * free and unmap internal shared memory, intentified by its block name:
+ * free and unmap internal shared memory, identified by its block name:
  * return -1 on error. 0 if OK.
  */
 int _odp_ishm_free_by_name(const char *name)
@@ -1460,8 +1460,8 @@ static int do_odp_ishm_term_local(void)
 * Go through the table of visible blocks for this process,
 * decreasing the refcnt of each visible blocks, and issuing
 * warning for those no longer referenced by any process.
-* Note that non-referenced blocks are nor freeed: this is
-* deliberate as this would imply that the sementic of the
+* Note that non-referenced blocks are not freed: this is
+* deliberate as this would imply that the semantic of the
 * freeing function would differ depending on whether we run
 * with odp_thread as processes or pthreads. With this approach,
  

Re: [lng-odp] [API-NEXT PATCHv2 1/5] api: packet: add support for packet references

2016-12-21 Thread Bala Manoharan
Regards,
Bala


On 20 December 2016 at 22:35, Bill Fischofer  wrote:
> On Tue, Dec 20, 2016 at 8:26 AM, Bala Manoharan
>  wrote:
>> On 20 December 2016 at 18:26, Bill Fischofer  
>> wrote:
>>> On Tue, Dec 20, 2016 at 5:59 AM, Bala Manoharan
>>>  wrote:
 Regards,
 Bala


 On 19 December 2016 at 20:23, Bill Fischofer  
 wrote:
> On Mon, Dec 19, 2016 at 4:06 AM, Bala Manoharan
>  wrote:
>> Comments inline
>>
>>
>> On 15 November 2016 at 20:14, Bill Fischofer  
>> wrote:
>>> Introduce three new APIs that support efficient sharing of portions of
>>> packets.
>>>
>>> odp_packet_ref_static() creates an alias for a base packet
>>>
>>> odp_packet_ref() creates a reference to a base packet
>>>
>>> odp_packet_ref_pkt() creates a reference to a base packet from a 
>>> supplied
>>> header packet
>>>
>>> In addition, three other APIs simplify working with references
>>>
>>> odp_packet_is_ref() says whether a packet is a reference
>>>
>>> odp_packet_has_ref() says whether a packet has had a reference made to 
>>> it
>>>
>>> odp_packet_unshared_len() gives the length of the prefix bytes that are
>>> unique to this reference. These are the only bytes of the packet that 
>>> may
>>> be modified safely.
>>>
>>> Signed-off-by: Bill Fischofer 
>>> ---
>>>  include/odp/api/spec/packet.h | 168 
>>> ++
>>>  1 file changed, 168 insertions(+)
>>>
>>> diff --git a/include/odp/api/spec/packet.h 
>>> b/include/odp/api/spec/packet.h
>>> index faf62e2..137024f 100644
>>> --- a/include/odp/api/spec/packet.h
>>> +++ b/include/odp/api/spec/packet.h
>>> @@ -256,6 +256,23 @@ uint32_t odp_packet_seg_len(odp_packet_t pkt);
>>>  uint32_t odp_packet_len(odp_packet_t pkt);
>>>
>>>  /**
>>> + * Packet unshared data len
>>> + *
>>> + * Returns the sum of data lengths over all unshared packet segments. 
>>> These
>>> + * are the only bytes that should be modified by the caller. The rest 
>>> of the
>>> + * packet should be treated as read-only.
>>> + *
>>> + * This routine will return 0 if odp_packet_has_ref() != 0 and will be 
>>> the
>>> + * same as odp_packet_len() if odp_packet_has_ref() == 0 and
>>> + * odp_packet_is_ref() == 0.
>>> + *
>>> + * @param pkt Packet handle
>>> + *
>>> + * @return Packet unshared data length
>>> + */
>>> +uint32_t odp_packet_unshared_len(odp_packet_t pkt);
>>> +
>>> +/**
>>>   * Packet headroom length
>>>   *
>>>   * Returns the current packet level headroom length.
>>> @@ -847,6 +864,157 @@ int odp_packet_split(odp_packet_t *pkt, uint32_t 
>>> len, odp_packet_t *tail);
>>>
>>>  /*
>>>   *
>>> + * References
>>> + * 
>>> + *
>>> + */
>>> +
>>> +/**
>>> + * Create a static reference to a packet
>>> + *
>>> + * A static reference is used to obtain an additional handle for 
>>> referring to
>>> + * a packet so that the storage behind it is not freed until all 
>>> references to
>>> + * the packet are freed. This can be used, for example, to support 
>>> efficient
>>> + * retransmission processing.
>>> + *
>>> + * The intent of a static reference is that both the base packet and 
>>> the
>>> + * returned reference will be treated as read-only after this call. 
>>> Results
>>> + * are unpredictable if this restriction is not observed.
>>> + *
>>> + * Static references have restrictions but may have performance 
>>> advantages on
>>> + * some platforms if the caller does not intend to modify the reference
>>> + * packet. If modification is needed (e.g., to prefix a unique header 
>>> onto the
>>> + * packet) then odp_packet_ref() or odp_packet_ref_pkt() should be 
>>> used.
>>> + *
>>> + * @param pktHandle of the base packet for which a static 
>>> reference is
>>> + *   to be created.
>>> + *
>>> + * @returnHandle of the static reference packet
>>> + * @retval ODP_PACKET_INVALID Operation failed. Base packet remains 
>>> unchanged.
>>> + */
>>> +odp_packet_t odp_packet_ref_static(odp_packet_t pkt);
>>
>> It is better to change the API signature to return the updated handle
>> of the base packet also to the application.
>> Similar to the API for odp_packet_extend_head().
>
> I think returning the packet ref directly rather than indirectly makes
> for easier coding on the part of 

Re: [lng-odp] [API-NEXT PATCH] linux-gen: _ishm: fixing typos

2016-12-21 Thread Bill Fischofer
On Wed, Dec 21, 2016 at 5:16 AM, Christophe Milard
 wrote:
> Fixing a set of iritating typos. just in comments.
>
> Signed-off-by: Christophe Milard 
> ---
>  platform/linux-generic/_ishm.c | 24 
>  1 file changed, 12 insertions(+), 12 deletions(-)
>
> diff --git a/platform/linux-generic/_ishm.c b/platform/linux-generic/_ishm.c
> index 449e357..939b74a 100644
> --- a/platform/linux-generic/_ishm.c
> +++ b/platform/linux-generic/_ishm.c
> @@ -75,7 +75,7 @@
>  /*
>   * Maximum number of internal shared memory blocks.
>   *
> - * This the the number of separate ISHM areas that can be reserved 
> concurrently
> + * This is the number of separate ISHM areas that can be reserved 
> concurrently
>   * (Note that freeing such blocks may take time, or possibly never happen
>   * if some of the block ownwers never procsync() after free). This number
>   * should take that into account)
> @@ -236,7 +236,7 @@ static void procsync(void);
>   * Take a piece of the preallocated virtual space to fit "size" bytes.
>   * (best fit). Size must be rounded up to an integer number of pages size.
>   * Possibly split the fragment to keep track of remaining space.
> - * Returns the allocated fragment (best_fragmnt) and the corresponding 
> address.
> + * Returns the allocated fragment (best_fragment) and the corresponding 
> address.
>   * External caller must ensure mutex before the call!
>   */
>  static void *alloc_fragment(uintptr_t size, int block_index, intptr_t align,
> @@ -282,11 +282,11 @@ static void *alloc_fragment(uintptr_t size, int 
> block_index, intptr_t align,
>
> /*
>  * if there is room between previous fragment and new one, (due to
> -* alignement requirement) then fragment (split) the space between
> +* alignment requirement) then fragment (split) the space between
>  * the end of the previous fragment and the beginning of the new one:
>  */
> if (border - (uintptr_t)(*best_fragmnt)->start > 0) {
> -   /* frangment space, i.e. take a new fragment descriptor... */
> +   /* fragment space, i.e. take a new fragment descriptor... */
> rem_fragmnt = ishm_ftbl->unused_fragmnts;
> if (!rem_fragmnt) {
> ODP_ERR("unable to get shmem fragment 
> descriptor!\n.");
> @@ -316,7 +316,7 @@ static void *alloc_fragment(uintptr_t size, int 
> block_index, intptr_t align,
> if (remainder == 0)
> return (*best_fragmnt)->start;
>
> -   /* otherwise, frangment space, i.e. take a new fragment descriptor... 
> */
> +   /* otherwise, fragment space, i.e. take a new fragment descriptor... 
> */
> rem_fragmnt = ishm_ftbl->unused_fragmnts;
> if (!rem_fragmnt) {
> ODP_ERR("unable to get shmem fragment descriptor!\n.");
> @@ -503,7 +503,7 @@ static void delete_file(ishm_block_t *block)
>   * performs the mapping, possibly allocating a fragment of the pre-reserved
>   * VA space if the _ODP_ISHM_SINGLE_VA flag was given.
>   * Sets fd, and returns the mapping address.
> - * This funstion will also set the _ODP_ISHM_SINGLE_VA flag if the alignment
> + * This function will also set the _ODP_ISHM_SINGLE_VA flag if the alignment
>   * requires it
>   * Mutex must be assured by the caller.
>   */
> @@ -724,7 +724,7 @@ static void procsync(void)
>
> last = ishm_proctable->nb_entries;
> while (i < last) {
> -   /* if the procecess sequence number doesn't match the main
> +   /* if the process sequence number doesn't match the main
>  * table seq number, this entry is obsolete
>  */
> block = 
> _tbl->block[ishm_proctable->entry[i].block_index];
> @@ -1033,7 +1033,7 @@ static int block_free(int block_index)
>  }
>
>  /*
> - * Free and unmap internal shared memory, intentified by its block number:
> + * Free and unmap internal shared memory, identified by its block number:
>   * return -1 on error. 0 if OK.
>   */
>  int _odp_ishm_free_by_index(int block_index)
> @@ -1049,7 +1049,7 @@ int _odp_ishm_free_by_index(int block_index)
>  }
>
>  /*
> - * free and unmap internal shared memory, intentified by its block name:
> + * free and unmap internal shared memory, identified by its block name:
>   * return -1 on error. 0 if OK.
>   */
>  int _odp_ishm_free_by_name(const char *name)
> @@ -1460,8 +1460,8 @@ static int do_odp_ishm_term_local(void)
>  * Go through the table of visible blocks for this process,
>  * decreasing the refcnt of each visible blocks, and issuing
>  * warning for those no longer referenced by any process.
> -* Note that non-referenced blocks are nor freeed: this is
> -* deliberate as this would imply that the sementic of the
> +* Note that non-referenced blocks are nor 

Re: [lng-odp] [API-NEXT PATCH v2] validation: test creating pool and timer pool with no name

2016-12-21 Thread Bill Fischofer
On Wed, Dec 21, 2016 at 5:27 AM, Matias Elo  wrote:
> Signed-off-by: Matias Elo 
> ---
>
> Reviewed-by: Petri Savolainen 

Reviewed-by: Bill Fischofer 

>
>  test/common_plat/validation/api/pool/pool.c   | 7 +--
>  test/common_plat/validation/api/timer/timer.c | 2 +-
>  2 files changed, 2 insertions(+), 7 deletions(-)
>
> diff --git a/test/common_plat/validation/api/pool/pool.c 
> b/test/common_plat/validation/api/pool/pool.c
> index d48ac2a..8687941 100644
> --- a/test/common_plat/validation/api/pool/pool.c
> +++ b/test/common_plat/validation/api/pool/pool.c
> @@ -8,19 +8,14 @@
>  #include "odp_cunit_common.h"
>  #include "pool.h"
>
> -static int pool_name_number = 1;
>  static const int default_buffer_size = 1500;
>  static const int default_buffer_num = 1000;
>
>  static void pool_create_destroy(odp_pool_param_t *params)
>  {
> odp_pool_t pool;
> -   char pool_name[ODP_POOL_NAME_LEN];
>
> -   snprintf(pool_name, sizeof(pool_name),
> -"test_pool-%d", pool_name_number++);
> -
> -   pool = odp_pool_create(pool_name, params);
> +   pool = odp_pool_create(NULL, params);
> CU_ASSERT_FATAL(pool != ODP_POOL_INVALID);
> CU_ASSERT(odp_pool_to_u64(pool) !=
>   odp_pool_to_u64(ODP_POOL_INVALID));
> diff --git a/test/common_plat/validation/api/timer/timer.c 
> b/test/common_plat/validation/api/timer/timer.c
> index 0007639..1945afa 100644
> --- a/test/common_plat/validation/api/timer/timer.c
> +++ b/test/common_plat/validation/api/timer/timer.c
> @@ -156,7 +156,7 @@ void timer_test_odp_timer_cancel(void)
> tparam.num_timers = 1;
> tparam.priv   = 0;
> tparam.clk_src= ODP_CLOCK_CPU;
> -   tp = odp_timer_pool_create("timer_pool0", );
> +   tp = odp_timer_pool_create(NULL, );
> if (tp == ODP_TIMER_POOL_INVALID)
> CU_FAIL_FATAL("Timer pool create failed");
>
> --
> 2.7.4
>


[lng-odp] [API-NEXT PATCH v2] validation: test creating pool and timer pool with no name

2016-12-21 Thread Matias Elo
Signed-off-by: Matias Elo 
---

Reviewed-by: Petri Savolainen 

 test/common_plat/validation/api/pool/pool.c   | 7 +--
 test/common_plat/validation/api/timer/timer.c | 2 +-
 2 files changed, 2 insertions(+), 7 deletions(-)

diff --git a/test/common_plat/validation/api/pool/pool.c 
b/test/common_plat/validation/api/pool/pool.c
index d48ac2a..8687941 100644
--- a/test/common_plat/validation/api/pool/pool.c
+++ b/test/common_plat/validation/api/pool/pool.c
@@ -8,19 +8,14 @@
 #include "odp_cunit_common.h"
 #include "pool.h"
 
-static int pool_name_number = 1;
 static const int default_buffer_size = 1500;
 static const int default_buffer_num = 1000;
 
 static void pool_create_destroy(odp_pool_param_t *params)
 {
odp_pool_t pool;
-   char pool_name[ODP_POOL_NAME_LEN];
 
-   snprintf(pool_name, sizeof(pool_name),
-"test_pool-%d", pool_name_number++);
-
-   pool = odp_pool_create(pool_name, params);
+   pool = odp_pool_create(NULL, params);
CU_ASSERT_FATAL(pool != ODP_POOL_INVALID);
CU_ASSERT(odp_pool_to_u64(pool) !=
  odp_pool_to_u64(ODP_POOL_INVALID));
diff --git a/test/common_plat/validation/api/timer/timer.c 
b/test/common_plat/validation/api/timer/timer.c
index 0007639..1945afa 100644
--- a/test/common_plat/validation/api/timer/timer.c
+++ b/test/common_plat/validation/api/timer/timer.c
@@ -156,7 +156,7 @@ void timer_test_odp_timer_cancel(void)
tparam.num_timers = 1;
tparam.priv   = 0;
tparam.clk_src= ODP_CLOCK_CPU;
-   tp = odp_timer_pool_create("timer_pool0", );
+   tp = odp_timer_pool_create(NULL, );
if (tp == ODP_TIMER_POOL_INVALID)
CU_FAIL_FATAL("Timer pool create failed");
 
-- 
2.7.4



[lng-odp] [API-NEXT PATCH v2 1/2] api: unify ODP_*_NAME_LEN specifications

2016-12-21 Thread Matias Elo
Unify name length definitions to always include terminating null character.

Signed-off-by: Matias Elo 
---

Reviewed-by: Petri Savolainen 
Reviewed-by: Bill Fischofer 

 include/odp/api/spec/classification.h | 2 +-
 include/odp/api/spec/pool.h   | 2 +-
 include/odp/api/spec/queue.h  | 2 +-
 include/odp/api/spec/schedule.h   | 2 +-
 include/odp/api/spec/shared_memory.h  | 2 +-
 include/odp/api/spec/timer.h  | 2 +-
 6 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/include/odp/api/spec/classification.h 
b/include/odp/api/spec/classification.h
index 0e442c7..0e1addd 100644
--- a/include/odp/api/spec/classification.h
+++ b/include/odp/api/spec/classification.h
@@ -44,7 +44,7 @@ extern "C" {
 
 /**
  * @def ODP_COS_NAME_LEN
- * Maximum ClassOfService name length in chars
+ * Maximum ClassOfService name length in chars including null char
  */
 
 /**
diff --git a/include/odp/api/spec/pool.h b/include/odp/api/spec/pool.h
index 041f4af..af2829b 100644
--- a/include/odp/api/spec/pool.h
+++ b/include/odp/api/spec/pool.h
@@ -36,7 +36,7 @@ extern "C" {
  * Invalid pool
  */
 
-/** Maximum queue name length in chars */
+/** Maximum pool name length in chars including null char */
 #define ODP_POOL_NAME_LEN  32
 
 /**
diff --git a/include/odp/api/spec/queue.h b/include/odp/api/spec/queue.h
index b0c5e31..7972fea 100644
--- a/include/odp/api/spec/queue.h
+++ b/include/odp/api/spec/queue.h
@@ -44,7 +44,7 @@ extern "C" {
 
 /**
  * @def ODP_QUEUE_NAME_LEN
- * Maximum queue name length in chars
+ * Maximum queue name length in chars including null char
  */
 
 /**
diff --git a/include/odp/api/spec/schedule.h b/include/odp/api/spec/schedule.h
index f976a4c..8244746 100644
--- a/include/odp/api/spec/schedule.h
+++ b/include/odp/api/spec/schedule.h
@@ -42,7 +42,7 @@ extern "C" {
 
 /**
  * @def ODP_SCHED_GROUP_NAME_LEN
- * Maximum schedule group name length in chars
+ * Maximum schedule group name length in chars including null char
  */
 
 /**
diff --git a/include/odp/api/spec/shared_memory.h 
b/include/odp/api/spec/shared_memory.h
index 885751d..074c883 100644
--- a/include/odp/api/spec/shared_memory.h
+++ b/include/odp/api/spec/shared_memory.h
@@ -40,7 +40,7 @@ extern "C" {
  * Synonym for buffer pool use
  */
 
-/** Maximum shared memory block name length in chars */
+/** Maximum shared memory block name length in chars including null char */
 #define ODP_SHM_NAME_LEN 32
 
 /*
diff --git a/include/odp/api/spec/timer.h b/include/odp/api/spec/timer.h
index 49221c4..46a4369 100644
--- a/include/odp/api/spec/timer.h
+++ b/include/odp/api/spec/timer.h
@@ -90,7 +90,7 @@ typedef enum {
ODP_TIMER_NOEVENT = -3
 } odp_timer_set_t;
 
-/** Maximum timer pool name length in chars (including null char) */
+/** Maximum timer pool name length in chars including null char */
 #define ODP_TIMER_POOL_NAME_LEN  32
 
 /** Timer pool parameters
-- 
2.7.4



[lng-odp] [API-NEXT PATCH v2 2/2] api: move ODP_*_NAME_LEN definitions from API to implementation

2016-12-21 Thread Matias Elo
Enables the implementations to choose the define values.

Signed-off-by: Matias Elo 
---

Reviewed-by: Petri Savolainen 
Reviewed-by: Bill Fischofer 

 include/odp/api/spec/pool.h   | 6 --
 include/odp/api/spec/shared_memory.h  | 6 --
 include/odp/api/spec/timer.h  | 6 --
 platform/linux-generic/include/odp/api/plat/pool_types.h  | 2 ++
 platform/linux-generic/include/odp/api/plat/shared_memory_types.h | 2 ++
 platform/linux-generic/include/odp/api/plat/timer_types.h | 2 ++
 6 files changed, 18 insertions(+), 6 deletions(-)

diff --git a/include/odp/api/spec/pool.h b/include/odp/api/spec/pool.h
index af2829b..c0de195 100644
--- a/include/odp/api/spec/pool.h
+++ b/include/odp/api/spec/pool.h
@@ -36,8 +36,10 @@ extern "C" {
  * Invalid pool
  */
 
-/** Maximum pool name length in chars including null char */
-#define ODP_POOL_NAME_LEN  32
+/**
+ * @def ODP_POOL_NAME_LEN
+ * Maximum pool name length in chars including null char
+ */
 
 /**
  * Pool capabilities
diff --git a/include/odp/api/spec/shared_memory.h 
b/include/odp/api/spec/shared_memory.h
index 074c883..1a9c129 100644
--- a/include/odp/api/spec/shared_memory.h
+++ b/include/odp/api/spec/shared_memory.h
@@ -40,8 +40,10 @@ extern "C" {
  * Synonym for buffer pool use
  */
 
-/** Maximum shared memory block name length in chars including null char */
-#define ODP_SHM_NAME_LEN 32
+/**
+ * @def ODP_SHM_NAME_LEN
+ * Maximum shared memory block name length in chars including null char
+ */
 
 /*
  * Shared memory flags:
diff --git a/include/odp/api/spec/timer.h b/include/odp/api/spec/timer.h
index 46a4369..75f9db9 100644
--- a/include/odp/api/spec/timer.h
+++ b/include/odp/api/spec/timer.h
@@ -90,8 +90,10 @@ typedef enum {
ODP_TIMER_NOEVENT = -3
 } odp_timer_set_t;
 
-/** Maximum timer pool name length in chars including null char */
-#define ODP_TIMER_POOL_NAME_LEN  32
+/**
+ * @def ODP_TIMER_POOL_NAME_LEN
+ * Maximum timer pool name length in chars including null char
+ */
 
 /** Timer pool parameters
  * Timer pool parameters are used when creating and querying timer pools.
diff --git a/platform/linux-generic/include/odp/api/plat/pool_types.h 
b/platform/linux-generic/include/odp/api/plat/pool_types.h
index 4e39de5..6baff09 100644
--- a/platform/linux-generic/include/odp/api/plat/pool_types.h
+++ b/platform/linux-generic/include/odp/api/plat/pool_types.h
@@ -30,6 +30,8 @@ typedef ODP_HANDLE_T(odp_pool_t);
 
 #define ODP_POOL_INVALID _odp_cast_scalar(odp_pool_t, 0x)
 
+#define ODP_POOL_NAME_LEN  32
+
 /**
  * Pool type
  */
diff --git a/platform/linux-generic/include/odp/api/plat/shared_memory_types.h 
b/platform/linux-generic/include/odp/api/plat/shared_memory_types.h
index 4d8bbcc..afa0bf9 100644
--- a/platform/linux-generic/include/odp/api/plat/shared_memory_types.h
+++ b/platform/linux-generic/include/odp/api/plat/shared_memory_types.h
@@ -31,6 +31,8 @@ typedef ODP_HANDLE_T(odp_shm_t);
 #define ODP_SHM_INVALID _odp_cast_scalar(odp_shm_t, 0)
 #define ODP_SHM_NULL ODP_SHM_INVALID
 
+#define ODP_SHM_NAME_LEN 32
+
 /** Get printable format of odp_shm_t */
 static inline uint64_t odp_shm_to_u64(odp_shm_t hdl)
 {
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 68d6f6f..8821bed 100644
--- a/platform/linux-generic/include/odp/api/plat/timer_types.h
+++ b/platform/linux-generic/include/odp/api/plat/timer_types.h
@@ -30,6 +30,8 @@ typedef struct odp_timer_pool_s *odp_timer_pool_t;
 
 #define ODP_TIMER_POOL_INVALID NULL
 
+#define ODP_TIMER_POOL_NAME_LEN  32
+
 typedef ODP_HANDLE_T(odp_timer_t);
 
 #define ODP_TIMER_INVALID _odp_cast_scalar(odp_timer_t, 0x)
-- 
2.7.4



Re: [lng-odp] [API-NEXT PATCH 1/3] api: unify ODP_*_NAME_LEN specifications

2016-12-21 Thread Elo, Matias (Nokia - FI/Espoo)

> For Parts 1 and 2:
> 
> Reviewed-by: Bill Fischofer 
> 
> As noted in Part 3, that also looks fine but should be a separate
> patch since it doesn't seem to be related to the purpose of Parts 1
> and 2.

OK, I'll fix this.

-Matias



[lng-odp] [API-NEXT PATCH 5/5] test: drv: shm: adding small allocation stress tests

2016-12-21 Thread Christophe Milard
stress tests added for allocation of small memory units using
odpdrv_shm_sreserve() and odpdrv_shm_sfree().

Signed-off-by: Christophe Milard 
---
 .../common_plat/validation/drv/drvshmem/drvshmem.c | 164 +
 .../common_plat/validation/drv/drvshmem/drvshmem.h |   1 +
 2 files changed, 165 insertions(+)

diff --git a/test/common_plat/validation/drv/drvshmem/drvshmem.c 
b/test/common_plat/validation/drv/drvshmem/drvshmem.c
index c129693..addb0f3 100644
--- a/test/common_plat/validation/drv/drvshmem/drvshmem.c
+++ b/test/common_plat/validation/drv/drvshmem/drvshmem.c
@@ -838,12 +838,176 @@ void drvshmem_test_small_basic(void)
odpdrv_shm_print_all("sreserve test: after all free");
 }
 
+/*
+ * thread part for the drvshmem_test_stress_small
+ */
+static int run_test_stress_small(void *arg ODP_UNUSED)
+{
+   odpdrv_shm_t shm;
+   uint8_t *address;
+   shared_test_data_t *glob_data;
+   uint8_t random_bytes[STRESS_RANDOM_SZ];
+   uint32_t index;
+   uint32_t size;
+   uint8_t data;
+   uint32_t iter;
+   uint32_t i;
+
+   shm = odpdrv_shm_lookup_by_name(MEM_NAME);
+   glob_data = odpdrv_shm_addr(shm);
+   CU_ASSERT_PTR_NOT_NULL(glob_data);
+
+   /* wait for general GO! */
+   odpdrv_barrier_wait(_data->test_barrier1);
+   /*
+
+* at each iteration: pick up a random index for
+* glob_data->stress[index]: If the entry is free, allocated small mem
+* randomly. If it is already allocated, make checks and free it:
+* Note that different tread can allocate or free a given block
+*/
+   for (iter = 0; iter < STRESS_ITERATION; iter++) {
+   /* get 4 random bytes from which index, size ,align, flags
+* and data will be derived:
+*/
+   odp_random_data(random_bytes, STRESS_RANDOM_SZ, 0);
+   index = random_bytes[0] & (STRESS_SIZE - 1);
+
+   odp_spinlock_lock(_data->stress_lock);
+
+   switch (glob_data->stress[index].state) {
+   case STRESS_FREE:
+   /* allocated a new block for this entry */
+
+   glob_data->stress[index].state = STRESS_BUSY;
+   odp_spinlock_unlock(_data->stress_lock);
+
+   size  = (random_bytes[1] + 1) << 4; /* up to 4Kb */
+   data  = random_bytes[2];
+
+   address = odpdrv_shm_sreserve(size);
+   glob_data->stress[index].address = address;
+   if (address == NULL) { /* out of mem ? */
+   odp_spinlock_lock(_data->stress_lock);
+   glob_data->stress[index].state = STRESS_ALLOC;
+   odp_spinlock_unlock(_data->stress_lock);
+   continue;
+   }
+
+   glob_data->stress[index].size = size;
+   glob_data->stress[index].data_val = data;
+
+   /* write some data: */
+   for (i = 0; i < size; i++)
+   address[i] = (data++) & 0xFF;
+   odp_spinlock_lock(_data->stress_lock);
+   glob_data->stress[index].state = STRESS_ALLOC;
+   odp_spinlock_unlock(_data->stress_lock);
+
+   break;
+
+   case STRESS_ALLOC:
+   /* free the block for this entry */
+
+   glob_data->stress[index].state = STRESS_BUSY;
+   odp_spinlock_unlock(_data->stress_lock);
+   address = glob_data->stress[index].address;
+
+   if (shm == NULL) { /* out of mem ? */
+   odp_spinlock_lock(_data->stress_lock);
+   glob_data->stress[index].state = STRESS_FREE;
+   odp_spinlock_unlock(_data->stress_lock);
+   continue;
+   }
+
+   /* check that data is reachable and correct: */
+   data = glob_data->stress[index].data_val;
+   size = glob_data->stress[index].size;
+   for (i = 0; i < size; i++) {
+   CU_ASSERT(address[i] == (data & 0xFF));
+   data++;
+   }
+
+   CU_ASSERT(!odpdrv_shm_sfree(address));
+
+   odp_spinlock_lock(_data->stress_lock);
+   glob_data->stress[index].state = STRESS_FREE;
+   odp_spinlock_unlock(_data->stress_lock);
+
+   break;
+
+   case STRESS_BUSY:
+   default:
+   odp_spinlock_unlock(_data->stress_lock);
+   break;
+   }
+

[lng-odp] [API-NEXT PATCH 4/5] test: drv: shm: adding small allocation tests

2016-12-21 Thread Christophe Milard
Tests for odpdrv_shm_sreserve() and odpdrv_shm_sfree are added,
testing the ishm small memory unit allocator.

Signed-off-by: Christophe Milard 
---
 .../common_plat/validation/drv/drvshmem/drvshmem.c | 77 ++
 .../common_plat/validation/drv/drvshmem/drvshmem.h |  1 +
 2 files changed, 78 insertions(+)

diff --git a/test/common_plat/validation/drv/drvshmem/drvshmem.c 
b/test/common_plat/validation/drv/drvshmem/drvshmem.c
index 0247a03..c129693 100644
--- a/test/common_plat/validation/drv/drvshmem/drvshmem.c
+++ b/test/common_plat/validation/drv/drvshmem/drvshmem.c
@@ -21,6 +21,10 @@
 #define STRESS_RANDOM_SZ 5
 #define STRESS_ITERATION 5000
 
+#define TEST_SZ 1000
+#define SZ_1K   1024
+#define BUFF_PATTERN 0xA3
+
 typedef enum {
STRESS_FREE, /* entry is free and can be allocated */
STRESS_BUSY, /* entry is being processed: don't touch */
@@ -762,11 +766,84 @@ void drvshmem_test_stress(void)
CU_ASSERT(odpdrv_shm_print_all("After stress tests") == base);
 }
 
+void drvshmem_test_small_basic(void)
+{
+   uint8_t *buff;
+   uint8_t *addrs[TEST_SZ];
+   uint8_t length;
+   int i, j;
+
+   /* alloc a 1k buffer, filling its contents: */
+   buff = odpdrv_shm_sreserve(SZ_1K);
+   CU_ASSERT_PTR_NOT_NULL(buff);
+   for (i = 0; i < SZ_1K; i++)
+   buff[i] = BUFF_PATTERN;
+   odpdrv_shm_print_all("sreserve test: 1K reserved");
+
+   /* alloc as many buffer a possible on increseasing sz */
+   for (i = 0; i < TEST_SZ; i++) {
+   length = i * 16;
+   addrs[i] = odpdrv_shm_sreserve(length);
+   /* if alloc was success, fill buffer for later check */
+   if (addrs[i]) {
+   for (j = 0; j < length; j++)
+   addrs[i][j] = (uint8_t)(length & 0xFF);
+   }
+   }
+   odpdrv_shm_print_all("sreserve test: after many smalloc");
+
+   /* release every 3rth buffer, checking contents: */
+   for (i = 0; i < TEST_SZ; i += 3) {
+   /* if buffer was allocated, check the pattern in it */
+   if (addrs[i]) {
+   length = i * 16;
+   for (j = 0; j < length; j++)
+   CU_ASSERT(addrs[i][j] ==
+ (uint8_t)(length & 0xFF));
+   }
+   odpdrv_shm_sfree(addrs[i]);
+   }
+   odpdrv_shm_print_all("sreserve test: after 1/3 free:");
+
+   /* realloc them:*/
+   for (i = 0; i < TEST_SZ; i += 3) {
+   length = i * 16;
+   addrs[i] = odpdrv_shm_sreserve(length);
+   /* if alloc was success, fill buffer for later check */
+   if (addrs[i]) {
+   for (j = 0; j < length; j++)
+   addrs[i][j] = (uint8_t)(length & 0xFF);
+   }
+   }
+   odpdrv_shm_print_all("sreserve test: after realloc:");
+
+   /* free all (except buff), checking contents: */
+   for (i = 0; i < TEST_SZ; i++) {
+   /* if buffer was allocated, check the pattern in it */
+   if (addrs[i]) {
+   length = i * 16;
+   for (j = 0; j < length; j++)
+   CU_ASSERT(addrs[i][j] ==
+ (uint8_t)(length & 0xFF))
+   }
+   odpdrv_shm_sfree(addrs[i]);
+   }
+   odpdrv_shm_print_all("sreserve test: after all but 1K free:");
+
+   /* check contents of our initial 1K buffer: */
+   for (i = 0; i < SZ_1K; i++)
+   CU_ASSERT((buff[i] == BUFF_PATTERN))
+   odpdrv_shm_sfree(buff);
+
+   odpdrv_shm_print_all("sreserve test: after all free");
+}
+
 odp_testinfo_t drvshmem_suite[] = {
ODP_TEST_INFO(drvshmem_test_basic),
ODP_TEST_INFO(drvshmem_test_reserve_after_fork),
ODP_TEST_INFO(drvshmem_test_singleva_after_fork),
ODP_TEST_INFO(drvshmem_test_stress),
+   ODP_TEST_INFO(drvshmem_test_small_basic),
ODP_TEST_INFO_NULL,
 };
 
diff --git a/test/common_plat/validation/drv/drvshmem/drvshmem.h 
b/test/common_plat/validation/drv/drvshmem/drvshmem.h
index f4c26a1..0dcbccb 100644
--- a/test/common_plat/validation/drv/drvshmem/drvshmem.h
+++ b/test/common_plat/validation/drv/drvshmem/drvshmem.h
@@ -14,6 +14,7 @@ void drvshmem_test_basic(void);
 void drvshmem_test_reserve_after_fork(void);
 void drvshmem_test_singleva_after_fork(void);
 void drvshmem_test_stress(void);
+void drvshmem_test_small_basic(void);
 
 /* test arrays: */
 extern odp_testinfo_t drvshmem_suite[];
-- 
2.7.4



[lng-odp] [API-NEXT PATCH 3/5] linux-gen: drv: shm: adding buddy allocator

2016-12-21 Thread Christophe Milard
A pool for buddy allocation of small memory sizes is created at boot
time and functions to reserve/free memory from this pool added
(using the _ishm equivalent, of course).
Memory allocated from this pool is sharable between ODP threads,
(which malloc() would not guarantee in the odptreads as process case)

Signed-off-by: Christophe Milard 
---
 platform/linux-generic/drv_shm.c   | 44 +-
 .../linux-generic/include/odp_config_internal.h| 10 +
 platform/linux-generic/include/odp_internal.h  |  4 ++
 platform/linux-generic/odp_init.c  | 13 +++
 4 files changed, 70 insertions(+), 1 deletion(-)

diff --git a/platform/linux-generic/drv_shm.c b/platform/linux-generic/drv_shm.c
index 9b2560d..dec3e6c 100644
--- a/platform/linux-generic/drv_shm.c
+++ b/platform/linux-generic/drv_shm.c
@@ -7,7 +7,14 @@
 #include 
 #include 
 #include 
+#include 
+#include 
+#include 
 #include <_ishm_internal.h>
+#include <_ishmbuddy_internal.h>
+
+#define BUDDYPOOL_NAME "drvshm_buddy_pool"
+static _odpdrv_shm_bpool_t *buddy_pool;
 
 static inline uint32_t from_handle(odpdrv_shm_t shm)
 {
@@ -98,5 +105,40 @@ int odpdrv_shm_info(odpdrv_shm_t shm, odpdrv_shm_info_t 
*info)
 
 int odpdrv_shm_print_all(const char *title)
 {
-   return _odp_ishm_status(title);
+   int res;
+
+   /* print shm status */
+   res =  _odp_ishm_status(title);
+   /* print buddy allocator status: */
+   (void)_odp_ishmbud_status(buddy_pool);
+
+   return res;
+}
+
+void *odpdrv_shm_sreserve(uint64_t size)
+{
+   return _odp_ishmbud_alloc(buddy_pool, size);
+}
+
+int odpdrv_shm_sfree(void *addr)
+{
+   return _odp_ishmbud_free(buddy_pool, addr);
+}
+
+int _odpdrv_shm_init_global(void)
+{
+   /* create a buddy pool of granularity = cache_line_size: */
+   buddy_pool = _odp_ishmbud_pool_create(BUDDYPOOL_NAME,
+ ODP_CONFIG_DRVSHM_BUDDY_POOL_SZ,
+ ODP_CACHE_LINE_SIZE, 0);
+   if (!buddy_pool)
+   return -1;
+
+   return 0;
+}
+
+int _odpdrv_shm_term_global(void)
+{
+   /* destroy the buddy pool: */
+   return _odp_ishmbud_pool_destroy(buddy_pool);
 }
diff --git a/platform/linux-generic/include/odp_config_internal.h 
b/platform/linux-generic/include/odp_config_internal.h
index c494660..0b810c8 100644
--- a/platform/linux-generic/include/odp_config_internal.h
+++ b/platform/linux-generic/include/odp_config_internal.h
@@ -116,6 +116,16 @@ extern "C" {
  */
 #define ODP_CONFIG_ISHM_VA_PREALLOC_SZ (536870912L)
 
+/*
+ * Size of the DRVSHM buddy allocator pool, available on south interface.
+ *
+ * This pool is preallocated at init time. Memory from this pool can
+ * be allocated using odpdrv_shm_sreserve().
+ * should be a power of 2 (or will be rounded as such)
+ * In bytes.
+ */
+#define ODP_CONFIG_DRVSHM_BUDDY_POOL_SZ (1048576UL)
+
 /* Maximum number of shared memory blocks available on the driver interface.
  *
  * This the the number of separate SHM areas that can be reserved concurrently
diff --git a/platform/linux-generic/include/odp_internal.h 
b/platform/linux-generic/include/odp_internal.h
index b313b1f..908891f 100644
--- a/platform/linux-generic/include/odp_internal.h
+++ b/platform/linux-generic/include/odp_internal.h
@@ -59,6 +59,7 @@ enum init_stage {
SYSINFO_INIT,
FDSERVER_INIT,
ISHM_INIT,
+   DRVSHM_INIT,
THREAD_INIT,
POOL_INIT,
QUEUE_INIT,
@@ -127,6 +128,9 @@ int _odp_ishm_init_local(void);
 int _odp_ishm_term_global(void);
 int _odp_ishm_term_local(void);
 
+int _odpdrv_shm_init_global(void);
+int _odpdrv_shm_term_global(void);
+
 int cpuinfo_parser(FILE *file, system_info_t *sysinfo);
 uint64_t odp_cpu_hz_current(int id);
 
diff --git a/platform/linux-generic/odp_init.c 
b/platform/linux-generic/odp_init.c
index 1b0d8f8..dd396be 100644
--- a/platform/linux-generic/odp_init.c
+++ b/platform/linux-generic/odp_init.c
@@ -120,6 +120,12 @@ int odp_init_global(odp_instance_t *instance,
}
stage = ISHM_INIT;
 
+   if (_odpdrv_shm_init_global()) {
+   ODP_ERR("ODP drvshm init failed.\n");
+   goto init_failed;
+   }
+   stage = DRVSHM_INIT;
+
if (odp_thread_init_global()) {
ODP_ERR("ODP thread init failed.\n");
goto init_failed;
@@ -273,6 +279,13 @@ int _odp_term_global(enum init_stage stage)
}
/* Fall through */
 
+   case DRVSHM_INIT:
+   if (_odpdrv_shm_term_global()) {
+   ODP_ERR("ODP drvshm term failed.\n");
+   rc = -1;
+   }
+   /* Fall through */
+
case ISHM_INIT:
if (_odp_ishm_term_global()) {
ODP_ERR("ODP ishm term failed.\n");
-- 
2.7.4



[lng-odp] [API-NEXT PATCH 1/5] linux-gen: _ishm: adding buddy allocation

2016-12-21 Thread Christophe Milard
_ishm now provides a function to create a pool for buddy memory allocation,
as well as functions to allocated/release memory from the created pool.

Signed-off-by: Christophe Milard 
---
 platform/linux-generic/Makefile.am |   2 +
 platform/linux-generic/_ishmbuddy.c| 491 +
 .../linux-generic/include/_ishmbuddy_internal.h|  44 ++
 3 files changed, 537 insertions(+)
 create mode 100644 platform/linux-generic/_ishmbuddy.c
 create mode 100644 platform/linux-generic/include/_ishmbuddy_internal.h

diff --git a/platform/linux-generic/Makefile.am 
b/platform/linux-generic/Makefile.am
index 999a7f5..8ae8f39 100644
--- a/platform/linux-generic/Makefile.am
+++ b/platform/linux-generic/Makefile.am
@@ -126,6 +126,7 @@ odpdrvplatinclude_HEADERS = \
 noinst_HEADERS = \
  ${srcdir}/include/_fdserver_internal.h \
  ${srcdir}/include/_ishm_internal.h \
+ ${srcdir}/include/_ishmbuddy_internal.h \
  ${srcdir}/include/_ishmphy_internal.h \
  ${srcdir}/include/odp_align_internal.h \
  ${srcdir}/include/odp_atomic_internal.h \
@@ -170,6 +171,7 @@ noinst_HEADERS = \
 __LIB__libodp_linux_la_SOURCES = \
   _fdserver.c \
   _ishm.c \
+  _ishmbuddy.c \
   _ishmphy.c \
   odp_atomic.c \
   odp_barrier.c \
diff --git a/platform/linux-generic/_ishmbuddy.c 
b/platform/linux-generic/_ishmbuddy.c
new file mode 100644
index 000..417c732
--- /dev/null
+++ b/platform/linux-generic/_ishmbuddy.c
@@ -0,0 +1,491 @@
+/* Copyright (c) 2016, Linaro Limited
+ * All rights reserved.
+ *
+ * SPDX-License-Identifier: BSD-3-Clause
+ */
+
+/* This file gathers the buddy allocation functionality provided by _ishm.
+ * _odp_ishmbud_pool_create() can be used to create a pool for buddy 
allocation.
+ * _odp_ishmbud_pool_create() will allocate a memory area using ishm_reserve()
+ * for both the control part (needed for tracking allocation/free...) and the
+ * user memory itself (part of which will be given at each ishmbud_alloc()).
+ * When creating the pool, the pool order, N, (i.e. the log2 of its size)
+ * and its minimum order, M, (i.e. the log2 of the minimum granularity of the
+ * pool) are given.
+ * Any allocation of less than 2^M bytes will result in 2^M bytes being
+ * allocated.
+ * All allocation are rounded to the nearest power of 2.
+ *
+ * The implementation of this buddy allocator is very traditional: it
+ * maintains N lists of free buffers.
+ * The control part actually contains these N queue heads, (N-M are actually
+ * used), the free buffers themselves being used for chaining (the chaining 
info
+ * is in the buffers: as they are "free" they should not be touched by the
+ * user). The control part also contains a array of bytes for remembering
+ * the size (actually the order) of the allocated buffers:
+ * There are 2^(N-M) such bytes, this number being the maximum number of
+ * allocated buffers (when all allocation are <= 2^M bytes)
+ * Buddy allocators handle fragmentation by splitting or merging blocks by 2.
+ * They guarantee a minimum efficiency of 50%, at worse case fragmentation.
+ *
+ * The reason for not using malloc() is that malloc does not guarantee
+ * memory sharability between ODP threads (regardless of their implememtation)
+ * which ishm_reserve() can do. see the comments around
+ * _odp_ishmbud_pool_create() and ishm_reserve() for more details.
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include <_ishm_internal.h>
+#include <_ishmbuddy_internal.h>
+#include 
+#include 
+#include 
+#include 
+#include 
+
+typedef _odpdrv_shm_bpool_t bpool_t; /* for shorter writing  */
+
+/* free buddy blocks contains the following structure, used to link the
+ * free blocks together.
+ */
+typedef struct bblock_t {
+   struct bblock_t *next;
+   uint32_t order;
+} bblock_t;
+
+/* value set in the 'order' table when the block is not allocated:   */
+#define BBLOCK_FREE 0
+
+/* compute ceil(log2(size)) */
+static uint8_t clog2(uint64_t size)
+{
+   uint64_t sz;
+   uint32_t bit;
+   uint8_t res;
+
+   sz = size;  /* we start by computing res = log2(sz)...   */
+   res = 0;
+   for (bit = 32; bit ; bit >>= 1) {
+   if (sz >= ((uint64_t)1 << bit)) {
+   sz >>= bit;
+   res += bit;
+   }
+   }
+   if (((uint64_t)1 << res) < size) /* ...and then ceil(x)  */
+   res++;
+
+   return res;
+}
+
+/*
+ * given a bblock address, and an order value, returns the address
+ * of the buddy bblock (the other "half")
+ */
+static inline bblock_t *get_bblock_buddy(bpool_t *bpool, bblock_t *addr,
+  

[lng-odp] [API-NEXT PATCH 2/5] drv: adding odpdrv_shm_sreserve and odpdrv_shm_sfree()

2016-12-21 Thread Christophe Milard
The two functions are meant to be used fo small amount of memory
allocation.
They are quicker, lighter and have a smaller granularity than the
odpdrv_shm_reserve() / odpdrv_shm_free_*() sets of function, but also
lacks some of their functionality (lookup, export, flags).

Signed-off-by: Christophe Milard 
---
 include/odp/drv/spec/shm.h | 41 -
 1 file changed, 40 insertions(+), 1 deletion(-)

diff --git a/include/odp/drv/spec/shm.h b/include/odp/drv/spec/shm.h
index ef64f5d..d2ccfe2 100644
--- a/include/odp/drv/spec/shm.h
+++ b/include/odp/drv/spec/shm.h
@@ -200,9 +200,11 @@ int odpdrv_shm_info(odpdrv_shm_t shm, odpdrv_shm_info_t 
*info);
 
 /**
  * Print all shared memory blocks and returns the number of allocated blocks.
+ * Also gives the status of the odpdrv_shm_sreserve() pool allocator.
  * This function is meant for debug.
  * @param title  A string to be printed before the shared memory status
- * @return   The total number of allocated blocks
+ * @return   The total number of allocated blocks (allocated with
+ *  odpdrv_shm_reserve()).
  */
 int odpdrv_shm_print_all(const char *title);
 
@@ -220,6 +222,43 @@ int odpdrv_shm_print_all(const char *title);
 uint64_t odpdrv_shm_to_u64(odpdrv_shm_t hdl);
 
 /**
+ * Reserve a small contiguous block of shared memory
+ * odpdrv_shm_sreserve() is meant to reserve small amount of memory from
+ * a preallocated pool. The size of this pool is limited, so
+ * odpdrv_shm_sreserve() should be used for small allocations (e.g. less than
+ * a 1/2 page size).
+ * While odpdrv_shm_reserve() granularity is large (likely the page size),
+ * odpdrv_shm_sreserve() has a much smaller granularity, making it ideal for
+ * small allocations such as those done to populate a linked list.
+ * Addresses returned by odpdrv_shm_sreserve() are sharable between all ODP
+ * threads. Buffer allocated through odpdrv_shm_sreserve() are enonymous and
+ * can therefore not by looked up by name. Neither can they be exported
+ * to other ODP instances.
+ * Buffers allocated with this function should be release with
+ * odpdrv_shm_sfree().
+ *
+ * @param[in] size   Block size in bytes
+ *
+ * @return The address of the reserved block.
+ * @retval NULL on failure
+ */
+void *odpdrv_shm_sreserve(uint64_t size);
+
+/**
+ * Free a contiguous block of shared memory previously allocated with
+ * odpdrv_shm_reserve()
+ *
+ * Frees a previously reserved block of shared memory.
+ * @note Freeing memory that is in use will result in UNDEFINED behavior
+ *
+ * @param[in] addr address (as returned by odpdrv_shm_sreserve()).
+ *
+ * @retval 0 on success
+ * @retval <0 on failure
+ */
+int odpdrv_shm_sfree(void *addr);
+
+/**
  * @}
  */
 
-- 
2.7.4



[lng-odp] [API-NEXT PATCH 0/5] small memory amount allocator for drv shm

2016-12-21 Thread Christophe Milard
This patch series aims at giving the ability for units using the ODP
drv (south) interface to allocate small unit of memory (hence going lower
than the page size, which is the lower limit for odp_drvshm_reserve).
The interface now provides two functions (whose usage is very similar to
malloc/free, but which also guarantee address uniqueness amoung all
ODP threads).
Internally, for the linux-gen implementation, this is implemeneted thanks
to a buddy allocator handling a preallocated pool.
Enumerators typically will need this to contruct the list of enumerated
devices. ODP will also need it for building list of things such as
enumerators, devio, drivers...
(PS:I guess this concept will be needed as well on the north (API) interface
when we start looking into having things working "in process mode")

Christophe Milard (5):
  linux-gen: _ishm: adding buddy allocation
  drv: adding odpdrv_shm_sreserve and odpdrv_shm_sfree()
  linux-gen: drv: shm: adding buddy allocator
  test: drv: shm: adding small allocation tests
  test: drv: shm: adding small allocation stress tests

 include/odp/drv/spec/shm.h |  41 +-
 platform/linux-generic/Makefile.am |   2 +
 platform/linux-generic/_ishmbuddy.c| 491 +
 platform/linux-generic/drv_shm.c   |  44 +-
 .../linux-generic/include/_ishmbuddy_internal.h|  44 ++
 .../linux-generic/include/odp_config_internal.h|  10 +
 platform/linux-generic/include/odp_internal.h  |   4 +
 platform/linux-generic/odp_init.c  |  13 +
 .../common_plat/validation/drv/drvshmem/drvshmem.c | 241 ++
 .../common_plat/validation/drv/drvshmem/drvshmem.h |   2 +
 10 files changed, 890 insertions(+), 2 deletions(-)
 create mode 100644 platform/linux-generic/_ishmbuddy.c
 create mode 100644 platform/linux-generic/include/_ishmbuddy_internal.h

-- 
2.7.4



[lng-odp] [API-NEXT PATCH] linux-gen: _ishm: fixing typos

2016-12-21 Thread Christophe Milard
Fixing a set of iritating typos. just in comments.

Signed-off-by: Christophe Milard 
---
 platform/linux-generic/_ishm.c | 24 
 1 file changed, 12 insertions(+), 12 deletions(-)

diff --git a/platform/linux-generic/_ishm.c b/platform/linux-generic/_ishm.c
index 449e357..939b74a 100644
--- a/platform/linux-generic/_ishm.c
+++ b/platform/linux-generic/_ishm.c
@@ -75,7 +75,7 @@
 /*
  * Maximum number of internal shared memory blocks.
  *
- * This the the number of separate ISHM areas that can be reserved concurrently
+ * This is the number of separate ISHM areas that can be reserved concurrently
  * (Note that freeing such blocks may take time, or possibly never happen
  * if some of the block ownwers never procsync() after free). This number
  * should take that into account)
@@ -236,7 +236,7 @@ static void procsync(void);
  * Take a piece of the preallocated virtual space to fit "size" bytes.
  * (best fit). Size must be rounded up to an integer number of pages size.
  * Possibly split the fragment to keep track of remaining space.
- * Returns the allocated fragment (best_fragmnt) and the corresponding address.
+ * Returns the allocated fragment (best_fragment) and the corresponding 
address.
  * External caller must ensure mutex before the call!
  */
 static void *alloc_fragment(uintptr_t size, int block_index, intptr_t align,
@@ -282,11 +282,11 @@ static void *alloc_fragment(uintptr_t size, int 
block_index, intptr_t align,
 
/*
 * if there is room between previous fragment and new one, (due to
-* alignement requirement) then fragment (split) the space between
+* alignment requirement) then fragment (split) the space between
 * the end of the previous fragment and the beginning of the new one:
 */
if (border - (uintptr_t)(*best_fragmnt)->start > 0) {
-   /* frangment space, i.e. take a new fragment descriptor... */
+   /* fragment space, i.e. take a new fragment descriptor... */
rem_fragmnt = ishm_ftbl->unused_fragmnts;
if (!rem_fragmnt) {
ODP_ERR("unable to get shmem fragment descriptor!\n.");
@@ -316,7 +316,7 @@ static void *alloc_fragment(uintptr_t size, int 
block_index, intptr_t align,
if (remainder == 0)
return (*best_fragmnt)->start;
 
-   /* otherwise, frangment space, i.e. take a new fragment descriptor... */
+   /* otherwise, fragment space, i.e. take a new fragment descriptor... */
rem_fragmnt = ishm_ftbl->unused_fragmnts;
if (!rem_fragmnt) {
ODP_ERR("unable to get shmem fragment descriptor!\n.");
@@ -503,7 +503,7 @@ static void delete_file(ishm_block_t *block)
  * performs the mapping, possibly allocating a fragment of the pre-reserved
  * VA space if the _ODP_ISHM_SINGLE_VA flag was given.
  * Sets fd, and returns the mapping address.
- * This funstion will also set the _ODP_ISHM_SINGLE_VA flag if the alignment
+ * This function will also set the _ODP_ISHM_SINGLE_VA flag if the alignment
  * requires it
  * Mutex must be assured by the caller.
  */
@@ -724,7 +724,7 @@ static void procsync(void)
 
last = ishm_proctable->nb_entries;
while (i < last) {
-   /* if the procecess sequence number doesn't match the main
+   /* if the process sequence number doesn't match the main
 * table seq number, this entry is obsolete
 */
block = _tbl->block[ishm_proctable->entry[i].block_index];
@@ -1033,7 +1033,7 @@ static int block_free(int block_index)
 }
 
 /*
- * Free and unmap internal shared memory, intentified by its block number:
+ * Free and unmap internal shared memory, identified by its block number:
  * return -1 on error. 0 if OK.
  */
 int _odp_ishm_free_by_index(int block_index)
@@ -1049,7 +1049,7 @@ int _odp_ishm_free_by_index(int block_index)
 }
 
 /*
- * free and unmap internal shared memory, intentified by its block name:
+ * free and unmap internal shared memory, identified by its block name:
  * return -1 on error. 0 if OK.
  */
 int _odp_ishm_free_by_name(const char *name)
@@ -1460,8 +1460,8 @@ static int do_odp_ishm_term_local(void)
 * Go through the table of visible blocks for this process,
 * decreasing the refcnt of each visible blocks, and issuing
 * warning for those no longer referenced by any process.
-* Note that non-referenced blocks are nor freeed: this is
-* deliberate as this would imply that the sementic of the
+* Note that non-referenced blocks are nor freed: this is
+* deliberate as this would imply that the semantic of the
 * freeing function would differ depending on whether we run
 * with odp_thread as processes or pthreads. With this approach,
 * the user should always free 

[lng-odp] [API-NEXT PATCH 2/2] test: shm: checking exported vs imported block length

2016-12-21 Thread Christophe Milard
Checking that the block size returned by odp_shm_info() matches
the exported block length.

Signed-off-by: Christophe Milard 
---
 test/linux-generic/validation/api/shmem/shmem_odp2.c | 8 
 1 file changed, 8 insertions(+)

diff --git a/test/linux-generic/validation/api/shmem/shmem_odp2.c 
b/test/linux-generic/validation/api/shmem/shmem_odp2.c
index e39dc76..7d8c682 100644
--- a/test/linux-generic/validation/api/shmem/shmem_odp2.c
+++ b/test/linux-generic/validation/api/shmem/shmem_odp2.c
@@ -28,6 +28,7 @@ int main(int argc, char *argv[])
odp_instance_t odp1;
odp_instance_t odp2;
odp_shm_t shm;
+   odp_shm_info_t  info;
test_shared_data_t *test_shared_data;
 
/* odp init: */
@@ -59,6 +60,13 @@ int main(int argc, char *argv[])
return 1;
}
 
+   /* check that the read size matches the allocated size (in other ODP):*/
+   if ((odp_shm_info(shm, )) ||
+   (info.size != sizeof(*test_shared_data))) {
+   fprintf(stderr, "error: odp_shm_info failed.\n");
+   return 1;
+   }
+
test_shared_data = odp_shm_addr(shm);
if (test_shared_data == NULL) {
fprintf(stderr, "error: odp_shm_addr failed.\n");
-- 
2.7.4



[lng-odp] [API-NEXT PATCH 1/2] linux-gen: _ishm: exporting/importing user len and flags

2016-12-21 Thread Christophe Milard
The size of the shared memory and its user flag set, as given at reserve
time, are exported and imported so that odp_shm_info() return proper
values on imported blocks

Signed-off-by: Christophe Milard 
---
 platform/linux-generic/_ishm.c | 38 +-
 .../validation/api/shmem/shmem_linux.c | 14 ++--
 2 files changed, 42 insertions(+), 10 deletions(-)

diff --git a/platform/linux-generic/_ishm.c b/platform/linux-generic/_ishm.c
index 449e357..8b32709 100644
--- a/platform/linux-generic/_ishm.c
+++ b/platform/linux-generic/_ishm.c
@@ -124,7 +124,9 @@
 #define EXPORT_FILE_LINE3_FMT "file: %s"
 #define EXPORT_FILE_LINE4_FMT "length: %" PRIu64
 #define EXPORT_FILE_LINE5_FMT "flags: %" PRIu32
-#define EXPORT_FILE_LINE6_FMT "align: %" PRIu32
+#define EXPORT_FILE_LINE6_FMT "user_length: %" PRIu64
+#define EXPORT_FILE_LINE7_FMT "user_flags: %" PRIu32
+#define EXPORT_FILE_LINE8_FMT "align: %" PRIu32
 /*
  * A fragment describes a piece of the shared virtual address space,
  * and is allocated only when allocation is done with the _ODP_ISHM_SINGLE_VA
@@ -477,7 +479,11 @@ static int create_file(int block_index, huge_flag_t huge, 
uint64_t len,
new_block->filename);
fprintf(export_file, EXPORT_FILE_LINE4_FMT "\n", len);
fprintf(export_file, EXPORT_FILE_LINE5_FMT "\n", flags);
-   fprintf(export_file, EXPORT_FILE_LINE6_FMT "\n", align);
+   fprintf(export_file, EXPORT_FILE_LINE6_FMT "\n",
+   new_block->user_len);
+   fprintf(export_file, EXPORT_FILE_LINE7_FMT "\n",
+   new_block->user_flags);
+   fprintf(export_file, EXPORT_FILE_LINE8_FMT "\n", align);
 
fclose(export_file);
}
@@ -800,6 +806,10 @@ int _odp_ishm_reserve(const char *name, uint64_t size, int 
fd,
else
new_block->name[0] = 0;
 
+   /* save user data: */
+   new_block->user_flags = user_flags;
+   new_block->user_len = size;
+
/* If a file descriptor is provided, get the real size and map: */
if (fd >= 0) {
fstat(fd, );
@@ -911,9 +921,11 @@ int _odp_ishm_find_exported(const char *remote_name, pid_t 
external_odp_pid,
FILE *export_file;
uint64_t len;
uint32_t flags;
+   uint64_t user_len;
+   uint32_t user_flags;
uint32_t align;
int fd;
-   int ret;
+   int block_index;
 
/* try to read the block description file: */
snprintf(export_filename, ISHM_FILENAME_MAXLEN,
@@ -943,7 +955,13 @@ int _odp_ishm_find_exported(const char *remote_name, pid_t 
external_odp_pid,
if (fscanf(export_file, EXPORT_FILE_LINE5_FMT " ", ) != 1)
goto error_exp_file;
 
-   if (fscanf(export_file, EXPORT_FILE_LINE6_FMT " ", ) != 1)
+   if (fscanf(export_file, EXPORT_FILE_LINE6_FMT " ", _len) != 1)
+   goto error_exp_file;
+
+   if (fscanf(export_file, EXPORT_FILE_LINE7_FMT " ", _flags) != 1)
+   goto error_exp_file;
+
+   if (fscanf(export_file, EXPORT_FILE_LINE8_FMT " ", ) != 1)
goto error_exp_file;
 
fclose(export_file);
@@ -960,13 +978,17 @@ int _odp_ishm_find_exported(const char *remote_name, 
pid_t external_odp_pid,
flags &= ~(uint32_t)_ODP_ISHM_EXPORT;
 
/* reserve the memory, providing the opened file descriptor: */
-   ret = _odp_ishm_reserve(local_name, 0, fd, align, flags, 0);
-   if (ret < 0) {
+   block_index = _odp_ishm_reserve(local_name, 0, fd, align, flags, 0);
+   if (block_index < 0) {
close(fd);
-   return ret;
+   return block_index;
}
 
-   return ret;
+   /* set inherited info: */
+   ishm_tbl->block[block_index].user_flags = user_flags;
+   ishm_tbl->block[block_index].user_len = user_len;
+
+   return block_index;
 
 error_exp_file:
fclose(export_file);
diff --git a/test/linux-generic/validation/api/shmem/shmem_linux.c 
b/test/linux-generic/validation/api/shmem/shmem_linux.c
index 39473f3..2f4c762 100644
--- a/test/linux-generic/validation/api/shmem/shmem_linux.c
+++ b/test/linux-generic/validation/api/shmem/shmem_linux.c
@@ -102,7 +102,8 @@
  */
 static int read_shmem_attribues(uint64_t ext_odp_pid, const char *blockname,
char *filename, uint64_t *len,
-   uint32_t *flags, uint32_t *align)
+   uint32_t *flags, uint64_t *user_len,
+   uint32_t *user_flags, uint32_t *align)
 {
char shm_attr_filename[PATH_MAX];
FILE *export_file;
@@ -130,6 +131,12 @@ static int read_shmem_attribues(uint64_t ext_odp_pid, 
const char *blockname,
if (fscanf(export_file, "flags: %" 

Re: [lng-odp] Suspected SPAM - [API-NEXT PATCH 1/3] api: unify ODP_*_NAME_LEN specifications

2016-12-21 Thread Savolainen, Petri (Nokia - FI/Espoo)
For the entire patch set:

Reviewed-by: Petri Savolainen 


> -Original Message-
> From: lng-odp [mailto:lng-odp-boun...@lists.linaro.org] On Behalf Of
> Matias Elo
> Sent: Tuesday, December 20, 2016 3:45 PM
> To: lng-odp@lists.linaro.org
> Subject: Suspected SPAM - [lng-odp] [API-NEXT PATCH 1/3] api: unify
> ODP_*_NAME_LEN specifications
> 
> Unify name length definitions to always include terminating null
> character.
> 
> Signed-off-by: Matias Elo 
> ---
>  include/odp/api/spec/classification.h | 2 +-
>  include/odp/api/spec/pool.h   | 2 +-
>  include/odp/api/spec/queue.h  | 2 +-
>  include/odp/api/spec/schedule.h   | 2 +-
>  include/odp/api/spec/shared_memory.h  | 2 +-
>  include/odp/api/spec/timer.h  | 2 +-
>  6 files changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/include/odp/api/spec/classification.h
> b/include/odp/api/spec/classification.h
> index 0e442c7..0e1addd 100644
> --- a/include/odp/api/spec/classification.h
> +++ b/include/odp/api/spec/classification.h
> @@ -44,7 +44,7 @@ extern "C" {
> 
>  /**
>   * @def ODP_COS_NAME_LEN
> - * Maximum ClassOfService name length in chars
> + * Maximum ClassOfService name length in chars including null char
>   */
> 
>  /**
> diff --git a/include/odp/api/spec/pool.h b/include/odp/api/spec/pool.h
> index 041f4af..af2829b 100644
> --- a/include/odp/api/spec/pool.h
> +++ b/include/odp/api/spec/pool.h
> @@ -36,7 +36,7 @@ extern "C" {
>   * Invalid pool
>   */
> 
> -/** Maximum queue name length in chars */
> +/** Maximum pool name length in chars including null char */
>  #define ODP_POOL_NAME_LEN  32
> 
>  /**
> diff --git a/include/odp/api/spec/queue.h b/include/odp/api/spec/queue.h
> index b0c5e31..7972fea 100644
> --- a/include/odp/api/spec/queue.h
> +++ b/include/odp/api/spec/queue.h
> @@ -44,7 +44,7 @@ extern "C" {
> 
>  /**
>   * @def ODP_QUEUE_NAME_LEN
> - * Maximum queue name length in chars
> + * Maximum queue name length in chars including null char
>   */
> 
>  /**
> diff --git a/include/odp/api/spec/schedule.h
> b/include/odp/api/spec/schedule.h
> index f976a4c..8244746 100644
> --- a/include/odp/api/spec/schedule.h
> +++ b/include/odp/api/spec/schedule.h
> @@ -42,7 +42,7 @@ extern "C" {
> 
>  /**
>   * @def ODP_SCHED_GROUP_NAME_LEN
> - * Maximum schedule group name length in chars
> + * Maximum schedule group name length in chars including null char
>   */
> 
>  /**
> diff --git a/include/odp/api/spec/shared_memory.h
> b/include/odp/api/spec/shared_memory.h
> index 885751d..074c883 100644
> --- a/include/odp/api/spec/shared_memory.h
> +++ b/include/odp/api/spec/shared_memory.h
> @@ -40,7 +40,7 @@ extern "C" {
>   * Synonym for buffer pool use
>   */
> 
> -/** Maximum shared memory block name length in chars */
> +/** Maximum shared memory block name length in chars including null char
> */
>  #define ODP_SHM_NAME_LEN 32
> 
>  /*
> diff --git a/include/odp/api/spec/timer.h b/include/odp/api/spec/timer.h
> index 49221c4..46a4369 100644
> --- a/include/odp/api/spec/timer.h
> +++ b/include/odp/api/spec/timer.h
> @@ -90,7 +90,7 @@ typedef enum {
>   ODP_TIMER_NOEVENT = -3
>  } odp_timer_set_t;
> 
> -/** Maximum timer pool name length in chars (including null char) */
> +/** Maximum timer pool name length in chars including null char */
>  #define ODP_TIMER_POOL_NAME_LEN  32
> 
>  /** Timer pool parameters
> --
> 2.7.4