diff mbox

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

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

Commit Message

Petri Savolainen May 30, 2017, 12:44 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 (opt) allows application give hints and
requirements about e.g. memory to be used for pools (or parts
of pools).

Additionally, pool configuration may be extended with a table of
num/len/opt 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 | 138 ++++++++++++++++++++++++++++++++++++++++----
 1 file changed, 127 insertions(+), 11 deletions(-)

-- 
2.11.0

Comments

Bill Fischofer May 31, 2017, 2:45 a.m. UTC | #1
On Tue, May 30, 2017 at 7:44 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 (opt) allows application give hints and

> requirements about e.g. memory to be used for pools (or parts

> of pools).

>

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

> num/len/opt 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 | 138 ++++++++++++++++++++++++++++++++++++++++----

>  1 file changed, 127 insertions(+), 11 deletions(-)

>

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

> index 6fc5b6b4..f9d9cbbb 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 packet pool subparameters */

> +#define ODP_POOL_PKT_SUB_MAX  7


Why this number? Seems overly generous.

> +

>  /**

>   * Pool capabilities

>   */

> @@ -156,6 +159,44 @@ typedef struct odp_pool_capability_t {

>  int odp_pool_capability(odp_pool_capability_t *capa);

>

>  /**

> + * Pool memory type

> + */

> +typedef enum odp_pool_mem_type_t {

> +       /** Default memory type */

> +       ODP_POOL_MEM_DEFAULT = 0,

> +

> +       /** Fast memory */

> +       ODP_POOL_MEM_FAST


What is "fast" memory? Need a bit more explanation of what's being
defined by this enum. Is this NUMA related? The NUMA design we
discussed at LAS16 [1] allowed a dev_id to be specified to designate
pool memory.

[1] http://patches.opendataplane.org/patch/7173/

> +

> +} odp_pool_mem_type_t;

> +

> +/**

> + * Pool segmentation options

> + */

> +typedef enum odp_pool_seg_opt_t {

> +       /** Default segmentation. Pool may or may not store packet data into

> +        *  multiple segments. */

> +       ODP_POOL_SEG_DEFAULT = 0,

> +

> +       /** Pool must not segment packets. All data of a packet must be stored

> +        *  into single, contiguous memory area. */

> +       ODP_POOL_SEG_DISABLED

> +

> +} odp_pool_seg_opt_t;


When we originally talked about ODP pools we had the notion of
supporting an unsegmented pool option, which this is resurrecting.
However why an enum here rather than a simple boolean (unsegmented
yes|no)? If we want to expand this into an enum, I'd think we'd want
something like:

typedef enum odp_pool_seg_opt_t {
        /** Pool may store data in segments of a fixed
implementation-chosen size */
        ODP_POOL_SEG_FIXED = 0,

        /** Pool may store data in segments of variable
implementation-chosen sizes */
        ODP_POOL_SEG_VARIABLE,

        /** Pool may not store data in multiple segments. All packets
must be in a single segment */
        ODP_POOL_SEG_UNSEGMENTED,
} odp_pool_seg_opt_t;

An alternative would be to split this: pool may support segments of
fixed or variable sizes and (orthogonal to this) packets must be
unsegmented. An unsegmented packet in a variable-segment pool means
that the implementation picks a segment size that best fits the
packet. An unsegmented packet in a fixed-segment pool means that the
fixed segment size must be large enough to hold the largest packet the
pool needs to store.

> +

> +/**

> + * Additional options for packet pool creation

> + */

> +typedef struct odp_pool_pkt_opt_t {

> +       /** Pool memory type */

> +       odp_pool_mem_type_t type;

> +

> +       /** Segmentation options */

> +       odp_pool_seg_opt_t  seg;

> +

> +} odp_pool_pkt_opt_t;

> +

> +/**

>   * Pool parameters

>   * Used to communicate pool creation options.

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

> @@ -185,25 +226,54 @@ 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;

>

> +                       /** Packet pool options

> +                        *

> +                        *  Options contain additional hints and requirements,

> +                        *  which quide implementation e.g. to select correct

> +                        *  memory type for the pool.

> +                        */

> +                       odp_pool_pkt_opt_t opt;

> +

> +                       /** Number of subparameters

> +                        *

> +                        *  The number of subparameter table (pkt.sub[]) entries

> +                        *  filled in. Subparameters continue pool configuration

> +                        *  after the three parameters ('num', 'len' and 'opt').

> +                        *  above. The value must not exceed

> +                        *  ODP_POOL_PKT_SUB_MAX. The default value is 0.

> +                        */

> +                       uint8_t num_sub;

> +

>                         /** 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;


How is an implementation supposed to guarantee this unless pool
allocation tracks packets rather than segments? The debate we had
earlier all centered around the supposition that implementations would
find it onerous to track packet allocation. Conversely, if an
implementation can track packet allocation then this additional
machinery seems like overkill.

> +

>                         /** 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 +284,36 @@ 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;

> +

> +                       /** Packet pool subparameters

> +                        *

> +                        *  This table gives more fine grained requirements for

> +                        *  pool configuration. The table continues from

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

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

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

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

> +                        *

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

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

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

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

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

> +                        *  parameter is not violated. A pool fulfills each

> +                        *  num/len requirement separately (when there are no

> +                        *  other allocations from the pool).

> +                        */

> +                       struct {

> +                               /** Number of packets */

> +                               uint32_t num;

> +

> +                               /** Packet length in bytes */

> +                               uint32_t len;

> +

> +                               /** Packet pool options */

> +                               odp_pool_pkt_opt_t opt;

> +

> +                       } sub[ODP_POOL_PKT_SUB_MAX];


It would be helpful to better understand the use cases we're trying to
accommodate here. How does the current pool API fall short of
application needs? This seems less like allowing an application to
specify its needs and more like trying to control low-level
implementation details, which is something we try to avoid in ODP APIs
as such controls assume an implementation model.

>                 } pkt;

