[lng-odp] [PATCHv2] validation: scheduler: release context according to scheduler sync type
For different scheduler sync type, need to call different release
function.
Signed-off-by: Kevin Wang
---
test/common_plat/validation/api/scheduler/scheduler.c | 18 ++
1 file changed, 10 insertions(+), 8 deletions(-)
diff --git a/test/common_plat/validation/api/scheduler/scheduler.c
b/test/common_plat/validation/api/scheduler/scheduler.c
index 952561c..2c669a1 100644
--- a/test/common_plat/validation/api/scheduler/scheduler.c
+++ b/test/common_plat/validation/api/scheduler/scheduler.c
@@ -129,6 +129,14 @@ static int exit_schedule_loop(void)
return ret;
}
+static void release_context(odp_schedule_sync_t sync)
+{
+ if (sync == ODP_SCHED_SYNC_ATOMIC)
+ odp_schedule_release_atomic();
+ else if (sync == ODP_SCHED_SYNC_ORDERED)
+ odp_schedule_release_ordered();
+}
+
void scheduler_test_wait_time(void)
{
int i;
@@ -251,8 +259,7 @@ void scheduler_test_queue_destroy(void)
CU_ASSERT_FATAL(u32[0] == MAGIC);
odp_buffer_free(buf);
- odp_schedule_release_ordered();
-
+ release_context(qp.sched.sync);
CU_ASSERT_FATAL(odp_queue_destroy(queue) == 0);
}
@@ -820,12 +827,7 @@ static int schedule_common_(void *arg)
}
}
- if (sync == ODP_SCHED_SYNC_ATOMIC)
- odp_schedule_release_atomic();
-
- if (sync == ODP_SCHED_SYNC_ORDERED)
- odp_schedule_release_ordered();
-
+ release_context(sync);
odp_ticketlock_lock(&globals->lock);
globals->buf_count -= num;
--
2.7.4
Re: [lng-odp] [API-NEXT][RFC][rebased] linux-gen: ipsec: draft IPsec implementation
Next version should be marked API-NEXT, whether or not it is still an
RFC since IPsec APIs cannot move to master until we have a complete
implementation / validation test suite.
I realize this is an RFC, but it doesn't apply to the current api-next:
Applying: linux-gen: ipsec: draft IPsec implementation
Checking patch platform/linux-generic/include/odp_internal.h...
Checking patch platform/linux-generic/include/odp_ipsec_internal.h...
Checking patch platform/linux-generic/include/protocols/ip.h...
Checking patch platform/linux-generic/odp_event.c...
Checking patch platform/linux-generic/odp_init.c...
Checking patch platform/linux-generic/odp_ipsec.c...
error: while searching for:
*/
#include
#include
int odp_ipsec_capability(odp_ipsec_capability_t *capa)
{
memset(capa, 0, sizeof(odp_ipsec_capability_t));
return 0;
}
int odp_ipsec_cipher_capability(odp_cipher_alg_t cipher,
odp_crypto_cipher_capability_t capa[], int num)
{
(void)cipher;
(void)capa;
(void)num;
return -1;
}
int odp_ipsec_auth_capability(odp_auth_alg_t auth,
odp_crypto_auth_capability_t capa[], int num)
{
(void)auth;
(void)capa;
(void)num;
return -1;
}
void odp_ipsec_config_init(odp_ipsec_config_t *config)
{
memset(config, 0, sizeof(odp_ipsec_config_t));
}
int odp_ipsec_config(const odp_ipsec_config_t *config)
{
(void)config;
return -1;
}
void odp_ipsec_sa_param_init(odp_ipsec_sa_param_t *param)
{
memset(param, 0, sizeof(odp_ipsec_sa_param_t));
}
odp_ipsec_sa_t odp_ipsec_sa_create(const odp_ipsec_sa_param_t *param)
{
(void)param;
return ODP_IPSEC_SA_INVALID;
}
error: patch failed: platform/linux-generic/odp_ipsec.c:5
Hunk #2 succeeded at 68 (offset -336 lines).
Hunk #3 succeeded at 866 (offset -336 lines).
Hunk #4 succeeded at 916 (offset -336 lines).
Applied patch platform/linux-generic/include/odp_internal.h cleanly.
Applied patch platform/linux-generic/include/odp_ipsec_internal.h cleanly.
Applied patch platform/linux-generic/include/protocols/ip.h cleanly.
Applied patch platform/linux-generic/odp_event.c cleanly.
Applied patch platform/linux-generic/odp_init.c cleanly.
Applying patch platform/linux-generic/odp_ipsec.c with 1 reject...
Rejected hunk #1.
Hunk #2 applied cleanly.
Hunk #3 applied cleanly.
Hunk #4 applied cleanly.
Patch failed at 0001 linux-gen: ipsec: draft IPsec implementation
On Tue, Apr 11, 2017 at 8:41 AM, Dmitry Eremin-Solenikov
wrote:
> For now it's only a preview with the following limitation:
> - No inline processing support
> - No SA lookups
> - Only IPv4 support
> - No tunnel support
> - No header modification according to RFCs
>
> Signed-off-by: Dmitry Eremin-Solenikov
> ---
> platform/linux-generic/include/odp_internal.h |4 +
> .../linux-generic/include/odp_ipsec_internal.h | 71 ++
> platform/linux-generic/include/protocols/ip.h | 52 +
> platform/linux-generic/odp_event.c |5 +
> platform/linux-generic/odp_init.c | 13 +
> platform/linux-generic/odp_ipsec.c | 1172
> +++-
> 6 files changed, 1287 insertions(+), 30 deletions(-)
> create mode 100644 platform/linux-generic/include/odp_ipsec_internal.h
>
> diff --git a/platform/linux-generic/include/odp_internal.h
> b/platform/linux-generic/include/odp_internal.h
> index 05c8a422..fd7848ac 100644
> --- a/platform/linux-generic/include/odp_internal.h
> +++ b/platform/linux-generic/include/odp_internal.h
> @@ -71,6 +71,7 @@ enum init_stage {
> CLASSIFICATION_INIT,
> TRAFFIC_MNGR_INIT,
> NAME_TABLE_INIT,
> + IPSEC_INIT,
> MODULES_INIT,
> ALL_INIT /* All init stages completed */
> };
> @@ -130,6 +131,9 @@ int _odp_ishm_init_local(void);
> int _odp_ishm_term_global(void);
> int _odp_ishm_term_local(void);
>
> +int odp_ipsec_init_global(void);
> +int odp_ipsec_term_global(void);
> +
> int _odp_modules_init_global(void);
>
> int cpuinfo_parser(FILE *file, system_info_t *sysinfo);
> diff --git a/platform/linux-generic/include/odp_ipsec_internal.h
> b/platform/linux-generic/include/odp_ipsec_internal.h
> new file mode 100644
> index ..c7620b88
> --- /dev/null
> +++ b/platform/linux-generic/include/odp_ipsec_internal.h
> @@ -0,0 +1,71 @@
> +/* Copyright (c) 2017, Linaro Limited
> + * All rights reserved.
> + *
> + * SPDX-License-Identifier:BSD-3-Clause
> + */
> +
> +/**
> + * @file
> + *
> + * ODP internal IPsec routines
> + */
> +
> +#ifndef ODP_IPSEC_INTERNAL_H_
> +#define ODP_IPSEC_INTERNAL_H_
> +
> +#ifdef __cplusplus
> +extern "C" {
> +#endif
> +
> +#include
> +#include
> +
> +/** @ingroup odp_ipsec
> + * @{
> + */
> +
> +typedef ODP_HANDLE_T(odp_ipsec_op_result_event_t);
> +
> +#define ODP_IPSEC_OP_RESULT_EVENT_INVALID \
> + _odp_cast_scalar(odp_ipsec_op_result_event_t, 0x)
> +
> +/**
> + * Get ipsec_op_result_event handle from event
> + *
> + * Converts an ODP_EVENT_IPSEC_RESULT_EVENT type event to an IPsec result
> event.
> + *
> + * @param ev Event hand
[lng-odp] [Bug 2941] test bug for debug
https://bugs.linaro.org/show_bug.cgi?id=2941 Maxim Uvarov changed: What|Removed |Added Assignee|bill.fischo...@linaro.org |maxim.uva...@linaro.org -- You are receiving this mail because: You are on the CC list for the bug.
Re: [lng-odp] [PATCH] linux-gen: set errno = 0 before mmap
Just following the recommendations: "Set errno to zero before calling a
library function known to set errno, and check errno only after the
function returns a value indicating failure"
I think in general it is not a good idea to set errno after a library call.
In this case it could be done, but other functions behave differently. See
https://www.securecoding.cert.org/confluence/pages/viewpage.action?pageId=6619179
--
Oriol Arcas
Software Engineer
Starflow Networks
On Thu, Apr 13, 2017 at 8:55 PM, Maxim Uvarov
wrote:
> Why not just set errno to 0 in the success case?
>
> On 04/13/17 17:05, Oriol Arcas wrote:
> > Fixes bug #2921.
> >
> > During odp_shm_reserve() mmap is tried with huge pages, and if it fails,
> > without huge pages. The first call leaves errno set, which can confuse
> > the client app.
> >
> > In general, errno must be cleared before calling any library call known
> > to set it (see SEI CERT rule ERR30-C).
> >
> > Signed-off-by: Oriol Arcas
> > ---
> > platform/linux-generic/_ishmphy.c | 5 +
> > 1 file changed, 5 insertions(+)
> >
> > diff --git a/platform/linux-generic/_ishmphy.c
> b/platform/linux-generic/_ishmphy.c
> > index d519af6..87c33ef 100644
> > --- a/platform/linux-generic/_ishmphy.c
> > +++ b/platform/linux-generic/_ishmphy.c
> > @@ -121,10 +121,12 @@ void *_odp_ishmphy_map(int fd, void *start,
> uint64_t size,
> >* The initial free maping can then be removed.
> >*/
> > mapped_addr = MAP_FAILED;
> > + errno = 0;
> > mapped_addr_tmp = mmap(NULL, size, PROT_READ | PROT_WRITE,
> > MAP_SHARED | mmap_flags, fd, 0);
> > if (mapped_addr_tmp != MAP_FAILED) {
> > /* If OK, do new map at right fixed location... */
> > + errno = 0;
> > mapped_addr = mmap(start,
> > size, PROT_READ | PROT_WRITE,
> > MAP_SHARED | MAP_FIXED |
> mmap_flags,
> > @@ -132,11 +134,13 @@ void *_odp_ishmphy_map(int fd, void *start,
> uint64_t size,
> > if (mapped_addr != start)
> > ODP_ERR("new map failed:%s\n",
> strerror(errno));
> > /* ... and remove initial mapping: */
> > + errno = 0;
> > if (munmap(mapped_addr_tmp, size))
> > ODP_ERR("munmap failed:%s\n",
> strerror(errno));
> > }
> > } else {
> > /* just do a new mapping in the VA space: */
> > + errno = 0;
> > mapped_addr = mmap(NULL, size, PROT_READ | PROT_WRITE,
> > MAP_SHARED | mmap_flags, fd, 0);
> > if ((mapped_addr >= common_va_address) &&
> > @@ -154,6 +158,7 @@ void *_odp_ishmphy_map(int fd, void *start, uint64_t
> size,
> > /* if locking is requested, lock it...*/
> > if (flags & _ODP_ISHM_LOCK) {
> > if (mlock(mapped_addr, size)) {
> > + errno = 0;
> > if (munmap(mapped_addr, size))
> > ODP_ERR("munmap failed:%s\n",
> strerror(errno));
> > ODP_ERR("mlock failed:%s\n", strerror(errno));
> >
>
>
[lng-odp] [Bug 2941] test bug for debug
https://bugs.linaro.org/show_bug.cgi?id=2941 --- Comment #3 from Maxim Uvarov --- https://github.com/muvarov/odp/commit/420f9d5427a3f9f0424930061ea0e3480e818d82 2017-04-13T19:29:08+03:00 Maxim Uvarov maxim.uva...@linaro.org some test commit short message that commit fixes: https://bugs.linaro.org/show_bug.cgi?id=2941 bug. Yea haaa That has to work!!! Signed-off-by: Maxim Uvarov -- You are receiving this mail because: You are on the CC list for the bug.
[lng-odp] [Bug 2941] test bug for debug
https://bugs.linaro.org/show_bug.cgi?id=2941 --- Comment #2 from Maxim Uvarov --- https://github.com/muvarov/odp/commit/420f9d5427a3f9f0424930061ea0e3480e818d82 2017-04-13T19:29:08+03:00 Maxim Uvarov maxim.uva...@linaro.org some test commit short message that commit fixes: https://bugs.linaro.org/show_bug.cgi?id=2941 bug. Yea haaa That has to work!!! Signed-off-by: Maxim Uvarov -- You are receiving this mail because: You are on the CC list for the bug.
[lng-odp] [Bug 2941] test bug for debug
https://bugs.linaro.org/show_bug.cgi?id=2941 --- Comment #1 from Maxim Uvarov --- https://github.com/muvarov/odp/commit/420f9d5427a3f9f0424930061ea0e3480e818d82 2017-04-13T19:29:08+03:00 Maxim Uvarov maxim.uva...@linaro.org some test commit short message that commit fixes: https://bugs.linaro.org/show_bug.cgi?id=2941 bug. Yea haaa That has to work!!! Signed-off-by: Maxim Uvarov -- You are receiving this mail because: You are on the CC list for the bug.
Re: [lng-odp] [PATCH] linux-gen: set errno = 0 before mmap
Why not just set errno to 0 in the success case?
On 04/13/17 17:05, Oriol Arcas wrote:
> Fixes bug #2921.
>
> During odp_shm_reserve() mmap is tried with huge pages, and if it fails,
> without huge pages. The first call leaves errno set, which can confuse
> the client app.
>
> In general, errno must be cleared before calling any library call known
> to set it (see SEI CERT rule ERR30-C).
>
> Signed-off-by: Oriol Arcas
> ---
> platform/linux-generic/_ishmphy.c | 5 +
> 1 file changed, 5 insertions(+)
>
> diff --git a/platform/linux-generic/_ishmphy.c
> b/platform/linux-generic/_ishmphy.c
> index d519af6..87c33ef 100644
> --- a/platform/linux-generic/_ishmphy.c
> +++ b/platform/linux-generic/_ishmphy.c
> @@ -121,10 +121,12 @@ void *_odp_ishmphy_map(int fd, void *start, uint64_t
> size,
>* The initial free maping can then be removed.
>*/
> mapped_addr = MAP_FAILED;
> + errno = 0;
> mapped_addr_tmp = mmap(NULL, size, PROT_READ | PROT_WRITE,
> MAP_SHARED | mmap_flags, fd, 0);
> if (mapped_addr_tmp != MAP_FAILED) {
> /* If OK, do new map at right fixed location... */
> + errno = 0;
> mapped_addr = mmap(start,
> size, PROT_READ | PROT_WRITE,
> MAP_SHARED | MAP_FIXED | mmap_flags,
> @@ -132,11 +134,13 @@ void *_odp_ishmphy_map(int fd, void *start, uint64_t
> size,
> if (mapped_addr != start)
> ODP_ERR("new map failed:%s\n", strerror(errno));
> /* ... and remove initial mapping: */
> + errno = 0;
> if (munmap(mapped_addr_tmp, size))
> ODP_ERR("munmap failed:%s\n", strerror(errno));
> }
> } else {
> /* just do a new mapping in the VA space: */
> + errno = 0;
> mapped_addr = mmap(NULL, size, PROT_READ | PROT_WRITE,
> MAP_SHARED | mmap_flags, fd, 0);
> if ((mapped_addr >= common_va_address) &&
> @@ -154,6 +158,7 @@ void *_odp_ishmphy_map(int fd, void *start, uint64_t size,
> /* if locking is requested, lock it...*/
> if (flags & _ODP_ISHM_LOCK) {
> if (mlock(mapped_addr, size)) {
> + errno = 0;
> if (munmap(mapped_addr, size))
> ODP_ERR("munmap failed:%s\n", strerror(errno));
> ODP_ERR("mlock failed:%s\n", strerror(errno));
>
[lng-odp] [Bug 2941] New: test bug for debug
https://bugs.linaro.org/show_bug.cgi?id=2941 Bug ID: 2941 Summary: test bug for debug Product: OpenDataPlane - linux- generic reference Version: v1.14.0.0 Hardware: Other OS: Linux Status: UNCONFIRMED Severity: enhancement Priority: --- Component: API Assignee: bill.fischo...@linaro.org Reporter: maxim.uva...@linaro.org CC: lng-odp@lists.linaro.org Target Milestone: --- -- You are receiving this mail because: You are on the CC list for the bug.
Re: [lng-odp] [API-NEXT PATCH v4 1/2] api: queue: added queue size param
For the series:
Reviewed-by: Balasubramanian Manoharan
> On 13-Apr-2017, at 9:13 PM, Honnappa Nagarahalli
> wrote:
>
> For this series: Reviewed-by: Honnappa Nagarahalli
>
>
>> On 13 April 2017 at 10:14, Bill Fischofer wrote:
>> For this series: Reviewed-by: Bill Fischofer
>>
>> This should still have Honnappa's review as well.
>>
>> On Thu, Apr 13, 2017 at 9:40 AM, Petri Savolainen
>> wrote:
>>> Added capability information about maximum number of queues
>>> and queue sizes. Both are defined per queue type, since
>>> plain and scheduled queues may have different implementations
>>> (e.g. one uses HW while the other is SW).
>>>
>>> Added queue size parameter, which specifies how large
>>> storage size application requires in minimum.
>>>
>>> Signed-off-by: Petri Savolainen
>>> ---
>>> include/odp/api/spec/queue.h | 39 ++-
>>> 1 file changed, 38 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/include/odp/api/spec/queue.h b/include/odp/api/spec/queue.h
>>> index 7972feac..9dd0a561 100644
>>> --- a/include/odp/api/spec/queue.h
>>> +++ b/include/odp/api/spec/queue.h
>>> @@ -100,7 +100,9 @@ typedef enum odp_queue_op_mode_t {
>>> * Queue capabilities
>>> */
>>> typedef struct odp_queue_capability_t {
>>> - /** Maximum number of event queues */
>>> + /** Maximum number of event queues of any type (default size). Use
>>> + * this in addition to queue type specific 'max_num', if both queue
>>> + * types are used simultaneously. */
>>>uint32_t max_queues;
>>>
>>>/** Maximum number of ordered locks per queue */
>>> @@ -112,6 +114,32 @@ typedef struct odp_queue_capability_t {
>>>/** Number of scheduling priorities */
>>>unsigned sched_prios;
>>>
>>> + /** Plain queue capabilities */
>>> + struct {
>>> + /** Maximum number of plain queues of the default size. */
>>> + uint32_t max_num;
>>> +
>>> + /** Maximum number of events a plain queue can store
>>> + * simultaneously. The value of zero means that plain
>>> + * queues do not have a size limit, but a single queue can
>>> + * store all available events. */
>>> + uint32_t max_size;
>>> +
>>> + } plain;
>>> +
>>> + /** Scheduled queue capabilities */
>>> + struct {
>>> + /** Maximum number of scheduled queues of the default size.
>>> */
>>> + uint32_t max_num;
>>> +
>>> + /** Maximum number of events a scheduled queue can store
>>> + * simultaneously. The value of zero means that scheduled
>>> + * queues do not have a size limit, but a single queue can
>>> + * store all available events. */
>>> + uint32_t max_size;
>>> +
>>> + } sched;
>>> +
>>> } odp_queue_capability_t;
>>>
>>> /**
>>> @@ -165,6 +193,15 @@ typedef struct odp_queue_param_t {
>>> * The implementation may use this value as a hint for the number of
>>> * context data bytes to prefetch. Default value is zero (no hint).
>>> */
>>>uint32_t context_len;
>>> +
>>> + /** Queue size
>>> + *
>>> + * The queue must be able to store at minimum this many events
>>> + * simultaneously. The value must not exceed 'max_size' queue
>>> + * capability. The value of zero means implementation specific
>>> + * default size. */
>>> + uint32_t size;
>>> +
>>> } odp_queue_param_t;
>>>
>>> /**
>>> --
>>> 2.11.0
>>>
Re: [lng-odp] [API-NEXT PATCH v4 1/2] api: queue: added queue size param
Hi Maxim,
Appreciate if this can be merged quickly, it will unblock some
more patches that are dependent on queue size.
Thank you,
Honnappa
On 13 April 2017 at 10:43, Honnappa Nagarahalli
wrote:
> For this series: Reviewed-by: Honnappa Nagarahalli
>
>
> On 13 April 2017 at 10:14, Bill Fischofer wrote:
>> For this series: Reviewed-by: Bill Fischofer
>>
>> This should still have Honnappa's review as well.
>>
>> On Thu, Apr 13, 2017 at 9:40 AM, Petri Savolainen
>> wrote:
>>> Added capability information about maximum number of queues
>>> and queue sizes. Both are defined per queue type, since
>>> plain and scheduled queues may have different implementations
>>> (e.g. one uses HW while the other is SW).
>>>
>>> Added queue size parameter, which specifies how large
>>> storage size application requires in minimum.
>>>
>>> Signed-off-by: Petri Savolainen
>>> ---
>>> include/odp/api/spec/queue.h | 39 ++-
>>> 1 file changed, 38 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/include/odp/api/spec/queue.h b/include/odp/api/spec/queue.h
>>> index 7972feac..9dd0a561 100644
>>> --- a/include/odp/api/spec/queue.h
>>> +++ b/include/odp/api/spec/queue.h
>>> @@ -100,7 +100,9 @@ typedef enum odp_queue_op_mode_t {
>>> * Queue capabilities
>>> */
>>> typedef struct odp_queue_capability_t {
>>> - /** Maximum number of event queues */
>>> + /** Maximum number of event queues of any type (default size). Use
>>> + * this in addition to queue type specific 'max_num', if both queue
>>> + * types are used simultaneously. */
>>> uint32_t max_queues;
>>>
>>> /** Maximum number of ordered locks per queue */
>>> @@ -112,6 +114,32 @@ typedef struct odp_queue_capability_t {
>>> /** Number of scheduling priorities */
>>> unsigned sched_prios;
>>>
>>> + /** Plain queue capabilities */
>>> + struct {
>>> + /** Maximum number of plain queues of the default size. */
>>> + uint32_t max_num;
>>> +
>>> + /** Maximum number of events a plain queue can store
>>> + * simultaneously. The value of zero means that plain
>>> + * queues do not have a size limit, but a single queue can
>>> + * store all available events. */
>>> + uint32_t max_size;
>>> +
>>> + } plain;
>>> +
>>> + /** Scheduled queue capabilities */
>>> + struct {
>>> + /** Maximum number of scheduled queues of the default size.
>>> */
>>> + uint32_t max_num;
>>> +
>>> + /** Maximum number of events a scheduled queue can store
>>> + * simultaneously. The value of zero means that scheduled
>>> + * queues do not have a size limit, but a single queue can
>>> + * store all available events. */
>>> + uint32_t max_size;
>>> +
>>> + } sched;
>>> +
>>> } odp_queue_capability_t;
>>>
>>> /**
>>> @@ -165,6 +193,15 @@ typedef struct odp_queue_param_t {
>>> * The implementation may use this value as a hint for the number
>>> of
>>> * context data bytes to prefetch. Default value is zero (no
>>> hint). */
>>> uint32_t context_len;
>>> +
>>> + /** Queue size
>>> + *
>>> + * The queue must be able to store at minimum this many events
>>> + * simultaneously. The value must not exceed 'max_size' queue
>>> + * capability. The value of zero means implementation specific
>>> + * default size. */
>>> + uint32_t size;
>>> +
>>> } odp_queue_param_t;
>>>
>>> /**
>>> --
>>> 2.11.0
>>>
Re: [lng-odp] [PATCHv2] validation: scheduler: modify the queue size for atomic queue
We need the ring size patch to go in first and then you can review this patch. Thank you, Honnappa On 11 April 2017 at 02:43, Kevin Wang wrote: > Thanks for remind. Awared that if we change it using this way, this patch > should be in the scalable scheduler patch serial based on api-next. > > Kevin > > 2017-04-11 15:37 GMT+08:00 Savolainen, Petri (Nokia - FI/Espoo) < > petri.savolai...@nokia-bell-labs.com>: > >> >> >> > -Original Message- >> > From: lng-odp [mailto:lng-odp-boun...@lists.linaro.org] On Behalf Of >> Kevin >> > Wang >> > Sent: Tuesday, April 11, 2017 5:38 AM >> > To: lng-odp@lists.linaro.org >> > Cc: Kevin Wang >> > Subject: [lng-odp] [PATCHv2] validation: scheduler: modify the queue size >> > for atomic queue >> > >> > Signed-off-by: Kevin Wang >> > --- >> > test/common_plat/validation/api/scheduler/scheduler.c | 2 ++ >> > 1 file changed, 2 insertions(+) >> > >> > diff --git a/test/common_plat/validation/api/scheduler/scheduler.c >> > b/test/common_plat/validation/api/scheduler/scheduler.c >> > index 952561c..f46d391 100644 >> > --- a/test/common_plat/validation/api/scheduler/scheduler.c >> > +++ b/test/common_plat/validation/api/scheduler/scheduler.c >> > @@ -1388,6 +1388,7 @@ static int create_queues(void) >> > >> > snprintf(name, sizeof(name), >> > "sched_%d_%d_a", i, j); >> > p.sched.sync = ODP_SCHED_SYNC_ATOMIC; >> > + p.size = BUFS_PER_QUEUE_EXCL; >> >> >> Does this build on top of current master ? Even api-next do not have yet >> the queue size parameter defined. >> >> -Petri >> >> >> > > > -- > Thanks, > Kevin
Re: [lng-odp] [API-NEXT PATCH v4 1/2] api: queue: added queue size param
For this series: Reviewed-by: Honnappa Nagarahalli
On 13 April 2017 at 10:14, Bill Fischofer wrote:
> For this series: Reviewed-by: Bill Fischofer
>
> This should still have Honnappa's review as well.
>
> On Thu, Apr 13, 2017 at 9:40 AM, Petri Savolainen
> wrote:
>> Added capability information about maximum number of queues
>> and queue sizes. Both are defined per queue type, since
>> plain and scheduled queues may have different implementations
>> (e.g. one uses HW while the other is SW).
>>
>> Added queue size parameter, which specifies how large
>> storage size application requires in minimum.
>>
>> Signed-off-by: Petri Savolainen
>> ---
>> include/odp/api/spec/queue.h | 39 ++-
>> 1 file changed, 38 insertions(+), 1 deletion(-)
>>
>> diff --git a/include/odp/api/spec/queue.h b/include/odp/api/spec/queue.h
>> index 7972feac..9dd0a561 100644
>> --- a/include/odp/api/spec/queue.h
>> +++ b/include/odp/api/spec/queue.h
>> @@ -100,7 +100,9 @@ typedef enum odp_queue_op_mode_t {
>> * Queue capabilities
>> */
>> typedef struct odp_queue_capability_t {
>> - /** Maximum number of event queues */
>> + /** Maximum number of event queues of any type (default size). Use
>> + * this in addition to queue type specific 'max_num', if both queue
>> + * types are used simultaneously. */
>> uint32_t max_queues;
>>
>> /** Maximum number of ordered locks per queue */
>> @@ -112,6 +114,32 @@ typedef struct odp_queue_capability_t {
>> /** Number of scheduling priorities */
>> unsigned sched_prios;
>>
>> + /** Plain queue capabilities */
>> + struct {
>> + /** Maximum number of plain queues of the default size. */
>> + uint32_t max_num;
>> +
>> + /** Maximum number of events a plain queue can store
>> + * simultaneously. The value of zero means that plain
>> + * queues do not have a size limit, but a single queue can
>> + * store all available events. */
>> + uint32_t max_size;
>> +
>> + } plain;
>> +
>> + /** Scheduled queue capabilities */
>> + struct {
>> + /** Maximum number of scheduled queues of the default size.
>> */
>> + uint32_t max_num;
>> +
>> + /** Maximum number of events a scheduled queue can store
>> + * simultaneously. The value of zero means that scheduled
>> + * queues do not have a size limit, but a single queue can
>> + * store all available events. */
>> + uint32_t max_size;
>> +
>> + } sched;
>> +
>> } odp_queue_capability_t;
>>
>> /**
>> @@ -165,6 +193,15 @@ typedef struct odp_queue_param_t {
>> * The implementation may use this value as a hint for the number of
>> * context data bytes to prefetch. Default value is zero (no hint).
>> */
>> uint32_t context_len;
>> +
>> + /** Queue size
>> + *
>> + * The queue must be able to store at minimum this many events
>> + * simultaneously. The value must not exceed 'max_size' queue
>> + * capability. The value of zero means implementation specific
>> + * default size. */
>> + uint32_t size;
>> +
>> } odp_queue_param_t;
>>
>> /**
>> --
>> 2.11.0
>>
Re: [lng-odp] [PATCH] validation: scheduler: Release context before the end of the scheduler test
We will re-spin this patch with the following changes: 1) Remove the extra context release introduced in 'scheduler_test_groups' 2) Remove the 'CU_ASSERT_FATAL(ret == 0)' change (or restore it back) New API will not be handled as part of this patch. Any concerns? Thanks, Honnappa On 10 April 2017 at 02:20, Savolainen, Petri (Nokia - FI/Espoo) wrote: > > > From: Kevin Wang [mailto:kevin.w...@linaro.org] > Sent: Monday, April 10, 2017 5:47 AM > To: Bill Fischofer > Cc: Ola Liljedahl ; Savolainen, Petri (Nokia - > FI/Espoo) ; Kevin Wang > ; lng-odp@lists.linaro.org > Subject: Re: [lng-odp] [PATCH] validation: scheduler: Release context before > the end of the scheduler test > > So seems a new API odp_schedule_release_context() is required. So I'll > redirect the patch to api-next. Should it be an internal function? > > Your scheduler or this patch set does not require a new API function. So, > better keep it simple and focus on master branch only. Common release_ctx may > be added into the API later on. It has (may have) more overhead than correct > usage of atomic/ordered release calls. > > -Petri > > >
Re: [lng-odp] [API-NEXT PATCH v4 1/2] api: queue: added queue size param
For this series: Reviewed-by: Bill Fischofer
This should still have Honnappa's review as well.
On Thu, Apr 13, 2017 at 9:40 AM, Petri Savolainen
wrote:
> Added capability information about maximum number of queues
> and queue sizes. Both are defined per queue type, since
> plain and scheduled queues may have different implementations
> (e.g. one uses HW while the other is SW).
>
> Added queue size parameter, which specifies how large
> storage size application requires in minimum.
>
> Signed-off-by: Petri Savolainen
> ---
> include/odp/api/spec/queue.h | 39 ++-
> 1 file changed, 38 insertions(+), 1 deletion(-)
>
> diff --git a/include/odp/api/spec/queue.h b/include/odp/api/spec/queue.h
> index 7972feac..9dd0a561 100644
> --- a/include/odp/api/spec/queue.h
> +++ b/include/odp/api/spec/queue.h
> @@ -100,7 +100,9 @@ typedef enum odp_queue_op_mode_t {
> * Queue capabilities
> */
> typedef struct odp_queue_capability_t {
> - /** Maximum number of event queues */
> + /** Maximum number of event queues of any type (default size). Use
> + * this in addition to queue type specific 'max_num', if both queue
> + * types are used simultaneously. */
> uint32_t max_queues;
>
> /** Maximum number of ordered locks per queue */
> @@ -112,6 +114,32 @@ typedef struct odp_queue_capability_t {
> /** Number of scheduling priorities */
> unsigned sched_prios;
>
> + /** Plain queue capabilities */
> + struct {
> + /** Maximum number of plain queues of the default size. */
> + uint32_t max_num;
> +
> + /** Maximum number of events a plain queue can store
> + * simultaneously. The value of zero means that plain
> + * queues do not have a size limit, but a single queue can
> + * store all available events. */
> + uint32_t max_size;
> +
> + } plain;
> +
> + /** Scheduled queue capabilities */
> + struct {
> + /** Maximum number of scheduled queues of the default size. */
> + uint32_t max_num;
> +
> + /** Maximum number of events a scheduled queue can store
> + * simultaneously. The value of zero means that scheduled
> + * queues do not have a size limit, but a single queue can
> + * store all available events. */
> + uint32_t max_size;
> +
> + } sched;
> +
> } odp_queue_capability_t;
>
> /**
> @@ -165,6 +193,15 @@ typedef struct odp_queue_param_t {
> * The implementation may use this value as a hint for the number of
> * context data bytes to prefetch. Default value is zero (no hint).
> */
> uint32_t context_len;
> +
> + /** Queue size
> + *
> + * The queue must be able to store at minimum this many events
> + * simultaneously. The value must not exceed 'max_size' queue
> + * capability. The value of zero means implementation specific
> + * default size. */
> + uint32_t size;
> +
> } odp_queue_param_t;
>
> /**
> --
> 2.11.0
>
Re: [lng-odp] [API-NEXT PATCH v2 0/4] Deprecated macros
On Thu, Apr 13, 2017 at 9:07 AM, Savolainen, Petri (Nokia - FI/Espoo) wrote: > > >> -Original Message- >> From: Dmitry Eremin-Solenikov [mailto:dmitry.ereminsoleni...@linaro.org] >> Sent: Thursday, April 13, 2017 1:57 PM >> To: Savolainen, Petri (Nokia - FI/Espoo) > labs.com>; lng-odp@lists.linaro.org >> Subject: Re: [lng-odp] [API-NEXT PATCH v2 0/4] Deprecated macros >> >> On 13.04.2017 10:33, Savolainen, Petri (Nokia - FI/Espoo) wrote: >> > >> > >> >> -Original Message- >> >> From: Dmitry Eremin-Solenikov >> [mailto:dmitry.ereminsoleni...@linaro.org] >> >> Sent: Wednesday, April 12, 2017 3:11 PM >> >> To: Savolainen, Petri (Nokia - FI/Espoo) > >> labs.com>; lng-odp@lists.linaro.org >> >> Subject: Re: [lng-odp] [API-NEXT PATCH v2 0/4] Deprecated macros >> >> >> >> On 12.04.2017 14:50, Savolainen, Petri (Nokia - FI/Espoo) wrote: >> >>> >> >>> >> -Original Message- >> From: Dmitry Eremin-Solenikov >> >> [mailto:dmitry.ereminsoleni...@linaro.org] >> Sent: Wednesday, April 12, 2017 2:32 PM >> To: Petri Savolainen ; lng- >> o...@lists.linaro.org >> Subject: Re: [lng-odp] [API-NEXT PATCH v2 0/4] Deprecated macros >> >> On 30.03.2017 16:58, Petri Savolainen wrote: >> > Replaced ODP_DEPRECATED macro (which was based on GCC __attribute__) >> with >> > compiler independent mechanism to control if deprecated API >> >> definitions >> are >> > visible to the application. ODP_DEPRECATED_API can be used both in >> application >> > and implementation to check if deprecated APIs are enabled. By >> default >> those are >> > disabled. Implementation may optimize the normal (new API) code >> path. >> > >> > ODP_DEPRECATE() macro is used to rename definitions, so that data >> structure >> > sizes are equal on both options. This enables implementation to >> serve >> both >> > options with a single library (if it wishes to do so). >> >> My main question remains as it was before: is it possible for the >> distribution to supply (unoptimized) ODP binary and headers and then >> >> for >> the application to select if it builds with or without deprecated API >> using that binary/headers? >> >>> >> >>> >> >>> Yes. This patch set keeps the same struct fields/etc for both modes - >> >> only the names are scrambled (with __deprecated_ prefix) when >> deprecated >> >> APIs are not supported (the default). >> >> >> >> If so. Consider I have built and installed ODP headers & binary built >> >> with --enable-deprecated-api. >> > >> > So, you have built and installed the implementation with --enable- >> deprecated-api. This means that applications using this install, see >> deprecated APIs. A distribution decides which way it supports ODP, with or >> without deprecated stuff. The default in our (odp-linux) configure is >> without, to minimize confusion and promote the new API definitions which >> should be always better for application than the old ones. >> > >> >> I was advocating for binary distributions and developers using packages >> from those distributions. And for those distributions providing _single_ >> binary version of ODP. > > Obviously, a distribution would select which ODP API versions are supported, > and if those versions include deprecated APIs or not. Deprecation is feature > of API spec and thus header files. A header file cannot be in both modes at > the same time. E.g. a struct either has a field e.g. int foo or int > _deprecated_foo, but not both at the same time. > > A single library can support both modes since struct sizes are the same > (_deprecated_foo is left with default values since application does not > read/write it). > > Now, if a distribution dictates that deprecated APIs are supported, an > implementation delivers headers with deprecated stuff and a library that > supports those. The same happens in the opposite case (headers without > deprecated stuff and an library that supports those). Depending on > implementation the library binary may be same or different. Distribution does > not care, since it needs to store headers and pairing libs for those in any > case. > > We do not guarantee binary compatibility over different API versions. > Application, API headers and implementation lib need to be on the same API > version - with or without deprecated stuff. Good summary. Having ODP part of a distro simply means that someone has installed it in the library catalog associated with that distro and configured it however they wished. At that point it's either part of the default install or can be installed with apt-get, yum, etc. What gets installed are the include files that allow applications to compile/link against that ODP installation, and the associated .so files that provide the runtime support for these apps. If I look on my Ubuntu 16.10 system I see /usr/include/clang and under that are several different release options (3.6, 3.6.3,
[lng-odp] [PATCH v4] example: add IPv4 fragmentation/reassembly example
Add an example application implementing lock-free IPv4 fragmentation
and reassembly functionality using ODP's packet "concat" and "split".
Signed-off-by: Joe Savage
Reviewed-and-tested-by: Bill Fischofer
---
(This code contribution is provided under the terms of agreement LES-LTM-21309)
doc/application-api-guide/examples.dox| 5 +
example/Makefile.am | 1 +
example/ipfragreass/.gitignore| 3 +
example/ipfragreass/Makefile.am | 23 +
example/ipfragreass/odp_ipfragreass.c | 376 +++
example/ipfragreass/odp_ipfragreass_atomics.h | 55 ++
example/ipfragreass/odp_ipfragreass_atomics_arm.h | 120
example/ipfragreass/odp_ipfragreass_fragment.c| 99 +++
example/ipfragreass/odp_ipfragreass_fragment.h| 28 +
example/ipfragreass/odp_ipfragreass_helpers.c | 124
example/ipfragreass/odp_ipfragreass_helpers.h | 79 +++
example/ipfragreass/odp_ipfragreass_ip.h | 251 +++
example/ipfragreass/odp_ipfragreass_reassemble.c | 771 ++
example/ipfragreass/odp_ipfragreass_reassemble.h | 211 ++
example/m4/configure.m4 | 1 +
15 files changed, 2147 insertions(+)
create mode 100644 example/ipfragreass/.gitignore
create mode 100644 example/ipfragreass/Makefile.am
create mode 100644 example/ipfragreass/odp_ipfragreass.c
create mode 100644 example/ipfragreass/odp_ipfragreass_atomics.h
create mode 100644 example/ipfragreass/odp_ipfragreass_atomics_arm.h
create mode 100644 example/ipfragreass/odp_ipfragreass_fragment.c
create mode 100644 example/ipfragreass/odp_ipfragreass_fragment.h
create mode 100644 example/ipfragreass/odp_ipfragreass_helpers.c
create mode 100644 example/ipfragreass/odp_ipfragreass_helpers.h
create mode 100644 example/ipfragreass/odp_ipfragreass_ip.h
create mode 100644 example/ipfragreass/odp_ipfragreass_reassemble.c
create mode 100644 example/ipfragreass/odp_ipfragreass_reassemble.h
diff --git a/doc/application-api-guide/examples.dox
b/doc/application-api-guide/examples.dox
index 60d4058..80fe467 100644
--- a/doc/application-api-guide/examples.dox
+++ b/doc/application-api-guide/examples.dox
@@ -28,3 +28,8 @@
* @example odp_timer_test.c
* ODP timer example application
*/
+
+ /**
+ * @example odp_ipfragreass.c
+ * ODP IPv4 lock-free fragmentation and reassembly example application
+ */
diff --git a/example/Makefile.am b/example/Makefile.am
index dfc07b6..9503a1b 100644
--- a/example/Makefile.am
+++ b/example/Makefile.am
@@ -2,6 +2,7 @@ SUBDIRS = classifier \
generator \
hello \
ipsec \
+ ipfragreass \
l2fwd_simple \
l3fwd \
packet \
diff --git a/example/ipfragreass/.gitignore b/example/ipfragreass/.gitignore
new file mode 100644
index 000..d25d758
--- /dev/null
+++ b/example/ipfragreass/.gitignore
@@ -0,0 +1,3 @@
+odp_ipfragreass
+*.log
+*.trs
diff --git a/example/ipfragreass/Makefile.am b/example/ipfragreass/Makefile.am
new file mode 100644
index 000..f805a69
--- /dev/null
+++ b/example/ipfragreass/Makefile.am
@@ -0,0 +1,23 @@
+include $(top_srcdir)/example/Makefile.inc
+
+bin_PROGRAMS = odp_ipfragreass$(EXEEXT)
+odp_ipfragreass_LDFLAGS = $(AM_LDFLAGS) -static
+odp_ipfragreass_CFLAGS = $(AM_CFLAGS) -I${top_srcdir}/example
+
+noinst_HEADERS = \
+ $(top_srcdir)/example/ipfragreass/odp_ipfragreass_atomics.h \
+
$(top_srcdir)/example/ipfragreass/odp_ipfragreass_atomics_arm.h \
+ $(top_srcdir)/example/ipfragreass/odp_ipfragreass_fragment.h \
+ $(top_srcdir)/example/ipfragreass/odp_ipfragreass_helpers.h \
+ $(top_srcdir)/example/ipfragreass/odp_ipfragreass_ip.h \
+
$(top_srcdir)/example/ipfragreass/odp_ipfragreass_reassemble.h \
+ $(top_srcdir)/example/example_debug.h
+
+dist_odp_ipfragreass_SOURCES = odp_ipfragreass.c \
+ odp_ipfragreass_fragment.c \
+ odp_ipfragreass_helpers.c \
+ odp_ipfragreass_reassemble.c
+
+if test_example
+TESTS = odp_ipfragreass
+endif
diff --git a/example/ipfragreass/odp_ipfragreass.c
b/example/ipfragreass/odp_ipfragreass.c
new file mode 100644
index 000..6ad0db9
--- /dev/null
+++ b/example/ipfragreass/odp_ipfragreass.c
@@ -0,0 +1,376 @@
+/* Copyright (c) 2017, Linaro Limited
+ * All rights reserved.
+ *
+ * SPDX-License-Identifier: BSD-3-Clause
+ */
+
+/**
+ * @file
+ *
+ * @example odp_ipfragreass.c ODP IPv4 lock-free fragmentation and reassembly
+ */
+
+#include
+#include
+#include
+#include
+
+#include
+#include
+
+#include "odp_ipfragreass_fragment.h"
+#include "odp_ipfragreass_reassemble.h"
+#include "odp_ipfragreass_helpers.h"
+
+#define NUM_PACKETS 200 /**< Number of packets to fragment/reassemble */
+#define MAX_WORKERS 32/**< Maximum number of worker threads *
[lng-odp] [API-NEXT PATCH v4 1/2] api: queue: added queue size param
Added capability information about maximum number of queues
and queue sizes. Both are defined per queue type, since
plain and scheduled queues may have different implementations
(e.g. one uses HW while the other is SW).
Added queue size parameter, which specifies how large
storage size application requires in minimum.
Signed-off-by: Petri Savolainen
---
include/odp/api/spec/queue.h | 39 ++-
1 file changed, 38 insertions(+), 1 deletion(-)
diff --git a/include/odp/api/spec/queue.h b/include/odp/api/spec/queue.h
index 7972feac..9dd0a561 100644
--- a/include/odp/api/spec/queue.h
+++ b/include/odp/api/spec/queue.h
@@ -100,7 +100,9 @@ typedef enum odp_queue_op_mode_t {
* Queue capabilities
*/
typedef struct odp_queue_capability_t {
- /** Maximum number of event queues */
+ /** Maximum number of event queues of any type (default size). Use
+ * this in addition to queue type specific 'max_num', if both queue
+ * types are used simultaneously. */
uint32_t max_queues;
/** Maximum number of ordered locks per queue */
@@ -112,6 +114,32 @@ typedef struct odp_queue_capability_t {
/** Number of scheduling priorities */
unsigned sched_prios;
+ /** Plain queue capabilities */
+ struct {
+ /** Maximum number of plain queues of the default size. */
+ uint32_t max_num;
+
+ /** Maximum number of events a plain queue can store
+ * simultaneously. The value of zero means that plain
+ * queues do not have a size limit, but a single queue can
+ * store all available events. */
+ uint32_t max_size;
+
+ } plain;
+
+ /** Scheduled queue capabilities */
+ struct {
+ /** Maximum number of scheduled queues of the default size. */
+ uint32_t max_num;
+
+ /** Maximum number of events a scheduled queue can store
+ * simultaneously. The value of zero means that scheduled
+ * queues do not have a size limit, but a single queue can
+ * store all available events. */
+ uint32_t max_size;
+
+ } sched;
+
} odp_queue_capability_t;
/**
@@ -165,6 +193,15 @@ typedef struct odp_queue_param_t {
* The implementation may use this value as a hint for the number of
* context data bytes to prefetch. Default value is zero (no hint). */
uint32_t context_len;
+
+ /** Queue size
+ *
+ * The queue must be able to store at minimum this many events
+ * simultaneously. The value must not exceed 'max_size' queue
+ * capability. The value of zero means implementation specific
+ * default size. */
+ uint32_t size;
+
} odp_queue_param_t;
/**
--
2.11.0
[lng-odp] [API-NEXT PATCH v4 2/2] validation: queue: test queue max_num per type
Updated implementation and test with type specific number of
queues.
Signed-off-by: Petri Savolainen
---
platform/linux-generic/odp_queue.c| 2 ++
test/common_plat/validation/api/queue/queue.c | 49 +--
2 files changed, 34 insertions(+), 17 deletions(-)
diff --git a/platform/linux-generic/odp_queue.c
b/platform/linux-generic/odp_queue.c
index fcf4bf5b..1114c95c 100644
--- a/platform/linux-generic/odp_queue.c
+++ b/platform/linux-generic/odp_queue.c
@@ -175,6 +175,8 @@ int odp_queue_capability(odp_queue_capability_t *capa)
capa->max_ordered_locks = sched_fn->max_ordered_locks();
capa->max_sched_groups = sched_fn->num_grps();
capa->sched_prios = odp_schedule_num_prio();
+ capa->plain.max_num = capa->max_queues;
+ capa->sched.max_num = capa->max_queues;
return 0;
}
diff --git a/test/common_plat/validation/api/queue/queue.c
b/test/common_plat/validation/api/queue/queue.c
index 1f7913a1..6a13c006 100644
--- a/test/common_plat/validation/api/queue/queue.c
+++ b/test/common_plat/validation/api/queue/queue.c
@@ -56,7 +56,7 @@ void queue_test_capa(void)
odp_queue_param_t qparams;
char name[ODP_QUEUE_NAME_LEN];
odp_queue_t queue[MAX_QUEUES];
- uint32_t num_queues, i;
+ uint32_t num_queues, min, i, j;
memset(&capa, 0, sizeof(odp_queue_capability_t));
CU_ASSERT(odp_queue_capability(&capa) == 0);
@@ -65,34 +65,49 @@ void queue_test_capa(void)
CU_ASSERT(capa.max_ordered_locks != 0);
CU_ASSERT(capa.max_sched_groups != 0);
CU_ASSERT(capa.sched_prios != 0);
+ CU_ASSERT(capa.plain.max_num != 0);
+ CU_ASSERT(capa.sched.max_num != 0);
+
+ min = capa.plain.max_num;
+ if (min > capa.sched.max_num)
+ min = capa.sched.max_num;
+
+ CU_ASSERT(capa.max_queues >= min);
for (i = 0; i < ODP_QUEUE_NAME_LEN; i++)
name[i] = 'A' + (i % 26);
name[ODP_QUEUE_NAME_LEN - 1] = 0;
- if (capa.max_queues > MAX_QUEUES)
- num_queues = MAX_QUEUES;
- else
- num_queues = capa.max_queues;
-
odp_queue_param_init(&qparams);
- for (i = 0; i < num_queues; i++) {
- generate_name(name, i);
- queue[i] = odp_queue_create(name, &qparams);
+ for (j = 0; j < 2; j++) {
+ if (j == 0) {
+ num_queues = capa.plain.max_num;
+ } else {
+ num_queues = capa.sched.max_num;
+ qparams.type = ODP_QUEUE_TYPE_SCHED;
+ }
+
+ if (num_queues > MAX_QUEUES)
+ num_queues = MAX_QUEUES;
- if (queue[i] == ODP_QUEUE_INVALID) {
- CU_FAIL("Queue create failed");
- num_queues = i;
- break;
+ for (i = 0; i < num_queues; i++) {
+ generate_name(name, i);
+ queue[i] = odp_queue_create(name, &qparams);
+
+ if (queue[i] == ODP_QUEUE_INVALID) {
+ CU_FAIL("Queue create failed");
+ num_queues = i;
+ break;
+ }
+
+ CU_ASSERT(odp_queue_lookup(name) != ODP_QUEUE_INVALID);
}
- CU_ASSERT(odp_queue_lookup(name) != ODP_QUEUE_INVALID);
+ for (i = 0; i < num_queues; i++)
+ CU_ASSERT(odp_queue_destroy(queue[i]) == 0);
}
-
- for (i = 0; i < num_queues; i++)
- CU_ASSERT(odp_queue_destroy(queue[i]) == 0);
}
void queue_test_mode(void)
--
2.11.0
Re: [lng-odp] [API-NEXT PATCH v2 0/4] Deprecated macros
> -Original Message- > From: Dmitry Eremin-Solenikov [mailto:dmitry.ereminsoleni...@linaro.org] > Sent: Thursday, April 13, 2017 1:57 PM > To: Savolainen, Petri (Nokia - FI/Espoo) labs.com>; lng-odp@lists.linaro.org > Subject: Re: [lng-odp] [API-NEXT PATCH v2 0/4] Deprecated macros > > On 13.04.2017 10:33, Savolainen, Petri (Nokia - FI/Espoo) wrote: > > > > > >> -Original Message- > >> From: Dmitry Eremin-Solenikov > [mailto:dmitry.ereminsoleni...@linaro.org] > >> Sent: Wednesday, April 12, 2017 3:11 PM > >> To: Savolainen, Petri (Nokia - FI/Espoo) >> labs.com>; lng-odp@lists.linaro.org > >> Subject: Re: [lng-odp] [API-NEXT PATCH v2 0/4] Deprecated macros > >> > >> On 12.04.2017 14:50, Savolainen, Petri (Nokia - FI/Espoo) wrote: > >>> > >>> > -Original Message- > From: Dmitry Eremin-Solenikov > >> [mailto:dmitry.ereminsoleni...@linaro.org] > Sent: Wednesday, April 12, 2017 2:32 PM > To: Petri Savolainen ; lng- > o...@lists.linaro.org > Subject: Re: [lng-odp] [API-NEXT PATCH v2 0/4] Deprecated macros > > On 30.03.2017 16:58, Petri Savolainen wrote: > > Replaced ODP_DEPRECATED macro (which was based on GCC __attribute__) > with > > compiler independent mechanism to control if deprecated API > >> definitions > are > > visible to the application. ODP_DEPRECATED_API can be used both in > application > > and implementation to check if deprecated APIs are enabled. By > default > those are > > disabled. Implementation may optimize the normal (new API) code > path. > > > > ODP_DEPRECATE() macro is used to rename definitions, so that data > structure > > sizes are equal on both options. This enables implementation to > serve > both > > options with a single library (if it wishes to do so). > > My main question remains as it was before: is it possible for the > distribution to supply (unoptimized) ODP binary and headers and then > >> for > the application to select if it builds with or without deprecated API > using that binary/headers? > >>> > >>> > >>> Yes. This patch set keeps the same struct fields/etc for both modes - > >> only the names are scrambled (with __deprecated_ prefix) when > deprecated > >> APIs are not supported (the default). > >> > >> If so. Consider I have built and installed ODP headers & binary built > >> with --enable-deprecated-api. > > > > So, you have built and installed the implementation with --enable- > deprecated-api. This means that applications using this install, see > deprecated APIs. A distribution decides which way it supports ODP, with or > without deprecated stuff. The default in our (odp-linux) configure is > without, to minimize confusion and promote the new API definitions which > should be always better for application than the old ones. > > > > I was advocating for binary distributions and developers using packages > from those distributions. And for those distributions providing _single_ > binary version of ODP. Obviously, a distribution would select which ODP API versions are supported, and if those versions include deprecated APIs or not. Deprecation is feature of API spec and thus header files. A header file cannot be in both modes at the same time. E.g. a struct either has a field e.g. int foo or int _deprecated_foo, but not both at the same time. A single library can support both modes since struct sizes are the same (_deprecated_foo is left with default values since application does not read/write it). Now, if a distribution dictates that deprecated APIs are supported, an implementation delivers headers with deprecated stuff and a library that supports those. The same happens in the opposite case (headers without deprecated stuff and an library that supports those). Depending on implementation the library binary may be same or different. Distribution does not care, since it needs to store headers and pairing libs for those in any case. We do not guarantee binary compatibility over different API versions. Application, API headers and implementation lib need to be on the same API version - with or without deprecated stuff. -Petri
[lng-odp] [PATCH] linux-gen: set errno = 0 before mmap
Fixes bug #2921.
During odp_shm_reserve() mmap is tried with huge pages, and if it fails,
without huge pages. The first call leaves errno set, which can confuse
the client app.
In general, errno must be cleared before calling any library call known
to set it (see SEI CERT rule ERR30-C).
Signed-off-by: Oriol Arcas
---
platform/linux-generic/_ishmphy.c | 5 +
1 file changed, 5 insertions(+)
diff --git a/platform/linux-generic/_ishmphy.c
b/platform/linux-generic/_ishmphy.c
index d519af6..87c33ef 100644
--- a/platform/linux-generic/_ishmphy.c
+++ b/platform/linux-generic/_ishmphy.c
@@ -121,10 +121,12 @@ void *_odp_ishmphy_map(int fd, void *start, uint64_t size,
* The initial free maping can then be removed.
*/
mapped_addr = MAP_FAILED;
+ errno = 0;
mapped_addr_tmp = mmap(NULL, size, PROT_READ | PROT_WRITE,
MAP_SHARED | mmap_flags, fd, 0);
if (mapped_addr_tmp != MAP_FAILED) {
/* If OK, do new map at right fixed location... */
+ errno = 0;
mapped_addr = mmap(start,
size, PROT_READ | PROT_WRITE,
MAP_SHARED | MAP_FIXED | mmap_flags,
@@ -132,11 +134,13 @@ void *_odp_ishmphy_map(int fd, void *start, uint64_t size,
if (mapped_addr != start)
ODP_ERR("new map failed:%s\n", strerror(errno));
/* ... and remove initial mapping: */
+ errno = 0;
if (munmap(mapped_addr_tmp, size))
ODP_ERR("munmap failed:%s\n", strerror(errno));
}
} else {
/* just do a new mapping in the VA space: */
+ errno = 0;
mapped_addr = mmap(NULL, size, PROT_READ | PROT_WRITE,
MAP_SHARED | mmap_flags, fd, 0);
if ((mapped_addr >= common_va_address) &&
@@ -154,6 +158,7 @@ void *_odp_ishmphy_map(int fd, void *start, uint64_t size,
/* if locking is requested, lock it...*/
if (flags & _ODP_ISHM_LOCK) {
if (mlock(mapped_addr, size)) {
+ errno = 0;
if (munmap(mapped_addr, size))
ODP_ERR("munmap failed:%s\n", strerror(errno));
ODP_ERR("mlock failed:%s\n", strerror(errno));
--
2.7.4
Re: [lng-odp] [PATCH] doc: userguide: add portability and usage info for odp time apis
Ping. This patch seems to have stalled. Does it need revision or just a review? On Fri, Feb 17, 2017 at 4:00 AM, Maxim Uvarov wrote: > > > On 17 February 2017 at 00:56, Bill Fischofer > wrote: >> >> On Thu, Feb 16, 2017 at 3:20 PM, Brian Brooks >> wrote: >> > On Mon, Feb 13, 2017 at 4:47 PM, Bill Fischofer >> > wrote: >> >> Clarify and expand on portability and performance considerations >> >> regarding the use of the ODP time APIs in fulfillment of JIRA >> >> issue https://projects.linaro.org/browse/ODP-575 >> >> >> >> Signed-off-by: Bill Fischofer >> >> --- >> >> doc/users-guide/users-guide.adoc | 32 +--- >> >> 1 file changed, 21 insertions(+), 11 deletions(-) >> >> >> >> diff --git a/doc/users-guide/users-guide.adoc >> >> b/doc/users-guide/users-guide.adoc >> >> index 41c57d1c..05bade8c 100755 >> >> --- a/doc/users-guide/users-guide.adoc >> >> +++ b/doc/users-guide/users-guide.adoc >> >> @@ -362,31 +362,41 @@ PktIOs are represented by handles of abstract >> >> type `odp_pktio_t`. >> >> >> >> === Time >> >> The time API is used to measure time intervals and track time flow of >> >> an >> >> -application and presents a convenient way to get access to a time >> >> source. >> >> -The time API consists of two main parts: local time API and global >> >> time API. >> >> +application and presents a convenient way to get access to an >> >> +implementation-defined time source. The time API consists of two main >> >> parts: >> >> +local time API and global time API. >> >> >> >> Local time >> >> -The local time API is designed to be used within one thread and can be >> >> faster >> >> -than the global time API. The local time API cannot be used between >> >> threads as >> >> -time consistency is not guaranteed, and in some cases that's enough. >> >> -So, local time stamps are local to the calling thread and must not be >> >> shared >> >> -with other threads. Current local time can be read with >> >> `odp_time_local()`. >> >> +The local time API is designed to be used within one thread >> > >> > How do I convert a local timestamp into a global one? >> >> You'd have to go through the ns conversion routines: >> >> local->global == odp_time_global_from_ns(odp_time_to_ns(local_time)); >> >> global->local == odp_time_local_from_ns(odp_time_to_ns(global_time)); >> >> Though I'm not sure what the use case would be for such conversions. >> >> > >> > What happens if the thread migrates to a different core? Thinking >> > cloud... >> > >> > What happens if a timestamp taken on one thread is used by another >> > thread? >> > Thinking multithreaded applications that use shared memory >> > synchronization... >> >> Presumably odp_time_local() should only be used on cores that are >> isolated to a specific CPU. "Floating" threads should use >> odp_time_global(). However, if you're being preempted and timesliced >> onto another core, it's not clear that odp_time_diff() is doing to be >> meaningful in such cases no matter what time you're using. >> >> > >> >> and obtaining >> >> +local time may be more efficient in some implementations than global >> >> +time. >> > >> > To elicit a response... I claim this is *not* true on ARMv8 and modern >> > x86 chips. So, no need for global vs. local notion of time. >> >> That's why this is a "may". I agree that the use case for the concept >> of local time is weak, but presumably this is a bit of "NUMA >> insurance" in the API? >> >> > >> >> Local time stamps are local to the calling thread and should not be >> >> +shared with other threads, as local time is not guaranteed to be >> >> consistent >> >> +between threads. Current local time can be read with >> >> `odp_time_local()`. >> >> >> >> Global time >> >> The global time API is designed to be used for tracking time between >> >> threads. >> >> -So, global time stamps can be shared between threads. Current global >> >> time can >> >> -be read with `odp_time_global()`. >> >> +So, global time stamps may safely be shared between threads. Current >> >> global >> >> +time can be read with `odp_time_global()`. >> >> >> >> -Both, local and global time is not wrapped during the application life >> >> cycle. >> >> +Both local and global time is not wrapped during the application life >> >> cycle. >> >> The time API includes functions to operate with time, such as >> >> `odp_time_diff()`, >> >> `odp_time_sum()`, `odp_time_cmp()`, conversion functions like >> >> `odp_time_to_ns()`, `odp_time_local_from_ns()`, >> >> `odp_time_global_from_ns()`. >> >> To get rate of time source `odp_time_local_res()`, >> >> `odp_time_global_res()` >> >> are used. To wait, `odp_time_wait_ns()` and `odp_time_wait_until()` >> >> are used, >> >> -during witch a thread potentially busy loop the entire wait time. >> >> +during which a thread potentially busy loops the entire wait time. >> >> >> >> The `odp_time_t` opaque type represents local or global timestamps. >> >> >> >> + Portability Considerations >> >> +The O
Re: [lng-odp] [API-NEXT PATCH v2 0/4] Deprecated macros
13.04.2017 15:31, Dmitry Eremin-Solenikov wrote: On 13.04.2017 15:26, Bill Fischofer wrote: On Thu, Apr 13, 2017 at 6:46 AM, Maxim Uvarov wrote: I vote for default build will not build with deprecating support. Yes all general propose distributives will need to enable --enable-deprecated-api to support some old software. But developers who work with git should not work with deprecated api. The opposite is more likely. Distros can distribute different levels of ODP and if you want the older version you just keep using the older version. So --enable-deprecated-api is really for embedded users who want (some) newer features but haven't (yet) converted older code. Fine, if you say so. It would be nice to hear a word from distro maintainers though. IIRC SuSe has ODP packaged. Canonical were working on that, weren't they? From my experience enterprise distros have to support some deprecated syscalls inside linux kernel to support some application which were sold as binaries only. I.e. they use old api. I think it does not matter if it's embedded or not. It does matter if customer bought only binaries and he wants to use new version of library for some reason. Maxim.
Re: [lng-odp] [API-NEXT PATCH 0/4] add maximum packet counts to pool info
On Thu, Apr 13, 2017 at 2:17 AM, Elo, Matias (Nokia - FI/Espoo) wrote: > >> On 12 Apr 2017, at 22:52, Bill Fischofer wrote: >> >> On Wed, Apr 12, 2017 at 7:58 AM, Matias Elo wrote: >>> On some packet pool implementations the number of available packets may vary >>> depending on the packet length (examples below). E.g. a packet pool may >>> include >>> smaller sub-pools for different packet length ranges. >> >> I'm not sure what the motivation is for this proposed change, but the >> pkt.num specified on odp_pool_create() sets the maximum number of >> packets that can be allocated from the pool, not the minimum. This is >> combined with the pkt.len field to say that pkt.num packets of len >> pkt.len can be allocated. > > The need for this information originally came up when fixing an issue in > pool_create(). pool_create() didn't allocate enough packets if the requested > length (pkt.len) packets were segmented. As a result of this fix it is now > possible to allocate more than pkt.num packets from the pool, if the > requested packet length fits into a single segment (or less segments than > the original pkt.len). This sounds like trying to fix an implementation bug with an API change. The implementation has all the info it needs with pkt.num, pkt.len, and pkt.max_len to realize the specified semantics. If there is segmentation involved in pkt.len it can take that into account in configuring the pool. > > The spec of pkt.num is changed to "The exact number of 'len' byte packets > that the pool must provide.". The current spec say this is "The number of packets that the pool must provide that are packet length 'len' bytes or smaller". That seems perfectly unambiguous. If a given implementation doesn't do that, that's a bug in that implementation and it should be fixed. The validation tests should be testing this, so if that's not being done then that's a bug in the validation test which should also be fixed. > >> If the application allocated packets larger >> than this size (up to pkt.max_len) then the actual number of packets >> that can be successfully allocated from the pool may be lower than >> pkt.num, but it will never be greater. > > Depending on the pool implementation this may not always be the case > (not likely) and the ascii pictures in the cover letter tried to visualise > this > possibility. For example, a pool may consists of smaller HW sub-pools for > different packet length ranges and these sub-pools may be different in size. How an implementation realizes the specified API semantics is not defined by ODP. It should never be possible to allocate more than pkt.num packets from a pool, and it should be possible to allocate exactly pkt.num packets of len pkt.len. That's what the configuration parameters control. > >> >> The reason for wanting the maximum number is to bound the number of >> "in flight" packets for QoS purposes. As a pool reaches this limit, >> RED and similar algorithms kick in to start dropping packets at RX. If >> there is no fixed maximum number of packets that makes QoS processing >> erratic. >> > > There is still a maximum and an application can query it with odp_pool_info() > (odp_pool_info_t.pkt.max_num). RED and similar work as previously as > there is no API for configuring them in ODP. True today, but we have discussed adding watermarks on pools that can be used to trigger such behavior. > >> We had previously floated the idea of allowing pool groups to be >> defined which would accommodate the practice where packets get >> assigned to different sub-pools based on their size, however this was >> seen to be overly complicated, especially on platforms that have >> hardware buffer managers. > > True, this gets complicated fast. The updated odp_pool_info_t includes new > maximum packet counts to provide more information to the application > without going to the specifics of the underlying pool implementation. > >> Do you have a specific use case that can't be handled by the current >> structures? > > Described above. It seems you've described problems with some implementations rather than application needs. The spec was designed to allow applications to specify their requirements on pool capacity, which it seems to do. > > > -Matias > > >
[lng-odp] [Bug 2921] odp_init_global() and odp_pool_create() set errno = 12
https://bugs.linaro.org/show_bug.cgi?id=2921 --- Comment #7 from Maxim Uvarov --- Then you need to submit patch to master. Then submit patch for Monarch as back port of master branch patch. -- You are receiving this mail because: You are on the CC list for the bug.
[lng-odp] [Bug 2940] odp_packet_seg_t fails ABI compatibility between odp-linux and odp-dpdk
https://bugs.linaro.org/show_bug.cgi?id=2940 --- Comment #3 from Bill Fischofer --- Patch v2 submitted http://patches.opendataplane.org/patch/8527/ to address Krishna comments. -- You are receiving this mail because: You are on the CC list for the bug.
Re: [lng-odp] [Bug 2921] odp_init_global() and odp_pool_create() set errno = 12
Maxim, the fix in Monarch is quite different than in master. In Monarch, it affects odp_shm_reserve() in odp_shared_memory.c. In master, it has been moved to _odp_ishmphy_map() in _ishmphy.c. Which patch should I submit? Or both? -- Oriol Arcas Software Engineer Starflow Networks On Wed, Apr 12, 2017 at 8:41 PM, wrote: > https://bugs.linaro.org/show_bug.cgi?id=2921 > > --- Comment #6 from Maxim Uvarov --- > yes, please provide patch. I think it might be applicable for current > master. > > -- > You are receiving this mail because: > You are on the CC list for the bug. >
Re: [lng-odp] [API-NEXT PATCH v2 0/4] Deprecated macros
On 13.04.2017 15:26, Bill Fischofer wrote: > On Thu, Apr 13, 2017 at 6:46 AM, Maxim Uvarov wrote: >> I vote for default build will not build with deprecating support. Yes all >> general propose distributives will need to enable --enable-deprecated-api >> to support some old software. But developers who work with git should not >> work with deprecated api. > > The opposite is more likely. Distros can distribute different levels > of ODP and if you want the older version you just keep using the older > version. So --enable-deprecated-api is really for embedded users who > want (some) newer features but haven't (yet) converted older code. Fine, if you say so. It would be nice to hear a word from distro maintainers though. IIRC SuSe has ODP packaged. Canonical were working on that, weren't they? -- With best wishes Dmitry
Re: [lng-odp] [PATCHv2] abi: packet: restore abi compatibility for odp_packet_seg_t type
Thanks.
On Thu, Apr 13, 2017 at 7:05 AM, Krishna Garapati
wrote:
> Reviewed-by: Balakrishna Garapati
>
> /Krishna
>
> On 13 April 2017 at 13:48, Bill Fischofer wrote:
>>
>> When running in --enable-abi-compat=yes mode, all ODP types need to be
>> of pointer width in the default ABI definition. The optimization of the
>> odp_packet_seg_t type to uint8_t can only be supported when running in
>> --enable-abi-compate=no mode. Change the ODP packet routines to use
Maxim: Just noticed this typo (should be --enable-abi-compat=no). Can
this be fixed in merge? If you'd prefer I'll send a v3.
>> type converter routines that have varying definitions based on whether
>> we're running in ABI compatibility mode and provide these variant
>> definitions to enable proper ABI compatibility while still supporting an
>> optimized typedef for non-ABI mode.
>>
>> This resolves Bug https://bugs.linaro.org/show_bug.cgi?id=2940
>>
>> Reported-by: Krishna Garapati
>> Signed-off-by: Bill Fischofer
>> ---
>> v2:
>> - Incorporate changes suggested by Krishna
>> - Added Reported-by attribution
>>
>> include/odp/arch/default/api/abi/packet.h | 7 +--
>> .../include/odp/api/plat/packet_inlines.h | 21
>> ++---
>> .../include/odp/api/plat/packet_types.h | 10 ++
>> platform/linux-generic/odp_packet.c | 12 +++-
>> 4 files changed, 40 insertions(+), 10 deletions(-)
>>
>> diff --git a/include/odp/arch/default/api/abi/packet.h
>> b/include/odp/arch/default/api/abi/packet.h
>> index 60a41b8..4aac75b 100644
>> --- a/include/odp/arch/default/api/abi/packet.h
>> +++ b/include/odp/arch/default/api/abi/packet.h
>> @@ -16,15 +16,18 @@ extern "C" {
>> /** @internal Dummy type for strong typing */
>> typedef struct { char dummy; /**< @internal Dummy */ } _odp_abi_packet_t;
>>
>> +/** @internal Dummy type for strong typing */
>> +typedef struct { char dummy; /**< *internal Dummy */ }
>> _odp_abi_packet_seg_t;
>> +
>> /** @ingroup odp_packet
>> * @{
>> */
>>
>> typedef _odp_abi_packet_t *odp_packet_t;
>> -typedef uint8_todp_packet_seg_t;
>> +typedef _odp_abi_packet_seg_t *odp_packet_seg_t;
>>
>> #define ODP_PACKET_INVALID((odp_packet_t)0x)
>> -#define ODP_PACKET_SEG_INVALID((odp_packet_seg_t)-1)
>> +#define ODP_PACKET_SEG_INVALID((odp_packet_seg_t)0x)
>> #define ODP_PACKET_OFFSET_INVALID (0x0fff)
>>
>> typedef enum {
>> diff --git a/platform/linux-generic/include/odp/api/plat/packet_inlines.h
>> b/platform/linux-generic/include/odp/api/plat/packet_inlines.h
>> index eb36aa9..3dd643f 100644
>> --- a/platform/linux-generic/include/odp/api/plat/packet_inlines.h
>> +++ b/platform/linux-generic/include/odp/api/plat/packet_inlines.h
>> @@ -21,6 +21,20 @@
>> /** @internal Inline function offsets */
>> extern const _odp_packet_inline_offset_t _odp_packet_inline;
>>
>> +#if ODP_ABI_COMPAT == 1
>> +/** @internal Inline function @param seg @return */
>> +static inline uint32_t _odp_packet_seg_to_ndx(odp_packet_seg_t seg)
>> +{
>> + return _odp_typeval(seg);
>> +}
>> +
>> +/** @internal Inline function @param ndx @return */
>> +static inline odp_packet_seg_t _odp_packet_seg_from_ndx(uint32_t ndx)
>> +{
>> + return _odp_cast_scalar(odp_packet_seg_t, ndx);
>> +}
>> +#endif
>> +
>> /** @internal Inline function @param pkt @return */
>> static inline void *_odp_packet_data(odp_packet_t pkt)
>> {
>> @@ -128,20 +142,21 @@ static inline odp_packet_seg_t
>> _odp_packet_first_seg(odp_packet_t pkt)
>> {
>> (void)pkt;
>>
>> - return 0;
>> + return _odp_packet_seg_from_ndx(0);
>> }
>>
>> /** @internal Inline function @param pkt @return */
>> static inline odp_packet_seg_t _odp_packet_last_seg(odp_packet_t pkt)
>> {
>> - return _odp_packet_num_segs(pkt) - 1;
>> + return _odp_packet_seg_from_ndx(_odp_packet_num_segs(pkt) - 1);
>> }
>>
>> /** @internal Inline function @param pkt @param seg @return */
>> static inline odp_packet_seg_t _odp_packet_next_seg(odp_packet_t pkt,
>> odp_packet_seg_t seg)
>> {
>> - if (odp_unlikely(seg >= _odp_packet_last_seg(pkt)))
>> + if (odp_unlikely(_odp_packet_seg_to_ndx(seg) >=
>> +
>> _odp_packet_seg_to_ndx(_odp_packet_last_seg(pkt
>> return ODP_PACKET_SEG_INVALID;
>>
>> return seg + 1;
>> diff --git a/platform/linux-generic/include/odp/api/plat/packet_types.h
>> b/platform/linux-generic/include/odp/api/plat/packet_types.h
>> index b8f665d..7e3c51e 100644
>> --- a/platform/linux-generic/include/odp/api/plat/packet_types.h
>> +++ b/platform/linux-generic/include/odp/api/plat/packet_types.h
>> @@ -40,6 +40,16 @@ typedef ODP_HANDLE_T(odp_packet_t);
>>
>> typedef uint8_t odp_packet_seg_t;
>>
>> +static inline uint8_t _odp_packet_seg_to_ndx(odp_packet_seg_t seg)
>> +{
>> + return (uint8_t)seg;
>> +}
>> +
>> +static inline odp_packet_seg_t _odp_p
Re: [lng-odp] [API-NEXT PATCH v2 0/4] Deprecated macros
On Thu, Apr 13, 2017 at 6:46 AM, Maxim Uvarov wrote: > I vote for default build will not build with deprecating support. Yes all > general propose distributives will need to enable --enable-deprecated-api > to support some old software. But developers who work with git should not > work with deprecated api. The opposite is more likely. Distros can distribute different levels of ODP and if you want the older version you just keep using the older version. So --enable-deprecated-api is really for embedded users who want (some) newer features but haven't (yet) converted older code. The key is that it is the embedded space that customizes and hyper tunes things, not the cloud, because the embedded space knows precisely the platform and configuration it's running on. By definition the cloud is more concerned with portability because the application does not control the environment--that's controlled by the operator. But no matter how you sugar-coat it, deprecation is always painful, which is why we should not do this at all, or at most only very rarely. So rarely that there is no need for a general framework for dealing with it as each deprecation will be considered on its own merits. Better to focus on structures that permit APIs to evolve over time without needing deprecation. As a practical matter, the question of deprecation is only relevant going forward from Tiger Moth / odp-cloud. There is no need to consider Monarch as "deprecatable" as it's never been part of any distro. > > Maxim. > > On 13 April 2017 at 13:57, Dmitry Eremin-Solenikov < > dmitry.ereminsoleni...@linaro.org> wrote: > >> On 13.04.2017 10:33, Savolainen, Petri (Nokia - FI/Espoo) wrote: >> > >> > >> >> -Original Message- >> >> From: Dmitry Eremin-Solenikov [mailto:dmitry.ereminsoleni...@linaro.org >> ] >> >> Sent: Wednesday, April 12, 2017 3:11 PM >> >> To: Savolainen, Petri (Nokia - FI/Espoo) > >> labs.com>; lng-odp@lists.linaro.org >> >> Subject: Re: [lng-odp] [API-NEXT PATCH v2 0/4] Deprecated macros >> >> >> >> On 12.04.2017 14:50, Savolainen, Petri (Nokia - FI/Espoo) wrote: >> >>> >> >>> >> -Original Message- >> From: Dmitry Eremin-Solenikov >> >> [mailto:dmitry.ereminsoleni...@linaro.org] >> Sent: Wednesday, April 12, 2017 2:32 PM >> To: Petri Savolainen ; lng- >> o...@lists.linaro.org >> Subject: Re: [lng-odp] [API-NEXT PATCH v2 0/4] Deprecated macros >> >> On 30.03.2017 16:58, Petri Savolainen wrote: >> > Replaced ODP_DEPRECATED macro (which was based on GCC __attribute__) >> with >> > compiler independent mechanism to control if deprecated API >> >> definitions >> are >> > visible to the application. ODP_DEPRECATED_API can be used both in >> application >> > and implementation to check if deprecated APIs are enabled. By >> default >> those are >> > disabled. Implementation may optimize the normal (new API) code path. >> > >> > ODP_DEPRECATE() macro is used to rename definitions, so that data >> structure >> > sizes are equal on both options. This enables implementation to serve >> both >> > options with a single library (if it wishes to do so). >> >> My main question remains as it was before: is it possible for the >> distribution to supply (unoptimized) ODP binary and headers and then >> >> for >> the application to select if it builds with or without deprecated API >> using that binary/headers? >> >>> >> >>> >> >>> Yes. This patch set keeps the same struct fields/etc for both modes - >> >> only the names are scrambled (with __deprecated_ prefix) when deprecated >> >> APIs are not supported (the default). >> >> >> >> If so. Consider I have built and installed ODP headers & binary built >> >> with --enable-deprecated-api. >> > >> > So, you have built and installed the implementation with >> --enable-deprecated-api. This means that applications using this install, >> see deprecated APIs. A distribution decides which way it supports ODP, with >> or without deprecated stuff. The default in our (odp-linux) configure is >> without, to minimize confusion and promote the new API definitions which >> should be always better for application than the old ones. >> > >> >> I was advocating for binary distributions and developers using packages >> from those distributions. And for those distributions providing _single_ >> binary version of ODP. >> >> >> -- >> With best wishes >> Dmitry >>
Re: [lng-odp] [PATCH v3] example: add IPv4 fragmentation/reassembly example
> > > A change of architecture shouldn't introduce any additional race > > > conditions or deadlocks, no. If this were the case, then I agree that > > > this example would be far too fragile to include. Any standards compliant > > > implementation of "__atomic_compare_exchange_n" will work fine - > > > including the default one used by ARM platforms. As the algorithm > > > underlying this example is supposed to be lock-free, though, a proper > > > lock-free compare exchange implementation is the icing on the cake, > > > showing off the properties of the algorithm in full. > > > > Does the *example* need to show of lock-freeness? Would be enough to just > > show of how IP frag/reassembly is usually done on ODP? > > > > Could all assembly and ARM #ifdefs be removed if we simply don't claim the > > code to be lock-free ? > > > > -Petri > > I vote for saying yes here. I.e. this example should show lock-freeness > and best performance usage doing ip frag with ODP. > > We can say that some examples can be very simple and minimal. Other > examples might be complex and effective. Idea for first one is to show how > some code works and idea for second one is to provide some building block > of complex thing which can be reused by application developers. And that > is great help from ARM writing such efficient building bocks for us. > > Maxim. I agree with Maxim. While it's perfectly possible for this patch to be modified such that it doesn't contain any ARM-specific code, I think the patch is more useful with this code. It better demonstrates what you might want to do on a real system, which frankly is what this example is all about. If you wanted a simple implementation of IPv4 reassembly, you definitely would not want to use the algorithm presented in this patch. This code is complicated exactly because it demonstrates an efficient, scalable approach that you might want to use in a real world application. And as such, I think the "proper" accompanying CAS implementations for ARM are warranted.
Re: [lng-odp] [PATCHv2] abi: packet: restore abi compatibility for odp_packet_seg_t type
Reviewed-by: Balakrishna Garapati
/Krishna
On 13 April 2017 at 13:48, Bill Fischofer wrote:
> When running in --enable-abi-compat=yes mode, all ODP types need to be
> of pointer width in the default ABI definition. The optimization of the
> odp_packet_seg_t type to uint8_t can only be supported when running in
> --enable-abi-compate=no mode. Change the ODP packet routines to use
> type converter routines that have varying definitions based on whether
> we're running in ABI compatibility mode and provide these variant
> definitions to enable proper ABI compatibility while still supporting an
> optimized typedef for non-ABI mode.
>
> This resolves Bug https://bugs.linaro.org/show_bug.cgi?id=2940
>
> Reported-by: Krishna Garapati
> Signed-off-by: Bill Fischofer
> ---
> v2:
> - Incorporate changes suggested by Krishna
> - Added Reported-by attribution
>
> include/odp/arch/default/api/abi/packet.h | 7 +--
> .../include/odp/api/plat/packet_inlines.h | 21
> ++---
> .../include/odp/api/plat/packet_types.h | 10 ++
> platform/linux-generic/odp_packet.c | 12 +++-
> 4 files changed, 40 insertions(+), 10 deletions(-)
>
> diff --git a/include/odp/arch/default/api/abi/packet.h
> b/include/odp/arch/default/api/abi/packet.h
> index 60a41b8..4aac75b 100644
> --- a/include/odp/arch/default/api/abi/packet.h
> +++ b/include/odp/arch/default/api/abi/packet.h
> @@ -16,15 +16,18 @@ extern "C" {
> /** @internal Dummy type for strong typing */
> typedef struct { char dummy; /**< @internal Dummy */ } _odp_abi_packet_t;
>
> +/** @internal Dummy type for strong typing */
> +typedef struct { char dummy; /**< *internal Dummy */ }
> _odp_abi_packet_seg_t;
> +
> /** @ingroup odp_packet
> * @{
> */
>
> typedef _odp_abi_packet_t *odp_packet_t;
> -typedef uint8_todp_packet_seg_t;
> +typedef _odp_abi_packet_seg_t *odp_packet_seg_t;
>
> #define ODP_PACKET_INVALID((odp_packet_t)0x)
> -#define ODP_PACKET_SEG_INVALID((odp_packet_seg_t)-1)
> +#define ODP_PACKET_SEG_INVALID((odp_packet_seg_t)0x)
> #define ODP_PACKET_OFFSET_INVALID (0x0fff)
>
> typedef enum {
> diff --git a/platform/linux-generic/include/odp/api/plat/packet_inlines.h
> b/platform/linux-generic/include/odp/api/plat/packet_inlines.h
> index eb36aa9..3dd643f 100644
> --- a/platform/linux-generic/include/odp/api/plat/packet_inlines.h
> +++ b/platform/linux-generic/include/odp/api/plat/packet_inlines.h
> @@ -21,6 +21,20 @@
> /** @internal Inline function offsets */
> extern const _odp_packet_inline_offset_t _odp_packet_inline;
>
> +#if ODP_ABI_COMPAT == 1
> +/** @internal Inline function @param seg @return */
> +static inline uint32_t _odp_packet_seg_to_ndx(odp_packet_seg_t seg)
> +{
> + return _odp_typeval(seg);
> +}
> +
> +/** @internal Inline function @param ndx @return */
> +static inline odp_packet_seg_t _odp_packet_seg_from_ndx(uint32_t ndx)
> +{
> + return _odp_cast_scalar(odp_packet_seg_t, ndx);
> +}
> +#endif
> +
> /** @internal Inline function @param pkt @return */
> static inline void *_odp_packet_data(odp_packet_t pkt)
> {
> @@ -128,20 +142,21 @@ static inline odp_packet_seg_t
> _odp_packet_first_seg(odp_packet_t pkt)
> {
> (void)pkt;
>
> - return 0;
> + return _odp_packet_seg_from_ndx(0);
> }
>
> /** @internal Inline function @param pkt @return */
> static inline odp_packet_seg_t _odp_packet_last_seg(odp_packet_t pkt)
> {
> - return _odp_packet_num_segs(pkt) - 1;
> + return _odp_packet_seg_from_ndx(_odp_packet_num_segs(pkt) - 1);
> }
>
> /** @internal Inline function @param pkt @param seg @return */
> static inline odp_packet_seg_t _odp_packet_next_seg(odp_packet_t pkt,
> odp_packet_seg_t seg)
> {
> - if (odp_unlikely(seg >= _odp_packet_last_seg(pkt)))
> + if (odp_unlikely(_odp_packet_seg_to_ndx(seg) >=
> +_odp_packet_seg_to_ndx(_odp_
> packet_last_seg(pkt
> return ODP_PACKET_SEG_INVALID;
>
> return seg + 1;
> diff --git a/platform/linux-generic/include/odp/api/plat/packet_types.h
> b/platform/linux-generic/include/odp/api/plat/packet_types.h
> index b8f665d..7e3c51e 100644
> --- a/platform/linux-generic/include/odp/api/plat/packet_types.h
> +++ b/platform/linux-generic/include/odp/api/plat/packet_types.h
> @@ -40,6 +40,16 @@ typedef ODP_HANDLE_T(odp_packet_t);
>
> typedef uint8_t odp_packet_seg_t;
>
> +static inline uint8_t _odp_packet_seg_to_ndx(odp_packet_seg_t seg)
> +{
> + return (uint8_t)seg;
> +}
> +
> +static inline odp_packet_seg_t _odp_packet_seg_from_ndx(uint8_t ndx)
> +{
> + return (odp_packet_seg_t)ndx;
> +}
> +
> #define ODP_PACKET_SEG_INVALID ((odp_packet_seg_t)-1)
>
> typedef enum {
> diff --git a/platform/linux-generic/odp_packet.c
> b/platform/linux-generic/odp_packet.c
> index b8aac6b..e99e8b8 100644
> --- a/platfor
[lng-odp] [PATCHv2] abi: packet: restore abi compatibility for odp_packet_seg_t type
When running in --enable-abi-compat=yes mode, all ODP types need to be
of pointer width in the default ABI definition. The optimization of the
odp_packet_seg_t type to uint8_t can only be supported when running in
--enable-abi-compate=no mode. Change the ODP packet routines to use
type converter routines that have varying definitions based on whether
we're running in ABI compatibility mode and provide these variant
definitions to enable proper ABI compatibility while still supporting an
optimized typedef for non-ABI mode.
This resolves Bug https://bugs.linaro.org/show_bug.cgi?id=2940
Reported-by: Krishna Garapati
Signed-off-by: Bill Fischofer
---
v2:
- Incorporate changes suggested by Krishna
- Added Reported-by attribution
include/odp/arch/default/api/abi/packet.h | 7 +--
.../include/odp/api/plat/packet_inlines.h | 21 ++---
.../include/odp/api/plat/packet_types.h | 10 ++
platform/linux-generic/odp_packet.c | 12 +++-
4 files changed, 40 insertions(+), 10 deletions(-)
diff --git a/include/odp/arch/default/api/abi/packet.h
b/include/odp/arch/default/api/abi/packet.h
index 60a41b8..4aac75b 100644
--- a/include/odp/arch/default/api/abi/packet.h
+++ b/include/odp/arch/default/api/abi/packet.h
@@ -16,15 +16,18 @@ extern "C" {
/** @internal Dummy type for strong typing */
typedef struct { char dummy; /**< @internal Dummy */ } _odp_abi_packet_t;
+/** @internal Dummy type for strong typing */
+typedef struct { char dummy; /**< *internal Dummy */ } _odp_abi_packet_seg_t;
+
/** @ingroup odp_packet
* @{
*/
typedef _odp_abi_packet_t *odp_packet_t;
-typedef uint8_todp_packet_seg_t;
+typedef _odp_abi_packet_seg_t *odp_packet_seg_t;
#define ODP_PACKET_INVALID((odp_packet_t)0x)
-#define ODP_PACKET_SEG_INVALID((odp_packet_seg_t)-1)
+#define ODP_PACKET_SEG_INVALID((odp_packet_seg_t)0x)
#define ODP_PACKET_OFFSET_INVALID (0x0fff)
typedef enum {
diff --git a/platform/linux-generic/include/odp/api/plat/packet_inlines.h
b/platform/linux-generic/include/odp/api/plat/packet_inlines.h
index eb36aa9..3dd643f 100644
--- a/platform/linux-generic/include/odp/api/plat/packet_inlines.h
+++ b/platform/linux-generic/include/odp/api/plat/packet_inlines.h
@@ -21,6 +21,20 @@
/** @internal Inline function offsets */
extern const _odp_packet_inline_offset_t _odp_packet_inline;
+#if ODP_ABI_COMPAT == 1
+/** @internal Inline function @param seg @return */
+static inline uint32_t _odp_packet_seg_to_ndx(odp_packet_seg_t seg)
+{
+ return _odp_typeval(seg);
+}
+
+/** @internal Inline function @param ndx @return */
+static inline odp_packet_seg_t _odp_packet_seg_from_ndx(uint32_t ndx)
+{
+ return _odp_cast_scalar(odp_packet_seg_t, ndx);
+}
+#endif
+
/** @internal Inline function @param pkt @return */
static inline void *_odp_packet_data(odp_packet_t pkt)
{
@@ -128,20 +142,21 @@ static inline odp_packet_seg_t
_odp_packet_first_seg(odp_packet_t pkt)
{
(void)pkt;
- return 0;
+ return _odp_packet_seg_from_ndx(0);
}
/** @internal Inline function @param pkt @return */
static inline odp_packet_seg_t _odp_packet_last_seg(odp_packet_t pkt)
{
- return _odp_packet_num_segs(pkt) - 1;
+ return _odp_packet_seg_from_ndx(_odp_packet_num_segs(pkt) - 1);
}
/** @internal Inline function @param pkt @param seg @return */
static inline odp_packet_seg_t _odp_packet_next_seg(odp_packet_t pkt,
odp_packet_seg_t seg)
{
- if (odp_unlikely(seg >= _odp_packet_last_seg(pkt)))
+ if (odp_unlikely(_odp_packet_seg_to_ndx(seg) >=
+_odp_packet_seg_to_ndx(_odp_packet_last_seg(pkt
return ODP_PACKET_SEG_INVALID;
return seg + 1;
diff --git a/platform/linux-generic/include/odp/api/plat/packet_types.h
b/platform/linux-generic/include/odp/api/plat/packet_types.h
index b8f665d..7e3c51e 100644
--- a/platform/linux-generic/include/odp/api/plat/packet_types.h
+++ b/platform/linux-generic/include/odp/api/plat/packet_types.h
@@ -40,6 +40,16 @@ typedef ODP_HANDLE_T(odp_packet_t);
typedef uint8_t odp_packet_seg_t;
+static inline uint8_t _odp_packet_seg_to_ndx(odp_packet_seg_t seg)
+{
+ return (uint8_t)seg;
+}
+
+static inline odp_packet_seg_t _odp_packet_seg_from_ndx(uint8_t ndx)
+{
+ return (odp_packet_seg_t)ndx;
+}
+
#define ODP_PACKET_SEG_INVALID ((odp_packet_seg_t)-1)
typedef enum {
diff --git a/platform/linux-generic/odp_packet.c
b/platform/linux-generic/odp_packet.c
index b8aac6b..e99e8b8 100644
--- a/platform/linux-generic/odp_packet.c
+++ b/platform/linux-generic/odp_packet.c
@@ -1185,7 +1185,7 @@ void *odp_packet_offset(odp_packet_t pkt, uint32_t
offset, uint32_t *len,
void *addr = packet_map(pkt_hdr, offset, len, &seg_idx);
if (addr != NULL && seg != NULL)
- *seg = seg_idx;
+ *se
Re: [lng-odp] [PATCH] abi: packet: restore abi compatibility for odp_packet_seg_t type
Thanks. v2 sent.
On Thu, Apr 13, 2017 at 3:32 AM, Krishna Garapati
wrote:
>
>
> On 13 April 2017 at 00:06, Bill Fischofer wrote:
>>
>> When running in --enable-abi-compat=yes mode, all ODP types need to be
>> of pointer width in the default ABI definition. The optimization of the
>> odp_packet_seg_t type to uint8_t can only be supported when running in
>> --enable-abi-compate=no mode. Change the ODP packet routines to use
>> type converter routines that have varying definitions based on whether
>> we're running in ABI compatibility mode and provide these variant
>> definitions to enable proper ABI compatibility while still supporting an
>> optimized typedef for non-ABI mode.
>>
>> This resolves Bug https://bugs.linaro.org/show_bug.cgi?id=2940
>>
>> Signed-off-by: Bill Fischofer
>> ---
>> include/odp/arch/default/api/abi/packet.h | 7 +--
>> .../include/odp/api/plat/packet_inlines.h | 21
>> ++---
>> .../include/odp/api/plat/packet_types.h | 10 ++
>> platform/linux-generic/odp_packet.c | 12 +++-
>> 4 files changed, 40 insertions(+), 10 deletions(-)
>>
>> diff --git a/include/odp/arch/default/api/abi/packet.h
>> b/include/odp/arch/default/api/abi/packet.h
>> index 60a41b8..4aac75b 100644
>> --- a/include/odp/arch/default/api/abi/packet.h
>> +++ b/include/odp/arch/default/api/abi/packet.h
>> @@ -16,15 +16,18 @@ extern "C" {
>> /** @internal Dummy type for strong typing */
>> typedef struct { char dummy; /**< @internal Dummy */ } _odp_abi_packet_t;
>>
>> +/** @internal Dummy type for strong typing */
>> +typedef struct { char dummy; /**< *internal Dummy */ }
>> _odp_abi_packet_seg_t;
>> +
>> /** @ingroup odp_packet
>> * @{
>> */
>>
>> typedef _odp_abi_packet_t *odp_packet_t;
>> -typedef uint8_todp_packet_seg_t;
>> +typedef _odp_abi_packet_seg_t *odp_packet_seg_t;
>>
>> #define ODP_PACKET_INVALID((odp_packet_t)0x)
>> -#define ODP_PACKET_SEG_INVALID((odp_packet_seg_t)-1)
>> +#define ODP_PACKET_SEG_INVALID((odp_packet_seg_t)0x)
>> #define ODP_PACKET_OFFSET_INVALID (0x0fff)
>>
>> typedef enum {
>> diff --git a/platform/linux-generic/include/odp/api/plat/packet_inlines.h
>> b/platform/linux-generic/include/odp/api/plat/packet_inlines.h
>> index eb36aa9..bd735c1 100644
>> --- a/platform/linux-generic/include/odp/api/plat/packet_inlines.h
>> +++ b/platform/linux-generic/include/odp/api/plat/packet_inlines.h
>> @@ -21,6 +21,20 @@
>> /** @internal Inline function offsets */
>> extern const _odp_packet_inline_offset_t _odp_packet_inline;
>>
>> +#if ODP_ABI_COMPAT
>
> Everywhere it explicitly says ODP_ABI_COMPAT == 1 or 0 except here. Would be
> good to see it here the same way.
>>
>> +/** @internal Inline function @param seg @return */
>> +static inline int _odp_packet_seg_to_ndx(odp_packet_seg_t seg)
>
> is "_ndx" a typo instead of _idx ?.
>>
>> +{
>> + return _odp_typeval(seg);
>
> odp_typeval converts "seg" to uint32_t and we return it as int.
>>
>> +}
>> +
>> +/** @internal Inline function @param ndx @return */
>> +static inline odp_packet_seg_t _odp_packet_seg_from_ndx(int ndx)
>> +{
>> + return _odp_cast_scalar(odp_packet_seg_t, ndx);
>> +}
>> +#endif
>> +
>> /** @internal Inline function @param pkt @return */
>> static inline void *_odp_packet_data(odp_packet_t pkt)
>> {
>> @@ -128,20 +142,21 @@ static inline odp_packet_seg_t
>> _odp_packet_first_seg(odp_packet_t pkt)
>> {
>> (void)pkt;
>>
>> - return 0;
>> + return _odp_packet_seg_from_ndx(0);
>> }
>>
>> /** @internal Inline function @param pkt @return */
>> static inline odp_packet_seg_t _odp_packet_last_seg(odp_packet_t pkt)
>> {
>> - return _odp_packet_num_segs(pkt) - 1;
>> + return _odp_packet_seg_from_ndx(_odp_packet_num_segs(pkt) - 1);
>> }
>>
>> /** @internal Inline function @param pkt @param seg @return */
>> static inline odp_packet_seg_t _odp_packet_next_seg(odp_packet_t pkt,
>> odp_packet_seg_t seg)
>> {
>> - if (odp_unlikely(seg >= _odp_packet_last_seg(pkt)))
>> + if (odp_unlikely(_odp_packet_seg_to_ndx(seg) >=
>> +
>> _odp_packet_seg_to_ndx(_odp_packet_last_seg(pkt
>> return ODP_PACKET_SEG_INVALID;
>>
>> return seg + 1;
>> diff --git a/platform/linux-generic/include/odp/api/plat/packet_types.h
>> b/platform/linux-generic/include/odp/api/plat/packet_types.h
>> index b8f665d..59e8b2f 100644
>> --- a/platform/linux-generic/include/odp/api/plat/packet_types.h
>> +++ b/platform/linux-generic/include/odp/api/plat/packet_types.h
>> @@ -40,6 +40,16 @@ typedef ODP_HANDLE_T(odp_packet_t);
>>
>> typedef uint8_t odp_packet_seg_t;
>>
>> +static inline int _odp_packet_seg_to_ndx(odp_packet_seg_t seg)
>> +{
>> + return (int)seg;
>> +}
>> +
>> +static inline odp_packet_seg_t _odp_packet_seg_from_ndx(int ndx)
>> +{
>> + return (odp_packet_seg_t)ndx;
>> +}
Re: [lng-odp] [API-NEXT PATCH v2 0/4] Deprecated macros
I vote for default build will not build with deprecating support. Yes all general propose distributives will need to enable --enable-deprecated-api to support some old software. But developers who work with git should not work with deprecated api. Maxim. On 13 April 2017 at 13:57, Dmitry Eremin-Solenikov < dmitry.ereminsoleni...@linaro.org> wrote: > On 13.04.2017 10:33, Savolainen, Petri (Nokia - FI/Espoo) wrote: > > > > > >> -Original Message- > >> From: Dmitry Eremin-Solenikov [mailto:dmitry.ereminsoleni...@linaro.org > ] > >> Sent: Wednesday, April 12, 2017 3:11 PM > >> To: Savolainen, Petri (Nokia - FI/Espoo) >> labs.com>; lng-odp@lists.linaro.org > >> Subject: Re: [lng-odp] [API-NEXT PATCH v2 0/4] Deprecated macros > >> > >> On 12.04.2017 14:50, Savolainen, Petri (Nokia - FI/Espoo) wrote: > >>> > >>> > -Original Message- > From: Dmitry Eremin-Solenikov > >> [mailto:dmitry.ereminsoleni...@linaro.org] > Sent: Wednesday, April 12, 2017 2:32 PM > To: Petri Savolainen ; lng- > o...@lists.linaro.org > Subject: Re: [lng-odp] [API-NEXT PATCH v2 0/4] Deprecated macros > > On 30.03.2017 16:58, Petri Savolainen wrote: > > Replaced ODP_DEPRECATED macro (which was based on GCC __attribute__) > with > > compiler independent mechanism to control if deprecated API > >> definitions > are > > visible to the application. ODP_DEPRECATED_API can be used both in > application > > and implementation to check if deprecated APIs are enabled. By > default > those are > > disabled. Implementation may optimize the normal (new API) code path. > > > > ODP_DEPRECATE() macro is used to rename definitions, so that data > structure > > sizes are equal on both options. This enables implementation to serve > both > > options with a single library (if it wishes to do so). > > My main question remains as it was before: is it possible for the > distribution to supply (unoptimized) ODP binary and headers and then > >> for > the application to select if it builds with or without deprecated API > using that binary/headers? > >>> > >>> > >>> Yes. This patch set keeps the same struct fields/etc for both modes - > >> only the names are scrambled (with __deprecated_ prefix) when deprecated > >> APIs are not supported (the default). > >> > >> If so. Consider I have built and installed ODP headers & binary built > >> with --enable-deprecated-api. > > > > So, you have built and installed the implementation with > --enable-deprecated-api. This means that applications using this install, > see deprecated APIs. A distribution decides which way it supports ODP, with > or without deprecated stuff. The default in our (odp-linux) configure is > without, to minimize confusion and promote the new API definitions which > should be always better for application than the old ones. > > > > I was advocating for binary distributions and developers using packages > from those distributions. And for those distributions providing _single_ > binary version of ODP. > > > -- > With best wishes > Dmitry >
Re: [lng-odp] [PATCH v3] example: add IPv4 fragmentation/reassembly example
On 13 April 2017 at 13:49, Savolainen, Petri (Nokia - FI/Espoo) < petri.savolai...@nokia-bell-labs.com> wrote: > > > ODP is about HW abstraction and CPU instructions are HW features. Of > > course, > > > applications are free to use other libs/APIs/GCC features, but should > > our > > > examples do that? > > > > I'm not sure I agree that ODP primarily exists to abstract > non-accelerator > > HW, as far as I'm concerned that's really the job of the language, other > > libraries, and at the end of the day the programmer. (Talk about biting > > off > > more than you can chew: abstracting all hardware in the system?!) > > But regardless... > > > CPU cycle count read may be an instruction. Time counter read may an > instruction. 128 bit atomic CAS may be an instruction. AES crypto may have > multiple instructions. Queue enqueue may be an instruction. Memory barrier > may be embedded into multiple instructions... > > An application that uses these instruction directly is not portable. An > application that uses those through ODP API is portable. > > > > > > > "There is a single exception to this, in that the standard 128-bit > > > "__atomic_compare_exchange_n" implementation on ARM is not lock-free > > ..." > > > > > > This is the point. How about the built-in implementation on other CPU > > > architectures: MIPS, PPC, Tilera, Kalray, ... etc ? Does the code build > > on > > > those. If it does build, does it have race conditions or dead locks due > > to a > > > slightly different synchronization scheme? > > > > A change of architecture shouldn't introduce any additional race > > conditions > > or deadlocks, no. If this were the case, then I agree that this example > > would > > be far too fragile to include. Any standards compliant implementation of > > "__atomic_compare_exchange_n" will work fine - including the default one > > used > > by ARM platforms. As the algorithm underlying this example is supposed to > > be > > lock-free, though, a proper lock-free compare exchange implementation is > > the > > icing on the cake, showing off the properties of the algorithm in full. > > > > > Does the *example* need to show of lock-freeness? Would be enough to just > show of how IP frag/reassembly is usually done on ODP? > > Could all assembly and ARM #ifdefs be removed if we simply don't claim the > code to be lock-free ? > > -Petri > > I vote for saying yes here. I.e. this example should show lock-freeness and best performance usage doing ip frag with ODP. We can say that some examples can be very simple and minimal. Other examples might be complex and effective. Idea for first one is to show how some code works and idea for second one is to provide some building block of complex thing which can be reused by application developers. And that is great help from ARM writing such efficient building bocks for us. Maxim.
Re: [lng-odp] [API-NEXT PATCH v2 0/4] Deprecated macros
On 13.04.2017 10:33, Savolainen, Petri (Nokia - FI/Espoo) wrote: > > >> -Original Message- >> From: Dmitry Eremin-Solenikov [mailto:dmitry.ereminsoleni...@linaro.org] >> Sent: Wednesday, April 12, 2017 3:11 PM >> To: Savolainen, Petri (Nokia - FI/Espoo) > labs.com>; lng-odp@lists.linaro.org >> Subject: Re: [lng-odp] [API-NEXT PATCH v2 0/4] Deprecated macros >> >> On 12.04.2017 14:50, Savolainen, Petri (Nokia - FI/Espoo) wrote: >>> >>> -Original Message- From: Dmitry Eremin-Solenikov >> [mailto:dmitry.ereminsoleni...@linaro.org] Sent: Wednesday, April 12, 2017 2:32 PM To: Petri Savolainen ; lng- o...@lists.linaro.org Subject: Re: [lng-odp] [API-NEXT PATCH v2 0/4] Deprecated macros On 30.03.2017 16:58, Petri Savolainen wrote: > Replaced ODP_DEPRECATED macro (which was based on GCC __attribute__) with > compiler independent mechanism to control if deprecated API >> definitions are > visible to the application. ODP_DEPRECATED_API can be used both in application > and implementation to check if deprecated APIs are enabled. By default those are > disabled. Implementation may optimize the normal (new API) code path. > > ODP_DEPRECATE() macro is used to rename definitions, so that data structure > sizes are equal on both options. This enables implementation to serve both > options with a single library (if it wishes to do so). My main question remains as it was before: is it possible for the distribution to supply (unoptimized) ODP binary and headers and then >> for the application to select if it builds with or without deprecated API using that binary/headers? >>> >>> >>> Yes. This patch set keeps the same struct fields/etc for both modes - >> only the names are scrambled (with __deprecated_ prefix) when deprecated >> APIs are not supported (the default). >> >> If so. Consider I have built and installed ODP headers & binary built >> with --enable-deprecated-api. > > So, you have built and installed the implementation with > --enable-deprecated-api. This means that applications using this install, see > deprecated APIs. A distribution decides which way it supports ODP, with or > without deprecated stuff. The default in our (odp-linux) configure is > without, to minimize confusion and promote the new API definitions which > should be always better for application than the old ones. > I was advocating for binary distributions and developers using packages from those distributions. And for those distributions providing _single_ binary version of ODP. -- With best wishes Dmitry
Re: [lng-odp] [PATCH v3] example: add IPv4 fragmentation/reassembly example
> > ODP is about HW abstraction and CPU instructions are HW features. Of > course, > > applications are free to use other libs/APIs/GCC features, but should > our > > examples do that? > > I'm not sure I agree that ODP primarily exists to abstract non-accelerator > HW, as far as I'm concerned that's really the job of the language, other > libraries, and at the end of the day the programmer. (Talk about biting > off > more than you can chew: abstracting all hardware in the system?!) > But regardless... CPU cycle count read may be an instruction. Time counter read may an instruction. 128 bit atomic CAS may be an instruction. AES crypto may have multiple instructions. Queue enqueue may be an instruction. Memory barrier may be embedded into multiple instructions... An application that uses these instruction directly is not portable. An application that uses those through ODP API is portable. > > > "There is a single exception to this, in that the standard 128-bit > > "__atomic_compare_exchange_n" implementation on ARM is not lock-free > ..." > > > > This is the point. How about the built-in implementation on other CPU > > architectures: MIPS, PPC, Tilera, Kalray, ... etc ? Does the code build > on > > those. If it does build, does it have race conditions or dead locks due > to a > > slightly different synchronization scheme? > > A change of architecture shouldn't introduce any additional race > conditions > or deadlocks, no. If this were the case, then I agree that this example > would > be far too fragile to include. Any standards compliant implementation of > "__atomic_compare_exchange_n" will work fine - including the default one > used > by ARM platforms. As the algorithm underlying this example is supposed to > be > lock-free, though, a proper lock-free compare exchange implementation is > the > icing on the cake, showing off the properties of the algorithm in full. > Does the *example* need to show of lock-freeness? Would be enough to just show of how IP frag/reassembly is usually done on ODP? Could all assembly and ARM #ifdefs be removed if we simply don't claim the code to be lock-free ? -Petri
[lng-odp] [Bug 2940] odp_packet_seg_t fails ABI compatibility between odp-linux and odp-dpdk
https://bugs.linaro.org/show_bug.cgi?id=2940 --- Comment #2 from Maxim Uvarov --- test: new comment from python script 1492079015.45 -- You are receiving this mail because: You are on the CC list for the bug.
Re: [lng-odp] [PATCH v3] example: add IPv4 fragmentation/reassembly example
On 13.04.2017 12:55, Savolainen, Petri (Nokia - FI/Espoo) wrote: > > >> -Original Message- >> From: Joe Savage [mailto:joe.sav...@arm.com] >> Sent: Thursday, April 13, 2017 11:57 AM >> To: Savolainen, Petri (Nokia - FI/Espoo) > labs.com>; Dmitry Eremin-Solenikov ; >> Brian Brooks >> Cc: nd ; lng-odp@lists.linaro.org >> Subject: Re: [lng-odp] [PATCH v3] example: add IPv4 >> fragmentation/reassembly example >> >> You can have arm, arm64, mips64, etc. directories inside >> examples/ipfrag. > > This does not scale in practice. If another example comes along, >> does > that also need to split whatever is determined to be "arch" code >> into > "arch" directories within its own folder? > > I recommend that this contribution be accepted without any further change. From my perspective, separate arm, aarch64 and generic headers would >> be sufficient, even if they are not put under separate directories. That was just a suggestion. >>> >>> The main concern I have with this is that: ODP examples should >> demonstrate how >>> ODP APIs are used to do X. In other words: "If you implement X like this >> with >>> ODP APIs, your code is: >>> >>> * 100% portable >>> * still good performance >>> * clean and easy to understand / maintain" >>> >>> Super optimized, architecture specific implementation of X would not >> drive that >>> purpose. It also leads to maintenance issues later on - a bug found and >> fixed >>> on CPU arch Y does not get automatically fixed for arch Z. >>> >>> We are drawing a line here if embedded assembly or otherwise arch >> dependent >>> code belongs to example applications. I'd say it does not, since HW >>> abstraction is the sole purpose of ODP. >> >> You know far more about ODP than I, so please correct me if I'm wrong >> here, >> but my understanding is that ODP is primarily about abstracting hardware >> **accelerators** (in a networking context). It does not, and should not, >> be >> overly involved with the details of how programmers may wish to write and >> abstract other parts of their programs. >> >> This example should run perfectly well on a wide range of CPUs - that is, >> pretty much anything with 32 bit or 64 bit pointers and that has a >> compiler >> supporting GCC's "__atomic_compare_exchange_n" for double the pointer size >> - >> and consists of code that is almost entirely independent of other >> architectural >> details. There is a single exception to this, in that the standard 128-bit >> "__atomic_compare_exchange_n" implementation on ARM is not lock-free, and >> so >> there are some short snippets of inline assembly, confined to a single >> file >> (in the new version of the patch I have locally), that resolve this. I >> don't >> see a problem here. > > > ODP is about HW abstraction and CPU instructions are HW features. Of course, > applications are free to use other libs/APIs/GCC features, but should our > examples do that? > > "There is a single exception to this, in that the standard 128-bit > "__atomic_compare_exchange_n" implementation on ARM is not lock-free ..." > > This is the point. How about the built-in implementation on other CPU > architectures: MIPS, PPC, Tilera, Kalray, ... etc ? Does the code build on > those. If it does build, does it have race conditions or dead locks due to a > slightly different synchronization scheme? > > I think the first option would be to remove 128 atomics. Other option is to > separate those into different header files, test this on different archs and > enable it only on tested archs (may be only on x86 and arm first). I would > not want an example to fail on e.g. Kalray platform, just because the example > decided to by-pass ODP API. > > -Petri > > I think we can not ask Joe to test this code on any platform. Simple he does not have such hardware and time. Maybe it's better to stage this example tp api-next. And everybody can play with it before we merge it to master. Next updates also can go with small incremental patches. Maxim.
Re: [lng-odp] [PATCH v3] example: add IPv4 fragmentation/reassembly example
> > > > >> You can have arm, arm64, mips64, etc. directories inside > > > > >> examples/ipfrag. > > > > > > > > > > This does not scale in practice. If another example comes along, does > > > > > that also need to split whatever is determined to be "arch" code into > > > > > "arch" directories within its own folder? > > > > > > > > > > I recommend that this contribution be accepted without any further > > > > change. > > > > > > > > From my perspective, separate arm, aarch64 and generic headers would be > > > > sufficient, even if they are not put under separate directories. That > > > > was just a suggestion. > > > > > > > > > > The main concern I have with this is that: ODP examples should > > > demonstrate how > > > ODP APIs are used to do X. In other words: "If you implement X like this > > > with > > > ODP APIs, your code is: > > > > > > * 100% portable > > > * still good performance > > > * clean and easy to understand / maintain" > > > > > > Super optimized, architecture specific implementation of X would not > > > drive that > > > purpose. It also leads to maintenance issues later on - a bug found and > > > fixed > > > on CPU arch Y does not get automatically fixed for arch Z. > > > > > > We are drawing a line here if embedded assembly or otherwise arch > > > dependent > > > code belongs to example applications. I'd say it does not, since HW > > > abstraction is the sole purpose of ODP. > > > > You know far more about ODP than I, so please correct me if I'm wrong here, > > but my understanding is that ODP is primarily about abstracting hardware > > **accelerators** (in a networking context). It does not, and should not, be > > overly involved with the details of how programmers may wish to write and > > abstract other parts of their programs. > > > > This example should run perfectly well on a wide range of CPUs — that is, > > pretty much anything with 32 bit or 64 bit pointers and that has a compiler > > supporting GCC's "__atomic_compare_exchange_n" for double the pointer size — > > and consists of code that is almost entirely independent of other > > architectural > > details. There is a single exception to this, in that the standard 128-bit > > "__atomic_compare_exchange_n" implementation on ARM is not lock-free, and so > > there are some short snippets of inline assembly, confined to a single file > > (in the new version of the patch I have locally), that resolve this. I don't > > see a problem here. > > ODP is about HW abstraction and CPU instructions are HW features. Of course, > applications are free to use other libs/APIs/GCC features, but should our > examples do that? I'm not sure I agree that ODP primarily exists to abstract non-accelerator HW, as far as I'm concerned that's really the job of the language, other libraries, and at the end of the day the programmer. (Talk about biting off more than you can chew: abstracting all hardware in the system?!) But regardless... > "There is a single exception to this, in that the standard 128-bit > "__atomic_compare_exchange_n" implementation on ARM is not lock-free ..." > > This is the point. How about the built-in implementation on other CPU > architectures: MIPS, PPC, Tilera, Kalray, ... etc ? Does the code build on > those. If it does build, does it have race conditions or dead locks due to a > slightly different synchronization scheme? A change of architecture shouldn't introduce any additional race conditions or deadlocks, no. If this were the case, then I agree that this example would be far too fragile to include. Any standards compliant implementation of "__atomic_compare_exchange_n" will work fine — including the default one used by ARM platforms. As the algorithm underlying this example is supposed to be lock-free, though, a proper lock-free compare exchange implementation is the icing on the cake, showing off the properties of the algorithm in full. > I think the first option would be to remove 128 atomics. Other option is to > separate those into different header files, test this on different archs and > enable it only on tested archs (may be only on x86 and arm first). I would > not want an example to fail on e.g. Kalray platform, just because the example > decided to by-pass ODP API. > > -Petri
Re: [lng-odp] [PATCH v3] example: add IPv4 fragmentation/reassembly example
> -Original Message- > From: Joe Savage [mailto:joe.sav...@arm.com] > Sent: Thursday, April 13, 2017 11:57 AM > To: Savolainen, Petri (Nokia - FI/Espoo) labs.com>; Dmitry Eremin-Solenikov ; > Brian Brooks > Cc: nd ; lng-odp@lists.linaro.org > Subject: Re: [lng-odp] [PATCH v3] example: add IPv4 > fragmentation/reassembly example > > > > >> You can have arm, arm64, mips64, etc. directories inside > > > >> examples/ipfrag. > > > > > > > > This does not scale in practice. If another example comes along, > does > > > > that also need to split whatever is determined to be "arch" code > into > > > > "arch" directories within its own folder? > > > > > > > > I recommend that this contribution be accepted without any further > > > change. > > > > > > From my perspective, separate arm, aarch64 and generic headers would > be > > > sufficient, even if they are not put under separate directories. That > > > was just a suggestion. > > > > > > > The main concern I have with this is that: ODP examples should > demonstrate how > > ODP APIs are used to do X. In other words: "If you implement X like this > with > > ODP APIs, your code is: > > > > * 100% portable > > * still good performance > > * clean and easy to understand / maintain" > > > > Super optimized, architecture specific implementation of X would not > drive that > > purpose. It also leads to maintenance issues later on - a bug found and > fixed > > on CPU arch Y does not get automatically fixed for arch Z. > > > > We are drawing a line here if embedded assembly or otherwise arch > dependent > > code belongs to example applications. I'd say it does not, since HW > > abstraction is the sole purpose of ODP. > > You know far more about ODP than I, so please correct me if I'm wrong > here, > but my understanding is that ODP is primarily about abstracting hardware > **accelerators** (in a networking context). It does not, and should not, > be > overly involved with the details of how programmers may wish to write and > abstract other parts of their programs. > > This example should run perfectly well on a wide range of CPUs - that is, > pretty much anything with 32 bit or 64 bit pointers and that has a > compiler > supporting GCC's "__atomic_compare_exchange_n" for double the pointer size > - > and consists of code that is almost entirely independent of other > architectural > details. There is a single exception to this, in that the standard 128-bit > "__atomic_compare_exchange_n" implementation on ARM is not lock-free, and > so > there are some short snippets of inline assembly, confined to a single > file > (in the new version of the patch I have locally), that resolve this. I > don't > see a problem here. ODP is about HW abstraction and CPU instructions are HW features. Of course, applications are free to use other libs/APIs/GCC features, but should our examples do that? "There is a single exception to this, in that the standard 128-bit "__atomic_compare_exchange_n" implementation on ARM is not lock-free ..." This is the point. How about the built-in implementation on other CPU architectures: MIPS, PPC, Tilera, Kalray, ... etc ? Does the code build on those. If it does build, does it have race conditions or dead locks due to a slightly different synchronization scheme? I think the first option would be to remove 128 atomics. Other option is to separate those into different header files, test this on different archs and enable it only on tested archs (may be only on x86 and arm first). I would not want an example to fail on e.g. Kalray platform, just because the example decided to by-pass ODP API. -Petri
Re: [lng-odp] [PATCH v5 1/3] validation: packet: increase test pool size
Ping.
> On 6 Apr 2017, at 14:41, Krishna Garapati
> wrote:
>
> for this patch series,
>
> Reviewed-by: Balakrishna Garapati
>
> /Krishna
>
> On 31 March 2017 at 14:18, Matias Elo wrote:
> Previously packet_test_concatsplit() could fail on some pool
> implementations as the pool ran out of buffers. Increase default pools size
> and use capability to make sure the value is valid.
>
> Signed-off-by: Matias Elo
> ---
> test/common_plat/validation/api/packet/packet.c | 7 ++-
> 1 file changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/test/common_plat/validation/api/packet/packet.c
> b/test/common_plat/validation/api/packet/packet.c
> index 669122a..1997139 100644
> --- a/test/common_plat/validation/api/packet/packet.c
> +++ b/test/common_plat/validation/api/packet/packet.c
> @@ -13,6 +13,8 @@
> #define PACKET_BUF_LEN ODP_CONFIG_PACKET_SEG_LEN_MIN
> /* Reserve some tailroom for tests */
> #define PACKET_TAILROOM_RESERVE 4
> +/* Number of packets in the test packet pool */
> +#define PACKET_POOL_NUM 300
>
> static odp_pool_t packet_pool, packet_pool_no_uarea,
> packet_pool_double_uarea;
> static uint32_t packet_len;
> @@ -109,6 +111,7 @@ int packet_suite_init(void)
> uint32_t udat_size;
> uint8_t data = 0;
> uint32_t i;
> + uint32_t num = PACKET_POOL_NUM;
>
> if (odp_pool_capability(&capa) < 0) {
> printf("pool_capability failed\n");
> @@ -130,13 +133,15 @@ int packet_suite_init(void)
> segmented_packet_len = capa.pkt.min_seg_len *
>capa.pkt.max_segs_per_pkt;
> }
> + if (capa.pkt.max_num != 0 && capa.pkt.max_num < num)
> + num = capa.pkt.max_num;
>
> odp_pool_param_init(¶ms);
>
> params.type = ODP_POOL_PACKET;
> params.pkt.seg_len= capa.pkt.min_seg_len;
> params.pkt.len= capa.pkt.min_seg_len;
> - params.pkt.num= 100;
> + params.pkt.num= num;
> params.pkt.uarea_size = sizeof(struct udata_struct);
>
> packet_pool = odp_pool_create("packet_pool", ¶ms);
> --
> 2.7.4
>
>
Re: [lng-odp] [PATCH v3] example: add IPv4 fragmentation/reassembly example
> > >> You can have arm, arm64, mips64, etc. directories inside > > >> examples/ipfrag. > > > > > > This does not scale in practice. If another example comes along, does > > > that also need to split whatever is determined to be "arch" code into > > > "arch" directories within its own folder? > > > > > > I recommend that this contribution be accepted without any further > > change. > > > > From my perspective, separate arm, aarch64 and generic headers would be > > sufficient, even if they are not put under separate directories. That > > was just a suggestion. > > > > The main concern I have with this is that: ODP examples should demonstrate how > ODP APIs are used to do X. In other words: "If you implement X like this with > ODP APIs, your code is: > > * 100% portable > * still good performance > * clean and easy to understand / maintain" > > Super optimized, architecture specific implementation of X would not drive > that > purpose. It also leads to maintenance issues later on - a bug found and fixed > on CPU arch Y does not get automatically fixed for arch Z. > > We are drawing a line here if embedded assembly or otherwise arch dependent > code belongs to example applications. I'd say it does not, since HW > abstraction is the sole purpose of ODP. You know far more about ODP than I, so please correct me if I'm wrong here, but my understanding is that ODP is primarily about abstracting hardware **accelerators** (in a networking context). It does not, and should not, be overly involved with the details of how programmers may wish to write and abstract other parts of their programs. This example should run perfectly well on a wide range of CPUs — that is, pretty much anything with 32 bit or 64 bit pointers and that has a compiler supporting GCC's "__atomic_compare_exchange_n" for double the pointer size — and consists of code that is almost entirely independent of other architectural details. There is a single exception to this, in that the standard 128-bit "__atomic_compare_exchange_n" implementation on ARM is not lock-free, and so there are some short snippets of inline assembly, confined to a single file (in the new version of the patch I have locally), that resolve this. I don't see a problem here.
Re: [lng-odp] [PATCH] abi: packet: restore abi compatibility for odp_packet_seg_t type
On 13 April 2017 at 00:06, Bill Fischofer wrote:
> When running in --enable-abi-compat=yes mode, all ODP types need to be
> of pointer width in the default ABI definition. The optimization of the
> odp_packet_seg_t type to uint8_t can only be supported when running in
> --enable-abi-compate=no mode. Change the ODP packet routines to use
> type converter routines that have varying definitions based on whether
> we're running in ABI compatibility mode and provide these variant
> definitions to enable proper ABI compatibility while still supporting an
> optimized typedef for non-ABI mode.
>
> This resolves Bug https://bugs.linaro.org/show_bug.cgi?id=2940
>
> Signed-off-by: Bill Fischofer
> ---
> include/odp/arch/default/api/abi/packet.h | 7 +--
> .../include/odp/api/plat/packet_inlines.h | 21
> ++---
> .../include/odp/api/plat/packet_types.h | 10 ++
> platform/linux-generic/odp_packet.c | 12 +++-
> 4 files changed, 40 insertions(+), 10 deletions(-)
>
> diff --git a/include/odp/arch/default/api/abi/packet.h
> b/include/odp/arch/default/api/abi/packet.h
> index 60a41b8..4aac75b 100644
> --- a/include/odp/arch/default/api/abi/packet.h
> +++ b/include/odp/arch/default/api/abi/packet.h
> @@ -16,15 +16,18 @@ extern "C" {
> /** @internal Dummy type for strong typing */
> typedef struct { char dummy; /**< @internal Dummy */ } _odp_abi_packet_t;
>
> +/** @internal Dummy type for strong typing */
> +typedef struct { char dummy; /**< *internal Dummy */ }
> _odp_abi_packet_seg_t;
> +
> /** @ingroup odp_packet
> * @{
> */
>
> typedef _odp_abi_packet_t *odp_packet_t;
> -typedef uint8_todp_packet_seg_t;
> +typedef _odp_abi_packet_seg_t *odp_packet_seg_t;
>
> #define ODP_PACKET_INVALID((odp_packet_t)0x)
> -#define ODP_PACKET_SEG_INVALID((odp_packet_seg_t)-1)
> +#define ODP_PACKET_SEG_INVALID((odp_packet_seg_t)0x)
> #define ODP_PACKET_OFFSET_INVALID (0x0fff)
>
> typedef enum {
> diff --git a/platform/linux-generic/include/odp/api/plat/packet_inlines.h
> b/platform/linux-generic/include/odp/api/plat/packet_inlines.h
> index eb36aa9..bd735c1 100644
> --- a/platform/linux-generic/include/odp/api/plat/packet_inlines.h
> +++ b/platform/linux-generic/include/odp/api/plat/packet_inlines.h
> @@ -21,6 +21,20 @@
> /** @internal Inline function offsets */
> extern const _odp_packet_inline_offset_t _odp_packet_inline;
>
> +#if ODP_ABI_COMPAT
>
Everywhere it explicitly says ODP_ABI_COMPAT == 1 or 0 except here. Would
be good to see it here the same way.
> +/** @internal Inline function @param seg @return */
> +static inline int _odp_packet_seg_to_ndx(odp_packet_seg_t seg)
>
is "_ndx" a typo instead of _idx ?.
> +{
> + return _odp_typeval(seg);
>
odp_typeval converts "seg" to uint32_t and we return it as int.
> +}
> +
> +/** @internal Inline function @param ndx @return */
> +static inline odp_packet_seg_t _odp_packet_seg_from_ndx(int ndx)
> +{
> + return _odp_cast_scalar(odp_packet_seg_t, ndx);
> +}
> +#endif
> +
> /** @internal Inline function @param pkt @return */
> static inline void *_odp_packet_data(odp_packet_t pkt)
> {
> @@ -128,20 +142,21 @@ static inline odp_packet_seg_t
> _odp_packet_first_seg(odp_packet_t pkt)
> {
> (void)pkt;
>
> - return 0;
> + return _odp_packet_seg_from_ndx(0);
> }
>
> /** @internal Inline function @param pkt @return */
> static inline odp_packet_seg_t _odp_packet_last_seg(odp_packet_t pkt)
> {
> - return _odp_packet_num_segs(pkt) - 1;
> + return _odp_packet_seg_from_ndx(_odp_packet_num_segs(pkt) - 1);
> }
>
> /** @internal Inline function @param pkt @param seg @return */
> static inline odp_packet_seg_t _odp_packet_next_seg(odp_packet_t pkt,
> odp_packet_seg_t seg)
> {
> - if (odp_unlikely(seg >= _odp_packet_last_seg(pkt)))
> + if (odp_unlikely(_odp_packet_seg_to_ndx(seg) >=
> +_odp_packet_seg_to_ndx(_odp_
> packet_last_seg(pkt
> return ODP_PACKET_SEG_INVALID;
>
> return seg + 1;
> diff --git a/platform/linux-generic/include/odp/api/plat/packet_types.h
> b/platform/linux-generic/include/odp/api/plat/packet_types.h
> index b8f665d..59e8b2f 100644
> --- a/platform/linux-generic/include/odp/api/plat/packet_types.h
> +++ b/platform/linux-generic/include/odp/api/plat/packet_types.h
> @@ -40,6 +40,16 @@ typedef ODP_HANDLE_T(odp_packet_t);
>
> typedef uint8_t odp_packet_seg_t;
>
> +static inline int _odp_packet_seg_to_ndx(odp_packet_seg_t seg)
> +{
> + return (int)seg;
> +}
> +
> +static inline odp_packet_seg_t _odp_packet_seg_from_ndx(int ndx)
> +{
> + return (odp_packet_seg_t)ndx;
> +}
> +
> #define ODP_PACKET_SEG_INVALID ((odp_packet_seg_t)-1)
>
> typedef enum {
> diff --git a/platform/linux-generic/odp_packet.c
> b/platform/linux-generic/odp_packet.c
> index b8aac6b..e99
Re: [lng-odp] [PATCH v2] build: fix native Clang build on ARMv8
> -Original Message- > From: lng-odp [mailto:lng-odp-boun...@lists.linaro.org] On Behalf Of > Dmitry Eremin-Solenikov > Sent: Thursday, April 13, 2017 1:13 AM > To: Brian Brooks ; lng-odp@lists.linaro.org > Subject: Re: [lng-odp] [PATCH v2] build: fix native Clang build on ARMv8 > > On 12.04.2017 21:30, Brian Brooks wrote: > > The build is broken when using clang on ARM. -mcx16 is being passed to > > clang when building natively on ARM. This combined with -Werror causes > > the breakage. Fix it by skipping anything related to -mcx16 when not > > building for x86-based architectures. See [1] for details. > > > > [1] https://lists.linaro.org/pipermail/lng-odp/2017-April/029684.html > > > > v2 > > - Add more description to commit message (Dmitry) This "v2" information should be under --- below. Otherwise it shows up in git log. Remove is from commit log and edit it there manually after format-patch command. -Petri > > > > Signed-off-by: Brian Brooks > > --- > > configure.ac | 30 -- > > 1 file changed, 16 insertions(+), 14 deletions(-) > > Reviewed-by: Dmitry Eremin-Solenikov
Re: [lng-odp] [PATCH v2 1/3] linux-gen: add internal helper for reading system thread id
Ping.
> On 28 Mar 2017, at 10:41, Elo, Matias (Nokia - FI/Espoo)
> wrote:
>
> Ping.
>
>> On 17 Mar 2017, at 14:16, Matias Elo wrote:
>>
>> Signed-off-by: Matias Elo
>> ---
>> platform/linux-generic/Makefile.am | 1 +
>> platform/linux-generic/include/odp_thread_internal.h | 20
>>
>> platform/linux-generic/odp_thread.c | 10 ++
>> 3 files changed, 31 insertions(+)
>> create mode 100644 platform/linux-generic/include/odp_thread_internal.h
>>
>> diff --git a/platform/linux-generic/Makefile.am
>> b/platform/linux-generic/Makefile.am
>> index 056ba67..b2ae971 100644
>> --- a/platform/linux-generic/Makefile.am
>> +++ b/platform/linux-generic/Makefile.am
>> @@ -144,6 +144,7 @@ noinst_HEADERS = \
>>${srcdir}/include/odp_schedule_if.h \
>>${srcdir}/include/odp_sorted_list_internal.h \
>>${srcdir}/include/odp_shm_internal.h \
>> + ${srcdir}/include/odp_thread_internal.h \
>>${srcdir}/include/odp_timer_internal.h \
>>${srcdir}/include/odp_timer_wheel_internal.h \
>>${srcdir}/include/odp_traffic_mngr_internal.h \
>> diff --git a/platform/linux-generic/include/odp_thread_internal.h
>> b/platform/linux-generic/include/odp_thread_internal.h
>> new file mode 100644
>> index 000..9a8e482
>> --- /dev/null
>> +++ b/platform/linux-generic/include/odp_thread_internal.h
>> @@ -0,0 +1,20 @@
>> +/* Copyright (c) 2017, Linaro Limited
>> + * All rights reserved.
>> + *
>> + * SPDX-License-Identifier: BSD-3-Clause
>> + */
>> +
>> +#ifndef ODP_THREAD_INTERNAL_H_
>> +#define ODP_THREAD_INTERNAL_H_
>> +
>> +#ifdef __cplusplus
>> +extern "C" {
>> +#endif
>> +
>> +pid_t sys_thread_id(void);
>> +
>> +#ifdef __cplusplus
>> +}
>> +#endif
>> +
>> +#endif
>> diff --git a/platform/linux-generic/odp_thread.c
>> b/platform/linux-generic/odp_thread.c
>> index 33a8a7f..e98fa7a 100644
>> --- a/platform/linux-generic/odp_thread.c
>> +++ b/platform/linux-generic/odp_thread.c
>> @@ -17,15 +17,19 @@
>> #include
>> #include
>> #include
>> +#include
>>
>> #include
>> #include
>> #include
>> +#include
>> +#include
>>
>> typedef struct {
>> int thr;
>> int cpu;
>> odp_thread_type_t type;
>> +pid_t sys_thr_id;
>> } thread_state_t;
>>
>>
>> @@ -135,6 +139,11 @@ static int free_id(int thr)
>> return thread_globals->num;
>> }
>>
>> +pid_t sys_thread_id(void)
>> +{
>> +return this_thread->sys_thr_id;
>> +}
>> +
>> int odp_thread_init_local(odp_thread_type_t type)
>> {
>> int id;
>> @@ -159,6 +168,7 @@ int odp_thread_init_local(odp_thread_type_t type)
>> thread_globals->thr[id].thr = id;
>> thread_globals->thr[id].cpu = cpu;
>> thread_globals->thr[id].type = type;
>> +thread_globals->thr[id].sys_thr_id = (pid_t)syscall(SYS_gettid);
>>
>> this_thread = &thread_globals->thr[id];
>>
>> --
>> 2.7.4
>>
>
Re: [lng-odp] [PATCH v3] example: add IPv4 fragmentation/reassembly example
> >> > >> You can have arm, arm64, mips64, etc. directories inside > >> examples/ipfrag. > > > > This does not scale in practice. If another example comes along, does > > that also need to split whatever is determined to be "arch" code into > > "arch" directories within its own folder? > > > > I recommend that this contribution be accepted without any further > change. > > From my perspective, separate arm, aarch64 and generic headers would be > sufficient, even if they are not put under separate directories. That > was just a suggestion. > The main concern I have with this is that: ODP examples should demonstrate how ODP APIs are used to do X. In other words: "If you implement X like this with ODP APIs, your code is: * 100% portable * still good performance * clean and easy to understand / maintain" Super optimized, architecture specific implementation of X would not drive that purpose. It also leads to maintenance issues later on - a bug found and fixed on CPU arch Y does not get automatically fixed for arch Z. We are drawing a line here if embedded assembly or otherwise arch dependent code belongs to example applications. I'd say it does not, since HW abstraction is the sole purpose of ODP. -Petri
Re: [lng-odp] [API-NEXT PATCH v2 0/4] Deprecated macros
> -Original Message- > From: Dmitry Eremin-Solenikov [mailto:dmitry.ereminsoleni...@linaro.org] > Sent: Wednesday, April 12, 2017 3:11 PM > To: Savolainen, Petri (Nokia - FI/Espoo) labs.com>; lng-odp@lists.linaro.org > Subject: Re: [lng-odp] [API-NEXT PATCH v2 0/4] Deprecated macros > > On 12.04.2017 14:50, Savolainen, Petri (Nokia - FI/Espoo) wrote: > > > > > >> -Original Message- > >> From: Dmitry Eremin-Solenikov > [mailto:dmitry.ereminsoleni...@linaro.org] > >> Sent: Wednesday, April 12, 2017 2:32 PM > >> To: Petri Savolainen ; lng- > >> o...@lists.linaro.org > >> Subject: Re: [lng-odp] [API-NEXT PATCH v2 0/4] Deprecated macros > >> > >> On 30.03.2017 16:58, Petri Savolainen wrote: > >>> Replaced ODP_DEPRECATED macro (which was based on GCC __attribute__) > >> with > >>> compiler independent mechanism to control if deprecated API > definitions > >> are > >>> visible to the application. ODP_DEPRECATED_API can be used both in > >> application > >>> and implementation to check if deprecated APIs are enabled. By default > >> those are > >>> disabled. Implementation may optimize the normal (new API) code path. > >>> > >>> ODP_DEPRECATE() macro is used to rename definitions, so that data > >> structure > >>> sizes are equal on both options. This enables implementation to serve > >> both > >>> options with a single library (if it wishes to do so). > >> > >> My main question remains as it was before: is it possible for the > >> distribution to supply (unoptimized) ODP binary and headers and then > for > >> the application to select if it builds with or without deprecated API > >> using that binary/headers? > > > > > > Yes. This patch set keeps the same struct fields/etc for both modes - > only the names are scrambled (with __deprecated_ prefix) when deprecated > APIs are not supported (the default). > > If so. Consider I have built and installed ODP headers & binary built > with --enable-deprecated-api. So, you have built and installed the implementation with --enable-deprecated-api. This means that applications using this install, see deprecated APIs. A distribution decides which way it supports ODP, with or without deprecated stuff. The default in our (odp-linux) configure is without, to minimize confusion and promote the new API definitions which should be always better for application than the old ones. > > How do I build: > > - application that uses deprecated API? > [I assume that the answer to this question is trivial: just build as > is]. If you build with our make system: use --enable-deprecated-api. If you do not, then first build and install the implementation with --enable-deprecated-api, and then build against that (== installed ODP API headers contain old stuff). > > - application that wants to be sure that it does not use deprecated API? If you build with our make system: use --disable-deprecated-api. If you do not, then first build and install the implementation with --disable-deprecated-api, and then build against that (== installed ODP API headers have renamed old stuff, so that application build fails if it still uses those) -Petri
Re: [lng-odp] [API-NEXT PATCH 0/4] add maximum packet counts to pool info
> On 12 Apr 2017, at 22:52, Bill Fischofer wrote: > > On Wed, Apr 12, 2017 at 7:58 AM, Matias Elo wrote: >> On some packet pool implementations the number of available packets may vary >> depending on the packet length (examples below). E.g. a packet pool may >> include >> smaller sub-pools for different packet length ranges. > > I'm not sure what the motivation is for this proposed change, but the > pkt.num specified on odp_pool_create() sets the maximum number of > packets that can be allocated from the pool, not the minimum. This is > combined with the pkt.len field to say that pkt.num packets of len > pkt.len can be allocated. The need for this information originally came up when fixing an issue in pool_create(). pool_create() didn't allocate enough packets if the requested length (pkt.len) packets were segmented. As a result of this fix it is now possible to allocate more than pkt.num packets from the pool, if the requested packet length fits into a single segment (or less segments than the original pkt.len). The spec of pkt.num is changed to "The exact number of 'len' byte packets that the pool must provide.". > If the application allocated packets larger > than this size (up to pkt.max_len) then the actual number of packets > that can be successfully allocated from the pool may be lower than > pkt.num, but it will never be greater. Depending on the pool implementation this may not always be the case (not likely) and the ascii pictures in the cover letter tried to visualise this possibility. For example, a pool may consists of smaller HW sub-pools for different packet length ranges and these sub-pools may be different in size. > > The reason for wanting the maximum number is to bound the number of > "in flight" packets for QoS purposes. As a pool reaches this limit, > RED and similar algorithms kick in to start dropping packets at RX. If > there is no fixed maximum number of packets that makes QoS processing > erratic. > There is still a maximum and an application can query it with odp_pool_info() (odp_pool_info_t.pkt.max_num). RED and similar work as previously as there is no API for configuring them in ODP. > We had previously floated the idea of allowing pool groups to be > defined which would accommodate the practice where packets get > assigned to different sub-pools based on their size, however this was > seen to be overly complicated, especially on platforms that have > hardware buffer managers. True, this gets complicated fast. The updated odp_pool_info_t includes new maximum packet counts to provide more information to the application without going to the specifics of the underlying pool implementation. > Do you have a specific use case that can't be handled by the current > structures? Described above. -Matias
Re: [lng-odp] [PATCHv2] linux-gen: pktio: miss an unlock operation before exit if error happens
Is this patch able to be merged if no more comments?
Kevin
2017-04-10 13:43 GMT+08:00 Kevin Wang :
> Just set the return value, and remove the return() function at the
> failure branch.
> https://bugs.linaro.org/show_bug.cgi?id=2933
>
> Signed-off-by: Kevin Wang
> Reviewed-by: Ola Liljedahl
> Reviewed-by: Brian Brooks
> ---
> platform/linux-generic/pktio/loop.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/platform/linux-generic/pktio/loop.c b/platform/linux-generic/
> pktio/loop.c
> index 7096283..61e98ad 100644
> --- a/platform/linux-generic/pktio/loop.c
> +++ b/platform/linux-generic/pktio/loop.c
> @@ -176,7 +176,7 @@ static int loopback_send(pktio_entry_t *pktio_entry,
> int index ODP_UNUSED,
> pktio_entry->s.stats.out_octets += bytes;
> } else {
> ODP_DBG("queue enqueue failed %i\n", ret);
> - return -1;
> + ret = -1;
> }
>
> odp_ticketlock_unlock(&pktio_entry->s.txl);
> --
> 2.7.4
>
>
--
Thanks,
Kevin
