Re: [PATCH v6 07/15] graph: introduce graph clone API for other worker core

[Jerin Jacob <jerinjacobk@gmail.com>]

On Tue, May 9, 2023 at 11:35 AM Zhirun Yan <zhirun.yan@intel.com> wrote:
>
> This patch adds graph API for supporting to clone the graph object for
> a specified worker core. The new graph will also clone all nodes.
>
> 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>
> ---
> +
> +static rte_graph_t
> +graph_clone(struct graph *parent_graph, const char *name)
> +{
> +       struct graph_node *graph_node;
> +       struct graph *graph;
> +
> +       graph_spinlock_lock();
> +
> +       /* Don't allow to clone a node from a cloned graph */

Both clone_name() and graph_clone() kind of duplicate
rte_node_clone(), Please check, Can we have common _private_ function
to reuse just the common code between them


> +
>  rte_graph_t
>  rte_graph_from_name(const char *name)
>  {
> diff --git a/lib/graph/graph_private.h b/lib/graph/graph_private.h
> index f63b339d81..52ca30ed56 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. */
> +       rte_graph_t parent_id;
> +       /**< Parent graph identifier. */
>         unsigned int lcore_id;
>         /**< Lcore identifier where the graph prefer to run on. */
>         size_t mem_sz;
> diff --git a/lib/graph/rte_graph.h b/lib/graph/rte_graph.h
> index c523809d1f..2f86c17de7 100644
> --- a/lib/graph/rte_graph.h
> +++ b/lib/graph/rte_graph.h
> @@ -247,6 +247,26 @@ rte_graph_t rte_graph_create(const char *name, struct rte_graph_param *prm);
>  __rte_experimental
>  int rte_graph_destroy(rte_graph_t id);
>
> +/**
> + * Clone Graph.
> + *
> + * Clone a graph from static graph (graph created from rte_graph_create). And

rte_graph_create->rte_graph_create()

> + * all cloned graphs attached to the parent graph MUST be destroyed together


Can we add reference count in the implementation to avoid this
limition. If this too much for this release, we can try to add next
release

> + * for fast schedule design limitation (stop ALL graph walk firstly).
> + *
> + * @param id
> + *   Static graph id to clone from.
> + * @param name
> + *   Name of the new graph. The library prepends the parent graph name to the
> + * user-specified name. The final graph name will be,
> + * "parent graph name" + "-" + name.
> + *
> + * @return
> + *   Valid graph id on success, RTE_GRAPH_ID_INVALID otherwise.
> + */
> +__rte_experimental
> +rte_graph_t rte_graph_clone(rte_graph_t id, const char *name);
> +
>  /**
>   * Get graph id from graph name.
>   *
> diff --git a/lib/graph/version.map b/lib/graph/version.map
> index 7de6f08f59..aaa86f66ed 100644
> --- a/lib/graph/version.map
> +++ b/lib/graph/version.map
> @@ -7,6 +7,7 @@ EXPERIMENTAL {
>
>         rte_graph_create;
>         rte_graph_destroy;
> +       rte_graph_clone;


Found new generic graph API, Please add test case for it at
app/test/test_graph.c.



>         rte_graph_dump;
>         rte_graph_export;
>         rte_graph_from_name;
> --
> 2.37.2
>