> -----Original Message-----
> From: Jerin Jacob <jerinjac...@gmail.com>
> Sent: Wednesday, June 7, 2023 3:43 PM
> To: Yan, Zhirun <zhirun....@intel.com>
> Cc: dev@dpdk.org; jer...@marvell.com; kirankum...@marvell.com;
> ndabilpu...@marvell.com; step...@networkplumber.org;
> pbhagavat...@marvell.com; Liang, Cunming <cunming.li...@intel.com>; Wang,
> Haiyue <haiyue.w...@intel.com>; mattias.ronnblom
> <mattias.ronnb...@ericsson.com>
> Subject: Re: [PATCH v9 04/17] graph: add get/set graph worker model APIs
>
> On Wed, Jun 7, 2023 at 9:30 AM Zhirun Yan <zhirun....@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.w...@intel.com>
> > Signed-off-by: Cunming Liang <cunming.li...@intel.com>
> > Signed-off-by: Zhirun Yan <zhirun....@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
>
Got it.
> > +#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
>
Ok.
> > +#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.
>
I agree. Thanks.
>
> > 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
>
Will change it to model.
> > + *
> > + * @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.
>
Ok. I will move it into rte_graph_worker.c.
>
>
> > +{
> > + 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.
Yes. Will move in next version.
Got it. rte_graph_worker_model_no_check_get() will be used in fast path.
Actually, I don’t find the performance impact about static inline, so should
the new
API declared with static inline keywords or put it into .c file also?
>
> > +
> > + 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.
Yes.
>
>
> > +
> > local: *;
> > };
> > --
> > 2.37.2
> >
