> From: Jerin Jacob [mailto:[email protected]]
> Sent: Tuesday, 23 June 2026 07.13
>
> On Mon, Jun 22, 2026 at 12:11 AM Morten Brørup
> <[email protected]> wrote:
> >
> > Added graph node profiling stats, build time configurable by enabling
> > RTE_GRAPH_PROFILE in rte_config.h.
> >
> > Signed-off-by: Morten Brørup <[email protected]>
>
> Please update app/test/test_graph.c to validate this featue.
Ack.
> > @@ -92,7 +92,62 @@ rte_graph_obj_dump(FILE *f, struct rte_graph *g,
> bool all)
> > fprintf(f, " total_sched_fail=%" PRId64
> "\n",
> > n->dispatch.total_sched_fail);
> > }
> > - fprintf(f, " total_calls=%" PRId64 "\n", n-
> >total_calls);
> > + fprintf(f, " total_calls=%" PRIu64 "\n", n-
> >total_calls);
> > + if (rte_graph_has_stats_feature()) {
> > + fprintf(f, " total_cycles=%" PRIu64 ",
> avg cycles/call=%.1f\n",
> > + n->total_cycles,
> > + n->total_calls == 0 ?
> (double)0 :
> > + (double)n->total_cycles /
> (double)n->total_calls);
> > + }
> > +#ifdef RTE_GRAPH_PROFILE
>
>
> Please introduce rte_graph_has_profile_featue() similar to
> rte_graph_has_stats_feature() to reduce if def clutter as possible.
Disagree, see below.
>
> > + uint64_t calls = n->usage_stats[0].calls;
> > + fprintf(f, " objs[0]\n");
> > + fprintf(f, " calls=%" PRIu64 ", cycles=%"
> PRIu64 ", avg cycles/call=%.1f\n",
> > + calls,
>
> >
> > diff --git a/lib/graph/rte_graph_worker_common.h
> b/lib/graph/rte_graph_worker_common.h
> > index 4ab53a533e..0d8039575d 100644
> > --- a/lib/graph/rte_graph_worker_common.h
> > +++ b/lib/graph/rte_graph_worker_common.h
> > @@ -144,12 +144,26 @@ struct __rte_cache_aligned rte_node {
> > rte_node_process_t process; /**< Process
> function. */
> > uint64_t process_u64;
> > };
> > + /** Fast path area cache line 3. */
> > +#ifdef RTE_GRAPH_PROFILE
> > + struct {
> > + uint64_t calls; /**< Calls processing
> resp. 0 or 1 objects. */
> > + uint64_t cycles; /**< Cycles spent
> processing resp. 0 or 1 objects. */
> > + } usage_stats[2]; /**< Usage when this node
> processed 0 or 1 objects. */
> > + uint64_t full_burst_calls; /**< Calls processing a
> full burst of objects. */
> > + uint64_t full_burst_cycles; /**< Cycles spent
> processing a full burst of objects. */
> > + uint64_t half_burst_calls; /**< Calls processing a
> half burst of objects. */
> > + uint64_t half_burst_cycles; /**< Cycles spent
> processing a half burst of objects. */
> > + /** Fast path area cache line 4. */
> > +#endif
>
> Is it an ABI breakage?
No. The modifications are enclosed in #ifdef, and disabled by default.
It is generally required that when rte_config.h options are modified, both the
application and DPDK itself are built together; and then API/ABI breakage
becomes irrelevant.
IMO, we should keep our structures lean in release builds. This means that
fields used for detailed profiling, advanced debugging, cookie validation, etc.
should use the #ifdef pattern rather than the rte_lib_has_some_feature()
pattern; especially if they affect the size of a structure. And when those
fields are not present, any code accessing them cannot use the
rte_lib_has_some_feature() pattern.
The mbuf and mempool libraries also use #ifdef pattern for similar features.
>
> > alignas(RTE_CACHE_LINE_MIN_SIZE) struct rte_node
> *nodes[]; /**< Next nodes. */
> > };
> > };
> >
