diff mbox

[API-NEXT,RFC] api: pool: additional packet length configuration

Message ID 20170504142400.23189-1-petri.savolainen@linaro.org
State New
Headers show

Commit Message

Petri Savolainen May 4, 2017, 2:24 p.m. UTC
Added packet pool parameters for more fine grained pool
configuration. The basic usage of the parameters is not changed,
except that implementation may now round up 'num' by default.
Application can limit the round up with new 'max_num' parameter.
Another new parameter (prio) allows application to prioritize
some pools (or parts of pools) over others. Implementation may
use this information to decide e.g. on HW resource usage.

Additionally, pool configuration may be extended with a table of
num/len/prio values. This gives application more flexibility to
specify requirements for various packet sizes.

Signed-off-by: Petri Savolainen <petri.savolainen@linaro.org>

---
 include/odp/api/spec/pool.h                        | 117 +++++++++++++++++++--
 include/odp/arch/default/api/abi/pool.h            |   2 +
 .../include/odp/api/plat/pool_types.h              |   2 +
 3 files changed, 110 insertions(+), 11 deletions(-)

-- 
2.11.0

Comments

Bill Fischofer May 4, 2017, 4:20 p.m. UTC | #1
On Thu, May 4, 2017 at 9:24 AM, Petri Savolainen <
petri.savolainen@linaro.org> wrote:

> Added packet pool parameters for more fine grained pool

> configuration. The basic usage of the parameters is not changed,

> except that implementation may now round up 'num' by default.

> Application can limit the round up with new 'max_num' parameter.

> Another new parameter (prio) allows application to prioritize

> some pools (or parts of pools) over others. Implementation may

> use this information to decide e.g. on HW resource usage.

>

> Additionally, pool configuration may be extended with a table of

> num/len/prio values. This gives application more flexibility to

> specify requirements for various packet sizes.

>

> Signed-off-by: Petri Savolainen <petri.savolainen@linaro.org>

> ---

>  include/odp/api/spec/pool.h                        | 117

> +++++++++++++++++++--

>  include/odp/arch/default/api/abi/pool.h            |   2 +

>  .../include/odp/api/plat/pool_types.h              |   2 +

>  3 files changed, 110 insertions(+), 11 deletions(-)

>

> diff --git a/include/odp/api/spec/pool.h b/include/odp/api/spec/pool.h

> index 6fc5b6b4..2bf37e5d 100644

> --- a/include/odp/api/spec/pool.h

> +++ b/include/odp/api/spec/pool.h

