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

[Jerin Jacob <jerinjacobk@gmail.com>]

On Tue, May 9, 2023 at 11:34 AM Zhirun Yan <zhirun.yan@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@intel.com>
> Signed-off-by: Cunming Liang <cunming.liang@intel.com>
> Signed-off-by: Zhirun Yan <zhirun.yan@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.


> +                           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.


>         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
>