[lng-odp] Maintaner change

[Nicolas Morey-Chaisemartin]

Hi everyone,

Just to let you know that this is my last week at Kalray.
Jérome Reybert (I think he's subscribing to this ML already) will be in charge 
of the ODP MPPA port.

If you have any questions on Kalray's port or need some feedback, please 
contact him from now on.

Regards

Nicolas

Re: [lng-odp] [PATCH 1/5] test: generator: compose sending packets from reference packet plus differences

[Nicolas Morey-Chaisemartin]

Le 02/13/2017 à 01:49 PM, Bogdan Pricope a écrit :
> Signed-off-by: Bogdan Pricope 
> ---
>  example/generator/odp_generator.c | 131 
> +-
>  1 file changed, 114 insertions(+), 17 deletions(-)
>
> diff --git a/example/generator/odp_generator.c 
> b/example/generator/odp_generator.c
> index 8062d87..d1e3ecc 100644
> --- a/example/generator/odp_generator.c
> +++ b/example/generator/odp_generator.c
> @@ -170,21 +170,20 @@ static int scan_ip(char *buf, unsigned int *paddr)
>  }
>  
>  /**
> - * set up an udp packet
> + * set up an udp packet reference
>   *
>   * @param pool Buffer pool to create packet in
>   *
>   * @return Handle of created packet
>   * @retval ODP_PACKET_INVALID  Packet could not be created
>   */
> -static odp_packet_t pack_udp_pkt(odp_pool_t pool)
> +static odp_packet_t setup_udp_pkt_ref(odp_pool_t pool)
>  {
>   odp_packet_t pkt;
>   char *buf;
>   odph_ethhdr_t *eth;
>   odph_ipv4hdr_t *ip;
>   odph_udphdr_t *udp;
> - unsigned short seq;
>  
>   pkt = odp_packet_alloc(pool, args->appl.payload + ODPH_UDPHDR_LEN +
>  ODPH_IPV4HDR_LEN + ODPH_ETHHDR_LEN);
> @@ -200,8 +199,10 @@ static odp_packet_t pack_udp_pkt(odp_pool_t pool)
>   memcpy((char *)eth->src.addr, args->appl.srcmac.addr, ODPH_ETHADDR_LEN);
>   memcpy((char *)eth->dst.addr, args->appl.dstmac.addr, ODPH_ETHADDR_LEN);
>   eth->type = odp_cpu_to_be_16(ODPH_ETHTYPE_IPV4);
> +
>   /* ip */
>   odp_packet_l3_offset_set(pkt, ODPH_ETHHDR_LEN);
> + odp_packet_has_ipv4_set(pkt, 1);
>   ip = (odph_ipv4hdr_t *)(buf + ODPH_ETHHDR_LEN);
>   ip->dst_addr = odp_cpu_to_be_32(args->appl.dstip);
>   ip->src_addr = odp_cpu_to_be_32(args->appl.srcip);
> @@ -209,12 +210,13 @@ static odp_packet_t pack_udp_pkt(odp_pool_t pool)
>   ip->tot_len = odp_cpu_to_be_16(args->appl.payload + ODPH_UDPHDR_LEN +
>  ODPH_IPV4HDR_LEN);
>   ip->proto = ODPH_IPPROTO_UDP;
> - seq = odp_atomic_fetch_add_u64(, 1) % 0x;
> - ip->id = odp_cpu_to_be_16(seq);
> + ip->id = 0;
> + ip->ttl = 64;
>   ip->chksum = 0;
> - odph_ipv4_csum_update(pkt);
> +
>   /* udp */
>   odp_packet_l4_offset_set(pkt, ODPH_ETHHDR_LEN + ODPH_IPV4HDR_LEN);
> + odp_packet_has_udp_set(pkt, 1);
>   udp = (odph_udphdr_t *)(buf + ODPH_ETHHDR_LEN + ODPH_IPV4HDR_LEN);
>   udp->src_port = 0;
>   udp->dst_port = 0;
> @@ -226,27 +228,60 @@ static odp_packet_t pack_udp_pkt(odp_pool_t pool)
>  }
>  
The calls to odp_packet_has_ipv4_set and odp_packet_has_udp_set are actually a 
bug fix needed to have the proper checksum computations.
Without these calls, the checksum are set to 0 as all the packet parse flags 
are 0.

It might be worth splitting this into a specific patch.
I posted one for monarch without any answer to it, but it's something that 
should be put in all branches as it's a bug fix.

Nicolas

[lng-odp] [MONARCH PATCH] example: generator: fix udp checksum computation

[Nicolas Morey-Chaisemartin]

odph_ipv4_udp_chksum checks for IPv4 and UDP in packet flags which were
not set as the packet parse is triggered when setting the l3 offset

Signed-off-by: Nicolas Morey-Chaisemartin <nmo...@kalray.eu>
---
 example/generator/odp_generator.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/example/generator/odp_generator.c 
b/example/generator/odp_generator.c
index b0053b9..5cc1bae 100644
--- a/example/generator/odp_generator.c
+++ b/example/generator/odp_generator.c
@@ -206,6 +206,7 @@ static odp_packet_t pack_udp_pkt(odp_pool_t pool)
eth->type = odp_cpu_to_be_16(ODPH_ETHTYPE_IPV4);
/* ip */
odp_packet_l3_offset_set(pkt, ODPH_ETHHDR_LEN);
+   odp_packet_has_ipv4_set(pkt, 1);
ip = (odph_ipv4hdr_t *)(buf + ODPH_ETHHDR_LEN);
ip->dst_addr = odp_cpu_to_be_32(args->appl.dstip);
ip->src_addr = odp_cpu_to_be_32(args->appl.srcip);
@@ -219,6 +220,7 @@ static odp_packet_t pack_udp_pkt(odp_pool_t pool)
odph_ipv4_csum_update(pkt);
/* udp */
odp_packet_l4_offset_set(pkt, ODPH_ETHHDR_LEN + ODPH_IPV4HDR_LEN);
+   odp_packet_has_udp_set(pkt, 1);
udp = (odph_udphdr_t *)(buf + ODPH_ETHHDR_LEN + ODPH_IPV4HDR_LEN);
udp->src_port = 0;
udp->dst_port = 0;
-- 
2.10.1.4.g0ffc436

Re: [lng-odp] pktio: enable_loop

[Nicolas Morey-Chaisemartin]

Le 01/24/2017 à 06:11 PM, Bala Manoharan a écrit :
> On 24 January 2017 at 22:25, Nicolas Morey-Chaisemartin
> <nmo...@kalray.eu> wrote:
>> Just a quick question about the API.
>>
>> Bala introduced a patch to check/enable loopback mode on interface.
>> But the patch adds an "enable_loop" in odp_pktio_config_t and a 
>> "loop_supported" in odp_pktio_capability_t.
>>
>> As there is a odp_pktio_config_t in odp_pktio_capability_t which the API 
>> describes as:
>> /** Supported pktio configuration options */
>>
>> Why is there a need for the loop_supported field?
> Yes. The "loop_supported" boolean could be removed from
> odp_pktio_capability_t struct since there is a redundancy.
> I will send a patch to remove the same.
>
> Regards,
> Bala
>>
>> Nicolas
>>

OK.
As this is in Monarch, I guess the expected behaviour there is to have both 
loop_supported and enable_loop set to the same value when checking capabilities?

Nicolas

[lng-odp] pktio: enable_loop

[Nicolas Morey-Chaisemartin]

Just a quick question about the API.

Bala introduced a patch to check/enable loopback mode on interface.
But the patch adds an "enable_loop" in odp_pktio_config_t and a 
"loop_supported" in odp_pktio_capability_t.

As there is a odp_pktio_config_t in odp_pktio_capability_t which the API 
describes as:
/** Supported pktio configuration options */

Why is there a need for the loop_supported field?


Nicolas

Re: [lng-odp] odp struct sizes (x86_64)

[Nicolas Morey-Chaisemartin]

It's called "pahole".
Should come with dwarves packages on most standard disto.

Nicolas

Le 01/13/2017 à 04:57 PM, Mike Holmes a écrit :
> how about a link to the tool ?
>
> On 13 January 2017 at 10:28, Bill Fischofer 
> wrote:
>
>> On Fri, Jan 13, 2017 at 9:09 AM, Maxim Uvarov 
>> wrote:
>>> Hello,
>>>
>>> Bill found interesting tool to print structures layout for some elf
>>> binary. Please find output in text file attached to that email. Hope
>>> it's useful for everybody who is working on performance things.
>>>
>>> Best regards,
>>> Maxim.
>> This is great info but there's a lot of it. Any chance the tool
>> supports HTML output? That would be easier to navigate.
>>
>
>

Re: [lng-odp] [PATCHv2] linux-generic: packet: fix buggy compiler error

[Nicolas Morey-Chaisemartin]