> @@ -41,6 +41,9 @@ extern "C" {

>   * Maximum pool name length in chars including null char

>   */

>

> +/** Maximum number of additional packet pool parameters */

> +#define ODP_POOL_PKT_ADD_MAX  7

> +

>  /**

>   * Pool capabilities

>   */

> @@ -127,6 +130,9 @@ typedef struct odp_pool_capability_t {

>                  * The value of zero means that limited only by the

> available

>                  * memory size for the pool. */

>                 uint32_t max_uarea_size;

> +

> +               /** Number of priority levels */

> +               uint8_t num_prio;

>


In what sense are these "priorities"? Perhaps just num_subpool?


>         } pkt;

>

>         /** Timeout pool capabilities  */

> @@ -156,6 +162,28 @@ typedef struct odp_pool_capability_t {

>  int odp_pool_capability(odp_pool_capability_t *capa);

>

>  /**

> + * @def ODP_POOL_PRIO_DEFAULT

>


Not clear what this controls or how it's expected to be used.


> + *

> + * Default pool priority. User does not care about the selected priority

> + * level.

> + */

> +

> +/**

> + * Additional parameters for packet pool creation

> + */

> +typedef struct odp_pool_param_pkt_add_t {

> +       /** Number of packets */

> +       uint32_t num;

> +

> +       /** Packet length in bytes */

> +       uint32_t len;

> +

> +       /** Priority level */

> +       uint8_t prio;

> +

> +} odp_pool_param_pkt_add_t;

>


This is a confusing choice of name. Since this is intended to introduce the
notion of subpools, why not just call it odp_pool_subpool_param_t?  An
array of these are then just part of the containing odp_pool_param_t struct.


> +

> +/**

>   * Pool parameters

>   * Used to communicate pool creation options.

>   * @note A single thread may not be able to allocate all 'num' elements

> @@ -185,25 +213,56 @@ typedef struct odp_pool_param_t {

>

>                 /** Parameters for packet pools */

>                 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 minimum number of packets that are packet

> length

> +                        *  'len' bytes or smaller. The maximum value is

> defined

> +                        *  by pool capability pkt.max_num. An

> implementation

> +                        *  may round up the value, as long as the

> 'max_num'

> +                        *  parameter below is not violated.

> +                        */

>                         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 minimum packet length that at least 'num'

> +                        *  packets are required. The maximum value is

> defined

> +                        *  by pool capability pkt.max_len. Use 0 for

> default.

> +                        */

>                         uint32_t len;

>

> +                       /** Priority level

> +                        *

> +                        *  Priority indicates the relative service level

> +                        *  between pools and packet length configuration

>


Needs more explanation. What is a "service level" in this context? When a
packet arrives and a CoS assigns it to this pool, what's supposed to
happen? When odp_packet_alloc() is called against this pool, again what's
supposed to happen?


> +                        *  within a pool. The highest priority level is

> zero.

> +                        *  Number of priorities is defined by pool

> capability

> +                        *  pkt.num_prio. Thus the lowest priority level is

> +                        *  pkt.num_prio - 1. The default value is

> +                        *  ODP_POOL_PRIO_DEFAULT.

> +                        */

> +                       uint8_t prio;

> +

> +                       /** Number of additional packet pool parameters

> +                        *

> +                        *  The number of pkt.add[] table entries filled.

> The

> +                        *  value must not exceed ODP_POOL_PKT_ADD_MAX.

>


ODP_POOL_SUBPOOL_MAX?


> +                        *  The default value is 0.

> +                        */

> +                       uint8_t num_add;

> +

>                         /** 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). */

>                         uint32_t max_len;

>

> +                       /** Maximum number of packets

> +                        *

> +                        *  This is the maximum number of packets of any

> length

> +                        *  that can be allocated from the pool. The

> maximum

> +                        *  value is defined by pool capability

> pkt.max_num.

> +                        *  Use 0 for no requirement for maximum number.

> +                        *  The default value is 0.

> +                        */

> +                       uint32_t max_num;

> +

>                         /** Minimum number of packet data bytes that are

> stored

>                             in the first segment of a packet. The maximum

> value

>                             is defined by pool capability pkt.max_seg_len.

> @@ -214,6 +273,26 @@ typedef struct odp_pool_param_t {

>                             defined by pool capability pkt.max_uarea_size.

>                             Specify as 0 if no user area is needed. */

>                         uint32_t uarea_size;

> +

> +                       /** Additional packet pool parameters

> +                        *

> +                        *  This table gives more fine grained

> requirements for

> +                        *  pool configuration. The table continues from

> +                        *  num/len/prio specification above. Therefore,

> +                        *  pkt.add[0].len must be greater than pkt.len,

> and

> +                        *  pkt.add[0].num refers to packet lengths between

> +                        *  pkt.len + 1 and pkt.add[0].len.

> +                        *

> +                        *  Table enties must be ordered by the packet

> length.

> +                        *  A number of packets figure (pkt.add[N].num)

> refers

> +                        *  to packet lengths between pkt.add[N-1].len + 1

> and

> +                        *  pkt.add[N].len. Each number of packets

> requirement

> +                        *  may be rounded up, as long as the 'max_num'

> +                        *  parameter is not violated. Also, more than one

> +                        *  number of packets requirement may not be

> fulfilled

> +                        *  simultaneously.

>


Not clear what you're referring to in this last remark.


> +                        */

> +                       odp_pool_param_pkt_add_t add[ODP_POOL_PKT_ADD_MAX];

>                 } pkt;

>

>                 /** Parameters for timeout pools */

> @@ -278,8 +357,24 @@ 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;

> +

> +       /** Copy of the pool parameters */

> +       odp_pool_param_t params;

> +

> +       /** Packet pool info */

> +       struct {

> +               /** Maximum number of packets of any length

> +                *

> +                *  This many packets in maximum can be allocated from the

> pool.

> +                *  Application can use this e.g. to prepare enough per

> packet

> +                *  contexts.

> +                */

> +               uint32_t max_num;

> +

> +       } pkt;

> +

>  } odp_pool_info_t;

>

