diff mbox series

[API-NEXT,v1,2/8] api: pool: add packet pool subparameters

Message ID 1508158805-5932-3-git-send-email-odpbot@yandex.ru
State Superseded
Headers show
Series [API-NEXT,v1,1/8] api: pool: relax packet pool param num | expand

Commit Message

Github ODP bot Oct. 16, 2017, 12:59 p.m. UTC
From: Petri Savolainen <petri.savolainen@linaro.org>


Additional packet length and number specification gives
more information to implementation about intended packet
length distribution in the pool. This enables e.g. correct
initialization when pool implementation is based on multiple
fixed packet / segment sizes (subpools). The specification
does require subpool implementation but allows it.

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

---
/** Email created from pull request 234 (psavol:next-pool-param)
 ** https://github.com/Linaro/odp/pull/234
 ** Patch: https://github.com/Linaro/odp/pull/234.patch
 ** Base sha: afeda4d14bb6f449cb269680cdbd56b26726eedf
 ** Merge commit sha: 54f5fc670a7c125b6b0098e34e68fe3b45875069
 **/
 include/odp/api/spec/pool.h | 47 +++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 47 insertions(+)

Comments

Bill Fischofer Oct. 16, 2017, 11:09 p.m. UTC | #1
On Mon, Oct 16, 2017 at 7:59 AM, Github ODP bot <odpbot@yandex.ru> wrote:
> From: Petri Savolainen <petri.savolainen@linaro.org>

>

> Additional packet length and number specification gives

> more information to implementation about intended packet

> length distribution in the pool. This enables e.g. correct

> initialization when pool implementation is based on multiple

> fixed packet / segment sizes (subpools). The specification

> does require subpool implementation but allows it.

>

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

> ---

> /** Email created from pull request 234 (psavol:next-pool-param)

>  ** https://github.com/Linaro/odp/pull/234

>  ** Patch: https://github.com/Linaro/odp/pull/234.patch

>  ** Base sha: afeda4d14bb6f449cb269680cdbd56b26726eedf

>  ** Merge commit sha: 54f5fc670a7c125b6b0098e34e68fe3b45875069

>  **/

>  include/odp/api/spec/pool.h | 47 +++++++++++++++++++++++++++++++++++++++++++++

>  1 file changed, 47 insertions(+)

>

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

> index f1c8b1158..7c9bee8ee 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_MAX_SUBPARAMS  7

> +

>  /**

>   * Pool capabilities

>   */

> @@ -134,6 +137,12 @@ 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;

> +

> +               /** Maximum number of subparameters

> +                *

> +                *  Maximum number of packet pool subparameters. Valid range is

> +                *  0 ... ODP_POOL_MAX_SUBPARAMS. */

> +               uint8_t max_num_sub;


max_num_subparam or max_subparam would be clearer. "sub" by itself
seems a bit too short to be intuitive.

>         } pkt;

>

>         /** Timeout pool capabilities  */

