[v2] Added timeout buffer type

New buffer types: timeout and any. Timeout is used for timeout nofications.
A pool initilized with buffer type 'any' can allocate buffers for different
usage (raw, packet, timeout, ...).

Signed-off-by: Petri Savolainen <petri.savolainen@linaro.org>
---
 include/odp_buffer.h                               |  8 ++--
 include/odp_timer.h                                | 31 +++++++++++--
 .../linux-generic/include/odp_buffer_internal.h    |  5 ++-
 .../linux-generic/include/odp_timer_internal.h     | 52 ++++++++++++++++++++++
 platform/linux-generic/source/odp_buffer.c         |  4 +-
 platform/linux-generic/source/odp_buffer_pool.c    | 33 +++++++++++---
 platform/linux-generic/source/odp_timer.c          | 13 ++++++
 7 files changed, 130 insertions(+), 16 deletions(-)
 create mode 100644 platform/linux-generic/include/odp_timer_internal.h

On 2014-05-15 16:44, Petri Savolainen wrote:
> New buffer types: timeout and any. Timeout is used for timeout nofications.
> A pool initilized with buffer type 'any' can allocate buffers for different
> usage (raw, packet, timeout, ...).
> 
> Signed-off-by: Petri Savolainen <petri.savolainen@linaro.org>
> ---
>  include/odp_buffer.h                               |  8 ++--
>  include/odp_timer.h                                | 31 +++++++++++--
>  .../linux-generic/include/odp_buffer_internal.h    |  5 ++-
>  .../linux-generic/include/odp_timer_internal.h     | 52 ++++++++++++++++++++++
>  platform/linux-generic/source/odp_buffer.c         |  4 +-
>  platform/linux-generic/source/odp_buffer_pool.c    | 33 +++++++++++---
>  platform/linux-generic/source/odp_timer.c          | 13 ++++++
>  7 files changed, 130 insertions(+), 16 deletions(-)
>  create mode 100644 platform/linux-generic/include/odp_timer_internal.h
> 
> diff --git a/include/odp_buffer.h b/include/odp_buffer.h
> index d79e76d..5bbb445 100644
> --- a/include/odp_buffer.h
> +++ b/include/odp_buffer.h
> @@ -62,9 +62,11 @@ size_t odp_buffer_size(odp_buffer_t buf);
>  int odp_buffer_type(odp_buffer_t buf);
>  
>  #define ODP_BUFFER_TYPE_INVALID (-1) /**< Buffer type invalid */
> -#define ODP_BUFFER_TYPE_RAW       0  /**< Raw buffer */
> -#define ODP_BUFFER_TYPE_PACKET    1  /**< Packet buffer */
> -#define ODP_BUFFER_TYPE_TIMER     2  /**< Timer buffer */
> +#define ODP_BUFFER_TYPE_ANY       0  /**< Buffer that can hold any other
> +					  buffer types */
> +#define ODP_BUFFER_TYPE_RAW       1  /**< Raw buffer, no metadata */
> +#define ODP_BUFFER_TYPE_PACKET    2  /**< Packet buffer */
> +#define ODP_BUFFER_TYPE_TIMEOUT   3  /**< Timeout buffer */
>  
>  /**
>   * Tests if buffer is part of a scatter/gather list
> diff --git a/include/odp_timer.h b/include/odp_timer.h
> index 80babd1..e7a6200 100644
> --- a/include/odp_timer.h
> +++ b/include/odp_timer.h
> @@ -25,8 +25,8 @@ extern "C" {
>  
>  
>  /**
> -* ODP timer handle
> -*/
> + * ODP timer handle
> + */
>  typedef uint32_t odp_timer_t;
>  
>  /** Invalid timer */
> @@ -34,8 +34,8 @@ typedef uint32_t odp_timer_t;
>  
>  
>  /**
> -* ODP timeout handle
> -*/
> + * ODP timeout handle
> + */
>  typedef odp_buffer_t odp_timer_tmo_t;
>  
>  /** Invalid timeout */
> @@ -43,6 +43,12 @@ typedef odp_buffer_t odp_timer_tmo_t;
>  
>  
>  /**
> + * Timeout notification
> + */
> +typedef void *odp_timeout_t;
> +
> +
> +/**
>   * Create a timer
>   *
>   * Creates a new timer with requested properties.
> @@ -133,6 +139,23 @@ odp_timer_tmo_t odp_timer_absolute_tmo(odp_timer_t timer, uint64_t tmo_tick,
>   */
>  int odp_timer_cancel_tmo(odp_timer_t timer, odp_timer_tmo_t tmo);
>  
> +/**
> + * Convert buffer handle to timeout handle
> + *
> + * @param buf  Buffer handle
> + *
> + * @return Timeout buffer handle
> + */
> +odp_timeout_t odp_timeout_from_buffer(odp_buffer_t buf);
> +
> +/**
> + * Return absolute timeout tick
> + *
> + * @param tmo Timeout buffer handle
> + *
> + * @return Absolute timeout tick
> + */
> +uint64_t odp_timeout_tick(odp_timeout_t tmo);
>  
>  #ifdef __cplusplus
>  }
> diff --git a/platform/linux-generic/include/odp_buffer_internal.h b/platform/linux-generic/include/odp_buffer_internal.h
> index f8ebc7c..d24c748 100644
> --- a/platform/linux-generic/include/odp_buffer_internal.h
> +++ b/platform/linux-generic/include/odp_buffer_internal.h
> @@ -89,8 +89,11 @@ typedef struct odp_buffer_hdr_t {
>  	size_t                   cur_offset; /* current offset */
>  	odp_atomic_int_t         ref_count;  /* reference count */
>  	odp_buffer_scatter_t     scatter;    /* Scatter/gather list */
> -	int                      type;       /* type of next header */
>  	odp_buffer_pool_t        pool;       /* buffer pool */
> +	int                      org_type;   /* original buffer type */
> +	int                      cur_type;   /* type of next header */

The variable doesn't match the comment, cur_type appear to imply the
current type.

> +
> +	int                      pad;        /* padding for payload align */
>  
>  	uint8_t                  payload[];  /* next header or data */
>  } odp_buffer_hdr_t;
> diff --git a/platform/linux-generic/include/odp_timer_internal.h b/platform/linux-generic/include/odp_timer_internal.h
> new file mode 100644
> index 0000000..97851a2
> --- /dev/null
> +++ b/platform/linux-generic/include/odp_timer_internal.h
> @@ -0,0 +1,52 @@
> +/* Copyright (c) 2013, Linaro Limited
> + * All rights reserved.
> + *
> + * SPDX-License-Identifier:     BSD-3-Clause
> + */
> +
> +
> +/**
> + * @file
> + *
> + * ODP timer timeout descriptor - implementation internal
> + */
> +
> +#ifndef ODP_TIMER_INTERNAL_H_
> +#define ODP_TIMER_INTERNAL_H_
> +
> +#ifdef __cplusplus
> +extern "C" {
> +#endif
> +
> +#include <odp_std_types.h>
> +#include <odp_queue.h>
> +#include <odp_buffer.h>
> +#include <odp_buffer_internal.h>
> +
> +struct odp_timeout_hdr_t;
> +
> +/**
> + * Timeout notification header
> + */
> +typedef struct odp_timeout_hdr_t {
> +	odp_buffer_hdr_t buf_hdr;
> +
> +	uint64_t         tick;
> +
> +	uint8_t payload[];
> +} odp_timeout_hdr_t;
> +
> +
> +
> +ODP_ASSERT(sizeof(odp_timeout_hdr_t) ==
> +	   ODP_OFFSETOF(odp_timeout_hdr_t, payload),
> +	   ODP_TIMEOUT_HDR_T__SIZE_ERR);
> +
> +ODP_ASSERT(sizeof(odp_timeout_hdr_t) % sizeof(uint64_t) == 0,
> +	   ODP_TIMEOUT_HDR_T__SIZE_ERR2);

Can't we change ODP_ASSERT to ODP_STRUCT_SIZE_ASSERT?
Maybe this can be a bug if we agree to change this? =)

