Re: [dpdk-dev] [PATCH v4 07/29] graph: implement create and destroy APIs

[Jerin Jacob <jerinjacobk@gmail.com>]

> > +++ b/lib/librte_graph/graph.c
> [...]
> > +static int
> > +graph_node_add(struct graph *graph, struct node *node)
> > +{
> > +     struct graph_node *graph_node;
> > +     size_t sz;
> > +
> > +     /* Skip the duplicate nodes */
> > +     STAILQ_FOREACH(graph_node, &graph->node_list, next)
> > +             if (strncmp(node->name, graph_node->node->name,
> > +                         RTE_NODE_NAMESIZE) == 0)
>
> Is it not a "deficiency" of a program to attempt to add node twice?
> If it is, then maybe a warning here?

The library takes care of adding nodes the graph
- when user-specified
OR
- when a node depends on another node (gets the info from next nodes)
and it not specified by the user

So this internal function may be called with the same node.


>
> [...]
> > +rte_graph_t
> > +rte_graph_create(const char *name, struct rte_graph_param *prm)
> > +{
> > +     struct graph *graph;
> > +     const char *pattern;
> > +     uint16_t i;
> > +

> > +
> > +     /* Do BFS from src nodes on the graph to find isolated nodes */
> > +     if (graph_has_isolated_node(graph))
> > +             goto graph_cleanup;
> > +
> > +     /* Initialize graph object */
> > +     graph->socket = prm->socket_id;
> > +     graph->src_node_count = graph_src_nodes_count(graph);
>
> Maybe reuse value of previous call (above)?

Yep. I will change it in v5.

>
> > +     graph->node_count = graph_nodes_count(graph);
> > +     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;
> > +}
> > +
>
> With regards
> Andrzej Ostruszka
>