>

>                 /** Parameters for timeout pools */

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

>

>  /**

> --

> 2.11.0

>
Bogdan Pricope May 31, 2017, 6:49 a.m. UTC | #2
It is me or this pool configuration looks very “cryptic” (lot of
params with some less intuitive naming)?


I mean, one would need to configure:
- num -  The minimum number of packets that are packet length
'len' bytes or smaller.
- ‘len’ - The minimum packet length that at least 'num' packets are required
-  'max_len'  –  Maximum packet length
-  'max_num' -  Maximum number of packets of any length
-  'num_sub'
-  'sub[]'

Having only num_sub and sub[] (with better naming) will not be enough?


On 31 May 2017 at 05:45, Bill Fischofer <bill.fischofer@linaro.org> wrote:
> On Tue, May 30, 2017 at 7:44 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 (opt) allows application give hints and

>> requirements about e.g. memory to be used for pools (or parts

>> of pools).

>>

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

>> num/len/opt 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 | 138 ++++++++++++++++++++++++++++++++++++++++----

>>  1 file changed, 127 insertions(+), 11 deletions(-)

>>

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

>> index 6fc5b6b4..f9d9cbbb 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 packet pool subparameters */

>> +#define ODP_POOL_PKT_SUB_MAX  7

>

> Why this number? Seems overly generous.

>

>> +

>>  /**

>>   * Pool capabilities

>>   */