Cheers,
Anders

> +
> +#ifdef __cplusplus
> +}
> +#endif
> +
> +#endif
> diff --git a/platform/linux-generic/source/odp_buffer.c b/platform/linux-generic/source/odp_buffer.c
> index afbe96a..ff7a892 100644
> --- a/platform/linux-generic/source/odp_buffer.c
> +++ b/platform/linux-generic/source/odp_buffer.c
> @@ -32,7 +32,7 @@ int odp_buffer_type(odp_buffer_t buf)
>  {
>  	odp_buffer_hdr_t *hdr = odp_buf_to_hdr(buf);
>  
> -	return hdr->type;
> +	return hdr->cur_type;
>  }
>  
>  
> @@ -86,7 +86,7 @@ int odp_buffer_snprint(char *str, size_t n, odp_buffer_t buf)
>  	len += snprintf(&str[len], n-len,
>  			"  ref_count    %i\n",        hdr->ref_count);
>  	len += snprintf(&str[len], n-len,
> -			"  type         %i\n",        hdr->type);
> +			"  type         %i\n",        hdr->cur_type);
>  	len += snprintf(&str[len], n-len,
>  			"  Scatter list\n");
>  	len += snprintf(&str[len], n-len,
> diff --git a/platform/linux-generic/source/odp_buffer_pool.c b/platform/linux-generic/source/odp_buffer_pool.c
> index 90214ba..3a5cb94 100644
> --- a/platform/linux-generic/source/odp_buffer_pool.c
> +++ b/platform/linux-generic/source/odp_buffer_pool.c
> @@ -9,6 +9,7 @@
>  #include <odp_buffer_pool_internal.h>
>  #include <odp_buffer_internal.h>
>  #include <odp_packet_internal.h>
> +#include <odp_timer_internal.h>
>  #include <odp_shared_memory.h>
>  #include <odp_align.h>
>  #include <odp_internal.h>
> @@ -39,6 +40,15 @@
>  
>  #define NULL_INDEX ((uint32_t)-1)
>  
> +union buffer_type_any_u {
> +	odp_buffer_hdr_t  buf;
> +	odp_packet_hdr_t  pkt;
> +	odp_timeout_hdr_t tmo;
> +};
> +
> +ODP_ASSERT(sizeof(union buffer_type_any_u) % sizeof(uint64_t) == 0,
> +	   BUFFER_TYPE_ANY_U__SIZE_ERR);
> +
>  
>  typedef union pool_entry_u {
>  	struct pool_entry_s s;
> @@ -177,8 +187,9 @@ static void add_chunk(pool_entry_t *pool, odp_buffer_chunk_hdr_t *chunk_hdr)
>  	if (pool->s.head) {
>  		/* link pool head to the chunk */
>  		add_buf_index(chunk_hdr, pool->s.head->buf_hdr.index);
> -	} else
> +	} else {
>  		add_buf_index(chunk_hdr, NULL_INDEX);
> +	}
>  
>  	pool->s.head = chunk_hdr;
>  	pool->s.free_bufs += ODP_BUFS_PER_CHUNK;
> @@ -214,6 +225,11 @@ static void fill_hdr(void *ptr, pool_entry_t *pool, uint32_t index,
>  	if (pool->s.buf_type == ODP_BUFFER_TYPE_PACKET) {
>  		odp_packet_hdr_t *packet_hdr = ptr;
>  		payload = packet_hdr->payload;
> +	} else if (pool->s.buf_type == ODP_BUFFER_TYPE_TIMEOUT) {
> +		odp_timeout_hdr_t *tmo_hdr = ptr;
> +		payload = tmo_hdr->payload;
> +	} else if (pool->s.buf_type == ODP_BUFFER_TYPE_ANY) {
> +		payload = ((uint8_t *)ptr) + sizeof(union buffer_type_any_u);
>  	}
>  
>  	memset(hdr, 0, size);
> @@ -224,7 +240,8 @@ static void fill_hdr(void *ptr, pool_entry_t *pool, uint32_t index,
>  	hdr->index = index;
>  	hdr->size  = pool->s.payload_size;
>  	hdr->pool  = pool->s.pool;
> -	hdr->type  = buf_type;
> +	hdr->org_type = pool->s.buf_type;
> +	hdr->cur_type = buf_type;
>  
>  	check_align(pool, hdr);
>  }
> @@ -251,11 +268,15 @@ static void link_bufs(pool_entry_t *pool)
>  	pool_size     = pool->s.pool_size;
>  	pool_base     = (uintptr_t) pool->s.pool_base_addr;
>  
> -	if (buf_type == ODP_BUFFER_TYPE_RAW)
> +	if (buf_type == ODP_BUFFER_TYPE_RAW) {
>  		hdr_size = sizeof(odp_buffer_hdr_t);
> -	else if (buf_type == ODP_BUFFER_TYPE_PACKET)
> +	} else if (buf_type == ODP_BUFFER_TYPE_PACKET) {
>  		hdr_size = sizeof(odp_packet_hdr_t);
> -	else {
> +	} else if (buf_type == ODP_BUFFER_TYPE_TIMEOUT) {
> +		hdr_size = sizeof(odp_timeout_hdr_t);
> +	} else if (buf_type == ODP_BUFFER_TYPE_ANY) {
> +		hdr_size = sizeof(union buffer_type_any_u);
> +	} else {
>  		ODP_ERR("odp_buffer_pool_create: Bad type %i\n",
>  			buf_type);
>  		exit(0);
> @@ -404,7 +425,7 @@ odp_buffer_t odp_buffer_alloc(odp_buffer_pool_t pool_id)
>  	if (chunk->chunk.num_bufs == 0) {
>  		/* give the chunk buffer */
>  		local_chunk[pool_id] = NULL;
> -		chunk->buf_hdr.type = pool->s.buf_type;
> +		chunk->buf_hdr.cur_type = pool->s.buf_type;
>  
>  		handle = chunk->buf_hdr.handle;
>  	} else {
> diff --git a/platform/linux-generic/source/odp_timer.c b/platform/linux-generic/source/odp_timer.c
> index 6fb5025..7c6c4f4 100644
> --- a/platform/linux-generic/source/odp_timer.c
> +++ b/platform/linux-generic/source/odp_timer.c
> @@ -5,6 +5,8 @@
>   */
>  
>  #include <odp_timer.h>
> +#include <odp_timer_internal.h>
> +#include <odp_buffer_pool_internal.h>
>  #include <odp_internal.h>
>  #include <odp_atomic.h>
>  #include <odp_spinlock.h>
> @@ -330,3 +332,14 @@ uint64_t odp_timer_current_tick(odp_timer_t timer)
>  	id = timer - 1;
>  	return odp_timer.timer[id].cur_tick;
>  }
> +
> +odp_timeout_t odp_timeout_from_buffer(odp_buffer_t buf)
> +{
> +	return (odp_timeout_t) odp_buf_to_hdr(buf);
> +}
> +
> +uint64_t odp_timeout_tick(odp_timeout_t tmo)
> +{
> +	odp_timeout_hdr_t *tmo_hdr = tmo;
> +	return tmo_hdr->tick;
> +}
> -- 
> 1.9.3
> 
> 
> _______________________________________________
> lng-odp mailing list
> lng-odp@lists.linaro.org
> http://lists.linaro.org/mailman/listinfo/lng-odp

> > diff --git a/platform/linux-generic/include/odp_buffer_internal.h
> b/platform/linux-generic/include/odp_buffer_internal.h
> > index f8ebc7c..d24c748 100644
> > --- a/platform/linux-generic/include/odp_buffer_internal.h
> > +++ b/platform/linux-generic/include/odp_buffer_internal.h
> > @@ -89,8 +89,11 @@ typedef struct odp_buffer_hdr_t {
> >  	size_t                   cur_offset; /* current offset */
> >  	odp_atomic_int_t         ref_count;  /* reference count */
> >  	odp_buffer_scatter_t     scatter;    /* Scatter/gather list */
> > -	int                      type;       /* type of next header */
> >  	odp_buffer_pool_t        pool;       /* buffer pool */
> > +	int                      org_type;   /* original buffer type */
> > +	int                      cur_type;   /* type of next header */
> 
> The variable doesn't match the comment, cur_type appear to imply the
> current type.

It's close enough - it's the current type of the next header (vs the original buffer type).


> +
> > +
> > +
> > +ODP_ASSERT(sizeof(odp_timeout_hdr_t) ==
> > +	   ODP_OFFSETOF(odp_timeout_hdr_t, payload),
> > +	   ODP_TIMEOUT_HDR_T__SIZE_ERR);
> > +
> > +ODP_ASSERT(sizeof(odp_timeout_hdr_t) % sizeof(uint64_t) == 0,
> > +	   ODP_TIMEOUT_HDR_T__SIZE_ERR2);
> 
> Can't we change ODP_ASSERT to ODP_STRUCT_SIZE_ASSERT?
> Maybe this can be a bug if we agree to change this? =)

Sure, someone could do that and find-replace all ODP_ASSERT (which use sizeof). ODP_STATIC_ASSERT won't compile with sizeof. It'd be another patch to replace ODP_ASSERT from all the files.


-Petri

On 2014-05-16 06:53, Savolainen, Petri (NSN - FI/Espoo) wrote:
> 
> 
> > > diff --git a/platform/linux-generic/include/odp_buffer_internal.h
> > b/platform/linux-generic/include/odp_buffer_internal.h
> > > index f8ebc7c..d24c748 100644
> > > --- a/platform/linux-generic/include/odp_buffer_internal.h
> > > +++ b/platform/linux-generic/include/odp_buffer_internal.h
> > > @@ -89,8 +89,11 @@ typedef struct odp_buffer_hdr_t {
> > >  	size_t                   cur_offset; /* current offset */
> > >  	odp_atomic_int_t         ref_count;  /* reference count */
> > >  	odp_buffer_scatter_t     scatter;    /* Scatter/gather list */
> > > -	int                      type;       /* type of next header */
> > >  	odp_buffer_pool_t        pool;       /* buffer pool */
> > > +	int                      org_type;   /* original buffer type */
> > > +	int                      cur_type;   /* type of next header */
> > 
> > The variable doesn't match the comment, cur_type appear to imply the
> > current type.
> 
> It's close enough - it's the current type of the next header (vs the original buffer type).

Thank you for the clarification.
However, I don't think its close enough. =/

What about:
cur_type -> next_type

Or change the comment to reflect what it is, because the comment are not
clear now.

If you do this change you can add me to Reviewed-by in the next version.

> 
> 
> > +
> > > +
> > > +
> > > +ODP_ASSERT(sizeof(odp_timeout_hdr_t) ==
> > > +	   ODP_OFFSETOF(odp_timeout_hdr_t, payload),
> > > +	   ODP_TIMEOUT_HDR_T__SIZE_ERR);
> > > +
> > > +ODP_ASSERT(sizeof(odp_timeout_hdr_t) % sizeof(uint64_t) == 0,
> > > +	   ODP_TIMEOUT_HDR_T__SIZE_ERR2);
> > 
> > Can't we change ODP_ASSERT to ODP_STRUCT_SIZE_ASSERT?
> > Maybe this can be a bug if we agree to change this? =)
> 
> Sure, someone could do that and find-replace all ODP_ASSERT (which use sizeof). ODP_STATIC_ASSERT won't compile with sizeof. It'd be another patch to replace ODP_ASSERT from all the files.

Thanks, then someone will do find-replace
's/ODP_ASSERT/ODP_STRUCT_SIZE_ASSERT/g' =)


- Anders