> -----Original Message-----
> From: Jerin Jacob <jerinjac...@gmail.com>
> Sent: Monday, February 20, 2023 9:51 PM
> To: Yan, Zhirun <zhirun....@intel.com>
> Cc: dev@dpdk.org; jer...@marvell.com; kirankum...@marvell.com;
> ndabilpu...@marvell.com; Liang, Cunming <cunming.li...@intel.com>; Wang,
> Haiyue <haiyue.w...@intel.com>
> Subject: Re: [PATCH v1 04/13] graph: add get/set graph worker model APIs
>
> On Thu, Nov 17, 2022 at 10:40 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>
> > ---
> > 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;
>
> This will break the multiprocess.
Thanks. I will use TLS for per-thread local storage.
>
> > +
> > +/** Graph worker models */
> > +enum rte_graph_worker_model {
> > +#define WORKER_MODEL_DEFAULT "default"
>
> Why need strings?
> Also, every symbol in a public header file should start with RTE_ to avoid
> namespace conflict.
It was used to config the model in app. I can put the string into example.
>
> > + RTE_GRAPH_MODEL_DEFAULT = 0,
> > +#define WORKER_MODEL_RTC "rtc"
> > + RTE_GRAPH_MODEL_RTC,
>
> Why not RTE_GRAPH_MODEL_RTC = RTE_GRAPH_MODEL_DEFAULT in enum
> itself.
Yes, will do in next version.
>
> > +#define WORKER_MODEL_GENERIC "generic"
>
> Generic is a very overloaded term. Use pipeline here i.e
> RTE_GRAPH_MODEL_PIPELINE
Actually, it's not a purely pipeline mode. I prefer to change to hybrid.
>
>
> > + RTE_GRAPH_MODEL_GENERIC,
> > + RTE_GRAPH_MODEL_MAX,
>
> No need for MAX, it will break the ABI for future. See other subsystem such as
> cryptodev.
Thanks, I will change it.
>
> > +};
>
> >