>  /**

> diff --git a/include/odp/arch/default/api/abi/pool.h

> b/include/odp/arch/default/api/abi/pool.h

> index 4637d19f..2e2eaf07 100644

> --- a/include/odp/arch/default/api/abi/pool.h

> +++ b/include/odp/arch/default/api/abi/pool.h

> @@ -26,6 +26,8 @@ typedef _odp_abi_pool_t *odp_pool_t;

>

>  #define ODP_POOL_NAME_LEN  32

>

> +#define ODP_POOL_PRIO_DEFAULT 0

> +

>  typedef enum odp_pool_type_t {

>         ODP_POOL_BUFFER  = ODP_EVENT_BUFFER,

>         ODP_POOL_PACKET  = ODP_EVENT_PACKET,

> diff --git a/platform/linux-generic/include/odp/api/plat/pool_types.h

> b/platform/linux-generic/include/odp/api/plat/pool_types.h

> index 8bc816d4..7bab2fd6 100644

> --- a/platform/linux-generic/include/odp/api/plat/pool_types.h

> +++ b/platform/linux-generic/include/odp/api/plat/pool_types.h

> @@ -36,6 +36,8 @@ typedef ODP_HANDLE_T(odp_pool_t);

>

>  #define ODP_POOL_NAME_LEN  32

>

> +#define ODP_POOL_PRIO_DEFAULT 0

> +

>  typedef enum odp_pool_type_t {

>         ODP_POOL_BUFFER  = ODP_EVENT_BUFFER,

>         ODP_POOL_PACKET  = ODP_EVENT_PACKET,

> --

> 2.11.0

>

>
Savolainen, Petri (Nokia - FI/Espoo) May 5, 2017, 8:01 a.m. UTC | #2
Indentation lost due to HTML mail format ...


From: Bill Fischofer [mailto:bill.fischofer@linaro.org] 

Sent: Thursday, May 04, 2017 7:20 PM
To: Petri Savolainen <petri.savolainen@linaro.org>
Cc: lng-odp-forward <lng-odp@lists.linaro.org>
Subject: Re: [lng-odp] [API-NEXT RFC] api: pool: additional packet length configuration



On Thu, May 4, 2017 at 9:24 AM, Petri Savolainen <mailto:petri.savolainen@linaro.org> wrote:
Added packet pool parameters for more fine grained pool
configuration. The basic usage of the parameters is not changed,
except that implementation may now round up 'num' by default.
Application can limit the round up with new 'max_num' parameter.
Another new parameter (prio) allows application to prioritize
some pools (or parts of pools) over others. Implementation may
use this information to decide e.g. on HW resource usage.

Additionally, pool configuration may be extended with a table of
num/len/prio values. This gives application more flexibility to
specify requirements for various packet sizes.

Signed-off-by: Petri Savolainen <mailto:petri.savolainen@linaro.org>

---
 include/odp/api/spec/pool.h                        | 117 +++++++++++++++++++--
 include/odp/arch/default/api/abi/pool.h            |   2 +
 .../include/odp/api/plat/pool_types.h              |   2 +
 3 files changed, 110 insertions(+), 11 deletions(-)

This is stated in the commit log:
"prio allows application to prioritize
some pools (or parts of pools) over others. Implementation may
use this information to decide e.g. on HW resource usage."

[petri]
This is not number of sub pools (spec does not expect sub- or single pool implementation). Prio is additional information to the implementation to decide where to use scarce HW resources. If implementation does care about this information, it can return num_prio == 0 here.



 
        } pkt;

        /** Timeout pool capabilities  */
