Re: [PATCH v2 2/2] lib/graph: default-align rte_graph_cluster_stats

[Jerin Jacob <jerinjacobk@gmail.com>]

On Tue, Jun 17, 2025 at 8:44 PM Marat Khalili <marat.khalili@huawei.com> wrote:
>
> This was flagged by undefined behaviour sanitizer: struct
> rte_graph_cluster_stats is declared as `__rte_cache_aligned` but was
> allocated using stdlib realloc which caused misaligned allocation. More
> than one test needs to be executed in series in order to reproduce the
> problem using graph_autotest, e.g:
>
>     app/dpdk-test --no-huge --no-pci -m128 graph_autotest graph_autotest
>
> First sanitizer message  (similar ones follow):
>
>     lib/graph/graph_stats.c:209:13: runtime error: member access within
>     misaligned address 0x606000008ea0 for type 'struct
>     rte_graph_cluster_stats', which requires 64 byte alignment
>
> To fix the issue remove `__rte_cache_aligned` attribute from struct
> rte_graph_cluster_stats and struct rte_graph_cluster_node_stats that it
> contains. There is no need to keep them cache-aligned since they are
> only used in the slow path.
>
> Fixes: af1ae8b6a32 ("graph: implement stats")
>
> Signed-off-by: Marat Khalili <marat.khalili@huawei.com>
> ---
>
> v2: Following the suggestions from Jerin Jacob changed the fix to simply
> remove non-default alignment. Although there are many ways to make
> alignment work, I personally agree that it is probably better to keep
> the code simple as long as performance is not an issue.
>
> Although reducing _actual_ alignment of allocated objects would probably
> be a binary compatibility issue, here we are only changing _declared_
> alignment to match the actual one, so it will more likely fix things
> than break them.
>
> devtools/test-meson-builds.sh completes successfully.
>
>  lib/graph/graph_stats.c | 2 +-
>  lib/graph/rte_graph.h   | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/lib/graph/graph_stats.c b/lib/graph/graph_stats.c
> index 57cd72e7cc..4324717f33 100644
> --- a/lib/graph/graph_stats.c
> +++ b/lib/graph/graph_stats.c
> @@ -29,7 +29,7 @@ struct cluster_node {
>         struct rte_node *nodes[];
>  };
>
> -struct __rte_cache_aligned rte_graph_cluster_stats {
> +struct rte_graph_cluster_stats {
>         /* Header */
>         rte_graph_cluster_stats_cb_t fn;
>         uint32_t cluster_node_size; /* Size of struct cluster_node */
> diff --git a/lib/graph/rte_graph.h b/lib/graph/rte_graph.h
> index 097d0dc9d5..826204cad9 100644
> --- a/lib/graph/rte_graph.h
> +++ b/lib/graph/rte_graph.h
> @@ -202,7 +202,7 @@ struct rte_graph_cluster_stats_param {
>   *
>   * @see struct rte_graph_cluster_stats_param::fn
>   */
> -struct __rte_cache_aligned rte_graph_cluster_node_stats {
> +struct rte_graph_cluster_node_stats {

This is a fastpath structure. No need to change the alignment here.

>         uint64_t ts;        /**< Current timestamp. */
>         uint64_t calls;     /**< Current number of calls made. */
>         uint64_t objs;      /**< Current number of objs processed. */
> --
> 2.43.0
>