>> @@ -156,6 +159,44 @@ typedef struct odp_pool_capability_t {

>>  int odp_pool_capability(odp_pool_capability_t *capa);

>>

>>  /**

>> + * Pool memory type

>> + */

>> +typedef enum odp_pool_mem_type_t {

>> +       /** Default memory type */

>> +       ODP_POOL_MEM_DEFAULT = 0,

>> +

>> +       /** Fast memory */

>> +       ODP_POOL_MEM_FAST

>

> What is "fast" memory? Need a bit more explanation of what's being

> defined by this enum. Is this NUMA related? The NUMA design we

> discussed at LAS16 [1] allowed a dev_id to be specified to designate

> pool memory.

>

> [1] http://patches.opendataplane.org/patch/7173/

>

>> +

>> +} odp_pool_mem_type_t;

>> +

>> +/**

>> + * Pool segmentation options

>> + */

>> +typedef enum odp_pool_seg_opt_t {

>> +       /** Default segmentation. Pool may or may not store packet data into

>> +        *  multiple segments. */

>> +       ODP_POOL_SEG_DEFAULT = 0,

>> +

>> +       /** Pool must not segment packets. All data of a packet must be stored

>> +        *  into single, contiguous memory area. */

>> +       ODP_POOL_SEG_DISABLED

>> +

>> +} odp_pool_seg_opt_t;

>

> When we originally talked about ODP pools we had the notion of

> supporting an unsegmented pool option, which this is resurrecting.

> However why an enum here rather than a simple boolean (unsegmented

> yes|no)? If we want to expand this into an enum, I'd think we'd want

> something like:

>

> typedef enum odp_pool_seg_opt_t {

>         /** Pool may store data in segments of a fixed

> implementation-chosen size */

>         ODP_POOL_SEG_FIXED = 0,

>

>         /** Pool may store data in segments of variable

> implementation-chosen sizes */

>         ODP_POOL_SEG_VARIABLE,

>

>         /** Pool may not store data in multiple segments. All packets

> must be in a single segment */

>         ODP_POOL_SEG_UNSEGMENTED,

> } odp_pool_seg_opt_t;

>

> An alternative would be to split this: pool may support segments of

> fixed or variable sizes and (orthogonal to this) packets must be

> unsegmented. An unsegmented packet in a variable-segment pool means

> that the implementation picks a segment size that best fits the

> packet. An unsegmented packet in a fixed-segment pool means that the

> fixed segment size must be large enough to hold the largest packet the

> pool needs to store.

>

>> +

>> +/**

>> + * Additional options for packet pool creation

>> + */

>> +typedef struct odp_pool_pkt_opt_t {

>> +       /** Pool memory type */

>> +       odp_pool_mem_type_t type;

>> +

>> +       /** Segmentation options */

>> +       odp_pool_seg_opt_t  seg;

>> +

>> +} odp_pool_pkt_opt_t;

>> +

>> +/**

>>   * Pool parameters

>>   * Used to communicate pool creation options.

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

>> @@ -185,25 +226,54 @@ 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;

>>

>> +                       /** Packet pool options

>> +                        *

>> +                        *  Options contain additional hints and requirements,

>> +                        *  which quide implementation e.g. to select correct

>> +                        *  memory type for the pool.

>> +                        */

>> +                       odp_pool_pkt_opt_t opt;

>> +

>> +                       /** Number of subparameters

>> +                        *

>> +                        *  The number of subparameter table (pkt.sub[]) entries

>> +                        *  filled in. Subparameters continue pool configuration

>> +                        *  after the three parameters ('num', 'len' and 'opt').

>> +                        *  above. The value must not exceed

>> +                        *  ODP_POOL_PKT_SUB_MAX. The default value is 0.

>> +                        */

>> +                       uint8_t num_sub;

>> +

>>                         /** 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;

>

> How is an implementation supposed to guarantee this unless pool

> allocation tracks packets rather than segments? The debate we had

> earlier all centered around the supposition that implementations would

> find it onerous to track packet allocation. Conversely, if an

> implementation can track packet allocation then this additional

> machinery seems like overkill.

>

>> +

>>                         /** 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 +284,36 @@ 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;

>> +

>> +                       /** Packet pool subparameters

>> +                        *

>> +                        *  This table gives more fine grained requirements for

>> +                        *  pool configuration. The table continues from

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

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

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

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

>> +                        *

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

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

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

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

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

>> +                        *  parameter is not violated. A pool fulfills each

>> +                        *  num/len requirement separately (when there are no

>> +                        *  other allocations from the pool).

>> +                        */

>> +                       struct {

>> +                               /** Number of packets */

>> +                               uint32_t num;

>> +

>> +                               /** Packet length in bytes */

>> +                               uint32_t len;

>> +

>> +                               /** Packet pool options */

>> +                               odp_pool_pkt_opt_t opt;

>> +

>> +                       } sub[ODP_POOL_PKT_SUB_MAX];

>

> It would be helpful to better understand the use cases we're trying to

> accommodate here. How does the current pool API fall short of

> application needs? This seems less like allowing an application to

> specify its needs and more like trying to control low-level

> implementation details, which is something we try to avoid in ODP APIs

> as such controls assume an implementation model.

>

>>                 } pkt;

