Hi Thomas,
This series has been under review for a long time and has been acknowledged.
Can we merge it for the next release?
> -----Original Message-----
> From: kirankum...@marvell.com <kirankum...@marvell.com>
> Sent: Monday, May 5, 2025 11:54 AM
> To: Jerin Jacob <jer...@marvell.com>; Kiran Kumar Kokkilagadda
> <kirankum...@marvell.com>; Nithin Kumar Dabilpuram
> <ndabilpu...@marvell.com>; Zhirun Yan <yanzhirun_...@163.com>
> Cc: dev@dpdk.org
> Subject: [PATCH v7 1/3] graph: avoid global node ID counter
>
> From: Kiran Kumar K <kirankum...@marvell.com>
>
> The node id is determined based on a global variable that is incremented every
> time a node is created. Adding changes to remove the global counter. Make
> sure that the node list is always ordered by increasing node ids. When
> creating
> a new node, pick a free id which is not allocated.
>
> Signed-off-by: Kiran Kumar K <kirankum...@marvell.com>
> Acked-by: Jerin Jacob <jer...@marvell.com>
> ---
> * V7 Changes:
> - Updated Acked-by
> lib/graph/graph_populate.c | 11 +++--
> lib/graph/graph_private.h | 8 ----
> lib/graph/node.c | 86 ++++++++++++++++++++++++++++++++------
> 3 files changed, 82 insertions(+), 23 deletions(-)
>
> diff --git a/lib/graph/graph_populate.c b/lib/graph/graph_populate.c index
> 1e6b08319e..026daecb21 100644
> --- a/lib/graph/graph_populate.c
> +++ b/lib/graph/graph_populate.c
> @@ -78,7 +78,6 @@ graph_nodes_populate(struct graph *_graph)
> struct rte_graph *graph = _graph->graph;
> struct graph_node *graph_node;
> rte_edge_t count, nb_edges;
> - const char *parent;
> rte_node_t pid;
>
> STAILQ_FOREACH(graph_node, &_graph->node_list, next) { @@ -
> 94,8 +93,14 @@ graph_nodes_populate(struct graph *_graph)
> memcpy(node->name, graph_node->node->name,
> RTE_GRAPH_NAMESIZE);
> pid = graph_node->node->parent_id;
> if (pid != RTE_NODE_ID_INVALID) { /* Cloned node */
> - parent = rte_node_id_to_name(pid);
> - memcpy(node->parent, parent,
> RTE_GRAPH_NAMESIZE);
> + struct node *pnode;
> +
> + STAILQ_FOREACH(pnode, node_list_head_get(), next)
> {
> + if (pnode->id == pid) {
> + memcpy(node->parent, pnode-
> >name, RTE_GRAPH_NAMESIZE);
> + break;
> + }
> + }
> }
> node->id = graph_node->node->id;
> node->parent_id = pid;
> diff --git a/lib/graph/graph_private.h b/lib/graph/graph_private.h index
> 813dd78b9d..3529640abf 100644
> --- a/lib/graph/graph_private.h
> +++ b/lib/graph/graph_private.h
> @@ -29,14 +29,6 @@ extern int rte_graph_logtype; #define graph_info(...)
> GRAPH_LOG(INFO, __VA_ARGS__) #define graph_dbg(...)
> GRAPH_LOG(DEBUG, __VA_ARGS__)
>
> -#define ID_CHECK(id, id_max)
> \
> - do { \
> - if ((id) >= (id_max)) { \
> - rte_errno = EINVAL; \
> - goto fail; \
> - } \
> - } while (0)
> -
> #define SET_ERR_JMP(err, where, fmt, ...)
> \
> do { \
> graph_err(fmt, ##__VA_ARGS__); \
> diff --git a/lib/graph/node.c b/lib/graph/node.c index
> 101981ec24..53a3f3f2d4 100644
> --- a/lib/graph/node.c
> +++ b/lib/graph/node.c
> @@ -16,9 +16,56 @@
> #include "graph_private.h"
>
> static struct node_head node_list = STAILQ_HEAD_INITIALIZER(node_list);
> -static rte_node_t node_id;
>
> -#define NODE_ID_CHECK(id) ID_CHECK(id, node_id)
> +static struct node *
> +node_from_id(rte_node_t id)
> +{
> + struct node *node = NULL;
> +
> + graph_spinlock_lock();
> + rte_errno = EINVAL;
> + STAILQ_FOREACH(node, &node_list, next) {
> + if (node->id == id) {
> + rte_errno = 0;
> + goto exit;
> + }
> + }
> +exit:
> + graph_spinlock_unlock();
> + return node;
> +}
> +
> +static rte_node_t
> +next_next_free_id(void)
> +{
> + struct node *node;
> + rte_node_t id = 0;
> +
> + STAILQ_FOREACH(node, &node_list, next) {
> + if (id < node->id)
> + break;
> + id = node->id + 1;
> + }
> + return id;
> +}
> +
> +static void
> +node_insert_ordered(struct node *node)
> +{
> + struct node *after, *g;
> +
> + after = NULL;
> + STAILQ_FOREACH(g, &node_list, next) {
> + if (g->id < node->id)
> + after = g;
> + else if (g->id > node->id)
> + break;
> + }
> + if (after == NULL)
> + STAILQ_INSERT_HEAD(&node_list, node, next);
> + else
> + STAILQ_INSERT_AFTER(&node_list, after, node, next); }
>
> /* Private functions */
> struct node_head *
> @@ -118,10 +165,10 @@ __rte_node_register(const struct
> rte_node_register *reg)
> }
>
> node->lcore_id = RTE_MAX_LCORE;
> - node->id = node_id++;
> + node->id = next_next_free_id();
>
> - /* Add the node at tail */
> - STAILQ_INSERT_TAIL(&node_list, node, next);
> + /* Add the node in ordered list */
> + node_insert_ordered(node);
> graph_spinlock_unlock();
>
> return node->id;
> @@ -197,7 +244,9 @@ rte_node_clone(rte_node_t id, const char *name) {
> struct node *node;
>
> - NODE_ID_CHECK(id);
> + if (node_from_id(id) == NULL)
> + goto fail;
> +
> STAILQ_FOREACH(node, &node_list, next)
> if (node->id == id)
> return node_clone(node, name);
> @@ -225,7 +274,8 @@ rte_node_id_to_name(rte_node_t id) {
> struct node *node;
>
> - NODE_ID_CHECK(id);
> + if (node_from_id(id) == NULL)
> + goto fail;
> STAILQ_FOREACH(node, &node_list, next)
> if (node->id == id)
> return node->name;
> @@ -240,7 +290,8 @@ rte_node_edge_count(rte_node_t id) {
> struct node *node;
>
> - NODE_ID_CHECK(id);
> + if (node_from_id(id) == NULL)
> + goto fail;
> STAILQ_FOREACH(node, &node_list, next)
> if (node->id == id)
> return node->nb_edges;
> @@ -310,7 +361,8 @@ rte_node_edge_shrink(rte_node_t id, rte_edge_t
> size)
> rte_edge_t rc = RTE_EDGE_ID_INVALID;
> struct node *node;
>
> - NODE_ID_CHECK(id);
> + if (node_from_id(id) == NULL)
> + goto fail;
> graph_spinlock_lock();
>
> STAILQ_FOREACH(node, &node_list, next) { @@ -338,7 +390,8 @@
> rte_node_edge_update(rte_node_t id, rte_edge_t from, const char
> **next_nodes,
> rte_edge_t rc = RTE_EDGE_ID_INVALID;
> struct node *n, *prev;
>
> - NODE_ID_CHECK(id);
> + if (node_from_id(id) == NULL)
> + goto fail;
> graph_spinlock_lock();
>
> prev = NULL;
> @@ -373,7 +426,8 @@ rte_node_edge_get(rte_node_t id, char
> *next_nodes[])
> rte_node_t rc = RTE_NODE_ID_INVALID;
> struct node *node;
>
> - NODE_ID_CHECK(id);
> + if (node_from_id(id) == NULL)
> + goto fail;
> graph_spinlock_lock();
>
> STAILQ_FOREACH(node, &node_list, next) { @@ -397,7 +451,8 @@
> node_scan_dump(FILE *f, rte_node_t id, bool all)
> struct node *node;
>
> RTE_ASSERT(f != NULL);
> - NODE_ID_CHECK(id);
> + if (node_from_id(id) == NULL)
> + goto fail;
>
> STAILQ_FOREACH(node, &node_list, next) {
> if (all == true) {
> @@ -429,5 +484,12 @@ RTE_EXPORT_SYMBOL(rte_node_max_count)
> rte_node_t
> rte_node_max_count(void)
> {
> + rte_node_t node_id = 0;
> + struct node *node;
> +
> + STAILQ_FOREACH(node, &node_list, next) {
> + if (node_id < node->id)
> + node_id = node->id;
> + }
> return node_id;
> }
> --
> 2.48.1
