Re: [PATCH v9 04/17] graph: add get/set graph worker model APIs

[Jerin Jacob <jerinjacobk@gmail.com>]

On Wed, Jun 7, 2023 at 9:30 AM Zhirun Yan <zhirun.yan@intel.com> wrote:
>
> Add new get/set APIs to configure graph worker model which is used to
> determine which model will be chosen.
>
> Signed-off-by: Haiyue Wang <haiyue.wang@intel.com>
> Signed-off-by: Cunming Liang <cunming.liang@intel.com>
> Signed-off-by: Zhirun Yan <zhirun.yan@intel.com>

>
> +/** Graph worker models */
> +/* If adding new entry, then update graph_model_is_valid API. */

When adding new model entry, update rte_graph_model_is_valid API logic

> +#define RTE_GRAPH_MODEL_RTC 0 /**< Run-To-Completion model. It is the default model. */
> +#define RTE_GRAPH_MODEL_DEFAULT RTE_GRAPH_MODEL_RTC /**< Default graph model. */

This line can come after RTE_GRAPH_MODEL_MCORE_DISPATCH

> +#define RTE_GRAPH_MODEL_MCORE_DISPATCH 1
> +/**< Dispatch model to support cross-core dispatching within core affinity. */
> +
>  /**
>   * @internal
>   *
> @@ -41,6 +48,7 @@ struct rte_graph {
>         rte_node_t nb_nodes;         /**< Number of nodes in the graph. */
>         rte_graph_off_t *cir_start;  /**< Pointer to circular buffer. */
>         rte_graph_off_t nodes_start; /**< Offset at which node memory starts. */
> +       uint32_t model;              /**< graph model */

uin8_t is enough and add uint8_t and uint16_t reserved. So that this
fastpath area can be used in future as needed.


>         rte_graph_t id; /**< Graph identifier. */
>         int socket;     /**< Socket ID where memory is allocated. */
>         char name[RTE_GRAPH_NAMESIZE];  /**< Name of the graph. */
> @@ -490,6 +498,59 @@ rte_node_next_stream_move(struct rte_graph *graph, struct rte_node *src,
>         }
>  }
>
> +/**
> + * Test the validity of model.
> + *
> + * @param id
> + *   Node id to check.

It is not node id

> + *
> + * @return
> + *   true if graph model is valid, false otherwise.
> + */
> +static __rte_always_inline
> +bool
> +graph_model_is_valid(uint32_t model)

Public API in header file, use rte_graph_...
Also, implementation can go to .c file. See below comment for no_check version.



> +{
> +       if (model > RTE_GRAPH_MODEL_MCORE_DISPATCH)
> +               return false;
> +
> +       return true;
> +}
> +
> +/**
> + * @note This function does not perform any locking, and is only safe to call
> + *    before graph running. It will set all graphs the same model.
> + *
> + * @param model
> + *   Name of the graph worker model.
> + *
> + * @return
> + *   0 on success, -1 otherwise.
> + */
> +__rte_experimental
> +int rte_graph_worker_model_set(uint32_t model);
> +
> +/**
> + * Get the graph worker model
> + *
> + * @note All graph will use the same model and this function will get model from the first one
> + *
> + * @param graph
> + *   Graph pointer.
> + *
> + * @return
> + *   Graph worker model on success.
> + */
> +__rte_experimental
> +static inline uint32_t
> +rte_graph_worker_model_get(struct rte_graph *graph)
> +{
> +       if (!graph_model_is_valid(graph->model))
> +               return -EINVAL;

Introduce rte_graph_worker_model_no_check_get() to skip this check to
use with fastpath.

rte_graph_worker_model_get can move to .c file.

> +
> +       return graph->model;
> +}


> +
>  #ifdef __cplusplus
>  }
>  #endif
> diff --git a/lib/graph/version.map b/lib/graph/version.map
> index 13b838752d..eea73ec9ca 100644
> --- a/lib/graph/version.map
> +++ b/lib/graph/version.map
> @@ -43,5 +43,8 @@ EXPERIMENTAL {
>         rte_node_next_stream_put;
>         rte_node_next_stream_move;
>
> +       rte_graph_worker_model_set;
> +       rte_graph_worker_model_get;

Add rte_graph_model_is_valid() in next verion.


> +
>         local: *;
>  };
> --
> 2.37.2
>