>>

>>                 /** Parameters for timeout pools */

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

>>

>>  /**

>> --

>> 2.11.0

>>
Savolainen, Petri (Nokia - FI/Espoo) May 31, 2017, 7:04 a.m. UTC | #3
> -----Original Message-----

> From: Bogdan Pricope [mailto:bogdan.pricope@linaro.org]

> Sent: Wednesday, May 31, 2017 9:50 AM

> To: Bill Fischofer <bill.fischofer@linaro.org>

> Cc: Petri Savolainen <petri.savolainen@linaro.org>; lng-odp-forward <lng-

> odp@lists.linaro.org>

> Subject: Re: [lng-odp] [API-NEXT PATCH v2] api: pool: additional packet

> length configuration

> 

> It is me or this pool configuration looks very “cryptic” (lot of

> params with some less intuitive naming)?

> 

> 

> I mean, one would need to configure:

> - num -  The minimum number of packets that are packet length

> 'len' bytes or smaller.

> - ‘len’ - The minimum packet length that at least 'num' packets are

> required

> -  'max_len'  –  Maximum packet length

> -  'max_num' -  Maximum number of packets of any length

> -  'num_sub'

> -  'sub[]'

> 

> Having only num_sub and sub[] (with better naming) will not be enough?

> 


Num and len are left (almost as is) for backwards compatibility and simplicity. Most users would only set num/len and nothing else. Advanced user will set multiple num/len pairs to give more granular specification what is required - so that e.g. implementation which has sub-pools in HW can initialize those optimally. 

You just need to realize that num/len/opt is the first sub[] table entry (sub[-1]). This is described in documentation. Backwards compatibility is valuable, but does not always result cleanest possible API.

-Petri
Savolainen, Petri (Nokia - FI/Espoo) May 31, 2017, 7:42 a.m. UTC | #4
> > +/** Maximum number of packet pool subparameters */

> > +#define ODP_POOL_PKT_SUB_MAX  7

> 

> Why this number? Seems overly generous.


This is the maximum number of subparams (== sub pools) the API can describe. There are HW buffer managers that can handle up to 8 sub pools, so anything less in API would limit those implementations. BTW: num/len/opt + sub[0...6] ==> 8 sub pools


> 

> > +

> >  /**

> >   * Pool capabilities

> >   */