@@ -156,6 +162,28 @@ typedef struct odp_pool_capability_t {
 int odp_pool_capability(odp_pool_capability_t *capa);

 /**
+ * @def ODP_POOL_PRIO_DEFAULT

Not clear what this controls or how it's expected to be used.

[petri]
This is the default value for the prio fields under == what param_init() fills in == application saying "I don't care"
 

+ *
+ * Default pool priority. User does not care about the selected priority
+ * level.
+ */
+
+/**
+ * Additional parameters for packet pool creation
+ */
+typedef struct odp_pool_param_pkt_add_t {
+       /** Number of packets */
+       uint32_t num;
+
+       /** Packet length in bytes */
+       uint32_t len;
+
+       /** Priority level */
+       uint8_t prio;
+
+} odp_pool_param_pkt_add_t;

This is a confusing choice of name. Since this is intended to introduce the notion of subpools, why not just call it odp_pool_subpool_param_t?  An array of these are then just part of the containing odp_pool_param_t struct.


[petri]
The add[] table is additional information/more fine grained requirements to the implementation. API does not refer to sub-pools,  since that's only one type of implementation. This API can be fulfilled as easily with a single segment size implementation (which e.g. odp-linux and odp-dpdk would do).

Used odp_pool_param_ there because it's part of odp_pool_param_t. Used pkt_ because it's part of pkt params. Used "add" for additional. One option is to remove the type and just define the struct inside odp_pool_param_t.


 
+
+/**
  * Pool parameters
  * Used to communicate pool creation options.
  * @note A single thread may not be able to allocate all 'num' elements
@@ -185,25 +213,56 @@ typedef struct odp_pool_param_t {

                /** Parameters for packet pools */
                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 minimum number of packets that are packet length
+                        *  'len' bytes or smaller. The maximum value is defined
+                        *  by pool capability pkt.max_num. An implementation
+                        *  may round up the value, as long as the 'max_num'
+                        *  parameter below is not violated.
+                        */
                        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 minimum packet length that at least 'num'
+                        *  packets are required. The maximum value is defined
+                        *  by pool capability pkt.max_len. Use 0 for default.
+                        */
                        uint32_t len;

+                       /** Priority level
+                        *
+                        *  Priority indicates the relative service level
+                        *  between pools and packet length configuration

Needs more explanation. What is a "service level" in this context? When a packet arrives and a CoS assigns it to this pool, what's supposed to happen? When odp_packet_alloc() is called against this pool, again what's supposed to happen?

[petri]
Relative importance of this pool / packet size within a pool, compared to other pools / packet sizes. Highest prio == use all the best HW resource for this, default == don't care, lowest == use lowest level of resources for this.

For example, highest prio pool may use HW pool manager and chip internal memory vs. lowest use SW pool implementation and main memory (DDR). HW and cores may see lower latency/higher performance for data of the high prio pool.


 
+                        *  within a pool. The highest priority level is zero.
+                        *  Number of priorities is defined by pool capability
+                        *  pkt.num_prio. Thus the lowest priority level is
+                        *  pkt.num_prio - 1. The default value is
+                        *  ODP_POOL_PRIO_DEFAULT.
+                        */
+                       uint8_t prio;
+
+                       /** Number of additional packet pool parameters
+                        *
+                        *  The number of pkt.add[] table entries filled. The
+                        *  value must not exceed ODP_POOL_PKT_ADD_MAX.

ODP_POOL_SUBPOOL_MAX?

[petri]
Sub-pool is one possible implementation.

 
+                        *  The default value is 0.
+                        */
+                       uint8_t num_add;
+
                        /** 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). */
                        uint32_t max_len;

+                       /** Maximum number of packets
+                        *
+                        *  This is the maximum number of packets of any length
+                        *  that can be allocated from the pool. The maximum
+                        *  value is defined by pool capability pkt.max_num.
+                        *  Use 0 for no requirement for maximum number.
+                        *  The default value is 0.
+                        */
+                       uint32_t max_num;
+
                        /** Minimum number of packet data bytes that are stored
                            in the first segment of a packet. The maximum value
                            is defined by pool capability pkt.max_seg_len.
@@ -214,6 +273,26 @@ typedef struct odp_pool_param_t {
                            defined by pool capability pkt.max_uarea_size.
                            Specify as 0 if no user area is needed. */
                        uint32_t uarea_size;
+
+                       /** Additional packet pool parameters
+                        *
+                        *  This table gives more fine grained requirements for
+                        *  pool configuration. The table continues from
+                        *  num/len/prio specification above. Therefore,
+                        *  pkt.add[0].len must be greater than pkt.len, and
+                        *  pkt.add[0].num refers to packet lengths between
+                        *  pkt.len + 1 and pkt.add[0].len.
+                        *
+                        *  Table enties must be ordered by the packet length.
+                        *  A number of packets figure (pkt.add[N].num) refers
+                        *  to packet lengths between pkt.add[N-1].len + 1 and
+                        *  pkt.add[N].len. Each number of packets requirement
+                        *  may be rounded up, as long as the 'max_num'
+                        *  parameter is not violated. Also, more than one
+                        *  number of packets requirement may not be fulfilled
+                        *  simultaneously.

Not clear what you're referring to in this last remark.

[petri]

pkt.add[N].num is guaranteed when only packet lengths (pkt.add[N-1].len + 1) ... pkt.add[N].len are allocated. But it is not guaranteed, if various other packet lengths are also allocated simultaneously.


-Petridiff --git a/include/odp/api/spec/pool.h b/include/odp/api/spec/pool.h
index 6fc5b6b4..2bf37e5d 100644
--- a/include/odp/api/spec/pool.h
+++ b/include/odp/api/spec/pool.h
@@ -41,6 +41,9 @@ extern "C" {
  * Maximum pool name length in chars including null char
  */

+/** Maximum number of additional packet pool parameters */
+#define ODP_POOL_PKT_ADD_MAX  7
+
 /**
  * Pool capabilities
  */
@@ -127,6 +130,9 @@ typedef struct odp_pool_capability_t {
                 * The value of zero means that limited only by the available
                 * memory size for the pool. */
                uint32_t max_uarea_size;
+
+               /** Number of priority levels */
+               uint8_t num_prio;

In what sense are these "priorities"? Perhaps just num_subpool?


Bill Fischofer May 5, 2017, 7:07 p.m. UTC | #3
On Fri, May 5, 2017 at 3:01 AM, Savolainen, Petri (Nokia - FI/Espoo)
<petri.savolainen@nokia.com> wrote:
> Indentation lost due to HTML mail format ...


I'm not sure why that's happening since I tell gmail to use plain text
for these, but for some reason your threads sometimes ignore this
instruction and I have to manually override. Out of curiosity, what
e-mail client to you use in composing these posts?

>

>

> From: Bill Fischofer [mailto:bill.fischofer@linaro.org]

> Sent: Thursday, May 04, 2017 7:20 PM

> To: Petri Savolainen <petri.savolainen@linaro.org>

> Cc: lng-odp-forward <lng-odp@lists.linaro.org>

> Subject: Re: [lng-odp] [API-NEXT RFC] api: pool: additional packet length configuration

>

>

>

> On Thu, May 4, 2017 at 9:24 AM, Petri Savolainen <mailto:petri.savolainen@linaro.org> wrote:

> Added packet pool parameters for more fine grained pool

> configuration. The basic usage of the parameters is not changed,

> except that implementation may now round up 'num' by default.

> Application can limit the round up with new 'max_num' parameter.

> Another new parameter (prio) allows application to prioritize

> some pools (or parts of pools) over others. Implementation may

> use this information to decide e.g. on HW resource usage.

>

> Additionally, pool configuration may be extended with a table of

> num/len/prio values. This gives application more flexibility to

> specify requirements for various packet sizes.

>

> Signed-off-by: Petri Savolainen <mailto:petri.savolainen@linaro.org>

> ---

>  include/odp/api/spec/pool.h                        | 117 +++++++++++++++++++--

>  include/odp/arch/default/api/abi/pool.h            |   2 +

>  .../include/odp/api/plat/pool_types.h              |   2 +

>  3 files changed, 110 insertions(+), 11 deletions(-)

>

> diff --git a/include/odp/api/spec/pool.h b/include/odp/api/spec/pool.h

> index 6fc5b6b4..2bf37e5d 100644

> --- a/include/odp/api/spec/pool.h

> +++ b/include/odp/api/spec/pool.h

> @@ -41,6 +41,9 @@ extern "C" {

>   * Maximum pool name length in chars including null char

>   */

>

> +/** Maximum number of additional packet pool parameters */

> +#define ODP_POOL_PKT_ADD_MAX  7

> +

>  /**

>   * Pool capabilities

>   */

> @@ -127,6 +130,9 @@ typedef struct odp_pool_capability_t {

>                  * The value of zero means that limited only by the available

>                  * memory size for the pool. */

>                 uint32_t max_uarea_size;

> +

> +               /** Number of priority levels */

> +               uint8_t num_prio;

>

> In what sense are these "priorities"? Perhaps just num_subpool?

>

> This is stated in the commit log:

> "prio allows application to prioritize

> some pools (or parts of pools) over others. Implementation may

> use this information to decide e.g. on HW resource usage."

>

> [petri]

> This is not number of sub pools (spec does not expect sub- or single pool implementation). Prio is additional information to the implementation to decide where to use scarce HW resources. If implementation does care about this information, it can return num_prio == 0 here.


Can you give an example of the type of implementation that would have
a non-zero value here? If you don't want to call this subpools are you
referring to some sort of cache hierarchy you expect to be associated
with a pool? That would seem strange since caches are typically driven
by usage, not static allocations, and presumably any decisions related
to priority need to be made statically at odp_packet_alloc() time.
What packet attributes (other than requested len) would this key off?

>

>

>

>

>         } pkt;

>

>         /** Timeout pool capabilities  */

> @@ -156,6 +162,28 @@ typedef struct odp_pool_capability_t {

>  int odp_pool_capability(odp_pool_capability_t *capa);

>

>  /**

> + * @def ODP_POOL_PRIO_DEFAULT

>

> Not clear what this controls or how it's expected to be used.

>

> [petri]

> This is the default value for the prio fields under == what param_init() fills in == application saying "I don't care"

>

>

> + *

> + * Default pool priority. User does not care about the selected priority

> + * level.

> + */

> +

> +/**

> + * Additional parameters for packet pool creation

> + */

> +typedef struct odp_pool_param_pkt_add_t {

> +       /** Number of packets */

> +       uint32_t num;

> +

> +       /** Packet length in bytes */

> +       uint32_t len;

> +

> +       /** Priority level */

> +       uint8_t prio;

> +

> +} odp_pool_param_pkt_add_t;

>

> This is a confusing choice of name. Since this is intended to introduce the notion of subpools, why not just call it odp_pool_subpool_param_t?  An array of these are then just part of the containing odp_pool_param_t struct.

>

>

> [petri]

> The add[] table is additional information/more fine grained requirements to the implementation. API does not refer to sub-pools,  since that's only one type of implementation. This API can be fulfilled as easily with a single segment size implementation (which e.g. odp-linux and odp-dpdk would do).

>

> Used odp_pool_param_ there because it's part of odp_pool_param_t. Used pkt_ because it's part of pkt params. Used "add" for additional. One option is to remove the type and just define the struct inside odp_pool_param_t.


Folding this into the base odp_pool_param_t might be clearer. In
addition to its length, "add" is not an obvious abbreviation for
"additional", and additional itself is just a filler (like extended,
enhanced, or Microsoft's favorite, "advanced").

>

>

>

> +

> +/**

>   * Pool parameters

>   * Used to communicate pool creation options.

>   * @note A single thread may not be able to allocate all 'num' elements

> @@ -185,25 +213,56 @@ typedef struct odp_pool_param_t {

>

>                 /** Parameters for packet pools */

>                 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 minimum number of packets that are packet length

> +                        *  'len' bytes or smaller. The maximum value is defined

> +                        *  by pool capability pkt.max_num. An implementation

> +                        *  may round up the value, as long as the 'max_num'

> +                        *  parameter below is not violated.

> +                        */

>                         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 minimum packet length that at least 'num'

> +                        *  packets are required. The maximum value is defined

> +                        *  by pool capability pkt.max_len. Use 0 for default.

> +                        */

>                         uint32_t len;

>

> +                       /** Priority level

> +                        *

> +                        *  Priority indicates the relative service level

> +                        *  between pools and packet length configuration

>

> Needs more explanation. What is a "service level" in this context? When a packet arrives and a CoS assigns it to this pool, what's supposed to happen? When odp_packet_alloc() is called against this pool, again what's supposed to happen?

>

> [petri]

> Relative importance of this pool / packet size within a pool, compared to other pools / packet sizes. Highest prio == use all the best HW resource for this, default == don't care, lowest == use lowest level of resources for this.

>

> For example, highest prio pool may use HW pool manager and chip internal memory vs. lowest use SW pool implementation and main memory (DDR). HW and cores may see lower latency/higher performance for data of the high prio pool.


I'm not sure how this would work in practice since pools are not
randomly picked but are deterministic chosen by the classifier (a pool
associated with a CoS) or the application when it calls
odp_packet_alloc(). So it seems the only variable input to this
selection function is packet length. For a given pool, a packet of a
given length can thus have only a single "priority". It seems that
this is a roundabout way of trying to configure a "best fit" selection
of packet length to subpools consisting of segments of various fixed
sizes. This can quickly become overly complicated if we try to put too
much "nuance" into this.

>

>

>

> +                        *  within a pool. The highest priority level is zero.

> +                        *  Number of priorities is defined by pool capability

> +                        *  pkt.num_prio. Thus the lowest priority level is

> +                        *  pkt.num_prio - 1. The default value is

> +                        *  ODP_POOL_PRIO_DEFAULT.

> +                        */

> +                       uint8_t prio;

> +

> +                       /** Number of additional packet pool parameters

> +                        *

> +                        *  The number of pkt.add[] table entries filled. The

> +                        *  value must not exceed ODP_POOL_PKT_ADD_MAX.

>

> ODP_POOL_SUBPOOL_MAX?

>

> [petri]

> Sub-pool is one possible implementation.

>

>

> +                        *  The default value is 0.

> +                        */

> +                       uint8_t num_add;

> +

>                         /** 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). */

>                         uint32_t max_len;

>

> +                       /** Maximum number of packets

> +                        *

> +                        *  This is the maximum number of packets of any length

> +                        *  that can be allocated from the pool. The maximum

> +                        *  value is defined by pool capability pkt.max_num.

> +                        *  Use 0 for no requirement for maximum number.

> +                        *  The default value is 0.

> +                        */

> +                       uint32_t max_num;

> +

>                         /** Minimum number of packet data bytes that are stored

>                             in the first segment of a packet. The maximum value

>                             is defined by pool capability pkt.max_seg_len.

> @@ -214,6 +273,26 @@ typedef struct odp_pool_param_t {

>                             defined by pool capability pkt.max_uarea_size.

>                             Specify as 0 if no user area is needed. */

>                         uint32_t uarea_size;

> +

> +                       /** Additional packet pool parameters

> +                        *

> +                        *  This table gives more fine grained requirements for

> +                        *  pool configuration. The table continues from

> +                        *  num/len/prio specification above. Therefore,

> +                        *  pkt.add[0].len must be greater than pkt.len, and

> +                        *  pkt.add[0].num refers to packet lengths between

> +                        *  pkt.len + 1 and pkt.add[0].len.

> +                        *

> +                        *  Table enties must be ordered by the packet length.

> +                        *  A number of packets figure (pkt.add[N].num) refers

> +                        *  to packet lengths between pkt.add[N-1].len + 1 and

> +                        *  pkt.add[N].len. Each number of packets requirement

> +                        *  may be rounded up, as long as the 'max_num'

> +                        *  parameter is not violated. Also, more than one

> +                        *  number of packets requirement may not be fulfilled

> +                        *  simultaneously.

>

> Not clear what you're referring to in this last remark.

>

> [petri]

>

> pkt.add[N].num is guaranteed when only packet lengths (pkt.add[N-1].len + 1) ... pkt.add[N].len are allocated. But it is not guaranteed, if various other packet lengths are also allocated simultaneously.


So how would an application be expected to use this information in
practice? And what sort of validation test would we need to write to
verify that an implementation is correctly conforming to this spec?

>

>

> -Petri

>

>
diff mbox

Patch

diff --git a/include/odp/api/spec/pool.h b/include/odp/api/spec/pool.h
index 6fc5b6b4..2bf37e5d 100644
--- a/include/odp/api/spec/pool.h
+++ b/include/odp/api/spec/pool.h
@@ -41,6 +41,9 @@  extern "C" {
  * Maximum pool name length in chars including null char
  */
 
+/** Maximum number of additional packet pool parameters */
+#define ODP_POOL_PKT_ADD_MAX  7
+
 /**
  * Pool capabilities
  */
@@ -127,6 +130,9 @@  typedef struct odp_pool_capability_t {
 		 * The value of zero means that limited only by the available
 		 * memory size for the pool. */
 		uint32_t max_uarea_size;
+
+		/** Number of priority levels */
+		uint8_t num_prio;
 	} pkt;
 
 	/** Timeout pool capabilities  */
@@ -156,6 +162,28 @@  typedef struct odp_pool_capability_t {
 int odp_pool_capability(odp_pool_capability_t *capa);
 
 /**
+ * @def ODP_POOL_PRIO_DEFAULT
+ *
+ * Default pool priority. User does not care about the selected priority
+ * level.
+ */
+
+/**
+ * Additional parameters for packet pool creation
+ */
+typedef struct odp_pool_param_pkt_add_t {
+	/** Number of packets */
+	uint32_t num;
+
+	/** Packet length in bytes */
+	uint32_t len;
+
+	/** Priority level */
+	uint8_t prio;
+
+} odp_pool_param_pkt_add_t;
+
+/**
  * Pool parameters
  * Used to communicate pool creation options.
  * @note A single thread may not be able to allocate all 'num' elements
@@ -185,25 +213,56 @@  typedef struct odp_pool_param_t {
 
 		/** Parameters for packet pools */
 		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 minimum number of packets that are packet length
+			 *  'len' bytes or smaller. The maximum value is defined
+			 *  by pool capability pkt.max_num. An implementation
+			 *  may round up the value, as long as the 'max_num'
+			 *  parameter below is not violated.
+			 */
 			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 minimum packet length that at least 'num'
+			 *  packets are required. The maximum value is defined
+			 *  by pool capability pkt.max_len. Use 0 for default.
+			 */
 			uint32_t len;
 
+			/** Priority level
+			 *
+			 *  Priority indicates the relative service level
+			 *  between pools and packet length configuration
+			 *  within a pool. The highest priority level is zero.
+			 *  Number of priorities is defined by pool capability
+			 *  pkt.num_prio. Thus the lowest priority level is
+			 *  pkt.num_prio - 1. The default value is
+			 *  ODP_POOL_PRIO_DEFAULT.
+			 */
+			uint8_t prio;
+
+			/** Number of additional packet pool parameters
+			 *
+			 *  The number of pkt.add[] table entries filled. The
+			 *  value must not exceed ODP_POOL_PKT_ADD_MAX.
+			 *  The default value is 0.
+			 */
+			uint8_t num_add;
+
 			/** 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). */
 			uint32_t max_len;
 
+			/** Maximum number of packets
+			 *
+			 *  This is the maximum number of packets of any length
+			 *  that can be allocated from the pool. The maximum
+			 *  value is defined by pool capability pkt.max_num.
+			 *  Use 0 for no requirement for maximum number.
+			 *  The default value is 0.
+			 */
+			uint32_t max_num;
+
 			/** Minimum number of packet data bytes that are stored
 			    in the first segment of a packet. The maximum value
 			    is defined by pool capability pkt.max_seg_len.
@@ -214,6 +273,26 @@  typedef struct odp_pool_param_t {
 			    defined by pool capability pkt.max_uarea_size.
 			    Specify as 0 if no user area is needed. */
 			uint32_t uarea_size;
+
+			/** Additional packet pool parameters
+			 *
+			 *  This table gives more fine grained requirements for
+			 *  pool configuration. The table continues from
+			 *  num/len/prio specification above. Therefore,
+			 *  pkt.add[0].len must be greater than pkt.len, and
+			 *  pkt.add[0].num refers to packet lengths between
+			 *  pkt.len + 1 and pkt.add[0].len.
+			 *
+			 *  Table enties must be ordered by the packet length.
+			 *  A number of packets figure (pkt.add[N].num) refers
+			 *  to packet lengths between pkt.add[N-1].len + 1 and
+			 *  pkt.add[N].len. Each number of packets requirement
+			 *  may be rounded up, as long as the 'max_num'
+			 *  parameter is not violated. Also, more than one
+			 *  number of packets requirement may not be fulfilled
+			 *  simultaneously.
+			 */
+			odp_pool_param_pkt_add_t add[ODP_POOL_PKT_ADD_MAX];
 		} pkt;
 
 		/** Parameters for timeout pools */
@@ -278,8 +357,24 @@  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;
+
+	/** Copy of the pool parameters */
+	odp_pool_param_t params;
+
+	/** Packet pool info */
+	struct {
+		/** Maximum number of packets of any length
+		 *
+		 *  This many packets in maximum can be allocated from the pool.
+		 *  Application can use this e.g. to prepare enough per packet
+		 *  contexts.
+		 */
+		uint32_t max_num;
+
+	} pkt;
+
 } odp_pool_info_t;
 
 /**
diff --git a/include/odp/arch/default/api/abi/pool.h b/include/odp/arch/default/api/abi/pool.h
index 4637d19f..2e2eaf07 100644
--- a/include/odp/arch/default/api/abi/pool.h
+++ b/include/odp/arch/default/api/abi/pool.h
@@ -26,6 +26,8 @@  typedef _odp_abi_pool_t *odp_pool_t;
 
 #define ODP_POOL_NAME_LEN  32
 
+#define ODP_POOL_PRIO_DEFAULT 0
+
 typedef enum odp_pool_type_t {
 	ODP_POOL_BUFFER  = ODP_EVENT_BUFFER,
 	ODP_POOL_PACKET  = ODP_EVENT_PACKET,
diff --git a/platform/linux-generic/include/odp/api/plat/pool_types.h b/platform/linux-generic/include/odp/api/plat/pool_types.h
index 8bc816d4..7bab2fd6 100644
--- a/platform/linux-generic/include/odp/api/plat/pool_types.h
+++ b/platform/linux-generic/include/odp/api/plat/pool_types.h
@@ -36,6 +36,8 @@  typedef ODP_HANDLE_T(odp_pool_t);
 
 #define ODP_POOL_NAME_LEN  32
 
+#define ODP_POOL_PRIO_DEFAULT 0
+
 typedef enum odp_pool_type_t {
 	ODP_POOL_BUFFER  = ODP_EVENT_BUFFER,
 	ODP_POOL_PACKET  = ODP_EVENT_PACKET,