Re: [PATCH v4 2/3] drivers/net: remove use of non-standard array range initialization

[David Marchand <david.marchand@redhat.com>]

Hello Andre,

On Mon, Jul 21, 2025 at 5:25 PM Andre Muezerie
<andremue@linux.microsoft.com> wrote:
>
> Array range initialization is non-standard and is not provided by
> all compilers. MSVC does not implement it and ends up emitting
> errors like the one below:
>
> drivers/net/r8169/r8169_phy.c(380):
>     error C2143: syntax error: missing ':' before '...'
> case CFG_METHOD_48 ... CFG_METHOD_57:

The commitlog (and title) is not aligned anymore with the patch
content: there is more than the array range init now, and the realtek
driver build error is not relevant anymore.
Please reword.

This comment also applies to the first patch of the series.


>
> The fix is to explicitly initialize each element in the range.
>
> Signed-off-by: Andre Muezerie <andremue@linux.microsoft.com>
> Acked-by: Howard Wang <howard_wang@realsil.com.cn>

Howard acked on the realtek driver update (in a previous revision).
Now that this was reworked in a separate change (if I am not wrong,
commit d9ee71b5f1bc ("net/r8169: support RTL8168 series")), I don't
think this ack should be kept.


> ---
>  drivers/common/mlx5/mlx5_malloc.h   |  4 ++--
>  drivers/net/mlx5/mlx5_flow_dv.c     |  5 ++++-
>  drivers/net/mlx5/mlx5_rx.c          |  4 ++--
>  drivers/net/mlx5/mlx5_utils.c       |  4 ++--
>  drivers/net/octeon_ep/otx_ep_mbox.c | 24 ++++++++++++++++++++----
>  5 files changed, 30 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/common/mlx5/mlx5_malloc.h b/drivers/common/mlx5/mlx5_malloc.h
> index 6e5cc3d851..27d3e5bdf2 100644
> --- a/drivers/common/mlx5/mlx5_malloc.h
> +++ b/drivers/common/mlx5/mlx5_malloc.h
> @@ -118,8 +118,8 @@ void mlx5_free(void *addr);
>                 mem; \
>         }))
>  #else
> -#define mlx5_malloc_numa_tolerant(flags, size, align, socket)
> -       (mlx5_malloc((flags) | MLX5_NUMA_TOLERANT, (size), (align), (socket)));
> +#define mlx5_malloc_numa_tolerant(flags, size, align, socket) \
> +       (mlx5_malloc((flags) | MLX5_NUMA_TOLERANT, (size), (align), (socket)))

The "outer" () are unneeded.


>  #endif
>  #ifdef __cplusplus
>  }
> diff --git a/drivers/net/mlx5/mlx5_flow_dv.c b/drivers/net/mlx5/mlx5_flow_dv.c
> index 7b9e5018b8..54e1d0490a 100644
> --- a/drivers/net/mlx5/mlx5_flow_dv.c
> +++ b/drivers/net/mlx5/mlx5_flow_dv.c
> @@ -19652,7 +19652,10 @@ mlx5_flow_dv_discover_counter_offset_support(struct rte_eth_dev *dev)
>                 .size = sizeof(value.buf),
>         };
>         struct mlx5dv_flow_matcher_attr dv_attr = {
> -               .type = IBV_FLOW_ATTR_NORMAL | IBV_FLOW_ATTR_FLAGS_EGRESS,
> +               /* Cast below is needed to avoid warning until bug
> +                * (https://bugs.dpdk.org/show_bug.cgi?id=1758) is fixed.
> +                */
> +               .type = IBV_FLOW_ATTR_NORMAL | (enum ibv_flow_attr_type)IBV_FLOW_ATTR_FLAGS_EGRESS,

I assigned the bug to Raslan, I hope mlx5 guys can have a look soon.


>                 .priority = 0,
>                 .match_criteria_enable = 0,
>                 .match_mask = (void *)&mask,
> diff --git a/drivers/net/mlx5/mlx5_rx.c b/drivers/net/mlx5/mlx5_rx.c
> index 5e8c312d00..1385b23d94 100644
> --- a/drivers/net/mlx5/mlx5_rx.c
> +++ b/drivers/net/mlx5/mlx5_rx.c
> @@ -1093,7 +1093,7 @@ mlx5_rx_burst(void *dpdk_rxq, struct rte_mbuf **pkts, uint16_t pkts_n)
>                                 break;
>                         }
>                         pkt = seg;
> -                       MLX5_ASSERT(len >= (rxq->crc_present << 2));
> +                       MLX5_ASSERT(len >= (int)(rxq->crc_present << 2));
>                         pkt->ol_flags &= RTE_MBUF_F_EXTERNAL;
>                         if (rxq->cqe_comp_layout && mcqe)
>                                 cqe = &rxq->title_cqe;
> @@ -1273,7 +1273,7 @@ mlx5_rx_burst_out_of_order(void *dpdk_rxq, struct rte_mbuf **pkts, uint16_t pkts
>                 }
>                 if (!pkt) {
>                         pkt = seg;
> -                       MLX5_ASSERT(len >= (rxq->crc_present << 2));
> +                       MLX5_ASSERT(len >= (int)(rxq->crc_present << 2));
>                         pkt->ol_flags &= RTE_MBUF_F_EXTERNAL;
>                         if (rxq->cqe_comp_layout && mcqe)
>                                 cqe = &rxq->title_cqe;
> diff --git a/drivers/net/mlx5/mlx5_utils.c b/drivers/net/mlx5/mlx5_utils.c
> index f31b1652bc..c95e69b40f 100644
> --- a/drivers/net/mlx5/mlx5_utils.c
> +++ b/drivers/net/mlx5/mlx5_utils.c
> @@ -29,8 +29,8 @@
>         mem; \
>  }))
>  #else
> -#define pool_malloc(pool, flags, size, align, socket)
> -       (pool)->cfg.malloc((uint32_t)(flags) | NUMA_TOLERANT, (size), (align), (socket));
> +#define pool_malloc(pool, flags, size, align, socket) \
> +       (((pool)->cfg.malloc(((uint32_t)(flags) | MLX5_NUMA_TOLERANT), (size), (align), (socket))))
>  #endif

Ugh, there is a regression in there, from ce2cf3403f2a ("net/mlx5:
support NUMA node fallback") iiuc.
I am surprised this was not caught by the CI with clang.

Do we really need all those (()) ?


-- 
David Marchand