> @@ -163,6 +172,18 @@ typedef struct odp_pool_capability_t {

>  int odp_pool_capability(odp_pool_capability_t *capa);

>

>  /**

> + * Packet pool subparameters

> + */

> +typedef struct odp_pool_pkt_subparam_t {

> +       /** Number of 'len' byte packets. */

> +       uint32_t num;

> +

> +       /** Packet length in bytes */

> +       uint32_t len;

> +

> +} odp_pool_pkt_subparam_t;

> +

> +/**

>   * Pool parameters

>   *

>   * A note for all pool types: a single thread may not be able to allocate all

> @@ -246,6 +267,32 @@ typedef struct odp_pool_param_t {

>                             capability pkt.max_headroom.

>                             Use zero if headroom is not needed. */

>                         uint32_t headroom;

> +

> +                       /** Number of subparameters

> +                        *

> +                        *  The number of subparameter table entries used.

> +                        *  The maximum value is defined by pool

> +                        *  capability pkt.max_num_sub. The default value is 0.

> +                        */

> +                       uint8_t num_sub;


uint8_t num_subparam would be consistent with the above suggestion.

> +

> +                       /** Subparameter table

> +                        *

> +                        *  Subparameters continue pool configuration with

> +                        *  additional packet length requirements. The first

> +                        *  table entry follows the num/len specification above.

> +                        *  So that, sub[0].len > 'len', and sub[0].num refers

> +                        *  to packet lengths between 'len' + 1 and sub[0].len.

> +                        *  Similarly, sub[1] follows sub[0] specification, and

> +                        *  so on.

> +                        *

> +                        *  Each requirement is supported separately and may be

> +                        *  rounded up, as long as the 'max_num' parameter is

> +                        *  not violated. It's implementation specific if some

> +                        *  requirements are supported simultaneously (e.g.

> +                        *  due to subpool design).


I thought these were supposed to be optimization hints, not
requirements. It's OK for the application to advise the implementation
about how it expects to make use of pools. It seems contrary to the
spirit of ODP to make demands on how ODP implementations must organize
pools internally. So, for example, the only thing we require is that
the pool support a minimum of num packets of size len and may contain
no more than max_num packets of any size. I don't think we can require
that the pool be able to satisfy limits specified in the
subparameters.

> +                        */

> +                       odp_pool_pkt_subparam_t sub[ODP_POOL_MAX_SUBPARAMS];

>                 } pkt;

>

>                 /** Parameters for timeout pools */

>
diff mbox series

Patch

diff --git a/include/odp/api/spec/pool.h b/include/odp/api/spec/pool.h
index f1c8b1158..7c9bee8ee 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_MAX_SUBPARAMS  7
+
 /**
  * Pool capabilities
  */
@@ -134,6 +137,12 @@  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;
+
+		/** Maximum number of subparameters
+		 *
+		 *  Maximum number of packet pool subparameters. Valid range is
+		 *  0 ... ODP_POOL_MAX_SUBPARAMS. */
+		uint8_t max_num_sub;
 	} pkt;
 
 	/** Timeout pool capabilities  */
@@ -163,6 +172,18 @@  typedef struct odp_pool_capability_t {
 int odp_pool_capability(odp_pool_capability_t *capa);
 
 /**
+ * Packet pool subparameters
+ */
+typedef struct odp_pool_pkt_subparam_t {
+	/** Number of 'len' byte packets. */
+	uint32_t num;
+
+	/** Packet length in bytes */
+	uint32_t len;
+
+} odp_pool_pkt_subparam_t;
+
+/**
  * Pool parameters
  *
  * A note for all pool types: a single thread may not be able to allocate all
@@ -246,6 +267,32 @@  typedef struct odp_pool_param_t {
 			    capability pkt.max_headroom.
 			    Use zero if headroom is not needed. */
 			uint32_t headroom;
+
+			/** Number of subparameters
+			 *
+			 *  The number of subparameter table entries used.
+			 *  The maximum value is defined by pool
+			 *  capability pkt.max_num_sub. The default value is 0.
+			 */
+			uint8_t num_sub;
+
+			/** Subparameter table
+			 *
+			 *  Subparameters continue pool configuration with
+			 *  additional packet length requirements. The first
+			 *  table entry follows the num/len specification above.
+			 *  So that, sub[0].len > 'len', and sub[0].num refers
+			 *  to packet lengths between 'len' + 1 and sub[0].len.
+			 *  Similarly, sub[1] follows sub[0] specification, and
+			 *  so on.
+			 *
+			 *  Each requirement is supported separately and may be
+			 *  rounded up, as long as the 'max_num' parameter is
+			 *  not violated. It's implementation specific if some
+			 *  requirements are supported simultaneously (e.g.
+			 *  due to subpool design).
+			 */
+			odp_pool_pkt_subparam_t sub[ODP_POOL_MAX_SUBPARAMS];
 		} pkt;
 
 		/** Parameters for timeout pools */