Re: [lng-odp] [API-NEXT PATCH 1/4] api: pool: add maximum packet counts to pool info

2017-04-27 Thread Elo, Matias (Nokia - FI/Espoo) 

>> -   /** The number of packets that the pool must provide
>> -   that are packet length 'len' bytes or smaller.
>> -   The maximum value is defined by pool capability
>> -   pkt.max_num. */
>> +   /** The exact number of 'len' byte packets that the 
>> pool
>> +   must provide. The maximum value is defined by 
>> pool
>> +   capability pkt.max_num. Pool is empty after
>> +   allocating all the 'len' byte packets. Pool 
>> capacity
>> +   for other packet lengths may vary. See
>> +   odp_pool_info_t for details. */
>>uint32_t num;
> 
> This documentation says that the pool must be empty after allocating
> "num" packets of size "len" but in reality it is possible that
> implementation might do some round-off on the pool allocation for
> better optimisation and hence there could be some minor additional
> packets which might be available in the pool.

This same issue is also in the old spec version ("The number of packets
that the pool must provide that are packet length 'len' bytes or smaller"),
just not as clearly defined. We'll start updating the patch tomorrow and
take this problem into consideration (as well as other issues presented in
the earlier conversations).

>> 
>> +   /** Maximum number of packets of any length */
>> +   uint32_t max_num;
>> +
>> +   /** Maximum number of minimum length packets */
>> +   uint32_t num_min_len;
> 
> What is the difference between "num_min_len" and "max_num" both might
> be the same since the maximum of any length packet will usually be the
> number of packets of minimum length?

In most cases max_num == num_min_len. However, in sub-pool implementations
this may not be always true.

-Matias

Re: [lng-odp] [API-NEXT PATCH 1/4] api: pool: add maximum packet counts to pool info

2017-04-18 Thread Bill Fischofer 
See comments below based on today's discussions.

On Wed, Apr 12, 2017 at 7:58 AM, Matias Elo  wrote:

> Add fields to odp_pool_info_t for maximum number of packets of any length,
> maximum number of minimum length packets, and maximum number of maximum
> length packets.
>
> odp_pool_param_t documentation is updated to reflect these new fields.
>
> Signed-off-by: Matias Elo 
> ---
>  include/odp/api/spec/pool.h | 43 ++
> -
>  1 file changed, 30 insertions(+), 13 deletions(-)
>
> diff --git a/include/odp/api/spec/pool.h b/include/odp/api/spec/pool.h
> index c0de195..402c9f2 100644
> --- a/include/odp/api/spec/pool.h
> +++ b/include/odp/api/spec/pool.h
> @@ -181,23 +181,27 @@ typedef struct odp_pool_param_t {
> uint32_t align;
> } buf;
> struct {
> -   /** The number of packets that the pool must
> provide
> -   that are packet length 'len' bytes or smaller.
> -   The maximum value is defined by pool capability
> -   pkt.max_num. */
> +   /** The exact number of 'len' byte packets that
> the pool
> +   must provide. The maximum value is defined by
> pool
> +   capability pkt.max_num. Pool is empty after
> +   allocating all the 'len' byte packets. Pool
> capacity
> +   for other packet lengths may vary. See
> +   odp_pool_info_t for details. */
> uint32_t num;
>

The root issue here seems to be to balance two things:

1. The application's desire to control the number of packets that may
reside in a given pool.

2. The implementation's desire to have flexibility in how it organizes the
pool internals given the application's stated needs for pool capacity.

The intent of the original provision of the odp_pool_param_t was to serve
these needs by allowing the application to set the pool capacity in terms
of the number of packets it must be able to hold and a set of hints (len,
max_len) that assist the implementation in allocating sufficient resources
to meet these needs.

The argument now seems to be that asking an ODP implementation to honor a
fixed pkt.num is too onerous, so the question is how best to accommodate
this without giving the implementation complete license to ignore any hints
the application may provide.

The crux of the argument is whether it is reasonable to require an ODP
packet pool to track packet allocation, as opposed to segment allocation
that just happen to be assembled into packets under the covers. I've argued
that this is not an onerous requirement, but others disagree.


>
> -   /** Minimum packet length that the pool must
> provide
> -   'num' packets. The number of packets may be
> less
> -   than 'num' when packets are larger than 'len'.
> -   The maximum value is defined by pool capability
> -   pkt.max_len. Use 0 for default. */
> +   /** The packet length that the pool must provide
> exactly
> +   'num' packets. The maximum value is defined by
> pool
> +   capability pkt.max_len. Pool capacity for other
> +   packet lengths may vary. See odp_pool_info_t
> for
> +   details. Use 0 for default. */
> uint32_t len;
>

As we discussed today, this wording doesn't quite work. Consider an
implementation that rounds the number of segments physically allocated up
to some higher boundary (required by HW). How is this limit to be enforced
unless the implementation tracks packet allocations rather than segment
allocations (which for the sake of argument we're assuming it's
unreasonable to ask it to do)?


>
> /** Maximum packet length that will be allocated
> from
> the pool. The maximum value is defined by pool
> -   capability pkt.max_len. Use 0 for default (the
> -   pool maximum). */
> +   capability pkt.max_len. Pool capacity for this
> +   packet length can be checked from pool info
> +   num_max_len (odp_pool_info_t). Use 0 for
> default
>

Why is this additional clause needed?


> +   (the pool maximum).*/
> uint32_t max_len;
>
> /** Minimum number of packet data bytes that are
> stored
> @@ -272,8 +276,21 @@ odp_pool_t odp_pool_lookup(const char *name);
>   * Used to get information about a pool.
>   */
>  typedef struct odp_pool_info_t {
> -   const char *name;  /**< pool name */
> -

Re: [lng-odp] [API-NEXT PATCH 1/4] api: pool: add maximum packet counts to pool info

2017-04-18 Thread Bala Manoharan 
Regards,
Bala


On 12 April 2017 at 18:28, Matias Elo  wrote:
> Add fields to odp_pool_info_t for maximum number of packets of any length,
> maximum number of minimum length packets, and maximum number of maximum
> length packets.
>
> odp_pool_param_t documentation is updated to reflect these new fields.
>
> Signed-off-by: Matias Elo 
> ---
>  include/odp/api/spec/pool.h | 43 ++-
>  1 file changed, 30 insertions(+), 13 deletions(-)
>
> diff --git a/include/odp/api/spec/pool.h b/include/odp/api/spec/pool.h
> index c0de195..402c9f2 100644
> --- a/include/odp/api/spec/pool.h
> +++ b/include/odp/api/spec/pool.h
> @@ -181,23 +181,27 @@ typedef struct odp_pool_param_t {
> uint32_t align;
> } buf;
> struct {
> -   /** The number of packets that the pool must provide
> -   that are packet length 'len' bytes or smaller.
> -   The maximum value is defined by pool capability
> -   pkt.max_num. */
> +   /** The exact number of 'len' byte packets that the 
> pool
> +   must provide. The maximum value is defined by pool
> +   capability pkt.max_num. Pool is empty after
> +   allocating all the 'len' byte packets. Pool 
> capacity
> +   for other packet lengths may vary. See
> +   odp_pool_info_t for details. */
> uint32_t num;

This documentation says that the pool must be empty after allocating
"num" packets of size "len" but in reality it is possible that
implementation might do some round-off on the pool allocation for
better optimisation and hence there could be some minor additional
packets which might be available in the pool.

>
> -   /** Minimum packet length that the pool must provide
> -   'num' packets. The number of packets may be less
> -   than 'num' when packets are larger than 'len'.
> -   The maximum value is defined by pool capability
> -   pkt.max_len. Use 0 for default. */
> +   /** The packet length that the pool must provide 
> exactly
> +   'num' packets. The maximum value is defined by 
> pool
> +   capability pkt.max_len. Pool capacity for other
> +   packet lengths may vary. See odp_pool_info_t for
> +   details. Use 0 for default. */
> uint32_t len;
>
> /** Maximum packet length that will be allocated from
> the pool. The maximum value is defined by pool
> -   capability pkt.max_len. Use 0 for default (the
> -   pool maximum). */
> +   capability pkt.max_len. Pool capacity for this
> +   packet length can be checked from pool info
> +   num_max_len (odp_pool_info_t). Use 0 for default
> +   (the pool maximum).*/
> uint32_t max_len;
>
> /** Minimum number of packet data bytes that are 
> stored
> @@ -272,8 +276,21 @@ odp_pool_t odp_pool_lookup(const char *name);
>   * Used to get information about a pool.
>   */
>  typedef struct odp_pool_info_t {
> -   const char *name;  /**< pool name */
> -   odp_pool_param_t params;   /**< pool parameters */
> +   /** Pool name */
> +   const char *name;
> +   /** Pool parameters */
> +   odp_pool_param_t params;
> +   /** Packet pool info */
> +   struct {
> +   /** Maximum number of packets of any length */
> +   uint32_t max_num;
> +
> +   /** Maximum number of minimum length packets */
> +   uint32_t num_min_len;

What is the difference between "num_min_len" and "max_num" both might
be the same since the maximum of any length packet will usually be the
number of packets of minimum length?

> +
> +   /** Maximum number of maximum length packets */
> +   uint32_t num_max_len;
> +   } pkt;
>  } odp_pool_info_t;
>
>  /**
> --
> 2.7.4
>

[lng-odp] [API-NEXT PATCH 1/4] api: pool: add maximum packet counts to pool info

2017-04-12 Thread Matias Elo 
Add fields to odp_pool_info_t for maximum number of packets of any length,
maximum number of minimum length packets, and maximum number of maximum
length packets.

odp_pool_param_t documentation is updated to reflect these new fields.

Signed-off-by: Matias Elo 
---
 include/odp/api/spec/pool.h | 43 ++-
 1 file changed, 30 insertions(+), 13 deletions(-)

diff --git a/include/odp/api/spec/pool.h b/include/odp/api/spec/pool.h
index c0de195..402c9f2 100644
--- a/include/odp/api/spec/pool.h
+++ b/include/odp/api/spec/pool.h
@@ -181,23 +181,27 @@ typedef struct odp_pool_param_t {
uint32_t align;
} buf;
struct {
-   /** The number of packets that the pool must provide
-   that are packet length 'len' bytes or smaller.
-   The maximum value is defined by pool capability
-   pkt.max_num. */
+   /** The exact number of 'len' byte packets that the pool
+   must provide. The maximum value is defined by pool
+   capability pkt.max_num. Pool is empty after
+   allocating all the 'len' byte packets. Pool capacity
+   for other packet lengths may vary. See
+   odp_pool_info_t for details. */
uint32_t num;
 
-   /** Minimum packet length that the pool must provide
-   'num' packets. The number of packets may be less
-   than 'num' when packets are larger than 'len'.
-   The maximum value is defined by pool capability
-   pkt.max_len. Use 0 for default. */
+   /** The packet length that the pool must provide exactly
+   'num' packets. The maximum value is defined by pool
+   capability pkt.max_len. Pool capacity for other
+   packet lengths may vary. See odp_pool_info_t for
+   details. Use 0 for default. */
uint32_t len;
 
/** Maximum packet length that will be allocated from
the pool. The maximum value is defined by pool
-   capability pkt.max_len. Use 0 for default (the
-   pool maximum). */
+   capability pkt.max_len. Pool capacity for this
+   packet length can be checked from pool info
+   num_max_len (odp_pool_info_t). Use 0 for default
+   (the pool maximum).*/
uint32_t max_len;
 
/** Minimum number of packet data bytes that are stored
@@ -272,8 +276,21 @@ odp_pool_t odp_pool_lookup(const char *name);
  * Used to get information about a pool.
  */
 typedef struct odp_pool_info_t {
-   const char *name;  /**< pool name */
-   odp_pool_param_t params;   /**< pool parameters */
+   /** Pool name */
+   const char *name;
+   /** Pool parameters */
+   odp_pool_param_t params;
+   /** Packet pool info */
+   struct {
+   /** Maximum number of packets of any length */
+   uint32_t max_num;
+
+   /** Maximum number of minimum length packets */
+   uint32_t num_min_len;
+
+   /** Maximum number of maximum length packets */
+   uint32_t num_max_len;
+   } pkt;
 } odp_pool_info_t;
 
 /**
-- 
2.7.4