> > From: Robin Jarry <rja...@redhat.com>
> > Sent: Friday, October 11, 2024 3:24 PM
> > To: Pavan Nikhilesh Bhagavatula <pbhagavat...@marvell.com>; Jerin Jacob
> > <jer...@marvell.com>; Nithin Kumar Dabilpuram
> > <ndabilpu...@marvell.com>; Kiran Kumar Kokkilagadda
> > <kirankum...@marvell.com>; zhirun....@intel.com; Zhirun Yan
> > <yanzhirun_...@163.com>
> > Cc: dev@dpdk.org
> > Subject: [EXTERNAL] Re: [PATCH v4 3/5] graph: add stats for node specific
> > errors
> >
> > Hi Pavan, , Aug 16, 2024 at 17: 09: > From: Pavan Nikhilesh
> > <pbhagavatula@ marvell. com> > > Add support for retrieving/printing stats
> > for node specific > errors using rte_graph_cluster_stats_get(). > >
> > Signed-off-
> > by: Pavan
> >
> > Hi Pavan,
> >
> > , Aug 16, 2024 at 17:09:
> > > From: Pavan Nikhilesh <pbhagavat...@marvell.com>
> > >
> > > Add support for retrieving/printing stats for node specific
> > > errors using rte_graph_cluster_stats_get().
> > >
> > > Signed-off-by: Pavan Nikhilesh <pbhagavat...@marvell.com>
> > > ---
> >
> > [snip]
> >
> > > diff --git a/lib/graph/rte_graph.h b/lib/graph/rte_graph.h
> > > index b28143d737..12b6461cf5 100644
> > > --- a/lib/graph/rte_graph.h
> > > +++ b/lib/graph/rte_graph.h
> > > @@ -223,6 +223,10 @@ struct __rte_cache_aligned
> > rte_graph_cluster_node_stats {
> > >
> > > uint64_t realloc_count; /**< Realloc count. */
> > >
> > > + uint8_t node_error_cntrs; /**< Number of
> > Node error counters. */
> > > + char (*node_error_desc)[RTE_NODE_ERROR_DESC_SIZE]; /**< Names
> > of the Node error counters. */
> >
> > Why do you need the parentheses here?
The parenthesis signify that it's a pointer to array that has fixed size
characters.
If we remove the parenthesis, compiler will treat it as array of pointers to
char.
../lib/graph/graph_stats.c: In function ‘stats_mem_populate’:
../lib/graph/graph_stats.c:274:42: error: assignment to expression with array
type
274 | cluster->stat.error_desc = rte_zmalloc_socket(
| ^
../lib/graph/graph_stats.c:277:46: error: the comparison will always evaluate
as ‘false’ for the address of ‘error_desc’ will never be NULL [-Werror=address]
277 | if (cluster->stat.error_desc == NULL) {
| ^~
In file included from ../lib/graph/graph_private.h:17,
from ../lib/graph/graph_stats.c:13:
../lib/graph/rte_graph.h:227:15: note: ‘error_desc’ declared here
227 | char *error_desc[RTE_NODE_ERROR_DESC_SIZE]; /**< Names of the
Node error counters. */
| ^~~~~~~~~~
cc1: all warnings being treated as errors
> >
> > > + uint64_t *node_error_count; /**< Total error
> > count per each error. */
> >
> > The node_ prefix is redundant here. Can you use something shorter?
> >
> > uint8_t errors_num; /**< Number of Node error counters. */
> > char (*errors_desc)[RTE_NODE_ERROR_DESC_SIZE];
> > uint64_t *errors_value;
> >
>
> I will shrink them in the next version.
>
> > Thanks!
