Re: [lng-odp] [API-NEXT PATCH 1/4] api: pool: add maximum packet counts to pool info
>> - /** 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
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 */ > - odp_pool_param_t params; /**< pool paramete
Re: [lng-odp] [API-NEXT PATCH 1/4] api: pool: add maximum packet counts to pool info
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
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