> > @@ -156,6 +159,44 @@ typedef struct odp_pool_capability_t {

> >  int odp_pool_capability(odp_pool_capability_t *capa);

> >

> >  /**

> > + * Pool memory type

> > + */

> > +typedef enum odp_pool_mem_type_t {

> > +       /** Default memory type */

> > +       ODP_POOL_MEM_DEFAULT = 0,

> > +

> > +       /** Fast memory */

> > +       ODP_POOL_MEM_FAST

> 

> What is "fast" memory? Need a bit more explanation of what's being

> defined by this enum. Is this NUMA related? The NUMA design we

> discussed at LAS16 [1] allowed a dev_id to be specified to designate

> pool memory.

> 

> [1] http://patches.opendataplane.org/patch/7173/



We can come up with better naming if needed, but this is the minimum information implementation needs to decide e.g. between placing the pool into DDR memory (default, very large in size) or internal SRAM (fast, but limited in size).

This is not NUMA. Already a single chip may have multiple options for which memory/HW resources to use.


> 

> > +

> > +} odp_pool_mem_type_t;

> > +

> > +/**

> > + * Pool segmentation options

> > + */

> > +typedef enum odp_pool_seg_opt_t {

> > +       /** Default segmentation. Pool may or may not store packet data

> into

> > +        *  multiple segments. */

> > +       ODP_POOL_SEG_DEFAULT = 0,

> > +

> > +       /** Pool must not segment packets. All data of a packet must be

> stored

> > +        *  into single, contiguous memory area. */

> > +       ODP_POOL_SEG_DISABLED

> > +

> > +} odp_pool_seg_opt_t;

> 

> When we originally talked about ODP pools we had the notion of

> supporting an unsegmented pool option, which this is resurrecting.

> However why an enum here rather than a simple boolean (unsegmented

> yes|no)? If we want to expand this into an enum, I'd think we'd want

> something like:

> 

> typedef enum odp_pool_seg_opt_t {

>         /** Pool may store data in segments of a fixed

> implementation-chosen size */

>         ODP_POOL_SEG_FIXED = 0,

> 

>         /** Pool may store data in segments of variable

> implementation-chosen sizes */

>         ODP_POOL_SEG_VARIABLE,

> 

>         /** Pool may not store data in multiple segments. All packets

> must be in a single segment */

>         ODP_POOL_SEG_UNSEGMENTED,

> } odp_pool_seg_opt_t;



This answers why it's enum. We can start with simple yes/no and extend later if needed. I think today default / unsegmented is enough. Default means no limitations, implementation can do anything.


> 

> An alternative would be to split this: pool may support segments of

> fixed or variable sizes and (orthogonal to this) packets must be

> unsegmented. An unsegmented packet in a variable-segment pool means

> that the implementation picks a segment size that best fits the

> packet. An unsegmented packet in a fixed-segment pool means that the

> fixed segment size must be large enough to hold the largest packet the

> pool needs to store



This option is defined per sub-len. So, with the same API application can ask that all packet sizes are unsegmented, or e.g. only first couple sub-lens. Unsegmented setting just means that number of segments must be 1 (up to the given packet length).


> >

> > +                       /** 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;

> 

> How is an implementation supposed to guarantee this unless pool

> allocation tracks packets rather than segments? The debate we had

> earlier all centered around the supposition that implementations would

> find it onerous to track packet allocation. Conversely, if an

> implementation can track packet allocation then this additional

> machinery seems like overkill.


Implementation initializes the pool with a certain number of segments which can for a certain number of packets of certain length. It just needs to check, that in the worst case (e.g. all packets are 1 byte length) the total number of packets cannot be larger than this. In pool initialization, the num/len specifications form the lower limit for number of segments, and this is the upper limit.


> > +                       struct {

> > +                               /** Number of packets */

> > +                               uint32_t num;

> > +

> > +                               /** Packet length in bytes */

> > +                               uint32_t len;

> > +

> > +                               /** Packet pool options */

> > +                               odp_pool_pkt_opt_t opt;

> > +

> > +                       } sub[ODP_POOL_PKT_SUB_MAX];

> 

> It would be helpful to better understand the use cases we're trying to

> accommodate here. How does the current pool API fall short of

> application needs? This seems less like allowing an application to

> specify its needs and more like trying to control low-level

> implementation details, which is something we try to avoid in ODP APIs

> as such controls assume an implementation model.


Implementation details is not interesting here. When application gives more information about the pool usage, implementation can use HW resources more efficiently. 

For example, application needs to handle short packets with lowest possible latency, but large packets do not have such requirement. The total number of application's per packet contexts is max_num.


	^
	|
   max_num	| - - - - - - - - - - -
	|
	|   *
NUM	|   * *
	|   * * *
	| o * * * *
	| o * * * * *
	+--------------------->
		LEN

o == small number of short packets into fast memory
* == all other packets into DDR


