RE: [EXT] [PATCH v1 04/13] graph: add get/set graph worker model APIs

["Yan, Zhirun" <zhirun.yan@intel.com>]

> -----Original Message-----
> From: Kiran Kumar Kokkilagadda <kirankumark@marvell.com>
> Sent: Tuesday, December 6, 2022 11:35 AM
> To: Yan, Zhirun <zhirun.yan@intel.com>; dev@dpdk.org; Jerin Jacob
> Kollanukkaran <jerinj@marvell.com>; Nithin Kumar Dabilpuram
> <ndabilpuram@marvell.com>
> Cc: Liang, Cunming <cunming.liang@intel.com>; Wang, Haiyue
> <haiyue.wang@intel.com>
> Subject: RE: [EXT] [PATCH v1 04/13] graph: add get/set graph worker model
> APIs
> 
> 
> 
> > -----Original Message-----
> > From: Zhirun Yan <zhirun.yan@intel.com>
> > Sent: 17 November 2022 10:39 AM
> > To: dev@dpdk.org; Jerin Jacob Kollanukkaran <jerinj@marvell.com>;
> > Kiran Kumar Kokkilagadda <kirankumark@marvell.com>; Nithin Kumar
> > Dabilpuram <ndabilpuram@marvell.com>
> > Cc: cunming.liang@intel.com; haiyue.wang@intel.com; Zhirun Yan
> > <zhirun.yan@intel.com>
> > Subject: [EXT] [PATCH v1 04/13] graph: add get/set graph worker model
> > APIs
> >
> > External Email
> >
> > ----------------------------------------------------------------------
> > 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>
> > ---
> >  lib/graph/rte_graph_worker.h        | 51 +++++++++++++++++++++++++++++
> >  lib/graph/rte_graph_worker_common.h | 13 ++++++++
> >  lib/graph/version.map               |  3 ++
> >  3 files changed, 67 insertions(+)
> >
> > diff --git a/lib/graph/rte_graph_worker.h
> > b/lib/graph/rte_graph_worker.h index
> > 54d1390786..a0ea0df153 100644
> > --- a/lib/graph/rte_graph_worker.h
> > +++ b/lib/graph/rte_graph_worker.h
> > @@ -1,5 +1,56 @@
> >  #include "rte_graph_model_rtc.h"
> >
> > +static enum rte_graph_worker_model worker_model =
> > +RTE_GRAPH_MODEL_DEFAULT;
> > +
> > +/**
> > + * @warning
> > + * @b EXPERIMENTAL: this API may change, or be removed, without prior
> > +notice
> > + * Set the graph worker model
> > + *
> > + * @note This function does not perform any locking, and is only safe to
> call
> > + *    before graph running.
> > + *
> > + * @param name
> > + *   Name of the graph worker model.
> > + *
> > + * @return
> > + *   0 on success, -1 otherwise.
> > + */
> > +__rte_experimental
> > +static inline int
> > +rte_graph_worker_model_set(enum rte_graph_worker_model model) {
> > +	if (model >= RTE_GRAPH_MODEL_MAX)
> > +		goto fail;
> > +
> > +	worker_model = model;
> > +	return 0;
> > +
> > +fail:
> > +	worker_model = RTE_GRAPH_MODEL_DEFAULT;
> > +	return -1;
> > +}
> > +
> > +/**
> > + * @warning
> > + * @b EXPERIMENTAL: this API may change, or be removed, without prior
> > +notice
> > + *
> > + * Get the graph worker model
> > + *
> > + * @param name
> > + *   Name of the graph worker model.
> > + *
> > + * @return
> > + *   Graph worker model on success.
> > + */
> > +__rte_experimental
> > +static inline
> > +enum rte_graph_worker_model
> > +rte_graph_worker_model_get(void)
> > +{
> > +	return worker_model;
> > +}
> > +
> >  /**
> >   * Perform graph walk on the circular buffer and invoke the process
> function
> >   * of the nodes and collect the stats.
> > diff --git a/lib/graph/rte_graph_worker_common.h
> > b/lib/graph/rte_graph_worker_common.h
> > index df33204336..507a344afd 100644
> > --- a/lib/graph/rte_graph_worker_common.h
> > +++ b/lib/graph/rte_graph_worker_common.h
> > @@ -86,6 +86,19 @@ struct rte_node {
> >  	struct rte_node *nodes[] __rte_cache_min_aligned; /**< Next nodes.
> > */  } __rte_cache_aligned;
> >
> > +
> > +
> > +/** Graph worker models */
> > +enum rte_graph_worker_model {
> > +#define WORKER_MODEL_DEFAULT "default"
> > +	RTE_GRAPH_MODEL_DEFAULT = 0,
> > +#define WORKER_MODEL_RTC "rtc"
> > +	RTE_GRAPH_MODEL_RTC,
> 
> Since default is RTC, do we need one more enum  for RTC? Can we just have
> default and generic and remove rtc?
> 

Thanks for your comments.

Actually, there are two kinds of user, professional user and normal.
For professional users, If app chose RTC or GENERIC, it means that there is a specific
requirement for worker model.
And the default is for normal users who don't care the model.

Also, in the future, if more worker model added, RTC will be more clear to describe this
model rather than default. 


> > +#define WORKER_MODEL_GENERIC "generic"
> > +	RTE_GRAPH_MODEL_GENERIC,
> > +	RTE_GRAPH_MODEL_MAX,
> > +};
> > +
> >  /**
> >   * @internal
> >   *
> > 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;
> > +
> >  	local: *;
> >  };
> > --
> > 2.25.1