Le 12/22/2016 à 02:49 PM, Maxim Uvarov a écrit :
> 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 fails on .len line. 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 
> ---
>  v2: - add comment (Mike)
>  - in description .len line reorder does not fix compilation (Bill)
>
>  platform/linux-generic/odp_packet.c | 4 
>  1 file changed, 4 insertions(+)
>
> diff --git a/platform/linux-generic/odp_packet.c 
> b/platform/linux-generic/odp_packet.c
> index 0d3fd05..599d839 100644
> --- a/platform/linux-generic/odp_packet.c
> +++ b/platform/linux-generic/odp_packet.c
> @@ -309,6 +309,10 @@ static inline void copy_num_segs(odp_packet_hdr_t *to, 
> odp_packet_hdr_t *from,
>   int i;
>  
>   for (i = 0; i < num; i++) {
> + /* check bellow also fixes gcc bug, refer to corresponding
> +  * git commit */
> + if (odp_unlikely((num + i) >= CONFIG_PACKET_MAX_SEGS))
> + ODP_ABORT("packet segmenation error\n");
There's a typo in segmentation

>   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] [PATCH v2] example: traffic_mgmt: use PRIu32 instead of %u

[Nicolas Morey-Chaisemartin]

Thanks. Could you apply it to the monarch branch too?

Nicolas

Le 12/15/2016 à 08:01 PM, Maxim Uvarov a écrit :
> Merged,
> Maxim.
>
> On 12/13/16 22:00, Bill Fischofer wrote:
>> On Tue, Dec 13, 2016 at 12:34 PM, Maxim Uvarov <maxim.uva...@linaro.org> 
>> wrote:
>>> On 12/13/16 19:55, Nicolas Morey-Chaisemartin wrote:
>>>> Signed-off-by: Nicolas Morey-Chaisemartin <nmo...@kalray.eu>
>>>> ---
>>>> v2: add missing include.
>>>> Should be applied to monarch_lts too
>>>>
>>>>  example/traffic_mgmt/odp_traffic_mgmt.c | 20 
>>>>  1 file changed, 12 insertions(+), 8 deletions(-)
>>>>
>>>> diff --git a/example/traffic_mgmt/odp_traffic_mgmt.c 
>>>> b/example/traffic_mgmt/odp_traffic_mgmt.c
>>>> index c4f5356..c83b615 100644
>>>> --- a/example/traffic_mgmt/odp_traffic_mgmt.c
>>>> +++ b/example/traffic_mgmt/odp_traffic_mgmt.c
>>>> @@ -10,6 +10,7 @@
>>>>
>>>>  #include 
>>>>  #include 
>>>> +#include 
>>>>  #include 
>>>>  #include 
>>>>  #include 
>>>> @@ -261,7 +262,8 @@ static uint32_t 
>>>> create_profile_set(profile_params_set_t *profile_params_set,
>>>>   if (name_idx == 0)
>>>>   snprintf(name, sizeof(name), "%s", base_name);
>>>>   else
>>>> - snprintf(name, sizeof(name), "%s-%u", base_name, name_idx);
>>>> + snprintf(name, sizeof(name), "%s-%" PRIu32,
>>>> +  base_name, name_idx);
>>>
>>> in printf we use printf("%s %" PRIu32 "\n", val);
>>>
>>> here a little bit different. But both clang and gcc work the same as it
>>> was written like:
>>> snprintf(, "%s-%" PRIu32 "");
>>>
>> Both are valid C, but the trailing "" is redundant as the string
>> includes the implicit concatenation of PRIu32.  The same would apply
>> to printf() however in this case the suffix "\n" is non-null to append
>> a newline char.
>>
>>> I hope there will be not issues.
>>>
>>>
>>> Maxim.
>>>
>>>
>>>>   odp_tm_shaper_params_init(_params);
>>>>   shaper  = _params_set->shaper_params;
>>>> @@ -289,7 +291,8 @@ static uint32_t 
>>>> create_profile_set(profile_params_set_t *profile_params_set,
>>>>   err_cnt++;
>>>>
>>>>   for (color = 0; color < ODP_NUM_PACKET_COLORS; color++) {
>>>> - snprintf(wred_name, sizeof(wred_name), "%s-%u", name, color);
>>>> + snprintf(wred_name, sizeof(wred_name), "%s-%" PRIu32,
>>>> +  name, color);
>>>>
>>>>   odp_tm_wred_params_init(_params);
>>>>   wred = _params_set->wred_params[color];
>>>> @@ -400,7 +403,7 @@ static int config_example_user(odp_tm_node_t 
>>>> cos_tm_node,
>>>>   profile_set->wred_profiles[2];
>>>>   tm_node_params.level= 2;
>>>>
>>>> - snprintf(user_name, sizeof(user_name), "Subscriber-%u", user_num);
>>>> + snprintf(user_name, sizeof(user_name), "Subscriber-%" PRIu32, 
>>>> user_num);
>>>>   user_tm_node = odp_tm_node_create(odp_tm_test, user_name,
>>>> _node_params);
>>>>   odp_tm_node_connect(user_tm_node, cos_tm_node);
>>>> @@ -478,8 +481,8 @@ static int config_company_node(const char 
>>>> *company_name)
>>>>   tm_node_params.wred_profile[ODP_PACKET_RED]=
>>>>   profile_set->wred_profiles[ODP_PACKET_RED];
>>>>
>>>> - snprintf(cos_node_name, sizeof(cos_node_name), "%s-Class-%u",
>>>> -  company_name, cos_idx);
>>>> + snprintf(cos_node_name, sizeof(cos_node_name),
>>>> +  "%s-Class-%" PRIu32, company_name, cos_idx);
>>>>   cos_tm_node = odp_tm_node_create(odp_tm_test, cos_node_name,
>>>>_node_params);
>>>>   odp_tm_node_connect(cos_tm_node, company_tm_node);
>>>> @@ -528,7 +531,7 @@ static int cre

Re: [lng-odp] [PATCH 0/6] Remove Linux specifics from odp/helpers

[Nicolas Morey-Chaisemartin]

Le 12/14/2016 à 11:14 AM, Savolainen, Petri (Nokia - FI/Espoo) a écrit :
>
> From: Mike Holmes [mailto:mike.hol...@linaro.org] 
> Sent: Tuesday, December 13, 2016 2:48 PM
> To: Savolainen, Petri (Nokia - FI/Espoo) 
> 
> Cc: lng-odp@lists.linaro.org
> Subject: Re: [lng-odp] [PATCH 0/6] Remove Linux specifics from odp/helpers
>
>
>
> On 13 December 2016 at 05:23, Savolainen, Petri (Nokia - FI/Espoo) 
>  wrote:
>
>
>> -Original Message-
>> From: lng-odp [mailto:lng-odp-boun...@lists.linaro.org] On Behalf Of Mike
>> Holmes
>> Sent: Monday, December 12, 2016 4:52 PM
>> To: lng-odp@lists.linaro.org
>> Subject: [lng-odp] [PATCH 0/6] Remove Linux specifics from odp/helpers
>>
>> To allow other implementations of the helpers to exist such as those used
>> on the
>> Kalray system, we need to remove the hardwired Linux naming. This exists
>> in the
>> api, the implementation of the helper lib and the resulting library.
> Opaque thread helpers may very well exist with Linux (or Windows or Unix) 
> specific helpers. Majority of the applications are developed against a 
> defined operating system, not against an operating system abstraction layer. 
> For example, a linux specific helper library (consuming and producing Linux 
> types) would be valuable to many applications, may be even more valuable than 
> the opaque thread helpers used by our validation suite.
>
> I agree, but the core reason for the helper lib is to support the tests, 
> performance tests and examples to keep all the implementations together and 
> tested uniformly in the reference implimentation.
>
> Anyone can use any OS specific code they want locally, we say helpers are not 
> mandatory for that reason.
>
> I am fine if we have a new alternate repo or other obvious delineation for os 
> specific help. However putting them in the main helper API makes portability 
> impossible and we want to promote that first and allow for all performance 
> optimizations to be done locally if needed. We do know that we have at least 
> two OS'es actively using ODP.
>
>  HTML stops here 
>
> The point is that with Linux and ODP an application is directly portable to 
> 90-95% of all networking SoCs. Also if an SoCs does not support Linux it's 
> typically because of lacking HW features such as MMU, which again may 
> restrict me to use it in the first place. So, typically 100% on the SoCs I'm 
> interested in do support Linux and thus I do not need an opaque OS helper but 
> a Linux helper.
>
>
> -Petri
>

I don't mind the helper library to have OS specific code in it. As long as:
- Source and hear file are separated so the helper lib can be compiled on 
another OS by removing the linux specific file from the Makefile.am
- The associated tests can be disabled
- The ODP test suite (at least the common_plat) part does not use any of them.

Nicolas

[lng-odp] [PATCH v2] example: traffic_mgmt: use PRIu32 instead of %u

[Nicolas Morey-Chaisemartin]

Signed-off-by: Nicolas Morey-Chaisemartin <nmo...@kalray.eu>
---
v2: add missing include.
Should be applied to monarch_lts too

 example/traffic_mgmt/odp_traffic_mgmt.c | 20 
 1 file changed, 12 insertions(+), 8 deletions(-)

diff --git a/example/traffic_mgmt/odp_traffic_mgmt.c 
b/example/traffic_mgmt/odp_traffic_mgmt.c
index c4f5356..c83b615 100644
--- a/example/traffic_mgmt/odp_traffic_mgmt.c
+++ b/example/traffic_mgmt/odp_traffic_mgmt.c
@@ -10,6 +10,7 @@
 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -261,7 +262,8 @@ static uint32_t create_profile_set(profile_params_set_t 
*profile_params_set,
if (name_idx == 0)
snprintf(name, sizeof(name), "%s", base_name);
else
-   snprintf(name, sizeof(name), "%s-%u", base_name, name_idx);
+   snprintf(name, sizeof(name), "%s-%" PRIu32,
+base_name, name_idx);
 
odp_tm_shaper_params_init(_params);
shaper  = _params_set->shaper_params;
@@ -289,7 +291,8 @@ static uint32_t create_profile_set(profile_params_set_t 
*profile_params_set,
err_cnt++;
 
for (color = 0; color < ODP_NUM_PACKET_COLORS; color++) {
-   snprintf(wred_name, sizeof(wred_name), "%s-%u", name, color);
+   snprintf(wred_name, sizeof(wred_name), "%s-%" PRIu32,
+name, color);
 
odp_tm_wred_params_init(_params);
wred = _params_set->wred_params[color];
@@ -400,7 +403,7 @@ static int config_example_user(odp_tm_node_t cos_tm_node,
profile_set->wred_profiles[2];
tm_node_params.level= 2;
 
-   snprintf(user_name, sizeof(user_name), "Subscriber-%u", user_num);
+   snprintf(user_name, sizeof(user_name), "Subscriber-%" PRIu32, user_num);
user_tm_node = odp_tm_node_create(odp_tm_test, user_name,
  _node_params);
odp_tm_node_connect(user_tm_node, cos_tm_node);
@@ -478,8 +481,8 @@ static int config_company_node(const char *company_name)
tm_node_params.wred_profile[ODP_PACKET_RED]=
profile_set->wred_profiles[ODP_PACKET_RED];
 
-   snprintf(cos_node_name, sizeof(cos_node_name), "%s-Class-%u",
-company_name, cos_idx);
+   snprintf(cos_node_name, sizeof(cos_node_name),
+"%s-Class-%" PRIu32, company_name, cos_idx);
cos_tm_node = odp_tm_node_create(odp_tm_test, cos_node_name,
 _node_params);
odp_tm_node_connect(cos_tm_node, company_tm_node);
@@ -528,7 +531,7 @@ static int create_and_config_tm(void)
odp_tm_test = odp_tm_create("TM test", , );
err_cnt = init_profile_sets();
if (err_cnt != 0)
-   printf("%s init_profile_sets encountered %u errors\n",
+   printf("%s init_profile_sets encountered %" PRIu32 " errors\n",
   __func__, err_cnt);
 
config_company_node("TestCompany");
@@ -644,7 +647,7 @@ static int traffic_generator(uint32_t pkts_to_send)
odp_atomic_inc_u32(_pkts_into_tm);
}
 
-   printf("%s odp_tm_enq_errs=%u\n", __func__, odp_tm_enq_errs);
+   printf("%s odp_tm_enq_errs=%" PRIu32 "\n", __func__, odp_tm_enq_errs);
 
/* Wait until the main traffic mgmt worker thread is idle and has no
* outstanding events (i.e. no timers, empty work queue, etc), but
@@ -786,7 +789,8 @@ int main(int argc, char *argv[])
 
pkts_into_tm = odp_atomic_load_u32(_pkts_into_tm);
pkts_from_tm = odp_atomic_load_u32(_pkts_from_tm);
-   printf("pkts_into_tm=%u pkts_from_tm=%u\n", pkts_into_tm, pkts_from_tm);
+   printf("pkts_into_tm=%" PRIu32 " pkts_from_tm=%" PRIu32 "\n",
+  pkts_into_tm, pkts_from_tm);
 
odp_tm_stats_print(odp_tm_test);
return 0;

Re: [lng-odp] [PATCH] example: traffic_mgmt: use PRIu32 instead of %u

[Nicolas Morey-Chaisemartin]

Weirdly it worked for me on master, but not in monarch.
I might have missed a clean between rebuilds.

I'll repost a new one


Nicolas


Le 12/13/2016 à 05:44 PM, Bill Fischofer a écrit :
> Sorry about that.  Yes, the file is missing:
>
> #include 
>
> in the #include list, then it compiles and runs fine.  Nicolas, can
> you submit a v2?
>
> On Tue, Dec 13, 2016 at 8:51 AM, Maxim Uvarov <maxim.uva...@linaro.org> wrote:
>> On 12/13/16 06:41, Bill Fischofer wrote:
>>> On Tue, Dec 6, 2016 at 9:25 AM, Nicolas Morey-Chaisemartin
>>> <nmo...@kalray.eu> wrote:
>>>> Signed-off-by: Nicolas Morey-Chaisemartin <nmo...@kalray.eu>
>>> Reviewed-by: Bill Fischofer <bill.fischo...@linaro.org>
>>>
>>>> ---
>>>>
>>>> This should be backported to monarch_lts too. However it requires an extra 
>>>> #include 
>>>>
>>>>  example/traffic_mgmt/odp_traffic_mgmt.c | 19 +++
>>>>  1 file changed, 11 insertions(+), 8 deletions(-)
>>>>
>>>> diff --git a/example/traffic_mgmt/odp_traffic_mgmt.c 
>>>> b/example/traffic_mgmt/odp_traffic_mgmt.c
>>>> index c4f5356..94e208b 100644
>>>> --- a/example/traffic_mgmt/odp_traffic_mgmt.c
>>>> +++ b/example/traffic_mgmt/odp_traffic_mgmt.c
>>>> @@ -261,7 +261,8 @@ static uint32_t 
>>>> create_profile_set(profile_params_set_t *profile_params_set,
>>>> if (name_idx == 0)
>>>> snprintf(name, sizeof(name), "%s", base_name);
>>>> else
>>>> -   snprintf(name, sizeof(name), "%s-%u", base_name, name_idx);
>>>> +   snprintf(name, sizeof(name), "%s-%" PRIu32,
>>>> +base_name, name_idx);
>>>>
>>>> odp_tm_shaper_params_init(_params);
>>>> shaper  = 
>>>> _params_set->shaper_params;
>>>> @@ -289,7 +290,8 @@ static uint32_t 
>>>> create_profile_set(profile_params_set_t *profile_params_set,
>>>> err_cnt++;
>>>>
>>>> for (color = 0; color < ODP_NUM_PACKET_COLORS; color++) {
>>>> -   snprintf(wred_name, sizeof(wred_name), "%s-%u", name, 
>>>> color);
>>>> +   snprintf(wred_name, sizeof(wred_name), "%s-%" PRIu32,
>>>> +name, color);
>>
>> Bad review :)
>>
>> If possible please run odp-check tests on patches before sending. Or at
>> least make check.
>>
>> Maxim.
>>
>>
>>>> odp_tm_wred_params_init(_params);
>>>> wred = _params_set->wred_params[color];
>>>> @@ -400,7 +402,7 @@ static int config_example_user(odp_tm_node_t 
>>>> cos_tm_node,
>>>> profile_set->wred_profiles[2];
>>>> tm_node_params.level= 2;
>>>>
>>>> -   snprintf(user_name, sizeof(user_name), "Subscriber-%u", user_num);
>>>> +   snprintf(user_name, sizeof(user_name), "Subscriber-%" PRIu32, 
>>>> user_num);
>>>> user_tm_node = odp_tm_node_create(odp_tm_test, user_name,
>>>>   _node_params);
>>>> odp_tm_node_connect(user_tm_node, cos_tm_node);
>>>> @@ -478,8 +480,8 @@ static int config_company_node(const char 
>>>> *company_name)
>>>> tm_node_params.wred_profile[ODP_PACKET_RED]=
>>>> profile_set->wred_profiles[ODP_PACKET_RED];
>>>>
>>>> -   snprintf(cos_node_name, sizeof(cos_node_name), 
>>>> "%s-Class-%u",
>>>> -company_name, cos_idx);
>>>> +   snprintf(cos_node_name, sizeof(cos_node_name),
>>>> +"%s-Class-%" PRIu32, company_name, cos_idx);
>>>> cos_tm_node = odp_tm_node_create(odp_tm_test, 
>>>> cos_node_name,
>>>>  _node_params);
>>>> odp_tm_node_connect(cos_tm_node, company_tm_node);
>>>> @@ -528,7 +530,7 @@ static int create_and_config_tm(void)
>>>> odp_tm_test = odp_tm_create("TM test", , );
>>>> err_cnt = init_profile_sets();
>>>> if (err_cnt != 0)
>>&g

Re: [lng-odp] [PATCH 6/6] helper: take default os from odp platform

[Nicolas Morey-Chaisemartin]

Le 12/13/2016 à 02:25 PM, Mike Holmes a écrit :
>
>
> On 13 December 2016 at 03:05, Nicolas Morey-Chaisemartin <nmo...@kalray.eu 
> <mailto:nmo...@kalray.eu>> wrote:
>
>
>
> Le 12/12/2016 à 03:52 PM, Mike Holmes a écrit :
> > Signed-off-by: Mike Holmes <mike.hol...@linaro.org 
> <mailto:mike.hol...@linaro.org>>
> > ---
> >  configure.ac <http://configure.ac>   | 4 ++--
> >  platform/linux-generic/m4/configure.m4 | 2 ++
> >  2 files changed, 4 insertions(+), 2 deletions(-)
> >
> > diff --git a/configure.ac <http://configure.ac> b/configure.ac 
> <http://configure.ac>
> > index f17bcae..eb5436c 100644
> > --- a/configure.ac <http://configure.ac>
> > +++ b/configure.ac <http://configure.ac>
> > @@ -158,9 +158,9 @@ ODP_CFLAGS="$ODP_CFLAGS 
> -DIMPLEMENTATION_NAME=$IMPLEMENTATION_NAME"
> >  
> ##
> >  AC_ARG_WITH([os],
> >  [AS_HELP_STRING([--with-os=os],
> > - [select platform to be used, default linux])],
> > + [select platform to be used, default ${default_helper_os}])],
> >  [],
> > -[with_os=linux
> > +[with_os=${default_helper_os}
> >  ])
> >
> >  AC_SUBST([with_os])
> > diff --git a/platform/linux-generic/m4/configure.m4 
> b/platform/linux-generic/m4/configure.m4
> > index d3e5528..bf6bbe2 100644
> > --- a/platform/linux-generic/m4/configure.m4
> > +++ b/platform/linux-generic/m4/configure.m4
> > @@ -38,3 +38,5 @@ 
> m4_include([platform/linux-generic/m4/odp_schedule.m4])
> >
> >  AC_CONFIG_FILES([platform/linux-generic/Makefile
> >   
> platform/linux-generic/include/odp/api/plat/static_inline.h])
> > +#define the odp helper implimentation to be built
> > +default_helper_os=linux
>
> I don't think this would work well with multiple platforms.
> Either all the platforms m4 are included and they would just override 
> this value for the others.
> Either only one is included, depending on the --with-platform flag, and 
> I'm not sure how this would work. Until platform is selected, the 
> default_helper_os will not be set.
>
> I think the AC_ARG_WITH --with-os should have an empty default value. 
> Platform configure.m4 should only set the value if it is empty.
>
>
> I was following the idea  that with the platform set this appears it would 
> auto set a default helper OS for that platfrom, and would I assume allow a 
> different platform to select a different default helper os.
> Thus it could make it simpler in majority use cases -  but I was hoping for 
> your feedback, I would say that alternatively the default is Linux as that 
> will be most common and thus a sensible default.

I guess it work if you select a platform. If you don't (./configure --help), 
it'll probably just show up an empty string if the default platform is not set.
And it's not really standard or user friendly to have a default option changing 
its value when changing another option. (platform = linux => os = linux, 
platform = mppa => os = mOS)

Maybe a simple check after including the configure.m4 files to make sure the 
value is not empty is needed.

Re: [lng-odp] [PATCH 4/6] helper: select os in configure

[Nicolas Morey-Chaisemartin]

Le 12/13/2016 à 09:54 AM, Christophe Milard a écrit :
> OK. this is somehow matching my understanding:
> . OS= linux, BSD, windows, OSE or mppa
> . Platform = HW board on which that stuff is run (implies different running 
> parametrs, such as pktio interface names, mem sizes...)
>
> Not sure how to reflect that propåerly in the current structure, where 
> platform really means both things (sometimes OS, sometimes HW), but that is 
> the next step :-), if we at least can agree on the vocabulary...
>
> I'd  like to see a way forward with a nice clean structure reflecting these 2 
> concepts, but hopefully this patchset is a step towards it. (at least, it 
> will show the problems that follows)
>
> Chrstophe
>
mppa is not an OS but I agree ;)

Nicolas
>
>
> On 13 December 2016 at 09:01, Nicolas Morey-Chaisemartin <nmo...@kalray.eu 
> <mailto:nmo...@kalray.eu>> wrote:
>
>
>
> Le 12/13/2016 à 08:46 AM, Christophe Milard a écrit :
> > On 12 December 2016 at 15:52, Mike Holmes <mike.hol...@linaro.org 
> <mailto:mike.hol...@linaro.org>> wrote:
> >
> >> Signed-off-by: Mike Holmes <mike.hol...@linaro.org 
> <mailto:mike.hol...@linaro.org>>
> >> ---
> >>  configure.ac <http://configure.ac>   | 14 ++
> >>  helper/Makefile.am |  2 +-
> >>  2 files changed, 15 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/configure.ac <http://configure.ac> b/configure.ac 
> <http://configure.ac>
> >> index 20ec479..39c040d 100644
> >> --- a/configure.ac <http://configure.ac>
> >> +++ b/configure.ac <http://configure.ac>
> >> @@ -154,6 +154,19 @@ fi
> >>  ODP_CFLAGS="$ODP_CFLAGS -DIMPLEMENTATION_NAME=$IMPLEMENTATION_NAME"
> >>
> >>  
> >> ##
> >> +# Determine which os to build helpers for
> >> +###
> >> ###
> >> +AC_ARG_WITH([os],
> >> +[AS_HELP_STRING([--with-os=os],
> >> +   [select platform to be used, default linux])],
> >>
> > Shouldn't it be " select OS to be used, default linux" ???
> > Here again, we hit the platform vs OS question, but if the platform and 
> OS
> > are different things, then it should be OS here! If there are the same
> > thing, then why not using the -with-platform which we already have...
> >
> > I guess we know what an OS is... but what is a platform?
> > -The HW in which ODP is running (implies memory size limitation, 
> different
> > pktio interfaces...). This is my understanding, but 
> odp/pltaform/linux-gen
> > is missleading and should be called odp/os/linux-gen
> > - A couple HW/OS, as this patch seems to imply?
> >
> > Christophe
> >
> > +[],
> We have the case here at Kalray where we have a single platform (mppa) 
> but 2 supported OS depending on what the user wants to do (non ODP compliant 
> code).
> The OS has no impact on the platform, but changes the helpers.
>
> So no the platform isn't fully OS dependent.
> And yes the help string should be "select OS to be used"
>
> Nicolas
>
>

Re: [lng-odp] [PATCH 6/6] helper: take default os from odp platform

[Nicolas Morey-Chaisemartin]

Le 12/12/2016 à 03:52 PM, Mike Holmes a écrit :
> Signed-off-by: Mike Holmes 
> ---
>  configure.ac   | 4 ++--
>  platform/linux-generic/m4/configure.m4 | 2 ++
>  2 files changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/configure.ac b/configure.ac
> index f17bcae..eb5436c 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -158,9 +158,9 @@ ODP_CFLAGS="$ODP_CFLAGS 
> -DIMPLEMENTATION_NAME=$IMPLEMENTATION_NAME"
>  ##
>  AC_ARG_WITH([os],
>  [AS_HELP_STRING([--with-os=os],
> - [select platform to be used, default linux])],
> + [select platform to be used, default ${default_helper_os}])],
>  [],
> -[with_os=linux
> +[with_os=${default_helper_os}
>  ])
>  
>  AC_SUBST([with_os])
> diff --git a/platform/linux-generic/m4/configure.m4 
> b/platform/linux-generic/m4/configure.m4
> index d3e5528..bf6bbe2 100644
> --- a/platform/linux-generic/m4/configure.m4
> +++ b/platform/linux-generic/m4/configure.m4
> @@ -38,3 +38,5 @@ m4_include([platform/linux-generic/m4/odp_schedule.m4])
>  
>  AC_CONFIG_FILES([platform/linux-generic/Makefile
>   
> platform/linux-generic/include/odp/api/plat/static_inline.h])
> +#define the odp helper implimentation to be built
> +default_helper_os=linux

I don't think this would work well with multiple platforms.
Either all the platforms m4 are included and they would just override this 
value for the others.
Either only one is included, depending on the --with-platform flag, and I'm not 
sure how this would work. Until platform is selected, the default_helper_os 
will not be set.

I think the AC_ARG_WITH --with-os should have an empty default value. Platform 
configure.m4 should only set the value if it is empty.

Nicolas

Re: [lng-odp] [PATCH 4/6] helper: select os in configure

[Nicolas Morey-Chaisemartin]

Le 12/13/2016 à 08:46 AM, Christophe Milard a écrit :
> On 12 December 2016 at 15:52, Mike Holmes  wrote:
>
>> Signed-off-by: Mike Holmes 
>> ---
>>  configure.ac   | 14 ++
>>  helper/Makefile.am |  2 +-
>>  2 files changed, 15 insertions(+), 1 deletion(-)
>>
>> diff --git a/configure.ac b/configure.ac
>> index 20ec479..39c040d 100644
>> --- a/configure.ac
>> +++ b/configure.ac
>> @@ -154,6 +154,19 @@ fi
>>  ODP_CFLAGS="$ODP_CFLAGS -DIMPLEMENTATION_NAME=$IMPLEMENTATION_NAME"
>>
>>  
>> ##
>> +# Determine which os to build helpers for
>> +###
>> ###
>> +AC_ARG_WITH([os],
>> +[AS_HELP_STRING([--with-os=os],
>> +   [select platform to be used, default linux])],
>>
> Shouldn't it be " select OS to be used, default linux" ???
> Here again, we hit the platform vs OS question, but if the platform and OS
> are different things, then it should be OS here! If there are the same
> thing, then why not using the -with-platform which we already have...
>
> I guess we know what an OS is... but what is a platform?
> -The HW in which ODP is running (implies memory size limitation, different
> pktio interfaces...). This is my understanding, but odp/pltaform/linux-gen
> is missleading and should be called odp/os/linux-gen
> - A couple HW/OS, as this patch seems to imply?
>
> Christophe
>
> +[],
We have the case here at Kalray where we have a single platform (mppa) but 2 
supported OS depending on what the user wants to do (non ODP compliant code).
The OS has no impact on the platform, but changes the helpers.

So no the platform isn't fully OS dependent.
And yes the help string should be "select OS to be used"

Nicolas

Re: [lng-odp] [PATCH 1/6] helper: rename linux.h to threads.h

[Nicolas Morey-Chaisemartin]

Not really an issue for me but this patch breaks the build.

You did not change the include in the helper test (at least thread.c), but they 
are removed in the next patch.

I know you guys like your patch to build/work individually :)


Nicolas


Le 12/12/2016 à 03:52 PM, Mike Holmes a écrit :
> Signed-off-by: Mike Holmes 
> ---
>  example/classifier/odp_classifier.c | 2 +-
>  example/generator/odp_generator.c   | 2 +-
>  example/ipsec/odp_ipsec.c   | 2 +-
>  example/l2fwd_simple/odp_l2fwd_simple.c | 2 +-
>  example/l3fwd/odp_l3fwd.c   | 2 +-
>  example/packet/odp_pktio.c  | 2 +-
>  example/switch/odp_switch.c | 2 +-
>  example/time/time_global_test.c | 2 +-
>  example/timer/odp_timer_test.c  | 2 +-
>  helper/Makefile.am  | 2 +-
>  helper/include/odp/helper/{linux.h => threads.h}| 0
>  helper/linux.c  | 2 +-
>  helper/test/odpthreads.c| 2 +-
>  test/common_plat/common/odp_cunit_common.c  | 2 +-
>  test/common_plat/miscellaneous/odp_api_from_cpp.cpp | 2 +-
>  test/common_plat/performance/odp_crypto.c   | 2 +-
>  test/common_plat/performance/odp_l2fwd.c| 2 +-
>  test/common_plat/performance/odp_pktio_perf.c   | 2 +-
>  test/common_plat/performance/odp_sched_latency.c| 2 +-
>  test/common_plat/performance/odp_scheduling.c   | 2 +-
>  test/common_plat/validation/api/timer/timer.c   | 2 +-
>  test/linux-generic/mmap_vlan_ins/mmap_vlan_ins.c| 2 +-
>  test/linux-generic/pktio_ipc/ipc_common.h   | 2 +-
>  test/linux-generic/ring/ring_stress.c   | 2 +-
>  24 files changed, 23 insertions(+), 23 deletions(-)
>  rename helper/include/odp/helper/{linux.h => threads.h} (100%)
>
> diff --git a/example/classifier/odp_classifier.c 
> b/example/classifier/odp_classifier.c
> index 1bd2414..84da0c6 100644
> --- a/example/classifier/odp_classifier.c
> +++ b/example/classifier/odp_classifier.c
> @@ -12,11 +12,11 @@
>  #include 
>  
>  #include 
> -#include 
>  #include 
>  #include 
>  #include 
>  #include 
> +#include 
>  #include 
>  
>  /** @def MAX_WORKERS
> diff --git a/example/generator/odp_generator.c 
> b/example/generator/odp_generator.c
> index 48d7f5f..b881601 100644
> --- a/example/generator/odp_generator.c
> +++ b/example/generator/odp_generator.c
> @@ -20,7 +20,7 @@
>  
>  #include 
>  
> -#include 
> +#include 
>  #include 
>  #include 
>  #include 
> diff --git a/example/ipsec/odp_ipsec.c b/example/ipsec/odp_ipsec.c
> index 6a9a9fe..e32519f 100644
> --- a/example/ipsec/odp_ipsec.c
> +++ b/example/ipsec/odp_ipsec.c
> @@ -24,7 +24,7 @@
>  
>  #include 
>  
> -#include 
> +#include 
>  #include 
>  #include 
>  #include 
> diff --git a/example/l2fwd_simple/odp_l2fwd_simple.c 
> b/example/l2fwd_simple/odp_l2fwd_simple.c
> index 0682d2d..2473a11 100644
> --- a/example/l2fwd_simple/odp_l2fwd_simple.c
> +++ b/example/l2fwd_simple/odp_l2fwd_simple.c
> @@ -10,9 +10,9 @@
>  #include 
>  
>  #include 
> -#include 
>  #include 
>  #include 
> +#include 
>  
>  #define POOL_NUM_PKT 8192
>  #define POOL_SEG_LEN 1856
> diff --git a/example/l3fwd/odp_l3fwd.c b/example/l3fwd/odp_l3fwd.c
> index 8919bd3..b2bf01b 100644
> --- a/example/l3fwd/odp_l3fwd.c
> +++ b/example/l3fwd/odp_l3fwd.c
> @@ -14,11 +14,11 @@
>  #include 
>  
>  #include 
> -#include 
>  #include 
>  #include 
>  #include 
>  #include 
> +#include 
>  
>  #include "odp_l3fwd_db.h"
>  #include "odp_l3fwd_lpm.h"
> diff --git a/example/packet/odp_pktio.c b/example/packet/odp_pktio.c
> index d1135cb..6e24deb 100644
> --- a/example/packet/odp_pktio.c
> +++ b/example/packet/odp_pktio.c
> @@ -13,9 +13,9 @@
>  #include 
>  
>  #include 
> -#include 
>  #include 
>  #include 
> +#include 
>  
>  /** @def MAX_WORKERS
>   * @brief Maximum number of worker threads
> diff --git a/example/switch/odp_switch.c b/example/switch/odp_switch.c
> index 4b944fe..a37e6b8 100644
> --- a/example/switch/odp_switch.c
> +++ b/example/switch/odp_switch.c
> @@ -11,9 +11,9 @@
>  #include 
>  
>  #include 
> -#include 
>  #include 
>  #include 
> +#include 
>  
>  /** Maximum number of worker threads */
>  #define MAX_WORKERS32
> diff --git a/example/time/time_global_test.c b/example/time/time_global_test.c
> index 380ec52..d304075 100644
> --- a/example/time/time_global_test.c
> +++ b/example/time/time_global_test.c
> @@ -6,8 +6,8 @@
>  
>  #include 
>  #include 
> -#include 
>  #include 
> +#include 
>  
>  #define MAX_WORKERS  32
>  #define ITERATION_NUM2048
> diff --git a/example/timer/odp_timer_test.c b/example/timer/odp_timer_test.c
> index 035ab2e..b847e2b 100644
> --- a/example/timer/odp_timer_test.c
> +++ b/example/timer/odp_timer_test.c
> @@ -14,7 +14,7 @@
>  #include 
>  
>  /* ODP 

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

[Nicolas Morey-Chaisemartin]

I just had a quick look and it looks good to me.

Nicolas

Le 12/10/2016 à 08:29 PM, Mike Holmes a écrit :
> https://github.com/mike-holmes-linaro/odp/tree/helpers-os 
> <https://github.com/mike-holmes-linaro/odp/tree/helpers-os>
>
> Helper lib now gets the default helper OS from the  platfrom in use via its 
> configure.m4
>
> On 9 December 2016 at 08:45, Nicolas Morey-Chaisemartin <nmo...@kalray.eu 
> <mailto:nmo...@kalray.eu>> wrote:
>
>
>
> Le 12/09/2016 à 02:41 PM, Mike Holmes a écrit :
>>
>>
>> On 9 December 2016 at 08:33, Nicolas Morey-Chaisemartin 
>> <nmo...@kalray.eu <mailto:nmo...@kalray.eu>> wrote:
>>
>>
>>
>>     Le 12/09/2016 à 02:18 PM, Mike Holmes a écrit :
>>>
>>>
>>> On 9 December 2016 at 04:41, Nicolas Morey-Chaisemartin 
>>> <nmo...@kalray.eu <mailto:nmo...@kalray.eu>> wrote:
>>>
>>> I'm OK with this. The only issue I have with this is that it 
>>> moves back platform/OS selection back to configure.ac <http://configure.ac> 
>>> while I've been trying to move it out 
>>> (https://github.com/nmorey/odp/tree/dev/generic-platforms 
>>> <https://github.com/nmorey/odp/tree/dev/generic-platforms>).
>>> Shouldn't each platform select the right OS ?
>>> I mean linux-generic will probably always use the linux helper 
>>> while the mppa implementation will use the right one for us (depending on 
>>> the compilation flags).
>>>
>>>
>>> I would actually like to make the helpers much more independent so 
>>> I am in line with your thinking.
>>> How about we just do exactly that and have helpers  build 
>>> completely independently with its own configure.ac <http://configure.ac> ?
>>
>> Won't the dependence from the platform test to the helper lib be an 
>> issue?
>> Splitting them up might end in a circualt dependency.
>>
>>
>> I am hoping to find and delete any circular dependencies that exist, the 
>> helper should depend on the odp api, the tests and examples should depend on 
>> the helpers
>>  
>>
>>
>> Build ODP means building the tests which means building the helpers 
>> which need ODP to be built (for odp_cpumask_* functions at least).
>>
>> I'm not sure we really need to pull them out of the ODP build 
>> system, but simply keep the configure flexible enough so platforms can 
>> tweak/change the settings from the platform side. 
>>
>> Platform could add their own options in their configure.m4 if they 
>> need them, or simply select the basic helper setup and export the OS.
>>
>>
>> Ok then I misunderstood this "Shouldn't each platform select the right 
>> OS ?" - I agree it should.
>>
>> I added the selector "with_os" to the common configure.ac 
>> <http://configure.ac> does that not allow it to be changed by platform if we 
>> keep it all under one configure setup? - perhaps it can be set in a per 
>> platform configure.m4 as you say, I need to fiddle with that.
> Yes my bad. It works like that. configure.m4 should be able to adjust 
> settings depending on the option, or overwrite the value if they know better.
>
>>
>>  
>>
>>
>>
>>
>>>  
>>>
>>>
>>> Also I think the lib should be renamed to libodphelper instead 
>>> of libodphelper-linux.
>>>
>>>
>>> agree
>>>  
>>>
>>>
>>> Any plans to get these patches in soon?
>>>
>>>
>>> with your help asap
>>>  
>>>
>>> Should I wait for your patch to get in master, or get them in 
>>> my patch series?
>>>
>>>
>>> Will work with you, let me make your suggested change to the lib 
>>> and circle back with you on your first point.
>> Glad to hear it :)
>>
>>
>>
>>
>> -- 
>> Mike Holmes
>> Program Manager - Linaro Networking Group
>> Linaro.org <http://www.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 <http://www.linaro.org/>* **│ *Open source software for ARM SoCs
> "Work should be fun and collaborative, the rest follows"
>

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

[Nicolas Morey-Chaisemartin]

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

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

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

[Nicolas Morey-Chaisemartin]

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

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

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

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


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

Re: [lng-odp] odp_rwlock_read_trylock

[Nicolas Morey-Chaisemartin]

Le 12/09/2016 à 10:45 AM, Savolainen, Petri (Nokia - FI/Espoo) a écrit :
>
>> -Original Message-
>> From: Nicolas Morey-Chaisemartin [mailto:nmo...@kalray.eu]
>> Sent: Thursday, December 08, 2016 5:30 PM
>> To: Savolainen, Petri (Nokia - FI/Espoo) <petri.savolainen@nokia-bell-
>> labs.com>; LNG ODP Mailman List <lng-odp@lists.linaro.org>
>> Subject: Re: [lng-odp] odp_rwlock_read_trylock
>>
>>
>>
>> Le 12/08/2016 à 03:50 PM, Savolainen, Petri (Nokia - FI/Espoo) a écrit :
>>>> -Original Message-
>>>> From: lng-odp [mailto:lng-odp-boun...@lists.linaro.org] On Behalf Of
>>>> Nicolas Morey-Chaisemartin
>>>> Sent: Wednesday, December 07, 2016 10:12 AM
>>>> To: LNG ODP Mailman List <lng-odp@lists.linaro.org>
>>>> Subject: [lng-odp] odp_rwlock_read_trylock
>>>>
>>>> HI,
>>>>
>>>>
>>>> While looking into the test/code, I noticed something weird in the
>>>> implementation of the rwlock read trylock on monarch.
>>>>
>>>> int odp_rwlock_read_trylock(odp_rwlock_t *rwlock)
>>>> {
>>>> uint32_t zero = 0;
>>>>
>>>> return odp_atomic_cas_acq_u32(>cnt, , (uint32_t)1);
>>>> }
>>>>
>>>>
>>>> This mean the trylock only succeed if no one was using the lock. But it
>>>> will fail if someone was owning it *even* if it is a reader.
>>>> Is this something expected? If yes, it should probably be detailed in
>> the
>>>> API.
>>> /**
>>>  * Try to acquire read permission to a reader/writer lock.
>>>  *
>>>  * @param rwlock Pointer to a reader/writer lock
>>>  *
>>>  * @retval  0 Lock was not available for read access
>>>  * @retval !0 Read access to lock acquired
>>>  */
>>> int odp_rwlock_read_trylock(odp_rwlock_t *rwlock);
>>>
>>>
>>> The spec just says that "read access" was not available. Implementation
>> decides when it's available and application just needs to re-try later on.
>> So, this implementation is OK by the spec. Supporting multiple readers
>> would improve parallelism, but it may not be optimal for performance if
>> the lock is free in the common case.
>>>
>>>> The lock implementation I wrote allows to get the lock if a reader
>> already
>>>> owns it. And causes the testsuite to deadlock:
>>>>
>>>> odp_rwlock_init(rw_lock);
>>>> /* CU_ASSERT(odp_rwlock_is_locked(rw_lock) == 0); */
>>>>
>>>> odp_rwlock_read_lock(rw_lock); => Lock is owned in read
>>>>
>>>> rc = odp_rwlock_read_trylock(rw_lock); => Locked is owned a second
>>>> time in read (rc = 1)
>>> You cannot do this on a single thread (lock the same lock twice). You
>> must use recursive locks if the thread holding a lock will lock it again.
>> The issue is odp_rwlock_read_trylock fails here, but odp_rwlock_read_lock
>> would succeed.
>> Usually rwlock are recursive on the read side as it doesn't really matter
>> to the lock if the same thread owns it N times, or N threads own it once
>> each.
>> IMHO, in a non-concurrent setup, read_trylock should always succeed if
>> read_lock would succeed.
>> (I say non concurrent here to not acount for the case of contention on the
>> lock where many retries could be needed to get the lock).
>
> We have recursive version of the lock (odp_rwlock_recursive_read_lock(), etc) 
> and application should use that if the same thread will call lock() twice (or 
> lock + try_lock).

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

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


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

Nicolas

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

[Nicolas Morey-Chaisemartin]

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

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

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

Nicolas


Le 12/08/2016 à 09:47 PM, Mike Holmes a écrit :
> So this https://github.com/mike-holmes-linaro/odp/tree/helpers-os is what I 
> was thinking
>
> It moves the Linux specifics to helper/os/linux and selects the helper os to 
> be built in the configure step
> It renames the linux.h helper include to threads.h which is what it 
> predominately is
> It also removes the unused Linux specific  APIs since no odp test or example 
> uses them and they break having an OS agnostic helper suite.
>
> On 8 December 2016 at 13:44, Mike Holmes <mike.hol...@linaro.org 
> <mailto:mike.hol...@linaro.org>> wrote:
>
>
>
> On 7 December 2016 at 01:54, Nicolas Morey-Chaisemartin <nmo...@kalray.eu 
> <mailto:nmo...@kalray.eu>> wrote:
>
>
>
> Le 12/06/2016 à 09:37 PM, Mike Holmes a écrit :
> > On 6 December 2016 at 12:08, Nicolas Morey-Chaisemartin
>     > <nmo...@kalray.eu <mailto:nmo...@kalray.eu>> wrote:
> >> Signed-off-by: Nicolas Morey-Chaisemartin <nmo...@kalray.eu 
> <mailto:nmo...@kalray.eu>>
> >> ---
> >>  helper/Makefile.am  | 6 +++---
> >>  helper/test/Makefile.am | 6 +++---
> >>  2 files changed, 6 insertions(+), 6 deletions(-)
> >>
> >> diff --git a/helper/Makefile.am b/helper/Makefile.am
> >> index d09d900..c75db00 100644
> >> --- a/helper/Makefile.am
> >> +++ b/helper/Makefile.am
> >> @@ -1,7 +1,7 @@
> >>  include $(top_srcdir)/platform/@with_platform@/Makefile.inc
> >>
> >>  pkgconfigdir = $(libdir)/pkgconfig
> >> -pkgconfig_DATA = $(top_builddir)/pkgconfig/libodphelper-linux.pc
> >> +pkgconfig_DATA = 
> $(top_builddir)/pkgconfig/libodphelper@ODP_LIB_FLAVOR@.pc
> > Why would the helpers be tied to the implementation being built ? 
> They
> > should be agnostic and depend on the OS
> I agree that they should not depend on the implementation of ODP. But 
> as for libodp, helper might not be ABI compatible.
> The issue is that there's no evident place in the connfigure to 
> select which implementation of the helper should be used.
> platform configure.m4 are the easiest place to add this which ties 
> this to the platform.
> But I could add an ODPH_LIB_IMPL. So several platforms could use the 
> same helper, while keeping the possibility of using non ABI compatible 
> versions in other platforms.
>
> > I think the helpers are broken or we need an alternative to linux.c 
> in
> > there, perhaps a odp/helpers/platform/linux/linux.c and
> > odp/helpers/platform/mpaa/linux.c
> I changed it in our git tree. helper/linux.c got moved to 
> helper/os/linux/linux.c
> And we added helper/os/nodeos/linux.c and helper/os/mos/linux.c (we 
> have 2 OS flavors usable with ODP)
> The platform configure.m4 is in charge of selecting the OS depending 
> on flags & compiler selected.
>
>
> I would be keen to do the same thing master, if it is OS differences then 
> that is much more in keeping with the intent.
> I will play with a patch to move linux.c under and OS directory called 
> linux
>  
>
> >
> >>  LIB   = $(top_builddir)/lib
> >>  AM_CFLAGS  = -I$(srcdir)/include
> >> @@ -31,7 +31,7 @@ noinst_HEADERS = \
> >>  $(srcdir)/odph_lineartable.h \
> >>  $(srcdir)/odph_list_internal.h
> >>
> >> -__LIB__libodphelper_linux_la_SOURCES = \
> >> +__LIB__libodphelper@ODP_LIB_FLAVOR@_la_SOURCES = \
> >> eth.c \
> >> ip.c \
> >> chksum.c \
> >> @@ -39,4 +39,4 @@ __LIB__libodphelper_linux_la_SOURCES = \
>  

Re: [lng-odp] odp_rwlock_read_trylock

[Nicolas Morey-Chaisemartin]

Le 12/08/2016 à 03:50 PM, Savolainen, Petri (Nokia - FI/Espoo) a écrit :
>
>> -Original Message-
>> From: lng-odp [mailto:lng-odp-boun...@lists.linaro.org] On Behalf Of
>> Nicolas Morey-Chaisemartin
>> Sent: Wednesday, December 07, 2016 10:12 AM
>> To: LNG ODP Mailman List <lng-odp@lists.linaro.org>
>> Subject: [lng-odp] odp_rwlock_read_trylock
>>
>> HI,
>>
>>
>> While looking into the test/code, I noticed something weird in the
>> implementation of the rwlock read trylock on monarch.
>>
>> int odp_rwlock_read_trylock(odp_rwlock_t *rwlock)
>> {
>> uint32_t zero = 0;
>>
>> return odp_atomic_cas_acq_u32(>cnt, , (uint32_t)1);
>> }
>>
>>
>> This mean the trylock only succeed if no one was using the lock. But it
>> will fail if someone was owning it *even* if it is a reader.
>> Is this something expected? If yes, it should probably be detailed in the
>> API.
> /**
>  * Try to acquire read permission to a reader/writer lock.
>  *
>  * @param rwlock Pointer to a reader/writer lock
>  *
>  * @retval  0 Lock was not available for read access
>  * @retval !0 Read access to lock acquired
>  */
> int odp_rwlock_read_trylock(odp_rwlock_t *rwlock);
>
>
> The spec just says that "read access" was not available. Implementation 
> decides when it's available and application just needs to re-try later on. 
> So, this implementation is OK by the spec. Supporting multiple readers would 
> improve parallelism, but it may not be optimal for performance if the lock is 
> free in the common case.
>
>
>>
>> The lock implementation I wrote allows to get the lock if a reader already
>> owns it. And causes the testsuite to deadlock:
>>
>> odp_rwlock_init(rw_lock);
>> /* CU_ASSERT(odp_rwlock_is_locked(rw_lock) == 0); */
>>
>> odp_rwlock_read_lock(rw_lock); => Lock is owned in read
>>
>> rc = odp_rwlock_read_trylock(rw_lock); => Locked is owned a second
>> time in read (rc = 1)
>
> You cannot do this on a single thread (lock the same lock twice). You must 
> use recursive locks if the thread holding a lock will lock it again.
>  

The issue is odp_rwlock_read_trylock fails here, but odp_rwlock_read_lock would 
succeed.
Usually rwlock are recursive on the read side as it doesn't really matter to 
the lock if the same thread owns it N times, or N threads own it once each.
IMHO, in a non-concurrent setup, read_trylock should always succeed if 
read_lock would succeed.
(I say non concurrent here to not acount for the case of contention on the lock 
where many retries could be needed to get the lock).

>
>> CU_ASSERT(rc == 0);
>> rc = odp_rwlock_write_trylock(rw_lock); => Expected failure
>> CU_ASSERT(rc == 0);
>>
>> odp_rwlock_read_unlock(rw_lock); => Lock is still owned once.
>>
>>
>> So either the API should describe more accurately what are the expected
>> success/failure case, or the test and implementation changed.
>>
>> On a side note, the API explicitly says that reader have the priority on
>> writers on rwlock. Is this really an API requirement?
>> Our rwlocks are the other way around to avoid the starvation issue. Do I
>> need to change them ? Or is it OK with the API?
>>
> Do you mean this:
>  * A reader/writer lock allows multiple simultaneous readers but only one
>  * writer at a time. A thread that wants write access will have to wait until
>  * there are no threads that want read access. This causes a risk for
>  * starvation. The trylock variants can be used to avoid blocking when
>  * the lock is not immediately available.
>
> It could be softened with "may have to wait" and "may cause a risk". 
> Different RW lock implementations should be possible.
>
> -Petri
What bothers me is this:

A thread that wants write access will have to wait until
 * there are no threads that want read access.

This clearly states that a writer cannot get the write lock if a reader *wants* 
it.
I think

A thread that wants write access will have to wait until
 * there are no threads that own read permission on the lock.


would relax this constraint and allow for write-priority rwlock.

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

[Nicolas Morey-Chaisemartin]

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

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


Nicolas


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

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

[Nicolas Morey-Chaisemartin]

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

Signed-off-by: Nicolas Morey-Chaisemartin <nmo...@kalray.eu>
---

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

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

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

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

[Nicolas Morey-Chaisemartin]

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

Signed-off-by: Nicolas Morey-Chaisemartin <nmo...@kalray.eu>
---

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

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

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

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

[Nicolas Morey-Chaisemartin]

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

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



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

[lng-odp] odp_rwlock_read_trylock

[Nicolas Morey-Chaisemartin]

HI,


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

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

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


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


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

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

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

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

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


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

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

Nicolas

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

[Nicolas Morey-Chaisemartin]

I usually force somme access to be uncached. A whole barrier would work, but 
the performance cost is way too big.


Just as an example:

static inline void odp_atomic_init_u64(odp_atomic_u64_t *atom, uint64_t val)
{
atom->v = val;
#if __GCC_ATOMIC_LLONG_LOCK_FREE < 2
__atomic_clear(>lock, __ATOMIC_RELAXED);
#endif
}


becomes

static inline void odp_atomic_init_u64(odp_atomic_u64_t *atom, uint64_t val)
{
STORE_U64(atom->v, val);
}

where STORE_U64 forces an uncached write directly to memory.

Nicolas

Le 12/07/2016 à 08:09 AM, Maxim Uvarov a écrit :
> Nicolas, what do you do to solve coherency problem? I think that maybe we are 
> missing some api in odp, like read/write barriers.
>
> Maxim.
>
> On 7 December 2016 at 09:54, Nicolas Morey-Chaisemartin <nmo...@kalray.eu 
> <mailto:nmo...@kalray.eu>> wrote:
>
>
>
> Le 12/06/2016 à 09:37 PM, Mike Holmes a écrit :
>     > On 6 December 2016 at 12:08, Nicolas Morey-Chaisemartin
> > <nmo...@kalray.eu <mailto:nmo...@kalray.eu>> wrote:
> >> Signed-off-by: Nicolas Morey-Chaisemartin <nmo...@kalray.eu 
> <mailto:nmo...@kalray.eu>>
> >> ---
> >>  helper/Makefile.am  | 6 +++---
> >>  helper/test/Makefile.am | 6 +++---
> >>  2 files changed, 6 insertions(+), 6 deletions(-)
> >>
> >> diff --git a/helper/Makefile.am b/helper/Makefile.am
> >> index d09d900..c75db00 100644
> >> --- a/helper/Makefile.am
> >> +++ b/helper/Makefile.am
> >> @@ -1,7 +1,7 @@
> >>  include $(top_srcdir)/platform/@with_platform@/Makefile.inc
> >>
> >>  pkgconfigdir = $(libdir)/pkgconfig
> >> -pkgconfig_DATA = $(top_builddir)/pkgconfig/libodphelper-linux.pc
> >> +pkgconfig_DATA = 
> $(top_builddir)/pkgconfig/libodphelper@ODP_LIB_FLAVOR@.pc
> > Why would the helpers be tied to the implementation being built ? They
> > should be agnostic and depend on the OS
> I agree that they should not depend on the implementation of ODP. But as 
> for libodp, helper might not be ABI compatible.
> The issue is that there's no evident place in the connfigure to select 
> which implementation of the helper should be used.
> platform configure.m4 are the easiest place to add this which ties this 
> to the platform.
> But I could add an ODPH_LIB_IMPL. So several platforms could use the same 
> helper, while keeping the possibility of using non ABI compatible versions in 
> other platforms.
>
> > I think the helpers are broken or we need an alternative to linux.c in
> > there, perhaps a odp/helpers/platform/linux/linux.c and
> > odp/helpers/platform/mpaa/linux.c
> I changed it in our git tree. helper/linux.c got moved to 
> helper/os/linux/linux.c
> And we added helper/os/nodeos/linux.c and helper/os/mos/linux.c (we have 
> 2 OS flavors usable with ODP)
> The platform configure.m4 is in charge of selecting the OS depending on 
> flags & compiler selected.
> >
> >>  LIB   = $(top_builddir)/lib
> >>  AM_CFLAGS  = -I$(srcdir)/include
> >> @@ -31,7 +31,7 @@ noinst_HEADERS = \
> >>  $(srcdir)/odph_lineartable.h \
> >>  $(srcdir)/odph_list_internal.h
> >>
> >> -__LIB__libodphelper_linux_la_SOURCES = \
> >> +__LIB__libodphelper@ODP_LIB_FLAVOR@_la_SOURCES = \
> >> eth.c \
> >> ip.c \
> >> chksum.c \
> >> @@ -39,4 +39,4 @@ __LIB__libodphelper_linux_la_SOURCES = \
> >> hashtable.c \
> >> lineartable.c
> >>
> >> -lib_LTLIBRARIES = $(LIB)/libodphelper-linux.la 
> <http://libodphelper-linux.la>
> >> +lib_LTLIBRARIES = $(LIB)/libodphelper@ODP_LIB_FLAVOR@.la
> >> diff --git a/helper/test/Makefile.am b/helper/test/Makefile.am
> >> index 545db73..02c260c 100644
> >> --- a/helper/test/Makefile.am
> >> +++ b/helper/test/Makefile.am
> >> @@ -28,10 +28,10 @@ EXTRA_DIST = odpthreads_as_processes 
> odpthreads_as_pthreads
> >>
> >>  dist_chksum_SOURCES = chksum.c
> >>  dist_odpthreads_SOURCES = odpthreads.c
> >> -odpthreads_LDADD = $(LIB)/libodphelper-linux.la 
> <http://libodphelper-linux.la> $(LIB)/libodp-linux.la <http://libodp-linux.la>
> >> +odpthreads_LDADD = $(LIB)/libodphe

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

[Nicolas Morey-Chaisemartin]

Le 12/06/2016 à 09:37 PM, Mike Holmes a écrit :
> On 6 December 2016 at 12:08, Nicolas Morey-Chaisemartin
> <nmo...@kalray.eu> wrote:
>> Signed-off-by: Nicolas Morey-Chaisemartin <nmo...@kalray.eu>
>> ---
>>  helper/Makefile.am  | 6 +++---
>>  helper/test/Makefile.am | 6 +++---
>>  2 files changed, 6 insertions(+), 6 deletions(-)
>>
>> diff --git a/helper/Makefile.am b/helper/Makefile.am
>> index d09d900..c75db00 100644
>> --- a/helper/Makefile.am
>> +++ b/helper/Makefile.am
>> @@ -1,7 +1,7 @@
>>  include $(top_srcdir)/platform/@with_platform@/Makefile.inc
>>
>>  pkgconfigdir = $(libdir)/pkgconfig
>> -pkgconfig_DATA = $(top_builddir)/pkgconfig/libodphelper-linux.pc
>> +pkgconfig_DATA = $(top_builddir)/pkgconfig/libodphelper@ODP_LIB_FLAVOR@.pc
> Why would the helpers be tied to the implementation being built ? They
> should be agnostic and depend on the OS
I agree that they should not depend on the implementation of ODP. But as for 
libodp, helper might not be ABI compatible.
The issue is that there's no evident place in the connfigure to select which 
implementation of the helper should be used.
platform configure.m4 are the easiest place to add this which ties this to the 
platform.
But I could add an ODPH_LIB_IMPL. So several platforms could use the same 
helper, while keeping the possibility of using non ABI compatible versions in 
other platforms.

> I think the helpers are broken or we need an alternative to linux.c in
> there, perhaps a odp/helpers/platform/linux/linux.c and
> odp/helpers/platform/mpaa/linux.c
I changed it in our git tree. helper/linux.c got moved to 
helper/os/linux/linux.c
And we added helper/os/nodeos/linux.c and helper/os/mos/linux.c (we have 2 OS 
flavors usable with ODP)
The platform configure.m4 is in charge of selecting the OS depending on flags & 
compiler selected.
>
>>  LIB   = $(top_builddir)/lib
>>  AM_CFLAGS  = -I$(srcdir)/include
>> @@ -31,7 +31,7 @@ noinst_HEADERS = \
>>  $(srcdir)/odph_lineartable.h \
>>  $(srcdir)/odph_list_internal.h
>>
>> -__LIB__libodphelper_linux_la_SOURCES = \
>> +__LIB__libodphelper@ODP_LIB_FLAVOR@_la_SOURCES = \
>> eth.c \
>> ip.c \
>> chksum.c \
>> @@ -39,4 +39,4 @@ __LIB__libodphelper_linux_la_SOURCES = \
>> hashtable.c \
>> lineartable.c
>>
>> -lib_LTLIBRARIES = $(LIB)/libodphelper-linux.la
>> +lib_LTLIBRARIES = $(LIB)/libodphelper@ODP_LIB_FLAVOR@.la
>> diff --git a/helper/test/Makefile.am b/helper/test/Makefile.am
>> index 545db73..02c260c 100644
>> --- a/helper/test/Makefile.am
>> +++ b/helper/test/Makefile.am
>> @@ -28,10 +28,10 @@ EXTRA_DIST = odpthreads_as_processes 
>> odpthreads_as_pthreads
>>
>>  dist_chksum_SOURCES = chksum.c
>>  dist_odpthreads_SOURCES = odpthreads.c
>> -odpthreads_LDADD = $(LIB)/libodphelper-linux.la $(LIB)/libodp-linux.la
>> +odpthreads_LDADD = $(LIB)/libodphelper@ODP_LIB_FLAVOR@.la 
>> $(LIB)/libodp@ODP_LIB_FLAVOR@.la
>>  dist_thread_SOURCES = thread.c
>> -thread_LDADD = $(LIB)/libodphelper-linux.la $(LIB)/libodp-linux.la
>> +thread_LDADD = $(LIB)/libodphelper@ODP_LIB_FLAVOR@.la 
>> $(LIB)/libodp@ODP_LIB_FLAVOR@.la
>>  dist_process_SOURCES = process.c
>>  dist_parse_SOURCES = parse.c
>> -process_LDADD = $(LIB)/libodphelper-linux.la $(LIB)/libodp-linux.la
>> +process_LDADD = $(LIB)/libodphelper@ODP_LIB_FLAVOR@.la 
>> $(LIB)/libodp@ODP_LIB_FLAVOR@.la
>>  dist_table_SOURCES = table.c
>> --
>> 2.10.1.4.g0ffc436
>>
>>
>
>

Re: [lng-odp] [PATCH v2 1/6] m4: add platform configurable library suffixes

[Nicolas Morey-Chaisemartin]

Le 12/06/2016 à 09:25 PM, Mike Holmes a écrit :
> On 6 December 2016 at 12:05, Nicolas Morey-Chaisemartin
> <nmo...@kalray.eu> wrote:
>> This is the suffix to be used to compute the name of ODP
>>  and ODP helper libraries.
>> platform can set a ODP_LIB_SUFFIX which is then exported as
>>  ODP_LIB_FLAVOR and ODP_LIB_FLAVOR_AM (for automake rules)
Crap. I just realized I forgot to drop the _AM part of the log.
I thought automake would complain if I use "-" in the rules to build libraries 
(we use _ right now) but it seems to be working nicely
> I think this should be ODP_LIB_IMPL -> (IMPLEMENTATION) I  initially
> thought platform but I believe that it it will be likely that one
> platform may be built in more than one way so there is not a 1:1
> correlation to odp/platform.
> An example might be Petris suggestions for building a static .so (API
> incompatible) in addition to the normal .so and .a files, this will
> need a different suffix.
> libodp.so -> regular ABI compatible library
> libodp-static.so -> modified non ABI compatible .so that it has a lib
> version of 0.0.0. This is not something that we should build but it is
> possible for a developer to set ODP_LIB_IMPL=static along with the ABI
> flags they want.
>
> And the others we know of are
> libodp-mpaa
> libodp-dpdk
If everyone agrees on this, I'll change it.

Nicolas

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

[Nicolas Morey-Chaisemartin]

Le 12/06/2016 à 10:17 PM, Maxim Uvarov a écrit :
> On 12/06/16 23:37, Mike Holmes wrote:
>> On 6 December 2016 at 12:08, Nicolas Morey-Chaisemartin
>> <nmo...@kalray.eu> wrote:
>>> Signed-off-by: Nicolas Morey-Chaisemartin <nmo...@kalray.eu>
>>> ---
>>>  helper/Makefile.am  | 6 +++---
>>>  helper/test/Makefile.am | 6 +++---
>>>  2 files changed, 6 insertions(+), 6 deletions(-)
>>>
>>> diff --git a/helper/Makefile.am b/helper/Makefile.am
>>> index d09d900..c75db00 100644
>>> --- a/helper/Makefile.am
>>> +++ b/helper/Makefile.am
>>> @@ -1,7 +1,7 @@
>>>  include $(top_srcdir)/platform/@with_platform@/Makefile.inc
>>>
>>>  pkgconfigdir = $(libdir)/pkgconfig
>>> -pkgconfig_DATA = $(top_builddir)/pkgconfig/libodphelper-linux.pc
>>> +pkgconfig_DATA = $(top_builddir)/pkgconfig/libodphelper@ODP_LIB_FLAVOR@.pc
>> Why would the helpers be tied to the implementation being built ? They
>> should be agnostic and depend on the OS
>> I think the helpers are broken or we need an alternative to linux.c in
>> there, perhaps a odp/helpers/platform/linux/linux.c and
>> odp/helpers/platform/mpaa/linux.c
>>
>
> Nicolas, do you have modification in helpers? We checked that
> linux-generic and odp-dpdk platform have the same source code for
> helpers. And we are doing efforts to make it the same.
>
> Maxim.
At this moment we have a different linux.c for our own thread/process 
implementation.
But I think I'll have to change some things in the hashtable helper to handle 
our lack of cache coherency.

Nicolas

[lng-odp] [PATCH v2 5/6] example: use ODP_LIB_FLAVOR to generate libodp name

[Nicolas Morey-Chaisemartin]

Signed-off-by: Nicolas Morey-Chaisemartin <nmo...@kalray.eu>
---
 example/Makefile.inc | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/example/Makefile.inc b/example/Makefile.inc
index 19d3994..62ead02 100644
--- a/example/Makefile.inc
+++ b/example/Makefile.inc
@@ -1,6 +1,6 @@
 include $(top_srcdir)/platform/@with_platform@/Makefile.inc
 LIB   = $(top_builddir)/lib
-LDADD = $(LIB)/libodp-linux.la $(LIB)/libodphelper-linux.la
+LDADD = $(LIB)/libodp@ODP_LIB_FLAVOR@.la $(LIB)/libodphelper@ODP_LIB_FLAVOR@.la
 AM_CFLAGS += \
-I$(srcdir) \
-I$(top_srcdir)/example \
-- 
2.10.1.4.g0ffc436

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

[Nicolas Morey-Chaisemartin]

Signed-off-by: Nicolas Morey-Chaisemartin <nmo...@kalray.eu>
---
 platform/Makefile.inc  | 6 +++---
 platform/linux-generic/Makefile.am | 4 ++--
 2 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/platform/Makefile.inc b/platform/Makefile.inc
index 2722946..a37fd10 100644
--- a/platform/Makefile.inc
+++ b/platform/Makefile.inc
@@ -1,12 +1,12 @@
 LIB   = $(top_builddir)/lib
 
 pkgconfigdir = $(libdir)/pkgconfig
-pkgconfig_DATA = $(top_builddir)/pkgconfig/libodp-linux.pc
+pkgconfig_DATA = $(top_builddir)/pkgconfig/libodp@ODP_LIB_FLAVOR@.pc
 
-.PHONY: pkgconfig/libodp-linux.pc
+.PHONY: pkgconfig/libodp@ODP_LIB_FLAVOR@.pc
 
 VPATH = $(srcdir) $(builddir)
-lib_LTLIBRARIES = $(LIB)/libodp-linux.la
+lib_LTLIBRARIES = $(LIB)/libodp@ODP_LIB_FLAVOR@.la
 
 AM_LDFLAGS += -version-number '$(ODP_LIBSO_VERSION)'
 
diff --git a/platform/linux-generic/Makefile.am 
b/platform/linux-generic/Makefile.am
index 22cf6f3..f08b549 100644
--- a/platform/linux-generic/Makefile.am
+++ b/platform/linux-generic/Makefile.am
@@ -138,7 +138,7 @@ noinst_HEADERS = \
  ${srcdir}/include/protocols/udp.h \
  ${srcdir}/Makefile.inc
 
-__LIB__libodp_linux_la_SOURCES = \
+__LIB__libodp@ODP_LIB_FLAVOR@_la_SOURCES = \
   odp_atomic.c \
   odp_barrier.c \
   odp_buffer.c \
@@ -198,5 +198,5 @@ __LIB__libodp_linux_la_SOURCES = \
   arch/@ARCH_DIR@/odp_sysinfo_parse.c
 
 if HAVE_PCAP
-__LIB__libodp_linux_la_SOURCES += pktio/pcap.c
+__LIB__libodp@ODP_LIB_FLAVOR@_la_SOURCES += pktio/pcap.c
 endif
-- 
2.10.1.4.g0ffc436

[lng-odp] [PATCH v2 3/6] test: use ODP_LIB_FLAVOR to generate libodp name

[Nicolas Morey-Chaisemartin]

Signed-off-by: Nicolas Morey-Chaisemartin <nmo...@kalray.eu>
---
 test/Makefile.inc| 2 +-
 test/common_plat/validation/api/Makefile.inc | 2 +-
 test/linux-generic/Makefile.inc  | 2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/test/Makefile.inc b/test/Makefile.inc
index 1ebc047..02cf929 100644
--- a/test/Makefile.inc
+++ b/test/Makefile.inc
@@ -4,7 +4,7 @@ LIB   = $(top_builddir)/lib
 #in the following line, the libs using the symbols should come before
 #the libs containing them! The includer is given a chance to add things
 #before libodp by setting PRE_LDADD before the inclusion.
-LDADD = $(PRE_LDADD) $(LIB)/libodphelper-linux.la $(LIB)/libodp-linux.la
+LDADD = $(PRE_LDADD) $(LIB)/libodphelper@ODP_LIB_FLAVOR@.la 
$(LIB)/libodp@ODP_LIB_FLAVOR@.la
 
 INCFLAGS = \
-I$(top_builddir)/platform/@with_platform@/include \
diff --git a/test/common_plat/validation/api/Makefile.inc 
b/test/common_plat/validation/api/Makefile.inc
index ffba620..c6c5524 100644
--- a/test/common_plat/validation/api/Makefile.inc
+++ b/test/common_plat/validation/api/Makefile.inc
@@ -13,4 +13,4 @@ AM_LDFLAGS += -static
 LIBCUNIT_COMMON = $(COMMON_DIR)/libcunit_common.la
 LIBCPUMASK_COMMON = $(COMMON_DIR)/libcpumask_common.la
 LIBTHRMASK_COMMON = $(COMMON_DIR)/libthrmask_common.la
-LIBODP = $(LIB)/libodphelper-linux.la $(LIB)/libodp-linux.la
+LIBODP = $(LIB)/libodphelper@ODP_LIB_FLAVOR@.la 
$(LIB)/libodp@ODP_LIB_FLAVOR@.la
diff --git a/test/linux-generic/Makefile.inc b/test/linux-generic/Makefile.inc
index 36745fe..badde82 100644
--- a/test/linux-generic/Makefile.inc
+++ b/test/linux-generic/Makefile.inc
@@ -6,7 +6,7 @@ AM_LDFLAGS += -static
 
 LIBCUNIT_COMMON = $(top_builddir)/test/common_plat/common/libcunit_common.la
 LIB   = $(top_builddir)/lib
-LIBODP = $(LIB)/libodphelper-linux.la $(LIB)/libodp-linux.la
+LIBODP = $(LIB)/libodphelper@ODP_LIB_FLAVOR@.la 
$(LIB)/libodp@ODP_LIB_FLAVOR@.la
 
 INCCUNIT_COMMON = -I$(top_srcdir)/test/common_plat/common
 INCODP =  \
-- 
2.10.1.4.g0ffc436

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

[Nicolas Morey-Chaisemartin]

Signed-off-by: Nicolas Morey-Chaisemartin <nmo...@kalray.eu>
---
 helper/Makefile.am  | 6 +++---
 helper/test/Makefile.am | 6 +++---
 2 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/helper/Makefile.am b/helper/Makefile.am
index d09d900..c75db00 100644
--- a/helper/Makefile.am
+++ b/helper/Makefile.am
@@ -1,7 +1,7 @@
 include $(top_srcdir)/platform/@with_platform@/Makefile.inc
 
 pkgconfigdir = $(libdir)/pkgconfig
-pkgconfig_DATA = $(top_builddir)/pkgconfig/libodphelper-linux.pc
+pkgconfig_DATA = $(top_builddir)/pkgconfig/libodphelper@ODP_LIB_FLAVOR@.pc
 
 LIB   = $(top_builddir)/lib
 AM_CFLAGS  = -I$(srcdir)/include
@@ -31,7 +31,7 @@ noinst_HEADERS = \
 $(srcdir)/odph_lineartable.h \
 $(srcdir)/odph_list_internal.h
 
-__LIB__libodphelper_linux_la_SOURCES = \
+__LIB__libodphelper@ODP_LIB_FLAVOR@_la_SOURCES = \
eth.c \
ip.c \
chksum.c \
@@ -39,4 +39,4 @@ __LIB__libodphelper_linux_la_SOURCES = \
hashtable.c \
lineartable.c
 
-lib_LTLIBRARIES = $(LIB)/libodphelper-linux.la
+lib_LTLIBRARIES = $(LIB)/libodphelper@ODP_LIB_FLAVOR@.la
diff --git a/helper/test/Makefile.am b/helper/test/Makefile.am
index 545db73..02c260c 100644
--- a/helper/test/Makefile.am
+++ b/helper/test/Makefile.am
@@ -28,10 +28,10 @@ EXTRA_DIST = odpthreads_as_processes odpthreads_as_pthreads
 
 dist_chksum_SOURCES = chksum.c
 dist_odpthreads_SOURCES = odpthreads.c
-odpthreads_LDADD = $(LIB)/libodphelper-linux.la $(LIB)/libodp-linux.la
+odpthreads_LDADD = $(LIB)/libodphelper@ODP_LIB_FLAVOR@.la 
$(LIB)/libodp@ODP_LIB_FLAVOR@.la
 dist_thread_SOURCES = thread.c
-thread_LDADD = $(LIB)/libodphelper-linux.la $(LIB)/libodp-linux.la
+thread_LDADD = $(LIB)/libodphelper@ODP_LIB_FLAVOR@.la 
$(LIB)/libodp@ODP_LIB_FLAVOR@.la
 dist_process_SOURCES = process.c
 dist_parse_SOURCES = parse.c
-process_LDADD = $(LIB)/libodphelper-linux.la $(LIB)/libodp-linux.la
+process_LDADD = $(LIB)/libodphelper@ODP_LIB_FLAVOR@.la 
$(LIB)/libodp@ODP_LIB_FLAVOR@.la
 dist_table_SOURCES = table.c
-- 
2.10.1.4.g0ffc436

[lng-odp] [PATCH v2 2/6] platform: move pkgconfig include to linux-generic

[Nicolas Morey-Chaisemartin]

Signed-off-by: Nicolas Morey-Chaisemartin <nmo...@kalray.eu>
---
 configure.ac   | 2 --
 platform/linux-generic/m4/configure.m4 | 4 +++-
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/configure.ac b/configure.ac
index 01d6448..0e1513f 100644
--- a/configure.ac
+++ b/configure.ac
@@ -293,8 +293,6 @@ AM_CXXFLAGS="-std=c++11"
 AC_CONFIG_FILES([Makefile
 helper/Makefile
 helper/test/Makefile
-pkgconfig/libodp-linux.pc
-pkgconfig/libodphelper-linux.pc
 ])
 
 AC_SEARCH_LIBS([timer_create],[rt posix4])
diff --git a/platform/linux-generic/m4/configure.m4 
b/platform/linux-generic/m4/configure.m4
index d5a7e1f..c478840 100644
--- a/platform/linux-generic/m4/configure.m4
+++ b/platform/linux-generic/m4/configure.m4
@@ -37,6 +37,8 @@ m4_include([platform/linux-generic/m4/odp_ipc.m4])
 m4_include([platform/linux-generic/m4/odp_schedule.m4])
 
 AC_CONFIG_FILES([platform/linux-generic/Makefile
- platform/linux-generic/include/odp/api/plat/static_inline.h])
+ platform/linux-generic/include/odp/api/plat/static_inline.h
+ pkgconfig/libodp-linux.pc
+ pkgconfig/libodphelper-linux.pc])
 
 ODP_LIB_FLAVOR=-linux
-- 
2.10.1.4.g0ffc436

[lng-odp] [PATCH v2 1/6] m4: add platform configurable library suffixes

[Nicolas Morey-Chaisemartin]

This is the suffix to be used to compute the name of ODP
 and ODP helper libraries.
platform can set a ODP_LIB_SUFFIX which is then exported as
 ODP_LIB_FLAVOR and ODP_LIB_FLAVOR_AM (for automake rules)

Signed-off-by: Nicolas Morey-Chaisemartin <nmo...@kalray.eu>
---
 configure.ac   | 1 +
 platform/linux-generic/m4/configure.m4 | 2 ++
 2 files changed, 3 insertions(+)

diff --git a/configure.ac b/configure.ac
index 3e89b0a..01d6448 100644
--- a/configure.ac
+++ b/configure.ac
@@ -310,6 +310,7 @@ AC_SUBST([CFLAGS])
 AC_SUBST([AM_LDFLAGS])
 AC_SUBST([LDFLAGS])
 AC_SUBST([EXEEXT])
+AC_SUBST([ODP_LIB_FLAVOR])
 
 AC_OUTPUT
 AC_MSG_RESULT([
diff --git a/platform/linux-generic/m4/configure.m4 
b/platform/linux-generic/m4/configure.m4
index d3e5528..d5a7e1f 100644
--- a/platform/linux-generic/m4/configure.m4
+++ b/platform/linux-generic/m4/configure.m4
@@ -38,3 +38,5 @@ m4_include([platform/linux-generic/m4/odp_schedule.m4])
 
 AC_CONFIG_FILES([platform/linux-generic/Makefile
  platform/linux-generic/include/odp/api/plat/static_inline.h])
+
+ODP_LIB_FLAVOR=-linux
-- 
2.10.1.4.g0ffc436

[lng-odp] [PATCH v2 0/6] Make lidodp/libodp helper name platform specific

[Nicolas Morey-Chaisemartin]

The lib suffix "linux" (libodp-linux, libodphelper-linux) is harcoded pretty 
much everywhere in the build system.
The goal of this series is to allow each platform to define its library suffix 
so they don't have to touch the generic files but simply the platform specific 
files.

v2:
- Moved to master (instead of monarch_lts)
- rename ODP_LIB_NAME to ODP_LIB_FLAVOR
- Supports empty ODP_LIB_FLAVOR (libs will be libodp.a libodphelper.a
- Fix mising replace (helpers, linux-generic specific tests)


Nicolas Morey-Chaisemartin (6):
  m4: add platform configurable library suffixes
  platform: move pkgconfig include to linux-generic
  test: use ODP_LIB_FLAVOR to generate libodp name
  helper: use ODP_LIB_FLAVOR to generate libodp name
  example: use ODP_LIB_FLAVOR to generate libodp name
  platform: use ODP_LIB_FLAVOR to generate libodp name

 configure.ac | 3 +--
 example/Makefile.inc | 2 +-
 helper/Makefile.am   | 6 +++---
 helper/test/Makefile.am  | 6 +++---
 platform/Makefile.inc| 6 +++---
 platform/linux-generic/Makefile.am   | 4 ++--
 platform/linux-generic/m4/configure.m4   | 6 +-
 test/Makefile.inc| 2 +-
 test/common_plat/validation/api/Makefile.inc | 2 +-
 test/linux-generic/Makefile.inc  | 2 +-
 10 files changed, 21 insertions(+), 18 deletions(-)

Re: [lng-odp] [PATCH] configure: add option to enable/disable helper debug prints

[Nicolas Morey-Chaisemartin]

Le 12/06/2016 à 05:35 PM, Christophe Milard a écrit :
> On 6 December 2016 at 15:28, Nicolas Morey-Chaisemartin <nmo...@kalray.eu>
> wrote:
>
>> As a side effect, debug print are turned off by default
>>
>> Signed-off-by: Nicolas Morey-Chaisemartin <nmo...@kalray.eu>
>> ---
>>  configure.ac   | 14 ++
>>  helper/Makefile.am |  3 ++-
>>  2 files changed, 16 insertions(+), 1 deletion(-)
>>
>> diff --git a/configure.ac b/configure.ac
>> index 3e89b0a..aa992fc 100644
>> --- a/configure.ac
>> +++ b/configure.ac
>> @@ -231,6 +231,19 @@ AC_ARG_ENABLE([debug-print],
>>  ODP_CFLAGS="$ODP_CFLAGS -DODP_DEBUG_PRINT=$ODP_DEBUG_PRINT"
>>
>>  
>> ##
>> +# Enable/disable ODPH_DEBUG_PRINT
>> +###
>> ###
>> +ODPH_DEBUG_PRINT=0
>> +AC_ARG_ENABLE([helper-debug-print],
>> +[  --enable-helper-debug-printdisplay debugging information for
>> ODP helpers],
>> +[if test "x$enableval" = "xyes"; then
>> +   ODPH_DEBUG_PRINT=1
>> + else
>> +   ODPH_DEBUG_PRINT=0
>> +fi])
>> +ODPH_CFLAGS="$ODPH_CFLAGS -DODPH_DEBUG_PRINT=$ODPH_DEBUG_PRINT"
>> +
>> +###
>> ###
>>  # Enable/disable ODP_DEBUG
>>  
>> ##
>>  ODP_DEBUG=0
>> @@ -310,6 +323,7 @@ AC_SUBST([CFLAGS])
>>  AC_SUBST([AM_LDFLAGS])
>>  AC_SUBST([LDFLAGS])
>>  AC_SUBST([EXEEXT])
>> +AC_SUBST([ODPH_CFLAGS])
>>
> ODPH_CPPLAGS. see my comment below.
>
>
>>  AC_OUTPUT
>>  AC_MSG_RESULT([
>> diff --git a/helper/Makefile.am b/helper/Makefile.am
>> index d09d900..2ef4e8b 100644
>> --- a/helper/Makefile.am
>> +++ b/helper/Makefile.am
>> @@ -4,7 +4,8 @@ pkgconfigdir = $(libdir)/pkgconfig
>>  pkgconfig_DATA = $(top_builddir)/pkgconfig/libodphelper-linux.pc
>>
>>  LIB   = $(top_builddir)/lib
>> -AM_CFLAGS  = -I$(srcdir)/include
>> +AM_CFLAGS  = $(ODPH_CFLAGS)
>> +AM_CFLAGS += -I$(srcdir)/include
>>
> Hi Nicolas,
> According to
>
> https://www.gnu.org/software/automake/manual/html_node/Program-Variables.html
> The above should be put in the AM_CPPFLAGS rather than AM_CFLAGS.
> It is wrong everywhere, and  I became aware of this subtle autotools
> difference recently, so I cannot really blame it on you, but if you can
> change it, we maybe should try to do it right from now...
>
> Christophe
So I should set
AM_CPPFLAGS = $(ODPH_CFLAGS) and keep the rest as is ?

Nicolas

[lng-odp] [PATCH] example: traffic_mgmt: use PRIu32 instead of %u

[Nicolas Morey-Chaisemartin]

Signed-off-by: Nicolas Morey-Chaisemartin <nmo...@kalray.eu>
---

This should be backported to monarch_lts too. However it requires an extra 
#include 

 example/traffic_mgmt/odp_traffic_mgmt.c | 19 +++
 1 file changed, 11 insertions(+), 8 deletions(-)

diff --git a/example/traffic_mgmt/odp_traffic_mgmt.c 
b/example/traffic_mgmt/odp_traffic_mgmt.c
index c4f5356..94e208b 100644
--- a/example/traffic_mgmt/odp_traffic_mgmt.c
+++ b/example/traffic_mgmt/odp_traffic_mgmt.c
@@ -261,7 +261,8 @@ static uint32_t create_profile_set(profile_params_set_t 
*profile_params_set,
if (name_idx == 0)
snprintf(name, sizeof(name), "%s", base_name);
else
-   snprintf(name, sizeof(name), "%s-%u", base_name, name_idx);
+   snprintf(name, sizeof(name), "%s-%" PRIu32,
+base_name, name_idx);
 
odp_tm_shaper_params_init(_params);
shaper  = _params_set->shaper_params;
@@ -289,7 +290,8 @@ static uint32_t create_profile_set(profile_params_set_t 
*profile_params_set,
err_cnt++;
 
for (color = 0; color < ODP_NUM_PACKET_COLORS; color++) {
-   snprintf(wred_name, sizeof(wred_name), "%s-%u", name, color);
+   snprintf(wred_name, sizeof(wred_name), "%s-%" PRIu32,
+name, color);
 
odp_tm_wred_params_init(_params);
wred = _params_set->wred_params[color];
@@ -400,7 +402,7 @@ static int config_example_user(odp_tm_node_t cos_tm_node,
profile_set->wred_profiles[2];
tm_node_params.level= 2;
 
-   snprintf(user_name, sizeof(user_name), "Subscriber-%u", user_num);
+   snprintf(user_name, sizeof(user_name), "Subscriber-%" PRIu32, user_num);
user_tm_node = odp_tm_node_create(odp_tm_test, user_name,
  _node_params);
odp_tm_node_connect(user_tm_node, cos_tm_node);
@@ -478,8 +480,8 @@ static int config_company_node(const char *company_name)
tm_node_params.wred_profile[ODP_PACKET_RED]=
profile_set->wred_profiles[ODP_PACKET_RED];
 
-   snprintf(cos_node_name, sizeof(cos_node_name), "%s-Class-%u",
-company_name, cos_idx);
+   snprintf(cos_node_name, sizeof(cos_node_name),
+"%s-Class-%" PRIu32, company_name, cos_idx);
cos_tm_node = odp_tm_node_create(odp_tm_test, cos_node_name,
 _node_params);
odp_tm_node_connect(cos_tm_node, company_tm_node);
@@ -528,7 +530,7 @@ static int create_and_config_tm(void)
odp_tm_test = odp_tm_create("TM test", , );
err_cnt = init_profile_sets();
if (err_cnt != 0)
-   printf("%s init_profile_sets encountered %u errors\n",
+   printf("%s init_profile_sets encountered %" PRIu32 " errors\n",
   __func__, err_cnt);
 
config_company_node("TestCompany");
@@ -644,7 +646,7 @@ static int traffic_generator(uint32_t pkts_to_send)
odp_atomic_inc_u32(_pkts_into_tm);
}
 
-   printf("%s odp_tm_enq_errs=%u\n", __func__, odp_tm_enq_errs);
+   printf("%s odp_tm_enq_errs=%" PRIu32 "\n", __func__, odp_tm_enq_errs);
 
/* Wait until the main traffic mgmt worker thread is idle and has no
* outstanding events (i.e. no timers, empty work queue, etc), but
@@ -786,7 +788,8 @@ int main(int argc, char *argv[])
 
pkts_into_tm = odp_atomic_load_u32(_pkts_into_tm);
pkts_from_tm = odp_atomic_load_u32(_pkts_from_tm);
-   printf("pkts_into_tm=%u pkts_from_tm=%u\n", pkts_into_tm, pkts_from_tm);
+   printf("pkts_into_tm=%" PRIu32 " pkts_from_tm=%" PRIu32 "\n",
+  pkts_into_tm, pkts_from_tm);
 
odp_tm_stats_print(odp_tm_test);
return 0;

[lng-odp] [PATCH] configure: add option to enable/disable helper debug prints

[Nicolas Morey-Chaisemartin]

As a side effect, debug print are turned off by default

Signed-off-by: Nicolas Morey-Chaisemartin <nmo...@kalray.eu>
---
 configure.ac   | 14 ++
 helper/Makefile.am |  3 ++-
 2 files changed, 16 insertions(+), 1 deletion(-)

diff --git a/configure.ac b/configure.ac
index 3e89b0a..aa992fc 100644
--- a/configure.ac
+++ b/configure.ac
@@ -231,6 +231,19 @@ AC_ARG_ENABLE([debug-print],
 ODP_CFLAGS="$ODP_CFLAGS -DODP_DEBUG_PRINT=$ODP_DEBUG_PRINT"
 
 ##
+# Enable/disable ODPH_DEBUG_PRINT
+##
+ODPH_DEBUG_PRINT=0
+AC_ARG_ENABLE([helper-debug-print],
+[  --enable-helper-debug-printdisplay debugging information for ODP 
helpers],
+[if test "x$enableval" = "xyes"; then
+   ODPH_DEBUG_PRINT=1
+ else
+   ODPH_DEBUG_PRINT=0
+fi])
+ODPH_CFLAGS="$ODPH_CFLAGS -DODPH_DEBUG_PRINT=$ODPH_DEBUG_PRINT"
+
+##
 # Enable/disable ODP_DEBUG
 ##
 ODP_DEBUG=0
@@ -310,6 +323,7 @@ AC_SUBST([CFLAGS])
 AC_SUBST([AM_LDFLAGS])
 AC_SUBST([LDFLAGS])
 AC_SUBST([EXEEXT])
+AC_SUBST([ODPH_CFLAGS])
 
 AC_OUTPUT
 AC_MSG_RESULT([
diff --git a/helper/Makefile.am b/helper/Makefile.am
index d09d900..2ef4e8b 100644
--- a/helper/Makefile.am
+++ b/helper/Makefile.am
@@ -4,7 +4,8 @@ pkgconfigdir = $(libdir)/pkgconfig
 pkgconfig_DATA = $(top_builddir)/pkgconfig/libodphelper-linux.pc
 
 LIB   = $(top_builddir)/lib
-AM_CFLAGS  = -I$(srcdir)/include
+AM_CFLAGS  = $(ODPH_CFLAGS)
+AM_CFLAGS += -I$(srcdir)/include
 AM_CFLAGS += -I$(top_srcdir)/platform/@with_platform@/include
 AM_CFLAGS += -I$(top_srcdir)/include
 AM_CFLAGS += -I$(top_builddir)/platform/@with_platform@/include
-- 
2.10.1.4.g0ffc436

Re: [lng-odp] Traffic Manager

[Nicolas Morey-Chaisemartin]

Le 12/06/2016 à 01:27 PM, Mike Holmes a écrit :
>
>
> On 6 December 2016 at 03:59, Nicolas Morey-Chaisemartin <nmo...@kalray.eu 
> <mailto:nmo...@kalray.eu>> wrote:
>
> Hi all,
>
>
> I'm currently moving odp-mppa to monarch_lts (we ware based on 1.6 until 
> now).
> I was wondering if the implementation MUST support the TM API.
>
>
> ODP has no intentionally optional APIs becasue that impedes portability, our 
> definition is currently that the validation suite must pass.
> Ideally that implies the performance tests and examples should also work.
>
> Bring this up at the public call today, I wonder if we need to be clearer in 
> the docs so clarifying is a good thing.

Well if I implement the API but expose 0 capabilities, the API is respected and 
code can be made portable. Although it'll need some work.
I could implement a very very simple TM with 1 priority, 1 level, 1 queue, 1 
output, but it probably wouldn't be usable by code that really requires a 
working  TM anyway.

By the way, the validation test for the TM is "broken". It assumes the MAX 
value allowed by the TM are greater than the define used in the test code.

Nicolas

[lng-odp] [PATCH 4/4] example: hello: only call sched_setaffinity if available

[Nicolas Morey-Chaisemartin]

Signed-off-by: Nicolas Morey-Chaisemartin <nmo...@kalray.eu>
---
 example/hello/odp_hello.c | 21 +
 1 file changed, 13 insertions(+), 8 deletions(-)

diff --git a/example/hello/odp_hello.c b/example/hello/odp_hello.c
index 6d114ee..9cb3498 100644
--- a/example/hello/odp_hello.c
+++ b/example/hello/odp_hello.c
@@ -58,8 +58,6 @@ int main(int argc, char *argv[])
 {
odp_instance_t inst;
options_t opt;
-   pid_t pid;
-   cpu_set_t cpu_set;
int i;
 
memset(, 0, sizeof(opt));
@@ -69,14 +67,21 @@ int main(int argc, char *argv[])
if (parse_args(argc, argv, ))
return -1;
 
-   pid = getpid();
-   CPU_ZERO(_set);
-   CPU_SET(opt.cpu, _set);
+#ifdef HAVE_SCHED_SETAFFINITY
+   {
+   pid_t pid;
+   cpu_set_t cpu_set;
 
-   if (sched_setaffinity(pid, sizeof(cpu_set_t), _set)) {
-   printf("Set CPU affinity failed.\n");
-   return -1;
+   pid = getpid();
+   CPU_ZERO(_set);
+   CPU_SET(opt.cpu, _set);
+
+   if (sched_setaffinity(pid, sizeof(cpu_set_t), _set)) {
+   printf("Set CPU affinity failed.\n");
+   return -1;
+   }
}
+#endif
 
if (odp_init_global(, NULL, NULL)) {
printf("Global init failed.\n");
-- 
2.10.1.4.g0ffc436

[lng-odp] [PATCH 3/4] example: traffic_mgmt: only call glibc specific functions if available

[Nicolas Morey-Chaisemartin]

Signed-off-by: Nicolas Morey-Chaisemartin <nmo...@kalray.eu>
---
 example/traffic_mgmt/odp_traffic_mgmt.c | 38 +
 1 file changed, 29 insertions(+), 9 deletions(-)

diff --git a/example/traffic_mgmt/odp_traffic_mgmt.c 
b/example/traffic_mgmt/odp_traffic_mgmt.c
index c4f5356..55add9d 100644
--- a/example/traffic_mgmt/odp_traffic_mgmt.c
+++ b/example/traffic_mgmt/odp_traffic_mgmt.c
@@ -11,7 +11,9 @@
 #include 
 #include 
 #include 
+#ifdef HAVE_BACKTRACE
 #include 
+#endif
 #include 
 #include 
 
@@ -710,11 +712,10 @@ static int process_cmd_line_options(uint32_t argc, char 
*argv[])
return 0;
 }
 
+#ifdef HAVE_SIGACTION
 static void signal_handler(int signal)
 {
-   size_t num_stack_frames;
const char  *signal_name;
-   void  *bt_array[128];
 
switch (signal) {
case SIGILL:
@@ -731,22 +732,34 @@ static void signal_handler(int signal)
signal_name = "UNKNOWN";  break;
}
 
-   num_stack_frames = backtrace(bt_array, 100);
printf("Received signal=%u (%s) exiting.", signal, signal_name);
-   backtrace_symbols_fd(bt_array, num_stack_frames, fileno(stderr));
+#ifdef HAVE_BACKTRACE
+   {
+   size_t num_stack_frames;
+   void  *bt_array[128];
+
+   num_stack_frames = backtrace(bt_array, 100);
+   backtrace_symbols_fd(bt_array, num_stack_frames,
+fileno(stderr));
+   }
+#endif
fflush(NULL);
+#ifdef HAVE_SYNC
sync();
+#endif
abort();
 }
+#endif
 
 int main(int argc, char *argv[])
 {
-   struct sigaction signal_action;
-   struct rlimitrlimit;
uint32_t pkts_into_tm, pkts_from_tm;
odp_instance_t instance;
int rc;
 
+#ifdef HAVE_SIGACTION
+   struct sigaction signal_action;
+
memset(_action, 0, sizeof(signal_action));
signal_action.sa_handler = signal_handler;
sigfillset(_action.sa_mask);
@@ -755,10 +768,17 @@ int main(int argc, char *argv[])
sigaction(SIGSEGV, _action, NULL);
sigaction(SIGTERM, _action, NULL);
sigaction(SIGBUS,  _action, NULL);
+#endif
+
+#if defined(HAVE_GETRLIMIT) && defined(HAVE_SETRLIMIT)
+   {
+   struct rlimitrlimit;
 
-   getrlimit(RLIMIT_CORE, );
-   rlimit.rlim_cur = rlimit.rlim_max;
-   setrlimit(RLIMIT_CORE, );
+   getrlimit(RLIMIT_CORE, );
+   rlimit.rlim_cur = rlimit.rlim_max;
+   setrlimit(RLIMIT_CORE, );
+   }
+#endif
 
rc = odp_init_global(, _INIT_PARAMS, NULL);
if (rc != 0) {
-- 
2.10.1.4.g0ffc436

[lng-odp] [PATCH 1/4] configure: check for non standard function availability

[Nicolas Morey-Chaisemartin]

Add check for sync, getrusage, getrlimit, setrlimit, sigaction,
 sched_setaffinity

Signed-off-by: Nicolas Morey-Chaisemartin <nmo...@kalray.eu>
---
 configure.ac | 5 -
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/configure.ac b/configure.ac
index 3e89b0a..6e5125e 100644
--- a/configure.ac
+++ b/configure.ac
@@ -62,7 +62,10 @@ AM_PROG_LIBTOOL
 # Checks for library functions.
 AC_FUNC_MALLOC
 AC_FUNC_MMAP
-AC_CHECK_FUNCS([bzero clock_gettime gethostbyname getpagesize gettimeofday 
memset munmap socket strchr strerror strrchr strstr strtoull])
+AC_CHECK_FUNCS([backtrace bzero clock_gettime gethostbyname getpagesize \
+   getrlimit getrusage gettimeofday memset munmap \
+   sched_setaffinity setrlimit sigaction socket 
strchr strerror \
+   strrchr strstr strtoull sync])
 
 # Checks for header files.
 AC_HEADER_RESOLV
-- 
2.10.1.4.g0ffc436

[lng-odp] [PATCH 2/4] performance: crypto: only call getrusage if available

[Nicolas Morey-Chaisemartin]

Signed-off-by: Nicolas Morey-Chaisemartin <nmo...@kalray.eu>
---
 test/common_plat/performance/odp_crypto.c | 5 +
 1 file changed, 5 insertions(+)

diff --git a/test/common_plat/performance/odp_crypto.c 
b/test/common_plat/performance/odp_crypto.c
index 49a9f4b..5d74072 100644
--- a/test/common_plat/performance/odp_crypto.c
+++ b/test/common_plat/performance/odp_crypto.c
@@ -271,8 +271,13 @@ static void
 fill_time_record(time_record_t *rec)
 {
gettimeofday(>tv, NULL);
+#ifdef HAVE_GETRUSAGE
getrusage(RUSAGE_SELF, >ru_self);
getrusage(RUSAGE_THREAD, >ru_thread);
+#else
+   memset(>ru_self, 0, sizeof(struct rusage));
+   memset(>ru_thread, 0, sizeof(struct rusage));
+#endif
 }
 
 /**
-- 
2.10.1.4.g0ffc436

[lng-odp] [PATCH 0/4] Support non glibc in examples

[Nicolas Morey-Chaisemartin]

Some common examples use glibc/Linux specific syscall.
This patch series add check in configure.ac for these call and disables them if 
not present.

You'll notice some subblocks have been added. They are there to avoid 
compilation warnings/errors about unused variables when these calls are not 
available

This series should apply (and be applied) on the monarch_lts branch too.

Nicolas Morey-Chaisemartin (4):
  configure: check for non standard function availability
  performance: crypto: only call getrusage if available
  example: traffic_mgmt: only call glibc specific functions if available
  example: hello: only call sched_setaffinity if available

 configure.ac  |  5 +++-
 example/hello/odp_hello.c | 21 ++---
 example/traffic_mgmt/odp_traffic_mgmt.c   | 38 +++
 test/common_plat/performance/odp_crypto.c |  5 
 4 files changed, 51 insertions(+), 18 deletions(-)

[lng-odp] Traffic Manager

[Nicolas Morey-Chaisemartin]

Hi all,


I'm currently moving odp-mppa to monarch_lts (we ware based on 1.6 until now).
I was wondering if the implementation MUST support the TM API.

The traffic_mngr.h is a bit hazy on that point. It looks to me as if 
odp_tm_capabilities returns 0, it announces that the TM API is not available.
All other calls should probably return error code to complete that.

TM is not something that we are really interested in for the moment, and as it 
pull a lot (a very big lot) of code and feature, it'll take some time to 
implement. In the meantime, i would still like to be API compliant.


Nicolas

Re: [lng-odp] [MONARCH PATCH 1/2] validation: pktio: use PRIu32 to print uint32_t values

[Nicolas Morey-Chaisemartin]

I didn't submit there for master because I cannot build odp-mppa with master 
yet (trying to merge monarch first).
And I'm pretty sure there'll be new patches like this to apply on master anyway.

Nicolas

Le 12/05/2016 à 08:21 PM, Mike Holmes a écrit :
> This series also applies to master with 3 way, it should go there first I
> think
>
> On 5 December 2016 at 11:33, Nicolas Morey-Chaisemartin <nmo...@kalray.eu>
> wrote:
>
>> Signed-off-by: Nicolas Morey-Chaisemartin <nmo...@kalray.eu>
>>
> Reviewed-by: Mike Holmes <mike.hol...@linaro.org>
>
>
>> ---
>>  test/validation/pktio/pktio.c | 10 +++---
>>  1 file changed, 7 insertions(+), 3 deletions(-)
>>
>> diff --git a/test/validation/pktio/pktio.c b/test/validation/pktio/pktio.c
>> index d53a419..4e93e9b 100644
>> --- a/test/validation/pktio/pktio.c
>> +++ b/test/validation/pktio/pktio.c
>> @@ -207,7 +207,8 @@ static uint32_t pktio_pkt_seq(odp_packet_t pkt)
>> }
>>
>> if (head.magic != TEST_SEQ_MAGIC) {
>> -   fprintf(stderr, "error: header magic invalid %u\n",
>> head.magic);
>> +   fprintf(stderr, "error: header magic invalid %" PRIu32
>> "\n",
>> +   head.magic);
>> return TEST_SEQ_INVALID;
>> }
>>
>> @@ -223,11 +224,14 @@ static uint32_t pktio_pkt_seq(odp_packet_t pkt)
>> seq = head.seq;
>> CU_ASSERT(seq != TEST_SEQ_INVALID);
>> } else {
>> -   fprintf(stderr, "error: tail magic invalid %u\n",
>> +   fprintf(stderr,
>> +   "error: tail magic invalid %" PRIu32 "\n",
>> tail.magic);
>> }
>> } else {
>> -   fprintf(stderr, "error: packet length invalid: %u (%u)\n",
>> +   fprintf(stderr,
>> +   "error: packet length invalid: "
>> +   "%" PRIu32 "(%" PRIu32 ")\n",
>> odp_packet_len(pkt), packet_len);
>> }
>>
>> --
>> 2.10.1.4.g0ffc436
>>
>>
>>
>

Re: [lng-odp] [MONARCH PATCH 0/5] Make lidodp/libodp helper name platform specific

[Nicolas Morey-Chaisemartin]

Le 12/05/2016 à 09:02 PM, Mike Holmes a écrit :
> I like this patch in principle, but it has to go into master first.
>
> I think that if you dont set ODP_LIB_NAME and you get something that is 
> expected to ab ABI compatible is good, if I add a platform name I would 
> expect it to be less portable.
> I like that odp-dpdk can reuse more of Linux generic infrastructure making it 
> even cheaper to support odp-dpdk and also possibly newer flavors such as 
> odp-cloud or other derivatives that make a spin on odp-linux for a particular 
> use case.
>
> Mike
>

Good. that's the goal of a lot of my patches ;)
The only things I'm not relly happy with in this patch is the name 
"ODP_LIB_NAME".
Wouldn't ODP_LIB_FLAVOR makes a little more sense ?

Do you think I should remost this series on master and merge it with the  
"[lng-odp] [PATCH 0/4] Make configure.ac generic" series?
Although it changes different things, the goal is one and the same: make it 
easier to add/build platform reusing all the build/test infrastructure of 
linux-generic.

Nicolas

> On 2 December 2016 at 09:09, Nicolas Morey-Chaisemartin <nmo...@kalray.eu 
> <mailto:nmo...@kalray.eu>> wrote:
>
>
>
> Le 12/02/2016 à 02:52 PM, Mike Holmes a écrit :
>>
>>
>> On 2 December 2016 at 08:50, Nicolas Morey-Chaisemartin 
>> <nmo...@kalray.eu <mailto:nmo...@kalray.eu>> wrote:
>>
>>
>>
>> Le 12/02/2016 à 02:45 PM, Maxim Uvarov a écrit :
>> > On 12/02/16 16:34, Nicolas Morey-Chaisemartin wrote:
>> >>
>> >> Le 12/02/2016 à 02:30 PM, Maxim Uvarov a écrit :
>> >>> That needs to go to master first, than if needed back-ported to 
>> Monarch.
>> >> Ok.
>> >>
>> >>> Nicolas, if you name library differently how will application 
>> know how
>> >>> to link with it?
>> >> 1) We provide a template makefile for all ODP apps.
>> >> 2) We don't have ABI compat with multiple libs as we build for 
>> our architecture, running our own OS with no dynamic library support
>> >>
>> >> We could use the same name as you do, but as we don't have any 
>> Linux running underneath, it doesn't make much sense to me.
>> >>
>> >> BTW, why did you rename those libraries?
>> >
>> > word 'generic' was confusing for some people.
>> >
>> > Maxim.
>>
>> Renaming the platform name is one thing. But why did libodp had to 
>> change to libodp-linux ?
>>
>>
>> It is not ABI compatible was the rational, we are workign on that.
>>
>> We need libodp to be ABI compatible and then libodpKalray etc to be non 
>> abi 
>>
>> Mike
>>  
>>
>
> So if I understood well:
> libodp => libodp-linux because of the static inlines/struct in the 
> linux-generic potentially break ABI compatibility with another implementation.
> libodp is reserved for someone without any platform specific 
> inlines/structs exported.
>
> I still think my series makes sense here. The only change i'd make is to 
> move the "-" before the suffix to ODP_LIB_NAME too.
> This ways a full ABI compatible platform sets ODP_LIB_NAME="" => it has 
> libodp.(so|a)
> Other platform add their suffix depending on who they are compatible with 
> (-linux, -mppa, etc.)
>
>
>
>
> -- 
> Mike Holmes
> Program Manager - Linaro Networking Group
> Linaro.org <http://www.linaro.org/>* **│ *Open source software for ARM SoCs
> "Work should be fun and collaborative, the rest follows"
>

[lng-odp] [MONARCH PATCH 2/2] validation: traffic_mngr: use PRI macro to print uint*

[Nicolas Morey-Chaisemartin]

Signed-off-by: Nicolas Morey-Chaisemartin <nmo...@kalray.eu>
---
 test/validation/traffic_mngr/traffic_mngr.c | 116 +++-
 1 file changed, 64 insertions(+), 52 deletions(-)

diff --git a/test/validation/traffic_mngr/traffic_mngr.c 
b/test/validation/traffic_mngr/traffic_mngr.c
index b857800..b41404e 100644
--- a/test/validation/traffic_mngr/traffic_mngr.c
+++ b/test/validation/traffic_mngr/traffic_mngr.c
@@ -956,8 +956,9 @@ static void dump_rcvd_pkts(uint32_t first_rcv_idx, uint32_t 
last_rcv_idx)
if (rcv_pkt_desc->matched)
xmt_idx = rcv_pkt_desc->xmt_pkt_desc->xmt_idx;
 
-   printf("rcv_idx=%u odp_pkt=0x%" PRIX64 " xmt_idx=%d "
-  "pkt_class=%u is_ipv4=%u unique_id=0x%X (rc=%d)\n",
+   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);
}
@@ -1376,15 +1377,15 @@ static tm_node_desc_t *create_tm_node(odp_tm_t
odp_tm,
node_params.max_fanin = FANIN_RATIO;
node_params.level = level;
if (parent_node_desc == NULL)
-   snprintf(node_name, sizeof(node_name), "node_%u",
+   snprintf(node_name, sizeof(node_name), "node_%" PRIu32,
 node_idx + 1);
else
-   snprintf(node_name, sizeof(node_name), "%s_%u",
+   snprintf(node_name, sizeof(node_name), "%s_%" PRIu32,
 parent_node_desc->node_name, node_idx + 1);
 
tm_node = odp_tm_node_create(odp_tm, node_name, _params);
if (tm_node == ODP_TM_INVALID) {
-   LOG_ERR("odp_tm_node_create() failed @ level=%u\n",
+   LOG_ERR("odp_tm_node_create() failed @ level=%" PRIu32 "\n",
level);
return NULL;
}
@@ -1397,7 +1398,7 @@ static tm_node_desc_t *create_tm_node(odp_tm_t
odp_tm,
 
rc = odp_tm_node_connect(tm_node, parent_node);
if (rc != 0) {
-   LOG_ERR("odp_tm_node_connect() failed @ level=%u\n",
+   LOG_ERR("odp_tm_node_connect() failed @ level=%" PRIu32 "\n",
level);
odp_tm_node_destroy(tm_node);
return NULL;
@@ -1431,8 +1432,8 @@ static tm_node_desc_t *create_tm_node(odp_tm_t
odp_tm,
rc = create_tm_queue(odp_tm, tm_node, node_idx, queue_desc,
 priority);
if (rc != 0) {
-   LOG_ERR("create_tm_queue() failed @ level=%u\n",
-   level);
+   LOG_ERR("create_tm_queue() failed @ "
+   "level=%" PRIu32 "\n", level);
while (priority > 0)
(void)destroy_tm_queue
(queue_desc->tm_queues[--priority]);
@@ -1457,7 +1458,7 @@ static tm_node_desc_t *create_tm_subtree(odp_tm_t
odp_tm,
node_desc = create_tm_node(odp_tm, level, num_levels,
   node_idx, parent_node);
if (node_desc == NULL) {
-   LOG_ERR("create_tm_node() failed @ level=%u\n", level);
+   LOG_ERR("create_tm_node() failed @ level=%" PRIu32 "\n", level);
return NULL;
}
 
@@ -1467,8 +1468,8 @@ static tm_node_desc_t *create_tm_subtree(odp_tm_t
odp_tm,
   num_levels, child_idx,
   node_desc);
if (child_desc == NULL) {
-   LOG_ERR("create_tm_subtree failed level=%u\n",
-   level);
+   LOG_ERR("create_tm_subtree failed "
+   "level=%" PRIu32 "\n", level);
 
return NULL;
}
@@ -1629,7 +1630,8 @@ static int create_tm_system(void)
egress.egress_kind = ODP_TM_EGRESS_PKT_IO;
egress.pktio  = xmt_pktio;
 
-   snprintf(tm_name, sizeof(tm_name), "TM_system_%u", num_odp_tm_systems);
+   snprintf(tm_name, sizeof(tm_name), "TM_system_%" PRIu32,
+num_odp_tm_systems);
odp_tm = odp_tm_create(tm_name, , );
if (odp_tm == ODP_TM_INVALID) {
LOG_ERR("odp_tm_create()

[lng-odp] [MONARCH PATCH 1/2] validation: pktio: use PRIu32 to print uint32_t values

[Nicolas Morey-Chaisemartin]

Signed-off-by: Nicolas Morey-Chaisemartin <nmo...@kalray.eu>
---
 test/validation/pktio/pktio.c | 10 +++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/test/validation/pktio/pktio.c b/test/validation/pktio/pktio.c
index d53a419..4e93e9b 100644
--- a/test/validation/pktio/pktio.c
+++ b/test/validation/pktio/pktio.c
@@ -207,7 +207,8 @@ static uint32_t pktio_pkt_seq(odp_packet_t pkt)
}
 
if (head.magic != TEST_SEQ_MAGIC) {
-   fprintf(stderr, "error: header magic invalid %u\n", head.magic);
+   fprintf(stderr, "error: header magic invalid %" PRIu32 "\n",
+   head.magic);
return TEST_SEQ_INVALID;
}
 
@@ -223,11 +224,14 @@ static uint32_t pktio_pkt_seq(odp_packet_t pkt)
seq = head.seq;
CU_ASSERT(seq != TEST_SEQ_INVALID);
} else {
-   fprintf(stderr, "error: tail magic invalid %u\n",
+   fprintf(stderr,
+   "error: tail magic invalid %" PRIu32 "\n",
tail.magic);
}
} else {
-   fprintf(stderr, "error: packet length invalid: %u (%u)\n",
+   fprintf(stderr,
+   "error: packet length invalid: "
+   "%" PRIu32 "(%" PRIu32 ")\n",
odp_packet_len(pkt), packet_len);
}
 
-- 
2.10.1.4.g0ffc436

Re: [lng-odp] [API-NEXT 1/2] api: init: add initializer function for odp_platform_init_t

[Nicolas Morey-Chaisemartin]

Le 12/02/2016 à 03:42 PM, Maxim Uvarov a écrit :
> if you provide NULL is has to be initialized to default, do not see
> reason for separate api for that.
>
> Maxim.
Because if you want to provide only 1 settings among many, you needs to make 
sure all other are set to defaults

Nicolas

>
> On 12/02/16 13:25, Nicolas Morey-Chaisemartin wrote:
>> Signed-off-by: Nicolas Morey-Chaisemartin <nmo...@kalray.eu>
>> ---
>>  include/odp/api/spec/init.h   | 8 
>>  platform/linux-generic/odp_init.c | 5 +
>>  2 files changed, 13 insertions(+)
>>
>> diff --git a/include/odp/api/spec/init.h b/include/odp/api/spec/init.h
>> index 154cdf8..0600691 100644
>> --- a/include/odp/api/spec/init.h
>> +++ b/include/odp/api/spec/init.h
>> @@ -165,6 +165,14 @@ typedef struct odp_init_t {
>>   * passing any required platform specific data.
>>   */
>>  
>> +/**
>> + * Initialize platform init structure
>> + *
>> + * Initialize an odp_platform_init_t to its default values for all fields
>> + *
>> + * @param platform_params Address of the odp_platform_init_t to be 
>> initialized
>> + */
>> +void odp_platform_init_init(odp_platform_init_t *platform_params);
>>  
>>  /**
>>   * Global ODP initialization
>> diff --git a/platform/linux-generic/odp_init.c 
>> b/platform/linux-generic/odp_init.c
>> index d40a83c..d74ab4c 100644
>> --- a/platform/linux-generic/odp_init.c
>> +++ b/platform/linux-generic/odp_init.c
>> @@ -415,3 +415,8 @@ int _odp_term_local(enum init_stage stage)
>>  
>>  return rc;
>>  }
>> +
>> +void odp_platform_init_init(odp_platform_init_t *platform_params)
>> +{
>> +platform_params->ipc_ns = 0;
>> +}
>>

Re: [lng-odp] [MONARCH PATCH 0/5] Make lidodp/libodp helper name platform specific

[Nicolas Morey-Chaisemartin]

Le 12/02/2016 à 02:52 PM, Mike Holmes a écrit :
>
>
> On 2 December 2016 at 08:50, Nicolas Morey-Chaisemartin <nmo...@kalray.eu 
> <mailto:nmo...@kalray.eu>> wrote:
>
>
>
> Le 12/02/2016 à 02:45 PM, Maxim Uvarov a écrit :
> > On 12/02/16 16:34, Nicolas Morey-Chaisemartin wrote:
> >>
> >> Le 12/02/2016 à 02:30 PM, Maxim Uvarov a écrit :
> >>> That needs to go to master first, than if needed back-ported to 
> Monarch.
> >> Ok.
> >>
> >>> Nicolas, if you name library differently how will application know how
> >>> to link with it?
> >> 1) We provide a template makefile for all ODP apps.
> >> 2) We don't have ABI compat with multiple libs as we build for our 
> architecture, running our own OS with no dynamic library support
> >>
> >> We could use the same name as you do, but as we don't have any Linux 
> running underneath, it doesn't make much sense to me.
> >>
> >> BTW, why did you rename those libraries?
> >
> > word 'generic' was confusing for some people.
> >
> > Maxim.
>
> Renaming the platform name is one thing. But why did libodp had to change 
> to libodp-linux ?
>
>
> It is not ABI compatible was the rational, we are workign on that.
>
> We need libodp to be ABI compatible and then libodpKalray etc to be non abi 
>
> Mike
>  
>

So if I understood well:
libodp => libodp-linux because of the static inlines/struct in the 
linux-generic potentially break ABI compatibility with another implementation.
libodp is reserved for someone without any platform specific inlines/structs 
exported.

I still think my series makes sense here. The only change i'd make is to move 
the "-" before the suffix to ODP_LIB_NAME too.
This ways a full ABI compatible platform sets ODP_LIB_NAME="" => it has 
libodp.(so|a)
Other platform add their suffix depending on who they are compatible with 
(-linux, -mppa, etc.)

Re: [lng-odp] [MONARCH PATCH 0/5] Make lidodp/libodp helper name platform specific

[Nicolas Morey-Chaisemartin]

Le 12/02/2016 à 02:45 PM, Maxim Uvarov a écrit :
> On 12/02/16 16:34, Nicolas Morey-Chaisemartin wrote:
>>
>> Le 12/02/2016 à 02:30 PM, Maxim Uvarov a écrit :
>>> That needs to go to master first, than if needed back-ported to Monarch.
>> Ok.
>>
>>> Nicolas, if you name library differently how will application know how
>>> to link with it?
>> 1) We provide a template makefile for all ODP apps.
>> 2) We don't have ABI compat with multiple libs as we build for our 
>> architecture, running our own OS with no dynamic library support
>>
>> We could use the same name as you do, but as we don't have any Linux running 
>> underneath, it doesn't make much sense to me.
>>
>> BTW, why did you rename those libraries?
>
> word 'generic' was confusing for some people.
>
> Maxim.

Renaming the platform name is one thing. But why did libodp had to change to 
libodp-linux ?

Nicolas

Re: [lng-odp] [MONARCH PATCH 0/5] Make lidodp/libodp helper name platform specific

[Nicolas Morey-Chaisemartin]

Le 12/02/2016 à 02:30 PM, Maxim Uvarov a écrit :
> That needs to go to master first, than if needed back-ported to Monarch.

Ok.

>
> Nicolas, if you name library differently how will application know how
> to link with it?

1) We provide a template makefile for all ODP apps.
2) We don't have ABI compat with multiple libs as we build for our 
architecture, running our own OS with no dynamic library support

We could use the same name as you do, but as we don't have any Linux running 
underneath, it doesn't make much sense to me.

BTW, why did you rename those libraries?
I've been away from the ML for a while and didn't catch up the few thousands of 
mail I missed :)

Nicolas

>
> I think that we can stay with fixed name for ABI compat mode and let
> platforms to change it in non ABI compat mode. Assuming that generic
> applications use only ABI compat .so.
>
> Maxim.
>
> On 12/02/16 15:16, Nicolas Morey-Chaisemartin wrote:
>> The lib suffix "linux" (libodp-linux, libodphelper-linux) is harcoded pretty 
>> much everywhere in the build system.
>> The goal of this series is to allow each platform to define its library 
>> suffix so they don't have to touch the generic files but simply the platform 
>> specific files.
>>
>> Nicolas Morey-Chaisemartin (5):
>>   m4: platform exports ODP_LIB_NAME
>>   platform: move odp-linux specific rules to linux-generic
>>   test: use ODP_LIB_NAME to generate libodp name
>>   helper: use ODP_LIB_NAME to generate libodp name
>>   example: use ODP_LIB_NAME to generate libodp name
>>
>>  configure.ac   | 3 +--
>>  example/Makefile.inc   | 2 +-
>>  helper/test/Makefile.am| 6 +++---
>>  platform/Makefile.inc  | 4 
>>  platform/linux-generic/Makefile.am | 6 ++
>>  platform/linux-generic/m4/configure.m4 | 6 +-
>>  test/Makefile.inc  | 2 +-
>>  test/validation/Makefile.inc   | 2 +-
>>  8 files changed, 18 insertions(+), 13 deletions(-)
>>

[lng-odp] [MONARCH PATCH 5/5] example: use ODP_LIB_NAME to generate libodp name

[Nicolas Morey-Chaisemartin]

Signed-off-by: Nicolas Morey-Chaisemartin <nmo...@kalray.eu>
---
 example/Makefile.inc | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/example/Makefile.inc b/example/Makefile.inc
index 70ba2c0..b14cd69 100644
--- a/example/Makefile.inc
+++ b/example/Makefile.inc
@@ -1,6 +1,6 @@
 include $(top_srcdir)/platform/@with_platform@/Makefile.inc
 LIB   = $(top_builddir)/lib
-LDADD = $(LIB)/libodp-linux.la $(LIB)/libodphelper-linux.la
+LDADD = $(LIB)/libodp-@ODP_LIB_NAME@.la $(LIB)/libodphelper-@ODP_LIB_NAME@.la
 AM_CFLAGS += \
-I$(srcdir) \
-I$(top_srcdir)/example \
-- 
2.10.1.4.g0ffc436

[lng-odp] [MONARCH PATCH 4/5] helper: use ODP_LIB_NAME to generate libodp name

[Nicolas Morey-Chaisemartin]

Signed-off-by: Nicolas Morey-Chaisemartin <nmo...@kalray.eu>
---
 helper/test/Makefile.am | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/helper/test/Makefile.am b/helper/test/Makefile.am
index 545db73..8073283 100644
--- a/helper/test/Makefile.am
+++ b/helper/test/Makefile.am
@@ -28,10 +28,10 @@ EXTRA_DIST = odpthreads_as_processes odpthreads_as_pthreads
 
 dist_chksum_SOURCES = chksum.c
 dist_odpthreads_SOURCES = odpthreads.c
-odpthreads_LDADD = $(LIB)/libodphelper-linux.la $(LIB)/libodp-linux.la
+odpthreads_LDADD = $(LIB)/libodphelper-@ODP_LIB_NAME@.la 
$(LIB)/libodp-@ODP_LIB_NAME@.la
 dist_thread_SOURCES = thread.c
-thread_LDADD = $(LIB)/libodphelper-linux.la $(LIB)/libodp-linux.la
+thread_LDADD = $(LIB)/libodphelper-@ODP_LIB_NAME@.la 
$(LIB)/libodp-@ODP_LIB_NAME@.la
 dist_process_SOURCES = process.c
 dist_parse_SOURCES = parse.c
-process_LDADD = $(LIB)/libodphelper-linux.la $(LIB)/libodp-linux.la
+process_LDADD = $(LIB)/libodphelper-@ODP_LIB_NAME@.la 
$(LIB)/libodp-@ODP_LIB_NAME@.la
 dist_table_SOURCES = table.c
-- 
2.10.1.4.g0ffc436

[lng-odp] [MONARCH PATCH 3/5] test: use ODP_LIB_NAME to generate libodp name

[Nicolas Morey-Chaisemartin]

Signed-off-by: Nicolas Morey-Chaisemartin <nmo...@kalray.eu>
---
 test/Makefile.inc| 2 +-
 test/validation/Makefile.inc | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/test/Makefile.inc b/test/Makefile.inc
index 5def923..bb1f40f 100644
--- a/test/Makefile.inc
+++ b/test/Makefile.inc
@@ -4,7 +4,7 @@ LIB   = $(top_builddir)/lib
 #in the following line, the libs using the symbols should come before
 #the libs containing them! The includer is given a chance to add things
 #before libodp by setting PRE_LDADD before the inclusion.
-LDADD = $(PRE_LDADD) $(LIB)/libodphelper-linux.la $(LIB)/libodp-linux.la
+LDADD = $(PRE_LDADD) $(LIB)/libodphelper-@ODP_LIB_NAME@.la 
$(LIB)/libodp-@ODP_LIB_NAME@.la
 
 INCFLAGS = -I$(top_srcdir)/test \
-I$(top_srcdir)/platform/@with_platform@/include \
diff --git a/test/validation/Makefile.inc b/test/validation/Makefile.inc
index 9f36f9d..d0e9714 100644
--- a/test/validation/Makefile.inc
+++ b/test/validation/Makefile.inc
@@ -13,4 +13,4 @@ AM_LDFLAGS += -static
 LIBCUNIT_COMMON = $(COMMON_DIR)/libcunit_common.la
 LIBCPUMASK_COMMON = $(COMMON_DIR)/libcpumask_common.la
 LIBTHRMASK_COMMON = $(COMMON_DIR)/libthrmask_common.la
-LIBODP = $(LIB)/libodphelper-linux.la $(LIB)/libodp-linux.la
+LIBODP = $(LIB)/libodphelper-@ODP_LIB_NAME@.la $(LIB)/libodp-@ODP_LIB_NAME@.la
-- 
2.10.1.4.g0ffc436

[lng-odp] [MONARCH PATCH 2/5] platform: move odp-linux specific rules to linux-generic

[Nicolas Morey-Chaisemartin]

Signed-off-by: Nicolas Morey-Chaisemartin <nmo...@kalray.eu>
---
 configure.ac   | 2 --
 platform/Makefile.inc  | 4 
 platform/linux-generic/Makefile.am | 6 ++
 platform/linux-generic/m4/configure.m4 | 4 +++-
 4 files changed, 9 insertions(+), 7 deletions(-)

diff --git a/configure.ac b/configure.ac
index 091054e..6e14855 100644
--- a/configure.ac
+++ b/configure.ac
@@ -252,8 +252,6 @@ AM_CXXFLAGS="-std=c++11"
 AC_CONFIG_FILES([Makefile
 helper/Makefile
 helper/test/Makefile
-pkgconfig/libodp-linux.pc
-pkgconfig/libodphelper-linux.pc
 scripts/Makefile
 ])
 
diff --git a/platform/Makefile.inc b/platform/Makefile.inc
index 053bd12..9c5a4d6 100644
--- a/platform/Makefile.inc
+++ b/platform/Makefile.inc
@@ -1,12 +1,8 @@
 LIB   = $(top_builddir)/lib
 
 pkgconfigdir = $(libdir)/pkgconfig
-pkgconfig_DATA = $(top_builddir)/pkgconfig/libodp-linux.pc
-
-.PHONY: pkgconfig/libodp-linux.pc
 
 VPATH = $(srcdir) $(builddir)
-lib_LTLIBRARIES = $(LIB)/libodp-linux.la
 
 AM_LDFLAGS += -version-number '$(ODP_LIBSO_VERSION)'
 
diff --git a/platform/linux-generic/Makefile.am 
b/platform/linux-generic/Makefile.am
index c8fd8cb..dafdfd8 100644
--- a/platform/linux-generic/Makefile.am
+++ b/platform/linux-generic/Makefile.am
@@ -4,6 +4,12 @@
 include $(top_srcdir)/platform/Makefile.inc
 include $(top_srcdir)/platform/@with_platform@/Makefile.inc
 
+pkgconfig_DATA = $(top_builddir)/pkgconfig/libodp-linux.pc
+
+.PHONY: pkgconfig/libodp-linux.pc
+
+lib_LTLIBRARIES = $(LIB)/libodp-linux.la
+
 AM_CFLAGS +=  -I$(srcdir)/include
 AM_CFLAGS +=  -I$(top_srcdir)/include
 
diff --git a/platform/linux-generic/m4/configure.m4 
b/platform/linux-generic/m4/configure.m4
index acfa68d..c8be275 100644
--- a/platform/linux-generic/m4/configure.m4
+++ b/platform/linux-generic/m4/configure.m4
@@ -36,6 +36,8 @@ m4_include([platform/linux-generic/m4/odp_dpdk.m4])
 m4_include([platform/linux-generic/m4/odp_ipc.m4])
 m4_include([platform/linux-generic/m4/odp_schedule.m4])
 
-AC_CONFIG_FILES([platform/linux-generic/Makefile])
+AC_CONFIG_FILES([platform/linux-generic/Makefile
+pkgconfig/libodp-linux.pc
+pkgconfig/libodphelper-linux.pc])
 
 ODP_LIB_NAME=linux
-- 
2.10.1.4.g0ffc436

[lng-odp] [MONARCH PATCH 1/5] m4: platform exports ODP_LIB_NAME

[Nicolas Morey-Chaisemartin]

This is the suffix to be used to compute the name of ODP
 and ODP helper libraries.

Signed-off-by: Nicolas Morey-Chaisemartin <nmo...@kalray.eu>
---
 configure.ac   | 1 +
 platform/linux-generic/m4/configure.m4 | 2 ++
 2 files changed, 3 insertions(+)

diff --git a/configure.ac b/configure.ac
index 48fe0be..091054e 100644
--- a/configure.ac
+++ b/configure.ac
@@ -270,6 +270,7 @@ AC_SUBST([CFLAGS])
 AC_SUBST([AM_LDFLAGS])
 AC_SUBST([LDFLAGS])
 AC_SUBST([EXEEXT])
+AC_SUBST([ODP_LIB_NAME])
 
 AC_OUTPUT
 AC_MSG_RESULT([
diff --git a/platform/linux-generic/m4/configure.m4 
b/platform/linux-generic/m4/configure.m4
index 1b1b883..acfa68d 100644
--- a/platform/linux-generic/m4/configure.m4
+++ b/platform/linux-generic/m4/configure.m4
@@ -37,3 +37,5 @@ m4_include([platform/linux-generic/m4/odp_ipc.m4])
 m4_include([platform/linux-generic/m4/odp_schedule.m4])
 
 AC_CONFIG_FILES([platform/linux-generic/Makefile])
+
+ODP_LIB_NAME=linux
-- 
2.10.1.4.g0ffc436

[lng-odp] [MONARCH PATCH 0/5] Make lidodp/libodp helper name platform specific

[Nicolas Morey-Chaisemartin]

The lib suffix "linux" (libodp-linux, libodphelper-linux) is harcoded pretty 
much everywhere in the build system.
The goal of this series is to allow each platform to define its library suffix 
so they don't have to touch the generic files but simply the platform specific 
files.

Nicolas Morey-Chaisemartin (5):
  m4: platform exports ODP_LIB_NAME
  platform: move odp-linux specific rules to linux-generic
  test: use ODP_LIB_NAME to generate libodp name
  helper: use ODP_LIB_NAME to generate libodp name
  example: use ODP_LIB_NAME to generate libodp name

 configure.ac   | 3 +--
 example/Makefile.inc   | 2 +-
 helper/test/Makefile.am| 6 +++---
 platform/Makefile.inc  | 4 
 platform/linux-generic/Makefile.am | 6 ++
 platform/linux-generic/m4/configure.m4 | 6 +-
 test/Makefile.inc  | 2 +-
 test/validation/Makefile.inc   | 2 +-
 8 files changed, 18 insertions(+), 13 deletions(-)

[lng-odp] [API-NEXT 2/2] validation: init: add test for init with platform parameters

[Nicolas Morey-Chaisemartin]

Signed-off-by: Nicolas Morey-Chaisemartin <nmo...@kalray.eu>
---
 test/common_plat/validation/api/init/init.c | 17 +
 test/common_plat/validation/api/init/init.h |  1 +
 2 files changed, 18 insertions(+)

diff --git a/test/common_plat/validation/api/init/init.c 
b/test/common_plat/validation/api/init/init.c
index 61055fa..0dc4d97 100644
--- a/test/common_plat/validation/api/init/init.c
+++ b/test/common_plat/validation/api/init/init.c
@@ -155,8 +155,25 @@ void init_test_odp_init_global(void)
CU_ASSERT(status == 0);
 }
 
+/* test normal ODP global init with platform params*/
+void init_test_odp_init_global_with_platform_params(void)
+{
+   int status;
+   odp_instance_t instance;
+   odp_platform_init_t platform_params;
+
+   odp_platform_init_init(_params);
+
+   status = odp_init_global(, NULL, _params);
+   CU_ASSERT_FATAL(status == 0);
+
+   status = odp_term_global(instance);
+   CU_ASSERT(status == 0);
+}
+
 odp_testinfo_t init_suite_ok[] = {
ODP_TEST_INFO(init_test_odp_init_global),
+   ODP_TEST_INFO(init_test_odp_init_global_with_platform_params),
ODP_TEST_INFO_NULL,
 };
 
diff --git a/test/common_plat/validation/api/init/init.h 
b/test/common_plat/validation/api/init/init.h
index cad9cf9..5fd789b 100644
--- a/test/common_plat/validation/api/init/init.h
+++ b/test/common_plat/validation/api/init/init.h
@@ -13,6 +13,7 @@
 void init_test_odp_init_global_replace_abort(void);
 void init_test_odp_init_global_replace_log(void);
 void init_test_odp_init_global(void);
+void init_test_odp_init_global_with_platform_params(void);
 
 /* test arrays: */
 extern odp_testinfo_t init_suite_abort[];
-- 
2.10.1.4.g0ffc436

[lng-odp] [API-NEXT 1/2] api: init: add initializer function for odp_platform_init_t

[Nicolas Morey-Chaisemartin]

Signed-off-by: Nicolas Morey-Chaisemartin <nmo...@kalray.eu>
---
 include/odp/api/spec/init.h   | 8 
 platform/linux-generic/odp_init.c | 5 +
 2 files changed, 13 insertions(+)

diff --git a/include/odp/api/spec/init.h b/include/odp/api/spec/init.h
index 154cdf8..0600691 100644
--- a/include/odp/api/spec/init.h
+++ b/include/odp/api/spec/init.h
@@ -165,6 +165,14 @@ typedef struct odp_init_t {
  * passing any required platform specific data.
  */
 
+/**
+ * Initialize platform init structure
+ *
+ * Initialize an odp_platform_init_t to its default values for all fields
+ *
+ * @param platform_params Address of the odp_platform_init_t to be initialized
+ */
+void odp_platform_init_init(odp_platform_init_t *platform_params);
 
 /**
  * Global ODP initialization
diff --git a/platform/linux-generic/odp_init.c 
b/platform/linux-generic/odp_init.c
index d40a83c..d74ab4c 100644
--- a/platform/linux-generic/odp_init.c
+++ b/platform/linux-generic/odp_init.c
@@ -415,3 +415,8 @@ int _odp_term_local(enum init_stage stage)
 
return rc;
 }
+
+void odp_platform_init_init(odp_platform_init_t *platform_params)
+{
+   platform_params->ipc_ns = 0;
+}
-- 
2.10.1.4.g0ffc436

Re: [lng-odp] [PATCH 1/4] configure.ac: do not set linux-generic as default platform

[Nicolas Morey-Chaisemartin]

Just realized I didn't update the help string.
One of a side effect of this series is there is NO default platform anymore.

I'll wait for some feedback and repost a v2 if needed

Le 12/02/2016 à 10:49 AM, Nicolas Morey-Chaisemartin a écrit :
> This prepares for the second patch and add support for repo
>  which do not have a linux-generic platform
>
> Signed-off-by: Nicolas Morey-Chaisemartin <nmo...@kalray.eu>
> ---
>  configure.ac | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/configure.ac b/configure.ac
> index be5a292..86173f4 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -123,7 +123,7 @@ AC_ARG_WITH([platform],
>  [AS_HELP_STRING([--with-platform=platform],
>   [select platform to be used, default linux-generic])],
>  [],
> -[with_platform=linux-generic
> +[AC_MSG_ERROR([No platform specified. Use --with-platform])
>  ])
>  
>  AC_SUBST([with_platform])

[lng-odp] [PATCH 4/4] configure.ac: move linux-generic conditionals to platform side

[Nicolas Morey-Chaisemartin]

Signed-off-by: Nicolas Morey-Chaisemartin <nmo...@kalray.eu>
---
 configure.ac  | 4 
 platform/linux-generic/m4/conditionals.m4 | 4 
 2 files changed, 4 insertions(+), 4 deletions(-)
 create mode 100644 platform/linux-generic/m4/conditionals.m4

diff --git a/configure.ac b/configure.ac
index c5cd9f2..9dbd9b9 100644
--- a/configure.ac
+++ b/configure.ac
@@ -169,10 +169,6 @@ AC_SUBST([testdir])
 ##
 # Set conditionals as computed within platform specific files
 ##
-AM_CONDITIONAL([netmap_support], [test x$netmap_support = xyes ])
-AM_CONDITIONAL([PKTIO_IPC], [test x$pktio_ipc_support = xyes])
-AM_CONDITIONAL([PKTIO_DPDK], [test x$pktio_dpdk_support = xyes ])
-AM_CONDITIONAL([HAVE_PCAP], [test $have_pcap = yes])
 AM_CONDITIONAL([SDK_INSTALL_PATH_], [test "x${SDK_INSTALL_PATH_}" = "x1"])
 AM_CONDITIONAL([test_installdir], [test "$testdir" != ""])
 AM_CONDITIONAL([cunit_support], [test x$cunit_support = xyes ])
diff --git a/platform/linux-generic/m4/conditionals.m4 
b/platform/linux-generic/m4/conditionals.m4
new file mode 100644
index 000..bd253a3
--- /dev/null
+++ b/platform/linux-generic/m4/conditionals.m4
@@ -0,0 +1,4 @@
+AM_CONDITIONAL([netmap_support], [test x$netmap_support = xyes ])
+AM_CONDITIONAL([PKTIO_IPC], [test x$pktio_ipc_support = xyes])
+AM_CONDITIONAL([PKTIO_DPDK], [test x$pktio_dpdk_support = xyes ])
+AM_CONDITIONAL([HAVE_PCAP], [test $have_pcap = yes])
-- 
2.10.1.4.g0ffc436

[lng-odp] [PATCH 2/4] bootstrap: auto generate platform list

[Nicolas Morey-Chaisemartin]

This list all the available platforms looking for
 platform/*/m4/configure.m4 and generate the appropriate m4
 file for the configure.ac to include

Signed-off-by: Nicolas Morey-Chaisemartin <nmo...@kalray.eu>
---
 bootstrap| 25 +
 configure.ac | 11 +--
 2 files changed, 26 insertions(+), 10 deletions(-)

diff --git a/bootstrap b/bootstrap
index 6fd91c8..274ce2a 100755
--- a/bootstrap
+++ b/bootstrap
@@ -1,5 +1,30 @@
 #! /bin/sh
 set -x
+
+# Auto generate the platform list
+PLATFORMS=$(ls platform/*/m4/configure.m4 | awk -F '/' '{ print $2}')
+GEN_M4="m4/platforms.m4"
+
+prefix=""
+echo "# Auto-Generated platform list" > $GEN_M4
+for platform in $PLATFORMS; do
+   cat << EOF >> $GEN_M4
+${prefix}if test "\${with_platform}" == "${platform}";
+then
+   m4_include([./platform/${platform}/m4/configure.m4])
+EOF
+   if [ -f test/${platform}/m4/configure.m4 ]; then
+   echo "  m4_include([./test/${platform}/m4/configure.m4])" >> 
$GEN_M4
+   fi
+   prefix="el"
+done
+cat << EOF >> $GEN_M4
+else
+echo "UNSUPPORTED PLATFORM: \${with_platform}"
+exit 1
+fi
+EOF
+
 aclocal -I config -I m4
 libtoolize --copy
 autoheader
diff --git a/configure.ac b/configure.ac
index 86173f4..c5cd9f2 100644
--- a/configure.ac
+++ b/configure.ac
@@ -132,16 +132,7 @@ AC_SUBST([platform_with_platform], 
["platform/${with_platform}"])
 ##
 # Run platform specific checks and settings
 ##
-IMPLEMENTATION_NAME=""
-if test "${with_platform}" == "linux-generic";
-then
-m4_include([./platform/linux-generic/m4/configure.m4])
-m4_include([./test/linux-generic/m4/configure.m4])
-IMPLEMENTATION_NAME="odp-linux"
-else
-echo "UNSUPPORTED PLATFORM: ${with_platform}"
-exit 1
-fi
+m4_include([m4/platforms.m4])
 
 ODP_CFLAGS="$ODP_CFLAGS -DIMPLEMENTATION_NAME=$IMPLEMENTATION_NAME"
 
-- 
2.10.1.4.g0ffc436

[lng-odp] [PATCH 3/4] bootstrap: auto-include platform specific conditional definitions

[Nicolas Morey-Chaisemartin]

Conditionals must always be defined if reference in a Makefile.am
 even if the Makefile does not apply to the platform being built.
As long as all the conditional are defined with default values in
platform//m4/conditionals.m4, bootstrap will automatically
include them so there won't be any issue when including multiple
platforms in the same repo

Signed-off-by: Nicolas Morey-Chaisemartin <nmo...@kalray.eu>
---
 bootstrap | 12 
 1 file changed, 12 insertions(+)

diff --git a/bootstrap b/bootstrap
index 274ce2a..1af732d 100755
--- a/bootstrap
+++ b/bootstrap
@@ -7,6 +7,7 @@ GEN_M4="m4/platforms.m4"
 
 prefix=""
 echo "# Auto-Generated platform list" > $GEN_M4
+
 for platform in $PLATFORMS; do
cat << EOF >> $GEN_M4
 ${prefix}if test "\${with_platform}" == "${platform}";
@@ -25,6 +26,17 @@ else
 fi
 EOF
 
+echo "# Include conditionals definitions if the platform has any" >> $GEN_M4
+for platform in $PLATFORMS; do
+   if [ ! -f platform/${platform}/m4/conditionals.m4 ]; then
+   continue
+   fi
+   cat << EOF >> $GEN_M4
+# Include conditional definitions for platform '${platform}'
+m4_include([./platform/${platform}/m4/conditionals.m4])
+EOF
+done;
+
 aclocal -I config -I m4
 libtoolize --copy
 autoheader
-- 
2.10.1.4.g0ffc436

[lng-odp] [PATCH 1/4] configure.ac: do not set linux-generic as default platform

[Nicolas Morey-Chaisemartin]

This prepares for the second patch and add support for repo
 which do not have a linux-generic platform

Signed-off-by: Nicolas Morey-Chaisemartin <nmo...@kalray.eu>
---
 configure.ac | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/configure.ac b/configure.ac
index be5a292..86173f4 100644
--- a/configure.ac
+++ b/configure.ac
@@ -123,7 +123,7 @@ AC_ARG_WITH([platform],
 [AS_HELP_STRING([--with-platform=platform],
[select platform to be used, default linux-generic])],
 [],
-[with_platform=linux-generic
+[AC_MSG_ERROR([No platform specified. Use --with-platform])
 ])
 
 AC_SUBST([with_platform])
-- 
2.10.1.4.g0ffc436

[lng-odp] [PATCH 0/4] Make configure.ac generic

[Nicolas Morey-Chaisemartin]

The goal of this patch series is to auto include all avalaible platform and not 
tie the configure.ac to linux-generic only.
It basically means that a platform could simply be a submodule clone into 
plat/ and be automatically included and compile with the appropriate 
--with-platform= option, without modifying configure.ac.

This applies to master, I haven't tried it on next yet.
I use it on monarch_lts too. It won't apply directly as the platform specific 
tests moved between the two branches;
If interested, I can send one for monarch too.

Nicolas Morey-Chaisemartin (4):
  configure.ac: do not set linux-generic as default platform
  bootstrap: auto generate platform list
  bootstrap: auto-include platform specific conditional definitions
  configure.ac: move linux-generic conditionals to platform side

 bootstrap | 37 +++
 configure.ac  | 17 ++
 platform/linux-generic/m4/conditionals.m4 |  4 
 3 files changed, 43 insertions(+), 15 deletions(-)
 create mode 100644 platform/linux-generic/m4/conditionals.m4

[lng-odp] [MONARCH PATCH] doc: remove VPATH override

[Nicolas Morey-Chaisemartin]

It breaks out-of-tree build of documentation as .adoc file are not in
 the VPATH and default rules cannot be applied to them.

Signed-off-by: Nicolas Morey-Chaisemartin <nmo...@kalray.eu>
---
 doc/Makefile.inc | 2 --
 1 file changed, 2 deletions(-)

diff --git a/doc/Makefile.inc b/doc/Makefile.inc
index 643b1d4..170c19f 100644
--- a/doc/Makefile.inc
+++ b/doc/Makefile.inc
@@ -1,5 +1,3 @@
-VPATH=$(top_builddir)/doc/images
-
 .msc.svg:
mscgen -T svg -i $^ -o $@
 

Re: [lng-odp] [MONARCH PATCH] git_hash: handle git worktree

[Nicolas Morey-Chaisemartin]

Hi everyone,

Could someone using gmail confirm that my mail didn't end up in your junk 
folder please :)

Nicolas

Le 11/18/2016 à 11:18 AM, Nicolas Morey-Chaisemartin a écrit :
> In git worktrees .git is a file and not a directory.
> This patches replaces [ -d .git ] test by [ -e .git ]
>  so it works in both cases
>
> Signed-off-by: Nicolas Morey-Chaisemartin <nmo...@kalray.eu>
> ---
>  scripts/git_hash.sh | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/scripts/git_hash.sh b/scripts/git_hash.sh
> index 336eb01..d0095d5 100755
> --- a/scripts/git_hash.sh
> +++ b/scripts/git_hash.sh
> @@ -6,7 +6,7 @@ if [ -z ${1} ]; then
>  fi
>  ROOTDIR=${1}
>  
> -if [ -d ${ROOTDIR}/.git ]; then
> +if [ -e ${ROOTDIR}/.git ]; then
>   hash=$(git --git-dir=${ROOTDIR}/.git describe --match 
> 'v[0-9]*\.[0-9]*\.[0-9]*\.[0-9]*'\
>  | tr -d "\n")
>   if [[ $(git --git-dir=${ROOTDIR}/.git diff --shortstat 2> /dev/null \
> @@ -19,7 +19,7 @@ if [ -d ${ROOTDIR}/.git ]; then
>   sed -i "s|-|.git|" ${ROOTDIR}/.scmversion
>   sed -i "s|-|.|g" ${ROOTDIR}/.scmversion
>   sed -i "s|^v||g" ${ROOTDIR}/.scmversion
> -elif [ ! -d ${ROOTDIR}/.git -a ! -f ${ROOTDIR}/.scmversion ]; then
> +elif [ ! -e ${ROOTDIR}/.git -a ! -f ${ROOTDIR}/.scmversion ]; then
>   echo -n "File ROOTDIR/.scmversion not found, "
>   echo "and not inside a git repository"
>   echo "Bailing out! Not recoverable!"

[lng-odp] [MONARCH PATCH] git_hash: handle git worktree

[Nicolas Morey-Chaisemartin]

In git worktrees .git is a file and not a directory.
This patches replaces [ -d .git ] test by [ -e .git ]
 so it works in both cases

Signed-off-by: Nicolas Morey-Chaisemartin <nmo...@kalray.eu>
---
 scripts/git_hash.sh | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/scripts/git_hash.sh b/scripts/git_hash.sh
index 336eb01..d0095d5 100755
--- a/scripts/git_hash.sh
+++ b/scripts/git_hash.sh
@@ -6,7 +6,7 @@ if [ -z ${1} ]; then
 fi
 ROOTDIR=${1}
 
-if [ -d ${ROOTDIR}/.git ]; then
+if [ -e ${ROOTDIR}/.git ]; then
hash=$(git --git-dir=${ROOTDIR}/.git describe --match 
'v[0-9]*\.[0-9]*\.[0-9]*\.[0-9]*'\
   | tr -d "\n")
if [[ $(git --git-dir=${ROOTDIR}/.git diff --shortstat 2> /dev/null \
@@ -19,7 +19,7 @@ if [ -d ${ROOTDIR}/.git ]; then
sed -i "s|-|.git|" ${ROOTDIR}/.scmversion
sed -i "s|-|.|g" ${ROOTDIR}/.scmversion
sed -i "s|^v||g" ${ROOTDIR}/.scmversion
-elif [ ! -d ${ROOTDIR}/.git -a ! -f ${ROOTDIR}/.scmversion ]; then
+elif [ ! -e ${ROOTDIR}/.git -a ! -f ${ROOTDIR}/.scmversion ]; then
echo -n "File ROOTDIR/.scmversion not found, "
echo "and not inside a git repository"
echo "Bailing out! Not recoverable!"

Re: [lng-odp] Mailing test again

[Nicolas Morey-Chaisemartin]

I think REMOVE_DKIM_HEADERS could be set to no.
It was added some time ago when I raised the issue of our mail ending up in 
junk but at the time, the mailing list was adding a footer to email which broke 
the DKIM authentication.
Now that it does not, I think it is safe to keep the DKIM headers?

I talked to our admins. The issue is that we use Zimbra and don't have any 
control on DKIM apart from on or off.

Nicolas

Le 07/29/2016 à 05:48 PM, Ciaran Moran a écrit :
> Nicolas: you are correct, docs:
> http://www.list.org/mailman-member/node21.html
> Thanks for the headers, very helpful. 
>
> All:
> I think Mailman is correctly using the "REMOVE_DKIM_HEADERS = Yes" option, 
> but I could be wrong (needs more testing my end). The only thing which stands 
> out is Kalray.eu's mailserver prepending not only a "DKIM-Signature" header, 
> but also a "DKIM-Filter" header (which only gives the dkim-filter version 
> number, and appears to be only for debugging).
>
> Next week, I plan to:
>
>   * Set up another mailing list;
>   * Try sending to this list from an Amazon SPF & DKIM-enabled domain;
>   * Ask Kalray.eu's mail admins if they will disable the "DKIM-Filter" header 
> and send a test mail to lists.linaro.org <http://lists.linaro.org>;
>   * Compare the headers and (hopefully!) figure out why Google is failing 
> mail from Kalray.eu (Zimbra) via lists.linaro.org <http://lists.linaro.org> 
> (Postfix) to Linaro.org (GMail).
>
>
> Best regards,
> Ciaran.
>
> On 29 July 2016 at 13:26, Nicolas Morey-Chaisemartin <nmo...@kalray.eu 
> <mailto:nmo...@kalray.eu>> wrote:
>
> I guess your mail server (or mailman) is smart enough to know that you 
> are both in CC and the ML and only sent you one copy (the direct one)
>
>
> Le 07/28/2016 à 10:38 PM, Bill Fischofer a écrit :
>> This e-mail didn't show up in my spam folder, but I'm wondering if 
>> that's because I was on the To: list explicitly? It does appear in the 
>> odp-lng list archives so it clearly was received.
>>
>> On Thu, Jul 28, 2016 at 9:09 AM, Nicolas Morey-Chaisemartin 
>> <nmo...@kalray.eu <mailto:nmo...@kalray.eu>> wrote:
>>
>>  Another mailing list test fo find oud why kalray's email end up in 
>> spam for gmail users.
>>
>> Nicolas
>>

Re: [lng-odp] Mailing test again

[Nicolas Morey-Chaisemartin]

I guess your mail server (or mailman) is smart enough to know that you are both 
in CC and the ML and only sent you one copy (the direct one)

Le 07/28/2016 à 10:38 PM, Bill Fischofer a écrit :
> This e-mail didn't show up in my spam folder, but I'm wondering if that's 
> because I was on the To: list explicitly? It does appear in the odp-lng list 
> archives so it clearly was received.
>
> On Thu, Jul 28, 2016 at 9:09 AM, Nicolas Morey-Chaisemartin <nmo...@kalray.eu 
> <mailto:nmo...@kalray.eu>> wrote:
>
>  Another mailing list test fo find oud why kalray's email end up in spam 
> for gmail users.
>
> Nicolas
>
>

[lng-odp] Mailing test again

[Nicolas Morey-Chaisemartin]

 Another mailing list test fo find oud why kalray's email end up in spam for 
gmail users.

Nicolas

[lng-odp] Test mail again

[Nicolas Morey-Chaisemartin]

Me again and again. Sorry for the spam.

As for the previous ones (that you probably didn't see anyway):
If people using GMail, can see this (or find this in their junk folder), could 
you reply to this mail.
In the case where it was marked as Junk, could you also forward Gmail infos 
about why the mail was marked like this.

Regards

Nicolas

[lng-odp] One more test mail

[Nicolas Morey-Chaisemartin]

Me again and again. Sorry for the spam.

As for the previous ones (that you probably didn't see anyway):
If people using GMail, can see this (or find this in their junk folder), could 
you reply to this mail.
In the case where it was marked as Junk, could you also forward Gmail infos 
about why the mail was marked like this.

Regards

Nicolas

[lng-odp] Yet Another Test Mail

[Nicolas Morey-Chaisemartin]

Me again. Sorry for the spam.

As for the previous one (that yuo probably didn't see anyway):
If people using GMail, can see this (or find this in their junk folder), could 
you reply to this mail.
In the case where it was marked as Junk, could you also forward Gmail infos 
about why the mail was marked like this.

Regards

Nicolas

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

[lng-odp] Another mailing list test for gmail users

[Nicolas Morey-Chaisemartin]

Sorry to do another mail like this,

I'm still trying to figure out what is wrong with our mail server setup, 
mailman et Gmail.
If people using GMail, can see this (or find this in their junk folder), could 
you reply to this mail.

In the case where it was marked as Junk, could you also forward Gmail infos 
about why the mail was marked like this.

Regards

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

[lng-odp] ML test

[Nicolas Morey-Chaisemartin]

Hi,


Can ayone using the ML on gmail can see this?

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


Thanks


Nicolas

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

[lng-odp] Mailing List test again

[Nicolas Morey-Chaisemartin (ODP)]

Sorry for the spam guys

Still trying to get the ODP mailing list to properly let me through :)

Maxim could you tell me if you see this one?

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

[lng-odp] Test mail

[Nicolas Morey-Chaisemartin]

Making sure if we can send mail to the ML again :)

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

[lng-odp] Error building user guide

[Nicolas Morey-Chaisemartin]

Hi,
I just pulled 1.7.0 and tried to build the official release (not our port) and 
I get some error messages during the doc build.
asciidoc: WARNING: implementers-guide.adoc: line 43: {sys:"/usr/bin/python2" -u 
-c "import base64,sys; base64.encode(sys.stdin,sys.stdout)" < 
"/work1/nmorey/workspace/alternates/master/odp-patches/doc/implementers-guide/./images/icons/callouts/1.png"}:
 non-zero exit status

This does not stop the doc from building but it's not very pretty and the doc 
get rebuilt everytime I run make.
Am I the only one to get this ?
If I remove the "-a icons" option it seems to work.

I am currently running asciidoc 8.6.8

By the way, the doc is the only "module" that gets build in the source tree 
instead of the build tree. Is this the wanted behaviour?
As we compile with multiple build trees (one per platform), it gets a bit messy 
when running make -j so I changed the Makefile to build everything in the 
build tree.
Is this something interesting for you guys?

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

[lng-odp] [PATCH] example: time: use PRIu32 where needed

[Nicolas Morey-Chaisemartin]

Signed-off-by: Nicolas Morey-Chaisemartin <nmo...@kalray.eu>
---
 example/time/time_global_test.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/example/time/time_global_test.c b/example/time/time_global_test.c
index 7cfd969..0c08f2c 100644
--- a/example/time/time_global_test.c
+++ b/example/time/time_global_test.c
@@ -78,7 +78,7 @@ static void print_log(test_globals_t *gbls)
 
err_num = odp_atomic_load_u32(>err_counter);
if (err_num)
-   printf("Number of errors: %u\n", err_num);
+   printf("Number of errors: %" PRIu32 "\n", err_num);
 }
 
 static void
@@ -185,7 +185,7 @@ static void *run_thread(void *ptr)
 * buffer is not the same thread.
 */
id = odp_atomic_fetch_inc_u32(>id_counter);
-   sprintf(queue_name, QUEUE_NAME_PREFIX "%d", id);
+   sprintf(queue_name, QUEUE_NAME_PREFIX "%" PRIu32 "", id);
queue = odp_queue_create(queue_name, NULL);
if (queue == ODP_QUEUE_INVALID)
EXAMPLE_ABORT("Cannot create thread queue, thread %d", thr);
-- 
2.6.3.373.g6d475bf.dirty

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

[lng-odp] [PATCH] linux-generic: pktio: do not include net/if.h and linux/if.h

[Nicolas Morey-Chaisemartin]

Both are incompatible:
In file included from platform/linux-generic/pktio/ethtool.c:13:0:
/usr/include/net/if.h:44:5: error: redeclaration of enumerator ‘IFF_UP’
 IFF_UP = 0x1,  /* Interface is up.  */
 ^
In file included from platform/linux-generic/pktio/ethtool.c:10:0:
/usr/include/linux/if.h:71:2: note: previous definition of ‘IFF_UP’ was here
  IFF_UP= 1<<0,  /* sysfs */
  ^
and so on..

Signed-off-by: Nicolas Morey-Chaisemartin <nmo...@kalray.eu>
---
 platform/linux-generic/pktio/ethtool.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/platform/linux-generic/pktio/ethtool.c 
b/platform/linux-generic/pktio/ethtool.c
index 1f29438..a90a781 100644
--- a/platform/linux-generic/pktio/ethtool.c
+++ b/platform/linux-generic/pktio/ethtool.c
@@ -10,7 +10,6 @@
 #include 
 #include 
 #include 
-#include 
 
 #include 
 #include 
@@ -158,7 +157,7 @@ int ethtool_stats_get_fd(int fd, const char *name, 
odp_pktio_stats_t *stats)
 {
struct ifreq ifr;
 
-   snprintf(ifr.ifr_name, IF_NAMESIZE, "%s", name);
+   snprintf(ifr.ifr_name, IFNAMSIZ, "%s", name);
 
return ethtool_stats(fd, , stats);
 }
-- 
2.6.3.373.g6d475bf.dirty

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

[lng-odp] [PATCH] validation: timer: replace %d by PRIu32

[Nicolas Morey-Chaisemartin]

Signed-off-by: Nicolas Morey-Chaisemartin <nmo...@kalray.eu>
---
 test/validation/timer/timer.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/test/validation/timer/timer.c b/test/validation/timer/timer.c
index 004670a..8d3ba47 100644
--- a/test/validation/timer/timer.c
+++ b/test/validation/timer/timer.c
@@ -294,13 +294,13 @@ static void *worker_entrypoint(void *arg TEST_UNUSED)
for (i = 0; i < NTIMERS; i++) {
tt[i].ev = odp_timeout_to_event(odp_timeout_alloc(tbp));
if (tt[i].ev == ODP_EVENT_INVALID) {
-   LOG_DBG("Failed to allocate timeout (%d/%d)\n",
+   LOG_DBG("Failed to allocate timeout (%" PRIu32 "/%d)\n",
i, NTIMERS);
break;
}
tt[i].tim = odp_timer_alloc(tp, queue, [i]);
if (tt[i].tim == ODP_TIMER_INVALID) {
-   LOG_DBG("Failed to allocate timer (%d/%d)\n",
+   LOG_DBG("Failed to allocate timer (%" PRIu32 "/%d)\n",
i, NTIMERS);
odp_timeout_free(tt[i].ev);
break;
-- 
2.6.3.373.g6d475bf.dirty

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

[lng-odp] [PATCH] scripts: git_hash: support gitfiles

[Nicolas Morey-Chaisemartin]

With recent git releases, the .git at the top of a repo is not necessary
 a directory but can be a gitfile pointing to a remote git directory.
This is commonly used by git-worktree and git-submodule.
This patch changes the check for .git to allow for any file (symlinks too)

Signed-off-by: Nicolas Morey-Chaisemartin <nmo...@kalray.eu>
---
 scripts/git_hash.sh | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/scripts/git_hash.sh b/scripts/git_hash.sh
index ccd62ab..e7f43f2 100755
--- a/scripts/git_hash.sh
+++ b/scripts/git_hash.sh
@@ -6,7 +6,7 @@ if [ -z ${1} ]; then
 fi
 ROOTDIR=${1}
 
-if [ -d ${ROOTDIR}/.git ]; then
+if [ -e ${ROOTDIR}/.git ]; then
hash=$(git --git-dir=${ROOTDIR}/.git describe | tr -d "\n")
if [[ $(git --git-dir=${ROOTDIR}/.git diff --shortstat 2> /dev/null \
| tail -n1) != "" ]]; then
@@ -18,7 +18,7 @@ if [ -d ${ROOTDIR}/.git ]; then
sed -i "s|-|.git|" ${ROOTDIR}/.scmversion
sed -i "s|-|.|g" ${ROOTDIR}/.scmversion
sed -i "s|^v||g" ${ROOTDIR}/.scmversion
-elif [ ! -d ${ROOTDIR}/.git -a ! -f ${ROOTDIR}/.scmversion ]; then
+elif [ ! -e ${ROOTDIR}/.git -a ! -f ${ROOTDIR}/.scmversion ]; then
echo -n "File ROOTDIR/.scmversion not found, "
echo "and not inside a git repository"
echo "Bailing out! Not recoverable!"
___
lng-odp mailing list
lng-odp@lists.linaro.org
https://lists.linaro.org/mailman/listinfo/lng-odp

[lng-odp] [PATCHv2 2/2] validation: init: add test for init with platform parameters

[Nicolas Morey-Chaisemartin]

Signed-off-by: Nicolas Morey-Chaisemartin <nmo...@kalray.eu>
---

Reposted due to mail issues

 test/validation/init/init.c | 16 
 test/validation/init/init.h |  1 +
 2 files changed, 17 insertions(+)

diff --git a/test/validation/init/init.c b/test/validation/init/init.c
index 665c3dc..ed5c817 100644
--- a/test/validation/init/init.c
+++ b/test/validation/init/init.c
@@ -144,8 +144,24 @@ void init_test_odp_init_global(void)
CU_ASSERT(status == 0);
 }
 
+/* test normal ODP global init with platform params*/
+void init_test_odp_init_global_with_platform_params(void)
+{
+   int status;
+   odp_platform_init_t platform_params;
+
+   odp_platform_init_init(_params);
+
+   status = odp_init_global(NULL, _params);
+   CU_ASSERT_FATAL(status == 0);
+
+   status = odp_term_global();
+   CU_ASSERT(status == 0);
+}
+
 odp_testinfo_t init_suite_ok[] = {
ODP_TEST_INFO(init_test_odp_init_global),
+   ODP_TEST_INFO(init_test_odp_init_global_with_platform_params),
ODP_TEST_INFO_NULL,
 };
 
diff --git a/test/validation/init/init.h b/test/validation/init/init.h
index 272d426..70240cf 100644
--- a/test/validation/init/init.h
+++ b/test/validation/init/init.h
@@ -13,6 +13,7 @@
 void init_test_odp_init_global_replace_abort(void);
 void init_test_odp_init_global_replace_log(void);
 void init_test_odp_init_global(void);
+void init_test_odp_init_global_with_platform_params(void);
 
 /* test arrays: */
 extern odp_testinfo_t init_suite_abort[];
___
lng-odp mailing list
lng-odp@lists.linaro.org
https://lists.linaro.org/mailman/listinfo/lng-odp

[lng-odp] [PATCHv2 1/2] api: init: add initializer function for odp_platform_init_t

[Nicolas Morey-Chaisemartin]

Signed-off-by: Nicolas Morey-Chaisemartin <nmo...@kalray.eu>
---

Reposted due to mail issues

 include/odp/api/spec/init.h   | 8 
 platform/linux-generic/odp_init.c | 4 
 2 files changed, 12 insertions(+)

diff --git a/include/odp/api/spec/init.h b/include/odp/api/spec/init.h
index 2c98109..6cbfb97 100644
--- a/include/odp/api/spec/init.h
+++ b/include/odp/api/spec/init.h
@@ -134,6 +134,14 @@ typedef struct odp_init_t {
  * passing any required platform specific data.
  */
 
+/**
+ * Initialize platform init structure
+ *
+ * Initialize an odp_platform_init_t to its default values for all fields
+ *
+ * @param platform_params Address of the odp_platform_init_t to be initialized
+ */
+void odp_platform_init_init(odp_platform_init_t *platform_params);
 
 /**
  * Global ODP initialization
diff --git a/platform/linux-generic/odp_init.c 
b/platform/linux-generic/odp_init.c
index a8c91a5..d80d29f 100644
--- a/platform/linux-generic/odp_init.c
+++ b/platform/linux-generic/odp_init.c
@@ -284,3 +284,7 @@ int _odp_term_local(enum init_stage stage)
 
return rc;
 }
+
+void odp_platform_init_init(odp_platform_init_t *platform_params ODP_UNUSED)
+{
+}
___
lng-odp mailing list
lng-odp@lists.linaro.org
https://lists.linaro.org/mailman/listinfo/lng-odp

[lng-odp] Mailman Test

[Nicolas Morey-Chaisemartin]

Test to make sure Mailman is not breaking DKIM anymore.

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

Re: [lng-odp] [API-NEXT PATCHv1 0/2] linux-generic:classification:fix the endian problem when verify ODP_PMR_CUSTOM_FRAME

[Nicolas Morey-Chaisemartin]

I don't completely agree with your assessment.
The fact that it ends up in a uint64_t is linux-generic specific. More 
precisely because uint64_t was already used in the pmr_term_value_t struct.
Nothing prevents another implementation to support any size of 
ODP_PMR_CUSTOM_FRAME (ie more than 64b)
In this case, Endianness doesn't make much sense (how would you convert a 9B 
wide data?)

I'd rather see something added to the documentation to warn that the data and 
mask provided to the PMR need to be in network order.

Nicolas

On 01/30/2016 09:31 AM, huanggaoyang wrote:
> I think it's a bug in classification:When verifying the pmr_term 
> ODP_PMR_CUSTOM_FRAME, 
> the big-endian packet data shouldn't directly memcpy and used as a uint64_t 
> on little-endian machine. 
> We should make a transition here when it's on little-endian machine. 
> Or we have to turn the value to big-endian when creating a 
> ODP_PMR_CUSTOM_FRAME pmr, it's different
> from other pmr term and make no sense.
> And I also add a test case for this ODP_PMR_CUSTOM_FRAME term in validation 
> test.
>
> huanggaoyang (2):
>   linux-generic:classification: on little endian machine, a transition
> should take after memcpy from a packet
>   linux-generic:classification:test: add a test case for term
> ODP_PMR_CUSTOM_FRAME
>
>  .../include/odp_classification_inlines.h   |   5 +
>  test/validation/classification/classification.h|   1 +
>  .../classification/odp_classification_test_pmr.c   | 114 
> -
>  3 files changed, 118 insertions(+), 2 deletions(-)
>

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

[lng-odp] [API-NEXT 2/2] validation: init: add test for init with platform parameters

[Nicolas Morey-Chaisemartin]

Signed-off-by: Nicolas Morey-Chaisemartin <nmo...@kalray.eu>
---
 test/validation/init/init.c | 16 
 test/validation/init/init.h |  1 +
 2 files changed, 17 insertions(+)

diff --git a/test/validation/init/init.c b/test/validation/init/init.c
index 62bd75c..f7424a9 100644
--- a/test/validation/init/init.c
+++ b/test/validation/init/init.c
@@ -144,8 +144,24 @@ void init_test_odp_init_global(void)
CU_ASSERT(status == 0);
 }
 
+/* test normal ODP global init with platform params*/
+void init_test_odp_init_global_with_platform_params(void)
+{
+   int status;
+   odp_platform_init_t platform_params;
+
+   odp_platform_init_init(_params);
+
+   status = odp_init_global(NULL, _params);
+   CU_ASSERT_FATAL(status == 0);
+
+   status = odp_term_global();
+   CU_ASSERT(status == 0);
+}
+
 odp_testinfo_t init_suite_ok[] = {
ODP_TEST_INFO(init_test_odp_init_global),
+   ODP_TEST_INFO(init_test_odp_init_global_with_platform_params),
ODP_TEST_INFO_NULL,
 };
 
diff --git a/test/validation/init/init.h b/test/validation/init/init.h
index 272d426..70240cf 100644
--- a/test/validation/init/init.h
+++ b/test/validation/init/init.h
@@ -13,6 +13,7 @@
 void init_test_odp_init_global_replace_abort(void);
 void init_test_odp_init_global_replace_log(void);
 void init_test_odp_init_global(void);
+void init_test_odp_init_global_with_platform_params(void);
 
 /* test arrays: */
 extern odp_testinfo_t init_suite_abort[];
-- 
2.6.3.373.g6d475bf.dirty

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

[lng-odp] [API-NEXT 1/2] api: init: add initializer function for odp_platform_init_t

[Nicolas Morey-Chaisemartin]

Signed-off-by: Nicolas Morey-Chaisemartin <nmo...@kalray.eu>
---

I'm not very happy with a odp_platform_init_init but odp_platform_init would be 
misleading for users
and odp_platform_param_init wouldn't fit ODP semantics.
If someone has a better idea, I'm all ears.

 include/odp/api/init.h| 8 
 platform/linux-generic/odp_init.c | 4 
 2 files changed, 12 insertions(+)

diff --git a/include/odp/api/init.h b/include/odp/api/init.h
index 381f77e..7994147 100644
--- a/include/odp/api/init.h
+++ b/include/odp/api/init.h
@@ -134,6 +134,14 @@ typedef struct odp_init_t {
  * passing any required platform specific data.
  */
 
+/**
+ * Initialize platform init structure
+ *
+ * Initialize an odp_platform_init_t to its default values for all fields
+ *
+ * @param platform_params Address of the odp_platform_init_t to be initialized
+ */
+void odp_platform_init_init(odp_platform_init_t *platform_params);
 
 /**
  * Global ODP initialization
diff --git a/platform/linux-generic/odp_init.c 
b/platform/linux-generic/odp_init.c
index 6ad3320..00a369c 100644
--- a/platform/linux-generic/odp_init.c
+++ b/platform/linux-generic/odp_init.c
@@ -284,3 +284,7 @@ int _odp_term_local(enum init_stage stage)
 
return rc;
 }
+
+void odp_platform_init_init(odp_platform_init_t *platform_params ODP_UNUSED)
+{
+}
___
lng-odp mailing list
lng-odp@lists.linaro.org
https://lists.linaro.org/mailman/listinfo/lng-odp

[lng-odp] [PATCH] linux-generic: crypto: fix AES-GCM compatibility with old version of OpenSSL

[Nicolas Morey-Chaisemartin]

Old version of OpenSSL require SET_TAG to be called before
 decrypting the data. New versions are compatible either way

Signed-off-by: Nicolas Morey-Chaisemartin <nmo...@kalray.eu>
---

Repost due to previous mail tagged as SPAM

 platform/linux-generic/odp_crypto.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/platform/linux-generic/odp_crypto.c 
b/platform/linux-generic/odp_crypto.c
index 65e8503..92bc2f3 100644
--- a/platform/linux-generic/odp_crypto.c
+++ b/platform/linux-generic/odp_crypto.c
@@ -397,6 +397,7 @@ odp_crypto_alg_err_t aes_gcm_decrypt(odp_crypto_op_params_t 
*params,
int plain_len = 0;
 
EVP_DecryptInit_ex(ctx, NULL, NULL, NULL, iv_enc);
+   EVP_CIPHER_CTX_ctrl(ctx, EVP_CTRL_GCM_SET_TAG, 16, tag);
 
/* Authenticate header data (if any) without encrypting them */
if (aad_head < cipherdata) {
@@ -414,8 +415,6 @@ odp_crypto_alg_err_t aes_gcm_decrypt(odp_crypto_op_params_t 
*params,
  auth_len - (aad_tail - aad_head));
}
 
-   EVP_CIPHER_CTX_ctrl(ctx, EVP_CTRL_GCM_SET_TAG, 16, tag);
-
if (EVP_DecryptFinal_ex(ctx, cipherdata + cipher_len, _len) < 0)
return ODP_CRYPTO_ALG_ERR_ICV_CHECK;
 
___
lng-odp mailing list
lng-odp@lists.linaro.org
https://lists.linaro.org/mailman/listinfo/lng-odp

Re: [lng-odp] Maintain code copy-pasted from linux-generic in ODP-DPDK

[Nicolas Morey-Chaisemartin]

On 01/13/2016 02:46 PM, Maxim Uvarov wrote:
> On 01/13/2016 00:15, Nicolas Morey-Chaisemartin wrote:
>> Hi,
>>
>> I have the same issue with our implementation.
>> I usually either work from the git diff of linux generic before and post 
>> update and backport changed or to a diff of the diff of linux-generic vs 
>> mppa platform before and after the merge.
>> Theoretically the diff you be clode to void (except line changed) once you 
>> backport everything.
>>
>> I've looked a bit around and if your code is close enough to the official 
>> one (ours start to be quite different on many files), you can try something 
>> like this:
>> MBOX=$(mktemp); (cd platform/linux-generic/ && git format-patch --relative 
>> COMMIT1..COMMIT2 --stdout -- . > $MBOX) && git am -3 --directory 
>> platform/dpdk/ $MBOX
>
> dpdk has symlinks to linux-generic so in many places it just reuses files.
>
> Maxim.
In tat case you can change the filter in git format-patch and use
--   ...
instead of
-- .
>
>> It will try to apply all the patches that went into the linux-generic file 
>> into the dpdk platform.
>> It seems to work OK although I can see a lot of conflicts (trying to merge 
>> API next into our master may be a bit hard for a first try)
>>
>> I'll give it a try when master gets few commits in linux-generic post 1.6
>>
>> Nicolas
>>
>> On 01/12/2016 07:57 PM, Zoltan Kiss wrote:
>>> Hi,
>>>
>>> We have a couple of places where the entire source file couldn't be copied 
>>> from linux-generic, but some of the functions are. E.g. the loopback 
>>> implementation from linux-generic's pktio code is something like that. It 
>>> would be nice to somehow get the updates and fixes for this codebase, but 
>>> my current best way is to compare the code of these copies and based on the 
>>> differences use diff to figure out which commits are relevant for us.
>>> Does anyone have a more automated idea for this problem?
>>>
>>> Regards,
>>>
>>> Zoltan Kiss
>>> ___
>>> lng-odp mailing list
>>> lng-odp@lists.linaro.org
>>> https://lists.linaro.org/mailman/listinfo/lng-odp
>> ___
>> lng-odp mailing list
>> lng-odp@lists.linaro.org
>> https://lists.linaro.org/mailman/listinfo/lng-odp
>
> ___
> lng-odp mailing list
> lng-odp@lists.linaro.org
> https://lists.linaro.org/mailman/listinfo/lng-odp

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

Re: [lng-odp] ODP 226: need for shmem->refresh()?

[Nicolas Morey-Chaisemartin]

A full memory barrier would actually work for us but the cost is unacceptable 
most of the time.

To get all the performances out of our core, it will be necessary for our ODP 
user to be aware the the non-cache coherency and deal with it manually.
However what I'm looking for (as a first step at least) is a clean way through 
ODP API to get all the standard sunny day application and tests reasonably well.

For that, we can't simply map the odp_mb_* primitives to full memory barriers 
at it will hurt advanced customers too.
What we discussed with Christophe this morning is a somewhat in the middle.

By providing flush/reset primitives on SHMEM, we can have an implementation 
that provide some coherency on the required shared part.
I think it could also be used on specific implementation/arch to prefetch data 
to the cache (when it's coherent).

Nicolas

On 01/14/2016 03:25 PM, Savolainen, Petri (Nokia - FI/Espoo) wrote:
>
> In general, this issue is about - how to support non cache coherent systems. 
> Additional calls specified for non-coherent  systems could be added (to shm 
> and potentially elsewhere) but should be optional for applications, since 
> it’s quite tricky to (efficiently) ensure cache coherency in SW. Also 
> non-coherent systems are already in minority and will be even more so in the 
> future.
>
>  
>
> E.g. single memory barrier / sync (pair) could do the trick in a coherent 
> system, whereas a non-coherent system would need multiple flush/refresh calls 
> (with different pointers). Each barrier (flush/refresh) would hurt 
> performance (compiler optimizer and OoO HW), but more importantly would be 
> painful to maintain (add one new pointer somewhere in your data structure and 
> forget to flush that one address -> stale data -> crash).
>
>  
>
> -Petri
>
>  
>
>  
>
> *From:*EXT Christophe Milard [mailto:christophe.mil...@linaro.org]
> *Sent:* Thursday, January 14, 2016 12:16 PM
> *To:* Hongbo Zhang; Mike Holmes; Petri Savolainen; Anders Roxell; LNG ODP 
> Mailman List; Nicolas Morey-Chaisemartin
> *Subject:* ODP 226: need for shmem->refresh()?
>
>  
>
> This is regarding ODP 226 (Tests assuming pthreads)   
>   
>
> Kalray is facing a problem actually larger than this thread vs process 
> problem: 
>
> The basic question is: when N processors share the same memory (shmem 
> object),  
>
> is it acceptable to force a cache update for the whole shmem'd area for N-1   
>   
>
> processors, as soon as one single processor updates any byte in the area. 
>   
>
> Typically, one processor will write something in the shmem, and onother will  
>   
>
> want to access it. Just the latter really needs to invalid its cache. 
>   
>
>   
>   
>
> Kallray typically does a cache invalidation on small ODP objects (e.g. 
> atomics) 
>
> but the cost of doing cache invalidate everywhere on all processors is too 
> high 
>
> for shared memory areas: updates on "shmemed" areas are not automaticaly 
> visible
>
> from other CPUs.
>
> 
>
> They would need something like a "refresh" method on the shmem object. The
>   
>
> processor which really needs the data would call it.  
>   
>
> .refresh() would invalidate the local cache (of that single 
> core),
>
> and possibly initiate a prefetch. 
>   
>
> For symetry purpose, maybe a .flush() is needed, to flush   
>   
>
> pending writes on a shmem object. Kalray has write-through caching at this
>   
>
> stage so the need is not as bad. Possibly these methods would remap to memory 
>   
>
> barriers on some implementations, but memory barriers are not local to shmem  
>   
>
> objects, and invalidating the whole CPU cache for a single shmem object is
>   
>
> costly.   
>   
>
>   
>   
>
> It would make sense to have a refresh() and flush() acting on the whole   
>   
>
> shmem object. But Kalray pointed out that the price for allocating very small 
>   
>
> shmem fragment is high (shmem area have name, handles and have page   
>   
>
> granularity). Allocating a shmem for a single atomic int is not efficient.
>   
>
> They would therefore like to see these methods acting on sub-areas of the 
>   
>
> share memory. (or do we see a case for shmem_malloc() here?)  
>   
>
>   
>   
>
> Hope this helps understanding the problem...
>
>  
>
> Christophe.   
>

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

Re: [lng-odp] [PATCH] validation: classification: remove double frees

[Nicolas Morey-Chaisemartin]

On 01/13/2016 01:42 PM, Stuart Haslam wrote:
> On Wed, Jan 13, 2016 at 03:30:03PM +0300, Maxim Uvarov wrote:
>> Please resend this patch with Review. Original patch is missing for
>> unknown reason.
>>
>> Maxim.
> Mails from Nicolas get flagged as spam by google, looks to be due to a
> DKIM failure.
>
>   "It has a from address in kalray.eu but has failed kalray.eu's required
>tests for authentication. "
>
> You'll probably find it in your Spam folder.
>
Thanks for the info.
I'll pass it to our IT guys hoping they can do something about it.

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

Re: [lng-odp] Maintain code copy-pasted from linux-generic in ODP-DPDK

[Nicolas Morey-Chaisemartin]

Hi,

I have the same issue with our implementation.
I usually either work from the git diff of linux generic before and post update 
and backport changed or to a diff of the diff of linux-generic vs mppa platform 
before and after the merge.
Theoretically the diff you be clode to void (except line changed) once you 
backport everything.

I've looked a bit around and if your code is close enough to the official one 
(ours start to be quite different on many files), you can try something like 
this:
MBOX=$(mktemp); (cd platform/linux-generic/ && git format-patch --relative 
COMMIT1..COMMIT2 --stdout -- . > $MBOX) && git am -3 --directory platform/dpdk/ 
$MBOX

It will try to apply all the patches that went into the linux-generic file into 
the dpdk platform.
It seems to work OK although I can see a lot of conflicts (trying to merge API 
next into our master may be a bit hard for a first try)

I'll give it a try when master gets few commits in linux-generic post 1.6

Nicolas

On 01/12/2016 07:57 PM, Zoltan Kiss wrote:
> Hi,
>
> We have a couple of places where the entire source file couldn't be copied 
> from linux-generic, but some of the functions are. E.g. the loopback 
> implementation from linux-generic's pktio code is something like that. It 
> would be nice to somehow get the updates and fixes for this codebase, but my 
> current best way is to compare the code of these copies and based on the 
> differences use diff to figure out which commits are relevant for us.
> Does anyone have a more automated idea for this problem?
>
> Regards,
>
> Zoltan Kiss
> ___
> lng-odp mailing list
> lng-odp@lists.linaro.org
> https://lists.linaro.org/mailman/listinfo/lng-odp

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

[lng-odp] [PATCH] linux-generic: crypto: fix AES-GCM compatibility with old version of OpenSSL

[Nicolas Morey-Chaisemartin]

Old version of OpenSSL require SET_TAG to be called before
 decrypting the data. New versions are compatible either way

Signed-off-by: Nicolas Morey-Chaisemartin <nmo...@kalray.eu>
---
 platform/linux-generic/odp_crypto.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/platform/linux-generic/odp_crypto.c 
b/platform/linux-generic/odp_crypto.c
index 65e8503..92bc2f3 100644
--- a/platform/linux-generic/odp_crypto.c
+++ b/platform/linux-generic/odp_crypto.c
@@ -397,6 +397,7 @@ odp_crypto_alg_err_t aes_gcm_decrypt(odp_crypto_op_params_t 
*params,
int plain_len = 0;
 
EVP_DecryptInit_ex(ctx, NULL, NULL, NULL, iv_enc);
+   EVP_CIPHER_CTX_ctrl(ctx, EVP_CTRL_GCM_SET_TAG, 16, tag);
 
/* Authenticate header data (if any) without encrypting them */
if (aad_head < cipherdata) {
@@ -414,8 +415,6 @@ odp_crypto_alg_err_t aes_gcm_decrypt(odp_crypto_op_params_t 
*params,
  auth_len - (aad_tail - aad_head));
}
 
-   EVP_CIPHER_CTX_ctrl(ctx, EVP_CTRL_GCM_SET_TAG, 16, tag);
-
if (EVP_DecryptFinal_ex(ctx, cipherdata + cipher_len, _len) < 0)
return ODP_CRYPTO_ALG_ERR_ICV_CHECK;
 
-- 
2.6.3.372.gcb93895

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