Today, it looks like this:

	^
	|
	|
	| 
     'num'	| * *
NUM	| * * *
	| * * * *
	| * * * * *
	| * * * * * *
	+--------------------->
	  'len'	LEN


* == everything goes to DDR


-Petri
diff mbox

Patch

diff --git a/include/odp/api/spec/pool.h b/include/odp/api/spec/pool.h
index 6fc5b6b4..f9d9cbbb 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 packet pool subparameters */
+#define ODP_POOL_PKT_SUB_MAX  7
+
 /**
  * Pool capabilities
  */
@@ -156,6 +159,44 @@  typedef struct odp_pool_capability_t {
 int odp_pool_capability(odp_pool_capability_t *capa);
 
 /**
+ * Pool memory type
+ */
+typedef enum odp_pool_mem_type_t {
+	/** Default memory type */
+	ODP_POOL_MEM_DEFAULT = 0,
+
+	/** Fast memory */
+	ODP_POOL_MEM_FAST
+
+} odp_pool_mem_type_t;
+
+/**
+ * Pool segmentation options
+ */
+typedef enum odp_pool_seg_opt_t {
+	/** Default segmentation. Pool may or may not store packet data into
+	 *  multiple segments. */
+	ODP_POOL_SEG_DEFAULT = 0,
+
+	/** Pool must not segment packets. All data of a packet must be stored
+	 *  into single, contiguous memory area. */
+	ODP_POOL_SEG_DISABLED
+
+} odp_pool_seg_opt_t;
+
+/**
+ * Additional options for packet pool creation
+ */
+typedef struct odp_pool_pkt_opt_t {
+	/** Pool memory type */
+	odp_pool_mem_type_t type;
+
+	/** Segmentation options */
+	odp_pool_seg_opt_t  seg;
+
+} odp_pool_pkt_opt_t;
+
+/**
  * Pool parameters
  * Used to communicate pool creation options.
  * @note A single thread may not be able to allocate all 'num' elements
@@ -185,25 +226,54 @@  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;
 
+			/** Packet pool options
+			 *
+			 *  Options contain additional hints and requirements,
+			 *  which quide implementation e.g. to select correct
+			 *  memory type for the pool.
+			 */
+			odp_pool_pkt_opt_t opt;
+
+			/** Number of subparameters
+			 *
+			 *  The number of subparameter table (pkt.sub[]) entries
+			 *  filled in. Subparameters continue pool configuration
+			 *  after the three parameters ('num', 'len' and 'opt').
+			 *  above. The value must not exceed
+			 *  ODP_POOL_PKT_SUB_MAX. The default value is 0.
+			 */
+			uint8_t num_sub;
+
 			/** 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 +284,36 @@  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;
+
+			/** Packet pool subparameters
+			 *
+			 *  This table gives more fine grained requirements for
+			 *  pool configuration. The table continues from
+			 *  num/len/opt specification above. Therefore,
+			 *  pkt.sub[0].len must be greater than pkt.len, and
+			 *  pkt.sub[0].num refers to packet lengths between
+			 *  pkt.len + 1 and pkt.sub[0].len.
+			 *
+			 *  Table enties must be ordered by the packet length.
+			 *  A number of packets figure (pkt.sub[N].num) refers
+			 *  to packet lengths between pkt.sub[N-1].len + 1 and
+			 *  pkt.sub[N].len. Each number of packets requirement
+			 *  may be rounded up, as long as the 'max_num'
+			 *  parameter is not violated. A pool fulfills each
+			 *  num/len requirement separately (when there are no
+			 *  other allocations from the pool).
+			 */
+			struct {
+				/** Number of packets */
+				uint32_t num;
+
+				/** Packet length in bytes */
+				uint32_t len;
+
+				/** Packet pool options */
+				odp_pool_pkt_opt_t opt;
+
+			} sub[ODP_POOL_PKT_SUB_MAX];
 		} pkt;
 
 		/** Parameters for timeout pools */
@@ -278,8 +378,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;
 
 /**