[PATCH v6 06/15] graph: introduce graph bind unbind API

[Yan, Zhirun]

> -----Original Message-----
> From: Jerin Jacob <jerinjacobk at gmail.com>
> Sent: Wednesday, May 24, 2023 2:24 PM
> To: Yan, Zhirun <zhirun.yan at intel.com>
> Cc: dev at dpdk.org; jerinj at marvell.com; kirankumark at marvell.com;
> ndabilpuram at marvell.com; stephen at networkplumber.org;
> pbhagavatula at marvell.com; Liang, Cunming <cunming.liang at intel.com>; Wang,
> Haiyue <haiyue.wang at intel.com>
> Subject: Re: [PATCH v6 06/15] graph: introduce graph bind unbind API
> 
> On Tue, May 9, 2023 at 11:34 AM Zhirun Yan <zhirun.yan at intel.com> wrote:
> >
> > Add lcore_id for graph to hold affinity core id where graph would run on.
> > Add bind/unbind API to set/unset graph affinity attribute. lcore_id
> > will be set as MAX by default, it means not enable this attribute.
> >
> > Signed-off-by: Haiyue Wang <haiyue.wang at intel.com>
> > Signed-off-by: Cunming Liang <cunming.liang at intel.com>
> > Signed-off-by: Zhirun Yan <zhirun.yan at intel.com>
> > ---
> >  lib/graph/graph.c         | 59 +++++++++++++++++++++++++++++++++++++++
> >  lib/graph/graph_private.h |  2 ++
> >  lib/graph/rte_graph.h     | 22 +++++++++++++++
> >  lib/graph/version.map     |  2 ++
> > +
> > +int
> > +rte_graph_model_dispatch_core_bind(rte_graph_t id, int lcore) {
> > +       struct graph *graph;
> > +
> > +       GRAPH_ID_CHECK(id);
> > +       if (!rte_lcore_is_enabled(lcore))
> > +               SET_ERR_JMP(ENOLINK, fail,
> > +                           "lcore %d not enabled\n",
> 
> \n is already part of it from graph_err() so no need to add it.
> Also, DPDK coding standard now supports 100 column as max width, so below
> "lcore" can be moved to the same line if there is space.
> 

Ok, I will merge into one line and remove \n.

> 
> > +                           lcore);
> > +
> > +       STAILQ_FOREACH(graph, &graph_list, next)
> > +               if (graph->id == id)
> > +                       break;
> > +
> > +       graph->lcore_id = lcore;
> > +       graph->socket = rte_lcore_to_socket_id(lcore);
> > +
> > +       /* check the availability of source node */
> > +       if (!graph_src_node_avail(graph))
> > +               graph->graph->head = 0;
> > +
> > +       return 0;
> > +
> > +fail:
> > +       return -rte_errno;
> > +}
> > +
> > +void
> > +rte_graph_model_dispatch_core_unbind(rte_graph_t id) {
> > +       struct graph *graph;
> > +
> > +       GRAPH_ID_CHECK(id);
> > +       STAILQ_FOREACH(graph, &graph_list, next)
> > +               if (graph->id == id)
> > +                       break;
> > +
> > +       graph->lcore_id = RTE_MAX_LCORE;
> > +
> > +fail:
> > +       return;
> > +}
> > +
> >  struct rte_graph *
> >  rte_graph_lookup(const char *name)
> >  {
> > @@ -346,6 +404,7 @@ rte_graph_create(const char *name, struct
> rte_graph_param *prm)
> >         graph->src_node_count = src_node_count;
> >         graph->node_count = graph_nodes_count(graph);
> >         graph->id = graph_id;
> > +       graph->lcore_id = RTE_MAX_LCORE;
> >         graph->num_pkt_to_capture = prm->num_pkt_to_capture;
> >         if (prm->pcap_filename)
> >                 rte_strscpy(graph->pcap_filename, prm->pcap_filename,
> > RTE_GRAPH_PCAP_FILE_SZ); diff --git a/lib/graph/graph_private.h
> > b/lib/graph/graph_private.h index bd4c576324..f63b339d81 100644
> > --- a/lib/graph/graph_private.h
> > +++ b/lib/graph/graph_private.h
> > @@ -99,6 +99,8 @@ struct graph {
> >         /**< Circular buffer mask for wrap around. */
> >         rte_graph_t id;
> >         /**< Graph identifier. */
> > +       unsigned int lcore_id;
> > +       /**< Lcore identifier where the graph prefer to run on. */
> 
> Could you move to end of existing fast path variables. Also, please extend the
> comments for new variable introduced ONLY for dispatch model.
> Something like " Lcore identifier where the graph prefer to run on."  to " Lcore
> identifier where the graph prefer to run on. Used with dispatch model" or so.

Yes, I will do in next version.

> 
> 
> >         size_t mem_sz;
> >         /**< Memory size of the graph. */
> >         int socket;
> > diff --git a/lib/graph/rte_graph.h b/lib/graph/rte_graph.h index
> > c9a77297fc..c523809d1f 100644
> > --- a/lib/graph/rte_graph.h
> > +++ b/lib/graph/rte_graph.h
> > @@ -285,6 +285,28 @@ char *rte_graph_id_to_name(rte_graph_t id);
> > __rte_experimental  int rte_graph_export(const char *name, FILE *f);
> >
> > +/**
> > + * Bind graph with specific lcore
> > + *
> > + * @param id
> > + *   Graph id to get the pointer of graph object
> > + * @param lcore
> > + * The lcore where the graph will run on
> > + * @return
> > + *   0 on success, error otherwise.
> > + */
> > +__rte_experimental
> > +int rte_graph_model_dispatch_core_bind(rte_graph_t id, int lcore);
> > +
> > +/**
> > + * Unbind graph with lcore
> > + *
> > + * @param id
> > + * Graph id to get the pointer of graph object  */ __rte_experimental
> > +void rte_graph_model_dispatch_core_unbind(rte_graph_t id);
> > +
> >  /**
> >   * Get graph object from its name.
> >   *
> > diff --git a/lib/graph/version.map b/lib/graph/version.map index
> > 1f090be74e..7de6f08f59 100644
> > --- a/lib/graph/version.map
> > +++ b/lib/graph/version.map
> > @@ -18,6 +18,8 @@ EXPERIMENTAL {
> >         rte_graph_node_get_by_name;
> >         rte_graph_obj_dump;
> >         rte_graph_walk;
> > +       rte_graph_model_dispatch_core_bind;
> > +       rte_graph_model_dispatch_core_unbind;
> >
> >         rte_graph_cluster_stats_create;
> >         rte_graph_cluster_stats_destroy;
> > --
> > 2.37.2
> >