[v6,07/15] graph: introduce graph clone API for other worker core
Checks
Commit Message
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>
---
lib/graph/graph.c | 110 ++++++++++++++++++++++++++++++++++++++
lib/graph/graph_private.h | 2 +
lib/graph/rte_graph.h | 20 +++++++
lib/graph/version.map | 1 +
4 files changed, 133 insertions(+)
Comments
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
>
> -----Original Message-----
> From: Jerin Jacob <jerinjacobk@gmail.com>
> Sent: Wednesday, May 24, 2023 3:14 PM
> To: Yan, Zhirun <zhirun.yan@intel.com>
> Cc: dev@dpdk.org; jerinj@marvell.com; kirankumark@marvell.com;
> ndabilpuram@marvell.com; stephen@networkplumber.org;
> pbhagavatula@marvell.com; Liang, Cunming <cunming.liang@intel.com>; Wang,
> Haiyue <haiyue.wang@intel.com>
> Subject: Re: [PATCH v6 07/15] graph: introduce graph clone API for other worker
> core
>
> 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_clone(), graph_clone() in graph.c and rte_node_clone(), node_clone() in node.c are different.
They are specific for node or graph.
I think they could remain the same.
Only clone_name(struct rte_node_register *reg, struct node *node, const char *name)
could be reuse.
And I will change the param to clone_name(char *name, char* ori_str, const char *append_str) and put
this func into graph_private.h
>
>
> > +
> > 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
Yes, I think it's good in next release. Thanks.
>
> > + * 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.
>
Yes, I will add test.
>
>
> > rte_graph_dump;
> > rte_graph_export;
> > rte_graph_from_name;
> > --
> > 2.37.2
> >
@@ -404,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->parent_id = RTE_GRAPH_ID_INVALID;
graph->lcore_id = RTE_MAX_LCORE;
graph->num_pkt_to_capture = prm->num_pkt_to_capture;
if (prm->pcap_filename)
@@ -468,6 +469,115 @@ rte_graph_destroy(rte_graph_t id)
return rc;
}
+static int
+clone_name(struct graph *graph, struct graph *parent_graph, const char *name)
+{
+ ssize_t sz, rc;
+
+#define SZ RTE_GRAPH_NAMESIZE
+ rc = rte_strscpy(graph->name, parent_graph->name, SZ);
+ if (rc < 0)
+ goto fail;
+ sz = rc;
+ rc = rte_strscpy(graph->name + sz, "-", RTE_MAX((int16_t)(SZ - sz), 0));
+ if (rc < 0)
+ goto fail;
+ sz += rc;
+ sz = rte_strscpy(graph->name + sz, name, RTE_MAX((int16_t)(SZ - sz), 0));
+ if (sz < 0)
+ goto fail;
+
+ return 0;
+fail:
+ rte_errno = E2BIG;
+ return -rte_errno;
+}
+
+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 */
+ if (parent_graph->parent_id != RTE_GRAPH_ID_INVALID)
+ SET_ERR_JMP(EEXIST, fail, "A cloned graph is not allowed to be cloned");
+
+ /* Create graph object */
+ graph = calloc(1, sizeof(*graph));
+ if (graph == NULL)
+ SET_ERR_JMP(ENOMEM, fail, "Failed to calloc cloned graph object");
+
+ /* Naming ceremony of the new graph. name is node->name + "-" + name */
+ if (clone_name(graph, parent_graph, name))
+ goto free;
+
+ /* Check for existence of duplicate graph */
+ if (rte_graph_from_name(graph->name) != RTE_GRAPH_ID_INVALID)
+ SET_ERR_JMP(EEXIST, free, "Found duplicate graph %s",
+ graph->name);
+
+ /* Clone nodes from parent graph firstly */
+ STAILQ_INIT(&graph->node_list);
+ STAILQ_FOREACH(graph_node, &parent_graph->node_list, next) {
+ if (graph_node_add(graph, graph_node->node))
+ goto graph_cleanup;
+ }
+
+ /* Just update adjacency list of all nodes in the graph */
+ if (graph_adjacency_list_update(graph))
+ goto graph_cleanup;
+
+ /* Initialize the graph object */
+ graph->src_node_count = parent_graph->src_node_count;
+ graph->node_count = parent_graph->node_count;
+ graph->parent_id = parent_graph->id;
+ graph->lcore_id = parent_graph->lcore_id;
+ graph->socket = parent_graph->socket;
+ graph->id = graph_id;
+
+ /* Allocate the Graph fast path memory and populate the data */
+ if (graph_fp_mem_create(graph))
+ goto graph_cleanup;
+
+ /* Call init() of the all the nodes in the graph */
+ if (graph_node_init(graph))
+ goto graph_mem_destroy;
+
+ /* All good, Lets add the graph to the list */
+ graph_id++;
+ STAILQ_INSERT_TAIL(&graph_list, graph, next);
+
+ graph_spinlock_unlock();
+ return graph->id;
+
+graph_mem_destroy:
+ graph_fp_mem_destroy(graph);
+graph_cleanup:
+ graph_cleanup(graph);
+free:
+ free(graph);
+fail:
+ graph_spinlock_unlock();
+ return RTE_GRAPH_ID_INVALID;
+}
+
+rte_graph_t
+rte_graph_clone(rte_graph_t id, const char *name)
+{
+ struct graph *graph;
+
+ GRAPH_ID_CHECK(id);
+ STAILQ_FOREACH(graph, &graph_list, next)
+ if (graph->id == id)
+ return graph_clone(graph, name);
+
+fail:
+ return RTE_GRAPH_ID_INVALID;
+}
+
rte_graph_t
rte_graph_from_name(const char *name)
{
@@ -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;
@@ -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
+ * all cloned graphs attached to the parent graph MUST be destroyed together
+ * 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.
*
@@ -7,6 +7,7 @@ EXPERIMENTAL {
rte_graph_create;
rte_graph_destroy;
+ rte_graph_clone;
rte_graph_dump;
rte_graph_export;
rte_graph_from_name;